Skip to content

Commit 25f210f

Browse files
schauderschauder
committed
Further Refactored AggregatePath to offer more semantic result types TableInfo and ColumnInfo.
They are now both records. ColumnInfo might also gain a `type` element. getIdColumnName() and getEffectiveIdColumnName() might also become ColumnInfo. For the other methods I currently can't offer an improvement. PersistentPropertyPathExtension is no longer part of the AggregatePath interface. The methods navigating the aggregate path now use the streaming api instead of recursion. This allowed to push helper methods down to the implementation, so they are no longer part of the interface.
1 parent 56d9305 commit 25f210f

20 files changed

+415
-429
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,8 @@ private Object readOrLoadProperty(@Nullable Object id, RelationalPersistentPrope
456456
private Iterable<Object> resolveRelation(@Nullable Object id, RelationalPersistentProperty property) {
457457

458458
Identifier identifier = id == null //
459-
? this.identifier.withPart(rootPath.getQualifierColumn(), key, Object.class) //
460-
: Identifier.of(rootPath.append(property).getReverseColumnName(), id, Object.class);
459+
? this.identifier.withPart(rootPath.getTableInfo().qualifierColumnInfo().name(), key, Object.class) //
460+
: Identifier.of(rootPath.append(property).getTableInfo().reverseColumnInfo().name(), id, Object.class);
461461

462462
PersistentPropertyPath<? extends RelationalPersistentProperty> propertyPath = path.append(property)
463463
.getRequiredPersistentPropertyPath();

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -399,8 +399,9 @@ private EntityRowMapper<?> getEntityRowMapper(AggregatePath path, Identifier ide
399399

400400
private RowMapper<?> getMapEntityRowMapper(AggregatePath path, Identifier identifier) {
401401

402-
SqlIdentifier keyColumn = path.getQualifierColumn();
403-
Assert.notNull(keyColumn, () -> "KeyColumn must not be null for " + path);
402+
AggregatePath.ColumnInfo qualifierColumnInfo = path.getTableInfo().qualifierColumnInfo();
403+
Assert.notNull(qualifierColumnInfo, () -> "Qualifier column must not be null for " + path);
404+
SqlIdentifier keyColumn = qualifierColumnInfo.name();
404405

405406
return new MapEntityRowMapper<>(path, converter, identifier, keyColumn);
406407
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcBackReferencePropertyValueProvider.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class JdbcBackReferencePropertyValueProvider implements PropertyValueProvider<Re
4545

4646
@Override
4747
public <T> T getPropertyValue(RelationalPersistentProperty property) {
48-
return (T) resultSet.getObject(basePath.append(property).getReverseColumnNameAlias().getReference());
48+
return (T) resultSet.getObject(basePath.append(property).getTableInfo().reverseColumnInfo().alias().getReference());
4949
}
5050

5151
public JdbcBackReferencePropertyValueProvider extendBy(RelationalPersistentProperty property) {

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcIdentifierBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public static JdbcIdentifierBuilder forBackReferences(JdbcConverter converter, A
5656
@Nullable Object value) {
5757

5858
Identifier identifier = Identifier.of( //
59-
path.getReverseColumnName(), //
59+
path.getTableInfo().reverseColumnInfo().name(), //
6060
value, //
6161
converter.getColumnType(path.getIdDefiningParentPath().getRequiredIdProperty()) //
6262
);
@@ -90,7 +90,7 @@ public JdbcIdentifierBuilder withQualifier(AggregatePath path, Object value) {
9090
Assert.notNull(path, "Path must not be null");
9191
Assert.notNull(value, "Value must not be null");
9292

93-
identifier = identifier.withPart(path.getQualifierColumn(), value, path.getQualifierColumnType());
93+
identifier = identifier.withPart(path.getTableInfo().qualifierColumnInfo().name(), value, path.getTableInfo().qualifierColumnType());
9494

9595
return this;
9696
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcPropertyValueProvider.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public boolean hasProperty(RelationalPersistentProperty property) {
5757
}
5858

5959
private String getColumnName(RelationalPersistentProperty property) {
60-
return basePath.append(property).getColumnAlias().getReference();
60+
AggregatePath.ColumnInfo columnInfo = basePath.append(property).getColumnInfo();
61+
return columnInfo.alias().getReference();
6162
}
6263

6364
public JdbcPropertyValueProvider extendBy(RelationalPersistentProperty property) {

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package org.springframework.data.jdbc.core.convert;
1717

1818
import org.springframework.data.relational.core.mapping.AggregatePath;
19-
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
2019
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
2120
import org.springframework.data.relational.core.sql.Column;
2221
import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -55,16 +54,18 @@ Table getTable() {
5554

5655
Table getTable(AggregatePath path) {
5756

58-
SqlIdentifier tableAlias = path.getTableAlias();
59-
Table table = Table.create(path.getQualifiedTableName());
57+
SqlIdentifier tableAlias = path.getTableInfo().tableAlias();
58+
Table table = Table.create(path.getTableInfo().qualifiedTableName());
6059
return tableAlias == null ? table : table.as(tableAlias);
6160
}
6261

6362
Column getColumn(AggregatePath path) {
64-
return getTable(path).column(path.getColumnName()).as(path.getColumnAlias());
63+
AggregatePath.ColumnInfo columnInfo = path.getColumnInfo();
64+
AggregatePath.ColumnInfo columnInfo1 = path.getColumnInfo();
65+
return getTable(path).column(columnInfo1.name()).as(columnInfo.alias());
6566
}
6667

6768
Column getReverseColumn(AggregatePath path) {
68-
return getTable(path).column(path.getReverseColumnName()).as(path.getReverseColumnNameAlias());
69+
return getTable(path).column(path.getTableInfo().reverseColumnInfo().name()).as(path.getTableInfo().reverseColumnInfo().alias());
6970
}
7071
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java

+12-13
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.springframework.data.relational.core.dialect.Dialect;
2828
import org.springframework.data.relational.core.dialect.RenderContextFactory;
2929
import org.springframework.data.relational.core.mapping.AggregatePath;
30-
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
3130
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
3231
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
3332
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
@@ -134,9 +133,9 @@ private Condition getSubselectCondition(AggregatePath path, Function<Column, Con
134133
return rootCondition.apply(filterColumn);
135134
}
136135

137-
Table subSelectTable = Table.create(parentPath.getQualifiedTableName());
138-
Column idColumn = subSelectTable.column(parentPath.getIdColumnName());
139-
Column selectFilterColumn = subSelectTable.column(parentPath.getEffectiveIdColumnName());
136+
Table subSelectTable = Table.create(parentPath.getTableInfo().qualifiedTableName());
137+
Column idColumn = subSelectTable.column(parentPath.getTableInfo().idColumnName());
138+
Column selectFilterColumn = subSelectTable.column(parentPath.getTableInfo().effectiveIdColumnName());
140139

141140
Condition innerCondition;
142141

@@ -219,7 +218,7 @@ String getFindAllByProperty(Identifier parentIdentifier,
219218

220219
AggregatePath path = mappingContext.getAggregatePath(propertyPath);
221220

222-
return getFindAllByProperty(parentIdentifier, path.getQualifierColumn(), path.isOrdered());
221+
return getFindAllByProperty(parentIdentifier, path.getTableInfo().qualifierColumnInfo(), path.isOrdered());
223222
}
224223

225224
/**
@@ -234,7 +233,7 @@ String getFindAllByProperty(Identifier parentIdentifier,
234233
* keyColumn must not be {@code null}.
235234
* @return a SQL String.
236235
*/
237-
String getFindAllByProperty(Identifier parentIdentifier, @Nullable SqlIdentifier keyColumn, boolean ordered) {
236+
String getFindAllByProperty(Identifier parentIdentifier, @Nullable AggregatePath.ColumnInfo keyColumn, boolean ordered) {
238237

239238
Assert.isTrue(keyColumn != null || !ordered,
240239
"If the SQL statement should be ordered a keyColumn to order by must be provided");
@@ -244,14 +243,14 @@ String getFindAllByProperty(Identifier parentIdentifier, @Nullable SqlIdentifier
244243
SelectBuilder.SelectWhere builder = selectBuilder( //
245244
keyColumn == null //
246245
? Collections.emptyList() //
247-
: Collections.singleton(keyColumn) //
246+
: Collections.singleton(keyColumn.name()) //
248247
);
249248

250249
Condition condition = buildConditionForBackReference(parentIdentifier, table);
251250
SelectBuilder.SelectWhereAndOr withWhereClause = builder.where(condition);
252251

253252
Select select = ordered //
254-
? withWhereClause.orderBy(table.column(keyColumn).as(keyColumn)).build() //
253+
? withWhereClause.orderBy(table.column(keyColumn.name()).as(keyColumn.alias())).build() //
255254
: withWhereClause.build();
256255

257256
return render(select);
@@ -586,8 +585,8 @@ Join getJoin(AggregatePath path) {
586585

587586
return new Join( //
588587
currentTable, //
589-
currentTable.column(path.getReverseColumnName()), //
590-
parentTable.column(idDefiningParentPath.getIdColumnName()) //
588+
currentTable.column(path.getTableInfo().reverseColumnInfo().name()), //
589+
parentTable.column(idDefiningParentPath.getTableInfo().idColumnName()) //
591590
);
592591
}
593592

@@ -710,13 +709,13 @@ private DeleteBuilder.DeleteWhereAndOr createBaseDeleteByIdIn(Table table) {
710709

711710
private String createDeleteByPathAndCriteria(AggregatePath path, Function<Column, Condition> rootCondition) {
712711

713-
Table table = Table.create(path.getQualifiedTableName());
712+
Table table = Table.create(path.getTableInfo().qualifiedTableName());
714713

715714
DeleteBuilder.DeleteWhere builder = Delete.builder() //
716715
.from(table);
717716
Delete delete;
718717

719-
Column filterColumn = table.column(path.getReverseColumnName());
718+
Column filterColumn = table.column(path.getTableInfo().reverseColumnInfo().name());
720719

721720
if (path.getLength() == 1) {
722721

@@ -926,7 +925,7 @@ private SelectBuilder.SelectJoin getExistsSelect() {
926925
for (PersistentPropertyPath<RelationalPersistentProperty> path : mappingContext
927926
.findPersistentPropertyPaths(entity.getType(), p -> true)) {
928927

929-
AggregatePath aggregatePath = mappingContext.getAggregatePath( path);
928+
AggregatePath aggregatePath = mappingContext.getAggregatePath(path);
930929

931930
// add a join if necessary
932931
Join join = getJoin(aggregatePath);

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/JdbcQueryCreator.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,8 @@ Join getJoin(SqlContext sqlContext, AggregatePath path) {
321321

322322
return new Join( //
323323
currentTable, //
324-
currentTable.column(path.getReverseColumnName()), //
325-
parentTable.column(idDefiningParentPath.getIdColumnName()) //
324+
currentTable.column(path.getTableInfo().reverseColumnInfo().name()), //
325+
parentTable.column(idDefiningParentPath.getTableInfo().idColumnName()) //
326326
);
327327
}
328328

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/SqlContext.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package org.springframework.data.jdbc.repository.query;
1717

1818
import org.springframework.data.relational.core.mapping.AggregatePath;
19-
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
2019
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
2120
import org.springframework.data.relational.core.sql.Column;
2221
import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -56,16 +55,19 @@ Table getTable() {
5655

5756
Table getTable(AggregatePath path) {
5857

59-
SqlIdentifier tableAlias = path.getTableAlias();
60-
Table table = Table.create(path.getQualifiedTableName());
58+
SqlIdentifier tableAlias = path.getTableInfo().tableAlias();
59+
Table table = Table.create(path.getTableInfo().qualifiedTableName());
6160
return tableAlias == null ? table : table.as(tableAlias);
6261
}
6362

6463
Column getColumn(AggregatePath path) {
65-
return getTable(path).column(path.getColumnName()).as(path.getColumnAlias());
64+
AggregatePath.ColumnInfo columnInfo = path.getColumnInfo();
65+
AggregatePath.ColumnInfo columnInfo1 = path.getColumnInfo();
66+
return getTable(path).column(columnInfo1.name()).as(columnInfo.alias());
6667
}
6768

6869
Column getReverseColumn(AggregatePath path) {
69-
return getTable(path).column(path.getReverseColumnName()).as(path.getReverseColumnNameAlias());
70+
return getTable(path).column(path.getTableInfo().reverseColumnInfo().name())
71+
.as(path.getTableInfo().reverseColumnInfo().alias());
7072
}
7173
}

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathTestUtils.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
*/
2929
public final class PersistentPropertyPathTestUtils {
3030

31-
private PropertyPathTestingUtils() {
31+
private PersistentPropertyPathTestUtils() {
3232
throw new UnsupportedOperationException("This is a utility class and cannot be instantiated");
3333
}
3434

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java

+25-20
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@
4040
import org.springframework.data.relational.core.dialect.Dialect;
4141
import org.springframework.data.relational.core.dialect.PostgresDialect;
4242
import org.springframework.data.relational.core.dialect.SqlServerDialect;
43+
import org.springframework.data.relational.core.mapping.AggregatePath;
4344
import org.springframework.data.relational.core.mapping.Column;
4445
import org.springframework.data.relational.core.mapping.DefaultNamingStrategy;
45-
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
4646
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
4747
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
4848
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
@@ -388,7 +388,7 @@ void findAllByPropertyWithMultipartIdentifier() {
388388
void findAllByPropertyWithKey() {
389389

390390
// this would get called when ListParent is th element type of a Map
391-
String sql = sqlGenerator.getFindAllByProperty(BACKREF, unquoted("key-column"), false);
391+
String sql = sqlGenerator.getFindAllByProperty(BACKREF, new AggregatePath.ColumnInfo(unquoted("key-column"),unquoted("key-column")), false);
392392

393393
assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name, " //
394394
+ "dummy_entity.x_other AS x_other, " //
@@ -411,7 +411,8 @@ void findAllByPropertyOrderedWithoutKey() {
411411
void findAllByPropertyWithKeyOrdered() {
412412

413413
// this would get called when ListParent is th element type of a Map
414-
String sql = sqlGenerator.getFindAllByProperty(BACKREF, unquoted("key-column"), true);
414+
String sql = sqlGenerator.getFindAllByProperty(BACKREF,
415+
new AggregatePath.ColumnInfo(unquoted("key-column"), unquoted("key-column")), true);
415416

416417
assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name, " //
417418
+ "dummy_entity.x_other AS x_other, " //
@@ -431,8 +432,10 @@ public void findAllByPropertyAvoidsDuplicateColumns() {
431432
final SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class);
432433
final String sql = sqlGenerator.getFindAllByProperty(
433434
Identifier.of(quoted("id"), "parent-id-value", DummyEntity.class), //
434-
quoted("X_L1ID"), // this key column collides with the name derived by the naming strategy for the id of
435-
// ReferencedEntity.
435+
new AggregatePath.ColumnInfo(quoted("X_L1ID"), quoted("X_L1ID")), // this key column collides with the name
436+
// derived by the naming strategy for the id
437+
// of
438+
// ReferencedEntity.
436439
false);
437440

438441
final String id = "referenced_entity.x_l1id AS x_l1id";
@@ -446,10 +449,11 @@ public void findAllByPropertyAvoidsDuplicateColumns() {
446449
void findAllByPropertyWithEmptyBackrefColumn() {
447450

448451
Identifier emptyIdentifier = Identifier.of(EMPTY, 0, Object.class);
449-
assertThatThrownBy(() -> sqlGenerator.getFindAllByProperty(emptyIdentifier, unquoted("key-column"), false)) //
450-
.isInstanceOf(IllegalArgumentException.class) //
451-
.hasMessageContaining(
452-
"An empty SqlIdentifier can't be used in condition. Make sure that all composite primary keys are defined in the query");
452+
assertThatThrownBy(() -> sqlGenerator.getFindAllByProperty(emptyIdentifier,
453+
new AggregatePath.ColumnInfo(unquoted("key-column"), unquoted("key-column")), false)) //
454+
.isInstanceOf(IllegalArgumentException.class) //
455+
.hasMessageContaining(
456+
"An empty SqlIdentifier can't be used in condition. Make sure that all composite primary keys are defined in the query");
453457
}
454458

455459
@Test // DATAJDBC-219
@@ -573,15 +577,16 @@ void readOnlyPropertyIncludedIntoQuery_when_generateFindAllByPropertySql() {
573577

574578
final SqlGenerator sqlGenerator = createSqlGenerator(EntityWithReadOnlyProperty.class);
575579

576-
assertThat(sqlGenerator.getFindAllByProperty(BACKREF, unquoted("key-column"), true)).isEqualToIgnoringCase( //
577-
"SELECT " //
578-
+ "entity_with_read_only_property.x_id AS x_id, " //
579-
+ "entity_with_read_only_property.x_name AS x_name, " //
580-
+ "entity_with_read_only_property.x_read_only_value AS x_read_only_value, " //
581-
+ "entity_with_read_only_property.key-column AS key-column " //
582-
+ "FROM entity_with_read_only_property " //
583-
+ "WHERE entity_with_read_only_property.backref = :backref " //
584-
+ "ORDER BY key-column" //
580+
assertThat(sqlGenerator.getFindAllByProperty(BACKREF,
581+
new AggregatePath.ColumnInfo(unquoted("key-column"), unquoted("key-column")), true)).isEqualToIgnoringCase( //
582+
"SELECT " //
583+
+ "entity_with_read_only_property.x_id AS x_id, " //
584+
+ "entity_with_read_only_property.x_name AS x_name, " //
585+
+ "entity_with_read_only_property.x_read_only_value AS x_read_only_value, " //
586+
+ "entity_with_read_only_property.key-column AS key-column " //
587+
+ "FROM entity_with_read_only_property " //
588+
+ "WHERE entity_with_read_only_property.backref = :backref " //
589+
+ "ORDER BY key-column" //
585590
);
586591
}
587592

@@ -933,8 +938,8 @@ private SqlIdentifier getAlias(Object maybeAliased) {
933938
@Nullable
934939
private org.springframework.data.relational.core.sql.Column generatedColumn(String path, Class<?> type) {
935940

936-
return createSqlGenerator(type, AnsiDialect.INSTANCE).getColumn(
937-
context.getAggregatePath(PersistentPropertyPathTestUtils.getPath(path, type, context)));
941+
return createSqlGenerator(type, AnsiDialect.INSTANCE)
942+
.getColumn(context.getAggregatePath(PersistentPropertyPathTestUtils.getPath(path, type, context)));
938943
}
939944

940945
private PersistentPropertyPath<RelationalPersistentProperty> getPath(String path, Class<?> baseType) {

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mapping/model/DefaultNamingStrategyUnitTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void getColumnName() {
5858
}
5959

6060
@Test // DATAJDBC-184
61-
public void getReverseColumnName() {
61+
public void getReverseColumnInfoName() {
6262
assertThat(target.getReverseColumnName(persistentEntity.getPersistentProperty("dummySubEntities")))
6363
.isEqualTo("dummy_entity");
6464
}

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private void forAllTableRepresentingPaths(Class<?> entityType,
127127
Consumer<PersistentPropertyPath<RelationalPersistentProperty>> pathConsumer) {
128128

129129
context.findPersistentPropertyPaths(entityType, property -> property.isEntity() && !property.isEmbedded()) //
130-
.filter(AggregatePath::isWritable) //
130+
.filter(path -> context.getAggregatePath(path).isWritable()) //
131131
.forEach(pathConsumer);
132132
}
133133
}

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/WritingContext.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class WritingContext<T> {
6464
this.rootIdValueSource = IdValueSource.forInstance(root,
6565
context.getRequiredPersistentEntity(aggregateChange.getEntityType()));
6666
this.paths = context.findPersistentPropertyPaths(entityType, (p) -> p.isEntity() && !p.isEmbedded()) //
67-
.filter(AggregatePath::isWritable).toList();
67+
.filter(ppp -> context.getAggregatePath(ppp).isWritable()).toList();
6868
}
6969

7070
/**

0 commit comments

Comments
 (0)