From 40c8f6760e15bb4c8b8c246919492d937323c92f Mon Sep 17 00:00:00 2001 From: Clement Sam Date: Wed, 10 Aug 2022 18:47:14 +0000 Subject: [PATCH] fix(check-update): check latest release from GitLab Currently the check-update command which checks for latest glab releases, checks from the releases page of the old GitHub repo. This MR changes this behaviour to check releases from the new gitlab-org/cli repo. --- api/default.go | 2 +- cmd/glab/main.go | 2 +- commands/cmdutils/cmdutils.go | 5 +- commands/issue/issueutils/utils.go | 5 +- commands/project/archive/repo_archive.go | 2 +- commands/root.go | 2 +- commands/snippet/create/create.go | 9 +- commands/update/check_update.go | 57 +++++++++--- commands/update/check_update_test.go | 110 ++++++++++++++++++----- commands/update/update.go | 43 --------- commands/update/update_test.go | 49 ---------- internal/config/writefile_windows.go | 5 +- 12 files changed, 148 insertions(+), 143 deletions(-) delete mode 100644 commands/update/update.go delete mode 100644 commands/update/update_test.go diff --git a/api/default.go b/api/default.go index f9fac053..5a6bd257 100644 --- a/api/default.go +++ b/api/default.go @@ -2,7 +2,7 @@ package api var DefaultListLimit = 30 -//IsValidToken checks if a token provided is valid. +// IsValidToken checks if a token provided is valid. // The token string must be 26 characters in length and have the 'glpat-' // prefix or just be 20 characters long to be recognized as a valid personal access token. // diff --git a/cmd/glab/main.go b/cmd/glab/main.go index b70ad90e..3af5dc97 100644 --- a/cmd/glab/main.go +++ b/cmd/glab/main.go @@ -154,7 +154,7 @@ func main() { checkUpdate, _ := cfg.Get("", "check_update") if checkUpdate, err := strconv.ParseBool(checkUpdate); err == nil && checkUpdate { - err = update.CheckUpdate(cmdFactory.IO, version, true) + err = update.CheckUpdate(cmdFactory, version, true) if err != nil && debug { printError(cmdFactory.IO, err, rootCmd, debug, false) } diff --git a/commands/cmdutils/cmdutils.go b/commands/cmdutils/cmdutils.go index b819c5ee..7259d31a 100644 --- a/commands/cmdutils/cmdutils.go +++ b/commands/cmdutils/cmdutils.go @@ -63,7 +63,8 @@ func LoadGitLabTemplate(tmplType, tmplName string) (string, error) { } // TODO: properly handle errors in this function. -// For now, it returns nil and empty slice if there's an error +// +// For now, it returns nil and empty slice if there's an error func ListGitLabTemplates(tmplType string) ([]string, error) { wdir, err := git.ToplevelDir() if err != nil { @@ -351,7 +352,7 @@ func PickMetadata() ([]Action, error) { return pickedActions, nil } -//IDsFromUsers collects all user IDs from a slice of users +// IDsFromUsers collects all user IDs from a slice of users func IDsFromUsers(users []*gitlab.User) []int { ids := make([]int, len(users)) for i, user := range users { diff --git a/commands/issue/issueutils/utils.go b/commands/issue/issueutils/utils.go index 93be6a68..a820f6e0 100644 --- a/commands/issue/issueutils/utils.go +++ b/commands/issue/issueutils/utils.go @@ -135,8 +135,9 @@ func IssueFromArg(apiClient *gitlab.Client, baseRepoFn func() (glrepo.Interface, } // FIXME: have a single regex to match either of the following -// OWNER/REPO/issues/id -// GROUP/NAMESPACE/REPO/issues/id +// +// OWNER/REPO/issues/id +// GROUP/NAMESPACE/REPO/issues/id var issueURLPersonalRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/issues/(\d+)`) var issueURLGroupRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/([^/]+)/issues/(\d+)`) diff --git a/commands/project/archive/repo_archive.go b/commands/project/archive/repo_archive.go index 5db34ae6..017fc3d7 100644 --- a/commands/project/archive/repo_archive.go +++ b/commands/project/archive/repo_archive.go @@ -108,7 +108,7 @@ func NewCmdArchive(f *cmdutils.Factory) *cobra.Command { return repoArchiveCmd } -//CloneWriter w +// CloneWriter w type CloneWriter struct { Total uint64 } diff --git a/commands/root.go b/commands/root.go index 3af92f7f..7f23cdb2 100644 --- a/commands/root.go +++ b/commands/root.go @@ -97,7 +97,7 @@ func NewCmdRoot(f *cmdutils.Factory, version, buildDate string) *cobra.Command { rootCmd.AddCommand(configCmd.NewCmdConfig(f)) rootCmd.AddCommand(completionCmd.NewCmdCompletion(f.IO)) rootCmd.AddCommand(versionCmd.NewCmdVersion(f.IO, version, buildDate)) - rootCmd.AddCommand(updateCmd.NewCheckUpdateCmd(f.IO, version)) + rootCmd.AddCommand(updateCmd.NewCheckUpdateCmd(f, version)) rootCmd.AddCommand(authCmd.NewCmdAuth(f)) // the commands below require apiClient and resolved repos diff --git a/commands/snippet/create/create.go b/commands/snippet/create/create.go index 7ad0a12d..c575622c 100644 --- a/commands/snippet/create/create.go +++ b/commands/snippet/create/create.go @@ -116,11 +116,12 @@ func runCreate(client *gitlab.Client, repo glrepo.Interface, opts *CreateOpts) e } // FIXME: Adding more then one file can't be done right now because the GitLab API library -// Doesn't support it yet. // -// See for the API reference: https://docs.gitlab.com/ee/api/snippets.html#create-new-snippet -// See for the library docs : https://pkg.go.dev/github.com/xanzy/go-gitlab#CreateSnippetOptions -// See for GitHub issue : https://github.com/xanzy/go-gitlab/issues/1372 +// Doesn't support it yet. +// +// See for the API reference: https://docs.gitlab.com/ee/api/snippets.html#create-new-snippet +// See for the library docs : https://pkg.go.dev/github.com/xanzy/go-gitlab#CreateSnippetOptions +// See for GitHub issue : https://github.com/xanzy/go-gitlab/issues/1372 func readSnippetsContent(opts *CreateOpts) (string, error) { if opts.isSnippetFromFile() { return readFromFile(opts.FilePath) diff --git a/commands/update/check_update.go b/commands/update/check_update.go index 6024026d..b6404f6e 100644 --- a/commands/update/check_update.go +++ b/commands/update/check_update.go @@ -1,49 +1,80 @@ package update import ( - "errors" "fmt" + "strings" - "github.com/profclems/glab/pkg/iostreams" + "github.com/profclems/glab/commands/cmdutils" + "github.com/hashicorp/go-version" "github.com/spf13/cobra" + "github.com/xanzy/go-gitlab" ) -func NewCheckUpdateCmd(s *iostreams.IOStreams, version string) *cobra.Command { +const defaultProjectURL = "https://gitlab.com/gitlab-org/cli" + +func NewCheckUpdateCmd(f *cmdutils.Factory, version string) *cobra.Command { var cmd = &cobra.Command{ Use: "check-update", Short: "Check for latest glab releases", Long: ``, Aliases: []string{"update"}, RunE: func(cmd *cobra.Command, args []string) error { - return CheckUpdate(s, version, false) + return CheckUpdate(f, version, false) }, } return cmd } -func CheckUpdate(s *iostreams.IOStreams, version string, silentErr bool) error { - latestRelease, err := GetUpdateInfo() - c := s.Color() +func CheckUpdate(f *cmdutils.Factory, version string, silentErr bool) error { + err := f.RepoOverride(defaultProjectURL) + if err != nil { + return err + } + repo, err := f.BaseRepo() + if err != nil { + return err + } + apiClient, err := f.HttpClient() + if err != nil { + return err + } + releases, _, err := apiClient.Releases.ListReleases(repo.FullName(), &gitlab.ListReleasesOptions{Page: 1, PerPage: 1}) if err != nil { if silentErr { return nil } - return errors.New("could not check for update! Make sure you have a stable internet connection") + return fmt.Errorf("could not check for update: %s", err.Error()) } + if len(releases) < 1 { + return fmt.Errorf("no release found for glab") + } + latestRelease := releases[0] + releaseURL := fmt.Sprintf("%s/-/releases/%s", defaultProjectURL, latestRelease.TagName) - if isOlderVersion(latestRelease.Version, version) { - fmt.Fprintf(s.StdOut, "%s %s → %s\n%s\n", + c := f.IO.Color() + if isOlderVersion(latestRelease.Name, version) { + fmt.Fprintf(f.IO.StdOut, "%s %s → %s\n%s\n", c.Yellow("A new version of glab has been released:"), - c.Red(version), c.Green(latestRelease.Version), - latestRelease.URL) + c.Red(version), c.Green(latestRelease.TagName), + releaseURL) } else { if silentErr { return nil } - fmt.Fprintf(s.StdOut, "%v %v", c.GreenCheck(), + fmt.Fprintf(f.IO.StdOut, "%v %v", c.GreenCheck(), c.Green("You are already using the latest version of glab\n")) } return nil } + +func isOlderVersion(latestVersion, appVersion string) bool { + latestVersion = strings.TrimSpace(latestVersion) + appVersion = strings.TrimSpace(appVersion) + + vv, ve := version.NewVersion(latestVersion) + vw, we := version.NewVersion(appVersion) + + return ve == nil && we == nil && vv.GreaterThan(vw) +} diff --git a/commands/update/check_update_test.go b/commands/update/check_update_test.go index eb2e9089..cd44abb6 100644 --- a/commands/update/check_update_test.go +++ b/commands/update/check_update_test.go @@ -1,33 +1,34 @@ package update import ( + "bytes" "fmt" + "net/http" "testing" + "github.com/profclems/glab/api" + "github.com/profclems/glab/commands/cmdutils" "github.com/profclems/glab/pkg/iostreams" "github.com/alecthomas/assert" "github.com/jarcoal/httpmock" + "github.com/xanzy/go-gitlab" ) func TestNewCheckUpdateCmd(t *testing.T) { httpmock.Activate() defer httpmock.DeactivateAndReset() - httpmock.RegisterResponder("GET", `https://api.github.com/repos/profclems/glab/releases/latest`, - httpmock.NewStringResponder(200, `{ - "url": "https://api.github.com/repos/profclems/glab/releases/33385584", - "html_url": "https://github.com/profclems/glab/releases/tag/v1.11.1", - "tag_name": "v1.11.1", + httpmock.RegisterResponder("GET", `https://gitlab.com/api/v4/projects/gitlab-org%2Fcli/releases`, + httpmock.NewStringResponder(200, `[{"tag_name": "v1.11.1", "name": "v1.11.1", - "draft": false, - "prerelease": false, "created_at": "2020-11-03T05:33:29Z", - "published_at": "2020-11-03T05:39:04Z"}`)) + "released_at": "2020-11-03T05:39:04Z"}]`)) + + factory, _, stdout, stderr, err := makeTestFactory() + assert.Nil(t, err) - ioStream, _, stdout, stderr := iostreams.Test() type args struct { - s *iostreams.IOStreams version string } tests := []struct { @@ -40,7 +41,6 @@ func TestNewCheckUpdateCmd(t *testing.T) { { name: "same version", args: args{ - s: ioStream, version: "v1.11.1", }, stdOut: "✓ You are already using the latest version of glab\n", @@ -49,16 +49,15 @@ func TestNewCheckUpdateCmd(t *testing.T) { { name: "older version", args: args{ - s: ioStream, version: "v1.11.0", }, - stdOut: "A new version of glab has been released: v1.11.0 → v1.11.1\nhttps://github.com/profclems/glab/releases/tag/v1.11.1\n", + stdOut: "A new version of glab has been released: v1.11.0 → v1.11.1\nhttps://gitlab.com/gitlab-org/cli/-/releases/v1.11.1\n", stdErr: "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := NewCheckUpdateCmd(tt.args.s, tt.args.version).Execute() + err := NewCheckUpdateCmd(factory, tt.args.version).Execute() if tt.wantErr { assert.Nil(t, err) } @@ -77,30 +76,93 @@ func TestNewCheckUpdateCmd_error(t *testing.T) { httpmock.Activate() defer httpmock.DeactivateAndReset() - httpmock.RegisterResponder("GET", `https://api.github.com/repos/profclems/glab/releases/latest`, + httpmock.RegisterResponder("GET", `https://gitlab.com/api/v4/projects/gitlab-org%2Fcli/releases`, httpmock.NewErrorResponder(fmt.Errorf("an error expected"))) - ioStream, _, stdout, stderr := iostreams.Test() + factory, _, stdout, stderr, err := makeTestFactory() + assert.Nil(t, err) - err := NewCheckUpdateCmd(ioStream, "1.11.0").Execute() + err = NewCheckUpdateCmd(factory, "1.11.0").Execute() assert.NotNil(t, err) - assert.Equal(t, "could not check for update! Make sure you have a stable internet connection", err.Error()) + assert.Equal(t, "could not check for update: Get \"https://gitlab.com/api/v4/projects/gitlab-org%2Fcli/releases?page=1&per_page=1\": an error expected", err.Error()) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) } -func TestNewCheckUpdateCmd_json_error(t *testing.T) { +func TestNewCheckUpdateCmd_no_release(t *testing.T) { httpmock.Activate() defer httpmock.DeactivateAndReset() - httpmock.RegisterResponder("GET", `https://api.github.com/repos/profclems/glab/releases/latest`, - httpmock.NewStringResponder(200, ``)) + httpmock.RegisterResponder("GET", `https://gitlab.com/api/v4/projects/gitlab-org%2Fcli/releases`, + httpmock.NewStringResponder(200, `[]`)) - ioStream, _, stdout, stderr := iostreams.Test() + factory, _, stdout, stderr, err := makeTestFactory() + assert.Nil(t, err) - err := NewCheckUpdateCmd(ioStream, "1.11.0").Execute() + err = NewCheckUpdateCmd(factory, "1.11.0").Execute() assert.NotNil(t, err) - assert.Equal(t, "could not check for update! Make sure you have a stable internet connection", err.Error()) + assert.Equal(t, "no release found for glab", err.Error()) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) } + +func Test_isOlderVersion(t *testing.T) { + type args struct { + latestVersion string + currentVersion string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "latest is newer", + args: args{"v1.10.0", "v1.9.1"}, + want: true, + }, + { + name: "latest is current", + args: args{"v1.9.2", "v1.9.2"}, + want: false, + }, + { + name: "latest is older", + args: args{"v1.9.0", "v1.9.2-pre.1"}, + want: false, + }, + { + name: "current is prerelease", + args: args{"v1.9.0", "v1.9.0-pre.1"}, + want: true, + }, + { + name: "latest is older (against prerelease)", + args: args{"v1.9.0", "v1.10.0-pre.1"}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isOlderVersion(tt.args.latestVersion, tt.args.currentVersion); got != tt.want { + t.Errorf("isOlderVersion(%s, %s) = %v, want %v", + tt.args.latestVersion, tt.args.currentVersion, got, tt.want) + } + }) + } +} + +func makeTestFactory() (factory *cmdutils.Factory, in *bytes.Buffer, out *bytes.Buffer, errOut *bytes.Buffer, err error) { + var apiClient *api.Client + apiClient, err = api.TestClient(http.DefaultClient, "", "gitlab.com", false) + if err != nil { + return + } + + factory = cmdutils.NewFactory() + factory.HttpClient = func() (*gitlab.Client, error) { + return apiClient.Lab(), nil + } + factory.IO, _, out, errOut = iostreams.Test() + return +} diff --git a/commands/update/update.go b/commands/update/update.go deleted file mode 100644 index 69aba29d..00000000 --- a/commands/update/update.go +++ /dev/null @@ -1,43 +0,0 @@ -package update - -import ( - "encoding/json" - "strings" - "time" - - "github.com/hashicorp/go-version" - - "github.com/profclems/glab/api/request" -) - -type ReleaseInfo struct { - Version string `json:"tag_name"` - PreRelease bool `json:"prerelease"` - URL string `json:"html_url"` - PublishedAt time.Time `json:"published_at"` -} - -// GetUpdateInfo checks for latest glab release and returns the ReleaseInfo -func GetUpdateInfo() (ReleaseInfo, error) { - releasesUrl := "https://api.github.com/repos/profclems/glab/releases/latest" - resp, err := request.MakeRequest("{}", releasesUrl, "GET") - var releaseInfo ReleaseInfo - if err != nil { - return ReleaseInfo{}, err - } - err = json.Unmarshal([]byte(resp), &releaseInfo) - if err != nil { - return ReleaseInfo{}, err - } - return releaseInfo, nil -} - -func isOlderVersion(latestVersion, appVersion string) bool { - latestVersion = strings.TrimSpace(latestVersion) - appVersion = strings.TrimSpace(appVersion) - - vv, ve := version.NewVersion(latestVersion) - vw, we := version.NewVersion(appVersion) - - return ve == nil && we == nil && vv.GreaterThan(vw) -} diff --git a/commands/update/update_test.go b/commands/update/update_test.go deleted file mode 100644 index 95558ee5..00000000 --- a/commands/update/update_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package update - -import "testing" - -func Test_isLatestVersion(t *testing.T) { - type args struct { - latestVersion string - currentVersion string - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "latest is newer", - args: args{"v1.10.0", "v1.9.1"}, - want: true, - }, - { - name: "latest is current", - args: args{"v1.9.2", "v1.9.2"}, - want: false, - }, - { - name: "latest is older", - args: args{"v1.9.0", "v1.9.2-pre.1"}, - want: false, - }, - { - name: "current is prerelease", - args: args{"v1.9.0", "v1.9.0-pre.1"}, - want: true, - }, - { - name: "latest is older (against prerelease)", - args: args{"v1.9.0", "v1.10.0-pre.1"}, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := isOlderVersion(tt.args.latestVersion, tt.args.currentVersion); got != tt.want { - t.Errorf("isOlderVersion(%s, %s) = %v, want %v", - tt.args.latestVersion, tt.args.currentVersion, got, tt.want) - } - }) - } -} diff --git a/internal/config/writefile_windows.go b/internal/config/writefile_windows.go index d4721dfd..b5a515d8 100644 --- a/internal/config/writefile_windows.go +++ b/internal/config/writefile_windows.go @@ -6,8 +6,9 @@ import ( ) // Note: this is not atomic, but apparently there's no way to atomically -// replace a file on windows which is why renameio doesn't support -// windows. +// +// replace a file on windows which is why renameio doesn't support +// windows. func WriteFile(filename string, data []byte, perm os.FileMode) error { return ioutil.WriteFile(filename, data, perm) }