Support adapting both query and parameters in JDBC SessionRepository

[runeflobakk]

Expected Behavior

Wish for JdbcIndexedSessionRepository:
Customize both the queries and how parameters are set on the PreparedStatement.

Current Behavior

Only the query strings are customizable by their respective setter-method (e.g. setCreateSessionQuery(String)), while populating the PreparedStatement with parameters specified by the query is fixed, e.g. inside JdbcSession.save().

While this certainly enables customizing the queries to use applicable per-database specific features, the queries themselves are implicitly tied to the parameters and their order, and may not offer the desired flexibility to adapt the SQL queries executed by the JdbcIndexedSessionRepository.

Context

I have the need to being able to invalidate/delete a session on-demand, but identified by another key than what is the SESSION_ID. The session identifier typically is assigned by another system to provide SSO, and this SSO system may request my application, which uses Spring Session, to remove a session.

Currently I work around this by using a separate table for this alternative session identifier, with reference to spring_session.session_id, and on session invalidate request, delete the applicable row in this auxillary table, resolve the Spring Session ID, and invoke SessionRepository.deleteById. This works, but I need to also maintain a separate mechanism for TTL and expiry of the rows in my auxillary session table.

It would be nice if I could just include this separate candidate key in the table schema for spring_session, and rely on the expiry mechanism and other mechanisms already present in Spring Session.

Implementation

The idea is to couple the query and how the PreparedStatement is populated, and this would enable me to adapt which and how columns are populated when creating/updating a session. My idea is to put the auxillary session identifier on the jakarta.servlet.http.HttpSession. This will of course store the identifier as a regular attribute in spring_session_attributes (which is all fine), but it also enables me to customize the JdbcIndexedSessionRepository to be aware of this particular session attribute, and include it as a value for the particular column where I want to store it. Of course I would need to adapt any DDL for spring_session to include this column, applicable index, etc. PostgreSqlJdbcIndexedSessionRepositoryAdaptedQueriesITests demonstrates this, where the spring_session table is altered to include a new custom column CANDIDATE_ID, and instead of using the existing setCreateSessionQuery, a new method adaptCreateSessionQuery is proposed to set both the query and customizing the PreparedStatement using a ParameterizedPreparedStatementSetter.

Contribution proposal

I have implemented a proposed solution to this in a fork, which I'd like to suggest as an general approach to support this use-case, and of course also to change according to any feedback if this can be regarded as a candidate for contribution to Spring Session.

The main changes can be seen in JdbcIndexedSessionRepository, where I, as a PoC, have changed the existing private CREATE_SESSION_QUERY and UPDATE_SESSION_QUERY to "bundles" with both the query, and PreparedStaement populating logic, as well as retrofitting their existing use. And the accompanying new methods to configure this using adaptCreateSessionQuery, adaptUpdateSessionQuery.

If this is regarded as an interesting contribution, I will of course make the implementation complete for all the queries, as well as any changes necessary to the code.

Can this proposal be an applicable contribution to Spring Session?
Thank you!

New API

This demonstrates the API I have in mind:

Setting the query string, and an additional parameter without affecting the existing parameters

sessionRepository.adaptCreateSessionQuery(query -> query
    .replaceQuery("INSERT INTO %TABLE_NAME% ( .. ) VALUES (?, ?, ... ?)")
    .setAdditionalParams((ps, session) -> ps.setString(8, session.getAttribute("my_custom_key"))))

Setting both the query string, and how all parameters are set

sessionRepository.adaptCreateSessionQuery(query -> new QueryAndParams(
    "INSERT INTO %TABLE_NAME% ( .. ) VALUES (?, ?, ... ?)",
    (ps, session) -> {
            ps.setString(1, JdbcSession.this.primaryKey);
            // the other "standard" spring_session columns
            ps.setString(8, session.getAttribute("my_custom_key")));
        })
)

Where session being a JdbcSession. This is of course also possible to adapt if it is considered to not fit into existing paradigms of the Spring Session API.

[marcusdacoregio]

Hi, @runeflobakk, Thanks for the report.

It is not clear to me yet why you need an auxiliary table instead of adding your attribute as a session attribute itself.
If you do something like httpSession.setAttribute("myAttribute", "value"), then it would be saved into spring_session_attributes, allowing you to select from that table instead of your auxiliary table.

This works, but I need to also maintain a separate mechanism for TTL and expiry of the rows in my auxillary session table.

