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
aquaproj · Nov 11, 2023 · 7bf061d · 7bf061d
1 parent e055ee3
commit 7bf061d
Show file tree

Hide file tree

Showing 22 changed files with 87 additions and 30 deletions.
diff --git a/pkg/cli/generate.go b/pkg/cli/generate.go
@@ -86,8 +86,15 @@ echo "cli/cli" | aqua g -f -
 - name: cli/[email protected]
 
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 }