Skip to content

Commit af39c4c

Browse files
author
Paola Nicosia
committed
feat: remove getById and deleteById in catalog cmds
1 parent a8b7134 commit af39c4c

File tree

4 files changed

+9
-231
lines changed

4 files changed

+9
-231
lines changed

‎internal/cmd/catalog/delete.go‎

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,14 @@ import (
2828
)
2929

3030
const (
31-
// deleteItemEndpointTemplate formatting template for item deletion by objectID backend endpoint; specify tenantID, objectID
32-
deleteItemEndpointTemplate = "/api/marketplace/tenants/%s/resources/%s"
3331
// deleteItemByTupleEndpointTemplate formatting template for item deletion by the tuple itemID versionID endpoint; specify companyID, itemID, version
3432
deleteItemByTupleEndpointTemplate = "/api/marketplace/tenants/%s/resources/%s/versions/%s"
3533

3634
cmdDeleteLongDescription = `Delete a single Catalog item
3735
38-
You need to specify either:
39-
- the companyId, itemId and version, via the respective flags (recommended). The company-id flag can be omitted if it is already set in the context.
40-
- the ObjectID of the item with the flag object-id
41-
42-
Passing the ObjectID is expected only when dealing with deprecated Catalog items missing the itemId and/or version fields.
43-
Otherwise, it is preferable to pass the tuple companyId-itemId-version.
36+
You need to specify the companyId, itemId and version, via the respective flags (recommended). The company-id flag can be omitted if it is already set in the context.
4437
`
45-
cmdUse = "delete { --item-id item-id --version version } | --object-id object-id [flags]..."
38+
cmdUse = "delete { --item-id item-id --version version }"
4639
)
4740

4841
var (
@@ -68,12 +61,6 @@ func DeleteCmd(options *clioptions.CLIOptions) *cobra.Command {
6861
return catalog.ErrMissingCompanyID
6962
}
7063

71-
if options.MarketplaceItemObjectID != "" {
72-
err = deleteItemByObjectID(cmd.Context(), client, companyID, options.MarketplaceItemObjectID)
73-
cobra.CheckErr(err)
74-
return nil
75-
}
76-
7764
if options.MarketplaceItemVersion != "" && options.MarketplaceItemID != "" {
7865
err = deleteItemByItemIDAndVersion(
7966
cmd.Context(),
@@ -90,32 +77,14 @@ func DeleteCmd(options *clioptions.CLIOptions) *cobra.Command {
9077
},
9178
}
9279

93-
itemObjectIDFlagName := options.AddMarketplaceItemObjectIDFlag(cmd.Flags())
94-
9580
itemIDFlagName := options.AddMarketplaceItemIDFlag(cmd.Flags())
9681
versionFlagName := options.AddMarketplaceVersionFlag(cmd.Flags())
9782

9883
cmd.MarkFlagsRequiredTogether(itemIDFlagName, versionFlagName)
99-
cmd.MarkFlagsMutuallyExclusive(itemObjectIDFlagName, itemIDFlagName)
100-
cmd.MarkFlagsMutuallyExclusive(itemObjectIDFlagName, versionFlagName)
101-
cmd.MarkFlagsOneRequired(itemObjectIDFlagName, itemIDFlagName, versionFlagName)
10284

10385
return cmd
10486
}
10587

106-
func deleteItemByObjectID(ctx context.Context, client *client.APIClient, companyID, objectID string) error {
107-
resp, err := client.
108-
Delete().
109-
APIPath(fmt.Sprintf(deleteItemEndpointTemplate, companyID, objectID)).
110-
Do(ctx)
111-
112-
if err != nil {
113-
return fmt.Errorf("error executing request: %w", err)
114-
}
115-
116-
return checkDeleteResponseErrors(resp)
117-
}
118-
11988
func deleteItemByItemIDAndVersion(ctx context.Context, client *client.APIClient, companyID, itemID, version string) error {
12089
resp, err := client.
12190
Delete().

‎internal/cmd/catalog/delete_test.go‎

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
)
2929

3030
const (
31-
mockDeleteObjectID = "object-id"
3231
mockDeleteCompanyID = "company-id"
3332
)
3433

@@ -40,25 +39,6 @@ func TestDeleteResourceCmd(t *testing.T) {
4039
})
4140
}
4241

