diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 91e797d2..c57be01b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,8 +13,12 @@ New Features * scrooge-core: `c.t.scrooge.ThriftUnion.fieldInfoForUnionClass` API for retrieving `ThriftStructFieldInfo` for a `ThriftUnion` member class without having to instantiate it. ``PHAB_ID=D871986`` + * scrooge-generator: Add @.generated annotation to Swift generated code ``PHAB_ID=D879262`` +* scrooge-generator: support thrift validations on nested fields that are struct, union, and + exception ``PHAB_ID=D911262`` + 22.4.0 ------ diff --git a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/ValidationsSpec.scala b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/ValidationsSpec.scala index ec5f6c42..e0891af7 100644 --- a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/ValidationsSpec.scala +++ b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/backend/ValidationsSpec.scala @@ -90,6 +90,15 @@ class ValidationsSpec extends JMockSpec with OneInstancePerTest { override def validateOnlyNonValidatedRequest( nonValidationRequest: NoValidationStruct ): Future[Boolean] = Future.True + + override def validateNestedRequest( + nestedNonRequest: NestedNonValidationStruct + ): Future[Boolean] = Future.True + + override def validateDeepNestedRequest( + deepNestedRequest: DeepNestedValidationstruct + ): Future[Boolean] = + Future.True } val clientReceiver = new InMemoryStatsReceiver @@ -105,6 +114,22 @@ class ValidationsSpec extends JMockSpec with OneInstancePerTest { Name.bound(Address(thriftServer.boundAddress.asInstanceOf[InetSocketAddress])), "client") + "validate nestedStruct with annotation in the innerStruct" in { _ => + val nestedNonValidationStruct = NestedNonValidationStruct("whatever", invalidStructRequest) + intercept[TApplicationException] { + await(methodPerEndpointClient.validateNestedRequest(nestedNonValidationStruct)) + } + } + + "validate deepNestedStruct with annotation in the deepInnerStruct" in { _ => + val nestedNonValidationStruct = NestedNonValidationStruct("whatever", invalidStructRequest) + val deepNestedValidationStruct = + DeepNestedValidationstruct("whatever", nestedNonValidationStruct) + intercept[TApplicationException] { + await(methodPerEndpointClient.validateDeepNestedRequest(deepNestedValidationStruct)) + } + } + "validateInstanceValue" should { "validate Struct" in { _ => val validationViolations = ValidationStruct.validateInstanceValue(invalidStructRequest) @@ -252,6 +277,15 @@ class ValidationsSpec extends JMockSpec with OneInstancePerTest { validationRequest: ValidationStruct, validationRequestViolations: Set[ThriftValidationViolation] ): Future[Boolean] = Future.value(validationRequestViolations.nonEmpty) + + override def validateNestedRequest( + nestedNonRequest: NestedNonValidationStruct + ): Future[Boolean] = Future.False + + override def validateDeepNestedRequest( + deepNestedRequest: DeepNestedValidationstruct + ): Future[Boolean] = + Future.False } val thriftServer = @@ -330,6 +364,15 @@ class ValidationsSpec extends JMockSpec with OneInstancePerTest { override def validateOnlyNonValidatedRequest( nonValidationRequest: NoValidationStruct ): Future[Boolean] = Future.False + + override def validateNestedRequest( + nestedNonRequest: NestedNonValidationStruct + ): Future[Boolean] = Future.False + + override def validateDeepNestedRequest( + deepNestedRequest: DeepNestedValidationstruct + ): Future[Boolean] = + Future.False } val thriftServer = diff --git a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/java_generator/ValidationsJavaGeneratorSpec.scala b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/java_generator/ValidationsJavaGeneratorSpec.scala index 51424acf..306779e5 100644 --- a/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/java_generator/ValidationsJavaGeneratorSpec.scala +++ b/scrooge-generator-tests/src/test/scala/com/twitter/scrooge/java_generator/ValidationsJavaGeneratorSpec.scala @@ -46,6 +46,15 @@ class ValidationsJavaGeneratorSpec extends Spec { override def validateOnlyValidatedRequest( validationRequest: ValidationStruct ): Future[lang.Boolean] = Future.value(true) + + override def validateNestedRequest( + nestedNonRequest: NestedNonValidationStruct + ): Future[lang.Boolean] = Future.value(true) + + override def validateDeepNestedRequest( + deepNestedRequest: DeepNestedValidationstruct + ): Future[lang.Boolean] = + Future.value(true) } "Java validateInstanceValue" should { @@ -203,6 +212,14 @@ class ValidationsJavaGeneratorSpec extends Spec { // should return false if `structRequest` is invalid Future.value(structRequestViolations.isEmpty) } + + override def validateNestedRequest( + nestedNonRequest: NestedNonValidationStruct + ): Future[lang.Boolean] = Future.value(true) + + override def validateDeepNestedRequest( + deepNestedRequest: DeepNestedValidationstruct + ): Future[lang.Boolean] = Future.value(true) } val validationStruct = new ValidationStruct( "email", diff --git a/scrooge-generator-tests/src/test/thrift/standalone/validations.thrift b/scrooge-generator-tests/src/test/thrift/standalone/validations.thrift index 0f718df3..1a175ce1 100644 --- a/scrooge-generator-tests/src/test/thrift/standalone/validations.thrift +++ b/scrooge-generator-tests/src/test/thrift/standalone/validations.thrift @@ -30,6 +30,16 @@ struct NonValidationStruct { 1: string stringField (structFieldKey = "") } +struct NestedNonValidationStruct { + 1: string stringField + 2: ValidationStruct nestedStruct +} + +struct DeepNestedValidationstruct { + 1: string stringField + 2: NestedNonValidationStruct deepNestedStruct +} + struct NestedValidationStruct { 1: string stringField (validation.email = "") 2: ValidationStruct nestedStructField @@ -69,4 +79,10 @@ service ValidationService { bool validateOnlyValidatedRequest ( 1: ValidationStruct validationRequest ) + bool validateNestedRequest ( + 1: NestedNonValidationStruct nestedNonRequest + ) + bool validateDeepNestedRequest ( + 1: DeepNestedValidationstruct deepNestedRequest + ) } diff --git a/scrooge-generator/src/main/scala/com/twitter/scrooge/backend/ServiceTemplate.scala b/scrooge-generator/src/main/scala/com/twitter/scrooge/backend/ServiceTemplate.scala index 362b8bd0..19853a5b 100644 --- a/scrooge-generator/src/main/scala/com/twitter/scrooge/backend/ServiceTemplate.scala +++ b/scrooge-generator/src/main/scala/com/twitter/scrooge/backend/ServiceTemplate.scala @@ -444,17 +444,21 @@ trait ServiceTemplate { self: TemplateGenerator => // return true if the `Field` is of struct type (struct, union, or exception), and any fields // of the `Field` has validation annotation (annotation key starts with "validation.") - private[this] def hasValidationAnnotation(field: Field): Boolean = + private[this] def hasValidationAnnotation(field: Field): Boolean = { field.fieldType match { case structType: StructType => val structLike = structType.struct structLike match { case _: Struct | _: Union | _: Exception_ => - structLike.fields.exists(_.hasValidationAnnotation) + structLike.fields.exists { field => + // recursively look for validation annotations for nestedFields + field.hasValidationAnnotation || hasValidationAnnotation(field) + } case _ => false } case _ => false } + } private[this] class NameDeduplicator() { private[this] val seenIDs = new mutable.HashSet[String]