Cleaned up and fixed Lineage methods (#15949)

Author: melinath

mmv1/api/metadata/field.go

@@ -1,7 +1,10 @@
 package metadata
 
 import (
+	"strings"
+
 	"github.com/GoogleCloudPlatform/magic-modules/mmv1/api"
+	"github.com/GoogleCloudPlatform/magic-modules/mmv1/google"
 )
 
 func FromProperties(props []*api.Type) []Field {
@@ -11,11 +14,13 @@ func FromProperties(props []*api.Type) []Field {
 			Json:         p.IsJsonField(),
 			ProviderOnly: p.ProviderOnly(),
 		}
+		lineage := p.Lineage()
+		apiLineage := p.ApiLineage()
 		if !p.ProviderOnly() {
-			f.ApiField = p.MetadataApiLineage()
+			f.ApiField = strings.Join(apiLineage, ".")
 		}
-		if p.ProviderOnly() || p.MetadataLineage() != p.MetadataDefaultLineage() {
-			f.Field = p.MetadataLineage()
+		if p.ProviderOnly() || !IsDefaultLineage(lineage, apiLineage) {
+			f.Field = strings.Join(lineage, ".")
 		}
 		fields = append(fields, f)
 	}
@@ -36,3 +41,18 @@ type Field struct {
 	// have `api_field` set to `*`.
 	Json bool `yaml:"json,omitempty"`
 }
+
+// Returns true if the lineage is the default we'd expect for a field, and false otherwise.
+// If any ancestor has a non-default lineage, this will return false.
+func IsDefaultLineage(lineage, apiLineage []string) bool {
+	if len(lineage) != len(apiLineage) {
+		return false
+	}
+	for i, part := range lineage {
+		apiPart := apiLineage[i]
+		if part != google.Underscore(apiPart) {
+			return false
+		}
+	}
+	return true
+}

mmv1/api/metadata/field_test.go

@@ -81,3 +81,70 @@ func TestFromProperties(t *testing.T) {
 		})
 	}
 }
+
+func TestIsDefaultLineage(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name       string
+		lineage    []string
+		apiLineage []string
+		want       bool
+	}{
+		{
+			name:       "empty",
+			lineage:    []string{},
+			apiLineage: []string{},
+			want:       true,
+		},
+		{
+			name:       "single field",
+			lineage:    []string{"foo_bar"},
+			apiLineage: []string{"fooBar"},
+			want:       true,
+		},
+		{
+			name:       "multiple fields",
+			lineage:    []string{"foo_bar", "baz_moo"},
+			apiLineage: []string{"fooBar", "bazMoo"},
+			want:       true,
+		},
+		{
+			name:       "longer lineage",
+			lineage:    []string{"foo_bar", "baz_moo"},
+			apiLineage: []string{"fooBar"},
+			want:       false,
+		},
+		{
+			name:       "longer apiLineage",
+			lineage:    []string{"foo_bar"},
+			apiLineage: []string{"fooBar", "bazMoo"},
+			want:       false,
+		},
+		{
+			name:       "parent override",
+			lineage:    []string{"foo_bar", "baz_moo"},
+			apiLineage: []string{"otherName", "bazMoo"},
+			want:       false,
+		},
+		{
+			name:       "child override",
+			lineage:    []string{"foo_bar", "baz_moo"},
+			apiLineage: []string{"fooBar", "otherName"},
+			want:       false,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := IsDefaultLineage(tc.lineage, tc.apiLineage)
+			if got != tc.want {
+				t.Errorf("IsDefaultLineage(%s) failed; want %t, got %t", tc.name, tc.want, got)
+			}
+		})
+	}
+}

mmv1/api/resource.go

