diff --git a/internal/config/config.go b/internal/config/config.go index 25f3e01e9..f3ed8cf6f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -36,6 +36,8 @@ type Config interface { Write() error Migrate(Migration) error + CacheDir() string + Aliases() *AliasConfig Authentication() *AuthConfig Browser(string) string @@ -191,6 +193,10 @@ func (c *cfg) Migrate(m Migration) error { return nil } +func (c *cfg) CacheDir() string { + return ghConfig.CacheDir() +} + func defaultFor(key string) (string, bool) { for _, co := range ConfigOptions() { if co.Key == key { diff --git a/internal/config/config_mock.go b/internal/config/config_mock.go index 586078a2c..fc25ad850 100644 --- a/internal/config/config_mock.go +++ b/internal/config/config_mock.go @@ -26,6 +26,9 @@ var _ Config = &ConfigMock{} // BrowserFunc: func(s string) string { // panic("mock out the Browser method") // }, +// CacheDirFunc: func() string { +// panic("mock out the CacheDir method") +// }, // EditorFunc: func(s string) string { // panic("mock out the Editor method") // }, @@ -72,6 +75,9 @@ type ConfigMock struct { // BrowserFunc mocks the Browser method. BrowserFunc func(s string) string + // CacheDirFunc mocks the CacheDir method. + CacheDirFunc func() string + // EditorFunc mocks the Editor method. EditorFunc func(s string) string @@ -115,6 +121,9 @@ type ConfigMock struct { // S is the s argument value. S string } + // CacheDir holds details about calls to the CacheDir method. + CacheDir []struct { + } // Editor holds details about calls to the Editor method. Editor []struct { // S is the s argument value. @@ -171,6 +180,7 @@ type ConfigMock struct { lockAliases sync.RWMutex lockAuthentication sync.RWMutex lockBrowser sync.RWMutex + lockCacheDir sync.RWMutex lockEditor sync.RWMutex lockGetOrDefault sync.RWMutex lockGitProtocol sync.RWMutex @@ -269,6 +279,33 @@ func (mock *ConfigMock) BrowserCalls() []struct { return calls } +// CacheDir calls CacheDirFunc. +func (mock *ConfigMock) CacheDir() string { + if mock.CacheDirFunc == nil { + panic("ConfigMock.CacheDirFunc: method is nil but Config.CacheDir was just called") + } + callInfo := struct { + }{} + mock.lockCacheDir.Lock() + mock.calls.CacheDir = append(mock.calls.CacheDir, callInfo) + mock.lockCacheDir.Unlock() + return mock.CacheDirFunc() +} + +// CacheDirCalls gets all the calls that were made to CacheDir. +// Check the length with: +// +// len(mockedConfig.CacheDirCalls()) +func (mock *ConfigMock) CacheDirCalls() []struct { +} { + var calls []struct { + } + mock.lockCacheDir.RLock() + calls = mock.calls.CacheDir + mock.lockCacheDir.RUnlock() + return calls +} + // Editor calls EditorFunc. func (mock *ConfigMock) Editor(s string) string { if mock.EditorFunc == nil { diff --git a/internal/config/stub.go b/internal/config/stub.go index 98c1ceaba..547ad951b 100644 --- a/internal/config/stub.go +++ b/internal/config/stub.go @@ -77,6 +77,9 @@ func NewFromString(cfgStr string) *ConfigMock { val, _ := cfg.GetOrDefault("", versionKey) return val } + mock.CacheDirFunc = func() string { + return cfg.CacheDir() + } return mock } diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index cc9aa0981..4c7fbf2d9 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -28,36 +28,53 @@ import ( "github.com/spf13/cobra" ) -type runLogCache interface { - Exists(string) bool - Create(string, io.Reader) error - Open(string) (*zip.ReadCloser, error) +type RunLogCache struct { + cacheDir string } -type rlc struct{} - -func (rlc) Exists(path string) bool { - if _, err := os.Stat(path); err != nil { - return false +func (c RunLogCache) Exists(key string) (bool, error) { + _, err := os.Stat(c.filepath(key)) + if err == nil { + return true, nil } - return true + + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + + return false, fmt.Errorf("checking cache entry: %v", err) } -func (rlc) Create(path string, content io.Reader) error { - err := os.MkdirAll(filepath.Dir(path), 0755) - if err != nil { - return fmt.Errorf("could not create cache: %w", err) + +func (c RunLogCache) Create(key string, content io.Reader) error { + if err := os.MkdirAll(filepath.Dir(c.cacheDir), 0755); err != nil { + return fmt.Errorf("creating cache directory: %v", err) } - out, err := os.Create(path) + out, err := os.Create(c.filepath(key)) if err != nil { - return err + return fmt.Errorf("creating cache entry: %v", err) } defer out.Close() - _, err = io.Copy(out, content) - return err + + if _, err := io.Copy(out, content); err != nil { + return fmt.Errorf("writing cache entry: %v", err) + + } + + return nil } -func (rlc) Open(path string) (*zip.ReadCloser, error) { - return zip.OpenReader(path) + +func (c RunLogCache) Open(key string) (*zip.ReadCloser, error) { + r, err := zip.OpenReader(c.filepath(key)) + if err != nil { + return nil, fmt.Errorf("opening cache entry: %v", err) + } + + return r, nil +} + +func (c RunLogCache) filepath(key string) string { + return filepath.Join(c.cacheDir, fmt.Sprintf("run-log-%s.zip", key)) } type ViewOptions struct { @@ -66,7 +83,7 @@ type ViewOptions struct { BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser Prompter shared.Prompter - RunLogCache runLogCache + RunLogCache RunLogCache RunID string JobID string @@ -85,12 +102,11 @@ type ViewOptions struct { func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { opts := &ViewOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - Prompter: f.Prompter, - Now: time.Now, - Browser: f.Browser, - RunLogCache: rlc{}, + IO: f.IOStreams, + HttpClient: f.HttpClient, + Prompter: f.Prompter, + Now: time.Now, + Browser: f.Browser, } cmd := &cobra.Command{ @@ -126,6 +142,15 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + config, err := f.Config() + if err != nil { + return err + } + + opts.RunLogCache = RunLogCache{ + cacheDir: config.CacheDir(), + } + if len(args) == 0 && opts.JobID == "" { if !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("run or job ID required when not running interactively") @@ -430,10 +455,14 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) { return resp.Body, nil } -func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run, attempt uint64) (*zip.ReadCloser, error) { - filename := fmt.Sprintf("run-log-%d-%d.zip", run.ID, run.StartedTime().Unix()) - filepath := filepath.Join(os.TempDir(), "gh-cli-cache", filename) - if !cache.Exists(filepath) { +func getRunLog(cache RunLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run, attempt uint64) (*zip.ReadCloser, error) { + cacheKey := fmt.Sprintf("%d-%d", run.ID, run.StartedTime().Unix()) + isCached, err := cache.Exists(cacheKey) + if err != nil { + return nil, err + } + + if !isCached { // Run log does not exist in cache so retrieve and store it logURL := fmt.Sprintf("%srepos/%s/actions/runs/%d/logs", ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID) @@ -461,13 +490,13 @@ func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface return nil, err } - err = cache.Create(filepath, respReader) + err = cache.Create(cacheKey, respReader) if err != nil { return nil, err } } - return cache.Open(filepath) + return cache.Open(cacheKey) } func promptForJob(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) { diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 1f5d4df75..88c8aa21d 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -12,6 +12,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmd/run/shared" @@ -137,6 +138,9 @@ func TestNewCmdView(t *testing.T) { f := &cmdutil.Factory{ IOStreams: ios, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, } argv, err := shlex.Split(tt.cli) @@ -1342,8 +1346,9 @@ func TestViewRun(t *testing.T) { browser := &browser.Stub{} tt.opts.Browser = browser - rlc := testRunLogCache{} - tt.opts.RunLogCache = rlc + tt.opts.RunLogCache = RunLogCache{ + cacheDir: t.TempDir(), + } t.Run(tt.name, func(t *testing.T) { err := runView(tt.opts) @@ -1503,18 +1508,6 @@ func Test_attachRunLog(t *testing.T) { } } -type testRunLogCache struct{} - -func (testRunLogCache) Exists(path string) bool { - return false -} -func (testRunLogCache) Create(path string, content io.Reader) error { - return nil -} -func (testRunLogCache) Open(path string) (*zip.ReadCloser, error) { - return zip.OpenReader("./fixtures/run_log.zip") -} - var barfTheFobLogOutput = heredoc.Doc(` cool job barz the fob log line 1 cool job barz the fob log line 2