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
The following commit(s) were added to refs/heads/master by this push:
new ea17cca [SCB-861]490,590 response not properly used
ea17cca is described below
commit ea17cca3e2e35a2f2b779533189f242b0b1f9324
Author: liubao <[email protected]>
AuthorDate: Wed Aug 22 11:13:17 2018 +0800
[SCB-861]490,590 response not properly used
---
.../client/CodeFirstRestTemplateSpringmvc.java | 5 ++
.../demo/springmvc/client/SpringmvcClient.java | 15 ++++--
.../demo/springmvc/client/TestResponse.java | 10 +---
.../demo/springmvc/server/CodeFirstSpringmvc.java | 22 ++++----
.../src/main/resources/microservice.yaml | 4 +-
.../swagger/invocation/response/ResponsesMeta.java | 7 +--
.../rest/client/http/DefaultHttpClientFilter.java | 21 +++++++-
.../client/http/TestDefaultHttpClientFilter.java | 59 ++++++++++++++++++++--
8 files changed, 108 insertions(+), 35 deletions(-)
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
index af27ccd..e34b340 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
@@ -173,6 +173,7 @@ public class CodeFirstRestTemplateSpringmvc extends
CodeFirstRestTemplate {
}
private void testFallback(RestTemplate template, String cseUrlPrefix) {
+ long start = System.currentTimeMillis();
String result = template.getForObject(cseUrlPrefix +
"/fallback/returnnull/hello", String.class);
TestMgr.check(result, "hello");
result = template.getForObject(cseUrlPrefix +
"/fallback/returnnull/throwexception", String.class);
@@ -199,6 +200,10 @@ public class CodeFirstRestTemplateSpringmvc extends
CodeFirstRestTemplate {
result = template.getForObject(cseUrlPrefix + "/fallback/force/hello",
String.class);
TestMgr.check(result, "mockedreslut");
+
+ // This test case is fallback testing and will return null if failed.
+ // In order to check if failed due to some unexpected timeout exception,
check the time.
+ TestMgr.check(System.currentTimeMillis() - start < 10000, true);
}
private void testResponseEntity(String microserviceName, RestTemplate
template, String cseUrlPrefix) {
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
index 37aa870..3e5aa1c 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
@@ -54,12 +54,19 @@ public class SpringmvcClient {
private static Controller controller;
public static void main(String[] args) throws Exception {
- Log4jUtils.init();
- BeanUtils.init();
+ try {
+ Log4jUtils.init();
+ BeanUtils.init();
- run();
+ run();
- TestMgr.summary();
+ TestMgr.summary();
+ } catch (Throwable e) {
+ TestMgr.check("success", "failed");
+ System.err.println("-------------- test failed -------------");
+ e.printStackTrace();
+ System.err.println("-------------- test failed -------------");
+ }
}
public static void run() {
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 0cd6028..95db734 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
@@ -146,15 +146,7 @@ public class TestResponse {
}
Objects.requireNonNull(exception);
// 2. CseException: bizKeeper exception
- Throwable wrappedException = exception.getCause();
- TestMgr.check(CseException.class, wrappedException.getClass());
- // 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 cause = wrappedException.getCause();
+ Throwable cause = exception.getCause();
TestMgr.check(InvalidFormatException.class, cause.getClass());
TestMgr.check(
"Cannot deserialize value of type `java.util.Date` from String
\"returnOK\": not a valid representation "
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
index 6ec2bc5..0ab7487 100644
---
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
+++
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
@@ -22,7 +22,6 @@ import java.io.InputStream;
import java.util.Date;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.http.HttpServletRequest;
@@ -51,6 +50,7 @@ import
org.apache.servicecomb.swagger.extend.annotations.ResponseHeaders;
import org.apache.servicecomb.swagger.invocation.Response;
import org.apache.servicecomb.swagger.invocation.context.ContextUtils;
import org.apache.servicecomb.swagger.invocation.context.InvocationContext;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.apache.servicecomb.swagger.invocation.response.Headers;
import org.slf4j.Logger;
@@ -270,42 +270,44 @@ public class CodeFirstSpringmvc {
return result;
}
+ // Using 490, 590 error code, the response type should be
CommonExceptionData. Or we need
+ // complex ExceptionConverters to deal with exceptions thrown by Hanlders,
etc.
@RequestMapping(path = "/fallback/returnnull/{name}", method =
RequestMethod.GET)
@ApiResponses(value = {@ApiResponse(code = 200, response = String.class,
message = "xxx"),
- @ApiResponse(code = 490, response = String.class, message = "xxx")})
+ @ApiResponse(code = 490, response = CommonExceptionData.class, message =
"xxx")})
public String fallbackReturnNull(@PathVariable(name = "name") String name) {
if ("throwexception".equals(name)) {
- throw new InvocationException(490, "490", "xxx");
+ throw new InvocationException(490, "490", new
CommonExceptionData("xxx"));
}
return name;
}
@RequestMapping(path = "/fallback/throwexception/{name}", method =
RequestMethod.GET)
@ApiResponses(value = {@ApiResponse(code = 200, response = String.class,
message = "xxx"),
- @ApiResponse(code = 490, response = String.class, message = "xxx")})
+ @ApiResponse(code = 490, response = CommonExceptionData.class, message =
"xxx")})
public String fallbackThrowException(@PathVariable(name = "name") String
name) {
if ("throwexception".equals(name)) {
- throw new InvocationException(490, "490", "xxx");
+ throw new InvocationException(490, "490", new
CommonExceptionData("xxx"));
}
return name;
}
@RequestMapping(path = "/fallback/fromcache/{name}", method =
RequestMethod.GET)
@ApiResponses(value = {@ApiResponse(code = 200, response = String.class,
message = "xxx"),
- @ApiResponse(code = 490, response = String.class, message = "xxx")})
+ @ApiResponse(code = 490, response = CommonExceptionData.class, message =
"xxx")})
public String fallbackFromCache(@PathVariable(name = "name") String name) {
if ("throwexception".equals(name)) {
- throw new InvocationException(490, "490", "xxx");
+ throw new InvocationException(490, "490", new
CommonExceptionData("xxx"));
}
return name;
}
@RequestMapping(path = "/fallback/force/{name}", method = RequestMethod.GET)
@ApiResponses(value = {@ApiResponse(code = 200, response = String.class,
message = "xxx"),
- @ApiResponse(code = 490, response = String.class, message = "xxx")})
+ @ApiResponse(code = 490, response = CommonExceptionData.class, message =
"xxx")})
public String fallbackForce(@PathVariable(name = "name") String name) {
if ("throwexception".equals(name)) {
- throw new InvocationException(490, "490", "xxx");
+ throw new InvocationException(490, "490", new
CommonExceptionData("xxx"));
}
return name;
}
@@ -317,7 +319,7 @@ public class CodeFirstSpringmvc {
@RequestMapping(path = "/testenum/{name}", method = RequestMethod.GET)
@ApiResponses(value = {@ApiResponse(code = 200, response = String.class,
message = "200 normal"),
- @ApiResponse(code = 490, response = String.class, message = "490
exception")})
+ @ApiResponse(code = 490, response = CommonExceptionData.class, message =
"490 exception")})
public String testEnum(@RequestParam(name = "username") String username,
@PathVariable(value = "name") NameType nameType) {
return nameType.toString();
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
index 9c047c8..fba7554 100644
--- a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
+++ b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
@@ -54,11 +54,11 @@ servicecomb:
uploads:
directory: target
rest:
- address: 0.0.0.0:8080?sslEnabled=true
+ address: 0.0.0.0:8082?sslEnabled=true
server:
compression: true
highway:
- address: 0.0.0.0:7070?sslEnabled=true
+ address: 0.0.0.0:7072?sslEnabled=true
handler:
chain:
Provider:
diff --git
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponsesMeta.java
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponsesMeta.java
index 856e3f0..881cb81 100644
---
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponsesMeta.java
+++
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponsesMeta.java
@@ -53,7 +53,6 @@ public class ResponsesMeta {
// 如果不传return类型进来，完全以swagger为标准，会导致生成的class不等于return
public void init(SwaggerToClassGenerator swaggerToClassGenerator, Operation
operation, Type returnType) {
initSuccessResponse(returnType);
- initInternalErrorResponse();
initGlobalDefaultMapper();
for (Entry<String, Response> entry : operation.getResponses().entrySet()) {
@@ -68,6 +67,8 @@ public class ResponsesMeta {
responseMeta.init(swaggerToClassGenerator, entry.getValue());
}
+ initInternalErrorResponse();
+
if (defaultResponse == null) {
// swagger中没有定义default，加上default专用于处理exception
ResponseMeta responseMeta = new ResponseMeta();
@@ -86,8 +87,8 @@ public class ResponsesMeta {
protected void initInternalErrorResponse() {
ResponseMeta internalErrorResponse = new ResponseMeta();
internalErrorResponse.setJavaType(COMMON_EXCEPTION_JAVA_TYPE);
- responseMap.put(ExceptionFactory.CONSUMER_INNER_STATUS_CODE,
internalErrorResponse);
- responseMap.put(ExceptionFactory.PRODUCER_INNER_STATUS_CODE,
internalErrorResponse);
+ responseMap.putIfAbsent(ExceptionFactory.CONSUMER_INNER_STATUS_CODE,
internalErrorResponse);
+ responseMap.putIfAbsent(ExceptionFactory.PRODUCER_INNER_STATUS_CODE,
internalErrorResponse);
}
protected void initGlobalDefaultMapper() {
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 60921c5..806a9c9 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
@@ -31,6 +31,9 @@ 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.Response;
+import org.apache.servicecomb.swagger.invocation.context.HttpStatus;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.apache.servicecomb.swagger.invocation.response.ResponseMeta;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -91,8 +94,22 @@ public class DefaultHttpClientFilter implements
HttpClientFilter {
result = produceProcessor.decodeResponse(responseEx.getBodyBuffer(),
responseMeta.getJavaType());
return Response.create(responseEx.getStatusType(), result);
} catch (Exception e) {
- LOGGER.error("failed to decode response body, exception type is [{}]",
e.getClass());
- return Response.createConsumerFail(e);
+ LOGGER.error("failed to decode response body, exception is [{}]",
e.getMessage());
+ String msg =
+ String.format("method %s, path %s, statusCode %d, reasonPhrase %s,
response content-type %s is not supported",
+ swaggerRestOperation.getHttpMethod(),
+ swaggerRestOperation.getAbsolutePath(),
+ responseEx.getStatus(),
+ responseEx.getStatusType().getReasonPhrase(),
+ responseEx.getHeader(HttpHeaders.CONTENT_TYPE));
+ if (HttpStatus.isSuccess(responseEx.getStatus())) {
+ return Response.createConsumerFail(
+ new InvocationException(400,
responseEx.getStatusType().getReasonPhrase(),
+ new CommonExceptionData(msg), e));
+ }
+ return Response.createConsumerFail(
+ new InvocationException(responseEx.getStatus(),
responseEx.getStatusType().getReasonPhrase(),
+ new CommonExceptionData(msg), e));
}
}
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 d14bc1d..9c0028b 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
@@ -36,7 +36,6 @@ 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;
@@ -136,6 +135,54 @@ public class TestDefaultHttpClientFilter {
result = handlerContext;
invocation.getOperationMeta();
result = operationMeta;
+ operationMeta.findResponseMeta(400);
+ result = responseMeta;
+ operationMeta.getExtData(RestConst.SWAGGER_REST_OPERATION);
+ result = swaggerRestOperation;
+ responseMeta.getJavaType();
+ result = SimpleType.constructUnsafe(Date.class);
+ responseEx.getStatus();
+ result = 400;
+ responseEx.getBodyBuffer();
+ result = new BufferImpl().appendString("abc");
+ }
+ };
+ new MockUp<DefaultHttpClientFilter>() {
+ @Mock
+ ProduceProcessor findProduceProcessor(RestOperationMeta restOperation,
HttpServletResponseEx responseEx) {
+ return new ProduceJsonProcessor();
+ }
+ };
+
+ Response response = filter.extractResponse(invocation, responseEx);
+ Assert.assertEquals(400, response.getStatusCode());
+ Assert.assertEquals(InvocationException.class,
response.<InvocationException>getResult().getClass());
+ InvocationException invocationException = response.getResult();
+ Assert.assertEquals(
+ "InvocationException: code=400;msg=CommonExceptionData [message=method
null, path null, statusCode 400, reasonPhrase null, response content-type null
is not supported]",
+ 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(
+ "method null, path null, statusCode 400, reasonPhrase null, response
content-type null is not supported",
+ commonExceptionData.getMessage());
+ }
+
+ @Test
+ public void extractResult_decodeError200(@Mocked Invocation invocation,
@Mocked ReadStreamPart part,
+ @Mocked OperationMeta operationMeta, @Mocked ResponseMeta responseMeta,
+ @Mocked RestOperationMeta swaggerRestOperation,
+ @Mocked HttpServletResponseEx responseEx) {
+ Map<String, Object> handlerContext = new HashMap<>();
+ new Expectations() {
+ {
+ invocation.getHandlerContext();
+ result = handlerContext;
+ invocation.getOperationMeta();
+ result = operationMeta;
operationMeta.findResponseMeta(200);
result = responseMeta;
operationMeta.getExtData(RestConst.SWAGGER_REST_OPERATION);
@@ -156,18 +203,20 @@ public class TestDefaultHttpClientFilter {
};
Response response = filter.extractResponse(invocation, responseEx);
- Assert.assertEquals(490, response.getStatusCode());
- Assert.assertEquals(ExceptionFactory.CONSUMER_INNER_REASON_PHRASE,
response.getReasonPhrase());
+ Assert.assertEquals(400, response.getStatusCode());
Assert.assertEquals(InvocationException.class,
response.<InvocationException>getResult().getClass());
InvocationException invocationException = response.getResult();
- Assert.assertEquals("InvocationException: code=490;msg=CommonExceptionData
[message=Cse Internal Bad Request]",
+ Assert.assertEquals(
+ "InvocationException: code=400;msg=CommonExceptionData [message=method
null, path null, statusCode 200, reasonPhrase null, response content-type null
is not supported]",
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());
+ Assert.assertEquals(
+ "method null, path null, statusCode 200, reasonPhrase null, response
content-type null is not supported",
+ commonExceptionData.getMessage());
}
@Test
