STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #6667 from cli/release-galore

Browse source 
More resilient release commands
This commit is contained in:
Mislav Marohnić 2022-12-19 15:33:15 +01:00 • committed by GitHub
commit 7caf7da2da
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 408 additions and 181 deletions
Download patch file Download diff file Expand all files Collapse all files

25
api/client.go
View file

@ -1,6 +1,7 @@
package api
import (
"context"
"encoding/json"
"errors"
"fmt"
@ -51,8 +52,8 @@ func (err HTTPError) ScopesSuggestion() string {
return err.scopesSuggestion
}
// GraphQL performs a GraphQL request and parses the response. If there are errors in the response,
// GraphQLError will be returned, but the data will also be parsed into the receiver.
// GraphQL performs a GraphQL request using the query string and parses the response into data receiver. If there are errors in the response,
// GraphQLError will be returned, but the receiver will also be partially populated.
func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error {
opts := clientOptions(hostname, c.http.Transport)
opts.Headers[graphqlFeatures] = features
@ -63,8 +64,8 @@ func (c Client) GraphQL(hostname string, query string, variables map[string]inte
return handleResponse(gqlClient.Do(query, variables, data))
}
// GraphQL performs a GraphQL mutation and parses the response. If there are errors in the response,
// GraphQLError will be returned, but the data will also be parsed into the receiver.
// Mutate performs a GraphQL mutation based on a struct and parses the response with the same struct as the receiver. If there are errors in the response,
// GraphQLError will be returned, but the receiver will also be partially populated.
func (c Client) Mutate(hostname, name string, mutation interface{}, variables map[string]interface{}) error {
opts := clientOptions(hostname, c.http.Transport)
opts.Headers[graphqlFeatures] = features
@ -75,8 +76,8 @@ func (c Client) Mutate(hostname, name string, mutation interface{}, variables ma
return handleResponse(gqlClient.Mutate(name, mutation, variables))
}
// GraphQL performs a GraphQL query and parses the response. If there are errors in the response,
// GraphQLError will be returned, but the data will also be parsed into the receiver.
// Query performs a GraphQL query based on a struct and parses the response with the same struct as the receiver. If there are errors in the response,
// GraphQLError will be returned, but the receiver will also be partially populated.
func (c Client) Query(hostname, name string, query interface{}, variables map[string]interface{}) error {
opts := clientOptions(hostname, c.http.Transport)
opts.Headers[graphqlFeatures] = features
@ -87,6 +88,18 @@ func (c Client) Query(hostname, name string, query interface{}, variables map[st
return handleResponse(gqlClient.Query(name, query, variables))
}
// QueryWithContext performs a GraphQL query based on a struct and parses the response with the same struct as the receiver. If there are errors in the response,
// GraphQLError will be returned, but the receiver will also be partially populated.
func (c Client) QueryWithContext(ctx context.Context, hostname, name string, query interface{}, variables map[string]interface{}) error {
opts := clientOptions(hostname, c.http.Transport)
opts.Headers[graphqlFeatures] = features
gqlClient, err := gh.GQLClient(&opts)
if err != nil {
return err
}
return handleResponse(gqlClient.QueryWithContext(ctx, name, query, variables))
}
// REST performs a REST request and parses the response.
func (c Client) REST(hostname string, method string, p string, body io.Reader, data interface{}) error {
opts := clientOptions(hostname, c.http.Transport)

2
go.mod
View file

@ -36,7 +36,7 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.5
golang.org/x/crypto v0.0.0-20210817164053-32db794688a5
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sync v0.1.0
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
golang.org/x/text v0.3.8
google.golang.org/grpc v1.49.0

4
go.sum
View file

@ -342,8 +342,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

18
pkg/cmd/release/create/create.go
View file

@ -426,6 +426,7 @@ func createRun(opts *CreateOptions) error {
}
hasAssets := len(opts.Assets) > 0
draftWhileUploading := false
if hasAssets && !opts.Draft {
// Check for an existing release
@ -437,6 +438,7 @@ func createRun(opts *CreateOptions) error {
}
}
// Save the release initially as draft and publish it after all assets have finished uploading
draftWhileUploading = true
params["draft"] = true
}
@ -445,6 +447,16 @@ func createRun(opts *CreateOptions) error {
return err
}
cleanupDraftRelease := func(err error) error {
if !draftWhileUploading {
return err
}
if cleanupErr := deleteRelease(httpClient, newRelease); cleanupErr != nil {
return fmt.Errorf("%w\ncleaning up draft failed: %v", err, cleanupErr)
}
return err
}
if hasAssets {
uploadURL := newRelease.UploadURL
if idx := strings.IndexRune(uploadURL, '{'); idx > 0 {
@ -455,13 +467,13 @@ func createRun(opts *CreateOptions) error {
err = shared.ConcurrentUpload(httpClient, uploadURL, opts.Concurrency, opts.Assets)
opts.IO.StopProgressIndicator()
if err != nil {
return err
return cleanupDraftRelease(err)
}
if !opts.Draft {
if draftWhileUploading {
rel, err := publishRelease(httpClient, newRelease.APIURL, opts.DiscussionCategory)
if err != nil {
return err
return cleanupDraftRelease(err)
}
newRelease = rel
}

96
pkg/cmd/release/create/create_test.go
View file

@ -728,6 +728,102 @@ func Test_createRun(t *testing.T) {
wantStderr: ``,
wantErr: `a release with the same tag name already exists: v1.2.3`,
},
{
name: "clean up draft after uploading files fails",
isTTY: false,
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(404, ``))
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{
"url": "https://api.github.com/releases/123",
"upload_url": "https://api.github.com/assets/upload",
"html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3"
}`))
reg.Register(httpmock.REST("POST", "assets/upload"), httpmock.StatusStringResponse(422, `{}`))
reg.Register(httpmock.REST("DELETE", "releases/123"), httpmock.StatusStringResponse(204, ``))
},
wantStdout: ``,
wantStderr: ``,
wantErr: `HTTP 422 (https://api.github.com/assets/upload?label=&name=ball.tgz)`,
},
{
name: "clean up draft after publishing fails",
isTTY: false,
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(404, ``))
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{
"url": "https://api.github.com/releases/123",
"upload_url": "https://api.github.com/assets/upload",
"html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3"
}`))
reg.Register(httpmock.REST("POST", "assets/upload"), httpmock.StatusStringResponse(201, `{}`))
reg.Register(httpmock.REST("PATCH", "releases/123"), httpmock.StatusStringResponse(500, `{}`))
reg.Register(httpmock.REST("DELETE", "releases/123"), httpmock.StatusStringResponse(204, ``))
},
wantStdout: ``,
wantStderr: ``,
wantErr: `HTTP 500 (https://api.github.com/releases/123)`,
},
{
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,

25
pkg/cmd/release/create/http.go
View file

@ -225,3 +225,28 @@ func publishRelease(httpClient *http.Client, releaseURL string, discussionCatego
err = json.Unmarshal(b, &release)
return &release, err
}
func deleteRelease(httpClient *http.Client, release *shared.Release) error {
req, err := http.NewRequest("DELETE", release.APIURL, nil)
if err != nil {
return err
}
resp, err := httpClient.Do(req)
if err != nil {
return err
}
if resp.Body != nil {
defer resp.Body.Close()
}
success := resp.StatusCode >= 200 && resp.StatusCode < 300
if !success {
return api.HandleHTTPError(resp)
}
if resp.StatusCode != 204 {
_, _ = io.Copy(io.Discard, resp.Body)
}
return nil
}

3
pkg/cmd/release/delete-asset/delete_asset.go
View file

@ -1,6 +1,7 @@
package deleteasset
import (
"context"
"fmt"
"net/http"
@ -66,7 +67,7 @@ func deleteAssetRun(opts *DeleteAssetOptions) error {
return err
}
release, err := shared.FetchRelease(httpClient, baseRepo, opts.TagName)
release, err := shared.FetchRelease(context.Background(), httpClient, baseRepo, opts.TagName)
if err != nil {
return err
}

15
pkg/cmd/release/delete-asset/delete_asset_test.go
View file

@ -8,6 +8,7 @@ import (
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -156,18 +157,18 @@ func Test_deleteAssetRun(t *testing.T) {
ios.SetStderrTTY(tt.isTTY)
fakeHTTP := &httpmock.Registry{}
fakeHTTP.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StringResponse(`{
shared.StubFetchRelease(t, fakeHTTP, "OWNER", "REPO", tt.opts.TagName, `{
"tag_name": "v1.2.3",
"draft": false,
"url": "https://api.github.com/repos/OWNER/REPO/releases/23456",
"assets": [
{
"url": "https://api.github.com/repos/OWNER/REPO/releases/assets/1",
"id": 1,
"name": "test-asset"
}
{
"url": "https://api.github.com/repos/OWNER/REPO/releases/assets/1",
"id": 1,
"name": "test-asset"
}
]
}`))
}`)
fakeHTTP.Register(httpmock.REST("DELETE", "repos/OWNER/REPO/releases/assets/1"), httpmock.StatusStringResponse(204, ""))
pm := &prompter.PrompterMock{}

3
pkg/cmd/release/delete/delete.go
View file

@ -1,6 +1,7 @@
package delete
import (
"context"
"fmt"
"net/http"
@ -69,7 +70,7 @@ func deleteRun(opts *DeleteOptions) error {
return err
}
release, err := shared.FetchRelease(httpClient, baseRepo, opts.TagName)
release, err := shared.FetchRelease(context.Background(), httpClient, baseRepo, opts.TagName)
if err != nil {
return err
}

5
pkg/cmd/release/delete/delete_test.go
View file

@ -9,6 +9,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -193,11 +194,11 @@ func Test_deleteRun(t *testing.T) {
}
fakeHTTP := &httpmock.Registry{}
fakeHTTP.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StringResponse(`{
shared.StubFetchRelease(t, fakeHTTP, "OWNER", "REPO", tt.opts.TagName, `{
"tag_name": "v1.2.3",
"draft": false,
"url": "https://api.github.com/repos/OWNER/REPO/releases/23456"
}`))
}`)
fakeHTTP.Register(httpmock.REST("DELETE", "repos/OWNER/REPO/releases/23456"), httpmock.StatusStringResponse(204, ""))
fakeHTTP.Register(httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/tags/v1.2.3"), httpmock.StatusStringResponse(204, ""))

7
pkg/cmd/release/download/download.go
View file

@ -1,6 +1,7 @@
package download
import (
"context"
"errors"
"fmt"
"io"
@ -142,14 +143,16 @@ func downloadRun(opts *DownloadOptions) error {
opts.IO.StartProgressIndicator()
defer opts.IO.StopProgressIndicator()
ctx := context.Background()
var release *shared.Release
if opts.TagName == "" {
release, err = shared.FetchLatestRelease(httpClient, baseRepo)
release, err = shared.FetchLatestRelease(ctx, httpClient, baseRepo)
if err != nil {
return err
}
} else {
release, err = shared.FetchRelease(httpClient, baseRepo, opts.TagName)
release, err = shared.FetchRelease(ctx, httpClient, baseRepo, opts.TagName)
if err != nil {
return err
}

19
pkg/cmd/release/download/download_test.go
View file

@ -9,6 +9,7 @@ import (
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -322,7 +323,7 @@ func Test_downloadRun(t *testing.T) {
ios.SetStderrTTY(tt.isTTY)
fakeHTTP := &httpmock.Registry{}
fakeHTTP.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StringResponse(`{
shared.StubFetchRelease(t, fakeHTTP, "OWNER", "REPO", tt.opts.TagName, `{
"assets": [
{ "name": "windows-32bit.zip", "size": 12,
"url": "https://api.github.com/assets/1234" },
@ -333,7 +334,7 @@ func Test_downloadRun(t *testing.T) {
],
"tarball_url": "https://api.github.com/repos/OWNER/REPO/tarball/v1.2.3",
"zipball_url": "https://api.github.com/repos/OWNER/REPO/zipball/v1.2.3"
}`))
}`)
fakeHTTP.Register(httpmock.REST("GET", "assets/1234"), httpmock.StringResponse(`1234`))
fakeHTTP.Register(httpmock.REST("GET", "assets/3456"), httpmock.StringResponse(`3456`))
fakeHTTP.Register(httpmock.REST("GET", "assets/5678"), httpmock.StringResponse(`5678`))
@ -378,7 +379,13 @@ func Test_downloadRun(t *testing.T) {
expectedAcceptHeader = "application/octet-stream, application/json"
}
assert.Equal(t, expectedAcceptHeader, fakeHTTP.Requests[1].Header.Get("Accept"))
for _, req := range fakeHTTP.Requests {
if req.Method != "GET" || req.URL.Path != "repos/OWNER/REPO/releases/tags/v1.2.3" {
// skip non-asset download requests
continue
}
assert.Equal(t, expectedAcceptHeader, req.Header.Get("Accept"), "for request %s", req.URL)
}
assert.Equal(t, tt.wantStdout, stdout.String())
assert.Equal(t, tt.wantStderr, stderr.String())
@ -509,15 +516,15 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
tt.opts.IO = ios
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StringResponse(`{
// defer reg.Verify(t) // FIXME: intermittetly fails due to StubFetchRelease internals
shared.StubFetchRelease(t, reg, "OWNER", "REPO", "v1.2.3", `{
"assets": [
{ "name": "windows-64bit.zip", "size": 34,
"url": "https://api.github.com/assets/3456" }
],
"tarball_url": "https://api.github.com/repos/OWNER/REPO/tarball/v1.2.3",
"zipball_url": "https://api.github.com/repos/OWNER/REPO/zipball/v1.2.3"
}`))
}`)
if tt.httpStubs != nil {
tt.httpStubs(reg)
}

3
pkg/cmd/release/edit/edit.go
View file

@ -1,6 +1,7 @@
package edit
import (
"context"
"fmt"
"net/http"
@ -95,7 +96,7 @@ func editRun(tag string, opts *EditOptions) error {
return err
}
release, err := shared.FetchRelease(httpClient, baseRepo, tag)
release, err := shared.FetchRelease(context.Background(), httpClient, baseRepo, tag)
if err != nil {
return err
}

11
pkg/cmd/release/edit/edit_test.go
View file

@ -9,6 +9,7 @@ import (
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -415,14 +416,14 @@ func Test_editRun(t *testing.T) {
ios.SetStderrTTY(tt.isTTY)
fakeHTTP := &httpmock.Registry{}
fakeHTTP.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.JSONResponse(map[string]interface{}{
"id": 12345,
"tag_name": "v1.2.3",
}))
// defer reg.Verify(t) // FIXME: intermittetly fails due to StubFetchRelease internals
shared.StubFetchRelease(t, fakeHTTP, "OWNER", "REPO", "v1.2.3", `{
"id": 12345,
"tag_name": "v1.2.3"
}`)
if tt.httpStubs != nil {
tt.httpStubs(t, fakeHTTP)
}
defer fakeHTTP.Verify(t)
tt.opts.IO = ios
tt.opts.HttpClient = func() (*http.Client, error) {

191
pkg/cmd/release/shared/fetch.go
View file

@ -1,6 +1,7 @@
package shared
import (
"context"
"encoding/json"
"errors"
"fmt"
@ -13,6 +14,9 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/shurcooL/githubv4"
"github.com/stretchr/testify/assert"
)
var ReleaseFields = []string{
@ -118,11 +122,83 @@ func (rel *Release) ExportData(fields []string) map[string]interface{} {
return data
}
// FetchRelease finds a repository release by its tagName.
func FetchRelease(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*Release, error) {
path := fmt.Sprintf("repos/%s/%s/releases/tags/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), tagName)
url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path
req, err := http.NewRequest("GET", url, nil)
var errNotFound = errors.New("release not found")
type fetchResult struct {
release *Release
error error
}
// FetchRelease finds a published repository release by its tagName, or a draft release by its pending tag name.
func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface, tagName string) (*Release, error) {
cc, cancel := context.WithCancel(ctx)
results := make(chan fetchResult, 2)
// published release lookup
go func() {
path := fmt.Sprintf("repos/%s/%s/releases/tags/%s", repo.RepoOwner(), repo.RepoName(), tagName)
release, err := fetchReleasePath(cc, httpClient, repo.RepoHost(), path)
results <- fetchResult{release: release, error: err}
}()
// draft release lookup
go func() {
release, err := fetchDraftRelease(cc, httpClient, repo, tagName)
results <- fetchResult{release: release, error: err}
}()
res := <-results
if errors.Is(res.error, errNotFound) {
res = <-results
cancel() // satisfy the linter even though no goroutines are running anymore
} else {
cancel()
<-results // drain the channel
}
return res.release, res.error
}
// FetchLatestRelease finds the latest published release for a repository.
func FetchLatestRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface) (*Release, error) {
path := fmt.Sprintf("repos/%s/%s/releases/latest", repo.RepoOwner(), repo.RepoName())
return fetchReleasePath(ctx, httpClient, repo.RepoHost(), path)
}
// fetchDraftRelease returns the first draft release that has tagName as its pending tag.
func fetchDraftRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Interface, tagName string) (*Release, error) {
// First use GraphQL to find a draft release by pending tag name, since REST doesn't have this ability.
var query struct {
Repository struct {
Release *struct {
DatabaseID int64
IsDraft bool
} `graphql:"release(tagName: $tagName)"`
} `graphql:"repository(owner: $owner, name: $name)"`
}
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"tagName": githubv4.String(tagName),
}
gql := api.NewClientFromHTTP(httpClient)
if err := gql.QueryWithContext(ctx, repo.RepoHost(), "RepositoryReleaseByTag", &query, variables); err != nil {
return nil, err
}
if query.Repository.Release == nil || !query.Repository.Release.IsDraft {
return nil, errNotFound
}
// Then, use REST to get information about the draft release. In theory, we could have fetched
// all the necessary information via GraphQL, but REST is safer for backwards compatibility.
path := fmt.Sprintf("repos/%s/%s/releases/%d", repo.RepoOwner(), repo.RepoName(), query.Repository.Release.DatabaseID)
return fetchReleasePath(ctx, httpClient, repo.RepoHost(), path)
}
func fetchReleasePath(ctx context.Context, httpClient *http.Client, host string, p string) (*Release, error) {
req, err := http.NewRequestWithContext(ctx, "GET", ghinstance.RESTPrefix(host)+p, nil)
if err != nil {
return nil, err
}
@ -134,102 +210,39 @@ func FetchRelease(httpClient *http.Client, baseRepo ghrepo.Interface, tagName st
defer resp.Body.Close()
if resp.StatusCode == 404 {
return FindDraftRelease(httpClient, baseRepo, tagName)
}
if resp.StatusCode > 299 {
_, _ = io.Copy(io.Discard, resp.Body)
return nil, errNotFound
} else if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}
b, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
var release Release
err = json.Unmarshal(b, &release)
if err != nil {
if err := json.NewDecoder(resp.Body).Decode(&release); err != nil {
return nil, err
}
return &release, nil
}
// FetchLatestRelease finds the latest published release for a repository.
func FetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*Release, error) {
path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName())
url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return nil, err
}
resp, err := httpClient.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()
if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}
b, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
var release Release
err = json.Unmarshal(b, &release)
if err != nil {
return nil, err
}
return &release, nil
type testingT interface {
Errorf(format string, args ...interface{})
}
// FindDraftRelease returns the latest draft release that matches tagName.
func FindDraftRelease(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*Release, error) {
path := fmt.Sprintf("repos/%s/%s/releases", baseRepo.RepoOwner(), baseRepo.RepoName())
url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path
perPage := 100
page := 1
for {
req, err := http.NewRequest("GET", fmt.Sprintf("%s?per_page=%d&page=%d", url, perPage, page), nil)
if err != nil {
return nil, err
}
resp, err := httpClient.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()
if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}
b, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
var releases []Release
err = json.Unmarshal(b, &releases)
if err != nil {
return nil, err
}
for _, r := range releases {
if r.IsDraft && r.TagName == tagName {
return &r, nil
}
}
//nolint:staticcheck
break
func StubFetchRelease(t testingT, reg *httpmock.Registry, owner, repoName, tagName, responseBody string) {
path := "repos/OWNER/REPO/releases/tags/v1.2.3"
if tagName == "" {
path = "repos/OWNER/REPO/releases/latest"
}
return nil, errors.New("release not found")
reg.Register(httpmock.REST("GET", path), httpmock.StringResponse(responseBody))
if tagName != "" {
reg.Register(
httpmock.GraphQL(`query RepositoryReleaseByTag\b`),
httpmock.GraphQLQuery(`{ "data": { "repository": { "release": null }}}`, func(q string, vars map[string]interface{}) {
assert.Equal(t, owner, vars["owner"])
assert.Equal(t, repoName, vars["name"])
assert.Equal(t, tagName, vars["tagName"])
}))
}
}

88
pkg/cmd/release/shared/upload.go
View file

@ -1,6 +1,7 @@
package shared
import (
"context"
"encoding/json"
"errors"
"io"
@ -13,8 +14,15 @@ import (
"time"
"github.com/cli/cli/v2/api"
"golang.org/x/sync/errgroup"
)
type httpDoer interface {
Do(*http.Request) (*http.Response, error)
}
type errNetwork struct{ error }
type AssetForUpload struct {
Name string
Label string
@ -90,64 +98,61 @@ func fileExt(fn string) string {
return path.Ext(fn)
}
func ConcurrentUpload(httpClient *http.Client, uploadURL string, numWorkers int, assets []*AssetForUpload) error {
func ConcurrentUpload(httpClient httpDoer, uploadURL string, numWorkers int, assets []*AssetForUpload) error {
if numWorkers == 0 {
return errors.New("the number of concurrent workers needs to be greater than 0")
}
jobs := make(chan AssetForUpload, len(assets))
results := make(chan error, len(assets))
if len(assets) < numWorkers {
numWorkers = len(assets)
}
for w := 1; w <= numWorkers; w++ {
go func() {
for a := range jobs {
results <- uploadWithDelete(httpClient, uploadURL, a)
}
}()
}
ctx := context.Background()
g, gctx := errgroup.WithContext(ctx)
g.SetLimit(numWorkers)
for _, a := range assets {
jobs <- *a
asset := *a
g.Go(func() error {
return uploadWithDelete(gctx, httpClient, uploadURL, asset)
})
}
close(jobs)
var uploadError error
for i := 0; i < len(assets); i++ {
if err := <-results; err != nil {
uploadError = err
}
}
return uploadError
return g.Wait()
}
var retryInterval = time.Millisecond * 200
const maxRetries = 3
func uploadWithDelete(httpClient *http.Client, uploadURL string, a AssetForUpload) error {
func shouldRetry(err error) bool {
var networkError errNetwork
if errors.As(err, &networkError) {
return true
}
var httpError api.HTTPError
return errors.As(err, &httpError) && httpError.StatusCode >= 500
}
func uploadWithDelete(ctx context.Context, httpClient httpDoer, uploadURL string, a AssetForUpload) error {
if a.ExistingURL != "" {
err := deleteAsset(httpClient, a.ExistingURL)
if err != nil {
if err := deleteAsset(ctx, httpClient, a.ExistingURL); err != nil {
return err
}
}
retries := 0
for {
var httpError api.HTTPError
_, err := uploadAsset(httpClient, uploadURL, a)
// retry upload several times upon receiving HTTP 5xx
if err == nil || !errors.As(err, &httpError) || httpError.StatusCode < 500 || retries < maxRetries {
_, err := uploadAsset(ctx, httpClient, uploadURL, a)
if err == nil || retries == maxRetries || !shouldRetry(err) {
return err
}
retries++
time.Sleep(time.Duration(retries) * time.Second)
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(time.Duration(retries) * retryInterval):
}
}
}
func uploadAsset(httpClient *http.Client, uploadURL string, asset AssetForUpload) (*ReleaseAsset, error) {
func uploadAsset(ctx context.Context, httpClient httpDoer, uploadURL string, asset AssetForUpload) (*ReleaseAsset, error) {
u, err := url.Parse(uploadURL)
if err != nil {
return nil, err
@ -163,7 +168,7 @@ func uploadAsset(httpClient *http.Client, uploadURL string, asset AssetForUpload
}
defer f.Close()
req, err := http.NewRequest("POST", u.String(), f)
req, err := http.NewRequestWithContext(ctx, "POST", u.String(), f)
if err != nil {
return nil, err
}
@ -173,7 +178,7 @@ func uploadAsset(httpClient *http.Client, uploadURL string, asset AssetForUpload
resp, err := httpClient.Do(req)
if err != nil {
return nil, err
return nil, errNetwork{err}
}
defer resp.Body.Close()
@ -182,22 +187,17 @@ func uploadAsset(httpClient *http.Client, uploadURL string, asset AssetForUpload
return nil, api.HandleHTTPError(resp)
}
b, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
var newAsset ReleaseAsset
err = json.Unmarshal(b, &newAsset)
if err != nil {
dec := json.NewDecoder(resp.Body)
if err := dec.Decode(&newAsset); err != nil {
return nil, err
}
return &newAsset, nil
}
func deleteAsset(httpClient *http.Client, assetURL string) error {
req, err := http.NewRequest("DELETE", assetURL, nil)
func deleteAsset(ctx context.Context, httpClient httpDoer, assetURL string) error {
req, err := http.NewRequestWithContext(ctx, "DELETE", assetURL, nil)
if err != nil {
return err
}

55
pkg/cmd/release/shared/upload_test.go
View file

@ -1,6 +1,14 @@
package shared
import "testing"
import (
"bytes"
"context"
"errors"
"io"
"net/http"
"testing"
"time"
)
func Test_typeForFilename(t *testing.T) {
tests := []struct {
@ -67,3 +75,48 @@ func Test_typeForFilename(t *testing.T) {
})
}
}
func Test_uploadWithDelete_retry(t *testing.T) {
retryInterval = time.Millisecond
ctx := context.Background()
tries := 0
client := funcClient(func(req *http.Request) (*http.Response, error) {
tries++
if tries == 1 {
return nil, errors.New("made up exception")
} else if tries == 2 {
return &http.Response{
Request: req,
StatusCode: 500,
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
}, nil
}
return &http.Response{
Request: req,
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
}, nil
})
err := uploadWithDelete(ctx, client, "http://example.com/upload", AssetForUpload{
Name: "asset",
Label: "",
Size: 8,
Open: func() (io.ReadCloser, error) {
return io.NopCloser(bytes.NewBufferString(`somebody`)), nil
},
MIMEType: "application/octet-stream",
})
if err != nil {
t.Errorf("uploadWithDelete() error: %v", err)
}
if tries != 3 {
t.Errorf("tries = %d, expected %d", tries, 3)
}
}
type funcClient func(*http.Request) (*http.Response, error)
func (f funcClient) Do(req *http.Request) (*http.Response, error) {
return f(req)
}

3
pkg/cmd/release/upload/upload.go
View file

@ -1,6 +1,7 @@
package upload
import (
"context"
"fmt"
"net/http"
"strings"
@ -80,7 +81,7 @@ func uploadRun(opts *UploadOptions) error {
return err
}
release, err := shared.FetchRelease(httpClient, baseRepo, opts.TagName)
release, err := shared.FetchRelease(context.Background(), httpClient, baseRepo, opts.TagName)
if err != nil {
return err
}

6
pkg/cmd/release/view/view.go
View file

@ -1,6 +1,7 @@
package view
import (
"context"
"fmt"
"io"
"net/http"
@ -82,15 +83,16 @@ func viewRun(opts *ViewOptions) error {
return err
}
ctx := context.Background()
var release *shared.Release
if opts.TagName == "" {
release, err = shared.FetchLatestRelease(httpClient, baseRepo)
release, err = shared.FetchLatestRelease(ctx, httpClient, baseRepo)
if err != nil {
return err
}
} else {
release, err = shared.FetchRelease(httpClient, baseRepo, opts.TagName)
release, err = shared.FetchRelease(ctx, httpClient, baseRepo, opts.TagName)
if err != nil {
return err
}

10
pkg/cmd/release/view/view_test.go
View file

@ -10,6 +10,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -213,13 +214,8 @@ func Test_viewRun(t *testing.T) {
ios.SetStdinTTY(tt.isTTY)
ios.SetStderrTTY(tt.isTTY)
path := "repos/OWNER/REPO/releases/tags/v1.2.3"
if tt.opts.TagName == "" {
path = "repos/OWNER/REPO/releases/latest"
}
fakeHTTP := &httpmock.Registry{}
fakeHTTP.Register(httpmock.REST("GET", path), httpmock.StringResponse(fmt.Sprintf(`{
shared.StubFetchRelease(t, fakeHTTP, "OWNER", "REPO", tt.opts.TagName, fmt.Sprintf(`{
"tag_name": "v1.2.3",
"draft": false,
"author": { "login": "MonaLisa" },
@ -231,7 +227,7 @@ func Test_viewRun(t *testing.T) {
{ "name": "windows.zip", "size": 12 },
{ "name": "linux.tgz", "size": 34 }
]
}`, tt.releasedAt.Format(time.RFC3339), tt.releaseBody)))
}`, tt.releasedAt.Format(time.RFC3339), tt.releaseBody))
tt.opts.IO = ios
tt.opts.HttpClient = func() (*http.Client, error) {