Remove external column name prefixing for embedded entities.

Original pull request #1604 See #1586
spring-projects · Sep 8, 2023 · f600442 · f600442
1 parent d6d9957
commit f600442
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 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
@@ -413,7 +413,7 @@ private Condition mapEmbeddedObjectCondition(CriteriaDefinition criteria, MapSql
 		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 SqlIdentifier getMappedColumnName() {
 				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/...ational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java b/...ational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java
@@ -334,9 +334,8 @@ static ColumnInfo of(AggregatePath path) {
 			// 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/...c/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java b/...c/main/java/org/springframework/data/relational/core/mapping/AggregatePathTableUtils.java
@@ -33,18 +33,6 @@
  */
 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/...rc/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java b/...rc/main/java/org/springframework/data/relational/core/mapping/AggregatePathTraversal.java
@@ -37,9 +37,9 @@ public static AggregatePath getIdDefiningPath(AggregatePath aggregatePath) {
 
 	public static AggregatePath getTableOwningPath(AggregatePath aggregatePath) {
 
-		Predicate<AggregatePath> idDefiningPathFilter = ap -> ap.isEntity() && !ap.isEmbedded();
+		Predicate<AggregatePath> 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/...ional/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java b/...ional/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedContext.java
@@ -32,8 +32,12 @@ public String getEmbeddedPrefix() {
 
 	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/...rg/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java b/...rg/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentProperty.java
@@ -43,6 +43,11 @@ public EmbeddedRelationalPersistentProperty(RelationalPersistentProperty delegat
 		this.context = context;
 	}
 
+	@Override
+	public boolean isEmbedded() {
+		return delegate.isEmbedded();
+	}
+
 	@Nullable
 	@Override
 	public String getEmbeddedPrefix() {

diff --git a/...ava/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java b/...ava/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java
@@ -214,7 +214,7 @@ public SqlIdentifier getColumnName() {
 
 		Assert.state(path != null, "Path is null");
 
-		return assembleColumnName(path.getLeafProperty().getColumnName());
+		return path.getLeafProperty().getColumnName();
 	}
 
 	/**
@@ -451,26 +451,6 @@ private SqlIdentifier assembleTableAlias() {
 
 	}
 
-	private SqlIdentifier assembleColumnName(SqlIdentifier suffix) {
-
-		Assert.state(path != null, "Path is null");
-
-		if (path.getLength() <= 1) {
-			return suffix;
-		}
-
-		PersistentPropertyPath<? extends RelationalPersistentProperty> 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/.../java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java b/.../java/org/springframework/data/relational/core/mapping/DefaultAggregatePathUnitTests.java
@@ -200,6 +200,7 @@ void isEmbedded() {
 			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 @@ void getRequiredLeafProperty() {
 
 		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())