BUG Store the timestamp in the cache

src/Storage/Sha1FileHashingService.php

@@ -178,21 +178,22 @@ private function getCache()
      */
     private function buildCacheKey($fileID, $fs)
     {
-        $filesystem = $this->getFilesystem($fs);
-
         $fsID = $this->getFilesystemKey($fs);
+        return base64_encode(sprintf('%s://%s', $fsID, $fileID));
+    }
 
-        // If we can't get a timestamp for the file then we'll use the current timestamp so it
-        // invalidates quickly
-        try {
-            $timestamp = $filesystem->has($fileID) ?
-                $filesystem->getTimestamp($fileID) :
-                DBDatetime::now()->getTimestamp();
-        } catch (Exception $e) {
-            $timestamp = DBDatetime::now()->getTimestamp();
-        }
-
-        return base64_encode(sprintf('%s://%s://%s', $fsID, $fileID, $timestamp));
+    /**
+     * Return the current timestamp for the provided file or the current time if the file doesn't exists.
+     * @param string $fileID
+     * @param string|Filesystem $fs
+     * @return string
+     */
+    private function getTimestamp($fileID, $fs)
+    {
+        $filesystem = $this->getFilesystem($fs);
+        return $filesystem->has($fileID) ?
+            $filesystem->getTimestamp($fileID) :
+            DBDatetime::now()->getTimestamp();
     }
 
     public function invalidate($fileID, $fs)
@@ -212,7 +213,14 @@ public function get($fileID, $fs)
     {
         if ($this->isCached()) {
             $key = $this->buildCacheKey($fileID, $fs);
-            return $this->getCache()->get($key, false);
+            $value = $this->getCache()->get($key, false);
+            if ($value) {
+                list($timestamp, $hash) = $value;
+                if ($timestamp === $this->getTimestamp($fileID, $fs)) {
+                    // Only return the cached hash if the cached timestamp matches the timestamp of the physical file
+                    return $hash;
+                }
+            }
         }
         return false;
     }
@@ -222,7 +230,13 @@ public function set($fileID, $fs, $hash)
     {
         if ($this->isCached()) {
             $key = $this->buildCacheKey($fileID, $fs);
-            $this->getCache()->set($key, $hash);
+            // We store the file's timestamp in the cache so we can compare, that way we can know if some outside
+            // process has touched the file
+            $value = [
+                $this->getTimestamp($fileID, $fs),
+                $hash
+            ];
+            $this->getCache()->set($key, $value);
         }
     }
 

tests/php/Flysystem/FlysystemAssetStoreTest.php

@@ -50,7 +50,7 @@ protected function setUp()
             ->getMock();
 
         $this->publicFilesystem = $this->getMockBuilder(Filesystem::class)
-            ->setMethods(['has', 'read', 'readStream'])
+            ->setMethods(['has', 'read', 'readStream', 'getTimestamp'])
             ->setConstructorArgs([$this->publicAdapter])
             ->getMock();
 
@@ -59,7 +59,7 @@ protected function setUp()
             ->getMock();
 
         $this->protectedFilesystem = $this->getMockBuilder(Filesystem::class)
-            ->setMethods(['has', 'read', 'readStream'])
+            ->setMethods(['has', 'read', 'readStream', 'getTimestamp'])
             ->setConstructorArgs([$this->protectedAdapter])
             ->getMock();
 

tests/php/Storage/Sha1FileHashingServiceTest.php

@@ -78,7 +78,7 @@ public function testComputeFromStream()
         }
     }
 
-    public function testcomputeFromFile()
+    public function testComputeFromFile()
     {
         $service = new Sha1FileHashingService();
         $service->disableCache();
@@ -137,6 +137,25 @@ public function testComputeWithCache()
         $this->assertEquals($hash, $service->computeFromFile('missing-file.text', AssetStore::VISIBILITY_PUBLIC));
     }
 
+    public function testComputeTouchFile()
+    {
+        $service = new Sha1FileHashingService();
+        $service->enableCache();
+
+        $this->assertEquals($this->publicHash, $service->computeFromFile($this->fileID, $this->publicFs));
+
+        // Our timestamp is accruate to the second, we need to wait a bit to make sure our new timestamp won't be in
+        // the same second as the old one.
+        sleep(2);
+        $this->publicFs->update($this->fileID, $this->protectedContent);
+
+        $this->assertEquals(
+            $this->protectedHash,
+            $service->computeFromFile($this->fileID, $this->publicFs),
+            'When a file is touched by an outside process, existing value for that file are invalidated'
+        );
+    }
+
     public function testCompare()
     {
         $service = new Sha1FileHashingService();