Merge pull request #7872 from pixelfed/fix/timeline-empty-page-cursor

Fix/timeline empty page cursor

app/Http/Controllers/Api/ApiV1Controller.php

@@ -3147,10 +3147,25 @@ class ApiV1Controller extends Controller
             $oldest = $res->last()['id'];
 
             $headers['Link'] = '<'.$baseUrl.'max_id='.$oldest.'>; rel="next",<'.$baseUrl.'min_id='.$newest.'>; rel="prev"';
-        } else {
-            $headers['Link'] = '<'.$baseUrl.'max_id='.collect($cursorIds)->map(fn ($id) => (int) $id)->min().'>; rel="next"';
+        } elseif ($cursorIds) {
+            // The page is empty but the window was not, so there is a way
+            // forward. Members arrive newest-first and continuation appends
+            // older windows, so display order is array order; the numeric
+            // minimum would be the wrong member as soon as a remote post
+            // carries a low id and a recent date, and the client would skip
+            // everything between the two.
+            if ($cursor['min']) {
+                // This client asked for content newer than its cursor, so an
+                // older cursor would point it back at posts it already has.
+                $headers['Link'] = '<'.$baseUrl.'min_id='.(int) reset($cursorIds).'>; rel="prev"';
+            } else {
+                $headers['Link'] = '<'.$baseUrl.'max_id='.(int) end($cursorIds).'>; rel="next"';
+            }
         }
 
+        // An empty fetched window emits no Link at all, which is how a client
+        // is told it has reached the end of the feed.
+
         return $this->json($res->toArray(), 200, $headers);
     }
 

tests/Feature/Timeline/HomeEmptyPageLinkCursorTest.php

@@ -0,0 +1,153 @@
+<?php
+
+use App\Models\CustomFilter;
+use App\Models\Follower;
+use App\Models\Profile;
+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);
+
+/*
+|--------------------------------------------------------------------------
+| The next cursor on an empty home page
+|--------------------------------------------------------------------------
+|
+| The feed is ranked by created_at, so a remote post can hold the lowest id
+| in a window and still sit at the top of it. Deriving the continuation
+| cursor from the numeric minimum therefore skips every member between that
+| post and the real bottom of the window, and the client never sees them.
+|
+*/
+
+afterEach(function () {
+    Redis::flushdb();
+});
+
+function emptyLinkHiddenStatus(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,
+        'caption' => 'hideme sunset photo',
+        'rendered' => 'hideme sunset photo',
+        'created_at' => now()->subMinutes($minutesAgo),
+    ], $overrides));
+
+    StatusService::get($status->id, false);
+    StatusService::getMastodon($status->id, false);
+
+    return $status;
+}
+
+function emptyLinkHideFilter(int $pid): void
+{
+    $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);
+}
+
+it('points the next cursor at the display-order oldest member of an empty page', function () {
+    $viewer = User::factory()->create();
+    $viewer->refresh();
+    $pid = $viewer->profile_id;
+
+    $local = User::factory()->create();
+    $local->refresh();
+    $remote = Profile::factory()->create(['user_id' => null, 'domain' => 'remote.example']);
+
+    foreach ([$local->profile_id, $remote->id] as $followingId) {
+        Follower::withoutEvents(fn () => Follower::create([
+            'profile_id' => $pid,
+            'following_id' => $followingId,
+        ]));
+    }
+
+    $locals = collect([30, 20, 10])->map(
+        fn ($minutesAgo) => emptyLinkHiddenStatus($local->profile_id, $minutesAgo)
+    );
+
+    // Fetched first and discarded last: a low id with a recent date.
+    $lowIdRecent = emptyLinkHiddenStatus($remote->id, 1, [
+        'id' => SnowflakeService::byDate(now()->subMonths(8)),
+        'local' => false,
+        'uri' => 'https://remote.example/p/'.uniqid(),
+    ]);
+
+    foreach ($locals->push($lowIdRecent) as $status) {
+        HomeTimelineService::add($pid, $status->id);
+    }
+
+    emptyLinkHideFilter($pid);
+
+    $displayOldest = (int) $locals
+        ->filter(fn ($s) => (int) $s->id !== (int) $lowIdRecent->id)
+        ->sortBy(fn ($s) => $s->created_at->timestamp)
+        ->first()
+        ->id;
+
+    expect((int) $lowIdRecent->id)->toBeLessThan($displayOldest);
+
+    Passport::actingAs($viewer, ['read']);
+
+    $res = $this->getJson('/api/v1/timelines/home?limit=5')->assertOk();
+
+    expect($res->json())->toBe([]);
+
+    $link = $res->headers->get('Link');
+
+    expect($link)->not->toBeNull()
+        ->and($link)->toContain('rel="next"')
+        ->and($link)->toContain('max_id='.$displayOldest);
+});
+
+it('emits no link when the fetched window itself is empty', function () {
+    $viewer = User::factory()->create();
+    $viewer->refresh();
+    $pid = $viewer->profile_id;
+
+    $author = User::factory()->create();
+    $author->refresh();
+    Follower::withoutEvents(fn () => Follower::create([
+        'profile_id' => $pid,
+        'following_id' => $author->profile_id,
+    ]));
+
+    $statuses = collect([30, 20, 10])->map(
+        fn ($minutesAgo) => emptyLinkHiddenStatus($author->profile_id, $minutesAgo, [
+            'caption' => 'ordinary photo',
+            'rendered' => 'ordinary photo',
+        ])
+    );
+
+    foreach ($statuses as $status) {
+        HomeTimelineService::add($pid, $status->id);
+    }
+
+    $oldest = (int) $statuses->sortBy(fn ($s) => $s->created_at->timestamp)->first()->id;
+
+    Passport::actingAs($viewer, ['read']);
+
+    $res = $this->getJson('/api/v1/timelines/home?limit=5&max_id='.$oldest)->assertOk();
+
+    expect($res->json())->toBe([])
+        ->and($res->headers->get('Link'))->toBeNull();
+});

