From e2d19ba0ffec4f3434aa0f57b0b6d6b164f300da Mon Sep 17 00:00:00 2001
From: Noel Cower <ncower@gmail.com>
Date: Thu, 2 Mar 2017 17:07:01 -0800
Subject: [PATCH] runtime: Add oneof lookup for fields

This modifies fieldByProtoName to do two things:

1. It may now return an error. It only does this for the oneof case
   right now, as the not-found case only causes the field to be
   skipped.

2. It will first look for the field name in the message's OneofTypes
   map. If the field matches a OneOf type, it will check first that the
   field is nil -- if it isn't, the field is already set and it returns
   an error. If nil, it allocates a new oneof type for the field and
   returns the first field of the oneof type.

The reason for rejecting multiple oneof fields is that the runtime
iterates over url.Values and gets pseudo-random key order, meaning that
one field is selected out of several. We could address this by sorting
the url.Values and picking the last seen, in which case the code is far
simpler but involves allocating and sorting an array of field names
before walking them (only to get predictable field selection).

This change amends the error result tested for in
TestPopulateParameters to include the name of oneof_value.

Related to issue #82.

Change-Id: I7a5ecc9ce397bd71156cd4ac8690bae782ecc55e
---
 runtime/query.go      | 25 ++++++++++++++++++++-----
 runtime/query_test.go |  2 +-
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/runtime/query.go b/runtime/query.go
index 69f0ba1d6a7..4b031219c1f 100644
--- a/runtime/query.go
+++ b/runtime/query.go
@@ -48,8 +48,11 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values []
 			return fmt.Errorf("non-aggregate type in the mid of path: %s", strings.Join(fieldPath, "."))
 		}
 		var f reflect.Value
-		f, props = fieldByProtoName(m, fieldName)
-		if !f.IsValid() {
+		var err error
+		f, props, err = fieldByProtoName(m, fieldName)
+		if err != nil {
+			return err
+		} else if !f.IsValid() {
 			grpclog.Printf("field not found in %T: %s", msg, strings.Join(fieldPath, "."))
 			return nil
 		}
@@ -92,14 +95,26 @@ func populateFieldValueFromPath(msg proto.Message, fieldPath []string, values []
 
 // 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, *proto.Properties) {
+func fieldByProtoName(m reflect.Value, name string) (reflect.Value, *proto.Properties, error) {
 	props := proto.GetProperties(m.Type())
+
+	// look up field name in oneof map
+	if op, ok := props.OneofTypes[name]; ok {
+		v := reflect.New(op.Type.Elem())
+		field := m.Field(op.Field)
+		if !field.IsNil() {
+			return reflect.Value{}, nil, fmt.Errorf("field already set for %s oneof", props.Prop[op.Field].OrigName)
+		}
+		field.Set(v)
+		return v.Elem().Field(0), op.Prop, nil
+	}
+
 	for _, p := range props.Prop {
 		if p.OrigName == name {
-			return m.FieldByName(p.Name), p
+			return m.FieldByName(p.Name), p, nil
 		}
 	}
-	return reflect.Value{}, nil
+	return reflect.Value{}, nil, nil
 }
 
 func populateRepeatedField(f reflect.Value, values []string, props *proto.Properties) error {
diff --git a/runtime/query_test.go b/runtime/query_test.go
index 82026a8ed84..07262fad67f 100644
--- a/runtime/query_test.go
+++ b/runtime/query_test.go
@@ -161,7 +161,7 @@ func TestPopulateParameters(t *testing.T) {
 			},
 			filter:  utilities.NewDoubleArray(nil),
 			want:    &proto3Message{},
-			wanterr: errors.New("field already set for oneof"),
+			wanterr: errors.New("field already set for oneof_value oneof"),
 		},
 	} {
 		msg := proto.Clone(spec.want)