Fix assemble table alias for embedded entity with empty prefix and reference

Author: IAM20

spring-data-jdbc-plus-sql/src/main/java/com/navercorp/spring/data/jdbc/plus/sql/convert/PropertyPathUtils.java

@@ -22,6 +22,7 @@
 import org.springframework.data.relational.core.sql.IdentifierProcessing;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * PropertyPathUtils to get ColumnAlias and TableAlias applied @SqlTableAlias
@@ -110,20 +111,24 @@ private static PersistentPropertyPathExtension getTableOwningAncestor(Persistent
 		return path.isEntity() && !path.isEmbedded() ? path : getTableOwningAncestor(path.getParentPath());
 	}
 
+	@Nullable
 	private static SqlIdentifier assembleTableAlias(PersistentPropertyPathExtension path) {
 
 		Assert.state(path != null, "Path is null");
 
 		String prefix = TableAliasUtils.getTableAliasPropertyPathPrefix(path);
 		if (path.getLength() == 1) {
 			Assert.notNull(prefix, "Prefix must not be null.");
-			return SqlIdentifier.quoted(prefix);
+			return StringUtils.hasText(prefix) ? SqlIdentifier.quoted(prefix) : null;
 		}
 
 		PersistentPropertyPathExtension parentPath = path.getParentPath();
 		SqlIdentifier sqlIdentifier = assembleTableAlias(parentPath);
 
-		return parentPath.isEmbedded() ? sqlIdentifier.transform(name -> name.concat(prefix))
-			: sqlIdentifier.transform(name -> name + "_" + prefix);
+		if (sqlIdentifier != null) {
+			return parentPath.isEmbedded() ? sqlIdentifier.transform(name -> name.concat(prefix))
+				: sqlIdentifier.transform(name -> name + "_" + prefix);
+		}
+		return SqlIdentifier.quoted(prefix);
 	}
 }

spring-data-jdbc-plus-sql/src/test/java/com/navercorp/spring/data/jdbc/plus/sql/convert/SqlGeneratorEmbeddedTest.java

@@ -3,6 +3,8 @@
 import static java.util.Collections.*;
 import static org.assertj.core.api.Assertions.*;
 import static org.springframework.data.relational.core.mapping.Embedded.*;
+import static org.springframework.data.relational.core.sql.SqlIdentifier.quoted;
+import static org.springframework.data.relational.core.sql.SqlIdentifier.unquoted;
 
 import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -12,11 +14,13 @@
 import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
 import org.springframework.data.jdbc.core.convert.JdbcConverter;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
+import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.relational.core.mapping.Column;
 import org.springframework.data.relational.core.mapping.Embedded;
 import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
+import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
 import org.springframework.data.relational.core.sql.Aliased;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
 
@@ -195,10 +199,10 @@ public void columnForEmbeddedProperty() {
 				c -> getAlias(c.getTable()), //
 				this::getAlias) //
 			.containsExactly( //
-				SqlIdentifier.unquoted("test"), //
-				SqlIdentifier.unquoted("dummy_entity"), //
+				unquoted("test"), //
+				unquoted("dummy_entity"), //
 				null, //
-				SqlIdentifier.unquoted("test"));
+				unquoted("test"));
 	}
 
 	@Test // DATAJDBC-340
@@ -226,10 +230,10 @@ public void columnForPrefixedEmbeddedProperty() {
 				c -> getAlias(c.getTable()), //
 				this::getAlias) //
 			.containsExactly( //
-				SqlIdentifier.unquoted("prefix_test"), //
-				SqlIdentifier.unquoted("dummy_entity"), //
+				unquoted("prefix_test"), //
+				unquoted("dummy_entity"), //
 				null, //
-				SqlIdentifier.unquoted("prefix_test"));
+				unquoted("prefix_test"));
 	}
 
 	@Test // DATAJDBC-340
@@ -249,8 +253,8 @@ public void columnForCascadedEmbeddedProperty() {
 				c -> c.getTable().getName(),
 				c -> getAlias(c.getTable()),
 				this::getAlias)
-			.containsExactly(SqlIdentifier.unquoted("attr1"), SqlIdentifier.unquoted("dummy_entity"), null,
-				SqlIdentifier.unquoted("attr1"));
+			.containsExactly(unquoted("attr1"), unquoted("dummy_entity"), null,
+				unquoted("attr1"));
 	}
 
 	@Test // DATAJDBC-340
@@ -261,14 +265,14 @@ public void joinForEmbeddedWithReference() {
 		SoftAssertions.assertSoftly(softly -> {
 
 			softly.assertThat(join.getJoinTable().getName())
-				.isEqualTo(SqlIdentifier.unquoted("other_entity"));
+				.isEqualTo(unquoted("other_entity"));
 			softly.assertThat(join.getJoinColumn().getTable()).isEqualTo(join.getJoinTable());
 			softly.assertThat(join.getJoinColumn().getName())
-				.isEqualTo(SqlIdentifier.unquoted("dummy_entity2"));
+				.isEqualTo(unquoted("dummy_entity2"));
 			softly.assertThat(join.getParentId().getName())
-				.isEqualTo(SqlIdentifier.unquoted("id"));
+				.isEqualTo(unquoted("id"));
 			softly.assertThat(join.getParentId().getTable().getName())
-				.isEqualTo(SqlIdentifier.unquoted("dummy_entity2"));
+				.isEqualTo(unquoted("dummy_entity2"));
 		});
 	}
 
@@ -282,10 +286,20 @@ public void columnForEmbeddedWithReferenceProperty() {
 				c -> getAlias(c.getTable()), //
 				this::getAlias) //
 			.containsExactly( //
-				SqlIdentifier.unquoted("value"), //
-				SqlIdentifier.unquoted("other_entity"), //
-				SqlIdentifier.quoted("prefix_other"), //
-				SqlIdentifier.unquoted("prefix_other_value"));
+				unquoted("value"), //
+				unquoted("other_entity"), //
+				quoted("prefix_other"), //
+				unquoted("prefix_other_value"));
+	}
+
+	@Test
+	public void getTableAlias() {
+		SoftAssertions.assertSoftly(softly -> {
+			softly.assertThat(PropertyPathUtils.getTableAlias(extPath("prefixEmbeddable.other")))
+				.isEqualTo(quoted("prefix_other"));
+			softly.assertThat(PropertyPathUtils.getTableAlias(extPath("embeddable.other")))
+				.isEqualTo(quoted("other"));
+		});
 	}
 
 	private SqlGenerator.Join generateJoin(String path, Class<?> type) {
@@ -303,6 +317,17 @@ private SqlIdentifier getAlias(Object maybeAliased) {
 		return null;
 	}
 
+	private PersistentPropertyPathExtension extPath(RelationalPersistentEntity<?> entity) {
+		return new PersistentPropertyPathExtension(context, entity);
+	}
+	private PersistentPropertyPathExtension extPath(String path) {
+		return new PersistentPropertyPathExtension(context, createSimplePath(path));
+	}
+
+	PersistentPropertyPath<RelationalPersistentProperty> createSimplePath(String path) {
+		return PropertyPathTestingUtils.toPath(path, DummyEntity3.class, context);
+	}
+
 	private org.springframework.data.relational.core.sql.Column generatedColumn(
 		String path, Class<?> type) {
 
@@ -350,6 +375,17 @@ static class DummyEntity2 {
 		EmbeddedWithReference embedded;
 	}
 
+	static class DummyEntity3 {
+		@Id
+		Long id;
+
+		@Embedded(onEmpty = OnEmpty.USE_NULL, prefix = "prefix_")
+		EmbeddedWithReference prefixEmbeddable;
+
+		@Embedded(onEmpty = OnEmpty.USE_NULL)
+		EmbeddedWithReference embeddable;
+	}
+
 	static class EmbeddedWithReference {
 		OtherEntity other;
 	}