Why is that? If you have a relationship with the spring_session table by using the session_id you can create a foreign constraint with ON DELETE CASCADE, this way the rows in your auxiliary table would be deleted when the related row in the spring_session table is deleted.

[runeflobakk]

Thanks for your reply @marcusdacoregio

The reason for this is that I need to be able to lookup the session by this custom identifier, and the value of session attributes are serialized to bytes (ATTRIBUTE_BYTES BYTEA NOT NULL). I can not lookup by a bytea value, at least it seems a bit fragile to serialize the identifier and then use it in WHERE spring_session_attributes.attribute_name AND spring_session_attributes.attribute_value = :custom_identifier_as_bytea.

When needing to invalidate the session on-demand, this happens outside the actual user session (it is a backend request from another system), so I cannot lookup the session by the spring session ID itself, because it is not available. I may of course have overlooked something here, but this is my understanding. :)

It is a good point about ON DELETE CASCADE, and is something I will investigate for our use-case.

Do you have any thought on being able to customize both the query (as is supported now), and additionally being able to customize the associated PreparedStatement parameters?

[marcusdacoregio]

I can not lookup by a bytea value

Do you mean that it is not possible or you are concerned about performance implications? You can probably decode the bytea field using some function from your database provider and there is probably a way to index that to have a better performance. If not, can you provide the details?

You can also customize the ConversionService that is used to convert the attribute value. This way you have better control over the resulting value in the database column. The documentation is pretty light in that regard, we can work together to make that better if it works for you.

[runeflobakk]

Do you mean that it is not possible or you are concerned about performance implications?

Well, yes, concerned about performance issues, but probably first and foremost because it felt a bit... "icky" to use a bytea as storing a lookup key. It would be nice to have this identifier as a first class citizen in the database, and being able to look up particular sessions outside of our application for any troubleshooting, not depending on how the value is serialized before persisted.
Any performance issues can maybe be solved with an index as with typically any column used for lookup-keys. I don't know if there is anything out-of-ordinary with indexing a bytea column, but I guess there shouldn't be. Using a bytea column for lookup I would maybe think is a bit unusual, though. It feels a bit like a hack. We were considering this when we did our implementation, but decided to put it into it's own table to be able to treat the auxiliary identifier more like a "first class citizen" value in the database. As we would probably do if we designed our own persisted session management.

Starting to look into the implementation of JdbcIndexedSessionRepository, I identified that the query-string, even though being customizable, is still tightly coupled with the indexes of the parameters set on the prepared statement. And this appeared to me to be a potentially valuable contribution, which would offer more flexibility with customizing the queries, is not tied to my use-case in particular, but could help me realize it in the manner I would ideally prefer. If you for any reason (not just my use-case) would need to customize the queries in a way that do not align with the fixed PreparedStatement parameters, then the current facilities do not allow that.

I am fully aware that this of course also enables shooting oneself in the foot. But I feel you are already somewhat there if you start to customize the queries, and they are implicitly tied to implementation details, and this enables actually to loosen this coupling, if needed.

Thank you for taking the time to get an understand of my rationale. I hope I am somewhat able to convey what my motivations are here :)

[runeflobakk]

The link you provided for customizing the ConversionService, is interesting. I need to read it, but I would prefer to store my custom session identifier in it's own column of an appropriate type, not just convert any attribute value to something else than a bytea. (I have other stuff on my session, which is nice to use the generic serializing facilities for).
Disclaimer: I have not studied the doc on customizing ConversionService yet.

[marcusdacoregio]

I see what you are saying, but I do not know what precedents we would open if we allowed that level of customization on the repository. If we would do that, I think another option would be to open the implementation and allow the users to extend the repository in order to provide their own stuff.

In other words, if we provide a strategy to customize how the queries are created/prepared, there are other places that we need to open, like the mapping from database <-> object. At first, it sounds like that wouldn't be too different than allowing the implementation to be extended.

Given your use case, I would say that investigating the ON DELETE CASCADE option seems like the best approach.

It is important to mention that you do not need your column to be bytea, if you provide a custom ConversionService that serializes objects to data that can be stored in another datatype, that's fine.

That said, I'd rather leave this issue open for the time being to check if we have more feedback and more use cases from the community before making such change.

[runeflobakk]

Absolutely! I think this is all fine! I was experimenting with a kind of "what if"-mentality, discovered this possibility, and primarily wanted to share my thoughts. If anything, it serves as feedback for further development.

Thanks again!