You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Craig McClanahan <cr...@apache.org> on 2004/04/01 08:37:55 UTC

Re: [digester] patch to keep stack of matched rules [also: Mixed Content support]

Simon Kitching wrote:

>Hi,
>
>Here's a simple patch that I think would make digester more efficient,
>by avoiding unnecessary calls to rules.getMatch().
>
>It also happens to make it easier to implement my proposed solution for
>the "mixed content" issue currently being discussed.
>
>The Digester with this patch applied passes all existing unit tests, of
>course.
>
>Any comments/opinions?
>
>  
>
I like the idea in general, but would ask one favor ... could you try 
the modified code with Tomcat and/or Struts as well?  I don't think our 
unit tests cover some of the interdependencies -- not that this patch 
looks like it would break anything, but I just like to make sure we're 
not messing up two primary customer projects.

I also promise to look at the mixed content stuff when I can ... 
unfortunately, that is looking like no earlier than Saturday :-(.

>Regards,
>
>Simon
>  
>
>  
>
Craig

>------------------------------------------------------------------------
>
>Index: Digester.java
>===================================================================
>RCS file: /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/digester/Digester.java,v
>retrieving revision 1.98
>diff -u -r1.98 Digester.java
>--- Digester.java	29 Mar 2004 20:34:54 -0000	1.98
>+++ Digester.java	1 Apr 2004 03:45:32 -0000
>@@ -141,6 +141,16 @@
> 
> 
>     /**
>+     * Stack whose elements are List objects, each containing a list of
>+     * Rule objects as returned from Rules.getMatch(). As each xml element
>+     * in the input is entered, the matching rules are pushed onto this
>+     * stack. After the end tag is reached, the matches are popped again.
>+     * The depth of is stack is therefore exactly the same as the current
>+     * "nesting" level of the input xml. 
>+     */
>+    protected ArrayStack matches = new ArrayStack(10);
>+    
>+    /**
>      * The class loader to use for instantiating application objects.
>      * If not specified, the context class loader, or the class loader
>      * used to load Digester itself, is used, based on the value of the
>@@ -1006,7 +1016,7 @@
>         }
> 
>         // Fire "body" events for all relevant rules
>-        List rules = getRules().match(namespaceURI, match);
>+        List rules = (List) matches.pop();
>         if ((rules != null) && (rules.size() > 0)) {
>             String bodyText = this.bodyText.toString();
>             Substitutor substitutor = getSubstitutor();
>@@ -1256,6 +1266,7 @@
> 
>         // Fire "begin" events for all relevant rules
>         List rules = getRules().match(namespaceURI, match);
>+        matches.push(rules);
>         if ((rules != null) && (rules.size() > 0)) {
>             Substitutor substitutor = getSubstitutor();
>             if (substitutor!= null) {
>
>  
>
>------------------------------------------------------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] patch to keep stack of matched rules [also: Mixed Content support]

Posted by Craig McClanahan <cr...@apache.org>.
Simon Kitching wrote:

>On Thu, 2004-04-01 at 18:37, Craig McClanahan wrote:
>  
>
>>Simon Kitching wrote:
>>
>>    
>>
>>>Hi,
>>>
>>>Here's a simple patch that I think would make digester more efficient,
>>>by avoiding unnecessary calls to rules.getMatch().
>>>
>>>      
>>>
>>I like the idea in general, but would ask one favor ... could you try 
>>the modified code with Tomcat and/or Struts as well?  I don't think our 
>>unit tests cover some of the interdependencies -- not that this patch 
>>looks like it would break anything, but I just like to make sure we're 
>>not messing up two primary customer projects.
>>    
>>
>
>I downloaded tomcat 5.0.19, and did a search for "*digester*".
>There was only one hit: server/lib/commons-digester.jar
>
>  
>
Cool ... thanks for doing this.

>I replaced that lib with one built with the proposed patch, and Tomcat
>ran fine.
>
>I also searched for "struts.jar", and found the "admin" webapp contained
>it and is therefore presumably a struts-based webapp. So I used the
>admin app a bit, and it also worked fine.
>
>  
>
Good.

>One question, though: according to the RELEASE-NOTES file, only libs in
>common/lib and shared/lib are made available to webapps. But the "admin"
>webapp doesn't bundle a copy of commons-digester.jar, so I don't
>understand where it is getting its digester functionality from...
>
>  
>
It only works when you try this on the first of April :-)

Actually, there is a perfectly logical explanation -- the Admin webapp 
includes a privileged="true" attribute in its <Context> definition.  
Among other things, this causes the "server" class loader (including the 
commons-digester.jar from server/lib) to be the parent class loader for 
the admin webapp's class loader -- thus, the digester found in 
server/lib is visible to the Admin webapp.

>Regards,
>
>Simon
>
>  
>
Craig


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] patch to keep stack of matched rules [also: Mixed Content support]

Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Thu, 2004-04-01 at 18:37, Craig McClanahan wrote:
> Simon Kitching wrote:
> 
> >Hi,
> >
> >Here's a simple patch that I think would make digester more efficient,
> >by avoiding unnecessary calls to rules.getMatch().
> >
> I like the idea in general, but would ask one favor ... could you try 
> the modified code with Tomcat and/or Struts as well?  I don't think our 
> unit tests cover some of the interdependencies -- not that this patch 
> looks like it would break anything, but I just like to make sure we're 
> not messing up two primary customer projects.

I downloaded tomcat 5.0.19, and did a search for "*digester*".
There was only one hit: server/lib/commons-digester.jar

I replaced that lib with one built with the proposed patch, and Tomcat
ran fine.

I also searched for "struts.jar", and found the "admin" webapp contained
it and is therefore presumably a struts-based webapp. So I used the
admin app a bit, and it also worked fine.

One question, though: according to the RELEASE-NOTES file, only libs in
common/lib and shared/lib are made available to webapps. But the "admin"
webapp doesn't bundle a copy of commons-digester.jar, so I don't
understand where it is getting its digester functionality from...

Regards,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org