Repository: tipsy/javalin
Pull request #1266 (2021-06-02 23:35:55)

Adding Args to Error Responses
I'm having some difficulty running the tests locally, not entirely sure why. I'm getting a failing test (I think from running Vue tests?), but I'm not able to locate where the error is. Nothing is telling me what failed. That said, this is meant to be a quick example of the API I was considering. I have not added/changed any tests. I would expect them all to pass still, the idea is that the previous API works exactly as it did, no added complexity if you don't use the new overloaded functions. If this seems like a reasonable API I'll be happy to contribute some additional documentation and tests.

tipsy: Thanks @MrThreepwood, it looks good!

> I'm having some difficulty running the tests locally, not entirely sure why. I'm getting a failing test (I think from running Vue tests?), but I'm not able to locate where the error is. Nothing is telling me what failed.

Is it timing out? You're not getting any output at all?

> That said, this is meant to be a quick example of the API I was considering. I have not added/changed any tests. I would expect them all to pass still, the idea is that the previous API works exactly as it did, no added complexity if you don't use the new overloaded functions. If this seems like a reasonable API I'll be happy to contribute some additional documentation and tests.

I will have to agree, it doesn't make things noticeably more complex. You mentioned earlier that you wanted an interface - do you still want that, or you think this data class with args is sufficient?

Some tests would be great.
tipsy: I'll go ahead and merge this and add some tests, thanks @MrThreepwood !

Patch:
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 0c1ede5d..55d1e7b3 100644
--- a/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/BaseValidator.kt
@@ -8,8 +8,8 @@ package io.javalin.core.validation
 
 typealias Check<T> = (T) -> Boolean
 
-data class Rule<T>(val fieldName: String, val check: Check<T?>, val error: String)
-data class ValidationError<T>(val message: String, val value: T?)
+data class Rule<T>(val fieldName: String, val check: Check<T?>, val error: ValidationError<T>)
+data class ValidationError<T>(val message: String, val args: Map<String, Any?> = mapOf(), var value: T? = null)
 class ValidationException(val errors: Map<String, List<ValidationError<Any>>>) : Exception()
 
 open class BaseValidator<T>(val value: T?, val fieldName: String) {
@@ -18,19 +18,24 @@ open class BaseValidator<T>(val value: T?, val fieldName: String) {
     private val errors by lazy {
         val errors = mutableMapOf<String, MutableList<ValidationError<T>>>()
         if (value == null && this !is NullableValidator) {
-            errors[fieldName] = mutableListOf(ValidationError("NULLCHECK_FAILED", value))
+            errors[fieldName] = mutableListOf(ValidationError("NULLCHECK_FAILED", value = value))
         }
         rules.forEach { rule ->
             if (value != null && !rule.check(value)) {
                 // the same validator can have multiple field names if it's a BodyValidator
                 errors.computeIfAbsent(rule.fieldName) { mutableListOf() }
-                errors[rule.fieldName]!!.add(ValidationError(rule.error, value))
+                errors[rule.fieldName]!!.add(rule.error.also { it.value = value })
             }
         }
         errors.mapValues { it.value.toList() }.toMap() // make immutable
     }
 
     fun addRule(fieldName: String, check: Check<T?>, error: String): BaseValidator<T> {
+        rules.add(Rule(fieldName, check, ValidationError(error)))
+        return this
+    }
+
+    fun addRule(fieldName: String, check: Check<T?>, error: ValidationError<T>): BaseValidator<T> {
         rules.add(Rule(fieldName, check, error))
         return this
     }
diff --git a/javalin/src/main/java/io/javalin/core/validation/BodyValidator.kt b/javalin/src/main/java/io/javalin/core/validation/BodyValidator.kt
index f776ca8a..117c2384 100644
--- a/javalin/src/main/java/io/javalin/core/validation/BodyValidator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/BodyValidator.kt
@@ -8,7 +8,9 @@ package io.javalin.core.validation
 
 open class BodyValidator<T>(value: T?) : BaseValidator<T>(value, value.fieldName()) {
     fun check(check: Check<T>, error: String) = check(value.fieldName(), check, error)
+    fun check(check: Check<T>, error: ValidationError<T>) = check(value.fieldName(), check, error)
     fun check(fieldName: String, check: Check<T>, error: String) = addRule(fieldName, { check(it!!) }, error) as BodyValidator<T>
+    fun check(fieldName: String, check: Check<T>, error: ValidationError<T>) = addRule(fieldName, { check(it!!) }, error) as BodyValidator<T>
     override fun get(): T = super.get()!!
 }
 
diff --git a/javalin/src/main/java/io/javalin/core/validation/JavalinValidation.kt b/javalin/src/main/java/io/javalin/core/validation/JavalinValidation.kt
index c7e8ef2c..566d7823 100644
--- a/javalin/src/main/java/io/javalin/core/validation/JavalinValidation.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/JavalinValidation.kt
@@ -51,5 +51,6 @@ object JavalinValidation {
     }
 }
 
+
 fun Iterable<Validator<*>>.collectErrors(): Map<String, List<ValidationError<out Any?>>> =
     JavalinValidation.collectErrors(this)
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 e667fdb6..599a3da1 100644
--- a/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/NullableValidator.kt
@@ -8,4 +8,5 @@ package io.javalin.core.validation
 
 open class NullableValidator<T>(value: T?, fieldName: String) : BaseValidator<T>(value, fieldName) {
     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 f10db821..0d140501 100644
--- a/javalin/src/main/java/io/javalin/core/validation/Validator.kt
+++ b/javalin/src/main/java/io/javalin/core/validation/Validator.kt
@@ -21,6 +21,8 @@ open class Validator<T>(value: T?, fieldName: String) : BaseValidator<T>(value,
     }
 
     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>
+
     override fun get(): T = super.get()!!
 
     companion object {
@@ -33,7 +35,7 @@ open class Validator<T>(value: T?, fieldName: String) : BaseValidator<T>(value,
                 throw InternalServerErrorResponse()
             }
             JavalinLogger.info("Parameter '${fieldName}' with value '${value}' is not a valid ${clazz.simpleName}")
-            throw ValidationException(mapOf(fieldName to listOf(ValidationError("TYPE_CONVERSION_FAILED", value))))
+            throw ValidationException(mapOf(fieldName to listOf(ValidationError("TYPE_CONVERSION_FAILED", value = value))))
         }
     }
 }
diff --git a/javalin/src/main/java/io/javalin/http/Context.kt b/javalin/src/main/java/io/javalin/http/Context.kt
index 358178fc..b7dcce0e 100644
--- a/javalin/src/main/java/io/javalin/http/Context.kt
+++ b/javalin/src/main/java/io/javalin/http/Context.kt
@@ -130,7 +130,7 @@ open class Context(@JvmField val req: HttpServletRequest, @JvmField val res: Htt
         BodyValidator(JavalinJson.fromJson(body(), clazz))
     } catch (e: Exception) {
         JavalinLogger.info("Couldn't deserialize body to ${clazz.simpleName}", e)
-        throw ValidationException(mapOf(clazz.simpleName to listOf(ValidationError("DESERIALIZATION_FAILED", body()))))
+        throw ValidationException(mapOf(clazz.simpleName to listOf(ValidationError("DESERIALIZATION_FAILED", value = body()))))
     }
 
     /** Reified version of [bodyValidator] */


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestJson.kt b/javalin/src/test/java/io/javalin/TestJson.kt
index 72ad0eff..64589ff6 100644
--- a/javalin/src/test/java/io/javalin/TestJson.kt
+++ b/javalin/src/test/java/io/javalin/TestJson.kt
@@ -75,7 +75,7 @@ class TestJson {
         app.get("/hello") { it.body<NonSerializableObject>() }
         assertThat(http.get("/hello").status).isEqualTo(400)
         val response = http.getBody("/hello").replace("\\s".toRegex(), "")
-        assertThat(response).isEqualTo("""{"NonSerializableObject":[{"message":"DESERIALIZATION_FAILED","value":""}]}""")
+        assertThat(response).isEqualTo("""{"NonSerializableObject":[{"message":"DESERIALIZATION_FAILED","args":{},"value":""}]}""")
     }
 
     @Test
diff --git a/javalin/src/test/java/io/javalin/TestValidation.kt b/javalin/src/test/java/io/javalin/TestValidation.kt
index 0d05d7b5..5e35c127 100644
--- a/javalin/src/test/java/io/javalin/TestValidation.kt
+++ b/javalin/src/test/java/io/javalin/TestValidation.kt
@@ -34,19 +34,19 @@ class TestValidation {
     @Test
     fun `pathParam gives correct error message`() = TestUtil.test { app, http ->
         app.get("/:param") { ctx -> ctx.pathParam<Int>("param").get() }
-        assertThat(http.get("/abc").body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","value":"abc"}]}""")
+        assertThat(http.get("/abc").body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","args":{},"value":"abc"}]}""")
     }
 
     @Test
     fun `queryParam gives correct error message`() = TestUtil.test { app, http ->
         app.get("/") { ctx -> ctx.queryParam<Int>("param").get() }
-        assertThat(http.get("/?param=abc").body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","value":"abc"}]}""")
+        assertThat(http.get("/?param=abc").body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","args":{},"value":"abc"}]}""")
     }
 
     @Test
     fun `formParam gives correct error message`() = TestUtil.test { app, http ->
         app.post("/") { ctx -> ctx.formParam<Int>("param").get() }
-        assertThat(http.post("/").body("param=abc").asString().body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","value":"abc"}]}""")
+        assertThat(http.post("/").body("param=abc").asString().body).isEqualTo("""{"param":[{"message":"TYPE_CONVERSION_FAILED","args":{},"value":"abc"}]}""")
         JavalinLogger.enabled = true
         val log = TestUtil.captureStdOut { http.post("/").body("param=abc").asString().body }
         assertThat(log).contains("Parameter 'param' with value 'abc' is not a valid Integer")
@@ -56,7 +56,7 @@ class TestValidation {
     @Test
     fun `notNullOrEmpty works for Validator`() = TestUtil.test { app, http ->
         app.get("/") { ctx -> ctx.queryParam<String>("my-qp").get() }
-        assertThat(http.get("/").body).isEqualTo("""{"my-qp":[{"message":"NULLCHECK_FAILED","value":null}]}""")
+        assertThat(http.get("/").body).isEqualTo("""{"my-qp":[{"message":"NULLCHECK_FAILED","args":{},"value":null}]}""")
         assertThat(http.get("/").status).isEqualTo(400)
     }
 
@@ -73,8 +73,8 @@ class TestValidation {
             val myInt = ctx.queryParam<Int>("my-qp").get()
             ctx.result((myInt * 2).toString())
         }
-        assertThat(http.get("/int").body).isEqualTo("""{"my-qp":[{"message":"NULLCHECK_FAILED","value":null}]}""")
-        assertThat(http.get("/int?my-qp=abc").body).isEqualTo("""{"my-qp":[{"message":"TYPE_CONVERSION_FAILED","value":"abc"}]}""")
+        assertThat(http.get("/int").body).isEqualTo("""{"my-qp":[{"message":"NULLCHECK_FAILED","args":{},"value":null}]}""")
+        assertThat(http.get("/int?my-qp=abc").body).isEqualTo("""{"my-qp":[{"message":"TYPE_CONVERSION_FAILED","args":{},"value":"abc"}]}""")
         assertThat(http.get("/int?my-qp=123").body).isEqualTo("246")
     }
 
@@ -83,7 +83,7 @@ class TestValidation {
         app.get("/") { ctx ->
             ctx.queryParam<String>("my-qp").check({ it.length > 5 }, "Length must be more than five").get()
         }
-        assertThat(http.get("/?my-qp=1").body).isEqualTo("""{"my-qp":[{"message":"Length must be more than five","value":"1"}]}""")
+        assertThat(http.get("/?my-qp=1").body).isEqualTo("""{"my-qp":[{"message":"Length must be more than five","args":{},"value":"1"}]}""")
     }
 
     @Test
@@ -92,7 +92,7 @@ class TestValidation {
             val myInt = ctx.queryParam<Int>("my-qp", "788").get()
             ctx.result(myInt.toString())
         }
-        assertThat(http.get("/?my-qp=a").body).isEqualTo("""{"my-qp":[{"message":"TYPE_CONVERSION_FAILED","value":"a"}]}""")
+        assertThat(http.get("/?my-qp=a").body).isEqualTo("""{"my-qp":[{"message":"TYPE_CONVERSION_FAILED","args":{},"value":"a"}]}""")
         assertThat(http.get("/?my-qp=1").body).isEqualTo("1")
         assertThat(http.get("/").body).isEqualTo("788")
     }
@@ -115,7 +115,7 @@ class TestValidation {
             ctx.json(toDate.isAfter(fromDate))
         }
         assertThat(http.get("/instant?from=1262347200000&to=1262347300000").body).isEqualTo("true")
-        assertThat(http.get("/instant?from=1262347200000&to=1262347100000").body).isEqualTo("""{"to":[{"message":"'to' has to be after 'from'","value":1262347100.000000000}]}""")
+        assertThat(http.get("/instant?from=1262347200000&to=1262347100000").body).isEqualTo("""{"to":[{"message":"'to' has to be after 'from'","args":{},"value":1262347100.000000000}]}""")
     }
 
     @Test
