Skip to content

Commit

Permalink
Refactor identifier and path logic to crd.go (#174)
Browse files Browse the repository at this point in the history 
Issue #, if available: [#922](aws-controllers-k8s/community#922)

Description of changes:
* Moving 2 helper funcs to become *methods* of CRD Struct for clearer encapsulation:
  * `GetSanitizedMemberPath`
  * `GetIdentifiers`

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • Loading branch information
@brycahta
brycahta authored Aug 31, 2021
1 parent 9f6ca92 commit 5182a53
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 76 deletions.
36 changes: 3 additions & 33 deletions pkg/generate/code/check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
"github.com/aws-controllers-k8s/code-generator/pkg/model"
"github.com/aws-controllers-k8s/code-generator/pkg/names"
)

// CheckExceptionMessage returns Go code that contains a condition to
Expand Down Expand Up @@ -137,7 +136,7 @@ func checkRequiredFieldsMissingFromShape(
continue
}

resVarPath, err := getSanitizedMemberPath(memberName, r, op, koVarName)
resVarPath, err := r.GetSanitizedMemberPath(memberName, op, koVarName)
if err != nil {
// If it isn't in our spec/status fields, we have a problem!
msg := fmt.Sprintf(
Expand Down Expand Up @@ -185,7 +184,7 @@ func checkRequiredFieldsMissingFromShapeReadMany(

reqIdentifier, _ := FindPluralizedIdentifiersInShape(r, shape)

resVarPath, err := getSanitizedMemberPath(reqIdentifier, r, op, koVarName)
resVarPath, err := r.GetSanitizedMemberPath(reqIdentifier, op, koVarName)
if err != nil {
return result
}
Expand All @@ -194,33 +193,4 @@ func checkRequiredFieldsMissingFromShapeReadMany(
return fmt.Sprintf("%sreturn %s\n", indent, result)
}

// getSanitizedMemberPath takes a shape member field, checks for renames, checks
// for existence in Spec and Status, then constructs and returns the var path.
// Returns error if memberName is not present in either Spec or Status.
func getSanitizedMemberPath(
memberName string,
r *model.CRD,
op *awssdkmodel.Operation,
koVarName string) (string, error) {
resVarPath := koVarName
cleanMemberNames := names.New(memberName)
cleanMemberName := cleanMemberNames.Camel
// Check that the field has potentially been renamed
renamedName, wasRenamed := r.InputFieldRename(
op.Name, memberName,
)
_, found := r.SpecFields[renamedName]
if found && !wasRenamed {
resVarPath = resVarPath + r.Config().PrefixConfig.SpecField + "." + cleanMemberName
} else if found {
resVarPath = resVarPath + r.Config().PrefixConfig.SpecField + "." + renamedName
} else {
_, found = r.StatusFields[memberName]
if !found {
return "", fmt.Errorf(
"the required field %s is NOT present in CR's Spec or Status", memberName)
}
resVarPath = resVarPath + r.Config().PrefixConfig.StatusField + "." + cleanMemberName
}
return resVarPath, nil
}

35 changes: 1 addition & 34 deletions pkg/generate/code/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,39 +77,6 @@ func FindARNIdentifiersInShape(
return identifiers
}

// FindIdentifiersInCRD returns the identifier fields of a given CRD which
// can be singular or plural. Note, these fields will be the *original* field
// names from the API model shape, not renamed field names.
func FindIdentifiersInCRD(
r *model.CRD) []string {
var identifiers []string
if r == nil {
return identifiers
}
identifierLookup := []string{
"Id",
"Ids",
r.Names.Original + "Id",
r.Names.Original + "Ids",
"Name",
"Names",
r.Names.Original + "Name",
r.Names.Original + "Names",
}

for _, id := range identifierLookup {
_, found := r.SpecFields[id]
if !found {
_, found = r.StatusFields[id]
}
if found {
identifiers = append(identifiers, id)
}
}

return identifiers
}

// FindPluralizedIdentifiersInShape returns the name of a Spec OR Status field
// that has a matching pluralized field in the given shape and the name of
// the corresponding shape field name. This method will returns the original
Expand All @@ -122,7 +89,7 @@ func FindPluralizedIdentifiersInShape(
shape *awssdkmodel.Shape,
) (crField string, shapeField string) {
shapeIdentifiers := FindIdentifiersInShape(r, shape)
crIdentifiers := FindIdentifiersInCRD(r)
crIdentifiers := r.GetIdentifiers()
if len(shapeIdentifiers) == 0 || len(crIdentifiers) == 0 {
return "", ""
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/generate/code/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestFindIdentifiersInCRD_S3_Bucket_ReadMany_Empty(t *testing.T) {
assert.Len(actualIdentifiers, 0)
}

func TestFindIdentifiersInCRD_EC2_VPC_StatusField(t *testing.T) {
func TestGetIdentifiers_EC2_VPC_StatusField(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

Expand All @@ -68,15 +68,15 @@ func TestFindIdentifiersInCRD_EC2_VPC_StatusField(t *testing.T) {
require.NotNil(crd)

expIdentifier := "VpcId"
actualIdentifiers := code.FindIdentifiersInCRD(crd)
actualIdentifiers := crd.GetIdentifiers()
assert.Len(actualIdentifiers, 1)
assert.Equal(
strings.TrimSpace(expIdentifier),
strings.TrimSpace(actualIdentifiers[0]),
)
}

func TestFindIdentifiersInCRD_S3_Bucket_SpecField(t *testing.T) {
func TestGetIdentifiers_S3_Bucket_SpecField(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

Expand All @@ -86,15 +86,15 @@ func TestFindIdentifiersInCRD_S3_Bucket_SpecField(t *testing.T) {
require.NotNil(crd)

expIdentifier := "Name"
actualIdentifiers := code.FindIdentifiersInCRD(crd)
actualIdentifiers := crd.GetIdentifiers()
assert.Len(actualIdentifiers, 1)
assert.Equal(
strings.TrimSpace(expIdentifier),
strings.TrimSpace(actualIdentifiers[0]),
)
}

func TestFindIdentifiersInCRD_APIGatewayV2_API_Multiple(t *testing.T) {
func TestGetIdentifiers_APIGatewayV2_API_Multiple(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

Expand All @@ -103,7 +103,7 @@ func TestFindIdentifiersInCRD_APIGatewayV2_API_Multiple(t *testing.T) {
require.NotNil(crd)

expIdentifiers := []string{"ApiId", "Name"}
actualIdentifiers := code.FindIdentifiersInCRD(crd)
actualIdentifiers := crd.GetIdentifiers()
assert.Len(actualIdentifiers, 2)
assert.True(ackcompare.SliceStringEqual(expIdentifiers, actualIdentifiers))
}
6 changes: 3 additions & 3 deletions pkg/generate/code/set_sdk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,8 @@ func setSDKReadMany(
}
}

// Field renames are handled in getSanitizedMemberPath
resVarPath, err = getSanitizedMemberPath(memberName, r, op, sourceVarName)
// Field renames are handled in GetSanitizedMemberPath
resVarPath, err = r.GetSanitizedMemberPath(memberName, op, sourceVarName)
if err != nil {
// if it's an identifier field check for singular/plural
if util.InStrings(memberName, shapeIdentifiers) {
Expand All @@ -799,7 +799,7 @@ func setSDKReadMany(
// 'Id' identifier.
if resVarPath == "" || (!strings.HasSuffix(resVarPath, "Id") ||
!strings.HasSuffix(resVarPath, "Ids")) {
resVarPath, err = getSanitizedMemberPath(flipped, r, op, sourceVarName)
resVarPath, err = r.GetSanitizedMemberPath(flipped, op, sourceVarName)
if err != nil {
panic(fmt.Sprintf(
"Unable to locate identifier field %s in "+
Expand Down
62 changes: 62 additions & 0 deletions pkg/model/crd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,68 @@ func (r *CRD) GetAllRenames(op OpType) (map[string]string, error) {
return renames, nil
}

// GetIdentifiers returns the identifier fields of a given CRD which
// can be singular or plural. Note, these fields will be the *original* field
// names from the API model shape, not renamed field names.
func (r *CRD) GetIdentifiers() []string {
var identifiers []string
if r == nil {
return identifiers
}
identifierLookup := []string{
"Id",
"Ids",
r.Names.Original + "Id",
r.Names.Original + "Ids",
"Name",
"Names",
r.Names.Original + "Name",
r.Names.Original + "Names",
}

for _, id := range identifierLookup {
_, found := r.SpecFields[id]
if !found {
_, found = r.StatusFields[id]
}
if found {
identifiers = append(identifiers, id)
}
}

return identifiers
}

// GetSanitizedMemberPath takes a shape member field, checks for renames, checks
// for existence in Spec and Status, then constructs and returns the var path.
// Returns error if memberName is not present in either Spec or Status.
func (r *CRD) GetSanitizedMemberPath(
memberName string,
op *awssdkmodel.Operation,
koVarName string) (string, error) {
resVarPath := koVarName
cleanMemberNames := names.New(memberName)
cleanMemberName := cleanMemberNames.Camel
// Check that the field has potentially been renamed
renamedName, wasRenamed := r.InputFieldRename(
op.Name, memberName,
)
_, found := r.SpecFields[renamedName]
if found && !wasRenamed {
resVarPath = resVarPath + r.Config().PrefixConfig.SpecField + "." + cleanMemberName
} else if found {
resVarPath = resVarPath + r.Config().PrefixConfig.SpecField + "." + renamedName
} else {
_, found = r.StatusFields[memberName]
if !found {
return "", fmt.Errorf(
"the required field %s is NOT present in CR's Spec or Status", memberName)
}
resVarPath = resVarPath + r.Config().PrefixConfig.StatusField + "." + cleanMemberName
}
return resVarPath, nil
}

// NewCRD returns a pointer to a new `ackmodel.CRD` struct that describes a
// single top-level resource in an AWS service API
func NewCRD(
Expand Down

0 comments on commit 5182a53

Please sign in to comment.