Fix #6391. Avoid Retrofit crashing because of @path params defined after @query params

Author: giuliopulina

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaClientCodegen.java

@@ -1,10 +1,13 @@
 package io.swagger.codegen.languages;
 
+import static java.util.Collections.sort;
+
 import com.google.common.collect.LinkedListMultimap;
 import io.swagger.codegen.*;
 import io.swagger.codegen.languages.features.BeanValidationFeatures;
 import io.swagger.codegen.languages.features.GzipFeatures;
 import io.swagger.codegen.languages.features.PerformBeanValidationFeatures;
+
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
@@ -49,6 +52,7 @@ public class JavaClientCodegen extends AbstractJavaCodegen
     protected boolean useGzipFeature = false;
     protected boolean useRuntimeException = false;
 
+
     public JavaClientCodegen() {
         super();
         outputFolder = "generated-code" + File.separator + "java";
@@ -316,10 +320,34 @@ public Map<String, Object> postProcessOperations(Map<String, Object> objs) {
                     if (operation.returnType == null) {
                         operation.returnType = "Void";
                     }
-                    if (usesRetrofit2Library() && StringUtils.isNotEmpty(operation.path) && operation.path.startsWith("/"))
+                    if (usesRetrofit2Library() && StringUtils.isNotEmpty(operation.path) && operation.path.startsWith("/")){
                         operation.path = operation.path.substring(1);
+                    }
+
+                    // sorting operation parameters to make sure path params are parsed before query params
+                    if (operation.allParams != null) {
+                        sort(operation.allParams, new Comparator<CodegenParameter>() {
+                            @Override
+                            public int compare(CodegenParameter one, CodegenParameter another) {
+                                if (one.isPathParam && another.isQueryParam) {
+                                    return -1;
+                                }
+                                if (one.isQueryParam && another.isPathParam){
+                                    return 1;
+                                }
+
+                                return 0;
+                            }
+                        });
+                        Iterator<CodegenParameter> iterator = operation.allParams.iterator();
+                        while (iterator.hasNext()){
+                            CodegenParameter param = iterator.next();
+                            param.hasMore = iterator.hasNext();
+                        }
+                    }
                 }
             }
+
         }
 
         // camelize path variables for Feign client

modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/JavaClientCodegenTest.java

@@ -1,5 +1,8 @@
 package io.swagger.codegen.languages;
 
+import static io.swagger.codegen.languages.JavaClientCodegen.RETROFIT_2;
+
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -9,6 +12,11 @@
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import com.google.common.collect.ImmutableMap;
+
+import io.swagger.codegen.CodegenOperation;
+import io.swagger.codegen.CodegenParameter;
+
 public class JavaClientCodegenTest {
 
     private static final String VENDOR_MIME_TYPE = "application/vnd.company.v1+json";
@@ -51,15 +59,15 @@ public void testContentTypePrioritization() {
         Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(jsonMimeType)), Arrays.asList(jsonMimeType));
         Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(vendorMimeType)), Arrays.asList(vendorMimeType));
 
-        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(xmlMimeType, jsonMimeType)), 
+        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(xmlMimeType, jsonMimeType)),
                 Arrays.asList(jsonMimeType, xmlMimeType));
-        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(jsonMimeType, xmlMimeType)), 
+        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(jsonMimeType, xmlMimeType)),
                 Arrays.asList(jsonMimeType, xmlMimeType));
-        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(jsonMimeType, vendorMimeType)), 
+        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(jsonMimeType, vendorMimeType)),
                 Arrays.asList(vendorMimeType, jsonMimeType));
-        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(textMimeType, xmlMimeType)), 
+        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(textMimeType, xmlMimeType)),
                 Arrays.asList(textMimeType, xmlMimeType));
-        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(xmlMimeType, textMimeType)), 
+        Assert.assertEquals(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(xmlMimeType, textMimeType)),
                 Arrays.asList(xmlMimeType, textMimeType));
 
         System.out.println(JavaClientCodegen.prioritizeContentTypes(Arrays.asList(
@@ -74,4 +82,53 @@ public void testContentTypePrioritization() {
 
         Assert.assertNull(priContentTypes.get(3).get("hasMore"));
     }
+
+    @Test
+    public void testParametersAreCorrectlyOrderedWhenUsingRetrofit(){
+        JavaClientCodegen javaClientCodegen = new JavaClientCodegen();
+        javaClientCodegen.setLibrary(RETROFIT_2);
+
+        CodegenOperation codegenOperation = new CodegenOperation();
+        CodegenParameter queryParamRequired = createQueryParam("queryParam1", true);
+        CodegenParameter queryParamOptional = createQueryParam("queryParam2", false);
+        CodegenParameter pathParam1 = createPathParam("pathParam1");
+        CodegenParameter pathParam2 = createPathParam("pathParam2");
+
+        codegenOperation.allParams = Arrays.asList(queryParamRequired, pathParam1, pathParam2, queryParamOptional);
+        Map<String, Object> operations = ImmutableMap.<String, Object>of("operation", Arrays.asList(codegenOperation));
+
+        Map<String, Object> objs = ImmutableMap.of("operations", operations, "imports", new ArrayList<Map<String, String>>());
+
+        javaClientCodegen.postProcessOperations(objs);
+
+        Assert.assertEquals(Arrays.asList(pathParam1, pathParam2, queryParamRequired, queryParamOptional), codegenOperation.allParams);
+        Assert.assertTrue(pathParam1.hasMore);
+        Assert.assertTrue(pathParam2.hasMore);
+        Assert.assertTrue(queryParamRequired.hasMore);
+        Assert.assertFalse(queryParamOptional.hasMore);
+
+    }
+
+    private CodegenParameter createPathParam(String name) {
+        CodegenParameter codegenParameter = createStringParam(name);
+        codegenParameter.isPathParam = true;
+        return codegenParameter;
+    }
+
+    private CodegenParameter createQueryParam(String name, boolean required) {
+        CodegenParameter codegenParameter = createStringParam(name);
+        codegenParameter.isQueryParam = true;
+        codegenParameter.required = required;
+        return codegenParameter;
+    }
+
+    private CodegenParameter createStringParam(String name){
+        CodegenParameter codegenParameter = new CodegenParameter();
+        codegenParameter.paramName = name;
+        codegenParameter.baseName = name;
+        codegenParameter.dataType = "String";
+        return codegenParameter;
+    }
+
+
 }