From 97404612f66fd9adfea8848ca01b03b54ae9a5c5 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Wed, 14 Jun 2023 13:28:51 +0200 Subject: [PATCH 1/5] Implement optional throwOnLock argument If the argument is true, an exception will be thrown if a file is currently locked (i.e. written). Resolves #12 --- .github/workflows/tests.yml | 1 + src/Contracts/FileCache.php | 12 ++++--- src/Exceptions/FileLockedException.php | 10 ++++++ src/FileCache.php | 30 +++++++++++------ src/Testing/FileCacheFake.php | 8 ++--- tests/FileCacheTest.php | 46 ++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 src/Exceptions/FileLockedException.php diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 378cc27..e92d278 100644 --- 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'] diff --git a/src/Contracts/FileCache.php b/src/Contracts/FileCache.php index c1e0741..d3fee2c 100644 --- 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 new file mode 100644 index 0000000..61091ca --- /dev/null +++ b/src/Exceptions/FileLockedException.php @@ -0,0 +1,10 @@ +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); } /** @@ -120,10 +121,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) { @@ -144,10 +145,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) { @@ -349,12 +350,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); @@ -366,6 +370,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 index fa62420..b4d0ed6 100644 --- 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 index b0f3fe9..895961d 100644 --- 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'); From 3d675b9d40225b7d743ec9dc1d63d92c1e251ac2 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Wed, 14 Jun 2023 13:42:06 +0200 Subject: [PATCH 2/5] Attempt to fix test action --- .github/workflows/tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e92d278..89e2ad4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -24,6 +24,9 @@ jobs: php-version: ${{ matrix.php-versions }} tools: phpunit:latest + - name: Update dependencies + run: composer update + - name: Setup Laravel run: composer require -W --dev laravel/laravel:${{ matrix.laravel-versions }} From 8e2a3542d7a133843fb2c08dff6064a6d8f510e8 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Wed, 14 Jun 2023 13:51:26 +0200 Subject: [PATCH 3/5] New attempt to fix test action --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 89e2ad4..16d38eb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -22,7 +22,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} - tools: phpunit:latest + tools: phpunit:^9.0 - name: Update dependencies run: composer update From 5cc66a8ed55813b36b04c54b7dff3c516da47299 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Wed, 14 Jun 2023 13:52:57 +0200 Subject: [PATCH 4/5] Another attempt to fix test action --- .github/workflows/tests.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 16d38eb..d5cc392 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -22,14 +22,10 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-versions }} - tools: phpunit:^9.0 - - - name: Update dependencies - run: composer update - name: Setup Laravel run: composer require -W --dev laravel/laravel:${{ matrix.laravel-versions }} - name: Run tests - run: phpunit + run: ./vedor/bin/phpunit From 9777e9212cb51aa4f45a0d45f339a84acf4ae22a Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Wed, 14 Jun 2023 13:53:13 +0200 Subject: [PATCH 5/5] Fix typo --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d5cc392..888e451 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -27,5 +27,5 @@ jobs: run: composer require -W --dev laravel/laravel:${{ matrix.laravel-versions }} - name: Run tests - run: ./vedor/bin/phpunit + run: ./vendor/bin/phpunit