From 20051a6785518c27d66a4306efbafadbbe94f17f Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 4 Dec 2024 12:42:49 -0500 Subject: [PATCH 01/11] Add Violation wrapper class --- .../buf/protovalidate/conformance/Main.java | 7 +- .../buf/protovalidate/ValidationResult.java | 25 +++++- .../build/buf/protovalidate/Violation.java | 86 +++++++++++++++++++ .../internal/errors/FieldPathUtils.java | 37 +++++--- .../internal/evaluator/AnyEvaluator.java | 20 +++-- .../internal/evaluator/EnumEvaluator.java | 11 ++- .../internal/evaluator/FieldEvaluator.java | 17 ++-- .../internal/evaluator/ListEvaluator.java | 2 +- .../internal/evaluator/MapEvaluator.java | 8 +- .../internal/evaluator/MessageEvaluator.java | 2 +- .../internal/evaluator/OneofEvaluator.java | 18 ++-- .../evaluator/UnknownDescriptorEvaluator.java | 7 +- .../internal/evaluator/ValueEvaluator.java | 2 +- .../internal/expression/CelPrograms.java | 2 +- .../internal/expression/CompiledProgram.java | 18 ++-- .../ValidatorDifferentJavaPackagesTest.java | 2 +- .../ValidatorDynamicMessageTest.java | 10 +-- 17 files changed, 207 insertions(+), 67 deletions(-) create mode 100644 src/main/java/build/buf/protovalidate/Violation.java diff --git a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java index 474a701a..a59e0de9 100644 --- a/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java +++ b/conformance/src/main/java/build/buf/protovalidate/conformance/Main.java @@ -20,7 +20,6 @@ import build.buf.protovalidate.exceptions.CompilationException; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.ValidateProto; -import build.buf.validate.Violation; import build.buf.validate.Violations; import build.buf.validate.conformance.harness.TestConformanceRequest; import build.buf.validate.conformance.harness.TestConformanceResponse; @@ -100,11 +99,11 @@ static TestResult testCase( private static TestResult validate(Validator validator, DynamicMessage dynamicMessage) { try { ValidationResult result = validator.validate(dynamicMessage); - List violations = result.getViolations(); - if (violations.isEmpty()) { + if (result.isSuccess()) { return TestResult.newBuilder().setSuccess(true).build(); } - Violations error = Violations.newBuilder().addAllViolations(violations).build(); + Violations error = + Violations.newBuilder().addAllViolations(result.toProto().getViolationsList()).build(); return TestResult.newBuilder().setValidationError(error).build(); } catch (CompilationException e) { return TestResult.newBuilder().setCompilationError(e.getMessage()).build(); diff --git a/src/main/java/build/buf/protovalidate/ValidationResult.java b/src/main/java/build/buf/protovalidate/ValidationResult.java index 32f5c201..40de9658 100644 --- a/src/main/java/build/buf/protovalidate/ValidationResult.java +++ b/src/main/java/build/buf/protovalidate/ValidationResult.java @@ -14,7 +14,8 @@ package build.buf.protovalidate; -import build.buf.validate.Violation; +import build.buf.validate.Violations; +import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -71,12 +72,28 @@ public String toString() { builder.append("Validation error:"); for (Violation violation : violations) { builder.append("\n - "); - if (!violation.getFieldPath().isEmpty()) { - builder.append(violation.getFieldPath()); + if (!violation.getProto().getFieldPath().isEmpty()) { + builder.append(violation.getProto().getFieldPath()); builder.append(": "); } - builder.append(String.format("%s [%s]", violation.getMessage(), violation.getConstraintId())); + builder.append( + String.format( + "%s [%s]", + violation.getProto().getMessage(), violation.getProto().getConstraintId())); } return builder.toString(); } + + /** + * Converts the validation result to its equivalent protobuf form. + * + * @return The protobuf form of this validation result. + */ + public build.buf.validate.Violations toProto() { + List protoViolations = new ArrayList<>(); + for (Violation violation : violations) { + protoViolations.add(violation.getProto()); + } + return Violations.newBuilder().addAllViolations(protoViolations).build(); + } } diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java new file mode 100644 index 00000000..8faf6110 --- /dev/null +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -0,0 +1,86 @@ +// Copyright 2023-2024 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 java.util.Objects; +import javax.annotation.Nullable; + +/** + * {@link Violation} contains all of the collected information about an individual constraint + * violation. + */ +public class Violation { + private final build.buf.validate.Violation proto; + + /** Builds a Violation instance. */ + public static class Builder { + @Nullable private build.buf.validate.Violation proto; + + /** + * Sets the underlying protobuf message that corresponds to the violation. + * + * @param proto The value to set for this field. + * @return The builder. + */ + public Builder setProto(build.buf.validate.Violation proto) { + this.proto = proto; + return this; + } + + /** + * Builds a Violation instance with the provided parameters. + * + * @return A Violation instance. + */ + public Violation build() { + return new Violation(Objects.requireNonNull(proto)); + } + + private Builder() {} + } + + /** + * Constructs a new empty builder. + * + * @return A new empty builder instance. + */ + public static Builder newBuilder() { + return new Builder(); + } + + Violation(build.buf.validate.Violation proto) { + this.proto = proto; + } + + /** + * Gets the protobuf data that corresponds to this constraint violation. + * + * @return The protobuf violation data. + */ + public build.buf.validate.Violation getProto() { + return proto; + } + + /** + * Constructs a {@link Builder} with all fields set to match this {@link Violation}. + * + * @return A new {@link Builder} instance. + */ + public Builder toBuilder() { + Builder builder = new Builder(); + builder.setProto(proto); + return builder; + } +} diff --git a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java index 79837df1..d1b8e0dc 100644 --- a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java +++ b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java @@ -14,9 +14,9 @@ package build.buf.protovalidate.internal.errors; +import build.buf.protovalidate.Violation; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.List; @@ -41,18 +41,21 @@ public static List prependFieldPaths( // special case that makes it significantly simpler to handle reverse-constructing paths with // maps and repeated fields. if (skipSubscript - && violation.getField().getElementsCount() > 0 - && violation.getField().getElements(0).getSubscriptCase() + && violation.getProto().getField().getElementsCount() > 0 + && violation.getProto().getField().getElements(0).getSubscriptCase() != FieldPathElement.SubscriptCase.SUBSCRIPT_NOT_SET) { result.add(violation); continue; } result.add( violation.toBuilder() - .setField( - FieldPath.newBuilder() - .addElements(element) - .addAllElements(violation.getField().getElementsList()) + .setProto( + violation.getProto().toBuilder() + .setField( + FieldPath.newBuilder() + .addElements(element) + .addAllElements(violation.getProto().getField().getElementsList()) + .build()) .build()) .build()); } @@ -72,10 +75,13 @@ public static List prependRulePaths( for (Violation violation : violations) { result.add( violation.toBuilder() - .setRule( - FieldPath.newBuilder() - .addAllElements(elements) - .addAllElements(violation.getRule().getElementsList()) + .setProto( + violation.getProto().toBuilder() + .setRule( + FieldPath.newBuilder() + .addAllElements(elements) + .addAllElements(violation.getProto().getRule().getElementsList()) + .build()) .build()) .build()); } @@ -91,9 +97,14 @@ public static List prependRulePaths( public static List calculateFieldPathStrings(List violations) { List result = new ArrayList<>(); for (Violation violation : violations) { - if (violation.getField().getElementsCount() > 0) { + if (violation.getProto().getField().getElementsCount() > 0) { result.add( - violation.toBuilder().setFieldPath(fieldPathString(violation.getField())).build()); + violation.toBuilder() + .setProto( + violation.getProto().toBuilder() + .setFieldPath(fieldPathString(violation.getProto().getField())) + .build()) + .build()); } else { result.add(violation); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index 16658a34..fc39a026 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -15,12 +15,12 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.AnyRules; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import com.google.protobuf.Message; import java.util.ArrayList; @@ -80,9 +80,12 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx if (!in.isEmpty() && !in.contains(typeURL)) { Violation violation = Violation.newBuilder() - .setRule(IN_RULE_PATH) - .setConstraintId("any.in") - .setMessage("type URL must be in the allow list") + .setProto( + build.buf.validate.Violation.newBuilder() + .setRule(IN_RULE_PATH) + .setConstraintId("any.in") + .setMessage("type URL must be in the allow list") + .build()) .build(); violationList.add(violation); if (failFast) { @@ -92,9 +95,12 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx if (!notIn.isEmpty() && notIn.contains(typeURL)) { Violation violation = Violation.newBuilder() - .setRule(NOT_IN_RULE_PATH) - .setConstraintId("any.not_in") - .setMessage("type URL must not be in the block list") + .setProto( + build.buf.validate.Violation.newBuilder() + .setRule(NOT_IN_RULE_PATH) + .setConstraintId("any.not_in") + .setMessage("type URL must not be in the block list") + .build()) .build(); violationList.add(violation); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 1ea95797..88cfdf95 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -15,12 +15,12 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.EnumRules; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import java.util.Collections; import java.util.List; @@ -85,9 +85,12 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx return new ValidationResult( Collections.singletonList( Violation.newBuilder() - .setRule(DEFINED_ONLY_RULE_PATH) - .setConstraintId("enum.defined_only") - .setMessage("value must be one of the defined enum values") + .setProto( + build.buf.validate.Violation.newBuilder() + .setRule(DEFINED_ONLY_RULE_PATH) + .setConstraintId("enum.defined_only") + .setMessage("value must be one of the defined enum values") + .build()) .build())); } return ValidationResult.EMPTY; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 0120197a..1c8737d0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -15,11 +15,11 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Message; import java.util.Collections; @@ -93,13 +93,16 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx return new ValidationResult( Collections.singletonList( Violation.newBuilder() - .setField( - FieldPath.newBuilder() - .addElements(FieldPathUtils.fieldPathElement(descriptor)) + .setProto( + build.buf.validate.Violation.newBuilder() + .setField( + FieldPath.newBuilder() + .addElements(FieldPathUtils.fieldPathElement(descriptor)) + .build()) + .setRule(requiredFieldRulePath) + .setConstraintId("required") + .setMessage("value is required") .build()) - .setRule(requiredFieldRulePath) - .setConstraintId("required") - .setMessage("value is required") .build())); } if (ignoreEmpty && !hasField) { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index 65a5d384..cf9cf6b0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -15,13 +15,13 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; import build.buf.validate.RepeatedRules; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index 79f0ffe8..118e970a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -15,13 +15,13 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; import build.buf.validate.MapRules; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.Collections; @@ -194,7 +194,11 @@ private List evalPairs(Value key, Value value, boolean failFast) throws ExecutionException { List keyViolations = keyEvaluator.evaluate(key, failFast).getViolations().stream() - .map(violation -> violation.toBuilder().setForKey(true).build()) + .map( + violation -> + violation.toBuilder() + .setProto(violation.getProto().toBuilder().setForKey(true).build()) + .build()) .collect(Collectors.toList()); final List valueViolations; if (failFast && !keyViolations.isEmpty()) { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java index 84a873be..86773e33 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java @@ -15,8 +15,8 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; -import build.buf.validate.Violation; import java.util.ArrayList; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java index 19beb5c8..1ecc2174 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java @@ -15,10 +15,10 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors.OneofDescriptor; import com.google.protobuf.Message; import java.util.Collections; @@ -57,12 +57,16 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx return new ValidationResult( Collections.singletonList( Violation.newBuilder() - .setField( - FieldPath.newBuilder() - .addElements( - FieldPathElement.newBuilder().setFieldName(descriptor.getName()))) - .setConstraintId("required") - .setMessage("exactly one field is required in oneof") + .setProto( + build.buf.validate.Violation.newBuilder() + .setField( + FieldPath.newBuilder() + .addElements( + FieldPathElement.newBuilder() + .setFieldName(descriptor.getName()))) + .setConstraintId("required") + .setMessage("exactly one field is required in oneof") + .build()) .build())); } return ValidationResult.EMPTY; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java index 6dbbefa3..cafdad88 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java @@ -15,8 +15,8 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors.Descriptor; import java.util.Collections; @@ -43,7 +43,10 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx return new ValidationResult( Collections.singletonList( Violation.newBuilder() - .setMessage("No evaluator available for " + desc.getFullName()) + .setProto( + build.buf.validate.Violation.newBuilder() + .setMessage("No evaluator available for " + desc.getFullName()) + .build()) .build())); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java index 9dec92cb..3d604354 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java @@ -15,8 +15,8 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; -import build.buf.validate.Violation; import java.util.ArrayList; import java.util.List; import java.util.Objects; diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java index 4b88149b..fdd0138d 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java @@ -15,10 +15,10 @@ package build.buf.protovalidate.internal.expression; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.evaluator.Evaluator; import build.buf.protovalidate.internal.evaluator.Value; -import build.buf.validate.Violation; import java.util.ArrayList; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java index d3576534..5b147fab 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -14,9 +14,9 @@ package build.buf.protovalidate.internal.expression; +import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.FieldPath; -import build.buf.validate.Violation; import javax.annotation.Nullable; import org.projectnessie.cel.Program; import org.projectnessie.cel.common.types.Err; @@ -69,22 +69,26 @@ public Violation eval(Variable bindings) throws ExecutionException { if ("".equals(value)) { return null; } - Violation.Builder violation = - Violation.newBuilder().setConstraintId(this.source.id).setMessage(value.toString()); + build.buf.validate.Violation.Builder violation = + build.buf.validate.Violation.newBuilder() + .setConstraintId(this.source.id) + .setMessage(value.toString()); if (rulePath != null) { violation.setRule(rulePath); } - return violation.build(); + return Violation.newBuilder().setProto(violation.build()).build(); } else if (value instanceof Boolean) { if (val.booleanValue()) { return null; } - Violation.Builder violation = - Violation.newBuilder().setConstraintId(this.source.id).setMessage(this.source.message); + build.buf.validate.Violation.Builder violation = + build.buf.validate.Violation.newBuilder() + .setConstraintId(this.source.id) + .setMessage(this.source.message); if (rulePath != null) { violation.setRule(rulePath); } - return violation.build(); + return Violation.newBuilder().setProto(violation.build()).build(); } else { throw new ExecutionException(String.format("resolved to an unexpected type %s", val)); } diff --git a/src/test/java/build/buf/protovalidate/ValidatorDifferentJavaPackagesTest.java b/src/test/java/build/buf/protovalidate/ValidatorDifferentJavaPackagesTest.java index 2e011147..6623f177 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDifferentJavaPackagesTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDifferentJavaPackagesTest.java @@ -192,7 +192,7 @@ private void expectViolation(Message msg, Violation violation) throws Validation private void expectViolations(Message msg, List expected) throws ValidationException { Validator validator = new Validator(); - List violations = validator.validate(msg).getViolations(); + List violations = validator.validate(msg).toProto().getViolationsList(); assertThat(violations).containsExactlyInAnyOrderElementsOf(expected); } } diff --git a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java index e3e14358..700a2b78 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -80,7 +80,7 @@ public void testFieldConstraintDynamicMessage() throws Exception { .setFieldPath("regex_string_field") .setMessage("value does not match regex pattern `^[a-z0-9]{1,9}$`") .build(); - assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + assertThat(new Validator().validate(messageBuilder.build()).toProto().getViolationsList()) .containsExactly(expectedViolation); } @@ -97,7 +97,7 @@ public void testOneofConstraintDynamicMessage() throws Exception { .setConstraintId("required") .setMessage("exactly one field is required in oneof") .build(); - assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + assertThat(new Validator().validate(messageBuilder.build()).toProto().getViolationsList()) .containsExactly(expectedViolation); } @@ -113,7 +113,7 @@ public void testMessageConstraintDynamicMessage() throws Exception { .setConstraintId("secondary_email_depends_on_primary") .setMessage("cannot set a secondary email without setting a primary one") .build(); - assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + assertThat(new Validator().validate(messageBuilder.build()).toProto().getViolationsList()) .containsExactly(expectedViolation); } @@ -155,7 +155,7 @@ public void testRequiredFieldConstraintDynamicMessageInvalid() throws Exception .setFieldPath("regex_string_field") .setMessage("value does not match regex pattern `^[a-z0-9]{1,9}$`") .build(); - assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + assertThat(new Validator().validate(messageBuilder.build()).toProto().getViolationsList()) .containsExactly(expectedViolation); } @@ -205,7 +205,7 @@ public void testPredefinedFieldConstraintDynamicMessageInvalid() throws Exceptio TypeRegistry.newBuilder().add(isIdent.getDescriptor().getContainingType()).build(); Config config = Config.newBuilder().setExtensionRegistry(registry).setTypeRegistry(typeRegistry).build(); - assertThat(new Validator(config).validate(messageBuilder.build()).getViolations()) + assertThat(new Validator(config).validate(messageBuilder.build()).toProto().getViolationsList()) .containsExactly(expectedViolation); } From 87ee0da7a32ae56fa0cf50dfdb7d477062085cdb Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 4 Dec 2024 13:13:20 -0500 Subject: [PATCH 02/11] Make the Value interface non-internal --- .../buf/protovalidate/{internal/evaluator => }/Value.java | 2 +- .../buf/protovalidate/internal/evaluator/AnyEvaluator.java | 1 + .../buf/protovalidate/internal/evaluator/EnumEvaluator.java | 1 + .../buf/protovalidate/internal/evaluator/Evaluator.java | 1 + .../protovalidate/internal/evaluator/FieldEvaluator.java | 1 + .../buf/protovalidate/internal/evaluator/ListEvaluator.java | 1 + .../buf/protovalidate/internal/evaluator/MapEvaluator.java | 1 + .../protovalidate/internal/evaluator/MessageEvaluator.java | 1 + .../buf/protovalidate/internal/evaluator/MessageValue.java | 6 ++---- .../buf/protovalidate/internal/evaluator/ObjectValue.java | 6 ++---- .../protovalidate/internal/evaluator/OneofEvaluator.java | 1 + .../internal/evaluator/UnknownDescriptorEvaluator.java | 1 + .../protovalidate/internal/evaluator/ValueEvaluator.java | 1 + .../buf/protovalidate/internal/expression/CelPrograms.java | 2 +- 14 files changed, 16 insertions(+), 10 deletions(-) rename src/main/java/build/buf/protovalidate/{internal/evaluator => }/Value.java (97%) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java b/src/main/java/build/buf/protovalidate/Value.java similarity index 97% rename from src/main/java/build/buf/protovalidate/internal/evaluator/Value.java rename to src/main/java/build/buf/protovalidate/Value.java index 61c4a151..b2da84eb 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java +++ b/src/main/java/build/buf/protovalidate/Value.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package build.buf.protovalidate.internal.evaluator; +package build.buf.protovalidate; import com.google.protobuf.Message; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index fc39a026..04810d03 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 88cfdf95..a71ded8a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java index 26ca0b21..59ef078d 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.exceptions.ExecutionException; /** diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 1c8737d0..7fdd33e1 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index cf9cf6b0..7339e27a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index 118e970a..831b745c 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java index 86773e33..1d00a824 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import java.util.ArrayList; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java index 73bb6f62..eb53e761 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java @@ -14,15 +14,13 @@ package build.buf.protovalidate.internal.evaluator; +import build.buf.protovalidate.Value; import com.google.protobuf.Message; import java.util.Collections; import java.util.List; import java.util.Map; -/** - * The {@link build.buf.protovalidate.internal.evaluator.Value} type that contains a {@link - * com.google.protobuf.Message}. - */ +/** The {@link Value} type that contains a {@link com.google.protobuf.Message}. */ public final class MessageValue implements Value { /** Object type since the object type is inferred from the field descriptor. */ diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java index 7390375a..d42986c4 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java @@ -14,6 +14,7 @@ package build.buf.protovalidate.internal.evaluator; +import build.buf.protovalidate.Value; import com.google.protobuf.AbstractMessage; import com.google.protobuf.Descriptors; import com.google.protobuf.Message; @@ -25,10 +26,7 @@ import javax.annotation.Nullable; import org.projectnessie.cel.common.ULong; -/** - * The {@link build.buf.protovalidate.internal.evaluator.Value} type that contains a field - * descriptor and its value. - */ +/** The {@link Value} type that contains a field descriptor and its value. */ public final class ObjectValue implements Value { /** diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java index 1ecc2174..e9a3b996 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.FieldPath; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java index cafdad88..cf861022 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import com.google.protobuf.Descriptors.Descriptor; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java index 3d604354..20c09a32 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java @@ -15,6 +15,7 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import java.util.ArrayList; diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java index fdd0138d..fc195929 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java @@ -15,10 +15,10 @@ package build.buf.protovalidate.internal.expression; import build.buf.protovalidate.ValidationResult; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.evaluator.Evaluator; -import build.buf.protovalidate.internal.evaluator.Value; import java.util.ArrayList; import java.util.List; From 6014a64d1d294cac30fa0b4e791cfaf3e9c89673 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 4 Dec 2024 13:58:50 -0500 Subject: [PATCH 03/11] Add fieldValue and ruleValue to Violation --- .../build/buf/protovalidate/Violation.java | 55 +++++++++++++++++-- .../internal/constraints/ConstraintCache.java | 11 +++- .../internal/evaluator/AnyEvaluator.java | 35 +++++++----- .../internal/evaluator/EnumEvaluator.java | 9 ++- .../internal/evaluator/EvaluatorBuilder.java | 6 +- .../internal/evaluator/FieldEvaluator.java | 14 +++-- .../internal/evaluator/ObjectValue.java | 2 +- .../internal/expression/CelPrograms.java | 2 +- .../internal/expression/CompiledProgram.java | 24 ++++++-- .../ValidatorDynamicMessageTest.java | 8 ++- 10 files changed, 127 insertions(+), 39 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 8faf6110..8efdafc4 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -23,10 +23,14 @@ */ public class Violation { private final build.buf.validate.Violation proto; + @Nullable Value fieldValue; + @Nullable Value ruleValue; /** Builds a Violation instance. */ public static class Builder { @Nullable private build.buf.validate.Violation proto; + @Nullable Value fieldValue; + @Nullable Value ruleValue; /** * Sets the underlying protobuf message that corresponds to the violation. @@ -39,13 +43,35 @@ public Builder setProto(build.buf.validate.Violation proto) { return this; } + /** + * Sets the field value that corresponds to the violation. + * + * @param fieldValue The field value corresponding to this violation. + * @return The builder. + */ + public Builder setFieldValue(@Nullable Value fieldValue) { + this.fieldValue = fieldValue; + return this; + } + + /** + * Sets the rule value that corresponds to the violation. + * + * @param ruleValue The rule value corresponding to this violation. + * @return The builder. + */ + public Builder setRuleValue(@Nullable Value ruleValue) { + this.ruleValue = ruleValue; + return this; + } + /** * Builds a Violation instance with the provided parameters. * * @return A Violation instance. */ public Violation build() { - return new Violation(Objects.requireNonNull(proto)); + return new Violation(Objects.requireNonNull(proto), fieldValue, ruleValue); } private Builder() {} @@ -60,8 +86,11 @@ public static Builder newBuilder() { return new Builder(); } - Violation(build.buf.validate.Violation proto) { + Violation( + build.buf.validate.Violation proto, @Nullable Value fieldValue, @Nullable Value ruleValue) { this.proto = proto; + this.fieldValue = fieldValue; + this.ruleValue = ruleValue; } /** @@ -73,14 +102,30 @@ public build.buf.validate.Violation getProto() { return proto; } + /** + * Gets the field value that corresponds to the violation. + * + * @return The field value corresponding to this violation. + */ + public @Nullable Value getFieldValue() { + return fieldValue; + } + + /** + * Gets the rule value that corresponds to the violation. + * + * @return The rule value corresponding to this violation. + */ + public @Nullable Value getRuleValue() { + return ruleValue; + } + /** * Constructs a {@link Builder} with all fields set to match this {@link Violation}. * * @return A new {@link Builder} instance. */ public Builder toBuilder() { - Builder builder = new Builder(); - builder.setProto(proto); - return builder; + return new Builder().setProto(proto).setFieldValue(fieldValue).setRuleValue(ruleValue); } } diff --git a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java index a290f99d..5f4a2cad 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -15,8 +15,10 @@ package build.buf.protovalidate.internal.constraints; import build.buf.protovalidate.Config; +import build.buf.protovalidate.Value; import build.buf.protovalidate.exceptions.CompilationException; import build.buf.protovalidate.internal.errors.FieldPathUtils; +import build.buf.protovalidate.internal.evaluator.ObjectValue; import build.buf.protovalidate.internal.expression.AstExpression; import build.buf.protovalidate.internal.expression.CompiledProgram; import build.buf.protovalidate.internal.expression.Expression; @@ -142,6 +144,7 @@ public List compile( Env ruleEnv = getRuleEnv(fieldDescriptor, message, rule.field, forItems); Variable ruleVar = Variable.newRuleVariable(message, message.getField(rule.field)); ProgramOption globals = ProgramOption.globals(ruleVar); + Value ruleValue = new ObjectValue(rule.field, message.getField(rule.field)); try { Program program = ruleEnv.program(rule.astExpression.ast, globals, PARTIAL_EVAL_OPTIONS); Program.EvalResult evalResult = program.eval(Activation.emptyActivation()); @@ -158,13 +161,17 @@ public List compile( Ast residual = ruleEnv.residualAst(rule.astExpression.ast, evalResult.getEvalDetails()); programs.add( new CompiledProgram( - ruleEnv.program(residual, globals), rule.astExpression.source, rule.rulePath)); + ruleEnv.program(residual, globals), + rule.astExpression.source, + rule.rulePath, + ruleValue)); } catch (Exception e) { programs.add( new CompiledProgram( ruleEnv.program(rule.astExpression.ast, globals), rule.astExpression.source, - rule.rulePath)); + rule.rulePath, + ruleValue)); } } return Collections.unmodifiableList(programs); diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index 04810d03..3afbd7a7 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -39,35 +39,38 @@ class AnyEvaluator implements Evaluator { private final Descriptors.FieldDescriptor typeURLDescriptor; private final Set in; + private final List inValue; private final Set notIn; + private final List notInValue; + + private static final Descriptors.FieldDescriptor ANY_DESCRIPTOR = + FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.ANY_FIELD_NUMBER); + + private static final Descriptors.FieldDescriptor IN_DESCRIPTOR = + AnyRules.getDescriptor().findFieldByNumber(AnyRules.IN_FIELD_NUMBER); + + private static final Descriptors.FieldDescriptor NOT_IN_DESCRIPTOR = + AnyRules.getDescriptor().findFieldByNumber(AnyRules.NOT_IN_FIELD_NUMBER); private static final FieldPath IN_RULE_PATH = FieldPath.newBuilder() - .addElements( - FieldPathUtils.fieldPathElement( - FieldConstraints.getDescriptor() - .findFieldByNumber(FieldConstraints.ANY_FIELD_NUMBER))) - .addElements( - FieldPathUtils.fieldPathElement( - AnyRules.getDescriptor().findFieldByNumber(AnyRules.IN_FIELD_NUMBER))) + .addElements(FieldPathUtils.fieldPathElement(ANY_DESCRIPTOR)) + .addElements(FieldPathUtils.fieldPathElement(IN_DESCRIPTOR)) .build(); private static final FieldPath NOT_IN_RULE_PATH = FieldPath.newBuilder() - .addElements( - FieldPathUtils.fieldPathElement( - FieldConstraints.getDescriptor() - .findFieldByNumber(FieldConstraints.ANY_FIELD_NUMBER))) - .addElements( - FieldPathUtils.fieldPathElement( - AnyRules.getDescriptor().findFieldByNumber(AnyRules.NOT_IN_FIELD_NUMBER))) + .addElements(FieldPathUtils.fieldPathElement(ANY_DESCRIPTOR)) + .addElements(FieldPathUtils.fieldPathElement(NOT_IN_DESCRIPTOR)) .build(); /** Constructs a new evaluator for {@link build.buf.validate.AnyRules} messages. */ AnyEvaluator(Descriptors.FieldDescriptor typeURLDescriptor, List in, List notIn) { this.typeURLDescriptor = typeURLDescriptor; this.in = stringsToSet(in); + this.inValue = in; this.notIn = stringsToSet(notIn); + this.notInValue = notIn; } @Override @@ -87,6 +90,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("any.in") .setMessage("type URL must be in the allow list") .build()) + .setFieldValue(val) + .setRuleValue(new ObjectValue(IN_DESCRIPTOR, this.inValue)) .build(); violationList.add(violation); if (failFast) { @@ -102,6 +107,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("any.not_in") .setMessage("type URL must not be in the block list") .build()) + .setFieldValue(val) + .setRuleValue(new ObjectValue(NOT_IN_DESCRIPTOR, this.notInValue)) .build(); violationList.add(violation); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index a71ded8a..56771ad7 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -36,15 +36,16 @@ class EnumEvaluator implements Evaluator { /** Captures all the defined values for this enum */ private final Set values; + private static final Descriptors.FieldDescriptor DEFINED_ONLY_DESCRIPTOR = + EnumRules.getDescriptor().findFieldByNumber(EnumRules.DEFINED_ONLY_FIELD_NUMBER); + private static final FieldPath DEFINED_ONLY_RULE_PATH = FieldPath.newBuilder() .addElements( FieldPathUtils.fieldPathElement( FieldConstraints.getDescriptor() .findFieldByNumber(FieldConstraints.ENUM_FIELD_NUMBER))) - .addElements( - FieldPathUtils.fieldPathElement( - EnumRules.getDescriptor().findFieldByNumber(EnumRules.DEFINED_ONLY_FIELD_NUMBER))) + .addElements(FieldPathUtils.fieldPathElement(DEFINED_ONLY_DESCRIPTOR)) .build(); /** @@ -92,6 +93,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("enum.defined_only") .setMessage("value must be one of the defined enum values") .build()) + .setFieldValue(val) + .setRuleValue(new ObjectValue(DEFINED_ONLY_DESCRIPTOR, true)) .build())); } return ValidationResult.EMPTY; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index f2e4a2e4..6b8bca61 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -529,7 +529,11 @@ private static List compileConstraints( .build(); } compiledPrograms.add( - new CompiledProgram(env.program(astExpression.ast), astExpression.source, rulePath)); + new CompiledProgram( + env.program(astExpression.ast), + astExpression.source, + rulePath, + new MessageValue(constraints.get(i)))); } return compiledPrograms; } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 7fdd33e1..72a9d7ab 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -30,12 +30,12 @@ /** Performs validation on a single message field, defined by its descriptor. */ class FieldEvaluator implements Evaluator { - private static final FieldPath requiredFieldRulePath = + private static final FieldDescriptor REQUIRED_DESCRIPTOR = + FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.REQUIRED_FIELD_NUMBER); + + private static final FieldPath REQUIRED_RULE_PATH = FieldPath.newBuilder() - .addElements( - FieldPathUtils.fieldPathElement( - FieldConstraints.getDescriptor() - .findFieldByNumber(FieldConstraints.REQUIRED_FIELD_NUMBER))) + .addElements(FieldPathUtils.fieldPathElement(REQUIRED_DESCRIPTOR)) .build(); /** The {@link ValueEvaluator} to apply to the field's value */ @@ -100,10 +100,12 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx FieldPath.newBuilder() .addElements(FieldPathUtils.fieldPathElement(descriptor)) .build()) - .setRule(requiredFieldRulePath) + .setRule(REQUIRED_RULE_PATH) .setConstraintId("required") .setMessage("value is required") .build()) + .setFieldValue(val) + .setRuleValue(new ObjectValue(REQUIRED_DESCRIPTOR, true)) .build())); } if (ignoreEmpty && !hasField) { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java index d42986c4..cd663207 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java @@ -43,7 +43,7 @@ public final class ObjectValue implements Value { * @param fieldDescriptor The field descriptor for the value. * @param value The value associated with the field descriptor. */ - ObjectValue(Descriptors.FieldDescriptor fieldDescriptor, Object value) { + public ObjectValue(Descriptors.FieldDescriptor fieldDescriptor, Object value) { this.fieldDescriptor = fieldDescriptor; this.value = value; } diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java index fc195929..1491a1e8 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java @@ -46,7 +46,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx Variable activation = Variable.newThisVariable(val.value(Object.class)); List violationList = new ArrayList<>(); for (CompiledProgram program : programs) { - Violation violation = program.eval(activation); + Violation violation = program.eval(val, activation); if (violation != null) { violationList.add(violation); if (failFast) { diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java index 5b147fab..9488ed48 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -14,6 +14,7 @@ package build.buf.protovalidate.internal.expression; +import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.FieldPath; @@ -36,29 +37,36 @@ public class CompiledProgram { /** The field path from FieldConstraints to the constraint rule value. */ @Nullable private final FieldPath rulePath; + /** The rule value. */ + @Nullable private final Value ruleValue; + /** * Constructs a new {@link CompiledProgram}. * * @param program The compiled CEL program. * @param source The original expression that was compiled into the program. * @param rulePath The field path from the FieldConstraints to the rule value. + * @param ruleValue The rule value. */ - public CompiledProgram(Program program, Expression source, @Nullable FieldPath rulePath) { + public CompiledProgram( + Program program, Expression source, @Nullable FieldPath rulePath, @Nullable Value ruleValue) { this.program = program; this.source = source; this.rulePath = rulePath; + this.ruleValue = ruleValue; } /** * Evaluate the compiled program with a given set of {@link Variable} bindings. * * @param bindings Variable bindings used for the evaluation. + * @param fieldValue Field value to return in violations. * @return The {@link build.buf.validate.Violation} from the evaluation, or null if there are no * violations. * @throws ExecutionException If the evaluation of the CEL program fails with an error. */ @Nullable - public Violation eval(Variable bindings) throws ExecutionException { + public Violation eval(Value fieldValue, Variable bindings) throws ExecutionException { Program.EvalResult evalResult = program.eval(bindings); Val val = evalResult.getVal(); if (val instanceof Err) { @@ -76,7 +84,11 @@ public Violation eval(Variable bindings) throws ExecutionException { if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder().setProto(violation.build()).build(); + return Violation.newBuilder() + .setProto(violation.build()) + .setFieldValue(fieldValue) + .setRuleValue(ruleValue) + .build(); } else if (value instanceof Boolean) { if (val.booleanValue()) { return null; @@ -88,7 +100,11 @@ public Violation eval(Variable bindings) throws ExecutionException { if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder().setProto(violation.build()).build(); + return Violation.newBuilder() + .setProto(violation.build()) + .setFieldValue(fieldValue) + .setRuleValue(ruleValue) + .build(); } else { throw new ExecutionException(String.format("resolved to an unexpected type %s", val)); } diff --git a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java index 700a2b78..5f70eb22 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -80,8 +80,12 @@ public void testFieldConstraintDynamicMessage() throws Exception { .setFieldPath("regex_string_field") .setMessage("value does not match regex pattern `^[a-z0-9]{1,9}$`") .build(); - assertThat(new Validator().validate(messageBuilder.build()).toProto().getViolationsList()) - .containsExactly(expectedViolation); + ValidationResult result = new Validator().validate(messageBuilder.build()); + assertThat(result.toProto().getViolationsList()).containsExactly(expectedViolation); + assertThat(result.getViolations().get(0).getFieldValue().value(String.class)) + .isEqualTo("0123456789"); + assertThat(result.getViolations().get(0).getRuleValue().value(String.class)) + .isEqualTo("^[a-z0-9]{1,9}$"); } @Test From 69e84dfb078208ab91190497a2e10f385da590c0 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 5 Dec 2024 09:37:04 -0500 Subject: [PATCH 04/11] Violation.Builder: Add missing visibility modifiers --- src/main/java/build/buf/protovalidate/Violation.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 8efdafc4..85d79043 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -25,12 +25,16 @@ public class Violation { private final build.buf.validate.Violation proto; @Nullable Value fieldValue; @Nullable Value ruleValue; + private final @Nullable Value fieldValue; + private final @Nullable Value ruleValue; /** Builds a Violation instance. */ public static class Builder { @Nullable private build.buf.validate.Violation proto; @Nullable Value fieldValue; @Nullable Value ruleValue; + private @Nullable Value fieldValue; + private @Nullable Value ruleValue; /** * Sets the underlying protobuf message that corresponds to the violation. @@ -86,7 +90,7 @@ public static Builder newBuilder() { return new Builder(); } - Violation( + private Violation( build.buf.validate.Violation proto, @Nullable Value fieldValue, @Nullable Value ruleValue) { this.proto = proto; this.fieldValue = fieldValue; From 456273ea761bc98e10f1c5a92c0a82c66f3a4159 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 5 Dec 2024 09:52:51 -0500 Subject: [PATCH 05/11] Violation.Builder: Move required arg to constructor --- .../build/buf/protovalidate/Violation.java | 20 +++++++++---------- .../internal/evaluator/AnyEvaluator.java | 6 ++---- .../internal/evaluator/EnumEvaluator.java | 3 +-- .../internal/evaluator/FieldEvaluator.java | 3 +-- .../internal/evaluator/OneofEvaluator.java | 3 +-- .../evaluator/UnknownDescriptorEvaluator.java | 3 +-- .../internal/expression/CompiledProgram.java | 6 ++---- 7 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 85d79043..313195d1 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -14,7 +14,6 @@ package build.buf.protovalidate; -import java.util.Objects; import javax.annotation.Nullable; /** @@ -23,16 +22,12 @@ */ public class Violation { private final build.buf.validate.Violation proto; - @Nullable Value fieldValue; - @Nullable Value ruleValue; private final @Nullable Value fieldValue; private final @Nullable Value ruleValue; /** Builds a Violation instance. */ public static class Builder { - @Nullable private build.buf.validate.Violation proto; - @Nullable Value fieldValue; - @Nullable Value ruleValue; + private build.buf.validate.Violation proto; private @Nullable Value fieldValue; private @Nullable Value ruleValue; @@ -75,19 +70,22 @@ public Builder setRuleValue(@Nullable Value ruleValue) { * @return A Violation instance. */ public Violation build() { - return new Violation(Objects.requireNonNull(proto), fieldValue, ruleValue); + return new Violation(proto, fieldValue, ruleValue); } - private Builder() {} + private Builder(build.buf.validate.Violation proto) { + this.proto = proto; + } } /** * Constructs a new empty builder. * + * @param proto The proto Violation to wrap. * @return A new empty builder instance. */ - public static Builder newBuilder() { - return new Builder(); + public static Builder newBuilder(build.buf.validate.Violation proto) { + return new Builder(proto); } private Violation( @@ -130,6 +128,6 @@ public build.buf.validate.Violation getProto() { * @return A new {@link Builder} instance. */ public Builder toBuilder() { - return new Builder().setProto(proto).setFieldValue(fieldValue).setRuleValue(ruleValue); + return new Builder(proto).setFieldValue(fieldValue).setRuleValue(ruleValue); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index 3afbd7a7..26f86a9a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -83,8 +83,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx String typeURL = (String) anyValue.getField(typeURLDescriptor); if (!in.isEmpty() && !in.contains(typeURL)) { Violation violation = - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setRule(IN_RULE_PATH) .setConstraintId("any.in") @@ -100,8 +99,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx } if (!notIn.isEmpty() && notIn.contains(typeURL)) { Violation violation = - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setRule(NOT_IN_RULE_PATH) .setConstraintId("any.not_in") diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 56771ad7..d6676b3e 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -86,8 +86,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx if (!values.contains(enumValue.getNumber())) { return new ValidationResult( Collections.singletonList( - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setRule(DEFINED_ONLY_RULE_PATH) .setConstraintId("enum.defined_only") diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 72a9d7ab..7f1d9579 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -93,8 +93,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx if (required && !hasField) { return new ValidationResult( Collections.singletonList( - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setField( FieldPath.newBuilder() diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java index e9a3b996..805fd9d6 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java @@ -57,8 +57,7 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx if (required && (message.getOneofFieldDescriptor(descriptor) == null)) { return new ValidationResult( Collections.singletonList( - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setField( FieldPath.newBuilder() diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java index cf861022..3d685f12 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java @@ -43,8 +43,7 @@ public boolean tautology() { public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { return new ValidationResult( Collections.singletonList( - Violation.newBuilder() - .setProto( + Violation.newBuilder( build.buf.validate.Violation.newBuilder() .setMessage("No evaluator available for " + desc.getFullName()) .build()) diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java index 9488ed48..c4752405 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -84,8 +84,7 @@ public Violation eval(Value fieldValue, Variable bindings) throws ExecutionExcep if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder() - .setProto(violation.build()) + return Violation.newBuilder(violation.build()) .setFieldValue(fieldValue) .setRuleValue(ruleValue) .build(); @@ -100,8 +99,7 @@ public Violation eval(Value fieldValue, Variable bindings) throws ExecutionExcep if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder() - .setProto(violation.build()) + return Violation.newBuilder(violation.build()) .setFieldValue(fieldValue) .setRuleValue(ruleValue) .build(); From 9123d250264a7b9a87e0455d07392939d9fc28bf Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 5 Dec 2024 09:55:07 -0500 Subject: [PATCH 06/11] Violation: Rename getProto -> toProto --- .../buf/protovalidate/ValidationResult.java | 9 ++++----- .../build/buf/protovalidate/Violation.java | 2 +- .../internal/errors/FieldPathUtils.java | 18 +++++++++--------- .../internal/evaluator/MapEvaluator.java | 2 +- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/ValidationResult.java b/src/main/java/build/buf/protovalidate/ValidationResult.java index 40de9658..c8bacee8 100644 --- a/src/main/java/build/buf/protovalidate/ValidationResult.java +++ b/src/main/java/build/buf/protovalidate/ValidationResult.java @@ -72,14 +72,13 @@ public String toString() { builder.append("Validation error:"); for (Violation violation : violations) { builder.append("\n - "); - if (!violation.getProto().getFieldPath().isEmpty()) { - builder.append(violation.getProto().getFieldPath()); + if (!violation.toProto().getFieldPath().isEmpty()) { + builder.append(violation.toProto().getFieldPath()); builder.append(": "); } builder.append( String.format( - "%s [%s]", - violation.getProto().getMessage(), violation.getProto().getConstraintId())); + "%s [%s]", violation.toProto().getMessage(), violation.toProto().getConstraintId())); } return builder.toString(); } @@ -92,7 +91,7 @@ public String toString() { public build.buf.validate.Violations toProto() { List protoViolations = new ArrayList<>(); for (Violation violation : violations) { - protoViolations.add(violation.getProto()); + protoViolations.add(violation.toProto()); } return Violations.newBuilder().addAllViolations(protoViolations).build(); } diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 313195d1..166ba06b 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -100,7 +100,7 @@ private Violation( * * @return The protobuf violation data. */ - public build.buf.validate.Violation getProto() { + public build.buf.validate.Violation toProto() { return proto; } diff --git a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java index d1b8e0dc..bae58d45 100644 --- a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java +++ b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java @@ -41,8 +41,8 @@ public static List prependFieldPaths( // special case that makes it significantly simpler to handle reverse-constructing paths with // maps and repeated fields. if (skipSubscript - && violation.getProto().getField().getElementsCount() > 0 - && violation.getProto().getField().getElements(0).getSubscriptCase() + && violation.toProto().getField().getElementsCount() > 0 + && violation.toProto().getField().getElements(0).getSubscriptCase() != FieldPathElement.SubscriptCase.SUBSCRIPT_NOT_SET) { result.add(violation); continue; @@ -50,11 +50,11 @@ public static List prependFieldPaths( result.add( violation.toBuilder() .setProto( - violation.getProto().toBuilder() + violation.toProto().toBuilder() .setField( FieldPath.newBuilder() .addElements(element) - .addAllElements(violation.getProto().getField().getElementsList()) + .addAllElements(violation.toProto().getField().getElementsList()) .build()) .build()) .build()); @@ -76,11 +76,11 @@ public static List prependRulePaths( result.add( violation.toBuilder() .setProto( - violation.getProto().toBuilder() + violation.toProto().toBuilder() .setRule( FieldPath.newBuilder() .addAllElements(elements) - .addAllElements(violation.getProto().getRule().getElementsList()) + .addAllElements(violation.toProto().getRule().getElementsList()) .build()) .build()) .build()); @@ -97,12 +97,12 @@ public static List prependRulePaths( public static List calculateFieldPathStrings(List violations) { List result = new ArrayList<>(); for (Violation violation : violations) { - if (violation.getProto().getField().getElementsCount() > 0) { + if (violation.toProto().getField().getElementsCount() > 0) { result.add( violation.toBuilder() .setProto( - violation.getProto().toBuilder() - .setFieldPath(fieldPathString(violation.getProto().getField())) + violation.toProto().toBuilder() + .setFieldPath(fieldPathString(violation.toProto().getField())) .build()) .build()); } else { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index 831b745c..d0fbe00b 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -198,7 +198,7 @@ private List evalPairs(Value key, Value value, boolean failFast) .map( violation -> violation.toBuilder() - .setProto(violation.getProto().toBuilder().setForKey(true).build()) + .setProto(violation.toProto().toBuilder().setForKey(true).build()) .build()) .collect(Collectors.toList()); final List valueViolations; From a512d02a283fcf4a9f764b7449bec129732b5126 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 5 Dec 2024 10:00:36 -0500 Subject: [PATCH 07/11] Revert "Make the Value interface non-internal" This reverts commit 87ee0da7a32ae56fa0cf50dfdb7d477062085cdb. --- .../buf/protovalidate/internal/evaluator/AnyEvaluator.java | 1 - .../buf/protovalidate/internal/evaluator/EnumEvaluator.java | 1 - .../buf/protovalidate/internal/evaluator/Evaluator.java | 1 - .../protovalidate/internal/evaluator/FieldEvaluator.java | 1 - .../buf/protovalidate/internal/evaluator/ListEvaluator.java | 1 - .../buf/protovalidate/internal/evaluator/MapEvaluator.java | 1 - .../protovalidate/internal/evaluator/MessageEvaluator.java | 1 - .../buf/protovalidate/internal/evaluator/MessageValue.java | 6 ++++-- .../buf/protovalidate/internal/evaluator/ObjectValue.java | 6 ++++-- .../protovalidate/internal/evaluator/OneofEvaluator.java | 1 - .../internal/evaluator/UnknownDescriptorEvaluator.java | 1 - .../buf/protovalidate/{ => internal/evaluator}/Value.java | 2 +- .../protovalidate/internal/evaluator/ValueEvaluator.java | 1 - .../buf/protovalidate/internal/expression/CelPrograms.java | 2 +- 14 files changed, 10 insertions(+), 16 deletions(-) rename src/main/java/build/buf/protovalidate/{ => internal/evaluator}/Value.java (97%) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index 26f86a9a..aa61db00 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index d6676b3e..65eb6373 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java index 59ef078d..26ca0b21 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.exceptions.ExecutionException; /** diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 7f1d9579..fbe31410 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index 7339e27a..cf9cf6b0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index d0fbe00b..599e4d06 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.FieldPathUtils; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java index 1d00a824..86773e33 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import java.util.ArrayList; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java index eb53e761..73bb6f62 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java @@ -14,13 +14,15 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.Value; import com.google.protobuf.Message; import java.util.Collections; import java.util.List; import java.util.Map; -/** The {@link Value} type that contains a {@link com.google.protobuf.Message}. */ +/** + * The {@link build.buf.protovalidate.internal.evaluator.Value} type that contains a {@link + * com.google.protobuf.Message}. + */ public final class MessageValue implements Value { /** Object type since the object type is inferred from the field descriptor. */ diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java index cd663207..8816ed3f 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java @@ -14,7 +14,6 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.Value; import com.google.protobuf.AbstractMessage; import com.google.protobuf.Descriptors; import com.google.protobuf.Message; @@ -26,7 +25,10 @@ import javax.annotation.Nullable; import org.projectnessie.cel.common.ULong; -/** The {@link Value} type that contains a field descriptor and its value. */ +/** + * The {@link build.buf.protovalidate.internal.evaluator.Value} type that contains a field + * descriptor and its value. + */ public final class ObjectValue implements Value { /** diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java index 805fd9d6..7cc677d8 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.validate.FieldPath; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java index 3d685f12..f5cb6f38 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import com.google.protobuf.Descriptors.Descriptor; diff --git a/src/main/java/build/buf/protovalidate/Value.java b/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java similarity index 97% rename from src/main/java/build/buf/protovalidate/Value.java rename to src/main/java/build/buf/protovalidate/internal/evaluator/Value.java index b2da84eb..61c4a151 100644 --- a/src/main/java/build/buf/protovalidate/Value.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package build.buf.protovalidate; +package build.buf.protovalidate.internal.evaluator; import com.google.protobuf.Message; import java.util.List; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java index 20c09a32..3d604354 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java @@ -15,7 +15,6 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import java.util.ArrayList; diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java index 1491a1e8..e4033204 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java @@ -15,10 +15,10 @@ package build.buf.protovalidate.internal.expression; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.evaluator.Evaluator; +import build.buf.protovalidate.internal.evaluator.Value; import java.util.ArrayList; import java.util.List; From c5f3d18129ed80e839474943bb39fdf2a64d9204 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 5 Dec 2024 10:19:46 -0500 Subject: [PATCH 08/11] Remove Value usages from public API --- .../build/buf/protovalidate/Violation.java | 59 +++++++++++++++---- .../internal/constraints/ConstraintCache.java | 2 +- .../internal/evaluator/AnyEvaluator.java | 8 +-- .../internal/evaluator/EnumEvaluator.java | 4 +- .../internal/evaluator/FieldEvaluator.java | 4 +- .../internal/evaluator/MessageValue.java | 7 +++ .../internal/evaluator/ObjectValue.java | 5 ++ .../internal/evaluator/Value.java | 10 ++++ .../internal/expression/CompiledProgram.java | 24 +++++--- .../ValidatorDynamicMessageTest.java | 6 +- 10 files changed, 96 insertions(+), 33 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 166ba06b..0ad70203 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -14,6 +14,7 @@ package build.buf.protovalidate; +import com.google.protobuf.Descriptors; import javax.annotation.Nullable; /** @@ -22,14 +23,18 @@ */ public class Violation { private final build.buf.validate.Violation proto; - private final @Nullable Value fieldValue; - private final @Nullable Value ruleValue; + private final @Nullable Object fieldValue; + private final @Nullable Descriptors.FieldDescriptor fieldDescriptor; + private final @Nullable Object ruleValue; + private final @Nullable Descriptors.FieldDescriptor ruleDescriptor; /** Builds a Violation instance. */ public static class Builder { private build.buf.validate.Violation proto; - private @Nullable Value fieldValue; - private @Nullable Value ruleValue; + private @Nullable Object fieldValue; + private @Nullable Descriptors.FieldDescriptor fieldDescriptor; + private @Nullable Object ruleValue; + private @Nullable Descriptors.FieldDescriptor ruleDescriptor; /** * Sets the underlying protobuf message that corresponds to the violation. @@ -46,10 +51,13 @@ public Builder setProto(build.buf.validate.Violation proto) { * Sets the field value that corresponds to the violation. * * @param fieldValue The field value corresponding to this violation. + * @param fieldDescriptor The field descriptor corresponding to the field value. * @return The builder. */ - public Builder setFieldValue(@Nullable Value fieldValue) { + public Builder setFieldValue( + @Nullable Object fieldValue, @Nullable Descriptors.FieldDescriptor fieldDescriptor) { this.fieldValue = fieldValue; + this.fieldDescriptor = fieldDescriptor; return this; } @@ -57,10 +65,13 @@ public Builder setFieldValue(@Nullable Value fieldValue) { * Sets the rule value that corresponds to the violation. * * @param ruleValue The rule value corresponding to this violation. + * @param ruleDescriptor The field descriptor corresponding to the rule value. * @return The builder. */ - public Builder setRuleValue(@Nullable Value ruleValue) { + public Builder setRuleValue( + @Nullable Object ruleValue, @Nullable Descriptors.FieldDescriptor ruleDescriptor) { this.ruleValue = ruleValue; + this.ruleDescriptor = ruleDescriptor; return this; } @@ -70,7 +81,7 @@ public Builder setRuleValue(@Nullable Value ruleValue) { * @return A Violation instance. */ public Violation build() { - return new Violation(proto, fieldValue, ruleValue); + return new Violation(proto, fieldValue, fieldDescriptor, ruleValue, ruleDescriptor); } private Builder(build.buf.validate.Violation proto) { @@ -89,10 +100,16 @@ public static Builder newBuilder(build.buf.validate.Violation proto) { } private Violation( - build.buf.validate.Violation proto, @Nullable Value fieldValue, @Nullable Value ruleValue) { + build.buf.validate.Violation proto, + @Nullable Object fieldValue, + @Nullable Descriptors.FieldDescriptor fieldDescriptor, + @Nullable Object ruleValue, + @Nullable Descriptors.FieldDescriptor ruleDescriptor) { this.proto = proto; this.fieldValue = fieldValue; + this.fieldDescriptor = fieldDescriptor; this.ruleValue = ruleValue; + this.ruleDescriptor = ruleDescriptor; } /** @@ -109,25 +126,45 @@ public build.buf.validate.Violation toProto() { * * @return The field value corresponding to this violation. */ - public @Nullable Value getFieldValue() { + public @Nullable Object getFieldValue() { return fieldValue; } + /** + * Gets the field descriptor that corresponds to the field value. + * + * @return The field descriptor that corresponds to the field value. + */ + public @Nullable Descriptors.FieldDescriptor getFieldDescriptor() { + return fieldDescriptor; + } + /** * Gets the rule value that corresponds to the violation. * * @return The rule value corresponding to this violation. */ - public @Nullable Value getRuleValue() { + public @Nullable Object getRuleValue() { return ruleValue; } + /** + * Gets the field descriptor that corresponds to the rule value. + * + * @return The field descriptor that corresponds to the rule value. + */ + public @Nullable Descriptors.FieldDescriptor getRuleDescriptor() { + return ruleDescriptor; + } + /** * Constructs a {@link Builder} with all fields set to match this {@link Violation}. * * @return A new {@link Builder} instance. */ public Builder toBuilder() { - return new Builder(proto).setFieldValue(fieldValue).setRuleValue(ruleValue); + return new Builder(proto) + .setFieldValue(fieldValue, fieldDescriptor) + .setRuleValue(ruleValue, ruleDescriptor); } } diff --git a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java index 5f4a2cad..2a4a8435 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -15,10 +15,10 @@ package build.buf.protovalidate.internal.constraints; import build.buf.protovalidate.Config; -import build.buf.protovalidate.Value; import build.buf.protovalidate.exceptions.CompilationException; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.protovalidate.internal.evaluator.ObjectValue; +import build.buf.protovalidate.internal.evaluator.Value; import build.buf.protovalidate.internal.expression.AstExpression; import build.buf.protovalidate.internal.expression.CompiledProgram; import build.buf.protovalidate.internal.expression.Expression; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index aa61db00..f1a2da04 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -88,8 +88,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("any.in") .setMessage("type URL must be in the allow list") .build()) - .setFieldValue(val) - .setRuleValue(new ObjectValue(IN_DESCRIPTOR, this.inValue)) + .setFieldValue(val.value(Object.class), val.fieldDescriptor()) + .setRuleValue(this.inValue, IN_DESCRIPTOR) .build(); violationList.add(violation); if (failFast) { @@ -104,8 +104,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("any.not_in") .setMessage("type URL must not be in the block list") .build()) - .setFieldValue(val) - .setRuleValue(new ObjectValue(NOT_IN_DESCRIPTOR, this.notInValue)) + .setFieldValue(val.value(Object.class), val.fieldDescriptor()) + .setRuleValue(this.notInValue, NOT_IN_DESCRIPTOR) .build(); violationList.add(violation); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 65eb6373..af03c0fb 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -91,8 +91,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("enum.defined_only") .setMessage("value must be one of the defined enum values") .build()) - .setFieldValue(val) - .setRuleValue(new ObjectValue(DEFINED_ONLY_DESCRIPTOR, true)) + .setFieldValue(val.value(Object.class), val.fieldDescriptor()) + .setRuleValue(true, DEFINED_ONLY_DESCRIPTOR) .build())); } return ValidationResult.EMPTY; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index fbe31410..571e30c0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -102,8 +102,8 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx .setConstraintId("required") .setMessage("value is required") .build()) - .setFieldValue(val) - .setRuleValue(new ObjectValue(REQUIRED_DESCRIPTOR, true)) + .setFieldValue(val.value(Object.class), val.fieldDescriptor()) + .setRuleValue(true, REQUIRED_DESCRIPTOR) .build())); } if (ignoreEmpty && !hasField) { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java index 73bb6f62..cd3c5bdf 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageValue.java @@ -14,10 +14,12 @@ package build.buf.protovalidate.internal.evaluator; +import com.google.protobuf.Descriptors; import com.google.protobuf.Message; import java.util.Collections; import java.util.List; import java.util.Map; +import javax.annotation.Nullable; /** * The {@link build.buf.protovalidate.internal.evaluator.Value} type that contains a {@link @@ -37,6 +39,11 @@ public MessageValue(Message value) { this.value = value; } + @Override + public @Nullable Descriptors.FieldDescriptor fieldDescriptor() { + return null; + } + @Override public Message messageValue() { return (Message) value; diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java index 8816ed3f..53dddaf0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java @@ -50,6 +50,11 @@ public ObjectValue(Descriptors.FieldDescriptor fieldDescriptor, Object value) { this.value = value; } + @Override + public Descriptors.FieldDescriptor fieldDescriptor() { + return fieldDescriptor; + } + @Nullable @Override public Message messageValue() { diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java b/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java index 61c4a151..b6a07f3e 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/Value.java @@ -14,6 +14,7 @@ package build.buf.protovalidate.internal.evaluator; +import com.google.protobuf.Descriptors; import com.google.protobuf.Message; import java.util.List; import java.util.Map; @@ -24,6 +25,15 @@ * value. */ public interface Value { + /** + * Get the field descriptor that corresponds to the underlying Value, if it is a message field. + * + * @return The underlying {@link Descriptors.FieldDescriptor}. null if the underlying value is not + * a message field. + */ + @Nullable + Descriptors.FieldDescriptor fieldDescriptor(); + /** * Get the underlying value as a {@link Message} type. * diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java index c4752405..a83910bd 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -14,9 +14,9 @@ package build.buf.protovalidate.internal.expression; -import build.buf.protovalidate.Value; import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.evaluator.Value; import build.buf.validate.FieldPath; import javax.annotation.Nullable; import org.projectnessie.cel.Program; @@ -84,10 +84,13 @@ public Violation eval(Value fieldValue, Variable bindings) throws ExecutionExcep if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder(violation.build()) - .setFieldValue(fieldValue) - .setRuleValue(ruleValue) - .build(); + Violation.Builder builder = + Violation.newBuilder(violation.build()) + .setFieldValue(fieldValue.value(Object.class), fieldValue.fieldDescriptor()); + if (ruleValue != null) { + builder.setRuleValue(ruleValue.value(Object.class), ruleValue.fieldDescriptor()); + } + return builder.build(); } else if (value instanceof Boolean) { if (val.booleanValue()) { return null; @@ -99,10 +102,13 @@ public Violation eval(Value fieldValue, Variable bindings) throws ExecutionExcep if (rulePath != null) { violation.setRule(rulePath); } - return Violation.newBuilder(violation.build()) - .setFieldValue(fieldValue) - .setRuleValue(ruleValue) - .build(); + Violation.Builder builder = + Violation.newBuilder(violation.build()) + .setFieldValue(fieldValue.value(Object.class), fieldValue.fieldDescriptor()); + if (ruleValue != null) { + builder.setRuleValue(ruleValue.value(Object.class), ruleValue.fieldDescriptor()); + } + return builder.build(); } else { throw new ExecutionException(String.format("resolved to an unexpected type %s", val)); } diff --git a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java index 5f70eb22..9bcbd680 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -82,10 +82,8 @@ public void testFieldConstraintDynamicMessage() throws Exception { .build(); ValidationResult result = new Validator().validate(messageBuilder.build()); assertThat(result.toProto().getViolationsList()).containsExactly(expectedViolation); - assertThat(result.getViolations().get(0).getFieldValue().value(String.class)) - .isEqualTo("0123456789"); - assertThat(result.getViolations().get(0).getRuleValue().value(String.class)) - .isEqualTo("^[a-z0-9]{1,9}$"); + assertThat(result.getViolations().get(0).getFieldValue()).isEqualTo("0123456789"); + assertThat(result.getViolations().get(0).getRuleValue()).isEqualTo("^[a-z0-9]{1,9}$"); } @Test From 867f237a11d380a69a5bd2dbee1bbfb8bc174110 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 6 Dec 2024 05:23:41 -0500 Subject: [PATCH 09/11] Refactor Violation entirely --- .../build/buf/protovalidate/Validator.java | 16 +- .../build/buf/protovalidate/Violation.java | 149 ++-------- .../internal/errors/ConstraintViolation.java | 268 ++++++++++++++++++ .../internal/errors/FieldPathUtils.java | 121 ++------ .../internal/evaluator/AnyEvaluator.java | 61 ++-- .../CelPrograms.java | 27 +- .../evaluator/EmbeddedMessageEvaluator.java | 42 +++ .../internal/evaluator/EnumEvaluator.java | 35 +-- .../internal/evaluator/Evaluator.java | 9 +- .../internal/evaluator/EvaluatorBase.java | 61 ++++ .../internal/evaluator/EvaluatorBuilder.java | 114 +++----- .../internal/evaluator/EvaluatorWrapper.java | 26 -- .../internal/evaluator/FieldEvaluator.java | 47 ++- .../internal/evaluator/ListEvaluator.java | 78 ++--- .../internal/evaluator/MapEvaluator.java | 141 +++------ .../internal/evaluator/MessageEvaluator.java | 22 +- .../internal/evaluator/OneofEvaluator.java | 34 +-- .../evaluator/UnknownDescriptorEvaluator.java | 17 +- .../internal/evaluator/ValueEvaluator.java | 45 ++- .../internal/expression/CompiledProgram.java | 38 ++- .../ValidatorDynamicMessageTest.java | 5 +- 21 files changed, 707 insertions(+), 649 deletions(-) create mode 100644 src/main/java/build/buf/protovalidate/internal/errors/ConstraintViolation.java rename src/main/java/build/buf/protovalidate/internal/{expression => evaluator}/CelPrograms.java (60%) create mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java create mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java delete mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorWrapper.java diff --git a/src/main/java/build/buf/protovalidate/Validator.java b/src/main/java/build/buf/protovalidate/Validator.java index 3cdb051b..259a597a 100644 --- a/src/main/java/build/buf/protovalidate/Validator.java +++ b/src/main/java/build/buf/protovalidate/Validator.java @@ -17,12 +17,14 @@ import build.buf.protovalidate.exceptions.CompilationException; import build.buf.protovalidate.exceptions.ValidationException; import build.buf.protovalidate.internal.celext.ValidateLibrary; -import build.buf.protovalidate.internal.errors.FieldPathUtils; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.evaluator.Evaluator; import build.buf.protovalidate.internal.evaluator.EvaluatorBuilder; import build.buf.protovalidate.internal.evaluator.MessageValue; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Message; +import java.util.ArrayList; +import java.util.List; import org.projectnessie.cel.Env; import org.projectnessie.cel.Library; @@ -75,11 +77,15 @@ public ValidationResult validate(Message msg) throws ValidationException { } Descriptor descriptor = msg.getDescriptorForType(); Evaluator evaluator = evaluatorBuilder.load(descriptor); - ValidationResult result = evaluator.evaluate(new MessageValue(msg), failFast); - if (result.isSuccess()) { - return result; + List result = evaluator.evaluate(new MessageValue(msg), failFast); + if (result.isEmpty()) { + return ValidationResult.EMPTY; + } + List violations = new ArrayList<>(); + for (ConstraintViolation.Builder builder : result) { + violations.add(builder.build()); } - return new ValidationResult(FieldPathUtils.calculateFieldPathStrings(result.getViolations())); + return new ValidationResult(violations); } /** diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index 0ad70203..b510ec00 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -18,153 +18,48 @@ import javax.annotation.Nullable; /** - * {@link Violation} contains all of the collected information about an individual constraint + * {@link Violation} provides all of the collected information about an individual constraint * violation. */ -public class Violation { - private final build.buf.validate.Violation proto; - private final @Nullable Object fieldValue; - private final @Nullable Descriptors.FieldDescriptor fieldDescriptor; - private final @Nullable Object ruleValue; - private final @Nullable Descriptors.FieldDescriptor ruleDescriptor; - - /** Builds a Violation instance. */ - public static class Builder { - private build.buf.validate.Violation proto; - private @Nullable Object fieldValue; - private @Nullable Descriptors.FieldDescriptor fieldDescriptor; - private @Nullable Object ruleValue; - private @Nullable Descriptors.FieldDescriptor ruleDescriptor; - - /** - * Sets the underlying protobuf message that corresponds to the violation. - * - * @param proto The value to set for this field. - * @return The builder. - */ - public Builder setProto(build.buf.validate.Violation proto) { - this.proto = proto; - return this; - } - - /** - * Sets the field value that corresponds to the violation. - * - * @param fieldValue The field value corresponding to this violation. - * @param fieldDescriptor The field descriptor corresponding to the field value. - * @return The builder. - */ - public Builder setFieldValue( - @Nullable Object fieldValue, @Nullable Descriptors.FieldDescriptor fieldDescriptor) { - this.fieldValue = fieldValue; - this.fieldDescriptor = fieldDescriptor; - return this; - } - +public interface Violation { + /** {@link FieldValue} represents a Protobuf field value inside a Protobuf message. */ + interface FieldValue { /** - * Sets the rule value that corresponds to the violation. + * Gets the value of the field, which may be null, a primitive, a Map or a List. * - * @param ruleValue The rule value corresponding to this violation. - * @param ruleDescriptor The field descriptor corresponding to the rule value. - * @return The builder. + * @return The value of the protobuf field. */ - public Builder setRuleValue( - @Nullable Object ruleValue, @Nullable Descriptors.FieldDescriptor ruleDescriptor) { - this.ruleValue = ruleValue; - this.ruleDescriptor = ruleDescriptor; - return this; - } + @Nullable + Object getValue(); /** - * Builds a Violation instance with the provided parameters. + * Gets the field descriptor of the field this value is from. * - * @return A Violation instance. + * @return A FieldDescriptor pertaining to this field. */ - public Violation build() { - return new Violation(proto, fieldValue, fieldDescriptor, ruleValue, ruleDescriptor); - } - - private Builder(build.buf.validate.Violation proto) { - this.proto = proto; - } - } - - /** - * Constructs a new empty builder. - * - * @param proto The proto Violation to wrap. - * @return A new empty builder instance. - */ - public static Builder newBuilder(build.buf.validate.Violation proto) { - return new Builder(proto); - } - - private Violation( - build.buf.validate.Violation proto, - @Nullable Object fieldValue, - @Nullable Descriptors.FieldDescriptor fieldDescriptor, - @Nullable Object ruleValue, - @Nullable Descriptors.FieldDescriptor ruleDescriptor) { - this.proto = proto; - this.fieldValue = fieldValue; - this.fieldDescriptor = fieldDescriptor; - this.ruleValue = ruleValue; - this.ruleDescriptor = ruleDescriptor; + Descriptors.FieldDescriptor getDescriptor(); } /** - * Gets the protobuf data that corresponds to this constraint violation. + * Gets the protobuf form of this violation. * - * @return The protobuf violation data. + * @return The protobuf form of this violation. */ - public build.buf.validate.Violation toProto() { - return proto; - } - - /** - * Gets the field value that corresponds to the violation. - * - * @return The field value corresponding to this violation. - */ - public @Nullable Object getFieldValue() { - return fieldValue; - } - - /** - * Gets the field descriptor that corresponds to the field value. - * - * @return The field descriptor that corresponds to the field value. - */ - public @Nullable Descriptors.FieldDescriptor getFieldDescriptor() { - return fieldDescriptor; - } + build.buf.validate.Violation toProto(); /** - * Gets the rule value that corresponds to the violation. + * Gets the value of the field this violation pertains to, or null if there is none. * - * @return The rule value corresponding to this violation. + * @return Value of the field associated with the violation, or null if there is none. */ - public @Nullable Object getRuleValue() { - return ruleValue; - } + @Nullable + FieldValue getFieldValue(); /** - * Gets the field descriptor that corresponds to the rule value. + * Gets the value of the rule this violation pertains to, or null if there is none. * - * @return The field descriptor that corresponds to the rule value. + * @return Value of the rule associated with the violation, or null if there is none. */ - public @Nullable Descriptors.FieldDescriptor getRuleDescriptor() { - return ruleDescriptor; - } - - /** - * Constructs a {@link Builder} with all fields set to match this {@link Violation}. - * - * @return A new {@link Builder} instance. - */ - public Builder toBuilder() { - return new Builder(proto) - .setFieldValue(fieldValue, fieldDescriptor) - .setRuleValue(ruleValue, ruleDescriptor); - } + @Nullable + FieldValue getRuleValue(); } diff --git a/src/main/java/build/buf/protovalidate/internal/errors/ConstraintViolation.java b/src/main/java/build/buf/protovalidate/internal/errors/ConstraintViolation.java new file mode 100644 index 00000000..711b012b --- /dev/null +++ b/src/main/java/build/buf/protovalidate/internal/errors/ConstraintViolation.java @@ -0,0 +1,268 @@ +// Copyright 2023-2024 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.internal.errors; + +import build.buf.protovalidate.Violation; +import build.buf.protovalidate.internal.evaluator.Value; +import build.buf.validate.FieldPath; +import build.buf.validate.FieldPathElement; +import com.google.protobuf.Descriptors; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Deque; +import java.util.List; +import java.util.Objects; +import javax.annotation.Nullable; + +/** + * {@link ConstraintViolation} contains all of the collected information about an individual + * constraint violation. + */ +public class ConstraintViolation implements Violation { + /** Static value to return when there are no violations. */ + public static final List NO_VIOLATIONS = new ArrayList<>(); + + /** {@link FieldValue} represents a Protobuf field value inside a Protobuf message. */ + public static class FieldValue implements Violation.FieldValue { + private final @Nullable Object value; + private final Descriptors.FieldDescriptor descriptor; + + /** + * Constructs a {@link FieldValue} from a value and a descriptor directly. + * + * @param value Bare Protobuf field value of field. + * @param descriptor Field descriptor pertaining to this field. + */ + public FieldValue(@Nullable Object value, Descriptors.FieldDescriptor descriptor) { + this.value = value; + this.descriptor = descriptor; + } + + /** + * Constructs a {@link FieldValue} from a {@link Value}. The value must be for a Protobuf field, + * e.g. it must have a FieldDescriptor. + * + * @param value A {@link Value} to create this {@link FieldValue} from. + */ + public FieldValue(Value value) { + this.value = value.value(Object.class); + this.descriptor = Objects.requireNonNull(value.fieldDescriptor()); + } + + @Override + public @Nullable Object getValue() { + return value; + } + + @Override + public Descriptors.FieldDescriptor getDescriptor() { + return descriptor; + } + } + + private final build.buf.validate.Violation proto; + private final @Nullable FieldValue fieldValue; + private final @Nullable FieldValue ruleValue; + + /** Builds a Violation instance. */ + public static class Builder { + private @Nullable String constraintId; + private @Nullable String message; + private boolean forKey = false; + private final Deque fieldPath = new ArrayDeque<>(); + private final Deque rulePath = new ArrayDeque<>(); + private @Nullable FieldValue fieldValue; + private @Nullable FieldValue ruleValue; + + /** + * Sets the constraint ID field of the resulting violation. + * + * @param constraintId Constraint ID value to use. + * @return The builder. + */ + public Builder setConstraintId(String constraintId) { + this.constraintId = constraintId; + return this; + } + + /** + * Sets the message field of the resulting violation. + * + * @param message Message value to use. + * @return The builder. + */ + public Builder setMessage(String message) { + this.message = message; + return this; + } + + /** + * Sets whether the violation is for a map key or not. + * + * @param forKey If true, signals that the resulting violation is for a map key. + * @return The builder. + */ + public Builder setForKey(boolean forKey) { + this.forKey = forKey; + return this; + } + + /** + * Adds field path elements to the end of the field path. + * + * @param fieldPathElements Field path elements to add. + * @return The builder. + */ + public Builder addAllFieldPathElements( + Collection fieldPathElements) { + this.fieldPath.addAll(fieldPathElements); + return this; + } + + /** + * Adds a field path element to the beginning of the field path. + * + * @param fieldPathElement A field path element to add to the beginning of the field path. + * @return The builder. + */ + public Builder addFirstFieldPathElement(@Nullable FieldPathElement fieldPathElement) { + if (fieldPathElement != null) { + fieldPath.addFirst(fieldPathElement); + } + return this; + } + + /** + * Adds field path elements to the end of the rule path. + * + * @param rulePathElements Field path elements to add. + * @return The builder. + */ + public Builder addAllRulePathElements(Collection rulePathElements) { + rulePath.addAll(rulePathElements); + return this; + } + + /** + * Adds a field path element to the beginning of the rule path. + * + * @param rulePathElements A field path element to add to the beginning of the rule path. + * @return The builder. + */ + public Builder addFirstRulePathElement(FieldPathElement rulePathElements) { + rulePath.addFirst(rulePathElements); + return this; + } + + /** + * Sets the field value that corresponds to the violation. + * + * @param fieldValue The field value corresponding to this violation. + * @return The builder. + */ + public Builder setFieldValue(@Nullable FieldValue fieldValue) { + this.fieldValue = fieldValue; + return this; + } + + /** + * Sets the rule value that corresponds to the violation. + * + * @param ruleValue The rule value corresponding to this violation. + * @return The builder. + */ + public Builder setRuleValue(@Nullable FieldValue ruleValue) { + this.ruleValue = ruleValue; + return this; + } + + /** + * Builds a Violation instance with the provided parameters. + * + * @return A Violation instance. + */ + public ConstraintViolation build() { + build.buf.validate.Violation.Builder protoBuilder = build.buf.validate.Violation.newBuilder(); + if (constraintId != null) { + protoBuilder.setConstraintId(constraintId); + } + if (message != null) { + protoBuilder.setMessage(message); + } + if (forKey) { + protoBuilder.setForKey(true); + } + if (!fieldPath.isEmpty()) { + FieldPath field = FieldPath.newBuilder().addAllElements(fieldPath).build(); + protoBuilder.setField(field).setFieldPath(FieldPathUtils.fieldPathString(field)); + } + if (!rulePath.isEmpty()) { + protoBuilder.setRule(FieldPath.newBuilder().addAllElements(rulePath)); + } + return new ConstraintViolation(protoBuilder.build(), fieldValue, ruleValue); + } + + private Builder() {} + } + + /** + * Creates a new empty builder for building a {@link ConstraintViolation}. + * + * @return A new, empty {@link Builder}. + */ + public static Builder newBuilder() { + return new Builder(); + } + + private ConstraintViolation( + build.buf.validate.Violation proto, + @Nullable FieldValue fieldValue, + @Nullable FieldValue ruleValue) { + this.proto = proto; + this.fieldValue = fieldValue; + this.ruleValue = ruleValue; + } + + /** + * Gets the protobuf data that corresponds to this constraint violation. + * + * @return The protobuf violation data. + */ + @Override + public build.buf.validate.Violation toProto() { + return proto; + } + + /** + * Gets the field value that corresponds to the violation. + * + * @return The field value corresponding to this violation. + */ + @Override + public @Nullable FieldValue getFieldValue() { + return fieldValue; + } + + /** + * Gets the rule value that corresponds to the violation. + * + * @return The rule value corresponding to this violation. + */ + @Override + public @Nullable FieldValue getRuleValue() { + return ruleValue; + } +} diff --git a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java index bae58d45..4b1103ae 100644 --- a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java +++ b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java @@ -14,104 +14,16 @@ package build.buf.protovalidate.internal.errors; -import build.buf.protovalidate.Violation; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; import com.google.protobuf.Descriptors; -import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; /** Utility class for manipulating error paths in violations. */ public final class FieldPathUtils { private FieldPathUtils() {} - /** - * Prepends the field paths of the given violations with the provided element. - * - * @param violations The list of violations to operate on. - * @param element The element to prefix to each field path. - * @param skipSubscript Skips prepending a field path if the first element has a subscript. - * @return The modified violations with prepended field paths. - */ - public static List prependFieldPaths( - List violations, FieldPathElement element, boolean skipSubscript) { - List result = new ArrayList<>(); - for (Violation violation : violations) { - // Special case: Here we skip prepending if the first element has a subscript. This is a weird - // special case that makes it significantly simpler to handle reverse-constructing paths with - // maps and repeated fields. - if (skipSubscript - && violation.toProto().getField().getElementsCount() > 0 - && violation.toProto().getField().getElements(0).getSubscriptCase() - != FieldPathElement.SubscriptCase.SUBSCRIPT_NOT_SET) { - result.add(violation); - continue; - } - result.add( - violation.toBuilder() - .setProto( - violation.toProto().toBuilder() - .setField( - FieldPath.newBuilder() - .addElements(element) - .addAllElements(violation.toProto().getField().getElementsList()) - .build()) - .build()) - .build()); - } - return result; - } - - /** - * Prepends the rule paths of the given violations with the provided elements. - * - * @param violations The list of violations to operate on. - * @param elements The elements to prefix to each rule path. - * @return The modified violations with prepended rule paths. - */ - public static List prependRulePaths( - List violations, Iterable elements) { - List result = new ArrayList<>(); - for (Violation violation : violations) { - result.add( - violation.toBuilder() - .setProto( - violation.toProto().toBuilder() - .setRule( - FieldPath.newBuilder() - .addAllElements(elements) - .addAllElements(violation.toProto().getRule().getElementsList()) - .build()) - .build()) - .build()); - } - return result; - } - - /** - * Calculates the field path strings for each violation. - * - * @param violations The list of violations to operate on. - * @return The modified violations with field path strings. - */ - public static List calculateFieldPathStrings(List violations) { - List result = new ArrayList<>(); - for (Violation violation : violations) { - if (violation.toProto().getField().getElementsCount() > 0) { - result.add( - violation.toBuilder() - .setProto( - violation.toProto().toBuilder() - .setFieldPath(fieldPathString(violation.toProto().getField())) - .build()) - .build()); - } else { - result.add(violation); - } - } - return result; - } - /** * Converts the provided field path to a string. * @@ -166,8 +78,7 @@ static String fieldPathString(FieldPath fieldPath) { * @param fieldDescriptor The field descriptor to generate a field path element for. * @return The field path element that corresponds to the provided field descriptor. */ - public static FieldPathElement.Builder fieldPathElement( - Descriptors.FieldDescriptor fieldDescriptor) { + public static FieldPathElement fieldPathElement(Descriptors.FieldDescriptor fieldDescriptor) { String name; if (fieldDescriptor.isExtension()) { name = "[" + fieldDescriptor.getFullName() + "]"; @@ -177,6 +88,32 @@ public static FieldPathElement.Builder fieldPathElement( return FieldPathElement.newBuilder() .setFieldNumber(fieldDescriptor.getNumber()) .setFieldName(name) - .setFieldType(fieldDescriptor.getType().toProto()); + .setFieldType(fieldDescriptor.getType().toProto()) + .build(); + } + + /** + * Provided a list of violations, adjusts it by prepending rule and field path elements. + * + * @param violations A list of violations. + * @param fieldPathElement A field path element to prepend, or null. + * @param rulePathElements Rule path elements to prepend. + * @return For convenience, the list of violations passed into the violations parameter. + */ + public static List updatePaths( + List violations, + @Nullable FieldPathElement fieldPathElement, + List rulePathElements) { + if (fieldPathElement != null || !rulePathElements.isEmpty()) { + for (ConstraintViolation.Builder violation : violations) { + for (int i = rulePathElements.size() - 1; i >= 0; i--) { + violation.addFirstRulePathElement(rulePathElements.get(i)); + } + if (fieldPathElement != null) { + violation.addFirstFieldPathElement(fieldPathElement); + } + } + } + return violations; } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index f1a2da04..f63cc1a4 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -14,9 +14,8 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.AnyRules; import build.buf.validate.FieldConstraints; @@ -35,7 +34,7 @@ * com.google.protobuf.Any}'s within an expression, breaking evaluation if the type is unknown at * runtime. */ -class AnyEvaluator implements Evaluator { +class AnyEvaluator extends EvaluatorBase implements Evaluator { private final Descriptors.FieldDescriptor typeURLDescriptor; private final Set in; private final List inValue; @@ -64,7 +63,12 @@ class AnyEvaluator implements Evaluator { .build(); /** Constructs a new evaluator for {@link build.buf.validate.AnyRules} messages. */ - AnyEvaluator(Descriptors.FieldDescriptor typeURLDescriptor, List in, List notIn) { + AnyEvaluator( + ValueEvaluator valueEvaluator, + Descriptors.FieldDescriptor typeURLDescriptor, + List in, + List notIn) { + super(valueEvaluator); this.typeURLDescriptor = typeURLDescriptor; this.in = stringsToSet(in); this.inValue = in; @@ -73,43 +77,42 @@ class AnyEvaluator implements Evaluator { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { Message anyValue = val.messageValue(); if (anyValue == null) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } - List violationList = new ArrayList<>(); + List violationList = new ArrayList<>(); String typeURL = (String) anyValue.getField(typeURLDescriptor); if (!in.isEmpty() && !in.contains(typeURL)) { - Violation violation = - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setRule(IN_RULE_PATH) - .setConstraintId("any.in") - .setMessage("type URL must be in the allow list") - .build()) - .setFieldValue(val.value(Object.class), val.fieldDescriptor()) - .setRuleValue(this.inValue, IN_DESCRIPTOR) - .build(); + ConstraintViolation.Builder violation = + ConstraintViolation.newBuilder() + .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(IN_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(getFieldPathElement()) + .setConstraintId("any.in") + .setMessage("type URL must be in the allow list") + .setFieldValue(new ConstraintViolation.FieldValue(val)) + .setRuleValue(new ConstraintViolation.FieldValue(this.inValue, IN_DESCRIPTOR)); violationList.add(violation); if (failFast) { - return new ValidationResult(violationList); + return violationList; } } if (!notIn.isEmpty() && notIn.contains(typeURL)) { - Violation violation = - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setRule(NOT_IN_RULE_PATH) - .setConstraintId("any.not_in") - .setMessage("type URL must not be in the block list") - .build()) - .setFieldValue(val.value(Object.class), val.fieldDescriptor()) - .setRuleValue(this.notInValue, NOT_IN_DESCRIPTOR) - .build(); + ConstraintViolation.Builder violation = + ConstraintViolation.newBuilder() + .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(NOT_IN_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(getFieldPathElement()) + .setConstraintId("any.not_in") + .setMessage("type URL must not be in the block list") + .setFieldValue(new ConstraintViolation.FieldValue(val)) + .setRuleValue(new ConstraintViolation.FieldValue(this.notInValue, NOT_IN_DESCRIPTOR)); violationList.add(violation); } - return new ValidationResult(violationList); + return violationList; } @Override diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java similarity index 60% rename from src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java rename to src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java index e4033204..83b75966 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java @@ -12,18 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -package build.buf.protovalidate.internal.expression; +package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; -import build.buf.protovalidate.internal.evaluator.Evaluator; -import build.buf.protovalidate.internal.evaluator.Value; +import build.buf.protovalidate.internal.errors.ConstraintViolation; +import build.buf.protovalidate.internal.errors.FieldPathUtils; +import build.buf.protovalidate.internal.expression.CompiledProgram; +import build.buf.protovalidate.internal.expression.Variable; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; /** Evaluator that executes a {@link CompiledProgram}. */ -public class CelPrograms implements Evaluator { +class CelPrograms extends EvaluatorBase implements Evaluator { /** A list of {@link CompiledProgram} that will be executed against the input message. */ private final List programs; @@ -32,7 +33,8 @@ public class CelPrograms implements Evaluator { * * @param compiledPrograms The programs to execute. */ - public CelPrograms(List compiledPrograms) { + CelPrograms(@Nullable ValueEvaluator valueEvaluator, List compiledPrograms) { + super(valueEvaluator); this.programs = compiledPrograms; } @@ -42,18 +44,19 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { Variable activation = Variable.newThisVariable(val.value(Object.class)); - List violationList = new ArrayList<>(); + List violations = new ArrayList<>(); for (CompiledProgram program : programs) { - Violation violation = program.eval(val, activation); + ConstraintViolation.Builder violation = program.eval(val, activation); if (violation != null) { - violationList.add(violation); + violations.add(violation); if (failFast) { break; } } } - return new ValidationResult(violationList); + return FieldPathUtils.updatePaths(violations, getFieldPathElement(), getRulePrefixElements()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java new file mode 100644 index 00000000..3db3352b --- /dev/null +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java @@ -0,0 +1,42 @@ +// Copyright 2023-2024 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.internal.evaluator; + +import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; +import build.buf.protovalidate.internal.errors.FieldPathUtils; +import java.util.ArrayList; +import java.util.List; + +class EmbeddedMessageEvaluator extends EvaluatorBase implements Evaluator { + private final MessageEvaluator messageEvaluator; + + EmbeddedMessageEvaluator(ValueEvaluator valueEvaluator, MessageEvaluator messageEvaluator) { + super(valueEvaluator); + this.messageEvaluator = messageEvaluator; + } + + @Override + public boolean tautology() { + return messageEvaluator.tautology(); + } + + @Override + public List evaluate(Value val, boolean failFast) + throws ExecutionException { + return FieldPathUtils.updatePaths( + messageEvaluator.evaluate(val, failFast), getFieldPathElement(), new ArrayList<>()); + } +} diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index af03c0fb..3ad333e9 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -15,8 +15,8 @@ package build.buf.protovalidate.internal.evaluator; import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.EnumRules; import build.buf.validate.FieldConstraints; @@ -31,7 +31,7 @@ * {@link EnumEvaluator} checks an enum value being a member of the defined values exclusively. This * check is handled outside CEL as enums are completely type erased to integers. */ -class EnumEvaluator implements Evaluator { +class EnumEvaluator extends EvaluatorBase implements Evaluator { /** Captures all the defined values for this enum */ private final Set values; @@ -52,7 +52,9 @@ class EnumEvaluator implements Evaluator { * * @param valueDescriptors the list of {@link Descriptors.EnumValueDescriptor} for the enum. */ - EnumEvaluator(List valueDescriptors) { + EnumEvaluator( + ValueEvaluator valueEvaluator, List valueDescriptors) { + super(valueEvaluator); if (valueDescriptors.isEmpty()) { this.values = Collections.emptySet(); } else { @@ -77,24 +79,23 @@ public boolean tautology() { * @throws ExecutionException if an error occurs during the evaluation. */ @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { Descriptors.EnumValueDescriptor enumValue = val.value(Descriptors.EnumValueDescriptor.class); if (enumValue == null) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } if (!values.contains(enumValue.getNumber())) { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setRule(DEFINED_ONLY_RULE_PATH) - .setConstraintId("enum.defined_only") - .setMessage("value must be one of the defined enum values") - .build()) - .setFieldValue(val.value(Object.class), val.fieldDescriptor()) - .setRuleValue(true, DEFINED_ONLY_DESCRIPTOR) - .build())); + return Collections.singletonList( + ConstraintViolation.newBuilder() + .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(DEFINED_ONLY_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(getFieldPathElement()) + .setConstraintId("enum.defined_only") + .setMessage("value must be one of the defined enum values") + .setFieldValue(new ConstraintViolation.FieldValue(val)) + .setRuleValue(new ConstraintViolation.FieldValue(true, DEFINED_ONLY_DESCRIPTOR))); } - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java index 26ca0b21..6c1e4802 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/Evaluator.java @@ -14,8 +14,9 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; +import java.util.List; /** * {@link Evaluator} defines a validation evaluator. evaluator implementations may elide type @@ -31,13 +32,13 @@ public interface Evaluator { /** * Checks that the provided val is valid. Unless failFast is true, evaluation attempts to find all - * {@link build.buf.validate.Violations} present in val instead of returning a {@link - * ValidationResult} on the first {@link build.buf.validate.Violation}. + * {@link ConstraintViolation} present in val instead of returning only the first {@link + * ConstraintViolation}. * * @param val The value to validate. * @param failFast If true, validation stops after the first failure. * @return The result of validation on the specified value. * @throws ExecutionException If evaluation fails to complete. */ - ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException; + List evaluate(Value val, boolean failFast) throws ExecutionException; } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java new file mode 100644 index 00000000..b46446ce --- /dev/null +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java @@ -0,0 +1,61 @@ +// Copyright 2023-2024 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.internal.evaluator; + +import build.buf.protovalidate.internal.errors.FieldPathUtils; +import build.buf.validate.FieldPath; +import build.buf.validate.FieldPathElement; +import java.util.ArrayList; +import java.util.List; +import javax.annotation.Nullable; + +class EvaluatorBase { + private static final List EMPTY_PREFIX = new ArrayList<>(); + + private final @Nullable FieldPath rulePrefix; + + private final @Nullable FieldPathElement fieldPathElement; + + EvaluatorBase(@Nullable ValueEvaluator evaluator) { + if (evaluator != null) { + this.rulePrefix = evaluator.getNestedRule(); + if (evaluator.getDescriptor() != null) { + this.fieldPathElement = FieldPathUtils.fieldPathElement(evaluator.getDescriptor()); + } else { + this.fieldPathElement = null; + } + } else { + this.rulePrefix = null; + this.fieldPathElement = null; + } + } + + EvaluatorBase(@Nullable FieldPath rulePrefix) { + this.rulePrefix = rulePrefix; + this.fieldPathElement = null; + } + + @Nullable + FieldPathElement getFieldPathElement() { + return fieldPathElement; + } + + List getRulePrefixElements() { + if (rulePrefix == null) { + return EMPTY_PREFIX; + } + return rulePrefix.getElementsList(); + } +} diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index 6b8bca61..f421255c 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -20,7 +20,6 @@ import build.buf.protovalidate.internal.constraints.DescriptorMappings; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.protovalidate.internal.expression.AstExpression; -import build.buf.protovalidate.internal.expression.CelPrograms; import build.buf.protovalidate.internal.expression.CompiledProgram; import build.buf.protovalidate.internal.expression.Expression; import build.buf.protovalidate.internal.expression.Variable; @@ -54,10 +53,9 @@ public class EvaluatorBuilder { private static final FieldPathElement CEL_FIELD_PATH_ELEMENT = FieldPathUtils.fieldPathElement( - FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.CEL_FIELD_NUMBER)) - .build(); + FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.CEL_FIELD_NUMBER)); - private volatile ImmutableMap evaluatorCache = ImmutableMap.of(); + private volatile ImmutableMap evaluatorCache = ImmutableMap.of(); private final Env env; private final boolean disableLazy; @@ -108,7 +106,7 @@ private Evaluator build(Descriptor desc) throws CompilationException { return eval; } // Rebuild cache with this descriptor (and any of its dependencies). - ImmutableMap updatedCache = + ImmutableMap updatedCache = new DescriptorCacheBuilder(env, constraints, evaluatorCache).build(desc); evaluatorCache = updatedCache; eval = updatedCache.get(desc); @@ -124,12 +122,12 @@ private static class DescriptorCacheBuilder { private final ConstraintResolver resolver = new ConstraintResolver(); private final Env env; private final ConstraintCache constraintCache; - private final HashMap cache; + private final HashMap cache; private DescriptorCacheBuilder( Env env, ConstraintCache constraintCache, - ImmutableMap previousCache) { + ImmutableMap previousCache) { this.env = Objects.requireNonNull(env, "env"); this.constraintCache = Objects.requireNonNull(constraintCache, "constraintCache"); this.cache = new HashMap<>(previousCache); @@ -143,14 +141,14 @@ private DescriptorCacheBuilder( * @return Immutable map of descriptors to evaluators. * @throws CompilationException If an error occurs compiling a constraint on the cache. */ - public ImmutableMap build(Descriptor descriptor) + public ImmutableMap build(Descriptor descriptor) throws CompilationException { createMessageEvaluator(descriptor); return ImmutableMap.copyOf(cache); } - private Evaluator createMessageEvaluator(Descriptor desc) throws CompilationException { - Evaluator eval = cache.get(desc); + private MessageEvaluator createMessageEvaluator(Descriptor desc) throws CompilationException { + MessageEvaluator eval = cache.get(desc); if (eval != null) { return eval; } @@ -197,7 +195,7 @@ private void processMessageExpressions( if (compiledPrograms.isEmpty()) { throw new CompilationException("compile returned null"); } - msgEval.append(new CelPrograms(compiledPrograms)); + msgEval.append(new CelPrograms(null, compiledPrograms)); } private void processOneofConstraints(Descriptor desc, MessageEvaluator msgEval) @@ -225,7 +223,7 @@ private void processFields(Descriptor desc, MessageEvaluator msgEval) private FieldEvaluator buildField( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints) throws CompilationException { - ValueEvaluator valueEvaluatorEval = new ValueEvaluator(); + ValueEvaluator valueEvaluatorEval = new ValueEvaluator(fieldDescriptor, null); boolean ignoreDefault = fieldDescriptor.hasPresence() && shouldIgnoreDefault(fieldConstraints); Object zero = null; @@ -240,7 +238,7 @@ private FieldEvaluator buildField( fieldDescriptor.hasPresence() || shouldIgnoreEmpty(fieldConstraints), fieldDescriptor.hasPresence() && shouldIgnoreDefault(fieldConstraints), zero); - buildValue(fieldDescriptor, fieldConstraints, null, fieldEvaluator.valueEvaluator); + buildValue(fieldDescriptor, fieldConstraints, fieldEvaluator.valueEvaluator); return fieldEvaluator; } @@ -263,27 +261,25 @@ private static boolean shouldIgnoreDefault(FieldConstraints constraints) { private void buildValue( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluator) throws CompilationException { - processIgnoreEmpty(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); + processIgnoreEmpty(fieldDescriptor, fieldConstraints, valueEvaluator); processFieldExpressions(fieldDescriptor, fieldConstraints, valueEvaluator); - processEmbeddedMessage(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processWrapperConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processStandardConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processAnyConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processEnumConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processMapConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); - processRepeatedConstraints(fieldDescriptor, fieldConstraints, itemsWrapper, valueEvaluator); + processEmbeddedMessage(fieldDescriptor, fieldConstraints, valueEvaluator); + processWrapperConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); + processStandardConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); + processAnyConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); + processEnumConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); + processMapConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); + processRepeatedConstraints(fieldDescriptor, fieldConstraints, valueEvaluator); } private void processIgnoreEmpty( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { - if (itemsWrapper != null && shouldIgnoreEmpty(fieldConstraints)) { + if (valueEvaluatorEval.getNestedRule() != null && shouldIgnoreEmpty(fieldConstraints)) { valueEvaluatorEval.setIgnoreEmpty(zeroValue(fieldDescriptor, true)); } } @@ -377,36 +373,36 @@ private void processFieldExpressions( List compiledPrograms = compileConstraints(constraintsCelList, finalEnv, true); if (!compiledPrograms.isEmpty()) { - valueEvaluatorEval.append(new CelPrograms(compiledPrograms)); + valueEvaluatorEval.append(new CelPrograms(valueEvaluatorEval, compiledPrograms)); } } private void processEmbeddedMessage( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE || shouldSkip(fieldConstraints) || fieldDescriptor.isMapField() - || (fieldDescriptor.isRepeated() && itemsWrapper == null)) { + || (fieldDescriptor.isRepeated() && valueEvaluatorEval.getNestedRule() == null)) { return; } - Evaluator embedEval = createMessageEvaluator(fieldDescriptor.getMessageType()); + Evaluator embedEval = + new EmbeddedMessageEvaluator( + valueEvaluatorEval, createMessageEvaluator(fieldDescriptor.getMessageType())); valueEvaluatorEval.append(embedEval); } private void processWrapperConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE || shouldSkip(fieldConstraints) || fieldDescriptor.isMapField() - || (fieldDescriptor.isRepeated() && itemsWrapper == null)) { + || (fieldDescriptor.isRepeated() && valueEvaluatorEval.getNestedRule() == null)) { return; } FieldDescriptor expectedWrapperDescriptor = @@ -416,102 +412,94 @@ private void processWrapperConstraints( || !fieldConstraints.hasField(expectedWrapperDescriptor)) { return; } - ValueEvaluator unwrapped = new ValueEvaluator(); + ValueEvaluator unwrapped = + new ValueEvaluator( + valueEvaluatorEval.getDescriptor(), valueEvaluatorEval.getNestedRule()); buildValue( - fieldDescriptor.getMessageType().findFieldByName("value"), - fieldConstraints, - itemsWrapper, - unwrapped); + fieldDescriptor.getMessageType().findFieldByName("value"), fieldConstraints, unwrapped); valueEvaluatorEval.append(unwrapped); } private void processStandardConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { List compile = - constraintCache.compile(fieldDescriptor, fieldConstraints, itemsWrapper != null); + constraintCache.compile( + fieldDescriptor, fieldConstraints, valueEvaluatorEval.getNestedRule() != null); if (compile.isEmpty()) { return; } - appendEvaluator(valueEvaluatorEval, new CelPrograms(compile), itemsWrapper); + valueEvaluatorEval.append(new CelPrograms(valueEvaluatorEval, compile)); } private void processAnyConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) { - if ((fieldDescriptor.isRepeated() && itemsWrapper == null) + if ((fieldDescriptor.isRepeated() && valueEvaluatorEval.getNestedRule() == null) || fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE || !fieldDescriptor.getMessageType().getFullName().equals("google.protobuf.Any")) { return; } FieldDescriptor typeURLDesc = fieldDescriptor.getMessageType().findFieldByName("type_url"); - AnyEvaluator anyEvaluatorEval = + valueEvaluatorEval.append( new AnyEvaluator( + valueEvaluatorEval, typeURLDesc, fieldConstraints.getAny().getInList(), - fieldConstraints.getAny().getNotInList()); - appendEvaluator(valueEvaluatorEval, anyEvaluatorEval, itemsWrapper); + fieldConstraints.getAny().getNotInList())); } private void processEnumConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) { if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.ENUM) { return; } if (fieldConstraints.getEnum().getDefinedOnly()) { Descriptors.EnumDescriptor enumDescriptor = fieldDescriptor.getEnumType(); - appendEvaluator( - valueEvaluatorEval, new EnumEvaluator(enumDescriptor.getValues()), itemsWrapper); + valueEvaluatorEval.append( + new EnumEvaluator(valueEvaluatorEval, enumDescriptor.getValues())); } } private void processMapConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { if (!fieldDescriptor.isMapField()) { return; } - MapEvaluator mapEval = new MapEvaluator(fieldDescriptor); + MapEvaluator mapEval = new MapEvaluator(valueEvaluatorEval, fieldDescriptor); buildValue( fieldDescriptor.getMessageType().findFieldByNumber(1), fieldConstraints.getMap().getKeys(), - MapEvaluator.KEYS_WRAPPER, mapEval.getKeyEvaluator()); buildValue( fieldDescriptor.getMessageType().findFieldByNumber(2), fieldConstraints.getMap().getValues(), - MapEvaluator.VALUES_WRAPPER, mapEval.getValueEvaluator()); - appendEvaluator(valueEvaluatorEval, mapEval, itemsWrapper); + valueEvaluatorEval.append(mapEval); } private void processRepeatedConstraints( FieldDescriptor fieldDescriptor, FieldConstraints fieldConstraints, - @Nullable EvaluatorWrapper itemsWrapper, ValueEvaluator valueEvaluatorEval) throws CompilationException { - if (fieldDescriptor.isMapField() || !fieldDescriptor.isRepeated() || itemsWrapper != null) { + if (fieldDescriptor.isMapField() + || !fieldDescriptor.isRepeated() + || valueEvaluatorEval.getNestedRule() != null) { return; } - ListEvaluator listEval = new ListEvaluator(fieldDescriptor); + ListEvaluator listEval = new ListEvaluator(valueEvaluatorEval); buildValue( - fieldDescriptor, - fieldConstraints.getRepeated().getItems(), - ListEvaluator.ITEMS_WRAPPER, - listEval.itemConstraints); - appendEvaluator(valueEvaluatorEval, listEval, itemsWrapper); + fieldDescriptor, fieldConstraints.getRepeated().getItems(), listEval.itemConstraints); + valueEvaluatorEval.append(listEval); } private static List compileConstraints( @@ -538,12 +526,4 @@ private static List compileConstraints( return compiledPrograms; } } - - private static void appendEvaluator( - ValueEvaluator valueEvaluatorEval, Evaluator embedEval, @Nullable EvaluatorWrapper wrapper) { - if (wrapper != null) { - embedEval = wrapper.wrap(embedEval); - } - valueEvaluatorEval.append(embedEval); - } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorWrapper.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorWrapper.java deleted file mode 100644 index 03c3cd5f..00000000 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorWrapper.java +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2023-2024 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.internal.evaluator; - -/** Wrappers are used to special-case certain aspects of handling nested rules. */ -interface EvaluatorWrapper { - /** - * Returns a wrapped evaluator for a given kind of nested rule. - * - * @param evaluator Evaluator to wrap inside a nested evaluator wrapper. - * @return A wrapped Evaluator that proxies the provided evaluator. - */ - Evaluator wrap(Evaluator evaluator); -} diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 571e30c0..29ce2f8f 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -14,9 +14,8 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; @@ -28,7 +27,7 @@ import javax.annotation.Nullable; /** Performs validation on a single message field, defined by its descriptor. */ -class FieldEvaluator implements Evaluator { +class FieldEvaluator extends EvaluatorBase implements Evaluator { private static final FieldDescriptor REQUIRED_DESCRIPTOR = FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.REQUIRED_FIELD_NUMBER); @@ -64,6 +63,7 @@ class FieldEvaluator implements Evaluator { boolean ignoreEmpty, boolean ignoreDefault, @Nullable Object zero) { + super(valueEvaluator); this.valueEvaluator = valueEvaluator; this.descriptor = descriptor; this.required = required; @@ -78,10 +78,11 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { Message message = val.messageValue(); if (message == null) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } boolean hasField; if (descriptor.isRepeated()) { @@ -90,36 +91,22 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx hasField = message.hasField(descriptor); } if (required && !hasField) { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setField( - FieldPath.newBuilder() - .addElements(FieldPathUtils.fieldPathElement(descriptor)) - .build()) - .setRule(REQUIRED_RULE_PATH) - .setConstraintId("required") - .setMessage("value is required") - .build()) - .setFieldValue(val.value(Object.class), val.fieldDescriptor()) - .setRuleValue(true, REQUIRED_DESCRIPTOR) - .build())); + return Collections.singletonList( + ConstraintViolation.newBuilder() + .addFirstFieldPathElement(FieldPathUtils.fieldPathElement(descriptor)) + .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(REQUIRED_RULE_PATH.getElementsList()) + .setConstraintId("required") + .setMessage("value is required") + .setRuleValue(new ConstraintViolation.FieldValue(true, REQUIRED_DESCRIPTOR))); } if (ignoreEmpty && !hasField) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } Object fieldValue = message.getField(descriptor); if (ignoreDefault && Objects.equals(zero, fieldValue)) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } - ValidationResult evalResult = - valueEvaluator.evaluate(new ObjectValue(descriptor, fieldValue), failFast); - List violations = - FieldPathUtils.prependFieldPaths( - evalResult.getViolations(), - FieldPathUtils.fieldPathElement(descriptor).build(), - descriptor.isRepeated()); - return new ValidationResult(violations); + return valueEvaluator.evaluate(new ObjectValue(descriptor, fieldValue), failFast); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index cf9cf6b0..0f7accec 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -14,22 +14,21 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; import build.buf.validate.RepeatedRules; -import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** Performs validation on the elements of a repeated field. */ -class ListEvaluator implements Evaluator { - /** Rule path to repeated items rules */ - private static final List REPEATED_ITEMS_RULE_PATH = +class ListEvaluator extends EvaluatorBase implements Evaluator { + /** Rule path to repeated rules */ + private static final FieldPath REPEATED_ITEMS_RULE_PATH = FieldPath.newBuilder() .addElements( FieldPathUtils.fieldPathElement( @@ -39,52 +38,15 @@ class ListEvaluator implements Evaluator { FieldPathUtils.fieldPathElement( RepeatedRules.getDescriptor() .findFieldByNumber(RepeatedRules.ITEMS_FIELD_NUMBER))) - .build() - .getElementsList(); - - public static final EvaluatorWrapper ITEMS_WRAPPER = new ItemsWrapper(); - - private static class ItemsEvaluator implements Evaluator { - /** Evaluator to wrap */ - private final Evaluator evaluator; - - public ItemsEvaluator(Evaluator evaluator) { - this.evaluator = evaluator; - } - - @Override - public boolean tautology() { - return this.evaluator.tautology(); - } - - @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - ValidationResult result = evaluator.evaluate(val, failFast); - if (result.isSuccess()) { - return result; - } - return new ValidationResult( - FieldPathUtils.prependRulePaths(result.getViolations(), REPEATED_ITEMS_RULE_PATH)); - } - } - - private static class ItemsWrapper implements EvaluatorWrapper { - @Override - public Evaluator wrap(Evaluator evaluator) { - return new ItemsEvaluator(evaluator); - } - } + .build(); /** Constraints are checked on every item of the list. */ final ValueEvaluator itemConstraints; - /** Field descriptor of the list field. */ - final Descriptors.FieldDescriptor fieldDescriptor; - /** Constructs a {@link ListEvaluator}. */ - ListEvaluator(Descriptors.FieldDescriptor fieldDescriptor) { - this.itemConstraints = new ValueEvaluator(); - this.fieldDescriptor = fieldDescriptor; + ListEvaluator(ValueEvaluator valueEvaluator) { + super(valueEvaluator); + this.itemConstraints = new ValueEvaluator(null, REPEATED_ITEMS_RULE_PATH); } @Override @@ -93,24 +55,24 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - List allViolations = new ArrayList<>(); + public List evaluate(Value val, boolean failFast) + throws ExecutionException { + List allViolations = new ArrayList<>(); List repeatedValues = val.repeatedValue(); for (int i = 0; i < repeatedValues.size(); i++) { - ValidationResult evalResult = itemConstraints.evaluate(repeatedValues.get(i), failFast); - if (evalResult.getViolations().isEmpty()) { + List violations = + itemConstraints.evaluate(repeatedValues.get(i), failFast); + if (violations.isEmpty()) { continue; } - List violations = - FieldPathUtils.prependFieldPaths( - evalResult.getViolations(), - FieldPathUtils.fieldPathElement(fieldDescriptor).setIndex(i).build(), - false); + FieldPathElement fieldPathElement = + Objects.requireNonNull(getFieldPathElement()).toBuilder().setIndex(i).build(); + FieldPathUtils.updatePaths(violations, fieldPathElement, getRulePrefixElements()); if (failFast && !violations.isEmpty()) { - return evalResult; + return violations; } allViolations.addAll(violations); } - return new ValidationResult(allViolations); + return allViolations; } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index 599e4d06..d795d0bf 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -14,9 +14,8 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; import build.buf.validate.FieldConstraints; import build.buf.validate.FieldPath; @@ -27,12 +26,13 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; /** Performs validation on a map field's key-value pairs. */ -class MapEvaluator implements Evaluator { +class MapEvaluator extends EvaluatorBase implements Evaluator { /** Rule path to map key rules */ - private static final List MAP_KEYS_RULE_PATH = + private static final FieldPath MAP_KEYS_RULE_PATH = FieldPath.newBuilder() .addElements( FieldPathUtils.fieldPathElement( @@ -41,11 +41,10 @@ class MapEvaluator implements Evaluator { .addElements( FieldPathUtils.fieldPathElement( MapRules.getDescriptor().findFieldByNumber(MapRules.KEYS_FIELD_NUMBER))) - .build() - .getElementsList(); + .build(); /** Rule path to map value rules */ - private static final List MAP_VALUES_RULE_PATH = + private static final FieldPath MAP_VALUES_RULE_PATH = FieldPath.newBuilder() .addElements( FieldPathUtils.fieldPathElement( @@ -54,74 +53,7 @@ class MapEvaluator implements Evaluator { .addElements( FieldPathUtils.fieldPathElement( MapRules.getDescriptor().findFieldByNumber(MapRules.VALUES_FIELD_NUMBER))) - .build() - .getElementsList(); - - public static final EvaluatorWrapper KEYS_WRAPPER = new KeysWrapper(); - - public static final EvaluatorWrapper VALUES_WRAPPER = new ValuesWrapper(); - - private static class KeysEvaluator implements Evaluator { - /** Evaluator to wrap */ - private final Evaluator evaluator; - - public KeysEvaluator(Evaluator evaluator) { - this.evaluator = evaluator; - } - - @Override - public boolean tautology() { - return this.evaluator.tautology(); - } - - @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - ValidationResult result = evaluator.evaluate(val, failFast); - if (result.isSuccess()) { - return result; - } - return new ValidationResult( - FieldPathUtils.prependRulePaths(result.getViolations(), MAP_KEYS_RULE_PATH)); - } - } - - private static class KeysWrapper implements EvaluatorWrapper { - @Override - public Evaluator wrap(Evaluator evaluator) { - return new KeysEvaluator(evaluator); - } - } - - private static class ValuesEvaluator implements Evaluator { - /** Evaluator to wrap */ - private final Evaluator evaluator; - - public ValuesEvaluator(Evaluator evaluator) { - this.evaluator = evaluator; - } - - @Override - public boolean tautology() { - return this.evaluator.tautology(); - } - - @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - ValidationResult result = evaluator.evaluate(val, failFast); - if (result.isSuccess()) { - return result; - } - return new ValidationResult( - FieldPathUtils.prependRulePaths(result.getViolations(), MAP_VALUES_RULE_PATH)); - } - } - - private static class ValuesWrapper implements EvaluatorWrapper { - @Override - public Evaluator wrap(Evaluator evaluator) { - return new ValuesEvaluator(evaluator); - } - } + .build(); /** Constraint for checking the map keys */ private final ValueEvaluator keyEvaluator; @@ -141,11 +73,12 @@ public Evaluator wrap(Evaluator evaluator) { /** * Constructs a {@link MapEvaluator}. * - * @param fieldDescriptor The descriptor of the map field being evaluated. + * @param valueEvaluator The value evaluator this constraint exists under. */ - MapEvaluator(Descriptors.FieldDescriptor fieldDescriptor) { - this.keyEvaluator = new ValueEvaluator(); - this.valueEvaluator = new ValueEvaluator(); + MapEvaluator(ValueEvaluator valueEvaluator, Descriptors.FieldDescriptor fieldDescriptor) { + super(valueEvaluator); + this.keyEvaluator = new ValueEvaluator(null, MAP_KEYS_RULE_PATH); + this.valueEvaluator = new ValueEvaluator(null, MAP_VALUES_RULE_PATH); this.fieldDescriptor = fieldDescriptor; this.keyFieldDescriptor = fieldDescriptor.getMessageType().findFieldByNumber(1); this.valueFieldDescriptor = fieldDescriptor.getMessageType().findFieldByNumber(2); @@ -175,49 +108,48 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - List violations = new ArrayList<>(); + public List evaluate(Value val, boolean failFast) + throws ExecutionException { + List violations = new ArrayList<>(); Map mapValue = val.mapValue(); for (Map.Entry entry : mapValue.entrySet()) { violations.addAll(evalPairs(entry.getKey(), entry.getValue(), failFast)); if (failFast && !violations.isEmpty()) { - return new ValidationResult(violations); + return violations; } } if (violations.isEmpty()) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } - return new ValidationResult(violations); + return violations; } - private List evalPairs(Value key, Value value, boolean failFast) + private List evalPairs(Value key, Value value, boolean failFast) throws ExecutionException { - List keyViolations = - keyEvaluator.evaluate(key, failFast).getViolations().stream() - .map( - violation -> - violation.toBuilder() - .setProto(violation.toProto().toBuilder().setForKey(true).build()) - .build()) + List keyViolations = + keyEvaluator.evaluate(key, failFast).stream() + .map(violation -> violation.setForKey(true)) .collect(Collectors.toList()); - final List valueViolations; + final List valueViolations; if (failFast && !keyViolations.isEmpty()) { // Don't evaluate value constraints if failFast is enabled and keys failed validation. // We still need to continue execution to the end to properly prefix violation field paths. - valueViolations = Collections.emptyList(); + valueViolations = ConstraintViolation.NO_VIOLATIONS; } else { - valueViolations = valueEvaluator.evaluate(value, failFast).getViolations(); + valueViolations = valueEvaluator.evaluate(value, failFast); } if (keyViolations.isEmpty() && valueViolations.isEmpty()) { return Collections.emptyList(); } - List violations = new ArrayList<>(keyViolations.size() + valueViolations.size()); + List violations = + new ArrayList<>(keyViolations.size() + valueViolations.size()); violations.addAll(keyViolations); violations.addAll(valueViolations); - FieldPathElement.Builder fieldPathElement = FieldPathUtils.fieldPathElement(fieldDescriptor); - fieldPathElement.setKeyType(keyFieldDescriptor.getType().toProto()); - fieldPathElement.setValueType(valueFieldDescriptor.getType().toProto()); + FieldPathElement.Builder fieldPathElementBuilder = + Objects.requireNonNull(getFieldPathElement()).toBuilder(); + fieldPathElementBuilder.setKeyType(keyFieldDescriptor.getType().toProto()); + fieldPathElementBuilder.setValueType(valueFieldDescriptor.getType().toProto()); switch (keyFieldDescriptor.getType().toProto()) { case TYPE_INT64: case TYPE_INT32: @@ -225,23 +157,24 @@ private List evalPairs(Value key, Value value, boolean failFast) case TYPE_SINT64: case TYPE_SFIXED32: case TYPE_SFIXED64: - fieldPathElement.setIntKey(key.value(Number.class).longValue()); + fieldPathElementBuilder.setIntKey(key.value(Number.class).longValue()); break; case TYPE_UINT32: case TYPE_UINT64: case TYPE_FIXED32: case TYPE_FIXED64: - fieldPathElement.setUintKey(key.value(Number.class).longValue()); + fieldPathElementBuilder.setUintKey(key.value(Number.class).longValue()); break; case TYPE_BOOL: - fieldPathElement.setBoolKey(key.value(Boolean.class)); + fieldPathElementBuilder.setBoolKey(key.value(Boolean.class)); break; case TYPE_STRING: - fieldPathElement.setStringKey(key.value(String.class)); + fieldPathElementBuilder.setStringKey(key.value(String.class)); break; default: throw new ExecutionException("Unexpected map key type"); } - return FieldPathUtils.prependFieldPaths(violations, fieldPathElement.build(), false); + FieldPathElement fieldPathElement = fieldPathElementBuilder.build(); + return FieldPathUtils.updatePaths(violations, fieldPathElement, getRulePrefixElements()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java index 86773e33..54a6075f 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MessageEvaluator.java @@ -14,9 +14,8 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import java.util.ArrayList; import java.util.List; @@ -36,19 +35,20 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - List violations = new ArrayList<>(); + public List evaluate(Value val, boolean failFast) + throws ExecutionException { + List allViolations = new ArrayList<>(); for (Evaluator evaluator : evaluators) { - ValidationResult evalResult = evaluator.evaluate(val, failFast); - if (failFast && !evalResult.getViolations().isEmpty()) { - return evalResult; + List violations = evaluator.evaluate(val, failFast); + if (failFast && !violations.isEmpty()) { + return violations; } - violations.addAll(evalResult.getViolations()); + allViolations.addAll(violations); } - if (violations.isEmpty()) { - return ValidationResult.EMPTY; + if (allViolations.isEmpty()) { + return ConstraintViolation.NO_VIOLATIONS; } - return new ValidationResult(violations); + return allViolations; } /** diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java index 7cc677d8..a529d0b1 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/OneofEvaluator.java @@ -14,14 +14,13 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; -import build.buf.validate.FieldPath; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.validate.FieldPathElement; import com.google.protobuf.Descriptors.OneofDescriptor; import com.google.protobuf.Message; import java.util.Collections; +import java.util.List; /** {@link OneofEvaluator} performs validation on a oneof union. */ public class OneofEvaluator implements Evaluator { @@ -48,26 +47,17 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { Message message = val.messageValue(); - if (message == null) { - return ValidationResult.EMPTY; + if (message == null || !required || (message.getOneofFieldDescriptor(descriptor) != null)) { + return ConstraintViolation.NO_VIOLATIONS; } - if (required && (message.getOneofFieldDescriptor(descriptor) == null)) { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setField( - FieldPath.newBuilder() - .addElements( - FieldPathElement.newBuilder() - .setFieldName(descriptor.getName()))) - .setConstraintId("required") - .setMessage("exactly one field is required in oneof") - .build()) - .build())); - } - return ValidationResult.EMPTY; + return Collections.singletonList( + ConstraintViolation.newBuilder() + .addFirstFieldPathElement( + FieldPathElement.newBuilder().setFieldName(descriptor.getName()).build()) + .setConstraintId("required") + .setMessage("exactly one field is required in oneof")); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java index f5cb6f38..76552636 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/UnknownDescriptorEvaluator.java @@ -14,11 +14,11 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import com.google.protobuf.Descriptors.Descriptor; import java.util.Collections; +import java.util.List; /** * An {@link Evaluator} for an unknown descriptor. This is returned only if lazy-building of @@ -39,13 +39,10 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder( - build.buf.validate.Violation.newBuilder() - .setMessage("No evaluator available for " + desc.getFullName()) - .build()) - .build())); + public List evaluate(Value val, boolean failFast) + throws ExecutionException { + return Collections.singletonList( + ConstraintViolation.newBuilder() + .setMessage("No evaluator available for " + desc.getFullName())); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java index 3d604354..a84359da 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java @@ -14,9 +14,10 @@ package build.buf.protovalidate.internal.evaluator; -import build.buf.protovalidate.ValidationResult; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; +import build.buf.validate.FieldPath; +import com.google.protobuf.Descriptors; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -27,6 +28,12 @@ * field, repeated elements, or the keys/values of a map. */ class ValueEvaluator implements Evaluator { + /** The {@link Descriptors.FieldDescriptor} targeted by this evaluator */ + @Nullable private final Descriptors.FieldDescriptor descriptor; + + /** The nested rule path that this value evaluator is for */ + @Nullable private final FieldPath nestedRule; + /** The default or zero-value for this value's type. */ @Nullable private Object zero; @@ -40,7 +47,18 @@ class ValueEvaluator implements Evaluator { private boolean ignoreEmpty; /** Constructs a {@link ValueEvaluator}. */ - ValueEvaluator() {} + ValueEvaluator(@Nullable Descriptors.FieldDescriptor descriptor, @Nullable FieldPath nestedRule) { + this.descriptor = descriptor; + this.nestedRule = nestedRule; + } + + public @Nullable Descriptors.FieldDescriptor getDescriptor() { + return descriptor; + } + + public @Nullable FieldPath getNestedRule() { + return nestedRule; + } @Override public boolean tautology() { @@ -48,22 +66,23 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { + public List evaluate(Value val, boolean failFast) + throws ExecutionException { if (this.shouldIgnore(val.value(Object.class))) { - return ValidationResult.EMPTY; + return ConstraintViolation.NO_VIOLATIONS; } - List violations = new ArrayList<>(); + List allViolations = new ArrayList<>(); for (Evaluator evaluator : evaluators) { - ValidationResult evalResult = evaluator.evaluate(val, failFast); - if (failFast && !evalResult.getViolations().isEmpty()) { - return evalResult; + List violations = evaluator.evaluate(val, failFast); + if (failFast && !violations.isEmpty()) { + return violations; } - violations.addAll(evalResult.getViolations()); + allViolations.addAll(violations); } - if (violations.isEmpty()) { - return ValidationResult.EMPTY; + if (allViolations.isEmpty()) { + return ConstraintViolation.NO_VIOLATIONS; } - return new ValidationResult(violations); + return allViolations; } /** diff --git a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java index a83910bd..afb594e6 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -14,8 +14,8 @@ package build.buf.protovalidate.internal.expression; -import build.buf.protovalidate.Violation; import build.buf.protovalidate.exceptions.ExecutionException; +import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.evaluator.Value; import build.buf.validate.FieldPath; import javax.annotation.Nullable; @@ -66,7 +66,8 @@ public CompiledProgram( * @throws ExecutionException If the evaluation of the CEL program fails with an error. */ @Nullable - public Violation eval(Value fieldValue, Variable bindings) throws ExecutionException { + public ConstraintViolation.Builder eval(Value fieldValue, Variable bindings) + throws ExecutionException { Program.EvalResult evalResult = program.eval(bindings); Val val = evalResult.getVal(); if (val instanceof Err) { @@ -77,38 +78,35 @@ public Violation eval(Value fieldValue, Variable bindings) throws ExecutionExcep if ("".equals(value)) { return null; } - build.buf.validate.Violation.Builder violation = - build.buf.validate.Violation.newBuilder() + ConstraintViolation.Builder builder = + ConstraintViolation.newBuilder() .setConstraintId(this.source.id) .setMessage(value.toString()); + if (fieldValue.fieldDescriptor() != null) { + builder.setFieldValue(new ConstraintViolation.FieldValue(fieldValue)); + } if (rulePath != null) { - violation.setRule(rulePath); + builder.addAllRulePathElements(rulePath.getElementsList()); } - Violation.Builder builder = - Violation.newBuilder(violation.build()) - .setFieldValue(fieldValue.value(Object.class), fieldValue.fieldDescriptor()); - if (ruleValue != null) { - builder.setRuleValue(ruleValue.value(Object.class), ruleValue.fieldDescriptor()); + if (ruleValue != null && ruleValue.fieldDescriptor() != null) { + builder.setRuleValue(new ConstraintViolation.FieldValue(ruleValue)); } - return builder.build(); + return builder; } else if (value instanceof Boolean) { if (val.booleanValue()) { return null; } - build.buf.validate.Violation.Builder violation = - build.buf.validate.Violation.newBuilder() + ConstraintViolation.Builder builder = + ConstraintViolation.newBuilder() .setConstraintId(this.source.id) .setMessage(this.source.message); if (rulePath != null) { - violation.setRule(rulePath); + builder.addAllRulePathElements(rulePath.getElementsList()); } - Violation.Builder builder = - Violation.newBuilder(violation.build()) - .setFieldValue(fieldValue.value(Object.class), fieldValue.fieldDescriptor()); - if (ruleValue != null) { - builder.setRuleValue(ruleValue.value(Object.class), ruleValue.fieldDescriptor()); + if (ruleValue != null && ruleValue.fieldDescriptor() != null) { + builder.setRuleValue(new ConstraintViolation.FieldValue(ruleValue)); } - return builder.build(); + return builder; } else { throw new ExecutionException(String.format("resolved to an unexpected type %s", val)); } diff --git a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java index 9bcbd680..9cd6d8a1 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -82,8 +82,9 @@ public void testFieldConstraintDynamicMessage() throws Exception { .build(); ValidationResult result = new Validator().validate(messageBuilder.build()); assertThat(result.toProto().getViolationsList()).containsExactly(expectedViolation); - assertThat(result.getViolations().get(0).getFieldValue()).isEqualTo("0123456789"); - assertThat(result.getViolations().get(0).getRuleValue()).isEqualTo("^[a-z0-9]{1,9}$"); + assertThat(result.getViolations().get(0).getFieldValue().getValue()).isEqualTo("0123456789"); + assertThat(result.getViolations().get(0).getRuleValue().getValue()) + .isEqualTo("^[a-z0-9]{1,9}$"); } @Test From 362b5bf5a73e1f59405baa368897eacf6da03aab Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 11 Dec 2024 22:26:02 -0500 Subject: [PATCH 10/11] Address review feedback --- .../java/build/buf/protovalidate/Validator.java | 2 +- .../internal/evaluator/AnyEvaluator.java | 13 +++++++------ .../internal/evaluator/CelPrograms.java | 11 ++++++++--- ...atorBase.java => ConstraintViolationHelper.java} | 6 +++--- .../evaluator/EmbeddedMessageEvaluator.java | 11 +++++++---- .../internal/evaluator/EnumEvaluator.java | 10 ++++++---- .../internal/evaluator/FieldEvaluator.java | 8 +++++--- .../internal/evaluator/ListEvaluator.java | 13 +++++++++---- .../internal/evaluator/MapEvaluator.java | 11 +++++++---- 9 files changed, 53 insertions(+), 32 deletions(-) rename src/main/java/build/buf/protovalidate/internal/evaluator/{EvaluatorBase.java => ConstraintViolationHelper.java} (91%) diff --git a/src/main/java/build/buf/protovalidate/Validator.java b/src/main/java/build/buf/protovalidate/Validator.java index 259a597a..a812649b 100644 --- a/src/main/java/build/buf/protovalidate/Validator.java +++ b/src/main/java/build/buf/protovalidate/Validator.java @@ -81,7 +81,7 @@ public ValidationResult validate(Message msg) throws ValidationException { if (result.isEmpty()) { return ValidationResult.EMPTY; } - List violations = new ArrayList<>(); + List violations = new ArrayList<>(result.size()); for (ConstraintViolation.Builder builder : result) { violations.add(builder.build()); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index f63cc1a4..55e8fc91 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -34,7 +34,8 @@ * com.google.protobuf.Any}'s within an expression, breaking evaluation if the type is unknown at * runtime. */ -class AnyEvaluator extends EvaluatorBase implements Evaluator { +class AnyEvaluator implements Evaluator { + private final ConstraintViolationHelper constraintViolationHelper; private final Descriptors.FieldDescriptor typeURLDescriptor; private final Set in; private final List inValue; @@ -68,7 +69,7 @@ class AnyEvaluator extends EvaluatorBase implements Evaluator { Descriptors.FieldDescriptor typeURLDescriptor, List in, List notIn) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.typeURLDescriptor = typeURLDescriptor; this.in = stringsToSet(in); this.inValue = in; @@ -88,9 +89,9 @@ public List evaluate(Value val, boolean failFast) if (!in.isEmpty() && !in.contains(typeURL)) { ConstraintViolation.Builder violation = ConstraintViolation.newBuilder() - .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) .addAllRulePathElements(IN_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(getFieldPathElement()) + .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) .setConstraintId("any.in") .setMessage("type URL must be in the allow list") .setFieldValue(new ConstraintViolation.FieldValue(val)) @@ -103,9 +104,9 @@ public List evaluate(Value val, boolean failFast) if (!notIn.isEmpty() && notIn.contains(typeURL)) { ConstraintViolation.Builder violation = ConstraintViolation.newBuilder() - .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) .addAllRulePathElements(NOT_IN_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(getFieldPathElement()) + .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) .setConstraintId("any.not_in") .setMessage("type URL must not be in the block list") .setFieldValue(new ConstraintViolation.FieldValue(val)) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java index 83b75966..ccc2c540 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java @@ -24,7 +24,9 @@ import javax.annotation.Nullable; /** Evaluator that executes a {@link CompiledProgram}. */ -class CelPrograms extends EvaluatorBase implements Evaluator { +class CelPrograms implements Evaluator { + private final ConstraintViolationHelper constraintViolationHelper; + /** A list of {@link CompiledProgram} that will be executed against the input message. */ private final List programs; @@ -34,7 +36,7 @@ class CelPrograms extends EvaluatorBase implements Evaluator { * @param compiledPrograms The programs to execute. */ CelPrograms(@Nullable ValueEvaluator valueEvaluator, List compiledPrograms) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.programs = compiledPrograms; } @@ -57,6 +59,9 @@ public List evaluate(Value val, boolean failFast) } } } - return FieldPathUtils.updatePaths(violations, getFieldPathElement(), getRulePrefixElements()); + return FieldPathUtils.updatePaths( + violations, + constraintViolationHelper.getFieldPathElement(), + constraintViolationHelper.getRulePrefixElements()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java similarity index 91% rename from src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java rename to src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java index b46446ce..7abcee01 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBase.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java @@ -21,14 +21,14 @@ import java.util.List; import javax.annotation.Nullable; -class EvaluatorBase { +class ConstraintViolationHelper { private static final List EMPTY_PREFIX = new ArrayList<>(); private final @Nullable FieldPath rulePrefix; private final @Nullable FieldPathElement fieldPathElement; - EvaluatorBase(@Nullable ValueEvaluator evaluator) { + ConstraintViolationHelper(@Nullable ValueEvaluator evaluator) { if (evaluator != null) { this.rulePrefix = evaluator.getNestedRule(); if (evaluator.getDescriptor() != null) { @@ -42,7 +42,7 @@ class EvaluatorBase { } } - EvaluatorBase(@Nullable FieldPath rulePrefix) { + ConstraintViolationHelper(@Nullable FieldPath rulePrefix) { this.rulePrefix = rulePrefix; this.fieldPathElement = null; } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java index 3db3352b..7eb3b8a9 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java @@ -17,14 +17,15 @@ import build.buf.protovalidate.exceptions.ExecutionException; import build.buf.protovalidate.internal.errors.ConstraintViolation; import build.buf.protovalidate.internal.errors.FieldPathUtils; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; -class EmbeddedMessageEvaluator extends EvaluatorBase implements Evaluator { +class EmbeddedMessageEvaluator implements Evaluator { + private final ConstraintViolationHelper constraintViolationHelper; private final MessageEvaluator messageEvaluator; EmbeddedMessageEvaluator(ValueEvaluator valueEvaluator, MessageEvaluator messageEvaluator) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.messageEvaluator = messageEvaluator; } @@ -37,6 +38,8 @@ public boolean tautology() { public List evaluate(Value val, boolean failFast) throws ExecutionException { return FieldPathUtils.updatePaths( - messageEvaluator.evaluate(val, failFast), getFieldPathElement(), new ArrayList<>()); + messageEvaluator.evaluate(val, failFast), + constraintViolationHelper.getFieldPathElement(), + Collections.emptyList()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 3ad333e9..1d690da4 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -31,7 +31,9 @@ * {@link EnumEvaluator} checks an enum value being a member of the defined values exclusively. This * check is handled outside CEL as enums are completely type erased to integers. */ -class EnumEvaluator extends EvaluatorBase implements Evaluator { +class EnumEvaluator implements Evaluator { + private final ConstraintViolationHelper constraintViolationHelper; + /** Captures all the defined values for this enum */ private final Set values; @@ -54,7 +56,7 @@ class EnumEvaluator extends EvaluatorBase implements Evaluator { */ EnumEvaluator( ValueEvaluator valueEvaluator, List valueDescriptors) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); if (valueDescriptors.isEmpty()) { this.values = Collections.emptySet(); } else { @@ -88,9 +90,9 @@ public List evaluate(Value val, boolean failFast) if (!values.contains(enumValue.getNumber())) { return Collections.singletonList( ConstraintViolation.newBuilder() - .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) .addAllRulePathElements(DEFINED_ONLY_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(getFieldPathElement()) + .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) .setConstraintId("enum.defined_only") .setMessage("value must be one of the defined enum values") .setFieldValue(new ConstraintViolation.FieldValue(val)) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index 29ce2f8f..c9b7bcf2 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -27,7 +27,7 @@ import javax.annotation.Nullable; /** Performs validation on a single message field, defined by its descriptor. */ -class FieldEvaluator extends EvaluatorBase implements Evaluator { +class FieldEvaluator implements Evaluator { private static final FieldDescriptor REQUIRED_DESCRIPTOR = FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.REQUIRED_FIELD_NUMBER); @@ -36,6 +36,8 @@ class FieldEvaluator extends EvaluatorBase implements Evaluator { .addElements(FieldPathUtils.fieldPathElement(REQUIRED_DESCRIPTOR)) .build(); + private final ConstraintViolationHelper constraintViolationHelper; + /** The {@link ValueEvaluator} to apply to the field's value */ public final ValueEvaluator valueEvaluator; @@ -63,7 +65,7 @@ class FieldEvaluator extends EvaluatorBase implements Evaluator { boolean ignoreEmpty, boolean ignoreDefault, @Nullable Object zero) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.valueEvaluator = valueEvaluator; this.descriptor = descriptor; this.required = required; @@ -94,7 +96,7 @@ public List evaluate(Value val, boolean failFast) return Collections.singletonList( ConstraintViolation.newBuilder() .addFirstFieldPathElement(FieldPathUtils.fieldPathElement(descriptor)) - .addAllRulePathElements(getRulePrefixElements()) + .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) .addAllRulePathElements(REQUIRED_RULE_PATH.getElementsList()) .setConstraintId("required") .setMessage("value is required") diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index 0f7accec..5533c081 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -26,7 +26,7 @@ import java.util.Objects; /** Performs validation on the elements of a repeated field. */ -class ListEvaluator extends EvaluatorBase implements Evaluator { +class ListEvaluator implements Evaluator { /** Rule path to repeated rules */ private static final FieldPath REPEATED_ITEMS_RULE_PATH = FieldPath.newBuilder() @@ -40,12 +40,14 @@ class ListEvaluator extends EvaluatorBase implements Evaluator { .findFieldByNumber(RepeatedRules.ITEMS_FIELD_NUMBER))) .build(); + private final ConstraintViolationHelper constraintViolationHelper; + /** Constraints are checked on every item of the list. */ final ValueEvaluator itemConstraints; /** Constructs a {@link ListEvaluator}. */ ListEvaluator(ValueEvaluator valueEvaluator) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.itemConstraints = new ValueEvaluator(null, REPEATED_ITEMS_RULE_PATH); } @@ -66,8 +68,11 @@ public List evaluate(Value val, boolean failFast) continue; } FieldPathElement fieldPathElement = - Objects.requireNonNull(getFieldPathElement()).toBuilder().setIndex(i).build(); - FieldPathUtils.updatePaths(violations, fieldPathElement, getRulePrefixElements()); + Objects.requireNonNull(constraintViolationHelper.getFieldPathElement()).toBuilder() + .setIndex(i) + .build(); + FieldPathUtils.updatePaths( + violations, fieldPathElement, constraintViolationHelper.getRulePrefixElements()); if (failFast && !violations.isEmpty()) { return violations; } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index d795d0bf..7edc2b93 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -30,7 +30,7 @@ import java.util.stream.Collectors; /** Performs validation on a map field's key-value pairs. */ -class MapEvaluator extends EvaluatorBase implements Evaluator { +class MapEvaluator implements Evaluator { /** Rule path to map key rules */ private static final FieldPath MAP_KEYS_RULE_PATH = FieldPath.newBuilder() @@ -55,6 +55,8 @@ class MapEvaluator extends EvaluatorBase implements Evaluator { MapRules.getDescriptor().findFieldByNumber(MapRules.VALUES_FIELD_NUMBER))) .build(); + private final ConstraintViolationHelper constraintViolationHelper; + /** Constraint for checking the map keys */ private final ValueEvaluator keyEvaluator; @@ -76,7 +78,7 @@ class MapEvaluator extends EvaluatorBase implements Evaluator { * @param valueEvaluator The value evaluator this constraint exists under. */ MapEvaluator(ValueEvaluator valueEvaluator, Descriptors.FieldDescriptor fieldDescriptor) { - super(valueEvaluator); + this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); this.keyEvaluator = new ValueEvaluator(null, MAP_KEYS_RULE_PATH); this.valueEvaluator = new ValueEvaluator(null, MAP_VALUES_RULE_PATH); this.fieldDescriptor = fieldDescriptor; @@ -147,7 +149,7 @@ private List evalPairs(Value key, Value value, bool violations.addAll(valueViolations); FieldPathElement.Builder fieldPathElementBuilder = - Objects.requireNonNull(getFieldPathElement()).toBuilder(); + Objects.requireNonNull(constraintViolationHelper.getFieldPathElement()).toBuilder(); fieldPathElementBuilder.setKeyType(keyFieldDescriptor.getType().toProto()); fieldPathElementBuilder.setValueType(valueFieldDescriptor.getType().toProto()); switch (keyFieldDescriptor.getType().toProto()) { @@ -175,6 +177,7 @@ private List evalPairs(Value key, Value value, bool throw new ExecutionException("Unexpected map key type"); } FieldPathElement fieldPathElement = fieldPathElementBuilder.build(); - return FieldPathUtils.updatePaths(violations, fieldPathElement, getRulePrefixElements()); + return FieldPathUtils.updatePaths( + violations, fieldPathElement, constraintViolationHelper.getRulePrefixElements()); } } From 485ebeafcdbda32c42c5e2c8aba7f2e9c679f613 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Thu, 12 Dec 2024 12:02:00 -0500 Subject: [PATCH 11/11] constraintViolationHelper -> helper --- .../internal/evaluator/AnyEvaluator.java | 12 ++++++------ .../internal/evaluator/CelPrograms.java | 8 +++----- .../internal/evaluator/EmbeddedMessageEvaluator.java | 6 +++--- .../internal/evaluator/EnumEvaluator.java | 8 ++++---- .../internal/evaluator/FieldEvaluator.java | 6 +++--- .../internal/evaluator/ListEvaluator.java | 11 ++++------- .../internal/evaluator/MapEvaluator.java | 9 ++++----- 7 files changed, 27 insertions(+), 33 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java index 55e8fc91..0a881c69 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -35,7 +35,7 @@ * runtime. */ class AnyEvaluator implements Evaluator { - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; private final Descriptors.FieldDescriptor typeURLDescriptor; private final Set in; private final List inValue; @@ -69,7 +69,7 @@ class AnyEvaluator implements Evaluator { Descriptors.FieldDescriptor typeURLDescriptor, List in, List notIn) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.typeURLDescriptor = typeURLDescriptor; this.in = stringsToSet(in); this.inValue = in; @@ -89,9 +89,9 @@ public List evaluate(Value val, boolean failFast) if (!in.isEmpty() && !in.contains(typeURL)) { ConstraintViolation.Builder violation = ConstraintViolation.newBuilder() - .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) + .addAllRulePathElements(helper.getRulePrefixElements()) .addAllRulePathElements(IN_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) + .addFirstFieldPathElement(helper.getFieldPathElement()) .setConstraintId("any.in") .setMessage("type URL must be in the allow list") .setFieldValue(new ConstraintViolation.FieldValue(val)) @@ -104,9 +104,9 @@ public List evaluate(Value val, boolean failFast) if (!notIn.isEmpty() && notIn.contains(typeURL)) { ConstraintViolation.Builder violation = ConstraintViolation.newBuilder() - .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) + .addAllRulePathElements(helper.getRulePrefixElements()) .addAllRulePathElements(NOT_IN_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) + .addFirstFieldPathElement(helper.getFieldPathElement()) .setConstraintId("any.not_in") .setMessage("type URL must not be in the block list") .setFieldValue(new ConstraintViolation.FieldValue(val)) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java index ccc2c540..4927b8af 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/CelPrograms.java @@ -25,7 +25,7 @@ /** Evaluator that executes a {@link CompiledProgram}. */ class CelPrograms implements Evaluator { - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; /** A list of {@link CompiledProgram} that will be executed against the input message. */ private final List programs; @@ -36,7 +36,7 @@ class CelPrograms implements Evaluator { * @param compiledPrograms The programs to execute. */ CelPrograms(@Nullable ValueEvaluator valueEvaluator, List compiledPrograms) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.programs = compiledPrograms; } @@ -60,8 +60,6 @@ public List evaluate(Value val, boolean failFast) } } return FieldPathUtils.updatePaths( - violations, - constraintViolationHelper.getFieldPathElement(), - constraintViolationHelper.getRulePrefixElements()); + violations, helper.getFieldPathElement(), helper.getRulePrefixElements()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java index 7eb3b8a9..0911ed20 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java @@ -21,11 +21,11 @@ import java.util.List; class EmbeddedMessageEvaluator implements Evaluator { - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; private final MessageEvaluator messageEvaluator; EmbeddedMessageEvaluator(ValueEvaluator valueEvaluator, MessageEvaluator messageEvaluator) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.messageEvaluator = messageEvaluator; } @@ -39,7 +39,7 @@ public List evaluate(Value val, boolean failFast) throws ExecutionException { return FieldPathUtils.updatePaths( messageEvaluator.evaluate(val, failFast), - constraintViolationHelper.getFieldPathElement(), + helper.getFieldPathElement(), Collections.emptyList()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java index 1d690da4..3ab433f4 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -32,7 +32,7 @@ * check is handled outside CEL as enums are completely type erased to integers. */ class EnumEvaluator implements Evaluator { - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; /** Captures all the defined values for this enum */ private final Set values; @@ -56,7 +56,7 @@ class EnumEvaluator implements Evaluator { */ EnumEvaluator( ValueEvaluator valueEvaluator, List valueDescriptors) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); if (valueDescriptors.isEmpty()) { this.values = Collections.emptySet(); } else { @@ -90,9 +90,9 @@ public List evaluate(Value val, boolean failFast) if (!values.contains(enumValue.getNumber())) { return Collections.singletonList( ConstraintViolation.newBuilder() - .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) + .addAllRulePathElements(helper.getRulePrefixElements()) .addAllRulePathElements(DEFINED_ONLY_RULE_PATH.getElementsList()) - .addFirstFieldPathElement(constraintViolationHelper.getFieldPathElement()) + .addFirstFieldPathElement(helper.getFieldPathElement()) .setConstraintId("enum.defined_only") .setMessage("value must be one of the defined enum values") .setFieldValue(new ConstraintViolation.FieldValue(val)) diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java index c9b7bcf2..07f3d69c 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -36,7 +36,7 @@ class FieldEvaluator implements Evaluator { .addElements(FieldPathUtils.fieldPathElement(REQUIRED_DESCRIPTOR)) .build(); - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; /** The {@link ValueEvaluator} to apply to the field's value */ public final ValueEvaluator valueEvaluator; @@ -65,7 +65,7 @@ class FieldEvaluator implements Evaluator { boolean ignoreEmpty, boolean ignoreDefault, @Nullable Object zero) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.valueEvaluator = valueEvaluator; this.descriptor = descriptor; this.required = required; @@ -96,7 +96,7 @@ public List evaluate(Value val, boolean failFast) return Collections.singletonList( ConstraintViolation.newBuilder() .addFirstFieldPathElement(FieldPathUtils.fieldPathElement(descriptor)) - .addAllRulePathElements(constraintViolationHelper.getRulePrefixElements()) + .addAllRulePathElements(helper.getRulePrefixElements()) .addAllRulePathElements(REQUIRED_RULE_PATH.getElementsList()) .setConstraintId("required") .setMessage("value is required") diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java index 5533c081..ab240f8e 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ListEvaluator.java @@ -40,14 +40,14 @@ class ListEvaluator implements Evaluator { .findFieldByNumber(RepeatedRules.ITEMS_FIELD_NUMBER))) .build(); - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; /** Constraints are checked on every item of the list. */ final ValueEvaluator itemConstraints; /** Constructs a {@link ListEvaluator}. */ ListEvaluator(ValueEvaluator valueEvaluator) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.itemConstraints = new ValueEvaluator(null, REPEATED_ITEMS_RULE_PATH); } @@ -68,11 +68,8 @@ public List evaluate(Value val, boolean failFast) continue; } FieldPathElement fieldPathElement = - Objects.requireNonNull(constraintViolationHelper.getFieldPathElement()).toBuilder() - .setIndex(i) - .build(); - FieldPathUtils.updatePaths( - violations, fieldPathElement, constraintViolationHelper.getRulePrefixElements()); + Objects.requireNonNull(helper.getFieldPathElement()).toBuilder().setIndex(i).build(); + FieldPathUtils.updatePaths(violations, fieldPathElement, helper.getRulePrefixElements()); if (failFast && !violations.isEmpty()) { return violations; } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java index 7edc2b93..501b316a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -55,7 +55,7 @@ class MapEvaluator implements Evaluator { MapRules.getDescriptor().findFieldByNumber(MapRules.VALUES_FIELD_NUMBER))) .build(); - private final ConstraintViolationHelper constraintViolationHelper; + private final ConstraintViolationHelper helper; /** Constraint for checking the map keys */ private final ValueEvaluator keyEvaluator; @@ -78,7 +78,7 @@ class MapEvaluator implements Evaluator { * @param valueEvaluator The value evaluator this constraint exists under. */ MapEvaluator(ValueEvaluator valueEvaluator, Descriptors.FieldDescriptor fieldDescriptor) { - this.constraintViolationHelper = new ConstraintViolationHelper(valueEvaluator); + this.helper = new ConstraintViolationHelper(valueEvaluator); this.keyEvaluator = new ValueEvaluator(null, MAP_KEYS_RULE_PATH); this.valueEvaluator = new ValueEvaluator(null, MAP_VALUES_RULE_PATH); this.fieldDescriptor = fieldDescriptor; @@ -149,7 +149,7 @@ private List evalPairs(Value key, Value value, bool violations.addAll(valueViolations); FieldPathElement.Builder fieldPathElementBuilder = - Objects.requireNonNull(constraintViolationHelper.getFieldPathElement()).toBuilder(); + Objects.requireNonNull(helper.getFieldPathElement()).toBuilder(); fieldPathElementBuilder.setKeyType(keyFieldDescriptor.getType().toProto()); fieldPathElementBuilder.setValueType(valueFieldDescriptor.getType().toProto()); switch (keyFieldDescriptor.getType().toProto()) { @@ -177,7 +177,6 @@ private List evalPairs(Value key, Value value, bool throw new ExecutionException("Unexpected map key type"); } FieldPathElement fieldPathElement = fieldPathElementBuilder.build(); - return FieldPathUtils.updatePaths( - violations, fieldPathElement, constraintViolationHelper.getRulePrefixElements()); + return FieldPathUtils.updatePaths(violations, fieldPathElement, helper.getRulePrefixElements()); } }