Merge pull request #57 from frantuma/issue-43

fehguy · fehguy · commit 5d1402b34275 · 2016-02-15T20:12:13.000-08:00
fix #43 - Parameter types handling
diff --git a/play-2.4/swagger-play2/app/play/modules/swagger/PlayReader.java b/play-2.4/swagger-play2/app/play/modules/swagger/PlayReader.java
@@ -1,7 +1,6 @@
 package play.modules.swagger;
 
 import com.fasterxml.jackson.databind.JavaType;
-import com.fasterxml.jackson.databind.type.TypeFactory;
 import io.swagger.annotations.*;
 import io.swagger.annotations.Info;
 import io.swagger.converter.ModelConverters;
@@ -13,18 +12,22 @@
 import io.swagger.models.parameters.Parameter;
 import io.swagger.models.properties.*;
 import io.swagger.util.BaseReaderUtils;
+import io.swagger.util.Json;
 import io.swagger.util.ParameterProcessor;
 import io.swagger.util.PrimitiveType;
 import io.swagger.util.ReflectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import play.Logger;
 import play.modules.swagger.util.CrossUtil;
 import play.routes.compiler.*;
+import scala.Option;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class PlayReader {
 
@@ -192,7 +195,7 @@ private Swagger read(Class<?> cls, boolean readHidden) {
                         }
                         path.set(httpMethod, operation);
                         try {
-                            readImplicitParameters(method, operation);
+                            readImplicitParameters(method, operation, cls);
                         } catch (Exception e) {
                             throw e;
                         }
@@ -350,19 +353,19 @@ protected void readInfoConfig(SwaggerDefinition config) {
         info.getVendorExtensions().putAll(BaseReaderUtils.parseExtensions(infoConfig.extensions()));
     }
 
-    private void readImplicitParameters(Method method, Operation operation) {
+    private void readImplicitParameters(Method method, Operation operation, Class<?> cls) {
         ApiImplicitParams implicitParams = method.getAnnotation(ApiImplicitParams.class);
         if (implicitParams != null && implicitParams.value().length > 0) {
             for (ApiImplicitParam param : implicitParams.value()) {
-                Parameter p = readImplicitParam(param);
+                Parameter p = readImplicitParam(param, cls);
                 if (p != null) {
                     operation.addParameter(p);
                 }
             }
         }
     }
 
-    protected io.swagger.models.parameters.Parameter readImplicitParam(ApiImplicitParam param) {
+    protected io.swagger.models.parameters.Parameter readImplicitParam(ApiImplicitParam param, Class<?> cls) {
         final Parameter p;
         if (param.paramType().equalsIgnoreCase("path")) {
             p = new PathParameter();
@@ -380,18 +383,31 @@ protected io.swagger.models.parameters.Parameter readImplicitParam(ApiImplicitPa
         }
         Type type = null;
         // Swagger ReflectionUtils can't handle file or array datatype
-        if (!"file".equalsIgnoreCase(param.dataType()) && !"array".equalsIgnoreCase(param.dataType())){
-            type = typeFromString(param.dataType());
+        if (!"".equalsIgnoreCase(param.dataType()) && !"file".equalsIgnoreCase(param.dataType()) && !"array".equalsIgnoreCase(param.dataType())){
+            type = typeFromString(param.dataType(), cls);
+
         }
-        return ParameterProcessor.applyAnnotations(getSwagger(), p, type == null ? String.class : type, Collections.singletonList(param));
+        Parameter result =  ParameterProcessor.applyAnnotations(getSwagger(), p, type == null ? String.class : type, Collections.singletonList(param));
+
+        if (result instanceof AbstractSerializableParameter && type != null) {
+            Property schema = createProperty(type);
+            ((AbstractSerializableParameter)p).setProperty(schema);
+        }
+
+        return result;
+
     }
 
-    private static Type typeFromString(String type) {
+    private static Type typeFromString(String type, Class<?> cls) {
         final PrimitiveType primitive = PrimitiveType.fromName(type);
         if (primitive != null) {
             return primitive.getKeyClass();
         }
         try {
+            Type routeType = getOptionTypeFromString (type, cls);
+
+            if (routeType != null) return routeType;
+
             return Thread.currentThread().getContextClassLoader().loadClass(type);
         } catch (Exception e) {
             Logger.error(String.format("Failed to resolve '%s' into class", type), e);
@@ -522,37 +538,69 @@ private Operation parseMethod(Class<?> cls, Method method, Route route) {
         return operation;
     }
 
-    private Type getParamType(Class<?> cls, Method method, String simpleTypeName) {
-        Type[] genericParameterTypes = method.getGenericParameterTypes();
-        for (Type genericParameterType : genericParameterTypes) {
-            final Type type = TypeFactory.defaultInstance().constructType(genericParameterType, cls);
-            final Class<?> paramClass = ((JavaType) type).getRawClass();
-            if (simpleTypeName.equalsIgnoreCase(paramClass.getSimpleName())) {
-                return type;
-            }
-            if (simpleTypeName.equalsIgnoreCase(paramClass.getName())) {
-                return type;
+
+    final static class OptionTypeResolver {
+        private Option<Integer> optionTypeInt;
+        private Option<Long> optionTypeLong;
+        private Option<Byte> optionTypeByte;
+        private Option<Boolean> optionTypeBoolean;
+        private Option<Character> optionTypeChar;
+        private Option<Float> optionTypeFloat;
+        private Option<Double> optionTypeDouble;
+        private Option<Short> optionTypeShort;
+
+        static Type resolveOptionType (String innerType, Class<?> cls) {
+            try {
+                return Json.mapper().getTypeFactory().constructType(
+                        OptionTypeResolver.class.getDeclaredField("optionType" + innerType).getGenericType(), cls);
+            } catch (NoSuchFieldException e) {
+                return null;
             }
         }
-        return null;
     }
 
-    private List<Annotation> getParamAnnotations(Class<?> cls, Type[] genericParameterTypes, Annotation[][] paramAnnotations, String simpleTypeName, int fieldPosition) {
-        Type type = TypeFactory.defaultInstance().constructType(genericParameterTypes[fieldPosition], cls);
-        Class<?> paramClass = ((JavaType)type).getRawClass();
-        if (simpleTypeName.equalsIgnoreCase(paramClass.getSimpleName())) {
-            return Arrays.asList(paramAnnotations[fieldPosition]);
+    private static Type getOptionTypeFromString (String simpleTypeName, Class<?> cls) {
+
+        if (simpleTypeName == null) return null;
+        String regex = "(Option|scala\\.Option)\\s*\\[\\s*(Int|Long|Float|Double|Byte|Short|Char|Boolean)\\s*\\]\\s*$";
+
+        Pattern pattern = Pattern.compile(regex);
+        Matcher matcher = pattern.matcher(simpleTypeName);
+        if (matcher.find())
+        {
+            String enhancedType = matcher.group(2);
+            return OptionTypeResolver.resolveOptionType(enhancedType, cls);
+        } else {
+            return null;
+        }
+    }
+    private Type getParamType(Class<?> cls, Method method, String simpleTypeName, int position) {
+
+        try {
+            Type type = getOptionTypeFromString (simpleTypeName, cls);
+            if (type != null) return type;
+
+            Type[] genericParameterTypes = method.getGenericParameterTypes();
+            return Json.mapper().getTypeFactory().constructType(genericParameterTypes[position], cls);
+        } catch (Exception e) {
+            Logger.error(String.format("Exception getting parameter type for method %s, param %s at position %d"), e);
+            return null;
         }
-        if (simpleTypeName.equalsIgnoreCase(paramClass.getName())) {
+
+    }
+
+    private List<Annotation> getParamAnnotations(Class<?> cls, Type[] genericParameterTypes, Annotation[][] paramAnnotations, String simpleTypeName, int fieldPosition) {
+        try {
             return Arrays.asList(paramAnnotations[fieldPosition]);
+        } catch (Exception e) {
+            Logger.error(String.format("Exception getting parameter type for method %s, param %s at position %d"), e);
+            return null;
         }
-        return null;
     }
 
     private List<Annotation> getParamAnnotations(Class<?> cls, Method method, String simpleTypeName, int fieldPosition) {
         Type[] genericParameterTypes = method.getGenericParameterTypes();
         Annotation[][] paramAnnotations = method.getParameterAnnotations();
-
         List<Annotation> annotations = getParamAnnotations(cls, genericParameterTypes, paramAnnotations, simpleTypeName, fieldPosition);
         if (annotations != null) {
             return annotations;
@@ -587,7 +635,7 @@ private List<Parameter> getParameters(Class<?> cls, Method method, Route route)
             if (def.startsWith("\"") && def.endsWith("\"")){
                 def = def.substring(1,def.length()-1);
             }
-            Type type = getParamType(cls, method, p.typeName());
+            Type type = getParamType(cls, method, p.typeName(), fieldPosition);
             Property schema = createProperty(type);
             if (route.path().has(p.name())) {
                 // it's a path param
@@ -602,9 +650,7 @@ private List<Parameter> getParameters(Class<?> cls, Method method, Route route)
             }
             parameter.setName(p.name());
             List<Annotation> annotations = getParamAnnotations(cls, method, p.typeName(), fieldPosition);
-
             ParameterProcessor.applyAnnotations(getSwagger(), parameter, type, annotations);
-
             parameters.add(parameter);
             fieldPosition++;
         }
@@ -623,15 +669,15 @@ private static Set<Scheme> parseSchemes(String schemes) {
     }
 
     private static boolean isVoid(Type type) {
-        final Class<?> cls = TypeFactory.defaultInstance().constructType(type).getRawClass();
+        final Class<?> cls = Json.mapper().getTypeFactory().constructType(type).getRawClass();
         return Void.class.isAssignableFrom(cls) || Void.TYPE.isAssignableFrom(cls);
     }
 
     private static boolean isValidResponse(Type type) {
         if (type == null) {
             return false;
         }
-        final JavaType javaType = TypeFactory.defaultInstance().constructType(type);
+        final JavaType javaType = Json.mapper().getTypeFactory().constructType(type);
         if (isVoid(javaType)) {
             return false;
         }
diff --git a/play-2.4/swagger-play2/build.sbt b/play-2.4/swagger-play2/build.sbt
@@ -8,8 +8,8 @@ crossScalaVersions := Seq("2.11.6", "2.11.7")
 
 libraryDependencies ++= Seq(
   "org.slf4j"          % "slf4j-api"                  % "1.6.4",
-  "io.swagger"         % "swagger-core"               % "1.5.7",
-  "io.swagger"        %% "swagger-scala-module"       % "1.0.0",
+  "io.swagger"         % "swagger-core"               % "1.5.8-SNAPSHOT",
+  "io.swagger"        %% "swagger-scala-module"       % "1.0.2-SNAPSHOT",
   "com.typesafe.play" %% "routes-compiler"            % "2.4.6",
   "com.typesafe.play" %% "play-ebean"                 % "2.0.0"            % "test",
   "org.specs2"        %% "specs2-core"                % "3.6.6"            % "test",
diff --git a/play-2.4/swagger-play2/test/PlayApiListingCacheSpec.scala b/play-2.4/swagger-play2/test/PlayApiListingCacheSpec.scala
@@ -2,7 +2,7 @@ import java.io.File
 
 import io.swagger.config.ScannerFactory
 import io.swagger.models.{ModelImpl, HttpMethod}
-import io.swagger.models.parameters.{BodyParameter, PathParameter}
+import io.swagger.models.parameters.{QueryParameter, BodyParameter, PathParameter}
 import io.swagger.models.properties.{RefProperty, ArrayProperty}
 import play.modules.swagger._
 import org.specs2.mutable._
@@ -24,6 +24,7 @@ GET /api/pointsofinterest testdata.PointOfInterestController.list(eastingMin:Dou
 GET /api/dog testdata.DogController.list
 PUT /api/dog testdata.DogController.add1
 GET /api/cat @testdata.CatController.list
+GET /api/cat43 @testdata.CatController.testIssue43(test_issue_43_param: Option[Int])
 PUT /api/cat @testdata.CatController.add1
 GET /api/fly testdata.FlyController.list
 PUT /api/dog testdata.DogController.add1
@@ -72,11 +73,13 @@ PUT /api/dog/:id testdata.DogController.add0(id:String)
       val docRoot = ""
       val swagger = ApiListingCache.listing(docRoot, "127.0.0.1")
 
-      Logger.debug ("swagger: " + toJsonString(swagger.get))
+      Logger.debug ("swagger: " + toJsonString(swagger))
+      swagger must beSome
+
       swagger must beSome
       swagger.get.getSwagger must beEqualTo("2.0")
       swagger.get.getBasePath must beEqualTo(basePath)
-      swagger.get.getPaths.size must beEqualTo(6)
+      swagger.get.getPaths.size must beEqualTo(7)
       swagger.get.getDefinitions.size must beEqualTo(5)
       swagger.get.getHost must beEqualTo(swaggerConfig.getHost)
       swagger.get.getInfo.getContact.getName must beEqualTo(swaggerConfig.getContact)
@@ -133,6 +136,23 @@ PUT /api/dog/:id testdata.DogController.add0(id:String)
       opCatPut.getResponses.get("200").getSchema.asInstanceOf[RefProperty].getSimpleRef must beEqualTo("ActionAnyContent")
       opCatPut.getProduces must beNull
 
+      val pathCat43 = swagger.get.getPaths.get("/cat43")
+      pathCat43.getOperations.size must beEqualTo(1)
+
+      val opCatGet43 = pathCat43.getOperationMap.get(HttpMethod.GET)
+      opCatGet43.getOperationId must beEqualTo("test issue #43_nick")
+      opCatGet43.getResponses.get("200").getSchema.asInstanceOf[ArrayProperty].getItems.asInstanceOf[RefProperty].getSimpleRef must beEqualTo("Cat")
+
+      opCatGet43.getParameters.head.getName must beEqualTo("test_issue_43_param")
+      opCatGet43.getParameters.head.getIn must beEqualTo("query")
+      opCatGet43.getParameters.head.asInstanceOf[QueryParameter].getType must beEqualTo("integer")
+
+      opCatGet43.getParameters.get(1).getName must beEqualTo("test_issue_43_implicit_param")
+      opCatGet43.getParameters.get(1).getIn must beEqualTo("query")
+      opCatGet43.getParameters.get(1).asInstanceOf[QueryParameter].getType must beEqualTo("integer")
+
+
+
       val pathDog = swagger.get.getPaths.get("/dog")
       pathDog.getOperations.size must beEqualTo(2)
 
diff --git a/play-2.4/swagger-play2/test/testdata/CatController.scala b/play-2.4/swagger-play2/test/testdata/CatController.scala
@@ -55,6 +55,18 @@ class CatController extends Controller {
     def no_route = Action {
       request => Ok("test case")
     }
+
+  @ApiOperation(value = "test issue #43",
+    nickname = "test issue #43_nick",
+    notes = "test issue #43_notes",
+    response = classOf[testdata.Cat],
+    responseContainer = "List",
+    httpMethod = "GET")
+  @ApiImplicitParams(Array(
+    new ApiImplicitParam(name = "test_issue_43_implicit_param", dataType = "Option[Int]", value = "test issue #43 implicit param", paramType = "query")))
+  def testIssue43(test_issue_43_param:  Option[Int]) = Action {
+    request => Ok("test issue #43")
+  }
 }
 
 case class Cat(id: Long, name: String)
