Avoid duplicate release when creating a release with assets (#6493)
When publishing a release, we rely on server-side validation to abort the operation if an existing published release with the same tag name already exists. However, then creating a release with assets, we first create a draft release, upload assets to it, then publish. If there was an existing release with the same tag name, the operation would fail but it would leave behind a temporary draft release with assets. This makes the operation fail earlier, before creating any records.
This commit is contained in:
parent
afef80e988
commit
e8dc825c7c
3 changed files with 65 additions and 2 deletions
|
|
@ -435,8 +435,16 @@ func createRun(opts *CreateOptions) error {
|
|||
|
||||
hasAssets := len(opts.Assets) > 0
|
||||
|
||||
// Avoid publishing the release until all assets have finished uploading
|
||||
if hasAssets {
|
||||
if hasAssets && !opts.Draft {
|
||||
// Check for an existing release
|
||||
if opts.TagName != "" {
|
||||
if ok, err := publishedReleaseExists(httpClient, baseRepo, opts.TagName); err != nil {
|
||||
return fmt.Errorf("error checking for existing release: %w", err)
|
||||
} else if ok {
|
||||
return fmt.Errorf("a release with the same tag name already exists: %s", opts.TagName)
|
||||
}
|
||||
}
|
||||
// Save the release initially as draft and publish it after all assets have finished uploading
|
||||
params["draft"] = true
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -573,6 +573,7 @@ func Test_createRun(t *testing.T) {
|
|||
Concurrency: 1,
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``))
|
||||
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{
|
||||
"url": "https://api.github.com/releases/123",
|
||||
"upload_url": "https://api.github.com/assets/upload",
|
||||
|
|
@ -608,6 +609,33 @@ func Test_createRun(t *testing.T) {
|
|||
wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final\n",
|
||||
wantStderr: ``,
|
||||
},
|
||||
{
|
||||
name: "upload files but release already exists",
|
||||
isTTY: true,
|
||||
opts: CreateOptions{
|
||||
TagName: "v1.2.3",
|
||||
Name: "",
|
||||
Body: "",
|
||||
BodyProvided: true,
|
||||
Draft: false,
|
||||
Target: "",
|
||||
Assets: []*shared.AssetForUpload{
|
||||
{
|
||||
Name: "ball.tgz",
|
||||
Open: func() (io.ReadCloser, error) {
|
||||
return io.NopCloser(bytes.NewBufferString(`TARBALL`)), nil
|
||||
},
|
||||
},
|
||||
},
|
||||
Concurrency: 1,
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(200, ``))
|
||||
},
|
||||
wantStdout: ``,
|
||||
wantStderr: ``,
|
||||
wantErr: `a release with the same tag name already exists: v1.2.3`,
|
||||
},
|
||||
{
|
||||
name: "upload files and create discussion",
|
||||
isTTY: true,
|
||||
|
|
@ -630,6 +658,7 @@ func Test_createRun(t *testing.T) {
|
|||
Concurrency: 1,
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``))
|
||||
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{
|
||||
"url": "https://api.github.com/releases/123",
|
||||
"upload_url": "https://api.github.com/assets/upload",
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
|
|
@ -127,6 +128,31 @@ func generateReleaseNotes(httpClient *http.Client, repo ghrepo.Interface, tagNam
|
|||
return &rn, err
|
||||
}
|
||||
|
||||
func publishedReleaseExists(httpClient *http.Client, repo ghrepo.Interface, tagName string) (bool, error) {
|
||||
path := fmt.Sprintf("repos/%s/%s/releases/tags/%s", repo.RepoOwner(), repo.RepoName(), url.PathEscape(tagName))
|
||||
url := ghinstance.RESTPrefix(repo.RepoHost()) + path
|
||||
req, err := http.NewRequest("HEAD", url, nil)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
resp, err := httpClient.Do(req)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if resp.Body != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
|
||||
if resp.StatusCode == 200 {
|
||||
return true, nil
|
||||
} else if resp.StatusCode == 404 {
|
||||
return false, nil
|
||||
} else {
|
||||
return false, api.HandleHTTPError(resp)
|
||||
}
|
||||
}
|
||||
|
||||
func createRelease(httpClient *http.Client, repo ghrepo.Interface, params map[string]interface{}) (*shared.Release, error) {
|
||||
bodyBytes, err := json.Marshal(params)
|
||||
if err != nil {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue