diff --git a/pom.xml b/pom.xml index 38a742d830..7c3f49b2af 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.3.0-SNAPSHOT + 3.3.x-GH-1851-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. diff --git a/src/main/antora/modules/ROOT/pages/repositories/query-methods-details.adoc b/src/main/antora/modules/ROOT/pages/repositories/query-methods-details.adoc index e417be1bcd..ea2fddb1b7 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/query-methods-details.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/query-methods-details.adoc @@ -119,7 +119,34 @@ So our method name would be as follows: List findByAddress_ZipCode(ZipCode zipCode); ---- -Because we treat the underscore character as a reserved character, we strongly advise following standard Java naming conventions (that is, not using underscores in property names but using camel case instead). +[NOTE] +==== +Because we treat underscores (`_`) as a reserved character, we strongly advise to follow standard Java naming conventions (that is, not using underscores in property names but applying camel case instead). +==== + +[CAUTION] +==== +.Field Names starting with underscore: +Field names may start with underscores like `String \_name`. +Make sure to preserve the `_` as in `\_name` and use double `_` to split nested paths like `user__name`. + +.Upper Case Field Names: +Field names that are all uppercase can be used as such. +Nested paths if applicable require splitting via `_` as in `USER_name`. + +.Field Names with 2nd uppercase letter: +Field names that consist of a starting lower case letter followed by an uppercase one like `String qCode` can be resolved by starting with two upper case letters as in `QCode`. +Please be aware of potential path ambiguities. + +.Path Ambiguities: +In the following sample the arrangement of properties `qCode` and `q`, with `q` containing a property called `code`, creates an ambiguity for the path `QCode`. +```java +record Container(String qCode, Code q) {} +record Code(String code) {} +``` +Since a direct match on a property is considered first, any potential nested paths will not be considered and the algorithm picks the `qCode` field. +In order to select the `code` field in `q` the underscore notation `Q_Code` is required. +==== [[repositories.collections-and-iterables]] == Repository Methods Returning Collections or Iterables diff --git a/src/main/java/org/springframework/data/mapping/PropertyPath.java b/src/main/java/org/springframework/data/mapping/PropertyPath.java index debffac2cb..70a77bea49 100644 --- a/src/main/java/org/springframework/data/mapping/PropertyPath.java +++ b/src/main/java/org/springframework/data/mapping/PropertyPath.java @@ -50,7 +50,7 @@ public class PropertyPath implements Streamable { private static final Pattern SPLITTER = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", DELIMITERS)); private static final Pattern SPLITTER_FOR_QUOTED = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", "\\.")); private static final Pattern NESTED_PROPERTY_PATTERN = Pattern.compile("\\p{Lu}[\\p{Ll}\\p{Nd}]*$"); - private static final Map cache = new ConcurrentReferenceHashMap<>(); + private static final Map cache = new ConcurrentReferenceHashMap<>(); private final TypeInformation owningType; private final String name; @@ -83,19 +83,31 @@ public class PropertyPath implements Streamable { Assert.notNull(owningType, "Owning type must not be null"); Assert.notNull(base, "Previously found properties must not be null"); - String propertyName = Introspector.decapitalize(name); - TypeInformation propertyType = owningType.getProperty(propertyName); + String decapitalized = Introspector.decapitalize(name); + Property property = lookupProperty(owningType, decapitalized); - if (propertyType == null) { - throw new PropertyReferenceException(propertyName, owningType, base); + if (property == null) { + property = lookupProperty(owningType, StringUtils.uncapitalize(name)); + } + + if (property == null) { + throw new PropertyReferenceException(decapitalized, owningType, base); } this.owningType = owningType; - this.typeInformation = propertyType; - this.isCollection = propertyType.isCollectionLike(); - this.name = propertyName; - this.actualTypeInformation = propertyType.getActualType() == null ? propertyType - : propertyType.getRequiredActualType(); + this.name = property.path(); + this.typeInformation = property.type(); + this.isCollection = this.typeInformation.isCollectionLike(); + this.actualTypeInformation = this.typeInformation.getActualType() == null ? this.typeInformation + : this.typeInformation.getRequiredActualType(); + } + + @Nullable + private static Property lookupProperty(TypeInformation owningType, String name) { + + TypeInformation propertyType = owningType.getProperty(name); + + return propertyType != null ? new Property(propertyType, name) : null; } /** @@ -351,7 +363,7 @@ public static PropertyPath from(String source, TypeInformation type) { Assert.hasText(source, "Source must not be null or empty"); Assert.notNull(type, "TypeInformation must not be null or empty"); - return cache.computeIfAbsent(new Key(type, source), it -> { + return cache.computeIfAbsent(new Property(type, source), it -> { List iteratorSource = new ArrayList<>(); @@ -487,5 +499,6 @@ public String toString() { return String.format("%s.%s", owningType.getType().getSimpleName(), toDotPath()); } - private record Key(TypeInformation type, String path) {}; + private record Property(TypeInformation type, String path) { + }; } diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java index 9a5d5aa2c8..45235cfecd 100644 --- a/src/main/java/org/springframework/data/util/TypeDiscoverer.java +++ b/src/main/java/org/springframework/data/util/TypeDiscoverer.java @@ -74,7 +74,7 @@ protected TypeDiscoverer(ResolvableType type) { .map(ResolvableType::toClass).collect(Collectors.toList())); } - static TypeDiscoverer td(ResolvableType type) { + static TypeDiscoverer ofCached(ResolvableType type) { Assert.notNull(type, "Type must not be null"); @@ -370,8 +370,8 @@ private Optional> getPropertyInformation(String fieldname) { var field = ReflectionUtils.findField(rawType, fieldname); return field != null ? Optional.of(TypeInformation.of(ResolvableType.forField(field, resolvableType))) - : Optional.ofNullable(BeanUtils.getPropertyDescriptor(rawType, fieldname)).map(it -> from(it, rawType)) - .map(TypeInformation::of); + : Optional.ofNullable(BeanUtils.getPropertyDescriptor(rawType, fieldname)) + .filter(it -> it.getName().equals(fieldname)).map(it -> from(it, rawType)).map(TypeInformation::of); } private ResolvableType from(PropertyDescriptor descriptor, Class rawType) { diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java index 3d49c87e71..2b2ff88f97 100644 --- a/src/main/java/org/springframework/data/util/TypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeInformation.java @@ -60,7 +60,7 @@ public static TypeInformation of(ResolvableType type) { Assert.notNull(type, "Type must not be null"); return type.hasGenerics() || (type.isArray() && type.getComponentType().hasGenerics()) // - || (type.getType() instanceof TypeVariable) ? TypeDiscoverer.td(type) : ClassTypeInformation.from(type); + || (type.getType() instanceof TypeVariable) ? TypeDiscoverer.ofCached(type) : ClassTypeInformation.from(type); } /** diff --git a/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java b/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java index 39003fe89a..7df3c8729d 100755 --- a/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java @@ -48,6 +48,16 @@ void parsesSimplePropertyCorrectly() { assertThat(reference.getOwningType()).isEqualTo(TypeInformation.of(Foo.class)); } + @Test // GH-1851 + void parsesRecordPropertyCorrectly() { + + var reference = PropertyPath.from("userName", MyRecord.class); + + assertThat(reference.hasNext()).isFalse(); + assertThat(reference.toDotPath()).isEqualTo("userName"); + assertThat(reference.getOwningType()).isEqualTo(TypeInformation.of(MyRecord.class)); + } + @Test void parsesPathPropertyCorrectly() { @@ -114,7 +124,7 @@ void handlesArrayCorrectly() { } @Test - void handlesInvalidCollectionCompountTypeProperl() { + void handlesInvalidCollectionCompoundTypeProperly() { try { PropertyPath.from("usersMame", Bar.class); @@ -292,6 +302,26 @@ void findsAllUppercaseProperty() { assertThat(path.getSegment()).isEqualTo("UUID"); } + @Test // GH-1851 + void findsSecondLetterUpperCaseProperty() { + + assertThat(PropertyPath.from("qCode", Foo.class).toDotPath()).isEqualTo("qCode"); + assertThat(PropertyPath.from("QCode", Foo.class).toDotPath()).isEqualTo("qCode"); + assertThat(PropertyPath.from("zIndex", MyRecord.class).toDotPath()).isEqualTo("zIndex"); + assertThat(PropertyPath.from("ZIndex", MyRecord.class).toDotPath()).isEqualTo("zIndex"); + assertThat(PropertyPath.from("_foo.QCode", Sample2.class).toDotPath()).isEqualTo("_foo.qCode"); + assertThat(PropertyPath.from("_fooQCode", Sample2.class).toDotPath()).isEqualTo("_foo.qCode"); + } + + @Test // GH-1851 + void favoursPropertyHitOverNestedPath() { + + assertThat(PropertyPath.from("qCode", NameAmbiguities.class).toDotPath()).isEqualTo("qCode"); + assertThat(PropertyPath.from("QCode", NameAmbiguities.class).toDotPath()).isEqualTo("qCode"); + assertThat(PropertyPath.from("Q_Code", NameAmbiguities.class).toDotPath()).isEqualTo("q.code"); + assertThat(PropertyPath.from("q_code", NameAmbiguities.class).toDotPath()).isEqualTo("q.code"); + } + @Test // DATACMNS-257 void findsNestedAllUppercaseProperty() { @@ -427,7 +457,26 @@ private class Foo { String userName; String _email; String UUID; + String qCode; String var_name_with_underscore; + + public String getqCode() { + return qCode; + } + + public void setqCode(String qCode) { + this.qCode = qCode; + } + } + + private static class NameAmbiguities { + + String qCode; + Code q; + } + + private static class Code { + String code; } private class Bar { @@ -487,4 +536,7 @@ private class Category { } private class B {} + + private record MyRecord(String userName, boolean zIndex) { + } } diff --git a/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java b/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java index 4c046098d3..8286051f90 100755 --- a/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java @@ -30,7 +30,6 @@ import org.springframework.data.domain.Limit; import org.springframework.data.domain.Sort; import org.springframework.data.mapping.PropertyPath; -import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.repository.query.parser.Part.IgnoreCaseType; import org.springframework.data.repository.query.parser.Part.Type; import org.springframework.data.repository.query.parser.PartTree.OrPart; @@ -618,28 +617,6 @@ void emptyTreeDoesNotContainParts() { assertThat(tree.hasPredicate()).isFalse(); } - /** - * This test does not verify a desired behaviour but documents a limitation. If it starts failing and everything else - * is green, remove the expectation to fail with an exception. - */ - @Test // DATACMNS-1570 - void specialCapitalizationInSubject() { - - assertThatThrownBy(() -> new PartTree("findByZIndex", SpecialCapitalization.class)) - .isInstanceOf(PropertyReferenceException.class); - } - - /** - * This test does not verify a desired behaviour but documents a limitation. If it starts failing and everything else - * is green, remove the expectation to fail with an exception. - */ - @Test // DATACMNS-1570 - void specialCapitalizationInOrderBy() { - - assertThatThrownBy(() -> new PartTree("findByOrderByZIndex", SpecialCapitalization.class)) - .isInstanceOf(PropertyReferenceException.class); - } - @Test // DATACMNS-1570 void allCapsInSubject() {