From df11bf2f218b82dc310959b5aff4cde1f165c7a8 Mon Sep 17 00:00:00 2001 From: jchadwick-buf <116005195+jchadwick-buf@users.noreply.github.com> Date: Thu, 12 Dec 2024 15:03:20 -0500 Subject: [PATCH] Add field and rule value to violations (#215) Adds the ability to access the captured rule and field value from a Violation. There's a bit of refactoring toil, so I tried to split the commits cleanly to make this easier to review. - The first commit adds the `Violation` wrapper class and plumbs it through all uses of `Violation`. - The second commit makes `Value` non-internal so we can use it to expose protobuf values in a cleaner fashion. - The third commit actually implements filling the `fieldValue` and `ruleValue`fields. **This is a breaking change.** The API changes in the following ways: - `ValidationResult` now provides a new wrapper `Violation` type instead of the `buf.validate.Violation` message. This new wrapper has a `getProto()` method to return a `buf.validate.Violation` message, and `ValidationResult` now has a `toProto()` method to return a `buf.validate.Violations` message. --- .../buf/protovalidate/conformance/Main.java | 7 +- .../buf/protovalidate/ValidationResult.java | 24 +- .../build/buf/protovalidate/Validator.java | 16 +- .../build/buf/protovalidate/Violation.java | 65 +++++ .../internal/constraints/ConstraintCache.java | 11 +- .../internal/errors/ConstraintViolation.java | 268 ++++++++++++++++++ .../internal/errors/FieldPathUtils.java | 110 ++----- .../internal/evaluator/AnyEvaluator.java | 75 +++-- .../CelPrograms.java | 30 +- .../evaluator/ConstraintViolationHelper.java | 61 ++++ .../evaluator/EmbeddedMessageEvaluator.java | 45 +++ .../internal/evaluator/EnumEvaluator.java | 38 ++- .../internal/evaluator/Evaluator.java | 9 +- .../internal/evaluator/EvaluatorBuilder.java | 120 ++++---- .../internal/evaluator/EvaluatorWrapper.java | 26 -- .../internal/evaluator/FieldEvaluator.java | 53 ++-- .../internal/evaluator/ListEvaluator.java | 76 ++--- .../internal/evaluator/MapEvaluator.java | 135 +++------ .../internal/evaluator/MessageEvaluator.java | 22 +- .../internal/evaluator/MessageValue.java | 7 + .../internal/evaluator/ObjectValue.java | 7 +- .../internal/evaluator/OneofEvaluator.java | 31 +- .../evaluator/UnknownDescriptorEvaluator.java | 15 +- .../internal/evaluator/Value.java | 10 + .../internal/evaluator/ValueEvaluator.java | 45 ++- .../internal/expression/CompiledProgram.java | 44 ++- .../ValidatorDifferentJavaPackagesTest.java | 2 +- .../ValidatorDynamicMessageTest.java | 15 +- 28 files changed, 862 insertions(+), 505 deletions(-) create mode 100644 src/main/java/build/buf/protovalidate/Violation.java 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 (57%) create mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java create mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java delete mode 100644 src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorWrapper.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..c8bacee8 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,27 @@ public String toString() { builder.append("Validation error:"); for (Violation violation : violations) { builder.append("\n - "); - if (!violation.getFieldPath().isEmpty()) { - builder.append(violation.getFieldPath()); + if (!violation.toProto().getFieldPath().isEmpty()) { + builder.append(violation.toProto().getFieldPath()); builder.append(": "); } - builder.append(String.format("%s [%s]", violation.getMessage(), violation.getConstraintId())); + builder.append( + String.format( + "%s [%s]", violation.toProto().getMessage(), violation.toProto().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.toProto()); + } + return Violations.newBuilder().addAllViolations(protoViolations).build(); + } } diff --git a/src/main/java/build/buf/protovalidate/Validator.java b/src/main/java/build/buf/protovalidate/Validator.java index 3cdb051b..a812649b 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<>(result.size()); + 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 new file mode 100644 index 00000000..b510ec00 --- /dev/null +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -0,0 +1,65 @@ +// 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 com.google.protobuf.Descriptors; +import javax.annotation.Nullable; + +/** + * {@link Violation} provides all of the collected information about an individual constraint + * violation. + */ +public interface Violation { + /** {@link FieldValue} represents a Protobuf field value inside a Protobuf message. */ + interface FieldValue { + /** + * Gets the value of the field, which may be null, a primitive, a Map or a List. + * + * @return The value of the protobuf field. + */ + @Nullable + Object getValue(); + + /** + * Gets the field descriptor of the field this value is from. + * + * @return A FieldDescriptor pertaining to this field. + */ + Descriptors.FieldDescriptor getDescriptor(); + } + + /** + * Gets the protobuf form of this violation. + * + * @return The protobuf form of this violation. + */ + build.buf.validate.Violation toProto(); + + /** + * Gets the value of the field this violation pertains to, or null if there is none. + * + * @return Value of the field associated with the violation, or null if there is none. + */ + @Nullable + FieldValue getFieldValue(); + + /** + * Gets the value of the rule this violation pertains to, or null if there is none. + * + * @return Value of the rule associated with the violation, or null if there is none. + */ + @Nullable + FieldValue getRuleValue(); +} 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..2a4a8435 100644 --- a/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java +++ b/src/main/java/build/buf/protovalidate/internal/constraints/ConstraintCache.java @@ -17,6 +17,8 @@ import build.buf.protovalidate.Config; 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; @@ -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/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 79837df1..4b1103ae 100644 --- a/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java +++ b/src/main/java/build/buf/protovalidate/internal/errors/FieldPathUtils.java @@ -16,91 +16,14 @@ 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; +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.getField().getElementsCount() > 0 - && violation.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()) - .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() - .setRule( - FieldPath.newBuilder() - .addAllElements(elements) - .addAllElements(violation.getRule().getElementsList()) - .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.getField().getElementsCount() > 0) { - result.add( - violation.toBuilder().setFieldPath(fieldPathString(violation.getField())).build()); - } else { - result.add(violation); - } - } - return result; - } - /** * Converts the provided field path to a string. * @@ -155,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() + "]"; @@ -166,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 16658a34..0a881c69 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/AnyEvaluator.java @@ -14,13 +14,12 @@ 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 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; @@ -36,69 +35,85 @@ * runtime. */ class AnyEvaluator implements Evaluator { + private final ConstraintViolationHelper helper; 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) { + AnyEvaluator( + ValueEvaluator valueEvaluator, + Descriptors.FieldDescriptor typeURLDescriptor, + List in, + List notIn) { + this.helper = new ConstraintViolationHelper(valueEvaluator); this.typeURLDescriptor = typeURLDescriptor; this.in = stringsToSet(in); + this.inValue = in; this.notIn = stringsToSet(notIn); + this.notInValue = notIn; } @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() - .setRule(IN_RULE_PATH) + ConstraintViolation.Builder violation = + ConstraintViolation.newBuilder() + .addAllRulePathElements(helper.getRulePrefixElements()) + .addAllRulePathElements(IN_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(helper.getFieldPathElement()) .setConstraintId("any.in") .setMessage("type URL must be in the allow list") - .build(); + .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() - .setRule(NOT_IN_RULE_PATH) + ConstraintViolation.Builder violation = + ConstraintViolation.newBuilder() + .addAllRulePathElements(helper.getRulePrefixElements()) + .addAllRulePathElements(NOT_IN_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(helper.getFieldPathElement()) .setConstraintId("any.not_in") .setMessage("type URL must not be in the block list") - .build(); + .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 57% 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 4b88149b..4927b8af 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,21 @@ // 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.exceptions.ExecutionException; -import build.buf.protovalidate.internal.evaluator.Evaluator; -import build.buf.protovalidate.internal.evaluator.Value; -import build.buf.validate.Violation; +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 implements Evaluator { + private final ConstraintViolationHelper helper; + /** A list of {@link CompiledProgram} that will be executed against the input message. */ private final List programs; @@ -32,7 +35,8 @@ public class CelPrograms implements Evaluator { * * @param compiledPrograms The programs to execute. */ - public CelPrograms(List compiledPrograms) { + CelPrograms(@Nullable ValueEvaluator valueEvaluator, List compiledPrograms) { + this.helper = new ConstraintViolationHelper(valueEvaluator); this.programs = compiledPrograms; } @@ -42,18 +46,20 @@ 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(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, helper.getFieldPathElement(), helper.getRulePrefixElements()); } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.java new file mode 100644 index 00000000..7abcee01 --- /dev/null +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintViolationHelper.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 ConstraintViolationHelper { + private static final List EMPTY_PREFIX = new ArrayList<>(); + + private final @Nullable FieldPath rulePrefix; + + private final @Nullable FieldPathElement fieldPathElement; + + ConstraintViolationHelper(@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; + } + } + + ConstraintViolationHelper(@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/EmbeddedMessageEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java new file mode 100644 index 00000000..0911ed20 --- /dev/null +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EmbeddedMessageEvaluator.java @@ -0,0 +1,45 @@ +// 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.Collections; +import java.util.List; + +class EmbeddedMessageEvaluator implements Evaluator { + private final ConstraintViolationHelper helper; + private final MessageEvaluator messageEvaluator; + + EmbeddedMessageEvaluator(ValueEvaluator valueEvaluator, MessageEvaluator messageEvaluator) { + this.helper = new ConstraintViolationHelper(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), + 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 1ea95797..3ab433f4 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EnumEvaluator.java @@ -16,11 +16,11 @@ import build.buf.protovalidate.ValidationResult; 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; import build.buf.validate.FieldPath; -import build.buf.validate.Violation; import com.google.protobuf.Descriptors; import java.util.Collections; import java.util.List; @@ -32,18 +32,21 @@ * check is handled outside CEL as enums are completely type erased to integers. */ class EnumEvaluator implements Evaluator { + private final ConstraintViolationHelper helper; + /** 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(); /** @@ -51,7 +54,9 @@ class EnumEvaluator implements Evaluator { * * @param valueDescriptors the list of {@link Descriptors.EnumValueDescriptor} for the enum. */ - EnumEvaluator(List valueDescriptors) { + EnumEvaluator( + ValueEvaluator valueEvaluator, List valueDescriptors) { + this.helper = new ConstraintViolationHelper(valueEvaluator); if (valueDescriptors.isEmpty()) { this.values = Collections.emptySet(); } else { @@ -76,20 +81,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() - .setRule(DEFINED_ONLY_RULE_PATH) - .setConstraintId("enum.defined_only") - .setMessage("value must be one of the defined enum values") - .build())); + return Collections.singletonList( + ConstraintViolation.newBuilder() + .addAllRulePathElements(helper.getRulePrefixElements()) + .addAllRulePathElements(DEFINED_ONLY_RULE_PATH.getElementsList()) + .addFirstFieldPathElement(helper.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/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index f2e4a2e4..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( @@ -529,17 +517,13 @@ 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; } } - - 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 0120197a..07f3d69c 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/FieldEvaluator.java @@ -14,12 +14,11 @@ 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 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; @@ -29,14 +28,16 @@ /** 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(); + private final ConstraintViolationHelper helper; + /** The {@link ValueEvaluator} to apply to the field's value */ public final ValueEvaluator valueEvaluator; @@ -64,6 +65,7 @@ class FieldEvaluator implements Evaluator { boolean ignoreEmpty, boolean ignoreDefault, @Nullable Object zero) { + this.helper = new ConstraintViolationHelper(valueEvaluator); this.valueEvaluator = valueEvaluator; this.descriptor = descriptor; this.required = required; @@ -78,10 +80,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,32 +93,22 @@ public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionEx hasField = message.hasField(descriptor); } if (required && !hasField) { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder() - .setField( - FieldPath.newBuilder() - .addElements(FieldPathUtils.fieldPathElement(descriptor)) - .build()) - .setRule(requiredFieldRulePath) - .setConstraintId("required") - .setMessage("value is required") - .build())); + return Collections.singletonList( + ConstraintViolation.newBuilder() + .addFirstFieldPathElement(FieldPathUtils.fieldPathElement(descriptor)) + .addAllRulePathElements(helper.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 65a5d384..ab240f8e 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.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 build.buf.validate.Violation; -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 = + /** Rule path to repeated rules */ + private static final FieldPath REPEATED_ITEMS_RULE_PATH = FieldPath.newBuilder() .addElements( FieldPathUtils.fieldPathElement( @@ -39,52 +38,17 @@ class ListEvaluator implements Evaluator { FieldPathUtils.fieldPathElement( RepeatedRules.getDescriptor() .findFieldByNumber(RepeatedRules.ITEMS_FIELD_NUMBER))) - .build() - .getElementsList(); + .build(); - 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); - } - } + private final ConstraintViolationHelper helper; /** 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) { + this.helper = new ConstraintViolationHelper(valueEvaluator); + this.itemConstraints = new ValueEvaluator(null, REPEATED_ITEMS_RULE_PATH); } @Override @@ -93,24 +57,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(helper.getFieldPathElement()).toBuilder().setIndex(i).build(); + FieldPathUtils.updatePaths(violations, fieldPathElement, helper.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 79f0ffe8..501b316a 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/MapEvaluator.java @@ -14,25 +14,25 @@ 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 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; 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 { /** 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,9 @@ class MapEvaluator implements Evaluator { .addElements( FieldPathUtils.fieldPathElement( MapRules.getDescriptor().findFieldByNumber(MapRules.VALUES_FIELD_NUMBER))) - .build() - .getElementsList(); + .build(); - 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); - } - } + private final ConstraintViolationHelper helper; /** Constraint for checking the map keys */ private final ValueEvaluator keyEvaluator; @@ -141,11 +75,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) { + 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; this.keyFieldDescriptor = fieldDescriptor.getMessageType().findFieldByNumber(1); this.valueFieldDescriptor = fieldDescriptor.getMessageType().findFieldByNumber(2); @@ -175,45 +110,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().setForKey(true).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(helper.getFieldPathElement()).toBuilder(); + fieldPathElementBuilder.setKeyType(keyFieldDescriptor.getType().toProto()); + fieldPathElementBuilder.setValueType(valueFieldDescriptor.getType().toProto()); switch (keyFieldDescriptor.getType().toProto()) { case TYPE_INT64: case TYPE_INT32: @@ -221,23 +159,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, helper.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 84a873be..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.exceptions.ExecutionException; -import build.buf.validate.Violation; +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/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 7390375a..53dddaf0 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ObjectValue.java @@ -45,11 +45,16 @@ 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; } + @Override + public Descriptors.FieldDescriptor fieldDescriptor() { + return fieldDescriptor; + } + @Nullable @Override public Message messageValue() { 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..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.exceptions.ExecutionException; -import build.buf.validate.FieldPath; +import build.buf.protovalidate.internal.errors.ConstraintViolation; 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; +import java.util.List; /** {@link OneofEvaluator} performs validation on a oneof union. */ public class OneofEvaluator implements Evaluator { @@ -48,23 +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() - .setField( - FieldPath.newBuilder() - .addElements( - FieldPathElement.newBuilder().setFieldName(descriptor.getName()))) - .setConstraintId("required") - .setMessage("exactly one field is required in oneof") - .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 6dbbefa3..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.exceptions.ExecutionException; -import build.buf.validate.Violation; +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,11 +39,10 @@ public boolean tautology() { } @Override - public ValidationResult evaluate(Value val, boolean failFast) throws ExecutionException { - return new ValidationResult( - Collections.singletonList( - Violation.newBuilder() - .setMessage("No evaluator available for " + desc.getFullName()) - .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/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/evaluator/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ValueEvaluator.java index 9dec92cb..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.exceptions.ExecutionException; -import build.buf.validate.Violation; +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 d3576534..afb594e6 100644 --- a/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/internal/expression/CompiledProgram.java @@ -15,8 +15,9 @@ package build.buf.protovalidate.internal.expression; 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 build.buf.validate.Violation; import javax.annotation.Nullable; import org.projectnessie.cel.Program; import org.projectnessie.cel.common.types.Err; @@ -36,29 +37,37 @@ 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 ConstraintViolation.Builder eval(Value fieldValue, Variable bindings) + throws ExecutionException { Program.EvalResult evalResult = program.eval(bindings); Val val = evalResult.getVal(); if (val instanceof Err) { @@ -69,22 +78,35 @@ public Violation eval(Variable bindings) throws ExecutionException { if ("".equals(value)) { return null; } - Violation.Builder violation = - Violation.newBuilder().setConstraintId(this.source.id).setMessage(value.toString()); + 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()); } - return violation.build(); + if (ruleValue != null && ruleValue.fieldDescriptor() != null) { + builder.setRuleValue(new ConstraintViolation.FieldValue(ruleValue)); + } + return builder; } else if (value instanceof Boolean) { if (val.booleanValue()) { return null; } - Violation.Builder violation = - Violation.newBuilder().setConstraintId(this.source.id).setMessage(this.source.message); + ConstraintViolation.Builder builder = + ConstraintViolation.newBuilder() + .setConstraintId(this.source.id) + .setMessage(this.source.message); if (rulePath != null) { - violation.setRule(rulePath); + builder.addAllRulePathElements(rulePath.getElementsList()); + } + if (ruleValue != null && ruleValue.fieldDescriptor() != null) { + builder.setRuleValue(new ConstraintViolation.FieldValue(ruleValue)); } - return violation.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/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..9cd6d8a1 100644 --- a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -80,8 +80,11 @@ 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()) - .containsExactly(expectedViolation); + ValidationResult result = new Validator().validate(messageBuilder.build()); + assertThat(result.toProto().getViolationsList()).containsExactly(expectedViolation); + assertThat(result.getViolations().get(0).getFieldValue().getValue()).isEqualTo("0123456789"); + assertThat(result.getViolations().get(0).getRuleValue().getValue()) + .isEqualTo("^[a-z0-9]{1,9}$"); } @Test @@ -97,7 +100,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 +116,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 +158,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 +208,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); }