Refactor CacheInterface so that it deals only with a value object (#45)

* Add CacheObject as a value object for the cache data * Refactor CacheInterface so that it deals only with CacheObject
sirbrillig · Mar 11, 2021 · 9821541 · 9821541
1 parent 249e9ee
commit 9821541
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 50 deletions.
diff --git a/PhpcsChanged/CacheInterface.php b/PhpcsChanged/CacheInterface.php
@@ -3,10 +3,10 @@
 
 namespace PhpcsChanged;
 
-use PhpcsChanged\CacheManager;
+use PhpcsChanged\CacheObject;
 
 interface CacheInterface {
-	public function load(CacheManager $manager): void;
+	public function load(): CacheObject;
 
-	public function save(CacheManager $manager): void;
+	public function save(CacheObject $cacheObject): void;
 }
diff --git a/PhpcsChanged/CacheManager.php b/PhpcsChanged/CacheManager.php
@@ -20,16 +20,6 @@ class CacheManager {
 	 */
 	private $fileDataByPath = [];
 
-	/**
-	 * @var string
-	 */
-	private $revisionId;
-
-	/**
-	 * @var string
-	 */
-	private $cacheVersion;
-
 	/**
 	 * @var bool
 	 */
@@ -40,28 +30,42 @@ class CacheManager {
 	 */
 	private $cache;
 
+	/**
+	 * @var CacheObject
+	 */
+	private $cacheObject;
+
 	/**
 	 * @var callable
 	 */
 	private $debug;
 
 	public function __construct(CacheInterface $cache, callable $debug = null) {
 		$this->cache = $cache;
-		$this->cacheVersion = getVersion();
 		$noopDebug = function(...$output) {}; // phpcs:ignore VariableAnalysis
 		$this->debug = $debug ?? $noopDebug;
 	}
 
 	public function load(): void {
 		($this->debug)("Loading cache...");
-		$this->cache->load($this);
+		$this->cacheObject = $this->cache->load();
+
 		// Don't try to use old cache versions
 		$version = getVersion();
-		if ($this->cacheVersion !== $version) {
-			($this->debug)("Cache version has changed ({$this->cacheVersion} -> {$version}). Clearing cache.");
+		if (! $this->cacheObject->cacheVersion) {
+			$this->cacheObject->cacheVersion = $version;
+		}
+		if ($this->cacheObject->cacheVersion !== $version) {
+			($this->debug)("Cache version has changed ({$this->cacheObject->cacheVersion} -> {$version}). Clearing cache.");
 			$this->clearCache();
-			$this->cacheVersion = $version;
+			$this->cacheObject->cacheVersion = $version;
+		}
+
+		// Keep a map of cache data so it's faster to access
+		foreach($this->cacheObject->entries as $entry) {
+			$this->addCacheEntry($entry);
 		}
+
 		$this->hasBeenModified = false;
 		($this->debug)("Cache loaded.");
 	}
@@ -72,16 +76,20 @@ public function save(): void {
 			return;
 		}
 		($this->debug)("Saving cache.");
-		$this->cache->save($this);
+
+		// Copy cache data map back to object
+		$this->cacheObject->entries = $this->getEntries();
+
+		$this->cache->save($this->cacheObject);
 		$this->hasBeenModified = false;
 	}
 
 	public function getRevision(): ?string {
-		return $this->revisionId;
+		return $this->cacheObject->revisionId;
 	}
 
 	public function getCacheVersion(): string {
-		return $this->cacheVersion;
+		return $this->cacheObject->cacheVersion;
 	}
 
 	/**
@@ -114,25 +122,24 @@ private function flattenArray($array): array {
 	}
 
 	public function setCacheVersion(string $cacheVersion): void {
-		if (! $this->cacheVersion || $this->cacheVersion === $cacheVersion) {
-			$this->cacheVersion = $cacheVersion;
+		if ($this->cacheObject->cacheVersion === $cacheVersion) {
 			return;
 		}
-		($this->debug)("Cache version has changed ('{$this->cacheVersion}' -> '{$cacheVersion}'). Clearing cache.");
+		($this->debug)("Cache version has changed ('{$this->cacheObject->cacheVersion}' -> '{$cacheVersion}'). Clearing cache.");
 		$this->hasBeenModified = true;
 		$this->clearCache();
-		$this->cacheVersion = $cacheVersion;
+		$this->cacheObject->cacheVersion = $cacheVersion;
 	}
 
 	public function setRevision(string $revisionId): void {
-		if (! $this->revisionId || $this->revisionId === $revisionId) {
-			$this->revisionId = $revisionId;
+		if (! $this->cacheObject->revisionId || $this->cacheObject->revisionId === $revisionId) {
+			$this->cacheObject->revisionId = $revisionId;
 			return;
 		}
-		($this->debug)("Revision has changed ('{$this->revisionId}' -> '{$revisionId}'). Clearing cache.");
+		($this->debug)("Revision has changed ('{$this->cacheObject->revisionId}' -> '{$revisionId}'). Clearing cache.");
 		$this->hasBeenModified = true;
 		$this->clearCache();
-		$this->revisionId = $revisionId;
+		$this->cacheObject->revisionId = $revisionId;
 	}
 
 	public function getCacheForFile(string $filePath, string $type, string $hash, string $phpcsStandard): ?string {
@@ -196,7 +203,8 @@ public function removeCacheEntry(CacheEntry $entry): void {
 	public function clearCache(): void {
 		($this->debug)("Cache cleared");
 		$this->hasBeenModified = true;
-		$this->revisionId = '';
+		$this->cacheObject->revisionId = '';
 		$this->fileDataByPath = [];
+		$this->cacheObject->entries = [];
 	}
 }
diff --git a/PhpcsChanged/CacheObject.php b/PhpcsChanged/CacheObject.php
@@ -0,0 +1,22 @@
+<?php
+declare(strict_types=1);
+
+namespace PhpcsChanged;
+
+class CacheObject {
+	/**
+	 * @var CacheEntry[]
+	 */
+	public $entries = [];
+
+	/**
+	 * @var string
+	 */
+	public $revisionId;
+
+	/**
+	 * @var string
+	 */
+	public $cacheVersion;
+}
+
diff --git a/PhpcsChanged/FileCache.php b/PhpcsChanged/FileCache.php
@@ -4,7 +4,7 @@
 namespace PhpcsChanged;
 
 use PhpcsChanged\CacheInterface;
-use PhpcsChanged\CacheManager;
+use PhpcsChanged\CacheObject;
 use PhpcsChanged\CacheEntry;
 
 define('DEFAULT_CACHE_FILE', '.phpcs-changed-cache');
@@ -15,9 +15,9 @@ class FileCache implements CacheInterface {
 	 */
 	public $cacheFilePath = DEFAULT_CACHE_FILE; // phpcs:ignore ImportDetection -- apparently ImportDetection does not understand constants
 
-	public function load(CacheManager $manager): void {
+	public function load(): CacheObject {
 		if (! file_exists($this->cacheFilePath)) {
-			return;
+			return new CacheObject();
 		}
 		$contents = file_get_contents($this->cacheFilePath);
 		if ($contents === false) {
@@ -27,21 +27,23 @@ public function load(CacheManager $manager): void {
 		if (! $this->isDecodedDataValid($decoded)) {
 			throw new \Exception('Invalid cache file');
 		}
-		$manager->setCacheVersion($decoded['cacheVersion']);
-		$manager->setRevision($decoded['revisionId']);
+		$cacheObject = new CacheObject();
+		$cacheObject->cacheVersion = $decoded['cacheVersion'];
+		$cacheObject->revisionId = $decoded['revisionId'];
 		foreach($decoded['entries'] as $entry) {
 			if (! $this->isDecodedEntryValid($entry)) {
 				throw new \Exception('Invalid cache file entry: ' . $entry);
 			}
-			$manager->addCacheEntry(CacheEntry::fromJson($entry));
+			$cacheObject->entries[] = CacheEntry::fromJson($entry);
 		}
+		return $cacheObject;
 	}
 
-	public function save(CacheManager $manager): void {
+	public function save(CacheObject $cacheObject): void {
 		$data = [
-			'cacheVersion' => $manager->getCacheVersion(),
-			'revisionId' => $manager->getRevision(),
-			'entries' => $manager->getEntries(),
+			'cacheVersion' => $cacheObject->cacheVersion,
+			'revisionId' => $cacheObject->revisionId,
+			'entries' => $cacheObject->entries,
 		];
 		$result = file_put_contents($this->cacheFilePath, json_encode($data));
 		if ($result === false) {

diff --git a/index.php b/index.php
@@ -18,6 +18,7 @@
 require_once __DIR__ . '/PhpcsChanged/ShellOperator.php';
 require_once __DIR__ . '/PhpcsChanged/UnixShell.php';
 require_once __DIR__ . '/PhpcsChanged/CacheEntry.php';
+require_once __DIR__ . '/PhpcsChanged/CacheObject.php';
 require_once __DIR__ . '/PhpcsChanged/CacheInterface.php';
 require_once __DIR__ . '/PhpcsChanged/CacheManager.php';
 require_once __DIR__ . '/PhpcsChanged/FileCache.php';

diff --git a/tests/helpers/TestCache.php b/tests/helpers/TestCache.php
@@ -4,7 +4,7 @@
 namespace PhpcsChangedTests;
 
 use PhpcsChanged\CacheInterface;
-use PhpcsChanged\CacheManager;
+use PhpcsChanged\CacheObject;
 use PhpcsChanged\CacheEntry;
 use function PhpcsChanged\getVersion;
 
@@ -34,27 +34,29 @@ class TestCache implements CacheInterface {
 	 */
 	public $disabled = false;
 
-	public function load(CacheManager $manager): void {
+	public function load(): CacheObject {
 		if ($this->disabled) {
-			return;
+			return new CacheObject();
 		}
 		$this->didSave = false;
-		$manager->setCacheVersion($this->cacheVersion ?? getVersion());
-		$manager->setRevision($this->revisionId);
+		$cacheObject = new CacheObject();
+		$cacheObject->cacheVersion = $this->cacheVersion ?? getVersion();
+		$cacheObject->revisionId = $this->revisionId;
 		foreach(array_values($this->savedFileData) as $entry) {
-			$manager->addCacheEntry(CacheEntry::fromJson($entry));
+			$cacheObject->entries[] = CacheEntry::fromJson($entry);
 		}
+		return $cacheObject;
 	}
 
-	public function save(CacheManager $manager): void {
+	public function save(CacheObject $cacheObject): void {
 		if ($this->disabled) {
 			return;
 		}
 		$this->didSave = true;
-		$this->setCacheVersion($manager->getCacheVersion());
-		$this->setRevision($manager->getRevision());
+		$this->setCacheVersion($cacheObject->cacheVersion);
+		$this->setRevision($cacheObject->revisionId);
 		$this->savedFileData = [];
-		foreach($manager->getEntries() as $entry) {
+		foreach($cacheObject->entries as $entry) {
 			$this->setEntry($entry->path, $entry->type, $entry->hash, $entry->phpcsStandard, $entry->data);
 		}
 	}