From ca5afa8b244b28845c65218f59d89b6bb35e370a Mon Sep 17 00:00:00 2001 From: adamw Date: Thu, 5 May 2022 12:16:42 +0200 Subject: [PATCH 1/3] Improve validators by introducing a dedicated ValidationResult type and simplifying ValidationError --- .../src/main/scala/sttp/tapir/Validator.scala | 120 ++++++------------ .../tapir/SchemaApplyValidationTest.scala | 40 +++--- .../test/scala/sttp/tapir/ValidatorTest.scala | 66 +++++----- .../apispec/schema/TSchemaToASchema.scala | 1 + .../tapir/integ/cats/ValidatorCatsTest.scala | 2 +- .../codec/refined/TapirCodecRefined.scala | 19 ++- .../codec/refined/TapirCodecRefinedTest.scala | 18 +-- .../decodefailure/DecodeFailureHandler.scala | 17 +-- 8 files changed, 118 insertions(+), 165 deletions(-) diff --git a/core/src/main/scala/sttp/tapir/Validator.scala b/core/src/main/scala/sttp/tapir/Validator.scala index 6e7fade437..ec10a2b6e3 100644 --- a/core/src/main/scala/sttp/tapir/Validator.scala +++ b/core/src/main/scala/sttp/tapir/Validator.scala @@ -77,94 +77,52 @@ object Validator extends ValidatorMacros { def enumeration[T](possibleValues: List[T], encode: EncodeToRaw[T], name: Option[SName] = None): Validator.Enumeration[T] = Enumeration(possibleValues, Some(encode), name) - /** Create a custom validator - * @param doValidate - * Validation function + /** Create a custom validator. + * @param validationLogic + * The logic of the validator * @param showMessage - * Custom message + * Description of the validator used when invoking [[Validator.show]]. */ - def custom[T](doValidate: T => List[ValidationError[_]], showMessage: Option[String] = None): Validator[T] = - Custom(doValidate, showMessage) + def custom[T](validationLogic: T => ValidationResult, showMessage: Option[String] = None): Validator[T] = + Custom(validationLogic, showMessage) // ---------- PRIMITIVE ---------- - sealed trait Primitive[T] extends Validator[T] - case class Min[T](value: T, exclusive: Boolean)(implicit val valueIsNumeric: Numeric[T]) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (implicitly[Numeric[T]].gt(t, value) || (!exclusive && implicitly[Numeric[T]].equiv(t, value))) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } + sealed trait Primitive[T] extends Validator[T] { + def doValidate(t: T): ValidationResult + override def apply(t: T): List[ValidationError[T]] = doValidate(t) match { + case ValidationResult.Valid => Nil + case ValidationResult.Invalid(customMessage) => List(ValidationError(this, t, Nil, customMessage)) } } + case class Min[T](value: T, exclusive: Boolean)(implicit val valueIsNumeric: Numeric[T]) extends Primitive[T] { + override def doValidate(t: T): ValidationResult = + ValidationResult.validWhen(implicitly[Numeric[T]].gt(t, value) || (!exclusive && implicitly[Numeric[T]].equiv(t, value))) + } case class Max[T](value: T, exclusive: Boolean)(implicit val valueIsNumeric: Numeric[T]) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (implicitly[Numeric[T]].lt(t, value) || (!exclusive && implicitly[Numeric[T]].equiv(t, value))) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: T): ValidationResult = + ValidationResult.validWhen(implicitly[Numeric[T]].lt(t, value) || (!exclusive && implicitly[Numeric[T]].equiv(t, value))) } case class Pattern[T <: String](value: String) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (t.matches(value)) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: T): ValidationResult = ValidationResult.validWhen(t.matches(value)) } case class MinLength[T <: String](value: Int) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (t.size >= value) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: T): ValidationResult = ValidationResult.validWhen(t.size >= value) } case class MaxLength[T <: String](value: Int) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (t.size <= value) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: T): ValidationResult = ValidationResult.validWhen(t.size <= value) } case class MinSize[T, C[_] <: Iterable[_]](value: Int) extends Primitive[C[T]] { - override def apply(t: C[T]): List[ValidationError[_]] = { - if (t.size >= value) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: C[T]): ValidationResult = ValidationResult.validWhen(t.size >= value) } case class MaxSize[T, C[_] <: Iterable[_]](value: Int) extends Primitive[C[T]] { - override def apply(t: C[T]): List[ValidationError[_]] = { - if (t.size <= value) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: C[T]): ValidationResult = ValidationResult.validWhen(t.size <= value) } - case class Custom[T](doValidate: T => List[ValidationError[_]], showMessage: Option[String] = None) extends Validator[T] { - override def apply(t: T): List[ValidationError[_]] = { - doValidate(t) - } + case class Custom[T](validationLogic: T => ValidationResult, showMessage: Option[String] = None) extends Primitive[T] { + override def doValidate(t: T): ValidationResult = validationLogic(t) } case class Enumeration[T](possibleValues: List[T], encode: Option[EncodeToRaw[T]], name: Option[SName]) extends Primitive[T] { - override def apply(t: T): List[ValidationError[_]] = { - if (possibleValues.contains(t)) { - List.empty - } else { - List(ValidationError.Primitive(this, t)) - } - } + override def doValidate(t: T): ValidationResult = ValidationResult.validWhen(possibleValues.contains(t)) /** Specify how values of this type can be encoded to a raw value (typically a [[String]]). This encoding will be used when generating * documentation. @@ -238,18 +196,22 @@ object Validator extends ValidatorMacros { } } -sealed trait ValidationError[T] { - def prependPath(f: FieldName): ValidationError[T] - def invalidValue: T - def path: List[FieldName] -} - -object ValidationError { - case class Primitive[T](validator: Validator.Primitive[T], invalidValue: T, path: List[FieldName] = Nil) extends ValidationError[T] { - override def prependPath(f: FieldName): ValidationError[T] = copy(path = f :: path) +sealed trait ValidationResult +object ValidationResult { + case object Valid extends ValidationResult + case class Invalid(customMessage: Option[String] = None) extends ValidationResult + object Invalid { + def apply(customMessage: String): Invalid = Invalid(Some(customMessage)) } - case class Custom[T](invalidValue: T, message: String, path: List[FieldName] = Nil) extends ValidationError[T] { - override def prependPath(f: FieldName): ValidationError[T] = copy(path = f :: path) - } + def validWhen(condition: Boolean): ValidationResult = if (condition) Valid else Invalid() +} + +case class ValidationError[T]( + validator: Validator.Primitive[T], + invalidValue: T, + path: List[FieldName] = Nil, + customMessage: Option[String] = None +) { + def prependPath(f: FieldName): ValidationError[T] = copy(path = f :: path) } diff --git a/core/src/test/scala/sttp/tapir/SchemaApplyValidationTest.scala b/core/src/test/scala/sttp/tapir/SchemaApplyValidationTest.scala index 5b86133369..5baf1e3d1f 100644 --- a/core/src/test/scala/sttp/tapir/SchemaApplyValidationTest.scala +++ b/core/src/test/scala/sttp/tapir/SchemaApplyValidationTest.scala @@ -14,7 +14,7 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { implicit val schemaForInt: Schema[Int] = Schema.schemaForInt.validate(Validator.min(10)) val schema = implicitly[Schema[Map[String, Int]]] - schema.applyValidation(Map("key" -> 0)).map(noPath(_)) shouldBe List(ValidationError.Primitive(Validator.min(10), 0)) + schema.applyValidation(Map("key" -> 0)).map(noPath(_)) shouldBe List(ValidationError(Validator.min(10), 0)) schema.applyValidation(Map("key" -> 12)) shouldBe empty } @@ -24,7 +24,7 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(None) shouldBe empty schema.applyValidation(Some(12)) shouldBe empty - schema.applyValidation(Some(5)) shouldBe List(ValidationError.Primitive(Validator.min(10), 5)) + schema.applyValidation(Some(5)) shouldBe List(ValidationError(Validator.min(10), 5)) } it should "validate iterable" in { @@ -33,7 +33,7 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(List.empty[Int]) shouldBe empty schema.applyValidation(List(11)) shouldBe empty - schema.applyValidation(List(5)) shouldBe List(ValidationError.Primitive(Validator.min(10), 5)) + schema.applyValidation(List(5)) shouldBe List(ValidationError(Validator.min(10), 5)) } it should "validate array" in { @@ -42,7 +42,7 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(Array.empty[Int]) shouldBe empty schema.applyValidation(Array(11)) shouldBe empty - schema.applyValidation(Array(5)) shouldBe List(ValidationError.Primitive(Validator.min(10), 5)) + schema.applyValidation(Array(5)) shouldBe List(ValidationError(Validator.min(10), 5)) } it should "skip collection validation for array if element validator is passing" in { @@ -93,13 +93,13 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { implicit val ageSchema: Schema[Int] = Schema.schemaForInt.validate(Validator.min(18)) val schema = Schema.derived[Person] schema.applyValidation(Person("notImportantButOld", 21)).map(noPath(_)) shouldBe List( - ValidationError.Primitive(Validator.pattern("^[A-Z].*"), "notImportantButOld") + ValidationError(Validator.pattern("^[A-Z].*"), "notImportantButOld") ) schema.applyValidation(Person("notImportantAndYoung", 15)).map(noPath(_)) shouldBe List( - ValidationError.Primitive(Validator.pattern("^[A-Z].*"), "notImportantAndYoung"), - ValidationError.Primitive(Validator.min(18), 15) + ValidationError(Validator.pattern("^[A-Z].*"), "notImportantAndYoung"), + ValidationError(Validator.min(18), 15) ) - schema.applyValidation(Person("ImportantButYoung", 15)).map(noPath(_)) shouldBe List(ValidationError.Primitive(Validator.min(18), 15)) + schema.applyValidation(Person("ImportantButYoung", 15)).map(noPath(_)) shouldBe List(ValidationError(Validator.min(18), 15)) schema.applyValidation(Person("ImportantAndOld", 21)) shouldBe empty } @@ -116,15 +116,15 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(RecursiveName("x", None)) shouldBe Nil schema.applyValidation(RecursiveName("", None)) shouldBe List( - ValidationError.Primitive(Validator.minLength(1), "", List(FieldName("name"))) + ValidationError(Validator.minLength(1), "", List(FieldName("name"))) ) schema.applyValidation(RecursiveName("x", Some(Vector(RecursiveName("x", None))))) shouldBe Nil schema.applyValidation(RecursiveName("x", Some(Vector(RecursiveName("", None))))) shouldBe List( - ValidationError.Primitive(Validator.minLength(1), "", List(FieldName("subNames"), FieldName("name"))) + ValidationError(Validator.minLength(1), "", List(FieldName("subNames"), FieldName("name"))) ) schema.applyValidation(RecursiveName("x", Some(Vector(RecursiveName("x", Some(Vector(RecursiveName("x", None)))))))) shouldBe Nil schema.applyValidation(RecursiveName("x", Some(Vector(RecursiveName("x", Some(Vector(RecursiveName("", None)))))))) shouldBe List( - ValidationError.Primitive(Validator.minLength(1), "", List(FieldName("subNames"), FieldName("subNames"), FieldName("name"))) + ValidationError(Validator.minLength(1), "", List(FieldName("subNames"), FieldName("subNames"), FieldName("name"))) ) } @@ -143,8 +143,8 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(Left(10)) shouldBe Nil schema.applyValidation(Right("x")) shouldBe Nil - schema.applyValidation(Left(0)) shouldBe List(ValidationError.Primitive(Validator.min(1), 0)) - schema.applyValidation(Right("")) shouldBe List(ValidationError.Primitive(Validator.minLength(1), "")) + schema.applyValidation(Left(0)) shouldBe List(ValidationError(Validator.min(1), 0)) + schema.applyValidation(Right("")) shouldBe List(ValidationError(Validator.minLength(1), "")) } it should "validate mapped either" in { @@ -159,14 +159,14 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { schema.applyValidation(EitherWrapper(Left(10))) shouldBe Nil schema.applyValidation(EitherWrapper(Right("x"))) shouldBe Nil - schema.applyValidation(EitherWrapper(Left(0))) shouldBe List(ValidationError.Primitive(Validator.min(1), 0)) - schema.applyValidation(EitherWrapper(Right(""))) shouldBe List(ValidationError.Primitive(Validator.minLength(1), "")) + schema.applyValidation(EitherWrapper(Left(0))) shouldBe List(ValidationError(Validator.min(1), 0)) + schema.applyValidation(EitherWrapper(Right(""))) shouldBe List(ValidationError(Validator.minLength(1), "")) } it should "validate oneOf object" in { case class SomeObject(value: String) object SomeObject { - implicit def someObjectSchema: Schema[SomeObject] = Schema.derived[SomeObject].validate(Validator.custom(_ => Nil)) + implicit def someObjectSchema: Schema[SomeObject] = Schema.derived[SomeObject].validate(Validator.custom(_ => ValidationResult.Valid)) } sealed trait Entity { @@ -180,12 +180,8 @@ class SchemaApplyValidationTest extends AnyFlatSpec with Matchers { override def kind: String = "person" } - Entity.entitySchema.applyValidation(Person(SomeObject("1234"))) + Entity.entitySchema.applyValidation(Person(SomeObject("1234"))) shouldBe Nil } - private def noPath[T](v: ValidationError[T]): ValidationError[T] = - v match { - case p: ValidationError.Primitive[T] => p.copy(path = Nil) - case c: ValidationError.Custom[T] => c.copy(path = Nil) - } + private def noPath[T](v: ValidationError[T]): ValidationError[T] = v.copy(path = Nil) } diff --git a/core/src/test/scala/sttp/tapir/ValidatorTest.scala b/core/src/test/scala/sttp/tapir/ValidatorTest.scala index b612ed4d8a..979185d892 100644 --- a/core/src/test/scala/sttp/tapir/ValidatorTest.scala +++ b/core/src/test/scala/sttp/tapir/ValidatorTest.scala @@ -8,7 +8,7 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val min = 1 val wrong = 0 val v = Validator.min(min) - v(wrong) shouldBe List(ValidationError.Primitive(v, wrong)) + v(wrong) shouldBe List(ValidationError(v, wrong)) v(min) shouldBe empty } @@ -16,8 +16,8 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val min = 1 val wrong = 0 val v = Validator.min(min, exclusive = true) - v(wrong) shouldBe List(ValidationError.Primitive(v, wrong)) - v(min) shouldBe List(ValidationError.Primitive(v, min)) + v(wrong) shouldBe List(ValidationError(v, wrong)) + v(min) shouldBe List(ValidationError(v, min)) v(min + 1) shouldBe empty } @@ -25,7 +25,7 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val max = 0 val wrong = 1 val v = Validator.max(max) - v(wrong) shouldBe List(ValidationError.Primitive(v, wrong)) + v(wrong) shouldBe List(ValidationError(v, wrong)) v(max) shouldBe empty } @@ -33,8 +33,8 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val max = 0 val wrong = 1 val v = Validator.max(max, exclusive = true) - v(wrong) shouldBe List(ValidationError.Primitive(v, wrong)) - v(max) shouldBe List(ValidationError.Primitive(v, max)) + v(wrong) shouldBe List(ValidationError(v, wrong)) + v(max) shouldBe List(ValidationError(v, max)) v(max - 1) shouldBe empty } @@ -43,8 +43,8 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val wrongNegative = -1 val wrongZero = 0 val v = Validator.positive[Int] - v(wrongNegative) shouldBe List(ValidationError.Primitive[Int](v, wrongNegative)) - v(wrongZero) shouldBe List(ValidationError.Primitive[Int](v, wrongZero)) + v(wrongNegative) shouldBe List(ValidationError[Int](v, wrongNegative)) + v(wrongZero) shouldBe List(ValidationError[Int](v, wrongZero)) v(value) shouldBe empty } @@ -53,7 +53,7 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val positiveValue = 1 val zeroValue = 0 val v = Validator.positiveOrZero[Int] - v(wrongNegative) shouldBe List(ValidationError.Primitive[Int](v, wrongNegative)) + v(wrongNegative) shouldBe List(ValidationError[Int](v, wrongNegative)) v(zeroValue) shouldBe empty v(positiveValue) shouldBe empty } @@ -63,8 +63,8 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val wrongPositive = 1 val wrongZero = 0 val v = Validator.negative[Int] - v(wrongPositive) shouldBe List(ValidationError.Primitive(v, wrongPositive)) - v(wrongZero) shouldBe List(ValidationError.Primitive(v, wrongZero)) + v(wrongPositive) shouldBe List(ValidationError(v, wrongPositive)) + v(wrongZero) shouldBe List(ValidationError(v, wrongZero)) v(value) shouldBe empty } @@ -77,8 +77,8 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val wrongMinOut = -1 val v = Validator.inRange(min, max) - v(wrongMaxOut) shouldBe List(ValidationError.Primitive(Validator.max(max), wrongMaxOut)) - v(wrongMinOut) shouldBe List(ValidationError.Primitive(Validator.min(min), wrongMinOut)) + v(wrongMaxOut) shouldBe List(ValidationError(Validator.max(max), wrongMaxOut)) + v(wrongMinOut) shouldBe List(ValidationError(Validator.min(min), wrongMinOut)) v(value1) shouldBe empty v(value2) shouldBe empty } @@ -87,61 +87,61 @@ class ValidatorTest extends AnyFlatSpec with Matchers { val expected = 1 val actual = List(1, 2, 3) val v = Validator.maxSize[Int, List](expected) - v(actual) shouldBe List(ValidationError.Primitive(v, actual)) + v(actual) shouldBe List(ValidationError(v, actual)) v(List(1)) shouldBe empty } it should "validate for minSize of collection" in { val expected = 3 val v = Validator.minSize[Int, List](expected) - v(List(1, 2)) shouldBe List(ValidationError.Primitive(v, List(1, 2))) + v(List(1, 2)) shouldBe List(ValidationError(v, List(1, 2))) v(List(1, 2, 3)) shouldBe empty } it should "validate for nonEmpty of collection" in { val v = Validator.nonEmpty[Int, List] - v(List()) shouldBe List(ValidationError.Primitive[List[Int]](Validator.minSize[Int, List](1), List())) + v(List()) shouldBe List(ValidationError[List[Int]](Validator.minSize[Int, List](1), List())) v(List(1)) shouldBe empty } it should "validate for fixedSize of collection" in { val v = Validator.fixedSize[Int, List](3) - v(List(1, 2)) shouldBe List(ValidationError.Primitive(Validator.minSize[Int, List](3), List(1, 2))) - v(List(1, 2, 3, 4)) shouldBe List(ValidationError.Primitive(Validator.maxSize[Int, List](3), List(1, 2, 3, 4))) + v(List(1, 2)) shouldBe List(ValidationError(Validator.minSize[Int, List](3), List(1, 2))) + v(List(1, 2, 3, 4)) shouldBe List(ValidationError(Validator.maxSize[Int, List](3), List(1, 2, 3, 4))) v(List(1, 2, 3)) shouldBe empty } it should "validate for matching regex pattern" in { val expected = "^apple$|^banana$" val wrong = "orange" - Validator.pattern(expected)(wrong) shouldBe List(ValidationError.Primitive(Validator.pattern(expected), wrong)) + Validator.pattern(expected)(wrong) shouldBe List(ValidationError(Validator.pattern(expected), wrong)) Validator.pattern(expected)("banana") shouldBe empty } it should "validate for minLength of string" in { val expected = 3 val v = Validator.minLength[String](expected) - v("ab") shouldBe List(ValidationError.Primitive(v, "ab")) + v("ab") shouldBe List(ValidationError(v, "ab")) v("abc") shouldBe empty } it should "validate for maxLength of string" in { val expected = 1 val v = Validator.maxLength[String](expected) - v("ab") shouldBe List(ValidationError.Primitive(v, "ab")) + v("ab") shouldBe List(ValidationError(v, "ab")) v("a") shouldBe empty } it should "validate for fixedLength of string" in { val v = Validator.fixedLength[String](3) - v("ab") shouldBe List(ValidationError.Primitive(Validator.minLength(3), "ab")) - v("abcd") shouldBe List(ValidationError.Primitive(Validator.maxLength(3), "abcd")) + v("ab") shouldBe List(ValidationError(Validator.minLength(3), "ab")) + v("abcd") shouldBe List(ValidationError(Validator.maxLength(3), "abcd")) v("abc") shouldBe empty } it should "validate for nonEmptyString of string" in { val v = Validator.nonEmptyString[String] - v("") shouldBe List(ValidationError.Primitive(Validator.minLength(1), "")) + v("") shouldBe List(ValidationError(Validator.minLength(1), "")) v("abc") shouldBe empty } @@ -150,29 +150,25 @@ class ValidatorTest extends AnyFlatSpec with Matchers { validator(4) shouldBe empty validator(7) shouldBe empty validator(11) shouldBe List( - ValidationError.Primitive(Validator.max(5), 11), - ValidationError.Primitive(Validator.max(10), 11) + ValidationError(Validator.max(5), 11), + ValidationError(Validator.max(10), 11) ) } it should "validate with all of validators" in { val validator = Validator.all(Validator.min(3), Validator.max(10)) validator(4) shouldBe empty - validator(2) shouldBe List(ValidationError.Primitive(Validator.min(3), 2)) - validator(11) shouldBe List(ValidationError.Primitive(Validator.max(10), 11)) + validator(2) shouldBe List(ValidationError(Validator.min(3), 2)) + validator(11) shouldBe List(ValidationError(Validator.max(10), 11)) } it should "validate with custom validator" in { val v = Validator.custom( { (x: Int) => - if (x > 5) { - List.empty - } else { - List(ValidationError.Custom(x, "X has to be greater than 5!")) - } + if (x > 5) ValidationResult.Valid else ValidationResult.Invalid("X has to be greater than 5!") } ) - v(0) shouldBe List(ValidationError.Custom(0, "X has to be greater than 5!")) + v(0) should matchPattern { case List(ValidationError(_, 0, Nil, Some("X has to be greater than 5!"))) => } } it should "validate coproduct enum" in { @@ -188,7 +184,7 @@ class ValidatorTest extends AnyFlatSpec with Matchers { it should "validate closed set of ints" in { val v = Validator.enumeration(List(1, 2, 3, 4)) v.apply(1) shouldBe empty - v.apply(0) shouldBe List(ValidationError.Primitive(v, 0)) + v.apply(0) shouldBe List(ValidationError(v, 0)) } } diff --git a/docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TSchemaToASchema.scala b/docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TSchemaToASchema.scala index d91abd8ea9..3b9fd7a526 100644 --- a/docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TSchemaToASchema.scala +++ b/docs/apispec-docs/src/main/scala/sttp/tapir/docs/apispec/schema/TSchemaToASchema.scala @@ -115,6 +115,7 @@ private[schema] class TSchemaToASchema(nameToSchemaReference: NameToSchemaRefere case Validator.MaxLength(value) => aschema.copy(maxLength = Some(value)) case Validator.MinSize(value) => aschema.copy(minItems = Some(value)) case Validator.MaxSize(value) => aschema.copy(maxItems = Some(value)) + case Validator.Custom(_, _) => aschema case Validator.Enumeration(_, None, _) => aschema case Validator.Enumeration(v, Some(encode), _) => addEnumeration(aschema, v, encode) } diff --git a/integrations/cats/src/test/scala/sttp/tapir/integ/cats/ValidatorCatsTest.scala b/integrations/cats/src/test/scala/sttp/tapir/integ/cats/ValidatorCatsTest.scala index d0683f5b6c..7c19f97594 100644 --- a/integrations/cats/src/test/scala/sttp/tapir/integ/cats/ValidatorCatsTest.scala +++ b/integrations/cats/src/test/scala/sttp/tapir/integ/cats/ValidatorCatsTest.scala @@ -23,6 +23,6 @@ class ValidatorCatsTest extends AnyFlatSpec with Matchers with Checkers { it should "fail with empty list - List" in { val value: List[String] = Nil val result: Seq[ValidationError[_]] = ValidatorCats.nonEmptyFoldable[List, String].apply(value) - result shouldBe List(ValidationError.Primitive(Validator.minSize[String, List](1), List(), List())) + result shouldBe List(ValidationError(Validator.minSize[String, List](1), List(), List())) } } diff --git a/integrations/refined/src/main/scala/sttp/tapir/codec/refined/TapirCodecRefined.scala b/integrations/refined/src/main/scala/sttp/tapir/codec/refined/TapirCodecRefined.scala index 3a2d46b241..fe2c164e20 100644 --- a/integrations/refined/src/main/scala/sttp/tapir/codec/refined/TapirCodecRefined.scala +++ b/integrations/refined/src/main/scala/sttp/tapir/codec/refined/TapirCodecRefined.scala @@ -93,12 +93,12 @@ trait TapirCodecRefined extends LowPriorityValidatorForPredicate { rightRefinedValidator -> rightPredValidator.validator ) .filter { case (refinedValidator, _) => refinedValidator.notValid(value) } - .map { case (_, primitiveValidator) => ValidationError.Primitive[N](primitiveValidator, value) } + .map { case (_, primitiveValidator) => ValidationError[N](primitiveValidator, value) } .toList if (primitivesErrors.isEmpty) { // this should not happen - List(ValidationError.Custom(value, refinedErrorMessage, List())) + List(ValidationError(Validator.Custom((_: N) => ValidationResult.Valid), value, Nil, Some(refinedErrorMessage))) } else { primitivesErrors } @@ -119,12 +119,12 @@ trait TapirCodecRefined extends LowPriorityValidatorForPredicate { rightRefinedValidator -> rightPredValidator.validator ) .filter { case (refinedValidator, _) => refinedValidator.notValid(value) } - .map { case (_, primitiveValidator) => ValidationError.Primitive[N](primitiveValidator, value) } + .map { case (_, primitiveValidator) => ValidationError[N](primitiveValidator, value) } .toList if (primitivesErrors.isEmpty) { // this should not happen - List(ValidationError.Custom(value, refinedErrorMessage, List())) + List(ValidationError(Validator.Custom((_: N) => ValidationResult.Valid), value, Nil, Some(refinedErrorMessage))) } else { primitivesErrors } @@ -147,7 +147,7 @@ object ValidatorForPredicate { new PrimitiveValidatorForPredicate[V, P] { override def validator: Validator.Primitive[V] = primitiveValidator override def validationErrors(value: V, refinedErrorMessage: String): List[ValidationError[_]] = - List(ValidationError.Primitive[V](primitiveValidator, value)) + List(ValidationError[V](primitiveValidator, value)) } } @@ -158,15 +158,12 @@ trait LowPriorityValidatorForPredicate { new ValidatorForPredicate[V, P] { override val validator: Validator.Custom[V] = Validator.Custom( { v => - if (refinedValidator.isValid(v)) { - List.empty - } else { - List(ValidationError.Custom(v, implicitly[ClassTag[P]].runtimeClass.toString)) - } + if (refinedValidator.isValid(v)) ValidationResult.Valid + else ValidationResult.Invalid(Some(implicitly[ClassTag[P]].runtimeClass.toString)) } ) // for the moment there is no way to get a human description of a predicate/validator without having a concrete value to run it override def validationErrors(value: V, refinedErrorMessage: String): List[ValidationError[_]] = - List(ValidationError.Custom[V](value, refinedErrorMessage)) + List(ValidationError[V](validator, value, Nil, Some(refinedErrorMessage))) } } diff --git a/integrations/refined/src/test/scala/sttp/tapir/codec/refined/TapirCodecRefinedTest.scala b/integrations/refined/src/test/scala/sttp/tapir/codec/refined/TapirCodecRefinedTest.scala index e5c556bf27..f1d85b353f 100644 --- a/integrations/refined/src/test/scala/sttp/tapir/codec/refined/TapirCodecRefinedTest.scala +++ b/integrations/refined/src/test/scala/sttp/tapir/codec/refined/TapirCodecRefinedTest.scala @@ -19,7 +19,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef "Generated codec" should "return DecodeResult.Invalid if subtype can't be refined with correct tapir validator if available" in { val expectedValidator: Validator[String] = Validator.minLength(1) nonEmptyStringCodec.decode("") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, "", _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, "", _, _))) if validator == expectedValidator => } } @@ -33,7 +33,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedMsg = refineV[IPv4]("192.168.0.1000").left.get IPStringCodec.decode("192.168.0.1000") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Custom("192.168.0.1000", `expectedMsg`, _))) => + case DecodeResult.InvalidValue(List(ValidationError(_, "192.168.0.1000", _, Some(`expectedMsg`)))) => } } @@ -44,7 +44,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[String] = Validator.pattern("[a-zA-Z][-a-zA-Z0-9_]*") identifierCodec.decode("-bad") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, "-bad", _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, "-bad", _, _))) if validator == expectedValidator => } } @@ -55,7 +55,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[String] = Validator.maxLength(2) identifierCodec.decode("bad") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, "bad", _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, "bad", _, _))) if validator == expectedValidator => } } @@ -66,7 +66,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[String] = Validator.minLength(42) identifierCodec.decode("bad") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, "bad", _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, "bad", _, _))) if validator == expectedValidator => } } @@ -77,7 +77,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[Int] = Validator.max(3, exclusive = true) limitedIntCodec.decode("3") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, 3, _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, 3, _, _))) if validator == expectedValidator => } } @@ -88,7 +88,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[Int] = Validator.max(3) limitedIntCodec.decode("4") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, 4, _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, 4, _, _))) if validator == expectedValidator => } } @@ -99,7 +99,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[Int] = Validator.min(3, exclusive = true) limitedIntCodec.decode("3") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, 3, _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, 3, _, _))) if validator == expectedValidator => } } @@ -110,7 +110,7 @@ class TapirCodecRefinedTest extends AnyFlatSpec with Matchers with TapirCodecRef val expectedValidator: Validator[Int] = Validator.min(3) limitedIntCodec.decode("2") should matchPattern { - case DecodeResult.InvalidValue(List(ValidationError.Primitive(validator, 2, _))) if validator == expectedValidator => + case DecodeResult.InvalidValue(List(ValidationError(validator, 2, _, _))) if validator == expectedValidator => } } diff --git a/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala b/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala index 18fd93840f..a3dcdd7770 100644 --- a/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala +++ b/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala @@ -229,16 +229,17 @@ object DefaultDecodeFailureHandler { * @param valueName * Name of the validated value to be used in error messages */ - def invalidValueMessage[T](ve: ValidationError[T], valueName: String): String = - ve match { - case p: ValidationError.Primitive[T] => - p.validator match { + def invalidValueMessage[T](ve: ValidationError[T], valueName: String): String = { + ve.customMessage match { + case Some(message) => s"expected $valueName to pass validation: $message, but was: ${ve.invalidValue}" + case None => + ve.validator match { case Validator.Min(value, exclusive) => s"expected $valueName to be greater than ${if (exclusive) "" else "or equal to "}$value, but was ${ve.invalidValue}" case Validator.Max(value, exclusive) => s"expected $valueName to be less than ${if (exclusive) "" else "or equal to "}$value, but was ${ve.invalidValue}" // TODO: convert to patterns when https://github.com/lampepfl/dotty/issues/12226 is fixed - case p: Validator.Pattern[T] => s"expected $valueName to match '${p.value}', but was '${ve.invalidValue}'" + case p: Validator.Pattern[T] => s"expected $valueName to match: ${p.value}, but was: ${ve.invalidValue}" case m: Validator.MinLength[T] => s"expected $valueName to have length greater than or equal to ${m.value}, but was ${ve.invalidValue}" case m: Validator.MaxLength[T] => @@ -247,14 +248,14 @@ object DefaultDecodeFailureHandler { s"expected size of $valueName to be greater than or equal to ${m.value}, but was ${ve.invalidValue.size}" case m: Validator.MaxSize[T, Iterable] => s"expected size of $valueName to be less than or equal to ${m.value}, but was ${ve.invalidValue.size}" + case Validator.Custom(_, _) => s"expected $valueName to pass validation, but was: ${ve.invalidValue}" case Validator.Enumeration(possibleValues, encode, _) => val encodedPossibleValues = encode.fold(possibleValues.map(_.toString))(e => possibleValues.flatMap(e(_).toList).map(_.toString)) - s"expected $valueName to be within $encodedPossibleValues, but was '${ve.invalidValue}'" + s"expected $valueName to be within $encodedPossibleValues, but was ${ve.invalidValue}" } - case c: ValidationError.Custom[T] => - s"expected $valueName to pass custom validation: ${c.message}, but was '${ve.invalidValue}'" } + } /** Default message describing the path to an invalid value. This is the path inside the validated object, e.g. * `user.address.street.name`. From dea18143932b2f855ce12d017399c5e681b74733 Mon Sep 17 00:00:00 2001 From: adamw Date: Thu, 5 May 2022 12:56:11 +0200 Subject: [PATCH 2/3] Fix test --- .../decodefailure/DefaultDecodeFailureHandlerTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala b/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala index 5fd888f031..28ded1f5cf 100644 --- a/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala +++ b/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala @@ -30,7 +30,7 @@ class DefaultDecodeFailureHandlerTest extends AnyFlatSpec with Matchers { // then DefaultDecodeFailureHandler.ValidationMessages.validationErrorsMessage( validationErrors - ) shouldBe "expected value to be within List(1, 10, 11), but was '4'" + ) shouldBe "expected value to be within List(1, 10, 11), but was 4" } it should "create an error message including failed json paths" in { From cae348a1c17967d2f61a8925f37d3b4732fa2e81 Mon Sep 17 00:00:00 2001 From: adamw Date: Thu, 5 May 2022 12:57:48 +0200 Subject: [PATCH 3/3] Better validation error message --- .../server/interceptor/decodefailure/DecodeFailureHandler.scala | 2 +- .../decodefailure/DefaultDecodeFailureHandlerTest.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala b/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala index a3dcdd7770..1eafda12f5 100644 --- a/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala +++ b/server/core/src/main/scala/sttp/tapir/server/interceptor/decodefailure/DecodeFailureHandler.scala @@ -252,7 +252,7 @@ object DefaultDecodeFailureHandler { case Validator.Enumeration(possibleValues, encode, _) => val encodedPossibleValues = encode.fold(possibleValues.map(_.toString))(e => possibleValues.flatMap(e(_).toList).map(_.toString)) - s"expected $valueName to be within $encodedPossibleValues, but was ${ve.invalidValue}" + s"expected $valueName to be one of ${encodedPossibleValues.mkString("(", ", ", ")")}, but was ${ve.invalidValue}" } } } diff --git a/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala b/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala index 28ded1f5cf..3cb80709d5 100644 --- a/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala +++ b/server/core/src/test/scala/sttp/tapir/server/interceptor/decodefailure/DefaultDecodeFailureHandlerTest.scala @@ -30,7 +30,7 @@ class DefaultDecodeFailureHandlerTest extends AnyFlatSpec with Matchers { // then DefaultDecodeFailureHandler.ValidationMessages.validationErrorsMessage( validationErrors - ) shouldBe "expected value to be within List(1, 10, 11), but was 4" + ) shouldBe "expected value to be one of (1, 10, 11), but was 4" } it should "create an error message including failed json paths" in {