Merge pull request #3132 from springfox/bugfix/sort-ordering

Fix method sort order (#182 )

springfox-data-rest/src/main/java/springfox/documentation/spring/data/rest/ActionSpecification.java

@@ -39,6 +39,7 @@ class ActionSpecification {
   private final List<ResolvedMethodParameter> parameters;
   private final ResolvedType returnType;
   private final HandlerMethod handlerMethod;
+  private final Class<?> entityType;
   private final String name;
   private final String path;
 
@@ -50,6 +51,7 @@ class ActionSpecification {
       Set<MediaType> produces,
       Set<MediaType> consumes,
       HandlerMethod handlerMethod,
+      Class<?> entityType,
       List<ResolvedMethodParameter> parameters,
       ResolvedType returnType) {
     this.name = name;
@@ -60,6 +62,7 @@ class ActionSpecification {
     this.parameters = parameters;
     this.returnType = returnType;
     this.handlerMethod = handlerMethod;
+    this.entityType = entityType;
   }
 
   public String getName() {
@@ -95,12 +98,12 @@ public Optional<HandlerMethod> getHandlerMethod() {
   }
 
   public Optional<Class<?>> getDeclaringClass() {
-    return getHandlerMethod().map(input -> {
+    return Optional.ofNullable(getHandlerMethod().map(input -> {
       Object bean = new OptionalDeferencer<>().convert(handlerMethod.getBean());
       if (AopUtils.isAopProxy(bean)) {
         return AopUtils.getTargetClass(bean);
       }
       return (Class<?>) bean;
-    });
+    }).orElse(entityType));
   }
 }

springfox-data-rest/src/main/java/springfox/documentation/spring/data/rest/SpecificationBuilder.java

@@ -196,6 +196,7 @@ Optional<ActionSpecification> build() {
               getProduces(),
               getConsumes(),
               null,
+              getType(),
               getParameters(),
               returnType(resolver))
           );
@@ -210,6 +211,10 @@ private String actionName(
           upperCamelCaseName(property.getName()));
     }
 
+    private Class<?> getType() {
+      return context.getEntityContext().entity().get().getType();
+    }
+
     private ResolvedType returnType(TypeResolver resolver) {
       return getSupportedMethods().contains(DELETE)
              ? resolver.resolve(Void.TYPE)
@@ -305,6 +310,7 @@ Optional<ActionSpecification> build() {
               getProduces(),
               getConsumes(),
               handlerMethod,
+              getType(),
               inputParameters(),
               inferReturnType(context, handlerMethod))
           );
@@ -349,6 +355,10 @@ private List<ResolvedMethodParameter> transferResolvedMethodParameterList(
           .collect(Collectors.toList());
     }
 
+    private Class<?> getType() {
+      return context.entity().get().getType();
+    }
+
     private List<ResolvedMethodParameter> inputParameters() {
       return !getParameters().isEmpty()
              ? getParameters()

springfox-spi/src/main/java/springfox/documentation/spi/service/contexts/Orderings.java

@@ -27,6 +27,7 @@
 import springfox.documentation.spi.service.DocumentationPlugin;
 
 import java.util.Comparator;
+import java.util.stream.Collectors;
 
 import static java.util.Optional.*;
 import static springfox.documentation.RequestHandler.*;
@@ -57,19 +58,35 @@ public static Comparator<ApiDescription> apiPathCompatator() {
   }
 
   public static Comparator<ResourceGroup> resourceGroupComparator() {
-    return Comparator.comparing(ResourceGroup::getGroupName);
+    return Comparator.comparing(Orderings::qualifiedResourceGroupName);
+  }
+
+  public static String qualifiedResourceGroupName(ResourceGroup resourceGroup) {
+    return String.format("%s.%s.%s", resourceGroup.getGroupName(),
+        resourceGroup.getControllerClass().map(cls -> cls.getName()).orElse("-"),
+        resourceGroup.getPosition());
   }
 
   public static Comparator<RequestMappingContext> methodComparator() {
     return Comparator.comparing(Orderings::qualifiedMethodName);
   }
 
-  private static String qualifiedMethodName(RequestMappingContext context) {
-    return String.format("%s.%s", context.getGroupName(), context.getName());
+  public static String qualifiedMethodName(RequestMappingContext context) {
+    return String.format("%s.%s.%s.%s", context.getGroupName(),
+        context.getReturnType().getBriefDescription(), context.getName(),
+        methodParametersSignature(context));
+  }
+
+  private static String methodParametersSignature(RequestMappingContext context) {
+    return context
+        .getParameters().stream().map(p -> String.format("%s-%s",
+            p.getParameterType().getBriefDescription(), p.getParameterIndex()))
+        .collect(Collectors.joining(",", "[", "]"));
   }
 
   public static Comparator<RequestHandler> byPatternsCondition() {
-    return Comparator.comparing(requestHandler -> sortedPaths(requestHandler.getPatternsCondition()));
+    return Comparator
+        .comparing(requestHandler -> sortedPaths(requestHandler.getPatternsCondition()));
   }
 
   public static Comparator<RequestHandler> byOperationName() {
@@ -81,7 +98,8 @@ public static Comparator<? super DocumentationPlugin> pluginOrdering() {
   }
 
   public static Comparator<DocumentationPlugin> byPluginType() {
-    return Comparator.comparingInt(documentationPlugin -> documentationPlugin.getDocumentationType().hashCode());
+    return Comparator
+        .comparingInt(documentationPlugin -> documentationPlugin.getDocumentationType().hashCode());
   }
 
   public static Comparator<DocumentationPlugin> byPluginName() {

springfox-spring-web/src/test/groovy/springfox/documentation/spi/service/contexts/OrderingsSpec.groovy

@@ -1,12 +1,38 @@
 package springfox.documentation.spi.service.contexts
 
 import spock.lang.Specification
+import springfox.documentation.RequestHandler
+import springfox.documentation.RequestHandlerKey
+import springfox.documentation.service.ResolvedMethodParameter
+import springfox.documentation.service.ResourceGroup
 import springfox.documentation.spi.DocumentationType
 import springfox.documentation.spring.web.plugins.Docket
+import springfox.documentation.spring.web.readers.operation.HandlerMethodResolver
+import springfox.documentation.spring.wrapper.PatternsRequestCondition
+import springfox.documentation.spring.web.dummy.DummyClass
+import springfox.documentation.spring.web.mixins.HandlerMethodsSupport
+import springfox.documentation.spring.web.dummy.models.SameFancyPet
+import springfox.documentation.spring.web.ControllerNamingUtils
+import springfox.documentation.spring.web.dummy.controllers.GenericRestController;
+import springfox.documentation.spring.web.dummy.controllers.PetRepository;
 
 import static java.util.stream.Collectors.*
 
+import java.lang.annotation.Annotation
+
+import com.fasterxml.classmate.ResolvedType
+import com.fasterxml.classmate.TypeResolver
+
+import org.springframework.http.MediaType
+import org.springframework.web.bind.annotation.RequestMethod
+import org.springframework.web.method.HandlerMethod
+import org.springframework.web.servlet.mvc.condition.NameValueExpression
+import org.springframework.web.servlet.mvc.method.RequestMappingInfo
+
 class OrderingsSpec extends Specification {
+
+  def methodResolver = new HandlerMethodResolver(new TypeResolver())
+
   def "Orderings dont crash when docket group names are null" () {
     given:
       Docket docket1 = new Docket(DocumentationType.SPRING_WEB)
@@ -19,4 +45,140 @@ class OrderingsSpec extends Specification {
       ordered[1] == docket1
       ordered[2] == docket2
   }
-}
+
+  def "Orderings is stable when ResourceGroup is based on empty class" () {
+    given:
+      ResourceGroup group1 =  new ResourceGroup("test", PetRepository, 0)
+      ResourceGroup group2 =  new ResourceGroup("test", GenericRestController, 0)
+      ResourceGroup group3 =  new ResourceGroup("test", null, 0)
+
+    when:
+      def ordered = [group1, group2, group3].stream().sorted(Orderings.resourceGroupComparator()).collect(toList())
+    then:
+      ordered[0] == group3
+      ordered[1] == group2
+      ordered[2] == group1
+  }
+
+  def "Orderings is stable when RequestMappingContext is based on overloaded methods" () {
+    given:
+      DocumentationContext documentationContext = Mock()
+
+      RequestMappingContext context1 =  requestMappingContext("0", documentationContext, SameFancyPet)
+      RequestMappingContext context2 =  requestMappingContext("0", documentationContext, SameFancyPet, String)
+      RequestMappingContext context3 =  requestMappingContext("0", documentationContext, String)
+
+    when:
+      def ordered = [context1, context2, context3].stream().sorted(Orderings.methodComparator()).collect(toList())
+    then:
+      ordered[0] == context3
+      ordered[1] == context2
+      ordered[2] == context1
+  }
+
+  private RequestMappingContext requestMappingContext(String id, DocumentationContext documentationContext, Class<?> ... params) {
+    new RequestMappingContext(
+        id,
+        documentationContext,
+        requestHandler(methodResolver, handlerMethod(params)))
+  }
+
+  private HandlerMethod handlerMethod(Class<?> ... params) {
+    def clazz = new DummyClass()
+    Class c = clazz.getClass();
+    new HandlerMethod(clazz, c.getMethod("methodToTestOrdering", params))
+  }
+
+  def requestHandler(HandlerMethodResolver methodResolver, HandlerMethod handlerMethod) {
+    new RequestHandler() {
+          @Override
+          Class<?> declaringClass() {
+            return null
+          }
+
+          @Override
+          boolean isAnnotatedWith(Class<? extends Annotation> annotation) {
+            return false
+          }
+
+          @Override
+          PatternsRequestCondition getPatternsCondition() {
+            return null
+          }
+
+          @Override
+          String groupName() {
+            return ControllerNamingUtils.controllerNameAsGroup(handlerMethod);
+          }
+
+          @Override
+          String getName() {
+            return handlerMethod.getMethod().getName();
+          }
+
+          @Override
+          Set<RequestMethod> supportedMethods() {
+            return null
+          }
+
+          @Override
+          Set<? extends MediaType> produces() {
+            return null
+          }
+
+          @Override
+          Set<? extends MediaType> consumes() {
+            return null
+          }
+
+          @Override
+          Set<NameValueExpression<String>> headers() {
+            return null
+          }
+
+          @Override
+          Set<NameValueExpression<String>> params() {
+            return null
+          }
+
+          @Override
+          def <T extends Annotation> Optional<T> findAnnotation(Class<T> annotation) {
+            return null
+          }
+
+          RequestHandlerKey key() {
+            handlerKey == null ? null : new RequestHandlerKey([] as Set, [] as Set, [] as Set, [] as Set)
+          }
+
+          @Override
+          List<ResolvedMethodParameter> getParameters() {
+            return methodResolver.methodParameters(handlerMethod);
+          }
+
+          @Override
+          ResolvedType getReturnType() {
+            return methodResolver.methodReturnType(handlerMethod);
+          }
+
+          @Override
+          def <T extends Annotation> Optional<T> findControllerAnnotation(Class<T> annotation) {
+            return null
+          }
+
+          @Override
+          springfox.documentation.spring.wrapper.RequestMappingInfo getRequestMapping() {
+            return null
+          }
+
+          @Override
+          HandlerMethod getHandlerMethod() {
+            return null
+          }
+
+          @Override
+          RequestHandler combine(RequestHandler other) {
+            return null
+          }
+        }
+  }
+}

springfox-spring-web/src/test/java/springfox/documentation/spring/web/dummy/DummyClass.java

@@ -499,6 +499,22 @@ public PetWithJsonView methodToTestJsonView(@RequestBody @JsonView(Views.FirstVi
     throw new UnsupportedOperationException();
   }
 
+  @ResponseBody
+  public List<DummyModels.BusinessModel> methodToTestOrdering(@RequestBody SameFancyPet fancyPet) {
+    throw new UnsupportedOperationException();
+  }
+
+  @ResponseBody
+  public List<DummyModels.BusinessModel> methodToTestOrdering(@RequestBody SameFancyPet fancyPet,
+      @RequestParam String id) {
+    throw new UnsupportedOperationException();
+  }
+
+  @ResponseBody
+  public List<DummyModels.BusinessModel> methodToTestOrdering(@RequestParam String id) {
+    throw new UnsupportedOperationException();
+  }
+
   public enum BusinessType {
     PRODUCT(1),
     SERVICE(2);