Skip to content

Commit

Permalink
Various small fixes
Browse files Browse the repository at this point in the history
1) Fix regression from
   #529. The
   code needs to handle enums at file scope and in a message. At the
   same time also handle arbitrary nesting.
2) Fix regression from #537
   when dealing with single element string oneofs (and possibly other
   types being returned as pointer types vs. value types and breaking in
   queries. This fixes that for both C++ and Go. It's possible this fix
   shoudl be in PG* but doing it here for simplicity.
3) Modify the C++ validator registry to have polymorphic lookup
   capability which I want to use in Envoy.

Signed-off-by: Matt Klein <mklein@lyft.com>
  • Loading branch information
Matt Klein committed Feb 26, 2022
1 parent 79071f0 commit 9a93c57
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 4 deletions.
3 changes: 2 additions & 1 deletion templates/cc/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ func (fns CCFuncs) cType(t pgs.FieldType) string {
return fns.cTypeOfString(fns.Type(t.Field()).Element().String())
}

return fns.cTypeOfString(fns.Type(t.Field()).String())
// Use Value() to strip any potential pointer type.
return fns.cTypeOfString(fns.Type(t.Field()).Value().String())
}

func (fns CCFuncs) cTypeOfString(s string) string {
Expand Down
2 changes: 1 addition & 1 deletion templates/go/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var (
_ = sort.Sort
{{ range $pkg, $path := enumPackages (externalEnums .) }}
_ = {{ $pkg }}.{{ (index (externalEnums $) 0).Parent.Name }}_{{ (index (externalEnums $) 0).Name }}(0)
_ = {{ $pkg }}.{{ enumName (index (externalEnums $) 0) }}(0)
{{ end }}
)
Expand Down
23 changes: 22 additions & 1 deletion templates/goshared/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func Register(tpl *template.Template, params pgs.Parameters) {
"typ": fns.Type,
"unwrap": fns.unwrap,
"externalEnums": fns.externalEnums,
"enumName": fns.enumName,
"enumPackages": fns.enumPackages,
})

Expand Down Expand Up @@ -197,6 +198,11 @@ func (fns goSharedFuncs) isBytes(f interface {
return f.ProtoType() == pgs.BytesT
}

func (fns goSharedFuncs) isMessage(i interface{}) bool {
_, ok := i.(pgs.Message)
return ok
}

func (fns goSharedFuncs) byteStr(x []byte) string {
elms := make([]string, len(x))
for i, b := range x {
Expand Down Expand Up @@ -228,7 +234,8 @@ func (fns goSharedFuncs) inType(f pgs.Field, x interface{}) string {
return fns.Type(f).String()
}
default:
return fns.Type(f).String()
// Use Value() to strip any potential pointer type.
return fns.Type(f).Value().String()
}
}

Expand Down Expand Up @@ -326,6 +333,20 @@ func (fns goSharedFuncs) externalEnums(file pgs.File) []pgs.Enum {
return out
}

func (fns goSharedFuncs) enumName(enum pgs.Enum) string {
out := string(enum.Name())
parent := enum.Parent()
for {
message, ok := parent.(pgs.Message)
if ok {
out = string(message.Name()) + "_" + out
parent = message.Parent()
} else {
return out
}
}
}

func (fns goSharedFuncs) enumPackages(enums []pgs.Enum) map[pgs.Name]pgs.FilePath {
out := make(map[pgs.Name]pgs.FilePath, len(enums))

Expand Down
1 change: 1 addition & 0 deletions tests/harness/cases/enums.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ message EnumNotIn { TestEnum val = 1 [(validate.rules).enum = { not_in: [1]
message EnumAliasNotIn { TestEnumAlias val = 1 [(validate.rules).enum = { not_in: [1]}]; }

message EnumExternal { other_package.Embed.Enumerated val = 1 [(validate.rules).enum.defined_only = true]; }
message EnumExternal2 { other_package.Embed.DoubleEmbed.DoubleEnumerated val = 1 [(validate.rules).enum.defined_only = true]; }

message RepeatedEnumDefined { repeated TestEnum val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }
message RepeatedExternalEnumDefined { repeated other_package.Embed.Enumerated val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }
Expand Down
4 changes: 4 additions & 0 deletions tests/harness/cases/other_package/embed.proto
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import "validate/validate.proto";

// Validate message embedding across packages.
message Embed {
message DoubleEmbed {
enum DoubleEnumerated { VALUE = 0; }
}

int64 val = 1 [(validate.rules).int64.gt = 0];

enum Enumerated { VALUE = 0; }
Expand Down
6 changes: 6 additions & 0 deletions tests/harness/cases/strings.proto
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,9 @@ message StringHttpHeaderName { string val = 1 [(validate.rules).string.well_know
message StringHttpHeaderValue { string val = 1 [(validate.rules).string.well_known_regex = HTTP_HEADER_VALUE]; }
message StringValidHeader { string val = 1 [(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE, strict: false}]; }
message StringUUIDIgnore { string val = 1 [(validate.rules).string = {uuid: true, ignore_empty: true}]; }

message StringInOneOf {
oneof foo {
string bar = 1 [(validate.rules).string = {in: "a" in: "b"}];
}
}
3 changes: 2 additions & 1 deletion validate/validate.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ class Validator : public BaseValidator {

static bool CheckMessage(const T& m, ValidationMsg* err)
{
auto val = static_cast<Validator<T>*>(validators()[std::type_index(typeid(T))]);
// Run typeid() on the variable vs. the type to allow polymorphic lookup of the validator.
auto val = static_cast<Validator<T>*>(validators()[std::type_index(typeid(m))]);
if (val) {
return val->check_(m, err);
}
Expand Down

0 comments on commit 9a93c57

Please sign in to comment.