Merge branch 'jmc-1154' into 'main'

fix(release): omit link_type when not specified uploading file

Closes #1154

See merge request https://gitlab.com/gitlab-org/cli/-/merge_requests/1149

Merged-by: Tomas Vik <tvik@gitlab.com>
Approved-by: Ahmed Hemdan <ahemdan@gitlab.com>
Approved-by: Tomas Vik <tvik@gitlab.com>
Reviewed-by: Ahmed Hemdan <ahemdan@gitlab.com>
Reviewed-by: Jay McCure <jmccure@gitlab.com>
Co-authored-by: Jay McCure <jmccure@gitlab.com>
This commit is contained in:
Tomas Vik 2023-01-25 09:01:48 +00:00
commit 136577d535
10 changed files with 401 additions and 78 deletions

View File

@ -53,7 +53,7 @@ type CreateOpts struct {
Config func() (config.Config, error)
}
func NewCmdCreate(f *cmdutils.Factory, runE func(opts *CreateOpts) error) *cobra.Command {
func NewCmdCreate(f *cmdutils.Factory) *cobra.Command {
opts := &CreateOpts{
IO: f.IO,
Config: f.Config,
@ -88,16 +88,16 @@ func NewCmdCreate(f *cmdutils.Factory, runE func(opts *CreateOpts) error) *cobra
Use release notes from a file
$ glab release create v1.0.1 -F changelog.md
Upload a release asset with a display name
Upload a release asset with a display name (type will default to 'other')
$ glab release create v1.0.1 '/path/to/asset.zip#My display label'
Upload a release asset with a display name and type
$ glab release create v1.0.1 '/path/to/asset.png#My display label#image'
Upload all assets in a specified folder
Upload all assets in a specified folder (types will default to 'other')
$ glab release create v1.0.1 ./dist/*
Upload all tarballs in a specified folder
Upload all tarballs in a specified folder (types will default to 'other')
$ glab release create v1.0.1 ./dist/*.tar.gz
Create a release with assets specified as JSON object
@ -150,10 +150,6 @@ func NewCmdCreate(f *cmdutils.Factory, runE func(opts *CreateOpts) error) *cobra
opts.NoteProvided = true
}
if runE != nil {
return runE(opts)
}
return createRun(opts)
},
}
@ -294,7 +290,7 @@ func createRun(opts *CreateOpts) error {
}
start := time.Now()
opts.IO.Logf("%s creating or updating release %s=%s %s=%s\n",
opts.IO.Logf("%s Creating or updating release %s=%s %s=%s\n",
color.ProgressIcon(),
color.Blue("repo"), repo.FullName(),
color.Blue("tag"), opts.TagName)
@ -346,7 +342,7 @@ func createRun(opts *CreateOpts) error {
if err != nil {
return releaseFailedErr(err, start)
}
opts.IO.Logf("%s release created\t%s=%s\n", color.GreenCheck(),
opts.IO.Logf("%s Release created\t%s=%s\n", color.GreenCheck(),
color.Blue("url"), fmt.Sprintf("%s://%s/%s/-/releases/%s",
glinstance.OverridableDefaultProtocol(), glinstance.OverridableDefault(),
repo.FullName(), release.TagName))
@ -371,35 +367,23 @@ func createRun(opts *CreateOpts) error {
return releaseFailedErr(err, start)
}
opts.IO.Logf("%s release updated\t%s=%s\n", color.GreenCheck(),
opts.IO.Logf("%s Release updated\t%s=%s\n", color.GreenCheck(),
color.Blue("url"), fmt.Sprintf("%s://%s/%s/-/releases/%s",
glinstance.OverridableDefaultProtocol(), glinstance.OverridableDefault(),
repo.FullName(), release.TagName))
}
// upload files and create asset link
if opts.AssetFiles != nil || opts.AssetLinks != nil {
opts.IO.Logf("\n%s Uploading release assets\n", color.ProgressIcon())
uploadCtx := upload.Context{
IO: opts.IO,
Client: client,
AssetsLinks: opts.AssetLinks,
AssetFiles: opts.AssetFiles,
}
if err = uploadCtx.UploadFiles(repo.FullName(), release.TagName); err != nil {
return releaseFailedErr(err, start)
}
// create asset link for assets provided as json
if err = uploadCtx.CreateReleaseAssetLinks(repo.FullName(), release.TagName); err != nil {
return releaseFailedErr(err, start)
}
// upload files and create asset links
err = releaseutils.CreateReleaseAssets(opts.IO, client, opts.AssetFiles, opts.AssetLinks, repo.FullName(), release.TagName)
if err != nil {
return releaseFailedErr(err, start)
}
if len(opts.Milestone) > 0 {
// close all associated milestones
for _, milestone := range opts.Milestone {
// run loading msg
opts.IO.StartSpinner("closing milestone %q", milestone)
opts.IO.StartSpinner("Closing milestone %q", milestone)
// close milestone
err := closeMilestone(opts, milestone)
// stop loading
@ -407,11 +391,11 @@ func createRun(opts *CreateOpts) error {
if err != nil {
opts.IO.Log(color.FailedIcon(), err.Error())
} else {
opts.IO.Logf("%s closed milestone %q\n", color.GreenCheck(), milestone)
opts.IO.Logf("%s Closed milestone %q\n", color.GreenCheck(), milestone)
}
}
}
opts.IO.Logf(color.Bold("%s release succeeded after %0.2fs\n"), color.GreenCheck(), time.Since(start).Seconds())
opts.IO.Logf(color.Bold("%s Release succeeded after %0.2fs\n"), color.GreenCheck(), time.Since(start).Seconds())
return nil
}

View File

@ -0,0 +1,201 @@
package create
import (
"io"
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"gitlab.com/gitlab-org/cli/pkg/httpmock"
"gitlab.com/gitlab-org/cli/commands/cmdtest"
"gitlab.com/gitlab-org/cli/test"
)
func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
ios, _, stdout, stderr := cmdtest.InitIOStreams(isTTY, "")
factory := cmdtest.InitFactory(ios, rt)
_, _ = factory.HttpClient()
cmd := NewCmdCreate(factory)
return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr)
}
func TestReleaseCreate(t *testing.T) {
tests := []struct {
name string
cli string
expectedDescription string
}{
{
name: "when a release is created",
cli: "0.0.1",
},
{
name: "when a release is created with a description",
cli: `0.0.1 --notes "bugfix release"`,
expectedDescription: "bugfix release",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
fakeHTTP := &httpmock.Mocker{
MatchURL: httpmock.PathAndQuerystring,
}
defer fakeHTTP.Verify(t)
fakeHTTP.RegisterResponder(http.MethodGet, "/api/v4/projects/OWNER/REPO/releases/0%2E0%2E1",
httpmock.NewStringResponse(http.StatusNotFound, `{"message":"404 Not Found"}`))
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/releases",
func(req *http.Request) (*http.Response, error) {
rb, _ := io.ReadAll(req.Body)
assert.Contains(t, string(rb), `"tag_name":"0.0.1"`)
if tc.expectedDescription != "" {
assert.Contains(t, string(rb), `"description":"bugfix release"`)
}
resp, _ := httpmock.NewStringResponse(http.StatusCreated,
`{
"name": "test_release",
"tag_name": "0.0.1",
"description": "bugfix release",
"created_at": "2023-01-19T02:58:32.622Z",
"released_at": "2023-01-19T02:58:32.622Z",
"upcoming_release": false,
"tag_path": "/OWNER/REPO/-/tags/0.0.1"
}`)(req)
return resp, nil
},
)
output, err := runCommand(fakeHTTP, false, tc.cli)
if assert.NoErrorf(t, err, "error running command `create %s`: %v", tc.cli, err) {
assert.Equal(t, ` Validating tag 0.0.1
Creating or updating release repo=OWNER/REPO tag=0.0.1
Release created url=https://gitlab.com/OWNER/REPO/-/releases/0.0.1
Release succeeded after 0.00s
`, output.Stderr())
assert.Empty(t, output.String())
}
})
}
}
func TestReleaseCreateWithFiles(t *testing.T) {
tests := []struct {
name string
cli string
wantType bool
expectedType string
expectedOut string
}{
{
name: "when a release is created and a file is uploaded using filename only",
cli: "0.0.1 fixtures/test_file.txt",
wantType: false,
},
{
name: "when a release is created and a filename is uploaded with display name and type",
cli: "0.0.1 fixtures/test_file.txt#test_file#other",
wantType: true,
expectedType: `"link_type":"other"`,
},
{
name: "when a release is created and a filename is uploaded with a type",
cli: "0.0.1 fixtures/test_file.txt##package",
wantType: true,
expectedType: `"link_type":"package"`,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
fakeHTTP := &httpmock.Mocker{
MatchURL: httpmock.PathAndQuerystring,
}
defer fakeHTTP.Verify(t)
fakeHTTP.RegisterResponder(http.MethodGet, "/api/v4/projects/OWNER/REPO/releases/0%2E0%2E1",
httpmock.NewStringResponse(http.StatusNotFound, `{"message":"404 Not Found"}`))
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/releases",
func(req *http.Request) (*http.Response, error) {
rb, _ := io.ReadAll(req.Body)
assert.Contains(t, string(rb), `"tag_name":"0.0.1"`)
resp, _ := httpmock.NewStringResponse(http.StatusCreated,
`{
"name": "test_release",
"tag_name": "0.0.1",
"description": null,
"created_at": "2023-01-19T02:58:32.622Z",
"released_at": "2023-01-19T02:58:32.622Z",
"upcoming_release": false,
"tag_path": "/OWNER/REPO/-/tags/0.0.1"
}`)(req)
return resp, nil
},
)
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/uploads",
httpmock.NewStringResponse(http.StatusCreated,
`{
"alt": "test_file",
"url": "/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt",
"full_path": "/namespace1/project1/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt",
"markdown": "![test_file](/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt)"
}`))
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/releases/0%2E0%2E1/assets/links",
func(req *http.Request) (*http.Response, error) {
rb, _ := io.ReadAll(req.Body)
if tc.wantType {
assert.Contains(t, string(rb), tc.expectedType)
} else {
assert.NotContains(t, string(rb), "link_type")
}
resp, _ := httpmock.NewStringResponse(http.StatusCreated, `{
"id":2,
"name":"test_file.txt",
"url":"https://gitlab.example.com/mynamespace/hello/-/jobs/688/artifacts/raw/fixtures/test_file.txt",
"direct_asset_url":"https://gitlab.example.com/mynamespace/hello/-/releases/0.0.1/downloads/fixtures/test_file.txt",
"external":false,
"link_type":"other"
}`)(req)
return resp, nil
},
)
output, err := runCommand(fakeHTTP, false, tc.cli)
if assert.NoErrorf(t, err, "error running command `create %s`: %v", tc.cli, err) {
assert.Equal(t, ` Validating tag 0.0.1
Creating or updating release repo=OWNER/REPO tag=0.0.1
Release created url=https://gitlab.com/OWNER/REPO/-/releases/0.0.1
Uploading release assets repo=OWNER/REPO tag=0.0.1
Uploading to release file=fixtures/test_file.txt name=test_file.txt
Release succeeded after 0.00s
`, output.Stderr())
assert.Empty(t, output.String())
}
})
}
}

View File

@ -0,0 +1 @@
test

View File

@ -21,8 +21,8 @@ func NewCmdRelease(f *cmdutils.Factory) *cobra.Command {
cmdutils.EnableRepoOverride(releaseCmd, f)
releaseCmd.AddCommand(releaseListCmd.NewCmdReleaseList(f))
releaseCmd.AddCommand(releaseCreateCmd.NewCmdCreate(f, nil))
releaseCmd.AddCommand(releaseUploadCmd.NewCmdUpload(f, nil))
releaseCmd.AddCommand(releaseCreateCmd.NewCmdCreate(f))
releaseCmd.AddCommand(releaseUploadCmd.NewCmdUpload(f))
releaseCmd.AddCommand(releaseDeleteCmd.NewCmdDelete(f, nil))
releaseCmd.AddCommand(releaseViewCmd.NewCmdView(f, nil))
releaseCmd.AddCommand(releaseDownloadCmd.NewCmdDownload(f, nil))

View File

@ -6,6 +6,8 @@ import (
"os"
"strings"
"gitlab.com/gitlab-org/cli/commands/cmdutils"
"gitlab.com/gitlab-org/cli/internal/glrepo"
"gitlab.com/gitlab-org/cli/pkg/glinstance"
@ -87,20 +89,53 @@ func AssetsFromArgs(args []string) (assets []*upload.ReleaseFile, err error) {
if label == "" {
label = fi.Name()
}
var linkTypeVal gitlab.LinkTypeValue
if linkType != "" {
linkTypeVal = gitlab.LinkTypeValue(linkType)
}
assets = append(assets, &upload.ReleaseFile{
rf := &upload.ReleaseFile{
Open: func() (io.ReadCloser, error) {
return os.Open(fn)
},
Name: fi.Name(),
Label: label,
Path: fn,
Type: &linkTypeVal,
})
}
// Only add a link type if it was specified
// Otherwise the GitLab API will default to 'other' if it was omitted
if linkType != "" {
linkTypeVal := gitlab.LinkTypeValue(linkType)
rf.Type = &linkTypeVal
}
assets = append(assets, rf)
}
return
}
func CreateReleaseAssets(io *iostreams.IOStreams, client *gitlab.Client, assetFiles []*upload.ReleaseFile, assetLinks []*upload.ReleaseAsset, repoName string, tagName string) error {
if assetFiles == nil && assetLinks == nil {
return nil
}
uploadCtx := upload.Context{
IO: io,
Client: client,
AssetsLinks: assetLinks,
AssetFiles: assetFiles,
}
color := io.Color()
io.Logf("%s Uploading release assets %s=%s %s=%s\n",
color.ProgressIcon(),
color.Blue("repo"), repoName,
color.Blue("tag"), tagName)
if err := uploadCtx.UploadFiles(repoName, tagName); err != nil {
return cmdutils.WrapError(err, "upload failed")
}
// create asset link for assets provided as json
if err := uploadCtx.CreateReleaseAssetLinks(repoName, tagName); err != nil {
return cmdutils.WrapError(err, "failed to create release link")
}
return nil
}

View File

@ -0,0 +1 @@
test

View File

@ -30,7 +30,7 @@ type UploadOpts struct {
Config func() (config.Config, error)
}
func NewCmdUpload(f *cmdutils.Factory, runE func(opts *UploadOpts) error) *cobra.Command {
func NewCmdUpload(f *cmdutils.Factory) *cobra.Command {
opts := &UploadOpts{
IO: f.IO,
Config: f.Config,
@ -56,16 +56,16 @@ func NewCmdUpload(f *cmdutils.Factory, runE func(opts *UploadOpts) error) *cobra
}
}(),
Example: heredoc.Doc(`
Upload a release asset with a display name
Upload a release asset with a display name (type will default to 'other')
$ glab release upload v1.0.1 '/path/to/asset.zip#My display label'
Upload a release asset with a display name and type
$ glab release upload v1.0.1 '/path/to/asset.png#My display label#image'
Upload all assets in a specified folder
Upload all assets in a specified folder (types will default to 'other')
$ glab release upload v1.0.1 ./dist/*
Upload all tarballs in a specified folder
Upload all tarballs in a specified folder (types will default to 'other')
$ glab release upload v1.0.1 ./dist/*.tar.gz
Upload release assets links specified as JSON string
@ -102,11 +102,7 @@ func NewCmdUpload(f *cmdutils.Factory, runE func(opts *UploadOpts) error) *cobra
}
}
if runE != nil {
return runE(opts)
}
return deleteRun(opts)
return uploadRun(opts)
},
}
@ -115,7 +111,7 @@ func NewCmdUpload(f *cmdutils.Factory, runE func(opts *UploadOpts) error) *cobra
return cmd
}
func deleteRun(opts *UploadOpts) error {
func uploadRun(opts *UploadOpts) error {
start := time.Now()
client, err := opts.HTTPClient()
@ -130,7 +126,7 @@ func deleteRun(opts *UploadOpts) error {
color := opts.IO.Color()
var resp *gitlab.Response
opts.IO.Logf("%s validating tag %s=%s %s=%s\n",
opts.IO.Logf("%s Validating tag %s=%s %s=%s\n",
color.ProgressIcon(),
color.Blue("repo"), repo.FullName(),
color.Blue("tag"), opts.TagName)
@ -143,28 +139,12 @@ func deleteRun(opts *UploadOpts) error {
return cmdutils.WrapError(err, "failed to fetch release")
}
opts.IO.Logf("%s uploading release assets %s=%s %s=%s\n",
color.ProgressIcon(),
color.Blue("repo"), repo.FullName(),
color.Blue("tag"), opts.TagName)
// upload files and create asset link
if opts.AssetFiles != nil || opts.AssetLinks != nil {
uploadCtx := upload.Context{
IO: opts.IO,
Client: client,
AssetsLinks: opts.AssetLinks,
AssetFiles: opts.AssetFiles,
}
if err = uploadCtx.UploadFiles(repo.FullName(), release.TagName); err != nil {
return cmdutils.WrapError(err, "upload failed")
}
// create asset link for assets provided as json
if err = uploadCtx.CreateReleaseAssetLinks(repo.FullName(), release.TagName); err != nil {
return cmdutils.WrapError(err, "failed to create release link")
}
// upload files and create asset links
err = releaseutils.CreateReleaseAssets(opts.IO, client, opts.AssetFiles, opts.AssetLinks, repo.FullName(), release.TagName)
if err != nil {
return cmdutils.WrapError(err, "creating release assets failed")
}
opts.IO.Logf(color.Bold("%s upload succeeded after %0.2fs\n"), color.GreenCheck(), time.Since(start).Seconds())
opts.IO.Logf(color.Bold("%s Upload succeeded after %0.2fs\n"), color.GreenCheck(), time.Since(start).Seconds())
return nil
}

View File

@ -0,0 +1,121 @@
package upload
import (
"io"
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"gitlab.com/gitlab-org/cli/pkg/httpmock"
"gitlab.com/gitlab-org/cli/commands/cmdtest"
"gitlab.com/gitlab-org/cli/test"
)
func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
ios, _, stdout, stderr := cmdtest.InitIOStreams(isTTY, "")
factory := cmdtest.InitFactory(ios, rt)
_, _ = factory.HttpClient()
cmd := NewCmdUpload(factory)
return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr)
}
func TestReleaseUpload(t *testing.T) {
tests := []struct {
name string
cli string
wantType bool
expectedType string
expectedOut string
}{
{
name: "when a file is uploaded using filename only, and does not send a link_type",
cli: "0.0.1 fixtures/test_file.txt",
wantType: false,
},
{
name: "when a file is uploaded using a filename, display name and type",
cli: "0.0.1 fixtures/test_file.txt#test_file#other",
wantType: true,
expectedType: `"link_type":"other"`,
},
{
name: "when a file is uploaded using a filename and type only",
cli: "0.0.1 fixtures/test_file.txt##package",
wantType: true,
expectedType: `"link_type":"package"`,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
fakeHTTP := &httpmock.Mocker{
MatchURL: httpmock.PathAndQuerystring,
}
defer fakeHTTP.Verify(t)
fakeHTTP.RegisterResponder(http.MethodGet, "/api/v4/projects/OWNER/REPO/releases/0%2E0%2E1",
httpmock.NewStringResponse(http.StatusOK,
`{
"name": "test1",
"tag_name": "0.0.1",
"description": null,
"created_at": "2023-01-19T02:58:32.622Z",
"released_at": "2023-01-19T02:58:32.622Z",
"upcoming_release": false,
"tag_path": "/OWNER/REPO/-/tags/0.0.1"
}`))
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/uploads",
httpmock.NewStringResponse(http.StatusCreated,
`{
"alt": "test_file",
"url": "/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt",
"full_path": "/namespace1/project1/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt",
"markdown": "![test_file](/uploads/66dbcd21ec5d24ed6ea225176098d52b/fixtures/test_file.txt)"
}`))
fakeHTTP.RegisterResponder(http.MethodPost, "/api/v4/projects/OWNER/REPO/releases/0%2E0%2E1/assets/links",
func(req *http.Request) (*http.Response, error) {
rb, _ := io.ReadAll(req.Body)
if tc.wantType {
assert.Contains(t, string(rb), tc.expectedType)
} else {
assert.NotContains(t, string(rb), "link_type")
}
resp, _ := httpmock.NewStringResponse(http.StatusCreated, `{
"id":2,
"name":"test_file.txt",
"url":"https://gitlab.example.com/mynamespace/hello/-/jobs/688/artifacts/raw/fixtures/test_file.txt",
"direct_asset_url":"https://gitlab.example.com/mynamespace/hello/-/releases/0.0.1/downloads/fixtures/test_file.txt",
"external":false,
"link_type":"other"
}`)(req)
return resp, nil
},
)
output, err := runCommand(fakeHTTP, false, tc.cli)
if assert.NoErrorf(t, err, "error running command `release upload %s`: %v", tc.cli, err) {
assert.Equal(t, ` Validating tag repo=OWNER/REPO tag=0.0.1
Uploading release assets repo=OWNER/REPO tag=0.0.1
Uploading to release file=fixtures/test_file.txt name=test_file.txt
Upload succeeded after 0.00s
`, output.Stderr())
assert.Empty(t, output.String())
}
})
}
}

View File

@ -46,16 +46,16 @@ $ glab release create v1.0.1 --notes "bugfix release"
Use release notes from a file
$ glab release create v1.0.1 -F changelog.md
Upload a release asset with a display name
Upload a release asset with a display name (type will default to 'other')
$ glab release create v1.0.1 '/path/to/asset.zip#My display label'
Upload a release asset with a display name and type
$ glab release create v1.0.1 '/path/to/asset.png#My display label#image'
Upload all assets in a specified folder
Upload all assets in a specified folder (types will default to 'other')
$ glab release create v1.0.1 ./dist/*
Upload all tarballs in a specified folder
Upload all tarballs in a specified folder (types will default to 'other')
$ glab release create v1.0.1 ./dist/*.tar.gz
Create a release with assets specified as JSON object

View File

@ -27,16 +27,16 @@ glab release upload <tag> [<files>...] [flags]
## Examples
```plaintext
Upload a release asset with a display name
Upload a release asset with a display name (type will default to 'other')
$ glab release upload v1.0.1 '/path/to/asset.zip#My display label'
Upload a release asset with a display name and type
$ glab release upload v1.0.1 '/path/to/asset.png#My display label#image'
Upload all assets in a specified folder
Upload all assets in a specified folder (types will default to 'other')
$ glab release upload v1.0.1 ./dist/*
Upload all tarballs in a specified folder
Upload all tarballs in a specified folder (types will default to 'other')
$ glab release upload v1.0.1 ./dist/*.tar.gz
Upload release assets links specified as JSON string