Clear place status caches after save to close invalidation race

handleImmediateAttributes cleared the old and new per-place status-id
caches while place_id was only mutated in memory, before $status->save()
committed the change. A concurrent PlaceController::show read landing in
that window re-warmed the old place cache from the pre-move DB state,
pinning a stale membership list for the 4-day TTL.

Move both PlaceService::clearStatusesByPlaceId calls to after save() so
the cache can only be re-warmed from post-move DB state. Add a regression
test that re-warms the old place cache during the updating event (fired
inside save, before the DB write) and asserts it does not survive.
pull/7553/head
Your Name 1 month ago
parent 641d7005bf
commit c32642e686

@ -91,6 +91,7 @@ class UpdateStatusService
if (isset($attributes['spoiler_text'])) {
$status->cw_summary = Purify::clean($attributes['spoiler_text']);
}
$oldPlaceId = null;
if (isset($attributes['location'])) {
$oldPlaceId = $status->getOriginal('place_id');
if (isset($attributes['location']['id'])) {
@ -98,20 +99,20 @@ class UpdateStatusService
} else {
$status->place_id = null;
}
if ($oldPlaceId != $status->place_id) {
if ($oldPlaceId) {
PlaceService::clearStatusesByPlaceId($oldPlaceId);
}
if ($status->place_id) {
PlaceService::clearStatusesByPlaceId($status->place_id);
}
}
}
if ($status->cw_summary && ! $status->is_nsfw) {
$status->cw_summary = null;
}
$status->edited_at = now();
$status->save();
if (isset($attributes['location']) && $oldPlaceId != $status->place_id) {
if ($oldPlaceId) {
PlaceService::clearStatusesByPlaceId($oldPlaceId);
}
if ($status->place_id) {
PlaceService::clearStatusesByPlaceId($status->place_id);
}
}
StatusService::del($status->id);
}

@ -49,6 +49,37 @@ it('clears old and new place caches when the location changes', function () {
expect(Cache::has(PlaceService::STATUSES_CACHE_KEY.'200'))->toBeFalse('new place cache must be cleared');
});
it('does not leave a concurrently re-warmed old place cache behind (REGRESSION)', function () {
$user = User::factory()->create();
$user->refresh();
$status = Status::factory()->create([
'profile_id' => $user->profile_id,
'type' => 'photo',
'scope' => 'public',
'place_id' => 100,
]);
// Simulate a concurrent PlaceController::show read that lands mid-save: the
// `updating` event fires inside save() *before* the DB UPDATE commits, so the
// DB still holds the old place_id here. Re-warm the old place cache from that
// pre-move state, exactly as a racing reader's Cache::remember would.
Status::updating(function ($model) {
expect((int) $model->getOriginal('place_id'))->toBe(100);
warmPlaceCache(100);
});
UpdateStatusService::handleImmediateAttributes($status, [
'location' => ['id' => 200],
]);
Status::flushEventListeners();
// The invalidation must win: after the move, the old place cache must not
// survive, otherwise place 100 lists a status that left it for up to 4 days.
expect(Cache::has(PlaceService::STATUSES_CACHE_KEY.'100'))->toBeFalse('stale re-warm of old place cache must be cleared after save');
});
it('clears the old place cache when the location is removed', function () {
$user = User::factory()->create();
$user->refresh();

Loading…
Cancel
Save