Merge pull request #113 from Szer/enum-validation

Added validation error on parsing enum values outside of valid enum values
papsign · Nov 2, 2021 · 3b3429f · 3b3429f
2 parents 60f21e7 + 88738df
commit 3b3429f
Show file tree

Hide file tree

Showing 6 changed files with 445 additions and 4 deletions.
diff --git a/src/main/kotlin/com/papsign/ktor/openapigen/annotations/type/enum/StrictEnumParsing.kt b/src/main/kotlin/com/papsign/ktor/openapigen/annotations/type/enum/StrictEnumParsing.kt
@@ -0,0 +1,5 @@
+package com.papsign.ktor.openapigen.annotations.type.enum
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.RUNTIME)
+annotation class StrictEnumParsing
diff --git a/...tlin/com/papsign/ktor/openapigen/parameters/parsers/converters/primitive/EnumConverter.kt b/...tlin/com/papsign/ktor/openapigen/parameters/parsers/converters/primitive/EnumConverter.kt
@@ -1,19 +1,36 @@
 package com.papsign.ktor.openapigen.parameters.parsers.converters.primitive
 
+import com.papsign.ktor.openapigen.annotations.type.enum.StrictEnumParsing
+import com.papsign.ktor.openapigen.exceptions.OpenAPIBadContentException
 import com.papsign.ktor.openapigen.parameters.parsers.converters.Converter
 import com.papsign.ktor.openapigen.parameters.parsers.converters.ConverterSelector
+import kotlin.reflect.KClass
 import kotlin.reflect.KType
+import kotlin.reflect.full.findAnnotation
 import kotlin.reflect.jvm.jvmErasure
 
-class EnumConverter(type: KType): Converter {
+class EnumConverter(val type: KType) : Converter {
+
+    private val isStrictParsing = (type.classifier as? KClass<*>)?.findAnnotation<StrictEnumParsing>() != null
 
     private val enumMap = type.jvmErasure.java.enumConstants.associateBy { it.toString() }
 
     override fun convert(value: String): Any? {
-        return enumMap[value]
+        if (!isStrictParsing)
+            return enumMap[value]
+
+        if (enumMap.containsKey(value)) {
+            return enumMap[value]
+        } else {
+            throw OpenAPIBadContentException(
+                "Invalid value [$value] for enum parameter of type ${type.jvmErasure.simpleName}. Expected: [${
+                    enumMap.values.joinToString(",")
+                }]"
+            )
+        }
     }
 
