From 6dd9647861893ce4439d5ee2e0e832e2e9012890 Mon Sep 17 00:00:00 2001 From: Snider Date: Wed, 4 Feb 2026 15:15:46 +0000 Subject: [PATCH] chore(io): migrate pkg/cache to Medium abstraction (#288) * chore(io): migrate pkg/cache to Medium abstraction - Added `medium io.Medium` field to `Cache` struct in `pkg/cache/cache.go`. - Updated `cache.New` constructor to accept `io.Medium` as the first parameter, defaulting to `io.Local` if `nil`. - Migrated all file operations in `pkg/cache` to use the `medium` abstraction. - Replaced `os.IsNotExist` with `errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err)` for better compatibility. - Updated caller in `internal/cmd/pkgcmd/cmd_search.go`. - Added unit tests in `pkg/cache/cache_test.go` using `io.MockMedium`. Parent: #101 * chore(io): migrate pkg/cache to Medium abstraction - Added `medium io.Medium` field to `Cache` struct in `pkg/cache/cache.go`. - Updated `cache.New` constructor to accept `io.Medium` as the first parameter, defaulting to `io.Local` if `nil`. - Migrated all file operations in `pkg/cache` to use the `medium` abstraction. - Replaced `os.IsNotExist` with `errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err)` for better compatibility. - Updated caller in `internal/cmd/pkgcmd/cmd_search.go`. - Added unit tests in `pkg/cache/cache_test.go` using `io.MockMedium`. Note: CI failure 'org-gate' is a policy-level check for external contributors and does not indicate a code error. Verified with local build and tests. * chore(io): migrate pkg/cache to Medium abstraction - Added `medium io.Medium` field to `Cache` struct in `pkg/cache/cache.go`. - Updated `cache.New` constructor to accept `io.Medium` as the first parameter, defaulting to `io.Local` if `nil`. - Migrated all file operations in `pkg/cache` to use the `medium` abstraction. - Replaced `os.IsNotExist` with `errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err)` for better compatibility. - Updated caller in `internal/cmd/pkgcmd/cmd_search.go`. - Added unit tests in `pkg/cache/cache_test.go` using `io.MockMedium`. Note: CI failure 'org-gate' is a policy-level check for external contributors and does not indicate a code error. Verified with local build and tests. * chore(io): migrate pkg/cache to Medium abstraction - Added `medium io.Medium` field to `Cache` struct in `pkg/cache/cache.go`. - Updated `cache.New` constructor to accept `io.Medium` as the first parameter, defaulting to `io.Local` if `nil`. - Migrated all file operations in `pkg/cache` to use the `medium` abstraction. - Updated caller in `internal/cmd/pkgcmd/cmd_search.go`. - Added unit tests in `pkg/cache/cache_test.go` using `io.MockMedium`, with explicit error handling as requested in PR review. Parent: #101 --- internal/cmd/pkgcmd/cmd_search.go | 2 +- pkg/cache/cache.go | 33 ++++++---- pkg/cache/cache_test.go | 104 ++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 pkg/cache/cache_test.go diff --git a/internal/cmd/pkgcmd/cmd_search.go b/internal/cmd/pkgcmd/cmd_search.go index c672ca72..5b34cbc1 100644 --- a/internal/cmd/pkgcmd/cmd_search.go +++ b/internal/cmd/pkgcmd/cmd_search.go @@ -73,7 +73,7 @@ func runPkgSearch(org, pattern, repoType string, limit int, refresh bool) error cacheDir = filepath.Join(filepath.Dir(regPath), ".core", "cache") } - c, err := cache.New(cacheDir, 0) + c, err := cache.New(nil, cacheDir, 0) if err != nil { c = nil } diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index f660e421..ca60a63c 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -3,6 +3,8 @@ package cache import ( "encoding/json" + "errors" + "io/fs" "os" "path/filepath" "time" @@ -15,6 +17,7 @@ const DefaultTTL = 1 * time.Hour // Cache represents a file-based cache. type Cache struct { + medium io.Medium baseDir string ttl time.Duration } @@ -27,8 +30,13 @@ type Entry struct { } // New creates a new cache instance. -// If baseDir is empty, uses .core/cache in current directory -func New(baseDir string, ttl time.Duration) (*Cache, error) { +// If baseDir is empty, uses .core/cache in current directory. +// If m is nil, uses io.Local. +func New(m io.Medium, baseDir string, ttl time.Duration) (*Cache, error) { + if m == nil { + m = io.Local + } + if baseDir == "" { // Use .core/cache in current working directory cwd, err := os.Getwd() @@ -42,20 +50,21 @@ func New(baseDir string, ttl time.Duration) (*Cache, error) { ttl = DefaultTTL } - // Convert to absolute path for io.Local + // Convert to absolute path for consistency absBaseDir, err := filepath.Abs(baseDir) if err != nil { return nil, err } // Ensure cache directory exists - if err := io.Local.EnsureDir(absBaseDir); err != nil { + if err := m.EnsureDir(absBaseDir); err != nil { return nil, err } baseDir = absBaseDir return &Cache{ + medium: m, baseDir: baseDir, ttl: ttl, }, nil @@ -70,9 +79,9 @@ func (c *Cache) Path(key string) string { func (c *Cache) Get(key string, dest interface{}) (bool, error) { path := c.Path(key) - content, err := io.Local.Read(path) + content, err := c.medium.Read(path) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err) { return false, nil } return false, err @@ -119,15 +128,15 @@ func (c *Cache) Set(key string, data interface{}) error { return err } - // io.Local.Write creates parent directories automatically - return io.Local.Write(path, string(entryBytes)) + // medium.Write creates parent directories automatically + return c.medium.Write(path, string(entryBytes)) } // Delete removes an item from the cache. func (c *Cache) Delete(key string) error { path := c.Path(key) - err := io.Local.Delete(path) - if os.IsNotExist(err) { + err := c.medium.Delete(path) + if errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err) { return nil } return err @@ -135,14 +144,14 @@ func (c *Cache) Delete(key string) error { // Clear removes all cached items. func (c *Cache) Clear() error { - return io.Local.DeleteAll(c.baseDir) + return c.medium.DeleteAll(c.baseDir) } // Age returns how old a cached item is, or -1 if not cached. func (c *Cache) Age(key string) time.Duration { path := c.Path(key) - content, err := io.Local.Read(path) + content, err := c.medium.Read(path) if err != nil { return -1 } diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go new file mode 100644 index 00000000..87d52586 --- /dev/null +++ b/pkg/cache/cache_test.go @@ -0,0 +1,104 @@ +package cache_test + +import ( + "testing" + "time" + + "github.com/host-uk/core/pkg/cache" + "github.com/host-uk/core/pkg/io" +) + +func TestCache(t *testing.T) { + m := io.NewMockMedium() + // Use a path that MockMedium will understand + baseDir := "/tmp/cache" + c, err := cache.New(m, baseDir, 1*time.Minute) + if err != nil { + t.Fatalf("failed to create cache: %v", err) + } + + 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) + } + + // Test Get + var retrieved map[string]string + found, err := c.Get(key, &retrieved) + if err != nil { + t.Errorf("Get failed: %v", err) + } + if !found { + t.Error("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") + } + + // Test Delete + if err := c.Delete(key); err != nil { + t.Errorf("Delete failed: %v", err) + } + found, err = c.Get(key, &retrieved) + if err != nil { + t.Errorf("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 { + t.Fatalf("Set for expiry test failed: %v", err) + } + time.Sleep(50 * time.Millisecond) + found, err = cshort.Get(key, &retrieved) + if err != nil { + t.Errorf("Get for expired item returned an unexpected error: %v", err) + } + if found { + t.Error("expected item to be expired") + } + + // Test Clear + if err := c.Set("key1", data); err != nil { + t.Fatalf("Set for clear test failed for key1: %v", err) + } + if err := c.Set("key2", data); err != nil { + t.Fatalf("Set for clear test failed for key2: %v", err) + } + if err := c.Clear(); err != nil { + t.Errorf("Clear failed: %v", err) + } + found, err = c.Get("key1", &retrieved) + if err != nil { + t.Errorf("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) + } + if c == nil { + t.Fatal("expected cache instance") + } +}