You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2018/08/14 08:30:10 UTC

[incubator-servicecomb-java-chassis] 03/03: [SCB-827] change DefaultHttpClientFilter to return response instead of throwing exception

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-servicecomb-java-chassis.git

commit 7aa41f45f96aeaf79608de922c96cc118329b73f
Author: yaohaishi <ya...@huawei.com>
AuthorDate: Tue Aug 14 13:42:57 2018 +0800

    [SCB-827] change DefaultHttpClientFilter to return response instead of throwing exception
---
 .../demo/springmvc/client/TestResponse.java        |  8 ++--
 .../rest/client/http/DefaultHttpClientFilter.java  | 20 ++++----
 .../client/http/TestDefaultHttpClientFilter.java   | 55 +++++++++++++---------
 3 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
index 844b562..0cd6028 100644
--- a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
+++ b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestResponse.java
@@ -151,14 +151,16 @@ public class TestResponse {
     // 3. InvocationException: decoder wrapping exception
     wrappedException = wrappedException.getCause();
     TestMgr.check(InvocationException.class, wrappedException.getClass());
+    TestMgr.check("InvocationException: code=490;msg=CommonExceptionData [message=Cse Internal Bad Request]",
+        wrappedException.getMessage());
     // 4. InvalidFormatException: decode exception
-    Object errorData = ((InvocationException) wrappedException).getErrorData();
-    TestMgr.check(InvalidFormatException.class, errorData.getClass());
+    Object cause = wrappedException.getCause();
+    TestMgr.check(InvalidFormatException.class, cause.getClass());
     TestMgr.check(
         "Cannot deserialize value of type `java.util.Date` from String \"returnOK\": not a valid representation "
             + "(error: Failed to parse Date value 'returnOK': Failed to parse date \"returnOK\": Invalid number: retu)\n"
             + " at [Source: (org.apache.servicecomb.foundation.vertx.stream.BufferInputStream); line: 1, column: 12] "
             + "(through reference chain: org.apache.servicecomb.demo.springmvc.decoderesponse.DecodeTestResponse[\"content\"])",
-        ((InvalidFormatException) errorData).getMessage());
+        ((InvalidFormatException) cause).getMessage());
   }
 }
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
index 659ba2a..7d54078 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/DefaultHttpClientFilter.java
@@ -29,9 +29,8 @@ import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
+import org.apache.servicecomb.swagger.invocation.InvocationType;
 import org.apache.servicecomb.swagger.invocation.Response;
-import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
-import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -64,10 +63,10 @@ public class DefaultHttpClientFilter implements HttpClientFilter {
     return restOperation.findProduceProcessor(contentTypeForFind);
   }
 
