-
-
Notifications
You must be signed in to change notification settings - Fork 624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add TTL to taskfile.Cache #1702
Open
jlucktay
wants to merge
14
commits into
go-task:main
Choose a base branch
from
jlucktay:issue-1402/add-ttl-to-taskfile-cache
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+253
−20
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
ba11775
docs(taskfile): add flowchart for intended behaviour of Cache
jlucktay 2a63051
test(taskfile): get basic coverage set up around taskfile.Cache
jlucktay 392fb90
test(taskfile): add coverage around new TTL functionality in Cache
jlucktay 2264ff4
feat(taskfile): implement TTL in Cache
jlucktay 582890f
feat: wire up new flag for cache TTL
jlucktay 0813113
refactor(taskfile): call cacheFilePath once and reuse the value
jlucktay 094f5da
test(taskfile): switch assertion on error
jlucktay 2af1126
test(taskfile): shorten TTL and sleep
jlucktay d733fc0
refactor(taskfile): combine two tests into one table-driven
jlucktay 6230c71
test(taskfile): move t.TempDir call inside helper
jlucktay 8b73ca1
docs(website): update caching section
jlucktay 3c9dab3
style: run 'gofumpt -w .'
jlucktay e309b73
docs(website): swap Latest and Next
jlucktay 741743b
docs(taskfile): update godoc comment
jlucktay File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
# The `taskfile` package | ||
|
||
```mermaid | ||
--- | ||
title: taskfile.Cache behaviour | ||
--- | ||
flowchart LR | ||
%% Beginning state | ||
start([A remote Taskfile | ||
is required]) | ||
|
||
%% Checks to decide | ||
cached{Remote Taskfile | ||
already cached?} | ||
|
||
subgraph checkTTL [Is the cached Taskfile still inside TTL?] | ||
%% Beginning state | ||
lastModified(Stat the cached | ||
Taskfile and get last | ||
modified timestamp) | ||
|
||
%% Check to decide | ||
timestampPlusTTL{Timestamp | ||
plus TTL is in | ||
the future?} | ||
|
||
%% Flowlines | ||
lastModified-->timestampPlusTTL | ||
end | ||
|
||
%% End states | ||
useCached([Use the | ||
cached Taskfile]) | ||
download(["(Re)download the | ||
remote Taskfile"]) | ||
|
||
%% Flowlines | ||
start-->cached | ||
cached-- Yes -->lastModified | ||
cached-- No -->download | ||
timestampPlusTTL-- Yes -->useCached | ||
timestampPlusTTL-- No -->download | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,24 +2,51 @@ package taskfile | |
|
||
import ( | ||
"crypto/sha256" | ||
"errors" | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
"time" | ||
) | ||
|
||
// DefaultCacheTTL is used when a value is not explicitly given to a new Cache. | ||
const DefaultCacheTTL = time.Duration(time.Hour * 24) | ||
|
||
type Cache struct { | ||
dir string | ||
ttl time.Duration | ||
} | ||
|
||
func NewCache(dir string) (*Cache, error) { | ||
// ErrExpired is returned when a cached file has expired. | ||
var ErrExpired = errors.New("task: cache expired") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should go in the |
||
|
||
type CacheOption func(*Cache) | ||
|
||
func NewCache(dir string, opts ...CacheOption) (*Cache, error) { | ||
dir = filepath.Join(dir, "remote") | ||
if err := os.MkdirAll(dir, 0o755); err != nil { | ||
return nil, err | ||
} | ||
return &Cache{ | ||
|
||
cache := &Cache{ | ||
dir: dir, | ||
}, nil | ||
ttl: DefaultCacheTTL, | ||
} | ||
|
||
// Apply options. | ||
for _, opt := range opts { | ||
opt(cache) | ||
} | ||
|
||
return cache, nil | ||
} | ||
|
||
// WithTTL will override the default TTL setting on a new Cache. | ||
func WithTTL(ttl time.Duration) CacheOption { | ||
return func(cache *Cache) { | ||
cache.ttl = ttl | ||
} | ||
} | ||
|
||
func checksum(b []byte) string { | ||
|
@@ -33,7 +60,19 @@ func (c *Cache) write(node Node, b []byte) error { | |
} | ||
|
||
func (c *Cache) read(node Node) ([]byte, error) { | ||
return os.ReadFile(c.cacheFilePath(node)) | ||
cfp := c.cacheFilePath(node) | ||
|
||
fi, err := os.Stat(cfp) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not stat cached file: %w", err) | ||
} | ||
|
||
expiresAt := fi.ModTime().Add(c.ttl) | ||
if expiresAt.Before(time.Now()) { | ||
return nil, ErrExpired | ||
} | ||
|
||
return os.ReadFile(cfp) | ||
} | ||
|
||
func (c *Cache) writeChecksum(node Node, checksum string) error { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
package taskfile | ||
|
||
import ( | ||
"fmt" | ||
"io" | ||
"os" | ||
"path/filepath" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/go-task/task/v3/internal/logger" | ||
) | ||
|
||
func ExampleWithTTL() { | ||
c, _ := NewCache(os.TempDir(), WithTTL(2*time.Minute+30*time.Second)) | ||
|
||
fmt.Println(c.ttl) | ||
// Output: 2m30s | ||
} | ||
|
||
var discardLogger = &logger.Logger{ | ||
Stdout: io.Discard, | ||
Stderr: io.Discard, | ||
} | ||
|
||
func primeNewCache(t *testing.T, cacheOpts ...CacheOption) (*Cache, *FileNode) { | ||
t.Helper() | ||
|
||
cache, err := NewCache(t.TempDir(), cacheOpts...) | ||
require.NoErrorf(t, err, "creating new cache") | ||
|
||
// Prime the temporary cache directory with a basic Taskfile. | ||
filename := "Taskfile.yaml" | ||
srcTaskfile := filepath.Join("testdata", filename) | ||
dstTaskfile := filepath.Join(cache.dir, filename) | ||
|
||
taskfileBytes, err := os.ReadFile(srcTaskfile) | ||
require.NoErrorf(t, err, "reading from testdata Taskfile (%s)", srcTaskfile) | ||
|
||
err = os.WriteFile(dstTaskfile, taskfileBytes, 0o640) | ||
require.NoErrorf(t, err, "writing to temporary Taskfile (%s)", dstTaskfile) | ||
|
||
// Create a new file node in the cache, with the entrypoint copied above. | ||
fileNode, err := NewFileNode(discardLogger, dstTaskfile, cache.dir) | ||
require.NoError(t, err, "creating new file node") | ||
|
||
return cache, fileNode | ||
} | ||
|
||
func TestCache(t *testing.T) { | ||
cache, fileNode := primeNewCache(t) | ||
|
||
// Attempt to read from cache, then write, then read again. | ||
_, err := cache.read(fileNode) | ||
require.ErrorIs(t, err, os.ErrNotExist, "reading from cache before writing should match error type") | ||
|
||
writeBytes := []byte("some bytes") | ||
err = cache.write(fileNode, writeBytes) | ||
require.NoError(t, err, "writing bytes to cache") | ||
|
||
readBytes, err := cache.read(fileNode) | ||
require.NoError(t, err, "reading from cache after write should not error") | ||
require.Equal(t, writeBytes, readBytes, "bytes read from cache should match bytes written") | ||
} | ||
|
||
func TestCacheInsideTTL(t *testing.T) { | ||
// Prime a new Cache with a TTL of one minute. | ||
cache, fileNode := primeNewCache(t, WithTTL(time.Minute)) | ||
|
||
// Write some bytes for the cached file. | ||
writeBytes := []byte("some bytes") | ||
err := cache.write(fileNode, writeBytes) | ||
require.NoError(t, err, "writing bytes to cache") | ||
|
||
// Reading from the cache while still inside the TTL should get the written bytes back. | ||
readBytes, err := cache.read(fileNode) | ||
require.NoError(t, err, "reading from cache inside TTL should not error") | ||
require.Equal(t, writeBytes, readBytes, "bytes read from cache should match bytes written") | ||
} | ||
|
||
func TestCacheOutsideTTL(t *testing.T) { | ||
// Prime a new Cache with an extremely short TTL. | ||
cache, fileNode := primeNewCache(t, WithTTL(time.Millisecond)) | ||
|
||
// Write some bytes for the cached file. | ||
writeBytes := []byte("some bytes") | ||
err := cache.write(fileNode, writeBytes) | ||
require.NoError(t, err, "writing bytes to cache") | ||
|
||
// Sleep for 5ms so that the cached file is outside of TTL. | ||
time.Sleep(5 * time.Millisecond) | ||
|
||
// Reading from the cache after sleeping past the end of TTL should get an error. | ||
readBytes, err := cache.read(fileNode) | ||
require.Empty(t, readBytes, "should not have read any bytes from cache") | ||
require.ErrorIs(t, err, ErrExpired, "should get 'expired' error when attempting to read from cache outside of TTL") | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package taskfile_test | ||
|
||
import ( | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/go-task/task/v3/taskfile" | ||
) | ||
|
||
func TestNewCache(t *testing.T) { | ||
testCases := map[string]struct { | ||
options []taskfile.CacheOption | ||
}{ | ||
"no options set": {}, | ||
|
||
"TTL option set": { | ||
options: []taskfile.CacheOption{taskfile.WithTTL(time.Hour)}, | ||
}, | ||
} | ||
|
||
for desc, testCase := range testCases { | ||
t.Run(desc, func(t *testing.T) { | ||
_, err := taskfile.NewCache(t.TempDir(), testCase.options...) | ||
require.NoError(t, err, "creating new cache") | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
version: '3' | ||
|
||
tasks: {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you are simply applying a gofumpt on this.
I'm surprised to see:
I would expect this variable to be used only where needed and to check the possible error