From 188b9d43884b8a4b72e56cf767613a397ffbb995 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 20 Feb 2020 11:06:24 +0100 Subject: [PATCH] DATAJDBC-491 - Correctly combine schema- and table name. We no correctly combine schema and table name using separate SqlIdentifier objects instead of leaving the schema name as part of the table name. Previously, the concatenated name failed to resolve with quoted identifiers. --- ...GeneratorFixedNamingStrategyUnitTests.java | 46 ++++++++++--------- .../core/mapping/CachingNamingStrategy.java | 1 + .../core/mapping/NamingStrategy.java | 7 +++ .../RelationalPersistentEntityImpl.java | 9 +++- ...lationalPersistentEntityImplUnitTests.java | 25 ++++++++++ 5 files changed, 65 insertions(+), 23 deletions(-) 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 40955405e..be3928932 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 @@ -83,16 +83,16 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { SoftAssertions softAssertions = new SoftAssertions(); softAssertions.assertThat(sql) // .isEqualTo( - "SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", " - + "\"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_NAME\" AS \"FIXEDCUSTOMPROPERTYPREFIX_NAME\", " + "SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", " + + "\"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_NAME\" AS \"FIXEDCUSTOMPROPERTYPREFIX_NAME\", " + "\"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" AS \"REF_FIXEDCUSTOMPROPERTYPREFIX_L1ID\", " + "\"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_CONTENT\" AS \"REF_FIXEDCUSTOMPROPERTYPREFIX_CONTENT\", " + "\"ref_further\".\"FIXEDCUSTOMPROPERTYPREFIX_L2ID\" AS \"REF_FURTHER_FIXEDCUSTOMPROPERTYPREFIX_L2ID\", " + "\"ref_further\".\"FIXEDCUSTOMPROPERTYPREFIX_SOMETHING\" AS \"REF_FURTHER_FIXEDCUSTOMPROPERTYPREFIX_SOMETHING\" " - + "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" " - + "LEFT OUTER JOIN \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" AS \"ref\" ON \"ref\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" = \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" L" - + "EFT OUTER JOIN \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" AS \"ref_further\" ON \"ref_further\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" = \"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id"); + + "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" " + + "LEFT OUTER JOIN \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" AS \"ref\" ON \"ref\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" = \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" L" + + "EFT OUTER JOIN \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" AS \"ref_further\" ON \"ref_further\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" = \"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id"); softAssertions.assertAll(); } @@ -122,8 +122,8 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.createDeleteByPath(getPath("ref")); - assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId"); + assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId"); } @Test // DATAJDBC-107 @@ -133,11 +133,12 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.createDeleteByPath(getPath("ref.further")); - assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN " - + "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " - + "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)"); + assertThat(sql) + .isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN " + + "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " + + "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)"); } @Test // DATAJDBC-107 @@ -147,7 +148,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.createDeleteAllSql(null); - assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\""); + assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\""); } @Test // DATAJDBC-107 @@ -157,8 +158,8 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.createDeleteAllSql(getPath("ref")); - assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL"); + assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL"); } @Test // DATAJDBC-107 @@ -168,11 +169,12 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.createDeleteAllSql(getPath("ref.further")); - assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN " - + "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " - + "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " - + "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)"); + assertThat(sql) + .isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN " + + "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" " + + "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" " + + "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)"); } @Test // DATAJDBC-113 @@ -183,7 +185,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { String sql = sqlGenerator.getDeleteByList(); assertThat(sql).isEqualTo( - "DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" IN (:ids)"); + "DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" IN (:ids)"); } private PersistentPropertyPath getPath(String path) { 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 c58f8ad19..77889e8b7 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 @@ -66,6 +66,7 @@ class CachingNamingStrategy implements NamingStrategy { * @see org.springframework.data.relational.core.mapping.NamingStrategy#getQualifiedTableName(java.lang.Class) */ @Override + @Deprecated public String getQualifiedTableName(Class type) { return qualifiedTableNames.computeIfAbsent(type, delegate::getQualifiedTableName); } 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 6872d2c27..e7625335b 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 @@ -72,6 +72,13 @@ public interface NamingStrategy { return ParsingUtils.reconcatenateCamelCase(property.getName(), "_"); } + /** + * @param type + * @return + * @deprecated since 2.0. The method returns a concatenated schema with table name which conflicts with escaping. Use + * rather {@link #getTableName(Class)} and {@link #getSchema()} independently + */ + @Deprecated default String getQualifiedTableName(Class type) { return this.getSchema() + (this.getSchema().equals("") ? "" : ".") + this.getTableName(type); } 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 262e5d830..957ba4837 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 @@ -78,7 +78,14 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity createDerivedSqlIdentifier(namingStrategy.getQualifiedTableName(getType()))); + return tableName.get().orElseGet(() -> { + + String schema = namingStrategy.getSchema(); + SqlIdentifier tableName = createDerivedSqlIdentifier(namingStrategy.getTableName(getType())); + + return StringUtils.hasText(schema) ? SqlIdentifier.from(createDerivedSqlIdentifier(schema), tableName) + : tableName; + }); } /* 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 0553eaef6..ddf9adcb7 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 @@ -19,7 +19,10 @@ import static org.assertj.core.api.Assertions.*; import static org.springframework.data.relational.core.sql.SqlIdentifier.*; import org.junit.Test; + import org.springframework.data.annotation.Id; +import org.springframework.data.relational.core.sql.IdentifierProcessing; +import org.springframework.data.relational.core.sql.SqlIdentifier; /** * Unit tests for {@link RelationalPersistentEntityImpl}. @@ -27,6 +30,7 @@ import org.springframework.data.annotation.Id; * @author Oliver Gierke * @author Kazuki Shimizu * @author Bastian Wilhelm + * @author Mark Paluch */ public class RelationalPersistentEntityImplUnitTests { @@ -56,6 +60,18 @@ public class RelationalPersistentEntityImplUnitTests { assertThat(entity.getTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")); } + @Test // DATAJDBC-491 + public void namingStrategyWithSchemaReturnsCompositeTableName() { + + mappingContext = new RelationalMappingContext(NamingStrategyWithSchema.INSTANCE); + RelationalPersistentEntity entity = mappingContext.getPersistentEntity(DummyEntityWithEmptyAnnotation.class); + + assertThat(entity.getTableName()) + .isEqualTo(SqlIdentifier.from(quoted("MY_SCHEMA"), quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION"))); + assertThat(entity.getTableName().toSql(IdentifierProcessing.ANSI)) + .isEqualTo("\"MY_SCHEMA\".\"DUMMY_ENTITY_WITH_EMPTY_ANNOTATION\""); + } + @Table("dummy_sub_entity") static class DummySubEntity { @Id @Column("renamedId") Long id; @@ -65,4 +81,13 @@ public class RelationalPersistentEntityImplUnitTests { static class DummyEntityWithEmptyAnnotation { @Id @Column() Long id; } + + enum NamingStrategyWithSchema implements NamingStrategy { + INSTANCE; + + @Override + public String getSchema() { + return "my_schema"; + } + } }