From 3bd90f8a2bfa133dae3030440b7bb56af7a9e058 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Tue, 3 Mar 2026 18:58:03 +0100 Subject: [PATCH 1/2] Add failing test --- .../build/buf/protovalidate/Issue427Test.java | 59 +++++++++++++++++++ .../proto/validationtest/issue427.proto | 29 +++++++++ 2 files changed, 88 insertions(+) create mode 100644 src/test/java/build/buf/protovalidate/Issue427Test.java create mode 100644 src/test/resources/proto/validationtest/issue427.proto diff --git a/src/test/java/build/buf/protovalidate/Issue427Test.java b/src/test/java/build/buf/protovalidate/Issue427Test.java new file mode 100644 index 00000000..eba70197 --- /dev/null +++ b/src/test/java/build/buf/protovalidate/Issue427Test.java @@ -0,0 +1,59 @@ +// Copyright 2023-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package build.buf.protovalidate; + +import static org.assertj.core.api.Assertions.assertThat; + +import build.buf.validate.Violation; +import org.junit.jupiter.api.Test; + +public class Issue427Test { + @Test + public void testMessageOneofWithNameOnly() throws Exception { + com.example.imports.validationtest.Issue427 msg = + com.example.imports.validationtest.Issue427.newBuilder().setName("foo").build(); + Validator validator = ValidatorFactory.newBuilder().build(); + assertThat(validator.validate(msg).toProto().getViolationsList()).isEmpty(); + } + + @Test + public void testMessageOneofWithTagsOnly() throws Exception { + com.example.imports.validationtest.Issue427 msg = + com.example.imports.validationtest.Issue427.newBuilder().addTags("a").addTags("b").build(); + Validator validator = ValidatorFactory.newBuilder().build(); + assertThat(validator.validate(msg).toProto().getViolationsList()).isEmpty(); + } + + @Test + public void testMessageOneofWithMappingsOnly() throws Exception { + com.example.imports.validationtest.Issue427 msg = + com.example.imports.validationtest.Issue427.newBuilder().putMappings("k", "v").build(); + Validator validator = ValidatorFactory.newBuilder().build(); + assertThat(validator.validate(msg).toProto().getViolationsList()).isEmpty(); + } + + @Test + public void testMessageOneofNoneSet() throws Exception { + com.example.imports.validationtest.Issue427 msg = + com.example.imports.validationtest.Issue427.getDefaultInstance(); + Validator validator = ValidatorFactory.newBuilder().build(); + assertThat(validator.validate(msg).toProto().getViolationsList()) + .containsExactly( + Violation.newBuilder() + .setRuleId("message.oneof") + .setMessage("one of name, tags, mappings must be set") + .build()); + } +} diff --git a/src/test/resources/proto/validationtest/issue427.proto b/src/test/resources/proto/validationtest/issue427.proto new file mode 100644 index 00000000..eb1fb0db --- /dev/null +++ b/src/test/resources/proto/validationtest/issue427.proto @@ -0,0 +1,29 @@ +// Copyright 2023-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package validationtest; + +import "buf/validate/validate.proto"; + +message Issue427 { + option (buf.validate.message).oneof = { + fields: ["name", "tags", "mappings"] + required: true + }; + string name = 1; + repeated string tags = 2; + map mappings = 3; +} From b52fa96a379d0af5f124f7f3df4be053cdb29798 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Tue, 3 Mar 2026 19:28:33 +0100 Subject: [PATCH 2/2] Fix the bug by handling repeated fields Map fields are considered repeated fields. --- .../buf/protovalidate/FieldEvaluator.java | 18 ++++++++++++------ .../protovalidate/MessageOneofEvaluator.java | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/FieldEvaluator.java index 8b0d1ce2..922e9153 100644 --- a/src/main/java/build/buf/protovalidate/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/FieldEvaluator.java @@ -99,12 +99,7 @@ public List evaluate(Value val, boolean failFast) if (message == null) { return RuleViolation.NO_VIOLATIONS; } - boolean hasField; - if (descriptor.isRepeated()) { - hasField = message.getRepeatedFieldCount(descriptor) != 0; - } else { - hasField = message.hasField(descriptor); - } + boolean hasField = isFieldSet(message, descriptor); if (required && !hasField) { return Collections.singletonList( RuleViolation.newBuilder() @@ -121,4 +116,15 @@ public List evaluate(Value val, boolean failFast) return valueEvaluator.evaluate( new ObjectValue(descriptor, message.getField(descriptor)), failFast); } + + /** + * Returns whether the given field is set on the message. Handles repeated and map fields, which + * are not supported by {@link Message#hasField}. + */ + static boolean isFieldSet(Message message, FieldDescriptor field) { + if (field.isRepeated()) { + return message.getRepeatedFieldCount(field) != 0; + } + return message.hasField(field); + } } diff --git a/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java b/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java index 410c02fd..3069838a 100644 --- a/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java @@ -51,7 +51,7 @@ public List evaluate(Value val, boolean failFast) } int hasCount = 0; for (FieldDescriptor field : fields) { - if (msg.hasField(field)) { + if (FieldEvaluator.isFieldSet(msg, field)) { hasCount++; } }