-
Notifications
You must be signed in to change notification settings - Fork 2k
Align JdbcChatMemoryRepositoryProperties with other modules in Spring Boot #3107
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,17 +17,21 @@ | |
| package org.springframework.ai.model.chat.memory.repository.jdbc.autoconfigure; | ||
|
|
||
| import org.springframework.boot.context.properties.ConfigurationProperties; | ||
| import org.springframework.boot.sql.init.DatabaseInitializationMode; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have decided that the core of Spring AI will not depend on spring boot, for the chat repository work, we created our own enum for this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oops, my bad, I mean thtat for the non autoconfig code. |
||
|
|
||
| /** | ||
| * @author Jonathan Leijendekker | ||
| * @author Thomas Vitale | ||
| * @author Yanming Zhou | ||
| * @since 1.0.0 | ||
| */ | ||
| @ConfigurationProperties(JdbcChatMemoryRepositoryProperties.CONFIG_PREFIX) | ||
| public class JdbcChatMemoryRepositoryProperties { | ||
|
|
||
| public static final String CONFIG_PREFIX = "spring.ai.chat.memory.repository.jdbc"; | ||
|
|
||
| private static final String DEFAULT_SCHEMA_LOCATION = "classpath:org/springframework/ai/chat/memory/jdbc/schema-@@platform@@.sql"; | ||
|
|
||
| /** | ||
| * Whether to initialize the schema on startup. Values: embedded, always, never. | ||
| * Default is embedded. | ||
|
|
@@ -38,7 +42,13 @@ public class JdbcChatMemoryRepositoryProperties { | |
| * Locations of schema (DDL) scripts. Supports comma-separated list. Default is | ||
| * classpath:org/springframework/ai/chat/memory/jdbc/schema-@@platform@@.sql | ||
| */ | ||
| private String schema = "classpath:org/springframework/ai/chat/memory/jdbc/schema-@@platform@@.sql"; | ||
| private String schema = DEFAULT_SCHEMA_LOCATION; | ||
|
|
||
| /** | ||
| * Platform to use in initialization scripts if the @@platform@@ placeholder is used. | ||
| * Auto-detected by default. | ||
| */ | ||
| private String platform; | ||
|
|
||
| public DatabaseInitializationMode getInitializeSchema() { | ||
| return this.initializeSchema; | ||
|
|
@@ -48,6 +58,14 @@ public void setInitializeSchema(DatabaseInitializationMode initializeSchema) { | |
| this.initializeSchema = initializeSchema; | ||
| } | ||
|
|
||
| public String getPlatform() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch. |
||
| return platform; | ||
| } | ||
|
|
||
| public void setPlatform(String platform) { | ||
| this.platform = platform; | ||
| } | ||
|
|
||
| public String getSchema() { | ||
| return this.schema; | ||
| } | ||
|
|
@@ -56,23 +74,4 @@ public void setSchema(String schema) { | |
| this.schema = schema; | ||
| } | ||
|
|
||
| public enum DatabaseInitializationMode { | ||
|
|
||
| /** | ||
| * Always initialize the database. | ||
| */ | ||
| ALWAYS, | ||
|
|
||
| /** | ||
| * Only initialize an embedded database. | ||
| */ | ||
| EMBEDDED, | ||
|
|
||
| /** | ||
| * Never initialize the database. | ||
| */ | ||
| NEVER | ||
|
|
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,71 +22,38 @@ | |
|
|
||
| import org.springframework.boot.jdbc.init.DataSourceScriptDatabaseInitializer; | ||
| import org.springframework.boot.jdbc.init.PlatformPlaceholderDatabaseDriverResolver; | ||
| import org.springframework.boot.sql.init.DatabaseInitializationMode; | ||
| import org.springframework.boot.sql.init.DatabaseInitializationSettings; | ||
| import org.springframework.util.StringUtils; | ||
|
|
||
| /** | ||
| * Performs database initialization for the JDBC Chat Memory Repository. | ||
| * | ||
| * @author Mark Pollack | ||
| * @author Yanming Zhou | ||
| * @since 1.0.0 | ||
| */ | ||
| class JdbcChatMemoryRepositorySchemaInitializer extends DataSourceScriptDatabaseInitializer { | ||
|
|
||
| private static final String DEFAULT_SCHEMA_LOCATION = "classpath:org/springframework/ai/chat/memory/jdbc/schema-@@platform@@.sql"; | ||
|
|
||
| JdbcChatMemoryRepositorySchemaInitializer(DataSource dataSource, JdbcChatMemoryRepositoryProperties properties) { | ||
| super(dataSource, getSettings(dataSource, properties)); | ||
| } | ||
|
|
||
| static DatabaseInitializationSettings getSettings(DataSource dataSource, | ||
| JdbcChatMemoryRepositoryProperties properties) { | ||
| var settings = new DatabaseInitializationSettings(); | ||
|
|
||
| // Determine schema locations | ||
| String schemaProp = properties.getSchema(); | ||
| List<String> schemaLocations; | ||
| PlatformPlaceholderDatabaseDriverResolver resolver = new PlatformPlaceholderDatabaseDriverResolver(); | ||
| try { | ||
| String url = dataSource.getConnection().getMetaData().getURL().toLowerCase(); | ||
| if (url.contains("hsqldb")) { | ||
| schemaLocations = List.of("classpath:org/springframework/ai/chat/memory/jdbc/schema-hsqldb.sql"); | ||
| } | ||
| else if (StringUtils.hasText(schemaProp)) { | ||
| schemaLocations = resolver.resolveAll(dataSource, schemaProp); | ||
| } | ||
| else { | ||
| schemaLocations = resolver.resolveAll(dataSource, DEFAULT_SCHEMA_LOCATION); | ||
| } | ||
| } | ||
| catch (Exception e) { | ||
| // fallback to default | ||
| if (StringUtils.hasText(schemaProp)) { | ||
| schemaLocations = resolver.resolveAll(dataSource, schemaProp); | ||
| } | ||
| else { | ||
| schemaLocations = resolver.resolveAll(dataSource, DEFAULT_SCHEMA_LOCATION); | ||
| } | ||
| } | ||
| settings.setSchemaLocations(schemaLocations); | ||
|
|
||
| // Determine initialization mode | ||
| JdbcChatMemoryRepositoryProperties.DatabaseInitializationMode init = properties.getInitializeSchema(); | ||
| DatabaseInitializationMode mode; | ||
| if (JdbcChatMemoryRepositoryProperties.DatabaseInitializationMode.ALWAYS.equals(init)) { | ||
| mode = DatabaseInitializationMode.ALWAYS; | ||
| } | ||
| else if (JdbcChatMemoryRepositoryProperties.DatabaseInitializationMode.NEVER.equals(init)) { | ||
| mode = DatabaseInitializationMode.NEVER; | ||
| } | ||
| else { | ||
| // embedded or default | ||
| mode = DatabaseInitializationMode.EMBEDDED; | ||
| } | ||
| settings.setMode(mode); | ||
| settings.setSchemaLocations(resolveSchemaLocations(dataSource, properties)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was not able to get the hsqldb autoconfiguration working with this, which is why there is all this code testing the URL. I agree this code in main needs to be improved, but without some more work on a better test that shows the hsqldb can automagically be configured, the current hack works.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what I did wrong, as I had added all this extra code. Your PR is working, though i can't figure out what I did wrong ;) |
||
| settings.setMode(properties.getInitializeSchema()); | ||
| settings.setContinueOnError(true); | ||
| return settings; | ||
| } | ||
|
|
||
| private static List<String> resolveSchemaLocations(DataSource dataSource, | ||
| JdbcChatMemoryRepositoryProperties properties) { | ||
| PlatformPlaceholderDatabaseDriverResolver platformResolver = new PlatformPlaceholderDatabaseDriverResolver(); | ||
| if (StringUtils.hasText(properties.getPlatform())) { | ||
| return platformResolver.resolveAll(properties.getPlatform(), properties.getSchema()); | ||
| } | ||
| return platformResolver.resolveAll(dataSource, properties.getSchema()); | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should merge this part into main