@@ -162,10 +162,10 @@ class TestValidation {
             value1 = "Bananas"
         })
 
-        """{"SerializeableObject":[{"message":"DESERIALIZATION_FAILED","value":"not-json"}]}""".let { expected ->
+        """{"SerializeableObject":[{"message":"DESERIALIZATION_FAILED","args":{},"value":"not-json"}]}""".let { expected ->
             assertThat(http.post("/json").body("not-json").asString().body).isEqualTo(expected)
         }
-        """{"SerializeableObject":[{"message":"value1 must be 'Bananas'","value":{"value1":"FirstValue","value2":"SecondValue"}}]}""".let { expected ->
+        """{"SerializeableObject":[{"message":"value1 must be 'Bananas'","args":{},"value":{"value1":"FirstValue","value2":"SecondValue"}}]}""".let { expected ->
             assertThat(http.post("/json").body(invalidJson).asString().body).isEqualTo(expected)
         }
 
@@ -182,9 +182,9 @@ class TestValidation {
                 .get()
         }
         val expected = """{"SerializeableObject":[
-            {"message":"UnnamedFieldCheck1","value":{"value1":"FirstValue","value2":"SecondValue"}},
-            {"message":"UnnamedFieldCheck2","value":{"value1":"First Value","value2":"SecondValue"}}],
-            "named_field":[{"message":"NamedFieldCheck3","value":{"value1":"FirstValue","value2":"SecondValue"}}]}""".replace("\\s".toRegex(), "")
+            {"message":"UnnamedFieldCheck1","args":{},"value":{"value1":"FirstValue","value2":"SecondValue"}},
+            {"message":"UnnamedFieldCheck2","args":{},"value":{"value1":"First Value","value2":"SecondValue"}}],
+            "named_field":[{"message":"NamedFieldCheck3","args":{},"value":{"value1":"FirstValue","value2":"SecondValue"}}]}""".replace("\\s".toRegex(), "")
         val response = http.post("/json").body(JavalinJson.toJson(SerializeableObject())).asString().body
         assertThat(response).isEqualTo(expected)
     }
@@ -238,7 +238,7 @@ class TestValidation {
 
         // Test invalid param
         http.get("/?id=4").apply {
-            assertThat(body).isEqualTo("""{"id":[{"message":"id was not greater than 10","value":4}]}""")
+            assertThat(body).isEqualTo("""{"id":[{"message":"id was not greater than 10","args":{},"value":4}]}""")
             assertThat(status).isEqualTo(400)
         }
 
@@ -300,13 +300,13 @@ class TestValidation {
         // Test invalid param
         http.post("/").body("{\"first_name\":\"Mathilde\"}").asString().apply {
             assertThat(status).isEqualTo(200)
-            assertThat(body).isEqualTo("""{"first_name":[{"message":"Too long","value":{"first_name":"Mathilde"}}]}""")
+            assertThat(body).isEqualTo("""{"first_name":[{"message":"Too long","args":{},"value":{"first_name":"Mathilde"}}]}""")
         }
 
         // Test invalid empty param
         http.post("/").body("{}").asString().apply {
             assertThat(status).isEqualTo(200)
-            assertThat(body).isEqualTo("""{"first_name":[{"message":"This field is mandatory","value":{}}]}""")
+            assertThat(body).isEqualTo("""{"first_name":[{"message":"This field is mandatory","args":{},"value":{}}]}""")
         }
     }
 }