From 11c3c9e33656a78c45e0a32034abf936a7cceb2c Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 20 Feb 2020 10:46:48 +0100 Subject: [PATCH 1/2] DATAJDBC-491 - Prepare issue branch. --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 4959b322ae..38b4c32595 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 858a5f4b4b..9707581e8d 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 17204c4a0f..bbba75118e 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 0a529399c3..6c8ee2dede 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATAJDBC-491-SNAPSHOT From d3907119808cbe43d7942a39eb421a0100c5e2ac Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 20 Feb 2020 11:06:24 +0100 Subject: [PATCH 2/2] 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 40955405e2..be39289322 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 void findOneWithOverriddenFixedTableName() { 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 void cascadingDeleteFirstLevel() { 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 void cascadingDeleteAllSecondLevel() { 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 void deleteAll() { 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 void cascadingDeleteAllFirstLevel() { 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 void cascadingDeleteSecondLevel() { 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 void deleteByList() { 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 c58f8ad193..77889e8b73 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 @@ public String getKeyColumn(RelationalPersistentProperty property) { * @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 6872d2c27e..e7625335b3 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 @@ default String getColumnName(RelationalPersistentProperty property) { 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 262e5d830b..957ba48379 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 @@ public void setForceQuote(boolean forceQuote) { */ @Override public SqlIdentifier getTableName() { - return tableName.get().orElseGet(() -> 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 0553eaef6a..ddf9adcb74 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.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 @@ * @author Oliver Gierke * @author Kazuki Shimizu * @author Bastian Wilhelm + * @author Mark Paluch */ public class RelationalPersistentEntityImplUnitTests { @@ -56,6 +60,18 @@ public void emptyTableAnnotationFallsBackToNamingStrategy() { 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 @@ static class DummySubEntity { static class DummyEntityWithEmptyAnnotation { @Id @Column() Long id; } + + enum NamingStrategyWithSchema implements NamingStrategy { + INSTANCE; + + @Override + public String getSchema() { + return "my_schema"; + } + } }