From 795e24451161645188a036c773ebc7ed7b1115d9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 5 Oct 2022 11:26:46 +0200 Subject: [PATCH] Polishing. Do not expose setForeignKeyNaming methods on NamingStrategy to make less assumptions about how a naming strategy gets implemented. Provide getRequiredLeafEntity method on PersistentPropertyPathExtension to reduce null and state assertion checks. Refine getTableName/getQualifiedTableName approach to reduce API surface and avoid deprecations. See #1147 Original pull request: #1324. --- .../jdbc/core/convert/BasicJdbcConverter.java | 5 +- .../data/jdbc/core/convert/SqlContext.java | 4 +- .../data/jdbc/core/convert/SqlGenerator.java | 10 ++-- .../jdbc/repository/query/SqlContext.java | 4 +- ...sistentPropertyPathExtensionUnitTests.java | 14 ++--- ...GeneratorFixedNamingStrategyUnitTests.java | 24 ++++---- .../core/convert/SqlGeneratorUnitTests.java | 44 ++++++++------- .../DefaultReactiveDataAccessStrategy.java | 2 +- .../data/r2dbc/core/R2dbcEntityTemplate.java | 12 ++-- .../core/mapping/CachingNamingStrategy.java | 5 -- .../core/mapping/DefaultNamingStrategy.java | 18 ++---- .../core/mapping/NamingStrategy.java | 23 +++----- .../PersistentPropertyPathExtension.java | 40 +++++++++++-- .../mapping/RelationalMappingContext.java | 19 +------ .../mapping/RelationalPersistentEntity.java | 19 ++----- .../RelationalPersistentEntityImpl.java | 42 ++++++-------- .../query/SimpleRelationalEntityMetadata.java | 2 +- .../MappingRelationalEntityInformation.java | 2 +- ...lationalPersistentEntityImplUnitTests.java | 56 +++++++++---------- 19 files changed, 163 insertions(+), 182 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 12f6bb1e..74a80e06 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 @@ -25,6 +25,7 @@ import java.util.Optional; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.convert.ConverterNotFoundException; @@ -498,9 +499,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc private boolean hasInstanceValues(@Nullable Object idValue) { - RelationalPersistentEntity persistentEntity = path.getLeafEntity(); - - Assert.state(persistentEntity != null, "Entity must not be null"); + RelationalPersistentEntity persistentEntity = path.getRequiredLeafEntity(); for (RelationalPersistentProperty embeddedProperty : persistentEntity) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java index b0326aec..7d19eb0b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java @@ -37,7 +37,7 @@ class SqlContext { SqlContext(RelationalPersistentEntity entity) { this.entity = entity; - this.table = Table.create(entity.getFullTableName()); + this.table = Table.create(entity.getQualifiedTableName()); } Column getIdColumn() { @@ -55,7 +55,7 @@ class SqlContext { Table getTable(PersistentPropertyPathExtension path) { SqlIdentifier tableAlias = path.getTableAlias(); - Table table = Table.create(path.getTableName()); + Table table = Table.create(path.getQualifiedTableName()); return tableAlias == null ? table : table.as(tableAlias); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index ec7027ec..7193a019 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -133,7 +133,7 @@ class SqlGenerator { return rootCondition.apply(filterColumn); } - Table subSelectTable = Table.create(parentPath.getTableName()); + Table subSelectTable = Table.create(parentPath.getQualifiedTableName()); Column idColumn = subSelectTable.column(parentPath.getIdColumnName()); Column selectFilterColumn = subSelectTable.column(parentPath.getEffectiveIdColumnName()); @@ -208,6 +208,7 @@ class SqlGenerator { * @param parentIdentifier name of the column of the FK back to the referencing entity. * @param propertyPath used to determine if the property is ordered and if there is a key column. * @return a SQL String. + * @since 3.0 */ String getFindAllByProperty(Identifier parentIdentifier, PersistentPropertyPath propertyPath) { @@ -706,7 +707,7 @@ class SqlGenerator { private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path, Function rootCondition) { - Table table = Table.create(path.getTableName()); + Table table = Table.create(path.getQualifiedTableName()); DeleteBuilder.DeleteWhere builder = Delete.builder() // .from(table); @@ -935,7 +936,7 @@ class SqlGenerator { private SelectBuilder.SelectOrdered applyQueryOnSelect(Query query, MapSqlParameterSource parameterSource, SelectBuilder.SelectWhere selectBuilder) { - Table table = Table.create(this.entity.getFullTableName()); + Table table = Table.create(this.entity.getQualifiedTableName()); SelectBuilder.SelectOrdered selectOrdered = query // .getCriteria() // @@ -1098,8 +1099,7 @@ class SqlGenerator { if (!property.isWritable()) { readOnlyColumnNames.add(columnName); } - - if (property.isInsertOnly()) { + if (property.isInsertOnly()) { insertOnlyColumnNames.add(columnName); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java index b9559cec..f9fa94a4 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java @@ -38,7 +38,7 @@ class SqlContext { SqlContext(RelationalPersistentEntity entity) { this.entity = entity; - this.table = Table.create(entity.getFullTableName()); + this.table = Table.create(entity.getQualifiedTableName()); } Column getIdColumn() { @@ -56,7 +56,7 @@ class SqlContext { Table getTable(PersistentPropertyPathExtension path) { SqlIdentifier tableAlias = path.getTableAlias(); - Table table = Table.create(path.getTableName()); + Table table = Table.create(path.getQualifiedTableName()); return tableAlias == null ? table : table.as(tableAlias); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java index 62ef1766..3ace05fc 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java @@ -101,13 +101,13 @@ public class PersistentPropertyPathExtensionUnitTests { assertSoftly(softly -> { - softly.assertThat(extPath(entity).getTableName()).isEqualTo(quoted("DUMMY_ENTITY")); - softly.assertThat(extPath("second").getTableName()).isEqualTo(quoted("SECOND")); - softly.assertThat(extPath("second.third2").getTableName()).isEqualTo(quoted("SECOND")); - softly.assertThat(extPath("second.third2.value").getTableName()).isEqualTo(quoted("SECOND")); - softly.assertThat(extPath("secondList.third2").getTableName()).isEqualTo(quoted("SECOND")); - softly.assertThat(extPath("secondList.third2.value").getTableName()).isEqualTo(quoted("SECOND")); - softly.assertThat(extPath("secondList").getTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath(entity).getQualifiedTableName()).isEqualTo(quoted("DUMMY_ENTITY")); + softly.assertThat(extPath("second").getQualifiedTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath("second.third2").getQualifiedTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath("second.third2.value").getQualifiedTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath("secondList.third2").getQualifiedTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath("secondList.third2.value").getQualifiedTableName()).isEqualTo(quoted("SECOND")); + softly.assertThat(extPath("secondList").getQualifiedTableName()).isEqualTo(quoted("SECOND")); }); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java index 9ce9c85d..85dc5b8e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java @@ -23,8 +23,8 @@ import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; -import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; @@ -37,9 +37,9 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp * @author Greg Turnquist * @author Mark Paluch */ -public class SqlGeneratorFixedNamingStrategyUnitTests { +class SqlGeneratorFixedNamingStrategyUnitTests { - final NamingStrategy fixedCustomTablePrefixStrategy = new NamingStrategy() { + private final NamingStrategy fixedCustomTablePrefixStrategy = new NamingStrategy() { @Override public String getSchema() { @@ -57,7 +57,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } }; - final NamingStrategy upperCaseLowerCaseStrategy = new NamingStrategy() { + private final NamingStrategy upperCaseLowerCaseStrategy = new NamingStrategy() { @Override public String getTableName(Class type) { @@ -73,7 +73,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { private RelationalMappingContext context = new JdbcMappingContext(); @Test // DATAJDBC-107 - public void findOneWithOverriddenFixedTableName() { + void findOneWithOverriddenFixedTableName() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -96,7 +96,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void findOneWithUppercasedTablesAndLowercasedColumns() { + void findOneWithUppercasedTablesAndLowercasedColumns() { SqlGenerator sqlGenerator = configureSqlGenerator(upperCaseLowerCaseStrategy); @@ -115,7 +115,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void cascadingDeleteFirstLevel() { + void cascadingDeleteFirstLevel() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -126,7 +126,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void cascadingDeleteAllSecondLevel() { + void cascadingDeleteAllSecondLevel() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -141,7 +141,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void deleteAll() { + void deleteAll() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -151,7 +151,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void cascadingDeleteAllFirstLevel() { + void cascadingDeleteAllFirstLevel() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -162,7 +162,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-107 - public void cascadingDeleteSecondLevel() { + void cascadingDeleteSecondLevel() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); @@ -177,7 +177,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { } @Test // DATAJDBC-113 - public void deleteByList() { + void deleteByList() { SqlGenerator sqlGenerator = configureSqlGenerator(fixedCustomTablePrefixStrategy); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 653f456f..9869c11a 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -26,6 +26,7 @@ import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Version; @@ -43,7 +44,6 @@ import org.springframework.data.relational.core.dialect.PostgresDialect; import org.springframework.data.relational.core.dialect.SqlServerDialect; import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.mapping.DefaultNamingStrategy; -import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; @@ -78,8 +78,8 @@ class SqlGeneratorUnitTests { private static final Identifier BACKREF = Identifier.of(unquoted("backref"), "some-value", String.class); - private final NamingStrategy namingStrategy = new PrefixingNamingStrategy(); - private final RelationalMappingContext context = new JdbcMappingContext(namingStrategy); + private final PrefixingNamingStrategy namingStrategy = new PrefixingNamingStrategy(); + private RelationalMappingContext context = new JdbcMappingContext(namingStrategy); private final JdbcConverter converter = new BasicJdbcConverter(context, (identifier, path) -> { throw new UnsupportedOperationException(); }); @@ -749,7 +749,6 @@ class SqlGeneratorUnitTests { @Test // DATAJDBC-340 void noColumnForReferencedEntity() { - assertThat(generatedColumn("ref", DummyEntity.class)).isNull(); } @@ -867,18 +866,19 @@ class SqlGeneratorUnitTests { @Test // GH-1161 void backReferenceShouldConsiderRenamedParent() { - context.setForeignKeyNaming(APPLY_RENAMING); + namingStrategy.setForeignKeyNaming(APPLY_RENAMING); + context = new JdbcMappingContext(namingStrategy); String sql = sqlGenerator.createDeleteInByPath(getPath("ref", RenamedDummy.class)); assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE referenced_entity.renamed IN (:ids)"); - } @Test // GH-1161 void backReferenceShouldIgnoreRenamedParent() { - context.setForeignKeyNaming(IGNORE_RENAMING); + namingStrategy.setForeignKeyNaming(IGNORE_RENAMING); + context = new JdbcMappingContext(namingStrategy); String sql = sqlGenerator.createDeleteInByPath(getPath("ref", RenamedDummy.class)); @@ -888,26 +888,30 @@ class SqlGeneratorUnitTests { @Test // GH-1161 void keyColumnShouldConsiderRenamedParent() { - context.setForeignKeyNaming(APPLY_RENAMING); - SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class); - String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), getPath("ref", RenamedDummy.class)); + namingStrategy.setForeignKeyNaming(APPLY_RENAMING); + context = new JdbcMappingContext(namingStrategy); - assertThat(sql) - .contains("referenced_entity.renamed_key AS renamed_key", "WHERE referenced_entity.parentId"); + SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class); + String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), + getPath("ref", RenamedDummy.class)); + + assertThat(sql).contains("referenced_entity.renamed_key AS renamed_key", "WHERE referenced_entity.parentId"); } @Test // GH-1161 void keyColumnShouldIgnoreRenamedParent() { - context.setForeignKeyNaming(IGNORE_RENAMING); + namingStrategy.setForeignKeyNaming(IGNORE_RENAMING); + context = new JdbcMappingContext(namingStrategy); + SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class); - String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), getPath("ref", RenamedDummy.class)); + String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parentId"), 23, RenamedDummy.class), + getPath("ref", RenamedDummy.class)); - assertThat(sql) - .contains("referenced_entity.renamed_dummy_key AS renamed_dummy_key", "WHERE referenced_entity.parentId"); + assertThat(sql).contains("referenced_entity.renamed_dummy_key AS renamed_dummy_key", + "WHERE referenced_entity.parentId"); } - @Nullable private SqlIdentifier getAlias(Object maybeAliased) { @@ -931,7 +935,8 @@ class SqlGeneratorUnitTests { @SuppressWarnings("unused") static class DummyEntity { - @Column("id1") @Id Long id; + @Column("id1") + @Id Long id; String name; ReferencedEntity ref; Set elements; @@ -1018,7 +1023,8 @@ class SqlGeneratorUnitTests { // these column names behave like single double quote in the name since the get quoted and then doubling the double // quote escapes it. - @Id @Column("test\"\"_@id") Long id; + @Id + @Column("test\"\"_@id") Long id; @Column("test\"\"_@123") String name; } diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java index 85fad494..fdacb536 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java @@ -277,7 +277,7 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra @Override public SqlIdentifier getTableName(Class type) { - return getRequiredPersistentEntity(type).getFullTableName(); + return getRequiredPersistentEntity(type).getQualifiedTableName(); } @Override diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java index 5444afa3..045d5439 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java @@ -465,7 +465,7 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw Assert.notNull(entity, "Entity must not be null"); - return doInsert(entity, getRequiredEntity(entity).getFullTableName()); + return doInsert(entity, getRequiredEntity(entity).getQualifiedTableName()); } Mono doInsert(T entity, SqlIdentifier tableName) { @@ -564,7 +564,7 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw Assert.notNull(entity, "Entity must not be null"); - return doUpdate(entity, getRequiredEntity(entity).getFullTableName()); + return doUpdate(entity, getRequiredEntity(entity).getQualifiedTableName()); } private Mono doUpdate(T entity, SqlIdentifier tableName) { @@ -644,13 +644,13 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw private String formatOptimisticLockingExceptionMessage(T entity, RelationalPersistentEntity persistentEntity) { return String.format("Failed to update table [%s]; Version does not match for row with Id [%s]", - persistentEntity.getFullTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); + persistentEntity.getQualifiedTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); } private String formatTransientEntityExceptionMessage(T entity, RelationalPersistentEntity persistentEntity) { return String.format("Failed to update table [%s]; Row with Id [%s] does not exist", - persistentEntity.getFullTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); + persistentEntity.getQualifiedTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); } @SuppressWarnings("unchecked") @@ -744,14 +744,14 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw } SqlIdentifier getTableName(Class entityClass) { - return getRequiredEntity(entityClass).getFullTableName(); + return getRequiredEntity(entityClass).getQualifiedTableName(); } SqlIdentifier getTableNameOrEmpty(Class entityClass) { RelationalPersistentEntity entity = this.mappingContext.getPersistentEntity(entityClass); - return entity != null ? entity.getFullTableName() : SqlIdentifier.EMPTY; + return entity != null ? entity.getQualifiedTableName() : SqlIdentifier.EMPTY; } private RelationalPersistentEntity getRequiredEntity(Class entityClass) { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java index 4cf7d955..e9319422 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java @@ -35,7 +35,6 @@ class CachingNamingStrategy implements NamingStrategy { private final Map columnNames = new ConcurrentHashMap<>(); private final Map keyColumns = new ConcurrentHashMap<>(); - private final Map, String> qualifiedTableNames = new ConcurrentReferenceHashMap<>(); private final Map, String> tableNames = new ConcurrentReferenceHashMap<>(); private final Lazy schema; @@ -83,8 +82,4 @@ class CachingNamingStrategy implements NamingStrategy { return columnNames.computeIfAbsent(property, delegate::getColumnName); } - @Override - public void setForeignKeyNaming(ForeignKeyNaming foreignKeyNaming) { - delegate.setForeignKeyNaming(foreignKeyNaming); - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DefaultNamingStrategy.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DefaultNamingStrategy.java index b282bec6..e22cde1d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DefaultNamingStrategy.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DefaultNamingStrategy.java @@ -15,26 +15,18 @@ */ package org.springframework.data.relational.core.mapping; -import org.jetbrains.annotations.NotNull; import org.springframework.util.Assert; /** - * The default naming strategy used by Spring Data Relational. Names are in SNAKE_CASE. + * The default naming strategy used by Spring Data Relational. Names are in {@code SNAKE_CASE}. * * @author Jens Schauder - * @since 2.4 + * @since 3.0 */ public class DefaultNamingStrategy implements NamingStrategy { - /** - * Since in most cases it doesn't make sense to have more than one {@link NamingStrategy} use of this instance is - * recommended. - */ - public static NamingStrategy INSTANCE = new DefaultNamingStrategy(); - private ForeignKeyNaming foreignKeyNaming = ForeignKeyNaming.APPLY_RENAMING; - @Override public void setForeignKeyNaming(ForeignKeyNaming foreignKeyNaming) { Assert.notNull(foreignKeyNaming, "foreignKeyNaming must not be null"); @@ -51,19 +43,17 @@ public class DefaultNamingStrategy implements NamingStrategy { @Override public String getReverseColumnName(PersistentPropertyPathExtension path) { - RelationalPersistentEntity leafEntity = path.getIdDefiningParentPath().getLeafEntity(); + RelationalPersistentEntity leafEntity = path.getIdDefiningParentPath().getRequiredLeafEntity(); return getColumnNameReferencing(leafEntity); } private String getColumnNameReferencing(RelationalPersistentEntity leafEntity) { - Assert.state(leafEntity != null, "Leaf Entity must not be null."); - if (foreignKeyNaming == ForeignKeyNaming.IGNORE_RENAMING) { return getTableName(leafEntity.getType()); } - return leafEntity.getSimpleTableName().getReference(); + return leafEntity.getTableName().getReference(); } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/NamingStrategy.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/NamingStrategy.java index a19a988b..05b6b227 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/NamingStrategy.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/NamingStrategy.java @@ -41,8 +41,12 @@ public interface NamingStrategy { * * @deprecated use {@link DefaultNamingStrategy#INSTANCE} instead. */ - @Deprecated(since = "2.4") - NamingStrategy INSTANCE = DefaultNamingStrategy.INSTANCE; + @Deprecated(since = "2.4") NamingStrategy INSTANCE = new DefaultNamingStrategy() { + @Override + public void setForeignKeyNaming(ForeignKeyNaming foreignKeyNaming) { + throw new UnsupportedOperationException("Cannot update immutable DefaultNamingStrategy"); + } + }; /** * Defaults to no schema. @@ -78,19 +82,18 @@ public interface NamingStrategy { /** * For a reference A -> B this is the name in the table for B which references A. * - * @param property The property who's column name in the owner table is required + * @param property The property whose column name in the owner table is required * @return a column name. Must not be {@code null}. */ default String getReverseColumnName(RelationalPersistentProperty property) { Assert.notNull(property, "Property must not be null"); - return property.getOwner().getSimpleTableName().getReference(IdentifierProcessing.NONE); + return property.getOwner().getTableName().getReference(IdentifierProcessing.NONE); } default String getReverseColumnName(PersistentPropertyPathExtension path) { - - return getTableName(path.getIdDefiningParentPath().getLeafEntity().getType()); + return getTableName(path.getIdDefiningParentPath().getRequiredLeafEntity().getType()); } /** @@ -105,12 +108,4 @@ public interface NamingStrategy { return getReverseColumnName(property) + "_key"; } - - /** - * Set the {@link ForeignKeyNaming} strategy used in this {@link NamingStrategy}. - * - * @param foreignKeyNaming the ForeignKeyNaming strategy to be used. Must not be {@literal null}. - * @since 2.4 - */ - default void setForeignKeyNaming(ForeignKeyNaming foreignKeyNaming) {} } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java index 068f198d..93cc0309 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java @@ -138,6 +138,30 @@ public class PersistentPropertyPathExtension { return path == null ? entity : context.getPersistentEntity(path.getRequiredLeafProperty().getActualType()); } + /** + * The {@link RelationalPersistentEntity} associated with the leaf of this path or throw {@link IllegalStateException} + * if the leaf cannot be resolved. + * + * @return the required {@link RelationalPersistentEntity} associated with the leaf of this path. + * @since 3.0 + * @throws IllegalStateException if the persistent entity cannot be resolved. + */ + public RelationalPersistentEntity getRequiredLeafEntity() { + + RelationalPersistentEntity entity = getLeafEntity(); + + if (entity == null) { + + if (this.path == null) { + throw new IllegalStateException("Couldn't resolve leaf PersistentEntity absent path"); + } + throw new IllegalStateException(String.format("Couldn't resolve leaf PersistentEntity for type %s", + path.getRequiredLeafProperty().getActualType())); + } + + return entity; + } + /** * @return {@literal true} when this is an empty path or the path references an entity. */ @@ -230,10 +254,22 @@ public class PersistentPropertyPathExtension { return parent; } + /** + * The fully qualified name of the table this path is tied to or of the longest ancestor path that is actually tied to + * a table. + * + * @return the name of the table. Guaranteed to be not {@literal null}. + * @since 3.0 + */ + public SqlIdentifier getQualifiedTableName() { + return getTableOwningAncestor().getRequiredLeafEntity().getQualifiedTableName(); + } + /** * The name of the table this path is tied to or of the longest ancestor path that is actually tied to a table. * * @return the name of the table. Guaranteed to be not {@literal null}. + * @since 3.0 */ public SqlIdentifier getTableName() { return getTableOwningAncestor().getRequiredLeafEntity().getTableName(); @@ -442,10 +478,6 @@ public class PersistentPropertyPathExtension { return getParentPath().assembleColumnName(suffix.transform(embeddedPrefix::concat)); } - private RelationalPersistentEntity getRequiredLeafEntity() { - return path == null ? entity : context.getRequiredPersistentEntity(path.getRequiredLeafProperty().getActualType()); - } - private SqlIdentifier prefixWithTableAlias(SqlIdentifier columnName) { SqlIdentifier tableAlias = getTableAlias(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalMappingContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalMappingContext.java index c3109fdc..f46b4384 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalMappingContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalMappingContext.java @@ -36,13 +36,12 @@ public class RelationalMappingContext private final NamingStrategy namingStrategy; private boolean forceQuote = true; - private ForeignKeyNaming foreignKeyNaming = ForeignKeyNaming.APPLY_RENAMING; /** * Creates a new {@link RelationalMappingContext}. */ public RelationalMappingContext() { - this(NamingStrategy.INSTANCE); + this(new DefaultNamingStrategy()); } /** @@ -54,7 +53,6 @@ public class RelationalMappingContext Assert.notNull(namingStrategy, "NamingStrategy must not be null"); - namingStrategy.setForeignKeyNaming(foreignKeyNaming); this.namingStrategy = new CachingNamingStrategy(namingStrategy); setSimpleTypeHolder(SimpleTypeHolder.DEFAULT); @@ -104,19 +102,4 @@ public class RelationalMappingContext return this.namingStrategy; } - /** - * Sets the {@link ForeignKeyNaming} to be used by this mapping context. - * - * @param foreignKeyNaming must not be {@literal null}. - * @since 2.4 - */ - public void setForeignKeyNaming(ForeignKeyNaming foreignKeyNaming) { - - Assert.notNull(foreignKeyNaming, "foreignKeyNaming must not be null"); - - this.foreignKeyNaming = foreignKeyNaming; - if (namingStrategy != null) { - namingStrategy.setForeignKeyNaming(foreignKeyNaming); - } - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java index f5e61976..8d3f8f96 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java @@ -24,36 +24,27 @@ import org.springframework.data.relational.core.sql.SqlIdentifier; * * @author Jens Schauder * @author Oliver Gierke + * @author Mark Paluch */ public interface RelationalPersistentEntity extends MutablePersistentEntity { /** - * Returns the name of the table backing the given entity. + * Returns the unqualified name of the table (i.e. without schema or owner) backing the given entity. * * @return the table name. - * @deprecated Use either {@link #getFullTableName()} or {@link #getSimpleTableName()} */ - @Deprecated(since = "2.4") SqlIdentifier getTableName(); /** - * Returns the name of the table backing the given entity, including the schema. + * Returns the qualified name of the table backing the given entity, including the schema. * * @return the table name including the schema if there is any specified. - * @since 2.4 + * @since 3.0 */ - default SqlIdentifier getFullTableName() { + default SqlIdentifier getQualifiedTableName() { return getTableName(); } - /** - * Returns the name of the table backing the given entity, without any schema. - * - * @return the table name. - * @since 2.4 - */ - SqlIdentifier getSimpleTableName(); - /** * Returns the column representing the identifier. * diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java index 4457b7bc..78afa879 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java @@ -51,15 +51,14 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity Optional.ofNullable(findAnnotation(Table.class)) - .map(Table::value) - .filter(StringUtils::hasText) - .map(this::createSqlIdentifier) - ); + this.tableName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Table.class)) // + .map(Table::value) // + .filter(StringUtils::hasText) // + .map(this::createSqlIdentifier)); - this.schemaName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Table.class)) - .map(Table::schema) - .filter(StringUtils::hasText) + this.schemaName = Lazy.of(() -> Optional.ofNullable(findAnnotation(Table.class)) // + .map(Table::schema) // + .filter(StringUtils::hasText) // .map(this::createSqlIdentifier)); } @@ -81,11 +80,15 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity explicitlySpecifiedTableName = tableName.get(); + SqlIdentifier schemalessTableIdentifier = createDerivedSqlIdentifier(namingStrategy.getTableName(getType())); + + return explicitlySpecifiedTableName.orElse(schemalessTableIdentifier); } @Override - public SqlIdentifier getFullTableName() { + public SqlIdentifier getQualifiedTableName() { SqlIdentifier schema = determineCurrentEntitySchema(); Optional explicitlySpecifiedTableName = tableName.get(); @@ -96,32 +99,21 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity SqlIdentifier.from(schema, sqlIdentifier)) + return explicitlySpecifiedTableName.map(sqlIdentifier -> SqlIdentifier.from(schema, sqlIdentifier)) .orElse(SqlIdentifier.from(schema, schemalessTableIdentifier)); } - @Override - public SqlIdentifier getSimpleTableName() { - - Optional explicitlySpecifiedTableName = tableName.get(); - SqlIdentifier schemalessTableIdentifier = createDerivedSqlIdentifier(namingStrategy.getTableName(getType())); - - return explicitlySpecifiedTableName.orElse(schemalessTableIdentifier); - } - /** * @return {@link SqlIdentifier} representing the current entity schema. If the schema is not specified, neither - * explicitly, nor via {@link NamingStrategy}, then return {@link null} + * explicitly, nor via {@link NamingStrategy}, then return {@link null} */ @Nullable private SqlIdentifier determineCurrentEntitySchema() { Optional explicitlySpecifiedSchema = schemaName.get(); return explicitlySpecifiedSchema.orElseGet( - () -> StringUtils.hasText(namingStrategy.getSchema()) - ? createDerivedSqlIdentifier(namingStrategy.getSchema()) - : null); + () -> StringUtils.hasText(namingStrategy.getSchema()) ? createDerivedSqlIdentifier(namingStrategy.getSchema()) + : null); } @Override diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/SimpleRelationalEntityMetadata.java b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/SimpleRelationalEntityMetadata.java index c99bf5c8..89077b3a 100755 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/SimpleRelationalEntityMetadata.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/SimpleRelationalEntityMetadata.java @@ -50,7 +50,7 @@ public class SimpleRelationalEntityMetadata implements RelationalEntityMetada } public SqlIdentifier getTableName() { - return tableEntity.getFullTableName(); + return tableEntity.getQualifiedTableName(); } public RelationalPersistentEntity getTableEntity() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/MappingRelationalEntityInformation.java b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/MappingRelationalEntityInformation.java index 5bd516f6..e5ea18f9 100755 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/MappingRelationalEntityInformation.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/repository/support/MappingRelationalEntityInformation.java @@ -87,7 +87,7 @@ public class MappingRelationalEntityInformation extends PersistentEntityI } public SqlIdentifier getTableName() { - return customTableName == null ? entityMetadata.getFullTableName() : customTableName; + return customTableName == null ? entityMetadata.getQualifiedTableName() : customTableName; } public String getIdAttribute() { diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java index b8293499..13400dff 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java @@ -32,55 +32,55 @@ import org.springframework.data.relational.core.sql.SqlIdentifier; * @author Mark Paluch * @author Mikhail Polivakha */ -public class RelationalPersistentEntityImplUnitTests { +class RelationalPersistentEntityImplUnitTests { - RelationalMappingContext mappingContext = new RelationalMappingContext(); + private RelationalMappingContext mappingContext = new RelationalMappingContext(); @Test // DATAJDBC-106 - public void discoversAnnotatedTableName() { + void discoversAnnotatedTableName() { - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(DummySubEntity.class); + RelationalPersistentEntity entity = mappingContext.getRequiredPersistentEntity(DummySubEntity.class); assertThat(entity.getTableName()).isEqualTo(quoted("dummy_sub_entity")); - assertThat(entity.getFullTableName()).isEqualTo(quoted("dummy_sub_entity")); - assertThat(entity.getSimpleTableName()).isEqualTo(quoted("dummy_sub_entity")); + assertThat(entity.getQualifiedTableName()).isEqualTo(quoted("dummy_sub_entity")); + assertThat(entity.getTableName()).isEqualTo(quoted("dummy_sub_entity")); } @Test // DATAJDBC-294 - public void considerIdColumnName() { + void considerIdColumnName() { - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(DummySubEntity.class); + RelationalPersistentEntity entity = mappingContext.getRequiredPersistentEntity(DummySubEntity.class); assertThat(entity.getIdColumn()).isEqualTo(quoted("renamedId")); } @Test // DATAJDBC-296 - public void emptyTableAnnotationFallsBackToNamingStrategy() { + void emptyTableAnnotationFallsBackToNamingStrategy() { - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(DummyEntityWithEmptyAnnotation.class); + RelationalPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(DummyEntityWithEmptyAnnotation.class); assertThat(entity.getTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); - assertThat(entity.getFullTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); - assertThat(entity.getSimpleTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); + assertThat(entity.getQualifiedTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); + assertThat(entity.getTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); } @Test // DATAJDBC-491 - public void namingStrategyWithSchemaReturnsCompositeTableName() { + void namingStrategyWithSchemaReturnsCompositeTableName() { mappingContext = new RelationalMappingContext(NamingStrategyWithSchema.INSTANCE); - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(DummyEntityWithEmptyAnnotation.class); + RelationalPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(DummyEntityWithEmptyAnnotation.class); SqlIdentifier simpleExpected = quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION"); SqlIdentifier fullExpected = SqlIdentifier.from(quoted("MY_SCHEMA"), simpleExpected); + assertThat(entity.getQualifiedTableName()) + .isEqualTo(fullExpected); assertThat(entity.getTableName()) - .isEqualTo(fullExpected); - assertThat(entity.getFullTableName()) - .isEqualTo(fullExpected); - assertThat(entity.getSimpleTableName()) .isEqualTo(simpleExpected); - assertThat(entity.getTableName().toSql(IdentifierProcessing.ANSI)) + assertThat(entity.getQualifiedTableName().toSql(IdentifierProcessing.ANSI)) .isEqualTo("\"MY_SCHEMA\".\"DUMMY_ENTITY_WITH_EMPTY_ANNOTATION\""); } @@ -88,34 +88,32 @@ public class RelationalPersistentEntityImplUnitTests { void testRelationalPersistentEntitySchemaNameChoice() { mappingContext = new RelationalMappingContext(NamingStrategyWithSchema.INSTANCE); - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(EntityWithSchemaAndName.class); + RelationalPersistentEntity entity = mappingContext.getRequiredPersistentEntity(EntityWithSchemaAndName.class); SqlIdentifier simpleExpected = quoted("I_AM_THE_SENATE"); SqlIdentifier expected = SqlIdentifier.from(quoted("DART_VADER"), simpleExpected); - assertThat(entity.getTableName()).isEqualTo(expected); - assertThat(entity.getFullTableName()).isEqualTo(expected); - assertThat(entity.getSimpleTableName()).isEqualTo(simpleExpected); + assertThat(entity.getQualifiedTableName()).isEqualTo(expected); + assertThat(entity.getTableName()).isEqualTo(simpleExpected); } @Test // GH-1099 void specifiedSchemaGetsCombinedWithNameFromNamingStrategy() { - RelationalPersistentEntity entity = mappingContext.getPersistentEntity(EntityWithSchema.class); + RelationalPersistentEntity entity = mappingContext.getRequiredPersistentEntity(EntityWithSchema.class); SqlIdentifier simpleExpected = quoted("ENTITY_WITH_SCHEMA"); SqlIdentifier expected = SqlIdentifier.from(quoted("ANAKYN_SKYWALKER"), simpleExpected); - assertThat(entity.getTableName()).isEqualTo(expected); - assertThat(entity.getFullTableName()).isEqualTo(expected); - assertThat(entity.getSimpleTableName()).isEqualTo(simpleExpected); + assertThat(entity.getQualifiedTableName()).isEqualTo(expected); + assertThat(entity.getTableName()).isEqualTo(simpleExpected); } @Table(schema = "ANAKYN_SKYWALKER") - static class EntityWithSchema { + private static class EntityWithSchema { @Id private Long id; } @Table(schema = "DART_VADER", name = "I_AM_THE_SENATE") - static class EntityWithSchemaAndName { + private static class EntityWithSchemaAndName { @Id private Long id; }