Add 'default' tag functionality to typeobj

message: |-
  Add 'default' tag functionality to typeobj

  Adding "default" to a typeobj tag now makes that field the default one to use if
  there's not a type field set in the input data. This is necessary because all
  the old commit objects in this repo don't have a type field set, but they will
  need to default to "commit".
change_hash: AP+DSWWe9K0wnnEV1wNt3u/Q89tEOf35zZGWjdES/d2t
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl5gOgkACgkQlcRvpqQRSKxG7hAAsWbbW+Vb7zmD3m32sQYQSY2e8OAClLGAB5tjWURp/bhFJUXf2T2aMPTPW4wNztQM2OgzcsIlAvFWRl8I/rUa9L04U/vvcA7vo03KYLLjTjqz6HEkeTg5is8gF/jJDdt7YBFpXJVaUu9b2hInpo87iyfsRYfxF832huvCrVxftd3eQcF/l33sWakwlc539M7UZ6V/kED2FQ0X8bhiOXCFrBrsb5cpedyNcdnFclOKk++y71pMoVU4r1Jm57i+f7HInxnFl7hm7Z82clgzYTPgr0Da43OB1RR/LOzTzPqHA/e9TUx/TSWd2Zu/MTdNiFRhNoMz0SdxXcXw805HGqoIK1s3pHiiaeujgFtnFWYuvnqQ5H7rOjf3lr+ig3pi9qsBR+/upOfw4iIP590OGC+JOu51PGe9NkOyRFi49j6j8BtFXxIVk49znMgqd242hOxh17u7LCHW/YS2nZVdFqoQZCNlpEJRuM18XvJ1c9bkulFNd4rkXEExEhedFEIm8fFWM04VwVbdimvRO4/YfU6MzPi/T/5hGhd1Gmauk/TwpQyoEsw/VDZU600uz0YVt2XnsoSRPcfcCX1uYjaYhrKadddCwuLpl8hvtWwd8EIaIfq6vcUVeBLxX53xb3mlo48wDOYDz7v3MJO/oKzoSLui/wAT/5fOoFVs6XT9F8AmhlM=
  account: mediocregopher

typeobj/typeobj.go

@@ -14,8 +14,46 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"strings"
 )
 
+type tagInfo struct {
+	val       string
+	isDefault bool
+}
+
+func parseTag(tag string) tagInfo {
+	parts := strings.Split(tag, ",")
+	return tagInfo{
+		val:       parts[0],
+		isDefault: len(parts) > 1 && parts[1] == "default",
+	}
+}
+
+func findTypeField(val reflect.Value, targetTypeTag string) (reflect.Value, reflect.StructField, error) {
+	typ := val.Type()
+
+	var defVal reflect.Value
+	var defTyp reflect.StructField
+	var defOk bool
+	for i := 0; i < val.NumField(); i++ {
+		fieldVal, fieldTyp := val.Field(i), typ.Field(i)
+		tagInfo := parseTag(fieldTyp.Tag.Get("type"))
+		if targetTypeTag != "" && tagInfo.val == targetTypeTag {
+			return fieldVal, fieldTyp, nil
+		} else if targetTypeTag == "" && tagInfo.isDefault {
+			defVal, defTyp, defOk = fieldVal, fieldTyp, true
+		}
+	}
+
+	if targetTypeTag == "" && defOk {
+		return defVal, defTyp, nil
+	} else if targetTypeTag == "" {
+		return reflect.Value{}, reflect.StructField{}, errors.New("type field not set")
+	}
+	return reflect.Value{}, reflect.StructField{}, fmt.Errorf("invalid type value %q", targetTypeTag)
+}
+
 // UnmarshalYAML is intended to be used within the UnmarshalYAML method of a
 // union struct. It will use the given input data's "type" field and match that
 // to the struct field tagged with that value. it will then unmarshal the input
