+1 on sean schofields patch

I never felt happy with SerializedView beeing serializable.
SerializedView was serializable long ago, then we changed it to be according to the spec and removed it again. Now, I remember once again, why I removed the Serializable interface at that time - thanks sean. ;-)
Well, at the beginning of this new discussion I had a weird feeling and a deja vu, but I could not remember what was the true reason I removed the marker at that time. So, I added Serializable again, but - of course - we should rather fix the cause and not the symptom. ;-)


Manfred


Matthias Wessendorf schrieb:
Since I yesterday tested Kalle's suggestion.
I just tested Sean's patch.

Well, I removed the changes, suggested in Kalle's
email and included Sean's patch.

Also that works too.

I am now +1 on commiting this.

Any thoughts?


-----Original Message-----
From: Sean Schofield [mailto:[EMAIL PROTECTED] Sent: Sunday, January 16, 2005 6:30 PM
To: MyFaces Development
Subject: MYFACES-81 concerning serialization and violation of JSF spec



Here is a copy of my original bug report for: http://issues.apache.org/jira/browse/MYFACES-81


The JIRA entry contains a patch to fix the problem.

========================================

StateManager.SerializedView should not implement Serializable

This inner class is not serializable in the 1.1 specification and the fact that it is in the MyFaces implementation is a violation of the specification.

It appears that this class was made serializable in order to implement server-side state saving. Not only is this a violation of the spec, but it does not even work properly. If the servlet container attempts to serialize the session it throws an exception. This is because the SerializedView class is not serializable even though it claims to implement this interface. Because it is an inner class, it contains an implicit reference to the parent class (in this case, this reference ends up being JspStateManagerImpl.) Serialization fails because this reference cannot be serialized (nor should it be.)

I have found another mechanism for handling server side state saving that does not violate the spec. I've included a patch that will fix this problem.

The solution is to include a private inner class called _SerializedView that can implement serializable without violating the spec. It has the same properties as the SerializedView class and according to the spec, these properties will be serializable (b/c they are ultimately generated from classes that implement StateHolder.) I store this class in the session instead and everything works fine.

I believe the spec should change so that the SerializedView class is its own standalone class that does implement serializable. That way, implementations can use this class for both the client-side and server-side state saving. That was what was being attempted in the MyFaces implementation but this approach can't work b/c of the missing serializable interface on SerializedView (and the implicit reference to the parent class.) If the spec were changed, we could get rid of this extra _SerializedView class that we are introducing and use the proposed new class instead.





Reply via email to