Skip to content

testocache: harden persistent cache and add namespaces#26

Open
dmedovich wants to merge 3 commits into
ozontech:mainfrom
dmedovich:feature/testocache-hardening
Open

testocache: harden persistent cache and add namespaces#26
dmedovich wants to merge 3 commits into
ozontech:mainfrom
dmedovich:feature/testocache-hardening

Conversation

@dmedovich

Copy link
Copy Markdown
Contributor

TL;DR

Closes #25

This PR hardens the file-backed cache behavior and adds scoped cache namespaces for plugins sharing the
same cache directory.

Changes

  • write cache entries through a temporary file followed by os.Rename;
  • validate the stored key before returning or removing an entry;
  • treat malformed and mismatched entries as missing;
  • ignore foreign and non-regular files in Keys;
  • preserve an existing .gitignore;
  • return ErrNotFound consistently for missing entries;
  • add testocache.Namespace for isolated keyspaces;
  • document the cache behavior and add regression tests.

The existing package-level API and FNV-based cache filenames remain compatible.

Testing

  • go test ./...
  • go test -race ./testocache
  • golangci-lint run --tests=false --timeout=5m
  • git diff --check

@metafates metafates left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, cool idea with the namespaces! Looks good overall, left some comments.

Also, thank you for the #23, will get it to a bit later

Comment thread testocache/cache.go Outdated
// Namespaces are isolated from each other and from the package-level cache
// functions. Namespace names must follow the same validity rules as cache keys.
func Namespace(name string) Cache {
return Cache{namespace: name, scoped: true}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think non-empty namespace already means scoped cache, so we can omit scoped field and just check that namespace != "" instead

or maybe let's just make it a method

func (c Cache) scoped() bool {
    return c.namespace != ""
}

Comment thread testocache/cache.go
Comment thread testocache/cache.go Outdated
func hash(key string) (string, error) {
h := fnv.New64a()
func validatePattern(pattern string) error {
if err := validate(pattern); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we have two validate functions now, let's rename validate to validateKey to make it more obvious

Comment thread testocache/cache_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

testocache.Keys returns an empty key for a new cache

2 participants