springframework-rcp-dev

[Springframework-rcp-dev] Class based FormModel

[Jan H. <jh...@sc...>]

Hi all,

I'm currently looking at a way to provide class based formModels. Well
actually it boils down to a different implementation of
MutablePropertyAccessStrategy called ClassPropertyAccessStrategy (CPAS).
It all kinda starts to work, but there are a few things that bother me:

- the current implementation (BeanPropertyAccessStrategy - BPAS) is
using the BeanWrapperImpl behind the scenes. This implies that if
there's a property method with the incorrect visibility, it will be
'fixed' to provide access. Do we need the same functionality here? I
personally don't think so.

- the BPAS is based on beans and thus can inspect collections/maps etc.
The CPAS cannot look into these and currently doesn't support them. Any
comments on this? Should we provide this and how? Geoffrey already gave
a remark about this: we could use annotations to provide information
about the element type and then go on. WDYT?

- I provided both options BPAS and CPAS in order not to break anything.
This leads to changes in
FormModelHelper/AbstractFormModel/DefaultFormModel.  Should we continue
with both of them or evaluate both and remove one from the mentioned
classes? The 'removed' can always be used when with the specific
constructor on DefaultFormModel which takes a
MutablePropertyAccessStrategy.

- I want to provide some feature to allow write access when
possible/needed. I'll explain with an example:

interface Person{
	String getName();
}

class Admin implements Person {
	String getName()...
}

class BadPerson implements Person{
	String getName()...
	void setName(String)...
}

Now creating a formModel based on Person will only provide you with read
access. Which can be ok for most cases. But when I want write access for
BadPerson, I don't want to create a separate formModel. So I want some
flexibility to state that, when possible, the property name may be
writeable. It would be nice to be able to define a number of properties
that may be writable if the current backing bean allows it. So the class
provided to create the formModel will define the properties and their
getters as a minimum together with the setters of those that are always
writable. If specified, a bean that provides a setter for one of these
properties might enable write access.

I think I can do this by providing some backingBean specific logic in
the CPAS. But I would like to hear your comments on this first.


Kind Regards,
Jan


 **** DISCLAIMER ****
 http://www.schaubroeck.be/maildisclaimer.htm

Re: [Springframework-rcp-dev] Class based FormModel

[Geoffrey De S. <ge0...@gm...>]

With kind regards,
Geoffrey De Smet

Jan Hoskens schreef:
> Hi all,
> 
> I'm currently looking at a way to provide class based formModels. Well
> actually it boils down to a different implementation of
> MutablePropertyAccessStrategy called ClassPropertyAccessStrategy (CPAS).
> It all kinda starts to work, but there are a few things that bother me:
> 
> - the current implementation (BeanPropertyAccessStrategy - BPAS) is
> using the BeanWrapperImpl behind the scenes. This implies that if
> there's a property method with the incorrect visibility, it will be
> 'fixed' to provide access. Do we need the same functionality here? I
> personally don't think so.

no, although if you look at hibernate (which is bean-2-database mapping, 
instead of bean-2-form mapping) it does support it. But when in doubt - 
leave it out, we can always put it back in.

> 
> - the BPAS is based on beans and thus can inspect collections/maps etc.
> The CPAS cannot look into these and currently doesn't support them. Any
> comments on this? Should we provide this and how? Geoffrey already gave
> a remark about this: we could use annotations to provide information
> about the element type and then go on. WDYT?

I think we can do more then we think:
For example if you give hibernate:

   class Boss {
     public List<Person> getMyList
   }

hibernate knows that it's a list of Person (not just Object), at runtime.

Erasure doesn't erase that knowledge on the Boss class methods.
(It does erase that knowlegde on list instances).
So it must be available with the Reflection API.

Nevertheless I believe in time we could use annotations on the beans to 
fine tune bindings. But of course they should also be extactly alike 
creatable with an xml, much like orm.xml is to define JPA/hibernate 
annotations on non-annotated classes.

> 
> - I provided both options BPAS and CPAS in order not to break anything.
> This leads to changes in
> FormModelHelper/AbstractFormModel/DefaultFormModel.  Should we continue
> with both of them or evaluate both and remove one from the mentioned
> classes? The 'removed' can always be used when with the specific
> constructor on DefaultFormModel which takes a
> MutablePropertyAccessStrategy.

