Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB #9625

vpavic · 2017-06-28T13:48:12Z

This resolves #9552.

Changes are mostly a revert of 33dd9d6 adopted with ones from #9554. There are however a few other changes:

new dependency management version spring-session-mongodb.version which is currently set to 2.0.0.BUILD-SNAPSHOT (next milestone should be 2.0.0.M2)
MongoDB related configuration properties are renamed to mongodb to be consistent with naming (module was previously named spring-session-data-mongo, now is spring-session-data-mongodb)
MongoDB related tests are moved to SessionAutoConfigurationMongoTests

Depending on release date of Spring Session MongoDB 2.0.0.M2 this could be targeted at Boot's 2.0.0.M3 (#9552 is currently targeted at 2.0.0.M4).

/cc @rwinch @gregturn

gregturn · 2017-06-28T14:58:29Z

Looks great!

philwebb · 2017-06-28T16:26:00Z

See also #9552 (comment) with some discussion about how we should deal with Spring Session now that it has been split up.

fmcejudo · 2017-07-10T19:06:09Z

.../src/main/java/org/springframework/boot/autoconfigure/session/MongoSessionConfiguration.java

+			Integer timeout = sessionProperties.getTimeout();
+			if (timeout != null) {
+				setMaxInactiveIntervalInSeconds(timeout);
+			}


as a suggestion, to be a bit more java 8 and fluent, you might use: Optional.ofNullable(sessionProperties.getTimeout()).ifPresent(this::setMaxInactiveIntervalInSeconds);

wilkinsona · 2017-09-21T12:41:40Z

Sorry, @vpavic. I'd missed that we had this PR for #9552. What I've done is largely the same as what you'd proposed here. One notable difference is that I've kept the store type as Mongo rather than MongoDB. That was purely to avoid a change from what we had in 1.5. If the Spring Session team feel that we should go with the DB suffix that I'm not opposed to doing so.

vpavic · 2017-09-21T12:41:46Z

I've rebased the changes from this PR on top of changes from c00a42f.

Basically the only difference is naming concerns from bullet no. 2 of the opening comment:

MongoDB related configuration properties are renamed to mongodb to be consistent with naming (module was previously named spring-session-mongo, now is spring-session-mongodb)

Those could still be worth getting in, both for reasons above and consistency with spring.data.mongodb.

wilkinsona · 2017-09-21T12:43:56Z

Reopening to consider the Mongo -> MongoDb renaming

wilkinsona · 2017-09-21T12:50:07Z

While the module is now named spring-session-mongodb its classes all just use Mongo (@EnableMongoHttpSession for example). Using Mongo rather than MongoDb is also consistent with Spring Data MongoDB's class names (MongoOperations for example) and Boot's existing Mongo-related classes (MongoAutoConfiguration and MongoProperties for example). For these reasons I think we should stick with Mongo rather than MongoDb.

vpavic · 2017-09-21T16:00:13Z

OK, I agree that class names should stay as they are. But my primary concern was actually StoreType#MONGO vs StoreType#MONGODB and configuration properties namespace spring.session.mongo vs spring.session.mongodb. Spring Data MongoDB's namespace is spring.data.mongodb.

wilkinsona · 2017-09-22T05:38:17Z

Hmm, you make a compelling point regarding the property prefix. Things are inconsistent at the moment and I don't like that.

wilkinsona · 2017-09-22T05:44:41Z

In summary, our configuration property prefix for Spring Data MongoDB is currently (in both 1.5.x and master) spring.data.mongodb whereas the prefix for Spring Session MongoDB is spring.session.mongo. We also have a MONGO value in the StoreType enum.

I think we should make this consistent. The proposal here is to align by changing the Spring Session-related names. Specifically, the prefix would become spring.session.mongodb and MONGO would become MONGODB in the StoreType enum. The is a breaking change from 1.5. Given the other Spring Session changes in 2.0, making a breaking change to our Spring Session support rather than our Spring Data MongoDB support feels to me like the right thing to do. Thoughts?

vpavic · 2017-09-22T05:58:07Z

Thanks for reconsidering this. I've update the PR to remove the class renaming noise - it should now more clearly reflect the changes that are under consideration.

wilkinsona · 2017-09-22T06:04:55Z

spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc

@@ -427,8 +427,8 @@ content into your application; rather pick only the properties that you need.
 	spring.session.jdbc.schema=classpath:org/springframework/session/jdbc/schema-@@platform@@.sql # Path to the SQL file to use to initialize the database schema.
 	spring.session.jdbc.table-name=SPRING_SESSION # Name of database table used to store sessions.

