Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made bigquery table schema diffsuppress func handle nested lists gracefully #4832

Merged
merged 6 commits into from
Jun 4, 2021
Merged

Made bigquery table schema diffsuppress func handle nested lists gracefully #4832

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff3c099
Added basic tests for biquery table schema diff suppress
melinath Jun 2, 2021
d003fc9
Only sort the top level of a bigquery schema when checking diff suppr…
melinath Jun 2, 2021
5e94dd0
Keep suppressing diff for subfield reordering
melinath Jun 2, 2021
a4bec31
Merged BigQueryDataTableJSONEquivalencyTestCases into diff suppress t…
melinath Jun 2, 2021
a181558
Formatting cleanup
melinath Jun 3, 2021
42de68e
Added a test with multiple levels of reordering and also policyTags a…
melinath Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions mmv1/third_party/terraform/resources/resource_bigquery_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A field can have fields nested inside of it [schema], so we still need to sort within the JSON function still, right? With this change reordering nested fields fields won't work as I'm reading it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I didn't know about nested fields! I'll make this support both.

// 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)
Expand Down
101 changes: 101 additions & 0 deletions mmv1/third_party/terraform/tests/resource_bigquery_table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,107 @@ 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,
},
"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 {
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()

Expand Down