How about we remove BPAS directly after the release of 0.3.0?

This of course depends how big the big pain is to switch.
It could be close to zero if we can make a CPAS intead of BPAS based on 
instance.getClass() etc in depricated methods.

> 
> - I want to provide some feature to allow write access when
> possible/needed. I'll explain with an example:
> 
> interface Person{
> 	String getName();
> }
> 
> class Admin implements Person {
> 	String getName()...
> }
> 
> class BadPerson implements Person{
> 	String getName()...
> 	void setName(String)...
> }
> 
> Now creating a formModel based on Person will only provide you with read
> access. Which can be ok for most cases. But when I want write access for
> BadPerson, I don't want to create a separate formModel. So I want some
> flexibility to state that, when possible, the property name may be
> writeable. It would be nice to be able to define a number of properties
> that may be writable if the current backing bean allows it. So the class
> provided to create the formModel will define the properties and their
> getters as a minimum together with the setters of those that are always
> writable. If specified, a bean that provides a setter for one of these
> properties might enable write access.

Personally, I believe the whole idea of determining read-only access 
based on existence getters/setters is flaky at best.
If something needs to be read-only, use 
getFieldMetaData(...).setReadOnly(true).
Same goes for setEnabled(false).

> 
> I think I can do this by providing some backingBean specific logic in
> the CPAS. But I would like to hear your comments on this first.
> 
> 
> Kind Regards,
> Jan
> 
> 
>  **** DISCLAIMER ****
>  http://www.schaubroeck.be/maildisclaimer.htm
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [Springframework-rcp-dev] Class based FormModel

[Jan H. <jh...@sc...>]

> > 
> > - the current implementation (BeanPropertyAccessStrategy - BPAS) is
> > using the BeanWrapperImpl behind the scenes. This implies that if
> > there's a property method with the incorrect visibility, it will be
> > 'fixed' to provide access. Do we need the same functionality here? I
> > personally don't think so.
> 
> no, although if you look at hibernate (which is bean-2-database mapping, 
> instead of bean-2-form mapping) it does support it. But when in doubt - 
> leave it out, we can always put it back in.

Yep, I agree.

> > 
> > - the BPAS is based on beans and thus can inspect collections/maps etc.
> > The CPAS cannot look into these and currently doesn't support them. Any
> > comments on this? Should we provide this and how? Geoffrey already gave
> > a remark about this: we could use annotations to provide information
> > about the element type and then go on. WDYT?
> 
> I think we can do more then we think:
> For example if you give hibernate:
> 
>    class Boss {
>      public List<Person> getMyList
>    }
> 
> hibernate knows that it's a list of Person (not just Object), at runtime.
> 
> Erasure doesn't erase that knowledge on the Boss class methods.
> (It does erase that knowlegde on list instances).
> So it must be available with the Reflection API.

You do implicitly refer to switch to java 1.5 when talking about
generics?  Not sure we can do this, though it's nice to know that it is
possible. 

> 
> Nevertheless I believe in time we could use annotations on the beans to 
> fine tune bindings. But of course they should also be extactly alike 
> creatable with an xml, much like orm.xml is to define JPA/hibernate 
> annotations on non-annotated classes.

Indeed, some form of annotation could provide a nice way out.

> 
> > 
> > - I provided both options BPAS and CPAS in order not to break anything.
> > This leads to changes in
> > FormModelHelper/AbstractFormModel/DefaultFormModel.  Should we continue
> > with both of them or evaluate both and remove one from the mentioned
> > classes? The 'removed' can always be used when with the specific
> > constructor on DefaultFormModel which takes a
> > MutablePropertyAccessStrategy.
> 
> How about we remove BPAS directly after the release of 0.3.0?
> 
> This of course depends how big the big pain is to switch.
> It could be close to zero if we can make a CPAS intead of BPAS based on 
> instance.getClass() etc in depricated methods.

Not a bad idea. We'll make a release before I put in the CPAS.

> Personally, I believe the whole idea of determining read-only access 
> based on existence getters/setters is flaky at best.
> If something needs to be read-only, use 
> getFieldMetaData(...).setReadOnly(true).
> Same goes for setEnabled(false).

The read-only access you mention should be at hand, no doubt about that.
Actually it's the other way around. Making something read-only is
limiting, while I'm referring to expanding to writable. The reason is
that you typically have a super interface/class which you would like to
use to create the form. This can be used throughout the whole process,
except for creating new objects which needs write access to (almost) all
fields. So you got a complete CPAS based on PropertyDescriptors of a
certain class/interface X and then you want the added write-access of
interface/class Y implements/extends X. Your setter isn't defined on X
so no propertyDescriptor exists. And you cannot go the other way around:
creating the CPAS with Y and then using it for objects that conform to X
won't work and will throw an exception.

Or do I not see where your aiming at? Then enlighten me ;-)


Kind Regards,
Jan


 **** DISCLAIMER ****
 http://www.schaubroeck.be/maildisclaimer.htm

Re: [Springframework-rcp-dev] Class based FormModel

[Andy D. <an...@ma...>]

Comments inlined below...

Jan Hoskens wrote:
> Hi all,
>
> I'm currently looking at a way to provide class based formModels. Well
> actually it boils down to a different implementation of
> MutablePropertyAccessStrategy called ClassPropertyAccessStrategy (CPAS).
> It all kinda starts to work, but there are a few things that bother me:
>
>   
How hard would it be to adapt ClassPropertyAccessStrategy to non-class 
based backing objects, such as Data Objects, Maps, or DOMs?  We actually 
use some of these already with Spring Rich (by using our own custom 
MutablePropertyAccessStrategy) - it would be nice to reuse some of 
ClassPropertyAccessStrategy if possible.
> - the current implementation (BeanPropertyAccessStrategy - BPAS) is
> using the BeanWrapperImpl behind the scenes. This implies that if
> there's a property method with the incorrect visibility, it will be
> 'fixed' to provide access. Do we need the same functionality here? I
> personally don't think so.
>   
I agree (for now).
> - the BPAS is based on beans and thus can inspect collections/maps etc.
> The CPAS cannot look into these and currently doesn't support them. Any
> comments on this? Should we provide this and how? Geoffrey already gave
> a remark about this: we could use annotations to provide information
> about the element type and then go on. WDYT?
>   
As Geoffrey has already mentioned, generic types are available at the 
Class level, just not the Collection instance level.  This means one can 
inspect the generic type signature of the Class to determine the 
Collection element type.  Even so, this should still be configurable, 
not just with annotations, but also at runtime (maybe some sort of 
binding context key to override the collection type).  This is useful 
when a broad collection is defined (List<MyBaseObject>), but at runtime 
we know it will contain only MySpecificBaseObject.
> - I provided both options BPAS and CPAS in order not to break anything.
> This leads to changes in
> FormModelHelper/AbstractFormModel/DefaultFormModel.  Should we continue
> with both of them or evaluate both and remove one from the mentioned
> classes? The 'removed' can always be used when with the specific
> constructor on DefaultFormModel which takes a
> MutablePropertyAccessStrategy.
>   
As it is, if CPAS directly uses Java reflection, then we will be forced 
to use our own MutablePropertyAccessStrategy, since some of our forms 
use non-JavaBean backing objects.  However, if CPAS is designed to use 
an abstracted interface for introspection, one that could easily be 
swapped out for some other type of backing object (such as a 
DataObject), then CPAS could do the job all by itself (we could reuse 
everything CPAS has to offer and only have to implement the 
introspection interface for our custom backing objects).
> - I want to provide some feature to allow write access when
> possible/needed. I'll explain with an example:
>
> interface Person{
> 	String getName();
> }
>
> class Admin implements Person {
> 	String getName()...
> }
>
> class BadPerson implements Person{
> 	String getName()...
> 	void setName(String)...
> }
>
> Now creating a formModel based on Person will only provide you with read
> access. Which can be ok for most cases. But when I want write access for
> BadPerson, I don't want to create a separate formModel. So I want some
> flexibility to state that, when possible, the property name may be
> writeable. It would be nice to be able to define a number of properties
> that may be writable if the current backing bean allows it. So the class
> provided to create the formModel will define the properties and their
> getters as a minimum together with the setters of those that are always
> writable. If specified, a bean that provides a setter for one of these
> properties might enable write access.
>
> I think I can do this by providing some backingBean specific logic in
> the CPAS. But I would like to hear your comments on this first.
>   
We just did something very similar to this.  In our case, we bound to a 
property, something like this:
"agency.address.name"
However, the "agency" happens to be null when the form is first 
displayed and isn't filled in until the user selects an agency from a 
combo box.  The requirement was that all components bound to the agency 
will be disabled until the agency becomes non-null.  Since we are using 
our own custom MutablePropertyAccessStrategy (MPAS), we decided the 
easiest path would be if a ValueModel could indicate whether it is 
"enabled" or not.  If a ValueModel is from a property path containing 
"null", then it is considered disabled, otherwise it is considered 
enabled.  So, we created a GuardedValueModel interface:

