refactor: address ax review feedback

This commit is contained in:
Virgil 2026-03-26 11:15:00 +00:00
parent 4b36f935dd
commit b570845e82
4 changed files with 130 additions and 79 deletions

View file

@ -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")
}

View file

@ -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")
}

View file

@ -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

View file

@ -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.