From 25f3465f1f1dc852da379b95a4303d3b7891da23 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 29 Nov 2019 15:53:37 +0000 Subject: [PATCH] Polishing contribution See gh-24087 --- spring-messaging/spring-messaging.gradle | 2 +- .../converter/AbstractMessageConverter.java | 31 +++--- .../MappingJackson2MessageConverter.java | 1 - .../MarshallingMessageConverter.java | 3 +- .../ProtobufJsonFormatMessageConverter.java | 67 +++++++++++++ .../converter/ProtobufMessageConverter.java | 95 ++++++------------- .../AbstractMessageBrokerConfiguration.java | 22 +---- .../converter/MessageConverterTests.java | 17 ++-- ...ava => ProtobufMessageConverterTests.java} | 39 ++++---- .../MessageBrokerConfigurationTests.java | 10 +- ...rotobufJsonFormatHttpMessageConverter.java | 21 ++-- spring-websocket/spring-websocket.gradle | 1 - .../MessageBrokerBeanDefinitionParser.java | 11 --- ...essageBrokerBeanDefinitionParserTests.java | 9 +- src/checkstyle/checkstyle-suppressions.xml | 1 + 15 files changed, 157 insertions(+), 173 deletions(-) create mode 100644 spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufJsonFormatMessageConverter.java rename spring-messaging/src/test/java/org/springframework/messaging/converter/{ProtobufMessageConverterTest.java => ProtobufMessageConverterTests.java} (87%) diff --git a/spring-messaging/spring-messaging.gradle b/spring-messaging/spring-messaging.gradle index 3855f3007c46..66ddd442ebd2 100644 --- a/spring-messaging/spring-messaging.gradle +++ b/spring-messaging/spring-messaging.gradle @@ -13,9 +13,9 @@ dependencies { optional("io.rsocket:rsocket-transport-netty") optional("com.fasterxml.jackson.core:jackson-databind") optional("javax.xml.bind:jaxb-api") + optional("com.google.protobuf:protobuf-java-util") optional("org.jetbrains.kotlinx:kotlinx-coroutines-core") optional("org.jetbrains.kotlinx:kotlinx-coroutines-reactor") - optional("com.google.protobuf:protobuf-java-util") testCompile("javax.inject:javax.inject-tck") testCompile("javax.servlet:javax.servlet-api") testCompile("javax.validation:validation-api") diff --git a/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java b/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java index d741a6007594..0dd92e12064b 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -17,10 +17,10 @@ package org.springframework.messaging.converter; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Arrays; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -48,7 +48,7 @@ public abstract class AbstractMessageConverter implements SmartMessageConverter protected final Log logger = LogFactory.getLog(getClass()); - private List supportedMimeTypes; + private final List supportedMimeTypes = new ArrayList<>(4); @Nullable private ContentTypeResolver contentTypeResolver = new DefaultContentTypeResolver(); @@ -59,28 +59,28 @@ public abstract class AbstractMessageConverter implements SmartMessageConverter /** - * Construct an {@code AbstractMessageConverter} supporting a single MIME type. + * Constructor with a single MIME type. * @param supportedMimeType the supported MIME type */ protected AbstractMessageConverter(MimeType supportedMimeType) { - setSupportedMimeTypes(Collections.singletonList(supportedMimeType)); + this(Collections.singletonList(supportedMimeType)); } /** - * Construct an {@code AbstractMessageConverter} supporting multiple MIME types. + * Constructor with one or more MIME types via vararg. * @param supportedMimeTypes the supported MIME types + * @since 5.2.2 */ - protected AbstractMessageConverter(Collection supportedMimeTypes) { - setSupportedMimeTypes(new ArrayList<>(supportedMimeTypes)); + protected AbstractMessageConverter(MimeType... supportedMimeTypes) { + this(Arrays.asList(supportedMimeTypes)); } /** - * Construct an {@code AbstractMessageConverter} supporting multiple MIME types. + * Constructor with a Collection of MIME types. * @param supportedMimeTypes the supported MIME types - * @since 5.2.2 */ - protected AbstractMessageConverter(MimeType... supportedMimeTypes) { - setSupportedMimeTypes(Arrays.asList(supportedMimeTypes)); + protected AbstractMessageConverter(Collection supportedMimeTypes) { + this.supportedMimeTypes.addAll(supportedMimeTypes); } @@ -92,12 +92,11 @@ public List getSupportedMimeTypes() { } /** - * Set the list of {@link MimeType} objects supported by this converter. + * Allows sub-classes to add more supported mime types. * @since 5.2.2 */ - protected void setSupportedMimeTypes(List supportedMimeTypes) { - Assert.notNull(supportedMimeTypes, "supportedMimeTypes must not be null"); - this.supportedMimeTypes = supportedMimeTypes; + protected void addSupportedMimeTypes(MimeType... supportedMimeTypes) { + this.supportedMimeTypes.addAll(Arrays.asList(supportedMimeTypes)); } /** diff --git a/spring-messaging/src/main/java/org/springframework/messaging/converter/MappingJackson2MessageConverter.java b/spring-messaging/src/main/java/org/springframework/messaging/converter/MappingJackson2MessageConverter.java index 16dd3e19bc89..4cb87dc22890 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/converter/MappingJackson2MessageConverter.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/converter/MappingJackson2MessageConverter.java @@ -22,7 +22,6 @@ import java.io.Writer; import java.lang.reflect.Type; import java.nio.charset.Charset; -import java.util.Arrays; import java.util.concurrent.atomic.AtomicReference; import com.fasterxml.jackson.annotation.JsonView; diff --git a/spring-messaging/src/main/java/org/springframework/messaging/converter/MarshallingMessageConverter.java b/spring-messaging/src/main/java/org/springframework/messaging/converter/MarshallingMessageConverter.java index d031157398c2..34701c849a7f 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/converter/MarshallingMessageConverter.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/converter/MarshallingMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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,7 +21,6 @@ import java.io.StringReader; import java.io.StringWriter; import java.io.Writer; -import java.util.Arrays; import javax.xml.transform.Result; import javax.xml.transform.Source; diff --git a/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufJsonFormatMessageConverter.java b/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufJsonFormatMessageConverter.java new file mode 100644 index 000000000000..2f07568b641d --- /dev/null +++ b/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufJsonFormatMessageConverter.java @@ -0,0 +1,67 @@ +/* + * Copyright 2002-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.messaging.converter; + +import com.google.protobuf.ExtensionRegistry; +import com.google.protobuf.util.JsonFormat; + +import org.springframework.lang.Nullable; + +/** + * Subclass of {@link ProtobufMessageConverter} for use with the official + * {@code "com.google.protobuf:protobuf-java-util"} library for JSON support. + * + *

Most importantly, this class allows for custom JSON parser and printer + * configurations through the {@link JsonFormat} utility. If no special parser + * or printer configuration is given, default variants will be used instead. + * + *

Requires Protobuf 3.x and {@code "com.google.protobuf:protobuf-java-util"} 3.x, + * with 3.3 or higher recommended. + * + * @author Rossen Stoyanchev + * @since 5.2.2 + */ +public class ProtobufJsonFormatMessageConverter extends ProtobufMessageConverter { + + /** + * Constructor with default instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and {@link ExtensionRegistry}. + */ + public ProtobufJsonFormatMessageConverter(@Nullable ExtensionRegistry extensionRegistry) { + this(null, null); + } + + /** + * Constructor with given instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and a default instance of {@link ExtensionRegistry}. + */ + public ProtobufJsonFormatMessageConverter( + @Nullable JsonFormat.Parser parser, @Nullable JsonFormat.Printer printer) { + + this(parser, printer, null); + } + + /** + * Constructor with given instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and {@link ExtensionRegistry}. + */ + public ProtobufJsonFormatMessageConverter(@Nullable JsonFormat.Parser parser, + @Nullable JsonFormat.Printer printer, @Nullable ExtensionRegistry extensionRegistry) { + + super(new ProtobufJavaUtilSupport(parser, printer), extensionRegistry); + } + +} diff --git a/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufMessageConverter.java b/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufMessageConverter.java index 2ac163a3a68f..1d5dbf820c84 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufMessageConverter.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/converter/ProtobufMessageConverter.java @@ -23,12 +23,12 @@ import java.lang.reflect.Method; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Map; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.Message; import com.google.protobuf.util.JsonFormat; + import org.springframework.lang.Nullable; import org.springframework.messaging.MessageHeaders; import org.springframework.util.ClassUtils; @@ -77,59 +77,23 @@ public class ProtobufMessageConverter extends AbstractMessageConverter { /** - * Construct a new {@code ProtobufMessageConverter}. + * Constructor with a default instance of {@link ExtensionRegistry}. */ public ProtobufMessageConverter() { - this((ProtobufFormatSupport) null, (ExtensionRegistry) null); + this(null, null); } /** - * Construct a new {@code ProtobufMessageConverter} with a registry that specifies - * protocol message extensions. - * - * @param extensionRegistry the registry to populate + * Constructor with a given {@code ExtensionRegistry}. */ - public ProtobufMessageConverter(@Nullable ExtensionRegistry extensionRegistry) { + public ProtobufMessageConverter(ExtensionRegistry extensionRegistry) { this(null, extensionRegistry); } - /** - * Construct a new {@code ProtobufMessageConverter} with the given - * {@code JsonFormat.Parser} and {@code JsonFormat.Printer} configuration. - * - * @param parser the JSON parser configuration - * @param printer the JSON printer configuration - */ - public ProtobufMessageConverter(@Nullable JsonFormat.Parser parser, @Nullable JsonFormat.Printer printer) { - this(new ProtobufJavaUtilSupport(parser, printer), (ExtensionRegistry) null); - } + ProtobufMessageConverter(@Nullable ProtobufFormatSupport formatSupport, + @Nullable ExtensionRegistry extensionRegistry) { - /** - * Construct a new {@code ProtobufMessageConverter} with the given - * {@code JsonFormat.Parser} and {@code JsonFormat.Printer} configuration, also - * accepting a registry that specifies protocol message extensions. - * - * @param parser the JSON parser configuration - * @param printer the JSON printer configuration - * @param extensionRegistry the registry to populate - */ - public ProtobufMessageConverter(@Nullable JsonFormat.Parser parser, - @Nullable JsonFormat.Printer printer, @Nullable ExtensionRegistry extensionRegistry) { - - this(new ProtobufJavaUtilSupport(parser, printer), extensionRegistry); - } - - /** - * Construct a new {@code ProtobufMessageConverter} with the given - * {@code ProtobufFormatSupport} configuration, also - * accepting a registry that specifies protocol message extensions. - * - * @param formatSupport support third party - * @param extensionRegistry the registry to populate - */ - public ProtobufMessageConverter(@Nullable ProtobufFormatSupport formatSupport, - @Nullable ExtensionRegistry extensionRegistry) { - super(PROTOBUF); + super(PROTOBUF, TEXT_PLAIN); if (formatSupport != null) { this.protobufFormatSupport = formatSupport; @@ -142,12 +106,13 @@ else if (ClassUtils.isPresent("com.google.protobuf.util.JsonFormat", getClass(). } if (this.protobufFormatSupport != null) { - setSupportedMimeTypes(Arrays.asList(protobufFormatSupport.supportedMediaTypes())); + addSupportedMimeTypes(this.protobufFormatSupport.supportedMediaTypes()); } this.extensionRegistry = (extensionRegistry == null ? ExtensionRegistry.newInstance() : extensionRegistry); } + @Override protected boolean supports(Class clazz) { return Message.class.isAssignableFrom(clazz); @@ -161,7 +126,9 @@ protected boolean canConvertTo(Object payload, @Nullable MessageHeaders headers) } @Override - protected Object convertFromInternal(org.springframework.messaging.Message message, Class targetClass, @Nullable Object conversionHint) { + protected Object convertFromInternal(org.springframework.messaging.Message message, + Class targetClass, @Nullable Object conversionHint) { + MimeType contentType = getMimeType(message.getHeaders()); final Object payload = message.getPayload(); @@ -175,18 +142,16 @@ protected Object convertFromInternal(org.springframework.messaging.Message me } Message.Builder builder = getMessageBuilder(targetClass); - try { if (PROTOBUF.isCompatibleWith(contentType)) { builder.mergeFrom((byte[]) payload, this.extensionRegistry); } - else if (protobufFormatSupport != null) { - this.protobufFormatSupport.merge( - message, charset, contentType, this.extensionRegistry, builder); + else if (this.protobufFormatSupport != null) { + this.protobufFormatSupport.merge(message, charset, contentType, this.extensionRegistry, builder); } } - catch (IOException e) { - throw new MessageConversionException(message, "Could not read proto message" + e.getMessage(), e); + catch (IOException ex) { + throw new MessageConversionException(message, "Could not read proto message" + ex.getMessage(), ex); } return builder.build(); @@ -194,13 +159,14 @@ else if (protobufFormatSupport != null) { @Override - protected Object convertToInternal(Object payload, @Nullable MessageHeaders headers, @Nullable Object conversionHint) { + protected Object convertToInternal( + Object payload, @Nullable MessageHeaders headers, @Nullable Object conversionHint) { + final Message message = (Message) payload; MimeType contentType = getMimeType(headers); if (contentType == null) { contentType = PROTOBUF; - } Charset charset = contentType.getCharset(); @@ -220,14 +186,13 @@ else if (this.protobufFormatSupport != null) { payload = new String(outputStream.toByteArray(), charset); } } - catch (IOException e) { - throw new MessageConversionException("Could not write proto message" + e.getMessage(), e); + catch (IOException ex) { + throw new MessageConversionException("Could not write proto message" + ex.getMessage(), ex); } return payload; } - /** * Create a new {@code Message.Builder} instance for the given class. *

This method uses a ConcurrentReferenceHashMap for caching method lookups. @@ -257,9 +222,9 @@ interface ProtobufFormatSupport { boolean supportsWriteOnly(@Nullable MimeType mediaType); - void merge(org.springframework.messaging.Message message, Charset charset, MimeType contentType, - ExtensionRegistry extensionRegistry, Message.Builder builder) - throws IOException, MessageConversionException; + void merge(org.springframework.messaging.Message message, + Charset charset, MimeType contentType, ExtensionRegistry extensionRegistry, + Message.Builder builder) throws IOException, MessageConversionException; void print(Message message, OutputStream output, MimeType contentType, Charset charset) throws IOException, MessageConversionException; @@ -283,7 +248,7 @@ public ProtobufJavaUtilSupport(@Nullable JsonFormat.Parser parser, @Nullable Jso @Override public MimeType[] supportedMediaTypes() { - return new MimeType[]{PROTOBUF, TEXT_PLAIN, APPLICATION_JSON}; + return new MimeType[]{APPLICATION_JSON}; } @Override @@ -292,8 +257,8 @@ public boolean supportsWriteOnly(@Nullable MimeType mimeType) { } @Override - public void merge(org.springframework.messaging.Message message, Charset charset, MimeType contentType, - ExtensionRegistry extensionRegistry, Message.Builder builder) + public void merge(org.springframework.messaging.Message message, Charset charset, + MimeType contentType, ExtensionRegistry extensionRegistry, Message.Builder builder) throws IOException, MessageConversionException { if (contentType.isCompatibleWith(APPLICATION_JSON)) { @@ -313,12 +278,12 @@ public void print(Message message, OutputStream output, MimeType contentType, Ch OutputStreamWriter writer = new OutputStreamWriter(output, charset); this.printer.appendTo(message, writer); writer.flush(); - } else { + } + else { throw new MessageConversionException( "protobuf-java-util does not support printing " + contentType); } } } - } diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java index 519fbac6d110..a3cc4f10c15a 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractMessageBrokerConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.MessageConverter; import org.springframework.messaging.converter.StringMessageConverter; -import org.springframework.messaging.converter.ProtobufMessageConverter; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; import org.springframework.messaging.handler.invocation.HandlerMethodReturnValueHandler; import org.springframework.messaging.simp.SimpLogging; @@ -94,15 +93,9 @@ public abstract class AbstractMessageBrokerConfiguration implements ApplicationC private static final String MVC_VALIDATOR_NAME = "mvcValidator"; - private static final boolean jackson2Present; + private static final boolean jackson2Present = ClassUtils.isPresent( + "com.fasterxml.jackson.databind.ObjectMapper", AbstractMessageBrokerConfiguration.class.getClassLoader()); - private static final boolean protobufPresent; - - static { - ClassLoader classLoader = AbstractMessageBrokerConfiguration.class.getClassLoader(); - jackson2Present = ClassUtils.isPresent("com.fasterxml.jackson.databind.ObjectMapper", classLoader); - protobufPresent = ClassUtils.isPresent("com.google.protobuf.Message", classLoader); - } @Nullable private ApplicationContext applicationContext; @@ -398,9 +391,6 @@ public CompositeMessageConverter brokerMessageConverter() { if (jackson2Present) { converters.add(createJacksonConverter()); } - if (protobufPresent) { - converters.add(createProtobufConverter()); - } } return new CompositeMessageConverter(converters); } @@ -413,14 +403,6 @@ protected MappingJackson2MessageConverter createJacksonConverter() { return converter; } - protected ProtobufMessageConverter createProtobufConverter() { - DefaultContentTypeResolver resolver = new DefaultContentTypeResolver(); - resolver.setDefaultMimeType(ProtobufMessageConverter.PROTOBUF); - final ProtobufMessageConverter converter = new ProtobufMessageConverter(); - converter.setContentTypeResolver(resolver); - return converter; - } - /** * Override this method to add custom message converters. * @param messageConverters the list to add converters to, initially empty diff --git a/spring-messaging/src/test/java/org/springframework/messaging/converter/MessageConverterTests.java b/spring-messaging/src/test/java/org/springframework/messaging/converter/MessageConverterTests.java index ff834b84e40a..dfb4ff7409ca 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/converter/MessageConverterTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/converter/MessageConverterTests.java @@ -78,8 +78,7 @@ public void supportsMimeTypeNotSpecified() { public void supportsMimeTypeNoneConfigured() { Message message = MessageBuilder.withPayload( "ABC").setHeader(MessageHeaders.CONTENT_TYPE, MimeTypeUtils.APPLICATION_JSON).build(); - final MimeType[] empty = {}; - this.converter = new TestMessageConverter(empty); + this.converter = new TestMessageConverter(new MimeType[0]); assertThat(this.converter.fromMessage(message, String.class)).isEqualTo("success-from"); } @@ -100,10 +99,8 @@ public void canConvertFromStrictContentTypeMatch() { @Test public void setStrictContentTypeMatchWithNoSupportedMimeTypes() { - final MimeType[] empty = {}; - this.converter = new TestMessageConverter(empty); - assertThatIllegalArgumentException().isThrownBy(() -> - this.converter.setStrictContentTypeMatch(true)); + this.converter = new TestMessageConverter(new MimeType[0]); + assertThatIllegalArgumentException().isThrownBy(() -> this.converter.setStrictContentTypeMatch(true)); } @Test @@ -158,15 +155,15 @@ protected boolean supports(Class clazz) { } @Override - protected Object convertFromInternal(Message message, Class targetClass, - @Nullable Object conversionHint) { + protected Object convertFromInternal( + Message message, Class targetClass, @Nullable Object conversionHint) { return "success-from"; } @Override - protected Object convertToInternal(Object payload, @Nullable MessageHeaders headers, - @Nullable Object conversionHint) { + protected Object convertToInternal( + Object payload, @Nullable MessageHeaders headers, @Nullable Object conversionHint) { return "success-to"; } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTest.java b/spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTests.java similarity index 87% rename from spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTest.java rename to spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTests.java index fd3b622fc1e6..63fa57513a13 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTest.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/converter/ProtobufMessageConverterTests.java @@ -16,13 +16,13 @@ package org.springframework.messaging.converter; -import java.io.IOException; import java.util.HashMap; import java.util.Map; import com.google.protobuf.ExtensionRegistry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import org.springframework.messaging.Message; import org.springframework.messaging.MessageHeaders; import org.springframework.messaging.protobuf.Msg; @@ -39,15 +39,18 @@ * * @author Parviz Rozikov */ -class ProtobufMessageConverterTest { +public class ProtobufMessageConverterTests { private ProtobufMessageConverter converter; private ExtensionRegistry extensionRegistry; private Msg testMsg; + private Message message; + private Message messageWithoutContentType; + private Message messageJson; @@ -58,20 +61,22 @@ public void setup() { this.testMsg = Msg.newBuilder().setFoo("Foo").setBlah(SecondMsg.newBuilder().setBlah(123).build()).build(); this.message = MessageBuilder.withPayload(this.testMsg.toByteArray()) .setHeader(CONTENT_TYPE, ProtobufMessageConverter.PROTOBUF).build(); - this.messageWithoutContentType = MessageBuilder.withPayload(this.testMsg.toByteArray()) + this.messageWithoutContentType = MessageBuilder.withPayload(this.testMsg.toByteArray()).build(); + this.messageJson = MessageBuilder.withPayload( + "{\n" + + " \"foo\": \"Foo\",\n" + + " \"blah\": {\n" + + " \"blah\": 123\n" + + " }\n" + + "}") + .setHeader(CONTENT_TYPE, APPLICATION_JSON) .build(); - this.messageJson = MessageBuilder.withPayload("{\n" + - " \"foo\": \"Foo\",\n" + - " \"blah\": {\n" + - " \"blah\": 123\n" + - " }\n" + - "}") - .setHeader(CONTENT_TYPE, APPLICATION_JSON).build(); } + @Test public void extensionRegistryNull() { - ProtobufMessageConverter converter = new ProtobufMessageConverter((ExtensionRegistry) null); + ProtobufMessageConverter converter = new ProtobufMessageConverter(null); assertThat(converter.extensionRegistry).isNotNull(); } @@ -92,13 +97,13 @@ public void canConvertTo() { @Test - public void convertFrom() throws IOException { + public void convertFrom() { final Msg msg = (Msg) converter.fromMessage(message, Msg.class); assertThat(msg).isEqualTo(testMsg); } @Test - public void convertTo() throws IOException { + public void convertTo() { final Message message = converter.toMessage(this.testMsg, this.message.getHeaders()); assertThat(message).isNotNull(); assertThat(message.getPayload()).isEqualTo(this.message.getPayload()); @@ -106,19 +111,19 @@ public void convertTo() throws IOException { @Test - public void convertFromNoContentType() throws IOException { + public void convertFromNoContentType(){ Msg result = (Msg) converter.fromMessage(messageWithoutContentType, Msg.class); assertThat(result).isEqualTo(testMsg); } @Test - public void defaultContentType() throws Exception { + public void defaultContentType() { assertThat(converter.getDefaultContentType(testMsg)).isEqualTo(ProtobufMessageConverter.PROTOBUF); } @Test - public void testJsonWithGoogleProtobuf() throws Exception { + public void testJsonWithGoogleProtobuf() { this.converter = new ProtobufMessageConverter( new ProtobufMessageConverter.ProtobufJavaUtilSupport(null, null), extensionRegistry); @@ -139,4 +144,4 @@ public void testJsonWithGoogleProtobuf() throws Exception { assertThat(msg).isEqualTo(this.testMsg); } -} \ No newline at end of file +} diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java index 4b643212b6cf..779c1de0526e 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/config/MessageBrokerConfigurationTests.java @@ -41,7 +41,6 @@ import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.MessageConverter; import org.springframework.messaging.converter.StringMessageConverter; -import org.springframework.messaging.converter.ProtobufMessageConverter; import org.springframework.messaging.handler.annotation.MessageMapping; import org.springframework.messaging.handler.annotation.SendTo; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; @@ -282,17 +281,13 @@ public void configureMessageConvertersDefault() { CompositeMessageConverter compositeConverter = config.brokerMessageConverter(); List converters = compositeConverter.getConverters(); - assertThat(converters).hasSize(4); + assertThat(converters).hasSize(3); assertThat(converters.get(0)).isInstanceOf(StringMessageConverter.class); assertThat(converters.get(1)).isInstanceOf(ByteArrayMessageConverter.class); assertThat(converters.get(2)).isInstanceOf(MappingJackson2MessageConverter.class); - assertThat(converters.get(3)).isInstanceOf(ProtobufMessageConverter.class); ContentTypeResolver resolver = ((MappingJackson2MessageConverter) converters.get(2)).getContentTypeResolver(); assertThat(((DefaultContentTypeResolver) resolver).getDefaultMimeType()).isEqualTo(MimeTypeUtils.APPLICATION_JSON); - - resolver = ((ProtobufMessageConverter) converters.get(3)).getContentTypeResolver(); - assertThat(((DefaultContentTypeResolver) resolver).getDefaultMimeType()).isEqualTo(ProtobufMessageConverter.PROTOBUF); } @Test @@ -344,13 +339,12 @@ protected boolean configureMessageConverters(List messageConve }; CompositeMessageConverter compositeConverter = config.brokerMessageConverter(); - assertThat(compositeConverter.getConverters()).hasSize(5); + assertThat(compositeConverter.getConverters()).hasSize(4); Iterator iterator = compositeConverter.getConverters().iterator(); assertThat(iterator.next()).isEqualTo(testConverter); assertThat(iterator.next()).isInstanceOf(StringMessageConverter.class); assertThat(iterator.next()).isInstanceOf(ByteArrayMessageConverter.class); assertThat(iterator.next()).isInstanceOf(MappingJackson2MessageConverter.class); - assertThat(iterator.next()).isInstanceOf(ProtobufMessageConverter.class); } @Test diff --git a/spring-web/src/main/java/org/springframework/http/converter/protobuf/ProtobufJsonFormatHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/protobuf/ProtobufJsonFormatHttpMessageConverter.java index 47bc2ad91a5d..a89df049cd79 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/protobuf/ProtobufJsonFormatHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/protobuf/ProtobufJsonFormatHttpMessageConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -42,18 +42,16 @@ public class ProtobufJsonFormatHttpMessageConverter extends ProtobufHttpMessageConverter { /** - * Construct a new {@code ProtobufJsonFormatHttpMessageConverter} with default - * {@code JsonFormat.Parser} and {@code JsonFormat.Printer} configuration. + * Constructor with default instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and {@link ExtensionRegistry}. */ public ProtobufJsonFormatHttpMessageConverter() { this(null, null, (ExtensionRegistry)null); } /** - * Construct a new {@code ProtobufJsonFormatHttpMessageConverter} with the given - * {@code JsonFormat.Parser} and {@code JsonFormat.Printer} configuration. - * @param parser the JSON parser configuration - * @param printer the JSON printer configuration + * Constructor with given instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and a default instance of {@link ExtensionRegistry}. */ public ProtobufJsonFormatHttpMessageConverter( @Nullable JsonFormat.Parser parser, @Nullable JsonFormat.Printer printer) { @@ -62,13 +60,8 @@ public ProtobufJsonFormatHttpMessageConverter( } /** - * Construct a new {@code ProtobufJsonFormatHttpMessageConverter} with the given - * {@code JsonFormat.Parser} and {@code JsonFormat.Printer} configuration, also - * accepting a registry that specifies protocol message extensions. - * @param parser the JSON parser configuration - * @param printer the JSON printer configuration - * @param extensionRegistry the registry to populate - * @since 5.1 + * Constructor with given instances of {@link JsonFormat.Parser}, + * {@link JsonFormat.Printer}, and {@link ExtensionRegistry}. */ public ProtobufJsonFormatHttpMessageConverter(@Nullable JsonFormat.Parser parser, @Nullable JsonFormat.Printer printer, @Nullable ExtensionRegistry extensionRegistry) { diff --git a/spring-websocket/spring-websocket.gradle b/spring-websocket/spring-websocket.gradle index b464a375073a..96355eef5369 100644 --- a/spring-websocket/spring-websocket.gradle +++ b/spring-websocket/spring-websocket.gradle @@ -21,7 +21,6 @@ dependencies { optional("io.undertow:undertow-servlet") optional("io.undertow:undertow-websockets-jsr") optional("com.fasterxml.jackson.core:jackson-databind") - optional("com.google.protobuf:protobuf-java-util") testCompile("org.apache.tomcat.embed:tomcat-embed-core") testCompile("org.apache.tomcat.embed:tomcat-embed-websocket") testCompile("io.projectreactor.netty:reactor-netty") diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java index 7323849b022c..31294cb57940 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java @@ -44,7 +44,6 @@ import org.springframework.messaging.converter.DefaultContentTypeResolver; import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.StringMessageConverter; -import org.springframework.messaging.converter.ProtobufMessageConverter; import org.springframework.messaging.simp.SimpMessagingTemplate; import org.springframework.messaging.simp.SimpSessionScope; import org.springframework.messaging.simp.broker.SimpleBrokerMessageHandler; @@ -116,13 +115,10 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { private static final boolean javaxValidationPresent; - private static final boolean protobufPresent; - static { ClassLoader classLoader = MessageBrokerBeanDefinitionParser.class.getClassLoader(); jackson2Present = ClassUtils.isPresent("com.fasterxml.jackson.databind.ObjectMapper", classLoader); javaxValidationPresent = ClassUtils.isPresent("javax.validation.Validator", classLoader); - protobufPresent = ClassUtils.isPresent("com.google.protobuf.Message", classLoader); } @@ -506,13 +502,6 @@ private RuntimeBeanReference registerMessageConverter( jacksonConverterDef.getPropertyValues().add("objectMapper", jacksonFactoryDef); converters.add(jacksonConverterDef); } - if (protobufPresent) { - final RootBeanDefinition protoConverterDef = new RootBeanDefinition(ProtobufMessageConverter.class); - RootBeanDefinition resolverDef = new RootBeanDefinition(DefaultContentTypeResolver.class); - resolverDef.getPropertyValues().add("defaultMimeType", ProtobufMessageConverter.PROTOBUF); - protoConverterDef.getPropertyValues().add("contentTypeResolver", resolverDef); - converters.add(protoConverterDef); - } } ConstructorArgumentValues cargs = new ConstructorArgumentValues(); cargs.addIndexedArgumentValue(0, converters); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java index ec0ded55043f..43e1ad59bdf8 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java @@ -41,7 +41,6 @@ import org.springframework.messaging.converter.DefaultContentTypeResolver; import org.springframework.messaging.converter.MappingJackson2MessageConverter; import org.springframework.messaging.converter.MessageConverter; -import org.springframework.messaging.converter.ProtobufMessageConverter; import org.springframework.messaging.converter.StringMessageConverter; import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver; import org.springframework.messaging.handler.invocation.HandlerMethodReturnValueHandler; @@ -340,18 +339,14 @@ public void annotationMethodMessageHandler() { assertThat(simpMessagingTemplate.getUserDestinationPrefix()).isEqualTo("/personal/"); List converters = compositeMessageConverter.getConverters(); - assertThat(converters).hasSize(4); + assertThat(converters).hasSize(3); assertThat(converters.get(0)).isInstanceOf(StringMessageConverter.class); assertThat(converters.get(1)).isInstanceOf(ByteArrayMessageConverter.class); assertThat(converters.get(2)).isInstanceOf(MappingJackson2MessageConverter.class); - assertThat(converters.get(3)).isInstanceOf(ProtobufMessageConverter.class); ContentTypeResolver resolver = ((MappingJackson2MessageConverter) converters.get(2)).getContentTypeResolver(); assertThat(((DefaultContentTypeResolver) resolver).getDefaultMimeType()).isEqualTo(MimeTypeUtils.APPLICATION_JSON); - resolver = ((ProtobufMessageConverter) converters.get(3)).getContentTypeResolver(); - assertThat(((DefaultContentTypeResolver) resolver).getDefaultMimeType()).isEqualTo(ProtobufMessageConverter.PROTOBUF); - DirectFieldAccessor handlerAccessor = new DirectFieldAccessor(annotationMethodMessageHandler); Object pathMatcher = handlerAccessor.getPropertyValue("pathMatcher"); String pathSeparator = (String) new DirectFieldAccessor(pathMatcher).getPropertyValue("pathSeparator"); @@ -420,7 +415,7 @@ public void messageConverters() { CompositeMessageConverter compositeConverter = this.appContext.getBean(CompositeMessageConverter.class); assertThat(compositeConverter).isNotNull(); - assertThat(compositeConverter.getConverters().size()).isEqualTo(5); + assertThat(compositeConverter.getConverters().size()).isEqualTo(4); assertThat(compositeConverter.getConverters().iterator().next().getClass()).isEqualTo(StringMessageConverter.class); } diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index bd0ffdcdb243..2726dce25761 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -52,6 +52,7 @@ +