I would like to get some feedback on a change I am considering. With the
current Introspection nested element support, the task's method
corresponding to the nested element is called before the nested element is
configured. I would like to add a mechanism to support the calling of this
method after the element is configured. This will only work for addXXX
style methods since for those methods the object creation can occur without
involving the task.
I have implemented this (the diff is attached), by checking for a method
addConfiguredXXX, rather than simply addXXX. The diff includes changes to
the Jar task to allow a manifest to be specified in the build file directly
<jar jarfile="taskjar2.jar" basedir=".">
<include name="src/**/*"/>
<manifest>
<attribute name="created-by" value="me"/>
<section name="foobar">
<attribute name="Sealed" value="True"/>
</section>
</manifest>
</jar>
Now, my main concern is with the use of addConfigured. It is potentially a
compatibility issue if some custom task has a nested element
<configured...>. I guess that is pretty low risk. The alternative would be
to use a different prefix, such as putXXX() or configXXX(). That should be
backward compatible, although it may expose methods to the buildfile as
nested elements which were not intended.
So, what do you think? It makes some tasks easier to write if the element
is configured before it is added to the task. Certainly this was the case
for manifests. If you think this is worthwhile, what about the naming of
the methods eligible for this?
Let me know your thoughts?
Conor
? diffs.txt
? main/org/apache/tools/ant/AntSecurityManager.java.new
? script/antx.bat
Index: main/org/apache/tools/ant/IntrospectionHelper.java
===================================================================
RCS file:
/home/cvs/jakarta-ant/src/main/org/apache/tools/ant/IntrospectionHelper.java,v
retrieving revision 1.18
diff -u -w -r1.18 IntrospectionHelper.java
--- main/org/apache/tools/ant/IntrospectionHelper.java 2001/07/26 07:51:28
1.18
+++ main/org/apache/tools/ant/IntrospectionHelper.java 2001/07/31 15:04:40
@@ -91,6 +91,11 @@
private Hashtable nestedCreators;
/**
+ * Holds methods to store configured nested elements.
+ */
+ private Hashtable nestedStorers;
+
+ /**
* The method to add PCDATA stuff.
*/
private Method addText = null;
@@ -110,6 +115,8 @@
attributeSetters = new Hashtable();
nestedTypes = new Hashtable();
nestedCreators = new Hashtable();
+ nestedStorers = new Hashtable();
+
this.bean = bean;
Method[] methods = bean.getMethods();
@@ -177,6 +184,39 @@
});
+ } else if (name.startsWith("addConfigured")
+ && java.lang.Void.TYPE.equals(returnType)
+ && args.length == 1
+ && !java.lang.String.class.equals(args[0])
+ && !args[0].isArray()
+ && !args[0].isPrimitive()) {
+
+ try {
+ final Constructor c =
+ args[0].getConstructor(new Class[] {});
+ String propName = getPropertyName(name, "addConfigured");
+ nestedTypes.put(propName, args[0]);
+ nestedCreators.put(propName, new NestedCreator() {
+
+ public Object create(Object parent)
+ throws InvocationTargetException,
IllegalAccessException, InstantiationException {
+
+ Object o = c.newInstance(new Object[] {});
+ return o;
+ }
+
+ });
+ nestedStorers.put(propName, new NestedStorer() {
+
+ public void store(Object parent, Object child)
+ throws InvocationTargetException,
IllegalAccessException, InstantiationException {
+
+ m.invoke(parent, new Object[] {child});
+ }
+
+ });
+ } catch (NoSuchMethodException nse) {
+ }
} else if (name.startsWith("add")
&& java.lang.Void.TYPE.equals(returnType)
&& args.length == 1
@@ -202,7 +242,6 @@
});
} catch (NoSuchMethodException nse) {
}
-
}
}
}
@@ -300,6 +339,35 @@
}
/**
+ * Creates a named nested element.
+ */
+ public void storeElement(Project project, Object element, Object child,
String elementName)
+ throws BuildException {
+ if (elementName == null) {
+ return;
+ }
+ NestedStorer ns = (NestedStorer)nestedStorers.get(elementName);
+ if (ns == null) {
+ return;
+ }
+ try {
+ ns.store(element, child);
+ } catch (IllegalAccessException ie) {
+ // impossible as getMethods should only return public methods
+ throw new BuildException(ie);
+ } catch (InstantiationException ine) {
+ // impossible as getMethods should only return public methods
+ throw new BuildException(ine);
+ } catch (InvocationTargetException ite) {
+ Throwable t = ite.getTargetException();
+ if (t instanceof BuildException) {
+ throw (BuildException) t;
+ }
+ throw new BuildException(t);
+ }
+ }
+
+ /**
* returns the type of a named nested element.
*/
public Class getElementType(String elementName)
@@ -555,6 +623,12 @@
public Object create(Object parent)
throws InvocationTargetException, IllegalAccessException,
InstantiationException;
}
+
+ private interface NestedStorer {
+ public void store(Object parent, Object child)
+ throws InvocationTargetException, IllegalAccessException,
InstantiationException;
+ }
+
private interface AttributeSetter {
public void set(Project p, Object parent, String value)
throws InvocationTargetException, IllegalAccessException,
Index: main/org/apache/tools/ant/ProjectHelper.java
===================================================================
RCS file:
/home/cvs/jakarta-ant/src/main/org/apache/tools/ant/ProjectHelper.java,v
retrieving revision 1.57
diff -u -w -r1.57 ProjectHelper.java
--- main/org/apache/tools/ant/ProjectHelper.java 2001/07/23 16:31:56
1.57
+++ main/org/apache/tools/ant/ProjectHelper.java 2001/07/31 15:04:40
@@ -552,11 +552,12 @@
configureId(child, attrs);
if (parentWrapper != null) {
- childWrapper = new RuntimeConfigurable(child);
+ childWrapper = new RuntimeConfigurable(child, propType);
childWrapper.setAttributes(attrs);
parentWrapper.addChild(childWrapper);
} else {
configure(child, attrs, project);
+ ih.storeElement(project, parent, child,
propType.toLowerCase());
}
} catch (BuildException exc) {
throw new SAXParseException(exc.getMessage(), locator, exc);
@@ -612,7 +613,7 @@
}
if (target != null) {
- wrapper = new RuntimeConfigurable(element);
+ wrapper = new RuntimeConfigurable(element, propType);
wrapper.setAttributes(attrs);
target.addDataType(wrapper);
} else {
@@ -688,6 +689,13 @@
IntrospectionHelper.getHelper(target.getClass()).addText(project,
target, text);
}
+ /**
+ * Stores a configured child element into its parent object
+ */
+ public static void storeChild(Project project, Object parent, Object
child, String tag) {
+ IntrospectionHelper ih =
IntrospectionHelper.getHelper(parent.getClass());
+ ih.storeElement(project, parent, child, tag);
+ }
/**
* Replace ${} style constructions in the given value with the string
value of
Index: main/org/apache/tools/ant/RuntimeConfigurable.java
===================================================================
RCS file:
/home/cvs/jakarta-ant/src/main/org/apache/tools/ant/RuntimeConfigurable.java,v
retrieving revision 1.7
diff -u -w -r1.7 RuntimeConfigurable.java
--- main/org/apache/tools/ant/RuntimeConfigurable.java 2001/07/23 03:56:12
1.7
+++ main/org/apache/tools/ant/RuntimeConfigurable.java 2001/07/31 15:04:40
@@ -68,6 +68,7 @@
*/
public class RuntimeConfigurable {
+ private String elementTag = null;
private Vector children = new Vector();
private Object wrappedObject = null;
private AttributeList attributes;
@@ -76,8 +77,9 @@
/**
* @param proxy The element to wrap.
*/
- public RuntimeConfigurable(Object proxy) {
+ public RuntimeConfigurable(Object proxy, String elementTag) {
wrappedObject = proxy;
+ this.elementTag = elementTag;
}
void setProxy(Object proxy) {
@@ -126,6 +128,11 @@
addText(new String(buf, start, end));
}
+ public String getElementTag() {
+ return elementTag;
+ }
+
+
/**
* Configure the wrapped element and all children.
*/
@@ -145,6 +152,7 @@
while (enum.hasMoreElements()) {
RuntimeConfigurable child = (RuntimeConfigurable)
enum.nextElement();
child.maybeConfigure(p);
+ ProjectHelper.storeChild(p, wrappedObject, child.wrappedObject,
child.getElementTag().toLowerCase());
}
if (id != null) {
Index: main/org/apache/tools/ant/Task.java
===================================================================
RCS file: /home/cvs/jakarta-ant/src/main/org/apache/tools/ant/Task.java,v
retrieving revision 1.19
diff -u -w -r1.19 Task.java
--- main/org/apache/tools/ant/Task.java 2001/07/22 13:12:28 1.19
+++ main/org/apache/tools/ant/Task.java 2001/07/31 15:04:40
@@ -203,7 +203,7 @@
*/
public RuntimeConfigurable getRuntimeConfigurableWrapper() {
if (wrapper == null) {
- wrapper = new RuntimeConfigurable(this);
+ wrapper = new RuntimeConfigurable(this, getTaskName());
}
return wrapper;
}
Index: main/org/apache/tools/ant/taskdefs/Jar.java
===================================================================
RCS file:
/home/cvs/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/Jar.java,v
retrieving revision 1.20
diff -u -w -r1.20 Jar.java
--- main/org/apache/tools/ant/taskdefs/Jar.java 2001/07/30 11:15:17 1.20
+++ main/org/apache/tools/ant/taskdefs/Jar.java 2001/07/31 15:04:45
@@ -83,6 +83,17 @@
super.setZipfile(jarFile);
}
+ public void addConfiguredManifest(Manifest newManifest) throws
ManifestException {
+ // we have to play a little trick here. The manifestFile in this
instance
+ // becomes the build file containing this manifest definition. If that
buildfile
+ // is more recent than the jar, the jar is rebuilt.
+ manifestFile = new File(getProject().getProperty("ant.file"));
+ if (manifest == null) {
+ manifest = getDefaultManifest();
+ }
+ manifest.merge(newManifest);
+ }
+
public void setManifest(File manifestFile) {
if (!manifestFile.exists()) {
throw new BuildException("Manifest file: " + manifestFile + " does
not exist.",
Index: main/org/apache/tools/ant/taskdefs/Manifest.java
===================================================================
RCS file:
/home/cvs/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/Manifest.java,v
retrieving revision 1.3
diff -u -w -r1.3 Manifest.java
--- main/org/apache/tools/ant/taskdefs/Manifest.java 2001/07/30 11:15:17
1.3
+++ main/org/apache/tools/ant/taskdefs/Manifest.java 2001/07/31 15:04:45
@@ -57,6 +57,8 @@
import java.util.*;
import java.io.*;
+import org.apache.tools.ant.BuildException;
+
/**
* Class to manage Manifest information
*
@@ -84,7 +86,7 @@
/**
* Class to hold manifest attributes
*/
- private class Attribute {
+ static public class Attribute {
/** The attribute's name */
private String name = null;
@@ -214,7 +216,9 @@
* Manifest. A section consists of a set of attribute values,
* separated from other sections by a blank line.
*/
- private class Section {
+ static public class Section {
+ private Vector warnings = new Vector();
+
/** The section's name if any. The main section in a manifest is
unnamed.*/
private String name = null;
@@ -266,7 +270,7 @@
}
else {
attribute = new Attribute(line);
- String nameReadAhead = addAttribute(attribute);
+ String nameReadAhead = addAttributeAndCheck(attribute);
if (nameReadAhead != null) {
return nameReadAhead;
}
@@ -292,6 +296,11 @@
// the merge file always wins
attributes.put(attributeName,
section.attributes.get(attributeName));
}
+
+ // add in the warnings
+ for (Enumeration e = section.warnings.elements();
e.hasMoreElements();) {
+ warnings.addElement(e.nextElement());
+ }
}
/**
@@ -338,6 +347,13 @@
attributes.remove(attributeName.toLowerCase());
}
+ public void addConfiguredAttribute(Attribute attribute) throws
ManifestException {
+ String check = addAttributeAndCheck(attribute);
+ if (check != null) {
+ throw new BuildException("Use the name attribute of the
section rather than using the Name attribute");
+ }
+ }
+
/**
* Add an attribute to the section
*
@@ -347,7 +363,10 @@
*
* @throws ManifestException if the attribute already exists in this
section.
*/
- public String addAttribute(Attribute attribute) throws
ManifestException {
+ public String addAttributeAndCheck(Attribute attribute) throws
ManifestException {
+ if (attribute.getName() == null || attribute.getValue() == null) {
+ throw new BuildException("Attributes must have name and
value");
+ }
if (attribute.getName().equalsIgnoreCase(ATTRIBUTE_NAME)) {
warnings.addElement("\"" + ATTRIBUTE_NAME + "\" attributes
should not occur in the " +
"main section and must be the first
element in all " +
@@ -368,8 +387,13 @@
}
return null;
}
+
+ public Enumeration getWarnings() {
+ return warnings.elements();
+ }
}
+
/** The version of this manifest */
private String manifestVersion = DEFAULT_MANIFEST_VERSION;
@@ -379,9 +403,6 @@
/** The named sections of this manifest */
private Hashtable sections = new Hashtable();
- /** Warnings for this manifest file */
- private Vector warnings = new Vector();
-
/** Construct an empty manifest */
public Manifest() {
}
@@ -428,13 +449,24 @@
// this line is the first attribute. set it and then let the
normal
// read handle the rest
Attribute firstAttribute = new Attribute(line);
- section.addAttribute(firstAttribute);
+ section.addAttributeAndCheck(firstAttribute);
}
section.setName(nextSectionName);
nextSectionName = section.read(reader);
+ addConfiguredSection(section);
+ }
+ }
+
+ public void addConfiguredSection(Section section) throws ManifestException
{
+ if (section.getName() == null) {
+ throw new BuildException("Sections must have a name");
+ }
sections.put(section.getName().toLowerCase(), section);
}
+
+ public void addConfiguredAttribute(Attribute attribute) throws
ManifestException {
+ mainSection.addConfiguredAttribute(attribute);
}
/**
@@ -460,10 +492,6 @@
}
}
- // add in the warnings
- for (Enumeration e = other.warnings.elements(); e.hasMoreElements();) {
- warnings.addElement(e.nextElement());
- }
}
/**
@@ -483,7 +511,7 @@
mainSection.write(writer);
if (signatureVersion != null) {
try {
- mainSection.addAttribute(new
Attribute(ATTRIBUTE_SIGNATURE_VERSION, signatureVersion));
+ mainSection.addConfiguredAttribute(new
Attribute(ATTRIBUTE_SIGNATURE_VERSION, signatureVersion));
}
catch (ManifestException e) {
// shouldn't happen - ignore
@@ -518,6 +546,20 @@
* @return an enumeration of warning strings
*/
public Enumeration getWarnings() {
+ Vector warnings = new Vector();
+
+ for (Enumeration e2 = mainSection.getWarnings();
e2.hasMoreElements();) {
+ warnings.addElement(e2.nextElement());
+ }
+
+ // create a vector and add in the warnings for all the sections
+ for (Enumeration e = sections.elements(); e.hasMoreElements();) {
+ Section section = (Section)e.nextElement();
+ for (Enumeration e2 = section.getWarnings();
e2.hasMoreElements();) {
+ warnings.addElement(e2.nextElement());
+ }
+ }
+
return warnings.elements();
}
}
