From f42b15c2372ec2783d1be237fa50c258bb4a9c17 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 28 Sep 2018 10:06:44 -0400 Subject: [PATCH 1/2] DATAJDBC-266 - Prepare branchc. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 4016f901e1..e412d6f301 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jdbc - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-266-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. From b64212db4e9b43c31f5919278059df6914edc180 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 28 Sep 2018 10:20:25 -0400 Subject: [PATCH 2/2] DATAJDBC-266 - Entities referenced by 1:1 don't need an Id. The id-property was used to determine if there is an instance at all, or if it was null. For entities that don't have an id that purpose is now fulfilled by selecting the backreference and checking it against null. See also: DATAJDBC-223. --- .../data/jdbc/core/EntityRowMapper.java | 34 +++++++---- .../data/jdbc/core/SqlGenerator.java | 11 ++++ .../AggregateTemplateIntegrationTests.java | 58 +++++++++++++++++++ .../data/jdbc/core/SqlGeneratorUnitTests.java | 22 +++++++ ...AggregateTemplateIntegrationTests-hsql.sql | 3 + ...regateTemplateIntegrationTests-mariadb.sql | 3 + ...ggregateTemplateIntegrationTests-mysql.sql | 3 + ...egateTemplateIntegrationTests-postgres.sql | 3 + 8 files changed, 125 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java index dbaa053ef3..2fb226ec7a 100644 --- a/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/jdbc/core/EntityRowMapper.java @@ -21,7 +21,6 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.data.mapping.MappingException; -import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.relational.core.conversion.RelationalConverter; @@ -118,21 +117,17 @@ private T populateProperties(T result, ResultSet resultSet) { @Nullable private Object readFrom(ResultSet resultSet, RelationalPersistentProperty property, String prefix) { - try { - - if (property.isEntity()) { - return readEntityFrom(resultSet, property); - } + if (property.isEntity()) { + return readEntityFrom(resultSet, property); + } - return converter.readValue(resultSet.getObject(prefix + property.getColumnName()), property.getTypeInformation()); + Object value = getObjectFromResultSet(resultSet, prefix + property.getColumnName()); + return converter.readValue(value, property.getTypeInformation()); - } catch (SQLException o_O) { - throw new MappingException(String.format("Could not read property %s from result set!", property), o_O); - } } @Nullable - private S readEntityFrom(ResultSet rs, PersistentProperty property) { + private S readEntityFrom(ResultSet rs, RelationalPersistentProperty property) { String prefix = property.getName() + "_"; @@ -140,7 +135,12 @@ private S readEntityFrom(ResultSet rs, PersistentProperty property) { RelationalPersistentEntity entity = (RelationalPersistentEntity) context .getRequiredPersistentEntity(property.getActualType()); - if (readFrom(rs, entity.getRequiredIdProperty(), prefix) == null) { + RelationalPersistentProperty idProperty = entity.getIdProperty(); + + if ((idProperty != null // + ? readFrom(rs, idProperty, prefix) // + : getObjectFromResultSet(rs, prefix + property.getReverseColumnName()) // + ) == null) { return null; } @@ -155,6 +155,16 @@ private S readEntityFrom(ResultSet rs, PersistentProperty property) { return instance; } + @Nullable + private Object getObjectFromResultSet(ResultSet rs, String backreferenceName) { + + try { + return rs.getObject(backreferenceName); + } catch (SQLException o_O) { + throw new MappingException(String.format("Could not read value %s from result set!", backreferenceName), o_O); + } + } + private S createInstance(RelationalPersistentEntity entity, ResultSet rs, String prefix) { return converter.createInstance(entity, parameter -> { 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 83ce833e38..a92d3f1ecd 100644 --- a/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -201,6 +201,17 @@ private void addColumnsAndJoinsForOneToOneReferences(SelectBuilder builder) { .as(joinAlias + "_" + refProperty.getColumnName()) // ); } + + // if the referenced property doesn't have an id, include the back reference in the select list. + // this enables determining if the referenced entity is present or null. + if (!refEntity.hasIdProperty()) { + + builder.column( // + cb -> cb.tableAlias(joinAlias) // + .column(property.getReverseColumnName()) // + .as(joinAlias + "_" + property.getReverseColumnName()) // + ); + } } } diff --git a/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java index 22d46b1fdc..bf25e4fa5d 100644 --- a/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/AggregateTemplateIntegrationTests.java @@ -210,6 +210,52 @@ public void changeReferencedEntity() { assertThat(reloadedLegoSet.manual.content).isEqualTo("new content"); } + @Test // DATAJDBC-266 + public void oneToOneChildWithoutId() { + + OneToOneParent parent = new OneToOneParent(); + + parent.content = "parent content"; + parent.child = new OneToOneChildNoId(); + parent.child.content = "child content"; + + template.save(parent); + + OneToOneParent reloaded = template.findById(parent.id, OneToOneParent.class); + + assertThat(reloaded.child.content).isEqualTo("child content"); + } + + @Test // DATAJDBC-266 + public void oneToOneNullChildWithoutId() { + + OneToOneParent parent = new OneToOneParent(); + + parent.content = "parent content"; + parent.child = null; + + template.save(parent); + + OneToOneParent reloaded = template.findById(parent.id, OneToOneParent.class); + + assertThat(reloaded.child).isNull(); + } + + @Test // DATAJDBC-266 + public void oneToOneNullAttributes() { + + OneToOneParent parent = new OneToOneParent(); + + parent.content = "parent content"; + parent.child = new OneToOneChildNoId(); + + template.save(parent); + + OneToOneParent reloaded = template.findById(parent.id, OneToOneParent.class); + + assertThat(reloaded.child).isNotNull(); + } + private static LegoSet createLegoSet() { LegoSet entity = new LegoSet(); @@ -241,6 +287,18 @@ static class Manual { } + static class OneToOneParent { + + @Id private Long id; + private String content; + + private OneToOneChildNoId child; + } + + static class OneToOneChildNoId { + private String content; + } + @Configuration @Import(TestConfiguration.class) static class Config { 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 984e195aca..6be1baa15c 100644 --- a/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java @@ -186,6 +186,19 @@ public void getInsertForEmptyColumnList() { assertThat(insert).endsWith("()"); } + @Test // DATAJDBC-266 + public void joinForOneToOneWithoutIdIncludesTheBackReferenceOfTheOuterJoin() { + + SqlGenerator sqlGenerator = createSqlGenerator(ParentOfNoIdChild.class); + + String findAll = sqlGenerator.getFindAll(); + + assertThat(findAll).containsSequence( + "SELECT", + "child.parent_of_no_id_child AS child_parent_of_no_id_child", + "FROM"); + } + private PersistentPropertyPath getPath(String path, Class base) { return PersistentPropertyPathTestUtils.getPath(context, path, base); } @@ -220,6 +233,15 @@ static class Element { String content; } + static class ParentOfNoIdChild { + @Id Long id; + NoIdChild child; + } + + static class NoIdChild { + + } + private static class PrefixingNamingStrategy implements NamingStrategy { @Override diff --git a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql index 20a0a290df..6b7c1a5aff 100644 --- a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql +++ b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-hsql.sql @@ -3,3 +3,6 @@ CREATE TABLE MANUAL ( id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) P ALTER TABLE MANUAL ADD FOREIGN KEY (LEGO_SET) REFERENCES LEGO_SET(id); + +CREATE TABLE ONE_TO_ONE_PARENT ( id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY, content VARCHAR(30)); +CREATE TABLE One_To_One_Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR(30)); diff --git a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql index 0ac78e637f..766bf60618 100644 --- a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql +++ b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mariadb.sql @@ -3,3 +3,6 @@ CREATE TABLE MANUAL ( id BIGINT AUTO_INCREMENT PRIMARY KEY, LEGO_SET BIGINT, CON ALTER TABLE MANUAL ADD FOREIGN KEY (LEGO_SET) REFERENCES LEGO_SET(id); + +CREATE TABLE ONE_TO_ONE_PARENT ( id BIGINT AUTO_INCREMENT PRIMARY KEY, content VARCHAR(30)); +CREATE TABLE One_To_One_Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR(30)); diff --git a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql index 0ac78e637f..766bf60618 100644 --- a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql +++ b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-mysql.sql @@ -3,3 +3,6 @@ CREATE TABLE MANUAL ( id BIGINT AUTO_INCREMENT PRIMARY KEY, LEGO_SET BIGINT, CON ALTER TABLE MANUAL ADD FOREIGN KEY (LEGO_SET) REFERENCES LEGO_SET(id); + +CREATE TABLE ONE_TO_ONE_PARENT ( id BIGINT AUTO_INCREMENT PRIMARY KEY, content VARCHAR(30)); +CREATE TABLE One_To_One_Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR(30)); diff --git a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql index e36e560791..432d687be7 100644 --- a/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql +++ b/src/test/resources/org.springframework.data.jdbc.core/AggregateTemplateIntegrationTests-postgres.sql @@ -6,3 +6,6 @@ CREATE TABLE MANUAL ( id SERIAL PRIMARY KEY, LEGO_SET BIGINT, CONTENT VARCHAR(20 ALTER TABLE MANUAL ADD FOREIGN KEY (LEGO_SET) REFERENCES LEGO_SET(id); + +CREATE TABLE ONE_TO_ONE_PARENT ( id SERIAL PRIMARY KEY, content VARCHAR(30)); +CREATE TABLE One_To_One_Child_No_Id (ONE_TO_ONE_PARENT INTEGER PRIMARY KEY, content VARCHAR(30));