Skip to content

Commit 9c0e475

Browse files
committed
refactor(lock): polish lock store review feedback
- Rename the lock store provider contract for interface consistency - Finalize immutable lock manager and lock implementations - Add native service return type and interface method docs - Localize lock exception messages - Clarify file lock cleanup and custom store docs - Address file force-release race pre-check and Predis prefix notes Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
1 parent 2819c58 commit 9c0e475

16 files changed

Lines changed: 105 additions & 37 deletions

File tree

system/Cache/Handlers/FileHandler.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
use CodeIgniter\Cache\Exceptions\CacheException;
1717
use CodeIgniter\Cache\LockStoreInterface;
18-
use CodeIgniter\Cache\LockStoreProvider;
18+
use CodeIgniter\Cache\LockStoreProviderInterface;
1919
use CodeIgniter\Cache\LockStores\FileLockStore;
2020
use CodeIgniter\I18n\Time;
2121
use Config\Cache;
@@ -26,7 +26,7 @@
2626
*
2727
* @see \CodeIgniter\Cache\Handlers\FileHandlerTest
2828
*/
29-
class FileHandler extends BaseHandler implements LockStoreProvider
29+
class FileHandler extends BaseHandler implements LockStoreProviderInterface
3030
{
3131
/**
3232
* Maximum key length.
@@ -50,7 +50,7 @@ class FileHandler extends BaseHandler implements LockStoreProvider
5050
*/
5151
protected $mode;
5252

53-
private ?LockStoreInterface $lockStore = null;
53+
private readonly LockStoreInterface $lockStore;
5454

5555
/**
5656
* Note: Use `CacheFactory::getHandler()` to instantiate.
@@ -74,6 +74,8 @@ public function __construct(Cache $config)
7474
$this->mode = $options['mode'];
7575
$this->prefix = $config->prefix;
7676

77+
$this->lockStore = new FileLockStore($this->path, $this->mode, $this->prefix);
78+
7779
helper('filesystem');
7880
}
7981

@@ -192,7 +194,7 @@ public function isSupported(): bool
192194

193195
public function lockStore(): LockStoreInterface
194196
{
195-
return $this->lockStore ??= new FileLockStore($this->path, $this->mode, $this->prefix);
197+
return $this->lockStore;
196198
}
197199

198200
/**

system/Cache/Handlers/PredisHandler.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
namespace CodeIgniter\Cache\Handlers;
1515

1616
use CodeIgniter\Cache\LockStoreInterface;
17-
use CodeIgniter\Cache\LockStoreProvider;
17+
use CodeIgniter\Cache\LockStoreProviderInterface;
1818
use CodeIgniter\Cache\LockStores\PredisLockStore;
1919
use CodeIgniter\Exceptions\CriticalError;
2020
use CodeIgniter\I18n\Time;
@@ -29,7 +29,7 @@
2929
*
3030
* @see \CodeIgniter\Cache\Handlers\PredisHandlerTest
3131
*/
32-
class PredisHandler extends BaseHandler implements LockStoreProvider
32+
class PredisHandler extends BaseHandler implements LockStoreProviderInterface
3333
{
3434
/**
3535
* Default config
@@ -210,6 +210,7 @@ public function isSupported(): bool
210210

211211
public function lockStore(): LockStoreInterface
212212
{
213+
// Predis applies the configured prefix at the client level.
213214
return $this->lockStore ??= new PredisLockStore($this->redis);
214215
}
215216

system/Cache/Handlers/RedisHandler.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
namespace CodeIgniter\Cache\Handlers;
1515

1616
use CodeIgniter\Cache\LockStoreInterface;
17-
use CodeIgniter\Cache\LockStoreProvider;
17+
use CodeIgniter\Cache\LockStoreProviderInterface;
1818
use CodeIgniter\Cache\LockStores\RedisLockStore;
1919
use CodeIgniter\Exceptions\CriticalError;
2020
use CodeIgniter\I18n\Time;
@@ -27,7 +27,7 @@
2727
*
2828
* @see \CodeIgniter\Cache\Handlers\RedisHandlerTest
2929
*/
30-
class RedisHandler extends BaseHandler implements LockStoreProvider
30+
class RedisHandler extends BaseHandler implements LockStoreProviderInterface
3131
{
3232
/**
3333
* Default config

system/Cache/LockStoreInterface.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,28 @@
1515

1616
interface LockStoreInterface
1717
{
18+
/**
19+
* Attempts to acquire a lock for the given owner and TTL.
20+
*/
1821
public function acquireLock(string $key, string $owner, int $ttl): bool;
1922

23+
/**
24+
* Releases the lock only when it is currently held by the given owner.
25+
*/
2026
public function releaseLock(string $key, string $owner): bool;
2127

28+
/**
29+
* Releases the lock without checking ownership.
30+
*/
2231
public function forceReleaseLock(string $key): bool;
2332

33+
/**
34+
* Extends the lock TTL only when it is currently held by the given owner.
35+
*/
2436
public function refreshLock(string $key, string $owner, int $ttl): bool;
2537

38+
/**
39+
* Returns the current owner token, or null when the lock is not held.
40+
*/
2641
public function getLockOwner(string $key): ?string;
2742
}

system/Cache/LockStoreProvider.php renamed to system/Cache/LockStoreProviderInterface.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313

1414
namespace CodeIgniter\Cache;
1515

16-
interface LockStoreProvider
16+
interface LockStoreProviderInterface
1717
{
18+
/**
19+
* Returns the atomic lock store for this cache handler.
20+
*/
1821
public function lockStore(): LockStoreInterface;
1922
}

system/Cache/LockStores/FileLockStore.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ public function releaseLock(string $key, string $owner): bool
5656

5757
public function forceReleaseLock(string $key): bool
5858
{
59-
return ! is_file($this->path . FileHandler::validateKey($key, $this->prefix))
60-
|| $this->withLockFile($key, static fn ($handle): bool => self::clearLockFile($handle), false);
59+
return $this->withLockFile($key, static fn ($handle): bool => self::clearLockFile($handle));
6160
}
6261

6362
public function refreshLock(string $key, string $owner, int $ttl): bool

system/Config/Services.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,8 @@ public static function cache(?Cache $config = null, bool $getShared = true)
134134

135135
/**
136136
* The locks service provides atomic locks backed by supported cache handlers.
137-
*
138-
* @return LockManager
139137
*/
140-
public static function locks(?CacheInterface $cache = null, bool $getShared = true)
138+
public static function locks(?CacheInterface $cache = null, bool $getShared = true): LockManager
141139
{
142140
if ($getShared) {
143141
return static::getSharedInstance('locks', $cache);

system/Language/en/Lock.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
// Lock language settings
15+
return [
16+
'unsupportedStore' => 'The cache handler "{0}" does not support locks.',
17+
];

system/Lock/Exceptions/LockException.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,6 @@ class LockException extends FrameworkException
1919
{
2020
public static function forUnsupportedStore(string $class): self
2121
{
22-
return new self(sprintf('The cache handler "%s" does not support locks.', $class));
22+
return new self(lang('Lock.unsupportedStore', [$class]));
2323
}
2424
}

system/Lock/Lock.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@
1717
use CodeIgniter\Cache\LockStoreInterface;
1818
use CodeIgniter\Exceptions\InvalidArgumentException;
1919

20-
class Lock implements LockInterface
20+
final readonly class Lock implements LockInterface
2121
{
22+
private const BLOCK_RETRY_MICROSECONDS = 100_000;
23+
2224
public function __construct(
23-
private readonly LockStoreInterface $store,
24-
private readonly string $key,
25-
private readonly int $ttl,
26-
private readonly string $owner,
25+
private LockStoreInterface $store,
26+
private string $key,
27+
private int $ttl,
28+
private string $owner,
2729
) {
2830
if ($ttl < 1) {
2931
throw new InvalidArgumentException('Lock TTL must be a positive integer.');
@@ -52,7 +54,7 @@ public function block(int $seconds): bool
5254
return true;
5355
}
5456

55-
usleep(100_000);
57+
usleep(self::BLOCK_RETRY_MICROSECONDS);
5658
} while (microtime(true) < $expiresAt);
5759

5860
return false;

0 commit comments

Comments
 (0)