public interface GuardedValueModel extends ValueModel, Guarded, 
PropertyChangePublisher
{
  String ENABLED_PROPERTY = "enabled";
}

And implement this interface in the custom ValueModels returned from our 
custom MPAS.  The ValueModel detects when the parent ValueModel becomes 
non-null, then changes its enabled state (as defined in the Guarded 
interface from Spring Rich), which fires a property change event.  We 
then link this property change event to the Form's FieldMetadata by 
overriding the postProcessNewValueModel method in DefaultFormModel:

  @Override
  protected void postProcessNewValueModel(final String formProperty, 
final ValueModel valueModel)
  {
    super.postProcessNewValueModel(formProperty, valueModel);
    final GuardedValueModel guarded;
    if(valueModel instanceof GuardedValueModel) {
      guarded = (GuardedValueModel)valueModel;
    } else if(valueModel instanceof ValueModelWrapper) {
      final ValueModel inner = 
((ValueModelWrapper)valueModel).getInnerMostWrappedValueModel();
      if(inner instanceof GuardedValueModel) {
        guarded = (GuardedValueModel)inner;
      } else {
        guarded = null;
      }
    } else {
      guarded = null;
    }

    if(guarded != null) {
      final FieldMetadata fieldMetadata = getFieldMetadata(formProperty);
      
guarded.addPropertyChangeListener(GuardedValueModel.ENABLED_PROPERTY, 
new PropertyChangeListener() {
        public void propertyChange(PropertyChangeEvent evt) {
          fieldMetadata.setEnabled(guarded.isEnabled());
        }
      });
    }
  }

This isn't perfect, but works for our needs.
We decided GuardedValueModel was easier than making, say, the "readable" 
and "writeable" properties of PropertyMetaAspectAccessor mutable (and 
observable) once we got into the guts of DefaultFormModel (and 
AbstractFormModel).

  - Andy

Re: [Springframework-rcp-dev] Class based FormModel

[Jan H. <jh...@sc...>]

> How hard would it be to adapt ClassPropertyAccessStrategy to non-class 
> based backing objects, such as Data Objects, Maps, or DOMs?  We actually 
> use some of these already with Spring Rich (by using our own custom 
> MutablePropertyAccessStrategy) - it would be nice to reuse some of 
> ClassPropertyAccessStrategy if possible.

I haven't thought of this yet, but this is a good moment to start. Is it
possible to send some examples of your implementation? Maybe you already
got some tests as well?

> > - the current implementation (BeanPropertyAccessStrategy - BPAS) is
> > using the BeanWrapperImpl behind the scenes. This implies that if
> > there's a property method with the incorrect visibility, it will be
> > 'fixed' to provide access. Do we need the same functionality here? I
> > personally don't think so.
> >   
> I agree (for now).

So at the moment we all do :-)

> > - the BPAS is based on beans and thus can inspect collections/maps etc.
> > The CPAS cannot look into these and currently doesn't support them. Any
> > comments on this? Should we provide this and how? Geoffrey already gave
> > a remark about this: we could use annotations to provide information
> > about the element type and then go on. WDYT?
> >   
> As Geoffrey has already mentioned, generic types are available at the 
> Class level, just not the Collection instance level.  This means one can 
> inspect the generic type signature of the Class to determine the 
> Collection element type.  Even so, this should still be configurable, 
> not just with annotations, but also at runtime (maybe some sort of 
> binding context key to override the collection type).  This is useful 
> when a broad collection is defined (List<MyBaseObject>), but at runtime 
> we know it will contain only MySpecificBaseObject.

Same remark as with Geoffrey: we're targeting 1.4 now. Should we move to
1.5 to use generics? 

