From 24e278759a8f37d37985b012dbe3f1ffb54fa2a4 Mon Sep 17 00:00:00 2001 From: Ashley Date: Wed, 21 Oct 2026 21:35:56 +1030 Subject: [PATCH] Prune home feed members that no longer render A member whose status has been deleted, or which cannot be resolved for any other reason, was dropped from the page and left in the sorted set. The feed then went on reporting hundreds of members while serving an empty page, and nothing in the system ever removed them. The hydration pass now removes such a member as it finds it and logs once per request when it pruned anything. Removing the last member makes Redis drop the set, so the feed becomes genuinely absent and is rebuilt in the same request. That cannot loop: the key is gone, so the branch is unreachable a second time. Drops are also counted by reason now, because the reason decides what the response is allowed to do. A page the viewer explicitly filtered out is not a defect to recover from, so it still prunes nothing and runs no query. --- app/Http/Controllers/Api/ApiV1Controller.php | 216 ++++++++++++------ .../Timeline/HomeUnrenderableWindowTest.php | 135 +++++++++++ 2 files changed, 278 insertions(+), 73 deletions(-) create mode 100644 tests/Feature/Timeline/HomeUnrenderableWindowTest.php diff --git a/app/Http/Controllers/Api/ApiV1Controller.php b/app/Http/Controllers/Api/ApiV1Controller.php index 52e7f9ac9..cec3da2f0 100644 --- a/app/Http/Controllers/Api/ApiV1Controller.php +++ b/app/Http/Controllers/Api/ApiV1Controller.php @@ -2913,103 +2913,173 @@ class ApiV1Controller extends Controller $ids = $this->homeColdBoot($pid, $cursor, $paddedLimit); } - $applyPhotosReblogsOnly = $photosReblogsOnly; - - $res = collect($ids) - ->map(function ($id) use ($napi, $pid) { - try { - $status = StatusService::get($id, false); + // Why a member was dropped decides what the response can do about it. + $drops = ['unrenderable' => 0, 'reblog' => 0, 'viewer' => 0, 'hide' => 0]; + $pruned = 0; + + $hydrate = function (array $ids) use ( + $napi, + $pid, + $mutedAndBlocked, + $domainBlocks, + $homeFilters, + $includeReblogs, + $photosReblogsOnly, + $postTypes, + $limit, + $liftReblogAuthor, + &$drops, + &$pruned + ) { + return collect($ids) + ->map(function ($id) use ($napi, $pid, &$drops, &$pruned) { + try { + $status = StatusService::get($id, false); + } catch (\Exception) { + $status = null; + } - if (! isset($status['account']['id']) || (($status['pf_type'] ?? null) === 'share' && empty($status['reblog']))) { + // A share whose boosted status has gone renders as nothing + // at all, same as a member with no account behind it. + $brokenShare = ($status['pf_type'] ?? null) === 'share' && empty($status['reblog']); + + if (! isset($status['account']['id']) || $brokenShare) { + // The member points at something that cannot be + // rendered any more. Leaving it in place is how a feed + // goes on reporting plenty of members while serving an + // empty page on every request. + $drops['unrenderable']++; + $pruned++; HomeTimelineService::rem($pid, $id); return null; } if (! $napi) { - $status = StatusService::getMastodon($id, false); - } - } catch (\Exception) { - return null; - } + try { + $status = StatusService::getMastodon($id, false); + } catch (\Exception) { + $status = null; + } - return isset($status['account']['id']) ? $status : null; - }) - ->filter() - ->filter(function ($status) use ($mutedAndBlocked, $domainBlocks) { - $accountIds = [(int) $status['account']['id']]; - if (! empty($status['reblog']['account']['id'])) { - $accountIds[] = (int) $status['reblog']['account']['id']; - } + if (! isset($status['account']['id'])) { + $drops['unrenderable']++; + $pruned++; + HomeTimelineService::rem($pid, $id); - foreach ($accountIds as $accountId) { - if (in_array($accountId, $mutedAndBlocked)) { - return false; + return null; + } } - } - if ($domainBlocks && count($domainBlocks)) { - foreach ([$status['url'] ?? null, $status['reblog']['url'] ?? null] as $url) { - if (! $url) { - continue; - } - $domain = strtolower((string) parse_url($url, PHP_URL_HOST)); - if (in_array($domain, $domainBlocks)) { + return $status; + }) + ->filter() + ->filter(function ($status) use ($mutedAndBlocked, $domainBlocks, &$drops) { + $accountIds = [(int) $status['account']['id']]; + if (! empty($status['reblog']['account']['id'])) { + $accountIds[] = (int) $status['reblog']['account']['id']; + } + + foreach ($accountIds as $accountId) { + if (in_array($accountId, $mutedAndBlocked)) { + $drops['viewer']++; + return false; } } - } - return true; - }) - ->filter(function ($status) use ($includeReblogs, $applyPhotosReblogsOnly, $postTypes) { - if (empty($status['reblog'])) { + if ($domainBlocks && count($domainBlocks)) { + foreach ([$status['url'] ?? null, $status['reblog']['url'] ?? null] as $url) { + if (! $url) { + continue; + } + $domain = strtolower((string) parse_url($url, PHP_URL_HOST)); + if (in_array($domain, $domainBlocks)) { + $drops['viewer']++; + + return false; + } + } + } + return true; - } + }) + ->filter(function ($status) use ($includeReblogs, $photosReblogsOnly, $postTypes, &$drops) { + if (empty($status['reblog'])) { + return true; + } + + if (! $includeReblogs) { + $drops['reblog']++; + + return false; + } + + // a boost must share a photo or video + if (! $photosReblogsOnly || in_array(data_get($status['reblog'], 'pf_type'), $postTypes)) { + return true; + } + + $drops['reblog']++; - if (! $includeReblogs) { return false; - } + }) + ->map(function ($status) use ($homeFilters, &$drops) { + $filterResults = CustomFilter::applyCachedFilters($homeFilters, $status); - // a boost must share a photo or video - return ! $applyPhotosReblogsOnly - || in_array(data_get($status['reblog'], 'pf_type'), $postTypes); - }) - ->map(function ($status) use ($homeFilters) { - $filterResults = CustomFilter::applyCachedFilters($homeFilters, $status); + if (! empty($filterResults)) { + $status['filtered'] = $filterResults; + $shouldHide = collect($filterResults)->contains(function ($result) { + return $result['filter']['filter_action'] === 'hide'; + }); - if (! empty($filterResults)) { - $status['filtered'] = $filterResults; - $shouldHide = collect($filterResults)->contains(function ($result) { - return $result['filter']['filter_action'] === 'hide'; - }); + if ($shouldHide) { + $drops['hide']++; - if ($shouldHide) { - return null; + return null; + } } - } - return $status; - }) - ->filter() - ->take($limit) - ->map(function ($status) use ($pid) { - $status['favourited'] = (bool) LikeService::liked($pid, $status['id']); - $status['reblogged'] = (bool) ReblogService::get($pid, $status['id']); - $status['bookmarked'] = (bool) BookmarkService::get($pid, $status['id']); - - if (! empty($status['reblog'])) { - $status['reblog']['favourited'] = (bool) LikeService::liked($pid, $status['reblog']['id']); - $status['reblog']['reblogged'] = (bool) ReblogService::get($pid, $status['reblog']['id']); - $status['reblog']['bookmarked'] = (bool) BookmarkService::get($pid, $status['reblog']['id']); - } + return $status; + }) + ->filter() + ->take($limit) + ->map(function ($status) use ($pid) { + $status['favourited'] = (bool) LikeService::liked($pid, $status['id']); + $status['reblogged'] = (bool) ReblogService::get($pid, $status['id']); + $status['bookmarked'] = (bool) BookmarkService::get($pid, $status['id']); + + if (! empty($status['reblog'])) { + $status['reblog']['favourited'] = (bool) LikeService::liked($pid, $status['reblog']['id']); + $status['reblog']['reblogged'] = (bool) ReblogService::get($pid, $status['reblog']['id']); + $status['reblog']['bookmarked'] = (bool) BookmarkService::get($pid, $status['reblog']['id']); + } - return $status; - }) - ->map(function ($status) use ($liftReblogAuthor) { - return $liftReblogAuthor ? $this->liftReblogAuthor($status) : $status; - }) - ->values(); + return $status; + }) + ->map(function ($status) use ($liftReblogAuthor) { + return $liftReblogAuthor ? $this->liftReblogAuthor($status) : $status; + }) + ->values(); + }; + + $res = $hydrate($ids); + + if ($pruned) { + Log::info('timelineHome: pruned unrenderable home feed members', [ + 'pid' => $pid, + 'count' => $pruned, + ]); + } + + // The prune can take the last member with it, and Redis drops an empty + // sorted set, so the feed is now genuinely absent. Rebuilding here + // cannot loop: the key is gone, so the branch is unreachable a second + // time within the request. + if ($res->isEmpty() && $drops['unrenderable'] > 0 && ! HomeTimelineService::cacheKeyExists($pid)) { + $ids = $this->homeColdBoot($pid, $cursor, $paddedLimit); + $res = $hydrate($ids); + } $headers = []; $baseUrl = config('app.url').'/api/v1/timelines/home?limit='.$limit.'&'.($napi ? '_pe=1&' : ''); diff --git a/tests/Feature/Timeline/HomeUnrenderableWindowTest.php b/tests/Feature/Timeline/HomeUnrenderableWindowTest.php new file mode 100644 index 000000000..a56db5277 --- /dev/null +++ b/tests/Feature/Timeline/HomeUnrenderableWindowTest.php @@ -0,0 +1,135 @@ +create(); + $viewer->refresh(); + $author = User::factory()->create(); + $author->refresh(); + Follower::withoutEvents(fn () => Follower::create([ + 'profile_id' => $viewer->profile_id, + 'following_id' => $author->profile_id, + ])); + + return [$viewer, $author]; +} + +function unrenderablePhoto(int $profileId, int $minutesAgo, array $overrides = []): Status +{ + $status = Status::factory()->create(array_merge([ + 'profile_id' => $profileId, + 'type' => 'photo', + 'scope' => 'public', + 'visibility' => 'public', + 'local' => true, + 'in_reply_to_id' => null, + 'reblog_of_id' => null, + 'uri' => null, + 'created_at' => now()->subMinutes($minutesAgo), + ], $overrides)); + StatusService::get($status->id, false); + StatusService::getMastodon($status->id, false); + + return $status; +} + +/** Remove a status so its feed member can never hydrate again. */ +function unrenderableDestroy(Status $status): void +{ + DB::table('statuses')->where('id', $status->id)->delete(); + Cache::forget(StatusService::key($status->id, true)); + Cache::forget(StatusService::key($status->id, false)); +} + +function unrenderableCandidateQueries(callable $fn): int +{ + $count = 0; + DB::listen(function ($query) use (&$count) { + if (preg_match('/max\(id\)/i', $query->sql)) { + $count++; + } + }); + $fn(); + + return $count; +} + +it('prunes members whose statuses have gone away and serves what remains', function () { + [$viewer, $author] = unrenderableViewer(); + $pid = $viewer->profile_id; + $stale = collect(range(1, 5))->map(fn ($i) => unrenderablePhoto($author->profile_id, 100 + $i)); + foreach ($stale as $status) { + HomeTimelineService::add($pid, $status->id); + } + foreach ($stale as $status) { + unrenderableDestroy($status); + } + // Eligible, recent, and not in the feed: the repair the request can make. + $fresh = unrenderablePhoto($author->profile_id, 5); + Passport::actingAs($viewer, ['read']); + $ids = collect($this->getJson('/api/v1/timelines/home?limit=10')->assertOk()->json()) + ->pluck('id')->map(fn ($id) => (int) $id)->all(); + expect($ids)->toContain((int) $fresh->id); + foreach ($stale as $status) { + expect(Redis::zscore(HomeTimelineService::CACHE_KEY.$pid, $status->id))->toBeNull(); + } +}); + +it('leaves a page the viewer filtered out alone', function () { + [$viewer, $author] = unrenderableViewer(); + $pid = $viewer->profile_id; + $statuses = collect(range(1, 3))->map(fn ($i) => unrenderablePhoto($author->profile_id, 10 + $i, [ + 'caption' => 'hideme sunset photo', + 'rendered' => 'hideme sunset photo', + ])); + foreach ($statuses as $status) { + HomeTimelineService::add($pid, $status->id); + } + $filter = CustomFilter::create([ + 'profile_id' => $pid, + 'title' => 'hide sunsets', + 'context' => ['home'], + 'action' => 1, + ]); + $filter->keywords()->create(['keyword' => 'hideme', 'whole_word' => false]); + Cache::forget('filters:v3:'.$pid); + Passport::actingAs($viewer, ['read']); + $candidateQueries = unrenderableCandidateQueries(function () { + $res = $this->getJson('/api/v1/timelines/home?limit=5')->assertOk(); + expect($res->json())->toBe([]); + }); + expect($candidateQueries)->toBe(0) + ->and(HomeTimelineService::count($pid))->toBe(3); +});