Rework scriptdef to use DynamicConfigurator

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

src/etc/testcases/taskdefs/optional/script/scriptdef.xml

@@ -109,4 +109,20 @@
       <attribute name="attr1"/>
     </scriptdef>
   </target>
+  
+  <target name="property">
+    <scriptdef name="scripttest" language="javascript">
+      <attribute name="attr1"/>
+      <![CDATA[
+        
+        project.log("Attribute value = " + attributes.get("attr1"));
+      ]]>
+    </scriptdef>
+
+    <property name="testproperty" value="test"/>    
+    <scripttest attr1="${testproperty}">
+    </scripttest>
+  </target>
+
+
 </project>

src/main/org/apache/tools/ant/DynamicConfigurator.java

@@ -69,7 +69,7 @@ public interface DynamicConfigurator {
      * @param value the new value of the attribute
      * @throws BuildException when any error occurs
      */    
-    public void setDynamicAttribute(String name, String value)
+    void setDynamicAttribute(String name, String value)
             throws BuildException;
 
     /**
@@ -79,5 +79,5 @@ public interface DynamicConfigurator {
      * @throws BuildException when any error occurs
      * @return the element created
      */    
-    public Object createDynamicElement(String name) throws BuildException;
+    Object createDynamicElement(String name) throws BuildException;
 }

src/main/org/apache/tools/ant/taskdefs/optional/script/ScriptDef.java

@@ -110,6 +110,10 @@ public class ScriptDef extends Task {
         this.name = name;
     }
 