> > - I provided both options BPAS and CPAS in order not to break anything.
> > This leads to changes in
> > FormModelHelper/AbstractFormModel/DefaultFormModel.  Should we continue
> > with both of them or evaluate both and remove one from the mentioned
> > classes? The 'removed' can always be used when with the specific
> > constructor on DefaultFormModel which takes a
> > MutablePropertyAccessStrategy.
> >   
> As it is, if CPAS directly uses Java reflection, then we will be forced 
> to use our own MutablePropertyAccessStrategy, since some of our forms 
> use non-JavaBean backing objects.  However, if CPAS is designed to use 
> an abstracted interface for introspection, one that could easily be 
> swapped out for some other type of backing object (such as a 
> DataObject), then CPAS could do the job all by itself (we could reuse 
> everything CPAS has to offer and only have to implement the 
> introspection interface for our custom backing objects).

At the moment it's pretty fixed to classes, but as said before it is the
moment to put in the extra bit of work to have some more flexibility.
I'll definitely polish things up before pushing it into RCP.

(other comment waaay below)

> We just did something very similar to this.  In our case, we bound to a 
> property, something like this:
> "agency.address.name"
> However, the "agency" happens to be null when the form is first 
> displayed and isn't filled in until the user selects an agency from a 
> combo box.  The requirement was that all components bound to the agency 
> will be disabled until the agency becomes non-null.  Since we are using 
> our own custom MutablePropertyAccessStrategy (MPAS), we decided the 
> easiest path would be if a ValueModel could indicate whether it is 
> "enabled" or not.  If a ValueModel is from a property path containing 
> "null", then it is considered disabled, otherwise it is considered 
> enabled.  So, we created a GuardedValueModel interface:
> 
> public interface GuardedValueModel extends ValueModel, Guarded, 
> PropertyChangePublisher
> {
>   String ENABLED_PROPERTY = "enabled";
> }
> 
> And implement this interface in the custom ValueModels returned from our 
> custom MPAS.  The ValueModel detects when the parent ValueModel becomes 
> non-null, then changes its enabled state (as defined in the Guarded 
> interface from Spring Rich), which fires a property change event.  We 
> then link this property change event to the Form's FieldMetadata by 
> overriding the postProcessNewValueModel method in DefaultFormModel:
> 
>   @Override
>   protected void postProcessNewValueModel(final String formProperty, 
> final ValueModel valueModel)
>   {
>     super.postProcessNewValueModel(formProperty, valueModel);
>     final GuardedValueModel guarded;
>     if(valueModel instanceof GuardedValueModel) {
>       guarded = (GuardedValueModel)valueModel;
>     } else if(valueModel instanceof ValueModelWrapper) {
>       final ValueModel inner = 
> ((ValueModelWrapper)valueModel).getInnerMostWrappedValueModel();
>       if(inner instanceof GuardedValueModel) {
>         guarded = (GuardedValueModel)inner;
>       } else {
>         guarded = null;
>       }
>     } else {
>       guarded = null;
>     }
> 
>     if(guarded != null) {
>       final FieldMetadata fieldMetadata = getFieldMetadata(formProperty);
>       
> guarded.addPropertyChangeListener(GuardedValueModel.ENABLED_PROPERTY, 
> new PropertyChangeListener() {
>         public void propertyChange(PropertyChangeEvent evt) {
>           fieldMetadata.setEnabled(guarded.isEnabled());
>         }
>       });
>     }
>   }
> 
> This isn't perfect, but works for our needs.
> We decided GuardedValueModel was easier than making, say, the "readable" 
> and "writeable" properties of PropertyMetaAspectAccessor mutable (and 
> observable) once we got into the guts of DefaultFormModel (and 
> AbstractFormModel).

Hmm, this looks like another use-case to me. You're referring to
transitive properties and their parent path which may return a null
value. I'm taking note of this, if I'm correctly the current BPAS
doesn't allow this and will give an exception when a parent path returns
null.

I guess your selected agency always has write/read access to the name
property and this doesn't change when having another implementation of
agency. So my use case would be having AgencyReadOnlyName and
AgencyWriteName (extending the former one). Then the field should switch
between read-only and writable.

Kind Regards,
Jan


 **** DISCLAIMER ****
 http://www.schaubroeck.be/maildisclaimer.htm