Make Definer use the new ClasspathUtils, make ClasspathUtils even more

reuse-friendly.

PR: 19213
Submitted by:	Marc Portier <mpo at apache dot org>


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@274505 13f79535-47bb-0310-9956-ffa450edef68

src/etc/testcases/core/loaderref/loaderref.xml

@@ -22,6 +22,7 @@ This build file is intended to be used for testing Ant
 
   <target name="testbadref" depends="compile" >
     <taskdef loaderref="loaderref-test"
+             name="test1"
              classname="Test1"
              classpath="${classes.dir}"/>
   </target>

src/main/org/apache/tools/ant/taskdefs/Definer.java

@@ -66,6 +66,7 @@ import org.apache.tools.ant.Project;
 import org.apache.tools.ant.Task;
 import org.apache.tools.ant.types.Path;
 import org.apache.tools.ant.types.Reference;
+import org.apache.tools.ant.util.ClasspathUtils;
 
 /**
  * Base class for Taskdef and Typedef - does all the classpath
@@ -79,21 +80,16 @@ import org.apache.tools.ant.types.Reference;
 public abstract class Definer extends Task {
     private String name;
     private String value;
-    private Path classpath;
     private File file;
     private String resource;
-    private boolean reverseLoader = false;
-    private String loaderId = null;
-    private String classpathId = null;
-
-    private static final String REUSE_LOADER_REF = "ant.reuse.loader";
+    private ClasspathUtils.Delegate cpDelegate;
 
     /**
      * @deprecated stop using this attribute
      * @ant.attribute ignore="true"
      */
     public void setReverseLoader(boolean reverseLoader) {
-        this.reverseLoader = reverseLoader;
+        this.cpDelegate.setReverseLoader(reverseLoader);
         log("The reverseloader attribute is DEPRECATED. It will be removed",
             Project.MSG_WARN);
     }
