refactor(cache): AX compliance pass 1
Remove banned encoding/json import; Entry.Data is now string using core.JSONMarshal/JSONMarshalString. Rename ds→separator, cwd→workingDirectory throughout. Add missing Good/Bad/Ugly tests for all public functions (29 tests, all pass). Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Claude
parent
fbf410e630
commit
1aed594690
2 changed files with 222 additions and 28 deletions
55
cache.go
55
cache.go
|
|
@ -4,7 +4,6 @@
|
|||
package cache
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"io/fs"
|
||||
"time"
|
||||
|
||||
|
|
@ -28,9 +27,9 @@ 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 string `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
|
||||
|
|
@ -43,12 +42,12 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error
|
|||
}
|
||||
|
||||
if baseDir == "" {
|
||||
cwd := currentDir()
|
||||
if cwd == "" || cwd == "." {
|
||||
workingDirectory := currentDir()
|
||||
if workingDirectory == "" || workingDirectory == "." {
|
||||
return nil, core.E("cache.New", "failed to resolve current working directory", nil)
|
||||
}
|
||||
|
||||
baseDir = normalizePath(core.JoinPath(cwd, ".core", "cache"))
|
||||
baseDir = normalizePath(core.JoinPath(workingDirectory, ".core", "cache"))
|
||||
} else {
|
||||
baseDir = absolutePath(baseDir)
|
||||
}
|
||||
|
|
@ -123,8 +122,8 @@ func (c *Cache) Get(key string, dest any) (bool, error) {
|
|||
return false, nil
|
||||
}
|
||||
|
||||
if err := core.JSONUnmarshal(entry.Data, dest); !err.OK {
|
||||
return false, core.E("cache.Get", "failed to unmarshal cached data", err.Value.(error))
|
||||
if result := core.JSONUnmarshalString(entry.Data, dest); !result.OK {
|
||||
return false, core.E("cache.Get", "failed to unmarshal cached data", result.Value.(error))
|
||||
}
|
||||
|
||||
return true, nil
|
||||
|
|
@ -152,18 +151,18 @@ func (c *Cache) Set(key string, data any) error {
|
|||
return core.E("cache.Set", "failed to marshal cache data", dataResult.Value.(error))
|
||||
}
|
||||
|
||||
ttl := c.ttl
|
||||
if ttl < 0 {
|
||||
cacheTTL := c.ttl
|
||||
if cacheTTL < 0 {
|
||||
return core.E("cache.Set", "cache ttl must be >= 0", nil)
|
||||
}
|
||||
if ttl == 0 {
|
||||
ttl = DefaultTTL
|
||||
if cacheTTL == 0 {
|
||||
cacheTTL = DefaultTTL
|
||||
}
|
||||
|
||||
entry := Entry{
|
||||
Data: dataResult.Value.([]byte),
|
||||
Data: string(dataResult.Value.([]byte)),
|
||||
CachedAt: time.Now(),
|
||||
ExpiresAt: time.Now().Add(ttl),
|
||||
ExpiresAt: time.Now().Add(cacheTTL),
|
||||
}
|
||||
|
||||
entryResult := core.JSONMarshal(entry)
|
||||
|
|
@ -258,22 +257,22 @@ func GitHubRepoKey(org, repo string) string {
|
|||
}
|
||||
|
||||
func pathSeparator() string {
|
||||
if ds := core.Env("DS"); ds != "" {
|
||||
return ds
|
||||
if separator := core.Env("DS"); separator != "" {
|
||||
return separator
|
||||
}
|
||||
|
||||
return "/"
|
||||
}
|
||||
|
||||
func normalizePath(path string) string {
|
||||
ds := pathSeparator()
|
||||
normalized := core.Replace(path, "\\", ds)
|
||||
separator := pathSeparator()
|
||||
normalized := core.Replace(path, "\\", separator)
|
||||
|
||||
if ds != "/" {
|
||||
normalized = core.Replace(normalized, "/", ds)
|
||||
if separator != "/" {
|
||||
normalized = core.Replace(normalized, "/", separator)
|
||||
}
|
||||
|
||||
return core.CleanPath(normalized, ds)
|
||||
return core.CleanPath(normalized, separator)
|
||||
}
|
||||
|
||||
func absolutePath(path string) string {
|
||||
|
|
@ -282,18 +281,18 @@ func absolutePath(path string) string {
|
|||
return normalized
|
||||
}
|
||||
|
||||
cwd := currentDir()
|
||||
if cwd == "" || cwd == "." {
|
||||
workingDirectory := currentDir()
|
||||
if workingDirectory == "" || workingDirectory == "." {
|
||||
return normalized
|
||||
}
|
||||
|
||||
return normalizePath(core.JoinPath(cwd, normalized))
|
||||
return normalizePath(core.JoinPath(workingDirectory, normalized))
|
||||
}
|
||||
|
||||
func currentDir() string {
|
||||
cwd := normalizePath(core.Env("PWD"))
|
||||
if cwd != "" && cwd != "." {
|
||||
return cwd
|
||||
workingDirectory := normalizePath(core.Env("PWD"))
|
||||
if workingDirectory != "" && workingDirectory != "." {
|
||||
return workingDirectory
|
||||
}
|
||||
|
||||
return normalizePath(core.Env("DIR_CWD"))
|
||||
|
|
|
|||
195
cache_test.go
195
cache_test.go
|
|
@ -75,6 +75,32 @@ func TestCache_New_Bad(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_New_Ugly(t *testing.T) {
|
||||
// New with zero ttl falls back to DefaultTTL; verify a set entry uses it.
|
||||
c, m := newTestCache(t, "/tmp/cache-default-ttl", 0)
|
||||
|
||||
const key = "ugly-key"
|
||||
if err := c.Set(key, map[string]string{"x": "y"}); err != nil {
|
||||
t.Fatalf("Set failed: %v", err)
|
||||
}
|
||||
|
||||
path, err := c.Path(key)
|
||||
if err != nil {
|
||||
t.Fatalf("Path failed: %v", err)
|
||||
}
|
||||
|
||||
raw, err := m.Read(path)
|
||||
if err != nil {
|
||||
t.Fatalf("Read failed: %v", err)
|
||||
}
|
||||
|
||||
entry := readEntry(t, raw)
|
||||
appliedTTL := entry.ExpiresAt.Sub(entry.CachedAt)
|
||||
if appliedTTL < cache.DefaultTTL || appliedTTL > cache.DefaultTTL+time.Second {
|
||||
t.Fatalf("expected ttl near %v, got %v", cache.DefaultTTL, appliedTTL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Path_Good(t *testing.T) {
|
||||
c, _ := newTestCache(t, "/tmp/cache-path", time.Minute)
|
||||
|
||||
|
|
@ -98,6 +124,15 @@ func TestCache_Path_Bad(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_Path_Ugly(t *testing.T) {
|
||||
// Path on a nil receiver returns an error rather than panicking.
|
||||
var c *cache.Cache
|
||||
_, err := c.Path("any-key")
|
||||
if err == nil {
|
||||
t.Fatal("expected Path to fail on nil receiver")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Get_Good(t *testing.T) {
|
||||
c, _ := newTestCache(t, "/tmp/cache", time.Minute)
|
||||
|
||||
|
|
@ -121,6 +156,20 @@ func TestCache_Get_Good(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_Get_Bad(t *testing.T) {
|
||||
// Get on a missing key returns (false, nil) — not an error.
|
||||
c, _ := newTestCache(t, "/tmp/cache-get-bad", time.Minute)
|
||||
|
||||
var retrieved map[string]string
|
||||
found, err := c.Get("nonexistent-key", &retrieved)
|
||||
if err != nil {
|
||||
t.Fatalf("expected no error for missing key, got: %v", err)
|
||||
}
|
||||
if found {
|
||||
t.Fatal("expected found=false for missing key")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Get_Ugly(t *testing.T) {
|
||||
c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond)
|
||||
|
||||
|
|
@ -152,6 +201,84 @@ func TestCache_Age_Good(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_Age_Bad(t *testing.T) {
|
||||
// Age returns -1 for a key that was never written.
|
||||
c, _ := newTestCache(t, "/tmp/cache-age-bad", time.Minute)
|
||||
|
||||
if age := c.Age("missing-key"); age != -1 {
|
||||
t.Errorf("expected -1 for missing key, got %v", age)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Age_Ugly(t *testing.T) {
|
||||
// Age returns -1 when the stored file contains corrupt JSON.
|
||||
c, medium := newTestCache(t, "/tmp/cache-age-ugly", time.Minute)
|
||||
|
||||
path, err := c.Path("corrupt-key")
|
||||
if err != nil {
|
||||
t.Fatalf("Path failed: %v", err)
|
||||
}
|
||||
|
||||
medium.Files[path] = "not-valid-json"
|
||||
|
||||
if age := c.Age("corrupt-key"); age != -1 {
|
||||
t.Errorf("expected -1 for corrupt entry, got %v", age)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Set_Good(t *testing.T) {
|
||||
// Set writes a valid entry that Get can later retrieve.
|
||||
c, _ := newTestCache(t, "/tmp/cache-set-good", time.Minute)
|
||||
|
||||
if err := c.Set("set-key", map[string]int{"count": 42}); err != nil {
|
||||
t.Fatalf("Set failed: %v", err)
|
||||
}
|
||||
|
||||
var retrieved map[string]int
|
||||
found, err := c.Get("set-key", &retrieved)
|
||||
if err != nil {
|
||||
t.Fatalf("Get after Set failed: %v", err)
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("expected to find item after Set")
|
||||
}
|
||||
if retrieved["count"] != 42 {
|
||||
t.Errorf("expected count=42, got %v", retrieved["count"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Set_Bad(t *testing.T) {
|
||||
// Set on a nil receiver returns an error rather than panicking.
|
||||
var c *cache.Cache
|
||||
if err := c.Set("key", map[string]string{"a": "b"}); err == nil {
|
||||
t.Fatal("expected Set to fail on nil receiver")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Set_Ugly(t *testing.T) {
|
||||
// Set overwrites an existing entry; subsequent Get returns the new value.
|
||||
c, _ := newTestCache(t, "/tmp/cache-set-ugly", time.Minute)
|
||||
|
||||
if err := c.Set("overwrite-key", map[string]string{"v": "first"}); err != nil {
|
||||
t.Fatalf("first Set failed: %v", err)
|
||||
}
|
||||
if err := c.Set("overwrite-key", map[string]string{"v": "second"}); err != nil {
|
||||
t.Fatalf("second Set failed: %v", err)
|
||||
}
|
||||
|
||||
var retrieved map[string]string
|
||||
found, err := c.Get("overwrite-key", &retrieved)
|
||||
if err != nil {
|
||||
t.Fatalf("Get failed: %v", err)
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("expected to find item")
|
||||
}
|
||||
if retrieved["v"] != "second" {
|
||||
t.Errorf("expected v=second after overwrite, got %q", retrieved["v"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_NilReceiver_Good(t *testing.T) {
|
||||
var c *cache.Cache
|
||||
var target map[string]string
|
||||
|
|
@ -231,6 +358,24 @@ func TestCache_Delete_Good(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_Delete_Bad(t *testing.T) {
|
||||
// Delete of a key that does not exist returns nil — idempotent.
|
||||
c, _ := newTestCache(t, "/tmp/cache-delete-bad", time.Minute)
|
||||
|
||||
if err := c.Delete("nonexistent-key"); err != nil {
|
||||
t.Fatalf("expected Delete of missing key to return nil, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Delete_Ugly(t *testing.T) {
|
||||
// Delete rejects a path traversal key with an error.
|
||||
c, _ := newTestCache(t, "/tmp/cache-delete-ugly", time.Minute)
|
||||
|
||||
if err := c.Delete("../../etc/shadow"); err == nil {
|
||||
t.Fatal("expected Delete to reject path traversal key")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Clear_Good(t *testing.T) {
|
||||
c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute)
|
||||
data := map[string]string{"foo": "bar"}
|
||||
|
|
@ -255,6 +400,23 @@ func TestCache_Clear_Good(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_Clear_Bad(t *testing.T) {
|
||||
// Clear on a nil receiver returns an error rather than panicking.
|
||||
var c *cache.Cache
|
||||
if err := c.Clear(); err == nil {
|
||||
t.Fatal("expected Clear to fail on nil receiver")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_Clear_Ugly(t *testing.T) {
|
||||
// Clear on an empty cache (no entries written) returns nil — idempotent.
|
||||
c, _ := newTestCache(t, "/tmp/cache-clear-ugly", time.Minute)
|
||||
|
||||
if err := c.Clear(); err != nil {
|
||||
t.Fatalf("expected Clear on empty cache to return nil, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_GitHubReposKey_Good(t *testing.T) {
|
||||
key := cache.GitHubReposKey("myorg")
|
||||
if key != "github/myorg/repos" {
|
||||
|
|
@ -262,9 +424,42 @@ func TestCache_GitHubReposKey_Good(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCache_GitHubReposKey_Bad(t *testing.T) {
|
||||
// GitHubReposKey with an empty org still produces a structurally valid path.
|
||||
key := cache.GitHubReposKey("")
|
||||
if key == "" {
|
||||
t.Error("expected a non-empty path even for empty org")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_GitHubReposKey_Ugly(t *testing.T) {
|
||||
// GitHubReposKey with an org containing slashes produces a deterministic path.
|
||||
key := cache.GitHubReposKey("org/sub")
|
||||
if key == "" {
|
||||
t.Error("expected a non-empty path for org with slash")
|
||||
}
|
||||
}
|
||||
|
||||
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 TestCache_GitHubRepoKey_Bad(t *testing.T) {
|
||||
// GitHubRepoKey with empty args still returns a non-empty path.
|
||||
key := cache.GitHubRepoKey("", "")
|
||||
if key == "" {
|
||||
t.Error("expected a non-empty path even for empty org and repo")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCache_GitHubRepoKey_Ugly(t *testing.T) {
|
||||
// GitHubRepoKey differentiates between two repositories in the same org.
|
||||
keyA := cache.GitHubRepoKey("org", "repo-a")
|
||||
keyB := cache.GitHubRepoKey("org", "repo-b")
|
||||
if keyA == keyB {
|
||||
t.Errorf("expected different keys for different repos, both got %q", keyA)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue