Corrected metadata generation for Map fields (#15960)

Author: melinath

mmv1/api/metadata/field.go

@@ -1,15 +1,28 @@
 package metadata
 
 import (
+	"slices"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/magic-modules/mmv1/api"
 	"github.com/GoogleCloudPlatform/magic-modules/mmv1/google"
 )
 
 func FromProperties(props []*api.Type) []Field {
+	// Sort props by lineage
+	slices.SortFunc(props, func(a, b *api.Type) int {
+		if strings.Join(a.Lineage(), ".") < strings.Join(b.Lineage(), ".") {
+			return -1
+		}
+		return 1
+	})
+
 	var fields []Field
 	for _, p := range props {
+		// Skip non-maps with nested fields
+		if !p.IsA("Map") && len(p.NestedProperties()) > 0 {
+			continue
+		}
 		f := Field{
 			Json:         p.IsJsonField(),
 			ProviderOnly: p.ProviderOnly(),
@@ -22,6 +35,13 @@ func FromProperties(props []*api.Type) []Field {
 		if p.ProviderOnly() || !IsDefaultLineage(lineage, apiLineage) {
 			f.Field = strings.Join(lineage, ".")
 		}
+		// For maps (which all have nested children), modify the entry slightly; the map field itself is skipped,
+		// but we need a `key` API field that corresponds to the key_name of the map field.
+		if p.IsA("Map") {
+			f.ApiField += ".key"
+			f.Field = strings.Join(append(lineage, p.KeyName), ".")
+		}
+
 		fields = append(fields, f)
 	}
 	return fields

mmv1/api/metadata/field_test.go

@@ -4,13 +4,16 @@ import (
 	"testing"
 
 	"github.com/GoogleCloudPlatform/magic-modules/mmv1/api"
+	"github.com/GoogleCloudPlatform/magic-modules/mmv1/google"
 	"github.com/google/go-cmp/cmp"
 )
 
 func TestFromProperties(t *testing.T) {
 	cases := []struct {
 		name             string
 		resourceMetadata *api.Resource
+		virtualFields    []*api.Type
+		parameters       []*api.Type
 		properties       []*api.Type
 		wantFields       []Field
 	}{
@@ -59,6 +62,113 @@ func TestFromProperties(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "nested field",
+			properties: []*api.Type{
+				{
+					Name: "root",
+					Type: "NestedObject",
+					Properties: []*api.Type{
+						{
+							Name: "foo",
+							Type: "NestedObject",
+							Properties: []*api.Type{
+								{
+									Name: "bars",
+									Type: "Array",
+									ItemType: &api.Type{
+										Type: "NestedObject",
+										Properties: []*api.Type{
+											{
+												Name: "fooBar",
+												Type: "String",
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantFields: []Field{
+				{
+					ApiField: "root.foo.bars.fooBar",
+				},
+			},
+		},
+		{
+			name: "nested virtual",
+			virtualFields: []*api.Type{
+				{
+					Name: "root",
+					Type: "NestedObject",
+					Properties: []*api.Type{
+						{
+							Name: "foo",
+							Type: "String",
+						},
+					},
+				},
+			},
+			wantFields: []Field{
+				{
+					Field:        "root.foo",
+					ProviderOnly: true,
+				},
+			},
+		},
+		{
+			name: "nested param",
+			parameters: []*api.Type{
+				{
+					Name: "root",
+					Type: "NestedObject",
+					Properties: []*api.Type{
+						{
+							Name:         "foo",
+							Type:         "String",
+							UrlParamOnly: true,
+						},
+					},
+				},
+			},
+			wantFields: []Field{
+				{
+					Field:        "root.foo",
+					ProviderOnly: true,
+				},
+			},
+		},
+		{
+			name: "map",
+			properties: []*api.Type{
+				{
+					Name:    "root",
+					Type:    "Map",
+					KeyName: "whatever",
+					ValueType: &api.Type{
+						Type: "NestedObject",
+						Properties: []*api.Type{
+							{
+								Name: "foo",
+								Type: "String",
+							},
+						},
+					},
+				},
+			},
+			wantFields: []Field{
+				{
+					Field:    "root.whatever",
+					ApiField: "root.key",
+				},
+				{
+					Field:    "root.foo",
+					ApiField: "root.value.foo",
+				},
+			},
+		},
 	}
 
 	for _, tc := range cases {
@@ -69,12 +179,12 @@ func TestFromProperties(t *testing.T) {
 			if r == nil {
 				r = &api.Resource{}
 			}
+			r.VirtualFields = tc.virtualFields
+			r.Parameters = tc.parameters
+			r.Properties = tc.properties
+			r.SetDefault(&api.Product{})
 
-			for _, p := range tc.properties {
-				p.SetDefault(r)
-			}
-
-			got := FromProperties(tc.properties)
+			got := FromProperties(r.AllNestedProperties(google.Concat(r.RootProperties(), r.UserVirtualFields())))
 			if diff := cmp.Diff(tc.wantFields, got); diff != "" {
 				t.Errorf("FromProperties() mismatch (-want +got):\n%s", diff)
 			}

mmv1/api/metadata/metadata.go

@@ -2,6 +2,7 @@ package metadata
 
 import (
 	"github.com/GoogleCloudPlatform/magic-modules/mmv1/api"
+	"github.com/GoogleCloudPlatform/magic-modules/mmv1/google"
 )
 
 // FromResource returns a Metadata object based on a Resource.
@@ -16,7 +17,7 @@ func FromResource(r api.Resource) Metadata {
 		CaiAssetNameFormat:  r.CAIFormatOverride(),
 		ApiVariantPatterns:  r.ApiVariantPatterns,
 		AutogenStatus:       r.AutogenStatus != "",
-		Fields:              FromProperties(r.LeafProperties()),
+		Fields:              FromProperties(r.AllNestedProperties(google.Concat(r.RootProperties(), r.UserVirtualFields()))),
 	}
 
 	if m.ApiVersion == "" {

mmv1/api/resource.go

@@ -752,28 +752,6 @@ func (r Resource) RootProperties() []*Type {
 	return props
 }
 
-// Returns a sorted list of all "leaf" properties, meaning properties that have
-// no children.
-func (r Resource) LeafProperties() []*Type {
-	types := r.AllNestedProperties(google.Concat(r.RootProperties(), r.UserVirtualFields()))
-
-	// Remove types that have children, because we only want "leaf" fields
-	types = slices.DeleteFunc(types, func(t *Type) bool {
-		nestedProperties := t.NestedProperties()
-		return len(nestedProperties) > 0
-	})
-
-	// Sort types by lineage
-	slices.SortFunc(types, func(a, b *Type) int {
-		if strings.Join(a.Lineage(), ".") < strings.Join(b.Lineage(), ".") {
-			return -1
-		}
-		return 1
-	})
-
-	return types
-}
-
 // Return the product-level async object, or the resource-specific one
 // if one exists.
 func (r Resource) GetAsync() *Async {

mmv1/api/resource_test.go

@@ -204,123 +204,6 @@ func TestResourceServiceVersion(t *testing.T) {
 	}
 }
 
-func TestLeafProperties(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		description string
-		obj         Resource
-		expected    Type
-	}{
-		{
-			description: "non-nested type",
-			obj: Resource{
-				BaseUrl: "test",
-				Properties: []*Type{
-					{
-						Name: "basic",
-						Type: "String",
-					},
-				},
-			},
-			expected: Type{
-				Name: "basic",
-			},
-		},
-		{
-			description: "nested type",
-			obj: Resource{
-				BaseUrl: "test",
-				Properties: []*Type{
-					{
-						Name: "root",
-						Type: "NestedObject",
-						Properties: []*Type{
-							{
-								Name: "foo",
-								Type: "NestedObject",
-								Properties: []*Type{
-									{
-										Name: "bars",
-										Type: "Array",
-										ItemType: &Type{
-											Type: "NestedObject",
-											Properties: []*Type{
-												{
-													Name: "fooBar",
-													Type: "String",
-												},
-											},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			expected: Type{
-				Name: "fooBar",
-			},
-		},
-		{
-			description: "nested virtual",
-			obj: Resource{
-				BaseUrl: "test",
-				VirtualFields: []*Type{
-					{
-						Name: "root",
-						Type: "NestedObject",
-						Properties: []*Type{
-							{
-								Name: "foo",
-								Type: "String",
-							},
-						},
-					},
-				},
-			},
-			expected: Type{
-				Name: "foo",
-			},
-		},
-		{
-			description: "nested param",
-			obj: Resource{
-				BaseUrl: "test",
-				Parameters: []*Type{
-					{
-						Name: "root",
-						Type: "NestedObject",
-						Properties: []*Type{
-							{
-								Name: "foo",
-								Type: "String",
-							},
-						},
-					},
-				},
-			},
-			expected: Type{
-				Name: "foo",
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		tc := tc
-
-		t.Run(tc.description, func(t *testing.T) {
-			t.Parallel()
-
-			tc.obj.SetDefault(nil)
-			if got, want := tc.obj.LeafProperties(), tc.expected; got[0].Name != want.Name {
-				t.Errorf("expected %q to be %q", got[0].Name, want.Name)
-			}
-		})
-	}
-}
-
 // TestMagicianLocation verifies that the current package is being executed from within
 // the RELATIVE_MAGICIAN_LOCATION ("mmv1/") directory structure. This ensures that references
 // to files relative to this location will remain valid even if the repository structure

mmv1/api/type.go

@@ -531,8 +531,8 @@ func (t Type) Lineage() []string {
 		return []string{google.Underscore(t.Name)}
 	}
 
-	// Skip arrays because otherwise the array will be included twice
-	if t.ParentMetadata.IsA("Array") {
+	// Skip arrays & maps because otherwise the parent field name will be duplicated
+	if t.ParentMetadata.IsA("Array") || t.ParentMetadata.IsA("Map") {
 		return t.ParentMetadata.Lineage()
 	}
 
@@ -555,6 +555,12 @@ func (t Type) ApiLineage() []string {
 		return t.ParentMetadata.ApiLineage()
 	}
 
+	// Insert `value` for children of Map fields, and exclude this type because
+	// it will have the same Name as the parent field.
+	if t.ParentMetadata.IsA("Map") {
+		return append(t.ParentMetadata.ApiLineage(), "value")
+	}
+
 	return append(t.ParentMetadata.ApiLineage(), t.ApiName)
 }