From 7bf061d9e12f29026d5a6446fb3aedfd864b33a4 Mon Sep 17 00:00:00 2001 From: dreamjz Date: Sat, 11 Nov 2023 21:50:32 +0800 Subject: [PATCH] Feat: Limit the number of versions retrieved by command generate and update. Sovle issue #2355 (#2447) * feat(limit query versions):Add limit flag to generate command * feat(limit query versions):Add limit parameter to FuzzyGetter.Get * feat(limit query versions):Add limit parameter to VersionGetter.List * feat(limit query versions): Set the default limit to 30 and Reduce the consumption of GitHub API * feat(limit query versions): Add help info for --limit/-l flag in g command * feat(limit query versions): Edit "defaultVerNumLimit" to "defaultVerCntLimit" for avoiding ambiguity * feat(limit query versions): Move constants * feat(limit query versions): Remove redundant code * feat(limit query versions): Add limit flag to update command * feat(limit query versions): Add help info for --limit/-l flag in update command * feat(limit query versions): Set limit before searching versions in update command * feat(limit query versions): Make some minor changes * feat(limit query versions): Remove code that could result in version data loss * feat(limit query versions): Remove nolint:gomnd directive * feat(limit query versions): Determine the 'per_page' value depending on the existence of filter * feat(limit query versions): Use the recommended approach for pagination * feat(limit query versions): Reduce cognitive complexity * feat(limit query versions): Reduce cyclomatic complexity * feat(limit query versions): Edit mock data * feat(limit query versions): fix wrong variable * feat(limit query versions): Non-positive number of limit will refer to no limitation * feat(limit query versions): fix lint issue of magic number --- pkg/cli/generate.go | 15 ++++++++++++++- pkg/cli/update.go | 15 ++++++++++++++- pkg/config/const.go | 7 +++++++ pkg/controller/generate/controller.go | 2 +- pkg/controller/generate/generate.go | 2 +- pkg/controller/update/controller.go | 2 +- pkg/controller/update/package.go | 2 +- pkg/controller/update/update.go | 1 + pkg/versiongetter/cargo.go | 2 +- pkg/versiongetter/cargo_test.go | 2 +- pkg/versiongetter/fuzzy_getter.go | 4 ++-- pkg/versiongetter/fuzzy_getter_test.go | 2 +- pkg/versiongetter/general.go | 17 +++++++++++++++-- pkg/versiongetter/github_release.go | 14 +++++++++----- pkg/versiongetter/github_release_test.go | 2 +- pkg/versiongetter/github_tag.go | 14 +++++++++----- pkg/versiongetter/github_tag_test.go | 2 +- pkg/versiongetter/mock_fuzzy_getter.go | 2 +- pkg/versiongetter/mock_github_release.go | 3 ++- pkg/versiongetter/mock_github_tag.go | 3 ++- pkg/versiongetter/mock_version_getter.go | 2 +- pkg/versiongetter/version_getter.go | 2 +- 22 files changed, 87 insertions(+), 30 deletions(-) diff --git a/pkg/cli/generate.go b/pkg/cli/generate.go index f1a54d04c..18c4fa23b 100644 --- a/pkg/cli/generate.go +++ b/pkg/cli/generate.go @@ -86,8 +86,15 @@ echo "cli/cli" | aqua g -f - - name: cli/cli@v2.2.0 You can select a version interactively with "-s" option. +By default, aqua g -s will only display 30 versions of package. +Use --limit/-l to change it. Non-positive number refers to no limit. +# Display 30 versions of selected by default $ aqua g -s +# Display all versions of selected package +$ aqua g -s -l -1 +# Display 5 versions of selected package +$ aqua g -s -l 5 The option "-pin" is useful to prevent the package from being updated by Renovate. @@ -137,7 +144,13 @@ func (r *Runner) newGenerateCommand() *cli.Command { &cli.BoolFlag{ Name: "select-version", Aliases: []string{"s"}, - Usage: `Select the installed version interactively`, + Usage: `Select the installed version interactively. Default to display 30 versions, use --limit/-l to change it.`, + }, + &cli.IntFlag{ + Name: "limit", + Aliases: []string{"l"}, + Usage: "The maximum number of versions. Non-positive number refers to no limit.", + Value: config.DefaultVerCnt, }, }, } diff --git a/pkg/cli/update.go b/pkg/cli/update.go index 9a142725f..4fc79d76c 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -24,7 +24,7 @@ func (r *Runner) newUpdateCommand() *cli.Command { &cli.BoolFlag{ Name: "select-version", Aliases: []string{"s"}, - Usage: `Select the version with fuzzy finder`, + Usage: `Select the version with fuzzy finder. Default to display 30 versions, use --limit/-l to change it.`, }, &cli.BoolFlag{ Name: "only-registry", @@ -36,6 +36,12 @@ func (r *Runner) newUpdateCommand() *cli.Command { Aliases: []string{"p"}, Usage: `Update only packages`, }, + &cli.IntFlag{ + Name: "limit", + Aliases: []string{"l"}, + Usage: "The maximum number of versions. Non-positive number refers to no limit.", + Value: config.DefaultVerCnt, + }, }, } } @@ -79,9 +85,16 @@ If -i option is used, registries aren't updated. If you want to select versions, please use the --select-version [-s] option. You can select versions with the fuzzy finder. You can not only update but also downgrade packages. +By default, -s will only display 30 versions of package. +Use --limit/-l to change it. Non-positive number refers to no limit. # Select updated packages and versions with fuzzy finder + # Display 30 versions by default $ aqua update -i -s + # Display only 5 versions + $ aqua update -i -s -l 5 + # Display all versions + $ aqua update -i -s -l -1 This command doesn't update packages if the field 'version' is used. diff --git a/pkg/config/const.go b/pkg/config/const.go index e364851e4..29e59dec2 100644 --- a/pkg/config/const.go +++ b/pkg/config/const.go @@ -3,3 +3,10 @@ package config const ( formatRaw = "raw" ) + +// Exported constant +const ( + // DefaultVerCnt is the default value for + // --limit/-l flag in command generate, update + DefaultVerCnt int = 30 +) diff --git a/pkg/controller/generate/controller.go b/pkg/controller/generate/controller.go index 6ecd6ebde..6b2eff805 100644 --- a/pkg/controller/generate/controller.go +++ b/pkg/controller/generate/controller.go @@ -31,7 +31,7 @@ type ConfigReader interface { } type FuzzyGetter interface { - Get(ctx context.Context, logE *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool) string + Get(ctx context.Context, logE *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool, limit int) string } type FuzzyFinder interface { diff --git a/pkg/controller/generate/generate.go b/pkg/controller/generate/generate.go index 969f99fde..a90bfd163 100644 --- a/pkg/controller/generate/generate.go +++ b/pkg/controller/generate/generate.go @@ -197,7 +197,7 @@ func (c *Controller) getOutputtedPkg(ctx context.Context, logE *logrus.Entry, pa outputPkg.Registry = "" } if outputPkg.Version == "" { - version := c.fuzzyGetter.Get(ctx, logE, pkg.PackageInfo, "", param.SelectVersion) + version := c.fuzzyGetter.Get(ctx, logE, pkg.PackageInfo, "", param.SelectVersion, param.Limit) if version == "" { outputPkg.Version = "[SET PACKAGE VERSION]" return outputPkg diff --git a/pkg/controller/update/controller.go b/pkg/controller/update/controller.go index 1211b9594..2c4d2471b 100644 --- a/pkg/controller/update/controller.go +++ b/pkg/controller/update/controller.go @@ -44,7 +44,7 @@ type ConfigReader interface { } type FuzzyGetter interface { - Get(ctx context.Context, logE *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool) string + Get(ctx context.Context, logE *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool, limit int) string } type RepositoriesService interface { diff --git a/pkg/controller/update/package.go b/pkg/controller/update/package.go index 218b3153d..d50a1b5ed 100644 --- a/pkg/controller/update/package.go +++ b/pkg/controller/update/package.go @@ -79,7 +79,7 @@ func (c *Controller) getPackageNewVersion(ctx context.Context, logE *logrus.Entr return "" } } - return c.fuzzyGetter.Get(ctx, logE, pkg.PackageInfo, pkg.Package.Version, param.SelectVersion) + return c.fuzzyGetter.Get(ctx, logE, pkg.PackageInfo, pkg.Package.Version, param.SelectVersion, param.Limit) } func (c *Controller) selectPackages(logE *logrus.Entry, cfgFilePath string) (map[string]struct{}, error) { diff --git a/pkg/controller/update/update.go b/pkg/controller/update/update.go index 4f50dc398..4513998e7 100644 --- a/pkg/controller/update/update.go +++ b/pkg/controller/update/update.go @@ -18,6 +18,7 @@ func (c *Controller) Update(ctx context.Context, logE *logrus.Entry, param *conf if len(param.Args) != 0 { return nil } + cfgFilePath, err := c.configFinder.Find(param.PWD, param.ConfigFilePath) if err != nil { return fmt.Errorf("find a configuration file: %w", err) diff --git a/pkg/versiongetter/cargo.go b/pkg/versiongetter/cargo.go index 3ef1fb040..bdd92c94e 100644 --- a/pkg/versiongetter/cargo.go +++ b/pkg/versiongetter/cargo.go @@ -27,7 +27,7 @@ func (c *CargoVersionGetter) Get(ctx context.Context, pkg *registry.PackageInfo, return c.client.GetLatestVersion(ctx, pkg.Crate) //nolint:wrapcheck } -func (c *CargoVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) { +func (c *CargoVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) { versionStrings, err := c.client.ListVersions(ctx, pkg.Crate) if err != nil { return nil, fmt.Errorf("list versions of the crate: %w", err) diff --git a/pkg/versiongetter/cargo_test.go b/pkg/versiongetter/cargo_test.go index 59fd40890..8e23bf729 100644 --- a/pkg/versiongetter/cargo_test.go +++ b/pkg/versiongetter/cargo_test.go @@ -109,7 +109,7 @@ func TestCargoVersionGetter_List(t *testing.T) { t.Parallel() cargoClient := versiongetter.NewMockCargoClient(d.versions) cargoGetter := versiongetter.NewCargo(cargoClient) - items, err := cargoGetter.List(ctx, d.pkg, d.filters) + items, err := cargoGetter.List(ctx, d.pkg, d.filters, -1) if err != nil { if d.isErr { return diff --git a/pkg/versiongetter/fuzzy_getter.go b/pkg/versiongetter/fuzzy_getter.go index 7669cbc4a..fecfcbf6f 100644 --- a/pkg/versiongetter/fuzzy_getter.go +++ b/pkg/versiongetter/fuzzy_getter.go @@ -26,14 +26,14 @@ type FuzzyFinder interface { FindMulti(items []*fuzzyfinder.Item, hasPreview bool) ([]int, error) } -func (g *FuzzyGetter) Get(ctx context.Context, _ *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool) string { //nolint:cyclop +func (g *FuzzyGetter) Get(ctx context.Context, _ *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool, limit int) string { //nolint:cyclop filters, err := createFilters(pkg) if err != nil { return "" } if useFinder { //nolint:nestif - versions, err := g.getter.List(ctx, pkg, filters) + versions, err := g.getter.List(ctx, pkg, filters, limit) if err != nil { return "" } diff --git a/pkg/versiongetter/fuzzy_getter_test.go b/pkg/versiongetter/fuzzy_getter_test.go index 4eb6f2080..cef351b0c 100644 --- a/pkg/versiongetter/fuzzy_getter_test.go +++ b/pkg/versiongetter/fuzzy_getter_test.go @@ -76,7 +76,7 @@ func TestFuzzyGetter_Get(t *testing.T) { //nolint:funlen finder := fuzzyfinder.NewMock(d.idxs, nil) vg := versiongetter.NewMockVersionGetter(d.versions) fg := versiongetter.NewFuzzy(finder, vg) - version := fg.Get(context.Background(), logE, d.pkg, d.currentVersion, d.useFinder) + version := fg.Get(context.Background(), logE, d.pkg, d.currentVersion, d.useFinder, -1) if version != d.version { t.Fatalf("wanted %s, got %s", d.version, version) } diff --git a/pkg/versiongetter/general.go b/pkg/versiongetter/general.go index 5d317021d..02bf800ef 100644 --- a/pkg/versiongetter/general.go +++ b/pkg/versiongetter/general.go @@ -7,6 +7,8 @@ import ( "github.com/aquaproj/aqua/v2/pkg/fuzzyfinder" ) +const ghMaxPerPage int = 100 + type GeneralVersionGetter struct { cargo *CargoVersionGetter ghTag *GitHubTagVersionGetter @@ -29,12 +31,12 @@ func (g *GeneralVersionGetter) Get(ctx context.Context, pkg *registry.PackageInf return getter.Get(ctx, pkg, filters) //nolint:wrapcheck } -func (g *GeneralVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) { +func (g *GeneralVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) { getter := g.get(pkg) if getter == nil { return nil, nil } - return getter.List(ctx, pkg, filters) //nolint:wrapcheck + return getter.List(ctx, pkg, filters, limit) //nolint:wrapcheck } func (g *GeneralVersionGetter) get(pkg *registry.PackageInfo) VersionGetter { @@ -52,3 +54,14 @@ func (g *GeneralVersionGetter) get(pkg *registry.PackageInfo) VersionGetter { } return g.ghRelease } + +// If filters exist, filter as much data as possible and +// per_page would be ghMaxPerPage. +// If there are no filters, set per_page to the limit +// to reduce the size of response data. +func itemNumPerPage(limit, filterNum int) int { + if limit > 0 && filterNum == 0 && ghMaxPerPage > limit { + return limit + } + return ghMaxPerPage +} diff --git a/pkg/versiongetter/github_release.go b/pkg/versiongetter/github_release.go index 6f2424db7..a0266fd27 100644 --- a/pkg/versiongetter/github_release.go +++ b/pkg/versiongetter/github_release.go @@ -63,16 +63,17 @@ func (g *GitHubReleaseVersionGetter) Get(ctx context.Context, pkg *registry.Pack } } -func (g *GitHubReleaseVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) { +func (g *GitHubReleaseVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) { repoOwner := pkg.RepoOwner repoName := pkg.RepoName opt := &github.ListOptions{ - PerPage: 30, //nolint:gomnd + PerPage: itemNumPerPage(limit, len(filters)), } + var items []*fuzzyfinder.Item tags := map[string]struct{}{} for { - releases, _, err := g.gh.ListReleases(ctx, repoOwner, repoName, opt) + releases, resp, err := g.gh.ListReleases(ctx, repoOwner, repoName, opt) if err != nil { return nil, fmt.Errorf("list tags: %w", err) } @@ -95,10 +96,13 @@ func (g *GitHubReleaseVersionGetter) List(ctx context.Context, pkg *registry.Pac }) } } - if len(releases) != opt.PerPage { + if limit > 0 && len(items) >= limit { // Reach the limit + return items[:limit], nil + } + if resp.NextPage == 0 { return items, nil } - opt.Page++ + opt.Page = resp.NextPage } } diff --git a/pkg/versiongetter/github_release_test.go b/pkg/versiongetter/github_release_test.go index 98c5372b5..6d79ebf1d 100644 --- a/pkg/versiongetter/github_release_test.go +++ b/pkg/versiongetter/github_release_test.go @@ -143,7 +143,7 @@ body(v1)`, t.Parallel() ghReleaseClient := versiongetter.NewMockGitHubReleaseClient(d.releases) ghReleaseGetter := versiongetter.NewGitHubRelease(ghReleaseClient) - items, err := ghReleaseGetter.List(ctx, d.pkg, d.filters) + items, err := ghReleaseGetter.List(ctx, d.pkg, d.filters, -1) if err != nil { if d.isErr { return diff --git a/pkg/versiongetter/github_tag.go b/pkg/versiongetter/github_tag.go index 4da67a3f8..11a8fdb94 100644 --- a/pkg/versiongetter/github_tag.go +++ b/pkg/versiongetter/github_tag.go @@ -46,16 +46,17 @@ func (g *GitHubTagVersionGetter) Get(ctx context.Context, pkg *registry.PackageI } } -func (g *GitHubTagVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) { +func (g *GitHubTagVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) { repoOwner := pkg.RepoOwner repoName := pkg.RepoName opt := &github.ListOptions{ - PerPage: 30, //nolint:gomnd + PerPage: itemNumPerPage(limit, len(filters)), } + var versions []string tagNames := map[string]struct{}{} for { - tags, _, err := g.gh.ListTags(ctx, repoOwner, repoName, opt) + tags, resp, err := g.gh.ListTags(ctx, repoOwner, repoName, opt) if err != nil { return nil, fmt.Errorf("list tags: %w", err) } @@ -69,10 +70,13 @@ func (g *GitHubTagVersionGetter) List(ctx context.Context, pkg *registry.Package versions = append(versions, tagName) } } - if len(tags) != opt.PerPage { + if limit > 0 && len(versions) >= limit { // Reach the limit + return fuzzyfinder.ConvertStringsToItems(versions[:limit]), nil + } + if resp.NextPage == 0 { return fuzzyfinder.ConvertStringsToItems(versions), nil } - opt.Page++ + opt.Page = resp.NextPage } } diff --git a/pkg/versiongetter/github_tag_test.go b/pkg/versiongetter/github_tag_test.go index 61c044fd4..dc797f7ce 100644 --- a/pkg/versiongetter/github_tag_test.go +++ b/pkg/versiongetter/github_tag_test.go @@ -125,7 +125,7 @@ func TestGitHubTagVersionGetter_List(t *testing.T) { //nolint:funlen t.Parallel() ghTagClient := versiongetter.NewMockGitHubTagClient(d.tags) ghTagGetter := versiongetter.NewGitHubTag(ghTagClient) - items, err := ghTagGetter.List(ctx, d.pkg, d.filters) + items, err := ghTagGetter.List(ctx, d.pkg, d.filters, -1) if err != nil { if d.isErr { return diff --git a/pkg/versiongetter/mock_fuzzy_getter.go b/pkg/versiongetter/mock_fuzzy_getter.go index d256bd35f..f1bfc1ae2 100644 --- a/pkg/versiongetter/mock_fuzzy_getter.go +++ b/pkg/versiongetter/mock_fuzzy_getter.go @@ -17,6 +17,6 @@ func NewMockFuzzyGetter(versions map[string]string) *MockFuzzyGetter { } } -func (g *MockFuzzyGetter) Get(ctx context.Context, _ *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool) string { +func (g *MockFuzzyGetter) Get(ctx context.Context, _ *logrus.Entry, pkg *registry.PackageInfo, currentVersion string, useFinder bool, limit int) string { return g.versions[pkg.GetName()] } diff --git a/pkg/versiongetter/mock_github_release.go b/pkg/versiongetter/mock_github_release.go index b3131a528..6502329b1 100644 --- a/pkg/versiongetter/mock_github_release.go +++ b/pkg/versiongetter/mock_github_release.go @@ -31,5 +31,6 @@ func (g *MockGitHubReleaseClient) ListReleases(ctx context.Context, owner, repo if !ok { return nil, nil, errors.New("repository isn't found") } - return releases, nil, nil + resp := &github.Response{} + return releases, resp, nil } diff --git a/pkg/versiongetter/mock_github_tag.go b/pkg/versiongetter/mock_github_tag.go index 23d33ed6a..d9b34bf79 100644 --- a/pkg/versiongetter/mock_github_tag.go +++ b/pkg/versiongetter/mock_github_tag.go @@ -27,5 +27,6 @@ func (g *MockGitHubTagClient) ListTags(ctx context.Context, owner string, repo s if m > len(tags) { m = len(tags) } - return tags[opts.Page*opts.PerPage : m], nil, nil + resp := &github.Response{} + return tags[opts.Page*opts.PerPage : m], resp, nil } diff --git a/pkg/versiongetter/mock_version_getter.go b/pkg/versiongetter/mock_version_getter.go index 59ce45acd..1feacd58d 100644 --- a/pkg/versiongetter/mock_version_getter.go +++ b/pkg/versiongetter/mock_version_getter.go @@ -26,7 +26,7 @@ func (g *MockVersionGetter) Get(ctx context.Context, pkg *registry.PackageInfo, return versions[0].Item, nil } -func (g *MockVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) { +func (g *MockVersionGetter) List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) { versions, ok := g.versions[pkg.GetName()] if !ok { return nil, errors.New("version isn't found") diff --git a/pkg/versiongetter/version_getter.go b/pkg/versiongetter/version_getter.go index 271fef4c7..6612c39c5 100644 --- a/pkg/versiongetter/version_getter.go +++ b/pkg/versiongetter/version_getter.go @@ -9,5 +9,5 @@ import ( type VersionGetter interface { Get(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) (string, error) - List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter) ([]*fuzzyfinder.Item, error) + List(ctx context.Context, pkg *registry.PackageInfo, filters []*Filter, limit int) ([]*fuzzyfinder.Item, error) }