Merge pull request #7866 from pixelfed/fix/timeline-forced-warm-empty

Keep a populated home feed when a forced warm publishes nothing

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;
         }

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);
     }
 }

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;

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();
 

tests/Feature/Timeline/HomeWarmEmptyResultTest.php

@@ -0,0 +1,89 @@
+<?php
+
+use App\Models\Follower;
+use App\Models\Status;
+use App\Models\User;
+use App\Services\HomeTimelineService;
+use App\Services\StatusService;
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Support\Facades\Redis;
+
+uses(LazilyRefreshDatabase::class);
+
+/*
+|--------------------------------------------------------------------------
+| What the home warm does when it finds nothing
+|--------------------------------------------------------------------------
+|
+| A forced warm that publishes no members must not take the existing feed
+| with it: a per-user feed can come up empty because the viewer's cached
+| relationships or the statuses behind their feed could not be resolved,
+| and throwing the live feed away is how a user who was reading a moment
+| ago ends up with nothing. warmCache also returns the number of members
+| it published, which is what lets a caller tell "rebuilt" from "empty".
+|
+*/
+
+afterEach(function () {
+    Redis::flushdb();
+});
+
+function warmEmptyPhoto(int $profileId, 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(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);
+});