finatra-validation: Fix Validator for Case Classes with Other Fields

Problem

The `Validator` throws an `IndexOutOfBoundsException` when trying to
validate a case class which contains fields that are not set via
constructor parameters. This is because of `Validator`'s use of
`AnnotationUtils`, and that in this scenario, `Validator` is breaking
the contract of how it should be used. `Validator` passed in an array
of fields returned from `getDeclaredFields`, which will return ALL
fields. `AnnotationUtils` is only expecting a subset of the list of
parameters that it would find via the constructor. The two don't match
and so an exception occurs.

Solution

After consultation with the team, we do not want to change
`AnnotationUtils` at this time. So, we instead fix `Validator` to abide
by the contract of the method and pass in the appropriate constructor
parameters, rather than ALL fields. So we swap out the use of
`getDeclaredFields` with a retrieval of parameters based on the first
constructor.

JIRA Issues: CSL-10170

Differential Revision: https://phabricator.twitter.biz/D549253

CHANGELOG.rst

@@ -21,6 +21,13 @@ Changed
 
 * finatra: Bump version of Jackson to 2.11.2. ``PHAB_ID=D538440``
 
+Fixed
+~~~~~
+
+* finatra-validation: `c.t.f.validation.Validator` would throw an `IndexOutOfBoundsException` when
+  trying to validate a case class which contained additional fields that are not included in the
+  constructor parameters. ``PHAB_ID=D549253``
+
 20.8.1
 ------
 

validation/src/main/scala/com/twitter/finatra/validation/Validator.scala

@@ -225,10 +225,15 @@ class Validator private[finatra] (cacheSize: Long, messageResolver: MessageResol
     reflectionCache.get(
       clazz,
       new Function[Class[_], AnnotatedClass] {
-        def apply(v1: Class[_]): AnnotatedClass =
-          createAnnotatedClass(
-            clazz,
-            AnnotationUtils.findAnnotations(clazz, clazz.getDeclaredFields.map(_.getName)))
+        def apply(v1: Class[_]): AnnotatedClass = {
+          // We must use the constructor parameters here (and not getDeclaredFields),
+          // because getDeclaredFields will return other non-constructor fields within
+          // the case class. We use `head` here, because if this case class doesn't have a
+          // constructor at all, we have larger issues.
+          val constructor = clazz.getConstructors.head
+          val parameters: Array[String] = constructor.getParameters.map(_.getName)
+          createAnnotatedClass(clazz, AnnotationUtils.findAnnotations(clazz, parameters))
+        }
       }
     )
   }

validation/src/test/scala/com/twitter/finatra/validation/tests/ValidatorTest.scala

@@ -4,6 +4,7 @@ import com.twitter.finatra.validation.ValidationResult.Invalid
 import com.twitter.finatra.validation.internal.AnnotatedClass
 import com.twitter.finatra.validation.tests.caseclasses.{
   MinIntExample,
+  Person,
   StateValidationExample,
   User
 }
@@ -68,6 +69,11 @@ class ValidatorTest extends Test {
     validator.validate(testUser) should be(())
   }
 
+  test("validate returns valid result even when case class has other fields") {
+    val testPerson: Person = Person(id = "9999", name = "April")
+    validator.validate(testPerson) should be(())
+  }
+
   test("validate returns invalid result") {
     val testUser: User = User(id = "", name = "April", gender = "F")
     val fieldAnnotation = getValidationAnnotations(classOf[User], "id").headOption

validation/src/test/scala/com/twitter/finatra/validation/tests/caseclasses.scala

@@ -107,6 +107,13 @@ object caseclasses {
       ValidationResult.validate(name.length > 0, "name cannot be empty")
     }
   }
+  // Other fields not in constructor
+  case class Person(@NotEmpty id: String, name: String) {
+    val company: String = "Twitter"
+    val city: String = "San Francisco"
+    val state: String = "California"
+  }
+
   // Customer defined validations
   case class StateValidationExample(@StateConstraint state: String)
   // Integration test