Skip to content

Commit

Permalink
Add field and rule value to violations (#64)
Browse files Browse the repository at this point in the history
Adds the ability to access the captured rule and field value from a
Violation.

C++ protobuf doesn't really have a native "value" representation unlike
other languages, so I've created a sort-of stand-in. The `ProtoField`
class acts as a reference to a field on a message, and provides an
interface to get a singular value as a `variant`. This should be good
enough for most users, but users can also grab the information needed
and use protobuf reflection directly instead if they have more
complicated needs.

**This is a breaking change.** The API changes in the following ways:
- `Validator::Validate()` now returns the wrapper type
`ValidationResult` instead of `Violations`. This can be converted into
the protobuf `Violations` form using the `proto()` method.
- `ValidationResult.violations()` returns a `ConstraintViolation`. The
underlying `Violation` can be accessed using the `proto()` method.
  • Loading branch information
jchadwick-buf authored Dec 12, 2024
1 parent 261b5b1 commit 15d5717
Show file tree
Hide file tree
Showing 11 changed files with 476 additions and 153 deletions.
2 changes: 1 addition & 1 deletion buf/validate/conformance/runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ harness::TestResult TestRunner::runTestCase(const google::protobuf::Message& mes
break;
}
} else if (violations_or.value().violations_size() > 0) {
*result.mutable_validation_error() = std::move(violations_or).value();
*result.mutable_validation_error() = violations_or->proto();
} else {
result.set_success(true);
}
Expand Down
12 changes: 12 additions & 0 deletions buf/validate/internal/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,23 @@ cc_library(
],
)

cc_library(
name = "proto_field",
hdrs = ["proto_field.h"],
deps = [
"@com_github_bufbuild_protovalidate//proto/protovalidate/buf/validate:validate_proto_cc",
"@com_google_absl//absl/status",
"@com_google_cel_cpp//eval/public:cel_value",
"@com_google_protobuf//:protobuf",
],
)

