Merge pull request #13 from biigle/throw-on-lock

Implement optional throwOnLock argument
biigle · Jun 14, 2023 · ce9203d · ce9203d
2 parents a21fbcc + 9777e92
commit ce9203d
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 21 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -9,6 +9,7 @@ on:
 jobs:
   run:
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     strategy:
       matrix:
         php-versions: ['8.0', '8.1']
@@ -21,11 +22,10 @@ jobs:
       uses: shivammathur/setup-php@v2
       with:
         php-version: ${{ matrix.php-versions }}
-        tools: phpunit:latest
 
     - name: Setup Laravel
       run: composer require -W --dev laravel/laravel:${{ matrix.laravel-versions }}
 
     - name: Run tests
-      run: phpunit
+      run: ./vendor/bin/phpunit
 
diff --git a/src/Contracts/FileCache.php b/src/Contracts/FileCache.php
@@ -12,10 +12,11 @@ interface FileCache
      * @param File $file
      * @param callable $callback Gets the file object and the path to the cached file
      * file as arguments.
+     * @param bool $throwOnLock Whether to throw an exception if a file is currently locked (i.e. written to). Otherwise the method will wait until the lock is released.
      *
      * @return mixed Result of the callback.
      */
-    public function get(File $file, callable $callback);
+    public function get(File $file, callable $callback, bool $throwOnLock);
 
     /**
      * Like `get` but deletes the cached file afterwards (if it is not used somewhere
@@ -24,10 +25,11 @@ public function get(File $file, callable $callback);
      * @param File $file
      * @param callable $callback Gets the file object and the path to the cached file
      * file as arguments.
+     * @param bool $throwOnLock Whether to throw an exception if a file is currently locked (i.e. written to). Otherwise the method will wait until the lock is released.
      *
      * @return mixed Result of the callback.
      */
-    public function getOnce(File $file, callable $callback);
+    public function getOnce(File $file, callable $callback, bool $throwOnLock);
 
     /**
      * Get a stream resource for an file. If the file is cached, the resource points
@@ -48,10 +50,11 @@ public function getStream(File $file);
      * @param array $files
      * @param callable $callback Gets the array of file objects and the array of paths
      * to the cached file files (in the same ordering) as arguments.
+     * @param bool $throwOnLock Whether to throw an exception if a file is currently locked (i.e. written to). Otherwise the method will wait until the lock is released.
      *
      * @return mixed Result of the callback.
      */
-    public function batch(array $files, callable $callback);
+    public function batch(array $files, callable $callback, bool $throwOnLock);
 
     /**
      * Like `batch` but deletes the cached files afterwards (if they are not used
@@ -60,10 +63,11 @@ public function batch(array $files, callable $callback);
      * @param array $files
      * @param callable $callback Gets the array of file objects and the array of paths
      * to the cached file files (in the same ordering) as arguments.
+     * @param bool $throwOnLock Whether to throw an exception if a file is currently locked (i.e. written to). Otherwise the method will wait until the lock is released.
      *
      * @return mixed Result of the callback.
      */
-    public function batchOnce(array $files, callable $callback);
+    public function batchOnce(array $files, callable $callback, bool $throwOnLock);
 
     /**
      * Remove cached files that are too old or exceed the maximum cache size.

diff --git a/src/Exceptions/FileLockedException.php b/src/Exceptions/FileLockedException.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace Biigle\FileCache\Exceptions;
+
+use Exception;
+
+class FileLockedException extends Exception
+{
+    //
+}
diff --git a/src/FileCache.php b/src/FileCache.php
@@ -4,6 +4,7 @@
 
 use Biigle\FileCache\Contracts\File;
 use Biigle\FileCache\Contracts\FileCache as FileCacheContract;
+use Biigle\FileCache\Exceptions\FileLockedException;
 use Exception;
 use Illuminate\Filesystem\Filesystem;
 use Illuminate\Filesystem\FilesystemManager;
@@ -66,21 +67,21 @@ public function exists(File $file)
     /**
      * {@inheritdoc}
      */
-    public function get(File $file, callable $callback)
+    public function get(File $file, callable $callback, bool $throwOnLock = false)
     {
         return $this->batch([$file], function ($files, $paths) use ($callback) {
             return call_user_func($callback, $files[0], $paths[0]);
-        });
+        }, $throwOnLock);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function getOnce(File $file, callable $callback)
+    public function getOnce(File $file, callable $callback, bool $throwOnLock = false)
     {
         return $this->batchOnce([$file], function ($files, $paths) use ($callback) {
             return call_user_func($callback, $files[0], $paths[0]);
-        });
+        }, $throwOnLock);
     }
 
     /**
@@ -118,10 +119,10 @@ public function getStream(File $file)
     /**
      * {@inheritdoc}
      */
