Repository: tipsy/javalin
Pull request #1396 (2021-09-19 22:33:35)

issue #1393 Create validator for typed value
Second attempt of solving issue #1393. This time without extending class hierarchy.


Patch:
diff --git a/README.md b/README.md
index 28064a48..c7db8aaa 100644
--- a/README.md
+++ b/README.md
@@ -34,14 +34,14 @@ General information:
 <dependency>
     <groupId>io.javalin</groupId>
     <artifactId>javalin</artifactId>
-    <version>4.0.0</version>
+    <version>4.0.1</version>
 </dependency>
 ```
 
 #### Gradle
 
 ```groovy
-implementation "io.javalin:javalin:4.0.0"
+implementation "io.javalin:javalin:4.0.1"
 ```
 
 ### Start programming (Java)
diff --git a/javalin-bundle/pom.xml b/javalin-bundle/pom.xml
index b9a55487..7e390897 100644
--- a/javalin-bundle/pom.xml
+++ b/javalin-bundle/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin-graphql/pom.xml b/javalin-graphql/pom.xml
index 54d4f6bd..ff3000ce 100644
--- a/javalin-graphql/pom.xml
+++ b/javalin-graphql/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin-openapi/pom.xml b/javalin-openapi/pom.xml
index cc3ed5ef..e9670cef 100644
--- a/javalin-openapi/pom.xml
+++ b/javalin-openapi/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin-without-jetty/pom.xml b/javalin-without-jetty/pom.xml
index 3ddc5ef9..e3286100 100644
--- a/javalin-without-jetty/pom.xml
+++ b/javalin-without-jetty/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin/pom.xml b/javalin/pom.xml
index 9d7fc5d0..4b107685 100644
--- a/javalin/pom.xml
+++ b/javalin/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin/src/main/java/io/javalin/core/util/OptionalDependency.kt b/javalin/src/main/java/io/javalin/core/util/OptionalDependency.kt
index 0d754be1..94a7f68a 100644
--- a/javalin/src/main/java/io/javalin/core/util/OptionalDependency.kt
+++ b/javalin/src/main/java/io/javalin/core/util/OptionalDependency.kt
@@ -31,8 +31,8 @@ enum class OptionalDependency(val displayName: String, val testClass: String, va
     JVMBROTLI("Jvm-Brotli", "com.nixxcode.jvmbrotli.common.BrotliLoader", "com.nixxcode.jvmbrotli", "jvmbrotli", "0.2.0"),
 
     // Templating
-    JTE("jte", "gg.jte.TemplateEngine", "gg.jte", "jte", "1.11.0"),
-    JTE_KOTLIN("jte-kotlin", "gg.jte.compiler.kotlin.KotlinClassCompiler", "gg.jte", "jte-kotlin", "1.11.0"),
+    JTE("jte", "gg.jte.TemplateEngine", "gg.jte", "jte", "1.12.0"),
+    JTE_KOTLIN("jte-kotlin", "gg.jte.compiler.kotlin.KotlinClassCompiler", "gg.jte", "jte-kotlin", "1.12.0"),
     VELOCITY("Velocity", "org.apache.velocity.app.VelocityEngine", "org.apache.velocity", "velocity-engine-core", "2.3"),
     FREEMARKER("Freemarker", "freemarker.template.Configuration", "org.freemarker", "freemarker", "2.3.30"),
     THYMELEAF("Thymeleaf", "org.thymeleaf.TemplateEngine", "org.thymeleaf", "thymeleaf", "3.0.12.RELEASE"),
diff --git a/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt b/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt
index aa5049c8..035d1e2d 100644
--- a/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt
@@ -15,28 +15,40 @@ data class Rule<T>(val fieldName: String, val check: Check<T?>, val error: Valid
 data class ValidationError<T>(val message: String, val args: Map<String, Any?> = mapOf(), var value: Any? = null)
 class ValidationException(val errors: Map<String, List<ValidationError<Any>>>) : RuntimeException()
 
-open class BaseValidator<T>(val stringValue: String?, val clazz: Class<T>, val fieldName: String, jsonMapper: JsonMapper? = null) {
-    private var typedValue: T? = null
+data class StringSource<T>(
+    val stringValue: String?,
+    val clazz: Class<T>,
+    val jsonMapper: JsonMapper? = null
+)
+
+open class BaseValidator<T>(val fieldName: String, protected var typedValue: T?, protected val stringSource: StringSource<T>?) {
     internal val rules = mutableListOf<Rule<T>>()
+
+    constructor(stringValue: String?, clazz: Class<T>, fieldName: String, jsonMapper: JsonMapper? = null) :
+        this(fieldName, null, StringSource<T>(stringValue, clazz, jsonMapper))
+
     private val errors by lazy {
-        if (this is BodyValidator) {
-            try {
-                typedValue = jsonMapper!!.fromJsonString(stringValue!!, clazz)
-            } catch (e: Exception) {
-                JavalinLogger.info("Couldn't deserialize body to ${clazz.simpleName}", e)
-                return@lazy mapOf(REQUEST_BODY to listOf(ValidationError("DESERIALIZATION_FAILED", value = stringValue)))
-            }
-        } else if (this is NullableValidator || this is Validator) {
-            try {
-                typedValue = JavalinValidation.convertValue(clazz, stringValue)
-            } catch (e: Exception) {
-                JavalinLogger.info("Parameter '${fieldName}' with value '${stringValue}' is not a valid ${clazz.simpleName}")
-                return@lazy mapOf(fieldName to listOf(ValidationError("TYPE_CONVERSION_FAILED", value = stringValue)))
-            }
-            if (this !is NullableValidator && typedValue == null) { // only check typedValue - null might map to 0, which could be valid?
-                return@lazy mapOf(fieldName to listOf(ValidationError("NULLCHECK_FAILED", value = stringValue)))
+        if (stringSource != null) {
+            if (this is BodyValidator) {
+                try {
+                    typedValue = stringSource.jsonMapper!!.fromJsonString(stringSource.stringValue!!, stringSource.clazz)
+                } catch (e: Exception) {
+                    JavalinLogger.info("Couldn't deserialize body to ${stringSource.clazz.simpleName}", e)
+                    return@lazy mapOf(REQUEST_BODY to listOf(ValidationError("DESERIALIZATION_FAILED", value = stringSource.stringValue)))
+                }
+            } else if (this is NullableValidator || this is Validator) {
+                try {
+                    typedValue = JavalinValidation.convertValue(stringSource.clazz, stringSource.stringValue)
+                } catch (e: Exception) {
+                    JavalinLogger.info("Parameter '$fieldName' with value '${stringSource.stringValue}' is not a valid ${stringSource.clazz.simpleName}")
+                    return@lazy mapOf(fieldName to listOf(ValidationError("TYPE_CONVERSION_FAILED", value = stringSource.stringValue)))
+                }
+                if (this !is NullableValidator && typedValue == null) { // only check typedValue - null might map to 0, which could be valid?
+                    return@lazy mapOf(fieldName to listOf(ValidationError("NULLCHECK_FAILED", value = stringSource.stringValue)))
+                }
             }
         }
+
         /** after this point [typedValue] replaces [stringValue] */
         val errors = mutableMapOf<String, MutableList<ValidationError<T>>>()
         rules.filter { !it.check(typedValue) }.forEach { failedRule ->
@@ -63,5 +75,4 @@ open class BaseValidator<T>(val stringValue: String?, val clazz: Class<T>, val f
     }
 
     fun errors(): Map<String, List<ValidationError<T>>> = errors
-
 }
diff --git a/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt b/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt
index 720377c4..a1c42f1e 100644
--- a/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt
@@ -6,7 +6,9 @@
 
 package io.javalin.core.validation
 
-open class NullableValidator<T>(stringValue: String?, clazz: Class<T>, fieldName: String) : BaseValidator<T>(stringValue, clazz, fieldName) {
+open class NullableValidator<T>(fieldName: String, typedValue: T? = null, stringSource: StringSource<T>? = null) : BaseValidator<T>(fieldName, typedValue, stringSource) {
+    constructor(stringValue: String?, clazz: Class<T>, fieldName: String) :
+        this(fieldName, null, StringSource<T>(stringValue, clazz))
     fun check(check: Check<T?>, error: String) = addRule(fieldName, check, error) as NullableValidator<T>
     fun check(check: Check<T?>, error: ValidationError<T>) = addRule(fieldName, check, error) as NullableValidator<T>
 }
diff --git a/javalin/src/main/java/io/javalin/core/validation/Validator.kt b/javalin/src/main/java/io/javalin/core/validation/Validator.kt
index 861eac42..3ec1c541 100644
--- a/javalin/src/main/java/io/javalin/core/validation/Validator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/Validator.kt
@@ -14,23 +14,25 @@ import org.jetbrains.annotations.NotNull
  * The non-nullable [Validator] uses [Rule] rules, but checks if value is null before calling them.
  * The [check] method wraps its non-nullable predicate in a nullable predicate
  */
-open class Validator<T>(stringValue: String?, clazz: Class<T>, fieldName: String) : BaseValidator<T>(stringValue, clazz, fieldName) {
+open class Validator<T>(fieldName: String, typedValue: T? = null, stringSource: StringSource<T>? = null) : BaseValidator<T>(fieldName, typedValue, stringSource) {
+
+    constructor(stringValue: String?, clazz: Class<T>, fieldName: String) :
+        this(fieldName, null, StringSource<T>(stringValue, clazz))
 
     fun allowNullable(): NullableValidator<T> {
-        if (this.rules.isEmpty()) return NullableValidator(stringValue, clazz, fieldName)
+        if (this.rules.isEmpty()) return NullableValidator(fieldName, typedValue, stringSource)
         throw IllegalStateException("Validator#allowNullable must be called before adding rules")
     }
 
     fun check(check: Check<T>, error: String) = addRule(fieldName, { check(it!!) }, error) as Validator<T>
     fun check(check: Check<T>, error: ValidationError<T>) = addRule(fieldName, { check(it!!) }, error) as Validator<T>
 
+    fun hasValue() = stringSource?.stringValue != null || typedValue != null
+
     @NotNull // there is a null-check in BaseValidator
     override fun get(): T = super.get()!!
 
-    fun getOrDefault(default: T): T = when {
-        stringValue == null -> default
-        else -> super.get()!!
-    }
+    fun getOrDefault(default: T) = if (hasValue()) super.get()!! else default
 
     companion object {
         @JvmStatic
diff --git a/javalin/src/main/java/io/javalin/http/JavalinResponseWrapper.kt b/javalin/src/main/java/io/javalin/http/JavalinResponseWrapper.kt
index 5c504bb7..c907b399 100644
--- a/javalin/src/main/java/io/javalin/http/JavalinResponseWrapper.kt
+++ b/javalin/src/main/java/io/javalin/http/JavalinResponseWrapper.kt
@@ -108,7 +108,7 @@ class OutputStreamWrapper(val res: HttpServletResponse, private val rwc: Respons
     fun finalize() {
         when {
             brotliEnabled && res.getHeader(CONTENT_ENCODING) == BR -> (compressingStream as BrotliOutputStream).close()
-            gzipEnabled && res.getHeader(CONTENT_ENCODING) == GZIP -> (compressingStream as LeveledGzipStream).finish()
+            gzipEnabled && res.getHeader(CONTENT_ENCODING) == GZIP -> (compressingStream as LeveledGzipStream).close()
         }
     }
 
diff --git a/pom.xml b/pom.xml
index 5bc137a5..0fba1353 100644
--- a/pom.xml
+++ b/pom.xml
@@ -18,7 +18,7 @@
 
     <groupId>io.javalin</groupId>
     <artifactId>javalin-parent</artifactId>
-    <version>4.0.1-SNAPSHOT</version>
+    <version>4.0.2-SNAPSHOT</version>
 
     <name>Javalin (parent)</name>
     <description>Javalin: Simple REST APIs for Java and Kotlin</description>
@@ -58,7 +58,7 @@
 
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
-        <kotlin.version>1.5.30</kotlin.version>
+        <kotlin.version>1.5.31</kotlin.version>
         <kotlin.compiler.jvmTarget>1.8</kotlin.compiler.jvmTarget>
         <kotlin.compiler.incremental>true</kotlin.compiler.incremental>
         <maven.build.timestamp.format>yyyy-MM-dd'T'HH:mm:ss'Z'</maven.build.timestamp.format>
@@ -99,7 +99,7 @@
         <swagger.ui.version>3.43.0</swagger.ui.version>
         <thymeleaf.version>3.0.12.RELEASE</thymeleaf.version>
         <velocity.version>2.3</velocity.version>
-        <jte.version>1.11.0</jte.version>
+        <jte.version>1.12.0</jte.version>
         <graphql-kotlin.version>4.1.1</graphql-kotlin.version>
         <reactor-core.version>3.2.9.RELEASE</reactor-core.version>
 


Test patch:
diff --git a/javalin-testtools/pom.xml b/javalin-testtools/pom.xml
index be593039..8dc17791 100644
--- a/javalin-testtools/pom.xml
+++ b/javalin-testtools/pom.xml
@@ -3,7 +3,7 @@
     <parent>
         <artifactId>javalin-parent</artifactId>
         <groupId>io.javalin</groupId>
-        <version>4.0.1-SNAPSHOT</version>
+        <version>4.0.2-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 
diff --git a/javalin/src/test/java/io/javalin/TestLogging.kt b/javalin/src/test/java/io/javalin/TestLogging.kt
index 370f6152..399117ea 100644
--- a/javalin/src/test/java/io/javalin/TestLogging.kt
+++ b/javalin/src/test/java/io/javalin/TestLogging.kt
@@ -90,7 +90,11 @@ class TestLogging {
             val stream = File(imagePath.toURI()).inputStream()
             it.result(stream)
         }
-        val log = captureStdOut { http.getBody("/") }
+        val log = captureStdOut {
+            http.getBody("/")
+            http.getBody("/")
+            // TODO: why must this be called twice on windows to avoid empty log output?
+        }
         assertThat(log).contains("Body is binary (not logged)")
     }
 }
diff --git a/javalin/src/test/java/io/javalin/TestValidation.kt b/javalin/src/test/java/io/javalin/TestValidation.kt
index 4e2448ce..76798166 100644
--- a/javalin/src/test/java/io/javalin/TestValidation.kt
+++ b/javalin/src/test/java/io/javalin/TestValidation.kt
@@ -12,6 +12,7 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
 import io.javalin.core.util.JavalinLogger
 import io.javalin.core.validation.JavalinValidation
+import io.javalin.core.validation.NullableValidator
 import io.javalin.core.validation.ValidationError
 import io.javalin.core.validation.ValidationException
 import io.javalin.core.validation.Validator
@@ -340,4 +341,78 @@ class TestValidation {
         assertThat(http.getBody("/?number=20&locale=en")).contains("The value has to at least 6 and at most 9")
         assertThat(http.getBody("/?number=20&locale=fr")).contains("La valeur doit au moins 6 et au plus 9")
     }
+
+    data class KeyValuePair(val key: String, val value: String)
+
+    @Test
+    fun `typed value non-nullable validator works for positive case`() {
+        val validator = Validator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it.key == "key" }, "unexpected key")
+        validator.check({ it.value == "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).isEmpty()
+    }
+
+    @Test
+    fun `typed value non-nullable validator works for negative case #1`() {
+        val validator = Validator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it.key != "key" }, "unexpected key")
+        validator.check({ it.value == "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).hasSize(1)
+        assertThat(errors["kvp"]).hasSize(1)
+    }
+
+    @Test
+    fun `typed value non-nullable validator works for negative case #2`() {
+        val validator = Validator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it.key != "key" }, "unexpected key")
+        validator.check({ it.value != "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).hasSize(1)
+        assertThat(errors["kvp"]).hasSize(2)
+    }
+
+    @Test
+    fun `typed value nullable validator works for positive case`() {
+        val validator = NullableValidator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it!!.key == "key" }, "unexpected key")
+        validator.check({ it!!.value == "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).isEmpty()
+    }
+
+    @Test
+    fun `typed value nullable validator works for negative case #1`() {
+        val validator = NullableValidator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it!!.key != "key" }, "unexpected key")
+        validator.check({ it!!.value == "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).hasSize(1)
+        assertThat(errors["kvp"]).hasSize(1)
+    }
+
+    @Test
+    fun `typed value nullable validator works for negative case #2`() {
+        val validator = NullableValidator("kvp", KeyValuePair("key", "value"))
+        validator.check({ it!!.key != "key" }, "unexpected key")
+        validator.check({ it!!.value != "value" }, "unexpected value")
+        val errors = validator.errors()
+        assertThat(errors).hasSize(1)
+        assertThat(errors["kvp"]).hasSize(2)
+    }
+
+    @Test
+    fun `typed value nullable validator works for null value`() {
+        val validator = NullableValidator<KeyValuePair>("kvp")
+        validator.check({ it!!.key == "key" }, "unexpected key")
+        validator.check({ it!!.value == "value" }, "unexpected value")
+    }
+
+    @Test
+    fun `typed value nullable validator constructed from a non-nullable one works for null value`() {
+        val validator = Validator<KeyValuePair>("kvp").allowNullable()
+        validator.check({ it!!.key == "key" }, "unexpected key")
+        validator.check({ it!!.value == "value" }, "unexpected value")
+    }
 }