feat: Add error handling and test for database open failures in cache

Author: presbrey

base_cache.go

@@ -67,16 +67,13 @@ func (c *baseCache) cleanupItem(key string) {
 
 // startCleanup starts the cleanup routine that removes old items
 func (c *baseCache) startCleanup() {
-	if c.maxAge <= 0 {
-		return
-	}
-
 	// Run cleanup immediately once
 	c.cleanup()
 
 	ticker := time.NewTicker(c.cleanupInterval)
 	defer ticker.Stop()
 
+	// Start cleanup loop
 	for {
 		select {
 		case <-ticker.C:

base_cache_test.go

@@ -96,4 +96,54 @@ func TestBaseCache(t *testing.T) {
 		// Ensure that lastUsed is set to a recent time (within the last 2 seconds)
 		assert.WithinDuration(t, time.Now(), entry.lastUsed, 2*time.Second, "expected entry.lastUsed to be recent")
 	})
+
+	t.Run("cleanup doesn't run when maxAge is zero or negative", func(t *testing.T) {
+		// Create base cache with zero maxAge
+		opts := Options{
+			CleanupInterval: 100 * time.Millisecond,
+			MaxAge:          0,
+		}
+
+		cache := newBaseCache(opts)
+		defer cache.Stop()
+
+		// Add test item
+		db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{})
+		cache.cacheMutex.Lock()
+		cache.dbCache["test"] = &dbCacheEntry{
+			db:       db,
+			lastUsed: time.Now().Add(-10 * time.Second),
+		}
+		cache.cacheMutex.Unlock()
+
+		// Wait for potential cleanup
+		time.Sleep(500 * time.Millisecond)
+
+		// Verify item still exists since cleanup should not run
+		cache.cacheMutex.RLock()
+		_, exists := cache.dbCache["test"]
+		cache.cacheMutex.RUnlock()
+
+		assert.True(t, exists, "item should not be cleaned up when maxAge is 0")
+
+		// Test with negative maxAge
+		opts.MaxAge = -1 * time.Second
+		cache = newBaseCache(opts)
+		defer cache.Stop()
+
+		cache.cacheMutex.Lock()
+		cache.dbCache["test"] = &dbCacheEntry{
+			db:       db,
+			lastUsed: time.Now().Add(-10 * time.Second),
+		}
+		cache.cacheMutex.Unlock()
+
+		time.Sleep(500 * time.Millisecond)
+
+		cache.cacheMutex.RLock()
+		_, exists = cache.dbCache["test"]
+		cache.cacheMutex.RUnlock()
+
+		assert.True(t, exists, "item should not be cleaned up when maxAge is negative")
+	})
 }

cache_test.go

@@ -110,6 +110,35 @@ func TestGet(t *testing.T) {
 	})
 }
 
+func TestOpenError(t *testing.T) {
+	cache := gormoize.ByDSN(nil)
+
+	// Create a dialector function that will cause an error by using an invalid DSN
+	failDialector := func(dsn string) gorm.Dialector {
+		// Use a non-existent directory to force an error
+		return sqlite.Open("/nonexistent/directory/db.sqlite")
+	}
+
+	// Attempt to open the database
+	db, err := cache.Open(failDialector, "test")
+
+	// Verify that the error is returned
+	if err == nil {
+		t.Error("expected error when opening invalid database")
+	}
+
+	// Verify that nil is returned for the database
+	if db != nil {
+		t.Error("expected nil database when error occurs")
+	}
+
+	// Verify that the failed connection is not cached
+	cachedDB := cache.Get("test")
+	if cachedDB != nil {
+		t.Error("failed connection should not be cached")
+	}
+}
+
 func TestConcurrentOpen(t *testing.T) {
 	// Clean up test database after tests
 	const testDSN = "concurrent_test.db"