Refactoring (aquaproj#2147)

suzuki-shunsuke · web-flow · commit 742dc7153e7f · 2023-07-31T08:01:38.000+09:00
* refactor: move error to util

* fix: use the basename of the package name as the default command name

* refactor: early return

* refactor: merge versionSelector

* refactor: rewrite if statements with switch

* refactor: move Copy to util
diff --git a/pkg/cargo/error.go b/pkg/cargo/error.go
diff --git a/pkg/cargo/versions.go b/pkg/cargo/versions.go
@@ -5,6 +5,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+
+	"github.com/aquaproj/aqua/v2/pkg/util"
 )
 
 type Payload struct {
@@ -91,7 +93,7 @@ func getCrate(ctx context.Context, client *http.Client, uri string) (*CratePaylo
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode >= 300 { //nolint:gomnd
-		return nil, resp.StatusCode, errHTTPStatusCodeIsGreaterEqualThan300
+		return nil, resp.StatusCode, util.ErrHTTPStatusCodeIsGreaterEqualThan300
 	}
 	payload := &CratePayload{}
 	if err := json.NewDecoder(resp.Body).Decode(payload); err != nil {
@@ -111,7 +113,7 @@ func listInstallableVersions(ctx context.Context, client *http.Client, uri strin
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode >= 300 { //nolint:gomnd
-		return nil, resp.StatusCode, errHTTPStatusCodeIsGreaterEqualThan300
+		return nil, resp.StatusCode, util.ErrHTTPStatusCodeIsGreaterEqualThan300
 	}
 	payload := &Payload{}
 	if err := json.NewDecoder(resp.Body).Decode(payload); err != nil {
diff --git a/pkg/config/registry/package_info.go b/pkg/config/registry/package_info.go
@@ -573,7 +573,7 @@ func (pkgInfo *PackageInfo) getDefaultCmdName() string {
 		}
 		return path.Base(pkgInfo.GetPath())
 	}
-	return ""
+	return path.Base(pkgInfo.GetName())
 }
 
 func (pkgInfo *PackageInfo) SLSASourceURI() string {
diff --git a/pkg/controller/exec/exec.go b/pkg/controller/exec/exec.go
@@ -87,26 +87,28 @@ func (ctrl *Controller) Exec(ctx context.Context, logE *logrus.Entry, param *con
 	if err != nil {
 		return err //nolint:wrapcheck
 	}
-	if findResult.Package != nil { //nolint:nestif
-		logE = logE.WithFields(logrus.Fields{
-			"package":         findResult.Package.Package.Name,
-			"package_version": findResult.Package.Package.Version,
-		})
+	if findResult.Package == nil {
+		return ctrl.execCommandWithRetry(ctx, logE, findResult.ExePath, args...)
+	}
 
-		policyCfgs, err := ctrl.policyConfigReader.Append(logE, findResult.ConfigFilePath, policyCfgs, globalPolicyPaths)
-		if err != nil {
-			return err //nolint:wrapcheck
-		}
+	logE = logE.WithFields(logrus.Fields{
+		"package":         findResult.Package.Package.Name,
+		"package_version": findResult.Package.Package.Version,
+	})
 
-		if param.DisableLazyInstall {
-			if _, err := ctrl.fs.Stat(findResult.ExePath); err != nil {
-				return logerr.WithFields(errExecNotFoundDisableLazyInstall, logE.WithField("doc", "https://aquaproj.github.io/docs/reference/codes/006").Data) //nolint:wrapcheck
-			}
-		}
-		if err := ctrl.install(ctx, logE, findResult, policyCfgs); err != nil {
-			return err
+	policyCfgs, err = ctrl.policyConfigReader.Append(logE, findResult.ConfigFilePath, policyCfgs, globalPolicyPaths)
+	if err != nil {
+		return err //nolint:wrapcheck
+	}
+
+	if param.DisableLazyInstall {
+		if _, err := ctrl.fs.Stat(findResult.ExePath); err != nil {
+			return logerr.WithFields(errExecNotFoundDisableLazyInstall, logE.WithField("doc", "https://aquaproj.github.io/docs/reference/codes/006").Data) //nolint:wrapcheck
 		}
 	}
+	if err := ctrl.install(ctx, logE, findResult, policyCfgs); err != nil {
+		return err
+	}
 	return ctrl.execCommandWithRetry(ctx, logE, findResult.ExePath, args...)
 }
 
diff --git a/pkg/controller/generate/cargo.go b/pkg/controller/generate/cargo.go
@@ -4,23 +4,24 @@ import (
 	"context"
 
 	"github.com/aquaproj/aqua/v2/pkg/config"
-	"github.com/ktr0731/go-fuzzyfinder"
 	"github.com/sirupsen/logrus"
 )
 
 func (ctrl *Controller) getCargoVersion(ctx context.Context, logE *logrus.Entry, param *config.Param, pkg *FindingPackage) string {
 	pkgInfo := pkg.PackageInfo
 	if param.SelectVersion {
-		versions, err := ctrl.cargoClient.ListVersions(ctx, *pkgInfo.Crate)
+		versionStrings, err := ctrl.cargoClient.ListVersions(ctx, *pkgInfo.Crate)
 		if err != nil {
 			logE.WithError(err).Warn("list versions")
 			return ""
 		}
-		idx, err := ctrl.crateVersionSelector.Find(versions)
+		versions := convertStringsToVersions(versionStrings)
+
+		idx, err := ctrl.versionSelector.Find(versions, false)
 		if err != nil {
 			return ""
 		}
-		return versions[idx]
+		return versions[idx].Version
 	}
 	version, err := ctrl.cargoClient.GetLatestVersion(ctx, *pkgInfo.Crate)
 	if err != nil {
@@ -29,28 +30,3 @@ func (ctrl *Controller) getCargoVersion(ctx context.Context, logE *logrus.Entry,
 	}
 	return version
 }
-
-type CrateVersionSelector interface {
-	Find(versions []string) (int, error)
-}
-
-type MockCrateVersionSelector struct {
-	Index int
-	Err   error
-}
-
-func (mock *MockCrateVersionSelector) Find(versions []string) (int, error) {
-	return mock.Index, mock.Err
-}
-
-type CrateVersionSelectorImpl struct{}
-
-func NewCrateVersionSelectorImpl() *CrateVersionSelectorImpl {
-	return &CrateVersionSelectorImpl{}
-}
-
-func (selector *CrateVersionSelectorImpl) Find(versions []string) (int, error) {
-	return fuzzyfinder.Find(versions, func(i int) string { //nolint:wrapcheck
-		return versions[i]
-	})
-}
diff --git a/pkg/controller/generate/generate.go b/pkg/controller/generate/generate.go
@@ -23,32 +23,30 @@ import (
 )
 
 type Controller struct {
-	stdin                io.Reader
-	github               RepositoriesService
-	registryInstaller    rgst.Installer
-	configFinder         ConfigFinder
-	configReader         reader.ConfigReader
-	fuzzyFinder          FuzzyFinder
-	versionSelector      VersionSelector
-	fs                   afero.Fs
-	outputter            Outputter
-	cargoClient          cargo.Client
-	crateVersionSelector CrateVersionSelector
+	stdin             io.Reader
+	github            RepositoriesService
+	registryInstaller rgst.Installer
+	configFinder      ConfigFinder
+	configReader      reader.ConfigReader
+	fuzzyFinder       FuzzyFinder
+	versionSelector   VersionSelector
+	fs                afero.Fs
+	outputter         Outputter
+	cargoClient       cargo.Client
 }
 
-func New(configFinder ConfigFinder, configReader reader.ConfigReader, registInstaller rgst.Installer, gh RepositoriesService, fs afero.Fs, fuzzyFinder FuzzyFinder, versionSelector VersionSelector, cargoClient cargo.Client, crateVersionSelector CrateVersionSelector) *Controller {
+func New(configFinder ConfigFinder, configReader reader.ConfigReader, registInstaller rgst.Installer, gh RepositoriesService, fs afero.Fs, fuzzyFinder FuzzyFinder, versionSelector VersionSelector, cargoClient cargo.Client) *Controller {
 	return &Controller{
-		stdin:                os.Stdin,
-		configFinder:         configFinder,
-		configReader:         configReader,
-		registryInstaller:    registInstaller,
-		github:               gh,
-		fs:                   fs,
-		fuzzyFinder:          fuzzyFinder,
-		versionSelector:      versionSelector,
-		cargoClient:          cargoClient,
-		crateVersionSelector: crateVersionSelector,
-		outputter:            output.New(os.Stdout, fs),
+		stdin:             os.Stdin,
+		configFinder:      configFinder,
+		configReader:      configReader,
+		registryInstaller: registInstaller,
+		github:            gh,
+		fs:                fs,
+		fuzzyFinder:       fuzzyFinder,
+		versionSelector:   versionSelector,
+		cargoClient:       cargoClient,
+		outputter:         output.New(os.Stdout, fs),
 	}
 }
 
diff --git a/pkg/controller/generate/generate_test.go b/pkg/controller/generate/generate_test.go
@@ -405,9 +405,8 @@ packages:
 			configReader := reader.New(fs, d.param)
 			fuzzyFinder := generate.NewMockFuzzyFinder(d.idxs, d.fuzzyFinderErr)
 			versionSelector := generate.NewMockVersionSelector(d.idx, d.versionSelectorErr)
-			crateVersionSelector := &generate.MockCrateVersionSelector{}
 			cargoClient := &cargo.MockClient{}
-			ctrl := generate.New(configFinder, configReader, registryInstaller, gh, fs, fuzzyFinder, versionSelector, cargoClient, crateVersionSelector)
+			ctrl := generate.New(configFinder, configReader, registryInstaller, gh, fs, fuzzyFinder, versionSelector, cargoClient)
 			if err := ctrl.Generate(ctx, logE, d.param, d.args...); err != nil {
 				if d.isErr {
 					return
diff --git a/pkg/controller/generate/github_release.go b/pkg/controller/generate/github_release.go
@@ -23,7 +23,7 @@ func (ctrl *Controller) selectVersionFromReleases(ctx context.Context, logE *log
 			URL:         release.GetHTMLURL(),
 		}
 	}
-	idx, err := ctrl.versionSelector.Find(versions)
+	idx, err := ctrl.versionSelector.Find(versions, true)
 	if err != nil {
 		return ""
 	}
diff --git a/pkg/controller/generate/github_tag.go b/pkg/controller/generate/github_tag.go
@@ -97,11 +97,10 @@ func (ctrl *Controller) selectVersionFromGitHubTag(ctx context.Context, logE *lo
 	versions := make([]*Version, len(tags))
 	for i, tag := range tags {
 		versions[i] = &Version{
-			Name:    tag.GetName(),
 			Version: tag.GetName(),
 		}
 	}
-	idx, err := ctrl.versionSelector.Find(versions)
+	idx, err := ctrl.versionSelector.Find(versions, false)
 	if err != nil {
 		return ""
 	}
diff --git a/pkg/controller/generate/version_selector.go b/pkg/controller/generate/version_selector.go
@@ -14,7 +14,7 @@ type Version struct {
 }
 
 type VersionSelector interface {
-	Find(versions []*Version) (int, error)
+	Find(versions []*Version, hasPreview bool) (int, error)
 }
 
 type versionSelector struct{}
@@ -35,20 +35,25 @@ type mockVersionSelector struct {
 	err error
 }
 
-func (selector *mockVersionSelector) Find(versions []*Version) (int, error) {
+func (selector *mockVersionSelector) Find(versions []*Version, hasPreview bool) (int, error) {
 	return selector.idx, selector.err
 }
 
-func (selector *versionSelector) Find(versions []*Version) (int, error) {
+func (selector *versionSelector) Find(versions []*Version, hasPreview bool) (int, error) {
+	if hasPreview {
+		return fuzzyfinder.Find(versions, func(i int) string { //nolint:wrapcheck
+			return getVersionItem(versions[i])
+		},
+			fuzzyfinder.WithPreviewWindow(func(i, w, h int) string {
+				if i < 0 {
+					return "No version matches"
+				}
+				return getVersionPreview(versions[i], i, w)
+			}))
+	}
 	return fuzzyfinder.Find(versions, func(i int) string { //nolint:wrapcheck
 		return getVersionItem(versions[i])
-	},
-		fuzzyfinder.WithPreviewWindow(func(i, w, h int) string {
-			if i < 0 {
-				return "No package matches"
-			}
-			return getVersionPreview(versions[i], i, w)
-		}))
+	})
 }
 
 func getVersionItem(version *Version) string {
@@ -60,7 +65,7 @@ func getVersionPreview(version *Version, i, w int) string {
 		return ""
 	}
 	s := version.Version
-	if version.Name != version.Version {
+	if version.Name != "" && version.Name != version.Version {
 		s += fmt.Sprintf(" (%s)", version.Name)
 	}
 	if version.URL != "" || version.Description != "" {
@@ -74,3 +79,13 @@ func getVersionPreview(version *Version, i, w int) string {
 	}
 	return s
 }
+
+func convertStringsToVersions(arr []string) []*Version {
+	versions := make([]*Version, len(arr))
+	for i, a := range arr {
+		versions[i] = &Version{
+			Version: a,
+		}
+	}
+	return versions
+}
diff --git a/pkg/controller/updatechecksum/update.go b/pkg/controller/updatechecksum/update.go
@@ -178,12 +178,9 @@ func (ctrl *Controller) updatePackage(ctx context.Context, logE *logrus.Entry, c
 }
 
 func (ctrl *Controller) getChecksums(ctx context.Context, logE *logrus.Entry, checksums *checksum.Checksums, pkg *config.Package, supportedEnvs []string) error {
-	if pkg.PackageInfo.Type == "go_install" {
-		logE.Debug("skip updating go_install package's checksum")
-		return nil
-	}
-	if pkg.PackageInfo.Type == "cargo" {
-		logE.Debug("skip updating cargo package's checksum")
+	switch pkg.PackageInfo.Type {
+	case "go_install", "cargo":
+		logE.WithField("package_type", pkg.PackageInfo.Type).Debug("skip updating the package's checksum")
 		return nil
 	}
 	logE.Info("updating a package checksum")
diff --git a/pkg/controller/wire.go b/pkg/controller/wire.go
@@ -188,10 +188,6 @@ func InitializeGenerateCommandController(ctx context.Context, param *config.Para
 			slsa.NewExecutor,
 			wire.Bind(new(slsa.Executor), new(*slsa.ExecutorImpl)),
 		),
-		wire.NewSet(
-			generate.NewCrateVersionSelectorImpl,
-			wire.Bind(new(generate.CrateVersionSelector), new(*generate.CrateVersionSelectorImpl)),
-		),
 		wire.NewSet(
 			cargo.NewClientImpl,
 			wire.Bind(new(cargo.Client), new(*cargo.ClientImpl)),
diff --git a/pkg/controller/wire_gen.go b/pkg/controller/wire_gen.go
diff --git a/pkg/unarchive/dmg.go b/pkg/unarchive/dmg.go
@@ -53,7 +53,7 @@ func (unarchiver *dmgUnarchiver) Unarchive(ctx context.Context, logE *logrus.Ent
 		}
 	}()
 
-	if err := cpDirWrap(unarchiver.fs, tmpMountPoint, unarchiver.dest); err != nil {
+	if err := util.Copy(unarchiver.fs, tmpMountPoint, unarchiver.dest); err != nil {
 		return fmt.Errorf("copy a directory: %w", err)
 	}
 
diff --git a/pkg/util/copy.go b/pkg/util/copy.go
diff --git a/pkg/util/error.go b/pkg/util/error.go

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@ import (`
`5`	`5`	`"encoding/json"`
`6`	`6`	`"fmt"`
`7`	`7`	`"net/http"`
	`8`	`+`
	`9`	`+ "github.com/aquaproj/aqua/v2/pkg/util"`
`8`	`10`	`)`
`9`	`11`
`10`	`12`	`type Payload struct {`
`@@ -91,7 +93,7 @@ func getCrate(ctx context.Context, client http.Client, uri string) (CratePaylo`
`91`	`93`	`}`
`92`	`94`	`defer resp.Body.Close()`
`93`	`95`	`if resp.StatusCode >= 300 { //nolint:gomnd`
`94`		`- return nil, resp.StatusCode, errHTTPStatusCodeIsGreaterEqualThan300`
	`96`	`+ return nil, resp.StatusCode, util.ErrHTTPStatusCodeIsGreaterEqualThan300`
`95`	`97`	`}`
`96`	`98`	`payload := &CratePayload{}`
`97`	`99`	`if err := json.NewDecoder(resp.Body).Decode(payload); err != nil {`
`@@ -111,7 +113,7 @@ func listInstallableVersions(ctx context.Context, client *http.Client, uri strin`
`111`	`113`	`}`
`112`	`114`	`defer resp.Body.Close()`
`113`	`115`	`if resp.StatusCode >= 300 { //nolint:gomnd`
`114`		`- return nil, resp.StatusCode, errHTTPStatusCodeIsGreaterEqualThan300`
	`116`	`+ return nil, resp.StatusCode, util.ErrHTTPStatusCodeIsGreaterEqualThan300`
`115`	`117`	`}`
`116`	`118`	`payload := &Payload{}`
`117`	`119`	`if err := json.NewDecoder(resp.Body).Decode(payload); err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -573,7 +573,7 @@ func (pkgInfo *PackageInfo) getDefaultCmdName() string {`
`573`	`573`	`}`
`574`	`574`	`return path.Base(pkgInfo.GetPath())`
`575`	`575`	`}`
`576`		`- return ""`
	`576`	`+ return path.Base(pkgInfo.GetName())`
`577`	`577`	`}`
`578`	`578`
`579`	`579`	`func (pkgInfo *PackageInfo) SLSASourceURI() string {`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ func (ctrl Controller) selectVersionFromReleases(ctx context.Context, logE log`
`23`	`23`	`URL: release.GetHTMLURL(),`
`24`	`24`	`}`
`25`	`25`	`}`
`26`		`- idx, err := ctrl.versionSelector.Find(versions)`
	`26`	`+ idx, err := ctrl.versionSelector.Find(versions, true)`
`27`	`27`	`if err != nil {`
`28`	`28`	`return ""`
`29`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -97,11 +97,10 @@ func (ctrl Controller) selectVersionFromGitHubTag(ctx context.Context, logE lo`
`97`	`97`	`versions := make([]*Version, len(tags))`
`98`	`98`	`for i, tag := range tags {`
`99`	`99`	`versions[i] = &Version{`
`100`		`- Name: tag.GetName(),`
`101`	`100`	`Version: tag.GetName(),`
`102`	`101`	`}`
`103`	`102`	`}`
`104`		`- idx, err := ctrl.versionSelector.Find(versions)`
	`103`	`+ idx, err := ctrl.versionSelector.Find(versions, false)`
`105`	`104`	`if err != nil {`
`106`	`105`	`return ""`
`107`	`106`	`}`