diff --git a/src/Database/Database.php b/src/Database/Database.php index 5f8d66de2..12d85006b 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3323,7 +3323,7 @@ private function cleanup( /** * Persist metadata with automatic rollback on failure * - * A failure raised after the metadata write committed (its cache invalidation or events), or a commit that + * A failure raised after the metadata write committed (its events), or a commit that * could not be confirmed, is rethrown unchanged and rolls nothing back: the definition it reports on may be stored. * * @param callable|null $rollbackOperation Cleanup operation to run if persistence fails (null if no cleanup needed) diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index e37fcda84..c73dbef17 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -16,6 +16,7 @@ use Utopia\Database\Attribute; use Utopia\Database\Cache\Epoch; use Utopia\Database\Cache\Owners; +use Utopia\Database\Cache\Registration; use Utopia\Database\Cache\Scope; use Utopia\Database\Capability; use Utopia\Database\Change; @@ -3545,7 +3546,9 @@ private function purgeCommittedCollection(string $collection): void } /** - * Purge a document's cache slot, and once more after the open invalidation scope ends. + * Purge a document's cache slot, and once more after the open invalidation scope ends. A write whose purge + * cannot be confirmed blocks its collection's epoch instead, before it commits, so the document cached + * before it is never served again; only when that fails too is the write refused. * * @throws Exception */ @@ -3575,7 +3578,20 @@ protected function purgeCachedDocumentInternal(string $collectionId, ?string $id ); } - $this->withRetries(fn () => $this->cache->purge($documentKey)); + $scoped = $collectionId !== self::METADATA && isset($this->documentCacheMutations[$context]); + try { + if ($scoped && isset($this->documentCacheMutations[$context][$collectionKey])) { + $this->cache->purge($documentKey); + } else { + $this->withRetries(fn () => $this->cache->purge($documentKey)); + } + } catch (Throwable $error) { + if (! $scoped) { + throw $error; + } + + $this->advanceDocumentCacheEpoch($collectionKey, $this->getDefinitionCacheKey($collectionId)); + } return true; } @@ -3619,7 +3635,7 @@ private function purgeCachedDefinitions(): void * Purge the documents a transaction wrote once it has committed or rolled back. A reader * outside the transaction may have cached the pre-commit row after the purge inside it, so * when this purge fails the collection's epoch is retired instead, which no such fill survives. - * A collection definition has no epoch of its own: its purge is tried once more. + * A collection definition has no epoch of its own, so its failed purge is the one thrown. * * @param array $documents Collection keys by document key */ @@ -3628,20 +3644,29 @@ protected function purgeWrittenDocuments(array $documents): void $definitions = $this->documentCacheDefinitions[$this->getEventContext()] ?? []; $failure = null; $retired = []; + $reachable = true; foreach ($documents as $documentKey => $collectionKey) { + if (isset($retired[$collectionKey])) { + continue; + } try { - $this->cache->purge($documentKey); + if ($reachable) { + $this->withRetries(fn () => $this->cache->purge($documentKey)); + } else { + $this->cache->purge($documentKey); + } } catch (Throwable $error) { - $failure ??= $error; + $reachable = false; + if (! isset($definitions[$collectionKey])) { + $failure ??= $error; + + continue; + } try { - if (! isset($definitions[$collectionKey])) { - $this->cache->purge($documentKey); - } elseif (! isset($retired[$collectionKey])) { - $retired[$collectionKey] = true; - $this->advanceDocumentCacheEpoch($collectionKey, $definitions[$collectionKey]); - } + $this->advanceDocumentCacheEpoch($collectionKey, $definitions[$collectionKey]); + $retired[$collectionKey] = true; } catch (Throwable) { - // The purge failure below reaches the caller either way. + $failure ??= $error; } } } @@ -3838,14 +3863,7 @@ private function activateDocumentCacheEpoch(string $collectionKey, string $token if ($owner !== false && $owner !== null && $owner !== $token) { throw new RuntimeException("Invalid document cache owner '{$token}' for '{$collectionKey}'"); } - $owned = $owner === $token; - if ($owned && ! $this->cache->purge($registration->key, $registration->field)) { - $owner = $this->cache->load($registration->key, self::TTL, $registration->field); - if ($owner !== false && $owner !== null) { - throw new RuntimeException("Failed to release document cache owner '{$token}' for '{$collectionKey}'"); - } - $owned = false; - } + $owned = $owner === $token && $this->releaseDocumentCacheOwner($registration, $collectionKey, $token); $startedKey = $collectionKey.'#started'; $finishedKey = $collectionKey.'#finished'; @@ -3874,19 +3892,7 @@ private function activateDocumentCacheEpoch(string $collectionKey, string $token return; } - $this->cache->purge($finishedKey); - $nextFinished = $this->cache->getGeneration($finishedKey); - - // A cache flush restarts generations, so an unchanged #finished - // only proves this purge was lost while the epoch read with it is - // still in place; publish the new epoch after this check, not before. - if ( - $nextFinished === $finished - && \is_string($epoch) - && $this->cache->load($epochKey, self::DOCUMENT_CACHE_PERMANENT) === $epoch - ) { - throw new RuntimeException("Failed to finish document cache invalidation '{$epochKey}'"); - } + $nextFinished = $this->finishDocumentCacheWrite($collectionKey, $finished, $epoch); $nextStarted = $this->cache->getGeneration($startedKey); if ($nextStarted === $nextFinished) { @@ -3912,11 +3918,68 @@ private function publishDocumentCacheEpoch(string $collectionKey, string $starte { $epochKey = $collectionKey.'#epoch'; $epoch = self::DOCUMENT_CACHE_ACTIVE_PREFIX.\bin2hex(\random_bytes(16)).self::DOCUMENT_CACHE_SEPARATOR.$started; - if ($this->cache->save($epochKey, $epoch) === false) { - throw new RuntimeException("Failed to activate document cache epoch '{$epochKey}'"); - } + $this->withRetries(function () use ($epochKey, $epoch): void { + if ($this->cache->save($epochKey, $epoch) === false) { + throw new RuntimeException("Failed to activate document cache epoch '{$epochKey}'"); + } + }); - $this->purgeCachedDefinition($definitionKey); + $this->withRetries(fn () => $this->purgeCachedDefinition($definitionKey)); + } + + /** + * Release this write's registration, and report whether it was still this write's to release. A purge + * that cannot be confirmed is retried while the registration is still in place. + */ + private function releaseDocumentCacheOwner(Registration $registration, string $collectionKey, string $token): bool + { + $released = true; + $this->withRetries(function () use ($registration, $collectionKey, $token, &$released): void { + if ($this->cache->purge($registration->key, $registration->field)) { + return; + } + + $owner = $this->cache->load($registration->key, self::TTL, $registration->field); + if ($owner !== false && $owner !== null) { + throw new RuntimeException("Failed to release document cache owner '{$token}' for '{$collectionKey}'"); + } + $released = false; + }); + + return $released; + } + + /** + * Count this write as finished and return the `#finished` generation after it. A cache flush restarts + * generations, so an unchanged `#finished` only proves the purge was lost while the epoch read with it + * is still in place; the purge is retried only then, so a purge that did land is never counted twice. + */ + private function finishDocumentCacheWrite(string $collectionKey, string $finished, mixed $epoch): string + { + $finishedKey = $collectionKey.'#finished'; + $epochKey = $collectionKey.'#epoch'; + $next = $finished; + $attempted = false; + $this->withRetries(function () use ($finishedKey, $epochKey, $finished, $epoch, &$next, &$attempted): void { + if ($attempted) { + $next = $this->cache->getGeneration($finishedKey); + } + if ($next === $finished) { + $attempted = true; + $this->cache->purge($finishedKey); + } + + $next = $this->cache->getGeneration($finishedKey); + if ( + $next === $finished + && \is_string($epoch) + && $this->cache->load($epochKey, self::DOCUMENT_CACHE_PERMANENT) === $epoch + ) { + throw new RuntimeException("Failed to finish document cache invalidation '{$epochKey}'"); + } + }); + + return $next; } /** diff --git a/src/Database/Trait/Transactions.php b/src/Database/Trait/Transactions.php index da8eefdea..a1ec7861c 100644 --- a/src/Database/Trait/Transactions.php +++ b/src/Database/Trait/Transactions.php @@ -294,26 +294,9 @@ private function withInvalidationScope(callable $callback): mixed $this->documentPurgeEvents[$context], ); - $failure = null; - try { - $this->purgeWrittenDocuments($documents); - } catch (Throwable $error) { - $failure = $error; - } - try { - $this->activateDocumentInvalidation($documentTokens); - } catch (Throwable $error) { - $failure ??= $error; - } - try { - $this->activateInvalidation($queryTokens); - } catch (Throwable $error) { - $failure ??= $error; - } - - $announcement = $this->announceDocumentPurges($purgeEvents); - $failure ??= $announcement; + $this->invalidateCommitted($documents, $documentTokens, $queryTokens); + $failure = $this->announceDocumentPurges($purgeEvents); if ($failure !== null) { $this->committedFailures ??= new WeakMap(); $this->committedFailures[$failure] = true; @@ -327,6 +310,32 @@ private function withInvalidationScope(callable $callback): mixed return $result; } + /** + * Invalidate the cache for writes that have committed. The writes are stored whatever the cache answers, and each + * step that cannot be confirmed leaves the cache fail-closed: a tombstone left in place lapses into a new epoch, + * never back to the old one. A failure is logged, as throwing it would report a stored write as failed. + * + * @param array $documents Collection keys by document key + * @param array $documentTokens + * @param array $queryTokens + */ + private function invalidateCommitted(array $documents, array $documentTokens, array $queryTokens): void + { + $steps = [ + fn () => $this->purgeWrittenDocuments($documents), + fn () => $this->activateDocumentInvalidation($documentTokens), + fn () => $this->activateInvalidation($queryTokens), + ]; + + foreach ($steps as $step) { + try { + $step(); + } catch (Throwable $error) { + Console::warning('Warning: Failed to invalidate the cache after a commit: '.$error->getMessage()); + } + } + } + private function queueDefinitionRefill(string $definitionKey, string $field, string $id): void { $tenant = $this->adapter->getTenant(); @@ -370,7 +379,7 @@ private function cacheTransactionDefinitions(array $refills): void /** * Whether the error was raised after its outermost transaction committed: the writes it - * reports on are stored, and only the invalidation or the events after the commit failed. + * reports on are stored, and only the events after the commit failed. */ private function failedAfterCommit(Throwable $error): bool { diff --git a/tests/unit/Cache/DatabaseTest.php b/tests/unit/Cache/DatabaseTest.php index e14d097e4..e20c01bcd 100644 --- a/tests/unit/Cache/DatabaseTest.php +++ b/tests/unit/Cache/DatabaseTest.php @@ -616,12 +616,8 @@ public function testActivationFailureAfterCommitLeavesSharedEpochBlocked(): void ); $cache->failActivations(); - try { - $writer->updateDocument('users', 'existing', new Document(['name' => 'updated'])); - $this->fail('Post-commit query-cache activation failure was not propagated'); - } catch (\RuntimeException $exception) { - $this->assertStringContainsString('activate query cache', $exception->getMessage()); - } + $updated = $writer->updateDocument('users', 'existing', new Document(['name' => 'updated'])); + $this->assertSame('updated', $updated->getAttribute('name'), 'A stored write must not be reported as failed because its query-cache activation failed'); $readerAdapter->observeFinds('users'); $this->assertSame( diff --git a/tests/unit/Cache/UnavailableCache.php b/tests/unit/Cache/UnavailableCache.php index c84a599ec..c207cb840 100644 --- a/tests/unit/Cache/UnavailableCache.php +++ b/tests/unit/Cache/UnavailableCache.php @@ -21,6 +21,8 @@ final class UnavailableCache implements CacheAdapter, Leasable private int $failingPurges = 0; + private int $failingSaves = 0; + public function __construct() { $this->cache = new RedisLeasableCache(); @@ -44,6 +46,14 @@ public function failNextPurges(int $count): void $this->failingPurges = $count; } + /** + * A blip on writes: the next saves report false, later ones are stored. + */ + public function failNextSaves(int $count): void + { + $this->failingSaves = $count; + } + public function recover(): void { $this->open = false; @@ -77,6 +87,12 @@ public function load(string $key, int $ttl, string $hash = ''): mixed #[\Override] public function save(string $key, array|string $data, string $hash = '', int $ttl = 0): bool|string|array { + if ($this->failingSaves > 0) { + $this->failingSaves--; + + return false; + } + return $this->writable() ? $this->cache->save($key, $data, $hash) : false; } diff --git a/tests/unit/CoreMinorsTest.php b/tests/unit/CoreMinorsTest.php index 871de5a6b..31b1d9589 100644 --- a/tests/unit/CoreMinorsTest.php +++ b/tests/unit/CoreMinorsTest.php @@ -337,7 +337,7 @@ public function testCreateIndexKeepsItsIndexWhenTheInvalidationAfterTheCommitFai $this->assertSame(0, $deletes, 'An index whose definition committed must not be rolled back'); $this->assertSame(1, $writes, 'A write that committed must not be repeated'); - $this->assertSame($failures[0] ?? null, $error, 'The failure after the commit must reach the caller as it was raised'); + $this->assertNull($error, 'A stored write must not be reported as failed because the invalidation after its commit failed'); $this->assertSame(['by_count'], $this->indexKeys($database, 'logs')); $this->assertTrue($this->hasSchemaIndex($database, 'logs', 'by_count'), 'The committed index must still exist'); } @@ -423,7 +423,7 @@ public function testCreateRelationshipKeepsItsWorkWhenTheInvalidationAfterTheCom $this->assertSame(['_index_profile'], $this->indexKeys($database, 'accounts'), 'The two-way index must still be created'); $this->assertTrue($this->hasSchemaIndex($database, 'profiles', '_index_account')); $this->assertTrue($this->hasSchemaIndex($database, 'accounts', '_index_profile')); - $this->assertSame($failures[0] ?? null, $error, 'The failure after the commit must reach the caller as it was raised'); + $this->assertNull($error, 'A stored write must not be reported as failed because the invalidation after its commit failed'); } /** diff --git a/tests/unit/CreateCollectionCleanupTest.php b/tests/unit/CreateCollectionCleanupTest.php index e66535753..2764ec843 100644 --- a/tests/unit/CreateCollectionCleanupTest.php +++ b/tests/unit/CreateCollectionCleanupTest.php @@ -98,10 +98,9 @@ public function commitTransaction(): bool $adapter->afterCommit = null; $cache->failing = false; - $this->assertNotNull($error, 'A failed invalidation after the commit must reach the caller'); + $this->assertNull($error, 'A stored collection must not be reported as failed because the invalidation after its commit failed'); $this->assertTrue($adapter->collectionExists('cleanup', 'logs'), 'A collection whose definition committed must keep its table'); $this->assertSame('logs', $database->getCollection('logs')->getId()); - $this->assertSame($failure, $error, 'The failure after the commit must reach the caller as it was raised'); $database->createDocument('logs', new Document(['message' => 'kept'])); $this->assertSame(1, $database->count('logs')); diff --git a/tests/unit/Documents/CacheUnavailableTest.php b/tests/unit/Documents/CacheUnavailableTest.php index 262bdc0fa..b0bd57f3f 100644 --- a/tests/unit/Documents/CacheUnavailableTest.php +++ b/tests/unit/Documents/CacheUnavailableTest.php @@ -104,6 +104,32 @@ public static function blips(): array ]; } + /** + * A purge that outlasts its retries used to refuse the write. Blocking the collection's epoch before the commit + * keeps the document cached before the write from being served, so the write commits instead. + * + * @param Closure(Database): mixed $write + */ + #[DataProvider('singleWrites')] + public function testAWritePastThePurgeRetriesCommitsBehindATombstone(Closure $write): void + { + $cache = new UnavailableCache(); + $database = $this->createDatabase($cache); + $cache->failNextPurges(3); + + $failure = $this->attempt(fn (): mixed => $write($database)); + + $this->assertNull($failure, 'A write the cache can still block must not be refused'); + $this->assertSame('updated', $this->stored($database)); + $this->assertSame('updated', $this->cached($database), 'The document must not be served from the cache as it was before the write'); + } + + /** @return array */ + public static function singleWrites(): array + { + return \array_diff_key(self::blips(), ['batch update' => true]); + } + public function testAPurgeFailingAfterTheCommitRetiresTheCollectionInstead(): void { $cache = new UnavailableCache(); @@ -111,13 +137,53 @@ public function testAPurgeFailingAfterTheCommitRetiresTheCollectionInstead(): vo $database = $this->createDatabase($cache, $adapter); $adapter->afterCommit = $cache->failWrites(...); - $this->attempt(fn (): Document => $database->updateDocument('webhooks', 'hook', new Document(['name' => 'updated']))); + $failure = $this->attempt(fn (): Document => $database->updateDocument('webhooks', 'hook', new Document(['name' => 'updated']))); $cache->recover(); + $this->assertNull($failure, 'A stored write must not be reported as failed because its cache invalidation failed'); $this->assertSame('updated', $this->stored($database)); $this->assertSame('updated', $this->cached($database), 'A committed write whose purge failed must not leave the old document cached'); } + /** + * Publishing the epoch after a batch write commits can miss a Redis timeout. The write is stored and its + * tombstone stays in place, so it must report success while readers keep bypassing the cache. + */ + public function testACommittedWriteWhoseEpochCannotBePublishedStaysFailClosed(): void + { + $cache = new UnavailableCache(); + $adapter = new CommitHookMemory(); + $database = $this->createDatabase($cache, $adapter); + $adapter->afterCommit = fn () => $cache->failNextSaves(3); + + $failure = $this->attempt(fn (): int => $database->updateDocuments( + 'webhooks', + new Document(['name' => 'updated']), + [Query::equal('$id', ['hook'])], + )); + + $this->assertNull($failure, 'A stored write must not be reported as failed because its epoch could not be published'); + $this->assertSame('updated', $this->stored($database)); + $this->assertSame('updated', $this->cached($database)); + $this->assertSame('updated', $this->cached($database), 'A refill while the tombstone stands must not cache the old document'); + } + + public function testAnEpochPublishBlipAfterTheCommitIsRetried(): void + { + $cache = new UnavailableCache(); + $adapter = new CommitHookMemory(); + $database = $this->createDatabase($cache, $adapter); + $adapter->afterCommit = fn () => $cache->failNextSaves(2); + + $database->updateDocuments('webhooks', new Document(['name' => 'updated']), [Query::equal('$id', ['hook'])]); + + $collectionKey = $database->getCacheBaseKeys('webhooks')[0]; + $epoch = $database->getCache()->load($collectionKey.'#epoch', Database::TTL); + $this->assertIsString($epoch); + $this->assertStringStartsNotWith('blocked', $epoch, 'A blip shorter than the retries must still publish a fresh epoch'); + $this->assertSame('updated', $this->cached($database)); + } + public function testPurgeCachedCollectionReportsACacheThatCannotConfirmIt(): void { $cache = new UnavailableCache(); diff --git a/tests/unit/Documents/DocumentCacheEpochTest.php b/tests/unit/Documents/DocumentCacheEpochTest.php index 1152e0ef8..eb8382051 100644 --- a/tests/unit/Documents/DocumentCacheEpochTest.php +++ b/tests/unit/Documents/DocumentCacheEpochTest.php @@ -98,12 +98,7 @@ public function testActivationFailureAfterCommitLeavesTheEpochBlocked(): void $this->assertSame('original', $database->getDocument('webhooks', 'hook')->getAttribute('name')); $cache->failActivations(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->fail('Document cache activation failure was not propagated'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('activate document cache epoch', $error->getMessage()); - } + $this->assertSame(1, $this->renameDocument($database, 'webhooks', 'hook', 'updated'), 'A stored write must not be reported as failed because its cache activation failed'); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); UncachedTwin::of($database)->updateDocument('webhooks', 'hook', new Document(['name' => 'changed'])); @@ -134,15 +129,10 @@ public function testActivationFailureDoesNotStrandOtherCollectionEpochs(): void $this->assertSame('original', $database->getDocument('logs', 'log')->getAttribute('name')); $cache->failActivations('collection:webhooks#epoch'); - try { - $database->withTransaction(function () use ($database): void { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->renameDocument($database, 'logs', 'log', 'updated'); - }); - $this->fail('Document cache activation failure was not propagated'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('activate document cache epoch', $error->getMessage()); - } + $database->withTransaction(function () use ($database): void { + $this->renameDocument($database, 'webhooks', 'hook', 'updated'); + $this->renameDocument($database, 'logs', 'log', 'updated'); + }); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); UncachedTwin::of($database)->updateDocument('webhooks', 'hook', new Document(['name' => 'changed'])); @@ -207,7 +197,7 @@ public function testCacheFlushAfterActivationReadsGenerationsDoesNotFailTheCommi $this->assertNull($database->findCollection('webhooks')); } - public function testActivationPurgeFailureStillPropagates(): void + public function testALostFinishAfterCommitDoesNotFailTheWrite(): void { $cache = new FlushDuringActivationMemory(); $database = $this->createDatabaseWithCache($cache); @@ -218,12 +208,7 @@ public function testActivationPurgeFailureStillPropagates(): void $this->assertTrue($cache->flush()); $cache->failDuringActivation(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->fail('Document cache activation purge failure was not propagated'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('finish document cache invalidation', $error->getMessage()); - } + $this->assertSame(1, $this->renameDocument($database, 'webhooks', 'hook', 'updated'), 'A stored write must not be reported as failed because its cache activation failed'); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); } @@ -292,7 +277,7 @@ public function testOverlappingWritesSucceedOnACacheWithoutFields(): void $this->assertSame('updated', $writer->getDocument('webhooks', 'hook')->getAttribute('name')); } - public function testActivationRejectsACorruptedOwnerRegistration(): void + public function testACorruptedOwnerRegistrationDoesNotFailTheCommittedWrite(): void { $cache = new RedisLeasableCache(); $database = $this->createDatabaseWithCache($cache); @@ -302,12 +287,7 @@ public function testActivationRejectsACorruptedOwnerRegistration(): void ])); $cache->corruptFieldWrites(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->fail('A corrupted document cache owner registration was accepted'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('Invalid document cache owner', $error->getMessage()); - } + $this->assertSame(1, $this->renameDocument($database, 'webhooks', 'hook', 'updated'), 'A stored write must not be reported as failed because its cache activation failed'); $cache->corruptFieldWrites(false); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); @@ -315,7 +295,7 @@ public function testActivationRejectsACorruptedOwnerRegistration(): void $this->assertSame('changed', $database->getDocument('webhooks', 'hook')->getAttribute('name'), 'A collection whose activation was refused stays uncached'); } - public function testActivationPropagatesAnOwnerReleaseFailure(): void + public function testAnOwnerReleaseFailureDoesNotFailTheCommittedWrite(): void { $cache = new RedisLeasableCache(); $database = $this->createDatabaseWithCache($cache); @@ -325,12 +305,7 @@ public function testActivationPropagatesAnOwnerReleaseFailure(): void ])); $cache->failFieldPurges(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->fail('A document cache owner release failure was not propagated'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('Failed to release document cache owner', $error->getMessage()); - } + $this->assertSame(1, $this->renameDocument($database, 'webhooks', 'hook', 'updated'), 'A stored write must not be reported as failed because its cache activation failed'); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); UncachedTwin::of($database)->updateDocument('webhooks', 'hook', new Document(['name' => 'changed'])); @@ -427,12 +402,7 @@ public function testALostActivationDoesNotKeepTheCollectionUncached(): void $database = $this->createCountedDatabase($adapter, $cache); $cache->failActivations(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'updated'); - $this->fail('The activation did not fail'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('activate document cache epoch', $error->getMessage()); - } + $this->renameDocument($database, 'webhooks', 'hook', 'updated'); $this->assertSame('updated', $database->getDocument('webhooks', 'hook')->getAttribute('name')); UncachedTwin::of($database)->updateDocument('webhooks', 'hook', new Document(['name' => 'changed'])); @@ -454,12 +424,7 @@ public function testAWriteAfterAnAbandonedWriteReenablesTheCollection(): void $database = $this->createCountedDatabase($adapter, $cache); $cache->abandonNextWrite(); - try { - $this->renameDocument($database, 'webhooks', 'hook', 'abandoned'); - $this->fail('The abandoned write released its registration'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('release document cache owner', $error->getMessage()); - } + $this->renameDocument($database, 'webhooks', 'hook', 'abandoned'); $this->renameDocument($database, 'webhooks', 'hook', 'second'); $this->assertSame('second', $database->getDocument('webhooks', 'hook')->getAttribute('name')); @@ -484,17 +449,12 @@ public function testAWriterYoungerThanTheTimeoutKeepsItsBlockUntilItsActivation( try { $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - try { - $writer->withTransaction(function () use ($writer, $reader, $cache): void { - $this->renameDocument($writer, 'users', 'user', 'updated'); - $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - $cache->failActivations(); - }); - $this->fail('The activation did not fail'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('activate document cache epoch', $error->getMessage()); - } + $writer->withTransaction(function () use ($writer, $reader, $cache): void { + $this->renameDocument($writer, 'users', 'user', 'updated'); + $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); + $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); + $cache->failActivations(); + }); $this->assertSame('updated', $reader->getDocument('users', 'user')->getAttribute('name'), 'A reader must not cache what it read while a write younger than the writer timeout was in flight'); } finally { @@ -531,18 +491,13 @@ public function testAWriteFinishingWhileAnotherIsInFlightLeavesTheCollectionBloc try { $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - try { - $writer->withTransaction(function () use ($writer, $reader, $cache): void { - $this->renameDocument($writer, 'users', 'user', 'updated'); - $reader->purgeCachedCollection('users'); - $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); - $cache->abandonNextWrite(); - }); - $this->fail('The abandoned write released its registration'); - } catch (\RuntimeException $error) { - $this->assertStringContainsString('release document cache owner', $error->getMessage()); - } + $writer->withTransaction(function () use ($writer, $reader, $cache): void { + $this->renameDocument($writer, 'users', 'user', 'updated'); + $reader->purgeCachedCollection('users'); + $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); + $this->assertSame('original', $reader->getDocument('users', 'user')->getAttribute('name')); + $cache->abandonNextWrite(); + }); $this->assertSame('updated', $reader->getDocument('users', 'user')->getAttribute('name'), 'A write that finishes while another is in flight must not re-enable the collection'); } finally { @@ -896,6 +851,8 @@ final class AbandoningCache implements CacheAdapter, Leasable private bool $abandoning = false; + private string $abandoned = ''; + public function __construct() { $this->cache = new RedisLeasableCache(); @@ -934,10 +891,14 @@ public function list(string $key): array #[\Override] public function purge(string $key, string $hash = ''): bool { - if ($this->abandoning && $hash !== '' && \str_ends_with($key, '#owners')) { - $this->abandoning = false; - - return false; + if ($hash !== '' && \str_ends_with($key, '#owners')) { + if ($this->abandoning) { + $this->abandoning = false; + $this->abandoned = $hash; + } + if ($hash === $this->abandoned) { + return false; + } } return $this->cache->purge($key, $hash); diff --git a/tests/unit/Documents/DocumentCacheInvalidationTest.php b/tests/unit/Documents/DocumentCacheInvalidationTest.php index 771f81fd9..edc4aa627 100644 --- a/tests/unit/Documents/DocumentCacheInvalidationTest.php +++ b/tests/unit/Documents/DocumentCacheInvalidationTest.php @@ -243,7 +243,7 @@ public function testAFailedPostCommitPurgeNeverServesAPreCommitFill(): void $failure = $error->getMessage(); } - $this->assertSame(self::PURGE_FAILURE, $failure, 'A failed purge after the commit must reach the caller'); + $this->assertNull($failure, 'A stored write must not be reported as failed because its purge after the commit failed'); $this->assertSame('hook', $duringCommit, 'A reader outside the transaction reads the committed row and caches it'); $this->assertSame('renamed', $reader->getDocument('webhooks', 'hook')->getAttribute('name'), 'A fill made before the commit must never be served after it, even when the purge after the commit fails'); diff --git a/tests/unit/Event/DocumentPurgeTest.php b/tests/unit/Event/DocumentPurgeTest.php index 3e633cb51..031dcd491 100644 --- a/tests/unit/Event/DocumentPurgeTest.php +++ b/tests/unit/Event/DocumentPurgeTest.php @@ -609,7 +609,7 @@ public function commitTransaction(): bool $cache->failing = true; }; - $this->assertSame($failure, $this->failureOf(static fn (): mixed => $write($database))); + $this->assertNull($this->failureOf(static fn (): mixed => $write($database)), 'A stored write must not be reported as failed because the invalidation after its commit failed'); $this->assertSame(['posts/first'], $this->purged($recorder)); }