-	# SPRING SESSION MONGO ({sc-spring-boot-autoconfigure}/session/MonogoSessionProperties.{sc-ext}[MongoSessionProperties])
-	spring.session.mongo.collection-name=sessions # Collection name used to store sessions.
+	# SPRING SESSION MONGODB ({sc-spring-boot-autoconfigure}/session/MongoDbSessionProperties.{sc-ext}[MongoDbSessionProperties])


This should stay as MongoSessionProperties

snicoll · 2017-09-25T07:19:48Z

Specifically, the prefix would become spring.session.mongodb and MONGO would become MONGODB in the StoreType enum

+1 to that. Sorry, we discussed this before and didn't catch that inconsistency either :(

wilkinsona · 2017-09-25T10:03:49Z

Thanks, @vpavic. I've merged this into master.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 28, 2017

philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 6, 2017

fmcejudo reviewed Jul 10, 2017

View reviewed changes

vpavic mentioned this pull request Sep 21, 2017

Consider reinstating auto-configuration for Spring Session Mongo DB #9552

Closed

vpavic force-pushed the gh-9552 branch from f39a7e7 to 58910d6 Compare September 21, 2017 12:38

wilkinsona closed this Sep 21, 2017

wilkinsona reopened this Sep 21, 2017

wilkinsona changed the title ~~Reinstate Spring Session MongoDB auto-config~~ Use MongoDB rather than Mongo in class names and properties for Spring Session MongoDB Sep 21, 2017

wilkinsona closed this Sep 21, 2017

wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed priority: normal type: enhancement A general enhancement labels Sep 21, 2017

wilkinsona reopened this Sep 22, 2017

wilkinsona added priority: normal type: enhancement A general enhancement and removed status: declined A suggestion or change that we don't feel we should currently apply labels Sep 22, 2017

wilkinsona changed the title ~~Use MongoDB rather than Mongo in class names and properties for Spring Session MongoDB~~ Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB Sep 22, 2017

wilkinsona added for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Sep 22, 2017

vpavic force-pushed the gh-9552 branch from 58910d6 to 43fc12a Compare September 22, 2017 05:58

wilkinsona reviewed Sep 22, 2017

View reviewed changes

Improve Spring Session MongoDB support naming

ea48022

vpavic force-pushed the gh-9552 branch from 43fc12a to ea48022 Compare September 22, 2017 06:10

wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Sep 25, 2017

wilkinsona added this to the 2.0.0.M5 milestone Sep 25, 2017

wilkinsona removed the status: waiting-for-triage An issue we've not yet triaged label Sep 25, 2017

wilkinsona self-assigned this Sep 25, 2017

wilkinsona closed this Sep 25, 2017

vpavic deleted the gh-9552 branch September 25, 2017 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB #9625

Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB #9625

Uh oh!

vpavic commented Jun 28, 2017 •

edited

Loading

Uh oh!

gregturn commented Jun 28, 2017

Uh oh!

philwebb commented Jun 28, 2017

Uh oh!

fmcejudo Jul 10, 2017

Uh oh!

wilkinsona commented Sep 21, 2017

Uh oh!

vpavic commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 21, 2017 •

edited

Loading

Uh oh!

vpavic commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 22, 2017

Uh oh!

wilkinsona commented Sep 22, 2017

Uh oh!

vpavic commented Sep 22, 2017

Uh oh!

wilkinsona Sep 22, 2017

Uh oh!

snicoll commented Sep 25, 2017

Uh oh!

wilkinsona commented Sep 25, 2017

Uh oh!

Uh oh!

Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB #9625

Consistent property prefixes for Spring Session MongoDB and Spring Data MongoDB #9625

Uh oh!

Conversation

vpavic commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gregturn commented Jun 28, 2017

Uh oh!

philwebb commented Jun 28, 2017

Uh oh!

fmcejudo Jul 10, 2017

Choose a reason for hiding this comment

Uh oh!

wilkinsona commented Sep 21, 2017

Uh oh!

vpavic commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vpavic commented Sep 21, 2017

Uh oh!

wilkinsona commented Sep 22, 2017

Uh oh!

wilkinsona commented Sep 22, 2017

Uh oh!

vpavic commented Sep 22, 2017

Uh oh!

wilkinsona Sep 22, 2017

Choose a reason for hiding this comment

Uh oh!

snicoll commented Sep 25, 2017

Uh oh!

wilkinsona commented Sep 25, 2017

Uh oh!

Uh oh!

vpavic commented Jun 28, 2017 •

edited

Loading

wilkinsona commented Sep 21, 2017 •

edited

Loading