Skip to content

Commit

Permalink
Even simpler handling of nesting
Browse files Browse the repository at this point in the history
  • Loading branch information
jchadwick-buf committed Dec 3, 2024
1 parent ba578ec commit bbd370d
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 58 deletions.
17 changes: 1 addition & 16 deletions internal/evaluator/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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{
Expand Down
35 changes: 11 additions & 24 deletions internal/evaluator/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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),
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -396,7 +396,7 @@ func (bldr *Builder) processStandardConstraints(
constraints,
bldr.extensionTypeResolver,
bldr.allowUnknownFields,
valEval.Nested != nestedNone,
valEval.NestedRule != nil,
)
if err != nil {
return err
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/evaluator/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
Expand Down
8 changes: 8 additions & 0 deletions internal/evaluator/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions internal/evaluator/repeated.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 3 additions & 17 deletions internal/evaluator/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit bbd370d

Please sign in to comment.