@@ -103,7 +99,7 @@ public abstract class Definer extends Task {
     }
 
     public Path getClasspath() {
-        return classpath;
+        return cpDelegate.getClasspath();
     }
 
     public File getFile() {
@@ -115,15 +111,15 @@ public abstract class Definer extends Task {
     }
 
     public boolean isReverseLoader() {
-        return reverseLoader;
+        return cpDelegate.isReverseLoader();
     }
 
     public String getLoaderId() {
-        return loaderId;
+        return cpDelegate.getClassLoadId();
     }
 
     public String getClasspathId() {
-        return classpathId;
+        return cpDelegate.getClassLoadId();
     }
 
     /**
@@ -132,21 +128,14 @@ public abstract class Definer extends Task {
      * @param classpath an Ant Path object containing the classpath.
      */
     public void setClasspath(Path classpath) {
-        if (this.classpath == null) {
-            this.classpath = classpath;
-        } else {
-            this.classpath.append(classpath);
-        }
+        this.cpDelegate.setClasspath(classpath);
     }
 
     /**
      * Create the classpath to be used when searching for component being defined
      */
     public Path createClasspath() {
-        if (this.classpath == null) {
-            this.classpath = new Path(getProject());
-        }
-        return this.classpath.createPath();
+        return this.cpDelegate.createClasspath();
     }
 
     /**
@@ -154,8 +143,7 @@ public abstract class Definer extends Task {
      * To actually share the same loader, set loaderref as well
      */
     public void setClasspathRef(Reference r) {
-        classpathId=r.getRefId();
-        createClasspath().setRefid(r);
+        this.cpDelegate.setClasspathref(r);
     }
 
     /**
@@ -170,7 +158,7 @@ public abstract class Definer extends Task {
      * @since Ant 1.5
      */
     public void setLoaderRef(Reference r) {
-        loaderId = r.getRefId();
+        this.cpDelegate.setLoaderRef(r);
     }
 
 
@@ -272,81 +260,12 @@ public abstract class Definer extends Task {
      * create a classloader for this definition
      */
     private ClassLoader createLoader() {
-        // magic property
-        if (getProject().getProperty(REUSE_LOADER_REF) != null) {
-            // Generate the 'reuse' name automatically from the reference.
-            // This allows <taskdefs> that work on both ant1.4 and ant1.5.
-            // ( in 1.4 it'll require the task/type to be in classpath if they
-            //   are used togheter ).
-            if (loaderId == null && classpathId != null) {
-                loaderId = "ant.loader." + classpathId;
-            }
-        }
-
-        // If a loader has been set ( either by loaderRef or magic property )
-        if (loaderId != null) {
-            Object reusedLoader = getProject().getReference(loaderId);
-            if (reusedLoader != null) {
-                if (!(reusedLoader instanceof ClassLoader)) {
-                    throw new BuildException("The specified loader id " +
-                        loaderId + " does not reference a class loader");
-                }
-
-                return (ClassLoader)reusedLoader;
-                //if (reusedLoader instanceof AntClassLoader) {
-                //    return (AntClassLoader)reusedLoader;
-                //}
-                // In future the reference object may be the <loader> type
-                // if (reusedLoader instanceof Loader ) {
-                //      return ((Loader)reusedLoader).getLoader(project);
-                // }
-            }
-        }
-
-        ClassLoader al = null;
-
-        if (classpath == null) {
-            // do we need to create another loader ?
-            al=project.getCoreLoader();
-            if (al != null ) {
-                return al;
-            }
-        }
-
-        if (classpath != null) {
-            project.log( "Creating new loader for taskdef using " + classpath +
-                    " reverse=" + reverseLoader, Project.MSG_DEBUG );
-            AntClassLoader acl = getProject().createClassLoader(classpath);
-            if (reverseLoader) {
-                acl.setParentFirst(false);
-                acl.addJavaLibraries();
-            }
-            al = acl;
-        } else {
-            // XXX Probably it would be better to reuse getClass().getClassLoader()
-            // I don't think we need a new ( identical ) loader for each task
-            AntClassLoader acl
-                = getProject().createClassLoader(Path.systemClasspath);
-            if (reverseLoader) {
-                acl.setParentFirst(false);
-                acl.addJavaLibraries();
-            }
-            al = acl;
-        }
+        ClassLoader al = this.cpDelegate.getClassLoader();
         // need to load Task via system classloader or the new
         // task we want to define will never be a Task but always
         // be wrapped into a TaskAdapter.
         ((AntClassLoader)al).addSystemPackageRoot("org.apache.tools.ant");
 
-
-        // If the loader is new, record it for future uses by other
-        // task/typedefs
-        if (loaderId != null) {
-            if (getProject().getReference(loaderId) == null) {
-                getProject().addReference(loaderId, al);
-            }
-        }
-
         return al;
     }
 
@@ -396,4 +315,15 @@ public abstract class Definer extends Task {
      * they will get to add a new definition of their type.
      */
     protected abstract void addDefinition(String name, Class c);
+    
+    
+    /**
+     * @see org.apache.tools.ant.Task#init()
+     * @since Ant 1.6
+     */
+    public void init() throws BuildException {
+        this.cpDelegate = ClasspathUtils.getDelegate(this);
+        super.init();
+    }
+
 }

src/main/org/apache/tools/ant/util/ClasspathUtils.java

@@ -56,6 +56,7 @@ package org.apache.tools.ant.util;
 import org.apache.tools.ant.AntClassLoader;
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
+import org.apache.tools.ant.ProjectComponent;
 import org.apache.tools.ant.types.Path;
 import org.apache.tools.ant.types.Reference;
 
@@ -63,35 +64,38 @@ import org.apache.tools.ant.types.Reference;
  * Offers some helper methods on the Path structure in ant.
  *
  * <p>Basic idea behind this utility class is to use it from inside the 
- * different ant objects (and user defined objects) that need dclassLoading 
+ * different ant objects (and user defined objects) that need classLoading 
  * for their operation.
- * Normally those would have a setClasspathRef() {for the @@classpathref} 
+ * Normally those would have a setClasspathRef() {for the @classpathref} 
  * and/or a createClasspath() {for the nested &lt;classpath&gt;}
- * Typically one would have</p> 
+ * Typically one would have in your Ant Task or DataType</p> 
  *
  * <pre><code>
+ * ClasspathUtils.Delegate cpDelegate;
+ * 
+ * public void init(){
+ *     this.cpDelegate = ClasspathUtils.getDelegate(this);
+ *     super.init();
+ * }
+ * 
  * public void setClasspathRef(Reference r) {
- *     this.classpathId = r.getRefId();
- *     createClasspath().setRefid(r);
+ *     this.cpDelegate.setClasspathRef(r);
  * }
  * 
  * public Path createClasspath() {
- *     if (this.classpath == null) {
- *         this.classpath = new Path(getProject());
- *     }
- *     return this.classpath.createPath();
+ *     return this.cpDelegate.createClasspath();
  * }
  * 
  * public void setClassname(String fqcn) {
- *     this.classname = fqcn;
+ *     this.cpDelegate.setClassname(fqcn);
  * }
  * </code></pre>
  * 
- * <p>when you actually need the classloading you can just:</p>
+ * <p>At execution time, when you actually need the classloading 
+ * you can just:</p>
  * 
  * <pre><code>
- *     ClassLoader cl = ClasspathUtils.getClassLoaderForPath(this.classpath, this.classpathId);
- *     Object o = ClasspathUtils.newInstance(this.classname, cl);
+ *     Object o = this.cpDelegate.newInstance();
  * </code></pre>
  *
  * @since Ant 1.6
@@ -110,12 +114,14 @@ public class ClasspathUtils {
      * @param pathId
      * @return
      */
-    public static ClassLoader getClassLoaderForPath(Project p, Reference ref) {
+    public static ClassLoader getClassLoaderForPath(
+        Project p, Reference ref) {
+
         return getClassLoaderForPath(p, ref, false);
     }
-    
+
     /** 
-     * Convenience overloaded version of {@link #geClassLoader(Path,
+     * Convenience overloaded version of {@link #geClassLoader(Project, Path,
      * String, boolean)}.
      *
      * <p>Delegates to the other one after extracting the referenced
@@ -128,31 +134,54 @@ public class ClasspathUtils {
      * classloader behaviour)
      * @return
      */
-    public static ClassLoader getClassLoaderForPath(Project p, Reference ref, 
-                                                    boolean reverseLoader) {
+    public static ClassLoader getClassLoaderForPath(
+        Project p, Reference ref, boolean reverseLoader) {
+            
         String pathId = ref.getRefId();
         Object path = p.getReference(pathId);
-        if (!(path instanceof Path)){
-            throw new BuildException ("The specified classpathref " + pathId + 
-                " does not reference a Path.");
-        }        
-        return getClassLoaderForPath((Path)path, pathId, reverseLoader);
+        if (!(path instanceof Path)) {
+            throw new BuildException(
+                "The specified classpathref "
+                    + pathId
+                    + " does not reference a Path.");
+        }
+        String loaderId = LOADER_ID_PREFIX + pathId;
+        return getClassLoaderForPath(p, (Path) path, loaderId, reverseLoader);
     }
 
     /** 
      * Convenience overloaded version of {@link
-     * #getClassLoaderForPath(Path, String, boolean)}.
+     * #getClassLoaderForPath(Project, Path, String, boolean)}.
      *
      * <p>Assumes the logical 'false' for the reverseLoader.</p>
      *  
      * @param path 
-     * @param pathId
+     * @param loaderId
      * @return
      */
-    public static ClassLoader getClassLoaderForPath(Path path, String pathId){
-        return getClassLoaderForPath(path, pathId, false);    
+    public static ClassLoader getClassLoaderForPath(
+        Project p, Path path, String loaderId) {
+
+        return getClassLoaderForPath(p, path, loaderId, false);
     }
-    
+
+    /** 
+     * Convenience overloaded version of {@link
+     * #getClassLoaderForPath(Project, Path, String, boolean, boolean)}.
+     *
+     * <p>Sets value for 'reuseLoader' to true if the magic property
+     * has been set.</p>
+     *  
+     * @param path 
+     * @param loaderId
+     * @return
+     */
+    public static ClassLoader getClassLoaderForPath(
+        Project p, Path path, String loaderId, boolean reverseLoader) {
+        return getClassLoaderForPath(p, path, loaderId, reverseLoader,
+                                     isMagicPropertySet(p));
+    }
+
     /**
      * Gets a classloader that loads classes from the classpath
      * defined in the path argument.
@@ -162,35 +191,38 @@ public class ClasspathUtils {
      * created loader with that id, and of course store it there upon
      * creation.</p>
      * @param path Path object to be used as classpath for this classloader
-     * @param pathId identification for this Path, will be used to
-     * identify the classLoader as well.
+     * @param loaderID identification for this Loader, 
      * @param reverseLoader if set to true this new loader will take
      * precedence over it's parent (which is contra the regular
+     * @param p Ant Project where the handled components are living in. 
      * classloader behaviour)
      * @return ClassLoader that uses the Path as its classpath. 
      */
-    public static ClassLoader getClassLoaderForPath(Path path, String pathId, 
-                                                    boolean reverseLoader) {
+    public static ClassLoader getClassLoaderForPath(
+        Project p, Path path, String loaderId, boolean reverseLoader,
+        boolean reuseLoader) {
+
         ClassLoader cl = null;
-        Project p = path.getProject();
-        String loaderId = LOADER_ID_PREFIX + pathId;
-        // code stolen from o.a.t.a.taskdefs.Definer, might be a todo
-        // to remove it there didn't look at the reverse loader stuff
-        // however (todo that first)
 
         // magic property
-        if (p.getProperty(REUSE_LOADER_REF) != null) {
-            //chose not to do the extra instanceof checking here, consider it 
-            // a programming error and not a config error if this fails
-            // so I assume the RuntimeException is OK 
-            cl = (ClassLoader)p.getReference(loaderId);
-        }
-        if (cl == null){
-            cl = getUniqueClassLoaderForPath(path, reverseLoader);
-            p.addReference(loaderId, cl);            
-        } 
-        
-        return cl;        
+        if (loaderId != null && reuseLoader) {
+            Object reusedLoader = p.getReference(loaderId);
+            if (reusedLoader != null && 
+                !(reusedLoader instanceof ClassLoader)) {
+                throw new BuildException("The specified loader id " + loaderId +
+                                         " does not reference a class loader");
+            }
+
+            cl = (ClassLoader) reusedLoader;
+        }
+        if (cl == null) {
+            cl = getUniqueClassLoaderForPath(p, path, reverseLoader);
+            if (loaderId != null && reuseLoader) {
+                p.addReference(loaderId, cl);
+            }
+        }
+
+        return cl;
     }
 
     /**
@@ -203,39 +235,256 @@ public class ClasspathUtils {
      * @param reverseLoader
      * @return
      */
-    public static ClassLoader getUniqueClassLoaderForPath(Path path,
-                                                          boolean reverseLoader) {
-        ClassLoader cl = null;
-        Project p = path.getProject();
+    public static ClassLoader getUniqueClassLoaderForPath(
+        Project p,
+        Path path,
+        boolean reverseLoader) {
 
-        AntClassLoader acl = p.createClassLoader(Path.systemClasspath);
+        AntClassLoader acl = p.createClassLoader(path != null 
+                                                 ? path : Path.systemClasspath);
         if (reverseLoader) {
             acl.setParentFirst(false);
             acl.addJavaLibraries();
         }
 
-        return cl;        
+        return acl;
     }
-    
-    public static Object newInstance(String className,
-                                     ClassLoader userDefinedLoader){
+
+    /**
+     * Creates a fresh object instance of the specified classname.
+     * 
+     * <p> This uses the userDefinedLoader to load the specified class, 
+     * and then makes an instance using the default no-argument constructor
+     * </p>
+     * 
+     * @param className the full qualified class name to load.
+     * @param userDefinedLoader the classloader to use.
+     * @return
+     * @throws BuildException when loading or instantiation failed.
+     */
+    public static Object newInstance(
+        String className,
+        ClassLoader userDefinedLoader) {
         try {
             Class clazz = userDefinedLoader.loadClass(className);
-            Object o = clazz.newInstance();        
-            return o; 
+            Object o = clazz.newInstance();
+            return o;
         } catch (ClassNotFoundException e) {
-            throw new BuildException("Class " + className + 
-                                     " not found by the specific classLoader.",
-                                     e);
+            throw new BuildException(
+                "Class "
+                    + className
+                    + " not found by the specific classLoader.",
+                e);
         } catch (InstantiationException e) {
-            throw new BuildException("Could not instantiate " + className 
-                                     + ". Specified class should have a no "
-                                     + "argument constructor.", e);
+            throw new BuildException(
+                "Could not instantiate "
+                    + className
+                    + ". Specified class should have a no "
+                    + "argument constructor.",
+                e);
         } catch (IllegalAccessException e) {
-            throw new BuildException("Could not instantiate " + className 
-                                     + ". Specified class should have a "
-                                     + "public constructor.", e);
+            throw new BuildException(
+                "Could not instantiate "
+                    + className
+                    + ". Specified class should have a "
+                    + "public constructor.",
+                e);
         }
     }
 
+    /**
+     * Obtains a delegate that helps out with classic classpath configuration.
+     * 
+     * @param component your projectComponent that needs the assistence
+     * @return the helper, delegate.
+     * @see ClasspathUtils.Delegate
+     */
+    public static Delegate getDelegate(ProjectComponent component) {
+        return new Delegate(component);
+    }
+
+    /**
+     * Checks for the magic property that enables class loader reuse
+     * for <taskdef> and <typedef> in Ant 1.5 and earlier.
+     */
+    private static boolean isMagicPropertySet(Project p) {
+        return p.getProperty(REUSE_LOADER_REF) != null;
+    }
+
+    /** 
+     * Delegate that helps out any specific ProjectComponent that needs
+     * dynamic classloading.
+     *  
+     * <p>Ant ProjectComponents that need a to be able to dynamically load
+     * Classes and instantiate them often expose the following ant syntax
+     * sugar: </p>
+     * 
+     * <ul><li> nested &lt;classpath&gt; </li>
+     * <li> attribute @classpathref </li>
+     * <li> attribute @classname </li></ul>
+     * 
+     * <p> This class functions as a delegate handling the configuration
+     * issues for this recuring pattern.  It's usage pattern, as the name 
+     * suggests is delegation, not inheritance. </p>
+     * 
+     * @since Ant 1.6
+     */
+    public static class Delegate {
+        private final ProjectComponent component;
+        private Path classpath;
+        private String classpathId;
+        private String className;
+        private String loaderId;
+        private boolean reverseLoader = false;
+        
+        /**
+         * Constructs Delegate
+         * @param component
+         */
+        Delegate(ProjectComponent component) {
+            this.component = component;
+        }
+        
+        /**
+         * Delegate method handling the @classpath attribute
+         * 
+         * <p>This attribute can set a path to add to the classpath</p> 
+         * 
+         * @param classpath
+         */
+        public void setClasspath(Path classpath) {
+            if (this.classpath == null) {
+                this.classpath = classpath;
+            } else {
+                this.classpath.append(classpath);
+            }
+        }
+        
+        /** 
+         * Delegate method handling the &lt;classpath&gt; tag.
+         * 
+         * <p>This nested path-like structure can set a path to add to the
+         * classpath</p>
+         * 
+         * @return
+         */
+        public Path createClasspath() {
+            if (this.classpath == null) {
+                this.classpath = new Path(component.getProject());
+            }
+            return this.classpath.createPath();
+        }
+        
+        /**
+         * Delegate method handling the @classname attribute.
+         * 
+         * <p>This attribute sets the full qualified class name of the class
+         * to lad and instantiate</p>
+         * 
+         * @param fcqn
+         */
+        public void setClassname(String fcqn) {
+            this.className = fcqn;
+        }
+        
+        /** 
+         * Delegate method handling the @classpathref attribute.
+         * 
+         * <p>This attribute can add a referenced path-like structure to the
+         * classpath</p> 
+         * 
+         * @param r
+         */
+        public void setClasspathref(Reference r) {
+            this.classpathId = r.getRefId();
+            createClasspath().setRefid(r);
+        }        
+        
+        /**
+         * Delegate method handling the @reverseLoader attribute.
+         * 
+         * <p>This attribute can set a boolean indicating that the used 
+         * classloader should NOT follow the classical parent-first scheme.
+         * </p>
+         * 
+         * <p>By default this is supposed to be false</p>
+         * 
+         * <p>Caution: this behaviour is contradictory to the normal way
+         * classloaders work.  Do not let your ProjectComponent use it if
+         * you are not really sure</p> 
+         * 
+         * @param reverseLoader
+         */
+        public void setReverseLoader(boolean reverseLoader) {
+            this.reverseLoader = reverseLoader;
+        }
+
+        /**
+         * Sets the loaderRef 
+         * @param r
+         */
+        public void setLoaderRef(Reference r) {
+            this.loaderId = r.getRefId();
+        }
+            
+
+        /** 
+         * Finds or creates the classloader for this
+         * @return
+         */
+        public ClassLoader getClassLoader() {
+            ClassLoader cl;
+            cl= ClasspathUtils.getClassLoaderForPath(
+                    getContextProject(),
+                    this.classpath,
+                    getClassLoadId(),
+                    this.reverseLoader,
+                    loaderId != null 
+                    || isMagicPropertySet(getContextProject()));
+            return cl;
+        }
+                
+        /**
+         * The project of the ProjectComponent we are working for.
+         */
+        private Project getContextProject() {
+            return this.component.getProject();            
+        }
+
+        /**
+         * Computes the loaderId based on the configuration of the component.
+         */
+        public String getClassLoadId() {
+            if (this.loaderId == null && this.classpathId != null) {
+                return ClasspathUtils.LOADER_ID_PREFIX + this.classpathId;
+            } else {
+                return this.loaderId;
+            }            
+        }
+
+        /** 
+         * Helper method obtaining a fresh instance of the class specified
+         * in the @classname and using the specified classpath.
+         *   
+         * @return the fresh instantiated object.
+         */
+        public Object newInstance() {
+            ClassLoader cl = getClassLoader();
+            return ClasspathUtils.newInstance(this.className, cl);
+        }
+
+        /**
+         * The classpath.
+         */
+        public Path getClasspath() {
+            return classpath;
+        }
+
+        public boolean isReverseLoader() {
+            return reverseLoader;
+        }
+
+        //TODO no methods yet for getClassname
+        //TODO no method for newInstance using a reverse-classloader
+    }
 }