43-
func deleteByIDMockServer(t *testing.T, statusCode int) *httptest.Server {
44-
t.Helper()
45-
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
46-
require.Equal(t,
47-
fmt.Sprintf(deleteItemEndpointTemplate, mockDeleteCompanyID, mockDeleteObjectID),
48-
r.RequestURI,
49-
)
50-
require.Equal(t, http.MethodDelete, r.Method)
51-
w.WriteHeader(statusCode)
52-
if statusCode != http.StatusNoContent {
53-
w.Write([]byte(`
54-
{
55-
"message": "some error message"
56-
}
57-
`))
58-
}
59-
}))
60-
}
61-
6242
func deleteByItemIDAndVersionMockServer(t *testing.T,
6343
statusCode int,
6444
mockItemID, mockVersion string,
@@ -83,61 +63,6 @@ func deleteByItemIDAndVersionMockServer(t *testing.T,
8363
}))
8464
}
8565

86-
func TestDeleteItemByObjectId(t *testing.T) {
87-
mockClientConfig := &client.Config{
88-
Transport: http.DefaultTransport,
89-
}
90-
testCases := map[string]struct {
91-
server *httptest.Server
92-
expectedErr error
93-
}{
94-
"valid delete response": {
95-
server: deleteByIDMockServer(t, http.StatusNoContent),
96-
expectedErr: nil,
97-
},
98-
"resource not found": {
99-
server: deleteByIDMockServer(t, http.StatusNotFound),
100-
expectedErr: catalog.ErrItemNotFound,
101-
},
102-
"internal server error": {
103-
server: deleteByIDMockServer(t, http.StatusInternalServerError),
104-
expectedErr: errServerDeleteItem,
105-
},
106-
"unexpected server response error": {
107-
server: deleteByIDMockServer(t, http.StatusBadGateway),
108-
expectedErr: errServerDeleteItem,
109-
},
110-
"unexpected server response 2xx": {
111-
server: deleteByIDMockServer(t, http.StatusAccepted),
112-
expectedErr: errUnexpectedDeleteItem,
113-
},
114-
"unexpected server response 4xx": {
115-
server: deleteByIDMockServer(t, http.StatusBadRequest),
116-
expectedErr: errUnexpectedDeleteItem,
117-
},
118-
}
119-
120-
for testName, testCase := range testCases {
121-
t.Run(testName, func(t *testing.T) {
122-
defer testCase.server.Close()
123-
mockClientConfig.Host = testCase.server.URL
124-
client, err := client.APIClientForConfig(mockClientConfig)
125-
require.NoError(t, err)
126-
err = deleteItemByObjectID(
127-
t.Context(),
128-
client,
129-
mockDeleteCompanyID,
130-
mockDeleteObjectID,
131-
)
132-
if testCase.expectedErr != nil {
133-
require.ErrorIs(t, err, testCase.expectedErr)
134-
} else {
135-
require.NoError(t, err)
136-
}
137-
})
138-
}
139-
}
140-
14166
func TestDeleteItemByItemIDAndVersion(t *testing.T) {
14267
mockClientConfig := &client.Config{
14368
Transport: http.DefaultTransport,

‎internal/cmd/catalog/get.go‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,13 @@ import (
2727
)
2828

2929
const (
30-
getItemByObjectIDEndpointTemplate = "/api/marketplace/%s"
3130
getItemByItemIDAndVersionEndpointTemplate = "/api/marketplace/tenants/%s/resources/%s/versions/%s"
3231

3332
cmdGetLongDescription = `Get a single Catalog item
3433
35-
You need to specify either:
36-
- the companyId, itemId and version, via the respective flags (recommended). The company-id flag can be omitted if it is already set in the context.
37-
- the ObjectID of the item with the flag object-id
38-
39-
Passing the ObjectID is expected only when dealing with deprecated Catalog items missing the itemId and/or version fields.
40-
Otherwise, it is preferable to pass the tuple companyId-itemId-version.
34+
You need to specify the companyId, itemId and version, via the respective flags. The company-id flag can be omitted if it is already set in the context.
4135
`
42-
cmdGetUse = "get { --item-id item-id --version version } | --object-id object-id [FLAGS]..."
36+
cmdGetUse = "get { --item-id item-id --version version }"
4337
)
4438

4539
// GetCmd return a new cobra command for getting a single catalog resource
@@ -57,7 +51,6 @@ func GetCmd(options *clioptions.CLIOptions) *cobra.Command {
5751
serializedItem, err := getItemEncodedWithFormat(
5852
cmd.Context(),
5953
client,
60-
options.MarketplaceItemObjectID,
6154
restConfig.CompanyID,
6255
options.MarketplaceItemID,
6356
options.MarketplaceItemVersion,
@@ -72,21 +65,13 @@ func GetCmd(options *clioptions.CLIOptions) *cobra.Command {
7265

7366
options.AddOutputFormatFlag(cmd.Flags(), encoding.JSON)
7467

75-
itemObjectIDFlagName := options.AddMarketplaceItemObjectIDFlag(cmd.Flags())
76-
7768
itemIDFlagName := options.AddMarketplaceItemIDFlag(cmd.Flags())
7869
versionFlagName := options.AddMarketplaceVersionFlag(cmd.Flags())
7970

8071
cmd.MarkFlagsRequiredTogether(itemIDFlagName, versionFlagName)
81-
cmd.MarkFlagsMutuallyExclusive(itemObjectIDFlagName, itemIDFlagName)
82-
cmd.MarkFlagsMutuallyExclusive(itemObjectIDFlagName, versionFlagName)
83-
cmd.MarkFlagsOneRequired(itemObjectIDFlagName, itemIDFlagName, versionFlagName)
8472

8573
return cmd
8674
}
87-
func getItemByObjectID(ctx context.Context, client *client.APIClient, objectID string) (*catalog.Item, error) {
88-
return performGetItemRequest(ctx, client, fmt.Sprintf(getItemByObjectIDEndpointTemplate, objectID))
89-
}
9075

9176
func getItemByItemIDAndVersion(ctx context.Context, client *client.APIClient, companyID, itemID, version string) (*catalog.Item, error) {
9277
endpoint := fmt.Sprintf(getItemByItemIDAndVersionEndpointTemplate, companyID, itemID, version)
@@ -117,17 +102,14 @@ func performGetItemRequest(ctx context.Context, client *client.APIClient, endpoi
117102
}
118103

119104
// getItemEncodedWithFormat retrieves the catalog item corresponding to the specified identifier, serialized with the specified outputFormat
120-
func getItemEncodedWithFormat(ctx context.Context, client *client.APIClient, objectID, companyID, itemID, version, outputFormat string) (string, error) {
105+
func getItemEncodedWithFormat(ctx context.Context, client *client.APIClient, companyID, itemID, version, outputFormat string) (string, error) {
121106
var item *catalog.Item
122107
var err error
123-
if objectID != "" {
124-
item, err = getItemByObjectID(ctx, client, objectID)
125-
} else {
126-
if companyID == "" {
127-
return "", catalog.ErrMissingCompanyID
128-
}
129-
item, err = getItemByItemIDAndVersion(ctx, client, companyID, itemID, version)
108+
if companyID == "" {
109+
return "", catalog.ErrMissingCompanyID
130110
}
111+
item, err = getItemByItemIDAndVersion(ctx, client, companyID, itemID, version)
112+
131113
if err != nil {
132114
return "", err
133115
}

‎internal/cmd/catalog/get_test.go‎

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
)
3030

3131
const (
32-
mockObjectID = "object-id"
3332
mockCompanyID = "some-company-id"
3433
mockItemID = "some-item-id"
3534
mockVersion = "1.0.0"
@@ -84,28 +83,6 @@ func TestGetResourceCmd(t *testing.T) {
8483
})
8584
}
8685

87-
func getItemByIDMockServer(t *testing.T, validResponse bool, statusCode int) *httptest.Server {
88-
t.Helper()
89-
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
90-
require.Equal(t,
91-
fmt.Sprintf(getItemByObjectIDEndpointTemplate, mockObjectID),
92-
r.RequestURI,
93-
)
94-
require.Equal(t, http.MethodGet, r.Method)
95-
96-
w.WriteHeader(statusCode)
97-
if statusCode == http.StatusNotFound || statusCode == http.StatusInternalServerError {
98-
w.Write([]byte(`{"message":"some error"}`))
99-
return
100-
}
101-
if validResponse {
102-
w.Write([]byte(validBodyJSONString))
103-
return
104-
}
105-
w.Write([]byte("invalid json"))
106-
}))
107-
}
108-
10986
func getItemByTupleMockServer(
11087
t *testing.T,
11188
validResponse bool,
@@ -134,80 +111,6 @@ func getItemByTupleMockServer(
134111
}))
135112
}
136113

137-
func TestGetItemEncodedByObjectId(t *testing.T) {
138-
clientConfig := &client.Config{
139-
Transport: http.DefaultTransport,
140-
}
141-
142-
testCases := map[string]struct {
143-
server *httptest.Server
144-
outputFormat string
145-
isExpectedErr bool
146-
}{
147-
"valid get response - json": {
148-
server: getItemByIDMockServer(t, true, http.StatusOK),
149-
outputFormat: encoding.JSON,
150-
},
151-
"valid get response - yaml": {
152-
server: getItemByIDMockServer(t, true, http.StatusOK),
153-
outputFormat: encoding.YAML,
154-
},
155-
"invalid body response": {
156-
server: getItemByIDMockServer(t, false, http.StatusOK),
157-
isExpectedErr: true,
158-
outputFormat: encoding.JSON,
159-
},
160-
"resource not found": {
161-
server: getItemByIDMockServer(t, true, http.StatusNotFound),
162-
163-
isExpectedErr: true,
164-
outputFormat: encoding.JSON,
165-
},
166-
"internal server error": {
167-
server: getItemByIDMockServer(t, true, http.StatusInternalServerError),
168-
outputFormat: encoding.JSON,
169-
isExpectedErr: true,
170-
},
171-
}
172-
173-
for testName, testCase := range testCases {
174-
t.Run(testName, func(t *testing.T) {
175-
defer testCase.server.Close()
176-
clientConfig.Host = testCase.server.URL
177-
client, err := client.APIClientForConfig(clientConfig)
178-
require.NoError(t, err)
179-
found, err := getItemEncodedWithFormat(
180-
t.Context(),
181-
client,
182-
mockObjectID,
183-
"",
184-
"",
185-
"",
186-
testCase.outputFormat,
187-
)
188-
if testCase.isExpectedErr {
189-
require.Zero(t, found)
190-
require.Error(t, err)
191-
} else {
192-
if testCase.outputFormat == encoding.JSON {
193-
require.JSONEq(t, validBodyJSONString, found)
194-
} else {
195-
foundMap := map[string]interface{}{}
196-
err := yaml.Unmarshal([]byte(found), &foundMap)
197-
require.NoError(t, err)
198-
199-
expectedMap := map[string]interface{}{}
200-
err = yaml.Unmarshal([]byte(found), &expectedMap)
201-
require.NoError(t, err)
202-
203-
require.Equal(t, expectedMap, foundMap)
204-
}
205-
require.NoError(t, err)
206-
}
207-
})
208-
}
209-
}
210-
211114
func TestGetItemEncodedByTuple(t *testing.T) {
212115
clientConfig := &client.Config{
213116
Transport: http.DefaultTransport,
@@ -304,7 +207,6 @@ func TestGetItemEncodedByTuple(t *testing.T) {
304207
found, err := getItemEncodedWithFormat(
305208
t.Context(),
306209
client,
307-
"",
308210
testCase.companyID,
309211
testCase.itemID,
310212
testCase.version,

0 commit comments

Comments
 (0)