I was reading #6649 and thought that since the fix got lost in the 2.1 merge (its here but not here) - why not support it fully. I can submit the original fix if that is preferable, but at work we use Mongo and i thought it was nice that you could customize the collection name, so thought it would work the same way on jdbc.
I don't mind submitting a PR to restore the intended functionality either, but making it fully functional seems preferable if possible.
i figure the fix could look something like this
diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/JdbcSessionDataSourceScriptDatabaseInitializer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/JdbcSessionDataSourceScriptDatabaseInitializer.java
index a5279ecb45..4669897a74 100644
--- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/JdbcSessionDataSourceScriptDatabaseInitializer.java
+++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/session/JdbcSessionDataSourceScriptDatabaseInitializer.java
@@ -16,6 +16,9 @@
package org.springframework.boot.autoconfigure.session;
+import java.io.IOException;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
import java.util.List;
import javax.sql.DataSource;
@@ -24,6 +27,8 @@ import org.springframework.boot.jdbc.DatabaseDriver;
import org.springframework.boot.jdbc.init.DataSourceScriptDatabaseInitializer;
import org.springframework.boot.jdbc.init.PlatformPlaceholderDatabaseDriverResolver;
import org.springframework.boot.sql.init.DatabaseInitializationSettings;
+import org.springframework.core.io.ByteArrayResource;
+import org.springframework.core.io.Resource;
import org.springframework.util.StringUtils;
/**
@@ -38,6 +43,9 @@ import org.springframework.util.StringUtils;
*/
public class JdbcSessionDataSourceScriptDatabaseInitializer extends DataSourceScriptDatabaseInitializer {
+ private final DatabaseInitializationSettings settings;
+ private JdbcSessionProperties properties;
+
/**
* Create a new {@link JdbcSessionDataSourceScriptDatabaseInitializer} instance.
* @param dataSource the Spring Session JDBC data source
@@ -46,6 +54,7 @@ public class JdbcSessionDataSourceScriptDatabaseInitializer extends DataSourceSc
*/
public JdbcSessionDataSourceScriptDatabaseInitializer(DataSource dataSource, JdbcSessionProperties properties) {
this(dataSource, getSettings(dataSource, properties));
+ this.properties = properties;
}
/**
@@ -57,6 +66,32 @@ public class JdbcSessionDataSourceScriptDatabaseInitializer extends DataSourceSc
public JdbcSessionDataSourceScriptDatabaseInitializer(DataSource dataSource,
DatabaseInitializationSettings settings) {
super(dataSource, settings);
+ this.settings = settings;
+ }
+
+ @Override
+ protected void runScripts(Scripts scripts) {
+ // if the table name is not the default table name, replace table name in scripts
+ if (!properties.getTableName().equals(new JdbcSessionProperties().getTableName())) {
+ scripts = fixSchemaWithCorrectTableName(scripts);
+ }
+
+ super.runScripts(scripts);
+ }
+
+ private Scripts fixSchemaWithCorrectTableName(Scripts scripts) {
+ Charset cs = settings.getEncoding() != null ? scripts.getEncoding() : Charset.defaultCharset();
+ ArrayList<Resource> resources = new ArrayList<>();
+ for (Resource resource : scripts) {
+ try {
+ resources.add(new ByteArrayResource(new String(resource.getInputStream().readAllBytes(), cs)
+ .replaceAll(new JdbcSessionProperties().getTableName(), properties.getTableName())
+ .getBytes(cs)));
+ } catch (IOException e) {
+ resources.add(resource);
+ }
+ }
+ return new Scripts(resources);
}
/**
as the core implementation, with tests and such to follow, if the approach seems generally acceptable.
thanks for reading!
Comment From: philwebb
I'm a little worried about doing a brute force search/replace on the schema-<vendor>.sql files. I wonder if it would be better for Spring Session to ship .sql files that use %TABLE_NAME% in the same way as they do in the queries defined in JdbcIndexedSessionRepository.
@vpavic might have some thoughts.
Comment From: spring-projects-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: alexanderankin
Would a pr to restore the previous functionality be welcome or would that also require discussion? I am still interested in the main idea but I think in the meantime it shouldn't be left like this as it's misleading
On Tue, Jan 17, 2023, 7:42 PM Spring Projects Issues ***@***.*** wrote:
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
— Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-boot/issues/33748#issuecomment-1386294754, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACECGJADYGVAILOONXMDX23WS436FANCNFSM6AAAAAATXMMPOY . You are receiving this because you authored the thread.Message ID: @.***>
Comment From: philwebb
@alexanderankin I agree that it's currently confusing and a PR would be most welcome. We can keep this issue for an enhancement to fully support things.
I think things have moved on a bit from when https://github.com/spring-projects/spring-boot/commit/dd3d2ad35e8011622899293528dd89b8fd886d65 was committed.
IMO If the initialization is going to happen and tableName has been changed then we should throw an exception if schema has not also been changed.
In the meantime, I've raised https://github.com/spring-projects/spring-session/issues/2225 to see if Spring Session will provide templated versions of the SQL.
Comment From: alexanderankin
I've added some code here - https://github.com/spring-projects/spring-boot/compare/main...alexanderankin:spring-boot:33748_validate-default-table-on-jdbc-session-init?expand=1 - to implement this idea:
IMO If the initialization is going to happen and tableName has been changed then we should throw an exception if schema has not also been changed.
eagerly awaiting news about what the team will think of the idea to support %-templating in the ddl's as well.
Comment From: philwebb
https://github.com/spring-projects/spring-session/pull/2257 has been merged so we now have a utility method to get the script with a different name.
Comment From: wilkinsona
IMO If the initialization is going to happen and tableName has been changed then we should throw an exception if schema has not also been changed.
I've just declined #33961 as I disagree with this. As was discussed on #6649, we can't be certain that a custom table name and the default scheme won't work. The user could have provided custom org/springframework/session/jdbc/schema-*.sql files in src/main/resources that override those in the spring-session-jdbc jar. If we failed fast, it would break this arrangement.
https://github.com/spring-projects/spring-session/pull/2257 has been merged so we now have a utility method to get the script with a different name.
There's a slight risk that using the utility method when the user's provided custom org/springframework/session/jdbc/schema-*.sql files will be a breaking change but it's quite unlikely, and certainly lower than the risk of failing fast being a breaking change. I think we can make the change to use the utility method when the table name has been customised as long as we describe it in the release notes and suggest that users delete their custom scripts if all they were doing is changing the table name.