From 482f330f02a9f269c836ce7d0253b323cc626d24 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 27 Oct 2017 16:09:05 +0200 Subject: [PATCH] DATAJDBC-144 - Using correct column names from NamingStrategy. The list of columns used in the SqlGenerator contained property names instead of column names, leading to errors when a non-trivial NamingStrategy was used. --- .../data/jdbc/core/SqlGenerator.java | 22 ++++++++--------- .../data/jdbc/core/SqlGeneratorUnitTests.java | 24 +++++++++++++------ .../JdbcRepositoryIntegrationTests.java | 2 ++ .../data/jdbc/testing/TestConfiguration.java | 8 ++++++- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java index a275c223..d3dd1789 100644 --- a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -43,8 +43,8 @@ class SqlGenerator { private final JdbcPersistentEntity entity; private final JdbcMappingContext context; - private final List propertyNames = new ArrayList<>(); - private final List nonIdPropertyNames = new ArrayList<>(); + private final List columnNames = new ArrayList<>(); + private final List nonIdColumnNames = new ArrayList<>(); private final Lazy findOneSql = Lazy.of(this::createFindOneSelectSql); private final Lazy findAllSql = Lazy.of(this::createFindAllSql); @@ -64,17 +64,17 @@ class SqlGenerator { this.context = context; this.entity = entity; this.sqlGeneratorSource = sqlGeneratorSource; - initPropertyNames(); + initColumnNames(); } - private void initPropertyNames() { + private void initColumnNames() { entity.doWithProperties((PropertyHandler) p -> { // the referencing column of referenced entity is expected to be on the other side of the relation if (!p.isEntity()) { - propertyNames.add(p.getName()); + columnNames.add(p.getColumnName()); if (!entity.isIdProperty(p)) { - nonIdPropertyNames.add(p.getName()); + nonIdColumnNames.add(p.getColumnName()); } } }); @@ -211,11 +211,11 @@ class SqlGenerator { String insertTemplate = "insert into %s (%s) values (%s)"; - List propertyNamesForInsert = new ArrayList<>(excludeId ? nonIdPropertyNames : propertyNames); - propertyNamesForInsert.addAll(additionalColumns); + List columnNamesForInsert = new ArrayList<>(excludeId ? nonIdColumnNames : columnNames); + columnNamesForInsert.addAll(additionalColumns); - String tableColumns = String.join(", ", propertyNamesForInsert); - String parameterNames = propertyNamesForInsert.stream().collect(Collectors.joining(", :", ":", "")); + String tableColumns = String.join(", ", columnNamesForInsert); + String parameterNames = columnNamesForInsert.stream().collect(Collectors.joining(", :", ":", "")); return String.format(insertTemplate, entity.getTableName(), tableColumns, parameterNames); } @@ -224,7 +224,7 @@ class SqlGenerator { String updateTemplate = "update %s set %s where %s = :%s"; - String setClause = propertyNames.stream()// + String setClause = columnNames.stream()// .map(n -> String.format("%s = :%s", n, n))// .collect(Collectors.joining(", ")); diff --git a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java index d4f0844e..c76ce9aa 100644 --- a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java @@ -26,6 +26,7 @@ import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.mapping.model.DefaultNamingStrategy; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.mapping.model.JdbcPersistentEntity; +import org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty; import org.springframework.data.jdbc.mapping.model.NamingStrategy; import org.springframework.data.mapping.PropertyPath; @@ -42,7 +43,7 @@ public class SqlGeneratorUnitTests { @Before public void setUp() { - NamingStrategy namingStrategy = new DefaultNamingStrategy(); + NamingStrategy namingStrategy = new PrefixingNamingStrategy(); JdbcMappingContext context = new JdbcMappingContext(namingStrategy); JdbcPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class); this.sqlGenerator = new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); @@ -56,10 +57,10 @@ public class SqlGeneratorUnitTests { SoftAssertions softAssertions = new SoftAssertions(); softAssertions.assertThat(sql) // .startsWith("SELECT") // - .contains("DummyEntity.id AS id,") // - .contains("DummyEntity.name AS name,") // - .contains("ref.l1id AS ref_l1id") // - .contains("ref.content AS ref_content").contains(" FROM DummyEntity") // + .contains("DummyEntity.x_id AS x_id,") // + .contains("DummyEntity.x_name AS x_name,") // + .contains("ref.x_l1id AS ref_x_l1id") // + .contains("ref.x_content AS ref_x_content").contains(" FROM DummyEntity") // // 1-N relationships do not get loaded via join .doesNotContain("Element AS elements"); softAssertions.assertAll(); @@ -79,7 +80,7 @@ public class SqlGeneratorUnitTests { String sql = sqlGenerator.createDeleteByPath(PropertyPath.from("ref.further", DummyEntity.class)); assertThat(sql).isEqualTo( - "DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT l1id FROM ReferencedEntity WHERE DummyEntity = :rootId)"); + "DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT x_l1id FROM ReferencedEntity WHERE DummyEntity = :rootId)"); } @Test // DATAJDBC-112 @@ -104,7 +105,7 @@ public class SqlGeneratorUnitTests { String sql = sqlGenerator.createDeleteAllSql(PropertyPath.from("ref.further", DummyEntity.class)); assertThat(sql).isEqualTo( - "DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT l1id FROM ReferencedEntity WHERE DummyEntity IS NOT NULL)"); + "DELETE FROM SecondLevelReferencedEntity WHERE ReferencedEntity IN (SELECT x_l1id FROM ReferencedEntity WHERE DummyEntity IS NOT NULL)"); } @SuppressWarnings("unused") @@ -135,4 +136,13 @@ public class SqlGeneratorUnitTests { @Id Long id; String content; } + + private static class PrefixingNamingStrategy extends DefaultNamingStrategy { + + @Override + public String getColumnName(JdbcPersistentProperty property) { + return "x_" + super.getColumnName(property); + } + + } } diff --git a/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java index 8f9003d2..2f1cd211 100644 --- a/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java @@ -63,8 +63,10 @@ public class JdbcRepositoryIntegrationTests { DummyEntityRepository dummyEntityRepository() { return factory.getRepository(DummyEntityRepository.class); } + } + @ClassRule public static final SpringClassRule classRule = new SpringClassRule(); @Rule public SpringMethodRule methodRule = new SpringMethodRule(); diff --git a/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java b/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java index c3021004..03f847a6 100644 --- a/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java +++ b/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java @@ -27,6 +27,7 @@ import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.jdbc.mapping.model.DefaultNamingStrategy; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; +import org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.jdbc.datasource.DataSourceTransactionManager; @@ -49,7 +50,12 @@ public class TestConfiguration { @Bean JdbcRepositoryFactory jdbcRepositoryFactory() { - final JdbcMappingContext context = new JdbcMappingContext(new DefaultNamingStrategy()); + final JdbcMappingContext context = new JdbcMappingContext(new DefaultNamingStrategy(){ + @Override + public String getColumnName(JdbcPersistentProperty property) { + return super.getColumnName(property); + } + }); return new JdbcRepositoryFactory( // publisher, //