From 8a03272efc96b9a5a8b56a4871ad06e67d9a0043 Mon Sep 17 00:00:00 2001 From: Tobias Walle Date: Mon, 19 Aug 2019 09:54:40 +0200 Subject: [PATCH] [openapi] Fix reflection errors with extended classes (#712) relates to comment in #709 --- .../io/javalin/core/util/ReflectionUtil.kt | 12 ++++- .../plugin/openapi/dsl/OpenApiBuilder.kt | 3 +- .../openapi/dsl/extractDocumentation.kt | 21 ++++---- .../java/io/javalin/openapi/TestOpenApi.kt | 50 ++++++++++++++++++- .../javalin/openapi/TestOpenApiAnnotations.kt | 23 +++++++-- .../openapi/TestOpenApiAnnotations_Java.java | 15 ++++++ 6 files changed, 106 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/javalin/core/util/ReflectionUtil.kt b/src/main/java/io/javalin/core/util/ReflectionUtil.kt index e3d22f0ae..efdaf8769 100644 --- 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 = (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 index 308174b51..dd0935f89 100644 --- 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 index f7244f34a..d77df91f2 100644 --- a/src/main/java/io/javalin/plugin/openapi/dsl/extractDocumentation.kt +++ b/src/main/java/io/javalin/plugin/openapi/dsl/extractDocumentation.kt @@ -4,8 +4,8 @@ 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 @@ -13,6 +13,7 @@ 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 index 30f2260b3..7dce37541 100644 --- 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 index 98a27994b..78bb8f49a 100644 --- 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 index 9bf66938a..9a574ef9d 100644 --- 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");