@@ -44,14 +82,14 @@ func UnmarshalYAML(i interface{}, unmarshal func(interface{}) error) error {
 	if err := unmarshal(valWrap.Interface()); err != nil {
 		return err
 	}
-	typeVal := valWrap.Elem().Field(0).String()
+
+	// necessary to set non-type fields into the original value
 	val.Set(valWrap.Elem().Field(1))
 
-	typ := val.Type()
+	typeVal := valWrap.Elem().Field(0).String()
-	for i := 0; i < val.NumField(); i++ {
+	fieldVal, fieldTyp, err := findTypeField(val, typeVal)
-		fieldVal, fieldTyp := val.Field(i), typ.Field(i)
+	if err != nil {
-		if fieldTyp.Tag.Get("type") != typeVal {
+		return err
-			continue
 	}
 
 	var valInto interface{}
@@ -63,9 +101,6 @@ func UnmarshalYAML(i interface{}, unmarshal func(interface{}) error) error {
 		valInto = fieldVal.Addr().Interface()
 	}
 	return unmarshal(valInto)
-	}
-
-	return fmt.Errorf("invalid type value %q", typeVal)
 }
 
 // val should be of kind struct
@@ -78,14 +113,14 @@ func element(val reflect.Value) (reflect.Value, string, []int, error) {
 	nonTypeFields := make([]int, 0, numFields)
 	for i := 0; i < numFields; i++ {
 		innerFieldVal := val.Field(i)
-		innerTypeTag := typ.Field(i).Tag.Get("type")
+		innerTagInfo := parseTag(typ.Field(i).Tag.Get("type"))
-		if innerTypeTag == "" {
+		if innerTagInfo.val == "" {
 			nonTypeFields = append(nonTypeFields, i)
 		} else if innerFieldVal.IsZero() {
 			continue
 		} else {
 			fieldVal = innerFieldVal
-			typeTag = innerTypeTag
+			typeTag = innerTagInfo.val
 		}
 	}
 

typeobj/typeobj_test.go

@@ -2,6 +2,7 @@ package typeobj
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
@@ -31,83 +32,124 @@ func (o *outer) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return UnmarshalYAML(o, unmarshal)
 }
 
+type outerWDefault struct {
+	Foo foo  `type:"foo,default"`
+	Bar *bar `type:"bar"`
+}
+
+func (o outerWDefault) MarshalYAML() (interface{}, error) {
+	return MarshalYAML(o)
+}
+
+func (o *outerWDefault) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	return UnmarshalYAML(o, unmarshal)
+}
+
 func TestTypeObj(t *testing.T) {
 
 	type test struct {
 		descr string
 		str   string
-		err   bool
-		other string
 
-		obj     outer
+		expErr        string
-		typeTag string
+		expObj        interface{}
-		elem    interface{}
+		expTypeTag    string
+		expElem       interface{}
+		expMarshalOut string // defaults to str
 	}
 
 	tests := []test{
 		{
 			descr:  "no type set",
 			str:    `{}`,
-			err:   true,
+			expErr: "type field not set",
+			expObj: outer{},
 		},
 		{
-			descr: "unknown type set",
+			descr:  "no type set with nontype field",
+			str:    `other_field: aaa`,
+			expErr: "type field not set",
+			expObj: outer{},
+		},
+		{
+			descr:         "no type set with default",
+			str:           `a: 1`,
+			expObj:        outerWDefault{Foo: foo{A: 1}},
+			expTypeTag:    "foo",
+			expElem:       foo{A: 1},
+			expMarshalOut: "type: foo\na: 1",
+		},
+		{
+			descr:  "invalid type value",
 			str:    "type: baz",
-			err:   true,
+			expErr: "invalid type value",
+			expObj: outer{},
 		},
 		{
 			descr:      "foo set",
-			str:     "type: foo\na: 1\n",
+			str:        "type: foo\na: 1",
-			obj:     outer{Foo: foo{A: 1}},
+			expObj:     outer{Foo: foo{A: 1}},
-			typeTag: "foo",
+			expTypeTag: "foo",
-			elem:    foo{A: 1},
+			expElem:    foo{A: 1},
 		},
 		{
 			descr:      "bar set",
-			str:     "type: bar\nb: 1\n",
+			str:        "type: bar\nb: 1",
-			obj:     outer{Bar: &bar{B: 1}},
+			expObj:     outer{Bar: &bar{B: 1}},
-			typeTag: "bar",
+			expTypeTag: "bar",
-			elem:    &bar{B: 1},
+			expElem:    &bar{B: 1},
 		},
 		{
 			descr:      "foo and other_field set",
-			str:     "type: foo\na: 1\nother_field: aaa\n",
+			str:        "type: foo\na: 1\nother_field: aaa",
-			obj:     outer{Foo: foo{A: 1}, Other: "aaa"},
+			expObj:     outer{Foo: foo{A: 1}, Other: "aaa"},
-			typeTag: "foo",
+			expTypeTag: "foo",
-			elem:    foo{A: 1},
+			expElem:    foo{A: 1},
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.descr, func(t *testing.T) {
-			var o outer
+
-			err := yaml.Unmarshal([]byte(test.str), &o)
+			intoV := reflect.New(reflect.TypeOf(test.expObj))
-			if test.err && err != nil {
+
+			err := yaml.Unmarshal([]byte(test.str), intoV.Interface())
+			if test.expErr != "" {
+				if err == nil || !strings.HasPrefix(err.Error(), test.expErr) {
+					t.Fatalf("expected error %q when unmarshaling but got: %v", test.expErr, err)
+				}
 				return
-			} else if test.err && err == nil {
+			} else if test.expErr == "" && err != nil {
-				t.Fatal("expected error when unmarshaling but there was none")
-			} else if !test.err && err != nil {
 				t.Fatalf("unmarshaling %q returned unexpected error: %v", test.str, err)
 			}
 
-			if !reflect.DeepEqual(o, test.obj) {
+			into := intoV.Elem().Interface()
-				t.Fatalf("test expected value:\n%s\nbut got value:\n%s", spew.Sprint(test.obj), spew.Sprint(o))
+			if !reflect.DeepEqual(into, test.expObj) {
+				t.Fatalf("test expected value:\n%s\nbut got value:\n%s", spew.Sprint(test.expObj), spew.Sprint(into))
 			}
 
-			elem, typeTag, err := Element(o)
+			elem, typeTag, err := Element(into)
 			if err != nil {
-				t.Fatalf("error when calling Element(%s): %v", spew.Sprint(o), err)
+				t.Fatalf("error when calling Element(%s): %v", spew.Sprint(into), err)
-			} else if !reflect.DeepEqual(elem, test.elem) {
+			} else if !reflect.DeepEqual(elem, test.expElem) {
-				t.Fatalf("test expected elem value:\n%s\nbut got value:\n%s", spew.Sprint(test.elem), spew.Sprint(elem))
+				t.Fatalf("test expected elem value:\n%s\nbut got value:\n%s", spew.Sprint(test.expElem), spew.Sprint(elem))
-			} else if typeTag != test.typeTag {
+			} else if typeTag != test.expTypeTag {
-				t.Fatalf("test expected typeTag value %q but got %q", test.typeTag, typeTag)
+				t.Fatalf("test expected typeTag value %q but got %q", test.expTypeTag, typeTag)
 			}
 
-			b, err := yaml.Marshal(o)
+			expMarshalOut := test.expMarshalOut
+			if expMarshalOut == "" {
+				expMarshalOut = test.str
+			}
+			expMarshalOut = strings.TrimSpace(expMarshalOut)
+
+			b, err := yaml.Marshal(into)
 			if err != nil {
-				t.Fatalf("error marshaling %s: %v", spew.Sprint(o), err)
+				t.Fatalf("error marshaling %s: %v", spew.Sprint(into), err)
-			} else if test.str != string(b) {
+			}
-				t.Fatalf("test expected to marshal to %q, but instead marshaled to %q", test.str, b)
+			marshalOut := strings.TrimSpace(string(b))
+			if marshalOut != expMarshalOut {
+				t.Fatalf("test expected to marshal to %q, but instead marshaled to %q", expMarshalOut, marshalOut)
 			}
 		})
 	}