From effe606b28e2f9254aa2c5f89be133c9969e0911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Wed, 16 Oct 2024 15:05:47 +0200 Subject: [PATCH] Refine record canonical constructor support in BeanUtils This commit refines the contribution with the following changes: - Move the support to findPrimaryConstructor - Use a for loop instead of a Stream for more efficiency - Support other visibilities than public - Polishing Closes gh-33707 --- .../org/springframework/beans/BeanUtils.java | 40 ++++++++++--------- .../springframework/beans/BeanUtilsTests.java | 20 +++++++--- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index be8221d62d5b..f4ca77b3e1ca 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java @@ -225,9 +225,10 @@ public static T instantiateClass(Constructor ctor, Object... args) throws /** * Return a resolvable constructor for the provided class, either a primary or single - * public constructor with arguments, or a single non-public constructor with arguments, - * or simply a default constructor. Callers have to be prepared to resolve arguments - * for the returned constructor's parameters, if any. + * public constructor with arguments, a single non-public constructor with arguments + * or simply a default constructor. + *

Callers have to be prepared to resolve arguments for the returned constructor's + * parameters, if any. * @param clazz the class to check * @throws IllegalStateException in case of no unique constructor found at all * @since 5.3 @@ -253,19 +254,6 @@ else if (ctors.length == 0) { return (Constructor) ctors[0]; } } - else if (clazz.isRecord()) { - try { - // if record -> use canonical constructor, which is always presented - Class[] paramTypes - = Arrays.stream(clazz.getRecordComponents()) - .map(RecordComponent::getType) - .toArray(Class[]::new); - return clazz.getDeclaredConstructor(paramTypes); - } - catch (NoSuchMethodException ex) { - // Giving up with record... - } - } // Several constructors -> let's try to take the default constructor try { @@ -282,11 +270,12 @@ else if (clazz.isRecord()) { /** * Return the primary constructor of the provided class. For Kotlin classes, this * returns the Java constructor corresponding to the Kotlin primary constructor - * (as defined in the Kotlin specification). Otherwise, in particular for non-Kotlin - * classes, this simply returns {@code null}. + * (as defined in the Kotlin specification). For Java records, this returns the + * canonical constructor. Otherwise, this simply returns {@code null}. * @param clazz the class to check * @since 5.0 - * @see Kotlin docs + * @see Kotlin constructors + * @see Record constructor declarations */ @Nullable public static Constructor findPrimaryConstructor(Class clazz) { @@ -294,6 +283,19 @@ public static Constructor findPrimaryConstructor(Class clazz) { if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isKotlinType(clazz)) { return KotlinDelegate.findPrimaryConstructor(clazz); } + if (clazz.isRecord()) { + try { + // Use the canonical constructor which is always present + RecordComponent[] components = clazz.getRecordComponents(); + Class[] paramTypes = new Class[components.length]; + for (int i = 0; i < components.length; i++) { + paramTypes[i] = components[i].getType(); + } + return clazz.getDeclaredConstructor(paramTypes); + } + catch (NoSuchMethodException ignored) { + } + } return null; } diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java index 0ee33280a351..b700314ba1cf 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java @@ -522,26 +522,36 @@ void isNotSimpleProperty(Class type) { } @Test - void resolveRecordConstructor() throws NoSuchMethodException { + void resolveMultipleRecordPublicConstructor() throws NoSuchMethodException { assertThat(BeanUtils.getResolvableConstructor(RecordWithMultiplePublicConstructors.class)) - .isEqualTo(getRecordWithMultipleVariationsConstructor()); + .isEqualTo(RecordWithMultiplePublicConstructors.class.getDeclaredConstructor(String.class, String.class)); + } + + @Test + void resolveMultipleRecordePackagePrivateConstructor() throws NoSuchMethodException { + assertThat(BeanUtils.getResolvableConstructor(RecordWithMultiplePackagePrivateConstructors.class)) + .isEqualTo(RecordWithMultiplePackagePrivateConstructors.class.getDeclaredConstructor(String.class, String.class)); } private void assertSignatureEquals(Method desiredMethod, String signature) { assertThat(BeanUtils.resolveSignature(signature, MethodSignatureBean.class)).isEqualTo(desiredMethod); } + public record RecordWithMultiplePublicConstructors(String value, String name) { + @SuppressWarnings("unused") public RecordWithMultiplePublicConstructors(String value) { this(value, "default value"); } } - private Constructor getRecordWithMultipleVariationsConstructor() throws NoSuchMethodException { - return RecordWithMultiplePublicConstructors.class.getConstructor(String.class, String.class); + record RecordWithMultiplePackagePrivateConstructors(String value, String name) { + @SuppressWarnings("unused") + RecordWithMultiplePackagePrivateConstructors(String value) { + this(value, "default value"); + } } - @SuppressWarnings("unused") private static class NumberHolder {