tests/Feature/Timeline/HomeFilteredWindowTest.php

@@ -2,9 +2,11 @@
 
 use App\Models\CustomFilter;
 use App\Models\Follower;
+use App\Models\Profile;
 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;
@@ -26,7 +28,7 @@ it('still returns a next link when the whole page is filtered out', function ()
     $author->refresh();
     Follower::create(['profile_id' => $pid, 'following_id' => $author->profile_id]);
 
-    $statuses = collect(range(1, 3))->map(function () use ($author) {
+    $statuses = collect([30, 20, 10])->map(function ($minutesAgo) use ($author) {
         $s = Status::factory()->create([
             'profile_id' => $author->profile_id,
             'type' => 'photo',
@@ -36,13 +38,33 @@ it('still returns a next link when the whole page is filtered out', function ()
             'rendered' => 'hideme sunset photo',
             'in_reply_to_id' => null,
             'reblog_of_id' => null,
+            'created_at' => now()->subMinutes($minutesAgo),
         ]);
         StatusService::getMastodon($s->id, false);
 
         return $s;
     });
 
-    foreach ($statuses as $s) {
+    // A remote post holds the lowest id in the window and the most recent
+    // date, so the numeric minimum and the display-order oldest differ.
+    $remoteAuthor = Profile::factory()->create(['user_id' => null, 'domain' => 'remote.example']);
+    $lowIdRecent = Status::factory()->create([
+        'id' => SnowflakeService::byDate(now()->subMonths(8)),
+        'profile_id' => $remoteAuthor->id,
+        'type' => 'photo',
+        'scope' => 'public',
+        'visibility' => 'public',
+        'local' => false,
+        'uri' => 'https://remote.example/p/'.uniqid(),
+        'caption' => 'hideme sunset photo',
+        'rendered' => 'hideme sunset photo',
+        'in_reply_to_id' => null,
+        'reblog_of_id' => null,
+        'created_at' => now()->subMinutes(1),
+    ]);
+    StatusService::getMastodon($lowIdRecent->id, false);
+
+    foreach ($statuses->concat([$lowIdRecent]) as $s) {
         HomeTimelineService::add($pid, $s->id);
     }
 
@@ -61,8 +83,14 @@ it('still returns a next link when the whole page is filtered out', function ()
 
     expect($response->json())->toBe([]);
 
+    // The continuation cursor has to be the oldest member in display order,
+    // which is not the lowest id once a remote post is in the window.
+    $displayOldest = (int) $statuses->sortBy(fn ($s) => $s->created_at->timestamp)->first()->id;
+
+    expect((int) $lowIdRecent->id)->toBeLessThan($displayOldest);
+
     $link = $response->headers->get('Link');
     expect($link)->not->toBeNull()
         ->and($link)->toContain('rel="next"')
-        ->and($link)->toContain('max_id='.$statuses->pluck('id')->map(fn ($id) => (int) $id)->min());
+        ->and($link)->toContain('max_id='.$displayOldest);
 });