From c7fce8695418bc924c69ab22814a2e62b8225fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 22 Apr 2024 11:53:52 +0200 Subject: [PATCH 1/7] Enable TTL index to auto-purge of expired cache and lock items --- src/Cache/MongoLock.php | 34 ++++++++++++++++++++------ src/Cache/MongoStore.php | 38 +++++++++++++++++++++-------- tests/Cache/MongoCacheStoreTest.php | 21 ++++++++++++---- tests/Cache/MongoLockTest.php | 9 +++++++ 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index 105a3df40..81f6ffcce 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -3,6 +3,8 @@ namespace MongoDB\Laravel\Cache; use Illuminate\Cache\Lock; +use Illuminate\Support\Carbon; +use MongoDB\BSON\UTCDateTime; use MongoDB\Laravel\Collection; use MongoDB\Operation\FindOneAndUpdate; use Override; @@ -41,11 +43,11 @@ public function acquire(): bool // or it is already owned by the same lock instance. $isExpiredOrAlreadyOwned = [ '$or' => [ - ['$lte' => ['$expiration', $this->currentTime()]], + ['$lte' => ['$expiration', $this->getUTCDateTime()]], ['$eq' => ['$owner', $this->owner]], ], ]; - $result = $this->collection->findOneAndUpdate( + $result = $this->collection->updateOne( ['_id' => $this->name], [ [ @@ -74,11 +76,11 @@ public function acquire(): bool ], ); - if (random_int(1, $this->lottery[1]) <= $this->lottery[0]) { - $this->collection->deleteMany(['expiration' => ['$lte' => $this->currentTime()]]); + if (! empty($this->lottery[0]) && random_int(1, $this->lottery[1]) <= $this->lottery[0]) { + $this->collection->deleteMany(['expiration' => ['$lte' => $this->getUTCDateTime()]]); } - return $result['owner'] === $this->owner; + return $result->getModifiedCount() > 0 || $result->getUpsertedCount() > 0; } /** @@ -107,6 +109,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 + ['expiration' => 1], + // Delay to remove items after expiration + ['expireAfterSeconds' => 0], + ); + } + /** * Returns the owner value written into the driver for this lock. */ @@ -116,7 +129,7 @@ protected function getCurrentOwner(): ?string return $this->collection->findOne( [ '_id' => $this->name, - 'expiration' => ['$gte' => $this->currentTime()], + 'expiration' => ['$gte' => $this->getUTCDateTime()], ], ['projection' => ['owner' => 1]], )['owner'] ?? null; @@ -125,10 +138,15 @@ protected function getCurrentOwner(): ?string /** * Get the UNIX timestamp indicating when the lock should expire. */ - private function expiresAt(): int + private function expiresAt(): UTCDateTime { $lockTimeout = $this->seconds > 0 ? $this->seconds : $this->defaultTimeoutInSeconds; - return $this->currentTime() + $lockTimeout; + return $this->getUTCDateTime($lockTimeout); + } + + protected function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime + { + return new UTCDateTime(Carbon::now()->addSeconds($additionalSeconds)); } } diff --git a/src/Cache/MongoStore.php b/src/Cache/MongoStore.php index 4a01c9161..634eb1054 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; @@ -95,7 +95,7 @@ public function put($key, $value, $seconds): bool [ '$set' => [ 'value' => $this->serialize($value), - 'expiration' => $this->currentTime() + $seconds, + 'expiration' => $this->getUTCDateTime($seconds), ], ], [ @@ -116,6 +116,8 @@ public function put($key, $value, $seconds): bool */ public function add($key, $value, $seconds): bool { + $isExpired = ['$lte' => ['$expiration', $this->getUTCDateTime()]]; + $result = $this->collection->updateOne( [ '_id' => $this->prefix . $key, @@ -125,15 +127,15 @@ public function add($key, $value, $seconds): bool '$set' => [ 'value' => [ '$cond' => [ - 'if' => ['$lte' => ['$expiration', $this->currentTime()]], + 'if' => $isExpired, 'then' => $this->serialize($value), 'else' => '$value', ], ], 'expiration' => [ '$cond' => [ - 'if' => ['$lte' => ['$expiration', $this->currentTime()]], - 'then' => $this->currentTime() + $seconds, + 'if' => $isExpired, + 'then' => $this->getUTCDateTime($seconds), 'else' => '$expiration', ], ], @@ -163,7 +165,7 @@ public function get($key): mixed return null; } - if ($result['expiration'] <= $this->currentTime()) { + if ($result['expiration'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return null; @@ -186,7 +188,7 @@ public function increment($key, $value = 1): int|float|false $result = $this->collection->findOneAndUpdate( [ '_id' => $this->prefix . $key, - 'expiration' => ['$gte' => $this->currentTime()], + 'expiration' => ['$gte' => $this->getUTCDateTime()], ], [ '$inc' => ['value' => $value], @@ -200,7 +202,7 @@ public function increment($key, $value = 1): int|float|false return false; } - if ($result['expiration'] <= $this->currentTime()) { + if ($result['expiration'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return false; @@ -257,7 +259,7 @@ public function forgetIfExpired($key): bool { $result = $this->collection->deleteOne([ '_id' => $this->prefix . $key, - 'expiration' => ['$lte' => $this->currentTime()], + 'expiration' => ['$lte' => $this->getUTCDateTime()], ]); return $result->getDeletedCount() > 0; @@ -275,6 +277,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 + ['expiration' => 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 +306,9 @@ private function unserialize($value): mixed return unserialize($value); } + + protected 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..e7f375405 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(), + 'expiration' => new UTCDateTime(Carbon::now()->addSeconds($ttl)), ]); } } diff --git a/tests/Cache/MongoLockTest.php b/tests/Cache/MongoLockTest.php index d08ee899c..2f25fda7b 100644 --- a/tests/Cache/MongoLockTest.php +++ b/tests/Cache/MongoLockTest.php @@ -88,6 +88,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'); From 087e8fd910f4fc42941023017cc83ad4034f2154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 22 Apr 2024 13:46:58 +0200 Subject: [PATCH 2/7] Simplify constructor arguments of MongoLock --- src/Cache/MongoLock.php | 26 +++++++------------------- src/Cache/MongoStore.php | 7 +++---- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index 81f6ffcce..5961cc390 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -16,12 +16,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, @@ -29,7 +28,6 @@ public function __construct( int $seconds, ?string $owner = null, private readonly array $lottery = [2, 100], - private readonly int $defaultTimeoutInSeconds = 86400, ) { parent::__construct($name, $seconds, $owner); } @@ -62,7 +60,7 @@ public function acquire(): bool 'expiration' => [ '$cond' => [ 'if' => $isExpiredOrAlreadyOwned, - 'then' => $this->expiresAt(), + 'then' => $this->getUTCDateTime($this->seconds), 'else' => '$expiration', ], ], @@ -135,17 +133,7 @@ protected function getCurrentOwner(): ?string )['owner'] ?? null; } - /** - * Get the UNIX timestamp indicating when the lock should expire. - */ - private function expiresAt(): UTCDateTime - { - $lockTimeout = $this->seconds > 0 ? $this->seconds : $this->defaultTimeoutInSeconds; - - return $this->getUTCDateTime($lockTimeout); - } - - protected function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime + private function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime { return new UTCDateTime(Carbon::now()->addSeconds($additionalSeconds)); } diff --git a/src/Cache/MongoStore.php b/src/Cache/MongoStore.php index 634eb1054..c369b4556 100644 --- a/src/Cache/MongoStore.php +++ b/src/Cache/MongoStore.php @@ -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, ); } @@ -307,7 +306,7 @@ private function unserialize($value): mixed return unserialize($value); } - protected function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime + private function getUTCDateTime(int $additionalSeconds = 0): UTCDateTime { return new UTCDateTime(Carbon::now()->addSeconds($additionalSeconds)); } From e0da8e9861d6f71a7e96a8d0b35dcb313c30622e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 22 Apr 2024 14:00:17 +0200 Subject: [PATCH 3/7] Remove useless expiration condition in cache increment --- src/Cache/MongoStore.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Cache/MongoStore.php b/src/Cache/MongoStore.php index c369b4556..0cf25578d 100644 --- a/src/Cache/MongoStore.php +++ b/src/Cache/MongoStore.php @@ -182,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->getUTCDateTime()], ], [ '$inc' => ['value' => $value], From 4a5a3d8f39f2d060f225036394539414fad2399c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 23 Apr 2024 09:43:09 +0200 Subject: [PATCH 4/7] Rename expiration field to expires_at for naming consistency --- src/Cache/MongoLock.php | 14 +++++++------- src/Cache/MongoStore.php | 18 +++++++++--------- tests/Cache/MongoCacheStoreTest.php | 2 +- tests/Cache/MongoLockTest.php | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index 5961cc390..08e0fd770 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -41,7 +41,7 @@ public function acquire(): bool // or it is already owned by the same lock instance. $isExpiredOrAlreadyOwned = [ '$or' => [ - ['$lte' => ['$expiration', $this->getUTCDateTime()]], + ['$lte' => ['$expires_at', $this->getUTCDateTime()]], ['$eq' => ['$owner', $this->owner]], ], ]; @@ -57,11 +57,11 @@ public function acquire(): bool 'else' => '$owner', ], ], - 'expiration' => [ + 'expires_at' => [ '$cond' => [ 'if' => $isExpiredOrAlreadyOwned, 'then' => $this->getUTCDateTime($this->seconds), - 'else' => '$expiration', + 'else' => '$expires_at', ], ], ], @@ -74,8 +74,8 @@ public function acquire(): bool ], ); - if (! empty($this->lottery[0]) && random_int(1, $this->lottery[1]) <= $this->lottery[0]) { - $this->collection->deleteMany(['expiration' => ['$lte' => $this->getUTCDateTime()]]); + if ($this->lottery[0] <= 0 && random_int(1, $this->lottery[1]) <= $this->lottery[0]) { + $this->collection->deleteMany(['expires_at' => ['$lte' => $this->getUTCDateTime()]]); } return $result->getModifiedCount() > 0 || $result->getUpsertedCount() > 0; @@ -112,7 +112,7 @@ public function createTTLIndex(): void { $this->collection->createIndex( // UTCDateTime field that holds the expiration date - ['expiration' => 1], + ['expires_at' => 1], // Delay to remove items after expiration ['expireAfterSeconds' => 0], ); @@ -127,7 +127,7 @@ protected function getCurrentOwner(): ?string return $this->collection->findOne( [ '_id' => $this->name, - 'expiration' => ['$gte' => $this->getUTCDateTime()], + 'expires_at' => ['$gte' => $this->getUTCDateTime()], ], ['projection' => ['owner' => 1]], )['owner'] ?? null; diff --git a/src/Cache/MongoStore.php b/src/Cache/MongoStore.php index 0cf25578d..e35d0f70d 100644 --- a/src/Cache/MongoStore.php +++ b/src/Cache/MongoStore.php @@ -94,7 +94,7 @@ public function put($key, $value, $seconds): bool [ '$set' => [ 'value' => $this->serialize($value), - 'expiration' => $this->getUTCDateTime($seconds), + 'expires_at' => $this->getUTCDateTime($seconds), ], ], [ @@ -115,7 +115,7 @@ public function put($key, $value, $seconds): bool */ public function add($key, $value, $seconds): bool { - $isExpired = ['$lte' => ['$expiration', $this->getUTCDateTime()]]; + $isExpired = ['$lte' => ['$expires_at', $this->getUTCDateTime()]]; $result = $this->collection->updateOne( [ @@ -131,11 +131,11 @@ public function add($key, $value, $seconds): bool 'else' => '$value', ], ], - 'expiration' => [ + 'expires_at' => [ '$cond' => [ 'if' => $isExpired, 'then' => $this->getUTCDateTime($seconds), - 'else' => '$expiration', + 'else' => '$expires_at', ], ], ], @@ -157,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->getUTCDateTime()) { + if ($result['expires_at'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return null; @@ -198,7 +198,7 @@ public function increment($key, $value = 1): int|float|false return false; } - if ($result['expiration'] <= $this->getUTCDateTime()) { + if ($result['expires_at'] <= $this->getUTCDateTime()) { $this->forgetIfExpired($key); return false; @@ -255,7 +255,7 @@ public function forgetIfExpired($key): bool { $result = $this->collection->deleteOne([ '_id' => $this->prefix . $key, - 'expiration' => ['$lte' => $this->getUTCDateTime()], + 'expires_at' => ['$lte' => $this->getUTCDateTime()], ]); return $result->getDeletedCount() > 0; @@ -278,7 +278,7 @@ public function createTTLIndex(): void { $this->collection->createIndex( // UTCDateTime field that holds the expiration date - ['expiration' => 1], + ['expires_at' => 1], // Delay to remove items after expiration ['expireAfterSeconds' => 0], ); diff --git a/tests/Cache/MongoCacheStoreTest.php b/tests/Cache/MongoCacheStoreTest.php index e7f375405..6f4ee79f4 100644 --- a/tests/Cache/MongoCacheStoreTest.php +++ b/tests/Cache/MongoCacheStoreTest.php @@ -236,7 +236,7 @@ private function insertToCacheTable(string $key, $value, $ttl = 60) ->insertOne([ '_id' => $this->withCachePrefix($key), 'value' => $value, - 'expiration' => new UTCDateTime(Carbon::now()->addSeconds($ttl)), + 'expires_at' => new UTCDateTime(Carbon::now()->addSeconds($ttl)), ]); } } diff --git a/tests/Cache/MongoLockTest.php b/tests/Cache/MongoLockTest.php index 2f25fda7b..641b1b9e1 100644 --- a/tests/Cache/MongoLockTest.php +++ b/tests/Cache/MongoLockTest.php @@ -53,7 +53,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' => now()->subDays(1)->getTimestamp()]); $otherLock = $this->getCache()->lock('foo'); $this->assertTrue($otherLock->get()); From 33eb65508c63d72f7bb799d2b49a669b77356b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 23 Apr 2024 09:43:25 +0200 Subject: [PATCH 5/7] Validate lottery value --- src/Cache/MongoLock.php | 6 ++++++ tests/Cache/MongoLockTest.php | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index 08e0fd770..d87f20408 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -4,11 +4,13 @@ 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 @@ -29,6 +31,10 @@ public function __construct( ?string $owner = null, private readonly array $lottery = [2, 100], ) { + 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); } diff --git a/tests/Cache/MongoLockTest.php b/tests/Cache/MongoLockTest.php index 641b1b9e1..82709dfe8 100644 --- a/tests/Cache/MongoLockTest.php +++ b/tests/Cache/MongoLockTest.php @@ -5,8 +5,11 @@ use Illuminate\Cache\Repository; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; +use InvalidArgumentException; use MongoDB\Laravel\Cache\MongoLock; +use MongoDB\Laravel\Collection; use MongoDB\Laravel\Tests\TestCase; +use PHPUnit\Framework\Attributes\TestWith; use function now; @@ -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'); From 42e890cf76dfdbf7802783f889998d2914a29682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 23 Apr 2024 09:51:02 +0200 Subject: [PATCH 6/7] Revert lock acquire to use findOneAndUpdate --- src/Cache/MongoLock.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Cache/MongoLock.php b/src/Cache/MongoLock.php index d87f20408..e9bd3d607 100644 --- a/src/Cache/MongoLock.php +++ b/src/Cache/MongoLock.php @@ -51,7 +51,7 @@ public function acquire(): bool ['$eq' => ['$owner', $this->owner]], ], ]; - $result = $this->collection->updateOne( + $result = $this->collection->findOneAndUpdate( ['_id' => $this->name], [ [ @@ -84,7 +84,9 @@ public function acquire(): bool $this->collection->deleteMany(['expires_at' => ['$lte' => $this->getUTCDateTime()]]); } - return $result->getModifiedCount() > 0 || $result->getUpsertedCount() > 0; + // 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; } /** From 8d02b3174ca3cfa91883a81c5e5607e0b3e246a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 23 Apr 2024 09:56:47 +0200 Subject: [PATCH 7/7] Fix test using UTCDateTime --- tests/Cache/MongoLockTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Cache/MongoLockTest.php b/tests/Cache/MongoLockTest.php index 82709dfe8..e3d2568d5 100644 --- a/tests/Cache/MongoLockTest.php +++ b/tests/Cache/MongoLockTest.php @@ -3,16 +3,16 @@ 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 PHPUnit\Framework\Attributes\TestWith; -use function now; - class MongoLockTest extends TestCase { public function tearDown(): void @@ -73,7 +73,7 @@ public function testExpiredLockCanBeRetrieved() { $lock = $this->getCache()->lock('foo'); $this->assertTrue($lock->get()); - DB::table('foo_cache_locks')->update(['expires_at' => 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());