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/07/02 14:16:29 UTC

[GitHub] [servicecomb-java-chassis] yhs0092 opened a new pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

yhs0092 opened a new pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865


   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] 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.
    - [x] Each commit in the pull request should have a meaningful subject line and body.
    - [x] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [x] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [x] 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

[GitHub] [servicecomb-java-chassis] wujimin commented on a change in pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#discussion_r449154346



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerConsumer.java
##########
@@ -16,14 +16,15 @@
  */
 package org.apache.servicecomb.swagger.engine;
 
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
 
 public class SwaggerConsumer {
   private Class<?> consumerIntf;
 
   // key is consumer method name
-  private Map<String, SwaggerConsumerOperation> operations = new HashMap<>();
+  private Map<Method, SwaggerConsumerOperation> operations = new HashMap<>();

Review comment:
       IdentityHashMap




----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] coveralls edited a comment on pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#issuecomment-653120133


   
   [![Coverage Status](https://coveralls.io/builds/31880477/badge)](https://coveralls.io/builds/31880477)
   
   Coverage increased (+0.003%) to 86.311% when pulling **f4a0e31ea2c10377b17ddcf8d23b0c5944a3949b on yhs0092:fix_mapping_problem_between_pojo_consumer_method_and_provider_swagger_operation** into **751a8519c7afc0b2a524ab5d8b792fb0df1472b3 on apache:master**.
   


----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] wujimin commented on a change in pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#discussion_r449155873



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerConsumer.java
##########
@@ -34,14 +35,14 @@ public void setConsumerIntf(Class<?> consumerIntf) {
   }
 
   public void addOperation(SwaggerConsumerOperation op) {
-    operations.put(op.getSchemaOperationId(), op);
+    operations.put(op.getConsumerMethod(), op);
   }
 
-  public SwaggerConsumerOperation findOperation(String consumerMethodName) {
-    return operations.get(consumerMethodName);
+  public SwaggerConsumerOperation findOperation(Method consumerMethod) {

Review comment:
       can still keep `findOperation(String consumerMethodName)`, and mark as VisiableForTest  
   this makes test easier




----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] coveralls commented on pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#issuecomment-653120133


   
   [![Coverage Status](https://coveralls.io/builds/31830812/badge)](https://coveralls.io/builds/31830812)
   
   Coverage increased (+0.01%) to 86.324% when pulling **5b206430be2269b223006a7309c64b2da5f6b4c4 on yhs0092:fix_mapping_problem_between_pojo_consumer_method_and_provider_swagger_operation** into **5f5264ff8ed39b9e0671514221c948ff36984ec1 on apache:master**.
   


----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] wujimin merged pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
wujimin merged pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865


   


----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] wujimin commented on a change in pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#discussion_r449153654



##########
File path: providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/definition/PojoConsumerMeta.java
##########
@@ -33,7 +34,7 @@
 
   private SchemaMeta schemaMeta;
 
-  private Map<String, PojoConsumerOperationMeta> operationMetas = new HashMap<>();
+  private Map<Method, PojoConsumerOperationMeta> operationMetas = new HashMap<>();

Review comment:
       Method to be key of HashMap is too expensive  
   should change HashMap to IdentityHashMap




----------------------------------------------------------------
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

[GitHub] [servicecomb-java-chassis] yhs0092 commented on a change in pull request #1865: [SCB-2031] Fix mapping problem between pojo consumer method and provider swagger operation

Posted by GitBox <gi...@apache.org>.
yhs0092 commented on a change in pull request #1865:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1865#discussion_r450145541



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerConsumer.java
##########
@@ -34,14 +35,14 @@ public void setConsumerIntf(Class<?> consumerIntf) {
   }
 
   public void addOperation(SwaggerConsumerOperation op) {
-    operations.put(op.getSchemaOperationId(), op);
+    operations.put(op.getConsumerMethod(), op);
   }
 
-  public SwaggerConsumerOperation findOperation(String consumerMethodName) {
-    return operations.get(consumerMethodName);
+  public SwaggerConsumerOperation findOperation(Method consumerMethod) {

Review comment:
       Done. Please review it.




----------------------------------------------------------------
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