From 1abcd3a1d122862f27776ed97ca70f7ee7635064 Mon Sep 17 00:00:00 2001 From: adamstruck Date: Mon, 29 Jan 2018 14:19:43 -0800 Subject: [PATCH 1/2] support for maps in query string of the form ?filters[key]=value --- runtime/query.go | 50 +++++++++++++++++++ runtime/query_test.go | 108 ++++++++++++++++++++++++++++++++---------- 2 files changed, 134 insertions(+), 24 deletions(-) diff --git a/runtime/query.go b/runtime/query.go index a71ffe13847..916a754a643 100644 --- a/runtime/query.go +++ b/runtime/query.go @@ -5,6 +5,7 @@ import ( "fmt" "net/url" "reflect" + "regexp" "strconv" "strings" "time" @@ -18,6 +19,15 @@ import ( // A value is ignored if its key starts with one of the elements in "filter". func PopulateQueryParameters(msg proto.Message, values url.Values, filter *utilities.DoubleArray) error { for key, values := range values { + re, err := regexp.Compile("^(.*)\\[(.*)\\]$") + if err != nil { + return err + } + match := re.FindStringSubmatch(key) + if len(match) == 3 { + key = match[1] + values = append([]string{match[2]}, values...) + } fieldPath := strings.Split(key, ".") if filter.HasCommonPrefix(fieldPath) { continue @@ -84,6 +94,11 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values [] case reflect.Struct: m = f continue + case reflect.Map: + if !isLast { + return fmt.Errorf("unexpected map field in %s", strings.Join(fieldPath, ".")) + } + return populateMapField(f, values, props) default: return fmt.Errorf("unexpected type %s in %T", f.Type(), msg) } @@ -125,6 +140,41 @@ func fieldByProtoName(m reflect.Value, name string) (reflect.Value, *proto.Prope return reflect.Value{}, nil, nil } +func populateMapField(f reflect.Value, values []string, props *proto.Properties) error { + if len(values) != 2 { + return fmt.Errorf("more than one value provided for key '%s' in map '%s'", values[0], props.Name) + } + + key, value := values[0], values[1] + keyType := f.Type().Key() + valueType := f.Type().Elem() + if f.IsNil() { + f.Set(reflect.MakeMap(f.Type())) + } + + keyConv, ok := convFromType[keyType.Kind()] + if !ok { + return fmt.Errorf("unsupported key type %s in map %s", keyType, props.Name) + } + valueConv, ok := convFromType[valueType.Kind()] + if !ok { + return fmt.Errorf("unsupported value type %s in map %s", valueType, props.Name) + } + + keyV := keyConv.Call([]reflect.Value{reflect.ValueOf(key)}) + if err := keyV[1].Interface(); err != nil { + return err.(error) + } + valueV := valueConv.Call([]reflect.Value{reflect.ValueOf(value)}) + if err := valueV[1].Interface(); err != nil { + return err.(error) + } + + f.SetMapIndex(keyV[0].Convert(keyType), valueV[0].Convert(valueType)) + + return nil +} + func populateRepeatedField(f reflect.Value, values []string, props *proto.Properties) error { elemType := f.Type().Elem() diff --git a/runtime/query_test.go b/runtime/query_test.go index 8d5ac5f7040..ae5d20f124d 100644 --- a/runtime/query_test.go +++ b/runtime/query_test.go @@ -6,7 +6,6 @@ import ( "net/url" "reflect" "testing" - "time" "github.com/golang/protobuf/proto" @@ -37,29 +36,48 @@ func TestPopulateParameters(t *testing.T) { }{ { values: url.Values{ - "float_value": {"1.5"}, - "double_value": {"2.5"}, - "int64_value": {"-1"}, - "int32_value": {"-2"}, - "uint64_value": {"3"}, - "uint32_value": {"4"}, - "bool_value": {"true"}, - "string_value": {"str"}, - "bytes_value": {"Ynl0ZXM"}, - "repeated_value": {"a", "b", "c"}, - "enum_value": {"1"}, - "repeated_enum": {"1", "2", "0"}, - "timestamp_value": {timeStr}, - "fieldmask_value": {fieldmaskStr}, - "wrapper_float_value": {"1.5"}, - "wrapper_double_value": {"2.5"}, - "wrapper_int64_value": {"-1"}, - "wrapper_int32_value": {"-2"}, - "wrapper_u_int64_value": {"3"}, - "wrapper_u_int32_value": {"4"}, - "wrapper_bool_value": {"true"}, - "wrapper_string_value": {"str"}, - "wrapper_bytes_value": {"Ynl0ZXM"}, + "float_value": {"1.5"}, + "double_value": {"2.5"}, + "int64_value": {"-1"}, + "int32_value": {"-2"}, + "uint64_value": {"3"}, + "uint32_value": {"4"}, + "bool_value": {"true"}, + "string_value": {"str"}, + "bytes_value": {"Ynl0ZXM"}, + "repeated_value": {"a", "b", "c"}, + "enum_value": {"1"}, + "repeated_enum": {"1", "2", "0"}, + "timestamp_value": {timeStr}, + "fieldmask_value": {fieldmaskStr}, + "wrapper_float_value": {"1.5"}, + "wrapper_double_value": {"2.5"}, + "wrapper_int64_value": {"-1"}, + "wrapper_int32_value": {"-2"}, + "wrapper_u_int64_value": {"3"}, + "wrapper_u_int32_value": {"4"}, + "wrapper_bool_value": {"true"}, + "wrapper_string_value": {"str"}, + "wrapper_bytes_value": {"Ynl0ZXM"}, + "map_value[key]": {"value"}, + "map_value[second]": {"bar"}, + "map_value[third]": {"zzz"}, + "map_value[fourth]": {""}, + `map_value[~!@#$%^&*()]`: {"value"}, + "map_value2[key]": {"-2"}, + "map_value3[-2]": {"value"}, + "map_value4[key]": {"-1"}, + "map_value5[-1]": {"value"}, + "map_value6[key]": {"3"}, + "map_value7[3]": {"value"}, + "map_value8[key]": {"4"}, + "map_value9[4]": {"value"}, + "map_value10[key]": {"1.5"}, + "map_value11[1.5]": {"value"}, + "map_value12[key]": {"2.5"}, + "map_value13[2.5]": {"value"}, + "map_value14[key]": {"true"}, + "map_value15[true]": {"value"}, }, filter: utilities.NewDoubleArray(nil), want: &proto3Message{ @@ -86,6 +104,27 @@ func TestPopulateParameters(t *testing.T) { WrapperBoolValue: &wrappers.BoolValue{true}, WrapperStringValue: &wrappers.StringValue{"str"}, WrapperBytesValue: &wrappers.BytesValue{[]byte("bytes")}, + MapValue: map[string]string{ + "key": "value", + "second": "bar", + "third": "zzz", + "fourth": "", + `~!@#$%^&*()`: "value", + }, + MapValue2: map[string]int32{"key": -2}, + MapValue3: map[int32]string{-2: "value"}, + MapValue4: map[string]int64{"key": -1}, + MapValue5: map[int64]string{-1: "value"}, + MapValue6: map[string]uint32{"key": 3}, + MapValue7: map[uint32]string{3: "value"}, + MapValue8: map[string]uint64{"key": 4}, + MapValue9: map[uint64]string{4: "value"}, + MapValue10: map[string]float32{"key": 1.5}, + MapValue11: map[float32]string{1.5: "value"}, + MapValue12: map[string]float64{"key": 2.5}, + MapValue13: map[float64]string{2.5: "value"}, + MapValue14: map[string]bool{"key": true}, + MapValue15: map[bool]string{true: "value"}, }, }, { @@ -217,11 +256,17 @@ func TestPopulateParameters(t *testing.T) { "nested.nested.string_value": {"t"}, "nested.string_value": {"u"}, "nested_non_null.string_value": {"v"}, + "nested.nested.map_value[first]": {"foo"}, + "nested.nested.map_value[second]": {"bar"}, }, filter: utilities.NewDoubleArray(nil), want: &proto3Message{ Nested: &proto2Message{ Nested: &proto3Message{ + MapValue: map[string]string{ + "first": "foo", + "second": "bar", + }, Nested: &proto2Message{ RepeatedValue: []string{"a", "b", "c"}, StringValue: proto.String("s"), @@ -503,6 +548,21 @@ type proto3Message struct { WrapperBoolValue *wrappers.BoolValue `protobuf:"bytes,23,opt,name=wrapper_bool_value,json=wrapperBoolValue" json:"wrapper_bool_value,omitempty"` WrapperStringValue *wrappers.StringValue `protobuf:"bytes,24,opt,name=wrapper_string_value,json=wrapperStringValue" json:"wrapper_string_value,omitempty"` WrapperBytesValue *wrappers.BytesValue `protobuf:"bytes,26,opt,name=wrapper_bytes_value,json=wrapperBytesValue" json:"wrapper_bytes_value,omitempty"` + MapValue map[string]string `protobuf:"bytes,27,opt,name=map_value,json=mapValue" json:"map_value,omitempty"` + MapValue2 map[string]int32 `protobuf:"bytes,28,opt,name=map_value2,json=mapValue2" json:"map_value2,omitempty"` + MapValue3 map[int32]string `protobuf:"bytes,29,opt,name=map_value3,json=mapValue3" json:"map_value3,omitempty"` + MapValue4 map[string]int64 `protobuf:"bytes,30,opt,name=map_value4,json=mapValue4" json:"map_value4,omitempty"` + MapValue5 map[int64]string `protobuf:"bytes,31,opt,name=map_value5,json=mapValue5" json:"map_value5,omitempty"` + MapValue6 map[string]uint32 `protobuf:"bytes,32,opt,name=map_value6,json=mapValue6" json:"map_value6,omitempty"` + MapValue7 map[uint32]string `protobuf:"bytes,33,opt,name=map_value7,json=mapValue7" json:"map_value7,omitempty"` + MapValue8 map[string]uint64 `protobuf:"bytes,34,opt,name=map_value8,json=mapValue8" json:"map_value8,omitempty"` + MapValue9 map[uint64]string `protobuf:"bytes,35,opt,name=map_value9,json=mapValue9" json:"map_value9,omitempty"` + MapValue10 map[string]float32 `protobuf:"bytes,36,opt,name=map_value10,json=mapValue10" json:"map_value10,omitempty"` + MapValue11 map[float32]string `protobuf:"bytes,37,opt,name=map_value11,json=mapValue11" json:"map_value11,omitempty"` + MapValue12 map[string]float64 `protobuf:"bytes,38,opt,name=map_value12,json=mapValue12" json:"map_value12,omitempty"` + MapValue13 map[float64]string `protobuf:"bytes,39,opt,name=map_value13,json=mapValue13" json:"map_value13,omitempty"` + MapValue14 map[string]bool `protobuf:"bytes,40,opt,name=map_value14,json=mapValue14" json:"map_value14,omitempty"` + MapValue15 map[bool]string `protobuf:"bytes,41,opt,name=map_value15,json=mapValue15" json:"map_value15,omitempty"` } func (m *proto3Message) Reset() { *m = proto3Message{} } From 7c431edb737909565d1d32ae3c019fa49834dd50 Mon Sep 17 00:00:00 2001 From: adamstruck Date: Tue, 30 Jan 2018 10:59:25 -0800 Subject: [PATCH 2/2] clarify error on nested field in map mid path --- runtime/query.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/query.go b/runtime/query.go index 916a754a643..fb8f4fc0bf7 100644 --- a/runtime/query.go +++ b/runtime/query.go @@ -96,7 +96,7 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values [] continue case reflect.Map: if !isLast { - return fmt.Errorf("unexpected map field in %s", strings.Join(fieldPath, ".")) + return fmt.Errorf("unexpected nested field %s in %s", fieldPath[i+1], strings.Join(fieldPath[:i+1], ".")) } return populateMapField(f, values, props) default: @@ -142,7 +142,7 @@ func fieldByProtoName(m reflect.Value, name string) (reflect.Value, *proto.Prope func populateMapField(f reflect.Value, values []string, props *proto.Properties) error { if len(values) != 2 { - return fmt.Errorf("more than one value provided for key '%s' in map '%s'", values[0], props.Name) + return fmt.Errorf("more than one value provided for key %s in map %s", values[0], props.Name) } key, value := values[0], values[1]