From 9a4c92bb1b911cca78eb1e398741f54c606986cd Mon Sep 17 00:00:00 2001
From: Dana Hoffman <danahoffman@google.com>
Date: Thu, 22 Feb 2018 09:27:02 -0800
Subject: [PATCH] add json omitted fields back when converting (#1098)

* add json omitted fields back when converting

* for testing: don't use json in convert

* try a combination of structs and mapstructure libraries

* Revert "try a combination of structs and mapstructure libraries"

This reverts commit eab11aa95d3abb74b240988e5c99d6e9525db96c.

* Revert "for testing: don't use json in convert"

This reverts commit 96af067b29dd147fcedb55995ebc8a17c6a9d1b2.
---
 google/api_versions.go      | 53 ++++++++++++++++++++++++-
 google/api_versions_test.go | 78 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/google/api_versions.go b/google/api_versions.go
index b54fa7a0..1013f1cf 100644
--- a/google/api_versions.go
+++ b/google/api_versions.go
@@ -3,6 +3,7 @@ package google
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 )
 
@@ -26,7 +27,7 @@ var OrderedContainerApiVersions = []ApiVersion{
 
 // Convert between two types by converting to/from JSON. Intended to switch
 // between multiple API versions, as they are strict supersets of one another.
-// Convert loses information about ForceSendFields and NullFields.
+// item and out are pointers to structs
 func Convert(item, out interface{}) error {
 	bytes, err := json.Marshal(item)
 	if err != nil {
@@ -38,9 +39,59 @@ func Convert(item, out interface{}) error {
 		return err
 	}
 
+	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())
+			}
+		}
+	}
+}
+
 type TerraformResourceData interface {
 	HasChange(string) bool
 	GetOk(string) (interface{}, bool)
diff --git a/google/api_versions_test.go b/google/api_versions_test.go
index b4d06ade..c18bee62 100644
--- a/google/api_versions_test.go
+++ b/google/api_versions_test.go
@@ -1,6 +1,9 @@
 package google
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+)
 
 type ExpectedApiVersions struct {
 	Create     ApiVersion
@@ -209,6 +212,79 @@ func TestApiVersion(t *testing.T) {
 	}
 }
 
+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