From a27d1a28ffb69aed618869d3815a3af7120effa2 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 17:43:41 -0700 Subject: [PATCH] Bypass conversion when possible Prior to this commit conversion between like types would often result in a copy of the object. This can be problematic in the case of large byte arrays and objects that do not have a default constructor. The ConversionService SPI now includes canBypassConvert methods that can be used to deduce when conversion is not needed. Several existing converters have been updated to ensure they only apply when source and target types differ. This change introduces new methods to the ConversionService that will break existing implementations. However, it anticipated that most users are consuming the ConversionService interface rather then extending it. Issue: SPR-9566 --- .../core/convert/ConversionService.java | 25 ++++++++++++++++++- .../support/ArrayToArrayConverter.java | 21 ++++++++++++---- .../FallbackObjectToStringConverter.java | 5 +++- .../support/GenericConversionService.java | 15 +++++++++++ .../NumberToNumberConverterFactory.java | 11 ++++++-- .../GenericConversionServiceTests.java | 25 +++++++++++++++++++ 6 files changed, 93 insertions(+), 9 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java index 378016c30b..b359453d0d 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -21,6 +21,7 @@ package org.springframework.core.convert; * Call {@link #convert(Object, Class)} to perform a thread-safe type conversion using this system. * * @author Keith Donald + * @author Phillip Webb * @since 3.0 */ public interface ConversionService { @@ -54,6 +55,28 @@ public interface ConversionService { */ boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType); + /** + * Returns true if conversion between the sourceType and targetType can be bypassed. + * More precisely this method will return true if objects of sourceType can be + * converted to the targetType by returning the source object unchanged. + * @param sourceType context about the source type to convert from (may be null if source is null) + * @param targetType context about the target type to convert to (required) + * @return true if conversion can be bypassed + * @throws IllegalArgumentException if targetType is null + */ + boolean canBypassConvert(Class sourceType, Class targetType); + + /** + * Returns true if conversion between the sourceType and targetType can be bypassed. + * More precisely this method will return true if objects of sourceType can be + * converted to the targetType by returning the source object unchanged. + * @param sourceType context about the source type to convert from (may be null if source is null) + * @param targetType context about the target type to convert to (required) + * @return true if conversion can be bypassed + * @throws IllegalArgumentException if targetType is null + */ + boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType); + /** * Convert the source to targetType. * @param source the source object to convert (may be null) diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java index 288072f90b..9a20642a17 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -18,6 +18,7 @@ package org.springframework.core.convert.support; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Set; import org.springframework.core.convert.ConversionService; @@ -26,18 +27,22 @@ import org.springframework.core.convert.converter.ConditionalGenericConverter; import org.springframework.util.ObjectUtils; /** - * Converts an Array to another Array. - * First adapts the source array to a List, then delegates to {@link CollectionToArrayConverter} to perform the target array conversion. + * Converts an Array to another Array. First adapts the source array to a List, then + * delegates to {@link CollectionToArrayConverter} to perform the target array conversion. * * @author Keith Donald + * @author Phillip Webb * @since 3.0 */ final class ArrayToArrayConverter implements ConditionalGenericConverter { private final CollectionToArrayConverter helperConverter; + private final ConversionService conversionService; + public ArrayToArrayConverter(ConversionService conversionService) { this.helperConverter = new CollectionToArrayConverter(conversionService); + this.conversionService = conversionService; } public Set getConvertibleTypes() { @@ -48,8 +53,14 @@ final class ArrayToArrayConverter implements ConditionalGenericConverter { return this.helperConverter.matches(sourceType, targetType); } - public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - return this.helperConverter.convert(Arrays.asList(ObjectUtils.toObjectArray(source)), sourceType, targetType); + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + if (conversionService.canBypassConvert(sourceType.getElementTypeDescriptor(), + targetType.getElementTypeDescriptor())) { + return source; + } + List sourceList = Arrays.asList(ObjectUtils.toObjectArray(source)); + return this.helperConverter.convert(sourceList, sourceType, targetType); } } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java index 204a912b0c..268f0300ab 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -41,6 +41,9 @@ final class FallbackObjectToStringConverter implements ConditionalGenericConvert public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { Class sourceClass = sourceType.getObjectType(); + if (String.class.equals(sourceClass)) { + return false; + } return CharSequence.class.isAssignableFrom(sourceClass) || StringWriter.class.isAssignableFrom(sourceClass) || ObjectToObjectConverter.hasValueOfMethodOrConstructor(sourceClass, String.class); } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 205c66363e..e7ac42ff9f 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -126,6 +126,21 @@ public class GenericConversionService implements ConfigurableConversionService { return (converter != null); } + public boolean canBypassConvert(Class sourceType, Class targetType) { + Assert.notNull(targetType, "The targetType to convert to cannot be null"); + return canBypassConvert(sourceType != null ? TypeDescriptor.valueOf(sourceType) + : null, TypeDescriptor.valueOf(targetType)); + } + + public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { + Assert.notNull(targetType, "The targetType to convert to cannot be null"); + if (sourceType == null) { + return true; + } + GenericConverter converter = getConverter(sourceType, targetType); + return (converter == NO_OP_CONVERTER); + } + @SuppressWarnings("unchecked") public T convert(Object source, Class targetType) { Assert.notNull(targetType,"The targetType to convert to cannot be null"); diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java index 273d36ab62..fc9475e8a9 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -16,6 +16,8 @@ package org.springframework.core.convert.support; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConversion; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.util.NumberUtils; @@ -38,12 +40,17 @@ import org.springframework.util.NumberUtils; * @see java.math.BigDecimal * @see NumberUtils */ -final class NumberToNumberConverterFactory implements ConverterFactory { +final class NumberToNumberConverterFactory implements ConverterFactory, + ConditionalConversion { public Converter getConverter(Class targetType) { return new NumberToNumber(targetType); } + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + return !sourceType.equals(targetType); + } + private final static class NumberToNumber implements Converter { private final Class targetType; diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java index 7d72d957b9..b99c76af1d 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java @@ -19,6 +19,7 @@ package org.springframework.core.convert.support; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -709,6 +710,30 @@ public class GenericConversionServiceTests { assertEquals(Object.class, last.getType()); } + @Test + public void convertOptimizeArray() throws Exception { + // SPR-9566 + GenericConversionService conversionService = new DefaultConversionService(); + byte[] byteArray = new byte[] { 1, 2, 3 }; + byte[] converted = conversionService.convert(byteArray, byte[].class); + assertSame(byteArray, converted); + } + + @Test + public void convertCannotOptimizeArray() throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + conversionService.addConverter(new Converter() { + public Byte convert(Byte source) { + return (byte) (source + 1); + } + }); + DefaultConversionService.addDefaultConverters(conversionService); + byte[] byteArray = new byte[] { 1, 2, 3 }; + byte[] converted = conversionService.convert(byteArray, byte[].class); + assertNotSame(byteArray, converted); + assertTrue(Arrays.equals(new byte[] { 2, 3, 4 }, converted)); + } + private static class MyConditionalConverter implements Converter, ConditionalConversion {