diff --git a/app/Actions/CreateMessageReaction.php b/app/Actions/CreateMessageReaction.php index 68eb21734..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; @@ -18,10 +19,26 @@ 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 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 { + Conversation::query()->whereKey($message->conversation_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();