From fc2e51a7734873fcf3c51b8ddba89e3d72ad7120 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 11 Feb 2016 14:06:23 +0000 Subject: [PATCH] Preserve ordering when auto-configuring WebSocket MessageConverters Previously, WebSocketMessagingAutoConfiguration added a single additional converter. This was a MappingJackson2MessageConverter configured with the auto-configured ObjectMapper. AbstractMessageBrokerConfiguration places additional converters before any of the default converters. This meant that the auto-configuration had the unwanted side-effect of changing the ordering of the converters. A MappingJackson2MessageConverter was now first in the list, whereas, by default, it's last in the list after a StringMessageConverter and a ByteArrayMessageConverter. This commit updates WebSocketMessagingAutoConfiguration so that it switches off the registration of the default converters and registers a StringMessageConverter, ByteArrayMessageConverter and MappingJackson2MessageConverter in that order. A test has been added to verify that the types of these three converters match the types of the default converters. A second test that verifies that String responses are converted correctly has also been added alongside the existing test that verified the behaviour for JSON responses. Closes gh-5123 --- .../WebSocketMessagingAutoConfiguration.java | 6 +- ...SocketMessagingAutoConfigurationTests.java | 67 +++++++++++++++++-- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfiguration.java index f0164264bf1..87d907a7ebb 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfiguration.java @@ -28,9 +28,11 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration; import org.springframework.context.annotation.Configuration; +import org.springframework.messaging.converter.ByteArrayMessageConverter; 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.config.AbstractMessageBrokerConfiguration; import org.springframework.util.MimeTypeUtils; import org.springframework.web.socket.config.annotation.AbstractWebSocketMessageBrokerConfigurer; @@ -72,8 +74,10 @@ public class WebSocketMessagingAutoConfiguration { DefaultContentTypeResolver resolver = new DefaultContentTypeResolver(); resolver.setDefaultMimeType(MimeTypeUtils.APPLICATION_JSON); converter.setContentTypeResolver(resolver); + messageConverters.add(new StringMessageConverter()); + messageConverters.add(new ByteArrayMessageConverter()); messageConverters.add(converter); - return true; + return false; } } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfigurationTests.java index 52853053d67..d84b235a3d9 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/websocket/WebSocketMessagingAutoConfigurationTests.java @@ -17,12 +17,15 @@ package org.springframework.boot.autoconfigure.websocket; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -39,6 +42,8 @@ import org.springframework.boot.context.web.ServerPortInfoApplicationContextInit import org.springframework.boot.test.EnvironmentTestUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.messaging.converter.CompositeMessageConverter; +import org.springframework.messaging.converter.MessageConverter; import org.springframework.messaging.converter.SimpleMessageConverter; import org.springframework.messaging.simp.annotation.SubscribeMapping; import org.springframework.messaging.simp.config.MessageBrokerRegistry; @@ -49,9 +54,11 @@ import org.springframework.messaging.simp.stomp.StompSession; import org.springframework.messaging.simp.stomp.StompSessionHandler; import org.springframework.messaging.simp.stomp.StompSessionHandlerAdapter; import org.springframework.stereotype.Controller; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.client.RestTemplate; import org.springframework.web.socket.client.standard.StandardWebSocketClient; import org.springframework.web.socket.config.annotation.AbstractWebSocketMessageBrokerConfigurer; +import org.springframework.web.socket.config.annotation.DelegatingWebSocketMessageBrokerConfiguration; import org.springframework.web.socket.config.annotation.EnableWebSocket; import org.springframework.web.socket.config.annotation.EnableWebSocketMessageBroker; import org.springframework.web.socket.config.annotation.StompEndpointRegistry; @@ -62,6 +69,7 @@ import org.springframework.web.socket.sockjs.client.Transport; import org.springframework.web.socket.sockjs.client.WebSocketTransport; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -92,7 +100,49 @@ public class WebSocketMessagingAutoConfigurationTests { } @Test - public void basicMessagingWithJson() throws Throwable { + public void basicMessagingWithJsonResponse() throws Throwable { + Object result = performStompSubscription("/app/json"); + assertThat(new String((byte[]) result), + is(equalTo(String.format("{%n \"foo\" : 5,%n \"bar\" : \"baz\"%n}")))); + } + + @Test + public void basicMessagingWithStringResponse() throws Throwable { + Object result = performStompSubscription("/app/string"); + assertThat(new String((byte[]) result), + is(equalTo(String.format("string data")))); + } + + @Test + public void customizedConverterTypesMatchDefaultConverterTypes() { + List customizedConverters = getCustomizedConverters(); + List defaultConverters = getDefaultConverters(); + assertThat(customizedConverters.size(), is(equalTo(defaultConverters.size()))); + Iterator customizedIterator = customizedConverters.iterator(); + Iterator defaultIterator = defaultConverters.iterator(); + while (customizedIterator.hasNext()) { + assertThat(customizedIterator.next(), + is(instanceOf(defaultIterator.next().getClass()))); + } + } + + private List getCustomizedConverters() { + List customizedConverters = new ArrayList(); + WebSocketMessagingAutoConfiguration.WebSocketMessageConverterConfiguration configuration = new WebSocketMessagingAutoConfiguration.WebSocketMessageConverterConfiguration(); + ReflectionTestUtils.setField(configuration, "objectMapper", new ObjectMapper()); + configuration.configureMessageConverters(customizedConverters); + return customizedConverters; + } + + @SuppressWarnings("unchecked") + private List getDefaultConverters() { + CompositeMessageConverter compositeDefaultConverter = new DelegatingWebSocketMessageBrokerConfiguration() + .brokerMessageConverter(); + return (List) ReflectionTestUtils + .getField(compositeDefaultConverter, "converters"); + } + + private Object performStompSubscription(final String topic) throws Throwable { EnvironmentTestUtils.addEnvironment(this.context, "server.port:0", "spring.jackson.serialization.indent-output:true"); this.context.register(WebSocketMessagingConfiguration.class); @@ -107,7 +157,7 @@ public class WebSocketMessagingAutoConfigurationTests { @Override public void afterConnected(StompSession session, StompHeaders connectedHeaders) { - session.subscribe("/app/data", new StompFrameHandler() { + session.subscribe(topic, new StompFrameHandler() { @Override public void handleFrame(StompHeaders headers, Object payload) { @@ -155,8 +205,8 @@ public class WebSocketMessagingAutoConfigurationTests { fail("Response was not received within 30 seconds"); } } - assertThat(new String((byte[]) result.get()), - is(equalTo(String.format("{%n \"foo\" : 5,%n \"bar\" : \"baz\"%n}")))); + + return result.get(); } @Configuration @@ -201,11 +251,16 @@ public class WebSocketMessagingAutoConfigurationTests { @Controller static class MessagingController { - @SubscribeMapping("/data") - Data getData() { + @SubscribeMapping("/json") + Data json() { return new Data(5, "baz"); } + @SubscribeMapping("/string") + String string() { + return "string data"; + } + } static class Data {