From 3aa41f16f95dbd0b9f4a3f7a2e04c0ff9ead0d53 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 22 Apr 2025 01:45:03 -0400 Subject: [PATCH 1/4] Implement getField CEL function I'm proposing this as an eventual replacement (before 1.0) of our hack around the fact that the in identifier is reserved in CEL. We need this for protovalidate-cc especially so we can start removing our cel-cpp patches. Protovalidate PR: https://github.com/bufbuild/protovalidate/pull/352 --- buf/validate/internal/BUILD.bazel | 1 + buf/validate/internal/extra_func.cc | 33 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/buf/validate/internal/BUILD.bazel b/buf/validate/internal/BUILD.bazel index 937d4f0..2d6e6d3 100644 --- a/buf/validate/internal/BUILD.bazel +++ b/buf/validate/internal/BUILD.bazel @@ -139,6 +139,7 @@ cc_library( "@com_google_absl//absl/status", "@com_google_cel_cpp//eval/public:cel_function_adapter", "@com_google_cel_cpp//eval/public:cel_function_registry", + "@com_google_cel_cpp//eval/public/containers:field_access", "@com_google_cel_cpp//eval/public/containers:container_backed_map_impl", "@com_googlesource_code_re2//:re2", ], diff --git a/buf/validate/internal/extra_func.cc b/buf/validate/internal/extra_func.cc index be5e500..c5a3e40 100644 --- a/buf/validate/internal/extra_func.cc +++ b/buf/validate/internal/extra_func.cc @@ -26,6 +26,7 @@ #include "eval/public/cel_function_adapter.h" #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_map_impl.h" +#include "eval/public/containers/field_access.h" #include "google/protobuf/arena.h" #include "re2/re2.h" @@ -52,6 +53,32 @@ bool isPathValid(const std::string_view path) { namespace cel = google::api::expr::runtime; +cel::CelValue getField( + google::protobuf::Arena* arena, cel::CelValue msgval, cel::CelValue nameval) { + if (!msgval.IsMessage()) { + auto* error = google::protobuf::Arena::Create( + arena, absl::StatusCode::kInvalidArgument, "expected a message value for first argument"); + return cel::CelValue::CreateError(error); + } + if (!nameval.IsString()) { + auto* error = google::protobuf::Arena::Create( + arena, absl::StatusCode::kInvalidArgument, "expected a string value for second argument"); + return cel::CelValue::CreateError(error); + } + const auto* message = msgval.MessageOrDie(); + auto name = nameval.StringOrDie(); + const auto* field = message->GetDescriptor()->FindFieldByName(name.value()); + if (field == nullptr) { + auto* error = google::protobuf::Arena::Create( + arena, absl::StatusCode::kInvalidArgument, "no such field"); + return cel::CelValue::CreateError(error); + } + if (cel::CelValue result; cel::CreateValueFromSingleField(message, field, arena, &result).ok()) { + return result; + } + return cel::CelValue::CreateNull(); +} + cel::CelValue isNan(google::protobuf::Arena* arena, cel::CelValue rhs) { if (!rhs.IsDouble()) { auto* error = google::protobuf::Arena::Create( @@ -352,6 +379,12 @@ absl::Status RegisterExtraFuncs( if (!status.ok()) { return status; } + auto getFieldStatus = + cel::FunctionAdapter::CreateAndRegister( + "getField", false, &getField, ®istry); + if (!getFieldStatus.ok()) { + return getFieldStatus; + } auto isNanStatus = cel::FunctionAdapter::CreateAndRegister( "isNan", true, &isNan, ®istry); if (!isNanStatus.ok()) { From 9586a4370b3ede033079fdb6a5428cab0b06adc0 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 23 Apr 2025 10:03:25 -0400 Subject: [PATCH 2/4] Update protovalidate to v0.11.0 (Yes this commit is a little large, but it's just due to the renames. Nothing substantial here.) --- MODULE.bazel | 2 +- MODULE.bazel.lock | 6 +-- buf/validate/BUILD.bazel | 2 +- buf/validate/internal/BUILD.bazel | 1 + buf/validate/internal/cel_constraint_rules.cc | 8 ++-- buf/validate/internal/cel_constraint_rules.h | 4 +- buf/validate/internal/cel_rules.h | 44 ++++++++--------- buf/validate/internal/constraints.cc | 36 +++++++------- buf/validate/internal/constraints.h | 12 ++--- buf/validate/internal/constraints_test.cc | 6 +-- buf/validate/internal/extra_func.cc | 11 ++++- buf/validate/internal/field_rules.cc | 48 +++++++++---------- buf/validate/internal/field_rules.h | 6 +-- buf/validate/internal/message_rules.cc | 2 +- buf/validate/internal/message_rules.h | 2 +- buf/validate/validator_test.cc | 32 ++++++------- deps/shared_deps.json | 4 +- 17 files changed, 117 insertions(+), 109 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 302555a..a46dfcb 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -44,4 +44,4 @@ archive_override( ] ) -bazel_dep(name = "protovalidate", version = "0.10.6", repo_name = "com_github_bufbuild_protovalidate") +bazel_dep(name = "protovalidate", version = "0.11.0", repo_name = "com_github_bufbuild_protovalidate") diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 6a39ec3..40cddaf 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -182,8 +182,8 @@ "https://bcr.bazel.build/modules/protoc-gen-validate/1.0.4.bcr.2/MODULE.bazel": "c4bd2c850211ff5b7dadf9d2d0496c1c922fdedc303c775b01dfd3b3efc907ed", "https://bcr.bazel.build/modules/protoc-gen-validate/1.0.4.bcr.2/source.json": "4cc97f70b521890798058600a927ce4b0def8ee84ff2a5aa632aabcb4234aa0b", "https://bcr.bazel.build/modules/protoc-gen-validate/1.0.4/MODULE.bazel": "b8913c154b16177990f6126d2d2477d187f9ddc568e95ee3e2d50fc65d2c494a", - "https://bcr.bazel.build/modules/protovalidate/0.10.6/MODULE.bazel": "82e0269928df6abaec450f0905b51129414c5115e2796fd3597df383a67d4aba", - "https://bcr.bazel.build/modules/protovalidate/0.10.6/source.json": "d2a06a3aa65b1d3f7ed4ed3db66ede5a63e89c9a1fb566ca2ffccdb05e753f32", + "https://bcr.bazel.build/modules/protovalidate/0.11.0/MODULE.bazel": "5b432a855e24764781c4a7088383851ca664758a9b5528fd25c7a5d4f5058c5a", + "https://bcr.bazel.build/modules/protovalidate/0.11.0/source.json": "45fe9dd57454e0369e8d413d8510eb6ee4707700f3d676b2c9882c824c2e091b", "https://bcr.bazel.build/modules/pybind11_bazel/2.11.1/MODULE.bazel": "88af1c246226d87e65be78ed49ecd1e6f5e98648558c14ce99176da041dc378e", "https://bcr.bazel.build/modules/pybind11_bazel/2.12.0/MODULE.bazel": "e6f4c20442eaa7c90d7190d8dc539d0ab422f95c65a57cc59562170c58ae3d34", "https://bcr.bazel.build/modules/pybind11_bazel/2.12.0/source.json": "6900fdc8a9e95866b8c0d4ad4aba4d4236317b5c1cd04c502df3f0d33afed680", @@ -507,7 +507,7 @@ "@@rules_buf+//buf:extensions.bzl%buf": { "general": { "bzlTransitiveDigest": "Cn52bY/1OxGKNv4TI5yExj2vL9hBgfRj4HOOcjWQTdQ=", - "usagesDigest": "YowoSc6ASRgqktVnQyCRjPbo29WMSLexfsrjmozKuOg=", + "usagesDigest": "7hiRiyq+jSKCw3zFt49WZKIGGEMdZp9Ybs6ZElSy66Y=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/buf/validate/BUILD.bazel b/buf/validate/BUILD.bazel index 6f0aa2b..704c65d 100644 --- a/buf/validate/BUILD.bazel +++ b/buf/validate/BUILD.bazel @@ -30,7 +30,7 @@ cc_test( deps = [ ":validator", "@com_github_bufbuild_protovalidate//proto/protovalidate-testing/buf/validate/conformance/cases:buf_validate_conformance_cases_proto_cc", - "@com_github_bufbuild_protovalidate//proto/protovalidate-testing/buf/validate/conformance/cases/custom_constraints:buf_validate_conformance_cases_custom_constraints_cc_proto", + "@com_github_bufbuild_protovalidate//proto/protovalidate-testing/buf/validate/conformance/cases/custom_rules:buf_validate_conformance_cases_custom_rules_cc_proto", "@com_google_cel_cpp//eval/public:activation", "@com_google_cel_cpp//eval/public:builtin_func_registrar", "@com_google_cel_cpp//eval/public:cel_expr_builder_factory", diff --git a/buf/validate/internal/BUILD.bazel b/buf/validate/internal/BUILD.bazel index fc65499..bbc1b77 100644 --- a/buf/validate/internal/BUILD.bazel +++ b/buf/validate/internal/BUILD.bazel @@ -140,6 +140,7 @@ cc_library( "@com_google_cel_cpp//eval/public:cel_function_adapter", "@com_google_cel_cpp//eval/public:cel_function_registry", "@com_google_cel_cpp//eval/public/containers:field_access", + "@com_google_cel_cpp//eval/public/containers:field_backed_list_impl", "@com_google_cel_cpp//eval/public/containers:container_backed_map_impl", "@com_googlesource_code_re2//:re2", ], diff --git a/buf/validate/internal/cel_constraint_rules.cc b/buf/validate/internal/cel_constraint_rules.cc index f30e472..b9ae605 100644 --- a/buf/validate/internal/cel_constraint_rules.cc +++ b/buf/validate/internal/cel_constraint_rules.cc @@ -40,7 +40,7 @@ absl::Status ProcessConstraint( // Add violation with the constraint message. Violation violation; violation.set_message(expr.constraint.message()); - violation.set_constraint_id(expr.constraint.id()); + violation.set_rule_id(expr.constraint.id()); if (expr.rulePath.has_value()) { *violation.mutable_rule() = *expr.rulePath; } @@ -51,7 +51,7 @@ absl::Status ProcessConstraint( // Add violation with custom message. Violation violation; violation.set_message(std::string(result.StringOrDie().value())); - violation.set_constraint_id(expr.constraint.id()); + violation.set_rule_id(expr.constraint.id()); if (expr.rulePath.has_value()) { *violation.mutable_rule() = *expr.rulePath; } @@ -89,7 +89,7 @@ cel::runtime::CelValue ProtoFieldToCelValue( absl::Status CelConstraintRules::Add( google::api::expr::runtime::CelExpressionBuilder& builder, - Constraint constraint, + Rule constraint, absl::optional rulePath, const google::protobuf::FieldDescriptor* rule) { auto pexpr_or = cel::parser::Parse(constraint.expression()); @@ -113,7 +113,7 @@ absl::Status CelConstraintRules::Add( std::string_view expression, absl::optional rulePath, const google::protobuf::FieldDescriptor* rule) { - Constraint constraint; + Rule constraint; *constraint.mutable_id() = id; *constraint.mutable_message() = message; *constraint.mutable_expression() = expression; diff --git a/buf/validate/internal/cel_constraint_rules.h b/buf/validate/internal/cel_constraint_rules.h index 4fe44b9..78d794b 100644 --- a/buf/validate/internal/cel_constraint_rules.h +++ b/buf/validate/internal/cel_constraint_rules.h @@ -26,7 +26,7 @@ namespace buf::validate::internal { // A compiled constraint expression. struct CompiledConstraint { - buf::validate::Constraint constraint; + buf::validate::Rule constraint; std::unique_ptr expr; const absl::optional rulePath; const google::protobuf::FieldDescriptor* rule; @@ -41,7 +41,7 @@ class CelConstraintRules : public ConstraintRules { absl::Status Add( google::api::expr::runtime::CelExpressionBuilder& builder, - Constraint constraint, + Rule constraint, absl::optional rulePath, const google::protobuf::FieldDescriptor* rule); absl::Status Add( diff --git a/buf/validate/internal/cel_rules.h b/buf/validate/internal/cel_rules.h index 6ab944e..b3ca160 100644 --- a/buf/validate/internal/cel_rules.h +++ b/buf/validate/internal/cel_rules.h @@ -33,27 +33,27 @@ constexpr int ruleFieldNumber() = delete; return fieldNumber; \ } -MAP_RULES_TO_FIELD_NUMBER(FloatRules, FieldConstraints::kFloatFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(DoubleRules, FieldConstraints::kDoubleFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(Int32Rules, FieldConstraints::kInt32FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(Int64Rules, FieldConstraints::kInt64FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(UInt32Rules, FieldConstraints::kUint32FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(UInt64Rules, FieldConstraints::kUint64FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(SInt32Rules, FieldConstraints::kSint32FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(SInt64Rules, FieldConstraints::kSint64FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(Fixed32Rules, FieldConstraints::kFixed32FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(Fixed64Rules, FieldConstraints::kFixed64FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(SFixed32Rules, FieldConstraints::kSfixed32FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(SFixed64Rules, FieldConstraints::kSfixed64FieldNumber) -MAP_RULES_TO_FIELD_NUMBER(BoolRules, FieldConstraints::kBoolFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(StringRules, FieldConstraints::kStringFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(BytesRules, FieldConstraints::kBytesFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(EnumRules, FieldConstraints::kEnumFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(RepeatedRules, FieldConstraints::kRepeatedFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(MapRules, FieldConstraints::kMapFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(AnyRules, FieldConstraints::kAnyFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(DurationRules, FieldConstraints::kDurationFieldNumber) -MAP_RULES_TO_FIELD_NUMBER(TimestampRules, FieldConstraints::kTimestampFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(FloatRules, FieldRules::kFloatFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(DoubleRules, FieldRules::kDoubleFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(Int32Rules, FieldRules::kInt32FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(Int64Rules, FieldRules::kInt64FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(UInt32Rules, FieldRules::kUint32FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(UInt64Rules, FieldRules::kUint64FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(SInt32Rules, FieldRules::kSint32FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(SInt64Rules, FieldRules::kSint64FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(Fixed32Rules, FieldRules::kFixed32FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(Fixed64Rules, FieldRules::kFixed64FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(SFixed32Rules, FieldRules::kSfixed32FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(SFixed64Rules, FieldRules::kSfixed64FieldNumber) +MAP_RULES_TO_FIELD_NUMBER(BoolRules, FieldRules::kBoolFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(StringRules, FieldRules::kStringFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(BytesRules, FieldRules::kBytesFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(EnumRules, FieldRules::kEnumFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(RepeatedRules, FieldRules::kRepeatedFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(MapRules, FieldRules::kMapFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(AnyRules, FieldRules::kAnyFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(DurationRules, FieldRules::kDurationFieldNumber) +MAP_RULES_TO_FIELD_NUMBER(TimestampRules, FieldRules::kTimestampFieldNumber) #undef MAP_RULES_TO_FIELD_NUMBER @@ -97,7 +97,7 @@ absl::Status BuildCelRules( FieldPath rulePath; *rulePath.mutable_elements()->Add() = fieldPathElement(field); *rulePath.mutable_elements()->Add() = - staticFieldPathElement()>(); + staticFieldPathElement()>(); if (!field->options().HasExtension(buf::validate::predefined)) { continue; } diff --git a/buf/validate/internal/constraints.cc b/buf/validate/internal/constraints.cc index 8db2f19..4ceef98 100644 --- a/buf/validate/internal/constraints.cc +++ b/buf/validate/internal/constraints.cc @@ -127,8 +127,8 @@ absl::Status MessageConstraintRules::Validate( absl::Status FieldConstraintRules::Validate( ConstraintContext& ctx, const google::protobuf::Message& message) const { static const google::protobuf::FieldDescriptor* requiredField = - FieldConstraints::descriptor()->FindFieldByNumber( - FieldConstraints::kRequiredFieldNumber); + FieldRules::descriptor()->FindFieldByNumber( + FieldRules::kRequiredFieldNumber); google::api::expr::runtime::Activation activation; cel::runtime::CelValue result; std::string subPath; @@ -141,11 +141,11 @@ absl::Status FieldConstraintRules::Validate( return absl::OkStatus(); } else if (required_) { Violation violation; - *violation.mutable_constraint_id() = "required"; + *violation.mutable_rule_id() = "required"; *violation.mutable_message() = "value is required"; *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field_); *violation.mutable_rule()->mutable_elements()->Add() = - staticFieldPathElement(); + staticFieldPathElement(); ctx.violations.emplace_back( std::move(violation), ProtoField{&message, field_}, @@ -161,11 +161,11 @@ absl::Status FieldConstraintRules::Validate( return absl::OkStatus(); } else if (required_) { Violation violation; - *violation.mutable_constraint_id() = "required"; + *violation.mutable_rule_id() = "required"; *violation.mutable_message() = "value is required"; *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field_); *violation.mutable_rule()->mutable_elements()->Add() = - staticFieldPathElement(); + staticFieldPathElement(); ctx.violations.emplace_back( std::move(violation), ProtoField{&message, field_}, @@ -176,11 +176,11 @@ absl::Status FieldConstraintRules::Validate( if (!message.GetReflection()->HasField(message, field_)) { if (required_) { Violation violation; - *violation.mutable_constraint_id() = "required"; + *violation.mutable_rule_id() = "required"; *violation.mutable_message() = "value is required"; *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field_); *violation.mutable_rule()->mutable_elements()->Add() = - staticFieldPathElement(); + staticFieldPathElement(); ctx.violations.emplace_back( std::move(violation), ProtoField{&message, field_}, @@ -235,11 +235,11 @@ absl::Status EnumConstraintRules::Validate( auto value = message.GetReflection()->GetEnumValue(message, field_); if (field_->enum_type()->FindValueByNumber(value) == nullptr) { Violation violation; - *violation.mutable_constraint_id() = "enum.defined_only"; + *violation.mutable_rule_id() = "enum.defined_only"; *violation.mutable_message() = "value must be one of the defined enum values"; *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field_); *violation.mutable_rule()->mutable_elements()->Add() = fieldPathElement(EnumRules::descriptor()->FindFieldByNumber(EnumRules::kDefinedOnlyFieldNumber)); - *violation.mutable_rule()->mutable_elements()->Add() = fieldPathElement(FieldConstraints::descriptor()->FindFieldByNumber(FieldConstraints::kEnumFieldNumber)); + *violation.mutable_rule()->mutable_elements()->Add() = fieldPathElement(FieldRules::descriptor()->FindFieldByNumber(FieldRules::kEnumFieldNumber)); ctx.violations.emplace_back( std::move(violation), ProtoField{&message, field_}, @@ -280,7 +280,7 @@ absl::Status RepeatedConstraintRules::Validate( ctx.appendFieldPathElement(element, pos); ctx.appendRulePathElement({ fieldPathElement(RepeatedRules::descriptor()->FindFieldByNumber(RepeatedRules::kItemsFieldNumber)), - fieldPathElement(FieldConstraints::descriptor()->FindFieldByNumber(FieldConstraints::kRepeatedFieldNumber)), + fieldPathElement(FieldRules::descriptor()->FindFieldByNumber(FieldRules::kRepeatedFieldNumber)), }, pos); ctx.setFieldValue(ProtoField{&message, field_, i}, pos); } @@ -320,7 +320,7 @@ absl::Status MapConstraintRules::Validate( if (ctx.violations.size() > pos) { ctx.appendRulePathElement({ fieldPathElement(MapRules::descriptor()->FindFieldByNumber(MapRules::kKeysFieldNumber)), - fieldPathElement(FieldConstraints::descriptor()->FindFieldByNumber(FieldConstraints::kMapFieldNumber)), + fieldPathElement(FieldRules::descriptor()->FindFieldByNumber(FieldRules::kMapFieldNumber)), }, pos); ctx.setFieldValue(ProtoField{&elemMsg, keyField}, pos); ctx.setForKey(pos); @@ -340,7 +340,7 @@ absl::Status MapConstraintRules::Validate( if (ctx.violations.size() > valuePos) { ctx.appendRulePathElement({ fieldPathElement(MapRules::descriptor()->FindFieldByNumber(MapRules::kValuesFieldNumber)), - fieldPathElement(FieldConstraints::descriptor()->FindFieldByNumber(FieldConstraints::kMapFieldNumber)), + fieldPathElement(FieldRules::descriptor()->FindFieldByNumber(FieldRules::kMapFieldNumber)), }, valuePos); ctx.setFieldValue(ProtoField{&elemMsg, valueField}, pos); } @@ -389,7 +389,7 @@ absl::Status FieldConstraintRules::ValidateAny( } if (!found) { Violation violation; - *violation.mutable_constraint_id() = "any.in"; + *violation.mutable_rule_id() = "any.in"; *violation.mutable_message() = "type URL must be in the allow list"; if (field.index() == -1) { *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field.descriptor()); @@ -397,7 +397,7 @@ absl::Status FieldConstraintRules::ValidateAny( *violation.mutable_rule()->mutable_elements()->Add() = staticFieldPathElement(); *violation.mutable_rule()->mutable_elements()->Add() = - staticFieldPathElement(); + staticFieldPathElement(); ctx.violations.emplace_back( std::move(violation), field, @@ -407,7 +407,7 @@ absl::Status FieldConstraintRules::ValidateAny( for (const auto& block : anyRules_->not_in()) { if (block == typeUri) { Violation violation; - *violation.mutable_constraint_id() = "any.not_in"; + *violation.mutable_rule_id() = "any.not_in"; *violation.mutable_message() = "type URL must not be in the block list"; if (field.index() == -1) { *violation.mutable_field()->mutable_elements()->Add() = fieldPathElement(field.descriptor()); @@ -415,7 +415,7 @@ absl::Status FieldConstraintRules::ValidateAny( *violation.mutable_rule()->mutable_elements()->Add() = staticFieldPathElement(); *violation.mutable_rule()->mutable_elements()->Add() = - staticFieldPathElement(); + staticFieldPathElement(); ctx.violations.emplace_back( std::move(violation), field, @@ -432,7 +432,7 @@ absl::Status OneofConstraintRules::Validate( if (required_) { if (!message.GetReflection()->HasOneof(message, oneof_)) { Violation violation; - *violation.mutable_constraint_id() = "required"; + *violation.mutable_rule_id() = "required"; *violation.mutable_message() = "exactly one field is required in oneof"; *violation.mutable_field()->mutable_elements()->Add() = oneofPathElement(*oneof_); ctx.violations.emplace_back(std::move(violation), absl::nullopt, absl::nullopt); diff --git a/buf/validate/internal/constraints.h b/buf/validate/internal/constraints.h index b23d6fd..00596ab 100644 --- a/buf/validate/internal/constraints.h +++ b/buf/validate/internal/constraints.h @@ -38,7 +38,7 @@ class FieldConstraintRules : public CelConstraintRules { public: FieldConstraintRules( const google::protobuf::FieldDescriptor* desc, - const FieldConstraints& field, + const FieldRules& field, const AnyRules* anyRules = nullptr) : fieldConstraints_(field), field_(desc), @@ -66,7 +66,7 @@ class FieldConstraintRules : public CelConstraintRules { [[nodiscard]] bool getIgnoreDefault() const { return ignoreDefault_; } protected: - const FieldConstraints& fieldConstraints_; + const FieldRules& fieldConstraints_; const google::protobuf::FieldDescriptor* field_ = nullptr; bool mapEntryField_ = false; bool ignoreEmpty_ = false; @@ -79,7 +79,7 @@ class EnumConstraintRules : public FieldConstraintRules { using Base = FieldConstraintRules; public: - EnumConstraintRules(const google::protobuf::FieldDescriptor* desc, const FieldConstraints& field) + EnumConstraintRules(const google::protobuf::FieldDescriptor* desc, const FieldRules& field) : Base(desc, field), definedOnly_(field.enum_().defined_only()) {} absl::Status Validate( @@ -95,7 +95,7 @@ class RepeatedConstraintRules : public FieldConstraintRules { public: RepeatedConstraintRules( const google::protobuf::FieldDescriptor* desc, - const FieldConstraints& field, + const FieldRules& field, std::unique_ptr itemRules) : Base(desc, field), itemRules_(std::move(itemRules)) {} @@ -112,7 +112,7 @@ class MapConstraintRules : public FieldConstraintRules { public: MapConstraintRules( const google::protobuf::FieldDescriptor* desc, - const FieldConstraints& field, + const FieldRules& field, std::unique_ptr keyRules, std::unique_ptr valueRules) : Base(desc, field), keyRules_(std::move(keyRules)), valueRules_(std::move(valueRules)) {} @@ -130,7 +130,7 @@ class OneofConstraintRules : public ConstraintRules { using Base = ConstraintRules; public: - OneofConstraintRules(const google::protobuf::OneofDescriptor* desc, const OneofConstraints& oneof) + OneofConstraintRules(const google::protobuf::OneofDescriptor* desc, const OneofRules& oneof) : oneof_(desc), required_(oneof.required()) {} absl::Status Validate( diff --git a/buf/validate/internal/constraints_test.cc b/buf/validate/internal/constraints_test.cc index c62ac57..296291c 100644 --- a/buf/validate/internal/constraints_test.cc +++ b/buf/validate/internal/constraints_test.cc @@ -44,7 +44,7 @@ class ExpressionTest : public testing::Test { google::protobuf::Arena arena_; absl::Status AddConstraint(std::string expr, std::string message, std::string id) { - Constraint constraint; + Rule constraint; constraint.set_expression(std::move(expr)); constraint.set_message(std::move(message)); constraint.set_id(std::move(id)); @@ -77,7 +77,7 @@ TEST_F(ExpressionTest, BoolResult) { ASSERT_TRUE(status.ok()) << status; ASSERT_EQ(violations.size(), 1); EXPECT_EQ(violations[0].proto().message(), "always fails"); - EXPECT_EQ(violations[0].proto().constraint_id(), "always-fails"); + EXPECT_EQ(violations[0].proto().rule_id(), "always-fails"); } TEST_F(ExpressionTest, StringResult) { @@ -90,7 +90,7 @@ TEST_F(ExpressionTest, StringResult) { ASSERT_TRUE(status.ok()) << status; ASSERT_EQ(violations.size(), 1); EXPECT_EQ(violations[0].proto().message(), "error"); - EXPECT_EQ(violations[0].proto().constraint_id(), "always-fails"); + EXPECT_EQ(violations[0].proto().rule_id(), "always-fails"); } TEST_F(ExpressionTest, Error) { diff --git a/buf/validate/internal/extra_func.cc b/buf/validate/internal/extra_func.cc index 12fea03..c8048e9 100644 --- a/buf/validate/internal/extra_func.cc +++ b/buf/validate/internal/extra_func.cc @@ -26,6 +26,7 @@ #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_map_impl.h" #include "eval/public/containers/field_access.h" +#include "eval/public/containers/field_backed_list_impl.h" #include "google/protobuf/arena.h" #include "re2/re2.h" @@ -72,8 +73,14 @@ cel::CelValue getField( arena, absl::StatusCode::kInvalidArgument, "no such field"); return cel::CelValue::CreateError(error); } - if (cel::CelValue result; cel::CreateValueFromSingleField(message, field, arena, &result).ok()) { - return result; + if (field->is_repeated()) { + return cel::CelValue::CreateList( + google::protobuf::Arena::Create( + arena, message, field, arena)); + } else { + if (cel::CelValue result; cel::CreateValueFromSingleField(message, field, arena, &result).ok()) { + return result; + } } return cel::CelValue::CreateNull(); } diff --git a/buf/validate/internal/field_rules.cc b/buf/validate/internal/field_rules.cc index 38d1d4a..050aefe 100644 --- a/buf/validate/internal/field_rules.cc +++ b/buf/validate/internal/field_rules.cc @@ -24,13 +24,13 @@ absl::StatusOr> NewFieldRules( google::protobuf::Arena* arena, google::api::expr::runtime::CelExpressionBuilder& builder, const google::protobuf::FieldDescriptor* field, - const FieldConstraints& fieldLvl) { + const FieldRules& fieldLvl) { if (fieldLvl.ignore() == IGNORE_ALWAYS) { return nullptr; } absl::StatusOr> rules_or; switch (fieldLvl.type_case()) { - case FieldConstraints::kBool: + case FieldRules::kBool: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -42,7 +42,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_BOOL, "google.protobuf.BoolValue"); break; - case FieldConstraints::kFloat: + case FieldRules::kFloat: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -54,7 +54,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_FLOAT, "google.protobuf.FloatValue"); break; - case FieldConstraints::kDouble: + case FieldRules::kDouble: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -66,7 +66,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_DOUBLE, "google.protobuf.DoubleValue"); break; - case FieldConstraints::kInt32: + case FieldRules::kInt32: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -78,7 +78,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_INT32, "google.protobuf.Int32Value"); break; - case FieldConstraints::kInt64: + case FieldRules::kInt64: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -90,7 +90,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_INT64, "google.protobuf.Int64Value"); break; - case FieldConstraints::kUint32: + case FieldRules::kUint32: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -102,7 +102,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_UINT32, "google.protobuf.UInt32Value"); break; - case FieldConstraints::kUint64: + case FieldRules::kUint64: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -114,7 +114,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_UINT64, "google.protobuf.UInt64Value"); break; - case FieldConstraints::kSint32: + case FieldRules::kSint32: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -125,7 +125,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.sint32(), google::protobuf::FieldDescriptor::TYPE_SINT32); break; - case FieldConstraints::kSint64: + case FieldRules::kSint64: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -136,7 +136,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.sint64(), google::protobuf::FieldDescriptor::TYPE_SINT64); break; - case FieldConstraints::kFixed32: + case FieldRules::kFixed32: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -147,7 +147,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.fixed32(), google::protobuf::FieldDescriptor::TYPE_FIXED32); break; - case FieldConstraints::kFixed64: + case FieldRules::kFixed64: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -158,7 +158,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.fixed64(), google::protobuf::FieldDescriptor::TYPE_FIXED64); break; - case FieldConstraints::kSfixed32: + case FieldRules::kSfixed32: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -169,7 +169,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.sfixed32(), google::protobuf::FieldDescriptor::TYPE_SFIXED32); break; - case FieldConstraints::kSfixed64: + case FieldRules::kSfixed64: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -180,7 +180,7 @@ absl::StatusOr> NewFieldRules( fieldLvl.sfixed64(), google::protobuf::FieldDescriptor::TYPE_SFIXED64); break; - case FieldConstraints::kString: + case FieldRules::kString: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -192,7 +192,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_STRING, "google.protobuf.StringValue"); break; - case FieldConstraints::kBytes: + case FieldRules::kBytes: rules_or = NewScalarFieldRules( messageFactory, allowUnknownFields, @@ -204,7 +204,7 @@ absl::StatusOr> NewFieldRules( google::protobuf::FieldDescriptor::TYPE_BYTES, "google.protobuf.BytesValue"); break; - case FieldConstraints::kEnum: { + case FieldRules::kEnum: { rules_or = std::make_unique(field, fieldLvl); auto status = BuildScalarFieldRules( *rules_or.value(), @@ -221,7 +221,7 @@ absl::StatusOr> NewFieldRules( } break; } - case FieldConstraints::kDuration: + case FieldRules::kDuration: if (field->cpp_type() != google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE || field->message_type()->full_name() != google::protobuf::Duration::descriptor()->full_name()) { @@ -237,7 +237,7 @@ absl::StatusOr> NewFieldRules( } } break; - case FieldConstraints::kTimestamp: + case FieldRules::kTimestamp: if (field->cpp_type() != google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE || field->message_type()->full_name() != google::protobuf::Timestamp::descriptor()->full_name()) { @@ -253,7 +253,7 @@ absl::StatusOr> NewFieldRules( } } break; - case FieldConstraints::kRepeated: + case FieldRules::kRepeated: if (!field->is_repeated()) { return absl::InvalidArgumentError("repeated field validator on non-repeated field"); } else if (field->is_map()) { @@ -283,7 +283,7 @@ absl::StatusOr> NewFieldRules( } } break; - case FieldConstraints::kMap: + case FieldRules::kMap: if (!field->is_map()) { return absl::InvalidArgumentError("map field validator on non-map field"); } else { @@ -318,7 +318,7 @@ absl::StatusOr> NewFieldRules( } } break; - case FieldConstraints::kAny: + case FieldRules::kAny: if (field->cpp_type() != google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE || field->message_type()->full_name() != google::protobuf::Any::descriptor()->full_name()) { return absl::InvalidArgumentError("any field validator on non-any field"); @@ -333,7 +333,7 @@ absl::StatusOr> NewFieldRules( } } break; - case FieldConstraints::TYPE_NOT_SET: + case FieldRules::TYPE_NOT_SET: rules_or = std::make_unique(field, fieldLvl); break; default: @@ -344,7 +344,7 @@ absl::StatusOr> NewFieldRules( for (int i = 0; i < fieldLvl.cel_size(); i++) { const auto& constraint = fieldLvl.cel(i); FieldPathElement celElement = - staticFieldPathElement(); + staticFieldPathElement(); celElement.set_index(i); FieldPath rulePath; *rulePath.mutable_elements()->Add() = celElement; diff --git a/buf/validate/internal/field_rules.h b/buf/validate/internal/field_rules.h index 16c9ac2..87bc77d 100644 --- a/buf/validate/internal/field_rules.h +++ b/buf/validate/internal/field_rules.h @@ -30,7 +30,7 @@ absl::Status BuildScalarFieldRules( google::protobuf::Arena* arena, google::api::expr::runtime::CelExpressionBuilder& builder, const google::protobuf::FieldDescriptor* field, - const FieldConstraints& fieldLvl, + const FieldRules& fieldLvl, const R& rules, google::protobuf::FieldDescriptor::Type expectedType, std::string_view wrapperName = "") { @@ -59,7 +59,7 @@ absl::StatusOr> NewScalarFieldRules( google::protobuf::Arena* arena, google::api::expr::runtime::CelExpressionBuilder& builder, const google::protobuf::FieldDescriptor* field, - const FieldConstraints& fieldLvl, + const FieldRules& fieldLvl, const R& rules, google::protobuf::FieldDescriptor::Type expectedType, std::string_view wrapperName = "") { @@ -87,6 +87,6 @@ absl::StatusOr> NewFieldRules( google::protobuf::Arena* arena, google::api::expr::runtime::CelExpressionBuilder& builder, const google::protobuf::FieldDescriptor* field, - const FieldConstraints& fieldLvl); + const FieldRules& fieldLvl); } // namespace buf::validate::internal diff --git a/buf/validate/internal/message_rules.cc b/buf/validate/internal/message_rules.cc index de7b9f7..8e822f0 100644 --- a/buf/validate/internal/message_rules.cc +++ b/buf/validate/internal/message_rules.cc @@ -20,7 +20,7 @@ namespace buf::validate::internal { absl::StatusOr> BuildMessageRules( google::api::expr::runtime::CelExpressionBuilder& builder, - const MessageConstraints& constraints) { + const MessageRules& constraints) { auto result = std::make_unique(); for (const auto& constraint : constraints.cel()) { if (auto status = result->Add(builder, constraint, absl::nullopt, nullptr); !status.ok()) { diff --git a/buf/validate/internal/message_rules.h b/buf/validate/internal/message_rules.h index 631d91e..4e98f13 100644 --- a/buf/validate/internal/message_rules.h +++ b/buf/validate/internal/message_rules.h @@ -35,6 +35,6 @@ Constraints NewMessageConstraints( absl::StatusOr> BuildMessageRules( google::api::expr::runtime::CelExpressionBuilder& builder, - const MessageConstraints& constraints); + const MessageRules& constraints); } // namespace buf::validate::internal \ No newline at end of file diff --git a/buf/validate/validator_test.cc b/buf/validate/validator_test.cc index 1b02f4f..5aab48c 100644 --- a/buf/validate/validator_test.cc +++ b/buf/validate/validator_test.cc @@ -16,7 +16,7 @@ #include "buf/validate/conformance/cases/bool.pb.h" #include "buf/validate/conformance/cases/bytes.pb.h" -#include "buf/validate/conformance/cases/custom_constraints/custom_constraints.pb.h" +#include "buf/validate/conformance/cases/custom_rules/custom_rules.pb.h" #include "buf/validate/conformance/cases/repeated.pb.h" #include "buf/validate/conformance/cases/strings.pb.h" #include "eval/public/activation.h" @@ -79,7 +79,7 @@ TEST(ValidatorTest, ValidateBool) { auto violations_or = validator.Validate(bool_const_false); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); ASSERT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "bool.const"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "bool.const"); EXPECT_EQ(violations_or.value().violations(0).proto().message(), "value must equal false"); } @@ -140,7 +140,7 @@ TEST(ValidatorTest, ValidateRelativeURIFailure) { auto violations_or = validator.Validate(str_uri); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.uri"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.uri"); EXPECT_EQ(violations_or.value().violations(0).proto().message(), "value must be a valid URI"); EXPECT_THAT( violations_or.value().violations(0).field_value(), @@ -200,7 +200,7 @@ TEST(ValidatorTest, ValidateBadURIRefFailure) { auto violations_or = validator.Validate(str_uri_ref); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.uri_ref"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.uri_ref"); EXPECT_EQ( violations_or.value().violations(0).proto().message(), "value must be a valid URI Reference"); EXPECT_THAT( @@ -223,7 +223,7 @@ TEST(ValidatorTest, ValidateStrRepeatedUniqueFailure) { auto violations_or = validator.Validate(str_repeated); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "repeated.unique"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "repeated.unique"); EXPECT_EQ( violations_or.value().violations(0).proto().message(), "repeated value must contain unique items"); @@ -248,7 +248,7 @@ TEST(ValidatorTest, ValidateStringContainsFailure) { auto violations_or = validator.Validate(str_contains); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.contains"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.contains"); EXPECT_EQ( violations_or.value().violations(0).proto().message(), "value does not contain substring `bar`"); @@ -278,7 +278,7 @@ TEST(ValidatorTest, ValidateBytesContainsFailure) { auto violations_or = validator.Validate(bytes_contains); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "bytes.contains"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "bytes.contains"); EXPECT_EQ(violations_or.value().violations(0).proto().message(), "value does not contain 626172"); } @@ -306,7 +306,7 @@ TEST(ValidatorTest, ValidateStartsWithFailure) { auto violations_or = validator.Validate(str_starts_with); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.prefix"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.prefix"); EXPECT_EQ( violations_or.value().violations(0).proto().message(), "value does not have prefix `foo`"); } @@ -335,7 +335,7 @@ TEST(ValidatorTest, ValidateEndsWithFailure) { auto violations_or = validator.Validate(str_ends_with); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.suffix"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.suffix"); EXPECT_EQ( violations_or.value().violations(0).proto().message(), "value does not have suffix `baz`"); } @@ -364,7 +364,7 @@ TEST(ValidatorTest, ValidateGarbageHostnameFailure) { auto violations_or = validator.Validate(str_hostname); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.hostname"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.hostname"); } TEST(ValidatorTest, ValidateHostnameFailure) { @@ -378,7 +378,7 @@ TEST(ValidatorTest, ValidateHostnameFailure) { auto violations_or = validator.Validate(str_hostname); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.hostname"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.hostname"); } TEST(ValidatorTest, ValidateHostnameDoubleDotFailure) { @@ -392,7 +392,7 @@ TEST(ValidatorTest, ValidateHostnameDoubleDotFailure) { auto violations_or = validator.Validate(str_hostname); ASSERT_TRUE(violations_or.ok()) << violations_or.status(); EXPECT_EQ(violations_or.value().violations_size(), 1); - EXPECT_EQ(violations_or.value().violations(0).proto().constraint_id(), "string.hostname"); + EXPECT_EQ(violations_or.value().violations(0).proto().rule_id(), "string.hostname"); } TEST(ValidatorTest, ValidateEndsWithSuccess) { @@ -409,7 +409,7 @@ TEST(ValidatorTest, ValidateEndsWithSuccess) { } TEST(ValidatorTest, MessageConstraint) { - conformance::cases::custom_constraints::MessageExpressions message_expressions; + conformance::cases::custom_rules::MessageExpressions message_expressions; message_expressions.mutable_e(); auto factory_or = ValidatorFactory::New(); ASSERT_TRUE(factory_or.ok()) << factory_or.status(); @@ -420,12 +420,12 @@ TEST(ValidatorTest, MessageConstraint) { ASSERT_TRUE(violations_or.ok()) << violations_or.status(); ASSERT_EQ(violations_or.value().violations_size(), 3); EXPECT_EQ( - violations_or.value().violations(0).proto().constraint_id(), "message_expression_scalar"); + violations_or.value().violations(0).proto().rule_id(), "message_expression_scalar"); EXPECT_EQ(violations_or.value().violations(0).proto().message(), "a must be less than b"); - EXPECT_EQ(violations_or.value().violations(1).proto().constraint_id(), "message_expression_enum"); + EXPECT_EQ(violations_or.value().violations(1).proto().rule_id(), "message_expression_enum"); EXPECT_EQ(violations_or.value().violations(1).proto().message(), "c must not equal d"); EXPECT_EQ( - violations_or.value().violations(2).proto().constraint_id(), "message_expression_nested"); + violations_or.value().violations(2).proto().rule_id(), "message_expression_nested"); EXPECT_EQ(violations_or.value().violations(2).proto().message(), "a must be greater than b"); } diff --git a/deps/shared_deps.json b/deps/shared_deps.json index 4f1abe6..766ee00 100644 --- a/deps/shared_deps.json +++ b/deps/shared_deps.json @@ -38,10 +38,10 @@ }, "protovalidate": { "meta": { - "version": "0.10.6" + "version": "0.11.0" }, "source": { - "sha256": "644862cbfef07b70a62fc2caffb9321e16a601e079b282f1307dc3019636f8ef", + "sha256": "d519e364a41fb3826a97b75cd7348d3e1e420319dde0ff397f42307bd5a44d93", "strip_prefix": "protovalidate-{version}", "urls": [ "https://github.com/bufbuild/protovalidate/releases/download/v{version}/protovalidate-{version}.tar.gz" From a4dabc41875e63d0a72aa766002ca7434594d030 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 23 Apr 2025 10:13:57 -0400 Subject: [PATCH 3/4] Remove now-unneeded field access patch --- MODULE.bazel | 5 +- MODULE.bazel.tmpl | 5 +- bazel/deps.bzl | 5 +- cmake/Deps.cmake | 5 +- ...Add-missing-include-for-absl-StrCat.patch} | 4 +- ...ge-field-access-using-index-operator.patch | 85 ------------------- ...h => 0002-Fix-build-on-Windows-MSVC.patch} | 4 +- 7 files changed, 12 insertions(+), 101 deletions(-) rename deps/patches/cel_cpp/{0002-Add-missing-include-for-absl-StrCat.patch => 0001-Add-missing-include-for-absl-StrCat.patch} (80%) delete mode 100644 deps/patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch rename deps/patches/cel_cpp/{0003-Fix-build-on-Windows-MSVC.patch => 0002-Fix-build-on-Windows-MSVC.patch} (99%) diff --git a/MODULE.bazel b/MODULE.bazel index a46dfcb..25548a6 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -34,9 +34,8 @@ archive_override( module_name = "cel-cpp", strip_prefix = "cel-cpp-0.11.0", patches=[ - "@protovalidate-cc//deps:patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], urls = [ diff --git a/MODULE.bazel.tmpl b/MODULE.bazel.tmpl index 2a2c1fc..5c3512d 100644 --- a/MODULE.bazel.tmpl +++ b/MODULE.bazel.tmpl @@ -34,9 +34,8 @@ archive_override( module_name = "cel-cpp", strip_prefix = "cel-cpp-{{cel_cpp.meta.version}}", patches=[ - "@protovalidate-cc//deps:patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], urls = [ diff --git a/bazel/deps.bzl b/bazel/deps.bzl index 47c0ed3..ebf2e50 100644 --- a/bazel/deps.bzl +++ b/bazel/deps.bzl @@ -101,9 +101,8 @@ cc_library( "com_google_cel_cpp": shared_dep( name="cel_cpp", patches=[ - "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch", - "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch", - "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch", + "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", + "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], ), diff --git a/cmake/Deps.cmake b/cmake/Deps.cmake index cef509d..95106b9 100644 --- a/cmake/Deps.cmake +++ b/cmake/Deps.cmake @@ -275,9 +275,8 @@ FetchContent_MakeAvailable(cel_spec) SharedDeps_GetSourceValue(PROTOVALIDATE_CC_CEL_CPP_URLS "cel_cpp" "urls" "${SHARED_DEPS}") SharedDeps_GetSourceValue(PROTOVALIDATE_CC_CEL_CPP_SHA256 "cel_cpp" "sha256" "${SHARED_DEPS}") set(CEL_CPP_PATCHES - ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch - ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch - ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch + ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch + ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch ) MakePatchCommand(CEL_CPP_PATCH_COMMAND "${CEL_CPP_PATCHES}") message(STATUS "protovalidate-cc: Fetching cel-cpp") diff --git a/deps/patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch b/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch similarity index 80% rename from deps/patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch rename to deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch index 681c8a4..be4edd8 100644 --- a/deps/patches/cel_cpp/0002-Add-missing-include-for-absl-StrCat.patch +++ b/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch @@ -1,7 +1,7 @@ -From dfd27892a34130d62b1f96851288be1b195342d2 Mon Sep 17 00:00:00 2001 +From 3364768a70aa78bdb9e83d1872e91658134bc051 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 5 Mar 2025 20:34:56 -0500 -Subject: [PATCH 2/3] Add missing #include for absl::StrCat +Subject: [PATCH 1/2] Add missing #include for absl::StrCat --- internal/testing.cc | 2 ++ diff --git a/deps/patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch b/deps/patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch deleted file mode 100644 index 6c8a959..0000000 --- a/deps/patches/cel_cpp/0001-Allow-message-field-access-using-index-operator.patch +++ /dev/null @@ -1,85 +0,0 @@ -From 27116d4f33e63ac7dbabc9fbdd64652531f894d2 Mon Sep 17 00:00:00 2001 -From: John Chadwick -Date: Fri, 24 Jan 2025 18:19:56 -0500 -Subject: [PATCH 1/3] Allow message field access using index operator - -CEL doesn't yet have a standard way to access field names that conflict -with identifiers. This temporary workaround enables the index operator -to access fields from a message. It can be used as follows: - - dyn(message)["field_name"] ---- - eval/eval/container_access_step.cc | 36 ++++++++++++++++++++++++++++++ - 1 file changed, 36 insertions(+) - -diff --git a/eval/eval/container_access_step.cc b/eval/eval/container_access_step.cc -index a9fa4c0f..11782d1c 100644 ---- a/eval/eval/container_access_step.cc -+++ b/eval/eval/container_access_step.cc -@@ -3,6 +3,7 @@ - #include - #include - #include -+#include - - #include "absl/status/status.h" - #include "absl/status/statusor.h" -@@ -21,6 +22,8 @@ - #include "eval/eval/evaluator_core.h" - #include "eval/eval/expression_step_base.h" - #include "eval/internal/errors.h" -+#include "eval/public/structs/legacy_type_adapter.h" -+#include "eval/public/structs/legacy_type_info_apis.h" - #include "internal/number.h" - #include "internal/status_macros.h" - #include "runtime/internal/errors.h" -@@ -193,6 +196,35 @@ void LookupInList(const ListValue& cel_list, const Value& key, - } - } - -+void LookupInMessage(const cel::StructValue& struct_value, const Value& key, ExecutionFrameBase& frame, Value& result) { -+ if (!key.IsString()) { -+ result = cel::ErrorValue(absl::UnknownError( -+ absl::StrCat("Index error: expected integer type, got ", -+ cel::KindToString(ValueKindToKind(key->kind()))))); -+ return; -+ } -+ auto found = struct_value.HasFieldByName(key.GetString().ToString()); -+ if (!found.ok()) { -+ result = cel::ErrorValue(CreateNoSuchKeyError(key->DebugString())); -+ return; -+ } -+ if (!*found) { -+ result = cel::NoSuchFieldError(key.AsString()->ToString()); -+ return; -+ } -+ auto status = struct_value.GetFieldByName( -+ key.GetString().ToString(), -+ ProtoWrapperTypeOptions::kUnsetProtoDefault, -+ frame.descriptor_pool(), -+ frame.message_factory(), -+ frame.arena(), -+ &result); -+ if (!status.ok()) { -+ result = cel::ErrorValue(status); -+ } -+ return; -+} -+ - void LookupInContainer(const Value& container, const Value& key, - ExecutionFrameBase& frame, Value& result) { - // Select steps can be applied to either maps or messages -@@ -205,6 +237,10 @@ void LookupInContainer(const Value& container, const Value& key, - LookupInList(Cast(container), key, frame, result); - return; - } -+ case ValueKind::kStruct: { -+ LookupInMessage(*container.AsStruct(), key, frame, result); -+ return; -+ } - default: - result = cel::ErrorValue(absl::InvalidArgumentError( - absl::StrCat("Invalid container type: '", --- -2.47.2 - diff --git a/deps/patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch b/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch similarity index 99% rename from deps/patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch rename to deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch index ef19c13..1859f54 100644 --- a/deps/patches/cel_cpp/0003-Fix-build-on-Windows-MSVC.patch +++ b/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch @@ -1,7 +1,7 @@ -From bc1f0c3182be2157c957d4a13ff74a950a466b80 Mon Sep 17 00:00:00 2001 +From a4f1a7b520817badd93d60ab0e9b80d0d0d0e7b2 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 21 Mar 2025 20:16:07 -0400 -Subject: [PATCH 3/3] Fix build on Windows/MSVC +Subject: [PATCH 2/2] Fix build on Windows/MSVC Some fixes+hacks+workarounds for build issues on MSVC. --- From 604bb72774769b76714c8869d3aec6438ff6adc2 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Wed, 23 Apr 2025 10:18:08 -0400 Subject: [PATCH 4/4] Remove stale missing include patch --- MODULE.bazel | 3 +-- MODULE.bazel.tmpl | 3 +-- bazel/deps.bzl | 3 +-- cmake/Deps.cmake | 3 +-- ...-Add-missing-include-for-absl-StrCat.patch | 25 ------------------- ...h => 0001-Fix-build-on-Windows-MSVC.patch} | 4 +-- 6 files changed, 6 insertions(+), 35 deletions(-) delete mode 100644 deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch rename deps/patches/cel_cpp/{0002-Fix-build-on-Windows-MSVC.patch => 0001-Fix-build-on-Windows-MSVC.patch} (99%) diff --git a/MODULE.bazel b/MODULE.bazel index 25548a6..6cf92d6 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -34,8 +34,7 @@ archive_override( module_name = "cel-cpp", strip_prefix = "cel-cpp-0.11.0", patches=[ - "@protovalidate-cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], urls = [ diff --git a/MODULE.bazel.tmpl b/MODULE.bazel.tmpl index 5c3512d..2331d40 100644 --- a/MODULE.bazel.tmpl +++ b/MODULE.bazel.tmpl @@ -34,8 +34,7 @@ archive_override( module_name = "cel-cpp", strip_prefix = "cel-cpp-{{cel_cpp.meta.version}}", patches=[ - "@protovalidate-cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", - "@protovalidate-cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", + "@protovalidate-cc//deps:patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], urls = [ diff --git a/bazel/deps.bzl b/bazel/deps.bzl index ebf2e50..d6c38df 100644 --- a/bazel/deps.bzl +++ b/bazel/deps.bzl @@ -101,8 +101,7 @@ cc_library( "com_google_cel_cpp": shared_dep( name="cel_cpp", patches=[ - "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch", - "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch", + "@com_github_bufbuild_protovalidate_cc//deps:patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch", ], patch_args=["-p1"], ), diff --git a/cmake/Deps.cmake b/cmake/Deps.cmake index 95106b9..895e6b5 100644 --- a/cmake/Deps.cmake +++ b/cmake/Deps.cmake @@ -275,8 +275,7 @@ FetchContent_MakeAvailable(cel_spec) SharedDeps_GetSourceValue(PROTOVALIDATE_CC_CEL_CPP_URLS "cel_cpp" "urls" "${SHARED_DEPS}") SharedDeps_GetSourceValue(PROTOVALIDATE_CC_CEL_CPP_SHA256 "cel_cpp" "sha256" "${SHARED_DEPS}") set(CEL_CPP_PATCHES - ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch - ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch + ${CMAKE_CURRENT_SOURCE_DIR}/deps/patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch ) MakePatchCommand(CEL_CPP_PATCH_COMMAND "${CEL_CPP_PATCHES}") message(STATUS "protovalidate-cc: Fetching cel-cpp") diff --git a/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch b/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch deleted file mode 100644 index be4edd8..0000000 --- a/deps/patches/cel_cpp/0001-Add-missing-include-for-absl-StrCat.patch +++ /dev/null @@ -1,25 +0,0 @@ -From 3364768a70aa78bdb9e83d1872e91658134bc051 Mon Sep 17 00:00:00 2001 -From: John Chadwick -Date: Wed, 5 Mar 2025 20:34:56 -0500 -Subject: [PATCH 1/2] Add missing #include for absl::StrCat - ---- - internal/testing.cc | 2 ++ - 1 file changed, 2 insertions(+) - -diff --git a/internal/testing.cc b/internal/testing.cc -index 77e4c65b..e0dfa75e 100644 ---- a/internal/testing.cc -+++ b/internal/testing.cc -@@ -14,6 +14,8 @@ - - #include "internal/testing.h" - -+#include "absl/strings/str_cat.h" -+ - namespace cel::internal { - - void AddFatalFailure(const char* file, int line, absl::string_view expression, --- -2.47.2 - diff --git a/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch b/deps/patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch similarity index 99% rename from deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch rename to deps/patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch index 1859f54..1ac170a 100644 --- a/deps/patches/cel_cpp/0002-Fix-build-on-Windows-MSVC.patch +++ b/deps/patches/cel_cpp/0001-Fix-build-on-Windows-MSVC.patch @@ -1,7 +1,7 @@ -From a4f1a7b520817badd93d60ab0e9b80d0d0d0e7b2 Mon Sep 17 00:00:00 2001 +From cb3a0807e258f86c76ece50a71768056fdff7808 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Fri, 21 Mar 2025 20:16:07 -0400 -Subject: [PATCH 2/2] Fix build on Windows/MSVC +Subject: [PATCH] Fix build on Windows/MSVC Some fixes+hacks+workarounds for build issues on MSVC. ---