Refactor SetResourceForStruct to iterate over targetShape members (#289)

Issue #, if available: aws-controllers-k8s/community#1146

After a recent update to `/pkg/model/field.go`, shapes _may_ contain **additional** fields/members with a "nested" format such as **DHCPOptions.Fields**: `[DHCPConfigurations TagSpecifications.Tags.Value Tags.Value TagSpecifications DHCPConfigurations.Values TagSpecifications.ResourceType TagSpecifications.Tags Tags.Key DHCPOptionsID OwnerID DHCPConfigurations.Key TagSpecifications.Tags.Key DryRun Tags]` The purpose of that update is to allow users to easily fetch nested shapes and their corresponding configurations using the "nested" field name as the key.

However, `SetResourceForStruct` will never have the opportunity to process and/or fetch shapes and configs for these "nested" fields because the algorithm processes fields **iff they exist in sourceShape**. *Note, sourceShape comes directly from `aws-sdk` in this context, so will NOT have these additional, nested fields.*

This patch iterates over `targetShape.Members` instead of `sourceShape` so these "nested" fields can be included in the lookup logic, but continues to use `sourceShape` index for consistency throughout codebase. There's no change in functionality, but this paves the way for a follow-up PR to check a targetShape's nested Field `SetConfig` if the Field cannot be found in the sourceShape: 

```
sourceMemberShapeRef := sourceShape.MemberRefs[targetMemberName]
		if sourceMemberShapeRef == nil {
			continue
		}

                // Future patch: check if targetMemberName has SetConfig logic that needs to be processed
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: brycahta

pkg/generate/code/common.go

@@ -14,6 +14,7 @@
 package code
 
 import (
+	"fmt"
 	"strings"
 
 	awssdkmodel "github.com/aws/aws-sdk-go/private/model/api"
@@ -188,3 +189,14 @@ func FindPrimaryIdentifierFieldNames(
 	}
 	return crField, shapeField
 }
+
+// GetMemberIndex returns the index of memberName by iterating over
+// shape's slice of member names for deterministic ordering
+func GetMemberIndex(shape *awssdkmodel.Shape, memberName string) (int, error) {
+	for index, shapeMemberName := range shape.MemberNames() {
+		if strings.EqualFold(shapeMemberName, memberName) {
+			return index, nil
+		}
+	}
+	return -1, fmt.Errorf("Could not find %s in shape %s", memberName, shape.ShapeName)
+}

pkg/generate/code/common_test.go

@@ -176,4 +176,27 @@ func TestFindPluralizedIdentifiersInShape_EC2_SG_ReadMany_Rename(t *testing.T) {
 
 	assert.Equal(expModelIdentifier, crIdentifier)
 	assert.Equal(expShapeIdentifier, shapeIdentifier)
-}
+}
+
+func TestGetMemberIndex_EC2_DHCPOptions_ReadMany(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForService(t, "ec2")
+
+	crd := testutil.GetCRDByName(t, g, "DhcpOptions")
+	require.NotNil(crd)
+
+	op := crd.Ops.ReadMany
+	inputShape := op.InputRef.Shape
+	// inputShape members: [DhcpOptionsIds DryRun Filters MaxResults NextToken]
+
+	memberIndex, _ := code.GetMemberIndex(inputShape, "DhcpOptionsIds")
+	assert.Equal(0, memberIndex)
+	memberIndex, _ = code.GetMemberIndex(inputShape, "MaxResults")
+	assert.Equal(3, memberIndex)
+
+	memberIndex, err := code.GetMemberIndex(inputShape, "notValidMember")
+	assert.Equal(-1, memberIndex)
+	assert.NotNil(err)
+}

pkg/generate/code/set_resource.go

@@ -1226,53 +1226,63 @@ func SetResourceForStruct(
 	sourceShape := sourceShapeRef.Shape
 	targetShape := targetShapeRef.Shape
 
-	for memberIndex, memberName := range sourceShape.MemberNames() {
-		targetMemberShapeRef := targetShape.MemberRefs[memberName]
-		if targetMemberShapeRef == nil {
+	for _, targetMemberName := range targetShape.MemberNames() {
+		sourceMemberShapeRef := sourceShape.MemberRefs[targetMemberName]
+		if sourceMemberShapeRef == nil {
 			continue
 		}
-		memberVarName := fmt.Sprintf("%sf%d", targetVarName, memberIndex)
-		memberShapeRef := sourceShape.MemberRefs[memberName]
-		memberShape := memberShapeRef.Shape
-		cleanNames := names.New(memberName)
-		sourceAdaptedVarName := sourceVarName + "." + memberName
+		// Upstream logic iterates over sourceShape members and therefore uses
+		// the sourceShape's index; continue using sourceShape's index here for consistency.
+		sourceMemberIndex, err := GetMemberIndex(sourceShape, targetMemberName)
+		if err != nil {
+			msg := fmt.Sprintf(
+				"could not determine source shape index: %v", err)
+			panic(msg)
+		}
+
+		targetMemberShapeRef := targetShape.MemberRefs[targetMemberName]
+		indexedVarName := fmt.Sprintf("%sf%d", targetVarName, sourceMemberIndex)
+		sourceMemberShape := sourceMemberShapeRef.Shape
+		targetMemberCleanNames := names.New(targetMemberName)
+		sourceAdaptedVarName := sourceVarName + "." + targetMemberName
 		out += fmt.Sprintf(
 			"%sif %s != nil {\n", indent, sourceAdaptedVarName,
 		)
 		qualifiedTargetVar := fmt.Sprintf(
-			"%s.%s", targetVarName, cleanNames.Camel,
+			"%s.%s", targetVarName, targetMemberCleanNames.Camel,
 		)
-		switch memberShape.Type {
+
+		switch sourceMemberShape.Type {
 		case "list", "structure", "map":
 			{
 				out += varEmptyConstructorK8sType(
 					cfg, r,
-					memberVarName,
+					indexedVarName,
 					targetMemberShapeRef.Shape,
 					indentLevel+1,
 				)
 				out += setResourceForContainer(
 					cfg, r,
-					cleanNames.Camel,
-					memberVarName,
+					targetMemberCleanNames.Camel,
+					indexedVarName,
 					targetMemberShapeRef,
 					nil,
 					sourceAdaptedVarName,
-					memberShapeRef,
+					sourceMemberShapeRef,
 					indentLevel+1,
 				)
 				out += setResourceForScalar(
 					qualifiedTargetVar,
-					memberVarName,
-					memberShapeRef,
+					indexedVarName,
+					sourceMemberShapeRef,
 					indentLevel+1,
 				)
 			}
 		default:
 			out += setResourceForScalar(
 				qualifiedTargetVar,
 				sourceAdaptedVarName,
-				memberShapeRef,
+				sourceMemberShapeRef,
 				indentLevel+1,
 			)
 		}