From 0816f4182ec46e05421e4f5fe9fae24dd63c05a6 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 28 Sep 2018 10:20:25 -0400 Subject: [PATCH] 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 | 36 +++++++----- .../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, 126 insertions(+), 13 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 dbaa053e..2fb226ec 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 java.util.Map; 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 @@ public class EntityRowMapper implements RowMapper { @Nullable private Object readFrom(ResultSet resultSet, RelationalPersistentProperty property, String prefix) { - try { - - if (property.isEntity()) { - return readEntityFrom(resultSet, property); - } - - return converter.readValue(resultSet.getObject(prefix + property.getColumnName()), property.getTypeInformation()); - - } catch (SQLException o_O) { - throw new MappingException(String.format("Could not read property %s from result set!", property), o_O); + if (property.isEntity()) { + return readEntityFrom(resultSet, property); } + + Object value = getObjectFromResultSet(resultSet, prefix + property.getColumnName()); + return converter.readValue(value, property.getTypeInformation()); + } @Nullable - private S readEntityFrom(ResultSet rs, PersistentProperty property) { + private S readEntityFrom(ResultSet rs, RelationalPersistentProperty property) { String prefix = property.getName() + "_"; @@ -140,7 +135,12 @@ public class EntityRowMapper implements RowMapper { 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 @@ public class EntityRowMapper implements RowMapper { 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 83ce833e..a92d3f1e 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 @@ class SqlGenerator { .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 22d46b1f..bf25e4fa 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 class AggregateTemplateIntegrationTests { 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 @@ public class AggregateTemplateIntegrationTests { } + 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 984e195a..6be1baa1 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 class SqlGeneratorUnitTests { 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 @@ public class SqlGeneratorUnitTests { 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 20a0a290..6b7c1a5a 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 0ac78e63..766bf606 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 0ac78e63..766bf606 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 e36e5607..432d687b 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));