Use primary copy method for Kotlin data classes.

We now resolve the copy method for Kotlin data classes that match the primary constructor. Previously, copy method resolution could find a secondary copy method as we didn't check for the primary constructor structure (parameter names and types).

Closes #2324.

Author: mp911de

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

@@ -20,11 +20,13 @@
 import kotlin.reflect.KFunction;
 import kotlin.reflect.KParameter;
 import kotlin.reflect.KParameter.Kind;
+import kotlin.reflect.KType;
 import kotlin.reflect.full.KClasses;
 import kotlin.reflect.jvm.ReflectJvmMapping;
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -37,7 +39,8 @@
 import org.springframework.util.Assert;
 
 /**
- * Value object to represent a Kotlin {@code copy} method.
+ * Value object to represent a Kotlin {@code copy} method. The lookup requires a {@code copy} method that matches the
+ * primary constructor of the class regardless of whether the primary constructor is the persistence constructor.
  *
  * @author Mark Paluch
  * @since 2.1
@@ -59,13 +62,14 @@ private KotlinCopyMethod(Method publicCopyMethod, Method syntheticCopyMethod) {
 		this.publicCopyMethod = publicCopyMethod;
 		this.syntheticCopyMethod = syntheticCopyMethod;
 		this.copyFunction = ReflectJvmMapping.getKotlinFunction(publicCopyMethod);
-		this.parameterCount = copyFunction.getParameters().size();
+		this.parameterCount = copyFunction != null ? copyFunction.getParameters().size() : 0;
 	}
 
 	/**
 	 * Attempt to lookup the Kotlin {@code copy} method. Lookup happens in two stages: Find the synthetic copy method and
 	 * then attempt to resolve its public variant.
 	 *
+	 * @param property the property that must be included in the copy method.
 	 * @param type the class.
 	 * @return {@link Optional} {@link KotlinCopyMethod}.
 	 */
@@ -155,7 +159,6 @@ boolean shouldUsePublicCopyMethod(PersistentEntity<?, ?> entity) {
 		return true;
 	}
 
-	@SuppressWarnings("unchecked")
 	private static Optional<Method> findPublicCopyMethod(Method defaultKotlinMethod) {
 
 		Class<?> type = defaultKotlinMethod.getDeclaringClass();
@@ -167,10 +170,7 @@ private static Optional<Method> findPublicCopyMethod(Method defaultKotlinMethod)
 			return Optional.empty();
 		}
 
