From f60044225f83f5951f76ede9cef3e819b7bbb93b Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 5 Sep 2023 16:30:14 +0200 Subject: [PATCH] Remove external column name prefixing for embedded entities. Original pull request #1604 See #1586 --- .../data/jdbc/core/convert/QueryMapper.java | 12 +--------- .../core/mapping/AggregatePath.java | 5 ++--- .../core/mapping/AggregatePathTableUtils.java | 12 ---------- .../core/mapping/AggregatePathTraversal.java | 4 ++-- .../core/mapping/EmbeddedContext.java | 8 +++++-- .../EmbeddedRelationalPersistentProperty.java | 5 +++++ .../PersistentPropertyPathExtension.java | 22 +------------------ .../DefaultAggregatePathUnitTests.java | 6 +++-- 8 files changed, 21 insertions(+), 53 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java index c6706c9f..65d9d3c2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java @@ -413,7 +413,7 @@ public class QueryMapper { Condition condition = null; for (RelationalPersistentProperty nestedProperty : persistentEntity) { - SqlIdentifier sqlIdentifier = nestedProperty.getColumnName().transform(prefix::concat); + SqlIdentifier sqlIdentifier = nestedProperty.getColumnName(); Object mappedNestedValue = convertValue(embeddedAccessor.getProperty(nestedProperty), nestedProperty.getTypeInformation()); SQLType sqlType = converter.getTargetSqlType(nestedProperty); @@ -768,16 +768,6 @@ public class QueryMapper { throw new IllegalStateException("Cannot obtain a single column name for embedded property"); } - if (this.property != null && this.path != null && this.path.getParentPath() != null) { - - RelationalPersistentProperty owner = this.path.getParentPath().getLeafProperty(); - - if (owner != null && owner.isEmbedded()) { - return this.property.getColumnName() - .transform(it -> Objects.requireNonNull(owner.getEmbeddedPrefix()).concat(it)); - } - } - return this.path == null || this.path.getLeafProperty() == null ? super.getMappedColumnName() : this.path.getLeafProperty().getColumnName(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java index 0ad660f9..3c5f6779 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java @@ -334,9 +334,8 @@ public interface AggregatePath extends Iterable { // TODO: Multi-valued paths cannot be represented with a single column // Assert.isTrue(!path.isMultiValued(), () -> "Cannot obtain ColumnInfo for multi-valued path"); - SqlIdentifier name = AggregatePathTableUtils.assembleColumnName(path, - path.getRequiredLeafProperty().getColumnName()); - return new ColumnInfo(name, AggregatePathTableUtils.prefixWithTableAlias(path, name)); + SqlIdentifier columnName = path.getRequiredLeafProperty().getColumnName(); + return new ColumnInfo(columnName, AggregatePathTableUtils.prefixWithTableAlias(path, columnName)); } } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java index fa02220f..a8f9eeae 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java @@ -33,18 +33,6 @@ import org.springframework.data.relational.core.sql.SqlIdentifier; */ class AggregatePathTableUtils { - public static SqlIdentifier assembleColumnName(AggregatePath path, SqlIdentifier suffix) { - return suffix.transform(constructEmbeddedPrefix(path)::concat); - } - - private static String constructEmbeddedPrefix(AggregatePath path) { - - return path.stream() // - .filter(p -> p != path) // - .takeWhile(AggregatePath::isEmbedded).map(p -> p.getRequiredLeafProperty().getEmbeddedPrefix()) // - .collect(new ReverseJoinCollector()); - } - public static SqlIdentifier prefixWithTableAlias(AggregatePath path, SqlIdentifier columnName) { AggregatePath tableOwner = AggregatePathTraversal.getTableOwningPath(path); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java index b462a299..3a696a04 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java @@ -37,9 +37,9 @@ public class AggregatePathTraversal { public static AggregatePath getTableOwningPath(AggregatePath aggregatePath) { - Predicate idDefiningPathFilter = ap -> ap.isEntity() && !ap.isEmbedded(); + Predicate tableOwningPathFilter = ap -> ap.isEntity() && !ap.isEmbedded(); - AggregatePath result = aggregatePath.filter(idDefiningPathFilter); + AggregatePath result = aggregatePath.filter(tableOwningPathFilter); if (result == null) { throw new NoSuchElementException(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java index 767aea78..d5f23073 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java @@ -32,8 +32,12 @@ record EmbeddedContext(RelationalPersistentProperty ownerProperty) { public String withEmbeddedPrefix(String name) { - if (ownerProperty.getEmbeddedPrefix() != null) { - return ownerProperty.getEmbeddedPrefix() + name; + if (!ownerProperty.isEmbedded()) { + return name; + } + String embeddedPrefix = ownerProperty.getEmbeddedPrefix(); + if (embeddedPrefix != null) { + return embeddedPrefix + name; } return name; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java index 46a5ecef..a63f4335 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java @@ -43,6 +43,11 @@ class EmbeddedRelationalPersistentProperty implements RelationalPersistentProper this.context = context; } + @Override + public boolean isEmbedded() { + return delegate.isEmbedded(); + } + @Nullable @Override public String getEmbeddedPrefix() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java index eb7be56d..28dd8c12 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java @@ -214,7 +214,7 @@ public class PersistentPropertyPathExtension { Assert.state(path != null, "Path is null"); - return assembleColumnName(path.getLeafProperty().getColumnName()); + return path.getLeafProperty().getColumnName(); } /** @@ -451,26 +451,6 @@ public class PersistentPropertyPathExtension { } - private SqlIdentifier assembleColumnName(SqlIdentifier suffix) { - - Assert.state(path != null, "Path is null"); - - if (path.getLength() <= 1) { - return suffix; - } - - PersistentPropertyPath parentPath = path.getParentPath(); - RelationalPersistentProperty parentLeaf = parentPath.getLeafProperty(); - - if (!parentLeaf.isEmbedded()) { - return suffix; - } - - String embeddedPrefix = parentLeaf.getEmbeddedPrefix(); - - return getParentPath().assembleColumnName(suffix.transform(embeddedPrefix::concat)); - } - private SqlIdentifier prefixWithTableAlias(SqlIdentifier columnName) { SqlIdentifier tableAlias = getTableAlias(); diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java index 5c5b9df9..54178be9 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java @@ -200,6 +200,7 @@ class DefaultAggregatePathUnitTests { softly.assertThat(path().isEmbedded()).isFalse(); softly.assertThat(path("withId").isEmbedded()).isFalse(); softly.assertThat(path("second2.third").isEmbedded()).isFalse(); + softly.assertThat(path("second2.third2").isEmbedded()).isTrue(); softly.assertThat(path("second2").isEmbedded()).isTrue(); }); @@ -390,8 +391,9 @@ class DefaultAggregatePathUnitTests { assertSoftly(softly -> { - softly.assertThat(path("second.third2.value").getRequiredLeafProperty()) - .isEqualTo(context.getRequiredPersistentEntity(Third.class).getPersistentProperty("value")); + RelationalPersistentProperty prop = path("second.third2.value").getRequiredLeafProperty(); + softly.assertThat(prop.getName()).isEqualTo("value"); + softly.assertThat(prop.getOwner().getType()).isEqualTo(Third.class); softly.assertThat(path("second.third").getRequiredLeafProperty()) .isEqualTo(context.getRequiredPersistentEntity(Second.class).getPersistentProperty("third")); softly.assertThat(path("secondList").getRequiredLeafProperty())