FIX: Prevent unnecessary cache writes (fixes silverstripe#34)

kinglozzer · May 30, 2022 · 3c22313 · 3c22313
1 parent c8865c7
commit 3c22313
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 30 deletions.
diff --git a/src/Collections/CachedConfigCollection.php b/src/Collections/CachedConfigCollection.php
@@ -141,26 +141,11 @@ public function getCollection()
             $this->building = false;
         }
 
-        // Save immediately.
-        // Note additional deferred save will occur in _destruct()
+        // Save collection in cache
         $this->cache->set(self::CACHE_KEY, $this->collection);
         return $this->collection;
     }
 
-    /**
-     * Commits the cache
-     */
-    public function __destruct()
-    {
-        // Ensure back-end cache is updated
-        if ($this->collection) {
-            $this->cache->set(self::CACHE_KEY, $this->collection);
-
-            // Prevent double-destruct
-            $this->collection = null;
-        }
-    }
-
     public function nest()
     {
         $collection = $this->getCollection();

diff --git a/tests/Collections/CachedConfigCollectionTest.php b/tests/Collections/CachedConfigCollectionTest.php
@@ -36,12 +36,6 @@ public function testCacheHit()
             ->with(CachedConfigCollection::CACHE_KEY)
             ->willReturn($mockCollection);
 
-        // Set called in __destruct
-        $mockCache
-            ->expects($this->once())
-            ->method('set')
-            ->with(CachedConfigCollection::CACHE_KEY, $mockCollection);
-
         $collection = new CachedConfigCollection();
         $collection->setCollectionCreator(function () {
             $this->fail("Invalid cache miss");
@@ -51,9 +45,6 @@ public function testCacheHit()
         // Check
         $this->assertTrue($collection->exists('test', 'name'));
         $this->assertEquals('value', $collection->get('test', 'name'));
-
-        // Write back changes to cache
-        $collection->__destruct();
     }
 
     public function testCacheMiss()
@@ -80,9 +71,9 @@ public function testCacheMiss()
             ->with('test', 'name', 0)
             ->willReturn(true);
 
-        // Cache will be generated, saved, and then saved again on __destruct()
+        // Cache will be generated, then saved
         $mockCache
-            ->expects($this->exactly(2))
+            ->expects($this->once())
             ->method('set')
             ->with(CachedConfigCollection::CACHE_KEY, $mockCollection);
 
@@ -95,9 +86,6 @@ public function testCacheMiss()
         // Check
         $this->assertTrue($collection->exists('test', 'name'));
         $this->assertEquals('value', $collection->get('test', 'name'));
-
-        // Write back changes to cache
-        $collection->__destruct();
     }
 
     public function testInfiniteLoop()