From 35a2f45736c0a4785858e88949944686990559be Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 1 Mar 2023 22:10:02 +0100 Subject: [PATCH] Tighten nullability contract of PersistentPropertyPath. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We should change the definition of `PersistentPropertyPath` to — in its public API — not allow empty instances anymore. Those violate the concept and bleed into the concept's API by having to make all methods nullable (returning null in exactly that "empty" case). An empty property path doesn't make any actual sense as you cannot reasonably answer the methods declared on the interface except by returning null, which then causes client code having to verify the returned values all the time. This is now changed into only making `PersistentPropertyPath.getParentPath()` nullable and letting it return null for single segment paths. Adapted client code accordingly. `….getRequiredLeadProperty()` is now deprecated in favour of `….getLeafProperty()` not returning null anymore. Fixes #2813. --- .../MappingAuditableBeanWrapperFactory.java | 2 +- .../mapping/PersistentPropertyAccessor.java | 22 ++++--- .../data/mapping/PersistentPropertyPath.java | 58 +++++++++++-------- .../PersistentPropertyPathAccessor.java | 13 ++--- .../DefaultPersistentPropertyPath.java | 56 +++++++++--------- .../InvalidPersistentPropertyPath.java | 2 +- .../PersistentPropertyPathFactory.java | 24 ++++---- .../model/ConvertingPropertyAccessor.java | 2 +- .../SimplePersistentPropertyPathAccessor.java | 16 +++-- .../PersistentPropertyAccessorUnitTests.java | 9 --- ...efaultPersistentPropertyPathUnitTests.java | 27 +++------ ...ersistentPropertyPathFactoryUnitTests.java | 5 +- 12 files changed, 115 insertions(+), 121 deletions(-) diff --git a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java index 021a11ee3..424de0631 100644 --- a/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java +++ b/src/main/java/org/springframework/data/auditing/MappingAuditableBeanWrapperFactory.java @@ -238,7 +238,7 @@ public class MappingAuditableBeanWrapperFactory extends DefaultAuditableBeanWrap property.forEach(it -> { - Class type = it.getRequiredLeafProperty().getType(); + Class type = it.getLeafProperty().getType(); this.accessor.setProperty(it, getDateValueToSet(value, type, accessor.getBean()), OPTIONS); }); diff --git a/src/main/java/org/springframework/data/mapping/PersistentPropertyAccessor.java b/src/main/java/org/springframework/data/mapping/PersistentPropertyAccessor.java index 9f6d83121..7e4629883 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentPropertyAccessor.java +++ b/src/main/java/org/springframework/data/mapping/PersistentPropertyAccessor.java @@ -62,24 +62,28 @@ public interface PersistentPropertyAccessor { Assert.notNull(path, "PersistentPropertyPath must not be null"); Assert.isTrue(!path.isEmpty(), "PersistentPropertyPath must not be empty"); + PersistentProperty> leafProperty = path.getLeafProperty(); PersistentPropertyPath> parentPath = path.getParentPath(); - PersistentProperty> leafProperty = path.getRequiredLeafProperty(); - PersistentProperty> parentProperty = parentPath.isEmpty() ? null - : parentPath.getLeafProperty(); - if (parentProperty != null && (parentProperty.isCollectionLike() || parentProperty.isMap())) { - throw new MappingException( - String.format("Cannot traverse collection or map intermediate %s", parentPath.toDotPath())); + if (parentPath != null) { + + PersistentProperty> parentProperty = parentPath.getLeafProperty(); + + if (parentProperty.isCollectionLike() || parentProperty.isMap()) { + throw new MappingException( + "Cannot traverse collection or map intermediate %s".formatted(parentPath.toDotPath())); + } } - Object parent = parentPath.isEmpty() ? getBean() : getProperty(parentPath); + Object parent = parentPath == null ? getBean() : getProperty(parentPath); if (parent == null) { String nullIntermediateMessage = "Cannot lookup property %s on null intermediate; Original path was: %s on %s"; throw new MappingException( - String.format(nullIntermediateMessage, parentProperty, path.toDotPath(), getBean().getClass().getName())); + String.format(nullIntermediateMessage, path.getParentPath(), path.toDotPath(), + getBean().getClass().getName())); } PersistentPropertyAccessor accessor = parent == getBean() // @@ -88,7 +92,7 @@ public interface PersistentPropertyAccessor { accessor.setProperty(leafProperty, value); - if (parentPath.isEmpty()) { + if (parentPath == null) { return; } diff --git a/src/main/java/org/springframework/data/mapping/PersistentPropertyPath.java b/src/main/java/org/springframework/data/mapping/PersistentPropertyPath.java index 5e7646907..0c74ca69e 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentPropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/PersistentPropertyPath.java @@ -30,9 +30,8 @@ public interface PersistentPropertyPath

> extends /** * Returns the dot based path notation using {@link PersistentProperty#getName()}. * - * @return + * @return will never be {@literal null}. */ - @Nullable String toDotPath(); /** @@ -40,18 +39,16 @@ public interface PersistentPropertyPath

> extends * {@link PersistentProperty}s to path segments. * * @param converter must not be {@literal null}. - * @return + * @return will never be {@literal null}. */ - @Nullable String toDotPath(Converter converter); /** * Returns a {@link String} path with the given delimiter based on the {@link PersistentProperty#getName()}. * - * @param delimiter must not be {@literal null}. - * @return + * @param delimiter must not be {@literal null} or empty. + * @return will never be {@literal null}. */ - @Nullable String toPath(String delimiter); /** @@ -60,9 +57,8 @@ public interface PersistentPropertyPath

> extends * * @param delimiter must not be {@literal null}. * @param converter must not be {@literal null}. - * @return + * @return will never be {@literal null}. */ - @Nullable String toPath(String delimiter, Converter converter); /** @@ -70,20 +66,21 @@ public interface PersistentPropertyPath

> extends * {@link PersistentProperty} for {@code bar}. For a simple {@code foo} it returns {@link PersistentProperty} for * {@code foo}. * - * @return + * @return will never be {@literal null}. */ - @Nullable P getLeafProperty(); + /** + * Returns the last property in the {@link PersistentPropertyPath}. So for {@code foo.bar} it will return the + * {@link PersistentProperty} for {@code bar}. For a simple {@code foo} it returns {@link PersistentProperty} for + * {@code foo}. + * + * @return will never be {@literal null}. + * @deprecated use {@link #getLeafProperty()} instead. + */ + @Deprecated(since = "3.1", forRemoval = true) default P getRequiredLeafProperty() { - - P property = getLeafProperty(); - - if (property == null) { - throw new IllegalStateException("No leaf property found"); - } - - return property; + return getLeafProperty(); } /** @@ -91,17 +88,26 @@ public interface PersistentPropertyPath

> extends * {@link PersistentProperty} for {@code foo}. For a simple {@code foo} it returns {@link PersistentProperty} for * {@code foo}. * - * @return + * @return will never be {@literal null}. */ - @Nullable P getBaseProperty(); + /** + * Returns whether the current path is located at the root of the traversal. In other words, if the path only contains + * a single property. + * + * @return whether the current path is located at the root of the traversal + */ + default boolean isRootPath() { + return getLength() == 1; + } + /** * Returns whether the given {@link PersistentPropertyPath} is a base path of the current one. This means that the * current {@link PersistentPropertyPath} is basically an extension of the given one. * * @param path must not be {@literal null}. - * @return + * @return whether the given {@link PersistentPropertyPath} is a base path of the current one. */ boolean isBasePathOf(PersistentPropertyPath

path); @@ -111,7 +117,7 @@ public interface PersistentPropertyPath

> extends * the current one the current {@link PersistentPropertyPath} will be returned as is. * * @param base must not be {@literal null}. - * @return + * @return will never be {@literal null}. */ PersistentPropertyPath

getExtensionForBaseOf(PersistentPropertyPath

base); @@ -121,13 +127,15 @@ public interface PersistentPropertyPath

> extends * returning the property. * * @return + * @throws IllegalStateException if the current path only consists of one segment. */ - PersistentPropertyPath

getParentPath(); + @Nullable + PersistentPropertyPath

