Skip to content

Commit

Permalink
chore(internal/protoveneer): support oneof fields (googleapis#10271)
Browse files Browse the repository at this point in the history
Proto oneofs translate into Go as an unexported type.
Here is one example:

  type CachedContent struct {
  	// Types that are assignable to Expiration:
  	//	*CachedContent_ExpireTime
  	//	*CachedContent_Ttl
  	Expiration isCachedContent_Expiration `protobuf_oneof:"expiration"`
    ...
  }

As the comment says, the `Expiration` field can be populated by one
of two exported types, but the type of the field itself is not exported
(it is an interface type that the two exported types satisfy).

Oneof fields like this cause a problem for this code generator, which
writes conversion code between veneers and protos that looks like

  return &pb.CachedContent{
    Expiration: conversionFunction(...),
    ...
  }

We cannot write conversion function because we cannot write
its return type.

This CL addresses the problem by adding two features. First,
the user can configure population functions that are passed the
two structs, proto and veneer. That makes it possible to set
oneof fields:

  func popcc(p *pb.CachedContent, v *CachedContent) {
    if [something about v] {
      p.Expiration = &pb.CachedContent_ExpireTime{..}
    } else {
      p.Expiration = &pb.CachedkContent_Ttl{...}
    }
  }

The second feature is the `noConvert` option, which prevents the field
from being set on the struct literal.

So by setting `noConvert` and specifying populate functions,
A oneof field can be initialized from a corresponding veneer type.
  • Loading branch information
jba authored May 29, 2024
1 parent 0dee490 commit 2260f47
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 10 deletions.
5 changes: 5 additions & 0 deletions internal/protoveneer/cmd/protoveneer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type typeConfig struct {
Fields map[string]fieldConfig
// Custom conversion functions: "tofunc, fromfunc"
ConvertToFrom string `yaml:"convertToFrom"`
// Custom population functions, that are called after field-by-field conversion: "tofunc, fromfunc"
PopulateToFrom string `yaml:"populateToFrom"`
// Doc string for the type, omitting the initial type name.
// This replaces the first line of the doc.
Doc string
Expand All @@ -69,6 +71,9 @@ type fieldConfig struct {
Type string // veneer type
// Omit from output.
Omit bool
// Generate the type, but not conversions.
// The populate functions (see [typeConfg.PopulateToFrom]) should set the field.
NoConvert bool `yaml:"noConvert"`
// Custom conversion functions: "tofunc, fromfunc"
ConvertToFrom string `yaml:"convertToFrom"`
}
Expand Down
59 changes: 49 additions & 10 deletions internal/protoveneer/cmd/protoveneer/protoveneer.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func run(ctx context.Context, configFile, pbDir, outDir string) error {
if !*noFormat {
src, err = format.Source(src)
if err != nil {
return err
return fmt.Errorf("formatting: %v", err)
}
}

Expand Down Expand Up @@ -284,15 +284,19 @@ func buildConverterMap(typeInfos []*typeInfo, conf *config) (map[string]converte
}

func parseCustomConverter(name, value string) (converter, error) {
toFunc, fromFunc, ok := strings.Cut(value, ",")
toFunc = strings.TrimSpace(toFunc)
fromFunc = strings.TrimSpace(fromFunc)
if !ok || toFunc == "" || fromFunc == "" {
toFunc, fromFunc := parseCommaPair(value)
if toFunc == "" || fromFunc == "" {
return nil, fmt.Errorf(`%s: ConvertToFrom = %q, want "toFunc, fromFunc"`, name, value)
}
return customConverter{toFunc, fromFunc}, nil
}

// parseCommaPair parses a string like "foo, bar" into "foo" and "bar".
func parseCommaPair(s string) (string, string) {
a, b, _ := strings.Cut(s, ",")
return strings.TrimSpace(a), strings.TrimSpace(b)
}

// makeConverter constructs a converter for the given type. Not every type is in the map: this
// function puts together converters for types like pointers, slices and maps, as well as
// named types.
Expand Down Expand Up @@ -340,9 +344,11 @@ type typeInfo struct {
values *ast.GenDecl // the list of values for an enum

// These fields are added later.
veneerName string // may be provided by config; else same as protoName
fields []*fieldInfo // for structs
valueNames []string // to generate String functions
veneerName string // may be provided by config; else same as protoName
fields []*fieldInfo // for structs
valueNames []string // to generate String functions
populateFrom string // name of function doing additional work converting from proto
populateTo string // name of function doing additional work converting to proto
}

// A fieldInfo holds information about a struct field.
Expand All @@ -351,6 +357,7 @@ type fieldInfo struct {
af *ast.Field
protoName, veneerName string
converter converter
noConvert bool
}

// collectDecls collects declaration information from a package.
Expand Down Expand Up @@ -445,6 +452,15 @@ func processType(ti *typeInfo, tconf *typeConfig, typeInfos map[string]*typeInfo
ti.fields = append(ti.fields, fi)
}
}
// Other processing.
if tconf != nil && tconf.PopulateToFrom != "" {
toFunc, fromFunc := parseCommaPair(tconf.PopulateToFrom)
if toFunc == "" || fromFunc == "" {
return fmt.Errorf(`%s: PopulateToFrom = %q, want "toFunc, fromFunc"`, ti.protoName, tconf.PopulateToFrom)
}
ti.populateTo = toFunc
ti.populateFrom = fromFunc
}
case *ast.Ident:
// Enum type. Nothing else to do with the type itself; but see processEnumValues.
default:
Expand Down Expand Up @@ -492,6 +508,7 @@ func processField(af *ast.Field, tc *typeConfig, typeInfos map[string]*typeInfo)
}
fi.converter = c
}
fi.noConvert = fc.NoConvert
}
}
af.Type = veneerType(af.Type, typeInfos)
Expand Down Expand Up @@ -756,22 +773,44 @@ func (ti *typeInfo) generateConversionMethods(pr func(string, ...any)) {
func (ti *typeInfo) generateToProto(pr func(string, ...any)) {
pr("func (v *%s) toProto() *pb.%s {\n", ti.veneerName, ti.protoName)
pr(" if v == nil { return nil }\n")
pr(" return &pb.%s{\n", ti.protoName)
if ti.populateTo == "" {
pr(" return &pb.%s{\n", ti.protoName)
} else {
pr(" p := &pb.%s{\n", ti.protoName)
}
for _, f := range ti.fields {
if f.noConvert {
continue
}
pr(" %s: %s,\n", f.protoName, f.converter.genTo("v."+f.veneerName))
}
pr(" }\n")
if ti.populateTo != "" {
pr(" %s(p, v)\n", ti.populateTo)
pr(" return p\n")
}
pr("}\n")
}

func (ti *typeInfo) generateFromProto(pr func(string, ...any)) {
pr("func (%s) fromProto(p *pb.%s) *%[1]s {\n", ti.veneerName, ti.protoName)
pr(" if p == nil { return nil }\n")
pr(" return &%s{\n", ti.veneerName)
if ti.populateFrom == "" {
pr(" return &%s{\n", ti.veneerName)
} else {
pr(" v := &%s{\n", ti.veneerName)
}
for _, f := range ti.fields {
if f.noConvert {
continue
}
pr(" %s: %s,\n", f.veneerName, f.converter.genFrom("p."+f.protoName))
}
pr(" }\n")
if ti.populateFrom != "" {
pr(" %s(v, p)\n", ti.populateFrom)
pr(" return v\n")
}
pr("}\n")
}

Expand Down
10 changes: 10 additions & 0 deletions internal/protoveneer/cmd/protoveneer/testdata/basic/basic.pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,13 @@ type Citation struct {
Struct *structpb.Struct
CreateTime *timestamppb.Timestamp
}

type unexported interface{ u() }

// This demonstrates using population functions to deal with
// proto oneof field, which has an unexported type.
// That can be a way to deal with proto oneofs.
type Pop struct {
X int
Y unexported
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ types:
fields:
Uri:
name: URI

Pop:
fields:
Y:
noConvert: true
populateToFrom: popYTo, popYFrom
30 changes: 30 additions & 0 deletions internal/protoveneer/cmd/protoveneer/testdata/basic/golden
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,33 @@ func (v HarmCategory) String() string {
}
return fmt.Sprintf("HarmCategory(%d)", v)
}

// Pop is this demonstrates using population functions to deal with
// proto oneof field, which has an unexported type.
// That can be a way to deal with proto oneofs.
type Pop struct {
X int
Y unexported
}

func (v *Pop) toProto() *pb.Pop {
if v == nil {
return nil
}
p := &pb.Pop{
X: v.X,
}
popYTo(p, v)
return p
}

func (Pop) fromProto(p *pb.Pop) *Pop {
if p == nil {
return nil
}
v := &Pop{
X: p.X,
}
popYFrom(v, p)
return v
}

0 comments on commit 2260f47

Please sign in to comment.