From 1bdc1578ddcdb831aaac5b71d0e08f52af36a993 Mon Sep 17 00:00:00 2001 From: Integralist Date: Fri, 29 Sep 2023 17:43:09 +0100 Subject: [PATCH] feat(spinner): abstract common pattern --- pkg/commands/compute/build.go | 147 +++++----------- pkg/commands/compute/deploy.go | 72 +++----- pkg/commands/compute/init.go | 301 +++++++++++---------------------- pkg/commands/compute/pack.go | 131 +++++--------- pkg/commands/compute/update.go | 46 ++--- pkg/commands/profile/create.go | 136 ++++++--------- pkg/commands/profile/update.go | 87 ++++------ pkg/commands/update/root.go | 174 ++++++++----------- pkg/text/spinner.go | 34 +++- 9 files changed, 410 insertions(+), 718 deletions(-) diff --git a/pkg/commands/compute/build.go b/pkg/commands/compute/build.go index 5f89b1f27..e9499a703 100644 --- a/pkg/commands/compute/build.go +++ b/pkg/commands/compute/build.go @@ -83,77 +83,41 @@ func (c *BuildCommand) Exec(in io.Reader, out io.Writer) (err error) { } }(c.Globals.ErrLog) - err = spinner.Start() - if err != nil { - return err - } - msg := "Verifying fastly.toml" - spinner.Message(msg + "...") - - err = c.Manifest.File.ReadError() - if err != nil { - if errors.Is(err, os.ErrNotExist) { - err = fsterr.ErrReadingManifest - } - c.Globals.ErrLog.Add(err) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + err = spinner.Process("Verifying fastly.toml", func(_ *text.SpinnerWrapper) error { + err = c.Manifest.File.ReadError() + if err != nil { + if errors.Is(err, os.ErrNotExist) { + err = fsterr.ErrReadingManifest + } + c.Globals.ErrLog.Add(err) + return err } - - return err - } - - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return err - } - - err = spinner.Start() + return nil + }) if err != nil { return err } - msg = "Identifying package name" - spinner.Message(msg + "...") - packageName, err := packageName(c) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + var pkgName string + err = spinner.Process("Identifying package name", func(_ *text.SpinnerWrapper) error { + pkgName, err = packageName(c) + if err != nil { + return err } - return err - } - - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return err - } - - err = spinner.Start() + return nil + }) if err != nil { return err } - msg = "Identifying toolchain" - spinner.Message(msg + "...") - toolchain, err := toolchain(c) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + var toolchain string + err = spinner.Process("Identifying toolchain", func(_ *text.SpinnerWrapper) error { + toolchain, err = identifyToolchain(c) + if err != nil { + return err } - return err - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return err } @@ -175,49 +139,28 @@ func (c *BuildCommand) Exec(in io.Reader, out io.Writer) (err error) { return err } - err = spinner.Start() - if err != nil { - return err - } - msg = "Creating package archive" - spinner.Message(msg + "...") - - dest := filepath.Join("pkg", fmt.Sprintf("%s.tar.gz", packageName)) - - // NOTE: The minimum package requirement is `fastly.toml` and `main.wasm`. - files := []string{ - manifest.Filename, - "bin/main.wasm", - } + dest := filepath.Join("pkg", fmt.Sprintf("%s.tar.gz", pkgName)) - files, err = c.includeSourceCode(files, language.SourceDirectory) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + err = spinner.Process("Creating package archive", func(_ *text.SpinnerWrapper) error { + // NOTE: The minimum package requirement is `fastly.toml` and `main.wasm`. + files := []string{ + manifest.Filename, + "bin/main.wasm", } - return err - } - - err = CreatePackageArchive(files, dest) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Files": files, - "Destination": dest, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + files, err = c.includeSourceCode(files, language.SourceDirectory) + if err != nil { + return err } - - return fmt.Errorf("error creating package archive: %w", err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() + err = CreatePackageArchive(files, dest) + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Files": files, + "Destination": dest, + }) + return fmt.Errorf("error creating package archive: %w", err) + } + return nil + }) if err != nil { return err } @@ -288,12 +231,12 @@ func packageName(c *BuildCommand) (string, error) { return sanitize.BaseName(name), nil } -// toolchain determines the programming language. +// identifyToolchain determines the programming language. // // It prioritises the --language flag over the manifest field. // Will error if neither are provided. // Lastly, it will normalise with a trim and lowercase. -func toolchain(c *BuildCommand) (string, error) { +func identifyToolchain(c *BuildCommand) (string, error) { var toolchain string switch { diff --git a/pkg/commands/compute/deploy.go b/pkg/commands/compute/deploy.go index 1b5a9fee6..54e8c4d78 100644 --- a/pkg/commands/compute/deploy.go +++ b/pkg/commands/compute/deploy.go @@ -803,30 +803,17 @@ func pkgCompare(client api.Interface, serviceID string, version int, filesHash s // pkgUpload uploads the package to the specified service and version. func pkgUpload(spinner text.Spinner, client api.Interface, serviceID string, version int, path string) error { - err := spinner.Start() - if err != nil { - return err - } - msg := "Uploading package" - spinner.Message(msg + "...") - - _, err = client.UpdatePackage(&fastly.UpdatePackageInput{ - ServiceID: serviceID, - ServiceVersion: version, - PackagePath: path, - }) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + return spinner.Process("Uploading package", func(_ *text.SpinnerWrapper) error { + _, err := client.UpdatePackage(&fastly.UpdatePackageInput{ + ServiceID: serviceID, + ServiceVersion: version, + PackagePath: path, + }) + if err != nil { + return fmt.Errorf("error uploading package: %w", err) } - - return fmt.Errorf("error uploading package: %w", err) - } - - spinner.StopMessage(msg) - return spinner.Stop() + return nil + }) } // setupObjects is a collection of backend objects created during setup. @@ -1162,33 +1149,20 @@ func processService(c *DeployCommand, serviceID string, serviceVersion int, spin } } - err := spinner.Start() - if err != nil { - return err - } - msg := fmt.Sprintf("Activating service (version %d)", serviceVersion) - spinner.Message(msg + "...") - - _, err = c.Globals.APIClient.ActivateVersion(&fastly.ActivateVersionInput{ - ServiceID: serviceID, - ServiceVersion: serviceVersion, - }) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr - } - - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Service ID": serviceID, - "Service Version": serviceVersion, + return spinner.Process(fmt.Sprintf("Activating service (version %d)", serviceVersion), func(_ *text.SpinnerWrapper) error { + _, err := c.Globals.APIClient.ActivateVersion(&fastly.ActivateVersionInput{ + ServiceID: serviceID, + ServiceVersion: serviceVersion, }) - return fmt.Errorf("error activating version: %w", err) - } - - spinner.StopMessage(msg) - return spinner.Stop() + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Service ID": serviceID, + "Service Version": serviceVersion, + }) + return fmt.Errorf("error activating version: %w", err) + } + return nil + }) } func getServiceDomain(apiClient api.Interface, serviceID string, serviceVersion int) (string, error) { diff --git a/pkg/commands/compute/init.go b/pkg/commands/compute/init.go index 18195ed35..c26180a44 100644 --- a/pkg/commands/compute/init.go +++ b/pkg/commands/compute/init.go @@ -376,95 +376,52 @@ func verifyDestination(path string, spinner text.Spinner, out io.Writer) (dst st } if err != nil && errors.Is(err, fs.ErrNotExist) { // normal-ish case text.Break(out) - - err := spinner.Start() - if err != nil { - return "", err - } - msg := fmt.Sprintf("Creating %s", dst) - spinner.Message(msg + "...") - - if err := os.MkdirAll(dst, 0o700); err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + err := spinner.Process(fmt.Sprintf("Creating %s", dst), func(_ *text.SpinnerWrapper) error { + if err := os.MkdirAll(dst, 0o700); err != nil { + return fmt.Errorf("error creating package destination: %w", err) } - return dst, fmt.Errorf("error creating package destination: %w", err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return "", err } } text.Break(out) - err = spinner.Start() - if err != nil { - return "", err - } - msg := "Validating directory permissions" - spinner.Message(msg + "...") - tmpname := make([]byte, 16) - n, err := rand.Read(tmpname) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + err = spinner.Process("Validating directory permissions", func(_ *text.SpinnerWrapper) error { + tmpname := make([]byte, 16) + n, err := rand.Read(tmpname) + if err != nil { + return fmt.Errorf("error generating random filename: %w", err) } - return dst, fmt.Errorf("error generating random filename: %w", err) - } - if n != 16 { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + if n != 16 { + return fmt.Errorf("failed to generate enough entropy (%d/%d)", n, 16) } - return dst, fmt.Errorf("failed to generate enough entropy (%d/%d)", n, 16) - } - // gosec flagged this: - // G304 (CWE-22): Potential file inclusion via variable - // - // Disabling as the input is determined by our own package. - /* #nosec */ - f, err := os.Create(filepath.Join(dst, fmt.Sprintf("tmp_%x", tmpname))) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + // gosec flagged this: + // G304 (CWE-22): Potential file inclusion via variable + // + // Disabling as the input is determined by our own package. + // #nosec + f, err := os.Create(filepath.Join(dst, fmt.Sprintf("tmp_%x", tmpname))) + if err != nil { + return fmt.Errorf("error creating file in package destination: %w", err) } - return dst, fmt.Errorf("error creating file in package destination: %w", err) - } - if err := f.Close(); err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + if err := f.Close(); err != nil { + return fmt.Errorf("error closing file in package destination: %w", err) } - return dst, fmt.Errorf("error closing file in package destination: %w", err) - } - if err := os.Remove(f.Name()); err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + if err := os.Remove(f.Name()); err != nil { + return fmt.Errorf("error removing file in package destination: %w", err) } - return dst, fmt.Errorf("error removing file in package destination: %w", err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return "", err } + return dst, nil } @@ -1045,192 +1002,124 @@ func updateManifest( authors []string, language *Language, ) (manifest.File, error) { - err := spinner.Start() - if err != nil { - return m, err - } - msg := "Reading fastly.toml" - spinner.Message(msg + "...") - + var returnEarly bool mp := filepath.Join(path, manifest.Filename) - if err := m.Read(mp); err != nil { - if language != nil { - if language.Name == "other" { - // We create a fastly.toml manifest on behalf of the user if they're - // bringing their own pre-compiled Wasm binary to be packaged. - m.ManifestVersion = manifest.ManifestLatestVersion - m.Name = name - m.Description = desc - m.Authors = authors - m.Language = language.Name - if err := m.Write(mp); err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return m, spinErr + err := spinner.Process("Reading fastly.toml", func(_ *text.SpinnerWrapper) error { + if err := m.Read(mp); err != nil { + if language != nil { + if language.Name == "other" { + // We create a fastly.toml manifest on behalf of the user if they're + // bringing their own pre-compiled Wasm binary to be packaged. + m.ManifestVersion = manifest.ManifestLatestVersion + m.Name = name + m.Description = desc + m.Authors = authors + m.Language = language.Name + if err := m.Write(mp); err != nil { + return fmt.Errorf("error saving fastly.toml: %w", err) } - return m, fmt.Errorf("error saving fastly.toml: %w", err) - } - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return m, spinErr + returnEarly = true + return nil // EXIT updateManifest } - return m, nil } + return fmt.Errorf("error reading fastly.toml: %w", err) } - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return m, spinErr - } - return m, fmt.Errorf("error reading fastly.toml: %w", err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return m, err } - - err = spinner.Start() - if err != nil { - return m, err + if returnEarly { + return m, nil } - msg = fmt.Sprintf("Setting package name in manifest to %q", name) - spinner.Message(msg + "...") - - m.Name = name - spinner.StopMessage(msg) - err = spinner.Stop() + err = spinner.Process(fmt.Sprintf("Setting package name in manifest to %q", name), func(_ *text.SpinnerWrapper) error { + m.Name = name + return nil + }) if err != nil { return m, err } - // NOTE: We allow an empty description to be set. - m.Description = desc + var descMsg string if desc != "" { - desc = " to '" + desc + "'" + descMsg = " to '" + desc + "'" } - err = spinner.Start() - if err != nil { - return m, err - } - msg = fmt.Sprintf("Setting description in manifest%s", desc) - spinner.Message(msg + "...") - - spinner.StopMessage(msg) - err = spinner.Stop() + err = spinner.Process(fmt.Sprintf("Setting description in manifest%s", descMsg), func(_ *text.SpinnerWrapper) error { + // NOTE: We allow an empty description to be set. + m.Description = desc + return nil + }) if err != nil { return m, err } if len(authors) > 0 { - err := spinner.Start() - if err != nil { - return m, err - } - msg := fmt.Sprintf("Setting authors in manifest to '%s'", strings.Join(authors, ", ")) - spinner.Message(msg + "...") - - m.Authors = authors - - spinner.StopMessage(msg) - err = spinner.Stop() + err = spinner.Process(fmt.Sprintf("Setting authors in manifest to '%s'", strings.Join(authors, ", ")), func(_ *text.SpinnerWrapper) error { + m.Authors = authors + return nil + }) if err != nil { return m, err } } if language != nil { - err := spinner.Start() - if err != nil { - return m, err - } - msg := fmt.Sprintf("Setting language in manifest to '%s'", language.Name) - spinner.Message(msg + "...") - - m.Language = language.Name - - spinner.StopMessage(msg) - err = spinner.Stop() + err = spinner.Process(fmt.Sprintf("Setting language in manifest to '%s'", language.Name), func(_ *text.SpinnerWrapper) error { + m.Language = language.Name + return nil + }) if err != nil { return m, err } } - err = spinner.Start() + err = spinner.Process("Saving manifest changes", func(_ *text.SpinnerWrapper) error { + if err := m.Write(mp); err != nil { + return fmt.Errorf("error saving fastly.toml: %w", err) + } + return nil + }) if err != nil { return m, err } - msg = "Saving manifest changes" - spinner.Message(msg + "...") - - if err := m.Write(mp); err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return m, spinErr - } - return m, fmt.Errorf("error saving fastly.toml: %w", err) - } - - spinner.StopMessage(msg) - return m, spinner.Stop() + return m, nil } // initializeLanguage for newly cloned package. func initializeLanguage(spinner text.Spinner, language *Language, languages []*Language, name, wd, path string) (*Language, error) { - err := spinner.Start() - if err != nil { - return nil, err - } - msg := "Initializing package" - spinner.Message(msg + "...") - - if wd != path { - err := os.Chdir(path) - if err != nil { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return nil, spinErr + err := spinner.Process("Initializing package", func(_ *text.SpinnerWrapper) error { + if wd != path { + err := os.Chdir(path) + if err != nil { + return fmt.Errorf("error changing to your project directory: %w", err) } - return nil, fmt.Errorf("error changing to your project directory: %w", err) } - } - // Language will not be set if user provides the --from flag. So we'll check - // the manifest content and ensure what's set there is the language instance - // used for the sake of `compute build` operations. - if language == nil { - var match bool - for _, l := range languages { - if strings.EqualFold(name, l.Name) { - language = l - match = true - break + // Language will not be set if user provides the --from flag. So we'll check + // the manifest content and ensure what's set there is the language instance + // used for the sake of `compute build` operations. + if language == nil { + var match bool + for _, l := range languages { + if strings.EqualFold(name, l.Name) { + language = l + match = true + break + } } - } - if !match { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return nil, spinErr + if !match { + return fmt.Errorf("unrecognised package language") } - return nil, fmt.Errorf("unrecognised package language") } - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return nil, err } + return language, nil } diff --git a/pkg/commands/compute/pack.go b/pkg/commands/compute/pack.go index b6394b1a3..adb1792f5 100644 --- a/pkg/commands/compute/pack.go +++ b/pkg/commands/compute/pack.go @@ -54,8 +54,10 @@ func (c *PackCommand) Exec(_ io.Reader, out io.Writer) (err error) { if err = c.manifest.File.ReadError(); err != nil { return err } + bin := "pkg/package/bin/main.wasm" bindir := filepath.Dir(bin) + err = filesystem.MakeDirectoryIfNotExists(bindir) if err != nil { c.Globals.ErrLog.AddWithContext(err, map[string]any{ @@ -71,6 +73,7 @@ func (c *PackCommand) Exec(_ io.Reader, out io.Writer) (err error) { }) return err } + dst, err := filepath.Abs(bin) if err != nil { c.Globals.ErrLog.AddWithContext(err, map[string]any{ @@ -79,106 +82,58 @@ func (c *PackCommand) Exec(_ io.Reader, out io.Writer) (err error) { return err } - err = spinner.Start() - if err != nil { - return err - } - msg := "Copying wasm binary" - spinner.Message(msg + "...") - - if err := filesystem.CopyFile(src, dst); err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Path (absolute)": src, - "Wasm destination (absolute)": dst, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr - } - - return fmt.Errorf("error copying wasm binary to '%s': %w", dst, err) - } - - if !filesystem.FileExists(bin) { - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + err = spinner.Process("Copying wasm binary", func(_ *text.SpinnerWrapper) error { + if err := filesystem.CopyFile(src, dst); err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Path (absolute)": src, + "Wasm destination (absolute)": dst, + }) + return fmt.Errorf("error copying wasm binary to '%s': %w", dst, err) } - return fsterr.RemediationError{ - Inner: fmt.Errorf("no wasm binary found"), - Remediation: "Run `fastly compute pack --path ` to copy your wasm binary to the required location", + if !filesystem.FileExists(bin) { + return fsterr.RemediationError{ + Inner: fmt.Errorf("no wasm binary found"), + Remediation: "Run `fastly compute pack --path ` to copy your wasm binary to the required location", + } } - } - - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return err - } - - err = spinner.Start() + return nil + }) if err != nil { return err } - msg = "Copying manifest" - spinner.Message(msg + "...") - - src = manifest.Filename - dst = fmt.Sprintf("pkg/package/%s", manifest.Filename) - if err := filesystem.CopyFile(src, dst); err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Manifest (destination)": dst, - "Manifest (source)": src, - }) - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + err = spinner.Process("Copying manifest", func(_ *text.SpinnerWrapper) error { + src = manifest.Filename + dst = fmt.Sprintf("pkg/package/%s", manifest.Filename) + if err := filesystem.CopyFile(src, dst); err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Manifest (destination)": dst, + "Manifest (source)": src, + }) + return fmt.Errorf("error copying manifest to '%s': %w", dst, err) } - - return fmt.Errorf("error copying manifest to '%s': %w", dst, err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return err - } - - err = spinner.Start() + return nil + }) if err != nil { return err } - msg = "Creating package.tar.gz file" - spinner.Message(msg + "...") - - tar := archiver.NewTarGz() - tar.OverwriteExisting = true - { - dir := "pkg/package" - src := []string{dir} - dst := fmt.Sprintf("%s.tar.gz", dir) - if err = tar.Archive(src, dst); err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Tar source": dir, - "Tar destination": dst, - }) - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + return spinner.Process("Creating package.tar.gz file", func(_ *text.SpinnerWrapper) error { + tar := archiver.NewTarGz() + tar.OverwriteExisting = true + { + dir := "pkg/package" + src := []string{dir} + dst := fmt.Sprintf("%s.tar.gz", dir) + if err = tar.Archive(src, dst); err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Tar source": dir, + "Tar destination": dst, + }) + return err } - - return err } - } - - spinner.StopMessage(msg) - return spinner.Stop() + return nil + }) } diff --git a/pkg/commands/compute/update.go b/pkg/commands/compute/update.go index 6aea8727d..1967037ab 100644 --- a/pkg/commands/compute/update.go +++ b/pkg/commands/compute/update.go @@ -111,38 +111,24 @@ func (c *UpdateCommand) Exec(_ io.Reader, out io.Writer) (err error) { } }() - err = spinner.Start() - if err != nil { - return err - } - msg := "Uploading package" - spinner.Message(msg + "...") - - _, err = c.Globals.APIClient.UpdatePackage(&fastly.UpdatePackageInput{ - ServiceID: serviceID, - ServiceVersion: serviceVersion.Number, - PackagePath: packagePath, - }) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Service ID": serviceID, - "Service Version": serviceVersion.Number, + err = spinner.Process("Uploading package", func(_ *text.SpinnerWrapper) error { + _, err = c.Globals.APIClient.UpdatePackage(&fastly.UpdatePackageInput{ + ServiceID: serviceID, + ServiceVersion: serviceVersion.Number, + PackagePath: packagePath, }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr - } - - return fsterr.RemediationError{ - Inner: fmt.Errorf("error uploading package: %w", err), - Remediation: "Run `fastly compute build` to produce a Compute@Edge package, alternatively use the --package flag to reference a package outside of the current project.", + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Service ID": serviceID, + "Service Version": serviceVersion.Number, + }) + return fsterr.RemediationError{ + Inner: fmt.Errorf("error uploading package: %w", err), + Remediation: "Run `fastly compute build` to produce a Compute@Edge package, alternatively use the --package flag to reference a package outside of the current project.", + } } - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return err } diff --git a/pkg/commands/profile/create.go b/pkg/commands/profile/create.go index 19000163e..53d777488 100644 --- a/pkg/commands/profile/create.go +++ b/pkg/commands/profile/create.go @@ -11,6 +11,7 @@ import ( "github.com/fastly/go-fastly/v8/fastly" + "github.com/fastly/cli/pkg/api" "github.com/fastly/cli/pkg/cmd" "github.com/fastly/cli/pkg/config" fsterr "github.com/fastly/cli/pkg/errors" @@ -126,72 +127,50 @@ var ErrEmptyToken = errors.New("token cannot be empty") // validateToken ensures the token can be used to acquire user data. func (c *CreateCommand) validateToken(token, endpoint string, spinner text.Spinner) (string, error) { - err := spinner.Start() - if err != nil { - return "", err - } - msg := "Validating token" - spinner.Message(msg + "...") - - client, err := c.clientFactory(token, endpoint) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Endpoint": endpoint, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + var ( + client api.Interface + err error + t *fastly.Token + ) + err = spinner.Process("Validating token", func(_ *text.SpinnerWrapper) error { + client, err = c.clientFactory(token, endpoint) + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Endpoint": endpoint, + }) + return fmt.Errorf("error regenerating Fastly API client: %w", err) } - return "", fmt.Errorf("error regenerating Fastly API client: %w", err) - } - - t, err := client.GetTokenSelf() - if err != nil { - c.Globals.ErrLog.Add(err) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + t, err = client.GetTokenSelf() + if err != nil { + c.Globals.ErrLog.Add(err) + return fmt.Errorf("error validating token: %w", err) } - - return "", fmt.Errorf("error validating token: %w", err) + return nil + }) + if err != nil { + return "", err } - if c.automationToken { - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return "", err - } return fmt.Sprintf("Automation Token (%s)", t.ID), nil } - user, err := client.GetUser(&fastly.GetUserInput{ - ID: t.UserID, - }) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "User ID": t.UserID, + var user *fastly.User + err = spinner.Process("Getting user data", func(_ *text.SpinnerWrapper) error { + user, err = client.GetUser(&fastly.GetUserInput{ + ID: t.UserID, }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr - } - - return "", fsterr.RemediationError{ - Inner: fmt.Errorf("error fetching token user: %w", err), - Remediation: "If providing an 'automation token', retry the command with the `--automation-token` flag set.", + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "User ID": t.UserID, + }) + return fsterr.RemediationError{ + Inner: fmt.Errorf("error fetching token user: %w", err), + Remediation: "If providing an 'automation token', retry the command with the `--automation-token` flag set.", + } } - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return "", err } @@ -200,35 +179,28 @@ func (c *CreateCommand) validateToken(token, endpoint string, spinner text.Spinn // updateInMemCfg persists the updated configuration data in-memory. func (c *CreateCommand) updateInMemCfg(email, token, endpoint string, def bool, spinner text.Spinner) error { - err := spinner.Start() - if err != nil { - return err - } - msg := "Persisting configuration" - spinner.Message(msg + "...") + return spinner.Process("Persisting configuration", func(_ *text.SpinnerWrapper) error { + c.Globals.Config.Fastly.APIEndpoint = endpoint - c.Globals.Config.Fastly.APIEndpoint = endpoint - - if c.Globals.Config.Profiles == nil { - c.Globals.Config.Profiles = make(config.Profiles) - } - c.Globals.Config.Profiles[c.profile] = &config.Profile{ - Default: def, - Email: email, - Token: token, - } - - // If the user wants the newly created profile to be their new default, then - // we'll call Set for its side effect of resetting all other profiles to have - // their Default field set to false. - if def { - if p, ok := profile.Set(c.profile, c.Globals.Config.Profiles); ok { - c.Globals.Config.Profiles = p + if c.Globals.Config.Profiles == nil { + c.Globals.Config.Profiles = make(config.Profiles) + } + c.Globals.Config.Profiles[c.profile] = &config.Profile{ + Default: def, + Email: email, + Token: token, } - } - spinner.StopMessage(msg) - return spinner.Stop() + // If the user wants the newly created profile to be their new default, then + // we'll call Set for its side effect of resetting all other profiles to have + // their Default field set to false. + if def { + if p, ok := profile.Set(c.profile, c.Globals.Config.Profiles); ok { + c.Globals.Config.Profiles = p + } + } + return nil + }) } func (c *CreateCommand) persistCfg() error { diff --git a/pkg/commands/profile/update.go b/pkg/commands/profile/update.go index 2a7c913b4..2a1a53f0a 100644 --- a/pkg/commands/profile/update.go +++ b/pkg/commands/profile/update.go @@ -144,69 +144,50 @@ func (c *UpdateCommand) Exec(in io.Reader, out io.Writer) error { // validateToken ensures the token can be used to acquire user data. func (c *UpdateCommand) validateToken(token, endpoint string, spinner text.Spinner) (string, error) { - err := spinner.Start() - if err != nil { - return "", err - } - msg := "Validating token" - spinner.Message(msg + "...") - - client, err := c.clientFactory(token, endpoint) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Endpoint": endpoint, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + var ( + client api.Interface + err error + t *fastly.Token + ) + err = spinner.Process("Validating token", func(_ *text.SpinnerWrapper) error { + client, err = c.clientFactory(token, endpoint) + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Endpoint": endpoint, + }) + return fmt.Errorf("error regenerating Fastly API client: %w", err) } - return "", fmt.Errorf("error regenerating Fastly API client: %w", err) - } - - t, err := client.GetTokenSelf() - if err != nil { - c.Globals.ErrLog.Add(err) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + t, err = client.GetTokenSelf() + if err != nil { + c.Globals.ErrLog.Add(err) + return fmt.Errorf("error validating token: %w", err) } - - return "", fmt.Errorf("error validating token: %w", err) + return nil + }) + if err != nil { + return "", err } - if c.automationToken { - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return "", err - } return fmt.Sprintf("Automation Token (%s)", t.ID), nil } - user, err := client.GetUser(&fastly.GetUserInput{ - ID: t.UserID, - }) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "User ID": t.UserID, + var user *fastly.User + err = spinner.Process("Getting user data", func(_ *text.SpinnerWrapper) error { + user, err = client.GetUser(&fastly.GetUserInput{ + ID: t.UserID, }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return "", spinErr + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "User ID": t.UserID, + }) + return fsterr.RemediationError{ + Inner: fmt.Errorf("error fetching token user: %w", err), + Remediation: "If providing an 'automation token', retry the command with the `--automation-token` flag set.", + } } - - return "", fmt.Errorf("error fetching token user: %w", err) - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return "", err } diff --git a/pkg/commands/update/root.go b/pkg/commands/update/root.go index ccd033a40..0138f3d00 100644 --- a/pkg/commands/update/root.go +++ b/pkg/commands/update/root.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" + "github.com/blang/semver" + "github.com/fastly/cli/pkg/cmd" "github.com/fastly/cli/pkg/filesystem" "github.com/fastly/cli/pkg/github" @@ -39,17 +41,15 @@ func (c *RootCommand) Exec(_ io.Reader, out io.Writer) error { return err } - err = spinner.Start() - if err != nil { - return err - } - msg := "Updating versioning information" - spinner.Message(msg + "...") - - current, latest, shouldUpdate := Check(revision.AppVersion, c.av) + var ( + current, latest semver.Version + shouldUpdate bool + ) - spinner.StopMessage(msg) - err = spinner.Stop() + err = spinner.Process("Updating versioning information", func(_ *text.SpinnerWrapper) error { + current, latest, shouldUpdate = Check(revision.AppVersion, c.av) + return nil + }) if err != nil { return err } @@ -64,120 +64,80 @@ func (c *RootCommand) Exec(_ io.Reader, out io.Writer) error { return nil } - err = spinner.Start() - if err != nil { - return err - } - msg = "Fetching latest release" - spinner.Message(msg + "...") - - downloadedBin, err := c.av.DownloadLatest() - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Current CLI version": current, - "Latest CLI version": latest, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + var downloadedBin string + err = spinner.Process("Fetching latest release", func(_ *text.SpinnerWrapper) error { + downloadedBin, err = c.av.DownloadLatest() + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Current CLI version": current, + "Latest CLI version": latest, + }) + return fmt.Errorf("error downloading latest release: %w", err) } - - return fmt.Errorf("error downloading latest release: %w", err) - } - defer os.RemoveAll(downloadedBin) - - spinner.StopMessage(msg) - err = spinner.Stop() - if err != nil { - return err - } - - err = spinner.Start() + return nil + }) if err != nil { return err } - msg = "Replacing binary" - spinner.Message(msg + "...") - - execPath, err := os.Executable() - if err != nil { - c.Globals.ErrLog.Add(err) + defer os.RemoveAll(downloadedBin) - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + var currentBin string + err = spinner.Process("Replacing binary", func(_ *text.SpinnerWrapper) error { + execPath, err := os.Executable() + if err != nil { + c.Globals.ErrLog.Add(err) + return fmt.Errorf("error determining executable path: %w", err) } - return fmt.Errorf("error determining executable path: %w", err) - } - - currentBin, err := filepath.Abs(execPath) - if err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Executable path": execPath, - }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + currentBin, err = filepath.Abs(execPath) + if err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Executable path": execPath, + }) + return fmt.Errorf("error determining absolute target path: %w", err) } - return fmt.Errorf("error determining absolute target path: %w", err) - } - - // Windows does not permit replacing a running executable, however it will - // permit it if you first move the original executable. So we first move the - // running executable to a new location, then we move the executable that we - // downloaded to the same location as the original. - // I've also tested this approach on nix systems and it works fine. - // - // Reference: - // https://github.com/golang/go/issues/21997#issuecomment-331744930 - - backup := currentBin + ".bak" - if err := os.Rename(currentBin, backup); err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Executable (source)": downloadedBin, - "Executable (destination)": currentBin, - }) - return fmt.Errorf("error moving the current executable: %w", err) - } - - if err = os.Remove(backup); err != nil { - c.Globals.ErrLog.Add(err) - } + // Windows does not permit replacing a running executable, however it will + // permit it if you first move the original executable. So we first move the + // running executable to a new location, then we move the executable that we + // downloaded to the same location as the original. + // I've also tested this approach on nix systems and it works fine. + // + // Reference: + // https://github.com/golang/go/issues/21997#issuecomment-331744930 + + backup := currentBin + ".bak" + if err := os.Rename(currentBin, backup); err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Executable (source)": downloadedBin, + "Executable (destination)": currentBin, + }) + return fmt.Errorf("error moving the current executable: %w", err) + } - // Move the downloaded binary to the same location as the current executable. - if err := os.Rename(downloadedBin, currentBin); err != nil { - c.Globals.ErrLog.AddWithContext(err, map[string]any{ - "Executable (source)": downloadedBin, - "Executable (destination)": currentBin, - }) - renameErr := err + if err = os.Remove(backup); err != nil { + c.Globals.ErrLog.Add(err) + } - // Failing that we'll try to io.Copy downloaded binary to the current binary. - if err := filesystem.CopyFile(downloadedBin, currentBin); err != nil { + // Move the downloaded binary to the same location as the current executable. + if err := os.Rename(downloadedBin, currentBin); err != nil { c.Globals.ErrLog.AddWithContext(err, map[string]any{ "Executable (source)": downloadedBin, "Executable (destination)": currentBin, }) - - spinner.StopFailMessage(msg) - spinErr := spinner.StopFail() - if spinErr != nil { - return spinErr + renameErr := err + + // Failing that we'll try to io.Copy downloaded binary to the current binary. + if err := filesystem.CopyFile(downloadedBin, currentBin); err != nil { + c.Globals.ErrLog.AddWithContext(err, map[string]any{ + "Executable (source)": downloadedBin, + "Executable (destination)": currentBin, + }) + return fmt.Errorf("error 'copying' latest binary in place: %w (following an error 'moving': %w)", err, renameErr) } - - return fmt.Errorf("error 'copying' latest binary in place: %w (following an error 'moving': %w)", err, renameErr) } - } - - spinner.StopMessage(msg) - err = spinner.Stop() + return nil + }) if err != nil { return err } diff --git a/pkg/text/spinner.go b/pkg/text/spinner.go index 5704f8231..19a51a484 100644 --- a/pkg/text/spinner.go +++ b/pkg/text/spinner.go @@ -16,6 +16,37 @@ type Spinner interface { StopFail() error StopMessage(message string) Stop() error + Process(msg string, fn SpinnerProcess) error +} + +// SpinnerProcess is the logic to execute in between the spinner start/stop. +type SpinnerProcess func(sp *SpinnerWrapper) error + +// SpinnerWrapper implements the Spinner interface. +type SpinnerWrapper struct { + *yacspin.Spinner +} + +// Process starts/stops the spinner with `msg` and executes `fn` in between. +func (sp *SpinnerWrapper) Process(msg string, fn SpinnerProcess) error { + err := sp.Start() + if err != nil { + return err + } + sp.Message(msg + "...") + + err = fn(sp) + if err != nil { + sp.StopFailMessage(msg) + spinStopErr := sp.StopFail() + if spinStopErr != nil { + return spinStopErr + } + return err + } + + sp.StopMessage(msg) + return sp.Stop() } // NewSpinner returns a new instance of a terminal prompt spinner. @@ -33,5 +64,6 @@ func NewSpinner(out io.Writer) (Spinner, error) { if err != nil { return nil, err } - return spinner, nil + + return &SpinnerWrapper{spinner}, nil }