Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions pkg/cmd/cache/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,33 +164,18 @@ func deleteCaches(opts *DeleteOptions, client *api.Client, repo ghrepo.Interface
cs := opts.IO.ColorScheme()
repoName := ghrepo.FullName(repo)
opts.IO.StartProgressIndicator()
base := fmt.Sprintf("repos/%s/actions/caches", repoName)

totalDeleted := 0
for _, cache := range toDelete {
// TODO(babakks): We use two different endpoints here which have different
// response schemas:
//
// 1. /repos/OWNER/REPO/actions/caches/ID (for deleting by cache ID)
// - returns HTTP 204 (NO CONTENT) on success
// 2. /repos/OWNER/REPO/actions/caches?key=KEY[&ref=REF] (for deleting by cache key, and optionally a ref)
// - returns HTTP 200 on success including information about the deleted caches
//
// So, if/when we decided to use the data in the response body we need
// to be careful with parsing. Probably want to split these API calls
// into separate functions.

path := ""
var count int
var err error
if id, ok := parseCacheID(cache); ok {
path = fmt.Sprintf("%s/%d", base, id)
err = deleteCacheByID(client, repo, id)
count = 1
} else {
path = fmt.Sprintf("%s?key=%s", base, url.QueryEscape(cache))

if opts.Ref != "" {
path += fmt.Sprintf("&ref=%s", url.QueryEscape(opts.Ref))
}
count, err = deleteCacheByKey(client, repo, cache, opts.Ref)
}

err := client.REST(repo.RepoHost(), "DELETE", path, nil, nil)
if err != nil {
var httpErr api.HTTPError
if errors.As(err, &httpErr) {
Expand All @@ -207,17 +192,45 @@ func deleteCaches(opts *DeleteOptions, client *api.Client, repo ghrepo.Interface
opts.IO.StopProgressIndicator()
return err
}

totalDeleted += count
}

opts.IO.StopProgressIndicator()

if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.Out, "%s Deleted %s from %s\n", cs.SuccessIcon(), text.Pluralize(len(toDelete), "cache"), repoName)
fmt.Fprintf(opts.IO.Out, "%s Deleted %s from %s\n", cs.SuccessIcon(), text.Pluralize(totalDeleted, "cache"), repoName)
}

return nil
}

func deleteCacheByID(client *api.Client, repo ghrepo.Interface, id int) error {
// returns HTTP 204 (NO CONTENT) on success
path := fmt.Sprintf("repos/%s/actions/caches/%d", ghrepo.FullName(repo), id)
return client.REST(repo.RepoHost(), "DELETE", path, nil, nil)
}

// deleteCacheByKey deletes cache entries by given key (and optional ref) and
// returns the number of deleted entries.
//
// Note that a key/ref combination does not necessarily map to a single cache
// entry. There may be more than one entries with the same key/ref combination,
// but those entries will have different IDs.
func deleteCacheByKey(client *api.Client, repo ghrepo.Interface, key, ref string) (int, error) {
Comment thread
babakks marked this conversation as resolved.
path := fmt.Sprintf("repos/%s/actions/caches?key=%s", ghrepo.FullName(repo), url.QueryEscape(key))
if ref != "" {
path += fmt.Sprintf("&ref=%s", url.QueryEscape(ref))
}
var payload shared.CachePayload
err := client.REST(repo.RepoHost(), "DELETE", path, nil, &payload)
if err != nil {
return 0, err
}

return payload.TotalCount, nil
}

func parseCacheID(arg string) (int, bool) {
id, err := strconv.Atoi(arg)
return id, err == nil
Expand Down
48 changes: 42 additions & 6 deletions pkg/cmd/cache/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,30 @@ func TestDeleteRun(t *testing.T) {
httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
"key": []string{"a weird_cache+key"},
}),
// The response is a JSON object but we don't need it here.
httpmock.StatusStringResponse(200, "{}"),
httpmock.JSONResponse(shared.CachePayload{
TotalCount: 1,
}),
)
},
tty: true,
wantStdout: "✓ Deleted 1 cache from OWNER/REPO\n",
},
{
name: "deletes multiple caches by key",
opts: DeleteOptions{Identifier: "shared-cache-key"},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
"key": []string{"shared-cache-key"},
}),
httpmock.JSONResponse(shared.CachePayload{
TotalCount: 5,
}),
)
},
tty: true,
wantStdout: "✓ Deleted 5 caches from OWNER/REPO\n",
},
{
name: "no caches to delete when deleting all",
opts: DeleteOptions{DeleteAll: true},
Expand Down Expand Up @@ -299,8 +316,9 @@ func TestDeleteRun(t *testing.T) {
"key": []string{"cache-key"},
"ref": []string{"refs/heads/main"},
}),
// The response is a JSON object but we don't need it here.
httpmock.StatusStringResponse(200, "{}"),
httpmock.JSONResponse(shared.CachePayload{
TotalCount: 1,
}),
)
},
tty: true,
Expand All @@ -315,13 +333,31 @@ func TestDeleteRun(t *testing.T) {
"key": []string{"cache-key"},
"ref": []string{"refs/heads/main"},
}),
// The response is a JSON object but we don't need it here.
httpmock.StatusStringResponse(200, "{}"),
httpmock.JSONResponse(shared.CachePayload{
TotalCount: 1,
}),
)
},
tty: false,
wantStdout: "",
},
{
name: "deletes multiple caches by key and ref",
opts: DeleteOptions{Identifier: "cache-key", Ref: "refs/heads/feature"},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.QueryMatcher("DELETE", "repos/OWNER/REPO/actions/caches", url.Values{
"key": []string{"cache-key"},
"ref": []string{"refs/heads/feature"},
}),
httpmock.JSONResponse(shared.CachePayload{
TotalCount: 3,
}),
)
},
tty: true,
wantStdout: "✓ Deleted 3 caches from OWNER/REPO\n",
},
{
// As of now, the API returns HTTP 404 for invalid or non-existent refs.
name: "cache key exists but ref is invalid/not-found",
Expand Down