Migrate ColumnInfos to use of the long path only.

Turns out the concept of base path becomes superfluous.

Original pull request #1957
See #574

Author: schauder

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java

@@ -199,15 +199,16 @@ private Identifier getParentKeys(DbAction.WithDependingOn<?> action, JdbcConvert
 
 	static Function<AggregatePath, Object> getIdMapper(Object idValue, AggregatePath path, JdbcConverter converter) {
 
+		RelationalPersistentProperty idProperty = path.getIdDefiningParentPath().getRequiredIdProperty();
 		RelationalPersistentEntity<?> entity = converter.getMappingContext()
-				.getPersistentEntity(path.getIdDefiningParentPath().getRequiredIdProperty());
+				.getPersistentEntity(idProperty);
 
 		if (entity == null) {
 			return aggregatePath -> idValue;
 		}
 
 		PersistentPropertyPathAccessor<Object> propertyPathAccessor = entity.getPropertyPathAccessor(idValue);
-		return aggregatePath -> propertyPathAccessor.getProperty(aggregatePath.getRequiredPersistentPropertyPath());
+		return aggregatePath -> propertyPathAccessor.getProperty(aggregatePath.getSubPathBasedOn(idProperty.getActualType()).getRequiredPersistentPropertyPath());
 	}
 
 	private Object getParentId(DbAction.WithDependingOn<?> action) {

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

@@ -465,7 +465,17 @@ private static Function<AggregatePath, Object> getWrappedValueProvider(Function<
 		RelationalPersistentProperty idProperty = idDefiningParentPath.getRequiredIdProperty();
 		AggregatePath idPath = idProperty.isEntity() ? idDefiningParentPath.append(idProperty) : idDefiningParentPath;
 
-		return ap -> valueProvider.apply(idPath.append(ap));
+		return ap -> valueProvider.apply(smartAppend(idPath, ap));
+	}
+
+	private static AggregatePath smartAppend(AggregatePath base, AggregatePath suffix) {
+
+		RelationalPersistentEntity<?> owner = suffix.getRequiredBaseProperty().getOwner();
+		if (owner.equals(base.getRequiredLeafEntity())) {
+			return base.append(suffix);
+		} else {
+			return smartAppend(base, suffix.getTail());
+		}
 	}
 
 	/**

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

@@ -929,7 +929,7 @@ private Column getSingleNonNullColumn() {
 		// return getTable(path).column(columnInfo.name()).as(columnInfo.alias());
 
 		AggregatePath.ColumnInfos columnInfos = mappingContext.getAggregatePath(entity).getTableInfo().idColumnInfos();
-		return columnInfos.any((ap, ci) -> sqlContext.getColumn(columnInfos.fullPath(ap)));
+		return columnInfos.any((ap, ci) -> sqlContext.getColumn(ap));
 	}
 
 	private List<Column> getIdColumns() {
@@ -938,7 +938,7 @@ private List<Column> getIdColumns() {
 
 		// sqlcontext.getColumn (vs sqlContext.getTable
 		return columnInfos
-				.toColumnList((aggregatePath, columnInfo) -> sqlContext.getColumn(columnInfos.fullPath(aggregatePath)));
+				.toColumnList((aggregatePath, columnInfo) -> sqlContext.getColumn(aggregatePath));
 	}
 
 	private Column getVersionColumn() {

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

@@ -88,8 +88,8 @@ <T> SqlIdentifierParameterSource forInsert(T instance, Class<T> domainType, Iden
 			AggregatePath.ColumnInfos columnInfos = context.getAggregatePath(persistentEntity).getTableInfo().idColumnInfos();
 
 			//  fullPath: because we use the result with a PropertyPathAccessor
-			columnInfos.forEach((ap, __) -> {
-				Object idValue = propertyPathAccessor.getProperty(columnInfos.fullPath(ap).getRequiredPersistentPropertyPath());
+			columnInfos.forEachLong((ap, __) -> {
+				Object idValue = propertyPathAccessor.getProperty(ap.getRequiredPersistentPropertyPath());
 				RelationalPersistentProperty idProperty = ap.getRequiredLeafProperty();
 				addConvertedPropertyValue(parameterSource, idProperty, idValue, idProperty.getColumnName());
 			});

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

@@ -144,6 +144,8 @@ static Function<AggregatePath, Object> getValueProvider(Object idValue, Aggregat
 			if (entity == null) {
 				return idValue;
 			} else {
+
+				ap = ap.getTail();
 				PersistentPropertyPathAccessor<Object> propertyPathAccessor = entity.getPropertyPathAccessor(idValue);
 				return propertyPathAccessor.getProperty(ap.getRequiredPersistentPropertyPath());
 			}

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/AggregatePath.java

@@ -289,6 +289,8 @@ default int compareTo(AggregatePath other) {
 		return toDotPath().compareTo(other.toDotPath());
 	}
 
+	AggregatePath getSubPathBasedOn(Class<?> baseType);
+
 	/**
 	 * Information about a table underlying an entity.
 	 *
@@ -343,7 +345,7 @@ static TableInfo of(AggregatePath path) {
 		private static ColumnInfos computeIdColumnInfos(AggregatePath tableOwner,
 				RelationalPersistentEntity<?> leafEntity) {
 
-			ColumnInfos idColumnInfos = ColumnInfos.empty(tableOwner);
+			ColumnInfos idColumnInfos = ColumnInfos.empty();
 			if (!leafEntity.hasIdProperty()) {
 				return idColumnInfos;
 			}
@@ -370,7 +372,7 @@ private static ColumnInfos computeBackReferenceColumnInfos(AggregatePath path) {
 			AggregatePath tableOwner = AggregatePathTraversal.getTableOwningPath(path);
 
 			if (tableOwner.isRoot()) {
-				return ColumnInfos.empty(tableOwner);
+				return ColumnInfos.empty();
 			}
 
 			AggregatePath idDefiningParentPath = tableOwner.getIdDefiningParentPath();
@@ -439,7 +441,7 @@ public ColumnInfo reverseColumnInfo() {
 		 * @return ColumnInfos representing the effective id of this entity. Guaranteed not to be {@literal null}.
 		 */
 		public ColumnInfos effectiveIdColumnInfos() {
-			return backReferenceColumnInfos.columnInfos.isEmpty() ? idColumnInfos : backReferenceColumnInfos;
+			return backReferenceColumnInfos.isEmpty() ? idColumnInfos : backReferenceColumnInfos;
 		}
 	}
 
@@ -484,33 +486,29 @@ static ColumnInfo of(AggregatePath path) {
 	 */
 	class ColumnInfos {
 
-		private final AggregatePath basePath;
 		private final Map<AggregatePath, ColumnInfo> columnInfos;
 		private final Map<Table, List<Column>> columnCache;
 
 		/**
 		 * Creates a new ColumnInfos instances based on the arguments.
 		 *
-		 * @param basePath The path on which all other paths in the other argument are based on. For the typical case of a
-		 *          composite id, this would be the path to the composite ids.
 		 * @param columnInfos A map, mapping {@literal AggregatePath} instances to the respective {@literal ColumnInfo}
 		 */
-		ColumnInfos(AggregatePath basePath, Map<AggregatePath, ColumnInfo> columnInfos) {
+		ColumnInfos(Map<AggregatePath, ColumnInfo> columnInfos) {
 
-			this.basePath = basePath;
-			this.columnInfos = columnInfos;
+			this.columnInfos = new TreeMap<>();
+			this.columnInfos.putAll(columnInfos);
 			this.columnCache = new HashMap<>(columnInfos.size(), 1f);
 		}
 
 		/**
 		 * An empty {@literal ColumnInfos} instance with a fixed base path. Useful as a base when collecting
 		 * {@link ColumnInfo} instances into an {@literal ColumnInfos} instance.
 		 *
-		 * @param basePath The path on which paths in the {@literal ColumnInfos} or derived objects will be based on.
 		 * @return an empty instance save the {@literal basePath}.
 		 */
-		public static ColumnInfos empty(AggregatePath basePath) {
-			return new ColumnInfos(basePath, new HashMap<>());
+		public static ColumnInfos empty() {
+			return new ColumnInfos(new HashMap<>());
 		}
 
 		/**
@@ -550,8 +548,7 @@ public ColumnInfo any() {
 		 */
 		public List<Column> toColumnList(Table table) {
 
-			return columnCache.computeIfAbsent(table,
-					t -> toColumnList((__,ci) -> t.column(ci.name)));
+			return columnCache.computeIfAbsent(table, t -> toColumnList((__, ci) -> t.column(ci.name)));
 		}
 
 		/**
@@ -608,6 +605,15 @@ public void forEach(BiConsumer<AggregatePath, ColumnInfo> consumer) {
 			columnInfos.forEach(consumer);
 		}
 
+		/**
+		 * Calls the consumer for each pair of {@link AggregatePath} and {@literal ColumnInfo}.
+		 *
+		 * @param consumer the function to call.
+		 */
+		public void forEachLong(BiConsumer<AggregatePath, ColumnInfo> consumer) {
+			columnInfos.forEach(consumer);
+		}
+
 		/**
 		 * Calls the {@literal mapper} for each pair one pair of {@link AggregatePath} and {@link ColumnInfo}, if there is
 		 * any.
@@ -632,16 +638,6 @@ public ColumnInfo get(AggregatePath path) {
 			return columnInfos.get(path);
 		}
 
-		/**
-		 * Constructs an {@link AggregatePath} from the {@literal basePath} and the provided argument.
-		 *
-		 * @param ap {@literal AggregatePath} to be appended to the {@literal basePath}.
-		 * @return the combined (@literal AggregatePath}
-		 */
-		public AggregatePath fullPath(AggregatePath ap) {
-			return basePath.append(ap);
-		}
-
 		/**
 		 * Return an {@link Iterable} of {@link AggregatePath} associated with this column infos.
 		 *
@@ -669,5 +665,4 @@ public int size() {
 			return columnInfos.size();
 		}
 	}
-
 }

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/ColumInfosBuilder.java

@@ -69,7 +69,7 @@ void add(RelationalPersistentProperty property, SqlIdentifier name, SqlIdentifie
 	 * @param columnInfo the {@literal ColumnInfo} added.
 	 */
 	void add(AggregatePath path, AggregatePath.ColumnInfo columnInfo) {
-		columnInfoMap.put(path.subtract(basePath), columnInfo);
+		columnInfoMap.put(path, columnInfo);
 	}
 
 	/**
@@ -78,7 +78,7 @@ void add(AggregatePath path, AggregatePath.ColumnInfo columnInfo) {
 	 * @return a {@literal ColumnInfos} instance containing all the added {@link AggregatePath.ColumnInfo} instances.
 	 */
 	AggregatePath.ColumnInfos build() {
-		return new AggregatePath.ColumnInfos(basePath, columnInfoMap);
+		return new AggregatePath.ColumnInfos(columnInfoMap);
 	}
 
 }

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DefaultAggregatePath.java

@@ -250,6 +250,24 @@ public AggregatePath subtract(@Nullable AggregatePath basePath) {
 		throw new IllegalStateException("Can't subtract [%s] from [%s]".formatted(basePath, this));
 	}
 
+	@Override
+	public AggregatePath getSubPathBasedOn(Class<?> baseType) {
+
+		if (isRoot()) {
+			if (rootType.getType() != baseType) {
+				throw new IllegalStateException("No matching path found for [%s]".formatted(baseType));
+			}
+			return this;
+		}
+
+		RelationalPersistentEntity<?> owner = getRequiredBaseProperty().getOwner();
+		if (owner.getType() == baseType) {
+			return this;
+		}
+		
+		return getTail().getSubPathBasedOn(baseType);
+	}
+
 	/**
 	 * Creates an {@link Iterator} that iterates over the current path and all ancestors. It will start with the current
 	 * path, followed by its parent until ending with the root.

spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/ColumnInfosUnitTests.java

@@ -41,7 +41,7 @@ class ColumnInfosUnitTests {
 	@Test // GH-574
 	void emptyColumnInfos() {
 
-		AggregatePath.ColumnInfos columnInfos = AggregatePath.ColumnInfos.empty(basePath(DummyEntity.class));
+		AggregatePath.ColumnInfos columnInfos = AggregatePath.ColumnInfos.empty();
 
 		assertThat(columnInfos.isEmpty()).isTrue();
 		assertThrows(NoSuchElementException.class, columnInfos::any);
@@ -76,10 +76,7 @@ void multiElementColumnInfos() {
 
 		List<String> collector = new ArrayList<>();
 		columnInfos.forEach((ap, ci) -> collector.add(ap.toDotPath() + "+" + ci.name()));
-		assertThat(collector).containsExactly("one+\"ONE\"", "two+\"TWO\"");
-
-		columnInfos.get(getPath(CompositeId.class, "one"));
-
+		assertThat(collector).containsExactly("id.one+\"ONE\"", "id.two+\"TWO\"");
 	}
 
 	private AggregatePath getPath(Class<?> type, String name) {