Skip to content

Validate Spring Session database initializer configuration #6649

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

Closed

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Aug 15, 2016

This PR adds Spring Session JDBC configuration validation that disables database initializer in case custom table name is configured with default schema.

Currently, it is possible to set custom table name but leave default schema configuration property. This will either result in an error in runtime since expected table (with custom name) won't be found, or, if user has set up the correct table by some other means, with created unused table.

In both cases the correct approach would be to fail fast and indicate invalid configuration.

Jdbc jdbcProperties = this.sessionProperties.getJdbc();
if (jdbcProperties.getInitializer().isEnabled()) {
if (Jdbc.DEFAULT_SCHEMA_LOCATION.equals(jdbcProperties.getSchema()) &&
!Jdbc.DEFAULT_TABLE_NAME.equals(jdbcProperties.getTableName())) {
Copy link
Member

Choose a reason for hiding this comment

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

I see no reason to move that business in @PostConstruct. You're not using the state of the (Configuration) class but solely the state of SessionProperties so you could just as well move that in SessionProperties itself if that's an invalid state (more on this later).

This commit adds Spring Session JDBC configuration validation that disables database initializer in case custom table name is configured with default schema.
@vpavic vpavic force-pushed the improve-session-jdbc-autoconfig branch from 394f74c to 4d2d7b0 Compare August 17, 2016 21:24
@vpavic
Copy link
Contributor Author

vpavic commented Aug 18, 2016

I've updated the PR according to feedback.

@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 11, 2016
@snicoll snicoll added this to the 1.4.1 milestone Sep 11, 2016
@snicoll snicoll self-assigned this Sep 11, 2016
snicoll pushed a commit that referenced this pull request Sep 12, 2016
This commit adds Spring Session JDBC configuration validation that
disables database initializer in case custom table name is configured
with default schema.

See gh-6649
@snicoll snicoll closed this in a347a78 Sep 12, 2016
snicoll added a commit that referenced this pull request Sep 12, 2016
* pr/6649:
  Polish contribution
  Validate Spring Session database initializer configuration
@snicoll
Copy link
Member

snicoll commented Sep 12, 2016

Thanks @vpavic, this is now merged in master.

@vpavic
Copy link
Contributor Author

vpavic commented Sep 12, 2016

Thanks @snicoll, I like the approach from a347a78. 👍

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

Successfully merging this pull request may close these issues.

4 participants