Cache invalid persistent property paths

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-commons</artifactId>
-	<version>3.2.0-SNAPSHOT</version>
+	<version>3.2.x-2837-SNAPSHOT</version>
 
 	<name>Spring Data Core</name>
 	<description>Core Spring concepts underpinning every Spring Data module.</description>

src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java

@@ -41,14 +41,15 @@
  * A factory implementation to create {@link PersistentPropertyPath} instances in various ways.
  *
  * @author Oliver Gierke
+ * @author Christoph Strobl
  * @since 2.1
  * @soundtrack Cypress Hill - Boom Biddy Bye Bye (Fugees Remix, Unreleased & Revamped)
  */
 class PersistentPropertyPathFactory<E extends PersistentEntity<?, P>, P extends PersistentProperty<P>> {
 
 	private static final Predicate<PersistentProperty<? extends PersistentProperty<?>>> IS_ENTITY = PersistentProperty::isEntity;
 
-	private final Map<TypeAndPath, PersistentPropertyPath<P>> propertyPaths = new ConcurrentReferenceHashMap<>();
+	private final Map<TypeAndPath, PathResolution> propertyPaths = new ConcurrentReferenceHashMap<>();
 	private final MappingContext<E, P> context;
 
 	public PersistentPropertyPathFactory(MappingContext<E, P> context) {
@@ -166,7 +167,10 @@ public <T> PersistentPropertyPaths<T, P> from(TypeInformation<T> type, Predicate
 	}
 
 	private PersistentPropertyPath<P> getPersistentPropertyPath(TypeInformation<?> type, String propertyPath) {
+		return getPotentiallyCachedPath(type, propertyPath).getResolvedPath();
+	}
 
+	private PathResolution getPotentiallyCachedPath(TypeInformation<?> type, String propertyPath) {
 		return propertyPaths.computeIfAbsent(TypeAndPath.of(type, propertyPath),
 				it -> createPersistentPropertyPath(it.getPath(), it.getType()));
 	}
@@ -178,7 +182,7 @@ private PersistentPropertyPath<P> getPersistentPropertyPath(TypeInformation<?> t
 	 * @param type must not be {@literal null}.
 	 * @return
 	 */
-	private PersistentPropertyPath<P> createPersistentPropertyPath(String propertyPath, TypeInformation<?> type) {
+	private PathResolution createPersistentPropertyPath(String propertyPath, TypeInformation<?> type) {
 
 		String trimmedPath = propertyPath.trim();
 		List<String> parts = trimmedPath.isEmpty() ? Collections.emptyList() : List.of(trimmedPath.split("\\."));
@@ -196,17 +200,14 @@ private PersistentPropertyPath<P> createPersistentPropertyPath(String propertyPa
 			Pair<DefaultPersistentPropertyPath<P>, E> pair = getPair(path, iterator, segment, current);
 
 			if (pair == null) {
-
-				String source = StringUtils.collectionToDelimitedString(parts, ".");
-
-				throw new InvalidPersistentPropertyPath(source, type, segment, currentPath);
+				return new PathResolution(parts, segment, type, currentPath);
 			}
 
 			path = pair.getFirst();
 			current = pair.getSecond();
 		}
 
-		return path;
+		return new PathResolution(path);
 	}
 
 	@Nullable
@@ -429,4 +430,44 @@ public int compare(PersistentPropertyPath<?> left, PersistentPropertyPath<?> rig
 			}
 		}
 	}
+
+	/**
+	 * Wrapper around {@link PersistentPropertyPath} that allows them to be cached. Retains behaviour be throwing
+	 * {@link InvalidPersistentPropertyPath} on access of {@link #getResolvedPath()} if no corresponding property was
+	 * found.
+	 */
+	private static class PathResolution {
+
+		private final PersistentPropertyPath<?> path;
+		private final boolean resolvable;
+		private String source, segment;
+		private TypeInformation<?> type;
+
+		public PathResolution(PersistentPropertyPath<?> path) {
+
+			this.path = path;
+			this.resolvable = true;
+		}
+
+		PathResolution(List<String> parts, String segment, TypeInformation<?> type, PersistentPropertyPath<?> path) {
+
+			this.source = StringUtils.collectionToDelimitedString(parts, ".");
+			this.segment = segment;
+			this.type = type;
+			this.path = path;
+			this.resolvable = false;
+		}
+
+		/**
+		 * @return the path if available.
+		 * @throws InvalidPersistentPropertyPath when the path could not be resolved to an actual property
+		 */
+		PersistentPropertyPath getResolvedPath() {
+
+			if (resolvable) {
+				return path;
+			}
+			throw new InvalidPersistentPropertyPath(source, type, segment, path);
+		}
+	}
 }

src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java

@@ -20,20 +20,25 @@
 import jakarta.inject.Inject;
 
 import java.util.List;
+import java.util.Map;
 
 import org.junit.jupiter.api.Test;
 import org.springframework.data.annotation.Reference;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.mapping.PersistentPropertyPaths;
 import org.springframework.data.mapping.PropertyPath;
+import org.springframework.data.mapping.context.PersistentPropertyPathFactory.TypeAndPath;
 import org.springframework.data.mapping.model.BasicPersistentEntity;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.StringUtils;
 
 /**
  * Unit tests for {@link PersistentPropertyPathFactory}.
  *
  * @author Oliver Gierke
+ * @author Christoph Strobl
  * @soundtrack Cypress Hill - Illusions (Q-Tip Remix, Unreleased & Revamped)
  */
 class PersistentPropertyPathFactoryUnitTests {
@@ -84,6 +89,16 @@ void cachesPersistentPropertyPaths() {
 				.isSameAs(factory.from(PersonSample.class, "persons.name"));
 	}
 
+	@Test // GH-2837
+	void cachesFailingPropertyPathLookup() {
+
+		assertThatExceptionOfType(InvalidPersistentPropertyPath.class)//
+				.isThrownBy(() -> factory.from(PersonSample.class, "persons.firstname"));
+
+		Map<TypeAndPath, ?> propertyPaths = (Map<TypeAndPath, ?>) ReflectionTestUtils.getField(factory, "propertyPaths");
+		assertThat(propertyPaths).containsKey(TypeAndPath.of(TypeInformation.of(PersonSample.class), "persons.firstname"));
+	}
+
 	@Test // DATACMNS-1275
 	void findsNestedPropertyByFilter() {