diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index 105a3df40..e9bd3d607 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -3,10 +3,14 @@ namespace MongoDB\Laravel\Cache; use Illuminate\Cache\Lock; +use Illuminate\Support\Carbon; +use InvalidArgumentException; +use MongoDB\BSON\UTCDateTime; use MongoDB\Laravel\Collection; use MongoDB\Operation\FindOneAndUpdate; use Override; +use function is_numeric; use function random_int; final class MongoLock extends Lock @@ -14,12 +18,11 @@ final class MongoLock extends Lock /** * Create a new lock instance. * - * @param Collection $collection The MongoDB collection - * @param string $name Name of the lock - * @param int $seconds Time-to-live of the lock in seconds - * @param string|null $owner A unique string that identifies the owner. Random if not set - * @param array $lottery The prune probability odds - * @param int $defaultTimeoutInSeconds The default number of seconds that a lock should be held + * @param Collection $collection The MongoDB collection + * @param string $name Name of the lock + * @param int $seconds Time-to-live of the lock in seconds + * @param string|null $owner A unique string that identifies the owner. Random if not set + * @param array{int, int} $lottery Probability [chance, total] of pruning expired cache items. Set to [0, 0] to disable */ public function __construct( private readonly Collection $collection, @@ -27,8 +30,11 @@ public function __construct( int $seconds, ?string $owner = null, private readonly array $lottery = [2, 100], - private readonly int $defaultTimeoutInSeconds = 86400, ) { + if (! is_numeric($this->lottery[0] ?? null) || ! is_numeric($this->lottery[1] ?? null) || $this->lottery[0] > $this->lottery[1]) { + throw new InvalidArgumentException('Lock lottery must be a couple of integers [$chance, $total] where $chance <= $total. Example [2, 100]'); + } + parent::__construct($name, $seconds, $owner); } @@ -41,7 +47,7 @@ public function acquire(): bool // or it is already owned by the same lock instance. $isExpiredOrAlreadyOwned = [ '$or' => [ - ['$lte' => ['$expiration', $this->currentTime()]], + ['$lte' => ['$expires_at', $this->getUTCDateTime()]], ['$eq' => ['$owner', $this->owner]], ], ]; @@ -57,11 +63,11 @@ public function acquire(): bool 'else' => '$owner', ], ], - 'expiration' => [ + 'expires_at' => [ '$cond' => [ 'if' => $isExpiredOrAlreadyOwned, - 'then' => $this->expiresAt(), - 'else' => '$expiration', + 'then' => $this->getUTCDateTime($this->seconds), + 'else' => '$expires_at', ], ], ], @@ -74,10 +80,12 @@ public function acquire(): bool ], ); - if (random_int(1, $this->lottery[1]) <= $this->lottery[0]) { - $this->collection->deleteMany(['expiration' => ['$lte' => $this->currentTime()]]); + if ($this->lottery[0] <= 0 && random_int(1, $this->lottery[1]) <= $this->lottery[0]) { + $this->collection->deleteMany(['expires_at' => ['$lte' => $this->getUTCDateTime()]]); } + // Compare the owner to check if the lock is owned. Acquiring the same lock + // with the same owner at the same instant would lead to not update the document return $result['owner'] === $this->owner; } @@ -107,6 +115,17 @@ public function forceRelease(): void ]); } + /** Creates a TTL index that automatically deletes expired objects. */ + public function createTTLIndex(): void + { + $this->collection->createIndex( + // UTCDateTime field that holds the expiration date + ['expires_at' => 1], + // Delay to remove items after expiration + ['expireAfterSeconds' => 0], + ); + } + /** * Returns the owner value written into the driver for this lock. */ @@ -116,19 +135,14 @@ protected function getCurrentOwner(): ?string return $this->collection->findOne( [ '_id' => $this->name, - 'expiration' => ['$gte' => $this->currentTime()], + 'expires_at' => ['$gte' => $this->getUTCDateTime()], ], ['projection' => ['owner' => 1]], )['owner'] ?? null; } - /** - * Get the UNIX timestamp indicating when the lock should expire. - */ - private function expiresAt(): int + private function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime { - $lockTimeout = $this->seconds > 0 ? $this->seconds : $this->defaultTimeoutInSeconds; - - return $this->currentTime() + $lockTimeout; + return new UTCDateTime(Carbon::now()->addSeconds($additionalSeconds)); } } diff --git a/src/Cache/MongoStore.php b/src/Cache/MongoStore.php index 4a01c9161..e35d0f70d 100644 --- a/src/Cache/MongoStore.php +++ b/src/Cache/MongoStore.php @@ -5,7 +5,8 @@ use Illuminate\Cache\RetrievesMultipleKeys; use Illuminate\Contracts\Cache\LockProvider; use Illuminate\Contracts\Cache\Store; -use Illuminate\Support\InteractsWithTime; +use Illuminate\Support\Carbon; +use MongoDB\BSON\UTCDateTime; use MongoDB\Laravel\Collection; use MongoDB\Laravel\Connection; use MongoDB\Operation\FindOneAndUpdate; @@ -20,7 +21,6 @@ final class MongoStore implements LockProvider, Store { - use InteractsWithTime; // Provides "many" and "putMany" in a non-optimized way use RetrievesMultipleKeys; @@ -34,7 +34,7 @@ final class MongoStore implements LockProvider, Store * @param string $prefix Prefix for the name of cache items * @param Connection|null $lockConnection The MongoDB connection to use for the lock, if different from the cache connection * @param string $lockCollectionName Name of the collection where locks are stored - * @param array{int, int} $lockLottery Probability [chance, total] of pruning expired cache items + * @param array{int, int} $lockLottery Probability [chance, total] of pruning expired cache items. Set to [0, 0] to disable * @param int $defaultLockTimeoutInSeconds Time-to-live of the locks in seconds */ public function __construct( @@ -62,10 +62,9 @@ public function lock($name, $seconds = 0, $owner = null): MongoLock return new MongoLock( ($this->lockConnection ?? $this->connection)->getCollection($this->lockCollectionName), $this->prefix . $name, - $seconds, + $seconds ?: $this->defaultLockTimeoutInSeconds, $owner, $this->lockLottery, - $this->defaultLockTimeoutInSeconds, ); } @@ -95,7 +94,7 @@ public function put($key, $value, $seconds): bool [ '$set' => [ 'value' => $this->serialize($value), - 'expiration' => $this->currentTime() + $seconds, + 'expires_at' => $this->getUTCDateTime($seconds), ], ], [ @@ -116,6 +115,8 @@ public function put($key, $value, $seconds): bool */ public function add($key, $value, $seconds): bool { + $isExpired = ['$lte' => ['$expires_at', $this->getUTCDateTime()]]; + $result = $this->collection->updateOne( [ '_id' => $this->prefix . $key, @@ -125,16 +126,16 @@ public function add($key, $value, $seconds): bool '$set' => [ 'value' => [ '$cond' => [ - 'if' => ['$lte' => ['$expiration', $this->currentTime()]], + 'if' => $isExpired, 'then' => $this->serialize($value), 'else' => '$value', ], ], - 'expiration' => [ + 'expires_at' => [ '$cond' => [ - 'if' => ['$lte' => ['$expiration', $this->currentTime()]], - 'then' => $this->currentTime() + $seconds, - 'else' => '$expiration', + 'if' => $isExpired, + 'then' => $this->getUTCDateTime($seconds), + 'else' => '$expires_at', ], ], ], @@ -156,14 +157,14 @@ public function get($key): mixed { $result = $this->collection->findOne( ['_id' => $this->prefix . $key], - ['projection' => ['value' => 1, 'expiration' => 1]], + ['projection' => ['value' => 1, 'expires_at' => 1]], ); if (! $result) { return null; } - if ($result['expiration'] <= $this->currentTime()) { + if ($result['expires_at'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return null; @@ -181,12 +182,9 @@ public function get($key): mixed #[Override] public function increment($key, $value = 1): int|float|false { - $this->forgetIfExpired($key); - $result = $this->collection->findOneAndUpdate( [ '_id' => $this->prefix . $key, - 'expiration' => ['$gte' => $this->currentTime()], ], [ '$inc' => ['value' => $value], @@ -200,7 +198,7 @@ public function increment($key, $value = 1): int|float|false return false; } - if ($result['expiration'] <= $this->currentTime()) { + if ($result['expires_at'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return false; @@ -257,7 +255,7 @@ public function forgetIfExpired($key): bool { $result = $this->collection->deleteOne([ '_id' => $this->prefix . $key, - 'expiration' => ['$lte' => $this->currentTime()], + 'expires_at' => ['$lte' => $this->getUTCDateTime()], ]); return $result->getDeletedCount() > 0; @@ -275,6 +273,17 @@ public function getPrefix(): string return $this->prefix; } + /** Creates a TTL index that automatically deletes expired objects. */ + public function createTTLIndex(): void + { + $this->collection->createIndex( + // UTCDateTime field that holds the expiration date + ['expires_at' => 1], + // Delay to remove items after expiration + ['expireAfterSeconds' => 0], + ); + } + private function serialize($value): string|int|float { // Don't serialize numbers, so they can be incremented @@ -293,4 +302,9 @@ private function unserialize($value): mixed return unserialize($value); } + + private function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime + { + return new UTCDateTime(Carbon::now()->addSeconds($additionalSeconds)); + } } diff --git a/tests/Cache/MongoCacheStoreTest.php b/tests/Cache/MongoCacheStoreTest.php index 4ee97e75a..6f4ee79f4 100644 --- a/tests/Cache/MongoCacheStoreTest.php +++ b/tests/Cache/MongoCacheStoreTest.php @@ -6,6 +6,7 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; +use MongoDB\BSON\UTCDateTime; use MongoDB\Laravel\Tests\TestCase; use function assert; @@ -200,7 +201,17 @@ public function testIncrementDecrement() $this->assertFalse($store->increment('foo', 5)); } - protected function getStore(): Repository + public function testTTLIndex() + { + $store = $this->getStore(); + $store->createTTLIndex(); + + // TTL index remove expired items asynchronously, this test would be very slow + $indexes = DB::connection('mongodb')->getCollection($this->getCacheCollectionName())->listIndexes(); + $this->assertCount(2, $indexes); + } + + private function getStore(): Repository { $repository = Cache::store('mongodb'); assert($repository instanceof Repository); @@ -208,24 +219,24 @@ protected function getStore(): Repository return $repository; } - protected function getCacheCollectionName(): string + private function getCacheCollectionName(): string { return config('cache.stores.mongodb.collection'); } - protected function withCachePrefix(string $key): string + private function withCachePrefix(string $key): string { return config('cache.prefix') . $key; } - protected function insertToCacheTable(string $key, $value, $ttl = 60) + private function insertToCacheTable(string $key, $value, $ttl = 60) { DB::connection('mongodb') ->getCollection($this->getCacheCollectionName()) ->insertOne([ '_id' => $this->withCachePrefix($key), 'value' => $value, - 'expiration' => Carbon::now()->addSeconds($ttl)->getTimestamp(), + 'expires_at' => new UTCDateTime(Carbon::now()->addSeconds($ttl)), ]); } } diff --git a/tests/Cache/MongoLockTest.php b/tests/Cache/MongoLockTest.php index d08ee899c..e3d2568d5 100644 --- a/tests/Cache/MongoLockTest.php +++ b/tests/Cache/MongoLockTest.php @@ -3,12 +3,15 @@ namespace MongoDB\Laravel\Tests\Cache; use Illuminate\Cache\Repository; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; +use InvalidArgumentException; +use MongoDB\BSON\UTCDateTime; use MongoDB\Laravel\Cache\MongoLock; +use MongoDB\Laravel\Collection; use MongoDB\Laravel\Tests\TestCase; - -use function now; +use PHPUnit\Framework\Attributes\TestWith; class MongoLockTest extends TestCase { @@ -19,6 +22,23 @@ public function tearDown(): void parent::tearDown(); } + #[TestWith([[5, 2]])] + #[TestWith([['foo', 10]])] + #[TestWith([[10, 'foo']])] + #[TestWith([[10]])] + public function testInvalidLottery(array $lottery) + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Lock lottery must be a couple of integers'); + + new MongoLock( + $this->createMock(Collection::class), + 'cache_lock', + 10, + lottery: $lottery, + ); + } + public function testLockCanBeAcquired() { $lock = $this->getCache()->lock('foo'); @@ -53,7 +73,7 @@ public function testExpiredLockCanBeRetrieved() { $lock = $this->getCache()->lock('foo'); $this->assertTrue($lock->get()); - DB::table('foo_cache_locks')->update(['expiration' => now()->subDays(1)->getTimestamp()]); + DB::table('foo_cache_locks')->update(['expires_at' => new UTCDateTime(Carbon::now('UTC')->subDays(1))]); $otherLock = $this->getCache()->lock('foo'); $this->assertTrue($otherLock->get()); @@ -88,6 +108,15 @@ public function testRestoreLock() $this->assertFalse($resoredLock->isOwnedByCurrentProcess()); } + public function testTTLIndex() + { + $store = $this->getCache()->lock('')->createTTLIndex(); + + // TTL index remove expired items asynchronously, this test would be very slow + $indexes = DB::connection('mongodb')->getCollection('foo_cache_locks')->listIndexes(); + $this->assertCount(2, $indexes); + } + private function getCache(): Repository { $repository = Cache::driver('mongodb');