+    public boolean isAttributeSupported(String attributeName) {
+        return attributeSet.contains(attributeName);
+    }
+    
     /**
      * Set the scripting language used by this script
      *
@@ -276,99 +280,74 @@ public class ScriptDef extends Task {
         project.addTaskDefinition(name, ScriptDefBase.class);
     }
 
-    /**
-     * Execute the script.
-     *
-     * @param scriptConfig the RuntimeConfigurable which contains the attribute
-     * definitions for the script task instance.
-     *
-     * @param elements a list of UnknownElements which contain the configuration
-     * of the nested elements of the script instance.
-     */
-    public void executeScript(RuntimeConfigurable scriptConfig, List elements) {
-        
-        Map configAttributes = scriptConfig.getAttributeMap();
-        for (Iterator i = configAttributes.keySet().iterator(); i.hasNext();) {
-            String attributeName = (String) i.next();
-            if (!attributeSet.contains(attributeName)) {
-                throw new BuildException("<" + name + "> does not support " 
-                    + "the \"" + attributeName + "\" attribute");
-            }
+    public Object createNestedElement(String elementName) {
+        NestedElement definition 
+            = (NestedElement) nestedElementMap.get(elementName);
+        if (definition == null) {                
+            throw new BuildException("<" + name + "> does not support " 
+                + "the <" + elementName + "> nested element");
         }
-
-        // handle nested elements
-        Map elementInstances = new HashMap();
-        for (Iterator i = elements.iterator(); i.hasNext();) {
-            UnknownElement element = (UnknownElement) i.next();
-            String elementTag = element.getTag().toLowerCase(Locale.US);
-            
-            NestedElement definition 
-                = (NestedElement) nestedElementMap.get(elementTag);
-            if (definition == null) {                
-                throw new BuildException("<" + name + "> does not support " 
-                    + "the <" + elementTag + "> nested element");
+        
+        Object instance = null;
+        String classname = definition.className;
+        if (classname == null) {
+            instance = getProject().createTask(definition.type);
+            if (instance == null) {
+                instance = getProject().createDataType(definition.type);
             }
+        } else {
+            // try the context classloader
+            ClassLoader loader 
+                = Thread.currentThread().getContextClassLoader();
             
-            // what is the type of the object to be created
-            Object instance = null;
-            String classname = definition.className;
-            if (classname == null) {
-                instance = getProject().createTask(definition.type);
-                if (instance == null) {
-                    instance = getProject().createDataType(definition.type);
-                }
-            } else {
-                // try the context classloader
-                ClassLoader loader 
-                    = Thread.currentThread().getContextClassLoader();
-                
-                Class instanceClass = null;
+            Class instanceClass = null;
+            try {
+                instanceClass = Class.forName(classname, true, loader);
+            } catch (Throwable e) {
+                // try normal method
                 try {
-                    instanceClass = Class.forName(classname, true, loader);
-                } catch (Throwable e) {
-                    // try normal method
-                    try {
-                        instanceClass = Class.forName(classname);
-                    } catch (Throwable e2) {
-                        throw new BuildException("scriptdef: Unable to load " 
-                            + "class " + classname + " for nested element <" 
-                            + elementTag + ">", e2);
-                    }
+                    instanceClass = Class.forName(classname);
+                } catch (Throwable e2) {
+                    throw new BuildException("scriptdef: Unable to load " 
+                        + "class " + classname + " for nested element <" 
+                        + elementName + ">", e2);
                 }
-                
-                try {
-                    instance = instanceClass.newInstance();
-                } catch (Throwable e) {
-                    throw new BuildException("scriptdef: Unable to create " 
-                        + "element of class " + classname + " for nested " 
-                        + "element <" + elementTag + ">", e);
-                }
-                getProject().setProjectReference(instance);
-            }
-            
-            if (instance == null) {
-                throw new BuildException("<" + name + "> is unable to create " 
-                    + "the <" + elementTag + "> nested element");
             }
-
-            element.configure(instance);
             
-            // find the appropriate list
-            List instanceList = (List) elementInstances.get(elementTag);
-            if (instanceList == null) {
-                instanceList = new ArrayList();
-                elementInstances.put(elementTag, instanceList);
+            try {
+                instance = instanceClass.newInstance();
+            } catch (Throwable e) {
+                throw new BuildException("scriptdef: Unable to create " 
+                    + "element of class " + classname + " for nested " 
+                    + "element <" + elementName + ">", e);
             }
-            instanceList.add(instance);
+            getProject().setProjectReference(instance);
         }
         
+        if (instance == null) {
+            throw new BuildException("<" + name + "> is unable to create " 
+                + "the <" + elementName + "> nested element");
+        }
+        return instance;
+    }
+    
+    /**
+     * Execute the script.
+     *
+     * @param scriptConfig the RuntimeConfigurable which contains the attribute
+     * definitions for the script task instance.
+     *
+     * @param elements a list of UnknownElements which contain the configuration
+     * of the nested elements of the script instance.
+     */
+    public void executeScript(Map attributes, Map elements) {
         try {
             BSFManager manager = new BSFManager();
             // execute the script
-            manager.declareBean("attributes", configAttributes, 
-                configAttributes.getClass());
-            manager.declareBean("elements", elementInstances, 
-                elementInstances.getClass());
+            manager.declareBean("attributes", attributes, 
+                attributes.getClass());
+            manager.declareBean("elements", elements, 
+                elements.getClass());
             manager.declareBean("project", getProject(), Project.class);
             manager.exec(language, "scriptdef <" + name + ">", 0, 0, script);
         } catch (BSFException e) {

src/main/org/apache/tools/ant/taskdefs/optional/script/ScriptDefBase.java

@@ -57,8 +57,9 @@ import org.apache.tools.ant.Task;
 import org.apache.tools.ant.TaskContainer;
 import org.apache.tools.ant.MagicNames;
 import org.apache.tools.ant.BuildException;
-
+import org.apache.tools.ant.DynamicConfigurator;
 import java.util.Map;
+import java.util.HashMap;
 import java.util.List;
 import java.util.ArrayList;
 
@@ -71,16 +72,23 @@ import java.util.ArrayList;
  * @author Conor MacNeill
  * @since Ant 1.6
  */
-public class ScriptDefBase extends Task implements TaskContainer {
+public class ScriptDefBase extends Task implements DynamicConfigurator {
     
-    /** Nested elements - UnknownElements passed by Ant */
-    private List nestedElements = new ArrayList();
+    /** Nested elements */
+    private Map nestedElementMap = new HashMap();
+
+    /** Attributes */
+    private Map attributes = new HashMap();
     
     /**
      * Locate the script defining task and execute the script by passing
      * control to it 
      */
     public void execute() {
+        getScript().executeScript(attributes, nestedElementMap);
+    }
+    
+    private ScriptDef getScript() {
         String name = getTaskType();
         Map scriptRepository 
             = (Map) getProject().getReference(MagicNames.SCRIPT_REPOSITORY);
@@ -92,24 +100,28 @@ public class ScriptDefBase extends Task implements TaskContainer {
         if (definition == null) {
             throw new BuildException("Script definition not found for " + name);
         }
-        definition.executeScript(getWrapper(), nestedElements);
+        return definition;
     }
-
-    /** 
-     * Method overridden to prevent configuration of this task by the core
-     */
-    public void maybeConfigure() {
-        // don't configure now - do it at script execute time
+    
+    public Object createDynamicElement(String name)  {
+        List nestedElementList = (List) nestedElementMap.get(name);
+        if (nestedElementList == null) {
+            nestedElementList = new ArrayList();
+            nestedElementMap.put(name, nestedElementList);
+        }
+        Object element = getScript().createNestedElement(name);
+        nestedElementList.add(element);
+        return element;
     }
     
-    /**
-     * TaskContainer method implemented to prevent processing of the task
-     * instance by the Ant Core.
-     *
-     * @param nestedElement the nestedElement as an UnknownElement.
-     */
-    public void addTask(Task nestedElement) {
-        nestedElements.add(nestedElement);
+    public void setDynamicAttribute(String name, String value) {
+        ScriptDef definition = getScript();
+        if (!definition.isAttributeSupported(name)) {
+                throw new BuildException("<" + getTaskType() 
+                    + "> does not support the \"" + name + "\" attribute");
+        }
+        
+        attributes.put(name, value);
     }
 }
 

src/testcases/org/apache/tools/ant/taskdefs/optional/script/ScriptDefTest.java

@@ -141,4 +141,15 @@ public class ScriptDefTest extends BuildFileTest {
             "Should have detected duplicate attribute definition", 
             "attr1 attribute more than once");
     }
+
+    public void testProperty() {
+        executeTarget("property");
+        // get the fileset and its basedir
+        Project project = getProject();
+        String log = getLog();
+        assertTrue("Expecting property in attribute value replaced", 
+            log.indexOf("Attribute value = test") != -1);
+    }
+
+
 }