Remove ContentTypeResolver from composite converter

Before this change CompositeMessageConverter had a ContentTypeResolver
field that was in turn set on all contained converters.

After this change that field is removed and effectively
CompositeMessageConverter is a simple container of other converters.
Each converter in turn must have been configured with a
ContentTypeResolver.

Doing so means it is less likely to have unexpected consequences when
configuring converters, the ContentTypeResolver set in the composite
converter overriding the one configured in a contained converter.

Also commit 676ce6 added default ContentTypeResolver initialization
to AbstractMessageConverter, which ensures that converters are still
straight forward to configure.

Issue: SPR-11462
This commit is contained in:
Rossen Stoyanchev 2014-02-27 21:25:10 -05:00
parent 0da1eefd74
commit 6016536055
6 changed files with 40 additions and 85 deletions

View File

@ -25,8 +25,8 @@ import org.springframework.messaging.MessageHeaders;
import org.springframework.util.Assert;
/**
* A {@link MessageConverter} that delegates to a list of other converters to invoke until
* one of them returns a non-null value.
* A {@link MessageConverter} that delegates to a list of other converters
* to be invoked until one of them returns a non-null result.
*
* @author Rossen Stoyanchev
* @since 4.0
@ -35,51 +35,16 @@ public class CompositeMessageConverter implements MessageConverter {
private final List<MessageConverter> converters;
private ContentTypeResolver contentTypeResolver;
/**
* Create a new instance with the given {@link MessageConverter}s in turn configuring
* each with a {@link DefaultContentTypeResolver}.
* Create an instance with the given converters.
*/
public CompositeMessageConverter(Collection<MessageConverter> converters) {
this(new ArrayList<MessageConverter>(converters), new DefaultContentTypeResolver());
}
/**
* Create an instance with the given {@link MessageConverter}s and configure all with
* the given {@link ContentTypeResolver}.
*/
public CompositeMessageConverter(Collection<MessageConverter> converters, ContentTypeResolver resolver) {
Assert.notEmpty(converters, "Converters must not be null");
Assert.notNull(resolver, "ContentTypeResolver must not be null");
Assert.notEmpty(converters, "Converters must not be empty");
this.converters = new ArrayList<MessageConverter>(converters);
this.contentTypeResolver = resolver;
applyContentTypeResolver(converters, resolver);
}
private static void applyContentTypeResolver(Collection<MessageConverter> converters,
ContentTypeResolver resolver) {
for (MessageConverter converter : converters) {
if (converter instanceof AbstractMessageConverter) {
((AbstractMessageConverter) converter).setContentTypeResolver(resolver);
}
}
}
public void setContentTypeResolver(ContentTypeResolver resolver) {
this.contentTypeResolver = resolver;
applyContentTypeResolver(getConverters(), resolver);
}
public ContentTypeResolver getContentTypeResolver() {
return this.contentTypeResolver;
}
public Collection<MessageConverter> getConverters() {
public List<MessageConverter> getConverters() {
return this.converters;
}
@ -108,7 +73,7 @@ public class CompositeMessageConverter implements MessageConverter {
@Override
public String toString() {
return "CompositeMessageConverter[contentTypeResolver=" + this.contentTypeResolver +
", converters=" + this.converters + "]";
return "CompositeMessageConverter[converters=" + this.converters + "]";
}
}

View File

@ -248,26 +248,20 @@ public abstract class AbstractMessageBrokerConfiguration implements ApplicationC
@Bean
public CompositeMessageConverter brokerMessageConverter() {
List<MessageConverter> converters = new ArrayList<MessageConverter>();
boolean registerDefaults = configureMessageConverters(converters);
if (registerDefaults) {
if (jackson2Present) {
converters.add(new MappingJackson2MessageConverter());
DefaultContentTypeResolver resolver = new DefaultContentTypeResolver();
resolver.setDefaultMimeType(MimeTypeUtils.APPLICATION_JSON);
MappingJackson2MessageConverter converter = new MappingJackson2MessageConverter();
converter.setContentTypeResolver(resolver);
converters.add(converter);
}
converters.add(new StringMessageConverter());
converters.add(new ByteArrayMessageConverter());
}
ContentTypeResolver contentTypeResolver = getContentTypeResolver();
if (contentTypeResolver == null) {
contentTypeResolver = new DefaultContentTypeResolver();
if (jackson2Present && registerDefaults) {
((DefaultContentTypeResolver) contentTypeResolver).setDefaultMimeType(MimeTypeUtils.APPLICATION_JSON);
}
}
return new CompositeMessageConverter(converters, contentTypeResolver);
return new CompositeMessageConverter(converters);
}
/**
@ -281,13 +275,6 @@ public abstract class AbstractMessageBrokerConfiguration implements ApplicationC
return true;
}
/**
* Override this method to provide a custom {@link ContentTypeResolver}.
*/
protected ContentTypeResolver getContentTypeResolver() {
return null;
}
@Bean
public UserDestinationResolver userDestinationResolver() {
DefaultUserDestinationResolver resolver = new DefaultUserDestinationResolver(userSessionRegistry());

View File

@ -17,7 +17,6 @@
package org.springframework.messaging.core;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@ -28,7 +27,6 @@ import org.springframework.messaging.Message;
import org.springframework.messaging.MessageHeaders;
import org.springframework.messaging.converter.*;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.util.MimeType;
import org.springframework.util.MimeTypeUtils;
import static org.junit.Assert.*;
@ -155,7 +153,7 @@ public class MessageSendingTemplateTests {
public void convertAndSendNoMatchingConverter() {
MessageConverter converter = new CompositeMessageConverter(
Arrays.<MessageConverter>asList(new MappingJackson2MessageConverter()), new DefaultContentTypeResolver());
Arrays.<MessageConverter>asList(new MappingJackson2MessageConverter()));
this.template.setMessageConverter(converter);
this.headers.put(MessageHeaders.CONTENT_TYPE, MimeTypeUtils.APPLICATION_XML);

View File

@ -275,14 +275,14 @@ public class MessageBrokerConfigurationTests {
AbstractMessageBrokerConfiguration config = new AbstractMessageBrokerConfiguration() {};
CompositeMessageConverter compositeConverter = config.brokerMessageConverter();
assertThat(compositeConverter.getConverters().size(), Matchers.is(3));
Iterator<MessageConverter> iterator = compositeConverter.getConverters().iterator();
assertThat(iterator.next(), Matchers.instanceOf(MappingJackson2MessageConverter.class));
assertThat(iterator.next(), Matchers.instanceOf(StringMessageConverter.class));
assertThat(iterator.next(), Matchers.instanceOf(ByteArrayMessageConverter.class));
List<MessageConverter> converters = compositeConverter.getConverters();
assertThat(converters.size(), Matchers.is(3));
assertThat(converters.get(0), Matchers.instanceOf(MappingJackson2MessageConverter.class));
assertThat(converters.get(1), Matchers.instanceOf(StringMessageConverter.class));
assertThat(converters.get(2), Matchers.instanceOf(ByteArrayMessageConverter.class));
DefaultContentTypeResolver resolver = (DefaultContentTypeResolver) compositeConverter.getContentTypeResolver();
assertEquals(MimeTypeUtils.APPLICATION_JSON, resolver.getDefaultMimeType());
ContentTypeResolver resolver = ((MappingJackson2MessageConverter) converters.get(0)).getContentTypeResolver();
assertEquals(MimeTypeUtils.APPLICATION_JSON, ((DefaultContentTypeResolver) resolver).getDefaultMimeType());
}
@Test
@ -300,9 +300,6 @@ public class MessageBrokerConfigurationTests {
assertThat(compositeConverter.getConverters().size(), Matchers.is(1));
Iterator<MessageConverter> iterator = compositeConverter.getConverters().iterator();
assertThat(iterator.next(), Matchers.is(testConverter));
DefaultContentTypeResolver resolver = (DefaultContentTypeResolver) compositeConverter.getContentTypeResolver();
assertNull(resolver.getDefaultMimeType());
}
@Test
@ -325,9 +322,6 @@ public class MessageBrokerConfigurationTests {
assertThat(iterator.next(), Matchers.instanceOf(MappingJackson2MessageConverter.class));
assertThat(iterator.next(), Matchers.instanceOf(StringMessageConverter.class));
assertThat(iterator.next(), Matchers.instanceOf(ByteArrayMessageConverter.class));
DefaultContentTypeResolver resolver = (DefaultContentTypeResolver) compositeConverter.getContentTypeResolver();
assertEquals(MimeTypeUtils.APPLICATION_JSON, resolver.getDefaultMimeType());
}
@Test

View File

@ -356,20 +356,18 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser {
if (convertersElement == null || Boolean.valueOf(convertersElement.getAttribute("register-defaults"))) {
convertersDef.setSource(source);
if (jackson2Present) {
convertersDef.add(new RootBeanDefinition(MappingJackson2MessageConverter.class));
RootBeanDefinition jacksonConverterDef = new RootBeanDefinition(MappingJackson2MessageConverter.class);
RootBeanDefinition resolverDef = new RootBeanDefinition(DefaultContentTypeResolver.class);
resolverDef.getPropertyValues().add("defaultMimeType", MimeTypeUtils.APPLICATION_JSON);
jacksonConverterDef.getPropertyValues().add("contentTypeResolver", resolverDef);
convertersDef.add(jacksonConverterDef);
}
convertersDef.add(new RootBeanDefinition(StringMessageConverter.class));
convertersDef.add(new RootBeanDefinition(ByteArrayMessageConverter.class));
}
RootBeanDefinition contentTypeResolverDef = new RootBeanDefinition(DefaultContentTypeResolver.class);
if (jackson2Present) {
contentTypeResolverDef.getPropertyValues().add("defaultMimeType", MimeTypeUtils.APPLICATION_JSON);
}
ConstructorArgumentValues cavs = new ConstructorArgumentValues();
cavs.addIndexedArgumentValue(0, convertersDef);
cavs.addIndexedArgumentValue(1, contentTypeResolverDef);
RootBeanDefinition brokerMessage = new RootBeanDefinition(CompositeMessageConverter.class, cavs, null);
return new RuntimeBeanReference(registerBeanDef(brokerMessage, parserCxt, source));

View File

@ -27,7 +27,11 @@ import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
import org.springframework.core.io.ClassPathResource;
import org.springframework.messaging.MessageHandler;
import org.springframework.messaging.converter.ByteArrayMessageConverter;
import org.springframework.messaging.converter.CompositeMessageConverter;
import org.springframework.messaging.converter.ContentTypeResolver;
import org.springframework.messaging.converter.DefaultContentTypeResolver;
import org.springframework.messaging.converter.MappingJackson2MessageConverter;
import org.springframework.messaging.converter.MessageConverter;
import org.springframework.messaging.converter.StringMessageConverter;
import org.springframework.messaging.simp.SimpMessagingTemplate;
@ -40,6 +44,7 @@ import org.springframework.messaging.simp.user.UserSessionRegistry;
import org.springframework.messaging.simp.stomp.StompBrokerRelayMessageHandler;
import org.springframework.messaging.support.AbstractSubscribableChannel;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
import org.springframework.util.MimeTypeUtils;
import org.springframework.web.HttpRequestHandler;
import org.springframework.web.context.support.GenericWebApplicationContext;
import org.springframework.web.servlet.HandlerMapping;
@ -52,6 +57,7 @@ import org.springframework.web.socket.server.support.WebSocketHttpRequestHandler
import org.springframework.web.socket.sockjs.support.SockJsHttpRequestHandler;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
/**
* Test fixture for MessageBrokerBeanDefinitionParser.
@ -220,7 +226,6 @@ public class MessageBrokerBeanDefinitionParserTests {
assertNotNull(messageConverter);
assertTrue(messageConverter instanceof CompositeMessageConverter);
CompositeMessageConverter compositeMessageConverter = this.appContext.getBean(CompositeMessageConverter.class);
assertNotNull(compositeMessageConverter);
@ -228,6 +233,14 @@ public class MessageBrokerBeanDefinitionParserTests {
assertNotNull(simpMessagingTemplate);
assertEquals("/personal", simpMessagingTemplate.getUserDestinationPrefix());
List<MessageConverter> converters = compositeMessageConverter.getConverters();
assertThat(converters.size(), Matchers.is(3));
assertThat(converters.get(0), Matchers.instanceOf(MappingJackson2MessageConverter.class));
assertThat(converters.get(1), Matchers.instanceOf(StringMessageConverter.class));
assertThat(converters.get(2), Matchers.instanceOf(ByteArrayMessageConverter.class));
ContentTypeResolver resolver = ((MappingJackson2MessageConverter) converters.get(0)).getContentTypeResolver();
assertEquals(MimeTypeUtils.APPLICATION_JSON, ((DefaultContentTypeResolver) resolver).getDefaultMimeType());
}
@Test