From 8f089d62859b17f503695be7523b2ef86cb71461 Mon Sep 17 00:00:00 2001 From: Daniel Supernault Date: Sun, 11 Oct 2026 05:32:17 -0600 Subject: [PATCH] Fix Story View count hydration/sync bug --- .../Stories/StoryApiV1Controller.php | 22 +--- app/Http/Controllers/StoryController.php | 21 +--- app/Services/StoryIndexService.php | 15 +++ app/Services/StoryViewService.php | 35 ++++++ app/Util/ActivityPub/Inbox/HandlesStories.php | 16 +-- .../Feature/Story/StoryViewCountIndexTest.php | 118 ++++++++++++++++++ 6 files changed, 180 insertions(+), 47 deletions(-) create mode 100644 app/Services/StoryViewService.php create mode 100644 tests/Feature/Story/StoryViewCountIndexTest.php diff --git a/app/Http/Controllers/Stories/StoryApiV1Controller.php b/app/Http/Controllers/Stories/StoryApiV1Controller.php index cf44e54bb..8cb470c37 100644 --- a/app/Http/Controllers/Stories/StoryApiV1Controller.php +++ b/app/Http/Controllers/Stories/StoryApiV1Controller.php @@ -17,6 +17,7 @@ use App\Services\DirectMessageService; use App\Services\MediaPathService; use App\Services\StoryIndexService; use App\Services\StoryService; +use App\Services\StoryViewService; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Arr; @@ -665,25 +666,14 @@ class StoryApiV1Controller extends Controller $following = Follower::whereProfileId($pid)->whereFollowingId($story->profile_id)->exists(); abort_if(! $following, 403, 'Invalid permission'); - $v = StoryView::firstOrCreate([ - 'story_id' => $id, - 'profile_id' => $pid, - ]); - - $index = app(StoryIndexService::class); - $index->markSeen($pid, $story->profile_id, $story->id, $story->created_at); - - if ($v->wasRecentlyCreated) { + $recorded = app(StoryViewService::class)->record($story, (int) $pid); - Story::findOrFail($story->id)->increment('view_count'); - - if ($story->local == false) { - StoryViewDeliver::dispatch($story, $authed)->onQueue('story'); - } - Cache::forget('stories:recent:by_id:'.$pid); - StoryService::addSeen($pid, $story->id); + if ($recorded && $story->local == false) { + StoryViewDeliver::dispatch($story, $authed)->onQueue('story'); } + Cache::forget(self::RECENT_KEY.$pid); + return ['code' => 200]; } diff --git a/app/Http/Controllers/StoryController.php b/app/Http/Controllers/StoryController.php index 6e1ab1a46..f2c055421 100644 --- a/app/Http/Controllers/StoryController.php +++ b/app/Http/Controllers/StoryController.php @@ -9,8 +9,8 @@ use App\Models\StoryView; use App\Services\AccountService; use App\Services\FollowerService; use App\Services\PollService; -use App\Services\StoryIndexService; use App\Services\StoryService; +use App\Services\StoryViewService; use App\Services\UserRoleService; use App\Transformer\ActivityPub\Verb\StoryVerb; use Illuminate\Contracts\View\View; @@ -199,26 +199,13 @@ class StoryController extends StoryComposeController $publicOnly = (bool) $profile->followedBy($authed); abort_if(! $publicOnly, 403); - $v = StoryView::firstOrCreate([ - 'story_id' => $id, - 'profile_id' => $authed->id, - ]); - - if ($v->wasRecentlyCreated) { - Story::findOrFail($story->id)->increment('view_count'); - $index = app(StoryIndexService::class); - $index->markSeen($authed->id, $story->profile_id, $story->id, $story->created_at); + $recorded = app(StoryViewService::class)->record($story, (int) $authed->id); - if ($story->local == false) { - StoryViewDeliver::dispatch($story, $authed)->onQueue('story'); - } + if ($recorded && $story->local == false) { + StoryViewDeliver::dispatch($story, $authed)->onQueue('story'); } - // Match the key recent() writes (pf:stories:recent-by-id:{viewer_pid}); - // the old key never matched, so the viewer's carousel snapshot (and its - // seen flags) stayed pinned for the full TTL. Cache::forget('pf:stories:recent-by-id:'.$authed->id); - StoryService::addSeen($authed->id, $story->id); return ['code' => 200]; } diff --git a/app/Services/StoryIndexService.php b/app/Services/StoryIndexService.php index add0ecbf4..ae6e35404 100644 --- a/app/Services/StoryIndexService.php +++ b/app/Services/StoryIndexService.php @@ -258,6 +258,21 @@ class StoryIndexService Redis::expire($key, $finalTtl); } + public function incrementViewCount(string|int $storyId, int $by = 1): void + { + $key = $this->storyKey($storyId); + + if (! $this->redisBool(fn () => Redis::exists($key))) { + return; + } + + Redis::hincrby($key, 'view_count', $by); + + if (! $this->redisBool(fn () => Redis::hexists($key, 'id'))) { + Redis::del($key); + } + } + /** * Keep the cached following set in sync when a follow is created. * Only touches the key if it already exists; otherwise the next carousel diff --git a/app/Services/StoryViewService.php b/app/Services/StoryViewService.php new file mode 100644 index 000000000..fe459da4e --- /dev/null +++ b/app/Services/StoryViewService.php @@ -0,0 +1,35 @@ +profile_id === $viewerId) { + return false; + } + + $view = StoryView::firstOrCreate([ + 'story_id' => $story->id, + 'profile_id' => $viewerId, + ]); + + $this->index->markSeen($viewerId, (int) $story->profile_id, (int) $story->id, $story->created_at); + StoryService::addSeen($viewerId, $story->id); + + if (! $view->wasRecentlyCreated) { + return false; + } + + Story::whereKey($story->id)->increment('view_count'); + $this->index->incrementViewCount($story->id); + + return true; + } +} diff --git a/app/Util/ActivityPub/Inbox/HandlesStories.php b/app/Util/ActivityPub/Inbox/HandlesStories.php index 6f62b3779..32c406ae6 100644 --- a/app/Util/ActivityPub/Inbox/HandlesStories.php +++ b/app/Util/ActivityPub/Inbox/HandlesStories.php @@ -5,11 +5,10 @@ namespace App\Util\ActivityPub\Inbox; use App\Jobs\StoryPipeline\StoryFetch; use App\Models\Status; use App\Models\Story; -use App\Models\StoryView; use App\Services\DirectMessageService; use App\Services\FollowerService; use App\Services\SanitizeService; -use App\Services\StoryIndexService; +use App\Services\StoryViewService; use App\Util\ActivityPub\Helpers; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; @@ -82,18 +81,7 @@ trait HandlesStories return; } - $view = StoryView::firstOrCreate([ - 'story_id' => $story->id, - 'profile_id' => $profile->id, - ]); - - $index = app(StoryIndexService::class); - $index->markSeen($profile->id, $story->profile_id, $story->id, $story->created_at); - - if ($view->wasRecentlyCreated == true) { - $story->view_count++; - $story->save(); - } + app(StoryViewService::class)->record($story, (int) $profile->id); } public function handleStoryReactionActivity(): void diff --git a/tests/Feature/Story/StoryViewCountIndexTest.php b/tests/Feature/Story/StoryViewCountIndexTest.php new file mode 100644 index 000000000..a60741ee4 --- /dev/null +++ b/tests/Feature/Story/StoryViewCountIndexTest.php @@ -0,0 +1,118 @@ +profile_id = $author->id; + $story->type = 'photo'; + $story->duration = 3; + $story->path = 'public/story/'.uniqid().'.jpg'; + $story->local = true; + $story->active = true; + $story->public = false; + $story->view_count = 0; + $story->expires_at = now()->addHours(24); + $story->save(); + + return $story->refresh(); +} + +function authorCarouselViewCount(Profile $author, Story $story): ?int +{ + $nodes = app(StoryIndexService::class)->fetchCarouselNodes( + (int) $author->id, + fn (array $ids) => collect($ids)->mapWithKeys(fn ($id) => [$id => [ + 'id' => (string) $id, + 'username' => 'user'.$id, + 'acct' => 'user'.$id, + 'avatar' => null, + 'local' => true, + ]])->all() + ); + + $item = collect($nodes) + ->where('is_author', true) + ->pluck('stories') + ->flatten(1) + ->firstWhere('id', (string) $story->id); + + return $item['view_count'] ?? null; +} + +beforeEach(function () { + Redis::flushdb(); +}); + +afterEach(function () { + Redis::flushdb(); +}); + +it('reflects new views in the author carousel without reindexing', function () { + $author = Profile::factory()->create(['user_id' => null]); + $local = Profile::factory()->create(['user_id' => null]); + $remote = Profile::factory()->create(['user_id' => null, 'domain' => 'remote.example']); + $story = makeViewCountStory($author); + + app(StoryIndexService::class)->indexStory($story); + + expect(authorCarouselViewCount($author, $story))->toBe(0); + + $views = app(StoryViewService::class); + + expect($views->record($story, (int) $local->id))->toBeTrue(); + expect($views->record($story, (int) $remote->id))->toBeTrue(); + + expect(authorCarouselViewCount($author, $story))->toBe(2); + expect($story->fresh()->view_count)->toBe(2); + expect(StoryView::whereStoryId($story->id)->count())->toBe(2); +}); + +it('counts a repeat view once and marks the story seen for the viewer', function () { + $author = Profile::factory()->create(['user_id' => null]); + $viewer = Profile::factory()->create(['user_id' => null]); + $story = makeViewCountStory($author); + + $index = app(StoryIndexService::class); + $index->indexStory($story); + + $views = app(StoryViewService::class); + + expect($views->record($story, (int) $viewer->id))->toBeTrue(); + expect($views->record($story, (int) $viewer->id))->toBeFalse(); + + expect(authorCarouselViewCount($author, $story))->toBe(1); + expect($story->fresh()->view_count)->toBe(1); + expect($index->hasSeen((int) $viewer->id, (int) $story->id))->toBeTrue(); +}); + +it('ignores the author viewing their own story', function () { + $author = Profile::factory()->create(['user_id' => null]); + $story = makeViewCountStory($author); + + app(StoryIndexService::class)->indexStory($story); + + expect(app(StoryViewService::class)->record($story, (int) $author->id))->toBeFalse(); + expect(authorCarouselViewCount($author, $story))->toBe(0); + expect($story->fresh()->view_count)->toBe(0); +}); + +it('does not create a partial index entry for an unindexed story', function () { + $author = Profile::factory()->create(['user_id' => null]); + $viewer = Profile::factory()->create(['user_id' => null]); + $story = makeViewCountStory($author); + + expect(app(StoryViewService::class)->record($story, (int) $viewer->id))->toBeTrue(); + + expect((bool) Redis::exists("story:{$story->id}"))->toBeFalse(); + expect($story->fresh()->view_count)->toBe(1); +});