Skip to content

Commit 15d5717

Browse files
author
jchadwick-buf
authored
Add field and rule value to violations (#64)
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.
1 parent 261b5b1 commit 15d5717

File tree

11 files changed

+476
-153
lines changed

11 files changed

+476
-153
lines changed

buf/validate/conformance/runner.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ harness::TestResult TestRunner::runTestCase(const google::protobuf::Message& mes
6969
break;
7070
}
7171
} else if (violations_or.value().violations_size() > 0) {
72-
*result.mutable_validation_error() = std::move(violations_or).value();
72+
*result.mutable_validation_error() = violations_or->proto();
7373
} else {
7474
result.set_success(true);
7575
}

buf/validate/internal/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,23 @@ cc_library(
3131
],
3232
)
3333

34+
cc_library(
35+
name = "proto_field",
36+
hdrs = ["proto_field.h"],
37+
deps = [
38+
"@com_github_bufbuild_protovalidate//proto/protovalidate/buf/validate:validate_proto_cc",
39+
"@com_google_absl//absl/status",
40+
"@com_google_cel_cpp//eval/public:cel_value",
41+
"@com_google_protobuf//:protobuf",
42+
],
43+
)
44+
3445
cc_library(
3546
name = "constraint_rules",
3647
hdrs = ["constraint_rules.h"],
3748
deps = [
3849
"@com_github_bufbuild_protovalidate//proto/protovalidate/buf/validate:validate_proto_cc",
50+
":proto_field",
3951
"@com_google_absl//absl/status",
4052
"@com_google_cel_cpp//eval/public:cel_value",
4153
"@com_google_protobuf//:protobuf",

buf/validate/internal/cel_constraint_rules.cc

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,32 @@ absl::Status ProcessConstraint(
3030
ConstraintContext& ctx,
3131
const google::api::expr::runtime::BaseActivation& activation,
3232
const CompiledConstraint& expr) {
33-
auto result_or = expr.expr->Evaluate(activation, ctx.arena);
33+
auto result_or= expr.expr->Evaluate(activation, ctx.arena);
3434
if (!result_or.ok()) {
3535
return result_or.status();
3636
}
3737
cel::runtime::CelValue result = std::move(result_or).value();
3838
if (result.IsBool()) {
3939
if (!result.BoolOrDie()) {
4040
// Add violation with the constraint message.
41-
Violation& violation = *ctx.violations.add_violations();
41+
Violation violation;
4242
violation.set_message(expr.constraint.message());
4343
violation.set_constraint_id(expr.constraint.id());
4444
if (expr.rulePath.has_value()) {
4545
*violation.mutable_rule() = *expr.rulePath;
4646
}
47+
ctx.violations.emplace_back(violation, absl::nullopt, absl::nullopt);
4748
}
4849
} else if (result.IsString()) {
4950
if (!result.StringOrDie().value().empty()) {
5051
// Add violation with custom message.
51-
Violation& violation = *ctx.violations.add_violations();
52+
Violation violation;
5253
violation.set_message(std::string(result.StringOrDie().value()));
5354
violation.set_constraint_id(expr.constraint.id());
5455
if (expr.rulePath.has_value()) {
5556
*violation.mutable_rule() = *expr.rulePath;
5657
}
58+
ctx.violations.emplace_back(violation, absl::nullopt, absl::nullopt);
5759
}
5860
} else if (result.IsError()) {
5961
const cel::runtime::CelError& error = *result.ErrorOrDie();
@@ -126,11 +128,15 @@ absl::Status CelConstraintRules::ValidateCel(
126128
absl::Status status = absl::OkStatus();
127129

128130
for (const auto& expr : exprs_) {
129-
if (rules_.IsMessage() && expr.rule) {
131+
if (rules_.IsMessage() && expr.rule != nullptr) {
130132
activation.InsertValue(
131133
"rule", ProtoFieldToCelValue(rules_.MessageOrDie(), expr.rule, ctx.arena));
132134
}
135+
int pos = ctx.violations.size();
133136
status = ProcessConstraint(ctx, activation, expr);
137+
if (rules_.IsMessage() && expr.rule != nullptr && ctx.violations.size() > pos) {
138+
ctx.setRuleValue(ProtoField{rules_.MessageOrDie(), expr.rule}, pos);
139+
}
134140
if (ctx.shouldReturn(status)) {
135141
break;
136142
}

buf/validate/internal/constraint_rules.h

Lines changed: 95 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,39 +15,97 @@
1515
#pragma once
1616

1717
#include "absl/status/status.h"
18+
#include "absl/strings/escaping.h"
1819
#include "buf/validate/validate.pb.h"
20+
#include "buf/validate/internal/proto_field.h"
1921
#include "eval/public/cel_value.h"
2022
#include "google/protobuf/arena.h"
2123
#include "google/protobuf/message.h"
2224

2325
namespace buf::validate::internal {
26+
inline std::string fieldPathString(const FieldPath &path);
27+
28+
/// ConstraintViolation is a wrapper for the protobuf Violation that provides additional in-memory
29+
/// information, specifically, references to the in-memory values for the field and rule.
30+
class ConstraintViolation {
31+
friend struct ConstraintContext;
32+
33+
public:
34+
ConstraintViolation(
35+
Violation proto,
36+
const absl::optional<ProtoField>& fieldValue,
37+
const absl::optional<ProtoField>& ruleValue)
38+
: proto_{std::move(proto)}, fieldValue_{fieldValue}, ruleValue_{ruleValue} {}
39+
40+
[[nodiscard]] const Violation& proto() const { return proto_; }
41+
[[nodiscard]] absl::optional<ProtoField> field_value() const { return fieldValue_; }
42+
[[nodiscard]] absl::optional<ProtoField> rule_value() const { return ruleValue_; }
43+
44+
private:
45+
Violation proto_;
46+
absl::optional<ProtoField> fieldValue_;
47+
absl::optional<ProtoField> ruleValue_;
48+
};
49+
2450
struct ConstraintContext {
2551
ConstraintContext() : failFast(false), arena(nullptr) {}
2652
ConstraintContext(const ConstraintContext&) = delete;
2753
void operator=(const ConstraintContext&) = delete;
2854

2955
bool failFast;
3056
google::protobuf::Arena* arena;
31-
Violations violations;
57+
std::vector<ConstraintViolation> violations;
3258

3359
[[nodiscard]] bool shouldReturn(const absl::Status status) {
34-
return !status.ok() || (failFast && violations.violations_size() > 0);
60+
return !status.ok() || (failFast && !violations.empty());
3561
}
3662

3763
void appendFieldPathElement(const FieldPathElement &element, int start) {
38-
for (int i = start; i < violations.violations_size(); i++) {
39-
auto* violation = violations.mutable_violations(i);
40-
*violation->mutable_field()->mutable_elements()->Add() = element;
64+
for (int i = start; i < violations.size(); i++) {
65+
*violations[i].proto_.mutable_field()->mutable_elements()->Add() = element;
4166
}
4267
}
4368

4469
void appendRulePathElement(std::initializer_list<FieldPathElement> suffix, int start) {
45-
for (int i = start; i < violations.violations_size(); i++) {
46-
auto* violation = violations.mutable_violations(i);
47-
auto* elements = violation->mutable_rule()->mutable_elements();
70+
for (int i = start; i < violations.size(); i++) {
71+
auto* elements = violations[i].proto_.mutable_rule()->mutable_elements();
4872
std::copy(suffix.begin(), suffix.end(), RepeatedPtrFieldBackInserter(elements));
4973
}
5074
}
75+
76+
void setFieldValue(ProtoField field, int start) {
77+
for (int i = start; i < violations.size(); i++) {
78+
violations[i].fieldValue_ = field;
79+
}
80+
}
81+
82+
void setRuleValue(ProtoField rule, int start) {
83+
for (int i = start; i < violations.size(); i++) {
84+
violations[i].ruleValue_ = rule;
85+
}
86+
}
87+
88+
void setForKey(int start) {
89+
for (int i = start; i < violations.size(); i++) {
90+
violations[i].proto_.set_for_key(true);
91+
}
92+
}
93+
94+
void finalize() {
95+
for (ConstraintViolation& violation : violations) {
96+
if (violation.proto().has_field()) {
97+
std::reverse(
98+
violation.proto_.mutable_field()->mutable_elements()->begin(),
99+
violation.proto_.mutable_field()->mutable_elements()->end());
100+
*violation.proto_.mutable_field_path() = internal::fieldPathString(violation.proto().field());
101+
}
102+
if (violation.proto().has_rule()) {
103+
std::reverse(
104+
violation.proto_.mutable_rule()->mutable_elements()->begin(),
105+
violation.proto_.mutable_rule()->mutable_elements()->end());
106+
}
107+
}
108+
}
51109
};
52110

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

123+
inline std::string fieldPathString(const FieldPath &path) {
124+
std::string result;
125+
for (const FieldPathElement& element : path.elements()) {
126+
if (!result.empty()) {
127+
result += '.';
128+
}
129+
switch (element.subscript_case()) {
130+
case FieldPathElement::kIndex:
131+
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.index()), "]");
132+
break;
133+
case FieldPathElement::kBoolKey:
134+
absl::StrAppend(&result, element.field_name(), element.bool_key() ? "[true]" : "[false]");
135+
break;
136+
case FieldPathElement::kIntKey:
137+
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.int_key()), "]");
138+
break;
139+
case FieldPathElement::kUintKey:
140+
absl::StrAppend(&result, element.field_name(), "[", std::to_string(element.uint_key()), "]");
141+
break;
142+
case FieldPathElement::kStringKey:
143+
absl::StrAppend(&result, element.field_name(), "[\"", absl::CEscape(element.string_key()), "\"]");
144+
break;
145+
case FieldPathElement::SUBSCRIPT_NOT_SET:
146+
absl::StrAppend(&result, element.field_name());
147+
}
148+
}
149+
return result;
150+
}
151+
65152
} // namespace buf::validate::internal

0 commit comments

Comments
 (0)