Skip to content

Commit e04f0b0

Browse files
mp911dechristophstrobl
authored andcommitted
Fallback to StringUtils.uncapitalize(…) when looking up property paths.
Naming restrictions for property paths used in query method names requite capitalization of the first letter regardless whether the property name uses a second-letter uppercase form (zIndex -> ZIndex, qCode -> QCode). In such cases, Introspector.decapitalize(…) shortcuts to non-decapitalization as it checks the second letter casing. This leads to the case that the property name cannot be resolved, assuming proper property naming (getzIndex(), zIndex()). Falling back to StringUtils.uncapitalize() allows catching such properties. Closes: #1851 Original Pull Request: #2940
1 parent aac6b9d commit e04f0b0

File tree

4 files changed

+58
-37
lines changed

4 files changed

+58
-37
lines changed

src/main/java/org/springframework/data/mapping/PropertyPath.java

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class PropertyPath implements Streamable<PropertyPath> {
5050
private static final Pattern SPLITTER = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", DELIMITERS));
5151
private static final Pattern SPLITTER_FOR_QUOTED = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", "\\."));
5252
private static final Pattern NESTED_PROPERTY_PATTERN = Pattern.compile("\\p{Lu}[\\p{Ll}\\p{Nd}]*$");
53-
private static final Map<Key, PropertyPath> cache = new ConcurrentReferenceHashMap<>();
53+
private static final Map<Property, PropertyPath> cache = new ConcurrentReferenceHashMap<>();
5454

5555
private final TypeInformation<?> owningType;
5656
private final String name;
@@ -83,19 +83,31 @@ public class PropertyPath implements Streamable<PropertyPath> {
8383
Assert.notNull(owningType, "Owning type must not be null");
8484
Assert.notNull(base, "Previously found properties must not be null");
8585

86-
String propertyName = Introspector.decapitalize(name);
87-
TypeInformation<?> propertyType = owningType.getProperty(propertyName);
86+
String decapitalized = Introspector.decapitalize(name);
87+
Property property = lookupProperty(owningType, decapitalized);
8888

89-
if (propertyType == null) {
90-
throw new PropertyReferenceException(propertyName, owningType, base);
89+
if (property == null) {
90+
property = lookupProperty(owningType, StringUtils.uncapitalize(name));
91+
}
92+
93+
if (property == null) {
94+
throw new PropertyReferenceException(decapitalized, owningType, base);
9195
}
9296

9397
this.owningType = owningType;
94-
this.typeInformation = propertyType;
95-
this.isCollection = propertyType.isCollectionLike();
96-
this.name = propertyName;
97-
this.actualTypeInformation = propertyType.getActualType() == null ? propertyType
98-
: propertyType.getRequiredActualType();
98+
this.name = property.path();
99+
this.typeInformation = property.type();
100+
this.isCollection = this.typeInformation.isCollectionLike();
101+
this.actualTypeInformation = this.typeInformation.getActualType() == null ? this.typeInformation
102+
: this.typeInformation.getRequiredActualType();
103+
}
104+
105+
@Nullable
106+
private static Property lookupProperty(TypeInformation<?> owningType, String name) {
107+
108+
TypeInformation<?> propertyType = owningType.getProperty(name);
109+
110+
return propertyType != null ? new Property(propertyType, name) : null;
99111
}
100112

101113
/**
@@ -351,7 +363,7 @@ public static PropertyPath from(String source, TypeInformation<?> type) {
351363
Assert.hasText(source, "Source must not be null or empty");
352364
Assert.notNull(type, "TypeInformation must not be null or empty");
353365

354-
return cache.computeIfAbsent(new Key(type, source), it -> {
366+
return cache.computeIfAbsent(new Property(type, source), it -> {
355367

356368
List<String> iteratorSource = new ArrayList<>();
357369

@@ -487,5 +499,6 @@ public String toString() {
487499
return String.format("%s.%s", owningType.getType().getSimpleName(), toDotPath());
488500
}
489501

490-
private record Key(TypeInformation<?> type, String path) {};
502+
private record Property(TypeInformation<?> type, String path) {
503+
};
491504
}

src/main/java/org/springframework/data/util/TypeDiscoverer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,8 @@ private Optional<TypeInformation<?>> getPropertyInformation(String fieldname) {
370370
var field = ReflectionUtils.findField(rawType, fieldname);
371371

372372
return field != null ? Optional.of(TypeInformation.of(ResolvableType.forField(field, resolvableType)))
373-
: Optional.ofNullable(BeanUtils.getPropertyDescriptor(rawType, fieldname)).map(it -> from(it, rawType))
374-
.map(TypeInformation::of);
373+
: Optional.ofNullable(BeanUtils.getPropertyDescriptor(rawType, fieldname))
374+
.filter(it -> it.getName().equals(fieldname)).map(it -> from(it, rawType)).map(TypeInformation::of);
375375
}
376376

377377
private ResolvableType from(PropertyDescriptor descriptor, Class<?> rawType) {

src/test/java/org/springframework/data/mapping/PropertyPathUnitTests.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ void parsesSimplePropertyCorrectly() {
4848
assertThat(reference.getOwningType()).isEqualTo(TypeInformation.of(Foo.class));
4949
}
5050

51+
@Test // GH-1851
52+
void parsesRecordPropertyCorrectly() {
53+
54+
var reference = PropertyPath.from("userName", MyRecord.class);
55+
56+
assertThat(reference.hasNext()).isFalse();
57+
assertThat(reference.toDotPath()).isEqualTo("userName");
58+
assertThat(reference.getOwningType()).isEqualTo(TypeInformation.of(MyRecord.class));
59+
}
60+
5161
@Test
5262
void parsesPathPropertyCorrectly() {
5363

@@ -292,6 +302,15 @@ void findsAllUppercaseProperty() {
292302
assertThat(path.getSegment()).isEqualTo("UUID");
293303
}
294304

305+
@Test // GH-1851
306+
void findsSecondLetterUpperCaseProperty() {
307+
308+
assertThat(PropertyPath.from("qCode", Foo.class).toDotPath()).isEqualTo("qCode");
309+
assertThat(PropertyPath.from("QCode", Foo.class).toDotPath()).isEqualTo("qCode");
310+
assertThat(PropertyPath.from("zIndex", MyRecord.class).toDotPath()).isEqualTo("zIndex");
311+
assertThat(PropertyPath.from("ZIndex", MyRecord.class).toDotPath()).isEqualTo("zIndex");
312+
}
313+
295314
@Test // DATACMNS-257
296315
void findsNestedAllUppercaseProperty() {
297316

@@ -427,7 +446,16 @@ private class Foo {
427446
String userName;
428447
String _email;
429448
String UUID;
449+
String qCode;
430450
String var_name_with_underscore;
451+
452+
public String getqCode() {
453+
return qCode;
454+
}
455+
456+
public void setqCode(String qCode) {
457+
this.qCode = qCode;
458+
}
431459
}
432460

433461
private class Bar {
@@ -487,4 +515,7 @@ private class Category {
487515
}
488516

489517
private class B {}
518+
519+
private record MyRecord(String userName, boolean zIndex) {
520+
}
490521
}

src/test/java/org/springframework/data/repository/query/parser/PartTreeUnitTests.java

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.springframework.data.domain.Limit;
3131
import org.springframework.data.domain.Sort;
3232
import org.springframework.data.mapping.PropertyPath;
33-
import org.springframework.data.mapping.PropertyReferenceException;
3433
import org.springframework.data.repository.query.parser.Part.IgnoreCaseType;
3534
import org.springframework.data.repository.query.parser.Part.Type;
3635
import org.springframework.data.repository.query.parser.PartTree.OrPart;
@@ -618,28 +617,6 @@ void emptyTreeDoesNotContainParts() {
618617
assertThat(tree.hasPredicate()).isFalse();
619618
}
620619

621-
/**
622-
* This test does not verify a desired behaviour but documents a limitation. If it starts failing and everything else
623-
* is green, remove the expectation to fail with an exception.
624-
*/
625-
@Test // DATACMNS-1570
626-
void specialCapitalizationInSubject() {
627-
628-
assertThatThrownBy(() -> new PartTree("findByZIndex", SpecialCapitalization.class))
629-
.isInstanceOf(PropertyReferenceException.class);
630-
}
631-
632-
/**
633-
* This test does not verify a desired behaviour but documents a limitation. If it starts failing and everything else
634-
* is green, remove the expectation to fail with an exception.
635-
*/
636-
@Test // DATACMNS-1570
637-
void specialCapitalizationInOrderBy() {
638-
639-
assertThatThrownBy(() -> new PartTree("findByOrderByZIndex", SpecialCapitalization.class))
640-
.isInstanceOf(PropertyReferenceException.class);
641-
}
642-
643620
@Test // DATACMNS-1570
644621
void allCapsInSubject() {
645622

0 commit comments

Comments
 (0)