From a91951d3f8ac42bd990fa143460c7b11f54c412c Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Fri, 4 May 2018 07:54:08 -0700 Subject: [PATCH] Revert "remove dead api version code and move things around" This reverts commit 7e291ce8392dc2766a158dd8d70007dcf791aa44. --- google/api_versions_test.go | 326 ++++++++++++++++++++ google/convert.go | 77 ----- google/convert_test.go | 79 ----- google/resource_compute_project_metadata.go | 3 + google/test_utils.go | 41 --- google/utils.go | 8 - 6 files changed, 329 insertions(+), 205 deletions(-) create mode 100644 google/api_versions_test.go delete mode 100644 google/convert.go delete mode 100644 google/convert_test.go delete mode 100644 google/test_utils.go diff --git a/google/api_versions_test.go b/google/api_versions_test.go new file mode 100644 index 00000000..c18bee62 --- /dev/null +++ b/google/api_versions_test.go @@ -0,0 +1,326 @@ +package google + +import ( + "reflect" + "testing" +) + +type ExpectedApiVersions struct { + Create ApiVersion + ReadDelete ApiVersion + Update ApiVersion +} + +func TestApiVersion(t *testing.T) { + baseVersion := v1 + betaVersion := v0beta + maxTestApiVersion := func(versionsInUse map[ApiVersion]struct{}) ApiVersion { + if _, ok := versionsInUse[betaVersion]; ok { + return betaVersion + } + return baseVersion + } + + cases := map[string]struct { + Features []Feature + FieldsInSchema map[string]interface{} + UpdatedFields []string + UpdateOnlyFields []Feature + ExpectedApiVersions + }{ + "no beta field": { + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: baseVersion, + }, + }, + "beta field not set": { + Features: []Feature{{Version: betaVersion, Item: "beta_field"}}, + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: baseVersion, + }, + }, + "beta field set": { + Features: []Feature{{Version: betaVersion, Item: "beta_field"}}, + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + "beta_field": "bar", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: betaVersion, + ReadDelete: betaVersion, + Update: betaVersion, + }, + }, + "update only beta field": { + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + }, + UpdatedFields: []string{"beta_update_field"}, + UpdateOnlyFields: []Feature{{Version: betaVersion, Item: "beta_update_field"}}, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: betaVersion, + }, + }, + "nested beta field not set": { + Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field"}}, + FieldsInSchema: map[string]interface{}{ + "list_field.#": 2, + "list_field.0.normal_field": "foo", + "list_field.1.normal_field": "bar", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: baseVersion, + }, + }, + "nested beta field set": { + Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field"}}, + FieldsInSchema: map[string]interface{}{ + "list_field.#": 2, + "list_field.0.normal_field": "foo", + "list_field.1.normal_field": "bar", + "list_field.1.beta_nested_field": "baz", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: betaVersion, + ReadDelete: betaVersion, + Update: betaVersion, + }, + }, + "double nested fields set": { + Features: []Feature{{Version: betaVersion, Item: "list_field.*.nested_list_field.*.beta_nested_field"}}, + FieldsInSchema: map[string]interface{}{ + "list_field.#": 1, + "list_field.0.nested_list_field.#": 1, + "list_field.0.nested_list_field.0.beta_nested_field": "foo", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: betaVersion, + ReadDelete: betaVersion, + Update: betaVersion, + }, + }, + "beta field has default value": { + Features: []Feature{{Version: betaVersion, Item: "beta_field", DefaultValue: "bar"}}, + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + "beta_field": "bar", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: baseVersion, + }, + }, + "beta field is updated to default value": { + Features: []Feature{{Version: betaVersion, Item: "beta_field", DefaultValue: "bar"}}, + FieldsInSchema: map[string]interface{}{ + "normal_field": "foo", + "beta_field": "bar", + }, + UpdatedFields: []string{"beta_field"}, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: betaVersion, + }, + }, + "nested beta field has default value": { + Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field", DefaultValue: "baz"}}, + FieldsInSchema: map[string]interface{}{ + "list_field.#": 2, + "list_field.0.normal_field": "foo", + "list_field.1.normal_field": "bar", + "list_field.1.beta_nested_field": "baz", + }, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: baseVersion, + }, + }, + "nested beta field is updated default value": { + Features: []Feature{{Version: betaVersion, Item: "list_field.*.beta_nested_field", DefaultValue: "baz"}}, + FieldsInSchema: map[string]interface{}{ + "list_field.#": 2, + "list_field.0.normal_field": "foo", + "list_field.1.normal_field": "bar", + "list_field.1.beta_nested_field": "baz", + }, + UpdatedFields: []string{"list_field.1.beta_nested_field"}, + ExpectedApiVersions: ExpectedApiVersions{ + Create: baseVersion, + ReadDelete: baseVersion, + Update: betaVersion, + }, + }, + } + + for tn, tc := range cases { + // Create + // All fields with value have HasChange set to true. + keys := make([]string, 0, len(tc.FieldsInSchema)) + for key := range tc.FieldsInSchema { + keys = append(keys, key) + } + + d := &ResourceDataMock{ + FieldsInSchema: tc.FieldsInSchema, + FieldsWithHasChange: keys, + } + + apiVersion := getApiVersion(d, v1, tc.Features, maxTestApiVersion) + if apiVersion != tc.ExpectedApiVersions.Create { + t.Errorf("bad: %s, Expected to see version %v for create, got version %v", tn, tc.ExpectedApiVersions.Create, apiVersion) + } + + // Read/Delete + // All fields have HasChange set to false. + d = &ResourceDataMock{ + FieldsInSchema: tc.FieldsInSchema, + } + + apiVersion = getApiVersion(d, v1, tc.Features, maxTestApiVersion) + if apiVersion != tc.ExpectedApiVersions.ReadDelete { + t.Errorf("bad: %s, Expected to see version %v for read/delete, got version %v", tn, tc.ExpectedApiVersions.ReadDelete, apiVersion) + } + + // Update + // Only fields defined as updated in the test case have HasChange set to true. + d = &ResourceDataMock{ + FieldsInSchema: tc.FieldsInSchema, + FieldsWithHasChange: tc.UpdatedFields, + } + + apiVersion = getApiVersionUpdate(d, v1, tc.Features, tc.UpdateOnlyFields, maxTestApiVersion) + if apiVersion != tc.ExpectedApiVersions.Update { + t.Errorf("bad: %s, Expected to see version %v for update, got version %v", tn, tc.ExpectedApiVersions.Update, apiVersion) + } + } +} + +func TestSetOmittedFields(t *testing.T) { + type Inner struct { + InnerNotOmitted string `json:"notOmitted"` + InnerOmitted []string `json:"-"` + } + type InputOuter struct { + NotOmitted string `json:"notOmitted"` + Omitted []string `json:"-"` + Struct Inner + Pointer *Inner + StructSlice []Inner + PointerSlice []*Inner + Unset *Inner + OnlyInInputType *Inner + } + type OutputOuter struct { + NotOmitted string `json:"notOmitted"` + Omitted []string `json:"-"` + Struct Inner + Pointer *Inner + StructSlice []Inner + PointerSlice []*Inner + Unset *Inner + OnlyInOutputType *Inner + } + + input := &InputOuter{ + NotOmitted: "foo", + Omitted: []string{"foo"}, + Struct: Inner{ + InnerNotOmitted: "foo", + InnerOmitted: []string{"foo"}, + }, + Pointer: &Inner{ + InnerNotOmitted: "foo", + InnerOmitted: []string{"foo"}, + }, + StructSlice: []Inner{ + { + InnerNotOmitted: "foo", + InnerOmitted: []string{"foo"}, + }, { + InnerNotOmitted: "bar", + InnerOmitted: []string{"bar"}, + }, + }, + PointerSlice: []*Inner{ + { + InnerNotOmitted: "foo", + InnerOmitted: []string{"foo"}, + }, { + InnerNotOmitted: "bar", + InnerOmitted: []string{"bar"}, + }, + }, + OnlyInInputType: &Inner{ + InnerNotOmitted: "foo", + InnerOmitted: []string{"foo"}, + }, + } + output := &OutputOuter{} + Convert(input, output) + if input.NotOmitted != output.NotOmitted || + !reflect.DeepEqual(input.Omitted, output.Omitted) || + !reflect.DeepEqual(input.Struct, output.Struct) || + !reflect.DeepEqual(input.Pointer, output.Pointer) || + !reflect.DeepEqual(input.StructSlice, output.StructSlice) || + !reflect.DeepEqual(input.PointerSlice, output.PointerSlice) || + !(input.Unset == nil && output.Unset == nil) { + t.Errorf("Structs were not equivalent after conversion:\nInput:%#v\nOutput: %#v", input, output) + } +} + +type ResourceDataMock struct { + FieldsInSchema map[string]interface{} + FieldsWithHasChange []string + id string +} + +func (d *ResourceDataMock) HasChange(key string) bool { + exists := false + for _, val := range d.FieldsWithHasChange { + if key == val { + exists = true + } + } + + return exists +} + +func (d *ResourceDataMock) GetOk(key string) (interface{}, bool) { + for k, v := range d.FieldsInSchema { + if key == k { + return v, true + } + } + + return nil, false +} + +func (d *ResourceDataMock) Set(key string, value interface{}) error { + d.FieldsInSchema[key] = value + return nil +} + +func (d *ResourceDataMock) SetId(v string) { + d.id = v +} + +func (d *ResourceDataMock) Id() string { + return d.id +} diff --git a/google/convert.go b/google/convert.go deleted file mode 100644 index 2b0d8980..00000000 --- a/google/convert.go +++ /dev/null @@ -1,77 +0,0 @@ -package google - -import ( - "encoding/json" - "reflect" -) - -// Convert between two types by converting to/from JSON. Intended to switch -// between multiple API versions, as they are strict supersets of one another. -// item and out are pointers to structs -func Convert(item, out interface{}) error { - bytes, err := json.Marshal(item) - if err != nil { - return err - } - - err = json.Unmarshal(bytes, out) - if err != nil { - return err - } - - // Converting between maps and structs only occurs when autogenerated resources convert the result - // of an HTTP request. Those results do not contain omitted fields, so no need to set them. - if _, ok := item.(map[string]interface{}); !ok { - setOmittedFields(item, out) - } - - return nil -} - -func setOmittedFields(item, out interface{}) { - // Both inputs must be pointers, see https://blog.golang.org/laws-of-reflection: - // "To modify a reflection object, the value must be settable." - iVal := reflect.ValueOf(item).Elem() - oVal := reflect.ValueOf(out).Elem() - - // Loop through all the fields of the struct to look for omitted fields and nested fields - for i := 0; i < iVal.NumField(); i++ { - iField := iVal.Field(i) - if isEmptyValue(iField) { - continue - } - - fieldInfo := iVal.Type().Field(i) - oField := oVal.FieldByName(fieldInfo.Name) - - // Only look at fields that exist in the output struct - if !oField.IsValid() { - continue - } - - // If the field contains a 'json:"="' tag, then it was omitted from the Marshal/Unmarshal - // call and needs to be added back in. - if fieldInfo.Tag.Get("json") == "-" { - oField.Set(iField) - } - - // If this field is a struct, *struct, []struct, or []*struct, recurse. - if iField.Kind() == reflect.Struct { - setOmittedFields(iField.Addr().Interface(), oField.Addr().Interface()) - } - if iField.Kind() == reflect.Ptr && iField.Type().Elem().Kind() == reflect.Struct { - setOmittedFields(iField.Interface(), oField.Interface()) - } - if iField.Kind() == reflect.Slice && iField.Type().Elem().Kind() == reflect.Struct { - for j := 0; j < iField.Len(); j++ { - setOmittedFields(iField.Index(j).Addr().Interface(), oField.Index(j).Addr().Interface()) - } - } - if iField.Kind() == reflect.Slice && iField.Type().Elem().Kind() == reflect.Ptr && - iField.Type().Elem().Elem().Kind() == reflect.Struct { - for j := 0; j < iField.Len(); j++ { - setOmittedFields(iField.Index(j).Interface(), oField.Index(j).Interface()) - } - } - } -} diff --git a/google/convert_test.go b/google/convert_test.go deleted file mode 100644 index fb63783c..00000000 --- a/google/convert_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package google - -import ( - "reflect" - "testing" -) - -func TestSetOmittedFields(t *testing.T) { - type Inner struct { - InnerNotOmitted string `json:"notOmitted"` - InnerOmitted []string `json:"-"` - } - type InputOuter struct { - NotOmitted string `json:"notOmitted"` - Omitted []string `json:"-"` - Struct Inner - Pointer *Inner - StructSlice []Inner - PointerSlice []*Inner - Unset *Inner - OnlyInInputType *Inner - } - type OutputOuter struct { - NotOmitted string `json:"notOmitted"` - Omitted []string `json:"-"` - Struct Inner - Pointer *Inner - StructSlice []Inner - PointerSlice []*Inner - Unset *Inner - OnlyInOutputType *Inner - } - - input := &InputOuter{ - NotOmitted: "foo", - Omitted: []string{"foo"}, - Struct: Inner{ - InnerNotOmitted: "foo", - InnerOmitted: []string{"foo"}, - }, - Pointer: &Inner{ - InnerNotOmitted: "foo", - InnerOmitted: []string{"foo"}, - }, - StructSlice: []Inner{ - { - InnerNotOmitted: "foo", - InnerOmitted: []string{"foo"}, - }, { - InnerNotOmitted: "bar", - InnerOmitted: []string{"bar"}, - }, - }, - PointerSlice: []*Inner{ - { - InnerNotOmitted: "foo", - InnerOmitted: []string{"foo"}, - }, { - InnerNotOmitted: "bar", - InnerOmitted: []string{"bar"}, - }, - }, - OnlyInInputType: &Inner{ - InnerNotOmitted: "foo", - InnerOmitted: []string{"foo"}, - }, - } - output := &OutputOuter{} - Convert(input, output) - if input.NotOmitted != output.NotOmitted || - !reflect.DeepEqual(input.Omitted, output.Omitted) || - !reflect.DeepEqual(input.Struct, output.Struct) || - !reflect.DeepEqual(input.Pointer, output.Pointer) || - !reflect.DeepEqual(input.StructSlice, output.StructSlice) || - !reflect.DeepEqual(input.PointerSlice, output.PointerSlice) || - !(input.Unset == nil && output.Unset == nil) { - t.Errorf("Structs were not equivalent after conversion:\nInput:%#v\nOutput: %#v", input, output) - } -} diff --git a/google/resource_compute_project_metadata.go b/google/resource_compute_project_metadata.go index ec9dd558..86ed6213 100644 --- a/google/resource_compute_project_metadata.go +++ b/google/resource_compute_project_metadata.go @@ -8,6 +8,9 @@ import ( "google.golang.org/api/compute/v1" ) +var ProjectMetadataBaseApiVersion = v1 +var ProjectMetadataVersionedFeatures = []Feature{} + func resourceComputeProjectMetadata() *schema.Resource { return &schema.Resource{ Create: resourceComputeProjectMetadataCreate, diff --git a/google/test_utils.go b/google/test_utils.go deleted file mode 100644 index de0cb1ea..00000000 --- a/google/test_utils.go +++ /dev/null @@ -1,41 +0,0 @@ -package google - -type ResourceDataMock struct { - FieldsInSchema map[string]interface{} - FieldsWithHasChange []string - id string -} - -func (d *ResourceDataMock) HasChange(key string) bool { - exists := false - for _, val := range d.FieldsWithHasChange { - if key == val { - exists = true - } - } - - return exists -} - -func (d *ResourceDataMock) GetOk(key string) (interface{}, bool) { - for k, v := range d.FieldsInSchema { - if key == k { - return v, true - } - } - - return nil, false -} - -func (d *ResourceDataMock) Set(key string, value interface{}) error { - d.FieldsInSchema[key] = value - return nil -} - -func (d *ResourceDataMock) SetId(v string) { - d.id = v -} - -func (d *ResourceDataMock) Id() string { - return d.id -} diff --git a/google/utils.go b/google/utils.go index 404bbf55..2f82d1a3 100644 --- a/google/utils.go +++ b/google/utils.go @@ -17,14 +17,6 @@ import ( "google.golang.org/api/googleapi" ) -type TerraformResourceData interface { - HasChange(string) bool - GetOk(string) (interface{}, bool) - Set(string, interface{}) error - SetId(string) - Id() string -} - // getRegionFromZone returns the region from a zone for Google cloud. func getRegionFromZone(zone string) string { if zone != "" && len(zone) > 2 {