Skip to content

Refactor userTypeResolver construction into separate method #1297

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

Refactor userTypeResolver construction into separate method #1297

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions ...c/main/java/org/springframework/data/cassandra/config/AbstractCassandraConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
* @author Matthew T. Adams
* @author John Blum
* @author Mark Paluch
* @author Ammar Khaku
*/
@Configuration
@SuppressWarnings("unused")
Expand All @@ -71,14 +72,11 @@ public CassandraConverter cassandraConverter() {

CqlSession cqlSession = getRequiredSession();

UserTypeResolver userTypeResolver =
new SimpleUserTypeResolver(cqlSession, CqlIdentifier.fromCql(getKeyspaceName()));

MappingCassandraConverter converter =
new MappingCassandraConverter(requireBeanOfType(CassandraMappingContext.class));

converter.setCodecRegistry(cqlSession.getContext().getCodecRegistry());
converter.setUserTypeResolver(userTypeResolver);
converter.setUserTypeResolver(userTypeResolver(cqlSession));
converter.setCustomConversions(requireBeanOfType(CassandraCustomConversions.class));

return converter;
Expand All @@ -96,11 +94,8 @@ public CassandraMappingContext cassandraMapping() throws ClassNotFoundException

CqlSession cqlSession = getRequiredSession();

UserTypeResolver userTypeResolver =
new SimpleUserTypeResolver(cqlSession, CqlIdentifier.fromCql(getKeyspaceName()));

CassandraMappingContext mappingContext =
new CassandraMappingContext(userTypeResolver, SimpleTupleTypeFactory.DEFAULT);
new CassandraMappingContext(userTypeResolver(cqlSession), SimpleTupleTypeFactory.DEFAULT);

CustomConversions customConversions = requireBeanOfType(CassandraCustomConversions.class);

Expand Down Expand Up @@ -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()));
Copy link
Contributor Author

@akhaku akhaku Aug 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

}
}