From ce2218267258e2c434267f3c3b514fbd7dc5c92d Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Tue, 3 Oct 2023 13:55:12 -0400 Subject: [PATCH 1/2] move resolver out --- internal/evaluator/builder.go | 5 +++-- internal/evaluator/builder_test.go | 3 ++- {internal/evaluator => resolver}/resolver.go | 2 +- validator.go | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) rename {internal/evaluator => resolver}/resolver.go (99%) diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index 7410247..7da7397 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -22,6 +22,7 @@ import ( "github.com/bufbuild/protovalidate-go/internal/constraints" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/expression" + "github.com/bufbuild/protovalidate-go/resolver" "github.com/google/cel-go/cel" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/dynamicpb" @@ -34,7 +35,7 @@ type Builder struct { cache atomic.Pointer[MessageCache] // copy-on-write cache. env *cel.Env constraints constraints.Cache - resolver StandardConstraintResolver + resolver resolver.StandardConstraintResolver Load func(desc protoreflect.MessageDescriptor) MessageEvaluator } @@ -42,7 +43,7 @@ type Builder struct { func NewBuilder( env *cel.Env, disableLazy bool, - res StandardConstraintResolver, + res resolver.StandardConstraintResolver, seedDesc ...protoreflect.MessageDescriptor, ) *Builder { bldr := &Builder{ diff --git a/internal/evaluator/builder_test.go b/internal/evaluator/builder_test.go index 3e5e486..5de94cd 100644 --- a/internal/evaluator/builder_test.go +++ b/internal/evaluator/builder_test.go @@ -20,6 +20,7 @@ import ( "github.com/bufbuild/protovalidate-go/celext" pb "github.com/bufbuild/protovalidate-go/internal/gen/tests/example/v1" + "github.com/bufbuild/protovalidate-go/resolver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -32,7 +33,7 @@ func TestBuildCache(t *testing.T) { env, err := celext.DefaultEnv(true) require.NoError(t, err, "failed to construct CEL environment") bldr := NewBuilder( - env, false, DefaultResolver{}, + env, false, resolver.DefaultResolver{}, ) wg := sync.WaitGroup{} for i := 0; i < 100; i++ { diff --git a/internal/evaluator/resolver.go b/resolver/resolver.go similarity index 99% rename from internal/evaluator/resolver.go rename to resolver/resolver.go index 9c444b1..bcb80ac 100644 --- a/internal/evaluator/resolver.go +++ b/resolver/resolver.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package evaluator +package resolver import ( "strings" diff --git a/validator.go b/validator.go index e5ab70d..bf29519 100644 --- a/validator.go +++ b/validator.go @@ -21,6 +21,7 @@ import ( "github.com/bufbuild/protovalidate-go/celext" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/evaluator" + "github.com/bufbuild/protovalidate-go/resolver" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -58,7 +59,7 @@ type Validator struct { // up the CEL execution environment if the configuration is invalid. See the // individual ValidatorOption for how they impact the fallibility of New. func New(options ...ValidatorOption) (*Validator, error) { - cfg := config{resolver: evaluator.DefaultResolver{}} + cfg := config{resolver: resolver.DefaultResolver{}} for _, opt := range options { opt(&cfg) } From 6b4858b3db3797207262e12aa34a0b6fbde0afdc Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Tue, 3 Oct 2023 15:07:20 -0400 Subject: [PATCH 2/2] move interface back, add godoc --- internal/evaluator/builder.go | 11 ++++++++--- resolver/resolver.go | 13 +++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index 7da7397..6e44b6a 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/protovalidate-go/internal/constraints" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/expression" - "github.com/bufbuild/protovalidate-go/resolver" "github.com/google/cel-go/cel" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/dynamicpb" @@ -35,15 +34,21 @@ type Builder struct { cache atomic.Pointer[MessageCache] // copy-on-write cache. env *cel.Env constraints constraints.Cache - resolver resolver.StandardConstraintResolver + resolver StandardConstraintResolver Load func(desc protoreflect.MessageDescriptor) MessageEvaluator } +type StandardConstraintResolver interface { + ResolveMessageConstraints(desc protoreflect.MessageDescriptor) *validate.MessageConstraints + ResolveOneofConstraints(desc protoreflect.OneofDescriptor) *validate.OneofConstraints + ResolveFieldConstraints(desc protoreflect.FieldDescriptor) *validate.FieldConstraints +} + // NewBuilder initializes a new Builder. func NewBuilder( env *cel.Env, disableLazy bool, - res resolver.StandardConstraintResolver, + res StandardConstraintResolver, seedDesc ...protoreflect.MessageDescriptor, ) *Builder { bldr := &Builder{ diff --git a/resolver/resolver.go b/resolver/resolver.go index bcb80ac..9a2cbc7 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -28,14 +28,11 @@ const ( previousExtensionIndex = "51071" ) -type StandardConstraintResolver interface { - ResolveMessageConstraints(desc protoreflect.MessageDescriptor) *validate.MessageConstraints - ResolveOneofConstraints(desc protoreflect.OneofDescriptor) *validate.OneofConstraints - ResolveFieldConstraints(desc protoreflect.FieldDescriptor) *validate.FieldConstraints -} - +// DefaultResolver resolves protovalidate constraints options from descriptors. type DefaultResolver struct{} +// ResolveMessageConstraints returns the MessageConstraints option set for the +// MessageDescriptor. func (r DefaultResolver) ResolveMessageConstraints(desc protoreflect.MessageDescriptor) *validate.MessageConstraints { constraints := resolveExt[protoreflect.MessageDescriptor, *validate.MessageConstraints](desc, validate.E_Message) if constraints == nil { @@ -44,6 +41,8 @@ func (r DefaultResolver) ResolveMessageConstraints(desc protoreflect.MessageDesc return constraints } +// ResolveOneofConstraints returns the OneofConstraints option set for the +// OneofDescriptor. func (r DefaultResolver) ResolveOneofConstraints(desc protoreflect.OneofDescriptor) *validate.OneofConstraints { constraints := resolveExt[protoreflect.OneofDescriptor, *validate.OneofConstraints](desc, validate.E_Oneof) if constraints == nil { @@ -52,6 +51,8 @@ func (r DefaultResolver) ResolveOneofConstraints(desc protoreflect.OneofDescript return constraints } +// ResolveFieldConstraints returns the FieldConstraints option set for the +// FieldDescriptor. func (r DefaultResolver) ResolveFieldConstraints(desc protoreflect.FieldDescriptor) *validate.FieldConstraints { constraints := resolveExt[protoreflect.FieldDescriptor, *validate.FieldConstraints](desc, validate.E_Field) if constraints == nil {