From 34b7cef2c96152433da6f380656baf9d27c9579a Mon Sep 17 00:00:00 2001
From: Ludovic Poitou <ludovic.poitou@forgerock.com>
Date: Fri, 20 Nov 2015 23:10:19 +0000
Subject: [PATCH] OPENDJ-2441, OPENDJ-2443: Make sure extensions can be loaded from both install and instance directories, includes a little bit of refactoring and code cleanup

---
 opendj-server-legacy/src/main/java/org/opends/server/admin/ClassLoaderProvider.java |  389 ++++++++++++++++++-------------------------------------
 1 files changed, 128 insertions(+), 261 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/admin/ClassLoaderProvider.java b/opendj-server-legacy/src/main/java/org/opends/server/admin/ClassLoaderProvider.java
index fc996e3..c95d123 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/admin/ClassLoaderProvider.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/admin/ClassLoaderProvider.java
@@ -45,11 +45,7 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 import java.util.jar.Attributes;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
@@ -60,37 +56,29 @@
 import org.opends.server.core.DirectoryServer;
 import org.forgerock.i18n.slf4j.LocalizedLogger;
 import org.opends.server.types.InitializationException;
-import org.forgerock.util.Reject;
-
 
 
 /**
- * Manages the class loader which should be used for loading
- * configuration definition classes and associated extensions.
+ * Manages the class loader which should be used for loading configuration definition classes and associated extensions.
  * <p>
- * For extensions which define their own extended configuration
- * definitions, the class loader will make sure that the configuration
- * definition classes are loaded and initialized.
+ * For extensions which define their own extended configuration definitions, the class loader will make sure
+ * that the configuration definition classes are loaded and initialized.
  * <p>
- * Initially the class loader provider is disabled, and calls to the
- * {@link #getClassLoader()} will return the system default class
- * loader.
+ * Initially the class loader provider is disabled, and calls to the {@link #getClassLoader()} will return
+ * the system default class loader.
  * <p>
- * Applications <b>MUST NOT</b> maintain persistent references to the
- * class loader as it can change at run-time.
+ * Applications <b>MUST NOT</b> maintain persistent references to the class loader as it can change at run-time.
  */
 public final class ClassLoaderProvider {
   private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
 
   /**
-   * Private URLClassLoader implementation. This is only required so
-   * that we can provide access to the addURL method.
+   * Private URLClassLoader implementation.
+   * This is only required so that we can provide access to the addURL method.
    */
   private static final class MyURLClassLoader extends URLClassLoader {
 
-    /**
-     * Create a class loader with the default parent class loader.
-     */
+    /** Create a class loader with the default parent class loader. */
     public MyURLClassLoader() {
       super(new URL[0]);
     }
@@ -115,30 +103,21 @@
      * @param jarFile
      *          The name of the Jar file.
      * @throws MalformedURLException
-     *           If a protocol handler for the URL could not be found,
-     *           or if some other error occurred while constructing
-     *           the URL.
+     *           If a protocol handler for the URL could not be found, or if some other error occurred
+     *           while constructing the URL.
      * @throws SecurityException
-     *           If a required system property value cannot be
-     *           accessed.
+     *           If a required system property value cannot be accessed.
      */
-    public void addJarFile(File jarFile) throws SecurityException,
-        MalformedURLException {
+    public void addJarFile(File jarFile) throws SecurityException, MalformedURLException {
       addURL(jarFile.toURI().toURL());
     }
 
   }
 
-  /**
-   * The name of the manifest file listing the core configuration
-   * definition classes.
-   */
+  /** The name of the manifest file listing the core configuration definition classes. */
   private static final String CORE_MANIFEST = "core.manifest";
 
-  /**
-   * The name of the manifest file listing a extension's configuration
-   * definition classes.
-   */
+  /** The name of the manifest file listing a extension's configuration definition classes. */
   private static final String EXTENSION_MANIFEST = "extension.manifest";
 
   /** The name of the lib directory. */
@@ -153,10 +132,7 @@
   /** Attribute name in jar's MANIFEST corresponding to the revision number. */
   private static final String REVISION_NUMBER = "Revision-Number";
 
-  /**
-   * The attribute names for build information is name, version and revision
-   * number.
-   */
+  /** The attribute names for build information is name, version and revision number. */
   private static final String[] BUILD_INFORMATION_ATTRIBUTE_NAMES =
                  new String[]{Attributes.Name.EXTENSION_NAME.toString(),
                               Attributes.Name.IMPLEMENTATION_VERSION.toString(),
@@ -166,8 +142,7 @@
   /**
    * Get the single application wide class loader provider instance.
    *
-   * @return Returns the single application wide class loader provider
-   *         instance.
+   * @return Returns the single application wide class loader provider instance.
    */
   public static ClassLoaderProvider getInstance() {
     return INSTANCE;
@@ -177,12 +152,10 @@
   private Set<File> jarFiles = new HashSet<>();
 
   /**
-   * Underlying class loader used to load classes and resources (null
-   * if disabled).<br>
-   * We contain a reference to the URLClassLoader rather than
-   * sub-class it so that it is possible to replace the loader at run-time.
-   * For example, when removing or replacing extension Jar files
-   * (the URLClassLoader only supports adding new URLs, not removal).
+   * Underlying class loader used to load classes and resources (null if disabled).<br>
+   * We contain a reference to the URLClassLoader rather than sub-class it so that it is possible to replace the
+   * loader at run-time. For example, when removing or replacing extension Jar files (the URLClassLoader
+   * only supports adding new URLs, not removal).
    */
   private MyURLClassLoader loader;
 
@@ -196,66 +169,13 @@
 
 
   /**
-   * Add the named extensions to this class loader provider.
-   *
-   * @param extensions
-   *          The names of the extensions to be loaded. The names
-   *          should not contain any path elements and must be located
-   *          within the extensions folder.
-   * @throws InitializationException
-   *           If one of the extensions could not be loaded and
-   *           initialized.
-   * @throws IllegalStateException
-   *           If this class loader provider is disabled.
-   * @throws IllegalArgumentException
-   *           If one of the extension names was not a single relative
-   *           path name element or was an absolute path.
-   */
-  public synchronized void addExtension(String... extensions)
-      throws InitializationException, IllegalStateException,
-      IllegalArgumentException {
-    Reject.ifNull(extensions);
-
-    if (loader == null) {
-      throw new IllegalStateException(
-          "Class loader provider is disabled.");
-    }
-
-    File libPath = new File(DirectoryServer.getInstanceRoot(), LIB_DIR);
-    File extensionsPath = new File(libPath, EXTENSIONS_DIR);
-
-    ArrayList<File> files = new ArrayList<>(extensions.length);
-    for (String extension : extensions) {
-      File file = new File(extensionsPath, extension);
-
-      // For security reasons we need to make sure that the file name
-      // passed in did not contain any path elements and names a file
-      // in the extensions folder.
-
-      // Can handle potential null parent.
-      if (!extensionsPath.equals(file.getParentFile())) {
-        throw new IllegalArgumentException("Illegal file name: "
-            + extension);
-      }
-
-      // The file is valid.
-      files.add(file);
-    }
-
-    // Add the extensions.
-    addExtension(files.toArray(new File[files.size()]));
-  }
-
-
-
-  /**
-   * Disable this class loader provider and removed any registered
-   * extensions.
+   * Disable this class loader provider and removed any registered extensions.
    *
    * @throws IllegalStateException
    *           If this class loader provider is already disabled.
    */
-  public synchronized void disable() throws IllegalStateException {
+  public synchronized void disable()
+      throws IllegalStateException {
     if (loader == null) {
       throw new IllegalStateException(
           "Class loader provider already disabled.");
@@ -267,39 +187,34 @@
 
 
   /**
-   * Enable this class loader provider using the application's
-   * class loader as the parent class loader.
+   * Enable this class loader provider using the application's class loader as the parent class loader.
    *
    * @throws InitializationException
-   *           If the class loader provider could not initialize
-   *           successfully.
+   *           If the class loader provider could not initialize successfully.
    * @throws IllegalStateException
    *           If this class loader provider is already enabled.
    */
-  public synchronized void enable() throws InitializationException,
-      IllegalStateException {
+  public synchronized void enable()
+      throws InitializationException, IllegalStateException {
     enable(RootCfgDefn.class.getClassLoader());
   }
 
 
 
   /**
-   * Enable this class loader provider using the provided parent class
-   * loader.
+   * Enable this class loader provider using the provided parent class loader.
    *
    * @param parent
    *          The parent class loader.
    * @throws InitializationException
-   *           If the class loader provider could not initialize
-   *           successfully.
+   *           If the class loader provider could not initialize successfully.
    * @throws IllegalStateException
    *           If this class loader provider is already enabled.
    */
   public synchronized void enable(ClassLoader parent)
       throws InitializationException, IllegalStateException {
     if (loader != null) {
-      throw new IllegalStateException(
-          "Class loader provider already enabled.");
+      throw new IllegalStateException("Class loader provider already enabled.");
     }
 
     if (parent != null) {
@@ -308,62 +223,38 @@
       loader = new MyURLClassLoader();
     }
 
-    // Forcefully load all configuration definition classes in
-    // OpenDS.jar.
+    // Forcefully load all configuration definition classes in OpenDJ.jar.
     initializeCoreComponents();
 
-    // Put extensions jars into the class loader and load all
-    // configuration definition classes in that they contain.
-    // First load the extension from the install directory, then
-    // from the instance directory.
-    File libDir ;
-    File installExtensionsPath ;
-    File instanceExtensionsPath ;
+    // Put extensions jars into the class loader and load all configuration definition classes in that they contain.
+    // First load the extension from the install directory, then from the instance directory.
+    File installExtensionsPath  = buildExtensionPath(DirectoryServer.getServerRoot());
+    File instanceExtensionsPath = buildExtensionPath(DirectoryServer.getInstanceRoot());
 
-
-    // load install dir extension
-    libDir = new File(DirectoryServer.getServerRoot(), LIB_DIR);
-    try
-    {
-      installExtensionsPath =
-        new File(libDir, EXTENSIONS_DIR).getCanonicalFile();
-    }
-    catch (Exception e)
-    {
-      installExtensionsPath = new File(libDir, EXTENSIONS_DIR);
-    }
     initializeAllExtensions(installExtensionsPath);
 
-    // load instance dir extension
-    libDir = new File(DirectoryServer.getInstanceRoot(),LIB_DIR);
-    try
-    {
-      instanceExtensionsPath =
-        new File(libDir, EXTENSIONS_DIR).getCanonicalFile();
-    }
-    catch (Exception e)
-    {
-      instanceExtensionsPath = new File(libDir, EXTENSIONS_DIR);
-    }
-    if (! installExtensionsPath.getAbsolutePath().equals(
-        instanceExtensionsPath.getAbsolutePath()))
-    {
+    if (! installExtensionsPath.getAbsolutePath().equals(instanceExtensionsPath.getAbsolutePath())) {
       initializeAllExtensions(instanceExtensionsPath);
     }
   }
 
+  private File buildExtensionPath(String directory)  {
+    File libDir = new File(directory, LIB_DIR);
+    try {
+      return new File(libDir, EXTENSIONS_DIR).getCanonicalFile();
+    } catch (Exception e) {
+      return new File(libDir, EXTENSIONS_DIR);
+    }
+  }
 
 
   /**
-   * Gets the class loader which should be used for loading classes
-   * and resources. When this class loader provider is disabled, the
-   * system default class loader will be returned by default.
+   * Gets the class loader which should be used for loading classes and resources. When this class loader provider
+   * is disabled, the system default class loader will be returned by default.
    * <p>
-   * Applications <b>MUST NOT</b> maintain persistent references to
-   * the class loader as it can change at run-time.
+   * Applications <b>MUST NOT</b> maintain persistent references to the class loader as it can change at run-time.
    *
-   * @return Returns the class loader which should be used for loading
-   *         classes and resources.
+   * @return Returns the class loader which should be used for loading classes and resources.
    */
   public synchronized ClassLoader getClassLoader() {
     if (loader != null) {
@@ -378,8 +269,7 @@
   /**
    * Indicates whether this class loader provider is enabled.
    *
-   * @return Returns <code>true</code> if this class loader provider
-   *         is enabled.
+   * @return Returns <code>true</code> if this class loader provider is enabled.
    */
   public synchronized boolean isEnabled() {
     return loader != null;
@@ -391,12 +281,11 @@
    * Add the named extensions to this class loader.
    *
    * @param extensions
-   *          The names of the extensions to be loaded.
+   *          A List of the names of the extensions to be loaded.
    * @throws InitializationException
-   *           If one of the extensions could not be loaded and
-   *           initialized.
+   *           If one of the extensions could not be loaded and initialized.
    */
-  private synchronized void addExtension(File... extensions)
+  private synchronized void addExtension(List<File> extensions)
       throws InitializationException {
     // First add the Jar files to the class loader.
     List<JarFile> jars = new LinkedList<>();
@@ -415,9 +304,8 @@
       } catch (Exception e) {
         logger.traceException(e);
 
-        LocalizableMessage message = ERR_ADMIN_CANNOT_OPEN_JAR_FILE.
-            get(extension.getName(), extension.getParent(),
-                stackTraceToSingleLineString(e));
+        LocalizableMessage message = ERR_ADMIN_CANNOT_OPEN_JAR_FILE
+            .get(extension.getName(), extension.getParent(), stackTraceToSingleLineString(e));
         throw new InitializationException(message);
       }
       jarFiles.add(extension);
@@ -438,22 +326,19 @@
    *         <code>null</code> if there is no information available.
    */
   public String printExtensionInformation() {
-    String pathname = DirectoryServer.getServerRoot() + File.separator + LIB_DIR + File.separator + EXTENSIONS_DIR;
-    File extensionsPath = new File(pathname);
+    File extensionsPath = buildExtensionPath(DirectoryServer.getServerRoot());
 
-    if (!extensionsPath.exists() || !extensionsPath.isDirectory()) {
-      // no extensions' directory
-      return null;
+    List<File> extensions = new ArrayList<>();
+    if (extensionsPath.exists() && extensionsPath.isDirectory()) {
+      extensions.addAll(listFiles(extensionsPath));
     }
 
-    File[] extensions = extensionsPath.listFiles(new FileFilter(){
-      public boolean accept(File pathname) {
-        // only files with names ending with ".jar"
-        return pathname.isFile() && pathname.getName().endsWith(".jar");
-      }
-    });
+    File instanceExtensionsPath = buildExtensionPath(DirectoryServer.getInstanceRoot());
+    if (!extensionsPath.getAbsolutePath().equals(instanceExtensionsPath.getAbsolutePath())) {
+      extensions.addAll(listFiles(instanceExtensionsPath));
+    }
 
-    if ( extensions.length == 0 ) {
+    if ( extensions.isEmpty() ) {
       return null;
     }
 
@@ -470,38 +355,50 @@
               EOL);
 
     for(File extension : extensions) {
-      // retrieve MANIFEST entry and display name, build number and revision
-      // number
-      try {
-        JarFile jarFile = new JarFile(extension);
-        JarEntry entry = jarFile.getJarEntry("admin/" + EXTENSION_MANIFEST);
-        if (entry == null)
-        {
-          continue;
-        }
-
-        String[] information = getBuildInformation(jarFile);
-
-        ps.append("Extension: ");
-        boolean addBlank = false;
-        for(String name : information) {
-          if ( addBlank ) {
-            ps.append(addBlank ? " " : ""); // add blank if not first append
-          } else {
-            addBlank = true;
-          }
-
-          ps.printf("%-20s", name);
-        }
-        ps.append(EOL);
-      } catch(Exception e) {
-        // ignore extra information for this extension
-      }
+      printExtensionDetails(ps, extension);
     }
 
     return baos.toString();
   }
 
+  private List<File> listFiles(File path){
+    if (path.exists() && path.isDirectory()) {
+      return Arrays.asList(path.listFiles(new FileFilter() {
+        public boolean accept(File pathname) {
+          // only files with names ending with ".jar"
+          return pathname.isFile() && pathname.getName().endsWith(".jar");
+        }
+      }));
+    }
+    return Collections.emptyList();
+  }
+
+  private void printExtensionDetails(PrintStream ps, File extension) {
+    // retrieve MANIFEST entry and display name, build number and revision number
+    try {
+      JarFile jarFile = new JarFile(extension);
+      JarEntry entry = jarFile.getJarEntry("admin/" + EXTENSION_MANIFEST);
+      if (entry == null) {
+        return;
+      }
+
+      String[] information = getBuildInformation(jarFile);
+
+      ps.append("Extension: ");
+      boolean addBlank = false;
+      for(String name : information) {
+        if ( addBlank ) {
+          ps.append(" ");
+        } else {
+          addBlank = true;
+        }
+        ps.printf("%-20s", name);
+      }
+      ps.append(EOL);
+    } catch(Exception e) {
+      // ignore extra information for this extension
+    }
+  }
 
 
   /**
@@ -511,11 +408,12 @@
    * <br>index 2: the revision number of the extension.
    *
    * @param extension the jar file of the extension
-   * @return a String array containing the name, the build number and the
-   *         revision number of the extension given in argument
+   * @return a String array containing the name, the build number and the revision number
+   *         of the extension given in argument
    * @throws java.io.IOException thrown if the jar file has been closed.
    */
-  private String[] getBuildInformation(JarFile extension) throws IOException {
+  private String[] getBuildInformation(JarFile extension)
+      throws IOException {
     String[] result = new String[3];
 
     // retrieve MANIFEST entry and display name, version and revision
@@ -540,24 +438,20 @@
 
 
   /**
-   * Put extensions jars into the class loader and load all
-   * configuration definition classes in that they contain.
+   * Put extensions jars into the class loader and load all configuration definition classes in that they contain.
    * @param extensionsPath Indicates where extensions are located.
    *
    * @throws InitializationException
-   *           If the extensions folder could not be accessed or if a
-   *           extension jar file could not be accessed or if one of
-   *           the configuration definition classes could not be
-   *           initialized.
+   *           If the extensions folder could not be accessed or if a extension jar file could not be accessed or
+   *           if one of the configuration definition classes could not be initialized.
    */
   private void initializeAllExtensions(File extensionsPath)
       throws InitializationException {
 
     try {
       if (!extensionsPath.exists()) {
-        // The extensions directory does not exist. This is not a
-        // critical problem.
-        logger.error(ERR_ADMIN_NO_EXTENSIONS_DIR, extensionsPath);
+        // The extensions directory does not exist. This is not a critical problem.
+        logger.warn(WARN_ADMIN_NO_EXTENSIONS_DIR, extensionsPath);
         return;
       }
 
@@ -566,25 +460,8 @@
         throw new InitializationException(ERR_ADMIN_EXTENSIONS_DIR_NOT_DIRECTORY.get(extensionsPath));
       }
 
-      // Get each extension file name.
-      FileFilter filter = new FileFilter() {
-
-        /**
-         * Must be a Jar file.
-         */
-        public boolean accept(File pathname) {
-          if (!pathname.isFile()) {
-            return false;
-          }
-
-          String name = pathname.getName();
-          return name.endsWith(".jar");
-        }
-
-      };
-
       // Add and initialize the extensions.
-      addExtension(extensionsPath.listFiles(filter));
+      addExtension(listFiles(extensionsPath));
     } catch (InitializationException e) {
       logger.traceException(e);
       throw e;
@@ -603,14 +480,12 @@
    * Make sure all core configuration definitions are loaded.
    *
    * @throws InitializationException
-   *           If the core manifest file could not be read or if one
-   *           of the configuration definition classes could not be
-   *           initialized.
+   *           If the core manifest file could not be read or if one of the configuration definition
+   *           classes could not be initialized.
    */
   private void initializeCoreComponents()
       throws InitializationException {
-    InputStream is = RootCfgDefn.class.getResourceAsStream("/admin/"
-        + CORE_MANIFEST);
+    InputStream is = RootCfgDefn.class.getResourceAsStream("/admin/" + CORE_MANIFEST);
 
     if (is == null) {
       LocalizableMessage message = ERR_ADMIN_CANNOT_FIND_CORE_MANIFEST.get(CORE_MANIFEST);
@@ -631,20 +506,17 @@
 
 
   /**
-   * Make sure all the configuration definition classes in a extension
-   * are loaded.
+   * Make sure all the configuration definition classes in a extension are loaded.
    *
    * @param jarFile
    *          The extension's Jar file.
    * @throws InitializationException
-   *           If the extension jar file could not be accessed or if
-   *           one of the configuration definition classes could not
-   *           be initialized.
+   *           If the extension jar file could not be accessed or if one of the configuration definition classes
+   *           could not be initialized.
    */
   private void initializeExtension(JarFile jarFile)
       throws InitializationException {
-    JarEntry entry = jarFile.getJarEntry("admin/"
-        + EXTENSION_MANIFEST);
+    JarEntry entry = jarFile.getJarEntry("admin/" + EXTENSION_MANIFEST);
     if (entry != null) {
       InputStream is;
       try {
@@ -652,8 +524,7 @@
       } catch (Exception e) {
         logger.traceException(e);
 
-        LocalizableMessage message = ERR_ADMIN_CANNOT_READ_EXTENSION_MANIFEST.get(
-            EXTENSION_MANIFEST, jarFile.getName(),
+        LocalizableMessage message = ERR_ADMIN_CANNOT_READ_EXTENSION_MANIFEST.get(EXTENSION_MANIFEST, jarFile.getName(),
             stackTraceToSingleLineString(e));
         throw new InitializationException(message);
       }
@@ -663,8 +534,8 @@
       } catch (InitializationException e) {
         logger.traceException(e);
 
-        LocalizableMessage message = ERR_CLASS_LOADER_CANNOT_LOAD_EXTENSION.get(jarFile
-            .getName(), EXTENSION_MANIFEST, stackTraceToSingleLineString(e));
+        LocalizableMessage message = ERR_CLASS_LOADER_CANNOT_LOAD_EXTENSION.get(jarFile.getName(), EXTENSION_MANIFEST,
+            stackTraceToSingleLineString(e));
         throw new InitializationException(message);
       }
       logExtensionsBuildInformation(jarFile);
@@ -687,14 +558,12 @@
 
 
   /**
-   * Forcefully load configuration definition classes named in a
-   * manifest file.
+   * Forcefully load configuration definition classes named in a manifest file.
    *
    * @param is
    *          The manifest file input stream.
    * @throws InitializationException
-   *           If the definition classes could not be loaded and
-   *           initialized.
+   *           If the definition classes could not be loaded and initialized.
    */
   private void loadDefinitionClasses(InputStream is)
       throws InitializationException {
@@ -732,8 +601,7 @@
       try {
         theClass = Class.forName(className, true, loader);
       } catch (Exception e) {
-        throw new InitializationException(
-            ERR_CLASS_LOADER_CANNOT_LOAD_CLASS.get(className, e.getMessage()), e);
+        throw new InitializationException(ERR_CLASS_LOADER_CANNOT_LOAD_CLASS.get(className, e.getMessage()), e);
       }
       if (AbstractManagedObjectDefinition.class.isAssignableFrom(theClass)) {
         // We need to instantiate it using its getInstance() static method.
@@ -763,8 +631,7 @@
         d.initialize();
       } catch (Exception e) {
         throw new InitializationException(
-            ERR_CLASS_LOADER_CANNOT_INITIALIZE_DEFN.get(
-                d.getName(), d.getClass().getName(), e.getMessage()), e);
+            ERR_CLASS_LOADER_CANNOT_INITIALIZE_DEFN.get(d.getName(), d.getClass().getName(), e.getMessage()), e);
       }
     }
   }

--
Gitblit v1.10.0