[openapi] Fix reflection errors with extended classes (javalin#712)

relates to comment in javalin#709
nixxcode · Aug 19, 2019 · 8a03272 · 8a03272
1 parent bbf7603
commit 8a03272
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 18 deletions.
diff --git a/src/main/java/io/javalin/core/util/ReflectionUtil.kt b/src/main/java/io/javalin/core/util/ReflectionUtil.kt
@@ -65,10 +65,18 @@ internal fun Any.getFieldValue(fieldName: String): Any {
     return field.get(this)
 }
 
-internal fun Class<*>.getDeclaredMethodByName(methodName: String): Method? = declaredMethods
-        .find { it.name == methodName }
+internal fun Class<*>.getMethodByName(methodName: String): Method? {
+    val isName = { method: Method -> method.name == methodName }
+    return declaredMethods.find(isName) ?: methods.find(isName)
+}
 
 internal fun Class<*>.getDeclaredFieldByName(methodName: String): Field? = declaredFields
         .find { it.name == methodName }
 
+internal val Class<*>.methodsNotDeclaredByObject
+    get(): Array<Method> = (declaredMethods + methods)
+            .toSet()
+            .filter { it.declaringClass != Object::class.java }
+            .toTypedArray()
+
 internal const val methodReferenceReflectionMethodName = "get\$Lambda"
diff --git a/src/main/java/io/javalin/plugin/openapi/dsl/OpenApiBuilder.kt b/src/main/java/io/javalin/plugin/openapi/dsl/OpenApiBuilder.kt
@@ -4,6 +4,7 @@ package io.javalin.plugin.openapi.dsl
 
 import io.javalin.apibuilder.CrudHandler
 import io.javalin.apibuilder.CrudHandlerLambdaKey
+import io.javalin.core.util.getMethodByName
 import io.javalin.http.Context
 import io.javalin.http.Handler
 import io.javalin.plugin.openapi.annotations.OpenApi
@@ -59,7 +60,7 @@ internal fun moveDocumentationFromAnnotationToHandler(
  * If the method is not documented, just returns the handler.
  */
 fun moveDocumentationFromAnnotationToHandler(javaClass: Class<*>, methodName: String, handler: Handler): Handler {
-    val method = javaClass.declaredMethods.find { it.name === methodName } ?: throw NoSuchMethodException(methodName)
+    val method = javaClass.getMethodByName(methodName) ?: throw NoSuchMethodException(methodName)
     return moveDocumentationFromAnnotationToHandler(method, handler)
 }
 

diff --git a/src/main/java/io/javalin/plugin/openapi/dsl/extractDocumentation.kt b/src/main/java/io/javalin/plugin/openapi/dsl/extractDocumentation.kt
@@ -4,15 +4,16 @@ import io.javalin.Javalin
 import io.javalin.core.event.HandlerMetaInfo
 import io.javalin.core.util.OptionalDependency
 import io.javalin.core.util.Util
-import io.javalin.core.util.getDeclaredMethodByName
 import io.javalin.core.util.getFieldValue
+import io.javalin.core.util.getMethodByName
 import io.javalin.core.util.isClass
 import io.javalin.core.util.isJavaAnonymousLambda
 import io.javalin.core.util.isJavaNonStaticMethodReference
 import io.javalin.core.util.isKotlinAnonymousLambda
 import io.javalin.core.util.isKotlinMethodReference
 import io.javalin.core.util.lambdaField
 import io.javalin.core.util.methodReferenceReflectionMethodName
+import io.javalin.core.util.methodsNotDeclaredByObject
 import io.javalin.plugin.openapi.CreateSchemaOptions
 import io.javalin.plugin.openapi.annotations.OpenApi
 import io.javalin.plugin.openapi.annotations.asOpenApiDocumentation
@@ -75,7 +76,7 @@ private fun HandlerMetaInfo.getOpenApiAnnotationFromReference(): OpenApi? {
 
 private fun HandlerMetaInfo.getOpenApiAnnotationFromHandler(): OpenApi? {
     return try {
-        val method = handler::class.java.declaredMethods.find { it.name == "handle" }!!
+        val method = handler::class.java.getMethodByName("handle")!!
         method.getOpenApiAnnotation()
     } catch (e: NullPointerException) {
         null
@@ -84,7 +85,7 @@ private fun HandlerMetaInfo.getOpenApiAnnotationFromHandler(): OpenApi? {
 
 private val HandlerMetaInfo.methodReferenceOfHandler: Method?
     get() = when {
-        handler.isClass -> (handler as Class<*>).declaredMethods[0]
+        handler.isClass -> (handler as Class<*>).methods[0]
         handler.isKotlinMethodReference -> {
             val functionValue = handler.getFieldValue("function") as KFunction<*>
             functionValue.javaMethod
@@ -98,24 +99,24 @@ private val HandlerMetaInfo.methodReferenceOfHandler: Method?
 private val HandlerMetaInfo.methodReferenceOfNonStaticJavaHandler: Method?
     get() {
         val handlerParentClass = handler.javaClass
-                .getDeclaredMethodByName(methodReferenceReflectionMethodName)
+                .getMethodByName(methodReferenceReflectionMethodName)
                 ?.parameters?.get(0)
                 ?.parameterizedType as Class<*>?
 
-        val declaredMethods = handlerParentClass
-                ?.declaredMethods
+        val methods = handlerParentClass
+                ?.methodsNotDeclaredByObject
                 ?: arrayOf()
 
         return when {
-            declaredMethods.isEmpty() -> null
-            declaredMethods.size == 1 -> declaredMethods[0]
+            methods.isEmpty() -> null
+            methods.size == 1 -> methods[0]
             else -> {
-                val methodThatMatchesHandler = findMethodByOpenApiAnnotation(declaredMethods)
+                val methodThatMatchesHandler = findMethodByOpenApiAnnotation(methods)
                 if (methodThatMatchesHandler != null) {
                     return methodThatMatchesHandler
                 }
 
-                val hasAnyMethodTheOpenApiAnnotation = declaredMethods.any { it.getOpenApiAnnotation() != null }
+                val hasAnyMethodTheOpenApiAnnotation = methods.any { it.getOpenApiAnnotation() != null }
                 if (hasAnyMethodTheOpenApiAnnotation && handlerParentClass != null) {
                     Javalin.log.warn("Unfortunately it is not possible to match the @OpenApi annotations to the handler in ${handlerParentClass.canonicalName}. " +
                             "Please add the `path` and the `method` information to the annotation, so the handler can be matched.")

diff --git a/src/test/java/io/javalin/openapi/TestOpenApi.kt b/src/test/java/io/javalin/openapi/TestOpenApi.kt
@@ -5,7 +5,17 @@
  */
 package io.javalin.openapi
 
-import cc.vileda.openapi.dsl.*
+import cc.vileda.openapi.dsl.components
+import cc.vileda.openapi.dsl.externalDocs
+import cc.vileda.openapi.dsl.get
+import cc.vileda.openapi.dsl.info
+import cc.vileda.openapi.dsl.openapiDsl
+import cc.vileda.openapi.dsl.path
+import cc.vileda.openapi.dsl.paths
+import cc.vileda.openapi.dsl.security
+import cc.vileda.openapi.dsl.securityScheme
+import cc.vileda.openapi.dsl.server
+import cc.vileda.openapi.dsl.tag
 import io.javalin.Javalin
 import io.javalin.TestUtil
 import io.javalin.apibuilder.ApiBuilder.crud
@@ -14,7 +24,11 @@ import io.javalin.http.Context
 import io.javalin.plugin.openapi.JavalinOpenApi
 import io.javalin.plugin.openapi.OpenApiOptions
 import io.javalin.plugin.openapi.OpenApiPlugin
-import io.javalin.plugin.openapi.dsl.*
+import io.javalin.plugin.openapi.dsl.OpenApiDocumentation
+import io.javalin.plugin.openapi.dsl.document
+import io.javalin.plugin.openapi.dsl.documentCrud
+import io.javalin.plugin.openapi.dsl.documented
+import io.javalin.plugin.openapi.dsl.documentedContent
 import io.swagger.v3.oas.models.OpenAPI
 import io.swagger.v3.oas.models.info.Info
 import io.swagger.v3.oas.models.security.SecurityScheme
@@ -224,6 +238,38 @@ class TestOpenApi {
         assertThat(actual.asJsonString()).isEqualTo(crudExampleJson)
     }
 
+    @Test
+    fun `createSchema() work with extended crudhandler without documentation`() {
+        val app = Javalin.create {}
+
+        open class BaseCrudHandler : CrudHandler {
+            override fun getAll(ctx: Context) {
+            }
+
+            override fun getOne(ctx: Context, resourceId: String) {
+            }
+
+            override fun create(ctx: Context) {
+            }
+
+            override fun update(ctx: Context, resourceId: String) {
+            }
+
+            override fun delete(ctx: Context, resourceId: String) {
+            }
+        }
+
+        open class ExtendedCrudHandler : BaseCrudHandler()
+
+        class DoubleExtendedCrudHandler : ExtendedCrudHandler()
+
+        app.routes {
+            // Should not throw exception
+            crud("users/:user-id", ExtendedCrudHandler())
+            crud("accounts/:account-id", DoubleExtendedCrudHandler())
+        }
+    }
+
     @Test
     fun `createSchema() throw error if enableOpenApi is not activated`() {
         val app = Javalin.create()

diff --git a/src/test/java/io/javalin/openapi/TestOpenApiAnnotations.kt b/src/test/java/io/javalin/openapi/TestOpenApiAnnotations.kt
@@ -137,21 +137,24 @@ fun getIgnore(ctx: Context) {
 
 // endregion complexExampleWithAnnotationsHandler
 // region handler types
-class ClassHandler : Handler {
+open class ClassHandler : Handler {
     @OpenApi(responses = [OpenApiResponse(status = "200")])
     override fun handle(ctx: Context) {
     }
 }
 
+class ExtendedClassHandler : ClassHandler()
+
 @OpenApi(responses = [OpenApiResponse(status = "200")])
 fun kotlinFunctionHandler(ctx: Context) {
 }
 
-object KotlinFieldHandlers {
+open class KotlinFieldHandlers {
     @OpenApi(responses = [OpenApiResponse(status = "200")])
     var kotlinFieldHandler = Handler { ctx -> }
 }
 
+class ExtendedKotlinFieldHandlers : KotlinFieldHandlers()
 
 // endregion handler types
 
@@ -221,6 +224,13 @@ class TestOpenApiAnnotations {
         }.assertEqualTo(simpleExample)
     }
 
+    @Test
+    fun `createOpenApiSchema() with extended class`() {
+        extractSchemaForTest {
+            it.get("/test", ExtendedClassHandler())
+        }.assertEqualTo(simpleExample)
+    }
+
     @Test
     fun `createOpenApiSchema() with kotlin function`() {
         extractSchemaForTest {
@@ -231,7 +241,14 @@ class TestOpenApiAnnotations {
     @Test
     fun `createOpenApiSchema() with kotlin field`() {
         extractSchemaForTest {
-            it.get("/test", KotlinFieldHandlers.kotlinFieldHandler)
+            it.get("/test", KotlinFieldHandlers().kotlinFieldHandler)
+        }.assertEqualTo(simpleExample)
+    }
+
+    @Test
+    fun `createOpenApiSchema() with kotlin field from extended class`() {
+        extractSchemaForTest {
+            it.get("/test", ExtendedKotlinFieldHandlers().kotlinFieldHandler)
         }.assertEqualTo(simpleExample)
     }
 }
diff --git a/src/test/java/io/javalin/openapi/TestOpenApiAnnotations_Java.java b/src/test/java/io/javalin/openapi/TestOpenApiAnnotations_Java.java
@@ -106,6 +106,9 @@ public void createHandler(Context ctx) {
     }
 }
 
+class ExtendedJavaMethodReference extends JavaMethodReference {
+}
+
 class JavaMethodReference2 {
     @OpenApi(
         path = "/test1",
@@ -224,6 +227,18 @@ public void testWithJavaMethodReference() {
         OpenApiTestUtils.assertEqualTo(schema, JsonKt.getSimpleExample());
     }
 
+    @Test
+    public void testWithExtendedJavaMethodReference() {
+        Info info = new Info().title("Example").version("1.0.0");
+        OpenApiOptions options = new OpenApiOptions(info);
+
+        OpenAPI schema = OpenApiTestUtils.extractSchemaForTest(options, app -> {
+            app.get("/test", new ExtendedJavaMethodReference()::createHandler);
+            return Unit.INSTANCE;
+        });
+        OpenApiTestUtils.assertEqualTo(schema, JsonKt.getSimpleExample());
+    }
+
     @Test
     public void testWithJavaMethodReferenceAndMultipleMethods() {
         Info info = new Info().title("Example").version("1.0.0");