From 54fed8ac5fcd683032cb63bea53957e0b893a3b2 Mon Sep 17 00:00:00 2001 From: mhyeon-lee Date: Sun, 16 Feb 2020 00:43:35 +0900 Subject: [PATCH] DATAJDBC-510 - Inject IdentifierProcessing into BasicJdbcConverter. Before the `IdentifierProcessing` was hardcoded and therefore did not match a properly configured `Dialect`. Original pull request: #192. --- .../jdbc/core/convert/BasicJdbcConverter.java | 26 ++++++++++++++++--- .../config/AbstractJdbcConfiguration.java | 9 ++++--- .../repository/config/JdbcConfiguration.java | 5 ++-- .../DefaultDataAccessStrategyUnitTests.java | 8 +++--- .../convert/EntityRowMapperUnitTests.java | 3 ++- .../SimpleJdbcRepositoryEventsUnitTests.java | 7 +++-- .../data/jdbc/testing/TestConfiguration.java | 7 +++-- 7 files changed, 48 insertions(+), 17 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java index 5454618cf..926da94bd 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java @@ -37,7 +37,6 @@ import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.relational.core.conversion.BasicRelationalConverter; import org.springframework.data.relational.core.conversion.RelationalConverter; -import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.Embedded; import org.springframework.data.relational.core.mapping.Embedded.OnEmpty; import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension; @@ -71,14 +70,14 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc private static final Converter, Map> ITERABLE_OF_ENTRY_TO_MAP_CONVERTER = new IterableOfEntryToMapConverter(); private final JdbcTypeFactory typeFactory; - private final IdentifierProcessing identifierProcessing = HsqlDbDialect.INSTANCE.getIdentifierProcessing(); + private final IdentifierProcessing identifierProcessing; private final RelationResolver relationResolver; /** * Creates a new {@link BasicRelationalConverter} given {@link MappingContext} and a * {@link JdbcTypeFactory#unsupported() no-op type factory} throwing {@link UnsupportedOperationException} on type - * creation. Use {@link #BasicJdbcConverter(MappingContext, RelationResolver, CustomConversions, JdbcTypeFactory)} + * creation. Use {@link #BasicJdbcConverter(MappingContext, RelationResolver, CustomConversions, JdbcTypeFactory, IdentifierProcessing)} * (MappingContext, RelationResolver, JdbcTypeFactory)} to convert arrays and large objects into JDBC-specific types. * * @param context must not be {@literal null}. @@ -94,6 +93,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc this.relationResolver = relationResolver; this.typeFactory = JdbcTypeFactory.unsupported(); + this.identifierProcessing = IdentifierProcessing.ANSI; } /** @@ -104,17 +104,37 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc * @param typeFactory must not be {@literal null} * @since 1.1 */ + @Deprecated public BasicJdbcConverter( MappingContext, ? extends RelationalPersistentProperty> context, RelationResolver relationResolver, CustomConversions conversions, JdbcTypeFactory typeFactory) { + this(context, relationResolver, conversions, typeFactory, IdentifierProcessing.ANSI); + } + + /** + * Creates a new {@link BasicRelationalConverter} given {@link MappingContext}. + * + * @param context must not be {@literal null}. + * @param relationResolver used to fetch additional relations from the database. Must not be {@literal null}. + * @param typeFactory must not be {@literal null} + * @param identifierProcessing must not be {@literal null} + * @since 2.0 + */ + public BasicJdbcConverter( + MappingContext, ? extends RelationalPersistentProperty> context, + RelationResolver relationResolver, CustomConversions conversions, JdbcTypeFactory typeFactory, + IdentifierProcessing identifierProcessing) { + super(context, conversions); Assert.notNull(typeFactory, "JdbcTypeFactory must not be null"); Assert.notNull(relationResolver, "RelationResolver must not be null"); + Assert.notNull(identifierProcessing, "IdentifierProcessing must not be null"); this.relationResolver = relationResolver; this.typeFactory = typeFactory; + this.identifierProcessing = identifierProcessing; } @Nullable diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java index 58e20d6c6..46b962cfa 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.NamingStrategy; -import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; /** @@ -48,6 +47,7 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; * @author Mark Paluch * @author Michael Simons * @author Christoph Strobl + * @author Myeonghyeon Lee * @since 1.1 */ @Configuration(proxyBeanMethods = false) @@ -80,17 +80,18 @@ public class AbstractJdbcConfiguration { */ @Bean public JdbcConverter jdbcConverter(JdbcMappingContext mappingContext, NamedParameterJdbcOperations operations, - @Lazy RelationResolver relationResolver, JdbcCustomConversions conversions) { + @Lazy RelationResolver relationResolver, JdbcCustomConversions conversions, Dialect dialect) { DefaultJdbcTypeFactory jdbcTypeFactory = new DefaultJdbcTypeFactory(operations.getJdbcOperations()); - return new BasicJdbcConverter(mappingContext, relationResolver, conversions, jdbcTypeFactory); + return new BasicJdbcConverter(mappingContext, relationResolver, conversions, jdbcTypeFactory, + dialect.getIdentifierProcessing()); } /** * Register custom {@link Converter}s in a {@link JdbcCustomConversions} object if required. These * {@link JdbcCustomConversions} will be registered with the - * {@link #jdbcConverter(JdbcMappingContext, NamedParameterJdbcOperations, RelationResolver, JdbcCustomConversions)}. + * {@link #jdbcConverter(JdbcMappingContext, NamedParameterJdbcOperations, RelationResolver, JdbcCustomConversions, Dialect)}. * Returns an empty {@link JdbcCustomConversions} instance by default. * * @return will never be {@literal null}. diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java index a0d9a6edd..3d96e19d8 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java @@ -48,6 +48,7 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; * @author Mark Paluch * @author Michael Simons * @author Christoph Strobl + * @author Myeonghyeon Lee * @deprecated Use {@link AbstractJdbcConfiguration} instead. */ @Configuration @@ -79,9 +80,9 @@ public class JdbcConfiguration { */ @Bean public RelationalConverter relationalConverter(RelationalMappingContext mappingContext, - @Lazy RelationResolver relationalResolver) { + @Lazy RelationResolver relationalResolver, Dialect dialect) { return new BasicJdbcConverter(mappingContext, relationalResolver, jdbcCustomConversions(), - JdbcTypeFactory.unsupported()); + JdbcTypeFactory.unsupported(), dialect.getIdentifierProcessing()); } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java index ba04d0310..0b1c41b6d 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java @@ -49,6 +49,7 @@ import org.springframework.jdbc.support.KeyHolder; * * @author Jens Schauder * @author Mark Paluch + * @author Myeonghyeon Lee */ public class DefaultDataAccessStrategyUnitTests { @@ -72,7 +73,7 @@ public class DefaultDataAccessStrategyUnitTests { Dialect dialect = HsqlDbDialect.INSTANCE; converter = new BasicJdbcConverter(context, relationResolver, new JdbcCustomConversions(), - new DefaultJdbcTypeFactory(jdbcOperations)); + new DefaultJdbcTypeFactory(jdbcOperations), dialect.getIdentifierProcessing()); accessStrategy = new DefaultDataAccessStrategy( // new SqlGeneratorSource(context, converter, dialect), // context, // @@ -115,12 +116,13 @@ public class DefaultDataAccessStrategyUnitTests { DelegatingDataAccessStrategy relationResolver = new DelegatingDataAccessStrategy(); + Dialect dialect = HsqlDbDialect.INSTANCE; JdbcConverter converter = new BasicJdbcConverter(context, relationResolver, new JdbcCustomConversions(Arrays.asList(BooleanToStringConverter.INSTANCE, StringToBooleanConverter.INSTANCE)), - new DefaultJdbcTypeFactory(jdbcOperations)); + new DefaultJdbcTypeFactory(jdbcOperations), dialect.getIdentifierProcessing()); DefaultDataAccessStrategy accessStrategy = new DefaultDataAccessStrategy( // - new SqlGeneratorSource(context, converter, HsqlDbDialect.INSTANCE), // + new SqlGeneratorSource(context, converter, dialect), // context, // converter, // namedJdbcOperations); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java index b957859d2..9e3ec785f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java @@ -59,6 +59,7 @@ import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.domain.Identifier; import org.springframework.data.repository.query.Param; import org.springframework.util.Assert; @@ -692,7 +693,7 @@ public class EntityRowMapperUnitTests { .findAllByPath(identifierOfValue(ID_FOR_ENTITY_REFERENCING_LIST), any(PersistentPropertyPath.class)); BasicJdbcConverter converter = new BasicJdbcConverter(context, accessStrategy, new JdbcCustomConversions(), - JdbcTypeFactory.unsupported()); + JdbcTypeFactory.unsupported(), IdentifierProcessing.ANSI); return new EntityRowMapper<>( // (RelationalPersistentEntity) context.getRequiredPersistentEntity(type), // diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java index 22f9cad49..79086e777 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java @@ -47,6 +47,7 @@ import org.springframework.data.jdbc.core.convert.SqlGeneratorSource; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; import org.springframework.data.jdbc.repository.support.SimpleJdbcRepository; +import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.event.AfterDeleteEvent; @@ -72,6 +73,7 @@ import org.springframework.lang.Nullable; * @author Oliver Gierke * @author Myeonghyeon Lee * @author Milan Milanov + * @author Myeonghyeon Lee */ public class SimpleJdbcRepositoryEventsUnitTests { @@ -86,9 +88,10 @@ public class SimpleJdbcRepositoryEventsUnitTests { RelationalMappingContext context = new JdbcMappingContext(); NamedParameterJdbcOperations operations = createIdGeneratingOperations(); DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy(); + Dialect dialect = HsqlDbDialect.INSTANCE; JdbcConverter converter = new BasicJdbcConverter(context, delegatingDataAccessStrategy, new JdbcCustomConversions(), - new DefaultJdbcTypeFactory(operations.getJdbcOperations())); - SqlGeneratorSource generatorSource = new SqlGeneratorSource(context, converter, HsqlDbDialect.INSTANCE); + new DefaultJdbcTypeFactory(operations.getJdbcOperations()), dialect.getIdentifierProcessing()); + SqlGeneratorSource generatorSource = new SqlGeneratorSource(context, converter, dialect); this.dataAccessStrategy = spy(new DefaultDataAccessStrategy(generatorSource, context, converter, operations)); delegatingDataAccessStrategy.setDelegate(dataAccessStrategy); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java index e08f40e74..d82dee460 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java @@ -54,6 +54,7 @@ import org.springframework.transaction.PlatformTransactionManager; * @author Jens Schauder * @author Mark Paluch * @author Fei Dong + * @author Myeonghyeon Lee */ @Configuration @ComponentScan // To pick up configuration classes (per activated profile) @@ -110,13 +111,15 @@ public class TestConfiguration { @Bean JdbcConverter relationalConverter(RelationalMappingContext mappingContext, @Lazy RelationResolver relationResolver, - CustomConversions conversions, @Qualifier("namedParameterJdbcTemplate") NamedParameterJdbcOperations template) { + CustomConversions conversions, @Qualifier("namedParameterJdbcTemplate") NamedParameterJdbcOperations template, + Dialect dialect) { return new BasicJdbcConverter( // mappingContext, // relationResolver, // conversions, // - new DefaultJdbcTypeFactory(template.getJdbcOperations()) // + new DefaultJdbcTypeFactory(template.getJdbcOperations()), // + dialect.getIdentifierProcessing() ); } }