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

Add ability to compress and skip unserializable session attributes #10747

Merged

Conversation

janbartel
Copy link
Contributor

Based on features suggested for PR #10740, add the ability for SessionData subclasses to provide ObjectOutputStream and ObjectInputStream to customize how sessions are serialized and deserialized. In particular, this will allow for compression of session data, and also the optional skipping of non-serializable session attributes.

@janbartel janbartel requested a review from gregw October 18, 2023 04:50
@janbartel janbartel self-assigned this Oct 18, 2023
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the fundamental mechanism, but find it a bit complex of how it could be used. Specifically JdbcTestHelper suggests that a whole helper class is needed which could perhaps be done by better utilities in the main classes.

The fact that all SessionDataStores do not use this mechanism, suggest that perhaps another subclass of AbstractSessionDataStore is needed. Perhaps a SerializedSessionDataStore to indicate that serialization is used.

It then feels like we could definitely offer a SerializedSessionDataStore.Compressed subclass that wraps any other SerializedSessionDataStore and does the stream wrapping for compression.

If the replaceObject method was delegated to a protected method on SerializedSessionDataStore, then it too could be intercepted with a wrapper provided by us.

@janbartel
Copy link
Contributor Author

I like the fundamental mechanism, but find it a bit complex of how it could be used. Specifically JdbcTestHelper suggests that a whole helper class is needed which could perhaps be done by better utilities in the main classes.

The JdbcTestHelper is just a helper for the test classes to make it easier to set up a database, and do some extra queries for test purposes.

To use the new feature in embedded mode, you would simply just create an instance of your extended JDBCSessionDataStore class and set that on your context:

   DefaultSessionCache sessionCache = new DefaultSessionCache(sessionManager);
  sessionCache.setSessionDataStore(new MyJDBCSessionDataStore());

If you want to use a SessionDataStoreFactory to automatically create instances of your extended JDBCSessionDataStore as necessary for each new context, you just have to extend the JDBCSessionDataStoreFactory and override the getSessionDataStore() method to return a new instance of your class rather than the standard JDBCSessionDataStore.

If you want to do this via a module, then of course you need to write a .mod file.

The fact that all SessionDataStores do not use this mechanism, suggest that perhaps another subclass of AbstractSessionDataStore is needed. Perhaps a SerializedSessionDataStore to indicate that serialization is used.
Maybe, I'll explore.

It then feels like we could definitely offer a SerializedSessionDataStore.Compressed subclass that wraps any other SerializedSessionDataStore and does the stream wrapping for compression.
Perhaps we could offer the gzip style of compression as an option, however I'm sure there are many more types of compression that users may be interested in.

If the replaceObject method was delegated to a protected method on SerializedSessionDataStore, then it too could be intercepted with a wrapper provided by us.
Maybe, I'll explore.

@janbartel janbartel requested a review from gregw October 23, 2023 23:40
gregw
gregw previously approved these changes Oct 25, 2023
@janbartel janbartel merged commit a5f06fc into jetty-12.0.x Oct 26, 2023
4 checks passed
@janbartel janbartel deleted the jetty-12.0.x-compressing-nonserializable-sessions branch October 26, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants