diff --git a/gradle.properties b/gradle.properties index 5cf58d43..a99c6707 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ # Version of buf.build/bufbuild/protovalidate to use. -protovalidate.version = v0.13.0 +protovalidate.version = v0.13.1 # 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 2453b53e..5e266f93 100644 --- a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java @@ -37,9 +37,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import org.jspecify.annotations.Nullable; /** A build-through cache of message evaluators keyed off the provided descriptor. */ @@ -208,16 +210,26 @@ private void processMessageOneofRules( Descriptor desc, MessageRules msgRules, MessageEvaluator msgEval) throws CompilationException { for (MessageOneofRule rule : msgRules.getOneofList()) { - List fields = new ArrayList<>(rule.getFieldsCount()); + if (rule.getFieldsCount() == 0) { + throw new CompilationException( + String.format( + "at least one field must be specified in oneof rule for the message %s", + desc.getFullName())); + } + Set fields = new LinkedHashSet<>(rule.getFieldsCount()); for (String name : rule.getFieldsList()) { FieldDescriptor field = desc.findFieldByName(name); if (field == null) { throw new CompilationException( - String.format("field \"%s\" not found in %s", name, desc.getFullName())); + String.format("field %s not found in %s", name, desc.getFullName())); + } + if (!fields.add(field)) { + throw new CompilationException( + String.format( + "duplicate %s in oneof rule for the message %s", name, desc.getFullName())); } - fields.add(field); } - msgEval.append(new MessageOneofEvaluator(fields, rule.getRequired())); + msgEval.append(new MessageOneofEvaluator(new ArrayList<>(fields), rule.getRequired())); } } diff --git a/src/main/resources/buf/validate/validate.proto b/src/main/resources/buf/validate/validate.proto index 424f63b6..a4377c5b 100644 --- a/src/main/resources/buf/validate/validate.proto +++ b/src/main/resources/buf/validate/validate.proto @@ -150,17 +150,18 @@ message MessageRules { // fields have explicit presence. This means that, for the purpose of determining // how many fields are set, explicitly setting such a field to its zero value is // effectively the same as not setting it at all. - // 3. This will generate validation errors when unmarshalling, even from the binary - // format. With a Protobuf oneof, if multiple fields are present in the serialized - // form, earlier values are usually silently ignored when unmarshalling, with only - // the last field being present when unmarshalling completes. + // 3. This will always generate validation errors for a message unmarshalled from + // serialized data that sets more than one field. With a Protobuf oneof, when + // multiple fields are present in the serialized form, earlier values are usually + // silently ignored when unmarshalling, with only the last field being set when + // unmarshalling completes. // // // ```proto // message MyMessage { // // Only one of `field1` or `field2` _can_ be present in this message. // option (buf.validate.message).oneof = { fields: ["field1", "field2"] }; - // // Only one of `field3` or `field4` _must_ be present in this message. + // // Exactly one of `field3` or `field4` _must_ be present in this message. // option (buf.validate.message).oneof = { fields: ["field3", "field4"], required: true }; // string field1 = 1; // bytes field2 = 2; @@ -173,7 +174,8 @@ message MessageRules { message MessageOneofRule { // A list of field names to include in the oneof. All field names must be - // defined in the message. + // defined in the message. At least one field must be specified, and + // duplicates are not permitted. repeated string fields = 1; // If true, one of the fields specified _must_ be set. optional bool required = 2;