You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by hr...@apache.org on 2005/05/26 01:35:00 UTC

svn commit: r178550 - in /struts/core/trunk/src: share/org/apache/struts/config/ActionConfig.java share/org/apache/struts/config/BaseConfig.java share/org/apache/struts/config/FormBeanConfig.java test/org/apache/struts/config/TestActionConfig.java test/org/apache/struts/config/TestFormBeanConfig.java

Author: hrabago
Date: Wed May 25 16:35:00 2005
New Revision: 178550

URL: http://svn.apache.org/viewcvs?rev=178550&view=rev
Log:
Copy arbitrary properties of inherited form bean properties, action mapping forwards and exception handlers.

Modified:
    struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java
    struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java
    struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java
    struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java
    struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java

Modified: struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java
URL: http://svn.apache.org/viewcvs/struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java?rev=178550&r1=178549&r2=178550&view=diff
==============================================================================
--- struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java (original)
+++ struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java Wed May 25 16:35:00 2005
@@ -711,7 +711,8 @@
 
                 BeanUtils.copyProperties(copy, baseHandler);
                 this.addExceptionConfig(copy);
-
+                copy.setProperties(baseHandler.getProperties());
+                
             } else {
 
                 // process any extension that this config might have
@@ -758,7 +759,8 @@
                 BeanUtils.copyProperties(copy, baseForward);
 
                 this.addForwardConfig(copy);
-
+                copy.setProperties(baseForward.getProperties());
+                
             } else {
 
                 // process any extension for this forward

Modified: struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java
URL: http://svn.apache.org/viewcvs/struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java?rev=178550&r1=178549&r2=178550&view=diff
==============================================================================
--- struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java (original)
+++ struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java Wed May 25 16:35:00 2005
@@ -114,8 +114,8 @@
 
     /**
      * Return the entire set of properties configured for this object.
-     * At this time, this only needs to be exposed 
-     * to support inheritance, so choosing a conservative access modifier ("protected"). 
+     * At this time, this only needs to be exposed to support inheritance, 
+     * so choosing a conservative access modifier ("protected"). 
      * @return
      */
     protected Properties getProperties() {
@@ -123,6 +123,15 @@
     }
 
     
+    /**
+     * Set the entire set of properties configured for this object.
+     * At this time, this only needs to be exposed to support inheritance, 
+     * so choosing a conservative access modifier ("protected"). 
+     */
+    protected void setProperties(Properties properties) {
+        this.properties = properties;
+    }
+
     /**
      * <p>Compare the properties of this config with that of the given and
      * copy those that are not present.  This method is used by subclasses

Modified: struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java
URL: http://svn.apache.org/viewcvs/struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java?rev=178550&r1=178549&r2=178550&view=diff
==============================================================================
--- struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java (original)
+++ struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java Wed May 25 16:35:00 2005
@@ -21,6 +21,7 @@
 
 
 import java.util.HashMap;
+import java.util.Properties;
 import java.lang.reflect.InvocationTargetException;
 
 import org.apache.commons.beanutils.DynaBean;
@@ -272,6 +273,7 @@
 
                 BeanUtils.copyProperties(prop, baseFpc);
                 this.addFormPropertyConfig(prop);
+                prop.setProperties(baseFpc.getProperties());
             }
 
         }

Modified: struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java
URL: http://svn.apache.org/viewcvs/struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java?rev=178550&r1=178549&r2=178550&view=diff
==============================================================================
--- struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java (original)
+++ struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java Wed May 25 16:35:00 2005
@@ -86,12 +86,14 @@
         baseConfig.addForwardConfig(forward);
         
         forward = new ForwardConfig("failure","/failure.jsp",false);
+        forward.setProperty("forwardCount", "10");
         baseConfig.addForwardConfig(forward);
         
         // setup an exception handler
         ExceptionConfig exceptionConfig = new ExceptionConfig();
         exceptionConfig.setType("java.sql.SQLException");
         exceptionConfig.setKey("msg.exception.sql");
+        exceptionConfig.setProperty("exceptionCount", "10");
         baseConfig.addExceptionConfig(exceptionConfig);
 
         // set some arbitrary properties
@@ -277,16 +279,23 @@
         assertNotNull("'failure' forward was not inherited", forward);
         assertEquals("Wrong type for 'failure'", origForward.getPath(), 
                 forward.getPath());
+        assertEquals("Arbitrary property not copied",
+                origForward.getProperty("forwardCount"), 
+                forward.getProperty("forwardCount"));
         
         // check our exceptions 
         ExceptionConfig[] handlers = subConfig.findExceptionConfigs();
         assertEquals("Wrong exception config count", 2, handlers.length);
     
         handler = subConfig.findExceptionConfig("java.sql.SQLException");
-        ExceptionConfig origHandler = subConfig.findExceptionConfig("java.sql.SQLException"); 
+        ExceptionConfig origHandler = baseConfig.findExceptionConfig("java.sql.SQLException"); 
         assertNotNull("'SQLException' handler was not found", handler);
         assertEquals("Wrong key for 'SQLException'", origHandler.getKey(), 
                 handler.getKey());
+        assertEquals("Arbitrary property not copied",
+                origHandler.getProperty("exceptionCount"), 
+                handler.getProperty("exceptionCount"));
+        
         
         handler = subConfig.findExceptionConfig("java.lang.NullPointerException");
         assertNotNull("'NullPointerException' handler disappeared",

Modified: struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java
URL: http://svn.apache.org/viewcvs/struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java?rev=178550&r1=178549&r2=178550&view=diff
==============================================================================
--- struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java (original)
+++ struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java Wed May 25 16:35:00 2005
@@ -90,6 +90,7 @@
         property = new FormPropertyConfig();
         property.setName("name");
         property.setType("java.lang.String");
+        property.setProperty("count", "10");
         baseForm.addFormPropertyConfig(property);
         
         property = new FormPropertyConfig();
@@ -321,6 +322,11 @@
                 property.getInitial());
         assertEquals("Wrong size value for message", 10, property.getSize());
         
+        property = subForm.findFormPropertyConfig("name");
+        original = baseForm.findFormPropertyConfig("name");
+        assertEquals("Arbitrary property not found", 
+                original.getProperty("count"), property.getProperty("count"));
+
         String count = subForm.getProperty("count");
         assertEquals("Arbitrary property was not inherited", 
                 baseFormCount, count); 



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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
If I understood what you're saying, then yes, that's how all other
config fields are, and I'll probably change 'properties' to conform to
that (unless someone else beats me to it).

Hubert

On 5/26/05, Riyaz Mansoor <rm...@gmail.com> wrote:
> Hubert Rabago wrote:
> 
> > Yes, but it doesn't mean I'm not open to other ideas.  In this case,
> > I started out reading from everywhere that fields should be private,
> > and everything should go through accessors (even subclasses).  But I've
> > been burned by that restriction LOTs of times (in some cases, I had
> > to rebuild entire libraries to get access to the fields I needed), so
> > I'll need some convincing.  If private fields are what's best, how come
> > every other config field is protected?
> 
> i suppose a middle ground would be to keep the properties object
> protected but provide and use only accessor methods within struts?
> 
> 
> :)
> 
> riyaz
>

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Riyaz Mansoor <rm...@gmail.com>.
Hubert Rabago wrote:

> Yes, but it doesn't mean I'm not open to other ideas.  In this case,
> I started out reading from everywhere that fields should be private,
> and everything should go through accessors (even subclasses).  But I've
> been burned by that restriction LOTs of times (in some cases, I had
> to rebuild entire libraries to get access to the fields I needed), so
> I'll need some convincing.  If private fields are what's best, how come
> every other config field is protected?

i suppose a middle ground would be to keep the properties object 
protected but provide and use only accessor methods within struts?


:)

riyaz

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Riyaz Mansoor <rm...@gmail.com>.
Hubert Rabago wrote:

> Yes, but it doesn't mean I'm not open to other ideas.  In this case,
> I started out reading from everywhere that fields should be private,
> and everything should go through accessors (even subclasses).  But I've
> been burned by that restriction LOTs of times (in some cases, I had
> to rebuild entire libraries to get access to the fields I needed), so
> I'll need some convincing.  If private fields are what's best, how come
> every other config field is protected?

i suppose a middle ground would be to keep the properties object 
protected but provide and use only accessor methods within struts?


:)

riyaz

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
On 5/26/05, Riyaz Mansoor <rm...@gmail.com> wrote:
> 
> at the moment the getProperties() returns the Map object. no necessity
> for the 2 accessor methods getProperty,setProperty
> 

The getProperties()/setProperties() accessors are protected and are
for subclasses.  The getProperty()/setProperty() accessors are public,
for use by everyone else.

Hubert

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Riyaz Mansoor <rm...@gmail.com>.
Hubert Rabago wrote:

> That's what we have now, accessors of "properties" in BaseConfig.  In
> fact, that's the only way to access them, which goes against the
> convention set by most (if not all) other config objects.  Other
> config objects declare their fields as protected, allowing custom
> subclasses to access those fields directly.  I'll probably change
> "properties" in BaseConfig to follow this convention.

at the moment the getProperties() returns the Map object. no necessity 
for the 2 accessor methods getProperty,setProperty

> Yes, but it doesn't mean I'm not open to other ideas.  In this case,
> I started out reading from everywhere that fields should be private,
> and everything should go through accessors (even subclasses).  But I've
> been burned by that restriction LOTs of times (in some cases, I had
> to rebuild entire libraries to get access to the fields I needed), so
> I'll need some convincing.  If private fields are what's best, how come
> every other config field is protected?

don't know about that :)

but yes the "officially" (if it can be called that) seems to be to make 
fields private and provide accessor methods.


riyaz

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
On 5/26/05, Martin Cooper <mf...@gmail.com> wrote:
> 
> I'm not sure how you can be burned by not having direct access to a
> member while there are getters and setters available. 

My mistake.  The cases that keep flashing in my head provided getters
(if even that) but no setters.  In those cases, I had problems because
the superclass would use the instance without going through the
getter, so even if I overrode the getter, I couldn't make the change
the behavior of the superclass.

Hubert

> On the other
> hand, making a member directly accessable severely curbs the ability
> to change the class implementation without breaking backwards
> compatibility. Suppose at some point that you decide you don't want to
> keep a reference to the member in the class, but retrieve it on the
> fly. Or perhaps you decide that you want to keep the reference in a
> cache of weak references. You're hosed, because any code that used
> that member is now broken.
> 
> This is something that I really dislike in a lot of the Struts code.
> For much of it, it's no doubt too late to change. However, I'd very
> much prefer that we don't continue to exacerbate the problem.
> 
> --
> Martin Cooper
> 
>

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Martin Cooper <mf...@gmail.com>.
On 5/26/05, Hubert Rabago <hr...@gmail.com> wrote:
> On 5/26/05, Riyaz Mansoor <rm...@gmail.com> wrote:
> > Hubert Rabago wrote:
> >
> > my intention here was to have _all_ the accessor methods of "properties"
> > in BaseConfig - i guess it can be a hinderance when the classes a
> > changing rapidly as u've mentioned. i think of it as a 'cleaner' way :)
> >
> 
> That's what we have now, accessors of "properties" in BaseConfig.  In
> fact, that's the only way to access them, which goes against the
> convention set by most (if not all) other config objects.  Other
> config objects declare their fields as protected, allowing custom
> subclasses to access those fields directly.  I'll probably change
> "properties" in BaseConfig to follow this convention.
> 
> >
> > eg: pass the childConfig to the ParentConfig instead of the passing the
> > parentConfig (baseConfig) to the childConfig.
> 
> It took me a while to understand what you were saying here, but I
> think I finally get it.  I'm not opposed to the idea, but stepping
> back, I'm not entirely convinced that one is cleaner than the other.
> We've been discussing this only from the perspective of copying
> arbitrary properties of config objects held by an ActionConfig.  The
> rest of the config objects have been doing
> "child-inherits-from-parent" as well.  If "parent-sets-child-values"
> is truly that much better, I'd want to use that for the rest of the
> config objects.  Not just when copying full config, but also when
> extending overridden configs.
> 
> >
> > again, extending classes would have limited access to the properties object.
> 
> This point is independent of "parent-sets-child-values" vs.
> "child-inherits-from-parent".  Whichever wins out there, doesn't
> convince me that superclasses should forbid subclasses from accessing
> fields.
> 
> >
> > it comes down to the first point u made. :) ie:
> >
> >  > I'm not sure I want to forbid config subclasses from accessing
> >  > their properties directly.  I've come across several libraries
> >  > and frameworks which used "private" all throughout and made it
> >  > difficult if not impossible for me to customize certain behavior.
> >
> > different ways of thinking ;)
> >
> 
> Yes, but it doesn't mean I'm not open to other ideas.  In this case,
> I started out reading from everywhere that fields should be private,
> and everything should go through accessors (even subclasses).  But I've
> been burned by that restriction LOTs of times (in some cases, I had
> to rebuild entire libraries to get access to the fields I needed), so
> I'll need some convincing.  If private fields are what's best, how come
> every other config field is protected?

I'm not sure how you can be burned by not having direct access to a
member while there are getters and setters available. On the other
hand, making a member directly accessable severely curbs the ability
to change the class implementation without breaking backwards
compatibility. Suppose at some point that you decide you don't want to
keep a reference to the member in the class, but retrieve it on the
fly. Or perhaps you decide that you want to keep the reference in a
cache of weak references. You're hosed, because any code that used
that member is now broken.

This is something that I really dislike in a lot of the Struts code.
For much of it, it's no doubt too late to change. However, I'd very
much prefer that we don't continue to exacerbate the problem.

--
Martin Cooper


> Hubert
> 
> > riyaz
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
> 
>

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
On 5/26/05, Riyaz Mansoor <rm...@gmail.com> wrote:
> Hubert Rabago wrote:
> 
> my intention here was to have _all_ the accessor methods of "properties"
> in BaseConfig - i guess it can be a hinderance when the classes a
> changing rapidly as u've mentioned. i think of it as a 'cleaner' way :)
> 

That's what we have now, accessors of "properties" in BaseConfig.  In
fact, that's the only way to access them, which goes against the
convention set by most (if not all) other config objects.  Other
config objects declare their fields as protected, allowing custom
subclasses to access those fields directly.  I'll probably change
"properties" in BaseConfig to follow this convention.

>
> eg: pass the childConfig to the ParentConfig instead of the passing the
> parentConfig (baseConfig) to the childConfig.

It took me a while to understand what you were saying here, but I
think I finally get it.  I'm not opposed to the idea, but stepping
back, I'm not entirely convinced that one is cleaner than the other. 
We've been discussing this only from the perspective of copying
arbitrary properties of config objects held by an ActionConfig.  The
rest of the config objects have been doing
"child-inherits-from-parent" as well.  If "parent-sets-child-values"
is truly that much better, I'd want to use that for the rest of the
config objects.  Not just when copying full config, but also when
extending overridden configs.

> 
> again, extending classes would have limited access to the properties object.

This point is independent of "parent-sets-child-values" vs.
"child-inherits-from-parent".  Whichever wins out there, doesn't
convince me that superclasses should forbid subclasses from accessing
fields.

> 
> it comes down to the first point u made. :) ie:
> 
>  > I'm not sure I want to forbid config subclasses from accessing
>  > their properties directly.  I've come across several libraries
>  > and frameworks which used "private" all throughout and made it
>  > difficult if not impossible for me to customize certain behavior.
> 
> different ways of thinking ;)
> 

Yes, but it doesn't mean I'm not open to other ideas.  In this case,
I started out reading from everywhere that fields should be private,
and everything should go through accessors (even subclasses).  But I've
been burned by that restriction LOTs of times (in some cases, I had
to rebuild entire libraries to get access to the fields I needed), so
I'll need some convincing.  If private fields are what's best, how come
every other config field is protected?

Hubert

> riyaz
>

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Riyaz Mansoor <rm...@gmail.com>.
Hubert Rabago wrote:

> I'm not sure I want to forbid config subclasses from accessing
> their properties directly.  I've come across several libraries 
> and frameworks which used "private" all throughout and made it
> difficult if not impossible for me to customize certain behavior.
> (I'm still running into those issues with the app I inherited
> here in my day job.)
> 
> The 'properties' field was initially declared as protected when 
> it was only in ActionConfig.  The goal was to reduce the likelihood
> that subclasses would be needed.  Now that it's in a class that's 
> intended to be subclassed, I wonder why it became private.  Joe?

there is a getProperties() method which retrieves the "properties" object

my intention here was to have _all_ the accessor methods of "properties" 
in BaseConfig - i guess it can be a hinderance when the classes a 
changing rapidly as u've mentioned. i think of it as a 'cleaner' way :)

>>classes, only ActionConfig uses "properties" and needs to change for
>>this - not much.
> 
> We also need it in FormBeanConfig.

:)

> BaseConfig already has an "inheritProperties(BaseConfig)" method, doing just
> what you've outlined.  This is used by subclasses when extending another 
> config object, not to be confused with the need to "copy" another config
> object, which is when ActionConfig and FormBeanConfig needs access to the
> properties field.

yes, very recently added :). again, what am trying to say is that 
instead of moving the "properties" object (using getProperties()) the 
properties can be inherited without retrieving the whole "properties" 
object.

eg: pass the childConfig to the ParentConfig instead of the passing the 
parentConfig (baseConfig) to the childConfig.

again, extending classes would have limited access to the properties object.

it comes down to the first point u made. :) ie:

 > I'm not sure I want to forbid config subclasses from accessing
 > their properties directly.  I've come across several libraries
 > and frameworks which used "private" all throughout and made it
 > difficult if not impossible for me to customize certain behavior.

different ways of thinking ;)

riyaz

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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
On 5/25/05, Riyaz Mansoor <rm...@gmail.com> wrote:
> 
> just a thought.
> 
> ie, not expose the "properties" object (as it is) by the
> getProperties,setProperties methods. 

I'm not sure I want to forbid config subclasses from accessing
their properties directly.  I've come across several libraries 
and frameworks which used "private" all throughout and made it
difficult if not impossible for me to customize certain behavior.
(I'm still running into those issues with the app I inherited
here in my day job.)

The 'properties' field was initially declared as protected when 
it was only in ActionConfig.  The goal was to reduce the likelihood
that subclasses would be needed.  Now that it's in a class that's 
intended to be subclassed, I wonder why it became private.  Joe?

> by my last viewing of the Config
> classes, only ActionConfig uses "properties" and needs to change for
> this - not much.
 
We also need it in FormBeanConfig.


> get/setProperties can be replaced by
> 
> void inheritProperties(childConfig) {
>      // for each "xx" property in (parent) class
>      //     if childConfig.getProperty("xx") == null
>      //         copy the arbitrary property
> }
> 
> 
> imho, i think this is a good way to do this, provides separation between
> BaseConfig and its extending classes

BaseConfig already has an "inheritProperties(BaseConfig)" method, doing just
what you've outlined.  This is used by subclasses when extending another 
config object, not to be confused with the need to "copy" another config
object, which is when ActionConfig and FormBeanConfig needs access to the
properties field.

Hubert

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


Re: svn commit: r178550 - in /struts/core/trunk/src: share/org/apache/struts/config/ActionConfig.java share/org/apache/struts/config/BaseConfig.java share/org/apache/struts/config/FormBeanConfig.java test/org/apache/struts/config/TestActionConfig.java test/org/apache/struts/config/TestFormBeanConfig.java

Posted by Riyaz Mansoor <rm...@gmail.com>.
just a thought.

ie, not expose the "properties" object (as it is) by the 
getProperties,setProperties methods. by my last viewing of the Config 
classes, only ActionConfig uses "properties" and needs to change for 
this - not much.

get/setProperties can be replaced by

void inheritProperties(childConfig) {
     // for each "xx" property in (parent) class
     //     if childConfig.getProperty("xx") == null
     //         copy the arbitrary property
}


imho, i think this is a good way to do this, provides separation between 
BaseConfig and its extending classes

is it worth submitting a patch?

riyaz


hrabago@apache.org wrote:
> Author: hrabago
> Date: Wed May 25 16:35:00 2005
> New Revision: 178550
> 
> URL: http://svn.apache.org/viewcvs?rev=178550&view=rev
> Log:
> Copy arbitrary properties of inherited form bean properties, action mapping forwards and exception handlers.
> 
> Modified:
>     struts/core/trunk/src/share/org/apache/struts/config/ActionConfig.java
>     struts/core/trunk/src/share/org/apache/struts/config/BaseConfig.java
>     struts/core/trunk/src/share/org/apache/struts/config/FormBeanConfig.java
>     struts/core/trunk/src/test/org/apache/struts/config/TestActionConfig.java
>     struts/core/trunk/src/test/org/apache/struts/config/TestFormBeanConfig.java
> 
>  
>      /**
>       * Return the entire set of properties configured for this object.
> -     * At this time, this only needs to be exposed 
> -     * to support inheritance, so choosing a conservative access modifier ("protected"). 
> +     * At this time, this only needs to be exposed to support inheritance, 
> +     * so choosing a conservative access modifier ("protected"). 
>       * @return
>       */
>      protected Properties getProperties() {
> @@ -123,6 +123,15 @@
>      }
>  
>      
> +    /**
> +     * Set the entire set of properties configured for this object.
> +     * At this time, this only needs to be exposed to support inheritance, 
> +     * so choosing a conservative access modifier ("protected"). 
> +     */
> +    protected void setProperties(Properties properties) {
> +        this.properties = properties;
> +    }
> +


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


Re: copy properties for inheritance (Re: svn commit: r178550)

Posted by Hubert Rabago <hr...@gmail.com>.
On 5/25/05, Joe Germuska <Jo...@germuska.com> wrote:
> I was wondering about this:
> 
> >                  BeanUtils.copyProperties(copy, baseHandler);
> >                  this.addExceptionConfig(copy);
> >-
> >+                copy.setProperties(baseHandler.getProperties());
> >+
> 
>  doesn't the strategy you implemented clobber properties which are
> set on the extending/copy?  Shouldn't this process somehow honor
> properties which were already set in the copy?  I think it should go
> through the list of property names in the baseHandler and, where the
> property is not already defined in the copy, in those cases it should
> copy them.
> 

This part of the code executes when the extending config object
(ActionConfig in this case) isn't overriding baseHandler:

            // Do we have this handler?
            ExceptionConfig copy =
                    this.findExceptionConfig(baseHandler.getType());

            if (copy == null) {

                // We don't have this, so let's copy it
                copy = (ExceptionConfig) RequestUtils
                        .applicationInstance(baseHandler.getClass().getName());

                BeanUtils.copyProperties(copy, baseHandler);
                this.addExceptionConfig(copy);
                copy.setProperties(baseHandler.getProperties());
                
            } else {

                // process any extension that this config might have
                copy.processExtends(getModuleConfig(), this);

            }


Hubert

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


copy properties for inheritance (Re: svn commit: r178550)

Posted by Joe Germuska <Jo...@Germuska.com>.
I was wondering about this:

>                  BeanUtils.copyProperties(copy, baseHandler);
>                  this.addExceptionConfig(copy);
>-
>+                copy.setProperties(baseHandler.getProperties());
>+

  doesn't the strategy you implemented clobber properties which are 
set on the extending/copy?  Shouldn't this process somehow honor 
properties which were already set in the copy?  I think it should go 
through the list of property names in the baseHandler and, where the 
property is not already defined in the copy, in those cases it should 
copy them.

I haven't studied this, so forgive me if I'm overlooking something....

Joe



-- 
Joe Germuska            
Joe@Germuska.com  
http://blog.germuska.com    
"Narrow minds are weapons made for mass destruction"  -The Ex

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