Skip to content

Commit

Permalink
Implement predefined field constraints (#144)
Browse files Browse the repository at this point in the history
  • Loading branch information
jchadwick-buf authored Sep 23, 2024
1 parent fc11a84 commit ca39053
Show file tree
Hide file tree
Showing 39 changed files with 11,699 additions and 2,495 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ GOLANGCI_LINT_VERSION ?= v1.60.1
# Set to use a different version of protovalidate-conformance.
# Should be kept in sync with the version referenced in proto/buf.lock and
# 'buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go' in go.mod.
CONFORMANCE_VERSION ?= v0.7.1
CONFORMANCE_VERSION ?= v0.8.1

.PHONY: help
help: ## Describe useful make targets
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/bufbuild/protovalidate-go
go 1.21

require (
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240717164558-a6c49f84cc0f.2
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240920164238-5a7b106cbb87.2
github.com/envoyproxy/protoc-gen-validate v1.1.0
github.com/google/cel-go v0.21.0
github.com/stretchr/testify v1.9.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240717164558-a6c49f84cc0f.2 h1:SZRVx928rbYZ6hEKUIN+vtGDkl7uotABRWGY4OAg5gM=
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240717164558-a6c49f84cc0f.2/go.mod h1:ylS4c28ACSI59oJrOdW4pHS4n0Hw4TgSPHn8rpHl4Yw=
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240920164238-5a7b106cbb87.2 h1:hl0FrmGlNpQZIGvU1/jDz0lsPDd0BhCE0QDRwPfLZcA=
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.34.2-20240920164238-5a7b106cbb87.2/go.mod h1:ylS4c28ACSI59oJrOdW4pHS4n0Hw4TgSPHn8rpHl4Yw=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
Expand Down
16 changes: 15 additions & 1 deletion internal/cmd/protovalidate-conformance-go/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,21 @@ func TestConformance(req *harness.TestConformanceRequest) (*harness.TestConforma
err = fmt.Errorf("failed to parse file descriptors: %w", err)
return nil, err
}
val, err := protovalidate.New()
registry := &protoregistry.Types{}
files.RangeFiles(func(file protoreflect.FileDescriptor) bool {
for i := 0; i < file.Extensions().Len(); i++ {
if err = registry.RegisterExtension(
dynamicpb.NewExtensionType(file.Extensions().Get(i)),
); err != nil {
return false
}
}
return err == nil
})
if err != nil {
return nil, err
}
val, err := protovalidate.New(protovalidate.WithExtensionTypeResolver(registry))
if err != nil {
err = fmt.Errorf("failed to initialize validator: %w", err)
return nil, err
Expand Down
56 changes: 51 additions & 5 deletions internal/constraints/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ package constraints

import (
"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate/priv"
"github.com/bufbuild/protovalidate-go/celext"
"github.com/bufbuild/protovalidate-go/internal/errors"
"github.com/bufbuild/protovalidate-go/internal/expression"
"github.com/bufbuild/protovalidate-go/internal/extensions"
"github.com/google/cel-go/cel"
"github.com/google/cel-go/common/types"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
)

// Cache is a build-through cache to computed standard constraints.
Expand All @@ -44,21 +46,45 @@ func (c *Cache) Build(
env *cel.Env,
fieldDesc protoreflect.FieldDescriptor,
fieldConstraints *validate.FieldConstraints,
extensionTypeResolver protoregistry.ExtensionTypeResolver,
allowUnknownFields bool,
forItems bool,
) (set expression.ProgramSet, err error) {
constraints, done, err := c.resolveConstraints(fieldDesc, fieldConstraints, forItems)
constraints, done, err := c.resolveConstraints(
fieldDesc,
fieldConstraints,
forItems,
)
if done {
return nil, err
}

if err = reparseUnrecognized(extensionTypeResolver, constraints); err != nil {
return nil, errors.NewCompilationErrorf("error reparsing message: %w", err)
}
if !allowUnknownFields && len(constraints.GetUnknown()) > 0 {
return nil, errors.NewCompilationErrorf("unknown constraints in %s; see protovalidate.WithExtensionTypeResolver", constraints.Descriptor().FullName())
}

env, err = c.prepareEnvironment(env, fieldDesc, constraints, forItems)
if err != nil {
return nil, err
}

var asts expression.ASTSet
constraints.Range(func(desc protoreflect.FieldDescriptor, _ protoreflect.Value) bool {
precomputedASTs, compileErr := c.loadOrCompileStandardConstraint(env, desc)
constraints.Range(func(desc protoreflect.FieldDescriptor, rule protoreflect.Value) bool {
fieldEnv, compileErr := env.Extend(
cel.Constant(
"rule",
celext.ProtoFieldToCELType(desc, true, forItems),
types.DefaultTypeAdapter.NativeToValue(rule.Interface()),
),
)
if compileErr != nil {
err = compileErr
return false
}
precomputedASTs, compileErr := c.loadOrCompileStandardConstraint(fieldEnv, desc)
if compileErr != nil {
err = compileErr
return false
Expand Down Expand Up @@ -137,7 +163,10 @@ func (c *Cache) loadOrCompileStandardConstraint(
if cachedConstraint, ok := c.cache[constraintFieldDesc]; ok {
return cachedConstraint, nil
}
exprs, _ := proto.GetExtension(constraintFieldDesc.Options(), priv.E_Field).(*priv.FieldConstraints)
exprs := extensions.Resolve[*validate.PredefinedConstraints](
constraintFieldDesc.Options(),
validate.E_Predefined,
)
set, err = expression.CompileASTs(exprs.GetCel(), env)
if err != nil {
return set, errors.NewCompilationErrorf(
Expand Down Expand Up @@ -170,3 +199,20 @@ func (c *Cache) getExpectedConstraintDescriptor(
return expected, ok
}
}

func reparseUnrecognized(
extensionTypeResolver protoregistry.ExtensionTypeResolver,
reflectMessage protoreflect.Message,
) error {
if unknown := reflectMessage.GetUnknown(); len(unknown) > 0 {
reflectMessage.SetUnknown(nil)
options := proto.UnmarshalOptions{
Resolver: extensionTypeResolver,
Merge: true,
}
if err := options.Unmarshal(unknown, reflectMessage.Interface()); err != nil {
return err
}
}
return nil
}
3 changes: 2 additions & 1 deletion internal/constraints/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
)

func getFieldDesc(t *testing.T, msg proto.Message, fld protoreflect.Name) protoreflect.FieldDescriptor {
Expand Down Expand Up @@ -100,7 +101,7 @@ func TestCache_BuildStandardConstraints(t *testing.T) {
require.NoError(t, err)
c := NewCache()

set, err := c.Build(env, test.desc, test.cons, test.forItems)
set, err := c.Build(env, test.desc, test.cons, protoregistry.GlobalTypes, false, test.forItems)
if test.exErr {
assert.Error(t, err)
} else {
Expand Down
4 changes: 3 additions & 1 deletion internal/errors/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package errors

import (
"errors"

"google.golang.org/protobuf/proto"
)

// Merge is a utility to resolve and combine errors resulting from
Expand Down Expand Up @@ -60,7 +62,7 @@ func MarkForKey(err error) {
var valErr *ValidationError
if errors.As(err, &valErr) {
for _, violation := range valErr.Violations {
violation.ForKey = true
violation.ForKey = proto.Bool(true)
}
}
}
16 changes: 8 additions & 8 deletions internal/errors/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestMerge(t *testing.T) {

t.Run("validation", func(t *testing.T) {
t.Parallel()
exErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: "foo"}}}
exErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
ok, err := Merge(nil, exErr, true)
var valErr *ValidationError
require.ErrorAs(t, err, &valErr)
Expand All @@ -72,7 +72,7 @@ func TestMerge(t *testing.T) {
t.Run("non-validation dst", func(t *testing.T) {
t.Parallel()
dstErr := errors.New("some error")
srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: "foo"}}}
srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
ok, err := Merge(dstErr, srcErr, true)
assert.Equal(t, dstErr, err)
assert.False(t, ok)
Expand All @@ -83,7 +83,7 @@ func TestMerge(t *testing.T) {

t.Run("non-validation src", func(t *testing.T) {
t.Parallel()
dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: "foo"}}}
dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
srcErr := errors.New("some error")
ok, err := Merge(dstErr, srcErr, true)
assert.Equal(t, srcErr, err)
Expand All @@ -96,18 +96,18 @@ func TestMerge(t *testing.T) {
t.Run("validation", func(t *testing.T) {
t.Parallel()

dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: "foo"}}}
srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: "bar"}}}
dstErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
srcErr := &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("bar")}}}
exErr := &ValidationError{Violations: []*validate.Violation{
{ConstraintId: "foo"},
{ConstraintId: "bar"},
{ConstraintId: proto.String("foo")},
{ConstraintId: proto.String("bar")},
}}
ok, err := Merge(dstErr, srcErr, true)
var valErr *ValidationError
require.ErrorAs(t, err, &valErr)
assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
assert.False(t, ok)
dstErr = &ValidationError{Violations: []*validate.Violation{{ConstraintId: "foo"}}}
dstErr = &ValidationError{Violations: []*validate.Violation{{ConstraintId: proto.String("foo")}}}
ok, err = Merge(dstErr, srcErr, false)
require.ErrorAs(t, err, &valErr)
assert.True(t, proto.Equal(exErr.ToProto(), valErr.ToProto()))
Expand Down
7 changes: 4 additions & 3 deletions internal/errors/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"

"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
"google.golang.org/protobuf/proto"
)

// A ValidationError is returned if one or more constraint violations were
Expand Down Expand Up @@ -53,11 +54,11 @@ func PrefixFieldPaths(err *ValidationError, format string, args ...any) {
for _, violation := range err.Violations {
switch {
case violation.GetFieldPath() == "": // no existing field path
violation.FieldPath = prefix
violation.FieldPath = proto.String(prefix)
case violation.GetFieldPath()[0] == '[': // field is a map/list
violation.FieldPath = prefix + violation.GetFieldPath()
violation.FieldPath = proto.String(prefix + violation.GetFieldPath())
default: // any other field
violation.FieldPath = fmt.Sprintf("%s.%s", prefix, violation.GetFieldPath())
violation.FieldPath = proto.String(fmt.Sprintf("%s.%s", prefix, violation.GetFieldPath()))
}
}
}
5 changes: 3 additions & 2 deletions internal/errors/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
"github.com/stretchr/testify/assert"
"google.golang.org/protobuf/proto"
)

func TestPrefixFieldPaths(t *testing.T) {
Expand Down Expand Up @@ -61,8 +62,8 @@ func TestPrefixFieldPaths(t *testing.T) {
t.Run(test.expected, func(t *testing.T) {
t.Parallel()
err := &ValidationError{Violations: []*validate.Violation{
{FieldPath: test.fieldPath},
{FieldPath: test.fieldPath},
{FieldPath: proto.String(test.fieldPath)},
{FieldPath: proto.String(test.fieldPath)},
}}
PrefixFieldPaths(err, test.format, test.args...)
for _, v := range err.Violations {
Expand Down
9 changes: 5 additions & 4 deletions internal/evaluator/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package evaluator
import (
"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
"github.com/bufbuild/protovalidate-go/internal/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
)

Expand All @@ -40,8 +41,8 @@ func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error {
if len(a.In) > 0 {
if _, ok := a.In[typeURL]; !ok {
err.Violations = append(err.Violations, &validate.Violation{
ConstraintId: "any.in",
Message: "type URL must be in the allow list",
ConstraintId: proto.String("any.in"),
Message: proto.String("type URL must be in the allow list"),
})
if failFast {
return err
Expand All @@ -52,8 +53,8 @@ func (a anyPB) Evaluate(val protoreflect.Value, failFast bool) error {
if len(a.NotIn) > 0 {
if _, ok := a.NotIn[typeURL]; ok {
err.Violations = append(err.Violations, &validate.Violation{
ConstraintId: "any.not_in",
Message: "type URL must not be in the block list",
ConstraintId: proto.String("any.not_in"),
Message: proto.String("type URL must not be in the block list"),
})
}
}
Expand Down
27 changes: 18 additions & 9 deletions internal/evaluator/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ import (
"github.com/bufbuild/protovalidate-go/internal/expression"
"github.com/google/cel-go/cel"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/types/dynamicpb"
)

// Builder is a build-through cache of message evaluators keyed off the provided
// descriptor.
type Builder struct {
mtx sync.Mutex // serializes cache writes.
cache atomic.Pointer[MessageCache] // copy-on-write cache.
env *cel.Env
constraints constraints.Cache
resolver StandardConstraintResolver
Load func(desc protoreflect.MessageDescriptor) MessageEvaluator
mtx sync.Mutex // serializes cache writes.
cache atomic.Pointer[MessageCache] // copy-on-write cache.
env *cel.Env
constraints constraints.Cache
resolver StandardConstraintResolver
extensionTypeResolver protoregistry.ExtensionTypeResolver
allowUnknownFields bool
Load func(desc protoreflect.MessageDescriptor) MessageEvaluator
}

type StandardConstraintResolver interface {
Expand All @@ -50,12 +53,16 @@ func NewBuilder(
env *cel.Env,
disableLazy bool,
res StandardConstraintResolver,
extensionTypeResolver protoregistry.ExtensionTypeResolver,
allowUnknownFields bool,
seedDesc ...protoreflect.MessageDescriptor,
) *Builder {
bldr := &Builder{
env: env,
constraints: constraints.NewCache(),
resolver: res,
env: env,
constraints: constraints.NewCache(),
resolver: res,
extensionTypeResolver: extensionTypeResolver,
allowUnknownFields: allowUnknownFields,
}

if disableLazy {
Expand Down Expand Up @@ -357,6 +364,8 @@ func (bldr *Builder) processStandardConstraints(
bldr.env,
fdesc,
constraints,
bldr.extensionTypeResolver,
bldr.allowUnknownFields,
forItems,
)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion internal/evaluator/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
)

func TestBuildCache(t *testing.T) {
Expand All @@ -33,7 +34,7 @@ func TestBuildCache(t *testing.T) {
env, err := celext.DefaultEnv(true)
require.NoError(t, err, "failed to construct CEL environment")
bldr := NewBuilder(
env, false, resolver.DefaultResolver{},
env, false, resolver.DefaultResolver{}, protoregistry.GlobalTypes, false,
)
wg := sync.WaitGroup{}
for i := 0; i < 100; i++ {
Expand Down
5 changes: 3 additions & 2 deletions internal/evaluator/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package evaluator
import (
"buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate"
"github.com/bufbuild/protovalidate-go/internal/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
)

Expand All @@ -31,8 +32,8 @@ type definedEnum struct {
func (d definedEnum) Evaluate(val protoreflect.Value, _ bool) error {
if d.ValueDescriptors.ByNumber(val.Enum()) == nil {
return &errors.ValidationError{Violations: []*validate.Violation{{
ConstraintId: "enum.defined_only",
Message: "value must be one of the defined enum values",
ConstraintId: proto.String("enum.defined_only"),
Message: proto.String("value must be one of the defined enum values"),
}}}
}
return nil
Expand Down
Loading

0 comments on commit ca39053

Please sign in to comment.