Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: added serialization support #37

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

davidef
Copy link

@davidef davidef commented Nov 25, 2022

All components added into a j2ee HttpSession must to be serializable.
This is also needed when using the editor in clustered environments to migrate active sessions between nodes.

@F0rce
Copy link
Owner

F0rce commented Nov 25, 2022

Hello @davidef

Thanks for making this PR. I really don't understand the motiviation / goal you had. Could you introduce me to the changes you made, that I understand it before I merge it, as I had no problems so far using @SuppressWarnings("serial"). Maybe thats just a Java Thing I never had to cope with.

@F0rce F0rce self-assigned this Nov 25, 2022
@davidef
Copy link
Author

davidef commented Dec 20, 2022

@F0rce sorry for the delay I've missed your reply. Objects in http session are required to be serializable when you replicate http session in a clustered environment. For example our application is running on 3 tomcat instances and usually we have sticky session so clients will always hit the same tomcat server. But when we deploy an update we update the servers one-by-one to prevent any downtime of our app; session will be serialized by the server that is being updated and deserialized on a different active server. Without the class being serializable this will fail.
This is the reason because there is a compiler warning.
Also when updating the code in the future the serialVersionUID need to be updated when there are changes in the object model to allow the cluster to detect that. Usually this is required if there are chances to class's fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants