Skip to content

Commit

Permalink
runtime: Use protobuf enum name instead of (reflect.Type).String
Browse files Browse the repository at this point in the history
This adds pass-through of the *proto.Properties for fields so that the
fields being populated can be addressed by their enum name when the
type is an enum (i.e., (*proto.Properties).Enum is set).

As a result of this, the tests now pass, using the protobuf enum name
as registered (i.e., fully qualified package name) instead of the Go
package name derived by calling (reflect.Type).String. This should have
no real impact on existing use, but it may be justified to re-add
a fallback case for that reflect.Type name for hand-written protobuf
types. Even then, however, it seems unlikely that it should be expected
for enums-by-name to work when registering the enum map under a name
other than the one it's referred to in protobuf.
  • Loading branch information
nilium committed Feb 23, 2017
1 parent 8303460 commit 4d319d2
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions runtime/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values []
if m.Kind() != reflect.Ptr {
return fmt.Errorf("unexpected type %T: %v", msg, msg)
}
var props *proto.Properties
m = m.Elem()
for i, fieldName := range fieldPath {
isLast := i == len(fieldPath)-1
if !isLast && m.Kind() != reflect.Struct {
return fmt.Errorf("non-aggregate type in the mid of path: %s", strings.Join(fieldPath, "."))
}
f := fieldByProtoName(m, fieldName)
var f reflect.Value
f, props = fieldByProtoName(m, fieldName)
if !f.IsValid() {
grpclog.Printf("field not found in %T: %s", msg, strings.Join(fieldPath, "."))
return nil
Expand All @@ -60,7 +62,7 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values []
if !isLast {
return fmt.Errorf("unexpected repeated field in %s", strings.Join(fieldPath, "."))
}
return populateRepeatedField(f, values)
return populateRepeatedField(f, values, props)
case reflect.Ptr:
if f.IsNil() {
m = reflect.New(f.Type().Elem())
Expand All @@ -82,26 +84,26 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values []
default:
grpclog.Printf("too many field values: %s", strings.Join(fieldPath, "."))
}
return populateField(m, values[0])
return populateField(m, values[0], props)
}

// fieldByProtoName looks up a field whose corresponding protobuf field name is "name".
// "m" must be a struct value. It returns zero reflect.Value if no such field found.
func fieldByProtoName(m reflect.Value, name string) reflect.Value {
func fieldByProtoName(m reflect.Value, name string) (reflect.Value, *proto.Properties) {
props := proto.GetProperties(m.Type())
for _, p := range props.Prop {
if p.OrigName == name {
return m.FieldByName(p.Name)
return m.FieldByName(p.Name), p
}
}
return reflect.Value{}
return reflect.Value{}, nil
}

func populateRepeatedField(f reflect.Value, values []string) error {
func populateRepeatedField(f reflect.Value, values []string, props *proto.Properties) error {
elemType := f.Type().Elem()

// is the destination field a slice of an enumeration type?
if enumValMap := proto.EnumValueMap(elemType.String()); enumValMap != nil {
if enumValMap := proto.EnumValueMap(props.Enum); enumValMap != nil {
return populateFieldEnumRepeated(f, values, enumValMap)
}

Expand All @@ -120,7 +122,7 @@ func populateRepeatedField(f reflect.Value, values []string) error {
return nil
}

func populateField(f reflect.Value, value string) error {
func populateField(f reflect.Value, value string, props *proto.Properties) error {
// Handle well known type
type wkt interface {
XXX_WellKnownType() string
Expand All @@ -145,7 +147,7 @@ func populateField(f reflect.Value, value string) error {
}

// is the destination field an enumeration type?
if enumValMap := proto.EnumValueMap(f.Type().String()); enumValMap != nil {
if enumValMap := proto.EnumValueMap(props.Enum); enumValMap != nil {
return populateFieldEnum(f, value, enumValMap)
}

Expand Down

0 comments on commit 4d319d2

Please sign in to comment.