Update flush mode related Spring Session configuration properties

[vpavic]

As of Spring Session Corn-M3 support for configuring flush mode for reactive Redis sessions is deprecated due to removal of immediate mode support.

See spring-projects/spring-session#1441.

PR also contains upgrade to Corn snapshots - this would be nice to have on Boot side as we'll have a few configuration refinements for the upcoming M3 release. I'll provide PRs for Boot side as those happen in Spring Session.

[snicoll]

Thanks for the PR. If the property is specific to servlet-based deployment, perhaps we should indicate that it has no effect for reactive-based apps or something?

[vpavic]

Thanks for the feedback. If you have a way to indicate that this property doesn't have effect on reactive session store, that would be nice to have.

Note that we have the same situation from the very beginning with spring.session.redis.cleanup-cron that is used in imperative session store but not reactive counterpart.

[snicoll]

If you have a way to indicate that this property doesn't have effect on reactive session store, that would be nice to have.

There is nothing of the sort but perhaps the description of the key should state that? Flagging for team attention to see what the rest of the team thinks.

[wilkinsona]

I wonder if we should create a separate group for Servlet-specific properties? Something like spring.session.redis.servlet perhaps? Or, if we think other stores are likely to have Servlet-specific configuration, it could be spring.session.servlet.redis.

[snicoll]

I like that. @vpavic can you please help us figure this out? It really depends how many properties/store are affected.

[vpavic]

A case could be made for both grouping by stack or by data store first, as some properties will naturally reflect the underlying data store, while others will reflect the stack. The flush mode is something that will be limited to imperative i.e. Servlet implementations.

Personally I like the stack first, data store second (spring.session.servlet.redis) as it feels more natural and more aligned with other places (e.g. spring.thymeleaf.). Also:

• there already are some spring.session.servlet. prefixed properties - filter-order and filter-dispatcher-types
• IIRC spring.session.timeout was introduced solely for reactive support as there's no direct natural counterpart to server.servlet.session.timeout

[vpavic]

I've added a commit to this PR that handles deprecation of RedisFlushMode and HazelcastFlushMode in favor of new, common, FlushMode enum (see spring-projects/spring-session#1465).

It would be nice to get the first commit to master (upgrade to snapshots) as I'm planning to do more changes on Spring Session's config infrastructure and follow them up with PRs against Spring Boot.

Perhaps it would be good to hold off any major restructuring of properties until that effort is completed.

[wilkinsona]

It would be nice to get the first commit to master (upgrade to snapshots)

I think we need more than just the first commit. An upgrade to snapshots alone results in some test failures:

[ERROR] Failures: 
[ERROR]   ReactiveSessionAutoConfigurationRedisTests.defaultConfig:51->lambda$validateSpringSessionUsesRedis$0:78 
Expecting
  <org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository@38389b00>
to have a property or a field named <"redisFlushMode">
[ERROR]   ReactiveSessionAutoConfigurationRedisTests.defaultConfigWithUniqueStoreImplementation:59->lambda$validateSpringSessionUsesRedis$0:78 
Expecting
  <org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository@7c77bc86>
to have a property or a field named <"redisFlushMode">
[ERROR]   ReactiveSessionAutoConfigurationRedisTests.redisSessionStoreWithCustomizations:69->lambda$validateSpringSessionUsesRedis$0:78 
Expecting
  <org.springframework.session.data.redis.ReactiveRedisOperationsSessionRepository@7b02ef87>
to have a property or a field named <"redisFlushMode">
[ERROR]   SessionAutoConfigurationHazelcastTests.customFlushMode:84->lambda$customFlushMode$1:87 
Expecting
  <org.springframework.session.hazelcast.HazelcastSessionRepository@13b9f811>
to have a property or a field named <"hazelcastFlushMode">
[ERROR]   SessionAutoConfigurationRedisTests.defaultConfig:67->lambda$validateSpringSessionUsesRedis$0:124 
Expecting
  <org.springframework.session.data.redis.RedisOperationsSessionRepository@ff696a1>
to have a property or a field named <"redisFlushMode">
[ERROR]   SessionAutoConfigurationRedisTests.defaultConfigWithUniqueStoreImplementation:78->lambda$validateSpringSessionUsesRedis$0:124 
Expecting
  <org.springframework.session.data.redis.RedisOperationsSessionRepository@50bdf4d2>
to have a property or a field named <"redisFlushMode">
[ERROR]   SessionAutoConfigurationRedisTests.redisSessionStoreWithCustomizations:88->lambda$validateSpringSessionUsesRedis$0:124 
Expecting
  <org.springframework.session.data.redis.RedisOperationsSessionRepository@32d855a4>
to have a property or a field named <"redisFlushMode">

What's the smallest set of changes that we need to move onto snapshots and avoid deprecation warnings please, @vpavic? Is it all three of the commits proposed here?

[vpavic]

Ah, yes - sorry @wilkinsona for the misleading comments. Since Spring Boot's tests use reflection to verify the state of configured that will fail since internally some member names have changed. I'll restructure commits on this PR shortly so that each commit is properly buildable.

[vpavic]

Actually, scratch that - since tests rely on reflection rather than public API (changes there are backwards compatible) it's difficult to split these 3 commit and get individual as stable states.

Either way, I'd prefer to get this in early and do the restructuring of properties later on. We're still in milestone phase so it shouldn't be an issue.

[wilkinsona]

Sounds good to me. I'll squash these three and get them into master.