You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Manfred Geiler <ma...@gmail.com> on 2005/07/27 21:48:30 UTC
Re: svn commit: r225327 - /myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Sylvain,
This change does not make much sense IMHO.
Why do you check for ArrayList instead of List. There is only a List
cast after that, so why not just check for List?
Is there anything a LinkedList does not correctly implement from the
List interface?!
-Manfred
2005/7/26, svieujot@apache.org <sv...@apache.org>:
> Author: svieujot
> Date: Tue Jul 26 08:25:26 2005
> New Revision: 225327
>
> URL: http://svn.apache.org/viewcvs?rev=225327&view=rev
> Log:
> Bugfix for savestate with a linkedlist (couldn't coerce a value of type "java.util.ArrayList" to type "java.util.LinkedList".
>
> Modified:
> myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
>
> Modified: myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> URL: http://svn.apache.org/viewcvs/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java?rev=225327&r1=225326&r2=225327&view=diff
> ==============================================================================
> --- myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java (original)
> +++ myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java Tue Jul 26 08:25:26 2005
> @@ -593,9 +593,9 @@
> Object attachedObject)
> {
> if (attachedObject == null) return null;
> - if (attachedObject instanceof List)
> + if (attachedObject instanceof ArrayList)
> {
> - ArrayList lst = new ArrayList(((List)attachedObject).size());
> + List lst = new ArrayList(((List)attachedObject).size());
> for (Iterator it = ((List)attachedObject).iterator(); it.hasNext(); )
> {
> lst.add(saveAttachedState(context, it.next()));
>
>
>
Re: svn commit: r225327 -
/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Sylvain Vieujot <sv...@apache.org>.
In fact, a much simpler solution is to ... not use saveAttachedState in
the UISaveState.
Sorry for not having spotted that earlier.
I'll commit that.
Sylvain.
On Tue, 2005-08-09 at 15:27 +0200, Manfred Geiler wrote:
> No, no way to make it non-static because it's defined as static in the spec.
> Hmm, no idea at the moment.
> Isn't it a simple workaround, to wrap your special List into an
> serializable managed bean?
>
> -Manfred
>
>
> 2005/8/9, Sylvain Vieujot <sv...@apache.org>:
> > One problem though :
> > UIComponentBase.saveAttachedState is static.
> > Do you think we can make it non-static to allow the overridden method to
> > work ?
> >
> >
> > On Tue, 2005-08-09 at 14:35 +0200, Manfred Geiler wrote:
> > > I think I get it now. Sorry for that.
> > > But the x:saveState should still return the same object (value & classes)
> > > as the given one.
> > > So, the right solution would be to override the
> > > UISaveState.saveAttachedState to just serialize the given bean.
> > >
> > > Do you agree with this solution ?
> >
> > Yes, sounds reasonable.
> >
> > -Manfred
> >
> >
Re: svn commit: r225327 - /myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Manfred Geiler <ma...@gmail.com>.
No, no way to make it non-static because it's defined as static in the spec.
Hmm, no idea at the moment.
Isn't it a simple workaround, to wrap your special List into an
serializable managed bean?
-Manfred
2005/8/9, Sylvain Vieujot <sv...@apache.org>:
> One problem though :
> UIComponentBase.saveAttachedState is static.
> Do you think we can make it non-static to allow the overridden method to
> work ?
>
>
> On Tue, 2005-08-09 at 14:35 +0200, Manfred Geiler wrote:
> > I think I get it now. Sorry for that.
> > But the x:saveState should still return the same object (value & classes)
> > as the given one.
> > So, the right solution would be to override the
> > UISaveState.saveAttachedState to just serialize the given bean.
> >
> > Do you agree with this solution ?
>
> Yes, sounds reasonable.
>
> -Manfred
>
>
Re: svn commit: r225327 -
/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Sylvain Vieujot <sv...@apache.org>.
One problem though :
UIComponentBase.saveAttachedState is static.
Do you think we can make it non-static to allow the overridden method to
work ?
On Tue, 2005-08-09 at 14:35 +0200, Manfred Geiler wrote:
> > I think I get it now. Sorry for that.
> > But the x:saveState should still return the same object (value & classes)
> > as the given one.
> > So, the right solution would be to override the
> > UISaveState.saveAttachedState to just serialize the given bean.
> >
> > Do you agree with this solution ?
>
> Yes, sounds reasonable.
>
> -Manfred
Re: svn commit: r225327 - /myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Manfred Geiler <ma...@gmail.com>.
> I think I get it now. Sorry for that.
> But the x:saveState should still return the same object (value & classes)
> as the given one.
> So, the right solution would be to override the
> UISaveState.saveAttachedState to just serialize the given bean.
>
> Do you agree with this solution ?
Yes, sounds reasonable.
-Manfred
Re: svn commit: r225327 -
/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Sylvain Vieujot <sv...@apache.org>.
Hello Manfred,
I think I get it now. Sorry for that.
But the x:saveState should still return the same object (value &
classes) as the given one.
So, the right solution would be to override the
UISaveState.saveAttachedState to just serialize the given bean.
Do you agree with this solution ?
Thanks,
Sylvain.
On Tue, 2005-08-09 at 13:27 +0200, Manfred Geiler wrote:
> Just had a look at the spec (i.e. javadoc) and the RI source code:
> How we handled the List was absolutely correct IMO.
> The problem you have, would also arise with the RI. So, I do not so a
> reason to do this "trick" in the MyFaces implementation. The drawback
> of your patch is: If someone has a List other than an ArrayList the
> method will fail.
>
> -Manfred
>
>
> 2005/8/3, Sylvain Vieujot <sv...@apache.org>:
> > Hello Manfred,
> >
> > Here is a specific example where savestate would fail :
> > <x:saveState value="#{myLinkedList}/>
> >
> > The save state would return a List, and not a LinkedList.
> > So if you use somewhere in your code myLinkedList.pool(), it would fail.
> > In fact it fails earlier, as the saved value can't be coerced to a
> > LinkedList.
> >
> > I think this is important as you expect the saveState to return exactly
> > what you saved, and not a object with a different type.
> >
> > In my applications, I have a base backing bean that uses this scheme :
> >
> > public class BaseFace implements Serializable {
> >
> > public LinkedList<Object> getStates(){
> > return new LinkedList<Object>();
> > }
> >
> > /**
> > * Restore the states.
> > */
> > public void setStates(@SuppressWarnings("unused")
> > LinkedList<Object> states) throws Exception{
> > // NoOp
> > }
> > ...
> > }
> >
> > It's quite nice because at you can extend this class easily and still have
> > an easy job saving / restoring the state :
> >
> > public class BaseFaceForObjectWithUnid<T extends ObjectWithUnid> extends
> > BaseFace {
> > ...
> >
> > @Override
> > public LinkedList<Object> getStates(){
> > LinkedList<Object> states = super.getStates();
> > states.add( object == null ? null : object.getUnid() );
> > return states;
> > }
> >
> > @Override
> > public void setStates(LinkedList<Object> states) throws Exception{
> > super.setStates(states);
> > setUnid(states == null ? null : (String)states.poll());
> > }
> > ...
> > }
> >
> > It worked fine until the related UIComponentBase had been modified.
> > So I limited the case for ArrayList.
> >
> > But I still have problems with this
> > UIComponentBase.saveAttachedState code, as I guess similar
> > problems with other types (like a custom class implementing List) could
> > arrise.
> > What I understand is that we need this "trick" to check if the List doesn't
> > contain any StateHolder.
> > If I'm right and if we still need that, then maybe we can do a recursive
> > check for any StateHolder object, and if there is none, then just serialize
> > the object.
> >
> > By the way, it's not exactly clear for me why we need to have a special
> > treatment for StateHolders.
> > Could you clarify this for me please ?
> >
> > Thanks,
> >
> > Sylvain.
> >
> >
> >
> > On Wed, 2005-07-27 at 21:48 +0200, Manfred Geiler wrote:
> > Sylvain,
> > This change does not make much sense IMHO.
> > Why do you check for ArrayList instead of List. There is only a List
> > cast after that, so why not just check for List?
> > Is there anything a LinkedList does not correctly implement from the
> > List interface?!
> >
> > -Manfred
> >
> > 2005/7/26, svieujot@apache.org <sv...@apache.org>:
> > > Author: svieujot
> > > Date: Tue Jul 26 08:25:26 2005
> > > New Revision: 225327
> > >
> > > URL: http://svn.apache.org/viewcvs?rev=225327&view=rev
> > > Log:
> > > Bugfix for savestate with a linkedlist (couldn't coerce a value of type
> > "java.util.ArrayList" to type "java.util.LinkedList".
> > >
> > > Modified:
> > >
> > myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > >
> > > Modified:
> > myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > > URL:
> > http://svn.apache.org/viewcvs/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java?rev=225327&r1=225326&r2=225327&view=diff
> > >
> > ==============================================================================
> > > ---
> > myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > (original)
> > > +++
> > myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > Tue Jul 26 08:25:26 2005
> > > @@ -593,9 +593,9 @@
> > > Object attachedObject)
> > > {
> > > if (attachedObject == null) return null;
> > > - if (attachedObject instanceof List)
> > > + if (attachedObject instanceof ArrayList)
> > > {
> > > - ArrayList lst = new
> > ArrayList(((List)attachedObject).size());
> > > + List lst = new
> > ArrayList(((List)attachedObject).size());
> > > for (Iterator it = ((List)attachedObject).iterator();
> > it.hasNext(); )
> > > {
> > > lst.add(saveAttachedState(context, it.next()));
> > >
> > >
> > >
> >
> >
Re: svn commit: r225327 - /myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Manfred Geiler <ma...@gmail.com>.
Just had a look at the spec (i.e. javadoc) and the RI source code:
How we handled the List was absolutely correct IMO.
The problem you have, would also arise with the RI. So, I do not so a
reason to do this "trick" in the MyFaces implementation. The drawback
of your patch is: If someone has a List other than an ArrayList the
method will fail.
-Manfred
2005/8/3, Sylvain Vieujot <sv...@apache.org>:
> Hello Manfred,
>
> Here is a specific example where savestate would fail :
> <x:saveState value="#{myLinkedList}/>
>
> The save state would return a List, and not a LinkedList.
> So if you use somewhere in your code myLinkedList.pool(), it would fail.
> In fact it fails earlier, as the saved value can't be coerced to a
> LinkedList.
>
> I think this is important as you expect the saveState to return exactly
> what you saved, and not a object with a different type.
>
> In my applications, I have a base backing bean that uses this scheme :
>
> public class BaseFace implements Serializable {
>
> public LinkedList<Object> getStates(){
> return new LinkedList<Object>();
> }
>
> /**
> * Restore the states.
> */
> public void setStates(@SuppressWarnings("unused")
> LinkedList<Object> states) throws Exception{
> // NoOp
> }
> ...
> }
>
> It's quite nice because at you can extend this class easily and still have
> an easy job saving / restoring the state :
>
> public class BaseFaceForObjectWithUnid<T extends ObjectWithUnid> extends
> BaseFace {
> ...
>
> @Override
> public LinkedList<Object> getStates(){
> LinkedList<Object> states = super.getStates();
> states.add( object == null ? null : object.getUnid() );
> return states;
> }
>
> @Override
> public void setStates(LinkedList<Object> states) throws Exception{
> super.setStates(states);
> setUnid(states == null ? null : (String)states.poll());
> }
> ...
> }
>
> It worked fine until the related UIComponentBase had been modified.
> So I limited the case for ArrayList.
>
> But I still have problems with this
> UIComponentBase.saveAttachedState code, as I guess similar
> problems with other types (like a custom class implementing List) could
> arrise.
> What I understand is that we need this "trick" to check if the List doesn't
> contain any StateHolder.
> If I'm right and if we still need that, then maybe we can do a recursive
> check for any StateHolder object, and if there is none, then just serialize
> the object.
>
> By the way, it's not exactly clear for me why we need to have a special
> treatment for StateHolders.
> Could you clarify this for me please ?
>
> Thanks,
>
> Sylvain.
>
>
>
> On Wed, 2005-07-27 at 21:48 +0200, Manfred Geiler wrote:
> Sylvain,
> This change does not make much sense IMHO.
> Why do you check for ArrayList instead of List. There is only a List
> cast after that, so why not just check for List?
> Is there anything a LinkedList does not correctly implement from the
> List interface?!
>
> -Manfred
>
> 2005/7/26, svieujot@apache.org <sv...@apache.org>:
> > Author: svieujot
> > Date: Tue Jul 26 08:25:26 2005
> > New Revision: 225327
> >
> > URL: http://svn.apache.org/viewcvs?rev=225327&view=rev
> > Log:
> > Bugfix for savestate with a linkedlist (couldn't coerce a value of type
> "java.util.ArrayList" to type "java.util.LinkedList".
> >
> > Modified:
> >
> myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> >
> > Modified:
> myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > URL:
> http://svn.apache.org/viewcvs/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java?rev=225327&r1=225326&r2=225327&view=diff
> >
> ==============================================================================
> > ---
> myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> (original)
> > +++
> myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> Tue Jul 26 08:25:26 2005
> > @@ -593,9 +593,9 @@
> > Object attachedObject)
> > {
> > if (attachedObject == null) return null;
> > - if (attachedObject instanceof List)
> > + if (attachedObject instanceof ArrayList)
> > {
> > - ArrayList lst = new
> ArrayList(((List)attachedObject).size());
> > + List lst = new
> ArrayList(((List)attachedObject).size());
> > for (Iterator it = ((List)attachedObject).iterator();
> it.hasNext(); )
> > {
> > lst.add(saveAttachedState(context, it.next()));
> >
> >
> >
>
>
Re: svn commit: r225327 -
/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
Posted by Sylvain Vieujot <sv...@apache.org>.
Hello Manfred,
Here is a specific example where savestate would fail :
<x:saveState value="#{myLinkedList}/>
The save state would return a List, and not a LinkedList.
So if you use somewhere in your code myLinkedList.pool(), it would fail.
In fact it fails earlier, as the saved value can't be coerced to a
LinkedList.
I think this is important as you expect the saveState to return exactly
what you saved, and not a object with a different type.
In my applications, I have a base backing bean that uses this scheme :
public class BaseFace implements Serializable {
public LinkedList<Object> getStates(){
return new LinkedList<Object>();
}
/**
* Restore the states.
*/
public void setStates(@SuppressWarnings("unused")
LinkedList<Object> states) throws Exception{
// NoOp
}
...
}
It's quite nice because at you can extend this class easily and still
have an easy job saving / restoring the state :
public class BaseFaceForObjectWithUnid<T extends ObjectWithUnid>
extends BaseFace {
...
@Override
public LinkedList<Object> getStates(){
LinkedList<Object> states = super.getStates();
states.add( object == null ? null : object.getUnid() );
return states;
}
@Override
public void setStates(LinkedList<Object> states) throws
Exception{
super.setStates(states);
setUnid(states == null ? null : (String)states.poll());
}
...
}
It worked fine until the related UIComponentBase had been modified.
So I limited the case for ArrayList.
But I still have problems with this UIComponentBase.saveAttachedState
code, as I guess similar problems with other types (like a custom class
implementing List) could arrise.
What I understand is that we need this "trick" to check if the List
doesn't contain any StateHolder.
If I'm right and if we still need that, then maybe we can do a recursive
check for any StateHolder object, and if there is none, then just
serialize the object.
By the way, it's not exactly clear for me why we need to have a special
treatment for StateHolders.
Could you clarify this for me please ?
Thanks,
Sylvain.
On Wed, 2005-07-27 at 21:48 +0200, Manfred Geiler wrote:
> Sylvain,
> This change does not make much sense IMHO.
> Why do you check for ArrayList instead of List. There is only a List
> cast after that, so why not just check for List?
> Is there anything a LinkedList does not correctly implement from the
> List interface?!
>
> -Manfred
>
> 2005/7/26, svieujot@apache.org <sv...@apache.org>:
> > Author: svieujot
> > Date: Tue Jul 26 08:25:26 2005
> > New Revision: 225327
> >
> > URL: http://svn.apache.org/viewcvs?rev=225327&view=rev
> > Log:
> > Bugfix for savestate with a linkedlist (couldn't coerce a value of type "java.util.ArrayList" to type "java.util.LinkedList".
> >
> > Modified:
> > myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> >
> > Modified: myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java
> > URL: http://svn.apache.org/viewcvs/myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java?rev=225327&r1=225326&r2=225327&view=diff
> > ==============================================================================
> > --- myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java (original)
> > +++ myfaces/api/trunk/src/java/javax/faces/component/UIComponentBase.java Tue Jul 26 08:25:26 2005
> > @@ -593,9 +593,9 @@
> > Object attachedObject)
> > {
> > if (attachedObject == null) return null;
> > - if (attachedObject instanceof List)
> > + if (attachedObject instanceof ArrayList)
> > {
> > - ArrayList lst = new ArrayList(((List)attachedObject).size());
> > + List lst = new ArrayList(((List)attachedObject).size());
> > for (Iterator it = ((List)attachedObject).iterator(); it.hasNext(); )
> > {
> > lst.add(saveAttachedState(context, it.next()));
> >
> >
> >