From 5d49dae10dc156ca81211ddf8241d3cd0771c499 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Fri, 17 Nov 2023 19:21:59 -0500 Subject: [PATCH 1/4] move two files --- {internal/expression => celext}/lookups.go | 2 +- {internal/expression => celext}/lookups_test.go | 2 +- internal/constraints/cache.go | 3 ++- internal/constraints/lookups_test.go | 4 ++-- internal/evaluator/builder.go | 5 +++-- 5 files changed, 9 insertions(+), 7 deletions(-) rename {internal/expression => celext}/lookups.go (99%) rename {internal/expression => celext}/lookups_test.go (99%) diff --git a/internal/expression/lookups.go b/celext/lookups.go similarity index 99% rename from internal/expression/lookups.go rename to celext/lookups.go index 5c26695..39aa598 100644 --- a/internal/expression/lookups.go +++ b/celext/lookups.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package expression +package celext import ( "github.com/google/cel-go/cel" diff --git a/internal/expression/lookups_test.go b/celext/lookups_test.go similarity index 99% rename from internal/expression/lookups_test.go rename to celext/lookups_test.go index f9fe452..3301e70 100644 --- a/internal/expression/lookups_test.go +++ b/celext/lookups_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package expression +package celext import ( "testing" diff --git a/internal/constraints/cache.go b/internal/constraints/cache.go index 02a3b9e..7d07123 100644 --- a/internal/constraints/cache.go +++ b/internal/constraints/cache.go @@ -17,6 +17,7 @@ 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/google/cel-go/cel" @@ -114,7 +115,7 @@ func (c *Cache) prepareEnvironment( ) (*cel.Env, error) { env, err := env.Extend( cel.Types(rules.Interface()), - cel.Variable("this", expression.ProtoFieldToCELType(fieldDesc, true, forItems)), + cel.Variable("this", celext.ProtoFieldToCELType(fieldDesc, true, forItems)), cel.Variable("rules", cel.ObjectType(string(rules.Descriptor().FullName()))), ) diff --git a/internal/constraints/lookups_test.go b/internal/constraints/lookups_test.go index 152201d..c26df72 100644 --- a/internal/constraints/lookups_test.go +++ b/internal/constraints/lookups_test.go @@ -17,7 +17,7 @@ package constraints import ( "testing" - "github.com/bufbuild/protovalidate-go/internal/expression" + "github.com/bufbuild/protovalidate-go/celext" "github.com/google/cel-go/cel" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/proto" @@ -84,7 +84,7 @@ func TestProtoKindToCELType(t *testing.T) { kind, typ := k, ty t.Run(kind.String(), func(t *testing.T) { t.Parallel() - assert.Equal(t, typ, expression.ProtoKindToCELType(kind)) + assert.Equal(t, typ, celext.ProtoKindToCELType(kind)) }) } } diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index b525711..1057cf9 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" + "github.com/bufbuild/protovalidate-go/celext" "github.com/bufbuild/protovalidate-go/internal/constraints" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/expression" @@ -278,9 +279,9 @@ func (bldr *Builder) processFieldExpressions( return nil } - celTyp := expression.ProtoFieldToCELType(fieldDesc, false, false) + celTyp := celext.ProtoFieldToCELType(fieldDesc, false, false) opts := append( - expression.RequiredCELEnvOptions(fieldDesc), + celext.RequiredCELEnvOptions(fieldDesc), cel.Variable("this", celTyp), ) compiledExpressions, err := expression.Compile(exprs, bldr.env, opts...) From 852ae235425c43a84b890ea37b7b363b2f1caa88 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Wed, 20 Dec 2023 18:12:44 -0500 Subject: [PATCH 2/4] move back --- internal/constraints/cache.go | 3 +-- internal/constraints/lookups_test.go | 4 ++-- internal/evaluator/builder.go | 5 ++--- {celext => internal/expression}/lookups.go | 2 +- {celext => internal/expression}/lookups_test.go | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) rename {celext => internal/expression}/lookups.go (99%) rename {celext => internal/expression}/lookups_test.go (99%) diff --git a/internal/constraints/cache.go b/internal/constraints/cache.go index 7d07123..02a3b9e 100644 --- a/internal/constraints/cache.go +++ b/internal/constraints/cache.go @@ -17,7 +17,6 @@ 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/google/cel-go/cel" @@ -115,7 +114,7 @@ func (c *Cache) prepareEnvironment( ) (*cel.Env, error) { env, err := env.Extend( cel.Types(rules.Interface()), - cel.Variable("this", celext.ProtoFieldToCELType(fieldDesc, true, forItems)), + cel.Variable("this", expression.ProtoFieldToCELType(fieldDesc, true, forItems)), cel.Variable("rules", cel.ObjectType(string(rules.Descriptor().FullName()))), ) diff --git a/internal/constraints/lookups_test.go b/internal/constraints/lookups_test.go index c26df72..152201d 100644 --- a/internal/constraints/lookups_test.go +++ b/internal/constraints/lookups_test.go @@ -17,7 +17,7 @@ package constraints import ( "testing" - "github.com/bufbuild/protovalidate-go/celext" + "github.com/bufbuild/protovalidate-go/internal/expression" "github.com/google/cel-go/cel" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/proto" @@ -84,7 +84,7 @@ func TestProtoKindToCELType(t *testing.T) { kind, typ := k, ty t.Run(kind.String(), func(t *testing.T) { t.Parallel() - assert.Equal(t, typ, celext.ProtoKindToCELType(kind)) + assert.Equal(t, typ, expression.ProtoKindToCELType(kind)) }) } } diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index 1057cf9..b525711 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -19,7 +19,6 @@ import ( "sync/atomic" "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" - "github.com/bufbuild/protovalidate-go/celext" "github.com/bufbuild/protovalidate-go/internal/constraints" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/expression" @@ -279,9 +278,9 @@ func (bldr *Builder) processFieldExpressions( return nil } - celTyp := celext.ProtoFieldToCELType(fieldDesc, false, false) + celTyp := expression.ProtoFieldToCELType(fieldDesc, false, false) opts := append( - celext.RequiredCELEnvOptions(fieldDesc), + expression.RequiredCELEnvOptions(fieldDesc), cel.Variable("this", celTyp), ) compiledExpressions, err := expression.Compile(exprs, bldr.env, opts...) diff --git a/celext/lookups.go b/internal/expression/lookups.go similarity index 99% rename from celext/lookups.go rename to internal/expression/lookups.go index 39aa598..5c26695 100644 --- a/celext/lookups.go +++ b/internal/expression/lookups.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package celext +package expression import ( "github.com/google/cel-go/cel" diff --git a/celext/lookups_test.go b/internal/expression/lookups_test.go similarity index 99% rename from celext/lookups_test.go rename to internal/expression/lookups_test.go index 3301e70..f9fe452 100644 --- a/celext/lookups_test.go +++ b/internal/expression/lookups_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package celext +package expression import ( "testing" From fe09bc70c2714d4d54e5ea99e0265fa8c9f944be Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Wed, 20 Dec 2023 18:21:37 -0500 Subject: [PATCH 3/4] Revert "move back" This reverts commit 852ae235425c43a84b890ea37b7b363b2f1caa88. --- {internal/expression => celext}/lookups.go | 2 +- {internal/expression => celext}/lookups_test.go | 2 +- internal/constraints/cache.go | 3 ++- internal/constraints/lookups_test.go | 4 ++-- internal/evaluator/builder.go | 5 +++-- 5 files changed, 9 insertions(+), 7 deletions(-) rename {internal/expression => celext}/lookups.go (99%) rename {internal/expression => celext}/lookups_test.go (99%) diff --git a/internal/expression/lookups.go b/celext/lookups.go similarity index 99% rename from internal/expression/lookups.go rename to celext/lookups.go index 5c26695..39aa598 100644 --- a/internal/expression/lookups.go +++ b/celext/lookups.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package expression +package celext import ( "github.com/google/cel-go/cel" diff --git a/internal/expression/lookups_test.go b/celext/lookups_test.go similarity index 99% rename from internal/expression/lookups_test.go rename to celext/lookups_test.go index f9fe452..3301e70 100644 --- a/internal/expression/lookups_test.go +++ b/celext/lookups_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package expression +package celext import ( "testing" diff --git a/internal/constraints/cache.go b/internal/constraints/cache.go index 02a3b9e..7d07123 100644 --- a/internal/constraints/cache.go +++ b/internal/constraints/cache.go @@ -17,6 +17,7 @@ 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/google/cel-go/cel" @@ -114,7 +115,7 @@ func (c *Cache) prepareEnvironment( ) (*cel.Env, error) { env, err := env.Extend( cel.Types(rules.Interface()), - cel.Variable("this", expression.ProtoFieldToCELType(fieldDesc, true, forItems)), + cel.Variable("this", celext.ProtoFieldToCELType(fieldDesc, true, forItems)), cel.Variable("rules", cel.ObjectType(string(rules.Descriptor().FullName()))), ) diff --git a/internal/constraints/lookups_test.go b/internal/constraints/lookups_test.go index 152201d..c26df72 100644 --- a/internal/constraints/lookups_test.go +++ b/internal/constraints/lookups_test.go @@ -17,7 +17,7 @@ package constraints import ( "testing" - "github.com/bufbuild/protovalidate-go/internal/expression" + "github.com/bufbuild/protovalidate-go/celext" "github.com/google/cel-go/cel" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/proto" @@ -84,7 +84,7 @@ func TestProtoKindToCELType(t *testing.T) { kind, typ := k, ty t.Run(kind.String(), func(t *testing.T) { t.Parallel() - assert.Equal(t, typ, expression.ProtoKindToCELType(kind)) + assert.Equal(t, typ, celext.ProtoKindToCELType(kind)) }) } } diff --git a/internal/evaluator/builder.go b/internal/evaluator/builder.go index b525711..1057cf9 100644 --- a/internal/evaluator/builder.go +++ b/internal/evaluator/builder.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go/buf/validate" + "github.com/bufbuild/protovalidate-go/celext" "github.com/bufbuild/protovalidate-go/internal/constraints" "github.com/bufbuild/protovalidate-go/internal/errors" "github.com/bufbuild/protovalidate-go/internal/expression" @@ -278,9 +279,9 @@ func (bldr *Builder) processFieldExpressions( return nil } - celTyp := expression.ProtoFieldToCELType(fieldDesc, false, false) + celTyp := celext.ProtoFieldToCELType(fieldDesc, false, false) opts := append( - expression.RequiredCELEnvOptions(fieldDesc), + celext.RequiredCELEnvOptions(fieldDesc), cel.Variable("this", celTyp), ) compiledExpressions, err := expression.Compile(exprs, bldr.env, opts...) From e08ddf4c305c15ae6ee43a417083dfb14de98924 Mon Sep 17 00:00:00 2001 From: Oliver Sun Date: Wed, 20 Dec 2023 18:27:02 -0500 Subject: [PATCH 4/4] move --- celext/lib.go | 19 ++++++ celext/lookups.go | 88 +++++++++++----------------- celext/lookups_test.go | 34 +++++++++++ internal/constraints/lookups_test.go | 36 ------------ 4 files changed, 88 insertions(+), 89 deletions(-) diff --git a/celext/lib.go b/celext/lib.go index 444f09b..63dfe8e 100644 --- a/celext/lib.go +++ b/celext/lib.go @@ -29,7 +29,9 @@ import ( "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/traits" "github.com/google/cel-go/ext" + "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" + "google.golang.org/protobuf/types/dynamicpb" ) // DefaultEnv produces a cel.Env with the necessary cel.EnvOption and @@ -49,6 +51,23 @@ func DefaultEnv(useUTC bool) (*cel.Env, error) { ) } +// RequiredCELEnvOptions returns the options required to have expressions which +// rely on the provided descriptor. +func RequiredCELEnvOptions(fieldDesc protoreflect.FieldDescriptor) []cel.EnvOption { + if fieldDesc.IsMap() { + return append( + RequiredCELEnvOptions(fieldDesc.MapKey()), + RequiredCELEnvOptions(fieldDesc.MapValue())..., + ) + } + if fieldDesc.Kind() == protoreflect.MessageKind { + return []cel.EnvOption{ + cel.Types(dynamicpb.NewMessage(fieldDesc.Message())), + } + } + return nil +} + // lib is the collection of functions and settings required by protovalidate // beyond the standard definitions of the CEL Specification: // diff --git a/celext/lookups.go b/celext/lookups.go index 39aa598..cb24d70 100644 --- a/celext/lookups.go +++ b/celext/lookups.go @@ -17,46 +17,8 @@ package celext import ( "github.com/google/cel-go/cel" "google.golang.org/protobuf/reflect/protoreflect" - "google.golang.org/protobuf/types/dynamicpb" ) -// ProtoKindToCELType maps a protoreflect.Kind to a compatible cel.Type. -func ProtoKindToCELType(kind protoreflect.Kind) *cel.Type { - switch kind { - case - protoreflect.FloatKind, - protoreflect.DoubleKind: - return cel.DoubleType - case - protoreflect.Int32Kind, - protoreflect.Int64Kind, - protoreflect.Sint32Kind, - protoreflect.Sint64Kind, - protoreflect.Sfixed32Kind, - protoreflect.Sfixed64Kind, - protoreflect.EnumKind: - return cel.IntType - case - protoreflect.Uint32Kind, - protoreflect.Uint64Kind, - protoreflect.Fixed32Kind, - protoreflect.Fixed64Kind: - return cel.UintType - case protoreflect.BoolKind: - return cel.BoolType - case protoreflect.StringKind: - return cel.StringType - case protoreflect.BytesKind: - return cel.BytesType - case - protoreflect.MessageKind, - protoreflect.GroupKind: - return cel.DynType - default: - return cel.DynType - } -} - // ProtoFieldToCELType resolves the CEL value type for the provided // FieldDescriptor. If generic is true, the specific subtypes of map and // repeated fields will be replaced with cel.DynType. If forItems is true, the @@ -92,22 +54,42 @@ func ProtoFieldToCELType(fieldDesc protoreflect.FieldDescriptor, generic, forIte return cel.ObjectType(string(fqn)) } } - return ProtoKindToCELType(fieldDesc.Kind()) + return protoKindToCELType(fieldDesc.Kind()) } -// RequiredCELEnvOptions returns the options required to have expressions which -// rely on the provided descriptor. -func RequiredCELEnvOptions(fieldDesc protoreflect.FieldDescriptor) []cel.EnvOption { - if fieldDesc.IsMap() { - return append( - RequiredCELEnvOptions(fieldDesc.MapKey()), - RequiredCELEnvOptions(fieldDesc.MapValue())..., - ) - } - if fieldDesc.Kind() == protoreflect.MessageKind { - return []cel.EnvOption{ - cel.Types(dynamicpb.NewMessage(fieldDesc.Message())), - } +// protoKindToCELType maps a protoreflect.Kind to a compatible cel.Type. +func protoKindToCELType(kind protoreflect.Kind) *cel.Type { + switch kind { + case + protoreflect.FloatKind, + protoreflect.DoubleKind: + return cel.DoubleType + case + protoreflect.Int32Kind, + protoreflect.Int64Kind, + protoreflect.Sint32Kind, + protoreflect.Sint64Kind, + protoreflect.Sfixed32Kind, + protoreflect.Sfixed64Kind, + protoreflect.EnumKind: + return cel.IntType + case + protoreflect.Uint32Kind, + protoreflect.Uint64Kind, + protoreflect.Fixed32Kind, + protoreflect.Fixed64Kind: + return cel.UintType + case protoreflect.BoolKind: + return cel.BoolType + case protoreflect.StringKind: + return cel.StringType + case protoreflect.BytesKind: + return cel.BytesType + case + protoreflect.MessageKind, + protoreflect.GroupKind: + return cel.DynType + default: + return cel.DynType } - return nil } diff --git a/celext/lookups_test.go b/celext/lookups_test.go index 3301e70..8ee382f 100644 --- a/celext/lookups_test.go +++ b/celext/lookups_test.go @@ -89,6 +89,40 @@ func TestCache_GetCELType(t *testing.T) { } } +func TestProtoKindToCELType(t *testing.T) { + t.Parallel() + + tests := map[protoreflect.Kind]*cel.Type{ + protoreflect.FloatKind: cel.DoubleType, + protoreflect.DoubleKind: cel.DoubleType, + protoreflect.Int32Kind: cel.IntType, + protoreflect.Int64Kind: cel.IntType, + protoreflect.Uint32Kind: cel.UintType, + protoreflect.Uint64Kind: cel.UintType, + protoreflect.Sint32Kind: cel.IntType, + protoreflect.Sint64Kind: cel.IntType, + protoreflect.Fixed32Kind: cel.UintType, + protoreflect.Fixed64Kind: cel.UintType, + protoreflect.Sfixed32Kind: cel.IntType, + protoreflect.Sfixed64Kind: cel.IntType, + protoreflect.BoolKind: cel.BoolType, + protoreflect.StringKind: cel.StringType, + protoreflect.BytesKind: cel.BytesType, + protoreflect.EnumKind: cel.IntType, + protoreflect.MessageKind: cel.DynType, + protoreflect.GroupKind: cel.DynType, + protoreflect.Kind(0): cel.DynType, + } + + for k, ty := range tests { + kind, typ := k, ty + t.Run(kind.String(), func(t *testing.T) { + t.Parallel() + assert.Equal(t, typ, protoKindToCELType(kind)) + }) + } +} + func getFieldDesc(t *testing.T, msg proto.Message, fld protoreflect.Name) protoreflect.FieldDescriptor { t.Helper() desc := msg.ProtoReflect().Descriptor().Fields().ByName(fld) diff --git a/internal/constraints/lookups_test.go b/internal/constraints/lookups_test.go index c26df72..6fa9423 100644 --- a/internal/constraints/lookups_test.go +++ b/internal/constraints/lookups_test.go @@ -17,8 +17,6 @@ package constraints import ( "testing" - "github.com/bufbuild/protovalidate-go/celext" - "github.com/google/cel-go/cel" "github.com/stretchr/testify/assert" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" @@ -54,37 +52,3 @@ func TestExpectedWrapperConstraints(t *testing.T) { }) } } - -func TestProtoKindToCELType(t *testing.T) { - t.Parallel() - - tests := map[protoreflect.Kind]*cel.Type{ - protoreflect.FloatKind: cel.DoubleType, - protoreflect.DoubleKind: cel.DoubleType, - protoreflect.Int32Kind: cel.IntType, - protoreflect.Int64Kind: cel.IntType, - protoreflect.Uint32Kind: cel.UintType, - protoreflect.Uint64Kind: cel.UintType, - protoreflect.Sint32Kind: cel.IntType, - protoreflect.Sint64Kind: cel.IntType, - protoreflect.Fixed32Kind: cel.UintType, - protoreflect.Fixed64Kind: cel.UintType, - protoreflect.Sfixed32Kind: cel.IntType, - protoreflect.Sfixed64Kind: cel.IntType, - protoreflect.BoolKind: cel.BoolType, - protoreflect.StringKind: cel.StringType, - protoreflect.BytesKind: cel.BytesType, - protoreflect.EnumKind: cel.IntType, - protoreflect.MessageKind: cel.DynType, - protoreflect.GroupKind: cel.DynType, - protoreflect.Kind(0): cel.DynType, - } - - for k, ty := range tests { - kind, typ := k, ty - t.Run(kind.String(), func(t *testing.T) { - t.Parallel() - assert.Equal(t, typ, celext.ProtoKindToCELType(kind)) - }) - } -}