From 5647318a0ad8832f23d89211a04213d4848aac3d Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 21 Jul 2022 15:00:16 -0700 Subject: [PATCH 1/6] WIP: Adding unit tests for aliases, so I can make changes --- README.md | 1 + cmd/root.go | 1 + external/aliases/aliases.go | 48 ++- external/aliases/aliases_test.go | 663 ++++++++++++++++++++++++++++++ external/resources/resources.yaml | 119 +++--- 5 files changed, 757 insertions(+), 75 deletions(-) create mode 100644 external/aliases/aliases_test.go diff --git a/README.md b/README.md index cdb7c0bc..9e404719 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ e.g. `export EPCC_API_BASE_URL=https://api.moltin.com` | EPCC_API_BASE_URL | This is the API base URL which can be retrieved via CM. | | EPCC_CLIENT_ID | This is the Client ID which can be retrieved via CM. | | EPCC_CLIENT_SECRET | This is the Client Secret which can be retrieved via CM. | +| EPCC_PROFILE | A profile name that allows for an independent session and isolation (e.g., distinct histories) | | EPCC_BETA_API_FEATURES | This variable allows you to set [Beta Headers](https://documentation.elasticpath.com/commerce-cloud/docs/api/basics/api-contract.html#beta-apis) for all API calls. | | EPCC_CLI_HTTP_HEADER_**N** | Setting any environment variable with this prefix will cause it's value to be parsed and added to all HTTP headers (e.g., `EPCC_CLI_HTTP_HEADER_0=Cache-Control: no-cache` will add `Cache-Control: no-cache` as a header). FYI, the surprising syntax is due to different encoding rules. | diff --git a/cmd/root.go b/cmd/root.go index fecbe9b6..16cad653 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -84,6 +84,7 @@ Environment Variables - EPCC_CLIENT_SECRET - The client secret (available in Commerce Manager) - EPCC_BETA_API_FEATURES - Beta features in the API we want to enable. - EPCC_CLI_HTTP_HEADER_[0,1,...] - An additional HTTP header to set with all requests, the format should be "HeaderName: value" +- EPCC_PROFILE - The name of the profile we will use (isolates namespace, credentials, etc...) `, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { diff --git a/external/aliases/aliases.go b/external/aliases/aliases.go index 78acb22a..2d8a2482 100644 --- a/external/aliases/aliases.go +++ b/external/aliases/aliases.go @@ -19,10 +19,12 @@ import ( // however, we should use file locking in the OS to stop multiple concurrent invocations. var filelock = sync.Mutex{} +var aliasDirectoryOverride = "" + func ClearAllAliases() error { aliasDataDirectory := getAliasDataDirectory() - if err := os.RemoveAll(aliasDataDirectory); !os.IsNotExist(err) { + if err := os.RemoveAll(aliasDataDirectory); err != nil && !os.IsNotExist(err) { return err } @@ -31,7 +33,7 @@ func ClearAllAliases() error { } func ClearAllAliasesForJsonApiType(jsonApiType string) error { - if err := os.Remove(getAliasFileForJsonApiType(getAliasDataDirectory(), jsonApiType)); !os.IsNotExist(err) { + if err := os.Remove(getAliasFileForJsonApiType(getAliasDataDirectory(), jsonApiType)); err != nil && !os.IsNotExist(err) { return err } @@ -99,15 +101,20 @@ func DeleteAliasesById(id string, jsonApiType string) { } func getAliasDataDirectory() string { - profileDirectory := profiles.GetProfileDataDirectory() - profileDataDirectory := filepath.FromSlash(profileDirectory + "/aliases/") + aliasDirectory := aliasDirectoryOverride + + if aliasDirectory == "" { + profileDirectory := profiles.GetProfileDataDirectory() + profileDataDirectory := filepath.FromSlash(profileDirectory + "/aliases/") + aliasDirectory = profileDataDirectory + } //built in check if dir exists - if err := os.MkdirAll(profileDataDirectory, 0700); err != nil { + if err := os.MkdirAll(aliasDirectory, 0700); err != nil { log.Errorf("could not make directory") } - return profileDataDirectory + return aliasDirectory } func getAliasFileForJsonApiType(profileDirectory string, resourceType string) string { @@ -127,19 +134,19 @@ func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]st data = []byte{} } - aliasMap := map[string]string{} + existingAliases := map[string]string{} - err = yaml.Unmarshal(data, aliasMap) + err = yaml.Unmarshal(data, existingAliases) if err != nil { log.Debugf("Could not unmarshall existing file %s, error %s", data, err) } - fn(aliasMap) + fn(existingAliases) // We will write to a temp file and then rename, to prevent data loss. rename's in the same folder are likely atomic in most settings. // Although we should probably sync on the file as well, that might be too much overhead, and I was too lazy to rewrite this // https://github.com/golang/go/issues/20599 tmpFileName := aliasFile + "." + uuid.New().String() - marshal, err := yaml.Marshal(aliasMap) + marshal, err := yaml.Marshal(existingAliases) if err != nil { log.Warnf("Could not save aliases for %s, error %v", tmpFileName, err) } @@ -153,24 +160,25 @@ func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]st if err != nil { log.Warnf("Could not save aliases for %s, error %v", tmpFileName, err) } - return aliasMap + return existingAliases } // This function saves all the aliases for a specific resource. -func saveAliasesForResource(jsonApiType string, aliases map[string]string) { +func saveAliasesForResource(jsonApiType string, newAliases map[string]string) { modifyAliases(jsonApiType, func(aliasMap map[string]string) { - for key, value := range aliases { - key0 := strings.Split(key, "=")[0] - for oldKey, oldValue := range aliasMap { - oldKey0 := strings.Split(oldKey, "=")[0] - oldKey1 := strings.Split(oldKey, "=")[1] - if oldValue == value && oldKey0 == key0 { - delete(aliasMap, oldKey0+"="+oldKey1) + for newAliasName, newAliasValue := range newAliases { + aliasNameKey := strings.Split(newAliasName, "=")[0] + for oldAliasName, oldValue := range aliasMap { + oldAliasKeyName := strings.Split(oldAliasName, "=")[0] + oldAliasValue := strings.Split(oldAliasName, "=")[1] + if oldValue == newAliasValue && oldAliasKeyName == aliasNameKey { + // This essentially deletes all the old aliases. + delete(aliasMap, oldAliasKeyName+"="+oldAliasValue) } } } - for key, value := range aliases { + for key, value := range newAliases { aliasMap[key] = value } }) diff --git a/external/aliases/aliases_test.go b/external/aliases/aliases_test.go new file mode 100644 index 00000000..c4e844f0 --- /dev/null +++ b/external/aliases/aliases_test.go @@ -0,0 +1,663 @@ +package aliases + +import ( + log "github.com/sirupsen/logrus" + "io/ioutil" + "testing" +) + +func init() { + dir, err := ioutil.TempDir("", "epcc-cli-aliases-testing") + if err != nil { + log.Panic("Could not create directory", err) + } + + aliasDirectoryOverride = dir + log.Infof("Alias directory for tests is %s", dir) +} + +func TestSavedAliasIsReturnedInAllAliasesForSingleResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 1 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasAppendsAndPreservesPreviousAliases(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } + + if aliases["id=456"] != "456" { + t.Errorf("Alias should exist for id=456") + } +} + +func TestDeleteAliasByIdDeletesAnAlias(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "foo" + } +}`) + + // Execute SUT + + DeleteAliasesById("123", "foo") + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 1 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["id=456"] != "456" { + t.Errorf("Alias should exist for id=456") + } +} + +func TestAllAliasesAreReturnedInAllAliasesForArrayResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": [{ + "id": "123", + "type": "foo" + }, { + "id": "456", + "type": "foo" + } + ] +} +`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } + + if aliases["id=456"] != "456" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForAnEmailInLegacyObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "email": "test@test.com", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["email=test@test.com"] != "123" { + t.Errorf("Alias should exist for email=test@test.com") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForAnSkuInLegacyObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "sku": "test", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["sku=test"] != "123" { + t.Errorf("Alias should exist for sku=test") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForASlugInLegacyObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "slug": "test", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["slug=test"] != "123" { + t.Errorf("Alias should exist for sku=test") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForANameInLegacyObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "name": "Test Testerson", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["name=Test_Testerson"] != "123" { + t.Errorf("Alias should exist for name=Test_Testerson") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForAnEmailInComplaintObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "attributes": { + "email": "test@test.com" + } + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["email=test@test.com"] != "123" { + t.Errorf("Alias should exist for email=test@test.com") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForAnSkuInComplaintObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "attributes": { + "sku": "test" + } + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["sku=test"] != "123" { + t.Errorf("Alias should exist for sku=test") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForASlugInComplaintObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "attributes": { + "slug": "test" + } + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["slug=test"] != "123" { + t.Errorf("Alias should exist for sku=test") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestSavedAliasIsReturnedForANameInComplaintObjectResponse(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "attributes": { + "name": "Test Testerson" + } + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["name=Test_Testerson"] != "123" { + t.Errorf("Alias should exist for name=Test_Testerson") + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } +} + +func TestResolveAliasValuesReturnsAliasForMatchingValue(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + value := ResolveAliasValuesOrReturnIdentity("foo", "id=123") + + // Verification + + if value != "123" { + t.Errorf("Alias value of 123 should have been returned, but got %s", value) + } +} + +func TestResolveAliasValuesReturnsRequestForUnMatchingValue(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + value := ResolveAliasValuesOrReturnIdentity("foo", "id=ABC") + + // Verification + + if value != "id=ABC" { + t.Errorf("Alias value of id=ABC should have been returned, but got %s", value) + } +} + +// This test helps prevent crashes from missing directories and some such. +func TestResolveAliasValuesReturnsRequestForUnMatchingValueAndType(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + value := ResolveAliasValuesOrReturnIdentity("bar", "id=XYZ") + + // Verification + + if value != "id=XYZ" { + t.Errorf("Alias value of id=XYZ should have been returned, but got %s", value) + } +} + +func TestClearAllAliasesClearsAllAliases(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "bar" + } +}`) + + // Execute SUT + err = ClearAllAliases() + if err != nil { + t.Errorf("Couldn't clear aliases %v", err) + } + + fooAliases := GetAliasesForJsonApiType("foo") + barAliases := GetAliasesForJsonApiType("bar") + + // Verification + if len(fooAliases) != 0 { + t.Errorf("There should be zero alias for the type foo, not %d", len(fooAliases)) + } + + if len(barAliases) != 0 { + t.Errorf("There should be zero alias for the type bar, not %d", len(barAliases)) + } + +} + +func TestClearAllAliasesForJsonTypeOnlyClearsJsonType(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "bar" + } +}`) + + // Execute SUT + err = ClearAllAliasesForJsonApiType("foo") + + if err != nil { + t.Errorf("Couldn't clear aliases %v", err) + } + + fooAliases := GetAliasesForJsonApiType("foo") + barAliases := GetAliasesForJsonApiType("bar") + + // Verification + if len(fooAliases) != 0 { + t.Errorf("There should be zero alias for the type foo, not %d", len(fooAliases)) + } + + if len(barAliases) != 1 { + t.Errorf("There should be one alias for the type bar, not %d", len(barAliases)) + } + +} diff --git a/external/resources/resources.yaml b/external/resources/resources.yaml index 1341a1de..c3e61525 100644 --- a/external/resources/resources.yaml +++ b/external/resources/resources.yaml @@ -404,61 +404,6 @@ customer-addresses: type: STRING country: type: STRING - -fields: - singular-name: "field" - json-api-type: "field" - json-api-format: "legacy" - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/index.html" - get-collection: - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/get-all-fields.html" - url: "/v2/fields" - get-entity: - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/get-a-field.html" - url: "/v2/fields/{fields}" - update-entity: - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/update-a-field.html" - url: "/v2/fields/{fields}" - delete-entity: - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/delete-a-field.html" - url: "/v2/fields/{fields}" - create-entity: - docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/create-a-field.html" - url: "/v2/fields" - content-type: application/json - attributes: - name: - type: STRING - slug: - type: STRING - field_type: - type: ENUM:string,integer,boolean,float,date,relationship - description: - type: STRING - required: - type: BOOL - default: - type: STRING - enabled: - type: BOOL - order: - type: INT - omit_null: - type: BOOL - validation_rules[n].type: - type: ENUM:enum,email,slug,between,one-to-many,one-to-one - validation_rules[n].options: - type: STRING - validation_rules[n].to: - type: STRING - validation_rules[n].options.to: - type: STRING - validation_rules[n].options.from: - type: STRING - relationships.flow.data.type: - type: ENUM:flow - relationships.flow.data.id: - type: RESOURCE_ID:flows files: singular-name: "file" json-api-type: "file" @@ -515,6 +460,70 @@ flows: type: STRING name: type: STRING +fields: + singular-name: "field" + json-api-type: "field" + json-api-format: "legacy" + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/index.html" + get-collection: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/get-all-fields.html" + url: "/v2/fields" + get-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/get-a-field.html" + url: "/v2/fields/{fields}" + update-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/update-a-field.html" + url: "/v2/fields/{fields}" + delete-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/delete-a-field.html" + url: "/v2/fields/{fields}" + create-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/fields/create-a-field.html" + url: "/v2/fields" + content-type: application/json + attributes: + name: + type: STRING + slug: + type: STRING + field_type: + type: ENUM:string,integer,boolean,float,date,relationship + description: + type: STRING + required: + type: BOOL + default: + type: STRING + enabled: + type: BOOL + order: + type: INT + omit_null: + type: BOOL + validation_rules[n].type: + type: ENUM:enum,email,slug,between,one-to-many,one-to-one + validation_rules[n].options: + type: STRING + validation_rules[n].to: + type: STRING + validation_rules[n].options.to: + type: STRING + validation_rules[n].options.from: + type: STRING + relationships.flow.data.type: + type: ENUM:flow + relationships.flow.data.id: + type: RESOURCE_ID:flows +entries: + singular-name: entry + json-api-type: entry + json-api-format: "legacy" + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/index.html" + get-collection: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/get-all-entries.html" + url: "/v2/flows/{flows}/entries" + parent_resource_value_overrides: + flows: slug integration-jobs: singular-name: "integration-job" json-api-type: "integration-job" From cf2f100672a0a53979da33532b0a3fac34e2f22d Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sat, 23 Jul 2022 11:41:56 -0700 Subject: [PATCH 2/6] Finished unit tests for aliases --- external/aliases/aliases.go | 16 ++- external/aliases/aliases_test.go | 193 ++++++++++++++++++++++++++++++- 2 files changed, 202 insertions(+), 7 deletions(-) diff --git a/external/aliases/aliases.go b/external/aliases/aliases.go index 2d8a2482..f05211f6 100644 --- a/external/aliases/aliases.go +++ b/external/aliases/aliases.go @@ -166,13 +166,19 @@ func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]st // This function saves all the aliases for a specific resource. func saveAliasesForResource(jsonApiType string, newAliases map[string]string) { modifyAliases(jsonApiType, func(aliasMap map[string]string) { - for newAliasName, newAliasValue := range newAliases { - aliasNameKey := strings.Split(newAliasName, "=")[0] - for oldAliasName, oldValue := range aliasMap { + + // Aliases have the format KEY=VALUE and this maps to an ID. + // This code checks for where two aliases have the same KEY and same ID, and replaces the old value, with the new one. + // This happens in cases where we store a name like "name=John_Smith" and then the user renames it to "name=Jane_Doe". + // The old alias for the same id name=John_Smith should be removed. + for newAliasName, newAliasReferencedId := range newAliases { + newAliasKeyName := strings.Split(newAliasName, "=")[0] + for oldAliasName, oldAliasReferencedId := range aliasMap { oldAliasKeyName := strings.Split(oldAliasName, "=")[0] oldAliasValue := strings.Split(oldAliasName, "=")[1] - if oldValue == newAliasValue && oldAliasKeyName == aliasNameKey { - // This essentially deletes all the old aliases. + + if oldAliasKeyName == newAliasKeyName && oldAliasReferencedId == newAliasReferencedId { + delete(aliasMap, oldAliasKeyName+"="+oldAliasValue) } } diff --git a/external/aliases/aliases_test.go b/external/aliases/aliases_test.go index c4e844f0..ffa7ea8c 100644 --- a/external/aliases/aliases_test.go +++ b/external/aliases/aliases_test.go @@ -3,6 +3,7 @@ package aliases import ( log "github.com/sirupsen/logrus" "io/ioutil" + "os" "testing" ) @@ -48,7 +49,7 @@ func TestSavedAliasIsReturnedInAllAliasesForSingleResponse(t *testing.T) { } } -func TestSavedAliasAppendsAndPreservesPreviousAliases(t *testing.T) { +func TestSavedAliasAppendsAndPreservesPreviousUnrelatedAliases(t *testing.T) { // Fixture Setup err := ClearAllAliases() @@ -81,7 +82,54 @@ func TestSavedAliasAppendsAndPreservesPreviousAliases(t *testing.T) { // Verification if len(aliases) != 2 { - t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + t.Errorf("There should be two aliases for the type foo, not %d", len(aliases)) + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } + + if aliases["id=456"] != "456" { + t.Errorf("Alias should exist for id=456") + } +} + +func TestSavedAliasIsReplacedWhenNewEntityHasTheSameAttributeValue(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "name": "Alpha" + } +}`) + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "foo", + "name":"Alpha" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 3 { + t.Errorf("There should be three aliases for the type foo, not %d", len(aliases)) } if aliases["id=123"] != "123" { @@ -91,6 +139,57 @@ func TestSavedAliasAppendsAndPreservesPreviousAliases(t *testing.T) { if aliases["id=456"] != "456" { t.Errorf("Alias should exist for id=456") } + + if aliases["name=Alpha"] != "456" { + t.Errorf("Alias should exist for id=456") + } +} + +func TestSavedAliasIsReplacedWhenSameEntityHasANewValue(t *testing.T) { + + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + // Execute SUT + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "name": "Alpha" + } +}`) + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo", + "name":"Beta" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + + if len(aliases) != 2 { + t.Errorf("There should be three aliases for the type foo, not %d", len(aliases)) + } + + if aliases["id=123"] != "123" { + t.Errorf("Alias should exist for id=123") + } + + if aliases["name=Beta"] != "123" { + t.Errorf("Alias should exist for id=123") + } } func TestDeleteAliasByIdDeletesAnAlias(t *testing.T) { @@ -661,3 +760,93 @@ func TestClearAllAliasesForJsonTypeOnlyClearsJsonType(t *testing.T) { } } + +func TestThatCorruptAliasFileDoesntCrashProgramWhenReadingAliases(t *testing.T) { + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + // Execute SUT + fileName := getAliasFileForJsonApiType(getAliasDataDirectory(), "foo") + + if err := os.Remove(getAliasFileForJsonApiType(getAliasDataDirectory(), "foo")); err != nil && !os.IsNotExist(err) { + t.Errorf("Should have been able to delete the file, but got %v ", err) + } + + err = ioutil.WriteFile(fileName, []byte("{{{"), 0600) + if err != nil { + t.Errorf("Couldn't save corrupted yaml file %v", err) + } + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + if len(aliases) != 0 { + t.Errorf("There should be zero alias for the type foo, not %d", len(aliases)) + } + +} + +func TestThatCorruptAliasFileDoesntCrashProgramWhenSavingAliases(t *testing.T) { + // Fixture Setup + err := ClearAllAliases() + if err != nil { + t.Fatalf("Could not clear aliases") + } + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "123", + "type": "foo" + } +}`) + + // Execute SUT + fileName := getAliasFileForJsonApiType(getAliasDataDirectory(), "foo") + + if err := os.Remove(getAliasFileForJsonApiType(getAliasDataDirectory(), "foo")); err != nil && !os.IsNotExist(err) { + t.Errorf("Should have been able to delete the file, but got %v ", err) + } + + err = ioutil.WriteFile(fileName, []byte("{{{"), 0600) + if err != nil { + t.Errorf("Couldn't save corrupted yaml file %v", err) + } + + SaveAliasesForResources( + // language=JSON + ` +{ + "data": { + "id": "456", + "type": "foo" + } +}`) + + aliases := GetAliasesForJsonApiType("foo") + + // Verification + if len(aliases) != 1 { + t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) + } + + if aliases["id=456"] != "456" { + t.Errorf("Alias should exist for id=456") + } + +} From f8f1befc7844e98e767eef975b5790635be30e7b Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sat, 23 Jul 2022 12:32:05 -0700 Subject: [PATCH 3/6] Checkpoint #170 - Tweak alias list output --- cmd/aliases.go | 13 +++- cmd/root.go | 1 + external/aliases/aliases.go | 104 ++++++++++++++++++------------- external/aliases/aliases_test.go | 94 +++++++++++++++++++--------- 4 files changed, 137 insertions(+), 75 deletions(-) diff --git a/cmd/aliases.go b/cmd/aliases.go index 34568434..d175d2bc 100644 --- a/cmd/aliases.go +++ b/cmd/aliases.go @@ -37,8 +37,19 @@ var aliasListCmd = &cobra.Command{ sort.Strings(sortedAliasNames) + fmt.Printf("%40s || Values\n", "Alias Name") + for _, alias := range sortedAliasNames { - fmt.Printf("%40s => %s\n", alias, aliases[alias]) + fmt.Printf("%40s => ID: %s", alias, aliases[alias].Id) + if aliases[alias].Sku != "" { + fmt.Printf(" Sku: %10s", aliases[alias].Sku) + } + + if aliases[alias].Slug != "" { + fmt.Printf(" Slug: %10s", aliases[alias].Slug) + } + + fmt.Println() } return nil diff --git a/cmd/root.go b/cmd/root.go index 16cad653..6e942c11 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -60,6 +60,7 @@ func init() { RootCmd.PersistentFlags().Uint16VarP(&rateLimit, "rate-limit", "", 10, "Request limit per second") aliasesCmd.AddCommand(aliasListCmd, aliasClearCmd) + } var persistentPreRunFuncs []func(cmd *cobra.Command, args []string) error diff --git a/external/aliases/aliases.go b/external/aliases/aliases.go index f05211f6..b9ddb0d5 100644 --- a/external/aliases/aliases.go +++ b/external/aliases/aliases.go @@ -21,6 +21,12 @@ var filelock = sync.Mutex{} var aliasDirectoryOverride = "" +type AliasAttributes struct { + Id string `yaml:"id"` + Slug string `yaml:"slug"` + Sku string `yaml:"sku"` +} + func ClearAllAliases() error { aliasDataDirectory := getAliasDataDirectory() @@ -41,11 +47,11 @@ func ClearAllAliasesForJsonApiType(jsonApiType string) error { } -func GetAliasesForJsonApiType(jsonApiType string) map[string]string { +func GetAliasesForJsonApiType(jsonApiType string) map[string]*AliasAttributes { profileDirectory := getAliasDataDirectory() aliasFile := getAliasFileForJsonApiType(profileDirectory, jsonApiType) - aliasMap := map[string]string{} + aliasMap := map[string]*AliasAttributes{} data, err := ioutil.ReadFile(aliasFile) if err != nil { @@ -64,7 +70,7 @@ func GetAliasesForJsonApiType(jsonApiType string) map[string]string { func ResolveAliasValuesOrReturnIdentity(jsonApiType string, value string) string { if result, ok := GetAliasesForJsonApiType(jsonApiType)[value]; ok { - return result + return result.Id } return value } @@ -77,10 +83,10 @@ func SaveAliasesForResources(jsonTxt string) { return } - results := map[string]map[string]string{} + results := map[string]map[string]*AliasAttributes{} visitResources(jsonStruct, "", results) - log.Tracef("All aliases: %s", results) + log.Tracef("All aliases: %v", results) for resourceType, aliases := range results { saveAliasesForResource(resourceType, aliases) @@ -89,9 +95,9 @@ func SaveAliasesForResources(jsonTxt string) { } func DeleteAliasesById(id string, jsonApiType string) { - modifyAliases(jsonApiType, func(m map[string]string) { + modifyAliases(jsonApiType, func(m map[string]*AliasAttributes) { for key, value := range m { - if value == id { + if value.Id == id { delete(m, key) } } @@ -122,7 +128,7 @@ func getAliasFileForJsonApiType(profileDirectory string, resourceType string) st return aliasFile } -func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]string { +func modifyAliases(jsonApiType string, fn func(map[string]*AliasAttributes)) map[string]*AliasAttributes { profileDirectory := getAliasDataDirectory() filelock.Lock() defer filelock.Unlock() @@ -134,7 +140,7 @@ func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]st data = []byte{} } - existingAliases := map[string]string{} + existingAliases := map[string]*AliasAttributes{} err = yaml.Unmarshal(data, existingAliases) if err != nil { @@ -164,8 +170,8 @@ func modifyAliases(jsonApiType string, fn func(map[string]string)) map[string]st } // This function saves all the aliases for a specific resource. -func saveAliasesForResource(jsonApiType string, newAliases map[string]string) { - modifyAliases(jsonApiType, func(aliasMap map[string]string) { +func saveAliasesForResource(jsonApiType string, newAliases map[string]*AliasAttributes) { + modifyAliases(jsonApiType, func(aliasMap map[string]*AliasAttributes) { // Aliases have the format KEY=VALUE and this maps to an ID. // This code checks for where two aliases have the same KEY and same ID, and replaces the old value, with the new one. @@ -177,7 +183,7 @@ func saveAliasesForResource(jsonApiType string, newAliases map[string]string) { oldAliasKeyName := strings.Split(oldAliasName, "=")[0] oldAliasValue := strings.Split(oldAliasName, "=")[1] - if oldAliasKeyName == newAliasKeyName && oldAliasReferencedId == newAliasReferencedId { + if oldAliasKeyName == newAliasKeyName && oldAliasReferencedId.Id == newAliasReferencedId.Id { delete(aliasMap, oldAliasKeyName+"="+oldAliasValue) } @@ -190,17 +196,17 @@ func saveAliasesForResource(jsonApiType string, newAliases map[string]string) { }) } -func visitResources(data map[string]interface{}, prefix string, results map[string]map[string]string) { +func visitResources(data map[string]interface{}, prefix string, results map[string]map[string]*AliasAttributes) { if typeObj, typeKeyExists := data["type"]; typeKeyExists { if idObj, idKeyExists := data["id"]; idKeyExists { if typeKeyValue, typeKeyIsString := typeObj.(string); typeKeyIsString { if idKeyValue, idKeyIsString := idObj.(string); idKeyIsString { aliases := generateAliasesForStruct(typeKeyValue, idKeyValue, data) - log.Tracef("Found a type and id pair %s => %s under prefix %s, aliases %s", typeKeyValue, idKeyValue, prefix, aliases) + log.Tracef("Found a type and id pair %s => %s under prefix %s, aliases %v", typeKeyValue, idKeyValue, prefix, aliases) if _, ok := results[typeKeyValue]; !ok { - results[typeKeyValue] = make(map[string]string) + results[typeKeyValue] = make(map[string]*AliasAttributes) } for aliasKey, aliasValue := range aliases { @@ -231,46 +237,45 @@ func visitResources(data map[string]interface{}, prefix string, results map[stri return } -func generateAliasesForStruct(typeKey string, idKey string, data map[string]interface{}) map[string]string { - results := map[string]string{ - // Identity, objects should be an alias of themselves. - "id=" + idKey: idKey, +func generateAliasesForStruct(typeKey string, idKey string, data map[string]interface{}) map[string]*AliasAttributes { + result := AliasAttributes{ + Id: idKey, } - if alias := getAliasForKey("name", data); alias != "" { - results[alias] = idKey + results := map[string]*AliasAttributes{ + // Identity, objects should be an alias of themselves. + "id=" + idKey: &result, } - if alias := getAliasForKey("sku", data); alias != "" { - results[alias] = idKey - } + jsonObjectsToInspect := make([]map[string]interface{}, 0) + jsonObjectsToInspect = append(jsonObjectsToInspect, data) - if alias := getAliasForKey("slug", data); alias != "" { - results[alias] = idKey + if val, ok := data["attributes"]; ok { + if attributeVal, ok := val.(map[string]interface{}); ok { + jsonObjectsToInspect = append(jsonObjectsToInspect, attributeVal) + } } - if alias := getAliasForKey("email", data); alias != "" { - results[alias] = idKey - } + for _, jsonObjectToInspect := range jsonObjectsToInspect { - if val, ok := data["attributes"]; ok { - if attributeVal, ok := val.(map[string]interface{}); ok { - if alias := getAliasForKey("name", attributeVal); alias != "" { - results[alias] = idKey - } + if alias := getAliasForKey("name", jsonObjectToInspect); alias != "" { + results[alias] = &result + } - if alias := getAliasForKey("sku", attributeVal); alias != "" { - results[alias] = idKey - } + if alias := getAliasForKey("sku", jsonObjectToInspect); alias != "" { + results[alias] = &result + result.Sku = getAttributeValueForKey("sku", jsonObjectToInspect) + } - if alias := getAliasForKey("slug", attributeVal); alias != "" { - results[alias] = idKey - } + if alias := getAliasForKey("slug", jsonObjectToInspect); alias != "" { + results[alias] = &result + result.Slug = getAttributeValueForKey("slug", jsonObjectToInspect) + } - if alias := getAliasForKey("email", attributeVal); alias != "" { - results[alias] = idKey - } + if alias := getAliasForKey("email", jsonObjectToInspect); alias != "" { + results[alias] = &result } + } return results @@ -289,3 +294,16 @@ func getAliasForKey(key string, data map[string]interface{}) string { return "" } } + +func getAttributeValueForKey(key string, data map[string]interface{}) string { + if val, ok := data[key]; ok { + if strVal, ok := val.(string); ok { + return strVal + } else { + return "" + } + } else { + return "" + } + +} diff --git a/external/aliases/aliases_test.go b/external/aliases/aliases_test.go index ffa7ea8c..b4b49792 100644 --- a/external/aliases/aliases_test.go +++ b/external/aliases/aliases_test.go @@ -44,7 +44,7 @@ func TestSavedAliasIsReturnedInAllAliasesForSingleResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -85,11 +85,11 @@ func TestSavedAliasAppendsAndPreservesPreviousUnrelatedAliases(t *testing.T) { t.Errorf("There should be two aliases for the type foo, not %d", len(aliases)) } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } - if aliases["id=456"] != "456" { + if aliases["id=456"] != nil && aliases["id=456"].Id != "456" { t.Errorf("Alias should exist for id=456") } } @@ -132,15 +132,15 @@ func TestSavedAliasIsReplacedWhenNewEntityHasTheSameAttributeValue(t *testing.T) t.Errorf("There should be three aliases for the type foo, not %d", len(aliases)) } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } - if aliases["id=456"] != "456" { + if aliases["id=456"] != nil && aliases["id=456"].Id != "456" { t.Errorf("Alias should exist for id=456") } - if aliases["name=Alpha"] != "456" { + if aliases["name=Alpha"] != nil && aliases["name=Alpha"].Id != "456" { t.Errorf("Alias should exist for id=456") } } @@ -180,14 +180,14 @@ func TestSavedAliasIsReplacedWhenSameEntityHasANewValue(t *testing.T) { // Verification if len(aliases) != 2 { - t.Errorf("There should be three aliases for the type foo, not %d", len(aliases)) + t.Errorf("There should be two aliases for the type foo, not %d", len(aliases)) } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } - if aliases["name=Beta"] != "123" { + if aliases["name=Beta"] != nil && aliases["name=Beta"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -230,7 +230,7 @@ func TestDeleteAliasByIdDeletesAnAlias(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["id=456"] != "456" { + if aliases["id=456"] != nil && aliases["id=456"].Id != "456" { t.Errorf("Alias should exist for id=456") } } @@ -267,11 +267,11 @@ func TestAllAliasesAreReturnedInAllAliasesForArrayResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } - if aliases["id=456"] != "456" { + if aliases["id=456"] != nil && aliases["id=456"].Id != "456" { t.Errorf("Alias should exist for id=123") } } @@ -304,11 +304,11 @@ func TestSavedAliasIsReturnedForAnEmailInLegacyObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["email=test@test.com"] != "123" { + if aliases["email=test@test.com"] != nil && aliases["email=test@test.com"].Id != "123" { t.Errorf("Alias should exist for email=test@test.com") } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -341,13 +341,21 @@ func TestSavedAliasIsReturnedForAnSkuInLegacyObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["sku=test"] != "123" { + if aliases["sku=test"] != nil && aliases["sku=test"].Id != "123" { t.Errorf("Alias should exist for sku=test") } - if aliases["id=123"] != "123" { + if aliases["sku=test"] != nil && aliases["sku=test"].Sku != "test" { + t.Errorf("Alias should exist for sku=test and have sku test") + } + + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } + + if aliases["id=123"] != nil && aliases["id=123"].Sku != "test" { + t.Errorf("Alias should exist for id=123 and have a sku of test") + } } func TestSavedAliasIsReturnedForASlugInLegacyObjectResponse(t *testing.T) { @@ -378,13 +386,21 @@ func TestSavedAliasIsReturnedForASlugInLegacyObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["slug=test"] != "123" { - t.Errorf("Alias should exist for sku=test") + if aliases["slug=test"] != nil && aliases["slug=test"].Id != "123" { + t.Errorf("Alias should exist for slug=test") } - if aliases["id=123"] != "123" { + if aliases["slug=test"] != nil && aliases["slug=test"].Slug != "test" { + t.Errorf("Alias should exist for slug=test and have slug value of test") + } + + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } + + if aliases["id=123"] != nil && aliases["id=123"].Slug != "test" { + t.Errorf("Alias should exist for id=123 and have a slug of test") + } } func TestSavedAliasIsReturnedForANameInLegacyObjectResponse(t *testing.T) { @@ -415,11 +431,11 @@ func TestSavedAliasIsReturnedForANameInLegacyObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["name=Test_Testerson"] != "123" { + if aliases["name=Test_Testerson"] != nil && aliases["name=Test_Testerson"].Id != "123" { t.Errorf("Alias should exist for name=Test_Testerson") } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -454,11 +470,11 @@ func TestSavedAliasIsReturnedForAnEmailInComplaintObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["email=test@test.com"] != "123" { + if aliases["email=test@test.com"] != nil && aliases["email=test@test.com"].Id != "123" { t.Errorf("Alias should exist for email=test@test.com") } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -493,13 +509,21 @@ func TestSavedAliasIsReturnedForAnSkuInComplaintObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["sku=test"] != "123" { + if aliases["sku=test"] != nil && aliases["sku=test"].Id != "123" { t.Errorf("Alias should exist for sku=test") } - if aliases["id=123"] != "123" { + if aliases["sku=test"] != nil && aliases["sku=test"].Sku != "test" { + t.Errorf("Alias should exist for sku=test and have a sku of test") + } + + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } + + if aliases["id=123"] != nil && aliases["id=123"].Sku != "test" { + t.Errorf("Alias should exist for id=123 and have a sku of test") + } } func TestSavedAliasIsReturnedForASlugInComplaintObjectResponse(t *testing.T) { @@ -532,13 +556,21 @@ func TestSavedAliasIsReturnedForASlugInComplaintObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["slug=test"] != "123" { - t.Errorf("Alias should exist for sku=test") + if aliases["slug=test"] != nil && aliases["slug=test"].Id != "123" { + t.Errorf("Alias should exist for slug=test") + } + + if aliases["slug=test"] != nil && aliases["slug=test"].Slug != "test" { + t.Errorf("Alias should exist for slug=test and have a slug of test") } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } + + if aliases["id=123"] != nil && aliases["id=123"].Slug != "test" { + t.Errorf("Alias should exist for id=123 and have a slug of test") + } } func TestSavedAliasIsReturnedForANameInComplaintObjectResponse(t *testing.T) { @@ -571,11 +603,11 @@ func TestSavedAliasIsReturnedForANameInComplaintObjectResponse(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["name=Test_Testerson"] != "123" { + if aliases["name=Test_Testerson"] != nil && aliases["name=Test_Testerson"].Id != "123" { t.Errorf("Alias should exist for name=Test_Testerson") } - if aliases["id=123"] != "123" { + if aliases["id=123"] != nil && aliases["id=123"].Id != "123" { t.Errorf("Alias should exist for id=123") } } @@ -845,7 +877,7 @@ func TestThatCorruptAliasFileDoesntCrashProgramWhenSavingAliases(t *testing.T) { t.Errorf("There should be one alias for the type foo, not %d", len(aliases)) } - if aliases["id=456"] != "456" { + if aliases["id=456"] != nil && aliases["id=456"].Id != "456" { t.Errorf("Alias should exist for id=456") } From 9704957b8d53d738e57203f0d330ce56d3b8fd19 Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sat, 23 Jul 2022 12:40:43 -0700 Subject: [PATCH 4/6] Another checkpoint --- cmd/create.go | 2 +- cmd/delete-all.go | 2 +- cmd/delete.go | 8 +++----- cmd/get.go | 26 ++++++++++++-------------- cmd/reset-store.go | 2 +- cmd/update.go | 6 +++--- external/apihelper/get_all_ids.go | 2 +- external/resources/uritemplates.go | 4 ++-- 8 files changed, 24 insertions(+), 28 deletions(-) diff --git a/cmd/create.go b/cmd/create.go index 11d0a9f0..7b181a33 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -39,7 +39,7 @@ var create = &cobra.Command{ } // Replace ids with args in resourceURL - resourceURL, err = resources.GenerateUrl(resource, resourceURL, args[1:]) + resourceURL, err = resources.GenerateUrl(resource, resource.CreateEntityInfo, args[1:]) if err != nil { return err diff --git a/cmd/delete-all.go b/cmd/delete-all.go index 69cf936c..4d394ed0 100644 --- a/cmd/delete-all.go +++ b/cmd/delete-all.go @@ -50,7 +50,7 @@ var DeleteAll = &cobra.Command{ for _, parentEntityIds := range allParentEntityIds { lastIds := make([][]string, 1) for { - resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo.Url, parentEntityIds) + resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo, parentEntityIds) if err != nil { return err diff --git a/cmd/delete.go b/cmd/delete.go index 1771564b..87b6db95 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -59,7 +59,7 @@ var delete = &cobra.Command{ return []string{}, cobra.ShellCompDirectiveNoFileComp } - idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL) + idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL.Url) if err != nil { return []string{}, cobra.ShellCompDirectiveNoFileComp @@ -67,7 +67,7 @@ var delete = &cobra.Command{ if len(args) > 0 && len(args) < 1+idCount { // Must be for a resource completion - types, err := resources.GetTypesOfVariablesNeeded(resourceURL) + types, err := resources.GetTypesOfVariablesNeeded(resourceURL.Url) if err != nil { return []string{}, cobra.ShellCompDirectiveNoFileComp @@ -99,10 +99,8 @@ func deleteResource(args []string) (*http.Response, error) { return nil, fmt.Errorf("resource %s doesn't support DELETE", args[0]) } - resourceURL := resource.DeleteEntityInfo.Url - // Replace ids with args in resourceURL - resourceURL, err := resources.GenerateUrl(resource, resourceURL, args[1:]) + resourceURL, err := resources.GenerateUrl(resource, resource.DeleteEntityInfo, args[1:]) if err != nil { return nil, err diff --git a/cmd/get.go b/cmd/get.go index 3e40b5d6..c7d06508 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -58,7 +58,7 @@ var get = &cobra.Command{ return []string{}, cobra.ShellCompDirectiveNoFileComp } - idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL) + idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL.Url) if err != nil { return []string{}, cobra.ShellCompDirectiveNoFileComp @@ -66,7 +66,7 @@ var get = &cobra.Command{ if len(args) > 0 && len(args) < 1+idCount { // Must be for a resource completion - types, err := resources.GetTypesOfVariablesNeeded(resourceURL) + types, err := resources.GetTypesOfVariablesNeeded(resourceURL.Url) if err != nil { return []string{}, cobra.ShellCompDirectiveNoFileComp @@ -104,22 +104,21 @@ var get = &cobra.Command{ }, } -func getUrl(resource resources.Resource, args []string) (string, error) { - resourceURL := "" +func getUrl(resource resources.Resource, args []string) (*resources.CrudEntityInfo, error) { + if resource.GetCollectionInfo == nil && resource.GetEntityInfo == nil { - return "", fmt.Errorf("resource %s doesn't support GET", args[0]) + return nil, fmt.Errorf("resource %s doesn't support GET", args[0]) } else if resource.GetCollectionInfo != nil && resource.GetEntityInfo == nil { - resourceURL = resource.GetCollectionInfo.Url + return resource.GetCollectionInfo, nil } else if resource.GetCollectionInfo == nil && resource.GetEntityInfo != nil { - resourceURL = resource.GetEntityInfo.Url + return resource.GetEntityInfo, nil } else { if _, ok := resources.GetPluralResources()[args[0]]; ok { - resourceURL = resource.GetCollectionInfo.Url + return resource.GetCollectionInfo, nil } else { - resourceURL = resource.GetEntityInfo.Url + return resource.GetEntityInfo, nil } } - return resourceURL, nil } func getResource(args []string) (*http.Response, error) { @@ -129,22 +128,21 @@ func getResource(args []string) (*http.Response, error) { return nil, fmt.Errorf("could not find resource %s", args[0]) } - var resourceURL string var idCount int - resourceURL, err2 := getUrl(resource, args) + resourceUrlInfo, err2 := getUrl(resource, args) if err2 != nil { return nil, err2 } - idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL) + idCount, err := resources.GetNumberOfVariablesNeeded(resourceUrlInfo.Url) if err != nil { return nil, err } // Replace ids with args in resourceURL - resourceURL, err = resources.GenerateUrl(resource, resourceURL, args[1:]) + resourceURL, err := resources.GenerateUrl(resource, resourceUrlInfo, args[1:]) if err != nil { return nil, err diff --git a/cmd/reset-store.go b/cmd/reset-store.go index 0c2928ae..dc0e786c 100644 --- a/cmd/reset-store.go +++ b/cmd/reset-store.go @@ -28,7 +28,7 @@ var ResetStore = &cobra.Command{ return fmt.Errorf("could not find resource %s, we need it to determine the store id.", args[0]) } - resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo.Url, make([]string, 0)) + resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo, make([]string, 0)) if err != nil { return err diff --git a/cmd/update.go b/cmd/update.go index 45324fef..7c7eef75 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -30,14 +30,14 @@ var update = &cobra.Command{ } // Count ids in UpdateEntity - resourceURL := resource.UpdateEntityInfo.Url - idCount, err := resources.GetNumberOfVariablesNeeded(resourceURL) + resourceUrlInfo := resource.UpdateEntityInfo + idCount, err := resources.GetNumberOfVariablesNeeded(resourceUrlInfo.Url) if err != nil { return err } // Replace ids with args in resourceURL - resourceURL, err = resources.GenerateUrl(resource, resourceURL, args[1:]) + resourceURL, err := resources.GenerateUrl(resource, resourceUrlInfo, args[1:]) if err != nil { return err } diff --git a/external/apihelper/get_all_ids.go b/external/apihelper/get_all_ids.go index 7ce49f04..bc2ea6f7 100644 --- a/external/apihelper/get_all_ids.go +++ b/external/apihelper/get_all_ids.go @@ -57,7 +57,7 @@ func GetAllIds(ctx context.Context, resource *resources.Resource) ([][]string, e // For each parent entity id we need to loop over the entire collection for _, parentEntityIds := range myParentEntityIds { - resourceURL, err := resources.GenerateUrl(*resource, resource.GetCollectionInfo.Url, parentEntityIds) + resourceURL, err := resources.GenerateUrl(*resource, resource.GetCollectionInfo, parentEntityIds) if err != nil { return myEntityIds, err diff --git a/external/resources/uritemplates.go b/external/resources/uritemplates.go index 044c3f60..94b47a23 100644 --- a/external/resources/uritemplates.go +++ b/external/resources/uritemplates.go @@ -8,8 +8,8 @@ import ( "strings" ) -func GenerateUrl(resource Resource, url string, args []string) (string, error) { - template, err := uritemplate.New(url) +func GenerateUrl(resource Resource, urlInfo *CrudEntityInfo, args []string) (string, error) { + template, err := uritemplate.New(urlInfo.Url) if err != nil { return "", fmt.Errorf("could not generate URI template for URL: %w", err) From 31990881879c887bd3cee71dcf98676ad117819c Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sun, 24 Jul 2022 16:16:05 -0700 Subject: [PATCH 5/6] Resolves #170 - Add support for entries --- cmd/create.go | 2 +- cmd/delete-all.go | 46 ++-- cmd/delete.go | 2 +- cmd/get.go | 2 +- cmd/reset-store.go | 2 +- cmd/update.go | 2 +- external/aliases/aliases.go | 53 ++-- external/aliases/aliases_test.go | 6 +- external/apihelper/get_all_ids.go | 11 +- .../map_collection_response_to_ids.go | 28 ++- external/completion/completion.go | 100 ++++---- external/id/idable_attributes.go | 7 + external/json/to_json.go | 6 +- external/resources/resources.go | 3 + external/resources/resources.yaml | 53 +++- external/resources/resources_test.go | 102 ++++---- external/resources/resources_yaml_test.go | 3 + external/resources/uritemplates.go | 67 +++++- external/resources/uritemplates_test.go | 226 ++++++++++++++++++ 19 files changed, 555 insertions(+), 166 deletions(-) create mode 100644 external/id/idable_attributes.go create mode 100644 external/resources/uritemplates_test.go diff --git a/cmd/create.go b/cmd/create.go index 7b181a33..164a6142 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -39,7 +39,7 @@ var create = &cobra.Command{ } // Replace ids with args in resourceURL - resourceURL, err = resources.GenerateUrl(resource, resource.CreateEntityInfo, args[1:]) + resourceURL, err = resources.GenerateUrl(resource.CreateEntityInfo, args[1:]) if err != nil { return err diff --git a/cmd/delete-all.go b/cmd/delete-all.go index 4d394ed0..c3c37dfb 100644 --- a/cmd/delete-all.go +++ b/cmd/delete-all.go @@ -7,6 +7,7 @@ import ( "github.com/elasticpath/epcc-cli/external/apihelper" "github.com/elasticpath/epcc-cli/external/completion" "github.com/elasticpath/epcc-cli/external/httpclient" + "github.com/elasticpath/epcc-cli/external/id" "github.com/elasticpath/epcc-cli/external/resources" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -48,9 +49,9 @@ var DeleteAll = &cobra.Command{ } for _, parentEntityIds := range allParentEntityIds { - lastIds := make([][]string, 1) + lastIds := make([][]id.IdableAttributes, 1) for { - resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo, parentEntityIds) + resourceURL, err := resources.GenerateUrlViaIdableAttributes(resource.GetCollectionInfo, parentEntityIds) if err != nil { return err @@ -68,7 +69,7 @@ var DeleteAll = &cobra.Command{ ids, err := apihelper.GetResourceIdsFromHttpResponse(resp) resp.Body.Close() - allIds := make([][]string, 0) + allIds := make([][]id.IdableAttributes, 0) for _, id := range ids { allIds = append(allIds, append(parentEntityIds, id)) } @@ -96,7 +97,7 @@ var DeleteAll = &cobra.Command{ break } - delPage(resource.PluralName, allIds) + delPage(resource.DeleteEntityInfo, allIds) } } @@ -116,9 +117,9 @@ var DeleteAll = &cobra.Command{ } // -func getParentIds(ctx context.Context, resource resources.Resource) ([][]string, error) { +func getParentIds(ctx context.Context, resource resources.Resource) ([][]id.IdableAttributes, error) { - myEntityIds := make([][]string, 0) + myEntityIds := make([][]id.IdableAttributes, 0) if resource.GetCollectionInfo == nil { return myEntityIds, fmt.Errorf("resource %s doesn't support GET collection", resource.PluralName) } @@ -130,7 +131,7 @@ func getParentIds(ctx context.Context, resource resources.Resource) ([][]string, } if len(types) == 0 { - myEntityIds = append(myEntityIds, make([]string, 0)) + myEntityIds = append(myEntityIds, make([]id.IdableAttributes, 0)) return myEntityIds, nil } else { immediateParentType := types[len(types)-1] @@ -145,19 +146,30 @@ func getParentIds(ctx context.Context, resource resources.Resource) ([][]string, } } -func delPage(resourceName string, ids [][]string) { +func delPage(urlInfo *resources.CrudEntityInfo, ids [][]id.IdableAttributes) { // Create a wait group to run DELETE in parallel wg := sync.WaitGroup{} - for _, id := range ids { + for _, idAttr := range ids { wg.Add(1) - go func(id []string) { - args := make([]string, 0) - args = append(args, resourceName) - args = append(args, id...) - - deleteResource(args) - wg.Done() - }(id) + go func(idAttr []id.IdableAttributes) { + + defer wg.Done() + // Find Resource + // Replace ids with args in resourceURL + resourceURL, err := resources.GenerateUrlViaIdableAttributes(urlInfo, idAttr) + + if err != nil { + return + } + + // Submit request + resp, err := httpclient.DoRequest(context.TODO(), "DELETE", resourceURL, "", nil) + if err != nil { + return + } + defer resp.Body.Close() + + }(idAttr) } wg.Wait() } diff --git a/cmd/delete.go b/cmd/delete.go index 87b6db95..186afb03 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -100,7 +100,7 @@ func deleteResource(args []string) (*http.Response, error) { } // Replace ids with args in resourceURL - resourceURL, err := resources.GenerateUrl(resource, resource.DeleteEntityInfo, args[1:]) + resourceURL, err := resources.GenerateUrl(resource.DeleteEntityInfo, args[1:]) if err != nil { return nil, err diff --git a/cmd/get.go b/cmd/get.go index c7d06508..911249ba 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -142,7 +142,7 @@ func getResource(args []string) (*http.Response, error) { } // Replace ids with args in resourceURL - resourceURL, err := resources.GenerateUrl(resource, resourceUrlInfo, args[1:]) + resourceURL, err := resources.GenerateUrl(resourceUrlInfo, args[1:]) if err != nil { return nil, err diff --git a/cmd/reset-store.go b/cmd/reset-store.go index dc0e786c..1b58ad30 100644 --- a/cmd/reset-store.go +++ b/cmd/reset-store.go @@ -28,7 +28,7 @@ var ResetStore = &cobra.Command{ return fmt.Errorf("could not find resource %s, we need it to determine the store id.", args[0]) } - resourceURL, err := resources.GenerateUrl(resource, resource.GetCollectionInfo, make([]string, 0)) + resourceURL, err := resources.GenerateUrl(resource.GetCollectionInfo, make([]string, 0)) if err != nil { return err diff --git a/cmd/update.go b/cmd/update.go index 7c7eef75..553fd04a 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -37,7 +37,7 @@ var update = &cobra.Command{ } // Replace ids with args in resourceURL - resourceURL, err := resources.GenerateUrl(resource, resourceUrlInfo, args[1:]) + resourceURL, err := resources.GenerateUrl(resourceUrlInfo, args[1:]) if err != nil { return err } diff --git a/external/aliases/aliases.go b/external/aliases/aliases.go index b9ddb0d5..11485dad 100644 --- a/external/aliases/aliases.go +++ b/external/aliases/aliases.go @@ -3,6 +3,7 @@ package aliases import ( "encoding/json" "fmt" + "github.com/elasticpath/epcc-cli/external/id" "github.com/elasticpath/epcc-cli/external/profiles" "github.com/google/uuid" log "github.com/sirupsen/logrus" @@ -21,12 +22,6 @@ var filelock = sync.Mutex{} var aliasDirectoryOverride = "" -type AliasAttributes struct { - Id string `yaml:"id"` - Slug string `yaml:"slug"` - Sku string `yaml:"sku"` -} - func ClearAllAliases() error { aliasDataDirectory := getAliasDataDirectory() @@ -47,11 +42,11 @@ func ClearAllAliasesForJsonApiType(jsonApiType string) error { } -func GetAliasesForJsonApiType(jsonApiType string) map[string]*AliasAttributes { +func GetAliasesForJsonApiType(jsonApiType string) map[string]*id.IdableAttributes { profileDirectory := getAliasDataDirectory() aliasFile := getAliasFileForJsonApiType(profileDirectory, jsonApiType) - aliasMap := map[string]*AliasAttributes{} + aliasMap := map[string]*id.IdableAttributes{} data, err := ioutil.ReadFile(aliasFile) if err != nil { @@ -68,9 +63,20 @@ func GetAliasesForJsonApiType(jsonApiType string) map[string]*AliasAttributes { return aliasMap } -func ResolveAliasValuesOrReturnIdentity(jsonApiType string, value string) string { +func ResolveAliasValuesOrReturnIdentity(jsonApiType string, value string, attribute string) string { if result, ok := GetAliasesForJsonApiType(jsonApiType)[value]; ok { - return result.Id + + if attribute == "id" { + return result.Id + } + if attribute == "slug" { + return result.Slug + } + + if attribute == "sku" { + return result.Sku + } + } return value } @@ -83,7 +89,7 @@ func SaveAliasesForResources(jsonTxt string) { return } - results := map[string]map[string]*AliasAttributes{} + results := map[string]map[string]*id.IdableAttributes{} visitResources(jsonStruct, "", results) log.Tracef("All aliases: %v", results) @@ -94,16 +100,15 @@ func SaveAliasesForResources(jsonTxt string) { } -func DeleteAliasesById(id string, jsonApiType string) { - modifyAliases(jsonApiType, func(m map[string]*AliasAttributes) { +func DeleteAliasesById(idStr string, jsonApiType string) { + modifyAliases(jsonApiType, func(m map[string]*id.IdableAttributes) { for key, value := range m { - if value.Id == id { + if value.Id == idStr { delete(m, key) } } }, ) - } func getAliasDataDirectory() string { @@ -128,7 +133,7 @@ func getAliasFileForJsonApiType(profileDirectory string, resourceType string) st return aliasFile } -func modifyAliases(jsonApiType string, fn func(map[string]*AliasAttributes)) map[string]*AliasAttributes { +func modifyAliases(jsonApiType string, fn func(map[string]*id.IdableAttributes)) map[string]*id.IdableAttributes { profileDirectory := getAliasDataDirectory() filelock.Lock() defer filelock.Unlock() @@ -140,7 +145,7 @@ func modifyAliases(jsonApiType string, fn func(map[string]*AliasAttributes)) map data = []byte{} } - existingAliases := map[string]*AliasAttributes{} + existingAliases := map[string]*id.IdableAttributes{} err = yaml.Unmarshal(data, existingAliases) if err != nil { @@ -170,8 +175,8 @@ func modifyAliases(jsonApiType string, fn func(map[string]*AliasAttributes)) map } // This function saves all the aliases for a specific resource. -func saveAliasesForResource(jsonApiType string, newAliases map[string]*AliasAttributes) { - modifyAliases(jsonApiType, func(aliasMap map[string]*AliasAttributes) { +func saveAliasesForResource(jsonApiType string, newAliases map[string]*id.IdableAttributes) { + modifyAliases(jsonApiType, func(aliasMap map[string]*id.IdableAttributes) { // Aliases have the format KEY=VALUE and this maps to an ID. // This code checks for where two aliases have the same KEY and same ID, and replaces the old value, with the new one. @@ -196,7 +201,7 @@ func saveAliasesForResource(jsonApiType string, newAliases map[string]*AliasAttr }) } -func visitResources(data map[string]interface{}, prefix string, results map[string]map[string]*AliasAttributes) { +func visitResources(data map[string]interface{}, prefix string, results map[string]map[string]*id.IdableAttributes) { if typeObj, typeKeyExists := data["type"]; typeKeyExists { if idObj, idKeyExists := data["id"]; idKeyExists { if typeKeyValue, typeKeyIsString := typeObj.(string); typeKeyIsString { @@ -206,7 +211,7 @@ func visitResources(data map[string]interface{}, prefix string, results map[stri log.Tracef("Found a type and id pair %s => %s under prefix %s, aliases %v", typeKeyValue, idKeyValue, prefix, aliases) if _, ok := results[typeKeyValue]; !ok { - results[typeKeyValue] = make(map[string]*AliasAttributes) + results[typeKeyValue] = make(map[string]*id.IdableAttributes) } for aliasKey, aliasValue := range aliases { @@ -237,12 +242,12 @@ func visitResources(data map[string]interface{}, prefix string, results map[stri return } -func generateAliasesForStruct(typeKey string, idKey string, data map[string]interface{}) map[string]*AliasAttributes { - result := AliasAttributes{ +func generateAliasesForStruct(typeKey string, idKey string, data map[string]interface{}) map[string]*id.IdableAttributes { + result := id.IdableAttributes{ Id: idKey, } - results := map[string]*AliasAttributes{ + results := map[string]*id.IdableAttributes{ // Identity, objects should be an alias of themselves. "id=" + idKey: &result, } diff --git a/external/aliases/aliases_test.go b/external/aliases/aliases_test.go index b4b49792..230e4a23 100644 --- a/external/aliases/aliases_test.go +++ b/external/aliases/aliases_test.go @@ -630,7 +630,7 @@ func TestResolveAliasValuesReturnsAliasForMatchingValue(t *testing.T) { "type": "foo" } }`) - value := ResolveAliasValuesOrReturnIdentity("foo", "id=123") + value := ResolveAliasValuesOrReturnIdentity("foo", "id=123", "id") // Verification @@ -658,7 +658,7 @@ func TestResolveAliasValuesReturnsRequestForUnMatchingValue(t *testing.T) { } }`) - value := ResolveAliasValuesOrReturnIdentity("foo", "id=ABC") + value := ResolveAliasValuesOrReturnIdentity("foo", "id=ABC", "id") // Verification @@ -687,7 +687,7 @@ func TestResolveAliasValuesReturnsRequestForUnMatchingValueAndType(t *testing.T) } }`) - value := ResolveAliasValuesOrReturnIdentity("bar", "id=XYZ") + value := ResolveAliasValuesOrReturnIdentity("bar", "id=XYZ", "id") // Verification diff --git a/external/apihelper/get_all_ids.go b/external/apihelper/get_all_ids.go index bc2ea6f7..6d6816bc 100644 --- a/external/apihelper/get_all_ids.go +++ b/external/apihelper/get_all_ids.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "github.com/elasticpath/epcc-cli/external/httpclient" + "github.com/elasticpath/epcc-cli/external/id" "github.com/elasticpath/epcc-cli/external/resources" log "github.com/sirupsen/logrus" "net/url" @@ -11,16 +12,16 @@ import ( ) // -func GetAllIds(ctx context.Context, resource *resources.Resource) ([][]string, error) { +func GetAllIds(ctx context.Context, resource *resources.Resource) ([][]id.IdableAttributes, error) { // TODO make this a channel based instead of array based // This must be an unbuffered channel since the receiver won't get the channel until after we have sent in some cases. //myEntityIds := make(chan<- []string, 1024) //defer close(myEntityIds) - myEntityIds := make([][]string, 0) + myEntityIds := make([][]id.IdableAttributes, 0) if resource == nil { - myEntityIds = append(myEntityIds, make([]string, 0)) + myEntityIds = append(myEntityIds, make([]id.IdableAttributes, 0)) return myEntityIds, nil } @@ -57,13 +58,13 @@ func GetAllIds(ctx context.Context, resource *resources.Resource) ([][]string, e // For each parent entity id we need to loop over the entire collection for _, parentEntityIds := range myParentEntityIds { - resourceURL, err := resources.GenerateUrl(*resource, resource.GetCollectionInfo, parentEntityIds) + resourceURL, err := resources.GenerateUrlViaIdableAttributes(resource.GetCollectionInfo, parentEntityIds) if err != nil { return myEntityIds, err } - lastPageIds := make([]string, 125) + lastPageIds := make([]id.IdableAttributes, 125) for i := 0; i < 10000; i += 25 { params := url.Values{} params.Add("page[limit]", "25") diff --git a/external/apihelper/map_collection_response_to_ids.go b/external/apihelper/map_collection_response_to_ids.go index 2f930cc8..9728e86a 100644 --- a/external/apihelper/map_collection_response_to_ids.go +++ b/external/apihelper/map_collection_response_to_ids.go @@ -3,12 +3,13 @@ package apihelper import ( json2 "encoding/json" "fmt" + "github.com/elasticpath/epcc-cli/external/id" log "github.com/sirupsen/logrus" "io/ioutil" "net/http" ) -func GetResourceIdsFromHttpResponse(resp *http.Response) ([]string, error) { +func GetResourceIdsFromHttpResponse(resp *http.Response) ([]id.IdableAttributes, error) { // Read the body body, err := ioutil.ReadAll(resp.Body) @@ -24,13 +25,34 @@ func GetResourceIdsFromHttpResponse(resp *http.Response) ([]string, error) { } // Collect ids from GET Collection output - var ids []string + var ids []id.IdableAttributes for _, val := range jsonStruct { if arrayType, ok := val.([]interface{}); ok { for _, value := range arrayType { if mapValue, ok := value.(map[string]interface{}); ok { + match := false + + idAttr := id.IdableAttributes{} if id, ok := mapValue["id"].(string); ok { - ids = append(ids, id) + + match = true + idAttr.Id = id + } + + if slug, ok := mapValue["slug"].(string); ok { + + match = true + idAttr.Slug = slug + } + + if sku, ok := mapValue["sku"].(string); ok { + + match = true + idAttr.Sku = sku + } + + if match { + ids = append(ids, idAttr) } } } diff --git a/external/completion/completion.go b/external/completion/completion.go index 542bc84a..4acd90d1 100644 --- a/external/completion/completion.go +++ b/external/completion/completion.go @@ -4,7 +4,6 @@ import ( "github.com/elasticpath/epcc-cli/external/aliases" "github.com/elasticpath/epcc-cli/external/resources" "github.com/spf13/cobra" - "log" "strconv" "strings" ) @@ -139,54 +138,59 @@ func Complete(c Request) ([]string, cobra.ShellCompDirective) { if i != -1 && j != -1 { attr = attr[:i+1] + "n" + attr[j:] } - log.Println("here: " + attr) - if c.Resource.Attributes[attr].Type == "BOOL" { - results = append(results, "true", "false") - } else if strings.HasPrefix(c.Resource.Attributes[attr].Type, "ENUM:") { - enums := strings.Replace(c.Resource.Attributes[attr].Type, "ENUM:", "", 1) - for _, k := range strings.Split(enums, ",") { - results = append(results, k) - } - } else if c.Resource.Attributes[attr].Type == "URL" { - results = append(results, "https://") - compDir = compDir | cobra.ShellCompDirectiveNoSpace - } else if strings.HasPrefix(c.Resource.Attributes[attr].Type, "RESOURCE_ID:") { - resourceType := strings.Replace(c.Resource.Attributes[attr].Type, "RESOURCE_ID:", "", 1) - - if aliasType, ok := resources.GetResourceByName(resourceType); ok { - for alias := range aliases.GetAliasesForJsonApiType(aliasType.JsonApiType) { - results = append(results, alias) + if attribute := c.Resource.Attributes[attr]; attribute != nil { + + if attribute.Type == "BOOL" { + results = append(results, "true", "false") + } else if strings.HasPrefix(attribute.Type, "ENUM:") { + enums := strings.Replace(attribute.Type, "ENUM:", "", 1) + for _, k := range strings.Split(enums, ",") { + results = append(results, k) + } + } else if attribute.Type == "URL" { + results = append(results, "https://") + compDir = compDir | cobra.ShellCompDirectiveNoSpace + } else if strings.HasPrefix(attribute.Type, "RESOURCE_ID:") { + resourceType := strings.Replace(attribute.Type, "RESOURCE_ID:", "", 1) + + if aliasType, ok := resources.GetResourceByName(resourceType); ok { + for alias := range aliases.GetAliasesForJsonApiType(aliasType.JsonApiType) { + results = append(results, alias) + } + } + } else if attribute.Type == "SINGULAR_RESOURCE_TYPE" { + results = append(results, resources.GetSingularResourceNames()...) + + } else if attribute.Type == "CURRENCY" { + currencies := []string{"AED", "AFN", "ALL", "AMD", "ANG", "AOA", "ARS", "AUD", "AWG", "AZN", + "BAM", "BBD", "BDT", "BGN", "BHD", "BIF", "BMD", "BND", "BOB", "BRL", "BSD", "BTN", "BWP", "BYN", "BZD", + "CAD", "CDF", "CHF", "CLP", "CNY", "COP", "CRC", "CUC", "CUP", "CVE", "CZK", + "DJF", "DKK", "DOP", "DZD", + "EGP", "ERN", "ETB", "EUR", + "FJD", "FKP", + "GBP", "GEL", "GGP", "GHS", "GIP", "GMD", "GNF", "GTQ", "GYD", + "HKD", "HNL", "HRK", "HTG", "HUF", + "IDR", "ILS", "IMP", "INR", "IQD", "IRR", "ISK", + "JEP", "JMD", "JOD", "JPY", + "KES", "KGS", "KHR", "KMF", "KPW", "KRW", "KWD", "KYD", "KZT", + "LAK", "LBP", "LKR", "LRD", "LSL", "LYD", + "MAD", "MDL", "MGA", "MKD", "MMK", "MNT", "MOP", "MRU", "MUR", "MVR", "MWK", "MXN", "MYR", "MZN", + "NAD", "NGN", "NIO", "NOK", "NPR", "NZD", + "OMR", + "PAB", "PEN", "PGK", "PHP", "PKR", "PLN", "PYG", + "QAR", + "RON", "RSD", "RUB", "RWF", + "SAR", "SBD", "SCR", "SDG", "SEK", "SGD", "SHP", "SLL", "SOS", "SPL", "SRD", "STN", "SVC", "SYP", "SZL", + "THB", "TJS", "TMT", "TND", "TOP", "TRY", "TTD", "TVD", "TWD", "TZS", + "UAH", "UGX", "USD", "UYU", "UZS", + "VEF", "VND", "VUV", + "WST", + "XAF", "XCD", "XDR", "XOF", "XPF", + "YER", + "ZAR", "ZMW", "ZWD"} + for _, currency := range currencies { + results = append(results, currency) } - } - } else if c.Resource.Attributes[c.Attribute].Type == "CURRENCY" { - currencies := []string{"AED", "AFN", "ALL", "AMD", "ANG", "AOA", "ARS", "AUD", "AWG", "AZN", - "BAM", "BBD", "BDT", "BGN", "BHD", "BIF", "BMD", "BND", "BOB", "BRL", "BSD", "BTN", "BWP", "BYN", "BZD", - "CAD", "CDF", "CHF", "CLP", "CNY", "COP", "CRC", "CUC", "CUP", "CVE", "CZK", - "DJF", "DKK", "DOP", "DZD", - "EGP", "ERN", "ETB", "EUR", - "FJD", "FKP", - "GBP", "GEL", "GGP", "GHS", "GIP", "GMD", "GNF", "GTQ", "GYD", - "HKD", "HNL", "HRK", "HTG", "HUF", - "IDR", "ILS", "IMP", "INR", "IQD", "IRR", "ISK", - "JEP", "JMD", "JOD", "JPY", - "KES", "KGS", "KHR", "KMF", "KPW", "KRW", "KWD", "KYD", "KZT", - "LAK", "LBP", "LKR", "LRD", "LSL", "LYD", - "MAD", "MDL", "MGA", "MKD", "MMK", "MNT", "MOP", "MRU", "MUR", "MVR", "MWK", "MXN", "MYR", "MZN", - "NAD", "NGN", "NIO", "NOK", "NPR", "NZD", - "OMR", - "PAB", "PEN", "PGK", "PHP", "PKR", "PLN", "PYG", - "QAR", - "RON", "RSD", "RUB", "RWF", - "SAR", "SBD", "SCR", "SDG", "SEK", "SGD", "SHP", "SLL", "SOS", "SPL", "SRD", "STN", "SVC", "SYP", "SZL", - "THB", "TJS", "TMT", "TND", "TOP", "TRY", "TTD", "TVD", "TWD", "TZS", - "UAH", "UGX", "USD", "UYU", "UZS", - "VEF", "VND", "VUV", - "WST", - "XAF", "XCD", "XDR", "XOF", "XPF", - "YER", - "ZAR", "ZMW", "ZWD"} - for _, currency := range currencies { - results = append(results, currency) } } } diff --git a/external/id/idable_attributes.go b/external/id/idable_attributes.go new file mode 100644 index 00000000..c29bb8f3 --- /dev/null +++ b/external/id/idable_attributes.go @@ -0,0 +1,7 @@ +package id + +type IdableAttributes struct { + Id string `yaml:"id"` + Slug string `yaml:"slug"` + Sku string `yaml:"sku"` +} diff --git a/external/json/to_json.go b/external/json/to_json.go index cd78e9a8..1072d668 100644 --- a/external/json/to_json.go +++ b/external/json/to_json.go @@ -74,7 +74,7 @@ func toJsonObject(args []string, noWrapping bool, compliant bool, attributes map resourceType := strings.Replace(attributeInfo.Type, "RESOURCE_ID:", "", 1) if aliasType, ok := resources.GetResourceByName(resourceType); ok { - val = aliases.ResolveAliasValuesOrReturnIdentity(aliasType.JsonApiType, val) + val = aliases.ResolveAliasValuesOrReturnIdentity(aliasType.JsonApiType, val, "id") } else { log.Warnf("Could not find a resource for %s, this is a bug.", resourceType) } @@ -157,7 +157,9 @@ func RunJQ(queryStr string, result interface{}) (interface{}, error) { break } if err, ok := v.(error); ok { - log.Fatalln(err) + partialResult, _ := gojson.Marshal(result) + + return nil, fmt.Errorf("error %w when running query %s on json %s", err, queryStr, partialResult) } result = v diff --git a/external/resources/resources.go b/external/resources/resources.go index a7350e0d..460cf0fd 100644 --- a/external/resources/resources.go +++ b/external/resources/resources.go @@ -73,6 +73,9 @@ type CrudEntityInfo struct { // Minimum resources so we don't keep trying to delete in MinResources int `yaml:"min"` + + // Override the attribute we use in the URL for a specific key + ParentResourceValueOverrides map[string]string `yaml:"parent_resource_value_overrides"` } type CrudEntityAttribute struct { diff --git a/external/resources/resources.yaml b/external/resources/resources.yaml index c3e61525..69de51c2 100644 --- a/external/resources/resources.yaml +++ b/external/resources/resources.yaml @@ -524,6 +524,55 @@ entries: url: "/v2/flows/{flows}/entries" parent_resource_value_overrides: flows: slug + get-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/get-an-entry.html" + url: "/v2/flows/{flows}/entries/{entries}" + parent_resource_value_overrides: + flows: slug + update-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/update-an-entry.html" + url: "/v2/flows/{flows}/entries/{entries}" + parent_resource_value_overrides: + flows: slug + create-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/create-an-entry.html" + url: "/v2/flows/{flows}/entries" + parent_resource_value_overrides: + flows: slug + delete-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entries/delete-an-entry.html" + url: "/v2/flows/{flows}/entries/{entries}" + parent_resource_value_overrides: + flows: slug +entries-relationship: + singular-name: entry-relationship + json-api-type: entry-relationship + json-api-format: "legacy" + no-wrapping: true + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entry-relationships/index.html" + update-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entry-relationships/update-entry-relationships.html" + url: "/v2/flows/{flows}/entries/{entries}/relationships/{fields}" + parent_resource_value_overrides: + flows: slug + fields: slug + create-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entry-relationships/create-an-entry-relationship.html" + url: "/v2/flows/{flows}/entries/{entries}/relationships/{fields}" + parent_resource_value_overrides: + flows: slug + fields: slug + delete-entity: + docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/advanced/custom-data/entry-relationships/delete-entry-relationships.html" + url: "/v2/flows/{flows}/entries/{entries}/relationships/{fields}" + parent_resource_value_overrides: + flows: slug + fields: slug + attributes: + "data[n].id": + type: string + "data[n].type": + type: SINGULAR_RESOURCE_TYPE integration-jobs: singular-name: "integration-job" json-api-type: "integration-job" @@ -1139,9 +1188,9 @@ pcm-node-products: docs: "https://documentation.elasticpath.com/commerce-cloud/docs/api/pcm/hierarchies/relationships/create-node-product-relationships.html" url: "/pcm/hierarchies/{pcm_hierarchies}/nodes/{pcm_nodes}/relationships/products" attributes: - "data[0].id": + "data[n].id": type: RESOURCE_ID:pcm-product - "data[0].type": + "data[n].type": type: ENUM:product pcm-nodes: singular-name: "pcm-node" diff --git a/external/resources/resources_test.go b/external/resources/resources_test.go index 6696f54e..925d69cd 100644 --- a/external/resources/resources_test.go +++ b/external/resources/resources_test.go @@ -21,77 +21,38 @@ func TestUriTemplatesAllReferenceValidResource(t *testing.T) { for key, val := range resources { if val.CreateEntityInfo != nil { - template, err := uritemplate.New(val.CreateEntityInfo.Url) - - if err != nil { - errors += fmt.Sprintf("Could not process CREATE uri for resource %s, error:%s\n", key, err) - } else { - for _, variable := range template.Varnames() { - resourceName := strings.ReplaceAll(variable, "_", "-") - if _, ok := resources[resourceName]; !ok { - errors += fmt.Sprintf("Error processing CREATE uri for resource %s, the URI template references a resource %s, but could not find it\n", key, resourceName) - } - } + err := validateCrudEntityInfo(*val.CreateEntityInfo) + if err != "" { + errors += fmt.Sprintf("Could not process CREATE uri for resource `%s`, error:\n%s\n", key, err) } } if val.UpdateEntityInfo != nil { - template, err := uritemplate.New(val.UpdateEntityInfo.Url) - - if err != nil { - errors += fmt.Sprintf("Could not process UPDATE uri for resource %s, error:%s\n", key, err) - } else { - for _, variable := range template.Varnames() { - resourceName := strings.ReplaceAll(variable, "_", "-") - if _, ok := resources[resourceName]; !ok { - errors += fmt.Sprintf("Error processing UPDATE uri for resource %s, the URI template references a resource %s, but could not find it\n", key, resourceName) - } - } + err := validateCrudEntityInfo(*val.UpdateEntityInfo) + if err != "" { + errors += fmt.Sprintf("Could not process UPDATE uri for resource `%s`, error:\n%s\n", key, err) } } if val.DeleteEntityInfo != nil { - template, err := uritemplate.New(val.DeleteEntityInfo.Url) - - if err != nil { - errors += fmt.Sprintf("Could not process DELETE uri for resource %s, error:%s\n", key, err) - } else { - for _, variable := range template.Varnames() { - resourceName := strings.ReplaceAll(variable, "_", "-") - if _, ok := resources[resourceName]; !ok { - errors += fmt.Sprintf("Error processing DELETE uri for resource %s, the URI template references a resource %s, but could not find it\n", key, resourceName) - } - } + + err := validateCrudEntityInfo(*val.DeleteEntityInfo) + if err != "" { + errors += fmt.Sprintf("Could not process DELETE uri for resource `%s`, error:\n%s\n", key, err) } } if val.GetEntityInfo != nil { - template, err := uritemplate.New(val.GetEntityInfo.Url) - - if err != nil { - errors += fmt.Sprintf("Could not process GET entity uri for resource %s, error:%s\n", key, err) - } else { - for _, variable := range template.Varnames() { - resourceName := strings.ReplaceAll(variable, "_", "-") - if _, ok := resources[resourceName]; !ok { - errors += fmt.Sprintf("Error processing GET entity uri for resource %s, the URI template references a resource %s, but could not find it\n", key, resourceName) - } - } + err := validateCrudEntityInfo(*val.GetEntityInfo) + if err != "" { + errors += fmt.Sprintf("Could not process GET entity uri for resource `%s`, error:\n%s\n", key, err) } } if val.GetCollectionInfo != nil { - template, err := uritemplate.New(val.GetCollectionInfo.Url) - - if err != nil { - errors += fmt.Sprintf("Could not process GET collection uri for resource %s, error:%s\n", key, err) - } else { - for _, variable := range template.Varnames() { - resourceName := strings.ReplaceAll(variable, "_", "-") - if _, ok := resources[resourceName]; !ok { - errors += fmt.Sprintf("Error processing GET collection uri for resource %s, the URI template references a resource %s, but could not find it\n", key, resourceName) - } - } + err := validateCrudEntityInfo(*val.GetCollectionInfo) + if err != "" { + errors += fmt.Sprintf("Could not process GET collection uri for resource `%s`, error:\n%s\n", key, err) } } } @@ -103,6 +64,37 @@ func TestUriTemplatesAllReferenceValidResource(t *testing.T) { } } +func validateCrudEntityInfo(info CrudEntityInfo) string { + errors := "" + + template, err := uritemplate.New(info.Url) + if err != nil { + errors += fmt.Sprintf("\tCould not process Uri %s for templates error:%s\n", info.Url, err) + } else { + variables := map[string]bool{} + for _, variable := range template.Varnames() { + variables[variable] = true + resourceName := strings.ReplaceAll(variable, "_", "-") + if _, ok := resources[resourceName]; !ok { + errors += fmt.Sprintf("\tError processing Uri %s, the URI template references a resource %s, but could not find it\n", info.Url, resourceName) + } + } + + for key, value := range info.ParentResourceValueOverrides { + if value != "slug" && value != "sku" && value != "id" { + errors += fmt.Sprintf("\tUrl %s has an invalid override for %s => %s\n", info.Url, key, value) + } + + if _, ok := variables[key]; !ok { + errors += fmt.Sprintf("\tUrl %s has an invalid override for %s, this key doesn't exist in the URL", info.Url, key) + } + } + + } + + return errors +} + func TestJsonSchemaValidate(t *testing.T) { sch, err := jsonschema.Compile("resources_schema.json") if err != nil { diff --git a/external/resources/resources_yaml_test.go b/external/resources/resources_yaml_test.go index 6fb6e9b6..f3386e09 100644 --- a/external/resources/resources_yaml_test.go +++ b/external/resources/resources_yaml_test.go @@ -6,6 +6,9 @@ import ( ) func TestResourceDocsExist(t *testing.T) { + if true { + return + } const httpStatusCodeOk = 200 Resources := GetPluralResources() diff --git a/external/resources/uritemplates.go b/external/resources/uritemplates.go index 94b47a23..404e94f9 100644 --- a/external/resources/uritemplates.go +++ b/external/resources/uritemplates.go @@ -3,12 +3,70 @@ package resources import ( "fmt" "github.com/elasticpath/epcc-cli/external/aliases" + "github.com/elasticpath/epcc-cli/external/id" log "github.com/sirupsen/logrus" "github.com/yosida95/uritemplate/v3" "strings" ) -func GenerateUrl(resource Resource, urlInfo *CrudEntityInfo, args []string) (string, error) { +func GenerateUrlViaIdableAttributes(urlInfo *CrudEntityInfo, args []id.IdableAttributes) (string, error) { + + template, err := uritemplate.New(urlInfo.Url) + + if err != nil { + return "", fmt.Errorf("could not generate URI template for URL: %w", err) + } + + vars := template.Varnames() + + if len(vars) > len(args) { + return "", fmt.Errorf("URI Template requires %d arguments, but only %d were passed", len(vars), len(args)) + } + + values := uritemplate.Values{} + + for idx, varName := range vars { + resourceType := convertUriTemplateValueToType(varName) + _, ok := GetResourceByName(resourceType) + if ok { + attribute := "id" + + if override, ok := urlInfo.ParentResourceValueOverrides[resourceType]; ok { + log.Tracef("url %s uses a type [%s] instead of id, so URL will be filled with this", urlInfo.Url, override) + attribute = override + } + + value := "" + if attribute == "id" { + value = args[idx].Id + } + + if attribute == "slug" { + value = args[idx].Slug + } + + if attribute == "sku" { + value = args[idx].Sku + } + + if value == "" { + log.Warnf("Value for attribute %s is empty, url may not generate correctly", attribute) + } + + values[varName] = uritemplate.String(value) + + } else { + log.Warnf("Could not find a resource with type %s, aliases are probably broken", resourceType) + values[varName] = uritemplate.String(args[idx].Id) + } + + } + + return template.Expand(values) + +} + +func GenerateUrl(urlInfo *CrudEntityInfo, args []string) (string, error) { template, err := uritemplate.New(urlInfo.Url) if err != nil { @@ -27,7 +85,12 @@ func GenerateUrl(resource Resource, urlInfo *CrudEntityInfo, args []string) (str resourceType := convertUriTemplateValueToType(varName) varType, ok := GetResourceByName(resourceType) if ok { - values[varName] = uritemplate.String(aliases.ResolveAliasValuesOrReturnIdentity(varType.JsonApiType, args[idx])) + attribute := "id" + if override, ok := urlInfo.ParentResourceValueOverrides[resourceType]; ok { + log.Tracef("url %s uses a type [%s] instead of id, so URL will be filled with this", urlInfo.Url, override) + attribute = override + } + values[varName] = uritemplate.String(aliases.ResolveAliasValuesOrReturnIdentity(varType.JsonApiType, args[idx], attribute)) } else { log.Warnf("Could not find a resource with type %s, aliases are probably broken", resourceType) values[varName] = uritemplate.String(args[idx]) diff --git a/external/resources/uritemplates_test.go b/external/resources/uritemplates_test.go new file mode 100644 index 00000000..6eae90e8 --- /dev/null +++ b/external/resources/uritemplates_test.go @@ -0,0 +1,226 @@ +package resources + +import ( + "github.com/elasticpath/epcc-cli/external/aliases" + "testing" +) + +func TestGetNumberOfVariablesReturnsErrorOnTemplate(t *testing.T) { + + // Fixture Setup + url := "/v2/{te" + + // Execute SUT + _, err := GetNumberOfVariablesNeeded(url) + + // Verification + if err == nil { + t.Errorf("An invalid uri template should have given us an error, not nil ") + } + +} + +func TestGetNumberOfVariablesNeededIsZeroWhenNoVariablesNeeded(t *testing.T) { + + // Fixture Setup + url := "/v2/flows" + + // Execute SUT + numberOfVariablesNeeded, _ := GetNumberOfVariablesNeeded(url) + + // Verification + + if numberOfVariablesNeeded != 0 { + t.Errorf("Expected that the number of variables needed was 0, but got %d", numberOfVariablesNeeded) + } + +} + +func TestGetNumberOfVariablesNeededIsOneWhenOneVariablesNeeded(t *testing.T) { + + // Fixture Setup + url := "/v2/flows/{flows}" + + // Execute SUT + numberOfVariablesNeeded, _ := GetNumberOfVariablesNeeded(url) + + // Verification + + if numberOfVariablesNeeded != 1 { + t.Errorf("Expected that the number of variables needed was 1, but got %d", numberOfVariablesNeeded) + } + +} + +func TestGetNumberOfVariablesNeededIsThreeWhenThreeVariablesNeeded(t *testing.T) { + + // Fixture Setup + url := "/v2/flows/{flows}/{accounts}/{customers}" + + // Execute SUT + numberOfVariablesNeeded, _ := GetNumberOfVariablesNeeded(url) + + // Verification + + if numberOfVariablesNeeded != 3 { + t.Errorf("Expected that the number of variables needed was 3, but got %d", numberOfVariablesNeeded) + } + +} + +func TestGetTypesOfVariablesNeededReturnsErrorWithInvalidUriTemplate(t *testing.T) { + // Fixture Setup + url := "/v2/{tes" + + // Execute SUT + _, err := GetTypesOfVariablesNeeded(url) + + // Verification + if err == nil { + t.Errorf("An invalid uri template should have given us an error, not nil ") + } +} + +func TestGetTypesOfVariablesNeededReturnsEmptyArrayWhenNoArguments(t *testing.T) { + // Fixture Setup + url := "/v2/customers" + + // Execute SUT + types, err := GetTypesOfVariablesNeeded(url) + + // Verification + if err != nil { + t.Errorf("We should not have gotten an error in this case :(, but got %v", err) + } + + if len(types) != 0 { + t.Errorf("Expected the number of types returned is 0, but got %d", len(types)) + } + +} + +func TestGetTypesOfVariablesNeededReturnsTypeInBaseCase(t *testing.T) { + // Fixture Setup + url := "/v2/{customers}" + + // Execute SUT + types, err := GetTypesOfVariablesNeeded(url) + + // Verification + if err != nil { + t.Errorf("We should not have gotten an error in this case :(, but got %v", err) + } + + if len(types) != 1 { + t.Errorf("Expected the number of types returned is 1, but got %d", len(types)) + } + + if types[0] != "customers" { + t.Errorf("Expected that the type of the first argument is customers, not %s", types[0]) + } +} + +func TestGetTypesOfVariablesNeededReturnsTypeWithThreeVariables(t *testing.T) { + // Fixture Setup + url := "/v2/{customers}/addresses/{flows}/flows/{entries}" + + // Execute SUT + types, err := GetTypesOfVariablesNeeded(url) + + // Verification + if err != nil { + t.Errorf("We should not have gotten an error in this case :(, but got %v", err) + } + + if len(types) != 3 { + t.Errorf("Expected the number of types returned is 3, but got %d", len(types)) + } + + if types[0] != "customers" { + t.Errorf("Expected that the type of the first argument is customers, not %s", types[0]) + } + + if types[1] != "flows" { + t.Errorf("Expected that the type of the second argument is flows, not %s", types[1]) + } + + if types[2] != "entries" { + t.Errorf("Expected that the type of the third argument is entries, not %s", types[2]) + } + +} + +func TestUriTemplatesTypeConversionConvertsUnderscoresToDashes(t *testing.T) { + // Fixture Setup + url := "/v2/customers/{customers}/addresses/{customer_addresses}" + + // Execute SUT + types, err := GetTypesOfVariablesNeeded(url) + + // Verification + if err != nil { + t.Errorf("We should not have gotten an error in this case :(, but got %v", err) + } + + if len(types) != 2 { + t.Errorf("Expected the number of types returned is 2, but got %d", len(types)) + } + + if types[0] != "customers" { + t.Errorf("Expected that the type of the first argument is customers, not %s", types[0]) + } + + if types[1] != "customer-addresses" { + t.Errorf("Expected that the type of the second argument is customer_addresses, not %s", types[1]) + } + +} + +func TestGenerateUrlHappyPath(t *testing.T) { + // Fixture Setup + crudEntityInfo := getValidCrudEntityInfo() + crudEntityInfo.Url = "/v2/flows/{flows}" + crudEntityInfo.ParentResourceValueOverrides = map[string]string{ + "flows": "slug", + } + + flowExample := `{ + "data": { + "id": "123", + "type": "flows" + "slug": "test" + } +}` + + err := aliases.ClearAllAliases() + if err != nil { + t.Errorf("Couldn't create test fixtures, error while cleaning aliases, %v", err) + } + aliases.SaveAliasesForResources(flowExample) + + expectedUrlWithSlugNotId := "/v2/flows/test" + + // Execute SUT + + actualUrl, err := GenerateUrl(&crudEntityInfo, []string{"slug=test"}) + + // Verification + + if err != nil { + t.Errorf("Should not have gotten error when generating URL.") + } + + if actualUrl != expectedUrlWithSlugNotId { + t.Errorf("Url should have been %s but got %s", expectedUrlWithSlugNotId, actualUrl) + } +} + +func getValidCrudEntityInfo() CrudEntityInfo { + return CrudEntityInfo{ + Docs: "https://www.google.ca", + Url: "/v2/flows/{flows}", + ContentType: "application/json", + QueryParameters: "", + MinResources: 0, + } +} From f7addcc6b21ce97aa397e5f07cd84072ddf83625 Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Sun, 24 Jul 2022 16:42:57 -0700 Subject: [PATCH 6/6] Fix some tests --- external/aliases/aliases.go | 10 ++++ external/aliases/aliases_test.go | 9 +--- external/resources/resources.yaml | 6 +-- external/resources/resources_schema.json | 42 +++++++++++++---- external/resources/resources_yaml_test.go | 3 -- external/resources/uritemplates_test.go | 57 ++++++++++++++++++++--- 6 files changed, 97 insertions(+), 30 deletions(-) diff --git a/external/aliases/aliases.go b/external/aliases/aliases.go index 11485dad..d20e72a4 100644 --- a/external/aliases/aliases.go +++ b/external/aliases/aliases.go @@ -312,3 +312,13 @@ func getAttributeValueForKey(key string, data map[string]interface{}) string { } } + +func InitializeAliasDirectoryForTesting() { + dir, err := ioutil.TempDir("", "epcc-cli-aliases-testing") + if err != nil { + log.Panic("Could not create directory", err) + } + + aliasDirectoryOverride = dir + log.Infof("Alias directory for tests is %s", dir) +} diff --git a/external/aliases/aliases_test.go b/external/aliases/aliases_test.go index 230e4a23..50b9c3c9 100644 --- a/external/aliases/aliases_test.go +++ b/external/aliases/aliases_test.go @@ -1,20 +1,13 @@ package aliases import ( - log "github.com/sirupsen/logrus" "io/ioutil" "os" "testing" ) func init() { - dir, err := ioutil.TempDir("", "epcc-cli-aliases-testing") - if err != nil { - log.Panic("Could not create directory", err) - } - - aliasDirectoryOverride = dir - log.Infof("Alias directory for tests is %s", dir) + InitializeAliasDirectoryForTesting() } func TestSavedAliasIsReturnedInAllAliasesForSingleResponse(t *testing.T) { diff --git a/external/resources/resources.yaml b/external/resources/resources.yaml index 69de51c2..31066018 100644 --- a/external/resources/resources.yaml +++ b/external/resources/resources.yaml @@ -569,9 +569,9 @@ entries-relationship: flows: slug fields: slug attributes: - "data[n].id": - type: string - "data[n].type": + data[n].id: + type: STRING + data[n].type: type: SINGULAR_RESOURCE_TYPE integration-jobs: singular-name: "integration-job" diff --git a/external/resources/resources_schema.json b/external/resources/resources_schema.json index ddcc9f3f..43fd8feb 100644 --- a/external/resources/resources_schema.json +++ b/external/resources/resources_schema.json @@ -7,14 +7,36 @@ "patternProperties": { "(.*?)": { "type" : "object", + "additionalProperties": false, "properties": { + "singular-name": { + "type":"string", + "pattern": "^[A-Za-z-][A-Za-z0-9-]*$" + }, + "json-api-type": { + "type":"string", + "pattern": "^[A-Za-z-_]+$" + }, + "json-api-format": { + "type":"string", + "enum": ["compliant", "legacy"] + }, + "no-wrapping": { + "type": "boolean" + }, + "suppress-reset-warning": { + "type": "boolean" + }, "docs": { "type": "string", "pattern": "^https://" }, "get-collection": { "type": "object", + "additionalProperties": false, "properties" : { "docs": { "type": "string", "pattern": "^https://" }, "url": { "type": "string" }, - "content-type": { "type": "string" } + "content-type": { "type": "string" }, + "parent_resource_value_overrides": { "type": "object" }, + "query": { "type": "string" } }, "required": [ "url", "docs"] }, @@ -23,7 +45,9 @@ "properties" : { "docs": { "type": "string", "pattern": "^https://" }, "url": { "type": "string" }, - "content-type": { "type": "string" } + "content-type": { "type": "string" }, + "parent_resource_value_overrides": { "type": "object" }, + "query": { "type": "string" } }, "required": [ "url", "docs"] }, @@ -32,7 +56,8 @@ "properties" : { "docs": { "type": "string", "pattern": "^https://" }, "url": { "type": "string" }, - "content-type": { "type": "string" } + "content-type": { "type": "string" }, + "parent_resource_value_overrides": { "type": "object" } }, "required": [ "url", "docs"] }, @@ -41,7 +66,8 @@ "properties" : { "docs": { "type": "string", "pattern": "^https://" }, "url": { "type": "string" }, - "content-type": { "type": "string" } + "content-type": { "type": "string" }, + "parent_resource_value_overrides": { "type": "object" } }, "required": [ "url", "docs"] }, @@ -62,19 +88,15 @@ "properties": { "type": { "type": "string", - "pattern": "(^STRING$|^URL$|^INT$|^ENUM|^FLOAT$|^BOOL$|^FILE$|^CURRENCY$|^RESOURCE_ID)" + "pattern": "^(STRING|URL|INT|ENUM:[a-z0-9A-Z._,-]+|FLOAT|BOOL|FILE|CURRENCY|SINGULAR_RESOURCE_TYPE|RESOURCE_ID:[a-z-]+)$" } }, "required": ["type"] } } - }, - "singular-name": { - "type":"string", - "pattern": "^[A-Za-z-][A-Za-z0-9-]*$" } }, - "required": [ "json-api-type", "json-api-format", "docs"] + "required": [ "json-api-type", "json-api-format", "docs", "singular-name"] } } } \ No newline at end of file diff --git a/external/resources/resources_yaml_test.go b/external/resources/resources_yaml_test.go index f3386e09..6fb6e9b6 100644 --- a/external/resources/resources_yaml_test.go +++ b/external/resources/resources_yaml_test.go @@ -6,9 +6,6 @@ import ( ) func TestResourceDocsExist(t *testing.T) { - if true { - return - } const httpStatusCodeOk = 200 Resources := GetPluralResources() diff --git a/external/resources/uritemplates_test.go b/external/resources/uritemplates_test.go index 6eae90e8..ad88b483 100644 --- a/external/resources/uritemplates_test.go +++ b/external/resources/uritemplates_test.go @@ -5,6 +5,10 @@ import ( "testing" ) +func init() { + aliases.InitializeAliasDirectoryForTesting() +} + func TestGetNumberOfVariablesReturnsErrorOnTemplate(t *testing.T) { // Fixture Setup @@ -176,8 +180,14 @@ func TestUriTemplatesTypeConversionConvertsUnderscoresToDashes(t *testing.T) { } -func TestGenerateUrlHappyPath(t *testing.T) { +func TestGenerateUrlHappyPathWithSlugParentResourceValueOverride(t *testing.T) { // Fixture Setup + + err := aliases.ClearAllAliases() + if err != nil { + t.Errorf("Couldn't create test fixtures, error while cleaning aliases, %v", err) + } + crudEntityInfo := getValidCrudEntityInfo() crudEntityInfo.Url = "/v2/flows/{flows}" crudEntityInfo.ParentResourceValueOverrides = map[string]string{ @@ -187,15 +197,11 @@ func TestGenerateUrlHappyPath(t *testing.T) { flowExample := `{ "data": { "id": "123", - "type": "flows" + "type": "flow", "slug": "test" } }` - err := aliases.ClearAllAliases() - if err != nil { - t.Errorf("Couldn't create test fixtures, error while cleaning aliases, %v", err) - } aliases.SaveAliasesForResources(flowExample) expectedUrlWithSlugNotId := "/v2/flows/test" @@ -215,6 +221,45 @@ func TestGenerateUrlHappyPath(t *testing.T) { } } +func TestGenerateUrlHappyPathWithNoParentResourceValueOverride(t *testing.T) { + // Fixture Setup + + err := aliases.ClearAllAliases() + if err != nil { + t.Errorf("Couldn't create test fixtures, error while cleaning aliases, %v", err) + } + + crudEntityInfo := getValidCrudEntityInfo() + crudEntityInfo.Url = "/v2/customers/{customers}" + crudEntityInfo.ParentResourceValueOverrides = map[string]string{} + + flowExample := `{ + "data": { + "id": "123", + "type": "customer", + "name": "Ron Swanson" + } +}` + + aliases.SaveAliasesForResources(flowExample) + + expectedUrlWithId := "/v2/customers/123" + + // Execute SUT + + actualUrl, err := GenerateUrl(&crudEntityInfo, []string{"name=Ron_Swanson"}) + + // Verification + + if err != nil { + t.Errorf("Should not have gotten error when generating URL.") + } + + if actualUrl != expectedUrlWithId { + t.Errorf("Url should have been %s but got %s", expectedUrlWithId, actualUrl) + } +} + func getValidCrudEntityInfo() CrudEntityInfo { return CrudEntityInfo{ Docs: "https://www.google.ca",