@@ -678,7 +678,7 @@ func (r Resource) SensitivePropsToString() string {
 	var props []string
 
 	for _, prop := range r.SensitiveProps() {
-		props = append(props, fmt.Sprintf("`%s`", prop.Lineage()))
+		props = append(props, fmt.Sprintf("`%s`", strings.Join(prop.Lineage(), ".")))
 	}
 
 	return strings.Join(props, ", ")
@@ -688,7 +688,7 @@ func (r Resource) WriteOnlyPropsToString() string {
 	var props []string
 
 	for _, prop := range r.WriteOnlyProps() {
-		props = append(props, fmt.Sprintf("`%s`", prop.Lineage()))
+		props = append(props, fmt.Sprintf("`%s`", strings.Join(prop.Lineage(), ".")))
 	}
 
 	return strings.Join(props, ", ")
@@ -765,7 +765,7 @@ func (r Resource) LeafProperties() []*Type {
 
 	// Sort types by lineage
 	slices.SortFunc(types, func(a, b *Type) int {
-		if a.MetadataLineage() < b.MetadataLineage() {
+		if strings.Join(a.Lineage(), ".") < strings.Join(b.Lineage(), ".") {
 			return -1
 		}
 		return 1
@@ -855,7 +855,7 @@ func (r *Resource) attachConstraintGroup(groupType string, source []string) *[]s
 }
 
 func buildWriteOnlyField(name string, versionFieldName string, originalField *Type) *Type {
-	originalFieldLineage := originalField.TerraformLineage()
+	originalFieldLineage := strings.Join(originalField.Lineage(), ".0.")
 	newFieldLineage := strings.ReplaceAll(originalFieldLineage, google.Underscore(originalField.Name), google.Underscore(name))
 	requiredWith := strings.ReplaceAll(originalFieldLineage, google.Underscore(originalField.Name), google.Underscore(versionFieldName))
 
@@ -907,7 +907,7 @@ func buildWriteOnlyField(name string, versionFieldName string, originalField *Ty
 
 func buildWriteOnlyVersionField(name string, originalField *Type, writeOnlyField *Type) *Type {
 	description := fmt.Sprintf("Triggers update of `%s` write-only. Increment this value when an update to `%s` is needed. For more info see [updating write-only arguments](/docs/providers/google/guides/using_write_only_arguments.html#updating-write-only-arguments)", google.Underscore(writeOnlyField.Name), google.Underscore(writeOnlyField.Name))
-	requiredWith := strings.ReplaceAll(originalField.TerraformLineage(), google.Underscore(originalField.Name), google.Underscore(writeOnlyField.Name))
+	requiredWith := strings.ReplaceAll(strings.Join(originalField.Lineage(), ".0."), google.Underscore(originalField.Name), google.Underscore(writeOnlyField.Name))
 
 	options := []func(*Type){
 		propertyWithType("Int"),
@@ -1069,7 +1069,7 @@ func (r Resource) IgnoreReadLabelsFields(props []*Type) []string {
 		if p.IsA("KeyValueLabels") ||
 			p.IsA("KeyValueTerraformLabels") ||
 			p.IsA("KeyValueAnnotations") {
-			fields = append(fields, p.TerraformLineage())
+			fields = append(fields, strings.Join(p.Lineage(), ".0."))
 		} else if p.IsA("NestedObject") && len(p.AllProperties()) > 0 {
 			fields = google.Concat(fields, r.IgnoreReadLabelsFields(p.AllProperties()))
 		}
@@ -1466,7 +1466,7 @@ func ignoreReadFields(props []*Type) []string {
 	var fields []string
 	for _, tp := range props {
 		if tp.IgnoreRead && !tp.UrlParamOnly && !tp.IsA("ResourceRef") {
-			fields = append(fields, tp.TerraformLineage())
+			fields = append(fields, strings.Join(tp.Lineage(), ".0."))
 		} else if tp.IsA("NestedObject") && tp.AllProperties() != nil {
 			fields = append(fields, ignoreReadFields(tp.AllProperties())...)
 		}
@@ -2546,13 +2546,13 @@ func (r Resource) TGCTestIgnorePropertiesToStrings() []string {
 		"timeouts",
 	}
 	for _, tp := range r.VirtualFields {
-		props = append(props, tp.MetadataLineage())
+		props = append(props, strings.Join(tp.Lineage(), "."))
 	}
 	for _, tp := range r.AllNestedProperties(r.RootProperties()) {
 		if tp.UrlParamOnly {
 			props = append(props, google.Underscore(tp.Name))
 		} else if tp.IsMissingInCai || tp.IgnoreRead || tp.ClientSide || tp.WriteOnlyLegacy {
-			props = append(props, tp.MetadataLineage())
+			props = append(props, strings.Join(tp.Lineage(), "."))
 		}
 	}
 

mmv1/api/type.go

@@ -524,82 +524,38 @@ func (t *Type) Validate(rName string) {
 // check the allowed types for Type field
 // check the allowed fields for each type, for example, KeyName is only allowed for Map
 
-// Prints a dot notation path to where the field is nested within the parent
-// object. eg: parent.meta.label.foo
-// The only intended purpose is to allow better error messages. Some objects
-// and at some points in the build this doesn't output a valid output.
-func (t Type) Lineage() string {
-	if t.ParentMetadata == nil {
-		return google.Underscore(t.Name)
-	}
-
-	return fmt.Sprintf("%s.%s", t.ParentMetadata.Lineage(), google.Underscore(t.Name))
-}
-
-// Returns the actual Terraform lineage for the field, formatted for resource metadata.
-// This will return a simple dot notation path, like: foo_field.bar_field
-func (t Type) MetadataLineage() string {
+// Returns a slice of Terraform field names representing where the field is nested within the parent resource.
+// For example, []string{"parent_field", "meta", "label", "foo_bar"}.
+func (t Type) Lineage() []string {
 	if t.ParentMetadata == nil || t.ParentMetadata.FlattenObject {
-		return google.Underscore(t.Name)
+		return []string{google.Underscore(t.Name)}
 	}
 
-	// Skip arrays because otherwise the array name will be included twice
+	// Skip arrays because otherwise the array will be included twice
 	if t.ParentMetadata.IsA("Array") {
-		return t.ParentMetadata.MetadataLineage()
+		return t.ParentMetadata.Lineage()
 	}
 
-	return fmt.Sprintf("%s.%s", t.ParentMetadata.MetadataLineage(), google.Underscore(t.Name))
+	return append(t.ParentMetadata.Lineage(), google.Underscore(t.Name))
 }
 
-// Returns the default Terraform lineage for the field, based on converting MetadataApiLineage
-// to snake_case. This is used to determine whether an explicit Terraform field name is required.
-// This will return a simple dot notation path like: foo_field.bar_field
-func (t Type) MetadataDefaultLineage() string {
-	apiLineage := t.MetadataApiLineage()
-	parts := strings.Split(apiLineage, ".")
-	var snakeParts []string
-	for _, p := range parts {
-		snakeParts = append(snakeParts, google.Underscore(p))
-	}
-	return strings.Join(snakeParts, ".")
-}
-
-// Returns the actual API lineage for the field (that is, using API names), formatted for
-// resource metadata. This will return a simple dot notation path, like: fooField.barField
-// This format is intended for to represent an API type.
-func (t Type) MetadataApiLineage() string {
-	apiName := t.ApiName
+// Returns a slice of API field names representing where the field is nested within the parent resource.
+// For example, []string{"parentField", "meta", "label", "fooBar"}. For fine-grained resources, this will
+// include the field on the API resource that the fine-grained resource manages.
+func (t Type) ApiLineage() []string {
 	if t.ParentMetadata == nil {
 		if !t.UrlParamOnly && t.ResourceMetadata.ApiResourceField != "" {
-			apiName = fmt.Sprintf("%s.%s", t.ResourceMetadata.ApiResourceField, apiName)
+			return []string{t.ResourceMetadata.ApiResourceField, t.ApiName}
 		}
-		return apiName
+		return []string{t.ApiName}
 	}
 
+	// Skip arrays because otherwise the array will be included twice
 	if t.ParentMetadata.IsA("Array") {
-		return t.ParentMetadata.MetadataApiLineage()
-	}
-
-	return fmt.Sprintf("%s.%s", t.ParentMetadata.MetadataApiLineage(), apiName)
-}
-
-// Returns the lineage in snake case
-func (t Type) LineageAsSnakeCase() string {
-	if t.ParentMetadata == nil {
-		return google.Underscore(t.Name)
-	}
-
-	return fmt.Sprintf("%s_%s", t.ParentMetadata.LineageAsSnakeCase(), google.Underscore(t.Name))
-}
-
-// Prints the access path of the field in the configration eg: metadata.0.labels
-// The only intended purpose is to get the value of the labes field by calling d.Get().
-func (t Type) TerraformLineage() string {
-	if t.ParentMetadata == nil || t.ParentMetadata.FlattenObject {
-		return google.Underscore(t.Name)
+		return t.ParentMetadata.ApiLineage()
 	}
 
-	return fmt.Sprintf("%s.0.%s", t.ParentMetadata.TerraformLineage(), google.Underscore(t.Name))
+	return append(t.ParentMetadata.ApiLineage(), t.ApiName)
 }
 
 func (t Type) EnumValuesToString(quoteSeperator string, addEmpty bool) string {
@@ -1114,7 +1070,7 @@ func (t Type) AllProperties() []*Type {
 func (t Type) UserProperties() []*Type {
 	if t.IsA("NestedObject") {
 		if t.Properties == nil {
-			log.Fatalf("Field '{%s}' properties are nil!", t.Lineage())
+			log.Fatalf("Field '{%s}' properties are nil!", strings.Join(t.Lineage(), "."))
 		}
 
 		return google.Reject(t.Properties, func(p *Type) bool {
@@ -1268,7 +1224,7 @@ func propertyWithAtLeastOneOfPointer(ptr *[]string) func(*Type) {
 func (t *Type) validateLabelsField() {
 	productName := t.ResourceMetadata.ProductMetadata.Name
 	resourceName := t.ResourceMetadata.Name
-	lineage := t.Lineage()
+	lineage := strings.Join(t.Lineage(), ".")
 	if lineage == "labels" || lineage == "metadata.labels" || lineage == "configuration.labels" {
 		if !t.IsA("KeyValueLabels") &&
 			// The label value must be empty string, so skip this resource