Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Date: 30 Nov 2004 04:40:54 -0000
Message-ID: <20041130044054.93223.qmail@minotaur.apache.org>
From: skitching@apache.org
To: jakarta-commons-cvs@apache.org
Subject: cvs commit:
 jakarta-commons/digester/src/java/org/apache/commons/digester
 SetNestedPropertiesRule.java

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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org