From b570845e82fdf35b2428a085aa4f4a7af9fbb05d Mon Sep 17 00:00:00 2001 From: Virgil Date: Thu, 26 Mar 2026 11:15:00 +0000 Subject: [PATCH 1/6] refactor: address ax review feedback --- cache.go | 27 ++++----- cache_test.go | 145 ++++++++++++++++++++++++++++++-------------- docs/development.md | 15 +++-- docs/index.md | 22 +++---- 4 files changed, 130 insertions(+), 79 deletions(-) diff --git a/cache.go b/cache.go index f532b47..cff8458 100644 --- a/cache.go +++ b/cache.go @@ -16,23 +16,22 @@ import ( // DefaultTTL is the default cache expiry time. const DefaultTTL = 1 * time.Hour -// Cache represents a file-based cache. +// Cache stores JSON-encoded entries in a Medium-backed cache rooted at baseDir. type Cache struct { medium coreio.Medium baseDir string ttl time.Duration } -// Entry represents a cached item with metadata. +// Entry is the serialized cache record written to the backing Medium. type Entry struct { Data json.RawMessage `json:"data"` CachedAt time.Time `json:"cached_at"` ExpiresAt time.Time `json:"expires_at"` } -// New creates a new cache instance. -// If medium is nil, uses coreio.Local (filesystem). -// If baseDir is empty, uses .core/cache in current directory. +// New creates a cache and applies default Medium, base directory, and TTL values +// when callers pass zero values. func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error) { if medium == nil { medium = coreio.Local @@ -63,8 +62,8 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error }, nil } -// Path returns the full path for a cache key. -// Returns an error if the key attempts path traversal. +// Path returns the storage path used for key and rejects path traversal +// attempts. func (c *Cache) Path(key string) (string, error) { path := filepath.Join(c.baseDir, key+".json") @@ -85,7 +84,7 @@ func (c *Cache) Path(key string) (string, error) { return path, nil } -// Get retrieves a cached item if it exists and hasn't expired. +// Get unmarshals the cached item into dest if it exists and has not expired. func (c *Cache) Get(key string, dest any) (bool, error) { path, err := c.Path(key) if err != nil { @@ -119,7 +118,7 @@ func (c *Cache) Get(key string, dest any) (bool, error) { return true, nil } -// Set stores an item in the cache. +// Set marshals data and stores it in the cache. func (c *Cache) Set(key string, data any) error { path, err := c.Path(key) if err != nil { @@ -154,7 +153,7 @@ func (c *Cache) Set(key string, data any) error { return nil } -// Delete removes an item from the cache. +// Delete removes the cached item for key. func (c *Cache) Delete(key string) error { path, err := c.Path(key) if err != nil { @@ -171,7 +170,7 @@ func (c *Cache) Delete(key string) error { return nil } -// Clear removes all cached items. +// Clear removes all cached items under the cache base directory. func (c *Cache) Clear() error { if err := c.medium.DeleteAll(c.baseDir); err != nil { return coreerr.E("cache.Clear", "failed to clear cache", err) @@ -179,7 +178,7 @@ func (c *Cache) Clear() error { return nil } -// Age returns how old a cached item is, or -1 if not cached. +// Age reports how long ago key was cached, or -1 if it is missing or unreadable. func (c *Cache) Age(key string) time.Duration { path, err := c.Path(key) if err != nil { @@ -201,12 +200,12 @@ func (c *Cache) Age(key string) time.Duration { // GitHub-specific cache keys -// GitHubReposKey returns the cache key for an org's repo list. +// GitHubReposKey returns the cache key used for an organisation's repo list. func GitHubReposKey(org string) string { return filepath.Join("github", org, "repos") } -// GitHubRepoKey returns the cache key for a specific repo's metadata. +// GitHubRepoKey returns the cache key used for a repository metadata entry. func GitHubRepoKey(org, repo string) string { return filepath.Join("github", org, repo, "meta") } diff --git a/cache_test.go b/cache_test.go index c33996e..8314ce6 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1,6 +1,8 @@ package cache_test import ( + "encoding/json" + "path/filepath" "testing" "time" @@ -8,72 +10,97 @@ import ( coreio "dappco.re/go/core/io" ) -func TestCache(t *testing.T) { +func newTestCache(t *testing.T, baseDir string, ttl time.Duration) (*cache.Cache, *coreio.MockMedium) { + t.Helper() + m := coreio.NewMockMedium() - // Use a path that MockMedium will understand - baseDir := "/tmp/cache" - c, err := cache.New(m, baseDir, 1*time.Minute) + c, err := cache.New(m, baseDir, ttl) if err != nil { t.Fatalf("failed to create cache: %v", err) } + return c, m +} + +func TestCacheSetAndGet(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache", time.Minute) + key := "test-key" data := map[string]string{"foo": "bar"} - // Test Set if err := c.Set(key, data); err != nil { - t.Errorf("Set failed: %v", err) + t.Fatalf("Set failed: %v", err) } - // Test Get var retrieved map[string]string found, err := c.Get(key, &retrieved) if err != nil { - t.Errorf("Get failed: %v", err) + t.Fatalf("Get failed: %v", err) } if !found { - t.Error("expected to find cached item") + t.Fatal("expected to find cached item") } if retrieved["foo"] != "bar" { t.Errorf("expected foo=bar, got %v", retrieved["foo"]) } +} - // Test Age - age := c.Age(key) - if age < 0 { - t.Error("expected age >= 0") +func TestCacheAge(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-age", time.Minute) + + if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + t.Fatalf("Set failed: %v", err) } - // Test Delete - if err := c.Delete(key); err != nil { - t.Errorf("Delete failed: %v", err) + if age := c.Age("test-key"); age < 0 { + t.Errorf("expected age >= 0, got %v", age) } - found, err = c.Get(key, &retrieved) +} + +func TestCacheDelete(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-delete", time.Minute) + + if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + t.Fatalf("Set failed: %v", err) + } + + if err := c.Delete("test-key"); err != nil { + t.Fatalf("Delete failed: %v", err) + } + + var retrieved map[string]string + found, err := c.Get("test-key", &retrieved) if err != nil { - t.Errorf("Get after delete returned an unexpected error: %v", err) + t.Fatalf("Get after delete returned an unexpected error: %v", err) } if found { t.Error("expected item to be deleted") } +} - // Test Expiry - cshort, err := cache.New(m, "/tmp/cache-short", 10*time.Millisecond) - if err != nil { - t.Fatalf("failed to create short-lived cache: %v", err) - } - if err := cshort.Set(key, data); err != nil { +func TestCacheExpiry(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) + + if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { t.Fatalf("Set for expiry test failed: %v", err) } + time.Sleep(50 * time.Millisecond) - found, err = cshort.Get(key, &retrieved) + + var retrieved map[string]string + found, err := c.Get("test-key", &retrieved) if err != nil { - t.Errorf("Get for expired item returned an unexpected error: %v", err) + t.Fatalf("Get for expired item returned an unexpected error: %v", err) } if found { t.Error("expected item to be expired") } +} + +func TestCacheClear(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute) + data := map[string]string{"foo": "bar"} - // Test Clear if err := c.Set("key1", data); err != nil { t.Fatalf("Set for clear test failed for key1: %v", err) } @@ -81,48 +108,74 @@ func TestCache(t *testing.T) { t.Fatalf("Set for clear test failed for key2: %v", err) } if err := c.Clear(); err != nil { - t.Errorf("Clear failed: %v", err) + t.Fatalf("Clear failed: %v", err) } - found, err = c.Get("key1", &retrieved) + + var retrieved map[string]string + found, err := c.Get("key1", &retrieved) if err != nil { - t.Errorf("Get after clear returned an unexpected error: %v", err) + t.Fatalf("Get after clear returned an unexpected error: %v", err) } if found { t.Error("expected key1 to be cleared") } } -func TestCacheDefaults(t *testing.T) { - // Test default Medium (io.Local) and default TTL - c, err := cache.New(nil, "", 0) - if err != nil { - t.Fatalf("failed to create cache with defaults: %v", err) +func TestCacheUsesDefaultBaseDirAndTTL(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + c, m := newTestCache(t, "", 0) + + const key = "defaults" + if err := c.Set(key, map[string]string{"foo": "bar"}); err != nil { + t.Fatalf("Set failed: %v", err) } - if c == nil { - t.Fatal("expected cache instance") + + path, err := c.Path(key) + if err != nil { + t.Fatalf("Path failed: %v", err) + } + + wantPath := filepath.Join(tmpDir, ".core", "cache", key+".json") + if path != wantPath { + t.Fatalf("expected default path %q, got %q", wantPath, path) + } + + raw, err := m.Read(path) + if err != nil { + t.Fatalf("Read failed: %v", err) + } + + var entry cache.Entry + if err := json.Unmarshal([]byte(raw), &entry); err != nil { + t.Fatalf("failed to unmarshal cache entry: %v", err) + } + + ttl := entry.ExpiresAt.Sub(entry.CachedAt) + if ttl < cache.DefaultTTL || ttl > cache.DefaultTTL+time.Second { + t.Fatalf("expected ttl near %v, got %v", cache.DefaultTTL, ttl) } } -func TestGitHubKeys(t *testing.T) { +func TestGitHubReposKey(t *testing.T) { key := cache.GitHubReposKey("myorg") if key != "github/myorg/repos" { t.Errorf("unexpected GitHubReposKey: %q", key) } +} - key = cache.GitHubRepoKey("myorg", "myrepo") +func TestGitHubRepoKey(t *testing.T) { + key := cache.GitHubRepoKey("myorg", "myrepo") if key != "github/myorg/myrepo/meta" { t.Errorf("unexpected GitHubRepoKey: %q", key) } } -func TestPathTraversalRejected(t *testing.T) { - m := coreio.NewMockMedium() - c, err := cache.New(m, "/tmp/cache-traversal", 1*time.Minute) - if err != nil { - t.Fatalf("failed to create cache: %v", err) - } +func TestCacheRejectsPathTraversal(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) - _, err = c.Path("../../etc/passwd") + _, err := c.Path("../../etc/passwd") if err == nil { t.Error("expected error for path traversal key, got nil") } diff --git a/docs/development.md b/docs/development.md index 841f25a..ffde628 100644 --- a/docs/development.md +++ b/docs/development.md @@ -11,16 +11,14 @@ This guide covers how to build, test, and contribute to `go-cache`. ## Prerequisites - **Go 1.26** or later -- Access to `forge.lthn.ai` modules (`GOPRIVATE=forge.lthn.ai/*`) +- Access to private modules (`GOPRIVATE=dappco.re/*,forge.lthn.ai/*`) - The `core` CLI (optional, for `core go test` and `core go qa`) ## Getting the Source -```bash -git clone ssh://git@forge.lthn.ai:2223/core/go-cache.git -cd go-cache -``` +Clone the repository from your configured remote, then use the +`dappco.re/go/core/cache` module path in code and `go get` operations. If you are working within the Go workspace at `~/Code/go.work`, the module is already available locally and dependency resolution will use workspace overrides. @@ -104,7 +102,7 @@ Tests follow the standard Go testing conventions. The codebase uses 1. Use `io.NewMockMedium()` rather than the real filesystem. 2. Keep TTLs short (milliseconds) when testing expiry behaviour. -3. Name test functions descriptively: `TestCacheExpiry`, `TestCacheDefaults`, etc. +3. Name test functions descriptively: `TestCacheExpiry`, `TestCacheUsesDefaultBaseDirAndTTL`, etc. Example of testing cache expiry: @@ -168,9 +166,10 @@ the [architecture](architecture.md) document for the full method mapping. ```go import ( - "forge.lthn.ai/core/go-cache" - "forge.lthn.ai/core/go-io/store" "time" + + "dappco.re/go/core/cache" + "dappco.re/go/core/io/store" ) // Use SQLite as the cache backend diff --git a/docs/index.md b/docs/index.md index 76cbfe8..8b07c06 100644 --- a/docs/index.md +++ b/docs/index.md @@ -1,6 +1,6 @@ --- title: go-cache -description: File-based caching with TTL expiry, storage-agnostic via the go-io Medium interface. +description: Storage-agnostic caching with TTL expiry via the core/io Medium interface. --- # go-cache @@ -8,7 +8,7 @@ description: File-based caching with TTL expiry, storage-agnostic via the go-io `go-cache` is a lightweight, storage-agnostic caching library for Go. It stores JSON-serialised entries with automatic TTL expiry and path-traversal protection. -**Module path:** `forge.lthn.ai/core/go-cache` +**Module path:** `dappco.re/go/core/cache` **Licence:** EUPL-1.2 @@ -20,7 +20,7 @@ import ( "fmt" "time" - "forge.lthn.ai/core/go-cache" + "dappco.re/go/core/cache" ) func main() { @@ -68,11 +68,11 @@ func main() { | Module | Version | Role | |-------------------------------|---------|---------------------------------------------| -| `forge.lthn.ai/core/go-io` | v0.0.3 | Storage abstraction (`Medium` interface) | -| `forge.lthn.ai/core/go-log` | v0.0.1 | Structured logging (indirect, via `go-io`) | +| `dappco.re/go/core/io` | v0.2.0 | Storage abstraction (`Medium` interface) | +| `dappco.re/go/core/log` | v0.1.0 | Structured error wrapping used by the package | There are no other runtime dependencies. The test suite uses the standard -library only (plus the `MockMedium` from `go-io`). +library only (plus the `MockMedium` from `core/io`). ## Key Concepts @@ -80,12 +80,12 @@ library only (plus the `MockMedium` from `go-io`). ### Storage Backends The cache does not read or write files directly. All I/O goes through the -`io.Medium` interface defined in `go-io`. This means the same cache logic works +`io.Medium` interface defined in `core/io`. This means the same cache logic works against: - **Local filesystem** (`io.Local`) -- the default -- **SQLite KV store** (`store.Medium` from `go-io/store`) -- **S3-compatible storage** (`go-io/s3`) +- **SQLite KV store** (`store.Medium` from `dappco.re/go/core/io/store`) +- **S3-compatible storage** (`dappco.re/go/core/io/s3`) - **In-memory mock** (`io.NewMockMedium()`) -- ideal for tests Pass any `Medium` implementation as the first argument to `cache.New()`. @@ -107,5 +107,5 @@ cache.GitHubReposKey("host-uk") // "github/host-uk/repos" cache.GitHubRepoKey("host-uk", "core") // "github/host-uk/core/meta" ``` -These are convenience helpers used by other packages in the ecosystem (such as -`go-devops`) to avoid key duplication when caching GitHub responses. +These are convenience helpers used by other packages in the ecosystem to avoid +key duplication when caching GitHub responses. -- 2.45.3 From cc60ab0df38cebe0aae9dff52b1711129c8f79fb Mon Sep 17 00:00:00 2001 From: Virgil Date: Thu, 26 Mar 2026 17:08:09 +0000 Subject: [PATCH 2/6] Polish AX v0.8.0 cache package Co-Authored-By: Virgil --- cache.go | 162 ++++++++++++++++++++++++++++++------------------ cache_test.go | 167 ++++++++++++++++++++++++++++---------------------- go.mod | 2 +- go.sum | 4 +- 4 files changed, 201 insertions(+), 134 deletions(-) diff --git a/cache.go b/cache.go index cff8458..356628a 100644 --- a/cache.go +++ b/cache.go @@ -2,15 +2,11 @@ package cache import ( - "encoding/json" - "errors" - "os" - "path/filepath" - "strings" + "io/fs" "time" + "dappco.re/go/core" coreio "dappco.re/go/core/io" - coreerr "dappco.re/go/core/log" ) // DefaultTTL is the default cache expiry time. @@ -25,34 +21,37 @@ type Cache struct { // Entry is the serialized cache record written to the backing Medium. type Entry struct { - Data json.RawMessage `json:"data"` - CachedAt time.Time `json:"cached_at"` - ExpiresAt time.Time `json:"expires_at"` + Data any `json:"data"` + CachedAt time.Time `json:"cached_at"` + ExpiresAt time.Time `json:"expires_at"` } // New creates a cache and applies default Medium, base directory, and TTL values // when callers pass zero values. +// +// c, err := cache.New(coreio.Local, "/tmp/cache", time.Hour) func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error) { if medium == nil { medium = coreio.Local } if baseDir == "" { - // Use .core/cache in current working directory - cwd, err := os.Getwd() - if err != nil { - return nil, coreerr.E("cache.New", "failed to get working directory", err) + cwd := currentDir() + if cwd == "" || cwd == "." { + return nil, core.E("cache.New", "failed to resolve current working directory", nil) } - baseDir = filepath.Join(cwd, ".core", "cache") + + baseDir = normalizePath(core.JoinPath(cwd, ".core", "cache")) + } else { + baseDir = absolutePath(baseDir) } if ttl == 0 { ttl = DefaultTTL } - // Ensure cache directory exists if err := medium.EnsureDir(baseDir); err != nil { - return nil, coreerr.E("cache.New", "failed to create cache directory", err) + return nil, core.E("cache.New", "failed to create cache directory", err) } return &Cache{ @@ -64,27 +63,23 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error // Path returns the storage path used for key and rejects path traversal // attempts. +// +// path, err := c.Path("github/acme/repos") func (c *Cache) Path(key string) (string, error) { - path := filepath.Join(c.baseDir, key+".json") + baseDir := absolutePath(c.baseDir) + path := absolutePath(core.JoinPath(baseDir, key+".json")) + pathPrefix := normalizePath(core.Concat(baseDir, pathSeparator())) - // Ensure the resulting path is still within baseDir to prevent traversal attacks - absBase, err := filepath.Abs(c.baseDir) - if err != nil { - return "", coreerr.E("cache.Path", "failed to get absolute path for baseDir", err) - } - absPath, err := filepath.Abs(path) - if err != nil { - return "", coreerr.E("cache.Path", "failed to get absolute path for key", err) - } - - if !strings.HasPrefix(absPath, absBase+string(filepath.Separator)) && absPath != absBase { - return "", coreerr.E("cache.Path", "invalid cache key: path traversal attempt", nil) + if path != baseDir && !core.HasPrefix(path, pathPrefix) { + return "", core.E("cache.Path", "invalid cache key: path traversal attempt", nil) } return path, nil } // Get unmarshals the cached item into dest if it exists and has not expired. +// +// found, err := c.Get("github/acme/repos", &repos) func (c *Cache) Get(key string, dest any) (bool, error) { path, err := c.Path(key) if err != nil { @@ -93,67 +88,67 @@ func (c *Cache) Get(key string, dest any) (bool, error) { dataStr, err := c.medium.Read(path) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if core.Is(err, fs.ErrNotExist) { return false, nil } - return false, coreerr.E("cache.Get", "failed to read cache file", err) + return false, core.E("cache.Get", "failed to read cache file", err) } var entry Entry - if err := json.Unmarshal([]byte(dataStr), &entry); err != nil { - // Invalid cache file, treat as miss + entryResult := core.JSONUnmarshalString(dataStr, &entry) + if !entryResult.OK { return false, nil } - // Check expiry if time.Now().After(entry.ExpiresAt) { return false, nil } - // Unmarshal the actual data - if err := json.Unmarshal(entry.Data, dest); err != nil { - return false, coreerr.E("cache.Get", "failed to unmarshal cached data", err) + dataResult := core.JSONMarshal(entry.Data) + if !dataResult.OK { + return false, core.E("cache.Get", "failed to marshal cached data", dataResult.Value.(error)) + } + + if err := core.JSONUnmarshal(dataResult.Value.([]byte), dest); !err.OK { + return false, core.E("cache.Get", "failed to unmarshal cached data", err.Value.(error)) } return true, nil } // Set marshals data and stores it in the cache. +// +// err := c.Set("github/acme/repos", repos) func (c *Cache) Set(key string, data any) error { path, err := c.Path(key) if err != nil { return err } - // Ensure parent directory exists - if err := c.medium.EnsureDir(filepath.Dir(path)); err != nil { - return coreerr.E("cache.Set", "failed to create directory", err) - } - - // Marshal the data - dataBytes, err := json.Marshal(data) - if err != nil { - return coreerr.E("cache.Set", "failed to marshal data", err) + if err := c.medium.EnsureDir(core.PathDir(path)); err != nil { + return core.E("cache.Set", "failed to create directory", err) } entry := Entry{ - Data: dataBytes, + Data: data, CachedAt: time.Now(), ExpiresAt: time.Now().Add(c.ttl), } - entryBytes, err := json.MarshalIndent(entry, "", " ") - if err != nil { - return coreerr.E("cache.Set", "failed to marshal cache entry", err) + entryResult := core.JSONMarshal(entry) + if !entryResult.OK { + return core.E("cache.Set", "failed to marshal cache entry", entryResult.Value.(error)) } - if err := c.medium.Write(path, string(entryBytes)); err != nil { - return coreerr.E("cache.Set", "failed to write cache file", err) + if err := c.medium.Write(path, string(entryResult.Value.([]byte))); err != nil { + return core.E("cache.Set", "failed to write cache file", err) } return nil } // Delete removes the cached item for key. +// +// err := c.Delete("github/acme/repos") func (c *Cache) Delete(key string) error { path, err := c.Path(key) if err != nil { @@ -161,24 +156,28 @@ func (c *Cache) Delete(key string) error { } err = c.medium.Delete(path) - if errors.Is(err, os.ErrNotExist) { + if core.Is(err, fs.ErrNotExist) { return nil } if err != nil { - return coreerr.E("cache.Delete", "failed to delete cache file", err) + return core.E("cache.Delete", "failed to delete cache file", err) } return nil } // Clear removes all cached items under the cache base directory. +// +// err := c.Clear() func (c *Cache) Clear() error { if err := c.medium.DeleteAll(c.baseDir); err != nil { - return coreerr.E("cache.Clear", "failed to clear cache", err) + return core.E("cache.Clear", "failed to clear cache", err) } return nil } // Age reports how long ago key was cached, or -1 if it is missing or unreadable. +// +// age := c.Age("github/acme/repos") func (c *Cache) Age(key string) time.Duration { path, err := c.Path(key) if err != nil { @@ -191,7 +190,8 @@ func (c *Cache) Age(key string) time.Duration { } var entry Entry - if err := json.Unmarshal([]byte(dataStr), &entry); err != nil { + entryResult := core.JSONUnmarshalString(dataStr, &entry) + if !entryResult.OK { return -1 } @@ -201,11 +201,57 @@ func (c *Cache) Age(key string) time.Duration { // GitHub-specific cache keys // GitHubReposKey returns the cache key used for an organisation's repo list. +// +// key := cache.GitHubReposKey("acme") func GitHubReposKey(org string) string { - return filepath.Join("github", org, "repos") + return core.JoinPath("github", org, "repos") } // GitHubRepoKey returns the cache key used for a repository metadata entry. +// +// key := cache.GitHubRepoKey("acme", "widgets") func GitHubRepoKey(org, repo string) string { - return filepath.Join("github", org, repo, "meta") + return core.JoinPath("github", org, repo, "meta") +} + +func pathSeparator() string { + if ds := core.Env("DS"); ds != "" { + return ds + } + + return "/" +} + +func normalizePath(path string) string { + ds := pathSeparator() + normalized := core.Replace(path, "\\", ds) + + if ds != "/" { + normalized = core.Replace(normalized, "/", ds) + } + + return core.CleanPath(normalized, ds) +} + +func absolutePath(path string) string { + normalized := normalizePath(path) + if core.PathIsAbs(normalized) { + return normalized + } + + cwd := currentDir() + if cwd == "" || cwd == "." { + return normalized + } + + return normalizePath(core.JoinPath(cwd, normalized)) +} + +func currentDir() string { + cwd := normalizePath(core.Env("PWD")) + if cwd != "" && cwd != "." { + return cwd + } + + return normalizePath(core.Env("DIR_CWD")) } diff --git a/cache_test.go b/cache_test.go index 8314ce6..edb856c 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1,11 +1,10 @@ package cache_test import ( - "encoding/json" - "path/filepath" "testing" "time" + "dappco.re/go/core" "dappco.re/go/core/cache" coreio "dappco.re/go/core/io" ) @@ -22,7 +21,75 @@ func newTestCache(t *testing.T, baseDir string, ttl time.Duration) (*cache.Cache return c, m } -func TestCacheSetAndGet(t *testing.T) { +func readEntry(t *testing.T, raw string) cache.Entry { + t.Helper() + + var entry cache.Entry + result := core.JSONUnmarshalString(raw, &entry) + if !result.OK { + t.Fatalf("failed to unmarshal cache entry: %v", result.Value) + } + + return entry +} + +func TestCache_New_Good(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + c, m := newTestCache(t, "", 0) + + const key = "defaults" + if err := c.Set(key, map[string]string{"foo": "bar"}); err != nil { + t.Fatalf("Set failed: %v", err) + } + + path, err := c.Path(key) + if err != nil { + t.Fatalf("Path failed: %v", err) + } + + wantPath := core.JoinPath(tmpDir, ".core", "cache", key+".json") + if path != wantPath { + t.Fatalf("expected default path %q, got %q", wantPath, path) + } + + raw, err := m.Read(path) + if err != nil { + t.Fatalf("Read failed: %v", err) + } + + entry := readEntry(t, raw) + ttl := entry.ExpiresAt.Sub(entry.CachedAt) + if ttl < cache.DefaultTTL || ttl > cache.DefaultTTL+time.Second { + t.Fatalf("expected ttl near %v, got %v", cache.DefaultTTL, ttl) + } +} + +func TestCache_Path_Good(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-path", time.Minute) + + path, err := c.Path("github/acme/repos") + if err != nil { + t.Fatalf("Path failed: %v", err) + } + + want := "/tmp/cache-path/github/acme/repos.json" + if path != want { + t.Fatalf("expected path %q, got %q", want, path) + } +} + +func TestCache_Path_Bad(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) + + _, err := c.Path("../../etc/passwd") + if err == nil { + t.Fatal("expected error for path traversal key, got nil") + } +} + +func TestCache_Get_Good(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache", time.Minute) key := "test-key" @@ -45,7 +112,26 @@ func TestCacheSetAndGet(t *testing.T) { } } -func TestCacheAge(t *testing.T) { +func TestCache_Get_Ugly(t *testing.T) { + c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) + + if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + t.Fatalf("Set for expiry test failed: %v", err) + } + + time.Sleep(50 * time.Millisecond) + + var retrieved map[string]string + found, err := c.Get("test-key", &retrieved) + if err != nil { + t.Fatalf("Get for expired item returned an unexpected error: %v", err) + } + if found { + t.Error("expected item to be expired") + } +} + +func TestCache_Age_Good(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-age", time.Minute) if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { @@ -57,7 +143,7 @@ func TestCacheAge(t *testing.T) { } } -func TestCacheDelete(t *testing.T) { +func TestCache_Delete_Good(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-delete", time.Minute) if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { @@ -78,26 +164,7 @@ func TestCacheDelete(t *testing.T) { } } -func TestCacheExpiry(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) - - if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { - t.Fatalf("Set for expiry test failed: %v", err) - } - - time.Sleep(50 * time.Millisecond) - - var retrieved map[string]string - found, err := c.Get("test-key", &retrieved) - if err != nil { - t.Fatalf("Get for expired item returned an unexpected error: %v", err) - } - if found { - t.Error("expected item to be expired") - } -} - -func TestCacheClear(t *testing.T) { +func TestCache_Clear_Good(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute) data := map[string]string{"foo": "bar"} @@ -121,62 +188,16 @@ func TestCacheClear(t *testing.T) { } } -func TestCacheUsesDefaultBaseDirAndTTL(t *testing.T) { - tmpDir := t.TempDir() - t.Chdir(tmpDir) - - c, m := newTestCache(t, "", 0) - - const key = "defaults" - if err := c.Set(key, map[string]string{"foo": "bar"}); err != nil { - t.Fatalf("Set failed: %v", err) - } - - path, err := c.Path(key) - if err != nil { - t.Fatalf("Path failed: %v", err) - } - - wantPath := filepath.Join(tmpDir, ".core", "cache", key+".json") - if path != wantPath { - t.Fatalf("expected default path %q, got %q", wantPath, path) - } - - raw, err := m.Read(path) - if err != nil { - t.Fatalf("Read failed: %v", err) - } - - var entry cache.Entry - if err := json.Unmarshal([]byte(raw), &entry); err != nil { - t.Fatalf("failed to unmarshal cache entry: %v", err) - } - - ttl := entry.ExpiresAt.Sub(entry.CachedAt) - if ttl < cache.DefaultTTL || ttl > cache.DefaultTTL+time.Second { - t.Fatalf("expected ttl near %v, got %v", cache.DefaultTTL, ttl) - } -} - -func TestGitHubReposKey(t *testing.T) { +func TestCache_GitHubReposKey_Good(t *testing.T) { key := cache.GitHubReposKey("myorg") if key != "github/myorg/repos" { t.Errorf("unexpected GitHubReposKey: %q", key) } } -func TestGitHubRepoKey(t *testing.T) { +func TestCache_GitHubRepoKey_Good(t *testing.T) { key := cache.GitHubRepoKey("myorg", "myrepo") if key != "github/myorg/myrepo/meta" { t.Errorf("unexpected GitHubRepoKey: %q", key) } } - -func TestCacheRejectsPathTraversal(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) - - _, err := c.Path("../../etc/passwd") - if err == nil { - t.Error("expected error for path traversal key, got nil") - } -} diff --git a/go.mod b/go.mod index c7424fc..7ff2452 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module dappco.re/go/core/cache go 1.26.0 require ( + dappco.re/go/core v0.8.0-alpha.1 dappco.re/go/core/io v0.2.0 - dappco.re/go/core/log v0.1.0 ) require forge.lthn.ai/core/go-log v0.0.4 // indirect diff --git a/go.sum b/go.sum index 76d01ec..bfbbbf3 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ +dappco.re/go/core v0.8.0-alpha.1 h1:gj7+Scv+L63Z7wMxbJYHhaRFkHJo2u4MMPuUSv/Dhtk= +dappco.re/go/core v0.8.0-alpha.1/go.mod h1:f2/tBZ3+3IqDrg2F5F598llv0nmb/4gJVCFzM5geE4A= dappco.re/go/core/io v0.2.0 h1:zuudgIiTsQQ5ipVt97saWdGLROovbEB/zdVyy9/l+I4= dappco.re/go/core/io v0.2.0/go.mod h1:1QnQV6X9LNgFKfm8SkOtR9LLaj3bDcsOIeJOOyjbL5E= -dappco.re/go/core/log v0.1.0 h1:pa71Vq2TD2aoEUQWFKwNcaJ3GBY8HbaNGqtE688Unyc= -dappco.re/go/core/log v0.1.0/go.mod h1:Nkqb8gsXhZAO8VLpx7B8i1iAmohhzqA20b9Zr8VUcJs= forge.lthn.ai/core/go-log v0.0.4 h1:KTuCEPgFmuM8KJfnyQ8vPOU1Jg654W74h8IJvfQMfv0= forge.lthn.ai/core/go-log v0.0.4/go.mod h1:r14MXKOD3LF/sI8XUJQhRk/SZHBE7jAFVuCfgkXoZPw= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= -- 2.45.3 From e8105f73853930e8d324d834892e5d958dd07c31 Mon Sep 17 00:00:00 2001 From: Virgil Date: Fri, 27 Mar 2026 03:07:30 +0000 Subject: [PATCH 3/6] chore(cache): verification pass -- 2.45.3 From e5fbd275631669bd505663eb6db3bf02f73065d4 Mon Sep 17 00:00:00 2001 From: Virgil Date: Fri, 27 Mar 2026 04:21:31 +0000 Subject: [PATCH 4/6] fix(cache): complete AX compliance cleanup Co-Authored-By: Virgil --- cache.go | 5 ++-- cache_test.go | 20 +++++++-------- docs/architecture.md | 58 ++++++++++++++++++++++---------------------- docs/development.md | 20 +++++++++------ docs/index.md | 28 +++++++++------------ 5 files changed, 66 insertions(+), 65 deletions(-) diff --git a/cache.go b/cache.go index 356628a..2a14d11 100644 --- a/cache.go +++ b/cache.go @@ -129,10 +129,11 @@ func (c *Cache) Set(key string, data any) error { return core.E("cache.Set", "failed to create directory", err) } + now := time.Now() entry := Entry{ Data: data, - CachedAt: time.Now(), - ExpiresAt: time.Now().Add(c.ttl), + CachedAt: now, + ExpiresAt: now.Add(c.ttl), } entryResult := core.JSONMarshal(entry) diff --git a/cache_test.go b/cache_test.go index edb856c..8f1dfa1 100644 --- a/cache_test.go +++ b/cache_test.go @@ -33,7 +33,7 @@ func readEntry(t *testing.T, raw string) cache.Entry { return entry } -func TestCache_New_Good(t *testing.T) { +func TestCache_New_UsesDefaultBaseDirAndTTL(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) @@ -66,7 +66,7 @@ func TestCache_New_Good(t *testing.T) { } } -func TestCache_Path_Good(t *testing.T) { +func TestCache_Path_ReturnsStoragePath(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-path", time.Minute) path, err := c.Path("github/acme/repos") @@ -80,7 +80,7 @@ func TestCache_Path_Good(t *testing.T) { } } -func TestCache_Path_Bad(t *testing.T) { +func TestCache_Path_RejectsTraversal(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) _, err := c.Path("../../etc/passwd") @@ -89,7 +89,7 @@ func TestCache_Path_Bad(t *testing.T) { } } -func TestCache_Get_Good(t *testing.T) { +func TestCache_Get_ReturnsCachedValue(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache", time.Minute) key := "test-key" @@ -112,7 +112,7 @@ func TestCache_Get_Good(t *testing.T) { } } -func TestCache_Get_Ugly(t *testing.T) { +func TestCache_Get_TreatsExpiredEntryAsMiss(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { @@ -131,7 +131,7 @@ func TestCache_Get_Ugly(t *testing.T) { } } -func TestCache_Age_Good(t *testing.T) { +func TestCache_Age_ReturnsElapsedDuration(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-age", time.Minute) if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { @@ -143,7 +143,7 @@ func TestCache_Age_Good(t *testing.T) { } } -func TestCache_Delete_Good(t *testing.T) { +func TestCache_Delete_RemovesEntry(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-delete", time.Minute) if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { @@ -164,7 +164,7 @@ func TestCache_Delete_Good(t *testing.T) { } } -func TestCache_Clear_Good(t *testing.T) { +func TestCache_Clear_RemovesAllEntries(t *testing.T) { c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute) data := map[string]string{"foo": "bar"} @@ -188,14 +188,14 @@ func TestCache_Clear_Good(t *testing.T) { } } -func TestCache_GitHubReposKey_Good(t *testing.T) { +func TestCache_GitHubReposKey_ReturnsReposPath(t *testing.T) { key := cache.GitHubReposKey("myorg") if key != "github/myorg/repos" { t.Errorf("unexpected GitHubReposKey: %q", key) } } -func TestCache_GitHubRepoKey_Good(t *testing.T) { +func TestCache_GitHubRepoKey_ReturnsMetadataPath(t *testing.T) { key := cache.GitHubRepoKey("myorg", "myrepo") if key != "github/myorg/myrepo/meta" { t.Errorf("unexpected GitHubRepoKey: %q", key) diff --git a/docs/architecture.md b/docs/architecture.md index b2199b0..2a6132b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -6,7 +6,7 @@ description: Internals of go-cache -- types, data flow, storage format, and secu # Architecture This document explains how `go-cache` works internally, covering its type -system, on-disc format, data flow, and security considerations. +system, on-disk format, data flow, and security considerations. ## Core Types @@ -35,16 +35,16 @@ immutable for the lifetime of the instance. ```go type Entry struct { - Data json.RawMessage `json:"data"` - CachedAt time.Time `json:"cached_at"` - ExpiresAt time.Time `json:"expires_at"` + Data any `json:"data"` + CachedAt time.Time `json:"cached_at"` + ExpiresAt time.Time `json:"expires_at"` } ``` -`Entry` is the envelope written to storage. It wraps the caller's data as raw -JSON and adds two timestamps for expiry tracking. Using `json.RawMessage` means -the data payload is stored verbatim -- no intermediate deserialisation happens -during writes. +`Entry` is the envelope written to storage. It wraps the caller's value and +adds two timestamps for expiry tracking. The entry is JSON-encoded as a whole +when written, and `Data` is re-marshalled on reads before decoding into the +caller-provided destination value. ## Constructor Defaults @@ -70,23 +70,20 @@ directory exists before any reads or writes. caller data | v -json.Marshal(data) -- serialise caller's value - | - v wrap in Entry{ -- add timestamps - Data: , - CachedAt: time.Now(), - ExpiresAt: time.Now().Add(ttl), + Data: , + CachedAt: now, + ExpiresAt: now.Add(ttl), } | v -json.MarshalIndent(entry) -- human-readable JSON +core.JSONMarshal(entry) -- serialise the full cache entry | v medium.Write(path, string) -- persist via the storage backend ``` -The resulting file on disc (or equivalent record in another medium) looks like: +The resulting file on disk (or equivalent record in another medium) looks like: ```json { @@ -106,7 +103,7 @@ automatically via `medium.EnsureDir()`. medium.Read(path) | v -json.Unmarshal -> Entry -- parse the envelope +core.JSONUnmarshalString -> Entry -- parse the envelope | v time.Now().After(ExpiresAt)? -- check TTL @@ -114,21 +111,24 @@ time.Now().After(ExpiresAt)? -- check TTL yes no | | v v -return false json.Unmarshal(entry.Data, dest) +return false core.JSONMarshal(entry.Data) (cache miss) | v + core.JSONUnmarshal(..., dest) + | + v return true (cache hit) ``` Key behaviours: -- If the file does not exist (`os.ErrNotExist`), `Get` returns `(false, nil)` -- +- If the file does not exist (`fs.ErrNotExist`), `Get` returns `(false, nil)` -- a miss, not an error. - If the file contains invalid JSON, it is treated as a miss (not an error). This prevents corrupted files from blocking the caller. - If the entry exists but has expired, it is treated as a miss. The stale file - is **not** deleted eagerly -- it remains on disc until explicitly removed or + is **not** deleted eagerly -- it remains on disk until explicitly removed or overwritten. @@ -162,11 +162,11 @@ the GitHub key helpers work: ```go func GitHubReposKey(org string) string { - return filepath.Join("github", org, "repos") + return core.JoinPath("github", org, "repos") } func GitHubRepoKey(org, repo string) string { - return filepath.Join("github", org, repo, "meta") + return core.JoinPath("github", org, repo, "meta") } ``` @@ -174,12 +174,12 @@ func GitHubRepoKey(org, repo string) string { ## Security: Path Traversal Prevention The `Path()` method guards against directory traversal attacks. After computing -the full path, it resolves both the base directory and the result to absolute -paths, then checks that the result is still a prefix of the base: +the full path, it normalises both the base directory and the result to absolute +paths, then checks that the result still lives under the cache root: ```go -if !strings.HasPrefix(absPath, absBase) { - return "", coreerr.E("cache.Path", "invalid cache key: path traversal attempt", nil) +if path != baseDir && !core.HasPrefix(path, pathPrefix) { + return "", core.E("cache.Path", "invalid cache key: path traversal attempt", nil) } ``` @@ -201,10 +201,10 @@ goroutine (e.g. a CLI command fetching GitHub data) and read by others, which avoids contention. -## Relationship to go-io +## Relationship to core/io `go-cache` delegates all storage operations to the `io.Medium` interface from -`go-io`. It uses only five methods: +`dappco.re/go/core/io`. It uses only five methods: | Method | Used by | |--------------|---------------------| @@ -216,4 +216,4 @@ avoids contention. This minimal surface makes it straightforward to swap storage backends. For tests, `io.NewMockMedium()` provides a fully in-memory implementation with no -disc access. +disk access. diff --git a/docs/development.md b/docs/development.md index ffde628..0a71758 100644 --- a/docs/development.md +++ b/docs/development.md @@ -47,7 +47,7 @@ go test -run TestCache ./... ``` The test suite uses `io.NewMockMedium()` for all storage operations, so no -files are written to disc and tests run quickly in any environment. +files are written to disk and tests run quickly in any environment. ## Test Coverage @@ -102,19 +102,22 @@ Tests follow the standard Go testing conventions. The codebase uses 1. Use `io.NewMockMedium()` rather than the real filesystem. 2. Keep TTLs short (milliseconds) when testing expiry behaviour. -3. Name test functions descriptively: `TestCacheExpiry`, `TestCacheUsesDefaultBaseDirAndTTL`, etc. +3. Name test functions descriptively: `TestCache_Get_TreatsExpiredEntryAsMiss`, + `TestCache_New_UsesDefaultBaseDirAndTTL`, etc. Example of testing cache expiry: ```go -func TestCacheExpiry(t *testing.T) { +func TestCache_Get_TreatsExpiredEntryAsMiss(t *testing.T) { m := io.NewMockMedium() c, err := cache.New(m, "/tmp/test", 10*time.Millisecond) if err != nil { t.Fatalf("failed to create cache: %v", err) } - c.Set("key", "value") + if err := c.Set("key", "value"); err != nil { + t.Fatalf("failed to set cache entry: %v", err) + } time.Sleep(50 * time.Millisecond) var result string @@ -160,9 +163,10 @@ binaries. ## Adding a New Storage Backend To use the cache with a different storage medium, implement the `io.Medium` -interface from `go-io` and pass it to `cache.New()`. The cache only requires -five methods: `EnsureDir`, `Read`, `Write`, `Delete`, and `DeleteAll`. See -the [architecture](architecture.md) document for the full method mapping. +interface from `dappco.re/go/core/io` and pass it to `cache.New()`. The cache +only requires five methods: `EnsureDir`, `Read`, `Write`, `Delete`, and +`DeleteAll`. See the [architecture](architecture.md) document for the full +method mapping. ```go import ( @@ -175,7 +179,7 @@ import ( // Use SQLite as the cache backend medium, err := store.NewMedium("/path/to/cache.db") if err != nil { - panic(err) + return } c, err := cache.New(medium, "cache", 30*time.Minute) diff --git a/docs/index.md b/docs/index.md index 8b07c06..a5cf928 100644 --- a/docs/index.md +++ b/docs/index.md @@ -16,12 +16,7 @@ JSON-serialised entries with automatic TTL expiry and path-traversal protection. ## Quick Start ```go -import ( - "fmt" - "time" - - "dappco.re/go/core/cache" -) +import "dappco.re/go/core/cache" func main() { // Create a cache with default settings: @@ -30,7 +25,7 @@ func main() { // - TTL: 1 hour c, err := cache.New(nil, "", 0) if err != nil { - panic(err) + return } // Store a value @@ -39,17 +34,17 @@ func main() { "role": "admin", }) if err != nil { - panic(err) + return } // Retrieve it (returns false if missing or expired) var profile map[string]string found, err := c.Get("user/profile", &profile) if err != nil { - panic(err) + return } if found { - fmt.Println(profile["name"]) // Alice + _ = profile["name"] // Use the cached value. } } ``` @@ -66,13 +61,14 @@ func main() { ## Dependencies -| Module | Version | Role | -|-------------------------------|---------|---------------------------------------------| -| `dappco.re/go/core/io` | v0.2.0 | Storage abstraction (`Medium` interface) | -| `dappco.re/go/core/log` | v0.1.0 | Structured error wrapping used by the package | +| Module | Version | Role | +|------------------------|------------------|------------------------------------------------| +| `dappco.re/go/core` | `v0.8.0-alpha.1` | JSON, path, string, and error helper utilities | +| `dappco.re/go/core/io` | `v0.2.0` | Storage abstraction (`Medium` interface) | -There are no other runtime dependencies. The test suite uses the standard -library only (plus the `MockMedium` from `core/io`). +The cache package imports only `dappco.re/go/core` and `dappco.re/go/core/io`. +The resolved build list also includes transitive dependencies from `core/io`'s +optional storage backends. ## Key Concepts -- 2.45.3 From def33a27106fdadb2348d26d68904447c052a2b0 Mon Sep 17 00:00:00 2001 From: Virgil Date: Fri, 27 Mar 2026 19:45:39 +0000 Subject: [PATCH 5/6] docs(cache): add exported API RFC --- specs/RFC.md | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 specs/RFC.md diff --git a/specs/RFC.md b/specs/RFC.md new file mode 100644 index 0000000..b09b7ef --- /dev/null +++ b/specs/RFC.md @@ -0,0 +1,77 @@ +# cache + +**Import:** `dappco.re/go/core/cache`, **Files:** 1 + +## Types + +### `Cache` + +```go +type Cache struct { + // unexported fields +} +``` + +`Cache` stores JSON-encoded entries in a `coreio.Medium`-backed cache rooted at a base directory. Each key-based operation resolves the key through `Path`, so path traversal is rejected before any read, write, or delete reaches the backing medium. + +#### `func (c *Cache) Path(key string) (string, error)` + +`Path` returns the absolute storage path for `key` with a `.json` suffix. If the resolved path would escape the cache base directory, it returns an error and rejects the key. + +#### `func (c *Cache) Get(key string, dest any) (bool, error)` + +`Get` reads the cached entry for `key` and unmarshals the entry's `Data` field into `dest`. It returns `false, nil` when the cache file does not exist, the stored JSON cannot be decoded into an `Entry`, or the entry has expired. Expired entries are treated as cache misses and are not deleted by `Get`. It returns an error for non-`fs.ErrNotExist` read failures and for failures while re-marshalling or unmarshalling the cached payload into `dest`. + +#### `func (c *Cache) Set(key string, data any) error` + +`Set` resolves the key, ensures the parent directory exists, wraps `data` in an `Entry`, and writes the JSON-encoded entry to the backing medium. `CachedAt` is set to the current time and `ExpiresAt` is set to `CachedAt + ttl`. + +#### `func (c *Cache) Delete(key string) error` + +`Delete` removes the cached item for `key`. A missing file is treated as success; other delete failures are wrapped and returned. + +#### `func (c *Cache) Clear() error` + +`Clear` deletes all cached data under the cache base directory by calling `DeleteAll` on the backing medium. + +#### `func (c *Cache) Age(key string) time.Duration` + +`Age` returns `time.Since(entry.CachedAt)` for the cached item at `key`. It does not check `ExpiresAt`, so expired entries still report their age if they can be read and decoded. It returns `-1` when the key is invalid, the file cannot be read, or the stored JSON cannot be decoded into an `Entry`. + +### `Entry` + +```go +type Entry struct { + Data any `json:"data"` + CachedAt time.Time `json:"cached_at"` + ExpiresAt time.Time `json:"expires_at"` +} +``` + +`Entry` is the serialized record written to the backing medium. `Data` holds the caller value, `CachedAt` records when it was written, and `ExpiresAt` is the cutoff used by `Get` to decide whether the entry is still valid. + +## Functions + +### `New` + +```go +func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error) +``` + +`New` creates a `Cache` and applies defaults for zero-valued arguments. When `medium` is `nil`, it uses `coreio.Local`. When `baseDir` is empty, it resolves the current working directory and uses `.core/cache` beneath it. When `baseDir` is provided, it is normalized to an absolute path. When `ttl` is `0`, it uses `DefaultTTL`. Before returning, it ensures the base directory exists in the backing medium. It returns an error if the working directory cannot be resolved for the default base directory or if directory creation fails. + +### `GitHubReposKey` + +```go +func GitHubReposKey(org string) string +``` + +`GitHubReposKey` returns the cache key for an organisation repository listing in the form `github/{org}/repos`. + +### `GitHubRepoKey` + +```go +func GitHubRepoKey(org, repo string) string +``` + +`GitHubRepoKey` returns the cache key for repository metadata in the form `github/{org}/{repo}/meta`. -- 2.45.3 From 9cc16d5792a0e66958a3987ec05531b67dfa94a6 Mon Sep 17 00:00:00 2001 From: Virgil Date: Sun, 29 Mar 2026 15:16:49 +0000 Subject: [PATCH 6/6] refactor(cache): standardize internal naming and remove shorthand identifiers Co-Authored-By: Virgil --- cache.go | 122 +++++++++++++++++++++++++------------------------- cache_test.go | 58 ++++++++++++------------ 2 files changed, 90 insertions(+), 90 deletions(-) diff --git a/cache.go b/cache.go index 2a14d11..9e8a7b0 100644 --- a/cache.go +++ b/cache.go @@ -16,7 +16,7 @@ const DefaultTTL = 1 * time.Hour type Cache struct { medium coreio.Medium baseDir string - ttl time.Duration + timeToLive time.Duration } // Entry is the serialized cache record written to the backing Medium. @@ -30,24 +30,24 @@ type Entry struct { // when callers pass zero values. // // c, err := cache.New(coreio.Local, "/tmp/cache", time.Hour) -func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error) { +func New(medium coreio.Medium, baseDir string, timeToLive time.Duration) (*Cache, error) { if medium == nil { medium = coreio.Local } if baseDir == "" { - cwd := currentDir() - if cwd == "" || cwd == "." { + currentWorkingDirectory := currentWorkingDirectory() + if currentWorkingDirectory == "" || currentWorkingDirectory == "." { return nil, core.E("cache.New", "failed to resolve current working directory", nil) } - baseDir = normalizePath(core.JoinPath(cwd, ".core", "cache")) + baseDir = normalizePath(core.JoinPath(currentWorkingDirectory, ".core", "cache")) } else { baseDir = absolutePath(baseDir) } - if ttl == 0 { - ttl = DefaultTTL + if timeToLive == 0 { + timeToLive = DefaultTTL } if err := medium.EnsureDir(baseDir); err != nil { @@ -55,9 +55,9 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error } return &Cache{ - medium: medium, - baseDir: baseDir, - ttl: ttl, + medium: medium, + baseDir: baseDir, + timeToLive: timeToLive, }, nil } @@ -65,28 +65,28 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error // attempts. // // path, err := c.Path("github/acme/repos") -func (c *Cache) Path(key string) (string, error) { - baseDir := absolutePath(c.baseDir) - path := absolutePath(core.JoinPath(baseDir, key+".json")) - pathPrefix := normalizePath(core.Concat(baseDir, pathSeparator())) +func (cacheInstance *Cache) Path(key string) (string, error) { + cacheBaseDir := absolutePath(cacheInstance.baseDir) + cachePath := absolutePath(core.JoinPath(cacheBaseDir, key+".json")) + cachePathPrefix := normalizePath(core.Concat(cacheBaseDir, pathSeparator())) - if path != baseDir && !core.HasPrefix(path, pathPrefix) { + if cachePath != cacheBaseDir && !core.HasPrefix(cachePath, cachePathPrefix) { return "", core.E("cache.Path", "invalid cache key: path traversal attempt", nil) } - return path, nil + return cachePath, nil } // Get unmarshals the cached item into dest if it exists and has not expired. // // found, err := c.Get("github/acme/repos", &repos) -func (c *Cache) Get(key string, dest any) (bool, error) { - path, err := c.Path(key) +func (cacheInstance *Cache) Get(key string, dest any) (bool, error) { + cachePath, err := cacheInstance.Path(key) if err != nil { return false, err } - dataStr, err := c.medium.Read(path) + rawData, err := cacheInstance.medium.Read(cachePath) if err != nil { if core.Is(err, fs.ErrNotExist) { return false, nil @@ -95,8 +95,8 @@ func (c *Cache) Get(key string, dest any) (bool, error) { } var entry Entry - entryResult := core.JSONUnmarshalString(dataStr, &entry) - if !entryResult.OK { + entryUnmarshalResult := core.JSONUnmarshalString(rawData, &entry) + if !entryUnmarshalResult.OK { return false, nil } @@ -104,12 +104,12 @@ func (c *Cache) Get(key string, dest any) (bool, error) { return false, nil } - dataResult := core.JSONMarshal(entry.Data) - if !dataResult.OK { - return false, core.E("cache.Get", "failed to marshal cached data", dataResult.Value.(error)) + dataMarshalResult := core.JSONMarshal(entry.Data) + if !dataMarshalResult.OK { + return false, core.E("cache.Get", "failed to marshal cached data", dataMarshalResult.Value.(error)) } - if err := core.JSONUnmarshal(dataResult.Value.([]byte), dest); !err.OK { + if err := core.JSONUnmarshal(dataMarshalResult.Value.([]byte), dest); !err.OK { return false, core.E("cache.Get", "failed to unmarshal cached data", err.Value.(error)) } @@ -119,13 +119,13 @@ func (c *Cache) Get(key string, dest any) (bool, error) { // Set marshals data and stores it in the cache. // // err := c.Set("github/acme/repos", repos) -func (c *Cache) Set(key string, data any) error { - path, err := c.Path(key) +func (cacheInstance *Cache) Set(key string, data any) error { + cachePath, err := cacheInstance.Path(key) if err != nil { return err } - if err := c.medium.EnsureDir(core.PathDir(path)); err != nil { + if err := cacheInstance.medium.EnsureDir(core.PathDir(cachePath)); err != nil { return core.E("cache.Set", "failed to create directory", err) } @@ -133,15 +133,15 @@ func (c *Cache) Set(key string, data any) error { entry := Entry{ Data: data, CachedAt: now, - ExpiresAt: now.Add(c.ttl), + ExpiresAt: now.Add(cacheInstance.timeToLive), } - entryResult := core.JSONMarshal(entry) - if !entryResult.OK { - return core.E("cache.Set", "failed to marshal cache entry", entryResult.Value.(error)) + entryMarshalResult := core.JSONMarshal(entry) + if !entryMarshalResult.OK { + return core.E("cache.Set", "failed to marshal cache entry", entryMarshalResult.Value.(error)) } - if err := c.medium.Write(path, string(entryResult.Value.([]byte))); err != nil { + if err := cacheInstance.medium.Write(cachePath, string(entryMarshalResult.Value.([]byte))); err != nil { return core.E("cache.Set", "failed to write cache file", err) } return nil @@ -150,13 +150,13 @@ func (c *Cache) Set(key string, data any) error { // Delete removes the cached item for key. // // err := c.Delete("github/acme/repos") -func (c *Cache) Delete(key string) error { - path, err := c.Path(key) +func (cacheInstance *Cache) Delete(key string) error { + cachePath, err := cacheInstance.Path(key) if err != nil { return err } - err = c.medium.Delete(path) + err = cacheInstance.medium.Delete(cachePath) if core.Is(err, fs.ErrNotExist) { return nil } @@ -169,8 +169,8 @@ func (c *Cache) Delete(key string) error { // Clear removes all cached items under the cache base directory. // // err := c.Clear() -func (c *Cache) Clear() error { - if err := c.medium.DeleteAll(c.baseDir); err != nil { +func (cacheInstance *Cache) Clear() error { + if err := cacheInstance.medium.DeleteAll(cacheInstance.baseDir); err != nil { return core.E("cache.Clear", "failed to clear cache", err) } return nil @@ -179,20 +179,20 @@ func (c *Cache) Clear() error { // Age reports how long ago key was cached, or -1 if it is missing or unreadable. // // age := c.Age("github/acme/repos") -func (c *Cache) Age(key string) time.Duration { - path, err := c.Path(key) +func (cacheInstance *Cache) Age(key string) time.Duration { + cachePath, err := cacheInstance.Path(key) if err != nil { return -1 } - dataStr, err := c.medium.Read(path) + rawData, err := cacheInstance.medium.Read(cachePath) if err != nil { return -1 } var entry Entry - entryResult := core.JSONUnmarshalString(dataStr, &entry) - if !entryResult.OK { + entryUnmarshalResult := core.JSONUnmarshalString(rawData, &entry) + if !entryUnmarshalResult.OK { return -1 } @@ -216,42 +216,42 @@ func GitHubRepoKey(org, repo string) string { } func pathSeparator() string { - if ds := core.Env("DS"); ds != "" { - return ds + if directorySeparator := core.Env("DS"); directorySeparator != "" { + return directorySeparator } return "/" } func normalizePath(path string) string { - ds := pathSeparator() - normalized := core.Replace(path, "\\", ds) + directorySeparator := pathSeparator() + normalized := core.Replace(path, "\\", directorySeparator) - if ds != "/" { - normalized = core.Replace(normalized, "/", ds) + if directorySeparator != "/" { + normalized = core.Replace(normalized, "/", directorySeparator) } - return core.CleanPath(normalized, ds) + return core.CleanPath(normalized, directorySeparator) } func absolutePath(path string) string { - normalized := normalizePath(path) - if core.PathIsAbs(normalized) { - return normalized + normalizedPath := normalizePath(path) + if core.PathIsAbs(normalizedPath) { + return normalizedPath } - cwd := currentDir() - if cwd == "" || cwd == "." { - return normalized + currentWorkingDirectory := currentWorkingDirectory() + if currentWorkingDirectory == "" || currentWorkingDirectory == "." { + return normalizedPath } - return normalizePath(core.JoinPath(cwd, normalized)) + return normalizePath(core.JoinPath(currentWorkingDirectory, normalizedPath)) } -func currentDir() string { - cwd := normalizePath(core.Env("PWD")) - if cwd != "" && cwd != "." { - return cwd +func currentWorkingDirectory() string { + workingDirectory := normalizePath(core.Env("PWD")) + if workingDirectory != "" && workingDirectory != "." { + return workingDirectory } return normalizePath(core.Env("DIR_CWD")) diff --git a/cache_test.go b/cache_test.go index 8f1dfa1..461f499 100644 --- a/cache_test.go +++ b/cache_test.go @@ -12,13 +12,13 @@ import ( func newTestCache(t *testing.T, baseDir string, ttl time.Duration) (*cache.Cache, *coreio.MockMedium) { t.Helper() - m := coreio.NewMockMedium() - c, err := cache.New(m, baseDir, ttl) + mockMedium := coreio.NewMockMedium() + cacheInstance, err := cache.New(mockMedium, baseDir, ttl) if err != nil { t.Fatalf("failed to create cache: %v", err) } - return c, m + return cacheInstance, mockMedium } func readEntry(t *testing.T, raw string) cache.Entry { @@ -37,14 +37,14 @@ func TestCache_New_UsesDefaultBaseDirAndTTL(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) - c, m := newTestCache(t, "", 0) + cacheInstance, mockMedium := newTestCache(t, "", 0) const key = "defaults" - if err := c.Set(key, map[string]string{"foo": "bar"}); err != nil { + if err := cacheInstance.Set(key, map[string]string{"foo": "bar"}); err != nil { t.Fatalf("Set failed: %v", err) } - path, err := c.Path(key) + path, err := cacheInstance.Path(key) if err != nil { t.Fatalf("Path failed: %v", err) } @@ -54,7 +54,7 @@ func TestCache_New_UsesDefaultBaseDirAndTTL(t *testing.T) { t.Fatalf("expected default path %q, got %q", wantPath, path) } - raw, err := m.Read(path) + raw, err := mockMedium.Read(path) if err != nil { t.Fatalf("Read failed: %v", err) } @@ -67,9 +67,9 @@ func TestCache_New_UsesDefaultBaseDirAndTTL(t *testing.T) { } func TestCache_Path_ReturnsStoragePath(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-path", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache-path", time.Minute) - path, err := c.Path("github/acme/repos") + path, err := cacheInstance.Path("github/acme/repos") if err != nil { t.Fatalf("Path failed: %v", err) } @@ -81,26 +81,26 @@ func TestCache_Path_ReturnsStoragePath(t *testing.T) { } func TestCache_Path_RejectsTraversal(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache-traversal", time.Minute) - _, err := c.Path("../../etc/passwd") + _, err := cacheInstance.Path("../../etc/passwd") if err == nil { t.Fatal("expected error for path traversal key, got nil") } } func TestCache_Get_ReturnsCachedValue(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache", time.Minute) key := "test-key" data := map[string]string{"foo": "bar"} - if err := c.Set(key, data); err != nil { + if err := cacheInstance.Set(key, data); err != nil { t.Fatalf("Set failed: %v", err) } var retrieved map[string]string - found, err := c.Get(key, &retrieved) + found, err := cacheInstance.Get(key, &retrieved) if err != nil { t.Fatalf("Get failed: %v", err) } @@ -113,16 +113,16 @@ func TestCache_Get_ReturnsCachedValue(t *testing.T) { } func TestCache_Get_TreatsExpiredEntryAsMiss(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) + cacheInstance, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond) - if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + if err := cacheInstance.Set("test-key", map[string]string{"foo": "bar"}); err != nil { t.Fatalf("Set for expiry test failed: %v", err) } time.Sleep(50 * time.Millisecond) var retrieved map[string]string - found, err := c.Get("test-key", &retrieved) + found, err := cacheInstance.Get("test-key", &retrieved) if err != nil { t.Fatalf("Get for expired item returned an unexpected error: %v", err) } @@ -132,30 +132,30 @@ func TestCache_Get_TreatsExpiredEntryAsMiss(t *testing.T) { } func TestCache_Age_ReturnsElapsedDuration(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-age", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache-age", time.Minute) - if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + if err := cacheInstance.Set("test-key", map[string]string{"foo": "bar"}); err != nil { t.Fatalf("Set failed: %v", err) } - if age := c.Age("test-key"); age < 0 { + if age := cacheInstance.Age("test-key"); age < 0 { t.Errorf("expected age >= 0, got %v", age) } } func TestCache_Delete_RemovesEntry(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-delete", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache-delete", time.Minute) - if err := c.Set("test-key", map[string]string{"foo": "bar"}); err != nil { + if err := cacheInstance.Set("test-key", map[string]string{"foo": "bar"}); err != nil { t.Fatalf("Set failed: %v", err) } - if err := c.Delete("test-key"); err != nil { + if err := cacheInstance.Delete("test-key"); err != nil { t.Fatalf("Delete failed: %v", err) } var retrieved map[string]string - found, err := c.Get("test-key", &retrieved) + found, err := cacheInstance.Get("test-key", &retrieved) if err != nil { t.Fatalf("Get after delete returned an unexpected error: %v", err) } @@ -165,21 +165,21 @@ func TestCache_Delete_RemovesEntry(t *testing.T) { } func TestCache_Clear_RemovesAllEntries(t *testing.T) { - c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute) + cacheInstance, _ := newTestCache(t, "/tmp/cache-clear", time.Minute) data := map[string]string{"foo": "bar"} - if err := c.Set("key1", data); err != nil { + if err := cacheInstance.Set("key1", data); err != nil { t.Fatalf("Set for clear test failed for key1: %v", err) } - if err := c.Set("key2", data); err != nil { + if err := cacheInstance.Set("key2", data); err != nil { t.Fatalf("Set for clear test failed for key2: %v", err) } - if err := c.Clear(); err != nil { + if err := cacheInstance.Clear(); err != nil { t.Fatalf("Clear failed: %v", err) } var retrieved map[string]string - found, err := c.Get("key1", &retrieved) + found, err := cacheInstance.Get("key1", &retrieved) if err != nil { t.Fatalf("Get after clear returned an unexpected error: %v", err) } -- 2.45.3