Refactor userTypeResolver construction into separate method #1297
Conversation
akhaku
commented
Aug 20, 2022
akhaku
commented
- You have read the Spring Data contribution guidelines.
- You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
- You submit test cases (unit or integration tests) that back your changes.
- You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).
This allows for easy overriding, for example in case you wish to use a different implementation.
| @@ -260,4 +255,8 @@ protected KeyspacePopulator keyspacePopulator() { | |||
| protected ByteArrayResource scriptOf(String content) { | |||
| return new ByteArrayResource(content.getBytes()); | |||
| } | |||
|
|
|||
| protected UserTypeResolver userTypeResolver(CqlSession cqlSession) { | |||
| return new SimpleUserTypeResolver(cqlSession, CqlIdentifier.fromCql(getKeyspaceName())); | |||
There was a problem hiding this comment.
I should mention that really what I'm trying to do is allow getKeyspaceName() to be null, but right now the docs in AbstractSessionConfiguration are explicit that it should be non-null... We already override the session factory bean and the only piece that's causing problems with the null keyspace right now is the UserTypeResolver, so moving the constructor to a protected method will let me override it without trying to unravel getKeyspace()'s nullability in AbstractSessionConfiguration
There was a problem hiding this comment.
Spring Boot follows the same approach, it would make sense to address this issue as well, see https://github.com/spring-projects/spring-boot/blob/19a7fee1d7d08c081ae1c9f4e4033c8dd71e0bd5/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/cassandra/CassandraDataAutoConfiguration.java#L100.
There was a problem hiding this comment.
Thank you for merging and polishing! I'm not entirely sure what you're getting at, are you just saying that this change is reasonable or that we should explore making a change in spring-boot's CassandraDataAutoConfiguration as well? Or are you saying it would be worth exploring allowing getKeyspace to return null?
spring-projects-issues
added
the
status: waiting-for-triage
|
@mp911de I'd love a blessing if you have a moment! Pretty small change, shouldn't have any impact. |
mp911de
added
type: enhancement
This allows for easy overriding, for example in case you wish to use a different implementation. See #1297
This allows for easy overriding, for example in case you wish to use a different implementation. See #1297
|
Thank you for your contribution. That's merged, polished, and backported now. |
