From 8a94345a7a272db23fb6eabd84ee6fb68c51a00f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 5 Dec 2023 16:03:51 +0100 Subject: [PATCH] Fix access of root property instead of child property. When the child of a one-to-one relationship has an id, the value for that id gets read in the wrong way. We get the column name for that id use that to access the value in the RowDocument. This results in either no value at all being found or even worse, the value of a root entity with a property of same name being accessed. This is fixed by using the full AggregatePath instead of just the property for accessing that value. Closes #1684 Original pull request: #1775 --- .../core/convert/MappingJdbcConverter.java | 2 +- ...JdbcAggregateTemplateIntegrationTests.java | 14 +++++++++++++ .../MappingJdbcConverterUnitTests.java | 20 +++++++++++++++++++ ...cAggregateTemplateIntegrationTests-db2.sql | 17 +++++++++++++++- ...bcAggregateTemplateIntegrationTests-h2.sql | 13 +++++++++++- ...AggregateTemplateIntegrationTests-hsql.sql | 12 +++++++++++ ...regateTemplateIntegrationTests-mariadb.sql | 13 +++++++++++- ...ggregateTemplateIntegrationTests-mssql.sql | 17 +++++++++++++++- ...ggregateTemplateIntegrationTests-mysql.sql | 14 ++++++++++++- ...gregateTemplateIntegrationTests-oracle.sql | 16 ++++++++++++++- ...egateTemplateIntegrationTests-postgres.sql | 16 ++++++++++++++- 11 files changed, 146 insertions(+), 8 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java index 746fd366..75c5d4c9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java @@ -327,7 +327,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements this.accessor = accessor; this.context = context; this.identifier = path.isEntity() - ? potentiallyAppendIdentifier(identifier, path.getRequiredLeafEntity(), delegate::getPropertyValue) + ? potentiallyAppendIdentifier(identifier, path.getRequiredLeafEntity(), property -> delegate.getValue(path.append(property))) : identifier; } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java index 7ccf0636..c7ac9275 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java @@ -1265,6 +1265,16 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { assertThat(enumMapOwners).containsExactly(enumMapOwner); } + @Test // GH-1684 + void oneToOneWithIdenticalIdColumnName(){ + + WithOneToOne saved = template.insert(new WithOneToOne("one", new Referenced(23L))); + + WithOneToOne reloaded = template.findById(saved.id, WithOneToOne.class); + + assertThat(reloaded).isEqualTo(saved); + } + private void saveAndUpdateAggregateWithVersion(VersionedAggregate aggregate, Function toConcreteNumber) { saveAndUpdateAggregateWithVersion(aggregate, toConcreteNumber, 0); @@ -2086,6 +2096,10 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { record EnumMapOwner(@Id Long id, String name, Map map) { } + record WithOneToOne(@Id String id,@MappedCollection(idColumn = "renamed") Referenced referenced){} + + record Referenced(@Id Long id) { + } @Configuration @Import(TestConfiguration.class) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java index 69ef266a..c56f6d41 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java @@ -30,6 +30,7 @@ import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.Date; import java.util.List; +import java.util.Map; import java.util.UUID; import org.assertj.core.api.SoftAssertions; @@ -39,8 +40,10 @@ import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.JdbcValue; import org.springframework.data.jdbc.support.JdbcUtil; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.domain.RowDocument; import org.springframework.data.util.TypeInformation; /** @@ -139,6 +142,17 @@ public class MappingJdbcConverterUnitTests { assertThat(typeFactory.arraySource).containsExactly(1, 2, 3, 4, 5); } + @Test // GH-1684 + void accessesCorrectValuesForOneToOneRelationshipWithIdenticallyNamedIdProperties() { + + RowDocument rowdocument = new RowDocument(Map.of("ID", "one", "REFERENCED_ID", 23)); + + WithOneToOne result = converter.readAndResolve(WithOneToOne.class, rowdocument); + + assertThat(result).isEqualTo(new WithOneToOne("one", new Referenced(23L))); + } + + private void checkConversionToTimestampAndBack(SoftAssertions softly, RelationalPersistentEntity persistentEntity, String propertyName, Object value) { @@ -284,4 +298,10 @@ public class MappingJdbcConverterUnitTests { return mock(Array.class); } } + + record WithOneToOne(@Id String id,@MappedCollection(idColumn = "renamed") Referenced referenced){} + + record Referenced(@Id Long id) { + } + } diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql index f5483405..52e70b17 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-db2.sql @@ -52,6 +52,10 @@ DROP TABLE AUTHOR; DROP TABLE ENUM_MAP_OWNER; +DROP TABLE REFERENCED; +DROP TABLE WITH_ONE_TO_ONE; + + CREATE TABLE LEGO_SET ( "id1" BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, @@ -428,4 +432,15 @@ CREATE TABLE ENUM_MAP_OWNER ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql index 982b9e31..7a87c5df 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-h2.sql @@ -385,4 +385,15 @@ CREATE TABLE ENUM_MAP_OWNER ( ID SERIAL PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql index 3ae51989..054f8a17 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-hsql.sql @@ -387,3 +387,15 @@ CREATE TABLE ENUM_MAP_OWNER ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, NAME VARCHAR(100) ); + + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql index 5e318e22..52f57d54 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mariadb.sql @@ -359,4 +359,15 @@ CREATE TABLE ENUM_MAP_OWNER ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + `renamed` VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql index 8323608b..c623581f 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mssql.sql @@ -401,4 +401,19 @@ CREATE TABLE ENUM_MAP_OWNER ( ID BIGINT IDENTITY PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + + +DROP TABLE REFERENCED; +DROP TABLE WITH_ONE_TO_ONE; + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql index a6989938..a2cb9a4e 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql @@ -364,4 +364,16 @@ CREATE TABLE ENUM_MAP_OWNER ( ID BIGINT AUTO_INCREMENT PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + `renamed` VARCHAR(100), + ID BIGINT +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql index 6f1700df..8cc59fdb 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql @@ -42,6 +42,9 @@ DROP TABLE AUTHOR CASCADE CONSTRAINTS PURGE; DROP TABLE ENUM_MAP_OWNER CASCADE CONSTRAINTS PURGE; +DROP TABLE REFERENCED CASCADE CONSTRAINTS PURGE; +DROP TABLE WITH_ONE_TO_ONE CASCADE CONSTRAINTS PURGE; + CREATE TABLE LEGO_SET ( "id1" NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY, @@ -409,4 +412,15 @@ CREATE TABLE ENUM_MAP_OWNER ( ID NUMBER GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID NUMBER +); diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql index 72b88cdd..bc7df72a 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql @@ -45,6 +45,9 @@ DROP TABLE AUTHOR; DROP TABLE ENUM_MAP_OWNER; +DROP TABLE REFERENCED; +DROP TABLE WITH_ONE_TO_ONE; + CREATE TABLE LEGO_SET ( "id1" SERIAL PRIMARY KEY, @@ -431,4 +434,15 @@ CREATE TABLE ENUM_MAP_OWNER ( ID BIGINT GENERATED BY DEFAULT AS IDENTITY (START WITH 1) PRIMARY KEY, NAME VARCHAR(100) -); \ No newline at end of file +); + +CREATE TABLE WITH_ONE_TO_ONE +( + ID VARCHAR(100) +); + +CREATE TABLE REFERENCED +( + "renamed" VARCHAR(100), + ID BIGINT +);