getParentPath() throws IllegalStateException; /** * Returns the length of the {@link PersistentPropertyPath}. * - * @return + * @return a value greater than 0. */ int getLength(); } diff --git a/src/main/java/org/springframework/data/mapping/PersistentPropertyPathAccessor.java b/src/main/java/org/springframework/data/mapping/PersistentPropertyPathAccessor.java index f2cd58574..289232404 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentPropertyPathAccessor.java +++ b/src/main/java/org/springframework/data/mapping/PersistentPropertyPathAccessor.java @@ -44,20 +44,19 @@ public interface PersistentPropertyPathAccessor extends PersistentPropertyAcc /** * Return the value pointed to by the given {@link PersistentPropertyPath}. If the given path is empty, the wrapped - * bean is returned. On each path segment value lookup, the resulting value is post-processed by handlers registered - * on the given {@link TraversalContext} context. This can be used to unwrap container types that are encountered - * during the traversal. + * bean is returned. On each path segment value lookup, the resulting value is post-processed depending on the given + * {@link GetOptions}. * * @param path must not be {@literal null}. - * @param context must not be {@literal null}. + * @param options must not be {@literal null}. * @return */ @Nullable - Object getProperty(PersistentPropertyPath> path, GetOptions context); + Object getProperty(PersistentPropertyPath> path, GetOptions options); /** - * Sets the given value for the {@link PersistentProperty} pointed to by the given {@link PersistentPropertyPath}. The - * lookup of intermediate values must not yield {@literal null}. + * Sets the given value for the {@link PersistentProperty} pointed to by the given {@link PersistentPropertyPath}. + * The lookup of intermediate values must not yield {@literal null}. * * @param path must not be {@literal null} or empty. * @param value can be {@literal null}. diff --git a/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java b/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java index 0b1f2927d..11ccbb485 100644 --- a/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPath.java @@ -38,7 +38,7 @@ import org.springframework.util.StringUtils; */ class DefaultPersistentPropertyPath

> implements PersistentPropertyPath

{ - private static final Converter, String> DEFAULT_CONVERTER = (source) -> source.getName(); + private static final Converter, String> DEFAULT_CONVERTER = PersistentProperty::getName; private static final String DEFAULT_DELIMITER = "."; private final List

properties; @@ -60,7 +60,7 @@ class DefaultPersistentPropertyPath

> implements * * @return */ - public static > DefaultPersistentPropertyPath empty() { + static > DefaultPersistentPropertyPath empty() { return new DefaultPersistentPropertyPath(Collections.emptyList()); } @@ -71,7 +71,7 @@ class DefaultPersistentPropertyPath

> implements * @return a new {@link DefaultPersistentPropertyPath} with the given property appended to the current one. * @throws IllegalArgumentException in case the property is not a property of the type of the current leaf property. */ - public DefaultPersistentPropertyPath

append(P property) { + DefaultPersistentPropertyPath

append(P property) { Assert.notNull(property, "Property must not be null"); @@ -79,7 +79,6 @@ class DefaultPersistentPropertyPath

> implements return new DefaultPersistentPropertyPath<>(Collections.singletonList(property)); } - @SuppressWarnings("null") Class leafPropertyType = getLeafProperty().getActualType(); Assert.isTrue(property.getOwner().getType().equals(leafPropertyType), @@ -91,45 +90,50 @@ class DefaultPersistentPropertyPath

> implements return new DefaultPersistentPropertyPath<>(properties); } - @Nullable + @Override public String toDotPath() { return toPath(DEFAULT_DELIMITER, DEFAULT_CONVERTER); } - @Nullable + @Override public String toDotPath(Converter converter) { return toPath(DEFAULT_DELIMITER, converter); } - @Nullable + @Override public String toPath(String delimiter) { return toPath(delimiter, DEFAULT_CONVERTER); } - @Nullable + @Override public String toPath(String delimiter, Converter converter) { Assert.hasText(delimiter, "Delimiter must not be null or empty"); Assert.notNull(converter, "Converter must not be null"); - String result = properties.stream() // + return properties.stream() // .map(converter::convert) // .filter(StringUtils::hasText) // .collect(Collectors.joining(delimiter)); - - return result.isEmpty() ? null : result; } - @Nullable + @Override public P getLeafProperty() { - return properties.isEmpty() ? null : properties.get(properties.size() - 1); + + Assert.state(properties.size() > 0, "Empty PersistentPropertyPath should not exist"); + + return properties.get(properties.size() - 1); } - @Nullable + @Override public P getBaseProperty() { - return properties.isEmpty() ? null : properties.get(0); + + Assert.state(properties.size() > 0, "Empty PersistentPropertyPath should not exist"); + + return properties.get(0); } + @Override public boolean isBasePathOf(PersistentPropertyPath

path) { Assert.notNull(path, "PersistentPropertyPath must not be null"); @@ -152,37 +156,31 @@ class DefaultPersistentPropertyPath

> implements return true; } + @Override public PersistentPropertyPath

getExtensionForBaseOf(PersistentPropertyPath

base) { if (!base.isBasePathOf(this)) { return this; } - List

result = new ArrayList<>(); - Iterator

iterator = iterator(); - - for (int i = 0; i < base.getLength(); i++) { - iterator.next(); - } - - while (iterator.hasNext()) { - result.add(iterator.next()); - } - - return new DefaultPersistentPropertyPath<>(result); + return new DefaultPersistentPropertyPath<>(properties.subList(base.getLength(), getLength())); } + @Nullable + @Override public PersistentPropertyPath

getParentPath() { int size = properties.size(); - return size == 0 ? this : new DefaultPersistentPropertyPath<>(properties.subList(0, size - 1)); + return size == 1 ? null : new DefaultPersistentPropertyPath<>(properties.subList(0, size - 1)); } + @Override public int getLength() { return properties.size(); } + @Override public Iterator

iterator() { return properties.iterator(); } @@ -202,7 +200,7 @@ class DefaultPersistentPropertyPath

> implements } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; diff --git a/src/main/java/org/springframework/data/mapping/context/InvalidPersistentPropertyPath.java b/src/main/java/org/springframework/data/mapping/context/InvalidPersistentPropertyPath.java index 7f488d308..598466016 100644 --- a/src/main/java/org/springframework/data/mapping/context/InvalidPersistentPropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/context/InvalidPersistentPropertyPath.java @@ -54,7 +54,7 @@ public class InvalidPersistentPropertyPath extends MappingException { public InvalidPersistentPropertyPath(String source, TypeInformation type, String unresolvableSegment, PersistentPropertyPath> resolvedPath) { - super(createMessage(resolvedPath.isEmpty() ? type : resolvedPath.getRequiredLeafProperty().getTypeInformation(), + super(createMessage(resolvedPath.isEmpty() ? type : resolvedPath.getLeafProperty().getTypeInformation(), unresolvableSegment)); Assert.notNull(source, "Source property path must not be null"); 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 ebc563441..e109e1186 100644 --- a/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java +++ b/src/main/java/org/springframework/data/mapping/context/PersistentPropertyPathFactory.java @@ -59,13 +59,13 @@ class PersistentPropertyPathFactory, P extends * Creates a new {@link PersistentPropertyPath} for the given property path on the given type. * * @param type must not be {@literal null}. - * @param propertyPath must not be {@literal null}. + * @param propertyPath must not be {@literal null} or empty. * @return */ public PersistentPropertyPath

from(Class type, String propertyPath) { Assert.notNull(type, "Type must not be null"); - Assert.notNull(propertyPath, "Property path must not be null"); + Assert.hasText(propertyPath, "Property path must not be null or empty"); return getPersistentPropertyPath(TypeInformation.of(type), propertyPath); } @@ -74,13 +74,13 @@ class PersistentPropertyPathFactory, P extends * Creates a new {@link PersistentPropertyPath} for the given property path on the given type. * * @param type must not be {@literal null}. - * @param propertyPath must not be {@literal null}. + * @param propertyPath must not be {@literal null} or empty. * @return */ public PersistentPropertyPath

from(TypeInformation type, String propertyPath) { Assert.notNull(type, "Type must not be null"); - Assert.notNull(propertyPath, "Property path must not be null"); + Assert.hasText(propertyPath, "Property path must not be null or empty"); return getPersistentPropertyPath(type, propertyPath); } @@ -181,10 +181,9 @@ class PersistentPropertyPathFactory, P extends private PersistentPropertyPath

createPersistentPropertyPath(String propertyPath, TypeInformation type) { String trimmedPath = propertyPath.trim(); + List parts = trimmedPath.isEmpty() ? Collections.emptyList() : List.of(trimmedPath.split("\\.")); - List parts = trimmedPath.isEmpty() // - ? Collections.emptyList() // - : Arrays.asList(trimmedPath.split("\\.")); + Assert.notEmpty(parts, "Cannot create PersistentPropertyPath from empty segments"); DefaultPersistentPropertyPath

path = DefaultPersistentPropertyPath.empty(); Iterator iterator = parts.iterator(); @@ -193,8 +192,7 @@ class PersistentPropertyPathFactory, P extends while (iterator.hasNext()) { String segment = iterator.next(); - final DefaultPersistentPropertyPath

currentPath = path; - + DefaultPersistentPropertyPath

currentPath = path; Pair, E> pair = getPair(path, iterator, segment, current); if (pair == null) { @@ -233,8 +231,7 @@ class PersistentPropertyPathFactory, P extends return Collections.emptyList(); } - E entity = context.getRequiredPersistentEntity(actualType); - return from(entity, filter, traversalGuard, basePath); + return from(context.getRequiredPersistentEntity(actualType), filter, traversalGuard, basePath); } private Collection> from(E entity, Predicate filter, Predicate

traversalGuard, @@ -258,7 +255,8 @@ class PersistentPropertyPathFactory, P extends } if (traversalGuard.and(IS_ENTITY).test(persistentProperty)) { - properties.addAll(from(context.getPersistentEntity(persistentProperty), filter, traversalGuard, currentPath)); + var persistentEntity = context.getRequiredPersistentEntity(persistentProperty); + properties.addAll(from(persistentEntity, filter, traversalGuard, currentPath)); } }; @@ -294,7 +292,7 @@ class PersistentPropertyPathFactory, P extends } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; diff --git a/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java b/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java index 1bfd371b6..90c603782 100644 --- a/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java +++ b/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java @@ -63,7 +63,7 @@ public class ConvertingPropertyAccessor extends SimplePersistentPropertyPathA @Override public void setProperty(PersistentPropertyPath> path, @Nullable Object value) { - Object converted = convertIfNecessary(value, path.getRequiredLeafProperty().getType()); + Object converted = convertIfNecessary(value, path.getLeafProperty().getType()); super.setProperty(path, converted); } diff --git a/src/main/java/org/springframework/data/mapping/model/SimplePersistentPropertyPathAccessor.java b/src/main/java/org/springframework/data/mapping/model/SimplePersistentPropertyPathAccessor.java index 02272f956..9ba1f4c69 100644 --- a/src/main/java/org/springframework/data/mapping/model/SimplePersistentPropertyPathAccessor.java +++ b/src/main/java/org/springframework/data/mapping/model/SimplePersistentPropertyPathAccessor.java @@ -24,7 +24,6 @@ import java.util.stream.Collectors; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.core.CollectionFactory; import org.springframework.data.mapping.AccessOptions; import org.springframework.data.mapping.AccessOptions.GetOptions; @@ -114,8 +113,8 @@ class SimplePersistentPropertyPathAccessor implements PersistentPropertyPathA setProperty(path, value, AccessOptions.defaultSetOptions()); } - @SuppressWarnings("unchecked") @Override + @SuppressWarnings("unchecked") public void setProperty(PersistentPropertyPath> path, @Nullable Object value, SetOptions options) { @@ -123,7 +122,13 @@ class SimplePersistentPropertyPathAccessor implements PersistentPropertyPathA Assert.isTrue(!path.isEmpty(), "PersistentPropertyPath must not be empty"); PersistentPropertyPath> parentPath = path.getParentPath(); - PersistentProperty> leafProperty = path.getRequiredLeafProperty(); + + if (parentPath == null) { + setProperty(path.getLeafProperty(), value); + return; + } + + PersistentProperty> leafProperty = path.getLeafProperty(); if (!options.propagate(parentPath.getLeafProperty())) { return; @@ -133,7 +138,7 @@ class SimplePersistentPropertyPathAccessor implements PersistentPropertyPathA ? DEFAULT_GET_OPTIONS.withNullValues(GetNulls.EARLY_RETURN) : DEFAULT_GET_OPTIONS; - Object parent = parentPath.isEmpty() ? getBean() : getProperty(parentPath, lookupOptions); + Object parent = getProperty(parentPath, lookupOptions); if (parent == null) { handleNull(path, options.getNullHandling()); @@ -146,7 +151,7 @@ class SimplePersistentPropertyPathAccessor implements PersistentPropertyPathA return; } - PersistentProperty> parentProperty = parentPath.getRequiredLeafProperty(); + PersistentProperty> parentProperty = parentPath.getLeafProperty(); Object newValue; @@ -193,6 +198,7 @@ class SimplePersistentPropertyPathAccessor implements PersistentPropertyPathA * @return */ @Nullable + @SuppressWarnings("null") private Object handleNull(PersistentPropertyPath> path, SetNulls handling) { if (SKIP.equals(handling)) { diff --git a/src/test/java/org/springframework/data/mapping/PersistentPropertyAccessorUnitTests.java b/src/test/java/org/springframework/data/mapping/PersistentPropertyAccessorUnitTests.java index 923f7490b..0721b3b8c 100644 --- a/src/test/java/org/springframework/data/mapping/PersistentPropertyAccessorUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/PersistentPropertyAccessorUnitTests.java @@ -81,15 +81,6 @@ public class PersistentPropertyAccessorUnitTests { assertThat(customer.firstname).isEqualTo("Oliver August"); } - @Test // DATACMNS-1275 - public void rejectsEmptyPathToSetValues() { - - setUp(new Order(null), ""); - - assertThatIllegalArgumentException() // - .isThrownBy(() -> accessor.setProperty(path, "Oliver August")); - } - @Test // DATACMNS-1275 public void rejectsIntermediateNullValuesForRead() { diff --git a/src/test/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPathUnitTests.java b/src/test/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPathUnitTests.java index 4da578807..3be97cca8 100755 --- a/src/test/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPathUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/DefaultPersistentPropertyPathUnitTests.java @@ -27,7 +27,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - import org.springframework.core.convert.converter.Converter; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyPath; @@ -110,18 +109,10 @@ class DefaultPersistentPropertyPathUnitTests

> { } @Test - void returnsEmptyPathForRootLevelProperty() { - assertThat(oneLeg.getParentPath()).isEmpty(); - } + void returnsNullForRootLevelProperty() { - @Test - void returnItselfForEmptyPath() { - - var parent = oneLeg.getParentPath(); - var parentsParent = parent.getParentPath(); - - assertThat(parentsParent).isEmpty(); - assertThat(parentsParent).isSameAs(parent); + assertThat(oneLeg.isRootPath()).isTrue(); + assertThat(oneLeg.getParentPath()).isNull(); } @Test @@ -132,27 +123,27 @@ class DefaultPersistentPropertyPathUnitTests

> { @Test // DATACMNS-444 void skipsMappedPropertyNameIfConverterReturnsNull() { - assertThat(twoLegs.toDotPath(source -> null)).isNull(); + assertThat(twoLegs.toDotPath(source -> null)).isEmpty(); } @Test // DATACMNS-444 void skipsMappedPropertyNameIfConverterReturnsEmptyStrings() { - assertThat(twoLegs.toDotPath(source -> "")).isNull(); + assertThat(twoLegs.toDotPath(source -> "")).isEmpty(); } @Test // DATACMNS-1466 - void returnsNullForLeafPropertyOnEmptyPath() { + void throwsExceptionForLeafPropertyOnEmptyPath() { PersistentPropertyPath

path = new DefaultPersistentPropertyPath

(Collections.emptyList()); - assertThat(path.getLeafProperty()).isNull(); + assertThatIllegalStateException().isThrownBy(() -> path.getLeafProperty()); } @Test // DATACMNS-1466 - void returnsNullForBasePropertyOnEmptyPath() { + void throwsExceptionForBasePropertyOnEmptyPath() { PersistentPropertyPath

path = new DefaultPersistentPropertyPath

(Collections.emptyList()); - assertThat(path.getBaseProperty()).isNull(); + assertThatIllegalStateException().isThrownBy(() -> path.getBaseProperty()); } } 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 41b5f8dc6..a682f38a4 100644 --- a/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/context/PersistentPropertyPathFactoryUnitTests.java @@ -22,7 +22,6 @@ import jakarta.inject.Inject; import java.util.List; import org.junit.jupiter.api.Test; - import org.springframework.data.annotation.Reference; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentPropertyPath; @@ -106,8 +105,8 @@ class PersistentPropertyPathFactoryUnitTests { } @Test // DATACMNS-1275 - void createsEmptyPropertyPathCorrectly() { - assertThat(factory.from(Wrapper.class, "")).isEmpty(); + void rejectsEmptyPropertyPathCreation() { + assertThatIllegalArgumentException().isThrownBy(() -> factory.from(Wrapper.class, "")); } @Test // DATACMNS-1275