From ff3c099753c3fa6d70b27f19ba072620c4fbbc15 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Wed, 2 Jun 2021 10:03:50 -0700 Subject: [PATCH 1/6] Added basic tests for biquery table schema diff suppress --- .../tests/resource_bigquery_table_test.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index cc3c93520c48..0b94cb8e1c54 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -11,6 +11,60 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) +func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { + cases := map[string]struct { + Old, New string + ExpectDiffSuppress bool + }{ + "empty schema": { + Old: "null", + New: "[]", + ExpectDiffSuppress: true, + }, + "empty schema -> non-empty": { + Old: "null", + New: `[ + { + "name": "PageNo", + "type": "INTEGER" + } + ]`, + ExpectDiffSuppress: false, + }, + "mode NULLABLE -> default mode (also NULLABLE)": { + Old: `[ + { + "mode": "NULLABLE", + "name": "PageNo", + "type": "INTEGER" + }, + { + "mode": "NULLABLE", + "name": "IngestTime", + "type": "TIMESTAMP" + } + ]`, + New: `[ + { + "name": "PageNo", + "type": "INTEGER" + }, + { + "name": "IngestTime", + "type": "TIMESTAMP" + } + ]`, + ExpectDiffSuppress: true, + }, + } + + for tn, tc := range cases { + if bigQueryTableSchemaDiffSuppress("schema", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Errorf("bad: %s, %q => %q expect DiffSuppress to return %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + } +} + func TestAccBigQueryTable_Basic(t *testing.T) { t.Parallel() From d003fc936a189a7e2a272b0bf88e7ce91b691906 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Wed, 2 Jun 2021 11:49:02 -0700 Subject: [PATCH 2/6] Only sort the top level of a bigquery schema when checking diff suppression --- .../resources/resource_bigquery_table.go | 35 +++++++++----- .../tests/resource_bigquery_table_test.go | 47 +++++++++++++++++++ 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_bigquery_table.go b/mmv1/third_party/terraform/resources/resource_bigquery_table.go index 4927593005c7..47d2fcd4fa00 100644 --- a/mmv1/third_party/terraform/resources/resource_bigquery_table.go +++ b/mmv1/third_party/terraform/resources/resource_bigquery_table.go @@ -53,14 +53,6 @@ func jsonCompareWithMapKeyOverride(a, b interface{}, compareMapKeyVal func(key s } else if len(arrayA) != len(arrayB) { return false, nil } - if err := bigQueryTablecheckNameExists(arrayA); err != nil { - return false, err - } - bigQueryTableSortArrayByName(arrayA) - if err := bigQueryTablecheckNameExists(arrayB); err != nil { - return false, err - } - bigQueryTableSortArrayByName(arrayB) for i := range arrayA { eq, err := jsonCompareWithMapKeyOverride(arrayA[i], arrayB[i], compareMapKeyVal) if err != nil { @@ -147,13 +139,34 @@ func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData) } var a, b interface{} if err := json.Unmarshal([]byte(old), &a); err != nil { - log.Printf("[DEBUG] unable to unmarshal json - %v", err) + log.Printf("[DEBUG] unable to unmarshal old json - %v", err) } if err := json.Unmarshal([]byte(new), &b); err != nil { - log.Printf("[DEBUG] unable to unmarshal json - %v", err) + log.Printf("[DEBUG] unable to unmarshal new json - %v", err) + } + + arrayA := a.([]interface{}) + arrayB, ok := b.([]interface{}) + if !ok { + return false + } else if len(arrayA) != len(arrayB) { + return false + } + + // Sort the first level of the schema by name so reordering the + // fields doesn't cause a diff. + if err := bigQueryTablecheckNameExists(arrayA); err != nil { + log.Printf("[DEBUG] unable to sort old schema by field name - %v", err) + return false + } + bigQueryTableSortArrayByName(arrayA) + if err := bigQueryTablecheckNameExists(arrayB); err != nil { + log.Printf("[DEBUG] unable to sort new schema by field name - %v", err) + return false } + bigQueryTableSortArrayByName(arrayB) - eq, err := jsonCompareWithMapKeyOverride(a, b, bigQueryTableMapKeyOverride) + eq, err := jsonCompareWithMapKeyOverride(arrayA, arrayB, bigQueryTableMapKeyOverride) if err != nil { log.Printf("[DEBUG] %v", err) log.Printf("[DEBUG] Error comparing JSON: %v, %v", old, new) diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index 0b94cb8e1c54..5688afa0d787 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -56,6 +56,53 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ]`, ExpectDiffSuppress: true, }, + "reordering fields": { + Old: `[ + { + "name": "PageNo", + "type": "INTEGER" + }, + { + "name": "IngestTime", + "type": "TIMESTAMP" + } + ]`, + New: `[ + { + "name": "IngestTime", + "type": "TIMESTAMP" + }, + { + "name": "PageNo", + "type": "INTEGER" + } + ]`, + ExpectDiffSuppress: true, + }, + "nested lists": { + Old: `[ + { + "mode": "NULLABLE", + "name": "providerphone", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + }, + "type":"STRING" + } + ]`, + New: `[ + { + "name": "providerphone", + "type": "STRING", + "policyTags": { + "names": ["projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678"] + } + } + ]`, + ExpectDiffSuppress: true, + }, } for tn, tc := range cases { From 5e94dd0660af46716e564d6083e49a828b400f99 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Wed, 2 Jun 2021 15:35:58 -0700 Subject: [PATCH 3/6] Keep suppressing diff for subfield reordering --- .../resources/resource_bigquery_table.go | 60 ++++++++----------- .../tests/resource_bigquery_table_test.go | 47 ++++++++++++++- 2 files changed, 71 insertions(+), 36 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_bigquery_table.go b/mmv1/third_party/terraform/resources/resource_bigquery_table.go index 47d2fcd4fa00..deadd8274e0e 100644 --- a/mmv1/third_party/terraform/resources/resource_bigquery_table.go +++ b/mmv1/third_party/terraform/resources/resource_bigquery_table.go @@ -7,6 +7,7 @@ import ( "fmt" "log" "sort" + "strconv" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -43,7 +44,7 @@ func bigQueryTablecheckNameExists(jsonList []interface{}) error { // Compares two json's while optionally taking in a compareMapKeyVal function. // This function will override any comparison of a given map[string]interface{} // on a specific key value allowing for a separate equality in specific scenarios -func jsonCompareWithMapKeyOverride(a, b interface{}, compareMapKeyVal func(key string, val1, val2 map[string]interface{}) bool) (bool, error) { +func jsonCompareWithMapKeyOverride(key string, a, b interface{}, compareMapKeyVal func(key string, val1, val2 map[string]interface{}) bool) (bool, error) { switch a.(type) { case []interface{}: arrayA := a.([]interface{}) @@ -53,8 +54,20 @@ func jsonCompareWithMapKeyOverride(a, b interface{}, compareMapKeyVal func(key s } else if len(arrayA) != len(arrayB) { return false, nil } + + // Sort fields by name so reordering them doesn't cause a diff. + if (key == "schema" || key == "fields") { + if err := bigQueryTablecheckNameExists(arrayA); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayA) + if err := bigQueryTablecheckNameExists(arrayB); err != nil { + return false, err + } + bigQueryTableSortArrayByName(arrayB) + } for i := range arrayA { - eq, err := jsonCompareWithMapKeyOverride(arrayA[i], arrayB[i], compareMapKeyVal) + eq, err := jsonCompareWithMapKeyOverride(strconv.Itoa(i), arrayA[i], arrayB[i], compareMapKeyVal) if err != nil { return false, err } else if !eq { @@ -70,22 +83,22 @@ func jsonCompareWithMapKeyOverride(a, b interface{}, compareMapKeyVal func(key s } var unionOfKeys map[string]bool = make(map[string]bool) - for key := range objectA { - unionOfKeys[key] = true + for subKey := range objectA { + unionOfKeys[subKey] = true } - for key := range objectB { - unionOfKeys[key] = true + for subKey := range objectB { + unionOfKeys[subKey] = true } - for key := range unionOfKeys { - eq := compareMapKeyVal(key, objectA, objectB) + for subKey := range unionOfKeys { + eq := compareMapKeyVal(subKey, objectA, objectB) if !eq { - valA, ok1 := objectA[key] - valB, ok2 := objectB[key] + valA, ok1 := objectA[subKey] + valB, ok2 := objectB[subKey] if !ok1 || !ok2 { return false, nil } - eq, err := jsonCompareWithMapKeyOverride(valA, valB, compareMapKeyVal) + eq, err := jsonCompareWithMapKeyOverride(subKey, valA, valB, compareMapKeyVal) if err != nil || !eq { return false, err } @@ -132,7 +145,7 @@ func bigQueryTableMapKeyOverride(key string, objectA, objectB map[string]interfa } // Compare the JSON strings are equal -func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { +func bigQueryTableSchemaDiffSuppress(name, old, new string, _ *schema.ResourceData) bool { // The API can return an empty schema which gets encoded to "null" during read. if old == "null" { old = "[]" @@ -145,28 +158,7 @@ func bigQueryTableSchemaDiffSuppress(_, old, new string, _ *schema.ResourceData) log.Printf("[DEBUG] unable to unmarshal new json - %v", err) } - arrayA := a.([]interface{}) - arrayB, ok := b.([]interface{}) - if !ok { - return false - } else if len(arrayA) != len(arrayB) { - return false - } - - // Sort the first level of the schema by name so reordering the - // fields doesn't cause a diff. - if err := bigQueryTablecheckNameExists(arrayA); err != nil { - log.Printf("[DEBUG] unable to sort old schema by field name - %v", err) - return false - } - bigQueryTableSortArrayByName(arrayA) - if err := bigQueryTablecheckNameExists(arrayB); err != nil { - log.Printf("[DEBUG] unable to sort new schema by field name - %v", err) - return false - } - bigQueryTableSortArrayByName(arrayB) - - eq, err := jsonCompareWithMapKeyOverride(arrayA, arrayB, bigQueryTableMapKeyOverride) + eq, err := jsonCompareWithMapKeyOverride(name, a, b, bigQueryTableMapKeyOverride) if err != nil { log.Printf("[DEBUG] %v", err) log.Printf("[DEBUG] Error comparing JSON: %v, %v", old, new) diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index 5688afa0d787..96f65a10192d 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -79,7 +79,50 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ]`, ExpectDiffSuppress: true, }, - "nested lists": { + "nested field ordering changes": { + Old: `[ + { + "name": "someValue", + "type": "INTEGER", + "fields": [ + { + "name": "value1", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal" + }, + { + "name": "value2", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + } + ] + } + ]`, + New: `[ + { + "name": "someValue", + "type": "INTEGER", + "fields": [ + { + "name": "value2", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + }, + { + "name": "value1", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal" + } + ] + } + ]`, + ExpectDiffSuppress: true, + }, + "policyTags": { Old: `[ { "mode": "NULLABLE", @@ -680,7 +723,7 @@ func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) { if err := json.Unmarshal([]byte(testcase.jsonB), &b); err != nil { panic(fmt.Sprintf("unable to unmarshal json - %v", err)) } - eq, err := jsonCompareWithMapKeyOverride(a, b, bigQueryTableMapKeyOverride) + eq, err := jsonCompareWithMapKeyOverride("schema", a, b, bigQueryTableMapKeyOverride) if err != nil { t.Errorf("ahhhh an error I did not expect this! especially not on testscase %v - %s", i, err) } From a4bec31d5150b3aa4d42c0853dbdac14d891d541 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Wed, 2 Jun 2021 15:55:45 -0700 Subject: [PATCH 4/6] Merged BigQueryDataTableJSONEquivalencyTestCases into diff suppress test cases --- .../tests/resource_bigquery_table_test.go | 211 ++++++++---------- 1 file changed, 97 insertions(+), 114 deletions(-) diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index 96f65a10192d..439e05cfee93 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -12,6 +12,8 @@ import ( ) func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { + t.Parallel() + cases := map[string]struct { Old, New string ExpectDiffSuppress bool @@ -31,31 +33,77 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ]`, ExpectDiffSuppress: false, }, + "no change": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + ExpectDiffSuppress: true, + }, + "remove key": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + New: "[{\"name\": \"someValue\", \"finalKey\" : {} }]", + ExpectDiffSuppress: false, + }, + "empty description -> default description (empty)": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + ExpectDiffSuppress: true, + }, + "empty description -> other description": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", + ExpectDiffSuppress: false, + }, + "mode NULLABLE -> other mode": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", + ExpectDiffSuppress: false, + }, "mode NULLABLE -> default mode (also NULLABLE)": { Old: `[ { "mode": "NULLABLE", "name": "PageNo", "type": "INTEGER" - }, - { - "mode": "NULLABLE", - "name": "IngestTime", - "type": "TIMESTAMP" } ]`, New: `[ { "name": "PageNo", "type": "INTEGER" - }, - { - "name": "IngestTime", - "type": "TIMESTAMP" } ]`, ExpectDiffSuppress: true, }, + "type INTEGER -> INT64": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", + ExpectDiffSuppress: true, + }, + "type INTEGER -> other": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", + ExpectDiffSuppress: false, + }, + "type FLOAT -> FLOAT64": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", + ExpectDiffSuppress: true, + }, + "type FLOAT -> default": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + ExpectDiffSuppress: false, + }, + "type BOOLEAN -> BOOL": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", + ExpectDiffSuppress: true, + }, + "type BOOLEAN -> default": { + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + ExpectDiffSuppress: false, + }, "reordering fields": { Old: `[ { @@ -79,6 +127,31 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ]`, ExpectDiffSuppress: true, }, + "reordering fields with value change": { + Old: `[ + { + "name": "PageNo", + "type": "INTEGER", + "description": "someVal" + }, + { + "name": "IngestTime", + "type": "TIMESTAMP" + } + ]`, + New: `[ + { + "name": "IngestTime", + "type": "TIMESTAMP" + }, + { + "name": "PageNo", + "type": "INTEGER", + "description": "otherVal" + } + ]`, + ExpectDiffSuppress: false, + }, "nested field ordering changes": { Old: `[ { @@ -149,9 +222,21 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { } for tn, tc := range cases { - if bigQueryTableSchemaDiffSuppress("schema", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { - t.Errorf("bad: %s, %q => %q expect DiffSuppress to return %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) - } + tc := tc + t.Run(tn, func(t *testing.T) { + t.Parallel() + + var a, b interface{} + if err := json.Unmarshal([]byte(tc.Old), &a); err != nil { + t.Fatalf(fmt.Sprintf("unable to unmarshal old json - %v", err)) + } + if err := json.Unmarshal([]byte(tc.New), &b); err != nil { + t.Fatalf(fmt.Sprintf("unable to unmarshal new json - %v", err)) + } + if bigQueryTableSchemaDiffSuppress("schema", tc.Old, tc.New, nil) != tc.ExpectDiffSuppress { + t.Fatalf("bad: %s, %q => %q expect DiffSuppress to return %t", tn, tc.Old, tc.New, tc.ExpectDiffSuppress) + } + }) } } @@ -712,27 +797,6 @@ func TestAccBigQueryDataTable_expandArray(t *testing.T) { }) } -func TestUnitBigQueryDataTable_jsonEquivalency(t *testing.T) { - t.Parallel() - - for i, testcase := range testUnitBigQueryDataTableJSONEquivalencyTestCases { - var a, b interface{} - if err := json.Unmarshal([]byte(testcase.jsonA), &a); err != nil { - panic(fmt.Sprintf("unable to unmarshal json - %v", err)) - } - if err := json.Unmarshal([]byte(testcase.jsonB), &b); err != nil { - panic(fmt.Sprintf("unable to unmarshal json - %v", err)) - } - eq, err := jsonCompareWithMapKeyOverride("schema", a, b, bigQueryTableMapKeyOverride) - if err != nil { - t.Errorf("ahhhh an error I did not expect this! especially not on testscase %v - %s", i, err) - } - if eq != testcase.equivalent { - t.Errorf("expected equivalency result of %v but got %v for testcase number %v", testcase.equivalent, eq, i) - } - } -} - func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) { t.Parallel() for _, testcase := range testUnitBigQueryDataTableIsChangableTestCases { @@ -779,12 +843,6 @@ func TestAccBigQueryTable_allowDestroy(t *testing.T) { }) } -type testUnitBigQueryDataTableJSONEquivalencyTestCase struct { - jsonA string - jsonB string - equivalent bool -} - type testUnitBigQueryDataTableJSONChangeableTestCase struct { name string jsonOld string @@ -906,81 +964,6 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ }, } -var testUnitBigQueryDataTableJSONEquivalencyTestCases = []testUnitBigQueryDataTableJSONEquivalencyTestCase{ - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - "[{\"name\": \"someValue\", \"finalKey\" : {} }]", - false, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", - false, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", - false, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", - false, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", - false, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", - true, - }, - { - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", - false, - }, - { - // order changes, name same - "[{\"name\": \"value1\", \"1\" : \"1\"},{\"name\": \"value2\", \"2\" : \"2\" }]", - "[{\"name\": \"value2\", \"2\" : \"2\" },{\"name\": \"value1\", \"1\" : \"1\" }]", - true, - }, - { - // order changes, value different - "[{\"name\": \"value1\", \"1\" : \"1\"},{\"name\": \"value2\", \"2\" : \"2\" }]", - "[{\"name\": \"value2\", \"2\" : \"random\" },{\"name\": \"value1\", \"1\" : \"1\" }]", - false, - }, -} - func testAccCheckBigQueryExtData(t *testing.T, expectedQuoteChar string) resource.TestCheckFunc { return func(s *terraform.State) error { for _, rs := range s.RootModule().Resources { From a18155899d2ccedc59a4a591960ccd1edef93043 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Thu, 3 Jun 2021 08:25:41 -0700 Subject: [PATCH 5/6] Formatting cleanup --- .../resources/resource_bigquery_table.go | 2 +- .../tests/resource_bigquery_table_test.go | 44 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/mmv1/third_party/terraform/resources/resource_bigquery_table.go b/mmv1/third_party/terraform/resources/resource_bigquery_table.go index deadd8274e0e..748d6526ad23 100644 --- a/mmv1/third_party/terraform/resources/resource_bigquery_table.go +++ b/mmv1/third_party/terraform/resources/resource_bigquery_table.go @@ -56,7 +56,7 @@ func jsonCompareWithMapKeyOverride(key string, a, b interface{}, compareMapKeyVa } // Sort fields by name so reordering them doesn't cause a diff. - if (key == "schema" || key == "fields") { + if key == "schema" || key == "fields" { if err := bigQueryTablecheckNameExists(arrayA); err != nil { return false, err } diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index 439e05cfee93..4df923873ba9 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -34,28 +34,28 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ExpectDiffSuppress: false, }, "no change": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", ExpectDiffSuppress: true, }, "remove key": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", - New: "[{\"name\": \"someValue\", \"finalKey\" : {} }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"finalKey\" : {} }]", + New: "[{\"name\": \"someValue\", \"finalKey\" : {} }]", ExpectDiffSuppress: false, }, "empty description -> default description (empty)": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", ExpectDiffSuppress: true, }, "empty description -> other description": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"description\": \"somethingRandom\" }]", ExpectDiffSuppress: false, }, "mode NULLABLE -> other mode": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"NULLABLE\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"mode\": \"somethingRandom\" }]", ExpectDiffSuppress: false, }, "mode NULLABLE -> default mode (also NULLABLE)": { @@ -75,33 +75,33 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ExpectDiffSuppress: true, }, "type INTEGER -> INT64": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INT64\" }]", ExpectDiffSuppress: true, }, "type INTEGER -> other": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"INTEGER\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"somethingRandom\" }]", ExpectDiffSuppress: false, }, "type FLOAT -> FLOAT64": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT64\" }]", ExpectDiffSuppress: true, }, "type FLOAT -> default": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"FLOAT\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", ExpectDiffSuppress: false, }, "type BOOLEAN -> BOOL": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOL\" }]", ExpectDiffSuppress: true, }, "type BOOLEAN -> default": { - Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", - New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", + Old: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\", \"type\": \"BOOLEAN\" }]", + New: "[{\"name\": \"someValue\", \"anotherKey\" : \"anotherValue\" }]", ExpectDiffSuppress: false, }, "reordering fields": { From 42de68eb333462c54bb8d261775252e0037b5402 Mon Sep 17 00:00:00 2001 From: Stephen Lewis Date: Thu, 3 Jun 2021 10:03:52 -0700 Subject: [PATCH 6/6] Added a test with multiple levels of reordering and also policyTags at multiple levels --- .../tests/resource_bigquery_table_test.go | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go index 4df923873ba9..2976e1aeae3e 100644 --- a/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/tests/resource_bigquery_table_test.go @@ -219,6 +219,125 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { ]`, ExpectDiffSuppress: true, }, + "multiple levels of reordering with policyTags set": { + Old: `[ + { + "mode": "NULLABLE", + "name": "providerphone", + "type":"STRING", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + }, + "fields": [ + { + "name": "value1", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + } + }, + { + "name": "value2", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + } + ] + }, + { + "name": "PageNo", + "type": "INTEGER" + }, + { + "name": "IngestTime", + "type": "TIMESTAMP", + "fields": [ + { + "name": "value3", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + } + }, + { + "name": "value4", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + } + ] + } + ]`, + New: `[ + { + "name": "IngestTime", + "type": "TIMESTAMP", + "fields": [ + { + "name": "value4", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + }, + { + "name": "value3", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + } + } + ] + }, + { + "mode": "NULLABLE", + "name": "providerphone", + "type":"STRING", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + }, + "fields": [ + { + "name": "value1", + "type": "INTEGER", + "mode": "NULLABLE", + "description": "someVal", + "policyTags": { + "names": [ + "projects/my-project/locations/us/taxonomies/12345678/policyTags/12345678" + ] + } + }, + { + "name": "value2", + "type": "BOOLEAN", + "mode": "NULLABLE", + "description": "someVal" + } + ] + }, + { + "name": "PageNo", + "type": "INTEGER" + } + ]`, + ExpectDiffSuppress: true, + }, } for tn, tc := range cases {