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 2020/01/17 08:25:22 UTC

[servicecomb-java-chassis] branch master updated: [SCB-1730]highway support primitive default values and convert char/byte/short

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/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new f0c5dd3  [SCB-1730]highway support primitive default values and convert char/byte/short
f0c5dd3 is described below

commit f0c5dd3391192ec3029b6f7e72ee4d47ff5bd568
Author: liubao <bi...@qq.com>
AuthorDate: Fri Jan 17 11:19:40 2020 +0800

    [SCB-1730]highway support primitive default values and convert char/byte/short
---
 .../org/apache/servicecomb/core/Invocation.java    |  1 -
 .../demo/springmvc/client/SpringmvcClient.java     | 38 ++++++++++---------
 .../demo/springmvc/server/ControllerImpl.java      |  3 ++
 .../scalar/AbstractScalarReadSchemas.java          |  2 +-
 .../deserializer/scalar/Int32ReadSchemas.java      | 15 ++++++--
 .../deserializer/scalar/StringReadSchemas.java     | 12 ++++--
 .../serializer/scalar/StringWriteSchemas.java      |  5 +++
 .../transport/highway/HighwayCodec.java            | 43 ++++++++++++++++++----
 8 files changed, 87 insertions(+), 32 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/Invocation.java b/core/src/main/java/org/apache/servicecomb/core/Invocation.java
index 488b246..131e514 100644
--- a/core/src/main/java/org/apache/servicecomb/core/Invocation.java
+++ b/core/src/main/java/org/apache/servicecomb/core/Invocation.java
@@ -209,7 +209,6 @@ public class Invocation extends SwaggerInvocation {
   public Object[] toProducerArguments() {
     Method method = operationMeta.getSwaggerProducerOperation().getProducerMethod();
     Object[] args = new Object[method.getParameterCount()];
-    // TODO: WEAK parameter name maybe override by annotations.
     for (int i = 0; i < method.getParameterCount(); i++) {
       args[i] = this.arguments.get(method.getParameters()[i].getName());
     }
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 958c06c..829a0dc 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
@@ -492,23 +492,27 @@ public class SpringmvcClient {
 
   private static void testSpringMvcDefaultValuesJavaPrimitiveAllTransport(RestTemplate template,
       String microserviceName) {
-    // TODO: WEAK primitive default value not supported in highway
-//    String cseUrlPrefix = "cse://" + microserviceName + "/SpringMvcDefaultValues/";
-//    //default values with primitive
-//    String result = template.postForObject(cseUrlPrefix + "/javaprimitiveint", null, String.class);
-//    TestMgr.check("Hello 0bobo", result);
-//
-//    result = template.postForObject(cseUrlPrefix + "/javaprimitivenumber", null, String.class);
-//    TestMgr.check("Hello 0.0false", result);
-//
-//    result = template.postForObject(cseUrlPrefix + "/javaprimitivestr", null, String.class);
-//    TestMgr.check("Hello", result);
-//
-//    result = template.postForObject(cseUrlPrefix + "/javaprimitivecomb", null, String.class);
-//    TestMgr.check("Hello nullnull", result);
-//
-//    result = template.postForObject(cseUrlPrefix + "/allprimitivetypes", null, String.class);
-//    TestMgr.check("Hello false,\0,0,0,0,0,0.0,0.0,null", result);
+    String cseUrlPrefix = "cse://" + microserviceName + "/SpringMvcDefaultValues/";
+    //default values with primitive
+    String result = template.postForObject(cseUrlPrefix + "/javaprimitiveint", null, String.class);
+    TestMgr.check("Hello 0bobo", result);
+
+    result = template.postForObject(cseUrlPrefix + "/javaprimitivenumber", null, String.class);
+    TestMgr.check("Hello 0.0false", result);
+
+    result = template.postForObject(cseUrlPrefix + "/javaprimitivestr", null, String.class);
+    TestMgr.check("Hello", result);
+
+    result = template.postForObject(cseUrlPrefix + "/javaprimitivecomb", null, String.class);
+    TestMgr.check("Hello nullnull", result);
+
+    result = template.postForObject(cseUrlPrefix + "/allprimitivetypes", null, String.class);
+    TestMgr.check("Hello false,\0,0,0,0,0,0.0,0.0,null", result);
+
+    result = template.postForObject(cseUrlPrefix
+            + "/allprimitivetypes?pBoolean=true&pChar=c&pByte=20&pShort=30&pInt=40&pLong=50&pFloat=60&pDouble=70&pDoubleWrap=80",
+        null, String.class);
+    TestMgr.check("Hello true,c,20,30,40,50,60.0,70.0,80.0", result);
   }
 
   private static void testSpringMvcDefaultValuesJavaPrimitiveRest(RestTemplate template, String microserviceName) {
diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
index 46adbe6..483c4f0 100644
--- a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
+++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
@@ -36,6 +36,8 @@ import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
 
+// This class tests "contract first", the controller.yaml will override annotations defined in class.
+
 @RestSchema(schemaId = "controller")
 @RequestMapping(path = "/springmvc/controller", produces = MediaType.APPLICATION_JSON)
 public class ControllerImpl {
@@ -57,6 +59,7 @@ public class ControllerImpl {
     return prefix + " " + user.getName();
   }
 
+  // Parameters defined in controller.yaml. The code definition do not have any parameters.
   @RequestMapping(path = "/sayhi", method = RequestMethod.GET)
   public String sayHi(HttpServletRequest request) throws Exception {
     String addr = request.getRemoteAddr();
diff --git a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/AbstractScalarReadSchemas.java b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/AbstractScalarReadSchemas.java
index 09ef8bd..83366a4 100644
--- a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/AbstractScalarReadSchemas.java
+++ b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/AbstractScalarReadSchemas.java
@@ -26,7 +26,7 @@ import io.protostuff.runtime.FieldSchema;
 
 public class AbstractScalarReadSchemas {
   static abstract class AbstractIntSchema<T> extends FieldSchema<T> {
-    protected final Setter<T, Integer> setter;
+    protected final Setter<T, Object> setter;
 
     public AbstractIntSchema(Field protoField, PropertyDescriptor propertyDescriptor) {
       super(protoField, propertyDescriptor.getJavaType());
diff --git a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/Int32ReadSchemas.java b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/Int32ReadSchemas.java
index 87177f6..2ad7f90 100644
--- a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/Int32ReadSchemas.java
+++ b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/Int32ReadSchemas.java
@@ -32,11 +32,14 @@ import io.protostuff.runtime.FieldSchema;
 public class Int32ReadSchemas {
   public static <T> FieldSchema<T> create(Field protoField, PropertyDescriptor propertyDescriptor) {
     JavaType javaType = propertyDescriptor.getJavaType();
-    if (int.class.equals(javaType.getRawClass())) {
+    if (int.class.equals(javaType.getRawClass()) || byte.class.equals(javaType.getRawClass()) || short.class
+        .equals(javaType.getRawClass())) {
       return new Int32PrimitiveSchema<>(protoField, propertyDescriptor);
     }
 
-    if (Integer.class.equals(javaType.getRawClass()) || javaType.isJavaLangObject()) {
+    if (Integer.class.equals(javaType.getRawClass()) || Byte.class.equals(javaType.getRawClass()) || Short.class
+        .equals(javaType.getRawClass()) || javaType
+        .isJavaLangObject()) {
       return new Int32Schema<>(protoField, propertyDescriptor);
     }
 
@@ -52,7 +55,13 @@ public class Int32ReadSchemas {
     @Override
     public int mergeFrom(InputEx input, T message) throws IOException {
       int value = input.readInt32();
-      setter.set(message, value);
+      if (Byte.class.equals(javaType.getRawClass()) || byte.class.equals(javaType.getRawClass())) {
+        setter.set(message, (byte) value);
+      } else if (Short.class.equals(javaType.getRawClass()) || short.class.equals(javaType.getRawClass())) {
+        setter.set(message, (short) value);
+      } else {
+        setter.set(message, value);
+      }
       return input.readFieldNumber();
     }
   }
diff --git a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/StringReadSchemas.java b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/StringReadSchemas.java
index 2c9d0d4..e2db31f 100644
--- a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/StringReadSchemas.java
+++ b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/deserializer/scalar/StringReadSchemas.java
@@ -31,7 +31,8 @@ import io.protostuff.runtime.FieldSchema;
 public class StringReadSchemas {
   public static <T> FieldSchema<T> create(Field protoField, PropertyDescriptor propertyDescriptor) {
     JavaType javaType = propertyDescriptor.getJavaType();
-    if (String.class.equals(javaType.getRawClass()) || javaType.isJavaLangObject()) {
+    if (String.class.equals(javaType.getRawClass()) || javaType.isJavaLangObject() || char.class
+        .equals(javaType.getRawClass()) || Character.class.equals(javaType.getRawClass())) {
       return new StringSchema<>(protoField, propertyDescriptor);
     }
 
@@ -40,7 +41,7 @@ public class StringReadSchemas {
   }
 
   private static class StringSchema<T> extends FieldSchema<T> {
-    private final Setter<T, String> setter;
+    private final Setter<T, Object> setter;
 
     public StringSchema(Field protoField, PropertyDescriptor propertyDescriptor) {
       super(protoField, propertyDescriptor.getJavaType());
@@ -50,7 +51,12 @@ public class StringReadSchemas {
     @Override
     public int mergeFrom(InputEx input, T message) throws IOException {
       String value = input.readString();
-      setter.set(message, value);
+      if (char.class
+          .equals(javaType.getRawClass()) || Character.class.equals(javaType.getRawClass())) {
+        setter.set(message, value.toCharArray()[0]);
+      } else {
+        setter.set(message, value);
+      }
       return input.readFieldNumber();
     }
   }
diff --git a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/serializer/scalar/StringWriteSchemas.java b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/serializer/scalar/StringWriteSchemas.java
index 77f18a9..8434e79 100644
--- a/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/serializer/scalar/StringWriteSchemas.java
+++ b/foundations/foundation-protobuf/src/main/java/org/apache/servicecomb/foundation/protobuf/internal/schema/serializer/scalar/StringWriteSchemas.java
@@ -55,6 +55,11 @@ public class StringWriteSchemas {
         return;
       }
 
+      if (value instanceof Character) {
+        output.writeScalarString(tag, tagSize, String.valueOf((char)value));
+        return;
+      }
+
       ProtoUtils.throwNotSupportWrite(protoField, value);
     }
   }
diff --git a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java
index 9278e56..9db0396 100644
--- a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java
+++ b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayCodec.java
@@ -17,21 +17,26 @@
 
 package org.apache.servicecomb.transport.highway;
 
+import java.lang.reflect.Type;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.servicecomb.codec.protobuf.definition.OperationProtobuf;
+import org.apache.servicecomb.codec.protobuf.definition.RequestRootDeserializer;
+import org.apache.servicecomb.codec.protobuf.definition.ResponseRootDeserializer;
 import org.apache.servicecomb.codec.protobuf.definition.ResponseRootSerializer;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.definition.OperationMeta;
-import org.apache.servicecomb.codec.protobuf.definition.RequestRootDeserializer;
-import org.apache.servicecomb.codec.protobuf.definition.ResponseRootDeserializer;
-import org.apache.servicecomb.foundation.protobuf.RootSerializer;
 import org.apache.servicecomb.foundation.vertx.client.tcp.TcpData;
 import org.apache.servicecomb.foundation.vertx.tcp.TcpOutputStream;
+import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.apache.servicecomb.transport.highway.message.RequestHeader;
 import org.apache.servicecomb.transport.highway.message.ResponseHeader;
 
+import com.google.common.base.Defaults;
+
+import io.swagger.models.parameters.Parameter;
 import io.vertx.core.buffer.Buffer;
 
 public final class HighwayCodec {
@@ -74,14 +79,36 @@ public final class HighwayCodec {
       Map<String, Object> swaggerArguments) {
     if (operationMeta.getSwaggerProducerOperation() != null && !invocation.isEdge()) {
       return operationMeta.getSwaggerProducerOperation().getArgumentsMapper()
-          .swaggerArgumentToInvocationArguments(invocation, swaggerArguments);
+          .swaggerArgumentToInvocationArguments(invocation,
+              addPrimitiveTypeDefaultValues(invocation, operationMeta, swaggerArguments));
     } else {
-      // edge
       return swaggerArguments;
     }
   }
 
   @SuppressWarnings({"rawtypes", "unchecked"})
+  private static Map<String, Object> addPrimitiveTypeDefaultValues(Invocation invocation, OperationMeta operationMeta,
+      Map<String, Object> swaggerArguments) {
+    // proto buffer never serialize default values, put it back in provider
+    SwaggerProducerOperation swaggerProducerOperation = operationMeta.getSwaggerProducerOperation();
+    if (swaggerProducerOperation != null && !invocation.isEdge()) {
+      List<Parameter> swaggerParameters = operationMeta.getSwaggerOperation()
+          .getParameters();
+      for (Parameter parameter : swaggerParameters) {
+        if (!parameter.getRequired() && swaggerArguments.get(parameter.getName()) == null) {
+          Type type = swaggerProducerOperation.getSwaggerParameterType(parameter.getName());
+          if (type instanceof Class) {
+            if (((Class) type).isPrimitive()) {
+              swaggerArguments.put(parameter.getName(), Defaults.defaultValue((Class) type));
+            }
+          }
+        }
+      }
+    }
+    return swaggerArguments;
+  }
+
+  @SuppressWarnings({"rawtypes", "unchecked"})
   public static void decodeRequest(Invocation invocation, RequestHeader header, OperationProtobuf operationProtobuf,
       Buffer bodyBuffer) throws Exception {
     RequestRootDeserializer<Object> requestDesirializer = operationProtobuf.getRequestRootDeserializer();
@@ -110,8 +137,10 @@ public final class HighwayCodec {
       invocation.getContext().putAll(header.getContext());
     }
 
-    ResponseRootDeserializer<Object> bodySchema = operationProtobuf.findResponseRootDeserializer(header.getStatusCode());
-    Object body = bodySchema.deserialize(tcpData.getBodyBuffer().getBytes(), invocation.findResponseType(header.getStatusCode()));
+    ResponseRootDeserializer<Object> bodySchema = operationProtobuf
+        .findResponseRootDeserializer(header.getStatusCode());
+    Object body = bodySchema
+        .deserialize(tcpData.getBodyBuffer().getBytes(), invocation.findResponseType(header.getStatusCode()));
 
     Response response = Response.create(header.getStatusCode(), header.getReasonPhrase(), body);
     response.setHeaders(header.getHeaders());