-    public function batch(array $files, callable $callback)
+    public function batch(array $files, callable $callback, bool $throwOnLock = false)
     {
-        $retrieved = array_map(function ($file) {
-            return $this->retrieve($file);
+        $retrieved = array_map(function ($file) use ($throwOnLock) {
+            return $this->retrieve($file, $throwOnLock);
         }, $files);
 
         $paths = array_map(function ($file) {
@@ -142,10 +143,10 @@ public function batch(array $files, callable $callback)
     /**
      * {@inheritdoc}
      */
-    public function batchOnce(array $files, callable $callback)
+    public function batchOnce(array $files, callable $callback, bool $throwOnLock = false)
     {
-        $retrieved = array_map(function ($file) {
-            return $this->retrieve($file);
+        $retrieved = array_map(function ($file) use ($throwOnLock) {
+            return $this->retrieve($file, $throwOnLock);
         }, $files);
 
         $paths = array_map(function ($file) {
@@ -347,12 +348,15 @@ protected function delete(SplFileInfo $file)
      * local file will be returned.
      *
      * @param File $file File to get the path for
+     * @param bool $throwOnLock Whether to throw an exception if a file is currently locked (i.e. written to). Otherwise the method will wait until the lock is released.
      * @throws Exception If the file could not be cached.
+     * @throws FileLockedException If the file is locked and `throwOnLock` was `true`.
+     *
      *
      * @return array Containing the 'path' to the file and the file 'handle'. Close the
      * handle when finished.
      */
-    protected function retrieve(File $file)
+    protected function retrieve(File $file, bool $throwOnLock = false)
     {
         $this->ensurePathExists();
         $cachedPath = $this->getCachedPath($file);
@@ -364,6 +368,10 @@ protected function retrieve(File $file)
         if ($handle === false) {
             // The file exists, get the file handle in read mode.
             $handle = fopen($cachedPath, 'r');
+            if ($throwOnLock && !flock($handle, LOCK_SH | LOCK_NB)) {
+                throw new FileLockedException;
+            }
+
             // Wait for any LOCK_EX that is set if the file is currently written.
             flock($handle, LOCK_SH);
 

diff --git a/src/Testing/FileCacheFake.php b/src/Testing/FileCacheFake.php
@@ -20,7 +20,7 @@ public function __construct()
     /**
      * {@inheritdoc}
      */
-    public function get(File $file, callable $callback)
+    public function get(File $file, callable $callback, bool $throwOnLock = false)
     {
         return $this->batch([$file], function ($files, $paths) use ($callback) {
             return call_user_func($callback, $files[0], $paths[0]);
@@ -30,7 +30,7 @@ public function get(File $file, callable $callback)
     /**
      * {@inheritdoc}
      */
-    public function getOnce(File $file, callable $callback)
+    public function getOnce(File $file, callable $callback, bool $throwOnLock = false)
     {
         return $this->get($file, $callback);
     }
@@ -50,7 +50,7 @@ public function getStream(File $file)
     /**
      * {@inheritdoc}
      */
-    public function batch(array $files, callable $callback)
+    public function batch(array $files, callable $callback, bool $throwOnLock = false)
     {
         $paths = array_map(function ($file) {
             $hash = hash('sha256', $file->getUrl());
@@ -64,7 +64,7 @@ public function batch(array $files, callable $callback)
     /**
      * {@inheritdoc}
      */
-    public function batchOnce(array $files, callable $callback)
+    public function batchOnce(array $files, callable $callback, bool $throwOnLock = false)
     {
         return $this->batch($files, $callback);
     }

diff --git a/tests/FileCacheTest.php b/tests/FileCacheTest.php
@@ -3,6 +3,7 @@
 namespace Biigle\FileCache\Tests;
 
 use Biigle\FileCache\Contracts\File;
+use Biigle\FileCache\Exceptions\FileLockedException;
 use Biigle\FileCache\FileCache;
 use Biigle\FileCache\GenericFile;
 use Exception;
@@ -168,6 +169,21 @@ public function testGetDiskCloudTooLarge()
         $path = $cache->get($file, $this->noop);
     }
 
+    public function testGetThrowOnLock()
+    {
+        $cache = new FileCache(['path' => $this->cachePath]);
+        $file = new GenericFile('abc://some/image.jpg');
+        $hash = hash('sha256', 'abc://some/image.jpg');
+        $path = "{$this->cachePath}/{$hash}";
+        touch($path, time() - 1);
+
+        $handle = fopen($path, 'w');
+        flock($handle, LOCK_EX);
+
+        $this->expectException(FileLockedException::class);
+        $cache->get($file, fn ($file, $path) => $file, true);
+    }
+
     public function testGetOnce()
     {
         $file = new GenericFile('test://test-image.jpg');
@@ -182,6 +198,21 @@ public function testGetOnce()
         $this->assertFalse($this->app['files']->exists("{$this->cachePath}/{$hash}"));
     }
 
+    public function testGetOnceThrowOnLock()
+    {
+        $cache = new FileCache(['path' => $this->cachePath]);
+        $file = new GenericFile('abc://some/image.jpg');
+        $hash = hash('sha256', 'abc://some/image.jpg');
+        $path = "{$this->cachePath}/{$hash}";
+        touch($path, time() - 1);
+
+        $handle = fopen($path, 'w');
+        flock($handle, LOCK_EX);
+
+        $this->expectException(FileLockedException::class);
+        $cache->getOnce($file, fn ($file, $path) => $file, true);
+    }
+
     public function testGetStreamCached()
     {
         $file = new GenericFile('test://test-image.jpg');
@@ -235,6 +266,21 @@ public function testBatch()
         $this->assertStringContainsString($hash, $paths[1]);
     }
 
+    public function testBatchThrowOnLock()
+    {
+        $cache = new FileCache(['path' => $this->cachePath]);
+        $file = new GenericFile('abc://some/image.jpg');
+        $hash = hash('sha256', 'abc://some/image.jpg');
+        $path = "{$this->cachePath}/{$hash}";
+        touch($path, time() - 1);
+
+        $handle = fopen($path, 'w');
+        flock($handle, LOCK_EX);
+
+        $this->expectException(FileLockedException::class);
+        $cache->batch([$file], fn ($file, $path) => $file, true);
+    }
+
     public function testBatchOnce()
     {
         $file = new GenericFile('test://test-image.jpg');