From b699557f169f91ade0106c045dee587ac89ddc9a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 17 Aug 2014 23:49:10 -0700 Subject: [PATCH] Decode into proper arrays of things, add many test cases /cc @armon - This changes how Consul has to define its structure. Ping me tomorrow to learn more, but going to leave it here for reference too: The Consul case (there is a test case) never worked even with go-libucl, because there is an ambiguity of whether you want the inner children or the array of outer elements (the slice in the Policy struct). I've added a new modifier you can specify with a tag called "expand" which will tell hcl to expand the value to its children for decoding. You can see me use it in the test case which verifies that the Consul ACLs parse. --- decoder.go | 86 ++++++++++++++------------- decoder_test.go | 90 +++++++++++++++++++---------- hcl/object.go | 15 +++-- test-fixtures/empty.hcl | 2 +- test-fixtures/structure2.json | 8 +-- test-fixtures/terraform_heroku.hcl | 5 ++ test-fixtures/terraform_heroku.json | 6 ++ 7 files changed, 133 insertions(+), 79 deletions(-) create mode 100644 test-fixtures/terraform_heroku.hcl create mode 100644 test-fixtures/terraform_heroku.json diff --git a/decoder.go b/decoder.go index 24e81da..1b76d93 100644 --- a/decoder.go +++ b/decoder.go @@ -99,22 +99,22 @@ func (d *decoder) decodeInterface(name string, o *hcl.Object, result reflect.Val switch o.Type { case hcl.ValueTypeObject: - /* + if name == "root" { + var temp map[string]interface{} + tempVal := reflect.ValueOf(temp) + result := reflect.MakeMap( + reflect.MapOf( + reflect.TypeOf(""), + tempVal.Type().Elem())) + + set = result + } else { var temp []map[string]interface{} tempVal := reflect.ValueOf(temp) result := reflect.MakeSlice( reflect.SliceOf(tempVal.Type().Elem()), 0, int(o.Len())) set = result - */ - - var temp map[string]interface{} - tempVal := reflect.ValueOf(temp) - result := reflect.MakeMap( - reflect.MapOf( - reflect.TypeOf(""), - tempVal.Type().Elem())) - - set = result + } case hcl.ValueTypeList: /* var temp []interface{} @@ -170,7 +170,7 @@ func (d *decoder) decodeInterface(name string, o *hcl.Object, result reflect.Val func (d *decoder) decodeMap(name string, o *hcl.Object, result reflect.Value) error { if o.Type != hcl.ValueTypeObject { - return fmt.Errorf("%s: not an object type (%s)", name, o.Type) + return fmt.Errorf("%s: not an object type for map (%s)", name, o.Type) } // If we have an interface, then we can address the interface, @@ -196,15 +196,12 @@ func (d *decoder) decodeMap(name string, o *hcl.Object, result reflect.Value) er } // Go through each element and decode it. - current := o - for current != nil { - if current.Value == nil { - current = current.Next + for _, o := range o.Elem(false) { + if o.Value == nil { continue } - m := current.Value.([]*hcl.Object) - for _, o := range m { + for _, o := range o.Elem(true) { // Make the field name fieldName := fmt.Sprintf("%s.%s", name, o.Key) @@ -226,8 +223,6 @@ func (d *decoder) decodeMap(name string, o *hcl.Object, result reflect.Value) er // Set the value on the map resultMap.SetMapIndex(key, val) } - - current = current.Next } // Set the final map if we can @@ -266,25 +261,29 @@ func (d *decoder) decodeSlice(name string, o *hcl.Object, result reflect.Value) resultSliceType, 0, 0) } + // Determine how we're doing this + expand := true + switch o.Type { + case hcl.ValueTypeObject: + expand = false + default: + // Array or anything else: we expand values and take it all + } + i := 0 - current := o - for current != nil { - for _, o := range current.Elem(true) { - fieldName := fmt.Sprintf("%s[%d]", name, i) + for _, o := range o.Elem(expand) { + fieldName := fmt.Sprintf("%s[%d]", name, i) - // Decode - val := reflect.Indirect(reflect.New(resultElemType)) - if err := d.decode(fieldName, o, val); err != nil { - return err - } - - // Append it onto the slice - result = reflect.Append(result, val) - - i += 1 + // Decode + val := reflect.Indirect(reflect.New(resultElemType)) + if err := d.decode(fieldName, o, val); err != nil { + return err } - current = current.Next + // Append it onto the slice + result = reflect.Append(result, val) + + i += 1 } set.Set(result) @@ -308,7 +307,7 @@ func (d *decoder) decodeString(name string, o *hcl.Object, result reflect.Value) func (d *decoder) decodeStruct(name string, o *hcl.Object, result reflect.Value) error { if o.Type != hcl.ValueTypeObject { return fmt.Errorf( - "%s: not an object type for struct (%T)", name, o) + "%s: not an object type for struct (%s)", name, o.Type) } // This slice will keep track of all the structs we'll be decoding. @@ -377,10 +376,16 @@ func (d *decoder) decodeStruct(name string, o *hcl.Object, result reflect.Value) fieldName := fieldType.Name + // This is whether or not we expand the object into its children + // later. + expand := false + tagValue := fieldType.Tag.Get(tagName) tagParts := strings.SplitN(tagValue, ",", 2) if len(tagParts) >= 2 { switch tagParts[1] { + case "expand": + expand = true case "decodedFields": decodedFieldsVal = append(decodedFieldsVal, field) continue @@ -406,10 +411,13 @@ func (d *decoder) decodeStruct(name string, o *hcl.Object, result reflect.Value) // Track the used key usedKeys[fieldName] = struct{}{} - // Create the field name and decode + // Create the field name and decode. We range over the elements + // because we actually want the value. fieldName = fmt.Sprintf("%s.%s", name, fieldName) - if err := d.decode(fieldName, obj, field); err != nil { - return err + for _, obj := range obj.Elem(expand) { + if err := d.decode(fieldName, obj, field); err != nil { + return err + } } decodedFields = append(decodedFields, fieldType.Name) diff --git a/decoder_test.go b/decoder_test.go index 446cd99..33a2292 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -24,31 +24,61 @@ func TestDecode_interface(t *testing.T) { "empty.hcl", false, map[string]interface{}{ - "resource": map[string]interface{}{ - "aws_instance": map[string]interface{}{ - "db": map[string]interface{}{}, - }, - }, - }, - }, - /* - { - "structure.hcl", - false, - map[string]interface{}{ - "foo": []interface{}{ - map[string]interface{}{ - "baz": []interface{}{ - map[string]interface{}{ - "foo": "bar", - "key": 7, - }, - }, + "resource": []map[string]interface{}{ + map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{}, }, }, }, }, - */ + }, + { + "terraform_heroku.hcl", + false, + map[string]interface{}{ + "name": "terraform-test-app", + "config_vars": []map[string]interface{}{ + map[string]interface{}{ + "FOO": "bar", + }, + }, + }, + }, + { + "structure_multi.hcl", + false, + map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{ + "baz": []map[string]interface{}{ + map[string]interface{}{"key": 7}, + }, + }, + map[string]interface{}{ + "bar": []map[string]interface{}{ + map[string]interface{}{"key": 12}, + }, + }, + }, + }, + }, + { + "structure_multi.json", + false, + map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{ + "baz": []map[string]interface{}{ + map[string]interface{}{"key": 7}, + }, + "bar": []map[string]interface{}{ + map[string]interface{}{"key": 12}, + }, + }, + }, + }, + }, } for _, tc := range cases { @@ -88,12 +118,8 @@ func TestDecode_equal(t *testing.T) { "structure_flat.json", }, { - "structure_multi.hcl", - "structure_multi.json", - }, - { - "structure2.hcl", - "structure2.json", + "terraform_heroku.hcl", + "terraform_heroku.json", }, } @@ -209,7 +235,7 @@ func TestDecode_structureArray(t *testing.T) { } type Policy struct { - Keys []KeyPolicy `hcl:"key"` + Keys []KeyPolicy `hcl:"key,expand"` } expected := Policy{ @@ -275,8 +301,10 @@ func TestDecode_structureMap(t *testing.T) { }, "amis": hclVariable{ - Default: map[string]interface{}{ - "east": "foo", + Default: []map[string]interface{}{ + map[string]interface{}{ + "east": "foo", + }, }, Fields: []string{"Default"}, }, @@ -293,7 +321,7 @@ func TestDecode_structureMap(t *testing.T) { err := Decode(&actual, testReadFile(t, f)) if err != nil { - t.Fatalf("err: %s", err) + t.Fatalf("Input: %s\n\nerr: %s", f, err) } if !reflect.DeepEqual(actual, expected) { diff --git a/hcl/object.go b/hcl/object.go index aa58081..6f1d9aa 100644 --- a/hcl/object.go +++ b/hcl/object.go @@ -62,7 +62,10 @@ func (o *Object) Elem(expand bool) []*Object { result := make([]*Object, 0, 1) current := o for current != nil { - result = append(result, current) + obj := *current + obj.Next = nil + result = append(result, &obj) + current = current.Next } @@ -77,10 +80,14 @@ func (o *Object) Elem(expand bool) []*Object { case ValueTypeList: return o.Value.([]*Object) case ValueTypeObject: - return o.Value.([]*Object) + result := make([]*Object, 0, 5) + for _, obj := range o.Elem(false) { + result = append(result, obj.Value.([]*Object)...) + } + return result + default: + return []*Object{o} } - - panic(fmt.Sprintf("Elem not supported for: %s", o.Type)) } // Len returns the number of objects in this object structure. diff --git a/test-fixtures/empty.hcl b/test-fixtures/empty.hcl index 8553eeb..5be1b23 100644 --- a/test-fixtures/empty.hcl +++ b/test-fixtures/empty.hcl @@ -1 +1 @@ -resource "aws_instance" "db" {} +resource "foo" {} diff --git a/test-fixtures/structure2.json b/test-fixtures/structure2.json index d133b1e..c51fcf5 100644 --- a/test-fixtures/structure2.json +++ b/test-fixtures/structure2.json @@ -1,10 +1,10 @@ { - "foo": { + "foo": [{ "baz": { "key": 7, "foo": "bar" - }, - + } + }, { "key": 7 - } + }] } diff --git a/test-fixtures/terraform_heroku.hcl b/test-fixtures/terraform_heroku.hcl new file mode 100644 index 0000000..fda9241 --- /dev/null +++ b/test-fixtures/terraform_heroku.hcl @@ -0,0 +1,5 @@ +name = "terraform-test-app" + +config_vars { + FOO = "bar" +} diff --git a/test-fixtures/terraform_heroku.json b/test-fixtures/terraform_heroku.json new file mode 100644 index 0000000..e8c6fac --- /dev/null +++ b/test-fixtures/terraform_heroku.json @@ -0,0 +1,6 @@ +{ + "name": "terraform-test-app", + "config_vars": { + "FOO": "bar" + } +}