From bbd370d3f99b6280f4072088fd243286f4f1bf99 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 3 Dec 2024 14:10:32 -0500 Subject: [PATCH] Even simpler handling of nesting --- internal/evaluator/base.go | 17 +---------------- internal/evaluator/builder.go | 35 +++++++++++----------------------- internal/evaluator/field.go | 2 +- internal/evaluator/map.go | 8 ++++++++ internal/evaluator/repeated.go | 7 +++++++ internal/evaluator/value.go | 20 +++---------------- 6 files changed, 31 insertions(+), 58 deletions(-) diff --git a/internal/evaluator/base.go b/internal/evaluator/base.go index 57be3b6..da65b95 100644 --- a/internal/evaluator/base.go +++ b/internal/evaluator/base.go @@ -40,7 +40,7 @@ func newBase(valEval *value) base { return base{ Descriptor: valEval.Descriptor, FieldPathElement: errors.FieldPathElement(valEval.Descriptor), - RulePrefix: rulePrefixForNesting(valEval.Nested), + RulePrefix: valEval.NestedRule, } } @@ -59,21 +59,6 @@ func (b *base) rulePath(suffix *validate.FieldPath) *validate.FieldPath { return prefixRulePath(b.RulePrefix, suffix) } -func rulePrefixForNesting(typ nestedType) *validate.FieldPath { - switch typ { - case nestedNone: - return nil - case nestedRepeatedItem: - return repeatedItemsRulePath - case nestedMapKey: - return mapKeysRulePath - case nestedMapValue: - return mapValuesRulePath - default: - return nil - } -} - func prefixRulePath(prefix *validate.FieldPath, suffix *validate.FieldPath) *validate.FieldPath { if len(prefix.GetElements()) > 0 { return &validate.FieldPath{ diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index a7e2b12..38b4ca3 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -278,9 +278,9 @@ func (bldr *Builder) processIgnoreEmpty( ) error { // the only time we need to ignore empty on a value is if it's evaluating a // field item (repeated element or map key/value). - val.IgnoreEmpty = val.Nested != nestedNone && bldr.shouldIgnoreEmpty(constraints) + val.IgnoreEmpty = val.NestedRule != nil && bldr.shouldIgnoreEmpty(constraints) if val.IgnoreEmpty { - val.Zero = bldr.zeroValue(fdesc, val.Nested != nestedNone) + val.Zero = bldr.zeroValue(fdesc, val.NestedRule != nil) } return nil } @@ -295,7 +295,7 @@ func (bldr *Builder) processFieldExpressions( Constraints: fieldConstraints.GetCel(), } - celTyp := celext.ProtoFieldToCELType(fieldDesc, false, eval.Nested != nestedNone) + celTyp := celext.ProtoFieldToCELType(fieldDesc, false, eval.NestedRule != nil) opts := append( celext.RequiredCELEnvOptions(fieldDesc), cel.Variable("this", celTyp), @@ -337,7 +337,7 @@ func (bldr *Builder) processEmbeddedMessage( if !isMessageField(fdesc) || bldr.shouldSkip(rules) || fdesc.IsMap() || - (fdesc.IsList() && valEval.Nested == nestedNone) { + (fdesc.IsList() && valEval.NestedRule == nil) { return nil } @@ -364,7 +364,7 @@ func (bldr *Builder) processWrapperConstraints( if !isMessageField(fdesc) || bldr.shouldSkip(rules) || fdesc.IsMap() || - (fdesc.IsList() && valEval.Nested == nestedNone) { + (fdesc.IsList() && valEval.NestedRule == nil) { return nil } @@ -374,7 +374,7 @@ func (bldr *Builder) processWrapperConstraints( } unwrapped := value{ Descriptor: valEval.Descriptor, - Nested: valEval.Nested, + NestedRule: valEval.NestedRule, } err := bldr.buildValue(fdesc.Message().Fields().ByName("value"), rules, &unwrapped, cache) if err != nil { @@ -396,7 +396,7 @@ func (bldr *Builder) processStandardConstraints( constraints, bldr.extensionTypeResolver, bldr.allowUnknownFields, - valEval.Nested != nestedNone, + valEval.NestedRule != nil, ) if err != nil { return err @@ -414,7 +414,7 @@ func (bldr *Builder) processAnyConstraints( valEval *value, _ MessageCache, ) error { - if (fdesc.IsList() && valEval.Nested == nestedNone) || + if (fdesc.IsList() && valEval.NestedRule == nil) || !isMessageField(fdesc) || fdesc.Message().FullName() != "google.protobuf.Any" { return nil @@ -464,15 +464,7 @@ func (bldr *Builder) processMapConstraints( return nil } - mapEval := kvPairs{ - base: newBase(valEval), - KeyConstraints: value{ - Nested: nestedMapKey, - }, - ValueConstraints: value{ - Nested: nestedMapValue, - }, - } + mapEval := newKVPairs(valEval) err := bldr.buildValue( fieldDesc.MapKey(), @@ -506,16 +498,11 @@ func (bldr *Builder) processRepeatedConstraints( valEval *value, cache MessageCache, ) error { - if !fdesc.IsList() || valEval.Nested != nestedNone { + if !fdesc.IsList() || valEval.NestedRule != nil { return nil } - listEval := listItems{ - base: newBase(valEval), - ItemConstraints: value{ - Nested: nestedRepeatedItem, - }, - } + listEval := newListItems(valEval) err := bldr.buildValue(fdesc, fieldConstraints.GetRepeated().GetItems(), &listEval.ItemConstraints, cache) if err != nil { diff --git a/internal/evaluator/field.go b/internal/evaluator/field.go index 18a8830..0b8388c 100644 --- a/internal/evaluator/field.go +++ b/internal/evaluator/field.go @@ -58,7 +58,7 @@ func (f field) EvaluateMessage(msg protoreflect.Message, failFast bool) (err err return &errors.ValidationError{Violations: []*errors.Violation{{ Proto: &validate.Violation{ Field: errors.FieldPath(f.Value.Descriptor), - Rule: prefixRulePath(rulePrefixForNesting(f.Value.Nested), requiredRulePath), + Rule: prefixRulePath(f.Value.NestedRule, requiredRulePath), ConstraintId: proto.String("required"), Message: proto.String("value is required"), }, diff --git a/internal/evaluator/map.go b/internal/evaluator/map.go index 90ae1fb..0fca1eb 100644 --- a/internal/evaluator/map.go +++ b/internal/evaluator/map.go @@ -54,6 +54,14 @@ type kvPairs struct { ValueConstraints value } +func newKVPairs(valEval *value) kvPairs { + return kvPairs{ + base: newBase(valEval), + KeyConstraints: value{NestedRule: mapKeysRulePath}, + ValueConstraints: value{NestedRule: mapValuesRulePath}, + } +} + func (m kvPairs) Evaluate(val protoreflect.Value, failFast bool) (err error) { var ok bool val.Map().Range(func(key protoreflect.MapKey, value protoreflect.Value) bool { diff --git a/internal/evaluator/repeated.go b/internal/evaluator/repeated.go index 75ff08a..032448e 100644 --- a/internal/evaluator/repeated.go +++ b/internal/evaluator/repeated.go @@ -41,6 +41,13 @@ type listItems struct { ItemConstraints value } +func newListItems(valEval *value) listItems { + return listItems{ + base: newBase(valEval), + ItemConstraints: value{NestedRule: repeatedItemsRulePath}, + } +} + func (r listItems) Evaluate(val protoreflect.Value, failFast bool) error { list := val.List() var ok bool diff --git a/internal/evaluator/value.go b/internal/evaluator/value.go index e409f67..753ba83 100644 --- a/internal/evaluator/value.go +++ b/internal/evaluator/value.go @@ -15,24 +15,10 @@ package evaluator import ( + "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" "google.golang.org/protobuf/reflect/protoreflect" ) -// nestedType specifies a kind of nested value, if the value is being evaluated -// as a map key, map value, or repeated item. -type nestedType uint8 - -const ( - // nestedNone specifies that the value is not being evaluated as a nested value. - nestedNone nestedType = iota - // nestedRepeatedItem specifies that the value is being evaluated as a repeated field item. - nestedRepeatedItem - // nestedMapKey specifies that the value is being evaluated as a map key. - nestedMapKey - // nestedMapValue specifies that the value is being evaluated as a map value. - nestedMapValue -) - // value performs validation on any concrete value contained within a singular // field, repeated elements, or the keys/values of a map. type value struct { @@ -42,12 +28,12 @@ type value struct { Constraints evaluators // Zero is the default or zero-value for this value's type Zero protoreflect.Value + // NestedRule specifies the nested rule type the value is for. + NestedRule *validate.FieldPath // IgnoreEmpty indicates that the Constraints should not be applied if the // value is unset or the default (typically zero) value. This only applies to // repeated elements or map keys/values with an ignore_empty rule. IgnoreEmpty bool - // Nested specifies the kind of nested field the value is for. - Nested nestedType } func (v *value) Evaluate(val protoreflect.Value, failFast bool) error {