From 6a9b78950bfdc8239d3eb1496355b2b167c82f9e Mon Sep 17 00:00:00 2001 From: junhyeong9812 Date: Sat, 13 Jun 2026 10:12:55 +0900 Subject: [PATCH] Fix property name resolution for data class accessors Property.resolveName() located the get/is accessor prefix with String.indexOf, which matches the prefix anywhere in the method name. A plain accessor whose name embeds such a prefix (for example budget()) had the wrong portion stripped and resolved to an empty or wrong property name, which in turn caused the backing field's annotations to be silently dropped. Match the get/is prefix only at the start of the method name and do not strip it when the method is a plain accessor for a data class, that is, a non-static no-arg method referring to an instance field of the same name. This supports Java records, Kotlin data classes, and custom Java data classes alike, without relying on java.lang.Record. As a consequence, a getter backed by a field of the exact same name (for example isUrgent()) now resolves to the field name. Signed-off-by: junhyeong9812 --- .../core/convert/Property.java | 42 +++- .../core/convert/PropertyTests.java | 208 ++++++++++++++++++ 2 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/core/convert/PropertyTests.java diff --git a/spring-core/src/main/java/org/springframework/core/convert/Property.java b/spring-core/src/main/java/org/springframework/core/convert/Property.java index 09bf46f46bb9..2967fe46ef74 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/Property.java +++ b/spring-core/src/main/java/org/springframework/core/convert/Property.java @@ -20,6 +20,7 @@ import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; @@ -133,21 +134,23 @@ Annotation[] getAnnotations() { private String resolveName() { if (this.readMethod != null) { - int index = this.readMethod.getName().indexOf("get"); - if (index != -1) { - index += 3; + String methodName = this.readMethod.getName(); + int index; + // For a get/is-prefixed name, strip the prefix unless the method is a + // plain accessor for a data class property whose name starts with that + // prefix, for example, a record component named "issue" + if (methodName.startsWith("get")) { + index = (isPlainAccessor(this.readMethod) ? 0 : 3); + } + else if (methodName.startsWith("is")) { + index = (isPlainAccessor(this.readMethod) ? 0 : 2); } else { - index = this.readMethod.getName().indexOf("is"); - if (index != -1) { - index += 2; - } - else { - // Record-style plain accessor method, for example, name() - index = 0; - } + // Plain accessor method for a data class, for example, a Java record + // component accessor such as name() + index = 0; } - return StringUtils.uncapitalize(this.readMethod.getName().substring(index)); + return StringUtils.uncapitalize(methodName.substring(index)); } else if (this.writeMethod != null) { int index = this.writeMethod.getName().indexOf("set"); @@ -162,6 +165,21 @@ else if (this.writeMethod != null) { } } + private static boolean isPlainAccessor(Method method) { + if (Modifier.isStatic(method.getModifiers()) || + method.getParameterCount() > 0 || method.getReturnType() == void.class) { + return false; + } + try { + // Accessor method referring to instance field of same name? + Field field = method.getDeclaringClass().getDeclaredField(method.getName()); + return !Modifier.isStatic(field.getModifiers()); + } + catch (Exception ex) { + return false; + } + } + private MethodParameter resolveMethodParameter() { MethodParameter read = resolveReadMethodParameter(); MethodParameter write = resolveWriteMethodParameter(); diff --git a/spring-core/src/test/java/org/springframework/core/convert/PropertyTests.java b/spring-core/src/test/java/org/springframework/core/convert/PropertyTests.java new file mode 100644 index 000000000000..bc89545630c4 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/convert/PropertyTests.java @@ -0,0 +1,208 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.convert; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link Property} name resolution. + * + * @author Junhyeong Kim + */ +class PropertyTests { + + @Test + void resolveNameForStandardGetter() throws Exception { + assertThat(readProperty(TestBean.class, "getName").getName()).isEqualTo("name"); + } + + @Test + void resolveNameForBooleanGetter() throws Exception { + assertThat(readProperty(TestBean.class, "isEnabled").getName()).isEqualTo("enabled"); + } + + @Test // regression guard for the indexOf -> startsWith fix: "get" embedded mid-name + void resolveNameForGetterEmbeddingGetInName() throws Exception { + // with the former indexOf-based resolution this resolved to "" (matched "get" in "isTarget") + assertThat(readProperty(TestBean.class, "isTarget").getName()).isEqualTo("target"); + } + + @Test + void resolveNameForSetter() throws Exception { + Method setter = TestBean.class.getMethod("setName", String.class); + assertThat(new Property(TestBean.class, null, setter).getName()).isEqualTo("name"); + } + + @Test // record component accessor whose name embeds the "get" prefix + void resolveNameForRecordAccessorEmbeddingGetPrefix() throws Exception { + assertThat(readProperty(SampleRecord.class, "budget").getName()).isEqualTo("budget"); + } + + @Test // record component accessor whose name starts with the "is" prefix + void resolveNameForRecordAccessorStartingWithIsPrefix() throws Exception { + assertThat(readProperty(SampleRecord.class, "issue").getName()).isEqualTo("issue"); + } + + @Test // plain record component accessor with no prefix collision (regression guard) + void resolveNameForPlainRecordAccessor() throws Exception { + assertThat(readProperty(SampleRecord.class, "name").getName()).isEqualTo("name"); + } + + @Test // a JavaBeans-style getter declared on a record must still be stripped + void resolveNameForGetterDeclaredOnRecord() throws Exception { + assertThat(readProperty(SampleRecord.class, "getWidget").getName()).isEqualTo("widget"); + } + + @Test // component literally named "get": proves plain accessor detection must precede startsWith + void resolveNameForRecordAccessorNamedGet() throws Exception { + assertThat(readProperty(EdgeRecord.class, "get").getName()).isEqualTo("get"); + } + + @Test // component literally named "is": proves plain accessor detection must precede startsWith + void resolveNameForRecordAccessorNamedIs() throws Exception { + assertThat(readProperty(EdgeRecord.class, "is").getName()).isEqualTo("is"); + } + + @Test // component literally named "getWidget": plain accessor detection must beat prefix stripping + void resolveNameForRecordAccessorNamedGetWidget() throws Exception { + assertThat(readProperty(EdgeRecord.class, "getWidget").getName()).isEqualTo("getWidget"); + } + + @Test // data class accessor whose name embeds the "get" prefix + void resolveNameForDataClassAccessorEmbeddingGetPrefix() throws Exception { + assertThat(readProperty(SampleDataClass.class, "budget").getName()).isEqualTo("budget"); + } + + @Test // data class accessor whose name starts with the "is" prefix + void resolveNameForDataClassAccessorStartingWithIsPrefix() throws Exception { + assertThat(readProperty(SampleDataClass.class, "issue").getName()).isEqualTo("issue"); + } + + @Test // plain data class accessor with no prefix collision (regression guard) + void resolveNameForPlainDataClassAccessor() throws Exception { + assertThat(readProperty(SampleDataClass.class, "name").getName()).isEqualTo("name"); + } + + @Test // a JavaBeans-style getter without a backing field must still be stripped + void resolveNameForGetterDeclaredOnDataClass() throws Exception { + assertThat(readProperty(SampleDataClass.class, "getWidget").getName()).isEqualTo("widget"); + } + + @Test // a boolean getter backed by a field of the exact same name resolves to the field name + void resolveNameForBooleanGetterBackedByFieldOfSameName() throws Exception { + assertThat(readProperty(SampleDataClass.class, "isUrgent").getName()).isEqualTo("isUrgent"); + } + + @Test // a static field of the same name must not make an instance getter a plain accessor + void resolveNameForGetterWithStaticFieldOfSameName() throws Exception { + assertThat(readProperty(StaticEdgeBean.class, "getCount").getName()).isEqualTo("count"); + } + + @Test // a static method must not be treated as a plain accessor + void resolveNameForStaticGetterWithInstanceFieldOfSameName() throws Exception { + assertThat(readProperty(StaticEdgeBean.class, "getLabel").getName()).isEqualTo("label"); + } + + + private static Property readProperty(Class objectType, String readMethodName) throws Exception { + Method readMethod = objectType.getMethod(readMethodName); + return new Property(objectType, readMethod, null); + } + + + @SuppressWarnings("unused") + static class TestBean { + + public String getName() { + return null; + } + + public boolean isEnabled() { + return false; + } + + public boolean isTarget() { + return false; + } + + public void setName(String name) { + } + } + + record SampleRecord(String name, String budget, String issue) { + + public String getWidget() { + return null; + } + } + + record EdgeRecord(String get, String is, String getWidget) { + } + + @SuppressWarnings("unused") + static class SampleDataClass { + + private final String name = null; + + private final String budget = null; + + private final String issue = null; + + private final boolean isUrgent = false; + + public String name() { + return this.name; + } + + public String budget() { + return this.budget; + } + + public String issue() { + return this.issue; + } + + public boolean isUrgent() { + return this.isUrgent; + } + + public String getWidget() { + return null; + } + } + + @SuppressWarnings("unused") + static class StaticEdgeBean { + + private static String getCount = null; + + private String getLabel = null; + + public String getCount() { + return null; + } + + public static String getLabel() { + return null; + } + } + +}