From 7ab1361231ea1572f1c9ff2e08ab5bb1e9767f6b Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 8 Mar 2013 11:32:35 -0500 Subject: [PATCH] INT-2955 Fix Collection/Map Conversion The fix for INT-2650 (to avoid unnecessary array copying) was too general in that it also prevented types in collections and maps from being converted. Add tests to illustrate that such payloads are not converted. Change code to only short circuit the conversion process if the payload is a primitive array. An existing test ensures that arrays are not copied. Add more tests to complete coverage - discovered another bug - the early exit after using a property editor to convert to a String was never taken - it was testing against the Class of the TypeConverter instead of the Type. If the target type is a String, we don't need to perform conversion after the property editor has done its conversion. Fix MessageHistory test (was testing MessageHeaders). --- .../util/BeanFactoryTypeConverter.java | 16 +- .../util/BeanFactoryTypeConverterTests.java | 157 +++++++++++++++++- 2 files changed, 168 insertions(+), 5 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/BeanFactoryTypeConverter.java b/spring-integration-core/src/main/java/org/springframework/integration/util/BeanFactoryTypeConverter.java index c5dec02b61..7bd17635c7 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/BeanFactoryTypeConverter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/BeanFactoryTypeConverter.java @@ -27,6 +27,9 @@ import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.expression.TypeConverter; +import org.springframework.integration.MessageHeaders; +import org.springframework.integration.history.MessageHistory; +import org.springframework.util.ClassUtils; /** * @author Dave Syer @@ -106,10 +109,15 @@ public class BeanFactoryTypeConverter implements TypeConverter, BeanFactoryAware * INT-2630 Spring 3.1 now converts ALL arguments; we know we don't need to convert MessageHeaders * or MessageHistory; the MapToMap converter requires a no-arg constructor. * Also INT-2650 - don't convert large byte[] - * This reverts the effective logic to Spring 3.0. */ - if (sourceType != null && sourceType.isAssignableTo(targetType)) { - return value; + if (sourceType != null) { + Class sourceClass = sourceType.getType(); + Class targetClass = targetType.getType(); + if ((sourceClass == MessageHeaders.class && targetClass == MessageHeaders.class) || + (sourceClass == MessageHistory.class && targetClass == MessageHistory.class) || + (sourceType.isAssignableTo(targetType) && ClassUtils.isPrimitiveArray(sourceClass))) { + return value; + } } if (conversionService.canConvert(sourceType, targetType)) { return conversionService.convert(value, sourceType, targetType); @@ -125,7 +133,7 @@ public class BeanFactoryTypeConverter implements TypeConverter, BeanFactoryAware editor.setValue(value); text = editor.getAsText(); } - if (String.class.isAssignableFrom(targetType.getClass())) { + if (String.class.isAssignableFrom(targetType.getType())) { return text; } return convertValue(text, TypeDescriptor.valueOf(String.class), targetType); diff --git a/spring-integration-core/src/test/java/org/springframework/integration/util/BeanFactoryTypeConverterTests.java b/spring-integration-core/src/test/java/org/springframework/integration/util/BeanFactoryTypeConverterTests.java index d60f0b175a..91312c7729 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/util/BeanFactoryTypeConverterTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/util/BeanFactoryTypeConverterTests.java @@ -17,6 +17,8 @@ package org.springframework.integration.util; import static junit.framework.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; @@ -28,7 +30,12 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -44,9 +51,14 @@ import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.integration.Message; import org.springframework.integration.MessageHeaders; +import org.springframework.integration.channel.QueueChannel; import org.springframework.integration.context.NamedComponent; +import org.springframework.integration.handler.MethodInvokingMessageProcessor; +import org.springframework.integration.handler.ServiceActivatingHandler; import org.springframework.integration.history.MessageHistory; import org.springframework.integration.message.GenericMessage; @@ -107,7 +119,7 @@ public class BeanFactoryTypeConverterTests { } }); MessageHistory history = MessageHistory.read(message); - assertSame(history, typeConverter.convertValue(history, TypeDescriptor.valueOf(MessageHeaders.class), TypeDescriptor.valueOf(MessageHeaders.class))); + assertSame(history, typeConverter.convertValue(history, TypeDescriptor.valueOf(MessageHistory.class), TypeDescriptor.valueOf(MessageHistory.class))); } @Test @@ -139,6 +151,128 @@ public class BeanFactoryTypeConverterTests { assertEquals("foo", typeConverter.convertValue(object, TypeDescriptor.valueOf(Object.class), TypeDescriptor.valueOf(String.class))); } + @SuppressWarnings("unchecked") + @Test + public void testMapOfMapOfCollectionIsConverted() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + DefaultConversionService conversionService = new DefaultConversionService(); + conversionService.addConverter(new Converter() { + public Bar convert(Foo source) { + return new Bar(); + } + }); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + beanFactory.setConversionService(conversionService); + typeConverter.setBeanFactory(beanFactory); + Map>> foos; + Map>> bars; + + TypeDescriptor sourceType = TypeDescriptor.map(Map.class, null, null); + TypeDescriptor targetType = TypeDescriptor.map(Map.class, TypeDescriptor.valueOf(String.class), + TypeDescriptor.map(Map.class, TypeDescriptor.valueOf(String.class), + TypeDescriptor.collection(Set.class, TypeDescriptor.valueOf(Bar.class)))); + + Set fooSet = new HashSet(); + fooSet.add(new Foo()); + Map> fooMap = new HashMap>(); + fooMap.put("foo", fooSet); + foos = new HashMap>>(); + foos.put("foo", fooMap); + + bars = (Map>>) typeConverter.convertValue(foos, sourceType, targetType); + assertTrue(bars.get("foo").get("foo").iterator().next() instanceof Bar); + + Service service = new Service(); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, "handle"); + processor.setConversionService(conversionService); + ServiceActivatingHandler handler = new ServiceActivatingHandler(processor); + QueueChannel replyChannel = new QueueChannel(); + handler.setOutputChannel(replyChannel); + handler.handleMessage(new GenericMessage>>>(foos)); + Message message = replyChannel.receive(0); + assertNotNull(message); + assertEquals("bar", message.getPayload()); + } + + @Test + public void testCollectionIsConverted() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + DefaultConversionService conversionService = new DefaultConversionService(); + conversionService.addConverter(new Converter() { + public Bar convert(Foo source) { + return new Bar(); + } + }); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + beanFactory.setConversionService(conversionService); + typeConverter.setBeanFactory(beanFactory); + + Service service = new Service(); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, "handle"); + processor.setConversionService(conversionService); + ServiceActivatingHandler handler = new ServiceActivatingHandler(processor); + QueueChannel replyChannel = new QueueChannel(); + handler.setOutputChannel(replyChannel); + handler.handleMessage(new GenericMessage>(Collections.singletonList(new Foo()))); + Message message = replyChannel.receive(0); + assertNotNull(message); + assertEquals("baz", message.getPayload()); + } + + @Test + public void testNullArg() { + DefaultConversionService conversionService = new DefaultConversionService(); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + Object foo = typeConverter.convertValue(null, null, TypeDescriptor.valueOf(Bar.class)); + assertNull(foo); + } + + @Test + public void testVoidArg() { + DefaultConversionService conversionService = new DefaultConversionService(); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + Object foo = typeConverter.convertValue(null, null, TypeDescriptor.valueOf(Void.class)); + assertNull(foo); + foo = typeConverter.convertValue(null, null, TypeDescriptor.valueOf(Void.TYPE)); + assertNull(foo); + } + + @Test + public void testEditorWithTargetString() { + DefaultConversionService conversionService = new DefaultConversionService(); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + UUID uuid = UUID.randomUUID(); + Object foo = typeConverter.convertValue(uuid, TypeDescriptor.valueOf(UUID.class), + TypeDescriptor.valueOf(String.class)); + assertEquals(uuid.toString(), foo); + } + + @Test + public void testEditorWithTargetFoo() { + DefaultConversionService conversionService = new DefaultConversionService(); + final Foo foo = new Foo(); + conversionService.addConverter(new Converter() { + public Foo convert(String source) { + return foo; + } + }); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + UUID uuid = UUID.randomUUID(); + Object convertedFoo = typeConverter.convertValue(uuid, TypeDescriptor.valueOf(UUID.class), + TypeDescriptor.valueOf(Foo.class)); + assertSame(foo, convertedFoo); + } + + @Test + public void testDelegateWithTargetUUID() { + DefaultConversionService conversionService = new DefaultConversionService(); + BeanFactoryTypeConverter typeConverter = new BeanFactoryTypeConverter(conversionService); + UUID uuid = UUID.randomUUID(); + Object converted = typeConverter.convertValue(uuid.toString(), TypeDescriptor.valueOf(String.class), + TypeDescriptor.valueOf(UUID.class)); + assertEquals(uuid, converted); + } + @Test public void initialConcurrency() throws Exception { ConversionService conversionService = mock(ConversionService.class); // can convert nothing so we drop down to P.E.s @@ -176,4 +310,25 @@ public class BeanFactoryTypeConverterTests { assertEquals(4, count.get()); assertFalse(concurrentlyInGetDefaultEditor.get()); } + + public static class Foo { + + } + + public static class Bar { + + } + + public static class Service { + + public String handle(Map>> payload) { + assertTrue(payload.get("foo").get("foo").iterator().next() instanceof Bar); + return "bar"; + } + + public String handle(Collection payload) { + assertTrue(payload.iterator().next() instanceof Bar); + return "baz"; + } + } }