From 63b0fb3fc103697d23592bd891c9624feaf952d8 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Wed, 18 Dec 2019 09:32:48 +0100 Subject: [PATCH] * reverted choice of Option functor and added filters to Config * moved regexp-base filtering loop upwards (fail early) * added unit test for asset lookup with or without filters Signed-off-by: Frederic BIDON --- selfupdate/detect.go | 51 +++++---- selfupdate/detect_test.go | 219 ++++++++++++++++++++++++++++++++++++++ selfupdate/options.go | 29 ----- selfupdate/update.go | 16 +-- selfupdate/updater.go | 23 +++- 5 files changed, 274 insertions(+), 64 deletions(-) delete mode 100644 selfupdate/options.go diff --git a/selfupdate/detect.go b/selfupdate/detect.go index 3496e59..64e693a 100644 --- a/selfupdate/detect.go +++ b/selfupdate/detect.go @@ -13,11 +13,7 @@ import ( var reVersion = regexp.MustCompile(`\d+\.\d+\.\d+`) func findAssetFromRelease(rel *github.RepositoryRelease, - suffixes []string, targetVersion string, opts ...Option) (*github.ReleaseAsset, semver.Version, bool) { - settings := defaultSettings() - for _, apply := range opts { - apply(settings) - } + suffixes []string, targetVersion string, filters []*regexp.Regexp) (*github.ReleaseAsset, semver.Version, bool) { if targetVersion != "" && targetVersion != rel.GetTagName() { log.Println("Skip", rel.GetTagName(), "not matching to specified version", targetVersion) @@ -54,16 +50,24 @@ func findAssetFromRelease(rel *github.RepositoryRelease, for _, asset := range rel.Assets { name := asset.GetName() - for _, s := range suffixes { - if strings.HasSuffix(name, s) { // require version, arch etc - if len(settings.filters) > 0 { - for _, filter := range settings.filters { // extra condition to select among several release artifacts - if filter.MatchString(name) { - return &asset, ver, true - } - } + if len(filters) > 0 { + // if some filters are defined, match them: if any one matches, the asset is selected + matched := false + for _, filter := range filters { + if filter.MatchString(name) { + log.Println("Selected filtered asset", name) + matched = true break } + log.Printf("Skipping asset %q not matching filter %v", name, filter) + } + if !matched { + continue + } + } + + for _, s := range suffixes { + if strings.HasSuffix(name, s) { // require version, arch etc // default: assume single artifact return &asset, ver, true } @@ -84,7 +88,8 @@ func findValidationAsset(rel *github.RepositoryRelease, validationName string) ( } func findReleaseAndAsset(rels []*github.RepositoryRelease, - targetVersion string, opts ...Option) (*github.RepositoryRelease, *github.ReleaseAsset, semver.Version, bool) { + targetVersion string, + filters []*regexp.Regexp) (*github.RepositoryRelease, *github.ReleaseAsset, semver.Version, bool) { // Generate candidates suffixes := make([]string, 0, 2*7*2) for _, sep := range []rune{'_', '-'} { @@ -106,7 +111,7 @@ func findReleaseAndAsset(rels []*github.RepositoryRelease, // Returned list from GitHub API is in the order of the date when created. // ref: https://github.com/rhysd/go-github-selfupdate/issues/11 for _, rel := range rels { - if a, v, ok := findAssetFromRelease(rel, suffixes, targetVersion, opts...); ok { + if a, v, ok := findAssetFromRelease(rel, suffixes, targetVersion, filters); ok { // Note: any version with suffix is less than any version without suffix. // e.g. 0.0.1 > 0.0.1-beta if release == nil || v.GTE(ver) { @@ -131,13 +136,13 @@ func findReleaseAndAsset(rels []*github.RepositoryRelease, // where 'foo' is a command name. '-' can also be used as a separator. File can be compressed with zip, gzip, zxip, tar&zip or tar&zxip. // So the asset can have a file extension for the corresponding compression format such as '.zip'. // On Windows, '.exe' also can be contained such as 'foo_windows_amd64.exe.zip'. -func (up *Updater) DetectLatest(slug string, opts ...Option) (release *Release, found bool, err error) { - return up.DetectVersion(slug, "", opts...) +func (up *Updater) DetectLatest(slug string) (release *Release, found bool, err error) { + return up.DetectVersion(slug, "") } // DetectVersion tries to get the given version of the repository on Github. `slug` means `owner/name` formatted string. // And version indicates the required version. -func (up *Updater) DetectVersion(slug string, version string, opts ...Option) (release *Release, found bool, err error) { +func (up *Updater) DetectVersion(slug string, version string) (release *Release, found bool, err error) { repo := strings.Split(slug, "/") if len(repo) != 2 || repo[0] == "" || repo[1] == "" { return nil, false, fmt.Errorf("Invalid slug format. It should be 'owner/name': %s", slug) @@ -154,7 +159,7 @@ func (up *Updater) DetectVersion(slug string, version string, opts ...Option) (r return nil, false, err } - rel, asset, ver, found := findReleaseAndAsset(rels, version, opts...) + rel, asset, ver, found := findReleaseAndAsset(rels, version, up.filters) if !found { return nil, false, nil } @@ -191,11 +196,11 @@ func (up *Updater) DetectVersion(slug string, version string, opts ...Option) (r // DetectLatest detects the latest release of the slug (owner/repo). // This function is a shortcut version of updater.DetectLatest() method. -func DetectLatest(slug string, opts ...Option) (*Release, bool, error) { - return DefaultUpdater().DetectLatest(slug, opts...) +func DetectLatest(slug string) (*Release, bool, error) { + return DefaultUpdater().DetectLatest(slug) } // DetectVersion detects the given release of the slug (owner/repo) from its version. -func DetectVersion(slug string, version string, opts ...Option) (*Release, bool, error) { - return DefaultUpdater().DetectVersion(slug, version, opts...) +func DetectVersion(slug string, version string) (*Release, bool, error) { + return DefaultUpdater().DetectVersion(slug, version) } diff --git a/selfupdate/detect_test.go b/selfupdate/detect_test.go index 6f1a8f2..f4487a9 100644 --- a/selfupdate/detect_test.go +++ b/selfupdate/detect_test.go @@ -3,10 +3,12 @@ package selfupdate import ( "fmt" "os" + "regexp" "strings" "testing" "github.com/blang/semver" + "github.com/google/go-github/github" ) func TestDetectReleaseWithVersionPrefix(t *testing.T) { @@ -240,3 +242,220 @@ func TestDetectFromGitHubEnterpriseRepo(t *testing.T) { t.Error("") } } + +func TestFindReleaseAndAsset(t *testing.T) { + EnableLog() + type findReleaseAndAssetFixture struct { + name string + rels *github.RepositoryRelease + targetVersion string + filters []*regexp.Regexp + expectedAsset string + expectedVersion string + expectedFound bool + } + + rel1 := "rel1" + v1 := "1.0.0" + rel11 := "rel11" + v11 := "1.1.0" + asset1 := "asset1.gz" + asset2 := "asset2.gz" + wrongAsset1 := "asset1.yaml" + asset11 := "asset11.gz" + url1 := "https://asset1" + url2 := "https://asset2" + url11 := "https://asset11" + for _, fixture := range []findReleaseAndAssetFixture{ + { + name: "empty fixture", + rels: nil, + targetVersion: "", + filters: nil, + expectedFound: false, + }, + { + name: "find asset, no filters", + rels: &github.RepositoryRelease{ + Name: &rel1, + TagName: &v1, + Assets: []github.ReleaseAsset{ + { + Name: &asset1, + URL: &url1, + }, + }, + }, + targetVersion: "1.0.0", + expectedAsset: asset1, + expectedVersion: "1.0.0", + expectedFound: true, + }, + { + name: "don't find asset with wrong extension, no filters", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &wrongAsset1, + URL: &url11, + }, + }, + }, + targetVersion: "1.1.0", + expectedFound: false, + }, + { + name: "find asset with different name, no filters", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset1, + URL: &url11, + }, + }, + }, + targetVersion: "1.1.0", + expectedAsset: asset1, + expectedVersion: "1.1.0", + expectedFound: true, + }, + { + name: "find asset, no filters", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset11, + URL: &url11, + }, + }, + }, + targetVersion: "1.1.0", + expectedAsset: asset11, + expectedVersion: "1.1.0", + filters: nil, + expectedFound: true, + }, + { + name: "find asset, match filter", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset11, + URL: &url11, + }, + { + Name: &asset1, + URL: &url1, + }, + }, + }, + targetVersion: "1.1.0", + filters: []*regexp.Regexp{regexp.MustCompile("11")}, + expectedAsset: asset11, + expectedVersion: "1.1.0", + expectedFound: true, + }, + { + name: "find asset, match another filter", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset11, + URL: &url11, + }, + { + Name: &asset1, + URL: &url1, + }, + }, + }, + targetVersion: "1.1.0", + filters: []*regexp.Regexp{regexp.MustCompile("([^1])1{1}([^1])")}, + expectedAsset: asset1, + expectedVersion: "1.1.0", + expectedFound: true, + }, + { + name: "find asset, match any filter", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset11, + URL: &url11, + }, + { + Name: &asset2, + URL: &url2, + }, + }, + }, + targetVersion: "1.1.0", + filters: []*regexp.Regexp{ + regexp.MustCompile("([^1])1{1}([^1])"), + regexp.MustCompile("([^1])2{1}([^1])"), + }, + expectedAsset: asset2, + expectedVersion: "1.1.0", + expectedFound: true, + }, + { + name: "find asset, match no filter", + rels: &github.RepositoryRelease{ + Name: &rel11, + TagName: &v11, + Assets: []github.ReleaseAsset{ + { + Name: &asset11, + URL: &url11, + }, + { + Name: &asset2, + URL: &url2, + }, + }, + }, + targetVersion: "1.1.0", + filters: []*regexp.Regexp{ + regexp.MustCompile("another"), + regexp.MustCompile("binary"), + }, + expectedFound: false, + }, + } { + asset, ver, found := findAssetFromRelease(fixture.rels, []string{".gz"}, fixture.targetVersion, fixture.filters) + if fixture.expectedFound { + if !found { + t.Errorf("expected to find an asset for this fixture: %q", fixture.name) + continue + } + if asset.Name == nil { + t.Errorf("invalid asset struct returned from fixture: %q, got: %v", fixture.name, asset) + continue + } + if *asset.Name != fixture.expectedAsset { + t.Errorf("expected asset %q in fixture: %q, got: %s", fixture.expectedAsset, fixture.name, *asset.Name) + continue + } + t.Logf("asset %v, %v", asset, ver) + } else { + if found { + t.Errorf("expected not to find an asset for this fixture: %q, but got: %v", fixture.name, asset) + } + } + } + +} + +//(*github.RepositoryRelease, *github.ReleaseAsset, semver.Version, bool) { diff --git a/selfupdate/options.go b/selfupdate/options.go deleted file mode 100644 index ab85f4d..0000000 --- a/selfupdate/options.go +++ /dev/null @@ -1,29 +0,0 @@ -package selfupdate - -import ( - "fmt" - "regexp" -) - -type settings struct { - filters []*regexp.Regexp -} - -// Option defines an optional feature for the self updater -type Option func(*settings) - -func defaultSettings() *settings { - return &settings{} -} - -// AssetFilter sets a filter to select the proper released asset -// from releases with multiple artifacts. The filter is regexp string. -func AssetFilter(filter string) Option { - return func(s *settings) { - rex, err := regexp.Compile(filter) - if err != nil { - panic(fmt.Sprintf("invalid regexp passed as option: %v", err)) - } - s.filters = append(s.filters, rex) - } -} diff --git a/selfupdate/update.go b/selfupdate/update.go index 3f21fbf..705afc8 100644 --- a/selfupdate/update.go +++ b/selfupdate/update.go @@ -106,7 +106,7 @@ func (up *Updater) UpdateTo(rel *Release, cmdPath string) error { // UpdateCommand updates a given command binary to the latest version. // 'slug' represents 'owner/name' repository on GitHub and 'current' means the current version. -func (up *Updater) UpdateCommand(cmdPath string, current semver.Version, slug string, opts ...Option) (*Release, error) { +func (up *Updater) UpdateCommand(cmdPath string, current semver.Version, slug string) (*Release, error) { if runtime.GOOS == "windows" && !strings.HasSuffix(cmdPath, ".exe") { // Ensure to add '.exe' to given path on Windows cmdPath = cmdPath + ".exe" @@ -124,7 +124,7 @@ func (up *Updater) UpdateCommand(cmdPath string, current semver.Version, slug st cmdPath = p } - rel, ok, err := up.DetectLatest(slug, opts...) + rel, ok, err := up.DetectLatest(slug) if err != nil { return nil, err } @@ -145,12 +145,12 @@ func (up *Updater) UpdateCommand(cmdPath string, current semver.Version, slug st // UpdateSelf updates the running executable itself to the latest version. // 'slug' represents 'owner/name' repository on GitHub and 'current' means the current version. -func (up *Updater) UpdateSelf(current semver.Version, slug string, opts ...Option) (*Release, error) { +func (up *Updater) UpdateSelf(current semver.Version, slug string) (*Release, error) { cmdPath, err := os.Executable() if err != nil { return nil, err } - return up.UpdateCommand(cmdPath, current, slug, opts...) + return up.UpdateCommand(cmdPath, current, slug) } // UpdateTo downloads an executable from assetURL and replace the current binary with the downloaded one. @@ -169,12 +169,12 @@ func UpdateTo(assetURL, cmdPath string) error { // UpdateCommand updates a given command binary to the latest version. // This function is a shortcut version of updater.UpdateCommand. -func UpdateCommand(cmdPath string, current semver.Version, slug string, opts ...Option) (*Release, error) { - return DefaultUpdater().UpdateCommand(cmdPath, current, slug, opts...) +func UpdateCommand(cmdPath string, current semver.Version, slug string) (*Release, error) { + return DefaultUpdater().UpdateCommand(cmdPath, current, slug) } // UpdateSelf updates the running executable itself to the latest version. // This function is a shortcut version of updater.UpdateSelf. -func UpdateSelf(current semver.Version, slug string, opts ...Option) (*Release, error) { - return DefaultUpdater().UpdateSelf(current, slug, opts...) +func UpdateSelf(current semver.Version, slug string) (*Release, error) { + return DefaultUpdater().UpdateSelf(current, slug) } diff --git a/selfupdate/updater.go b/selfupdate/updater.go index 4239233..9eba202 100644 --- a/selfupdate/updater.go +++ b/selfupdate/updater.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "os" + "regexp" "github.com/google/go-github/github" gitconfig "github.com/tcnksm/go-gitconfig" @@ -13,9 +14,10 @@ import ( // Updater is responsible for managing the context of self-update. // It contains GitHub client and its context. type Updater struct { - api *github.Client - apiCtx context.Context + api *github.Client + apiCtx context.Context validator Validator + filters []*regexp.Regexp } // Config represents the configuration of self-update. @@ -30,6 +32,10 @@ type Config struct { EnterpriseUploadURL string // Validator represents types which enable additional validation of downloaded release. Validator Validator + // Filters are regexp used to filter on specific assets for releases with multiple assets. + // An asset is selected if it matches any of those, in addition to the regular tag, os, arch, extensions. + // Please make sure that your filter(s) uniquely match an asset. + Filters []string } func newHTTPClient(ctx context.Context, token string) *http.Client { @@ -53,9 +59,18 @@ func NewUpdater(config Config) (*Updater, error) { ctx := context.Background() hc := newHTTPClient(ctx, token) + filtersRe := make([]*regexp.Regexp, 0, len(config.Filters)) + for _, filter := range config.Filters { + re, erx := regexp.Compile(filter) + if erx != nil { + return nil, erx + } + filtersRe = append(filtersRe, re) + } + if config.EnterpriseBaseURL == "" { client := github.NewClient(hc) - return &Updater{client, ctx, config.Validator}, nil + return &Updater{api: client, apiCtx: ctx, validator: config.Validator, filters: filtersRe}, nil } u := config.EnterpriseUploadURL @@ -66,7 +81,7 @@ func NewUpdater(config Config) (*Updater, error) { if err != nil { return nil, err } - return &Updater{api: client, apiCtx: ctx, validator: config.Validator}, nil + return &Updater{api: client, apiCtx: ctx, validator: config.Validator, filters: filtersRe}, nil } // DefaultUpdater creates a new updater instance with default configuration.