-    companion object: ConverterSelector {
+    companion object : ConverterSelector {
         override fun canHandle(type: KType): Boolean {
             return type.jvmErasure.java.isEnum
         }

diff --git a/src/test/kotlin/com/papsign/ktor/openapigen/EnumNonStrictTestServer.kt b/src/test/kotlin/com/papsign/ktor/openapigen/EnumNonStrictTestServer.kt
@@ -0,0 +1,163 @@
+package com.papsign.ktor.openapigen
+
+import com.papsign.ktor.openapigen.annotations.Path
+import com.papsign.ktor.openapigen.annotations.parameters.QueryParam
+import com.papsign.ktor.openapigen.exceptions.OpenAPIBadContentException
+import com.papsign.ktor.openapigen.exceptions.OpenAPIRequiredFieldException
+import com.papsign.ktor.openapigen.route.apiRouting
+import com.papsign.ktor.openapigen.route.path.normal.get
+import com.papsign.ktor.openapigen.route.response.respond
+import io.ktor.application.*
+import io.ktor.features.*
+import io.ktor.http.*
+import io.ktor.response.*
+import io.ktor.server.testing.*
+import kotlin.test.*
+
+enum class NonStrictTestEnum {
+    VALID,
+    ALSO_VALID,
+}
+
+@Path("/")
+data class NullableNonStrictEnumParams(@QueryParam("") val type: NonStrictTestEnum? = null)
+
+@Path("/")
+data class NonNullableNonStrictEnumParams(@QueryParam("") val type: NonStrictTestEnum)
+
+class NonStrictEnumTestServer {
+
+    companion object {
+        // test server for nullable enums
+        private fun Application.nullableEnum() {
+            install(OpenAPIGen)
+            install(StatusPages) {
+                exception<OpenAPIBadContentException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+            }
+            apiRouting {
+                get<NullableNonStrictEnumParams, String> { params ->
+                    if (params.type != null)
+                        assertTrue { NonStrictTestEnum.values().contains(params.type) }
+                    respond(params.type?.toString() ?: "null")
+                }
+            }
+        }
+
+        // test server for non-nullable enums
+        private fun Application.nonNullableEnum() {
+            install(OpenAPIGen)
+            install(StatusPages) {
+                exception<OpenAPIRequiredFieldException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+                exception<OpenAPIBadContentException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+            }
+            apiRouting {
+                get<NonNullableNonStrictEnumParams, String> { params ->
+                    assertTrue { NonStrictTestEnum.values().contains(params.type) }
+                    respond(params.type.toString())
+                }
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum could be omitted and it will be null`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("null", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum should be parsed correctly`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("VALID", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=ALSO_VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("ALSO_VALID", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum parsing should be case-sensitive and should return 200 with null result`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=valid").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("null", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=also_valid").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("null", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum parsing should return 200 with null result on parse values outside of enum`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=what").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("null", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum cannot be omitted`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals("The field type is required", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum should be parsed correctly`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("VALID", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=ALSO_VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("ALSO_VALID", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum parsing should be case-sensitive and should throw on passing wrong case`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals("The field type is required", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=also_valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals("The field type is required", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum parsing should not parse values outside of enum`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=what").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals("The field type is required", response.content)
+            }
+        }
+    }
+}
diff --git a/src/test/kotlin/com/papsign/ktor/openapigen/EnumStrictTestServer.kt b/src/test/kotlin/com/papsign/ktor/openapigen/EnumStrictTestServer.kt
@@ -0,0 +1,183 @@
+package com.papsign.ktor.openapigen
+
+import com.papsign.ktor.openapigen.annotations.Path
+import com.papsign.ktor.openapigen.annotations.parameters.QueryParam
+import com.papsign.ktor.openapigen.annotations.type.enum.StrictEnumParsing
+import com.papsign.ktor.openapigen.exceptions.OpenAPIBadContentException
+import com.papsign.ktor.openapigen.exceptions.OpenAPIRequiredFieldException
+import com.papsign.ktor.openapigen.route.apiRouting
+import com.papsign.ktor.openapigen.route.path.normal.get
+import com.papsign.ktor.openapigen.route.response.respond
+import io.ktor.application.*
+import io.ktor.features.*
+import io.ktor.http.*
+import io.ktor.response.*
+import io.ktor.server.testing.*
+import kotlin.test.*
+
+@StrictEnumParsing
+enum class StrictTestEnum {
+    VALID,
+    ALSO_VALID,
+}
+
+@Path("/")
+data class NullableStrictEnumParams(@QueryParam("") val type: StrictTestEnum? = null)
+
+@Path("/")
+data class NonNullableStrictEnumParams(@QueryParam("") val type: StrictTestEnum)
+
+class EnumStrictTestServer {
+
+    companion object {
+        // test server for nullable enums
+        private fun Application.nullableEnum() {
+            install(OpenAPIGen)
+            install(StatusPages) {
+                exception<OpenAPIBadContentException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+            }
+            apiRouting {
+                get<NullableStrictEnumParams, String> { params ->
+                    if (params.type != null)
+                        assertTrue { StrictTestEnum.values().contains(params.type) }
+                    respond(params.type?.toString() ?: "null")
+                }
+            }
+        }
+
+        // test server for non-nullable enums
+        private fun Application.nonNullableEnum() {
+            install(OpenAPIGen)
+            install(StatusPages) {
+                exception<OpenAPIRequiredFieldException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+                exception<OpenAPIBadContentException> { e ->
+                    call.respond(HttpStatusCode.BadRequest, e.localizedMessage)
+                }
+            }
+            apiRouting {
+                get<NonNullableStrictEnumParams, String> { params ->
+                    assertTrue { StrictTestEnum.values().contains(params.type) }
+                    respond(params.type.toString())
+                }
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum could be omitted and it will be null`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("null", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum should be parsed correctly`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("VALID", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=ALSO_VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("ALSO_VALID", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum parsing should be case-sensitive and should throw on passing wrong case`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [valid] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+            handleRequest(HttpMethod.Get, "/?type=also_valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [also_valid] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+        }
+    }
+
+    @Test
+    fun `nullable enum parsing should not parse values outside of enum`() {
+        withTestApplication({ nullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=what").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [what] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum cannot be omitted`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals("The field type is required", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum should be parsed correctly`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("VALID", response.content)
+            }
+            handleRequest(HttpMethod.Get, "/?type=ALSO_VALID").apply {
+                assertEquals(HttpStatusCode.OK, response.status())
+                assertEquals("ALSO_VALID", response.content)
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum parsing should be case-sensitive and should throw on passing wrong case`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [valid] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+            handleRequest(HttpMethod.Get, "/?type=also_valid").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [also_valid] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+        }
+    }
+
+    @Test
+    fun `non-nullable enum parsing should not parse values outside of enum`() {
+        withTestApplication({ nonNullableEnum() }) {
+            handleRequest(HttpMethod.Get, "/?type=what").apply {
+                assertEquals(HttpStatusCode.BadRequest, response.status())
+                assertEquals(
+                    "Invalid value [what] for enum parameter of type StrictTestEnum. Expected: [VALID,ALSO_VALID]",
+                    response.content
+                )
+            }
+        }
+    }
+}