From 44a472c6958ce42af130d97bf47533b58efb3d04 Mon Sep 17 00:00:00 2001
From: Oliver Drotbohm <odrotbohm@vmware.com>
Date: Mon, 14 Feb 2022 09:28:28 +0100
Subject: [PATCH] Simpler check to avoid primitive conversion in
 ConvertingPropertyAccessor.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We now use Spring's ClassUtils.isAssignable(…) directly. Unit tests to verify that conversion is skipped for primitive / boxed scenarios.

See #2546.
---
 .../model/ConvertingPropertyAccessor.java     |  2 +-
 .../ConvertingPropertyAccessorUnitTests.java  | 64 +++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java b/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java
index 95c1bac17..cb7b03324 100644
--- a/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java
+++ b/src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java
@@ -116,7 +116,7 @@ public class ConvertingPropertyAccessor<T> extends SimplePersistentPropertyPathA
 
 		return (S) (source == null //
 				? null //
-				: ClassUtils.resolvePrimitiveIfNecessary(type).isAssignableFrom(source.getClass()) //
+				: ClassUtils.isAssignable(type, source.getClass())
 						? source //
 						: conversionService.convert(source, type));
 	}
diff --git a/src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java b/src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java
index f79b3cede..0a653a4df 100755
--- a/src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java
+++ b/src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java
@@ -16,15 +16,23 @@
 package org.springframework.data.mapping.model;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
 import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.Value;
 
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.support.DefaultConversionService;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.mapping.context.SampleMappingContext;
@@ -133,6 +141,40 @@ public class ConvertingPropertyAccessorUnitTests {
 		assertThat(convertingAccessor.getBean().getCustomer().getFirstname()).isEqualTo("2");
 	}
 
+	@TestFactory // #2546
+	Stream<DynamicTest> doesNotInvokeConversionForMatchingPrimitives() {
+
+		IntegerWrapper wrapper = new IntegerWrapper();
+		wrapper.primitive = 42;
+		wrapper.boxed = 42;
+
+		SampleMappingContext context = new SampleMappingContext();
+		PersistentEntity<Object, SamplePersistentProperty> entity = context
+				.getRequiredPersistentEntity(IntegerWrapper.class);
+
+		SamplePersistentProperty primitiveProperty = entity.getRequiredPersistentProperty("primitive");
+		SamplePersistentProperty boxedProperty = entity.getRequiredPersistentProperty("boxed");
+
+		PersistentPropertyAccessor<IntegerWrapper> accessor = entity.getPropertyAccessor(wrapper);
+		ConversionService conversionService = mock(ConversionService.class);
+
+		ConvertingPropertyAccessor<IntegerWrapper> convertingAccessor = new ConvertingPropertyAccessor<>(accessor,
+				conversionService);
+
+		Stream<PrimitiveFixture> fixtures = Stream.of(
+				PrimitiveFixture.$(boxedProperty, int.class),
+				PrimitiveFixture.$(boxedProperty, Integer.class),
+				PrimitiveFixture.$(primitiveProperty, int.class),
+				PrimitiveFixture.$(primitiveProperty, Integer.class));
+
+		return DynamicTest.stream(fixtures, it -> {
+
+			convertingAccessor.getProperty(it.property, it.type);
+
+			verify(conversionService, never()).convert(any(), eq(it.type));
+		});
+	}
+
 	private static ConvertingPropertyAccessor getAccessor(Object entity, ConversionService conversionService) {
 
 		PersistentPropertyAccessor wrapper = new BeanWrapper<>(entity);
@@ -161,4 +203,26 @@ public class ConvertingPropertyAccessorUnitTests {
 	static class Customer {
 		String firstname;
 	}
+
+	static class IntegerWrapper {
+		int primitive;
+		Integer boxed;
+	}
+
+	@Value(staticConstructor = "$")
+	static class PrimitiveFixture implements Named<PrimitiveFixture> {
+
+		PersistentProperty<?> property;
+		Class<?> type;
+
+		@Override
+		public String getName() {
+			return String.format("Accessing %s as %s does not cause conversion.", property, type);
+		}
+
+		@Override
+		public PrimitiveFixture getPayload() {
+			return this;
+		}
+	}
 }