From 9cc16d5792a0e66958a3987ec05531b67dfa94a6 Mon Sep 17 00:00:00 2001 From: Virgil Date: Sun, 29 Mar 2026 15:16:49 +0000 Subject: [PATCH] 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) }