You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/04/02 17:09:58 UTC

[GitHub] [servicecomb-java-chassis] heyile opened a new pull request #1688: [SCB-1828] Support @JSONVIEW

heyile opened a new pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688
 
 
   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#discussion_r404469994
 
 

 ##########
 File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessorManager.java
 ##########
 @@ -17,40 +17,89 @@
 
 package org.apache.servicecomb.common.rest.codec.produce;
 
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
-import java.util.Set;
+import java.util.Map;
 
 import javax.ws.rs.core.MediaType;
 
 import org.apache.servicecomb.foundation.common.RegisterManager;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class ProduceProcessorManager extends RegisterManager<String, Map<String, ProduceProcessor>> {
+  private static final Logger LOGGER = LoggerFactory.getLogger(ProduceProcessorManager.class);
 
-public final class ProduceProcessorManager extends RegisterManager<String, ProduceProcessor> {
   private static final List<ProduceProcessor> produceProcessor =
       SPIServiceUtils.getSortedService(ProduceProcessor.class);
 
   private static final String NAME = "produce processor mgr";
 
   public static final String DEFAULT_TYPE = MediaType.APPLICATION_JSON;
 
+  public static final String DEFAULT_SERIAL_CLASS = "servicecomb_default_class";
+
   public static final ProduceProcessorManager INSTANCE = new ProduceProcessorManager();
 
-  public static final ProduceProcessor JSON_PROCESSOR =
-      SPIServiceUtils.getTargetService(ProduceProcessor.class, ProduceJsonProcessor.class);
+  private Map<String, ProduceProcessor> jsonProcessorMap;
 
-  public static final ProduceProcessor PLAIN_PROCESSOR =
-      SPIServiceUtils.getTargetService(ProduceProcessor.class, ProduceTextPlainProcessor.class);
+  private Map<String, ProduceProcessor> plainProcessorMap;
 
-  public static final ProduceProcessor DEFAULT_PROCESSOR = JSON_PROCESSOR;
+  private Map<String, ProduceProcessor> defaultProcessorMap;
 
   private ProduceProcessorManager() {
     super(NAME);
-    Set<String> set = new HashSet<>();
     produceProcessor.forEach(processor -> {
-      if (set.add(processor.getName())) {
-        register(processor.getName(), processor);
-      }
+      Map<String, ProduceProcessor> prodProcessorMap = getObjMap()
+          .computeIfAbsent(processor.getName(), key -> new HashMap<>());
+      prodProcessorMap.putIfAbsent(processor.getSerializationView(), processor);
     });
+    jsonProcessorMap = ensureFindValue(MediaType.APPLICATION_JSON);
+    plainProcessorMap = ensureFindValue(MediaType.TEXT_PLAIN);
+    defaultProcessorMap = jsonProcessorMap;
+  }
+
+  public static ProduceProcessor cloneNewProduceProcessor(String acceptType, Class<?> serialViewClass,
+      Map<String, ProduceProcessor> produceViewMap) {
+    ProduceProcessor newInstance;
+    try {
+      newInstance = produceViewMap.get(DEFAULT_SERIAL_CLASS).getClass().newInstance();
+      newInstance.setSerializationView(serialViewClass);
+      return newInstance;
+    } catch (Throwable e) {
+      LOGGER.error(String.format("Failed to create produceProcessor with %s", acceptType), e);
 
 Review comment:
   ```LOGGER.error("Failed to create produceProcessor with {}", acceptType, e);  ``` will handle formation. 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] heyile commented on issue #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

heyile commented on issue #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#issuecomment-608178340
 
 
   > org.apache.servicecomb.swagger.generator.core.AbstractOperationGenerator#fillBodyParameter 里调用的readAsProperty(type)是不是也应该改成 readAsProperty(type, JsonView)， org.apache.servicecomb.swagger.generator.core.processor.annotation.AnnotationUtils#generateResponseProperty 同理~
   
   我确认下

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] Nick-The-Uncharted commented on issue #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

Nick-The-Uncharted commented on issue #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#issuecomment-610717603
 
 
   D:/repository/io/swagger/swagger-core/1.5.22/swagger-core-1.5.22-sources.jar!/io/swagger/util/ParameterProcessor.java:263 看了下swagger代码 parameter没变应该是这行用的 io.swagger.converter.ModelConverters#readAsProperty(java.lang.reflect.Type) 没带jsonView，新版本的swagger-core似乎处理了，调用链太深没能跟下去。。。

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] Nick-The-Uncharted removed a comment on issue #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

Nick-The-Uncharted removed a comment on issue #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#issuecomment-608189661
 
 
   如果能超越spring的处理 支持jsonview里带多个class不知道会不会有实际意义。 这样可能不需要很多view的继承，组合不同view不知道会不会让使用更方便，比如 @JsonView(WithTimeStamp.class, WithConfidencial.class，WithNormal.class)， 并不知道spring限制只能带一个class的理由是啥=。=

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#discussion_r404466862
 
 

 ##########
 File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/BodyProcessorCreator.java
 ##########
 @@ -64,16 +67,34 @@
   public static class BodyProcessor implements ParamValueProcessor {
     protected JavaType targetType;
 
+    protected Class<?> serialViewClass;
+
     private boolean isString;
 
     protected boolean isRequired;
 
     public BodyProcessor(JavaType targetType, boolean isString, boolean isRequired) {
+      this(targetType, null, isString, isRequired);
+    }
+
+    public BodyProcessor(JavaType targetType, String serialViewClass, boolean isString, boolean isRequired) {
+      if (serialViewClass != null) {
+        try {
+          this.serialViewClass = Class.forName(serialViewClass);
+        } catch (Throwable e) {
+          //ignore
 
 Review comment:
   It's better to add a log and throw an exception. This is an invalid usage when happens that users need to know. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] heyile commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

heyile commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#discussion_r404755084
 
 

 ##########
 File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java
 ##########
 @@ -257,31 +263,71 @@ private void addParam(RestParam param) {
     paramMap.put(param.getParamName(), param);
   }
 
+  public ProduceProcessor findProduceProcessor(String type, String serialView) {
 
 Review comment:
   done, I will init it at **RestOperationMeta#createProduceProcessors** .

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#discussion_r404481112
 
 

 ##########
 File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/definition/RestOperationMeta.java
 ##########
 @@ -257,31 +263,71 @@ private void addParam(RestParam param) {
     paramMap.put(param.getParamName(), param);
   }
 
+  public ProduceProcessor findProduceProcessor(String type, String serialView) {
 
 Review comment:
   Maybe this code can be better. Because:
   
   
   ProduceProcessor can be constructed at initialization, and will not change when excepting requests. So only `findProduceProcessor(String type)` can expose to the callers. `findProduceProcessor(String type, String serialView)` is not needed. And `ensureFindProduceProcessor` do not need to processing annotations for each request, but process once when initialization. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] heyile commented on issue #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

heyile commented on issue #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#issuecomment-608178096
 
 
   > readAsProperty
   
   我看下...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW

Posted by GitBox <gi...@apache.org>.

liubao68 commented on a change in pull request #1688: [SCB-1828] Support @JSONVIEW 
URL: https://github.com/apache/servicecomb-java-chassis/pull/1688#discussion_r404471261
 
 

 ##########
 File path: common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessorManager.java
 ##########
 @@ -17,40 +17,89 @@
 
 package org.apache.servicecomb.common.rest.codec.produce;
 
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
-import java.util.Set;
+import java.util.Map;
 
 import javax.ws.rs.core.MediaType;
 
 import org.apache.servicecomb.foundation.common.RegisterManager;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class ProduceProcessorManager extends RegisterManager<String, Map<String, ProduceProcessor>> {
+  private static final Logger LOGGER = LoggerFactory.getLogger(ProduceProcessorManager.class);
 
-public final class ProduceProcessorManager extends RegisterManager<String, ProduceProcessor> {
   private static final List<ProduceProcessor> produceProcessor =
       SPIServiceUtils.getSortedService(ProduceProcessor.class);
 
   private static final String NAME = "produce processor mgr";
 
   public static final String DEFAULT_TYPE = MediaType.APPLICATION_JSON;
 
+  public static final String DEFAULT_SERIAL_CLASS = "servicecomb_default_class";
+
   public static final ProduceProcessorManager INSTANCE = new ProduceProcessorManager();
 
-  public static final ProduceProcessor JSON_PROCESSOR =
-      SPIServiceUtils.getTargetService(ProduceProcessor.class, ProduceJsonProcessor.class);
+  private Map<String, ProduceProcessor> jsonProcessorMap;
 
-  public static final ProduceProcessor PLAIN_PROCESSOR =
-      SPIServiceUtils.getTargetService(ProduceProcessor.class, ProduceTextPlainProcessor.class);
+  private Map<String, ProduceProcessor> plainProcessorMap;
 
-  public static final ProduceProcessor DEFAULT_PROCESSOR = JSON_PROCESSOR;
+  private Map<String, ProduceProcessor> defaultProcessorMap;
 
   private ProduceProcessorManager() {
     super(NAME);
-    Set<String> set = new HashSet<>();
     produceProcessor.forEach(processor -> {
-      if (set.add(processor.getName())) {
-        register(processor.getName(), processor);
-      }
+      Map<String, ProduceProcessor> prodProcessorMap = getObjMap()
+          .computeIfAbsent(processor.getName(), key -> new HashMap<>());
+      prodProcessorMap.putIfAbsent(processor.getSerializationView(), processor);
     });
+    jsonProcessorMap = ensureFindValue(MediaType.APPLICATION_JSON);
+    plainProcessorMap = ensureFindValue(MediaType.TEXT_PLAIN);
+    defaultProcessorMap = jsonProcessorMap;
+  }
+
+  public static ProduceProcessor cloneNewProduceProcessor(String acceptType, Class<?> serialViewClass,
+      Map<String, ProduceProcessor> produceViewMap) {
+    ProduceProcessor newInstance;
+    try {
+      newInstance = produceViewMap.get(DEFAULT_SERIAL_CLASS).getClass().newInstance();
+      newInstance.setSerializationView(serialViewClass);
+      return newInstance;
+    } catch (Throwable e) {
+      LOGGER.error(String.format("Failed to create produceProcessor with %s", acceptType), e);
+    }
+    return produceViewMap.get(DEFAULT_SERIAL_CLASS);
+  }
+
+  public Map<String, ProduceProcessor> getJsonProcessorMap() {
 
 Review comment:
   It's not good to expose Map to callers. Maybe 
   ```
   public ProduceProcessor getProduceProcessor(String name) 
   ```
   is better. Following code are the same. 
   
   And for setters, they seam not used, and can deleted. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services