Browse Source

Simpler check to avoid primitive conversion in ConvertingPropertyAccessor.

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

See #2546.
pull/2571/head
Oliver Drotbohm 4 years ago committed by Mark Paluch
parent
commit
92d8de08e4
No known key found for this signature in database
GPG Key ID: 4406B84C1661DCD1
  1. 2
      src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java
  2. 72
      src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java

2
src/main/java/org/springframework/data/mapping/model/ConvertingPropertyAccessor.java

@ -104,7 +104,7 @@ public class ConvertingPropertyAccessor<T> extends SimplePersistentPropertyPathA
return (S) (source == null // return (S) (source == null //
? null // ? null //
: ClassUtils.resolvePrimitiveIfNecessary(type).isAssignableFrom(source.getClass()) // : ClassUtils.isAssignable(type, source.getClass())
? source // ? source //
: conversionService.convert(source, type)); : conversionService.convert(source, type));
} }

72
src/test/java/org/springframework/data/mapping/model/ConvertingPropertyAccessorUnitTests.java

@ -16,16 +16,23 @@
package org.springframework.data.mapping.model; package org.springframework.data.mapping.model;
import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*; import static org.mockito.Mockito.*;
import lombok.AllArgsConstructor; import lombok.AllArgsConstructor;
import lombok.Data; import lombok.Data;
import lombok.Value; import lombok.Value;
import org.junit.jupiter.api.Test; 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.ConversionService;
import org.springframework.core.convert.support.DefaultConversionService; 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.PersistentPropertyAccessor;
import org.springframework.data.mapping.context.SampleMappingContext; import org.springframework.data.mapping.context.SampleMappingContext;
import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.data.mapping.context.SamplePersistentProperty;
@ -122,17 +129,47 @@ public class ConvertingPropertyAccessorUnitTests {
var context = new SampleMappingContext(); var context = new SampleMappingContext();
var accessor = context.getPersistentEntity(Order.class).getPropertyAccessor(order); var accessor = context.getPersistentEntity(Order.class).getPropertyAccessor(order);
var convertingAccessor = new ConvertingPropertyAccessor<Order>(accessor, var convertingAccessor = new ConvertingPropertyAccessor<Order>(accessor, new DefaultConversionService());
new DefaultConversionService());
var path = context.getPersistentPropertyPath("customer.firstname", var path = context.getPersistentPropertyPath("customer.firstname", Order.class);
Order.class);
convertingAccessor.setProperty(path, 2); convertingAccessor.setProperty(path, 2);
assertThat(convertingAccessor.getBean().getCustomer().getFirstname()).isEqualTo("2"); 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) { private static ConvertingPropertyAccessor getAccessor(Object entity, ConversionService conversionService) {
PersistentPropertyAccessor wrapper = new BeanWrapper<>(entity); PersistentPropertyAccessor wrapper = new BeanWrapper<>(entity);
@ -142,8 +179,7 @@ public class ConvertingPropertyAccessorUnitTests {
private static SamplePersistentProperty getIdProperty() { private static SamplePersistentProperty getIdProperty() {
var mappingContext = new SampleMappingContext(); var mappingContext = new SampleMappingContext();
var entity = mappingContext var entity = mappingContext.getRequiredPersistentEntity(Entity.class);
.getRequiredPersistentEntity(Entity.class);
return entity.getPersistentProperty("id"); return entity.getPersistentProperty("id");
} }
@ -161,4 +197,26 @@ public class ConvertingPropertyAccessorUnitTests {
static class Customer { static class Customer {
String firstname; 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;
}
}
} }

Loading…
Cancel
Save