From 0ec840f0da27a19684e9626e0cb1908e36540ab3 Mon Sep 17 00:00:00 2001 From: Christen Dijoux Date: Sat, 24 Oct 2026 23:08:20 -0400 Subject: [PATCH 1/2] fix: keep the reaction cap when reactions arrive at the same moment --- app/Actions/CreateMessageReaction.php | 14 ++++++++++++ .../MessageReactionControllerTest.php | 22 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/app/Actions/CreateMessageReaction.php b/app/Actions/CreateMessageReaction.php index 68eb21734..071b40907 100644 --- a/app/Actions/CreateMessageReaction.php +++ b/app/Actions/CreateMessageReaction.php @@ -18,10 +18,24 @@ public function __construct(private UpdateMessageReactionsVersion $updateMessage /** * Inserted unless already there, so reacting twice with the same emoji leaves one reaction. + * + * The request checks the cap before this runs, but reactions sent at the same moment each see the others missing. + * So the message is locked and the cap counted again here: a reaction past it changes nothing. */ public function handle(User $user, Message $message, string $emoji): void { DB::transaction(function () use ($user, $message, $emoji): void { + Message::query()->whereKey($message->id)->lockForUpdate()->first(); + + $reacted = MessageReaction::query() + ->where('message_id', $message->id) + ->where('user_id', $user->id) + ->pluck('emoji'); + + if ($reacted->count() >= MessageReaction::MAX_PER_MESSAGE && $reacted->doesntContain($emoji)) { + return; + } + $inserted = MessageReaction::query()->insertOrIgnore([ 'message_id' => $message->id, 'user_id' => $user->id, diff --git a/tests/Feature/Controllers/MessageReactionControllerTest.php b/tests/Feature/Controllers/MessageReactionControllerTest.php index efc91fc2d..eec19a286 100644 --- a/tests/Feature/Controllers/MessageReactionControllerTest.php +++ b/tests/Feature/Controllers/MessageReactionControllerTest.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use App\Actions\CreateMessageReaction; use App\Enums\RemotesFrom; use App\Models\Message; use App\Models\MessageReaction; @@ -67,6 +68,27 @@ $this->actingAs($mark)->postJson(route('message-reaction.store', $message), ['emoji' => '💀'])->assertOk(); }); +it('keeps the cap when reactions sent at the same moment all passed the check', function (): void { + $mark = User::factory()->withRemotes()->create(); + $nuno = User::factory()->withRemotes()->create(); + $message = sendChatMessage($mark, $nuno); + $emoji = ['😂', '✅', '❤️', '🚀', '👀', '🔥', '🎉', '👍', '💯', '🙏', '🤔', '👏']; + + foreach ($emoji as $each) { + resolve(CreateMessageReaction::class)->handle($nuno, $message, $each); + } + + // Sent at the same moment as the twelfth, so the check let them through. + resolve(CreateMessageReaction::class)->handle($nuno, $message, '💀'); + resolve(CreateMessageReaction::class)->handle($nuno, $message, '😂'); + resolve(CreateMessageReaction::class)->handle($mark, $message, '💀'); + + expect(MessageReaction::query()->where('message_id', $message->id)->where('user_id', $nuno->id)->pluck('emoji')->all()) + ->toEqualCanonicalizing($emoji) + ->and(MessageReaction::query()->where('message_id', $message->id)->where('user_id', $mark->id)->pluck('emoji')->all()) + ->toBe(['💀']); +}); + it('keeps reactions to the people who can see the message', function (): void { $mark = User::factory()->withRemotes()->create(); $nuno = User::factory()->withRemotes()->create(); From ae73f364badd5f8a8d89baa113b9478bf1e8e402 Mon Sep 17 00:00:00 2001 From: Christen Dijoux <123602948+Akiyo974@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:43:19 -0400 Subject: [PATCH 2/2] fix: lock the conversation first when adding a reaction Locking the message first reversed the order pins and removed reactions use (conversation, then message), so a reaction added while another was removed or the message pinned could deadlock. --- app/Actions/CreateMessageReaction.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/Actions/CreateMessageReaction.php b/app/Actions/CreateMessageReaction.php index 071b40907..9491920b5 100644 --- a/app/Actions/CreateMessageReaction.php +++ b/app/Actions/CreateMessageReaction.php @@ -4,6 +4,7 @@ namespace App\Actions; +use App\Models\Conversation; use App\Models\Message; use App\Models\MessageReaction; use App\Models\User; @@ -20,12 +21,14 @@ public function __construct(private UpdateMessageReactionsVersion $updateMessage * Inserted unless already there, so reacting twice with the same emoji leaves one reaction. * * The request checks the cap before this runs, but reactions sent at the same moment each see the others missing. - * So the message is locked and the cap counted again here: a reaction past it changes nothing. + * So the cap is counted again here, under the conversation's lock: a reaction past it changes nothing. The + * conversation is the row locked first, as pins and removed reactions lock it before the message, so none of them + * can deadlock with this. */ public function handle(User $user, Message $message, string $emoji): void { DB::transaction(function () use ($user, $message, $emoji): void { - Message::query()->whereKey($message->id)->lockForUpdate()->first(); + Conversation::query()->whereKey($message->conversation_id)->lockForUpdate()->first(); $reacted = MessageReaction::query() ->where('message_id', $message->id)