Skip to content

Commit 1d39c97

Browse files
committed
Downgrade to Protobuf Java Format 1.2
Issue: SPR-14171
1 parent fd954ef commit 1d39c97

File tree

3 files changed

+77
-72
lines changed

3 files changed

+77
-72
lines changed

build.gradle

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ configure(allprojects) { project ->
6262
ext.okhttpVersion = "2.7.5"
6363
ext.openjpaVersion = "2.4.1"
6464
ext.poiVersion = "3.13"
65-
ext.protobufVersion = "2.6.1"
6665
ext.reactorVersion = "2.0.7.RELEASE"
6766
ext.romeVersion = "1.5.1"
6867
ext.seleniumVersion = "2.48.2"
@@ -716,8 +715,8 @@ project("spring-web") {
716715
exclude group: "javax.servlet", module: "javax.servlet-api"
717716
}
718717
optional("log4j:log4j:1.2.17")
719-
optional("com.googlecode.protobuf-java-format:protobuf-java-format:1.4")
720-
optional("com.google.protobuf:protobuf-java:${protobufVersion}")
718+
optional("com.google.protobuf:protobuf-java:2.6.1")
719+
optional("com.googlecode.protobuf-java-format:protobuf-java-format:1.2")
721720
optional("javax.mail:javax.mail-api:${javamailVersion}")
722721
testCompile(project(":spring-context-support")) // for JafMediaTypeFactory
723722
testCompile("xmlunit:xmlunit:${xmlunitVersion}")
Lines changed: 63 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,7 +30,6 @@
3030
import com.googlecode.protobuf.format.JsonFormat;
3131
import com.googlecode.protobuf.format.XmlFormat;
3232

33-
import org.springframework.http.HttpHeaders;
3433
import org.springframework.http.HttpInputMessage;
3534
import org.springframework.http.HttpOutputMessage;
3635
import org.springframework.http.MediaType;
@@ -41,20 +40,20 @@
4140

4241

4342
/**
44-
* An {@code HttpMessageConverter} that can read and write Protobuf
45-
* {@link com.google.protobuf.Message} using
46-
* <a href="https://developers.google.com/protocol-buffers/">Google Protocol buffers</a>.
43+
* An {@code HttpMessageConverter} that reads and writes {@link com.google.protobuf.Message}s
44+
* using <a href="https://developers.google.com/protocol-buffers/">Google Protocol Buffers</a>.
4745
*
48-
* <p>By default it supports {@code "application/json"}, {@code "application/xml"},
49-
* {@code "text/plain"} and {@code "application/x-protobuf"} while writing also
50-
* supports {@code "text/html"}
46+
* <p>By default, it supports {@code "application/x-protobuf"}, {@code "text/plain"},
47+
* {@code "application/json"}, {@code "application/xml"}, while also writing {@code "text/html"}.
5148
*
52-
* <p>To generate Message Java classes you need to install the protoc binary.
49+
* <p>To generate {@code Message} Java classes, you need to install the {@code protoc} binary.
5350
*
54-
* <p>Tested against Protobuf version 2.5.0.
51+
* <p>Requires Protobuf 2.5/2.6 and Protobuf Java Format 1.2.
52+
* (Note: Does not work with later Protobuf Java Format versions in Spring 4.2 yet.)
5553
*
5654
* @author Alex Antonov
5755
* @author Brian Clozel
56+
* @author Juergen Hoeller
5857
* @since 4.1
5958
*/
6059
public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<Message> {
@@ -67,10 +66,10 @@ public class ProtobufHttpMessageConverter extends AbstractHttpMessageConverter<M
6766

6867
public static final String X_PROTOBUF_MESSAGE_HEADER = "X-Protobuf-Message";
6968

70-
private static final ConcurrentHashMap<Class<?>, Method> methodCache = new ConcurrentHashMap<Class<?>, Method>();
7169

70+
private static final ConcurrentHashMap<Class<?>, Method> methodCache = new ConcurrentHashMap<Class<?>, Method>();
7271

73-
private ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
72+
private final ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
7473

7574

7675
/**
@@ -85,7 +84,7 @@ public ProtobufHttpMessageConverter() {
8584
* that allows the registration of message extensions.
8685
*/
8786
public ProtobufHttpMessageConverter(ExtensionRegistryInitializer registryInitializer) {
88-
super(PROTOBUF, MediaType.TEXT_PLAIN, MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON);
87+
super(PROTOBUF, MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML);
8988
if (registryInitializer != null) {
9089
registryInitializer.initializeExtensionRegistry(this.extensionRegistry);
9190
}
@@ -97,56 +96,46 @@ protected boolean supports(Class<?> clazz) {
9796
return Message.class.isAssignableFrom(clazz);
9897
}
9998

99+
@Override
100+
protected MediaType getDefaultContentType(Message message) {
101+
return PROTOBUF;
102+
}
103+
100104
@Override
101105
protected Message readInternal(Class<? extends Message> clazz, HttpInputMessage inputMessage)
102106
throws IOException, HttpMessageNotReadableException {
103107

104108
MediaType contentType = inputMessage.getHeaders().getContentType();
105-
contentType = (contentType != null ? contentType : PROTOBUF);
106-
107-
Charset charset = getCharset(inputMessage.getHeaders());
108-
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
109+
if (contentType == null) {
110+
contentType = PROTOBUF;
111+
}
112+
Charset charset = contentType.getCharSet();
113+
if (charset == null) {
114+
charset = DEFAULT_CHARSET;
115+
}
109116

110117
try {
111118
Message.Builder builder = getMessageBuilder(clazz);
112-
113-
if (MediaType.APPLICATION_JSON.isCompatibleWith(contentType)) {
114-
JsonFormat.merge(reader, this.extensionRegistry, builder);
115-
}
116-
else if (MediaType.TEXT_PLAIN.isCompatibleWith(contentType)) {
119+
if (MediaType.TEXT_PLAIN.isCompatibleWith(contentType)) {
120+
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
117121
TextFormat.merge(reader, this.extensionRegistry, builder);
118122
}
123+
else if (MediaType.APPLICATION_JSON.isCompatibleWith(contentType)) {
124+
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
125+
JsonFormat.merge(reader, this.extensionRegistry, builder);
126+
}
119127
else if (MediaType.APPLICATION_XML.isCompatibleWith(contentType)) {
128+
InputStreamReader reader = new InputStreamReader(inputMessage.getBody(), charset);
120129
XmlFormat.merge(reader, this.extensionRegistry, builder);
121130
}
122131
else {
123132
builder.mergeFrom(inputMessage.getBody(), this.extensionRegistry);
124133
}
125134
return builder.build();
126135
}
127-
catch (Exception e) {
128-
throw new HttpMessageNotReadableException("Could not read Protobuf message: " + e.getMessage(), e);
129-
}
130-
}
131-
132-
private Charset getCharset(HttpHeaders headers) {
133-
if (headers == null || headers.getContentType() == null || headers.getContentType().getCharSet() == null) {
134-
return DEFAULT_CHARSET;
135-
}
136-
return headers.getContentType().getCharSet();
137-
}
138-
139-
/**
140-
* Create a new {@code Message.Builder} instance for the given class.
141-
* <p>This method uses a ConcurrentHashMap for caching method lookups.
142-
*/
143-
private Message.Builder getMessageBuilder(Class<? extends Message> clazz) throws Exception {
144-
Method method = methodCache.get(clazz);
145-
if (method == null) {
146-
method = clazz.getMethod("newBuilder");
147-
methodCache.put(clazz, method);
136+
catch (Exception ex) {
137+
throw new HttpMessageNotReadableException("Could not read Protobuf message: " + ex.getMessage(), ex);
148138
}
149-
return (Message.Builder) method.invoke(clazz);
150139
}
151140

152141
/**
@@ -155,46 +144,48 @@ private Message.Builder getMessageBuilder(Class<? extends Message> clazz) throws
155144
*/
156145
@Override
157146
protected boolean canWrite(MediaType mediaType) {
158-
return super.canWrite(mediaType) || MediaType.TEXT_HTML.isCompatibleWith(mediaType);
147+
return (super.canWrite(mediaType) || MediaType.TEXT_HTML.isCompatibleWith(mediaType));
159148
}
160149

161150
@Override
162151
protected void writeInternal(Message message, HttpOutputMessage outputMessage)
163152
throws IOException, HttpMessageNotWritableException {
164153

165154
MediaType contentType = outputMessage.getHeaders().getContentType();
166-
Charset charset = getCharset(contentType);
155+
if (contentType == null) {
156+
contentType = getDefaultContentType(message);
157+
}
158+
Charset charset = contentType.getCharSet();
159+
if (charset == null) {
160+
charset = DEFAULT_CHARSET;
161+
}
167162

168-
if (MediaType.TEXT_HTML.isCompatibleWith(contentType)) {
169-
final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
170-
HtmlFormat.print(message, outputStreamWriter);
163+
if (MediaType.TEXT_PLAIN.isCompatibleWith(contentType)) {
164+
OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
165+
TextFormat.print(message, outputStreamWriter);
171166
outputStreamWriter.flush();
172167
}
173168
else if (MediaType.APPLICATION_JSON.isCompatibleWith(contentType)) {
174-
final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
169+
OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
175170
JsonFormat.print(message, outputStreamWriter);
176171
outputStreamWriter.flush();
177172
}
178-
else if (MediaType.TEXT_PLAIN.isCompatibleWith(contentType)) {
179-
final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
180-
TextFormat.print(message, outputStreamWriter);
181-
outputStreamWriter.flush();
182-
}
183173
else if (MediaType.APPLICATION_XML.isCompatibleWith(contentType)) {
184-
final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
174+
OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
185175
XmlFormat.print(message, outputStreamWriter);
186176
outputStreamWriter.flush();
187177
}
178+
else if (MediaType.TEXT_HTML.isCompatibleWith(contentType)) {
179+
OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputMessage.getBody(), charset);
180+
HtmlFormat.print(message, outputStreamWriter);
181+
outputStreamWriter.flush();
182+
}
188183
else if (PROTOBUF.isCompatibleWith(contentType)) {
189184
setProtoHeader(outputMessage, message);
190185
FileCopyUtils.copy(message.toByteArray(), outputMessage.getBody());
191186
}
192187
}
193188

194-
private Charset getCharset(MediaType contentType) {
195-
return contentType.getCharSet() != null ? contentType.getCharSet() : DEFAULT_CHARSET;
196-
}
197-
198189
/**
199190
* Set the "X-Protobuf-*" HTTP headers when responding with a message of
200191
* content type "application/x-protobuf"
@@ -206,9 +197,18 @@ private void setProtoHeader(HttpOutputMessage response, Message message) {
206197
response.getHeaders().set(X_PROTOBUF_MESSAGE_HEADER, message.getDescriptorForType().getFullName());
207198
}
208199

209-
@Override
210-
protected MediaType getDefaultContentType(Message message) {
211-
return PROTOBUF;
200+
201+
/**
202+
* Create a new {@code Message.Builder} instance for the given class.
203+
* <p>This method uses a ConcurrentHashMap for caching method lookups.
204+
*/
205+
private static Message.Builder getMessageBuilder(Class<? extends Message> clazz) throws Exception {
206+
Method method = methodCache.get(clazz);
207+
if (method == null) {
208+
method = clazz.getMethod("newBuilder");
209+
methodCache.put(clazz, method);
210+
}
211+
return (Message.Builder) method.invoke(clazz);
212212
}
213213

214214
}

spring-web/src/test/java/org/springframework/http/converter/protobuf/ProtobufHttpMessageConverterTests.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,9 +31,9 @@
3131
import static org.junit.Assert.*;
3232
import static org.mockito.Mockito.*;
3333

34-
3534
/**
36-
* Test suite for {@link ProtobufHttpMessageConverter}
35+
* Test suite for {@link ProtobufHttpMessageConverter}.
36+
*
3737
* @author Alex Antonov
3838
*/
3939
public class ProtobufHttpMessageConverterTests {
@@ -52,6 +52,7 @@ public void setUp() {
5252
this.testMsg = Msg.newBuilder().setFoo("Foo").setBlah(SecondMsg.newBuilder().setBlah(123).build()).build();
5353
}
5454

55+
5556
@Test
5657
public void extensionRegistryInitialized() {
5758
verify(this.registryInitializer, times(1)).initializeExtensionRegistry(anyObject());
@@ -61,7 +62,8 @@ public void extensionRegistryInitialized() {
6162
public void extensionRegistryNull() {
6263
try {
6364
new ProtobufHttpMessageConverter(null);
64-
} catch (Exception e) {
65+
}
66+
catch (Exception ex) {
6567
fail("Unable to create ProtobufHttpMessageConverter with null extensionRegistry");
6668
}
6769
}
@@ -73,6 +75,7 @@ public void canRead() {
7375
assertTrue(this.converter.canRead(Msg.class, MediaType.APPLICATION_JSON));
7476
assertTrue(this.converter.canRead(Msg.class, MediaType.APPLICATION_XML));
7577
assertTrue(this.converter.canRead(Msg.class, MediaType.TEXT_PLAIN));
78+
7679
// only supported as an output format
7780
assertFalse(this.converter.canRead(Msg.class, MediaType.TEXT_HTML));
7881
}
@@ -114,9 +117,11 @@ public void write() throws IOException {
114117
Message result = Msg.parseFrom(outputMessage.getBodyAsBytes());
115118
assertEquals(this.testMsg, result);
116119

117-
String messageHeader = outputMessage.getHeaders().getFirst(ProtobufHttpMessageConverter.X_PROTOBUF_MESSAGE_HEADER);
120+
String messageHeader =
121+
outputMessage.getHeaders().getFirst(ProtobufHttpMessageConverter.X_PROTOBUF_MESSAGE_HEADER);
118122
assertEquals("Msg", messageHeader);
119-
String schemaHeader = outputMessage.getHeaders().getFirst(ProtobufHttpMessageConverter.X_PROTOBUF_SCHEMA_HEADER);
123+
String schemaHeader =
124+
outputMessage.getHeaders().getFirst(ProtobufHttpMessageConverter.X_PROTOBUF_SCHEMA_HEADER);
120125
assertEquals("sample.proto", schemaHeader);
121126
}
122127

@@ -132,4 +137,5 @@ public void getContentLength() throws Exception {
132137
this.converter.write(this.testMsg, contentType, outputMessage);
133138
assertEquals(-1, outputMessage.getHeaders().getContentLength());
134139
}
140+
135141
}

0 commit comments

Comments
 (0)