diff --git a/gradle.properties b/gradle.properties index b49d4b1a..93441422 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ # Version of buf.build/bufbuild/protovalidate to use. -protovalidate.version = v0.11.1 +protovalidate.version = v0.12.0 # Arguments to the protovalidate-conformance CLI protovalidate.conformance.args = --strict_message --strict_error --expected_failures=expected-failures.yaml diff --git a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java index b2e05efb..c3fe31e3 100644 --- a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java @@ -379,6 +379,7 @@ private void processEmbeddedMessage( private void processWrapperRules( FieldDescriptor fieldDescriptor, FieldRules fieldRules, ValueEvaluator valueEvaluatorEval) throws CompilationException { + if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE || fieldDescriptor.isMapField() || (fieldDescriptor.isRepeated() && !valueEvaluatorEval.hasNestedRule())) { @@ -386,9 +387,30 @@ private void processWrapperRules( } FieldDescriptor expectedWrapperDescriptor = DescriptorMappings.expectedWrapperRules(fieldDescriptor.getMessageType().getFullName()); + + // Verify that the expected wrapper rules for this field are equal to the rules specified on + // the field + if (expectedWrapperDescriptor != null) { + FieldDescriptor oneofFieldDescriptor = + fieldRules.getOneofFieldDescriptor(DescriptorMappings.FIELD_RULES_ONEOF_DESC); + // If there are no field rules set, just return + if (oneofFieldDescriptor == null) { + return; + } + if (!expectedWrapperDescriptor + .getMessageType() + .getFullName() + .equals(oneofFieldDescriptor.getMessageType().getFullName())) { + throw new CompilationException( + String.format( + "mismatched message rules, %s is not a valid rule for field %s", + oneofFieldDescriptor.getName(), fieldDescriptor.getName())); + } + } if (expectedWrapperDescriptor == null || !fieldRules.hasField(expectedWrapperDescriptor)) { return; } + ValueEvaluator unwrapped = new ValueEvaluator( valueEvaluatorEval.getDescriptor(), valueEvaluatorEval.getNestedRule()); @@ -399,6 +421,17 @@ private void processWrapperRules( private void processStandardRules( FieldDescriptor fieldDescriptor, FieldRules fieldRules, ValueEvaluator valueEvaluatorEval) throws CompilationException { + + // If this is a wrapper field, just return. Wrapper fields are handled by + // processWrapperRules and their unwrapped values are passed through the process gauntlet. + if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) { + FieldDescriptor expectedWrapperDescriptor = + DescriptorMappings.expectedWrapperRules(fieldDescriptor.getMessageType().getFullName()); + if (expectedWrapperDescriptor != null) { + return; + } + } + List compile = ruleCache.compile(fieldDescriptor, fieldRules, valueEvaluatorEval.hasNestedRule()); if (compile.isEmpty()) { diff --git a/src/main/java/build/buf/protovalidate/RuleCache.java b/src/main/java/build/buf/protovalidate/RuleCache.java index 25d37b18..5ea1af99 100644 --- a/src/main/java/build/buf/protovalidate/RuleCache.java +++ b/src/main/java/build/buf/protovalidate/RuleCache.java @@ -290,6 +290,7 @@ private ResolvedRule resolveRules( // indicating whether it is for items. FieldDescriptor expectedRuleDescriptor = DescriptorMappings.getExpectedRuleDescriptor(fieldDescriptor, forItems); + if (expectedRuleDescriptor != null && !oneofFieldDescriptor.getFullName().equals(expectedRuleDescriptor.getFullName())) { // If the expected rule does not match the actual oneof rule, throw a @@ -303,9 +304,19 @@ private ResolvedRule resolveRules( } // If the expected rule descriptor is null or if the field rules do not have the - // oneof field descriptor - // there are no rules to resolve, so return null. + // oneof field descriptor there are no rules to resolve, so return null. if (expectedRuleDescriptor == null || !fieldRules.hasField(oneofFieldDescriptor)) { + if (expectedRuleDescriptor == null) { + // The only expected rule descriptor for message fields is for well known types. + // If we didn't find a descriptor and this is a message, there must be a mismatch. + if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) { + throw new CompilationException( + String.format( + "mismatched message rules, %s is not a valid rule for field %s", + oneofFieldDescriptor.getName(), fieldDescriptor.getName())); + } + } + return null; } diff --git a/src/main/resources/buf/validate/validate.proto b/src/main/resources/buf/validate/validate.proto index 40236f65..18c297a7 100644 --- a/src/main/resources/buf/validate/validate.proto +++ b/src/main/resources/buf/validate/validate.proto @@ -190,6 +190,7 @@ message FieldRules { // - proto2 scalar fields (both optional and required) // - proto3 scalar fields must be non-zero to be considered populated // - repeated and map fields must be non-empty to be considered populated + // - map keys/values and repeated items are always considered populated // // ```proto // message MyMessage { @@ -625,7 +626,7 @@ message FloatRules { // message MyFloat { // float value = 1 [ // (buf.validate.field).float.example = 1.0, - // (buf.validate.field).float.example = "Infinity" + // (buf.validate.field).float.example = inf // ]; // } // ``` @@ -846,7 +847,7 @@ message DoubleRules { // message MyDouble { // double value = 1 [ // (buf.validate.field).double.example = 1.0, - // (buf.validate.field).double.example = "Infinity" + // (buf.validate.field).double.example = inf // ]; // } // ``` @@ -4241,6 +4242,9 @@ message RepeatedRules { // in the field. Even for repeated message fields, validation is executed // against each item unless skip is explicitly specified. // + // Note that repeated items are always considered populated. The `required` + // rule does not apply. + // // ```proto // message MyRepeated { // // The items in the field `value` must follow the specified rules. @@ -4299,6 +4303,9 @@ message MapRules { //Specifies the rules to be applied to each key in the field. // + // Note that map keys are always considered populated. The `required` + // rule does not apply. + // // ```proto // message MyMap { // // The keys in the field `value` must follow the specified rules. @@ -4316,6 +4323,9 @@ message MapRules { // field. Message values will still have their validations evaluated unless //skip is specified here. // + // Note that map values are always considered populated. The `required` + // rule does not apply. + // // ```proto // message MyMap { // // The values in the field `value` must follow the specified rules. @@ -4351,7 +4361,9 @@ message AnyRules { // ```proto // message MyAny { // // The `value` field must have a `type_url` equal to one of the specified values. - // google.protobuf.Any value = 1 [(buf.validate.field).any.in = ["type.googleapis.com/MyType1", "type.googleapis.com/MyType2"]]; + // google.protobuf.Any value = 1 [(buf.validate.field).any = { + // in: ["type.googleapis.com/MyType1", "type.googleapis.com/MyType2"] + // }]; // } // ``` repeated string in = 2; @@ -4360,8 +4372,10 @@ message AnyRules { // // ```proto // message MyAny { - // // The field `value` must not have a `type_url` equal to any of the specified values. - // google.protobuf.Any value = 1 [(buf.validate.field).any.not_in = ["type.googleapis.com/ForbiddenType1", "type.googleapis.com/ForbiddenType2"]]; + // // The `value` field must not have a `type_url` equal to any of the specified values. + // google.protobuf.Any value = 1 [(buf.validate.field).any = { + // not_in: ["type.googleapis.com/ForbiddenType1", "type.googleapis.com/ForbiddenType2"] + // }]; // } // ``` repeated string not_in = 3; @@ -4783,7 +4797,6 @@ message TimestampRules { // ]; // } // ``` - repeated google.protobuf.Timestamp example = 10 [(predefined).cel = { id: "timestamp.example" expression: "true" diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index c13bb515..5a6591bd 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -60,7 +60,7 @@ class FormatTest { private static List formatErrorTests; @BeforeAll - private static void setUp() throws Exception { + public static void setUp() throws Exception { // The test data from the cel-spec conformance tests List celSpecSections = loadTestData("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto");