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 2021/09/27 13:07:37 UTC

[GitHub] [servicecomb-java-chassis] fanjiwang1992 opened a new pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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


   fix swagger generator duplicate param model exception bug


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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       add a check `TestMgr.fail` if this invocation is successful




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 merged pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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


   


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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] fanjiwang1992 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -210,20 +211,30 @@ public static void addDefinitions(Swagger swagger, Type paramType) {
     if (javaType.isTypeOrSubTypeOf(DynamicEnum.class)) {
       return;
     }
-
     Map<String, Model> models = ModelConverters.getInstance().readAll(javaType);
     for (Entry<String, Model> entry : models.entrySet()) {
       if (null != swagger.getDefinitions()) {
         Model tempModel = swagger.getDefinitions().get(entry.getKey());
         if (null != tempModel && !tempModel.equals(entry.getValue())) {
-          LOGGER.warn("duplicate param model: " + entry.getKey());
-          throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          if (modelNotDuplicate(tempModel, entry.getValue())) {
+            swagger.addDefinition(entry.getKey(), tempModel);
+            continue;
+          } else {
+            LOGGER.warn("duplicate param model: " + entry.getKey());
+            throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          }
         }
       }
       swagger.addDefinition(entry.getKey(), entry.getValue());
     }
   }
 
+  private static boolean modelNotDuplicate(Model tempModel, Model model){
+    String tempModelClass = (String) tempModel.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
+    String modelClass = (String) model.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
+    return tempModelClass.equals(modelClass);
+  }
+

Review comment:
       done




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] codecov-commenter commented on pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2600:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2600#issuecomment-939296238


   # [Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2600](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8ed0eff) into [master](https://codecov.io/gh/apache/servicecomb-java-chassis/commit/3286fa4e6235b14809b62e35becb1cbb49ca260d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3286fa4) will **decrease** coverage by `0.00%`.
   > The diff coverage is `62.50%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/graphs/tree.svg?width=650&height=150&src=pr&token=KXfDcr9rX2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2600      +/-   ##
   ============================================
   - Coverage     77.72%   77.71%   -0.01%     
     Complexity     1429     1429              
   ============================================
     Files          1598     1598              
     Lines         42652    42674      +22     
     Branches       3591     3592       +1     
   ============================================
   + Hits          33151    33166      +15     
   - Misses         8000     8007       +7     
     Partials       1501     1501              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...xrs/client/validation/ValidationServiceClient.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVtby9kZW1vLWpheHJzL2pheHJzLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvZGVtby9qYXhycy9jbGllbnQvdmFsaWRhdGlvbi9WYWxpZGF0aW9uU2VydmljZUNsaWVudC5qYXZh) | `71.92% <62.50%> (-3.68%)` | :arrow_down: |
   | [...a/org/apache/servicecomb/swagger/SwaggerUtils.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3dhZ2dlci9zd2FnZ2VyLWdlbmVyYXRvci9nZW5lcmF0b3ItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvc3dhZ2dlci9Td2FnZ2VyVXRpbHMuamF2YQ==) | `92.10% <62.50%> (-1.38%)` | :arrow_down: |
   | [.../servicecomb/registry/discovery/DiscoveryTree.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Zm91bmRhdGlvbnMvZm91bmRhdGlvbi1yZWdpc3RyeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvcmVnaXN0cnkvZGlzY292ZXJ5L0Rpc2NvdmVyeVRyZWUuamF2YQ==) | `100.00% <0.00%> (+3.50%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [3286fa4...8ed0eff](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] fanjiwang1992 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       done




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -216,8 +218,14 @@ public static void addDefinitions(Swagger swagger, Type paramType) {
       if (null != swagger.getDefinitions()) {
         Model tempModel = swagger.getDefinitions().get(entry.getKey());
         if (null != tempModel && !tempModel.equals(entry.getValue())) {
-          LOGGER.warn("duplicate param model: " + entry.getKey());
-          throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          String tempModelClass = (String) tempModel.getVendorExtensions().get(X_JAVA_CLASS);
+          if (tempModelClass.equals(entry.getValue().getVendorExtensions().get(X_JAVA_CLASS))) {
+            swagger.addDefinition(entry.getKey(), tempModel);
+            continue;
+          } else {
+            LOGGER.warn("duplicate param model: " + entry.getKey());
+            throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          }
         }

Review comment:
       extract a method is better:
   
   ```
   if(modelNotDuplicate(...)) {
      LOGGER.warn("duplicate param model: " + entry.getKey());
             throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
   }
   ```




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       add a check `TestMgr.fail` if this invocation is successful. Because if this test is successful, will not go to InvocationException logic. 




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       ```
   
         Teacher teacher = new Teacher();
         teacher.setName("teacher");
         teacher.setAge("20");
         Teacher response = template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);
         TestMgr.check(response.getName(), "teacher");
   
   
   
       try {
         Teacher teacher = new Teacher();
         teacher.setName("teacher");
         teacher.setAge("20");
          template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);
         TestMgr.fail("should throw exception");
       } catch (InvocationException e) {
         TestMgr.check(400, e.getStatus().getStatusCode());
         TestMgr.check(e.getErrorData().toString().contains("不能为空"), true);
       }
   
   ``
   
   I mean add 
   
   `TestMgr.fail("should throw exception");`




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -76,6 +76,8 @@
 
   private static final Logger LOGGER = LoggerFactory.getLogger(SwaggerUtils.class);
 
+  private static String X_JAVA_CLASS = "x-java-class";

Review comment:
       Maybe can use `SwaggerConst.EXT_JAVA_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.

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       ```
   
         Teacher teacher = new Teacher();
         teacher.setName("teacher");
         teacher.setAge("20");
         Teacher response = template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);
         TestMgr.check(response.getName(), "teacher");
   
   
   
       try {
         Teacher teacher = new Teacher();
         teacher.setName("teacher");
         teacher.setAge("20");
          template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);
         TestMgr.fail("should throw exception");
       } catch (InvocationException e) {
         TestMgr.check(400, e.getStatus().getStatusCode());
         TestMgr.check(e.getErrorData().toString().contains("不能为空"), true);
       }
   
   ``
   
   How about this?




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] codecov-commenter edited a comment on pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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


   # [Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2600](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dc0b65e) into [master](https://codecov.io/gh/apache/servicecomb-java-chassis/commit/eacfbf6dd6f12671d044e884743ab4bb1607e324?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eacfbf6) will **decrease** coverage by `0.00%`.
   > The diff coverage is `74.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/graphs/tree.svg?width=650&height=150&src=pr&token=KXfDcr9rX2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2600      +/-   ##
   ============================================
   - Coverage     77.74%   77.73%   -0.01%     
     Complexity     1429     1429              
   ============================================
     Files          1598     1598              
     Lines         42657    42685      +28     
     Branches       3592     3593       +1     
   ============================================
   + Hits          33162    33183      +21     
   - Misses         7996     8003       +7     
     Partials       1499     1499              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...xrs/client/validation/ValidationServiceClient.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGVtby9kZW1vLWpheHJzL2pheHJzLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvZGVtby9qYXhycy9jbGllbnQvdmFsaWRhdGlvbi9WYWxpZGF0aW9uU2VydmljZUNsaWVudC5qYXZh) | `72.41% <64.70%> (-3.20%)` | :arrow_down: |
   | [...a/org/apache/servicecomb/swagger/SwaggerUtils.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3dhZ2dlci9zd2FnZ2VyLWdlbmVyYXRvci9nZW5lcmF0b3ItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvc3dhZ2dlci9Td2FnZ2VyVXRpbHMuamF2YQ==) | `92.30% <83.33%> (-1.18%)` | :arrow_down: |
   | [...b/core/invocation/timeout/PassingTimeStrategy.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VydmljZWNvbWIvY29yZS9pbnZvY2F0aW9uL3RpbWVvdXQvUGFzc2luZ1RpbWVTdHJhdGVneS5qYXZh) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
   | [...ache/servicecomb/foundation/common/net/IpPort.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Zm91bmRhdGlvbnMvZm91bmRhdGlvbi1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NlcnZpY2Vjb21iL2ZvdW5kYXRpb24vY29tbW9uL25ldC9JcFBvcnQuamF2YQ==) | `86.66% <0.00%> (+3.33%)` | :arrow_up: |
   | [...che/servicecomb/http/client/task/AbstractTask.java](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50cy9odHRwLWNsaWVudC1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NlcnZpY2Vjb21iL2h0dHAvY2xpZW50L3Rhc2svQWJzdHJhY3RUYXNrLmphdmE=) | `63.82% <0.00%> (+4.25%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [eacfbf6...dc0b65e](https://codecov.io/gh/apache/servicecomb-java-chassis/pull/2600?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] fanjiwang1992 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
##########
@@ -81,5 +82,15 @@ private static void testValidation() {
       TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
       TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
     }
+
+    try {
+      Teacher teacher = new Teacher();
+      teacher.setAge("20");
+      template.postForObject(urlPrefix + "/sayTeacherInfo", teacher, Teacher.class);

Review comment:
       done

##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -76,6 +76,8 @@
 
   private static final Logger LOGGER = LoggerFactory.getLogger(SwaggerUtils.class);
 
+  private static String X_JAVA_CLASS = "x-java-class";

Review comment:
       done

##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -216,8 +218,14 @@ public static void addDefinitions(Swagger swagger, Type paramType) {
       if (null != swagger.getDefinitions()) {
         Model tempModel = swagger.getDefinitions().get(entry.getKey());
         if (null != tempModel && !tempModel.equals(entry.getValue())) {
-          LOGGER.warn("duplicate param model: " + entry.getKey());
-          throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          String tempModelClass = (String) tempModel.getVendorExtensions().get(X_JAVA_CLASS);
+          if (tempModelClass.equals(entry.getValue().getVendorExtensions().get(X_JAVA_CLASS))) {
+            swagger.addDefinition(entry.getKey(), tempModel);
+            continue;
+          } else {
+            LOGGER.warn("duplicate param model: " + entry.getKey());
+            throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          }
         }

Review comment:
       done




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2600: [SCB-2344] fix swagger generator duplicate param model exception bug

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



##########
File path: swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/SwaggerUtils.java
##########
@@ -210,20 +211,30 @@ public static void addDefinitions(Swagger swagger, Type paramType) {
     if (javaType.isTypeOrSubTypeOf(DynamicEnum.class)) {
       return;
     }
-
     Map<String, Model> models = ModelConverters.getInstance().readAll(javaType);
     for (Entry<String, Model> entry : models.entrySet()) {
       if (null != swagger.getDefinitions()) {
         Model tempModel = swagger.getDefinitions().get(entry.getKey());
         if (null != tempModel && !tempModel.equals(entry.getValue())) {
-          LOGGER.warn("duplicate param model: " + entry.getKey());
-          throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          if (modelNotDuplicate(tempModel, entry.getValue())) {
+            swagger.addDefinition(entry.getKey(), tempModel);
+            continue;
+          } else {
+            LOGGER.warn("duplicate param model: " + entry.getKey());
+            throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
+          }
         }
       }
       swagger.addDefinition(entry.getKey(), entry.getValue());
     }
   }
 
+  private static boolean modelNotDuplicate(Model tempModel, Model model){
+    String tempModelClass = (String) tempModel.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
+    String modelClass = (String) model.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
+    return tempModelClass.equals(modelClass);
+  }
+

Review comment:
       ```
       for (Entry<String, Model> entry : models.entrySet()) {
       if (modelNotDuplicate(swagger, entry) {
             swagger.addDefinition(entry.getKey(), entry.getValue());
       } else{
               LOGGER.warn("duplicate param model: " + entry.getKey());
               throw new IllegalArgumentException("duplicate param model: " + entry.getKey());
       }
      }
   
     /////////////////////
   
   
     private static boolean modelNotDuplicate(swagger, entry){
          if (null != swagger.getDefinitions()) {
              return false;
          }
          ....
   
           Model tempModel = swagger.getDefinitions().get(entry.getKey());
           if (null != tempModel && !tempModel.equals(entry.getValue())) 
   
   
       String tempModelClass = (String) tempModel.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
       String modelClass = (String) model.getVendorExtensions().get(SwaggerConst.EXT_JAVA_CLASS);
       return tempModelClass.equals(modelClass);
     }
   ```




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

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org