Refactor SetResourceForStruct to iterate over targetShape members
#289
Conversation
| } | ||
| } | ||
| return -1, fmt.Errorf("Could not find %s in shape %s", memberName, shape.ShapeName) | ||
| } |
There was a problem hiding this comment.
Nice job with this. It will allow the index numbers in variable names to remain consistent, cutting down on unnecessary code changes in generated controllers. thank you!
| for memberIndex, memberName := range sourceShape.MemberNames() { | ||
| targetMemberShapeRef := targetShape.MemberRefs[memberName] | ||
| if targetMemberShapeRef == nil { | ||
| for _, targetMemberName := range targetShape.MemberNames() { |
There was a problem hiding this comment.
This will be just fine for now (and is a perfect stepping stone for future commits, as you allude to in your PR summary). Long-term, I'd be keen to change this to loop over r.Fields[targetMemberName].MemberFields, though, which will contain all the additional custom fields as well.
| 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brycahta, jaypipes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…mbers (#292) Issue #, if available: aws-controllers-k8s/community#1146 This is a follow-up to #289. A `targetShape` without members in `SetResourceForStruct` will not be processed today; however, this is **not** desired behavior for all APIs such as EC2's DHCPOptions resource. This patch will look up and consume `SetConfig` in `SetResourceForStruct` when `targetShape` has no members like when DHCPOptions' field, `DHCPConfigurations.Values []*string`, aligns (by name) with aws-sdk's output shape, `DhcpConfigurations.Values []*AttributeValue`. Instead of skipping the processing of `Values []*string` field, it will check if a `SetConfig` exists and if it's valid (referencing a *real* shape in sourceShape), then use that updated field to source the target field's value resulting in using `DhcpConfigurations.Values.Value *string` to populate the resource's `DHCPConfigurations.Values []*string`. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
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,
SetResourceForStructwill 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 fromaws-sdkin this context, so will NOT have these additional, nested fields.This patch iterates over
targetShape.Membersinstead ofsourceShapeso these "nested" fields can be included in the lookup logic, but continues to usesourceShapeindex 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 FieldSetConfigif the Field cannot be found in the sourceShape:By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.