-		List<KParameter> constructorArguments = primaryConstructor.getParameters() //
-				.stream() //
-				.filter(it -> it.getKind() == Kind.VALUE) //
-				.collect(Collectors.toList());
+		List<KParameter> constructorArguments = getComponentArguments(primaryConstructor);
 
 		return Arrays.stream(type.getDeclaredMethods()).filter(it -> it.getName().equals("copy") //
 				&& !it.isSynthetic() //
@@ -207,7 +207,7 @@ private static boolean parameterMatches(List<KParameter> constructorArguments, K
 
 			KParameter constructorParameter = constructorArguments.get(constructorArgIndex);
 
-			if (!constructorParameter.getName().equals(parameter.getName())
+			if (constructorParameter.getName() == null || !constructorParameter.getName().equals(parameter.getName())
 					|| !constructorParameter.getType().equals(parameter.getType())) {
 				return false;
 			}
@@ -220,14 +220,70 @@ private static boolean parameterMatches(List<KParameter> constructorArguments, K
 
 	private static Optional<Method> findSyntheticCopyMethod(Class<?> type) {
 
+		KClass<?> kotlinClass = JvmClassMappingKt.getKotlinClass(type);
+		KFunction<?> primaryConstructor = KClasses.getPrimaryConstructor(kotlinClass);
+
+		if (primaryConstructor == null) {
+			return Optional.empty();
+		}
+
 		return Arrays.stream(type.getDeclaredMethods()) //
 				.filter(it -> it.getName().equals("copy$default") //
 						&& Modifier.isStatic(it.getModifiers()) //
 						&& it.getReturnType().equals(type))
 				.filter(Method::isSynthetic) //
+				.filter(it -> matchesPrimaryConstructor(it.getParameterTypes(), primaryConstructor))
 				.findFirst();
 	}
 
+	/**
+	 * Verify that the {@code parameterTypes} match arguments of the {@link KFunction primaryConstructor}.
+	 */
+	private static boolean matchesPrimaryConstructor(Class<?>[] parameterTypes, KFunction<?> primaryConstructor) {
+
+		List<KParameter> constructorArguments = getComponentArguments(primaryConstructor);
+
+		int defaultingArgs = KotlinDefaultMask.from(primaryConstructor, kParameter -> false).getDefaulting().length;
+
+		if (parameterTypes.length != 1 /* $this */ + constructorArguments.size() + defaultingArgs + 1 /* object marker */) {
+			return false;
+		}
+
+		// $this comes first
+		if (!isAssignableFrom(parameterTypes[0], primaryConstructor.getReturnType())) {
+			return false;
+		}
+
+		for (int i = 0; i < constructorArguments.size(); i++) {
+
+			KParameter kParameter = constructorArguments.get(i);
+
+			if (!isAssignableFrom(parameterTypes[i + 1], kParameter.getType())) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
+	private static List<KParameter> getComponentArguments(KFunction<?> primaryConstructor) {
+		return primaryConstructor.getParameters() //
+				.stream() //
+				.filter(it -> it.getKind() == Kind.VALUE) //
+				.collect(Collectors.toList());
+	}
+
+	private static boolean isAssignableFrom(Class<?> target, KType source) {
+
+		Type parameterType = ReflectJvmMapping.getJavaType(source);
+
+		if (parameterType instanceof Class) {
+			return target.isAssignableFrom((Class<?>) parameterType);
+		}
+
+		return false;
+	}
+
 	/**
 	 * Value object to represent Kotlin {@literal copy$default} invocation metadata.
 	 *

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

@@ -18,10 +18,12 @@
 import static org.assertj.core.api.Assertions.*;
 
 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.PersistentPropertyAccessor;
 import org.springframework.data.mapping.context.SampleMappingContext;
 import org.springframework.data.mapping.context.SamplePersistentProperty;
 import org.springframework.data.repository.core.EntityInformation;
@@ -32,6 +34,7 @@
  *
  * @author John Blum
  * @author Oliver Gierke
+ * @author Mark Paluch
  */
 public class ClassGeneratingPropertyAccessorFactoryEntityTypeTests {
 
@@ -53,6 +56,17 @@ public void getIdentifierOfClassBasedEntity() {
 		assertThat(getEntityInformation(Person.class).getId(jonDoe)).isEqualTo(jonDoe.name);
 	}
 
+	@Test // #2324
+	public void shouldGeneratePropertyAccessorForKotlinClassWithMultipleCopyMethods() {
+
+		ClassGeneratingPropertyAccessorFactory factory = new ClassGeneratingPropertyAccessorFactory();
+		PersistentPropertyAccessor<WithCustomCopyMethod> propertyAccessor = factory.getPropertyAccessor(
+				mappingContext.getRequiredPersistentEntity(WithCustomCopyMethod.class),
+				new WithCustomCopyMethod("", "", "", 1, LocalDateTime.MAX, LocalDateTime.MAX, ""));
+
+		assertThat(propertyAccessor).isNotNull();
+	}
+
 	private EntityInformation<Object, Serializable> getEntityInformation(Class<?> type) {
 
 		PersistentEntity<Object, SamplePersistentProperty> entity = mappingContext.getRequiredPersistentEntity(type);

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

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2021 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.data.mapping.model;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.Optional;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.data.mapping.context.SampleMappingContext;
+
+/**
+ * Unit tests for {@link KotlinCopyMethod}.
+ *
+ * @author Mark Paluch
+ */
+class KotlinCopyMethodUnitTests {
+
+	SampleMappingContext mappingContext = new SampleMappingContext();
+
+	@Test // #2324
+	void shouldLookupPrimaryConstructor() {
+
+		Optional<KotlinCopyMethod> optional = KotlinCopyMethod.findCopyMethod(DataClassKt.class);
+
+		assertThat(optional).hasValueSatisfying(actual -> {
+			// $this, 1 component, 1 defaulting mask, 1 Object marker
+			assertThat(actual.getSyntheticCopyMethod().getParameterCount()).isEqualTo(4);
+
+			// $this, 1 component
+			assertThat(actual.getCopyFunction().getParameters()).hasSize(2);
+		});
+	}
+
+	@Test // #2324
+	void shouldLookupPrimaryConstructorWhenTwoCopyMethodsArePresent() {
+
+		Optional<KotlinCopyMethod> optional = KotlinCopyMethod.findCopyMethod(WithCustomCopyMethod.class);
+
+		assertThat(optional).hasValueSatisfying(actual -> {
+			// $this, 7 components, 1 defaulting mask, 1 Object marker
+			assertThat(actual.getSyntheticCopyMethod().getParameterCount()).isEqualTo(10);
+
+			// $this, 7 components
+			assertThat(actual.getCopyFunction().getParameters()).hasSize(8);
+
+			assertThat(
+					actual.shouldUsePublicCopyMethod(mappingContext.getRequiredPersistentEntity(WithCustomCopyMethod.class)))
+							.isFalse();
+		});
+	}
+
+	@Test // #2324
+	void shouldUsePublicKotlinMethodForSinglePropertyEntities() {
+
+		KotlinCopyMethod copyMethod = KotlinCopyMethod.findCopyMethod(DataClassKt.class).get();
+
+		assertThat(copyMethod.shouldUsePublicCopyMethod(mappingContext.getRequiredPersistentEntity(DataClassKt.class)))
+				.isTrue();
+	}
+}

src/test/kotlin/org/springframework/data/mapping/model/DataClasses.kt

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.mapping.model
 
+import java.time.LocalDateTime
 import java.util.*
 
 /**
@@ -31,3 +32,30 @@ data class ExtendedDataClassKt(val id: Long, val name: String) {
 data class SingleSettableProperty constructor(val id: UUID = UUID.randomUUID()) {
 	val version: Int? = null
 }
+
+data class WithCustomCopyMethod(
+	val id: String?,
+	val userId: String,
+	val status: String,
+	val attempts: Int,
+	val createdAt: LocalDateTime,
+	val updatedAt: LocalDateTime,
+	val sessionId: String?
+) {
+
+	fun copy(
+		status: String,
+		updatedAt: LocalDateTime,
+		sessionId: String,
+		attempts: Int = this.attempts
+	) = WithCustomCopyMethod(
+		this.id,
+		this.userId,
+		status,
+		attempts,
+		this.createdAt,
+		updatedAt,
+		sessionId
+	)
+
+}