diff --git a/pom.xml b/pom.xml index 5df738c14f..fab7a1aa2e 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.2.0-SNAPSHOT + 3.2.x-2837-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. diff --git a/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java b/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java index e109e1186c..8431aaa212 100644 --- a/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java +++ b/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java @@ -41,6 +41,7 @@ * 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) */ @@ -48,7 +49,7 @@ class PersistentPropertyPathFactory, P extends private static final Predicate>> IS_ENTITY = PersistentProperty::isEntity; - private final Map> propertyPaths = new ConcurrentReferenceHashMap<>(); + private final Map propertyPaths = new ConcurrentReferenceHashMap<>(); private final MappingContext context; public PersistentPropertyPathFactory(MappingContext context) { @@ -166,7 +167,10 @@ public PersistentPropertyPaths from(TypeInformation type, Predicate } private PersistentPropertyPath

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

getPersistentPropertyPath(TypeInformation t * @param type must not be {@literal null}. * @return */ - private PersistentPropertyPath

createPersistentPropertyPath(String propertyPath, TypeInformation type) { + private PathResolution createPersistentPropertyPath(String propertyPath, TypeInformation type) { String trimmedPath = propertyPath.trim(); List parts = trimmedPath.isEmpty() ? Collections.emptyList() : List.of(trimmedPath.split("\\.")); @@ -196,17 +200,14 @@ private PersistentPropertyPath

createPersistentPropertyPath(String propertyPa Pair, 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 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); + } + } } diff --git a/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java b/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java index a682f38a44..60b8ad3ea9 100644 --- a/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java @@ -20,6 +20,7 @@ import jakarta.inject.Inject; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.springframework.data.annotation.Reference; @@ -27,13 +28,17 @@ 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 propertyPaths = (Map) ReflectionTestUtils.getField(factory, "propertyPaths"); + assertThat(propertyPaths).containsKey(TypeAndPath.of(TypeInformation.of(PersonSample.class), "persons.firstname")); + } + @Test // DATACMNS-1275 void findsNestedPropertyByFilter() {