Merge pull request #7867 from pixelfed/fix/timeline-widen-warm

Widen the home warm before it concludes there is nothing to show

app/Services/HomeTimelineService.php

@@ -96,98 +96,160 @@ class HomeTimelineService
      */
     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) {
-                $following = Follower::whereProfileId($id)->pluck('following_id');
+        if (self::count($id) != 0 && $force != true) {
+            return 0;
+        }
 
-                return $following->push($id)->toArray();
-            });
+        $limit = $limit > 0 ? $limit : 100;
 
-            $minId = SnowflakeService::byDate(now()->subMonths(6));
+        $following = self::followingForWarm($id);
+        $domainBlocks = UserDomainBlock::whereProfileId($id)->pluck('domain')->toArray();
 
-            $filters = UserFilterService::filters($id);
+        $added = self::publishWarm($id, self::warmCandidates($following, $limit, true, false), $domainBlocks);
 
-            if ($filters && count($filters)) {
-                $following = array_diff($following, $filters);
-            }
+        if ($added > 0) {
+            return $added;
+        }
+
+        // Nothing publishable inside the fast path. Before concluding there is
+        // nothing to show at all, drop the two assumptions that path makes for
+        // speed — that anything worth showing was posted in the last six
+        // months, and that a boost is not content — and re-read a following
+        // list that resolved to the viewer alone, which is what a list cached
+        // before a follow landed looks like.
+        if (self::followingIsSelfOnly($following, $id)) {
+            Cache::forget('profile:following:'.$id);
+            $following = self::followingForWarm($id);
+        }
+
+        return self::publishWarm($id, self::warmCandidates($following, $limit, false, true), $domainBlocks);
+    }
+
+    protected static function followingForWarm($id): array
+    {
+        $following = Cache::remember('profile:following:'.$id, 1209600, function () use ($id) {
+            $following = Follower::whereProfileId($id)->pluck('following_id');
+
+            return $following->push($id)->toArray();
+        });
 
-            $domainBlocks = UserDomainBlock::whereProfileId($id)->pluck('domain')->toArray();
+        $filters = UserFilterService::filters($id);
+
+        if ($filters && count($filters)) {
+            $following = array_diff($following, $filters);
+        }
+
+        return array_values($following);
+    }
+
+    protected static function followingIsSelfOnly(array $following, $id): bool
+    {
+        return count($following) === 1 && (int) reset($following) === (int) $id;
+    }
+
+    /**
+     * The candidate ids for one warm pass.
+     *
+     * The per-account latest is merged with the newest overall so a prolific
+     * account cannot crowd everyone else out of a capped feed.
+     */
+    protected static function warmCandidates(array $following, int $limit, bool $windowed, bool $includeShares)
+    {
+        $types = ['photo', 'photo:album', 'video', 'video:album', 'photo:video:album'];
+        $nullable = ['in_reply_to_id'];
 
-            $baseQuery = function () use ($following, $minId) {
-                return Status::where('id', '>', $minId)
-                    ->whereIn('profile_id', $following)
-                    ->whereNull(['in_reply_to_id', 'reblog_of_id'])
-                    ->whereIn('type', ['photo', 'photo:album', 'video', 'video:album', 'photo:video:album'])
-                    ->whereIn('visibility', ['public', 'unlisted', 'private']);
-            };
+        if ($includeShares) {
+            $types[] = 'share';
+        } else {
+            $nullable[] = 'reblog_of_id';
+        }
 
-            $latestPerAccount = $baseQuery()
-                ->select(DB::raw('max(id) as id'))
-                ->groupBy('profile_id')
+        $minId = SnowflakeService::byDate(now()->subMonths(6));
+
+        $baseQuery = function () use ($following, $windowed, $minId, $types, $nullable) {
+            return Status::when($windowed, function ($q) use ($minId) {
+                return $q->where('id', '>', $minId);
+            })
+                ->whereIn('profile_id', $following)
+                ->whereNull($nullable)
+                ->whereIn('type', $types)
+                ->whereIn('visibility', ['public', 'unlisted', 'private']);
+        };
+
+        $latestPerAccount = $baseQuery()
+            ->select(DB::raw('max(id) as id'))
+            ->groupBy('profile_id')
+            ->pluck('id')
+            ->sortDesc()
+            ->take($limit)
+            ->values();
+
+        $remaining = $limit - $latestPerAccount->count();
+
+        $recentIds = $remaining > 0
+            ? $baseQuery()
+                ->whereNotIn('id', $latestPerAccount->all())
+                ->orderByDesc('id')
+                ->limit($remaining)
                 ->pluck('id')
-                ->sortDesc()
-                ->take($limit)
-                ->values();
-
-            $remaining = $limit - $latestPerAccount->count();
-
-            $recentIds = $remaining > 0
-                ? $baseQuery()
-                    ->whereNotIn('id', $latestPerAccount->all())
-                    ->orderByDesc('id')
-                    ->limit($remaining)
-                    ->pluck('id')
-                : collect();
-
-            $ids = $latestPerAccount
-                ->merge($recentIds)
-                ->unique()
-                ->values();
-
-            $tmpKey = self::CACHE_KEY.$id.':rebuild:'.Str::random(8);
-            $added = 0;
-            foreach ($ids as $sid) {
-                try {
-                    $status = StatusService::get($sid, false);
-                } catch (\Exception) {
-                    continue;
-                }
-                if (! $status || ! isset($status['account'], $status['url'])) {
-                    continue;
-                }
-                if ($domainBlocks && count($domainBlocks)) {
-                    $domain = strtolower(parse_url($status['url'], PHP_URL_HOST));
-                    if (in_array($domain, $domainBlocks)) {
-                        continue;
-                    }
-                }
-                $score = ! empty($status['created_at']) ? strtotime($status['created_at']) : false;
-                Redis::zadd($tmpKey, $score !== false ? $score : self::snowflakeScore($sid), $sid);
-                $added++;
-            }
+            : collect();
 
-            if ($added === 0) {
-                // 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 $latestPerAccount
+            ->merge($recentIds)
+            ->unique()
+            ->values();
+    }
 
-                return 0;
+    /**
+     * Hydrate the candidates into a scratch set and publish it atomically.
+     *
+     * Returns the number of members published, 0 if none were.
+     */
+    protected static function publishWarm($id, $candidates, array $domainBlocks): int
+    {
+        $tmpKey = self::CACHE_KEY.$id.':rebuild:'.Str::random(8);
+        $added = 0;
+
+        foreach ($candidates as $sid) {
+            try {
+                $status = StatusService::get($sid, false);
+            } catch (\Exception) {
+                continue;
             }
 
-            if ($added > self::FEED_LIMIT) {
-                Redis::zremrangebyrank($tmpKey, 0, -(self::FEED_LIMIT + 1));
+            if (! $status || ! isset($status['account'], $status['url'])) {
+                continue;
+            }
+            if ($domainBlocks && count($domainBlocks)) {
+                $domain = strtolower(parse_url($status['url'], PHP_URL_HOST));
+                if (in_array($domain, $domainBlocks)) {
+                    continue;
+                }
             }
+            $score = ! empty($status['created_at']) ? strtotime($status['created_at']) : false;
+            Redis::zadd($tmpKey, $score !== false ? $score : self::snowflakeScore($sid), $sid);
+            $added++;
+        }
 
-            Redis::rename($tmpKey, self::CACHE_KEY.$id);
-            Redis::expire(self::CACHE_KEY.$id, self::CACHE_TTL);
+        if ($added === 0) {
+            // 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 $added;
+            return 0;
         }
 
-        return 0;
+        if ($added > self::FEED_LIMIT) {
+            Redis::zremrangebyrank($tmpKey, 0, -(self::FEED_LIMIT + 1));
+        }
+
+        Redis::rename($tmpKey, self::CACHE_KEY.$id);
+        Redis::expire(self::CACHE_KEY.$id, self::CACHE_TTL);
+
+        return $added;
     }
 }

tests/Feature/Timeline/HomeWarmEmptyResultTest.php

@@ -4,9 +4,12 @@ use App\Models\Follower;
 use App\Models\Status;
 use App\Models\User;
 use App\Services\HomeTimelineService;
+use App\Services\SnowflakeService;
 use App\Services\StatusService;
 use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Redis;
+use Laravel\Passport\Passport;
 
 uses(LazilyRefreshDatabase::class);
 
@@ -22,6 +25,14 @@ uses(LazilyRefreshDatabase::class);
 | ago ends up with nothing. warmCache also returns the number of members
 | it published, which is what lets a caller tell "rebuilt" from "empty".
 |
+| Before it concludes there is nothing to show at all, the warm widens.
+| The fast pass scopes candidates to the last six months and excludes
+| boosts. When that pass publishes nothing, a second pass drops the id
+| floor, admits shares, and re-reads a following list that resolved to the
+| viewer alone. Those are the routes by which a viewer whose follows are
+| dormant, who follows only boosters, or whose cached following list is
+| stale ends up with a feed that reports empty and stays that way.
+|
 */
 
 afterEach(function () {
@@ -47,6 +58,26 @@ function warmEmptyPhoto(int $profileId, array $overrides = []): Status
     return $status;
 }
 
+function warmEmptyShare(int $sharerPid, int $originalPid): Status
+{
+    $original = warmEmptyPhoto($originalPid);
+    $share = Status::factory()->create([
+        'profile_id' => $sharerPid,
+        'type' => 'share',
+        'scope' => 'public',
+        'visibility' => 'public',
+        'local' => true,
+        'in_reply_to_id' => null,
+        'reblog_of_id' => $original->id,
+        'uri' => null,
+        'created_at' => now()->subMinutes(20),
+    ]);
+    StatusService::get($share->id, false);
+    StatusService::getMastodon($share->id, false);
+
+    return $share;
+}
+
 function warmEmptyViewerFollowing(): array
 {
     $viewer = User::factory()->create();
@@ -87,3 +118,55 @@ it('reports how many members the warm published', function () {
     $barren->refresh();
     expect(HomeTimelineService::warmCache($barren->profile_id, true, 100))->toBe(0);
 });
+
+it('warms a dormant follow whose only post predates the six month window', function () {
+    [$viewer, $author] = warmEmptyViewerFollowing();
+    $pid = $viewer->profile_id;
+    // The warm's candidate query floors on a six-month-old snowflake, so an
+    // id below that floor is invisible to it even though it is eligible.
+    $old = warmEmptyPhoto($author->profile_id, [
+        'id' => SnowflakeService::byDate(now()->subMonths(8)),
+        'created_at' => now()->subMonths(8),
+    ]);
+    expect((int) $old->id)->toBeLessThan((int) SnowflakeService::byDate(now()->subMonths(6)));
+    HomeTimelineService::warmCache($pid, true, 400);
+    expect(collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all())
+        ->toContain((int) $old->id);
+    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) $old->id);
+});
+
+it('warms a follow set that has only ever boosted', function () {
+    [$viewer, $author] = warmEmptyViewerFollowing();
+    $pid = $viewer->profile_id;
+    $stranger = User::factory()->create();
+    $stranger->refresh();
+    $shares = collect(range(1, 2))->map(
+        fn () => warmEmptyShare($author->profile_id, $stranger->profile_id)
+    );
+    HomeTimelineService::warmCache($pid, true, 400);
+    expect(HomeTimelineService::cacheKeyExists($pid))->toBeTrue();
+    $cached = collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all();
+    foreach ($shares as $share) {
+        expect($cached)->toContain((int) $share->id);
+    }
+    Passport::actingAs($viewer, ['read']);
+    $res = $this->getJson('/api/v1/timelines/home?_pe=1&min_id=1&include_reblogs=1&limit=10')
+        ->assertOk()
+        ->json();
+    expect($res)->not->toBeEmpty();
+});
+
+it('rereads the follow relationship when the cached following list is only the viewer', function () {
+    [$viewer, $author] = warmEmptyViewerFollowing();
+    $pid = $viewer->profile_id;
+    $status = warmEmptyPhoto($author->profile_id);
+    // A following list cached before the follow landed: without a live
+    // re-read the warm concludes the viewer follows nobody for 14 days.
+    Cache::put('profile:following:'.$pid, [$pid], 1209600);
+    HomeTimelineService::warmCache($pid, true, 400);
+    expect(collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all())
+        ->toContain((int) $status->id);
+});