Fixed: remove misplaced StringEncoder setting when dealing with protobuf (#2453)

spring-cloud-netflix-core/pom.xml

@@ -201,6 +201,18 @@
 			<artifactId>jackson-dataformat-smile</artifactId>
 			<optional>true</optional>
 		</dependency>
+		<dependency>
+			<groupId>com.googlecode.protobuf-java-format</groupId>
+			<artifactId>protobuf-java-format</artifactId>
+			<version>1.4</version>
+			<scope>test</scope>
+		</dependency>
+		<dependency>
+			<groupId>com.google.protobuf</groupId>
+			<artifactId>protobuf-java</artifactId>
+			<version>3.4.0</version>
+			<scope>test</scope>
+		</dependency>
 	</dependencies>
 	<profiles>
 		<profile>

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java

@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.lang.reflect.Type;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 
 import org.apache.commons.logging.Log;
@@ -36,12 +37,14 @@ import org.springframework.http.converter.HttpMessageConverter;
 import feign.RequestTemplate;
 import feign.codec.EncodeException;
 import feign.codec.Encoder;
+import org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter;
 
 import static org.springframework.cloud.netflix.feign.support.FeignUtils.getHeaders;
 import static org.springframework.cloud.netflix.feign.support.FeignUtils.getHttpHeaders;
 
 /**
  * @author Spencer Gibb
+ * @author ScienJus
  */
 public class SpringEncoder implements Encoder {
 
@@ -98,12 +101,17 @@ public class SpringEncoder implements Encoder {
 					// with the modified headers
 					request.headers(getHeaders(outputMessage.getHeaders()));
 
-					// do not use charset for binary data
+					// do not use charset for binary data and protobuf
+					Charset charset;
 					if (messageConverter instanceof ByteArrayHttpMessageConverter) {
-						request.body(outputMessage.getOutputStream().toByteArray(), null);
+						charset = null;
+					} else if (messageConverter instanceof ProtobufHttpMessageConverter &&
+							ProtobufHttpMessageConverter.PROTOBUF.isCompatibleWith(outputMessage.getHeaders().getContentType())) {
+						charset = null;
 					} else {
-						request.body(outputMessage.getOutputStream().toByteArray(), Charset.forName("UTF-8"));
+						charset = StandardCharsets.UTF_8;
 					}
+					request.body(outputMessage.getOutputStream().toByteArray(), charset);
 					return;
 				}
 			}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/encoding/proto/ProtobufNotInClasspathTest.java

+/*
+ * Copyright 2012-2013 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
+ *
+ *      http://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.cloud.netflix.feign.encoding.proto;
+
+import feign.RequestTemplate;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.ObjectFactory;
+import org.springframework.boot.autoconfigure.web.HttpMessageConverters;
+import org.springframework.cloud.ClassPathExclusions;
+import org.springframework.cloud.FilteredClassPathRunner;
+import org.springframework.cloud.netflix.feign.support.SpringEncoder;
+import org.springframework.http.converter.StringHttpMessageConverter;
+
+/**
+ * Test {@link SpringEncoder} when protobuf is not in classpath
+ *
+ * @author ScienJus
+ */
+@RunWith(FilteredClassPathRunner.class)
+@ClassPathExclusions("protobuf-*.jar")
+public class ProtobufNotInClasspathTest {
+
+    @Test
+    public void testEncodeWhenProtobufNotInClasspath() {
+        ObjectFactory<HttpMessageConverters> converters = new ObjectFactory<HttpMessageConverters>() {
+            @Override
+            public HttpMessageConverters getObject() throws BeansException {
+                return new HttpMessageConverters(new StringHttpMessageConverter());
+            }
+        };
+        RequestTemplate requestTemplate = new RequestTemplate();
+        requestTemplate.method("POST");
+        new SpringEncoder(converters).encode("a=b", String.class, requestTemplate);
+    }
+
+}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/encoding/proto/ProtobufSpringEncoderTest.java

+/*
+ * Copyright 2012-2013 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
+ *
+ *      http://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.cloud.netflix.feign.encoding.proto;
+
+import com.google.protobuf.InvalidProtocolBufferException;
+import feign.RequestTemplate;
+import feign.httpclient.ApacheHttpClient;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolVersion;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.message.BasicHttpResponse;
+import org.apache.http.message.BasicStatusLine;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.BDDMockito;
+import org.mockito.Matchers;
+import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.ObjectFactory;
+import org.springframework.boot.autoconfigure.web.HttpMessageConverters;
+import org.springframework.cloud.netflix.feign.support.SpringEncoder;
+import org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Test {@link SpringEncoder} with {@link ProtobufHttpMessageConverter}
+ *
+ * @author ScienJus
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ProtobufSpringEncoderTest {
+
+    @Mock
+    private HttpClient httpClient;
+
+    // a protobuf object with some content
+    private Request request = Request.newBuilder()
+            .setId(1000000)
+            .setMsg("Erlang/OTP 最初是爱立信为开发电信设备系统设计的编程语言平台，" +
+                    "电信设备(路由器、接入网关、…)典型设计是通过背板连接主控板卡与多块业务板卡的分布式系统。")
+            .build();
+
+    @Test
+    public void testProtobuf() throws IOException, URISyntaxException {
+        // protobuf convert to request by feign and ProtobufHttpMessageConverter
+        RequestTemplate requestTemplate = newRequestTemplate();
+        newEncoder().encode(request, Request.class, requestTemplate);
+        HttpEntity entity = toApacheHttpEntity(requestTemplate);
+        byte[] bytes = read(entity.getContent(), (int) entity.getContentLength());
+
+        Assert.assertArrayEquals(bytes, request.toByteArray());
+        Request copy = Request.parseFrom(bytes);
+        Assert.assertEquals(request, copy);
+    }
+
+    @Test
+    public void testProtobufWithCharsetWillFail() throws IOException, URISyntaxException {
+        // protobuf convert to request by feign and ProtobufHttpMessageConverter
+        RequestTemplate requestTemplate = newRequestTemplate();
+        newEncoder().encode(request, Request.class, requestTemplate);
+        // set a charset
+        requestTemplate.body(requestTemplate.body(), StandardCharsets.UTF_8);
+        HttpEntity entity = toApacheHttpEntity(requestTemplate);
+        byte[] bytes = read(entity.getContent(), (int) entity.getContentLength());
+
+        // http request-body is different with original protobuf body
+        Assert.assertNotEquals(bytes.length, request.toByteArray().length);
+        try {
+            Request copy = Request.parseFrom(bytes);
+            Assert.fail("Expected an InvalidProtocolBufferException to be thrown");
+        } catch (InvalidProtocolBufferException e) {
+            // success
+        }
+    }
+
+    private SpringEncoder newEncoder() {
+        ObjectFactory<HttpMessageConverters> converters = new ObjectFactory<HttpMessageConverters>() {
+            @Override
+            public HttpMessageConverters getObject() throws BeansException {
+                return new HttpMessageConverters(new ProtobufHttpMessageConverter());
+            }
+        };
+        return new SpringEncoder(converters);
+    }
+
+    private RequestTemplate newRequestTemplate() {
+        RequestTemplate requestTemplate = new RequestTemplate();
+        requestTemplate.method("POST");
+        return requestTemplate;
+    }
+
+    private HttpEntity toApacheHttpEntity(RequestTemplate requestTemplate) throws IOException, URISyntaxException {
+        final List<HttpUriRequest> request = new ArrayList<>(1);
+        BDDMockito.given(httpClient.execute(Matchers.<HttpUriRequest>any())).will(new Answer<HttpResponse>() {
+            @Override
+            public HttpResponse answer(InvocationOnMock invocationOnMock) throws Throwable {
+                request.add((HttpUriRequest) invocationOnMock.getArguments()[0]);
+                return new BasicHttpResponse(new BasicStatusLine(new ProtocolVersion("http", 1, 1), 200, null));
+            }
+        });
+        new ApacheHttpClient(httpClient).execute(requestTemplate.request(), new feign.Request.Options());
+        HttpUriRequest httpUriRequest = request.get(0);
+        return  ((HttpEntityEnclosingRequestBase)httpUriRequest).getEntity();
+    }
+
+    private byte[] read(InputStream in, int length) throws IOException {
+        byte[] bytes = new byte[length];
+        in.read(bytes);
+        return bytes;
+    }
+
+}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/encoding/proto/ProtobufTest.java

+/*
+ * Copyright 2012-2013 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
+ *
+ *      http://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.
+ */
+
+// Generated by the protocol buffer compiler.  DO NOT EDIT!
+// source: protobuf_test.proto
+
+package org.springframework.cloud.netflix.feign.encoding.proto;
+
+public final class ProtobufTest {
+  private ProtobufTest() {}
+  public static void registerAllExtensions(
+      com.google.protobuf.ExtensionRegistryLite registry) {
+  }
+
+  public static void registerAllExtensions(
+      com.google.protobuf.ExtensionRegistry registry) {
+    registerAllExtensions(
+        (com.google.protobuf.ExtensionRegistryLite) registry);
+  }
+  static final com.google.protobuf.Descriptors.Descriptor
+    internal_static_Request_descriptor;
+  static final 
+    com.google.protobuf.GeneratedMessageV3.FieldAccessorTable
+      internal_static_Request_fieldAccessorTable;
+
+  public static com.google.protobuf.Descriptors.FileDescriptor
+      getDescriptor() {
+    return descriptor;
+  }
+  private static  com.google.protobuf.Descriptors.FileDescriptor
+      descriptor;
+  static {
+    String[] descriptorData = {
+      "\n\023protobuf_test.proto\"\"\n\007Request\022\n\n\002id\030\001" +
+      " \001(\005\022\013\n\003msg\030\002 \001(\tB\024\n\020feign.httpclientP\001b" +
+      "\006proto3"
+    };
+    com.google.protobuf.Descriptors.FileDescriptor.InternalDescriptorAssigner assigner =
+        new com.google.protobuf.Descriptors.FileDescriptor.    InternalDescriptorAssigner() {
+          public com.google.protobuf.ExtensionRegistry assignDescriptors(
+              com.google.protobuf.Descriptors.FileDescriptor root) {
+            descriptor = root;
+            return null;
+          }
+        };
+    com.google.protobuf.Descriptors.FileDescriptor
+      .internalBuildGeneratedFileFrom(descriptorData,
+        new com.google.protobuf.Descriptors.FileDescriptor[] {
+        }, assigner);
+    internal_static_Request_descriptor =
+      getDescriptor().getMessageTypes().get(0);
+    internal_static_Request_fieldAccessorTable = new
+      com.google.protobuf.GeneratedMessageV3.FieldAccessorTable(
+        internal_static_Request_descriptor,
+        new String[] { "Id", "Msg", });
+  }
+
+  // @@protoc_insertion_point(outer_class_scope)
+}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/encoding/proto/RequestOrBuilder.java

+/*
+ * Copyright 2012-2013 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
+ *
+ *      http://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.
+ */
+
+// Generated by the protocol buffer compiler.  DO NOT EDIT!
+// source: protobuf_test.proto
+
+package org.springframework.cloud.netflix.feign.encoding.proto;
+
+public interface RequestOrBuilder extends
+    // @@protoc_insertion_point(interface_extends:Request)
+    com.google.protobuf.MessageOrBuilder {
+
+  /**
+   * <code>int32 id = 1;</code>
+   */
+  int getId();
+
+  /**
+   * <code>string msg = 2;</code>
+   */
+  String getMsg();
+  /**
+   * <code>string msg = 2;</code>
+   */
+  com.google.protobuf.ByteString
+      getMsgBytes();
+}