From 886ed49d124b16f74adfb5ba60156f040df573c1 Mon Sep 17 00:00:00 2001 From: Alex Standiford Date: Sat, 11 Jul 2026 18:39:52 -0400 Subject: [PATCH 1/2] fix(cache): invalidate identity-keyed entries on compound-key updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit where()/getModels() hydration caches each row under its table identity (primary columns). updateCompound() only invalidated the caller-provided compound context, so an update keyed on a compound business key (e.g. a config row's type/subtype/configKey) left the identity-keyed cache entry serving the pre-update row until TTL — observed in production-shaped E2E as reads returning stale entities after writes. updateCompound() now resolves the row's table identity before the update and invalidates that context too. The resolution is skipped for the common update-by-identity path (the caller's key already IS the identity) and for tables without identity fields. Identity values are projected from the query result so the cache context contains identity fields only, regardless of what the query strategy returns. Co-Authored-By: Claude Fable 5 --- lib/Traits/WithDatastoreHandlerMethods.php | 55 +++++++++++++++++ .../WithDatastoreHandlerMethodsTest.php | 59 +++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/lib/Traits/WithDatastoreHandlerMethods.php b/lib/Traits/WithDatastoreHandlerMethods.php index dbd3ac6..cdc93b5 100644 --- a/lib/Traits/WithDatastoreHandlerMethods.php +++ b/lib/Traits/WithDatastoreHandlerMethods.php @@ -456,11 +456,66 @@ public function updateCompound($ids, array $attributes): void $record = $this->findFromCompound($ids); $this->maybeThrowForDuplicateUniqueFields($attributes, $ids); + // Resolve the row's table identity (primary columns) BEFORE the + // update, while the row still matches $ids. where()/getModels() + // hydration caches each row under its table identity, so an update + // keyed on a compound business key (e.g. a config's + // type/subtype/configKey) must invalidate that entry too — deleting + // only the caller-provided context leaves the identity-keyed entry + // serving stale reads until TTL. + $identities = $this->resolveTableIdentitiesForUpdate($ids); + $this->serviceProvider->queryStrategy->update($this->table, $ids, $attributes); $this->serviceProvider->cacheableService->delete($this->getCacheContextForItem($ids)); + + foreach ($identities as $identity) { + $this->serviceProvider->cacheableService->delete($this->getCacheContextForItem($identity)); + } + $this->serviceProvider->eventStrategy->broadcast(new RecordUpdated($record::class, $ids, $attributes)); } + /** + * Resolves the table-identity (primary column) values for rows matching + * the given compound key — the identity arrays the where()/getModels() + * read path keys its cache entries on. + * + * @param array $ids + * @return array> + * @throws DatastoreErrorException + */ + protected function resolveTableIdentitiesForUpdate(array $ids): array + { + $identityFields = $this->table->getFieldsForIdentity(); + + // Nothing to resolve when the table defines no identity fields, or + // when the caller's key IS the table identity (the standard + // update-by-id path) — the direct context deletion already covers it. + if (empty($identityFields) || array_keys($ids) === $identityFields) { + return []; + } + + $clauseBuilder = (clone $this->serviceProvider->clauseBuilder)->reset()->useTable($this->table); + + foreach ($ids as $key => $id) { + $clauseBuilder->andWhere($key, '=', $id); + } + + $rows = $this->serviceProvider->queryStrategy->query( + $this->serviceProvider->queryBuilder + ->select(...$identityFields) + ->from($this->table) + ->where($clauseBuilder) + ->limit(1) + ); + + // Project to identity fields regardless of what the query strategy + // returned — the cache context must contain identity values only. + $identityFlip = array_flip($identityFields); + + return array_map(fn(array $row) => array_intersect_key($row, $identityFlip), $rows); + } + /** * @param array $attributes * @param array $fields diff --git a/tests/Unit/Traits/WithDatastoreHandlerMethodsTest.php b/tests/Unit/Traits/WithDatastoreHandlerMethodsTest.php index 8e3ce0c..77c7212 100644 --- a/tests/Unit/Traits/WithDatastoreHandlerMethodsTest.php +++ b/tests/Unit/Traits/WithDatastoreHandlerMethodsTest.php @@ -302,6 +302,65 @@ public function testUpdateCompoundInvalidatesCacheRegardlessOfIdentityType(): vo $this->assertSame($expected, $deletedKeys[0]); } + public function testUpdateByCompoundBusinessKeyInvalidatesIdentityKeyedEntry(): void + { + // where()/getModels() hydration caches rows under the TABLE identity. + // An update keyed on a compound business key (e.g. a config's + // type/subtype/configKey) must invalidate that identity-keyed entry + // too, or reads keep serving the pre-update row until TTL. + $loggerStrategy = $this->createMock(LoggerStrategy::class); + $eventStrategy = $this->createMock(EventStrategy::class); + $queryStrategy = $this->createMock(QueryStrategy::class); + + $deletedKeys = []; + $cacheableService = $this->createMock(CacheableService::class); + $cacheableService->method('getWithCache') + ->willReturnCallback(fn(string $operation, array $context, callable $callback) => $callback()); + $cacheableService->expects($this->exactly(2)) + ->method('delete') + ->willReturnCallback(function (array $context) use (&$deletedKeys) { + $deletedKeys[] = $context; + }); + + // Serves both findFromCompound (full row) and the identity resolution + // (identity columns only) — the identity value is what matters. + $queryStrategy->method('query')->willReturn([['id' => 42, 'configKey' => 'rate', 'value' => 'old']]); + + $table = $this->createMock(Table::class); + $table->method('getName')->willReturn('test_records'); + $table->method('getFieldsForIdentity')->willReturn(['id']); + + $tableSchemaService = $this->createMock(TableSchemaService::class); + $tableSchemaService->method('getUniqueColumns')->willReturn([]); + $modelAdapter = $this->createMock(ModelAdapter::class); + $modelAdapter->method('toModel')->willReturn(new TestModel(42)); + + $serviceProvider = new DatabaseServiceProvider( + $loggerStrategy, + $queryStrategy, + new DummyQueryBuilder(), + new DummyClauseBuilder(), + $cacheableService, + $eventStrategy + ); + + $handler = new DummyDatastoreHandler( + $serviceProvider, + $table, + $tableSchemaService, + TestModel::class, + $modelAdapter + ); + + $handler->updateCompound(['configKey' => 'rate'], ['value' => 'new']); + + $this->assertCount(2, $deletedKeys); + // First deletion: the caller-provided compound context (existing behavior). + $this->assertSame($handler->exposeCacheContext(['configKey' => 'rate']), $deletedKeys[0]); + // Second deletion: the row's identity context — the key hydrated reads cache under. + $this->assertSame($handler->exposeCacheContext(['id' => 42]), $deletedKeys[1]); + } + public function testFindFromCompoundIncludesTableAndIdentityWhenRecordIsMissing(): void { $queryStrategy = $this->createMock(QueryStrategy::class); From 7e4f6af4b381dab6f385a91eb15963ab3541ecd6 Mon Sep 17 00:00:00 2001 From: Alex Standiford Date: Sat, 11 Jul 2026 19:40:50 -0400 Subject: [PATCH 2/2] fix: builder call order, order-only reorder shortcut, non-fatal resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - from() before select(): the builder prefixes selected columns with the current table's alias, which is stale until from() runs. - When the caller's key has the same fields as the table identity in a different order, reorder locally instead of querying — the cache context is order-sensitive. - Resolution failures degrade to a possibly-stale entry (logged) instead of breaking the write. Co-Authored-By: Claude Fable 5 --- lib/Traits/WithDatastoreHandlerMethods.php | 50 +++++++++++++++++----- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/lib/Traits/WithDatastoreHandlerMethods.php b/lib/Traits/WithDatastoreHandlerMethods.php index cdc93b5..21d03d5 100644 --- a/lib/Traits/WithDatastoreHandlerMethods.php +++ b/lib/Traits/WithDatastoreHandlerMethods.php @@ -495,19 +495,49 @@ protected function resolveTableIdentitiesForUpdate(array $ids): array return []; } - $clauseBuilder = (clone $this->serviceProvider->clauseBuilder)->reset()->useTable($this->table); + // Same fields, different ORDER: the cache context is order-sensitive, + // so reorder to table order locally — no query needed. + if (count($ids) === count($identityFields) + && array_diff(array_keys($ids), $identityFields) === [] + ) { + $reordered = []; + foreach ($identityFields as $field) { + $reordered[$field] = $ids[$field]; + } - foreach ($ids as $key => $id) { - $clauseBuilder->andWhere($key, '=', $id); + return [$reordered]; } - $rows = $this->serviceProvider->queryStrategy->query( - $this->serviceProvider->queryBuilder - ->select(...$identityFields) - ->from($this->table) - ->where($clauseBuilder) - ->limit(1) - ); + // Genuinely different key: resolve the identity from the database. + // Invalidation is best-effort — a resolution failure must degrade to + // a possibly-stale cache entry, never break the write itself. + try { + $clauseBuilder = (clone $this->serviceProvider->clauseBuilder)->reset()->useTable($this->table); + + foreach ($ids as $key => $id) { + $clauseBuilder->andWhere($key, '=', $id); + } + + // from() BEFORE select(): the builder prefixes selected columns + // with the CURRENT table's alias, which is stale until from() + // runs — the same call order every read path in this trait uses. + $rows = $this->serviceProvider->queryStrategy->query( + $this->serviceProvider->queryBuilder + ->from($this->table) + ->select(...$identityFields) + ->where($clauseBuilder) + ->limit(1) + ); + } catch (RecordNotFoundException $e) { + return []; + } catch (\Throwable $e) { + $this->serviceProvider->loggerStrategy->warning( + 'Cache-invalidation identity resolution failed; a stale cache entry may persist until TTL: ' . $e->getMessage(), + ['table' => $this->table->getName()] + ); + + return []; + } // Project to identity fields regardless of what the query strategy // returned — the cache context must contain identity values only.