Consider only instance `with` methods for property access.

We now filter static `with…` methods from wither method introspection to avoid considering a static method a wither method.

Previously, we did not check modifiers and so static methods could be considered withers.

Closes #3472

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

@@ -85,6 +85,13 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 	@Nullable
 	Method getGetter();
 
+	/**
+	 * Returns the required getter method to access the property value or throw {@link IllegalStateException} if no getter
+	 * method is available.
+	 *
+	 * @return the getter method to access the property value.
+	 * @throws IllegalStateException if no getter method available.
+	 */
 	default Method getRequiredGetter() {
 
 		Method getter = getGetter();
@@ -105,6 +112,14 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 	@Nullable
 	Method getSetter();
 
+	/**
+	 * Returns the required setter method to set a property value or throw {@link IllegalStateException} if no setter
+	 * method is available.
+	 *
+	 * @return the setter method to set a property value.
+	 * @throws IllegalStateException if no setter method available.
+	 * @see #getSetter()
+	 */
 	default Method getRequiredSetter() {
 
 		Method setter = getSetter();
@@ -117,11 +132,11 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 	}
 
 	/**
-	 * Returns the with {@link Method} to set a property value on a new object instance. Might return {@literal null} in
-	 * case there is no with available.
+	 * Returns the {@code with} {@link Method} to set a property value on a new object instance. Might return
+	 * {@literal null} in case there is no {@code with} available.
 	 * <p>
-	 * With {@link Method methods} are property-bound instance {@link Method methods} that accept a single argument of the
-	 * property type creating a new object instance.
+	 * {@code with} {@link Method methods} are property-bound instance {@link Method methods} that accept a single
+	 * argument of the property type creating a new object instance.
 	 *
 	 * <pre class="code">
 	 * class Person {
@@ -143,6 +158,14 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 	@Nullable
 	Method getWither();
 
+	/**
+	 * Returns the required {@code with} {@link Method} to set a property value on a new object instance or throw
+	 * {@link IllegalStateException} if no wither method is available.
+	 *
+	 * @return the with {@link Method} to set a property value on and return a new object instance.
+	 * @throws IllegalStateException if no with {@code with} {@link Method} available.
+	 * @see #getWither()
+	 */
 	default Method getRequiredWither() {
 
 		Method wither = getWither();
@@ -154,9 +177,20 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 		return wither;
 	}
 
+	/**
+	 * Returns the backing {@link Field} if available. Might return {@literal null} in case there is no backing field.
+	 *
+	 * @return the backing {@link Field} if available, otherwise {@literal null}.
+	 */
 	@Nullable
 	Field getField();
 
+	/**
+	 * Returns the required backing {@link Field} or throw {@link IllegalStateException} if no backing field is available.
+	 *
+	 * @return the required backing {@link Field}.
+	 * @throws IllegalStateException if no backing field available.
+	 */
 	default Field getRequiredField() {
 
 		Field field = getField();
@@ -181,7 +215,7 @@ public interface PersistentProperty<P extends PersistentProperty<P>> {
 	Association<P> getAssociation();
 
 	/**
-	 * Get the {@link Association} of this property.
+	 * Get the requires {@link Association} of this property.
 	 *
 	 * @return never {@literal null}.
 	 * @throws IllegalStateException if not involved in an {@link Association}.

src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java

@@ -1029,11 +1029,7 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert
 				mv.visitLabel(propertyStackMap.get(property.getName()).label);
 				mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
 
-				if (supportsMutation(property)) {
-					visitSetProperty0(entity, property, mv, internalClassName);
-				} else {
-					mv.visitJumpInsn(GOTO, dfltLabel);
-				}
+				visitSetProperty0(entity, property, mv, internalClassName, () -> mv.visitJumpInsn(GOTO, dfltLabel));
 			}
 
 			mv.visitLabel(dfltLabel);
@@ -1046,7 +1042,7 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert
 		 * called as if the method had the expected signature and not array/varargs.
 		 */
 		private static void visitSetProperty0(PersistentEntity<?, ?> entity, PersistentProperty<?> property,
-				MethodVisitor mv, String internalClassName) {
+				MethodVisitor mv, String internalClassName, Runnable onImmutable) {
 
 			Method setter = property.getSetter();
 			Method wither = property.getWither();
@@ -1055,12 +1051,12 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert
 
 				if (wither != null) {
 					visitWithProperty(entity, property, mv, internalClassName, wither);
-				}
-
-				if (KotlinDetector.isKotlinType(entity.getType()) && KotlinCopyMethod.hasKotlinCopyMethodFor(property)) {
+				} else if (KotlinDetector.isKotlinType(entity.getType()) && KotlinCopyMethod.hasKotlinCopyMethodFor(property)) {
 					visitKotlinCopy(entity, property, mv, internalClassName);
+				} else {
+					onImmutable.run();
+					return;
 				}
-
 			} else if (property.usePropertyAccess() && setter != null) {
 				visitSetProperty(entity, property, mv, internalClassName, setter);
 			} else {
@@ -1232,28 +1228,27 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert
 		private static void visitSetField(PersistentEntity<?, ?> entity, PersistentProperty<?> property, MethodVisitor mv,
 				String internalClassName) {
 
-			Field field = property.getField();
-			if (field != null) {
-				if (generateSetterMethodHandle(entity, field)) {
-					// $fieldSetter.invoke(bean, object)
-					mv.visitFieldInsn(GETSTATIC, internalClassName, fieldSetterName(property),
-							referenceName(JAVA_LANG_INVOKE_METHOD_HANDLE));
-					mv.visitVarInsn(ALOAD, 3);
-					mv.visitVarInsn(ALOAD, 2);
-					mv.visitMethodInsn(INVOKEVIRTUAL, JAVA_LANG_INVOKE_METHOD_HANDLE, "invoke",
-							String.format("(%s%s)V", referenceName(JAVA_LANG_OBJECT), referenceName(JAVA_LANG_OBJECT)), false);
-				} else {
-					// bean.field
-					mv.visitVarInsn(ALOAD, 3);
-					mv.visitVarInsn(ALOAD, 2);
+			Field field = property.getRequiredField();
 
-					Class<?> fieldType = field.getType();
+			if (generateSetterMethodHandle(entity, field)) {
+				// $fieldSetter.invoke(bean, object)
+				mv.visitFieldInsn(GETSTATIC, internalClassName, fieldSetterName(property),
+						referenceName(JAVA_LANG_INVOKE_METHOD_HANDLE));
+				mv.visitVarInsn(ALOAD, 3);
+				mv.visitVarInsn(ALOAD, 2);
+				mv.visitMethodInsn(INVOKEVIRTUAL, JAVA_LANG_INVOKE_METHOD_HANDLE, "invoke",
+						String.format("(%s%s)V", referenceName(JAVA_LANG_OBJECT), referenceName(JAVA_LANG_OBJECT)), false);
+			} else {
+				// bean.field
+				mv.visitVarInsn(ALOAD, 3);
+				mv.visitVarInsn(ALOAD, 2);
 
-					mv.visitTypeInsn(CHECKCAST, Type.getInternalName(autoboxType(fieldType)));
-					autoboxIfNeeded(autoboxType(fieldType), fieldType, mv);
-					mv.visitFieldInsn(PUTFIELD, Type.getInternalName(field.getDeclaringClass()), field.getName(),
-							signatureTypeName(fieldType));
-				}
+				Class<?> fieldType = field.getType();
+
+				mv.visitTypeInsn(CHECKCAST, Type.getInternalName(autoboxType(fieldType)));
+				autoboxIfNeeded(autoboxType(fieldType), fieldType, mv);
+				mv.visitFieldInsn(PUTFIELD, Type.getInternalName(field.getDeclaringClass()), field.getName(),
+						signatureTypeName(fieldType));
 			}
 		}
 

src/main/java/org/springframework/data/mapping/model/Property.java

@@ -19,6 +19,7 @@ import java.beans.FeatureDescriptor;
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
@@ -272,7 +273,7 @@ public class Property {
 			if (owner.isAssignableFrom(owner.getReturnType(it))) {
 				resultHolder.set(it);
 			}
-		}, it -> isMethodWithSingleParameterOfType(it, methodName, rawType));
+		}, it -> isMethodWithSingleParameterOfType(it, methodName, rawType) && !Modifier.isStatic(it.getModifiers()));
 
 		Method method = resultHolder.get();
 		return method != null ? Optional.of(method) : Optional.empty();

src/test/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactoryEntityTypeTests.java

@@ -21,6 +21,7 @@ import java.io.Serializable;
 import java.time.LocalDateTime;
 
 import org.junit.jupiter.api.Test;
+
 import org.springframework.data.annotation.Id;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.context.SampleMappingContext;
@@ -55,7 +56,7 @@ public class ClassGeneratingPropertyAccessorFactoryEntityTypeTests {
 		assertThat(getEntityInformation(Person.class).getId(jonDoe)).isEqualTo(jonDoe.name);
 	}
 
-	@Test // #2324
+	@Test // GH-2324
 	void shouldGeneratePropertyAccessorForKotlinClassWithMultipleCopyMethods() {
 
 		var factory = new ClassGeneratingPropertyAccessorFactory();
@@ -66,6 +67,16 @@ public class ClassGeneratingPropertyAccessorFactoryEntityTypeTests {
 		assertThat(propertyAccessor).isNotNull();
 	}
 
+	@Test // GH-3472
+	void shouldIgnoreStaticWithMethod() {
+
+		var factory = new ClassGeneratingPropertyAccessorFactory();
+		var propertyAccessor = factory.getPropertyAccessor(mappingContext.getRequiredPersistentEntity(WithStaticWith.class),
+				new WithStaticWith(null));
+
+		assertThat(propertyAccessor).isNotNull();
+	}
+
 	private EntityInformation<Object, Serializable> getEntityInformation(Class<?> type) {
 
 		PersistentEntity<Object, SamplePersistentProperty> entity = mappingContext.getRequiredPersistentEntity(type);
@@ -94,4 +105,11 @@ public class ClassGeneratingPropertyAccessorFactoryEntityTypeTests {
 			this.name = name;
 		}
 	}
+
+	public record WithStaticWith(String string) {
+
+		public static WithStaticWith withString(String string) {
+			return new WithStaticWith(string);
+		}
+	}
 }

src/test/java/org/springframework/data/mapping/model/PropertyUnitTests.java

@@ -18,6 +18,7 @@ package org.springframework.data.mapping.model;
 import static org.assertj.core.api.Assertions.*;
 
 import org.junit.jupiter.api.Test;
+
 import org.springframework.data.util.TypeInformation;
 import org.springframework.util.ReflectionUtils;
 
@@ -28,21 +29,18 @@ import org.springframework.util.ReflectionUtils;
  */
 class PropertyUnitTests {
 
-	@Test // DATACMNS-1322
+	@Test // DATACMNS-1322, GH-3472
 	void shouldNotFindWitherMethod() {
 
-		assertThat(Property
-				.of(TypeInformation.of(ImmutableType.class), ReflectionUtils.findField(ImmutableType.class, "id")).getWither())
-						.isEmpty();
-		assertThat(
-				Property.of(TypeInformation.of(ImmutableType.class), ReflectionUtils.findField(ImmutableType.class, "name"))
-						.getWither()).isEmpty();
+		assertThat(getProperty(ImmutableType.class, "id").getWither()).isEmpty();
+		assertThat(getProperty(ImmutableType.class, "name").getWither()).isEmpty();
+		assertThat(getProperty(StaticWither.class, "someField").getWither()).isEmpty();
 	}
 
 	@Test // DATACMNS-1322
 	void shouldDiscoverWitherMethod() {
 
-		var property = Property.of(TypeInformation.of(WitherType.class), ReflectionUtils.findField(WitherType.class, "id"));
+		var property = getProperty(WitherType.class, "id");
 
 		assertThat(property.getWither()).isPresent().hasValueSatisfying(actual -> {
 			assertThat(actual.getName()).isEqualTo("withId");
@@ -53,8 +51,7 @@ class PropertyUnitTests {
 	@Test // DATACMNS-1421
 	void shouldDiscoverDerivedWitherMethod() {
 
-		var property = Property.of(TypeInformation.of(DerivedWitherClass.class),
-				ReflectionUtils.findField(DerivedWitherClass.class, "id"));
+		var property = getProperty(DerivedWitherClass.class, "id");
 
 		assertThat(property.getWither()).isPresent().hasValueSatisfying(actual -> {
 			assertThat(actual.getName()).isEqualTo("withId");
@@ -66,12 +63,15 @@ class PropertyUnitTests {
 	@Test // DATACMNS-1421
 	void shouldNotDiscoverWitherMethodWithIncompatibleReturnType() {
 
-		var property = Property.of(TypeInformation.of(AnotherLevel.class),
-				ReflectionUtils.findField(AnotherLevel.class, "id"));
+		var property = getProperty(AnotherLevel.class, "id");
 
 		assertThat(property.getWither()).isEmpty();
 	}
 
+	private static Property getProperty(Class<?> type, String fieldName) {
+		return Property.of(TypeInformation.of(type), ReflectionUtils.findField(type, fieldName));
+	}
+
 	static class ImmutableType {
 
 		final String id;
@@ -166,4 +166,12 @@ class PropertyUnitTests {
 			return new AnotherLevel(id);
 		}
 	}
+
+	public record StaticWither(String someField) {
+
+		public static StaticWither withSomeField(String someField) {
+			return new StaticWither(someField);
+		}
+	}
+
 }