You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Remy Maucherat <re...@apache.org> on 2003/11/11 19:40:03 UTC
[digester] [PATCH] Adding Ant-like properties support
Hi,
I described the feature a couple weeks ago, and here's my patch.
It currently only replaces attributes processed by the setProperties
rule. This did sound good enough to me. I read about processing text
nodes too (does Ant do this also ?), so maybe we can improve this
patch/feature more.
Remy
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Thu, 2003-11-27 at 00:01, Simon Kitching wrote:
> Attached is a complete implementation of the "variable expansion"
> feature. I do also have some unit tests for it, but unfortunately they
> are still on my home PC so I will have to send them tomorrow.
And here it is (plus a patch to add the test to the build.xml)
I guess that we will also need a rule to allow the users to set
variables from their xml.
Here's an initial suggestion to kick the discussion off:
class SetVariableRule {
public static final int BEHAVIOUR_JAVA = 0;
public static final int BEHAVIOUR_ANT = 1;
public static final int BEHAVIOUR_CONSTANT = 2;
public SetVariableRule(String nameAttr, String valAttr,
Map source, int behaviour) {
// ..
}
// on startElement, fetches the varname from nameAttr,
// and the value from valAttr, and stores the (name,val)
// pair into the specified source.
//
// behaviour controls what happens if the name is already
// in the source:
// * java --> overwrite
// * ant --> don't change (leave original value)
// * constant --> throw exception
}
and used like this:
HashMap source = new HashMap();
VarExpander varExpander = new MultiVarExpander();
varExpander.addSource("$", source);
digester.setVarExpander(varExpander);
digester.addSetVariable("*/property", "name", "value",
source, BEHAVIOUR_ANT);
<root>
<property name="what" value="drink beer"/>
<property name="when" value="now"/>
<action what="${what}" when="${when}"/>
</root>
Thoughts?
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Wed, 2003-11-26 at 09:52, robert burrell donkin wrote:
> On 24 Nov 2003, at 23:54, Simon Kitching wrote:
> > I'm *really* in favour of providing expansion in body text as well as
> > attributes.
>
> yep. but i'd favour a sequential approach. remy needs attributes right
> now, so let's do attributes right now :)
>
> > I also personally favour an interface rather than an abstract class.
> > Who
> > knows what implementations people might come up with in the future :
> > retrieve values from LDAP? support emacs macros in variables?.
> > If the "replacement strategy" is an abstract class that brings
> > implementation baggage with it, then it might be more difficult to
> > implement those...
>
> this would be an abstract class that is (from a higher OOP perspective)
> an interface but will be implemented in java as an abstract class. it
> will contain no implementation. most folks will probably subclass the
> base implementation.
>
> > On the other hand, an abstract class *does* allow us to expand the
> > "interface" later by adding operations with default implementations,
> > without breaking code. I personally still feel more comfortable with a
> > pure interface in this case, though.
>
> IMHO being able to maintain backwards compatibility is crucial for a
> good library. unless i can see the need for different inheritance
> hierarchies to implement the interface (which is more common in
> frameworks than library's), experience has taught me that (when
> developing library code) the flexibility that implementing as an
> abstract class gives in java pays dividends in long run.
Attached is a complete implementation of the "variable expansion"
feature. I do also have some unit tests for it, but unfortunately they
are still on my home PC so I will have to send them tomorrow.
This code implements the "multiple sources, configurable var marker"
features I talked about earlier. The implementation came together so
well that I am not sure that there is any point in trying to provide an
alternative "simpler" implementation that just supports one source and
"$" as the marker. If you're happy with this, class MultiVarExpander
could just be renamed to BaseVarExpander or similar.
Preferring abstract classes over interfaces is a new "pattern" to me,
but I am willing to give it a go. Class VarExpander is therefore an
abstract class rather than an interface.
Note that this implementation does almost *no* dynamic memory allocation
at all, and therefore should perform very well. Only one "attributes
wrapper" object exists per Digester instance; it is reused each time
startElement(...) is called. I can't think of any reason why
startElement would be called re-entrantly, so this should be safe.
It also implements expansion of text in body text (adds about 2 extra
lines of code).
Regards,
Simon
>
> - robert
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On 24 Nov 2003, at 23:54, Simon Kitching wrote:
> On Tue, 2003-11-25 at 12:31, robert burrell donkin wrote:
>> On 24 Nov 2003, at 22:54, Simon Kitching wrote:
>>
>>> On Tue, 2003-11-25 at 11:32, robert burrell donkin wrote:
>>>> i've finally found time to consider both proposals (apologies for
>>>> the
>>>> delay).
>>>>
>>>> 1. i think that i like some parts of remy's solution but would
>>>> prefer
>>>> the actual implementation to be pluggable through a strategy
>>>> interface
>>>> (probably implemented as an abstract class). i know that this is a
>>>> little slower but i think that in the long term, it's better for
>>>> digester not to be tied to a particular implementation (no matter
>>>> how
>>>> good ;)
>>>
>>> Could you expand on this "strategy interface" a little, Robert? I'm
>>> not
>>> quite sure what you mean here.
>>>
>>> Are you suggesting something like this?
>>> interface VarExpander {
>>> /** expand any variable references in the string */
>>> String expand(String raw);
>>> }
>>>
>>> and
>>>
>>> Digester.setVarExpander(VarExpander varExpander)
>>
>> a little. i was using interface in the modelling sense (which you'll
>> probably have guessed will mean that i'm going to make it an abstract
>> class since it'll probably need to be expanded in the future).
>>
>> abstract class ReplacementStrategy {
>>
>> public abstract Attributes replaceAttributes(Attributes
>> baseAttributes);
>> }
>>
>> with a getter and setter. the actual interface is still a little fluid
>> in my mind at the moment. the name might also change.
>
> I'm *really* in favour of providing expansion in body text as well as
> attributes.
yep. but i'd favour a sequential approach. remy needs attributes right
now, so let's do attributes right now :)
> I think that these two representations are equivalent:
> <person name="${name}" email="${email}"/
> or
> <person>
> <name>${name}</name>
> <email>${email}</email>
> </person>
>
> If we only support attribute expansion, then the latter can never be
> used. The "VarExpander" suggestion wraps the concept of "expanding a
> string" without tying it to "attribute strings". It can be called for
> body text too. Can't think of any other text that might need expanding,
> but if there is, then it can be called in that context too.
i've often been surprised over the years when making calls like this.
> I also personally favour an interface rather than an abstract class.
> Who
> knows what implementations people might come up with in the future :
> retrieve values from LDAP? support emacs macros in variables?.
> If the "replacement strategy" is an abstract class that brings
> implementation baggage with it, then it might be more difficult to
> implement those...
this would be an abstract class that is (from a higher OOP perspective)
an interface but will be implemented in java as an abstract class. it
will contain no implementation. most folks will probably subclass the
base implementation.
> On the other hand, an abstract class *does* allow us to expand the
> "interface" later by adding operations with default implementations,
> without breaking code. I personally still feel more comfortable with a
> pure interface in this case, though.
IMHO being able to maintain backwards compatibility is crucial for a
good library. unless i can see the need for different inheritance
hierarchies to implement the interface (which is more common in
frameworks than library's), experience has taught me that (when
developing library code) the flexibility that implementing as an
abstract class gives in java pays dividends in long run.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Tue, 2003-11-25 at 12:31, robert burrell donkin wrote:
> On 24 Nov 2003, at 22:54, Simon Kitching wrote:
>
> > On Tue, 2003-11-25 at 11:32, robert burrell donkin wrote:
> >> i've finally found time to consider both proposals (apologies for the
> >> delay).
> >>
> >> 1. i think that i like some parts of remy's solution but would prefer
> >> the actual implementation to be pluggable through a strategy interface
> >> (probably implemented as an abstract class). i know that this is a
> >> little slower but i think that in the long term, it's better for
> >> digester not to be tied to a particular implementation (no matter how
> >> good ;)
> >
> > Could you expand on this "strategy interface" a little, Robert? I'm not
> > quite sure what you mean here.
> >
> > Are you suggesting something like this?
> > interface VarExpander {
> > /** expand any variable references in the string */
> > String expand(String raw);
> > }
> >
> > and
> >
> > Digester.setVarExpander(VarExpander varExpander)
>
> a little. i was using interface in the modelling sense (which you'll
> probably have guessed will mean that i'm going to make it an abstract
> class since it'll probably need to be expanded in the future).
>
> abstract class ReplacementStrategy {
>
> public abstract Attributes replaceAttributes(Attributes
> baseAttributes);
> }
>
> with a getter and setter. the actual interface is still a little fluid
> in my mind at the moment. the name might also change.
I'm *really* in favour of providing expansion in body text as well as
attributes.
I think that these two representations are equivalent:
<person name="${name}" email="${email}"/
or
<person>
<name>${name}</name>
<email>${email}</email>
</person>
If we only support attribute expansion, then the latter can never be
used. The "VarExpander" suggestion wraps the concept of "expanding a
string" without tying it to "attribute strings". It can be called for
body text too. Can't think of any other text that might need expanding,
but if there is, then it can be called in that context too.
I also personally favour an interface rather than an abstract class. Who
knows what implementations people might come up with in the future :
retrieve values from LDAP? support emacs macros in variables?.
If the "replacement strategy" is an abstract class that brings
implementation baggage with it, then it might be more difficult to
implement those...
On the other hand, an abstract class *does* allow us to expand the
"interface" later by adding operations with default implementations,
without breaking code. I personally still feel more comfortable with a
pure interface in this case, though.
Cheers, Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On 24 Nov 2003, at 22:54, Simon Kitching wrote:
> On Tue, 2003-11-25 at 11:32, robert burrell donkin wrote:
>> i've finally found time to consider both proposals (apologies for the
>> delay).
>>
>> 1. i think that i like some parts of remy's solution but would prefer
>> the actual implementation to be pluggable through a strategy interface
>> (probably implemented as an abstract class). i know that this is a
>> little slower but i think that in the long term, it's better for
>> digester not to be tied to a particular implementation (no matter how
>> good ;)
>
> Could you expand on this "strategy interface" a little, Robert? I'm not
> quite sure what you mean here.
>
> Are you suggesting something like this?
> interface VarExpander {
> /** expand any variable references in the string */
> String expand(String raw);
> }
>
> and
>
> Digester.setVarExpander(VarExpander varExpander)
a little. i was using interface in the modelling sense (which you'll
probably have guessed will mean that i'm going to make it an abstract
class since it'll probably need to be expanded in the future).
abstract class ReplacementStrategy {
public abstract Attributes replaceAttributes(Attributes
baseAttributes);
}
with a getter and setter. the actual interface is still a little fluid
in my mind at the moment. the name might also change.
> so that the user can control exactly how variables are detected in
> strings, and how the replacement data is located?
that's pretty much right.
> That sounds good to me. The default could be Remy's implementation
> which
> requires the syntax ${xxx} and which always draws from one source. My
> suggestion of an "enhanced" version that allows more flexible variable
> "detection" and multiple sources could be provided as an alternative
> [provided anyone thinks it is useful enough to add to the Digester
> base]. This seems symmetrical with the BaseRules/ExtendedBaseRules
> approach.
i was planning on making the default no substitution :)
this will preserve backward's compatibility.
remy's will be the headline (or basic) substitution implementation.
>> 2. i think that simon's correct that the right place to do this is in
>> digester rather than in particular rules. coding this into rules is
>> just going to cause confusion for users and difficulties when some
>> rules support it and some don't.
>
> I was actually working on this last night. I've come up with the idea
> of
> a "lazy evaluation" wrapper for the org.xml.sax.Attributes class, which
> only checks attribute values for variables when the attribute value is
> asked for.
>
> I hope to be able to post this to the list in about 24 hours time.
>
> This hopefully solves Remy's (quite correct) objection about
> performance
> with the code I originally posted.
i was planning to use something along those lines. lazy is probably
good but maybe need to think about caching results. i think that we
should be able to re-use the same instance without worrying about
re-entancy (but i'll need to double check that tommorrow).
>> 3. i'd prefer not to subclass and instead use a seamless substitution.
>> i'd probably start with just the attribute values and provide just the
>> behaviour in remy's post.
>
> Is this in reference to the implementation I posted which subclassed
> Digester in order to provide this functionality? That was never
> intended
> to be a final solution; it's just what I currently have in production.
> For production code, I don't like modifying libraries :-). I agree that
> var expansion should really be integrated into the existing digester
> stuff without subclassing Digester.
>
> The stuff I posted wasn't a prepared patch; I just grabbed some stuff I
> already had floating around and sent it out as an alternative
> suggestion. Sorry if that wasn't clear.
>
> If you meant something else by "prefer not to subclass" please let me
> know.
it was just a comment :)
i'd like to see support for this in the core digester implementation
(rather than in a subclass).
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Tue, 2003-11-25 at 11:32, robert burrell donkin wrote:
> i've finally found time to consider both proposals (apologies for the
> delay).
>
> 1. i think that i like some parts of remy's solution but would prefer
> the actual implementation to be pluggable through a strategy interface
> (probably implemented as an abstract class). i know that this is a
> little slower but i think that in the long term, it's better for
> digester not to be tied to a particular implementation (no matter how
> good ;)
Could you expand on this "strategy interface" a little, Robert? I'm not
quite sure what you mean here.
Are you suggesting something like this?
interface VarExpander {
/** expand any variable references in the string */
String expand(String raw);
}
and
Digester.setVarExpander(VarExpander varExpander)
so that the user can control exactly how variables are detected in
strings, and how the replacement data is located?
That sounds good to me. The default could be Remy's implementation which
requires the syntax ${xxx} and which always draws from one source. My
suggestion of an "enhanced" version that allows more flexible variable
"detection" and multiple sources could be provided as an alternative
[provided anyone thinks it is useful enough to add to the Digester
base]. This seems symmetrical with the BaseRules/ExtendedBaseRules
approach.
>
> 2. i think that simon's correct that the right place to do this is in
> digester rather than in particular rules. coding this into rules is
> just going to cause confusion for users and difficulties when some
> rules support it and some don't.
I was actually working on this last night. I've come up with the idea of
a "lazy evaluation" wrapper for the org.xml.sax.Attributes class, which
only checks attribute values for variables when the attribute value is
asked for.
I hope to be able to post this to the list in about 24 hours time.
This hopefully solves Remy's (quite correct) objection about performance
with the code I originally posted.
>
> 3. i'd prefer not to subclass and instead use a seamless substitution.
> i'd probably start with just the attribute values and provide just the
> behaviour in remy's post.
Is this in reference to the implementation I posted which subclassed
Digester in order to provide this functionality? That was never intended
to be a final solution; it's just what I currently have in production.
For production code, I don't like modifying libraries :-). I agree that
var expansion should really be integrated into the existing digester
stuff without subclassing Digester.
The stuff I posted wasn't a prepared patch; I just grabbed some stuff I
already had floating around and sent it out as an alternative
suggestion. Sorry if that wasn't clear.
If you meant something else by "prefer not to subclass" please let me
know.
> 4. for the stock implementation of the strategy, i'd like to use remy's
> algorithm.
>
> 5. extra features (and more flexible behaviour) can be added later as
> necessary.
>
> remy - is there any chance of a unit test (or two) so that i can ensure
> that implementation hasn't missed anything?
>
> i'll hold off starting this till tommorrow so other people have a
> chance to comment (and find any holes in my logic ;) before i start
> work.
>
> - robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by robert burrell donkin <rd...@apache.org>.
i've finally found time to consider both proposals (apologies for the
delay).
1. i think that i like some parts of remy's solution but would prefer
the actual implementation to be pluggable through a strategy interface
(probably implemented as an abstract class). i know that this is a
little slower but i think that in the long term, it's better for
digester not to be tied to a particular implementation (no matter how
good ;)
2. i think that simon's correct that the right place to do this is in
digester rather than in particular rules. coding this into rules is
just going to cause confusion for users and difficulties when some
rules support it and some don't.
3. i'd prefer not to subclass and instead use a seamless substitution.
i'd probably start with just the attribute values and provide just the
behaviour in remy's post.
4. for the stock implementation of the strategy, i'd like to use remy's
algorithm.
5. extra features (and more flexible behaviour) can be added later as
necessary.
remy - is there any chance of a unit test (or two) so that i can ensure
that implementation hasn't missed anything?
i'll hold off starting this till tommorrow so other people have a
chance to comment (and find any holes in my logic ;) before i start
work.
- robert
On 11 Nov 2003, at 18:40, Remy Maucherat wrote:
> Hi,
>
> I described the feature a couple weeks ago, and here's my patch.
>
> It currently only replaces attributes processed by the setProperties
> rule. This did sound good enough to me. I read about processing text
> nodes too (does Ant do this also ?), so maybe we can improve this
> patch/feature more.
>
> Remy
>
> /*
> * $Header:
> /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/
> digester/SetPropertyRule.java,v 1.14 2003/10/09 21:09:46 rdonkin Exp $
> * $Revision: 1.14 $
> * $Date: 2003/10/09 21:09:46 $
> *
> * ====================================================================
> *
> * The Apache Software License, Version 1.1
> *
> * Copyright (c) 2001-2003 The Apache Software Foundation. All rights
> * reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> * are met:
> *
> * 1. Redistributions of source code must retain the above copyright
> * notice, this list of conditions and the following disclaimer.
> *
> * 2. Redistributions in binary form must reproduce the above copyright
> * notice, this list of conditions and the following disclaimer in
> * the documentation and/or other materials provided with the
> * distribution.
> *
> * 3. The end-user documentation included with the redistribution,
> * if any, must include the following acknowledgement:
> * "This product includes software developed by the
> * Apache Software Foundation (http://www.apache.org/)."
> * Alternately, this acknowledgement may appear in the software
> itself,
> * if and wherever such third-party acknowledgements normally
> appear.
> *
> * 4. The names "Apache", "The Jakarta Project", "Commons", and
> "Apache Software
> * Foundation" must not be used to endorse or promote products
> derived
> * from this software without prior written permission. For written
> * permission, please contact apache@apache.org.
> *
> * 5. Products derived from this software may not be called "Apache",
> * "Apache" nor may "Apache" appear in their names without prior
> * written permission of the Apache Software Foundation.
> *
> * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
> * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
> * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> * SUCH DAMAGE.
> * ====================================================================
> *
> * This software consists of voluntary contributions made by many
> * individuals on behalf of the Apache Software Foundation. For more
> * information on the Apache Software Foundation, please see
> * <http://www.apache.org/>.
> *
> */
>
>
> package org.apache.commons.digester;
>
> /**
> * This maps provides properties mapping to system classes. This base
> * implementation lookups properties in the system properties.
> *
> * @author Remy Maucherat
> * @version $Revision: 1.14 $ $Date: 2003/10/09 21:09:46 $
> */
>
> public class PropertySource {
>
> public String getProperty(String key) {
> return System.getProperty(key);
> }
>
> }
> Index: src/java/org/apache/commons/digester/Digester.java
> ===================================================================
> RCS file:
> /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/
> digester/Digester.java,v
> retrieving revision 1.83
> diff -u -r1.83 Digester.java
> --- src/java/org/apache/commons/digester/Digester.java 9 Oct 2003
> 21:09:46 -0000 1.83
> +++ src/java/org/apache/commons/digester/Digester.java 11 Nov 2003
> 18:27:01 -0000
> @@ -281,6 +281,12 @@
>
>
> /**
> + * Properties source.
> + */
> + protected PropertySource[] propertySources = null;
> +
> +
> + /**
> * The public identifier of the DTD we are currently parsing under
> * (if any).
> */
> @@ -624,6 +630,30 @@
> }
>
>
> + /**
> + * Return the property sources set for this digester.
> + */
> + public PropertySource[] getPropertySources() {
> +
> + return (this.propertySources);
> +
> + }
> +
> +
> + /**
> + * Set the property sources for this Digester. Setting this to a
> non
> + * empty array will enable property replacement for all attribute
> values.
> + * Properties can be referred to in the XML as ${property.name}.
> + *
> + * @param errorHandler The new error handler
> + */
> + public void setPropertySources(PropertySource[] propertySources) {
> +
> + this.propertySources = propertySources;
> +
> + }
> +
> +
> /**
> * Set the publid id of the current file being parse.
> * @param publicId the DTD/Schema public's id.
> Index: src/java/org/apache/commons/digester/SetPropertiesRule.java
> ===================================================================
> RCS file:
> /home/cvs/jakarta-commons/digester/src/java/org/apache/commons/
> digester/SetPropertiesRule.java,v
> retrieving revision 1.15
> diff -u -r1.15 SetPropertiesRule.java
> --- src/java/org/apache/commons/digester/SetPropertiesRule.java 9 Oct
> 2003 21:09:46 -0000 1.15
> +++ src/java/org/apache/commons/digester/SetPropertiesRule.java 11 Nov
> 2003 18:27:01 -0000
> @@ -223,6 +223,12 @@
> }
> String value = attributes.getValue(i);
>
> + // Properties replacement
> + PropertySource[] propertySources =
> digester.getPropertySources();
> + if (propertySources != null && (propertySources.length >
> 0)) {
> + value = replaceProperties(value, propertySources);
> + }
> +
> // we'll now check for custom mappings
> for (int n = 0; n<attNamesLength; n++) {
> if (name.equals(attributeNames[n])) {
> @@ -311,6 +317,64 @@
> StringBuffer sb = new StringBuffer("SetPropertiesRule[");
> sb.append("]");
> return (sb.toString());
> +
> + }
> +
> +
> + /**
> + * Replace ${NAME} with the property value
> + */
> + public static String replaceProperties
> + (String value, PropertySource[] properties) {
> +
> + StringBuffer sb = null;
> + int prev=0;
> + // assert value!=nil
> + int pos;
> + while( (pos=value.indexOf( "$", prev )) >= 0 ) {
> + if(pos>0) {
> + if (sb == null) {
> + sb = new StringBuffer();
> + }
> + sb.append( value.substring( prev, pos ) );
> + }
> + if( pos == (value.length() - 1)) {
> + sb.append('$');
> + prev = pos + 1;
> + }
> + else if (value.charAt( pos + 1 ) != '{' ) {
> + sb.append( value.charAt( pos + 1 ) );
> + prev=pos+2; // XXX
> + } else {
> + int endName=value.indexOf( '}', pos );
> + if( endName < 0 ) {
> + sb.append( value.substring( pos ));
> + prev=value.length();
> + continue;
> + }
> + String n=value.substring( pos+2, endName );
> + String v= null;
> + if( v==null && properties != null) {
> + for( int i=0; i < properties.length; i++ ) {
> + v=properties[i].getProperty( n );
> + if( v!=null ) {
> + break;
> + }
> + }
> + }
> + if( v== null )
> + v = "${"+n+"}";
> +
> + sb.append( v );
> + prev=endName+1;
> + }
> + }
> + if (sb != null) {
> + if (prev < value.length()) sb.append( value.substring(
> prev ) );
> + return sb.toString();
> + } else {
> + return value;
> + }
>
> }
>
>
> ---------------------------------------------------------------------
> 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] Adding Ant-like properties support
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On 11 Nov 2003, at 21:39, Craig R. McClanahan wrote:
> Quoting Remy Maucherat <re...@apache.org>:
>
>> Remy Maucherat wrote:
>>
>>> Hi,
>>>
>>> I described the feature a couple weeks ago, and here's my patch.
>>>
>>> It currently only replaces attributes processed by the setProperties
>>> rule. This did sound good enough to me. I read about processing text
>>> nodes too (does Ant do this also ?), so maybe we can improve this
>>> patch/feature more.
>>
>> BTW, I do have commit access on digester, so I can commit it myself if
>> the change is acceptable (for a first implementation).
>>
>> Remy
>>
>
> At first glance it looks fine ... I will have a little time this
> evening to
> analyze it more thoroughly, and integrate it, if Robert doesn't beat
> me.
the design seems fine to me. i'll probably (for once) leave the legwork
to craig :)
(i'm *so* busy catching up right now.)
> Some unit tests to exercise the behavior would also be very helpful
> :-).
+1
BTW i'd be happy for remy to have digester karma
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by "Craig R. McClanahan" <cr...@apache.org>.
Quoting Remy Maucherat <re...@apache.org>:
> Remy Maucherat wrote:
>
> > Hi,
> >
> > I described the feature a couple weeks ago, and here's my patch.
> >
> > It currently only replaces attributes processed by the setProperties
> > rule. This did sound good enough to me. I read about processing text
> > nodes too (does Ant do this also ?), so maybe we can improve this
> > patch/feature more.
>
> BTW, I do have commit access on digester, so I can commit it myself if
> the change is acceptable (for a first implementation).
>
> Remy
>
At first glance it looks fine ... I will have a little time this evening to
analyze it more thoroughly, and integrate it, if Robert doesn't beat me. Some
unit tests to exercise the behavior would also be very helpful :-).
Craig
>
>
> ---------------------------------------------------------------------
> 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] Adding Ant-like properties support
Posted by Remy Maucherat <re...@apache.org>.
Remy Maucherat wrote:
> Hi,
>
> I described the feature a couple weeks ago, and here's my patch.
>
> It currently only replaces attributes processed by the setProperties
> rule. This did sound good enough to me. I read about processing text
> nodes too (does Ant do this also ?), so maybe we can improve this
> patch/feature more.
BTW, I do have commit access on digester, so I can commit it myself if
the change is acceptable (for a first implementation).
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
Hi Remy,
On Wed, 2003-11-12 at 23:01, Remy Maucherat wrote:
> Simon Kitching wrote:
> > Hi Remy,
> >
> > I'm really keen to have this sort of feature in Digester.
> >
> > I've had this kind of functionality in my local application for some
> > time now, but it's implemented in a rather different manner.
> >
> > Attached is my current implementation, for comparison.
> >
> > Here's the major differences:
> >
> > *
> > Your initial patch only does substitution for SetPropertiesRule.
> > What about CallParamRule, etc?
> >
> > The attached implementation automatically applies to all attributes, and
> > therefore to all rules both built-in and custom. While somewhat less
> > efficient than your current proposal, it isn't likely to be invoked very
> > often (variable substitution is expected to be the exception, not the
> > normal case). See later for a proposed optimisation to the current
> > implementation.
>
> - it could be not very efficient (you reinstantiate all attribute
> lists); JBoss does the exact same thing, and I don't know if it's
> significant, so I chose a lower overhead method; I think I'll try it
> though, since it is more generic and easier to implement
I agree there is an efficiency hit for the implementation I suggest. My
email proposed some improvements to the existing implementation that
should reduce this by at least 50% (depending on how many input elements
actually take advantage of variable substitution).
And if performance is a problem, we could design a custom class which
implements org.xml.sax.Attributes, and reuse the same instance all the
time. The data inside can be structured as Arrays of values; I'm pretty
sure it is possible to create a class that would not need any dynamic
allocation at all to "copy" the attributes data into it. Yes, the
copying is needed but that is no worse than the copying going on when
implementing it in SetPropertiesRule et. al.
> - it doesn't support scoping like Ant does (and is a feature Tomcat
> would use)
Could you explain this, Remy? I don't see what you mean by scoping..
> - personally, I'm used to ${...}, like all Ant users, probably; I don't
> see any reason to add support for customizing this (I don't like featurism)
Anyone else like/dislike the suggestions that:
(a)
there should be some syntax available to suppress variable substitution,
so it is actually possible to pass the string "${" to a method when
variable substitution is enabled?
(b)
the variable marker" char should be customisable?
(c)
multiple sources should be supported?
As I said in a previous email, I'm not sure at the moment that (c) can
be efficiently implemented.
Cheers,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Remy Maucherat <re...@apache.org>.
Simon Kitching wrote:
> Hi Remy,
>
> I'm really keen to have this sort of feature in Digester.
>
> I've had this kind of functionality in my local application for some
> time now, but it's implemented in a rather different manner.
>
> Attached is my current implementation, for comparison.
>
> Here's the major differences:
>
> *
> Your initial patch only does substitution for SetPropertiesRule.
> What about CallParamRule, etc?
>
> The attached implementation automatically applies to all attributes, and
> therefore to all rules both built-in and custom. While somewhat less
> efficient than your current proposal, it isn't likely to be invoked very
> often (variable substitution is expected to be the exception, not the
> normal case). See later for a proposed optimisation to the current
> implementation.
- it could be not very efficient (you reinstantiate all attribute
lists); JBoss does the exact same thing, and I don't know if it's
significant, so I chose a lower overhead method; I think I'll try it
though, since it is more generic and easier to implement
- it doesn't support scoping like Ant does (and is a feature Tomcat
would use)
- personally, I'm used to ${...}, like all Ant users, probably; I don't
see any reason to add support for customizing this (I don't like featurism)
- it should be easy to disable the feature, and it should be disabled by
default
- as a result of the point above, I see zero value in escaping
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [digester] [PATCH] Adding Ant-like properties support
Posted by Simon Kitching <si...@ecnetwork.co.nz>.
Hi Remy,
I'm really keen to have this sort of feature in Digester.
I've had this kind of functionality in my local application for some
time now, but it's implemented in a rather different manner.
Attached is my current implementation, for comparison.
Here's the major differences:
*
Your initial patch only does substitution for SetPropertiesRule.
What about CallParamRule, etc?
The attached implementation automatically applies to all attributes, and
therefore to all rules both built-in and custom. While somewhat less
efficient than your current proposal, it isn't likely to be invoked very
often (variable substitution is expected to be the exception, not the
normal case). See later for a proposed optimisation to the current
implementation.
*
The attached implementation does substitution for variables in body
text, and in the same style it does it for attributes.
*
The attached iplementation provides functionality to set the "marker"
character rather than hard-wiring "$". In fact, it supports any
arbitrary string prefix, not just a single char.
Features that both your patch and my implementation are missing:
*
There isn't any escape syntax to allow ${...} to really be passed as the
value of an attribute. I think a leading backslash implying that the
variable suppression should not occur would be a good idea.
*
I would actually like to see a different "source" associated with
different "marker" variables. ${foo} and #{foo} could then be expanded
from different sources. You could then even use syntax like:
env{...} --> substitutes from environment variables
user{...} --> substitutes from info about the current user
session{...} --> substitutes from the current session variables
assuming of course that the Digester instance was properly configured
with those mappings before parsing began. NOTE: I'm not sure an
efficient "variable expansion" routine can be implemented for multiple
mappings. The expansion doesn't actually have to be too efficient, but
it *is* necessary to be able to quickly detect for any string whether a
variable substitution occurs in it.
Other notes:
My implementation of the beginElement method isn't as optimised as it
could be. An AttributesImpl object is always created; it would be
smarter to only create one if a variable substitution actually occurs in
the attributes (which will be the exception rather than the normal
case).
beginElement(...)
if no attribute value has variable substitution
newAttributes = attributes
else if attributes.instanceOf(AttributesImpl)
set the new attribute values
else
newAttributes = new AttributesImpl(attributes)
set the new attribute values
for each matching rule
begin(..., newAttributes)
-----------
Note when reading the attached code:
* Start with ConnectorDigester.java, then ConstantsManager, then
StrUtils.
* ConstantsManager is pretty much the same as the PropertySource
class in the original patch; a manager of the key->value mappings
available to be referenced from the input xml.
Regards,
Simon