skitching 2004/11/29 20:40:54
Modified: digester/src/java/org/apache/commons/digester
SetNestedPropertiesRule.java
Log:
Fix for bugzilla #31393, reported by James Pine. SetNestedPropertiesRule
failed when executed re-entrantly; removed over-zealous optimisation that
made it fail in that case (though it now runs slower in the usual case).
Revision Changes Path
1.9 +45 -17
jakarta-commons/digester/src/java/org/apache/commons/digester/SetNestedPropertiesRule.java
Index: SetNestedPropertiesRule.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/digester/src/java/org/apache/commons/digester/SetNestedPropertiesRule.java,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- SetNestedPropertiesRule.java 10 May 2004 06:52:50 -0000 1.8
+++ SetNestedPropertiesRule.java 30 Nov 2004 04:40:54 -0000 1.9
@@ -81,6 +81,11 @@
* underlying Rules implementation for the same pattern, so other rules
* are not "disabled" during processing of a SetNestedPropertiesRule.</p>
*
+ * <p>TODO: Optimise this class. Currently, each time begin is called,
+ * new AnyChildRules and AnyChildRule objects are created. It should be
+ * possible to cache these in normal use (though watch out for when a rule
+ * instance is invoked re-entrantly!).</p>
+ *
* @since 1.6
*/
@@ -94,10 +99,6 @@
private Log log = null;
- private AnyChildRule anyChildRule = new AnyChildRule();
- private AnyChildRules newRules = new AnyChildRules(anyChildRule);
- private Rules oldRules = null;
-
private boolean trimData = true;
private boolean allowUnknownChildElements = false;
@@ -185,7 +186,6 @@
public void setDigester(Digester digester) {
super.setDigester(digester);
log = digester.getLogger();
- anyChildRule.setDigester(digester);
}
/**
@@ -203,8 +203,16 @@
}
/**
- * When set to true, any child element for which there is no
+ * Determines whether an error is reported when a nested element is
+ * encountered for which there is no corresponding property-setter
+ * method.
+ * <p>
+ * When set to false, any child element for which there is no
* corresponding object property will cause an error to be reported.
+ * <p>
+ * When set to false, any child element for which there is no
+ * corresponding object property will simply be ignored.
+ * <p>
* The default value of this attribute is false (not allowed).
*/
public void setAllowUnknownChildElements(boolean
allowUnknownChildElements) {
@@ -225,7 +233,10 @@
*/
public void begin(String namespace, String name, Attributes attributes)
throws Exception {
- oldRules = digester.getRules();
+ Rules oldRules = digester.getRules();
+ AnyChildRule anyChildRule = new AnyChildRule();
+ anyChildRule.setDigester(digester);
+ AnyChildRules newRules = new AnyChildRules(anyChildRule);
newRules.init(digester.getMatch()+"/", oldRules);
digester.setRules(newRules);
}
@@ -236,7 +247,8 @@
* child-element-matching.
*/
public void body(String bodyText) throws Exception {
- digester.setRules(oldRules);
+ AnyChildRules newRules = (AnyChildRules) digester.getRules();
+ digester.setRules(newRules.getOldRules());
}
/**
@@ -293,21 +305,25 @@
(matchPath.indexOf('/', matchPrefix.length()) == -1)) {
// The current element is a direct child of the element
- // specified in the init method, so include it as the
- // first rule in the matches list. The way that
- // SetNestedPropertiesRule is used, it is in fact very
- // likely to be the only match, so we optimise that
- // solution by keeping a list with only the AnyChildRule
- // instance in it.
+ // specified in the init method, so we want to ensure that
+ // the rule passed to this object's constructor is included
+ // in the returned list of matching rules.
if ((match == null || match.size()==0)) {
+ // The "real" rules class doesn't have any matches for
+ // the specified path, so we return a list containing
+ // just one rule: the one passed to this object's
+ // constructor.
return rules;
}
else {
- // it might not be safe to modify the returned list,
+ // The "real" rules class has rules that match the
current
+ // node, so we return this list *plus* the rule passed to
+ // this object's constructor.
+ //
+ // It might not be safe to modify the returned list,
// so clone it first.
LinkedList newMatch = new LinkedList(match);
- //newMatch.addFirst(rule);
newMatch.addLast(rule);
return newMatch;
}
@@ -327,6 +343,10 @@
matchPrefix = prefix;
decoratedRules = rules;
}
+
+ public Rules getOldRules() {
+ return decoratedRules;
+ }
}
private class AnyChildRule extends Rule {
@@ -395,7 +415,15 @@
}
}
+ try
+ {
BeanUtils.setProperty(top, propName, value);
+ }
+ catch(NullPointerException e) {
+ digester.log.error("NullPointerException: "
+ + "top=" + top + ",propName=" + propName + ",value=" +
value + "!");
+ throw e;
+ }
}
public void end(String namespace, String name) throws Exception {
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