cc_library(
name = "constraint_rules",
hdrs = ["constraint_rules.h"],
deps = [
"@com_github_bufbuild_protovalidate//proto/protovalidate/buf/validate:validate_proto_cc",
":proto_field",
"@com_google_absl//absl/status",
"@com_google_cel_cpp//eval/public:cel_value",
"@com_google_protobuf//:protobuf",
Expand Down
14 changes: 10 additions & 4 deletions buf/validate/internal/cel_constraint_rules.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,32 @@ absl::Status ProcessConstraint(
ConstraintContext& ctx,
const google::api::expr::runtime::BaseActivation& activation,
const CompiledConstraint& expr) {
auto result_or = expr.expr->Evaluate(activation, ctx.arena);
auto result_or= expr.expr->Evaluate(activation, ctx.arena);
if (!result_or.ok()) {
return result_or.status();
}
cel::runtime::CelValue result = std::move(result_or).value();
if (result.IsBool()) {
if (!result.BoolOrDie()) {
// Add violation with the constraint message.
Violation& violation = *ctx.violations.add_violations();
Violation violation;
violation.set_message(expr.constraint.message());
violation.set_constraint_id(expr.constraint.id());
if (expr.rulePath.has_value()) {
*violation.mutable_rule() = *expr.rulePath;
}
ctx.violations.emplace_back(violation, absl::nullopt, absl::nullopt);
}
} else if (result.IsString()) {
if (!result.StringOrDie().value().empty()) {
// Add violation with custom message.
Violation& violation = *ctx.violations.add_violations();
Violation violation;
violation.set_message(std::string(result.StringOrDie().value()));
violation.set_constraint_id(expr.constraint.id());
if (expr.rulePath.has_value()) {
*violation.mutable_rule() = *expr.rulePath;
}
ctx.violations.emplace_back(violation, absl::nullopt, absl::nullopt);
}
} else if (result.IsError()) {
const cel::runtime::CelError& error = *result.ErrorOrDie();
Expand Down Expand Up @@ -126,11 +128,15 @@ absl::Status CelConstraintRules::ValidateCel(
absl::Status status = absl::OkStatus();

for (const auto& expr : exprs_) {
if (rules_.IsMessage() && expr.rule) {
if (rules_.IsMessage() && expr.rule != nullptr) {
activation.InsertValue(
"rule", ProtoFieldToCelValue(rules_.MessageOrDie(), expr.rule, ctx.arena));
}
int pos = ctx.violations.size();
status = ProcessConstraint(ctx, activation, expr);
if (rules_.IsMessage() && expr.rule != nullptr && ctx.violations.size() > pos) {
ctx.setRuleValue(ProtoField{rules_.MessageOrDie(), expr.rule}, pos);
}
if (ctx.shouldReturn(status)) {
break;
}
Expand Down
103 changes: 95 additions & 8 deletions buf/validate/internal/constraint_rules.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,39 +15,97 @@
#pragma once

#include "absl/status/status.h"
#include "absl/strings/escaping.h"
#include "buf/validate/validate.pb.h"
#include "buf/validate/internal/proto_field.h"
#include "eval/public/cel_value.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/message.h"

namespace buf::validate::internal {
inline std::string fieldPathString(const FieldPath &path);

/// ConstraintViolation is a wrapper for the protobuf Violation that provides additional in-memory
/// information, specifically, references to the in-memory values for the field and rule.
class ConstraintViolation {
friend struct ConstraintContext;

public:
ConstraintViolation(
Violation proto,
const absl::optional<ProtoField>& fieldValue,
const absl::optional<ProtoField>& ruleValue)
: proto_{std::move(proto)}, fieldValue_{fieldValue}, ruleValue_{ruleValue} {}

[[nodiscard]] const Violation& proto() const { return proto_; }
[[nodiscard]] absl::optional<ProtoField> field_value() const { return fieldValue_; }
[[nodiscard]] absl::optional<ProtoField> rule_value() const { return ruleValue_; }

private:
Violation proto_;
absl::optional<ProtoField> fieldValue_;
absl::optional<ProtoField> ruleValue_;
};

struct ConstraintContext {
ConstraintContext() : failFast(false), arena(nullptr) {}
ConstraintContext(const ConstraintContext&) = delete;
void operator=(const ConstraintContext&) = delete;

bool failFast;
google::protobuf::Arena* arena;
Violations violations;
std::vector<ConstraintViolation> violations;

[[nodiscard]] bool shouldReturn(const absl::Status status) {
return !status.ok() || (failFast && violations.violations_size() > 0);
return !status.ok() || (failFast && !violations.empty());
}

void appendFieldPathElement(const FieldPathElement &element, int start) {
for (int i = start; i < violations.violations_size(); i++) {
auto* violation = violations.mutable_violations(i);
*violation->mutable_field()->mutable_elements()->Add() = element;
for (int i = start; i < violations.size(); i++) {
*violations[i].proto_.mutable_field()->mutable_elements()->Add() = element;
}
}

void appendRulePathElement(std::initializer_list<FieldPathElement> suffix, int start) {
for (int i = start; i < violations.violations_size(); i++) {
auto* violation = violations.mutable_violations(i);
auto* elements = violation->mutable_rule()->mutable_elements();
for (int i = start; i < violations.size(); i++) {
auto* elements = violations[i].proto_.mutable_rule()->mutable_elements();
std::copy(suffix.begin(), suffix.end(), RepeatedPtrFieldBackInserter(elements));
}
}

void setFieldValue(ProtoField field, int start) {
for (int i = start; i < violations.size(); i++) {
violations[i].fieldValue_ = field;
}
}

void setRuleValue(ProtoField rule, int start) {
for (int i = start; i < violations.size(); i++) {
violations[i].ruleValue_ = rule;
}
}

void setForKey(int start) {
for (int i = start; i < violations.size(); i++) {
violations[i].proto_.set_for_key(true);
}
}

void finalize() {
for (ConstraintViolation& violation : violations) {
if (violation.proto().has_field()) {
std::reverse(
violation.proto_.mutable_field()->mutable_elements()->begin(),
violation.proto_.mutable_field()->mutable_elements()->end());
*violation.proto_.mutable_field_path() = internal::fieldPathString(violation.proto().field());
}
if (violation.proto().has_rule()) {
std::reverse(
violation.proto_.mutable_rule()->mutable_elements()->begin(),
violation.proto_.mutable_rule()->mutable_elements()->end());
}
}
}
};

class ConstraintRules {
Expand All @@ -62,4 +120,33 @@ class ConstraintRules {
ConstraintContext& ctx, const google::protobuf::Message& message) const = 0;
};

inline std::string fieldPathString(const FieldPath &path) {
std::string result;
for (const FieldPathElement& element : path.elements()) {
if (!result.empty()) {
result += '.';
}
switch (element.subscript_case()) {
case FieldPathElement::kIndex:
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.index()), "]");
break;
case FieldPathElement::kBoolKey:
absl::StrAppend(&result, element.field_name(), element.bool_key() ? "[true]" : "[false]");
break;
case FieldPathElement::kIntKey:
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.int_key()), "]");
break;
case FieldPathElement::kUintKey:
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.uint_key()), "]");
break;
case FieldPathElement::kStringKey:
absl::StrAppend(&result, element.field_name(), "[\"", absl::CEscape(element.string_key()), "\"]");
break;
case FieldPathElement::SUBSCRIPT_NOT_SET:
absl::StrAppend(&result, element.field_name());
}
}
return result;
}

} // namespace buf::validate::internal
Loading

0 comments on commit 15d5717

Please sign in to comment.