twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 405 forks source link

@Max and @Min do not work with Option[Int] #565

Closed notxcain closed 2 years ago

notxcain commented 3 years ago

Describe the bug @Max and @Min annotation do not work with field of type Option[Int]

To Reproduce Steps to reproduce the behavior: Try to validate case class Foo(@Min(2) bar: Option[Int]) and get

jakarta.validation.UnexpectedTypeException: No validator could be found for constraint 'interfacejakarta.validation.constraints.Min' validating type 'scala.Option'. Check configuration for 'Foo.bar'

Expected behavior Validation works for Option[Int] as per docs.

cacoco commented 3 years ago

@notxcain can you provide details on the code you're using? E.g. what version of the library, etc?

As documented, this case should work as expected here: https://twitter.github.io/util/guide/util-validator/index.html#with-option

Thanks!

mosesn commented 3 years ago

@notxcain just wanted to make sure you saw @cacoco's message! Do you know what version of Finatra you were using?

zmichaelov commented 2 years ago

Hi all, I'm seeing the same issue. I'm using version 21.11.0

cacoco commented 2 years ago

@zmichaelov can you provide details, such as a link to the failing code, or provide a reproducible case? Thanks!

zmichaelov commented 2 years ago

@cacoco certainly!

I was following the Car example in the util-validator docs

Code used to reproduce:

import com.twitter.util.jackson.ScalaObjectMapper
import com.twitter.util.validation.ScalaValidator
import jakarta.validation.constraints.Min
import org.scalatest.funsuite.AnyFunSuite

case class Car(@Min(1000) towingCapacity: Option[Int] = None)

class ValidationTest extends AnyFunSuite {

  private val mapper = ScalaObjectMapper.builder.objectMapper
  private val validator = ScalaValidator()

  test("check constraints") {
    // This works
    val car = Car(Some(100))
    validator.validate(car)

    // This throws an error
    val json = """{ "towing_capacity": 100 }"""
    val result = mapper.parse[Car](json)
  }
}

The error I get is:

No validator could be found for constraint 'interface jakarta.validation.constraints.Min' validating type 'scala.Option'. Check configuration for 'Car.towing_capacity'
jakarta.validation.UnexpectedTypeException: No validator could be found for constraint 'interface jakarta.validation.constraints.Min' validating type 'scala.Option'. Check configuration for 'Car.towing_capacity'
    at com.twitter.util.validation.ScalaValidator.isValid(ScalaValidator.scala:1333)
    at com.twitter.util.validation.ScalaValidator.isValidOption(ScalaValidator.scala:1373)
    at com.twitter.util.validation.ScalaValidator.isValid(ScalaValidator.scala:1295)
    at com.twitter.util.validation.ScalaValidator.isValid(ScalaValidator.scala:1276)
    at com.twitter.util.validation.ScalaValidator.validateField(ScalaValidator.scala:1007)
    at com.twitter.util.validation.ScalaValidator.validateExecutableParameters(ScalaValidator.scala:899)
    at com.twitter.util.validation.ScalaValidator.validateExecutableParameters(ScalaValidator.scala:694)
    at com.twitter.util.jackson.caseclass.CaseClassDeserializer$.executeFieldValidations(CaseClassDeserializer.scala:92)
jyanJing commented 2 years ago

Thank you @zmichaelov for the details, we are looking into it.

jyanJing commented 2 years ago

The fix is landed: https://github.com/twitter/util/commit/3b94d2e628d6d62d678006c3177f46dfeb1e0ec9, it should be included in our upcoming July release. Closing this issue, please feel free to reopen if you are still experiencing the same issue.

zmichaelov commented 2 years ago

Thanks! and will do!