diff --git a/app/Console/Commands/Admin/RebuildHomeFeeds.php b/app/Console/Commands/Admin/RebuildHomeFeeds.php index ca47fb70d..6be3d6638 100644 --- a/app/Console/Commands/Admin/RebuildHomeFeeds.php +++ b/app/Console/Commands/Admin/RebuildHomeFeeds.php @@ -122,7 +122,7 @@ class RebuildHomeFeeds extends Command } if ($sync) { - HomeTimelineService::warmCache($pid, true, 400, true); + HomeTimelineService::warmCache($pid, true, 400); return; } diff --git a/app/Jobs/HomeFeedPipeline/FeedWarmCachePipeline.php b/app/Jobs/HomeFeedPipeline/FeedWarmCachePipeline.php index 6be72de55..06c855c91 100644 --- a/app/Jobs/HomeFeedPipeline/FeedWarmCachePipeline.php +++ b/app/Jobs/HomeFeedPipeline/FeedWarmCachePipeline.php @@ -73,6 +73,6 @@ class FeedWarmCachePipeline implements ShouldBeUniqueUntilProcessing, ShouldQueu return; } - HomeTimelineService::warmCache($pid, true, 400, true); + HomeTimelineService::warmCache($pid, true, 400); } } diff --git a/app/Services/HomeTimelineService.php b/app/Services/HomeTimelineService.php index 4054615d0..2256c0df3 100644 --- a/app/Services/HomeTimelineService.php +++ b/app/Services/HomeTimelineService.php @@ -87,7 +87,14 @@ class HomeTimelineService } } - public static function warmCache($id, $force = false, $limit = 100, $returnIds = false) + /** + * Rebuild the cached feed, returning the number of members published. + * + * Returns 0 both when nothing was published and when the guard declined + * to run; the read path only ever forces, so the two cannot collide + * there, and every other caller treats them the same way. + */ + public static function warmCache($id, bool $force = false, int $limit = 100): int { if (self::count($id) == 0 || $force == true) { $following = Cache::remember('profile:following:'.$id, 1209600, function () use ($id) { @@ -160,9 +167,15 @@ class HomeTimelineService } if ($added === 0) { - Redis::del(self::CACHE_KEY.$id); - - return $returnIds ? $ids : 1; + // The instance-wide feeds clear themselves when a warm finds + // nothing, because their content is the instance's content. A + // per-user feed is different: a warm can come up empty because + // this user's relationships or the statuses behind their feed + // could not be resolved, and throwing away a feed that was + // serving content is how a user ends up with a blank timeline. + Redis::del($tmpKey); + + return 0; } if ($added > self::FEED_LIMIT) { @@ -172,7 +185,7 @@ class HomeTimelineService Redis::rename($tmpKey, self::CACHE_KEY.$id); Redis::expire(self::CACHE_KEY.$id, self::CACHE_TTL); - return $returnIds ? $ids : 1; + return $added; } return 0; diff --git a/tests/Feature/Timeline/HomeChronologicalOrderTest.php b/tests/Feature/Timeline/HomeChronologicalOrderTest.php index 629fde567..c12ef146d 100644 --- a/tests/Feature/Timeline/HomeChronologicalOrderTest.php +++ b/tests/Feature/Timeline/HomeChronologicalOrderTest.php @@ -52,7 +52,7 @@ it('ranks home posts by created_at even when a remote post has a newer id but ol StatusService::get($recent->id, false); StatusService::get($staleRemote->id, false); - HomeTimelineService::warmCache($pid, true, 400, true); + HomeTimelineService::warmCache($pid, true, 400); $ids = collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all(); diff --git a/tests/Feature/Timeline/HomeWarmEmptyResultTest.php b/tests/Feature/Timeline/HomeWarmEmptyResultTest.php new file mode 100644 index 000000000..9c6c79de9 --- /dev/null +++ b/tests/Feature/Timeline/HomeWarmEmptyResultTest.php @@ -0,0 +1,89 @@ +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(30), + ], $overrides)); + StatusService::get($status->id, false); + StatusService::getMastodon($status->id, false); + + return $status; +} + +function warmEmptyViewerFollowing(): array +{ + $viewer = User::factory()->create(); + $viewer->refresh(); + $author = User::factory()->create(); + $author->refresh(); + Follower::create([ + 'profile_id' => $viewer->profile_id, + 'following_id' => $author->profile_id, + ]); + + return [$viewer, $author]; +} + +it('keeps a populated feed when a forced warm publishes nothing', function () { + $viewer = User::factory()->create(); + $viewer->refresh(); + $pid = $viewer->profile_id; + // A real status by an author the viewer does not follow: the warm's + // candidate query cannot find it, so the warm publishes nothing. + $stranger = User::factory()->create(); + $stranger->refresh(); + $status = warmEmptyPhoto($stranger->profile_id); + HomeTimelineService::add($pid, $status->id); + expect(HomeTimelineService::count($pid))->toBe(1); + HomeTimelineService::warmCache($pid, true, 400); + expect(HomeTimelineService::count($pid))->toBe(1) + ->and(collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all()) + ->toContain((int) $status->id); +}); + +it('reports how many members the warm published', function () { + [$viewer, $author] = warmEmptyViewerFollowing(); + $pid = $viewer->profile_id; + collect(range(1, 3))->each(fn () => warmEmptyPhoto($author->profile_id)); + expect(HomeTimelineService::warmCache($pid, true, 100))->toBe(3); + $barren = User::factory()->create(); + $barren->refresh(); + expect(HomeTimelineService::warmCache($barren->profile_id, true, 100))->toBe(0); +});