-  protected Object extractResponse(Invocation invocation, HttpServletResponseEx responseEx) {
+  protected Response extractResponse(Invocation invocation, HttpServletResponseEx responseEx) {
     Object result = invocation.getHandlerContext().get(RestConst.READ_STREAM_PART);
     if (result != null) {
-      return result;
+      return Response.create(responseEx.getStatusType(), result);
     }
 
     OperationMeta operationMeta = invocation.getOperationMeta();
@@ -83,23 +82,22 @@ public class DefaultHttpClientFilter implements HttpClientFilter {
               responseEx.getStatusType().getReasonPhrase(),
               responseEx.getHeader(HttpHeaders.CONTENT_TYPE));
       LOGGER.error(msg);
-      return ExceptionFactory.createConsumerException(new InvocationException(responseEx.getStatus(), responseEx.getStatusType().getReasonPhrase(), msg));
+      return Response.createFail(InvocationType.CONSUMER, msg);
     }
 
     try {
-      return produceProcessor.decodeResponse(responseEx.getBodyBuffer(), responseMeta.getJavaType());
+      result = produceProcessor.decodeResponse(responseEx.getBodyBuffer(), responseMeta.getJavaType());
+      return Response.create(responseEx.getStatusType(), result);
     } catch (Exception e) {
-      LOGGER.error("failed to decode response body", e);
-      throw ExceptionFactory.createConsumerException(e);
+      LOGGER.error("failed to decode response body, exception type is [{}]", e.getClass());
+      return Response.createConsumerFail(e);
     }
   }
 
   @Override
   public Response afterReceiveResponse(Invocation invocation, HttpServletResponseEx responseEx) {
-    Object result = extractResponse(invocation, responseEx);
+    Response response = extractResponse(invocation, responseEx);
 
-    Response response =
-        Response.create(responseEx.getStatusType(), result);
     for (String headerName : responseEx.getHeaderNames()) {
       Collection<String> headerValues = responseEx.getHeaders(headerName);
       for (String headerValue : headerValues) {
diff --git a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
index 3088b40..3279ab3 100644
--- a/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
+++ b/transports/transport-rest/transport-rest-client/src/test/java/org/apache/servicecomb/transport/rest/client/http/TestDefaultHttpClientFilter.java
@@ -17,8 +17,6 @@
 
 package org.apache.servicecomb.transport.rest.client.http;
 
-import static org.junit.Assert.fail;
-
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
@@ -36,12 +34,13 @@ import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
 import org.apache.servicecomb.foundation.vertx.http.ReadStreamPart;
 import org.apache.servicecomb.swagger.invocation.Response;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
 import org.junit.Assert;
 import org.junit.Test;
 
-import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.type.SimpleType;
 
 import io.vertx.core.MultiMap;
@@ -105,17 +104,22 @@ public class TestDefaultHttpClientFilter {
   }
 
   @Test
-  public void extractResult_readStreamPart(@Mocked Invocation invocation, @Mocked ReadStreamPart part) {
+  public void extractResult_readStreamPart(@Mocked Invocation invocation, @Mocked ReadStreamPart part,
+      @Mocked HttpServletResponseEx responseEx) {
     Map<String, Object> handlerContext = new HashMap<>();
     handlerContext.put(RestConst.READ_STREAM_PART, part);
     new Expectations() {
       {
         invocation.getHandlerContext();
         result = handlerContext;
+        responseEx.getStatusType();
+        result = Status.OK;
       }
     };
 
-    Assert.assertSame(part, filter.extractResponse(invocation, null));
+    Response response = filter.extractResponse(invocation, responseEx);
+    Assert.assertSame(part, response.getResult());
+    Assert.assertEquals(Status.OK, response.getStatus());
   }
 
   @Test
@@ -148,17 +152,20 @@ public class TestDefaultHttpClientFilter {
         return new ProduceJsonProcessor();
       }
     };
-    try {
-      filter.extractResponse(invocation, responseEx);
-      fail("an exception is expected!");
-    } catch (Exception e) {
-      Assert.assertEquals(InvocationException.class, e.getClass());
-      Assert.assertEquals(JsonParseException.class, ((InvocationException) e).getErrorData().getClass());
-      JsonParseException jsonParseException = (JsonParseException) ((InvocationException) e).getErrorData();
-      Assert.assertEquals("Unrecognized token 'abc': was expecting ('true', 'false' or 'null')\n"
-              + " at [Source: (org.apache.servicecomb.foundation.vertx.stream.BufferInputStream); line: 1, column: 7]",
-          jsonParseException.getMessage());
-    }
+
+    Response response = filter.extractResponse(invocation, responseEx);
+    Assert.assertEquals(490, response.getStatusCode());
+    Assert.assertEquals(ExceptionFactory.CONSUMER_INNER_REASON_PHRASE, response.getReasonPhrase());
+    Assert.assertEquals(InvocationException.class, response.<InvocationException>getResult().getClass());
+    InvocationException invocationException = response.getResult();
+    Assert.assertEquals("InvocationException: code=490;msg=CommonExceptionData [message=Cse Internal Bad Request]",
+        invocationException.getMessage());
+    Assert.assertEquals("Unrecognized token 'abc': was expecting ('true', 'false' or 'null')\n"
+            + " at [Source: (org.apache.servicecomb.foundation.vertx.stream.BufferInputStream); line: 1, column: 7]",
+        invocationException.getCause().getMessage());
+    Assert.assertEquals(CommonExceptionData.class, invocationException.getErrorData().getClass());
+    CommonExceptionData commonExceptionData = (CommonExceptionData) invocationException.getErrorData();
+    Assert.assertEquals("Cse Internal Bad Request", commonExceptionData.getMessage());
   }
 
   @Test
@@ -173,18 +180,22 @@ public class TestDefaultHttpClientFilter {
         operationMeta.getExtData(RestConst.SWAGGER_REST_OPERATION);
         result = swaggerRestOperation;
         responseEx.getStatus();
-        result = 401;
+        result = 200;
       }
     };
 
     Response response = filter.afterReceiveResponse(invocation, responseEx);
-    InvocationException exception = response.getResult();
+    Assert.assertEquals(490, response.getStatusCode());
+    Assert.assertEquals(ExceptionFactory.CONSUMER_INNER_REASON_PHRASE, response.getReasonPhrase());
+    Assert.assertEquals(InvocationException.class, response.<InvocationException>getResult().getClass());
+    InvocationException invocationException = response.getResult();
     Assert.assertEquals(
-        401,
-        exception.getStatusCode());
+        490,
+        invocationException.getStatusCode());
     Assert.assertEquals(
-        "method null, path null, statusCode 401, reasonPhrase null, response content-type null is not supported",
-        exception.getErrorData());
+        "CommonExceptionData [message=method null, path null, statusCode 200, reasonPhrase null, "
+            + "response content-type null is not supported]",
+        invocationException.getErrorData().toString());
   }
 
   @Test