From bdce4a7f96c8e50a16e32a65536eab8dc3eec1c8 Mon Sep 17 00:00:00 2001 From: Noel Cower Date: Thu, 23 Feb 2017 12:41:34 -0800 Subject: [PATCH 1/3] runtime: Fix typo in Error call --- runtime/query_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/query_test.go b/runtime/query_test.go index ce08976037b..1da5d215b57 100644 --- a/runtime/query_test.go +++ b/runtime/query_test.go @@ -136,7 +136,7 @@ func TestPopulateParameters(t *testing.T) { msg.Reset() err := runtime.PopulateQueryParameters(msg, spec.values, spec.filter) if err != nil { - t.Errorf("runtime.PoplateQueryParameters(msg, %v, %v) failed with %v; want success", spec.values, spec.filter, err) + t.Errorf("runtime.PopulateQueryParameters(msg, %v, %v) failed with %v; want success", spec.values, spec.filter, err) continue } if got, want := msg, spec.want; !proto.Equal(got, want) { From e98d5dbebe626ba8a38211afe9a8e989cebb3b73 Mon Sep 17 00:00:00 2001 From: Noel Cower Date: Thu, 23 Feb 2017 12:41:59 -0800 Subject: [PATCH 2/3] runtime: Make enum type distinct for query test This also means that the reflect-based name tests will now fail. This is intentional, we want to use the protobuf enum's name to look it up via proto.EnumValueMap, not the reflect-based type name, since that can vary. --- runtime/query_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/runtime/query_test.go b/runtime/query_test.go index 1da5d215b57..58cc3c5b669 100644 --- a/runtime/query_test.go +++ b/runtime/query_test.go @@ -241,8 +241,8 @@ type proto3Message struct { BoolValue bool `protobuf:"varint,8,opt,name=bool_value" json:"bool_value,omitempty"` StringValue string `protobuf:"bytes,9,opt,name=string_value" json:"string_value,omitempty"` RepeatedValue []string `protobuf:"bytes,10,rep,name=repeated_value" json:"repeated_value,omitempty"` - EnumValue EnumValue `protobuf:"varint,11,opt,name=enum_value,json=enumValue,enum=api.EnumValue" json:"enum_value,omitempty"` - RepeatedEnum []EnumValue `protobuf:"varint,12,rep,packed,name=repeated_enum,json=repeated_enum,enum=api.EnumValue" json:"repeated_enum,omitempty"` + EnumValue EnumValue `protobuf:"varint,11,opt,name=enum_value,json=enumValue,enum=runtime_test_api.EnumValue" json:"enum_value,omitempty"` + RepeatedEnum []EnumValue `protobuf:"varint,12,rep,packed,name=repeated_enum,json=repeated_enum,enum=runtime_test_api.EnumValue" json:"repeated_enum,omitempty"` TimestampValue *timestamp.Timestamp `protobuf:"bytes,11,opt,name=timestamp_value" json:"timestamp_value,omitempty"` } @@ -268,8 +268,8 @@ type proto2Message struct { BoolValue *bool `protobuf:"varint,8,opt,name=bool_value" json:"bool_value,omitempty"` StringValue *string `protobuf:"bytes,9,opt,name=string_value" json:"string_value,omitempty"` RepeatedValue []string `protobuf:"bytes,10,rep,name=repeated_value" json:"repeated_value,omitempty"` - EnumValue EnumValue `protobuf:"varint,11,opt,name=enum_value,json=enumValue,enum=api.EnumValue" json:"enum_value,omitempty"` - RepeatedEnum []EnumValue `protobuf:"varint,12,rep,packed,name=repeated_enum,json=repeated_enum,enum=api.EnumValue" json:"repeated_enum,omitempty"` + EnumValue EnumValue `protobuf:"varint,11,opt,name=enum_value,json=enumValue,enum=runtime_test_api.EnumValue" json:"enum_value,omitempty"` + RepeatedEnum []EnumValue `protobuf:"varint,12,rep,packed,name=repeated_enum,json=repeated_enum,enum=runtime_test_api.EnumValue" json:"repeated_enum,omitempty"` XXX_unrecognized []byte `json:"-"` } @@ -367,5 +367,5 @@ var EnumValue_value = map[string]int32{ } func init() { - proto.RegisterEnum("runtime_test.EnumValue", EnumValue_name, EnumValue_value) + proto.RegisterEnum("runtime_test_api.EnumValue", EnumValue_name, EnumValue_value) } From 479142a37e5f11a58ac18cf7a2c535a1af99bf63 Mon Sep 17 00:00:00 2001 From: Noel Cower Date: Thu, 23 Feb 2017 12:48:00 -0800 Subject: [PATCH 3/3] runtime: Use protobuf enum name instead of (reflect.Type).String 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. --- runtime/query.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/runtime/query.go b/runtime/query.go index fea54a1d5e2..85d0e23612a 100644 --- a/runtime/query.go +++ b/runtime/query.go @@ -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 @@ -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()) @@ -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) } @@ -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 @@ -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) }