skitching 2004/03/22 23:11:00 Modified: digester/src/java/org/apache/commons/digester/plugins PluginCreateRule.java PluginRules.java Log: Significant changes to simplify design. There is now effectively a stack of Rules objects; entering the scope of a plugin pushes a new PluginRules instance onto that stack and leaving the scope of the plugin pops it from the stack. Revision Changes Path 1.14 +192 -201 jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginCreateRule.java Index: PluginCreateRule.java =================================================================== RCS file: /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginCreateRule.java,v retrieving revision 1.13 retrieving revision 1.14 diff -u -r1.13 -r1.14 --- PluginCreateRule.java 29 Feb 2004 02:22:15 -0000 1.13 +++ PluginCreateRule.java 23 Mar 2004 07:11:00 -0000 1.14 @@ -83,13 +83,6 @@ */ private PluginConfigurationException initException; - /** - * Our private set of rules associated with the concrete class that - * the user requested to be instantiated. This object is only valid - * between a call to begin() and the corresponding call to end(). - */ - private PluginRules localRules; - //-------------------- static methods ----------------------------------- /** @@ -350,13 +343,6 @@ * the plugin class are then loaded into that new Rules object. * Finally, any custom rules that are associated with the current pattern * (such as SetPropertiesRules) have their begin methods executed. - * <p> - * Because a PluginCreateRule is also a Delegate, this method is also - * called on the start of any element occurring below the pattern - * associated with this rule. In this case, this method acts like the - * Digester's startElement method: it fires the begin() method of every - * custom rule associated with the plugin class that matches that pattern. - * See [EMAIL PROTECTED] #delegateBegin}. * * @param namespace * @param name @@ -382,151 +368,134 @@ throw initException; } - String currMatch = digester.getMatch(); - if (currMatch.length() == pattern.length()) { - // ok here we are actually instantiating a new plugin object, - // and storing its rules into a new Rules object - if (localRules != null) { - throw new PluginAssertionFailure( - "Begin called when localRules is not null."); - } - - PluginRules oldRules = (PluginRules) digester.getRules(); - localRules = new PluginRules(this, oldRules); - PluginManager pluginManager = localRules.getPluginManager(); - Declaration currDeclaration = null; + String path = digester.getMatch(); + + // create a new Rules object and effectively push it onto a stack of + // rules objects. The stack is actually a linked list; using the + // PluginRules constructor below causes the new instance to link + // to the previous head-of-stack, then the Digester.setRules() makes + // the new instance the new head-of-stack. + PluginRules oldRules = (PluginRules) digester.getRules(); + PluginRules newRules = new PluginRules(path, oldRules); + digester.setRules(newRules); + + // load any custom rules associated with the plugin + PluginManager pluginManager = newRules.getPluginManager(); + Declaration currDeclaration = null; - if (debug) { - log.debug("PluginCreateRule.begin: installing new plugin: " + - "oldrules=" + oldRules.toString() + - ", localrules=" + localRules.toString()); - } + if (debug) { + log.debug("PluginCreateRule.begin: installing new plugin: " + + "oldrules=" + oldRules.toString() + + ", newrules=" + newRules.toString()); + } - String pluginClassName; - if (pluginClassAttrNs == null) { - // Yep, this is ugly. - // - // In a namespace-aware parser, the one-param version will - // return attributes with no namespace. - // - // In a non-namespace-aware parser, the two-param version will - // never return any attributes, ever. - pluginClassName = attributes.getValue(pluginClassAttr); - } else { - pluginClassName = - attributes.getValue(pluginClassAttrNs, pluginClassAttr); - } + String pluginClassName; + if (pluginClassAttrNs == null) { + // Yep, this is ugly. + // + // In a namespace-aware parser, the one-param version will + // return attributes with no namespace. + // + // In a non-namespace-aware parser, the two-param version will + // never return any attributes, ever. + pluginClassName = attributes.getValue(pluginClassAttr); + } else { + pluginClassName = + attributes.getValue(pluginClassAttrNs, pluginClassAttr); + } - String pluginId; - if (pluginIdAttrNs == null) { - pluginId = attributes.getValue(pluginIdAttr); - } - else { - pluginId = - attributes.getValue(pluginIdAttrNs, pluginIdAttr); - } - - if (pluginClassName != null) { - currDeclaration = pluginManager.getDeclarationByClass( - pluginClassName); - - if (currDeclaration == null) { - currDeclaration = new Declaration(pluginClassName); - try { - currDeclaration.init(digester); - } catch(PluginWrappedException pwe) { - throw new PluginInvalidInputException( - pwe.getMessage(), pwe.getCause()); - } - pluginManager.addDeclaration(currDeclaration); - } - } else if (pluginId != null) { - currDeclaration = pluginManager.getDeclarationById(pluginId); - - if (currDeclaration == null) { + String pluginId; + if (pluginIdAttrNs == null) { + pluginId = attributes.getValue(pluginIdAttr); + } + else { + pluginId = + attributes.getValue(pluginIdAttrNs, pluginIdAttr); + } + + if (pluginClassName != null) { + currDeclaration = pluginManager.getDeclarationByClass( + pluginClassName); + + if (currDeclaration == null) { + currDeclaration = new Declaration(pluginClassName); + try { + currDeclaration.init(digester); + } catch(PluginWrappedException pwe) { throw new PluginInvalidInputException( - "Plugin id [" + pluginId + "] is not defined."); + pwe.getMessage(), pwe.getCause()); } - } else if (defaultPlugin != null) { - currDeclaration = defaultPlugin; + pluginManager.addDeclaration(currDeclaration); } - else { + } else if (pluginId != null) { + currDeclaration = pluginManager.getDeclarationById(pluginId); + + if (currDeclaration == null) { throw new PluginInvalidInputException( - "No plugin class specified for element " + - pattern); - } - - // now load up the custom rules into a private Rules instance - digester.setRules(localRules); - - currDeclaration.configure(digester, pattern); - - Class pluginClass = currDeclaration.getPluginClass(); - - Object instance = pluginClass.newInstance(); - getDigester().push(instance); - if (debug) { - log.debug( - "PluginCreateRule.begin" + ": pattern=[" + pattern + "]" + - " match=[" + digester.getMatch() + "]" + - " pushed instance of plugin [" + pluginClass.getName() + "]"); + "Plugin id [" + pluginId + "] is not defined."); } - - digester.setRules(oldRules); - - ((PluginRules) oldRules).beginPlugin(this); + } else if (defaultPlugin != null) { + currDeclaration = defaultPlugin; } - - // fire the begin method of all custom rules - Rules oldRules = digester.getRules(); - - if (debug) { - log.debug("PluginCreateRule.begin: firing nested rules: " + - "oldrules=" + oldRules.toString() + - ", localrules=" + localRules.toString()); + else { + throw new PluginInvalidInputException( + "No plugin class specified for element " + + pattern); } + + // now load up the custom rules + currDeclaration.configure(digester, pattern); - // assert oldRules = localRules.oldRules - digester.setRules(localRules); - delegateBegin(namespace, name, attributes); - digester.setRules(oldRules); - + // and now create an instance of the plugin class + Class pluginClass = currDeclaration.getPluginClass(); + + Object instance = pluginClass.newInstance(); + getDigester().push(instance); if (debug) { - log.debug("PluginCreateRule.begin: restored old rules to " + - "oldrules=" + oldRules.toString()); + log.debug( + "PluginCreateRule.begin" + ": pattern=[" + pattern + "]" + + " match=[" + digester.getMatch() + "]" + + " pushed instance of plugin [" + pluginClass.getName() + "]"); } + + // and now we have to fire any custom rules which would have + // been matched by the same path that matched this rule, had + // they been loaded at that time. + List rules = newRules.getDecoratedRules().match(namespace, path); + fireBeginMethods(rules, namespace, name, attributes); } /** - * Invoked by the digester when the closing tag matching this Rule's - * pattern is encountered. See [EMAIL PROTECTED] #delegateBody}. + * Process the body text of this element. * - * @see #begin + * @param text The body text of this element */ public void body(String namespace, String name, String text) - throws Exception { - - Rules oldRules = digester.getRules(); - // assert oldRules == localRules.oldRules - digester.setRules(localRules); - delegateBody(namespace, name, text); - digester.setRules(oldRules); + throws Exception { + + // While this class itself has no work to do in the body method, + // we do need to fire the body methods of all dynamically-added + // rules matching the same path as this rule. During begin, we had + // to manually execute the dynamic rules' begin methods because they + // didn't exist in the digester's Rules object when the match begin. + // So in order to ensure consistent ordering of rule execution, the + // PluginRules class deliberately avoids returning any such rules + // in later calls to the match method, instead relying on this + // object to execute them at the appropriate time. + // + // Note that this applies only to rules matching exactly the path + // which is also matched by this PluginCreateRule. + + String path = digester.getMatch(); + PluginRules newRules = (PluginRules) digester.getRules(); + List rules = newRules.getDecoratedRules().match(namespace, path); + fireBodyMethods(rules, namespace, name, text); } - + /** * Invoked by the digester when the closing tag matching this Rule's * pattern is encountered. * </p> - * As noted on method begin, because PluginCreateRule is a Delegate, - * this method is also called at the end tag of every pattern that - * is "below" the pattern associated with this rule. In this case, we - * fire the end method of every custom rule associated with the - * current plugin class. See [EMAIL PROTECTED] #delegateEnd}. - * <p> - * If we are really encountering the end tag associated with this rule - * (rather than the end of an element "below" that tag), then we - * remove the object we pushed onto the digester stack when the - * opening tag was encountered. * * @param namespace Description of the Parameter * @param name Description of the Parameter @@ -536,21 +505,21 @@ */ public void end(String namespace, String name) throws Exception { - - Rules oldRules = digester.getRules(); - // assert oldRules == localRules.parentRules - digester.setRules(localRules); - delegateEnd(namespace, name); - digester.setRules(oldRules); - - String currMatch = digester.getMatch(); - if (currMatch.length() == pattern.length()) { - // the end of the element on which the PluginCreateRule has - // been mounted has been reached. - localRules = null; - ((PluginRules) oldRules).endPlugin(this); - digester.pop(); - } + + + // see body method for more info + String path = digester.getMatch(); + PluginRules newRules = (PluginRules) digester.getRules(); + List rules = newRules.getDecoratedRules().match(namespace, path); + fireEndMethods(rules, namespace, name); + + // pop the stack of PluginRules instances, which + // discards all custom rules associated with this plugin + digester.setRules(newRules.getParent()); + + // and get rid of the instance of the plugin class from the + // digester object stack. + digester.pop(); } /** @@ -570,70 +539,92 @@ } /** - * Here we act like Digester.begin, finding a match for the pattern - * in our private rules object, then executing the begin method of - * each matching rule. - */ - public void delegateBegin(String namespace, String name, - org.xml.sax.Attributes attributes) - throws java.lang.Exception { + * Duplicate the processing that the Digester does when firing the + * begin methods of rules. It would be really nice if the Digester + * class provided a way for this functionality to just be invoked + * directly. + */ + public void fireBeginMethods(List rules, + String namespace, String name, + org.xml.sax.Attributes list) + throws java.lang.Exception { - // Fire "begin" events for all relevant rules - Log log = digester.getLogger(); - boolean debug = log.isDebugEnabled(); - String match = digester.getMatch(); - List rules = digester.getRules().match(namespace, match); - Iterator ri = rules.iterator(); - while (ri.hasNext()) { - Rule rule = (Rule) ri.next(); - if (debug) { - log.debug(" Fire begin() for " + rule); + if ((rules != null) && (rules.size() > 0)) { + Log log = digester.getLogger(); + boolean debug = log.isDebugEnabled(); + for (int i = 0; i < rules.size(); i++) { + try { + Rule rule = (Rule) rules.get(i); + if (debug) { + log.debug(" Fire begin() for " + rule); + } + rule.begin(namespace, name, list); + } catch (Exception e) { + throw digester.createSAXException(e); + } catch (Error e) { + throw e; + } } - rule.begin(namespace, name, attributes); } } - + /** - * Here we act like Digester.body, except against our private rules. - */ - public void delegateBody(String namespace, String name, String text) - throws Exception { - // Fire "body" events for all relevant rules - Log log = digester.getLogger(); - boolean debug = log.isDebugEnabled(); - String match = digester.getMatch(); - List rules = digester.getRules().match(namespace, match); - Iterator ri = rules.iterator(); - while (ri.hasNext()) { - Rule rule = (Rule) ri.next(); - if (debug) { - log.debug(" Fire body() for " + rule); + * Duplicate the processing that the Digester does when firing the + * body methods of rules. It would be really nice if the Digester + * class provided a way for this functionality to just be invoked + * directly. + */ + private void fireBodyMethods(List rules, + String namespaceURI, String name, + String text) throws Exception { + + if ((rules != null) && (rules.size() > 0)) { + Log log = digester.getLogger(); + boolean debug = log.isDebugEnabled(); + for (int i = 0; i < rules.size(); i++) { + try { + Rule rule = (Rule) rules.get(i); + if (debug) { + log.debug(" Fire body() for " + rule); + } + rule.body(namespaceURI, name, text); + } catch (Exception e) { + throw digester.createSAXException(e); + } catch (Error e) { + throw e; + } } - rule.body(namespace, name, text); } } /** - * Here we act like Digester.end. + * Duplicate the processing that the Digester does when firing the + * end methods of rules. It would be really nice if the Digester + * class provided a way for this functionality to just be invoked + * directly. */ - public void delegateEnd(String namespace, String name) - throws Exception { + public void fireEndMethods(List rules, + String namespaceURI, String name) + throws Exception { + // Fire "end" events for all relevant rules in reverse order - Log log = digester.getLogger(); - boolean debug = log.isDebugEnabled(); - String match = digester.getMatch(); - List rules = digester.getRules().match(namespace, match); - ListIterator ri = rules.listIterator(); - while (ri.hasNext()) { - ri.next(); - } - - while (ri.hasPrevious()) { - Rule rule = (Rule) ri.previous(); - if (debug) { - log.debug(" Fire end() for " + rule); + if (rules != null) { + Log log = digester.getLogger(); + boolean debug = log.isDebugEnabled(); + for (int i = 0; i < rules.size(); i++) { + int j = (rules.size() - i) - 1; + try { + Rule rule = (Rule) rules.get(j); + if (debug) { + log.debug(" Fire end() for " + rule); + } + rule.end(namespaceURI, name); + } catch (Exception e) { + throw digester.createSAXException(e); + } catch (Error e) { + throw e; + } } - rule.end(namespace, name); } } } 1.12 +96 -103 jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginRules.java Index: PluginRules.java =================================================================== RCS file: /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginRules.java,v retrieving revision 1.11 retrieving revision 1.12 diff -u -r1.11 -r1.12 --- PluginRules.java 29 Feb 2004 02:22:15 -0000 1.11 +++ PluginRules.java 23 Mar 2004 07:11:00 -0000 1.12 @@ -34,34 +34,53 @@ /** * A custom digester Rules manager which must be used as the Rules object * when using the plugins module functionality. + * <p> + * During parsing, a linked list of PluginCreateRule instances develop, and + * this list also acts like a stack. The original instance that was set before + * the Digester started parsing is always at the tail of the list, and the + * Digester always holds a reference to the instance at the head of the list + * in the rules member. Initially, this list/stack holds just one instance, + * ie head and tail are the same object. + * <p> + * When the start of an xml element causes a PluginCreateRule to fire, a new + * PluginRules instance is created and inserted at the head of the list (ie + * pushed onto the stack of Rules objects). Digester.getRules() therefore + * returns this new Rules object, and any custom rules associated with that + * plugin are added to that instance. + * <p> + * When the end of the xml element is encountered (and therefore the + * PluginCreateRule end method fires), the stack of Rules objects is popped, + * so that Digester.getRules returns the previous Rules object. */ public class PluginRules implements Rules { - /** - * The rules implementation that we are "enhancing" with plugins - * functionality, as per the Decorator pattern. - */ - private Rules decoratedRules; - /** * The Digester instance with which this Rules instance is associated. */ protected Digester digester = null; - /** - * The currently active PluginCreateRule. When the begin method of a - * PluginCreateRule is encountered, this is set. When the end method is - * encountered, this is cleared. Any attempt to call match() while this - * attribute is set just causes this single rule to be returned. + /** + * The rules implementation that we are "enhancing" with plugins + * functionality, as per the Decorator pattern. */ - private PluginCreateRule currPluginCreateRule = null; + private Rules decoratedRules; /** Object which contains information about all known plugins. */ private PluginManager pluginManager; - /** The parent rules object for this object. */ - private Rules parent; + /** + * The path below which this rules object has responsibility. + * For paths shorter than or equal the mountpoint, the parent's + * match is called. + */ + private String mountPoint = null; + + /** + * The Rules object that holds rules applying "above" the mountpoint, + * ie the next Rules object down in the stack. + */ + private PluginRules parent = null; // ------------------------------------------------------------- Constructor @@ -86,22 +105,22 @@ /** * Constructs a Rules instance which has a parent Rules object - * (not a delegate rules object). One of these is created - * each time a PluginCreateRule's begin method fires, in order to - * manage the custom rules associated with whatever concrete plugin - * class the user has specified. + * (which is different from having a delegate rules object). * <p> - * The first parameter is not actually used; it is required solely - * because a constructor with a single Rules parameter already - * exists. - * <p> - * The parent is recorded so that lookups of Declarations can - * "inherit" declarations from further up the tree. - */ - PluginRules(PluginCreateRule pcr, PluginRules parent) { + * One of these is created each time a PluginCreateRule's begin method + * fires, in order to manage the custom rules associated with whatever + * concrete plugin class the user has specified. + */ + PluginRules(String mountPoint, PluginRules parent) { + // no need to set digester or decoratedRules.digester, + // because when Digester.setRules is called, the setDigester + // method on this object will be called. + decoratedRules = new RulesBase(); - this.parent = parent; pluginManager = new PluginManager(parent.pluginManager); + + this.mountPoint = mountPoint; + this.parent = parent; } // ------------------------------------------------------------- Properties @@ -162,6 +181,15 @@ // --------------------------------------------------------- Public Methods /** + * This package-scope method is used by the PluginCreateRule class to + * get direct access to the rules that were dynamically added by the + * plugin. No other class should need access to this object. + */ + Rules getDecoratedRules() { + return decoratedRules; + } + + /** * Return the list of rules registered with this object, in the order * they were registered with this object. * <p> @@ -196,6 +224,27 @@ { pattern = pattern.substring(1); } + + if (mountPoint != null) { + if (!pattern.equals(mountPoint) + && !pattern.startsWith(mountPoint + "/")) { + // This can only occur if a plugin attempts to add a + // rule with a pattern that doesn't start with the + // prefix passed to the addRules method. Plugins mustn't + // add rules outside the scope of the tag they were specified + // on, so refuse this. + + // alas, can't throw exception + log.warn( + "An attempt was made to add a rule with a pattern that" + + "is not at or below the mountpoint of the current" + + " PluginRules object." + + " Rule pattern: " + pattern + + ", mountpoint: " + mountPoint + + ", rule type: " + rule.getClass().getName()); + return; + } + } decoratedRules.add(pattern, rule); @@ -236,111 +285,55 @@ * in the order originally registered through the <code>add()</code> * method. * - * @param pattern Nesting pattern to be matched + * @param path the path to the xml nodes to be matched. * * @deprecated Call match(namespaceURI,pattern) instead. */ - public List match(String pattern) { - return (match(null, pattern)); + public List match(String path) { + return (match(null, path)); } /** * Return a List of all registered Rule instances that match the specified - * nesting pattern, or a zero-length List if there are no matches. If more + * nodepath, or a zero-length List if there are no matches. If more * than one Rule instance matches, they <strong>must</strong> be returned * in the order originally registered through the <code>add()</code> * method. * <p> - * If we have encountered the start of a PluginCreateRule and have not - * yet encountered the end tag, then the currPluginCreateRule attribute - * will be non-null. In this case, we just return this rule object as the - * sole match. The calling Digester will then invoke the begin/body/end - * methods on this rule, which are responsible for invoking all rules - * matching nodes below itself. - * * @param namespaceURI Namespace URI for which to select matching rules, * or <code>null</code> to match regardless of namespace URI - * @param pattern Nesting pattern to be matched + * @param path the path to the xml nodes to be matched. */ - public List match(String namespaceURI, String pattern) { + public List match(String namespaceURI, String path) { Log log = LogUtils.getLogger(digester); boolean debug = log.isDebugEnabled(); if (debug) { log.debug( - "Matching pattern [" + pattern + + "Matching path [" + path + "] on rules object " + this.toString()); } List matches; - if ((currPluginCreateRule != null) && - (pattern.length() > currPluginCreateRule.getPattern().length())) { - // assert pattern.startsWith(currPluginCreateRule.getPattern()) + if ((mountPoint != null) && + (path.length() <= mountPoint.length())) { if (debug) { log.debug( - "Pattern [" + pattern + "] matching PluginCreateRule " + - currPluginCreateRule.toString()); + "Path [" + path + "] delegated to parent."); } - matches = new ArrayList(1); - matches.add(currPluginCreateRule); - } + + matches = parent.match(namespaceURI, path); + + // Note that in the case where path equals mountPoint, + // we deliberately return only the rules from the parent, + // even though this object may hold some rules matching + // this same path. See PluginCreateRule's begin, body and end + // methods for the reason. + } else { - matches = decoratedRules.match(namespaceURI, pattern); + matches = decoratedRules.match(namespaceURI, path); } return matches; - } - - /** - * Called when a pattern matches a PluginCreateRule, to indicate that - * any attempt to match any following XML elements should simply - * return a single match: this PluginCreateRule. - * <p> - * In other words, once the plugin element starts, all following - * subelements cause the rule object to be "matched", until the - * endPlugin method is called. - */ - public void beginPlugin(PluginCreateRule pcr) { - Log log = LogUtils.getLogger(digester); - boolean debug = log.isDebugEnabled(); - - if (currPluginCreateRule != null) { - throw new PluginAssertionFailure( - "endPlugin called when currPluginCreateRule is not null."); - } - - if (debug) { - log.debug( - "Entering PluginCreateRule " + pcr.toString() + - " on rules object " + this.toString()); - } - - currPluginCreateRule = pcr; - } - - /** - * Called when a pattern matches the end of a PluginCreateRule. - * See [EMAIL PROTECTED] #beginPlugin}. - */ - public void endPlugin(PluginCreateRule pcr) { - Log log = LogUtils.getLogger(digester); - boolean debug = log.isDebugEnabled(); - - if (currPluginCreateRule == null) { - throw new PluginAssertionFailure( - "endPlugin called when currPluginCreateRule is null."); - } - - if (currPluginCreateRule != pcr) { - throw new PluginAssertionFailure( - "endPlugin called with unexpected PluginCreateRule instance."); - } - - currPluginCreateRule = null; - if (debug) { - log.debug( - "Leaving PluginCreateRule " + pcr.toString() + - " on rules object " + this.toString()); - } } }
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]