[1.x] Correctly dispatches presence events (#216)

* return only unique users

* prevent duplicate events

src/Protocols/Pusher/Channels/Concerns/InteractsWithPresenceChannels.php

@@ -15,12 +15,20 @@ public function subscribe(Connection $connection, ?string $auth = null, ?string
     {
         $this->verify($connection, $auth, $data);
 
+        $userData = $data ? json_decode($data, associative: true, flags: JSON_THROW_ON_ERROR) : [];
+
+        if ($this->userIsSubscribed($userData['user_id'] ?? null)) {
+            parent::subscribe($connection, $auth, $data);
+
+            return;
+        }
+
         parent::subscribe($connection, $auth, $data);
 
         parent::broadcastInternally(
             [
                 'event' => 'pusher_internal:member_added',
-                'data' => $data ? json_decode($data, associative: true, flags: JSON_THROW_ON_ERROR) : [],
+                'data' => $userData,
                 'channel' => $this->name(),
             ],
             $connection
@@ -32,24 +40,26 @@ public function subscribe(Connection $connection, ?string $auth = null, ?string
      */
     public function unsubscribe(Connection $connection): void
     {
-        if (! $subscription = $this->connections->find($connection)) {
-            parent::unsubscribe($connection);
+        $subscription = $this->connections->find($connection);
 
-            return;
-        }
+        parent::unsubscribe($connection);
 
-        if ($userId = $subscription->data('user_id')) {
-            parent::broadcast(
-                [
-                    'event' => 'pusher_internal:member_removed',
-                    'data' => ['user_id' => $userId],
-                    'channel' => $this->name(),
-                ],
-                $connection
-            );
+        if (
+            ! $subscription ||
+            ! $subscription->data('user_id') ||
+            $this->userIsSubscribed($subscription->data('user_id'))
+        ) {
+            return;
         }
 
-        parent::unsubscribe($connection);
+        parent::broadcast(
+            [
+                'event' => 'pusher_internal:member_removed',
+                'data' => ['user_id' => $subscription->data('user_id')],
+                'channel' => $this->name(),
+            ],
+            $connection
+        );
     }
 
     /**
@@ -58,7 +68,8 @@ public function unsubscribe(Connection $connection): void
     public function data(): array
     {
         $connections = collect($this->connections->all())
-            ->map(fn ($connection) => $connection->data());
+            ->map(fn ($connection) => $connection->data())
+            ->unique('user_id');
 
         if ($connections->contains(fn ($connection) => ! isset($connection['user_id']))) {
             return [
@@ -78,4 +89,16 @@ public function data(): array
             ],
         ];
     }
+
+    /**
+     * Determine if the given user is subscribed to the channel.
+     */
+    protected function userIsSubscribed(?string $userId): bool
+    {
+        if (! $userId) {
+            return false;
+        }
+
+        return collect($this->connections->all())->map(fn ($connection) => (string) $connection->data('user_id'))->contains($userId);
+    }
 }

tests/Feature/Protocols/Pusher/Reverb/ServerTest.php

@@ -470,3 +470,15 @@
     expect($response->getStatusCode())->toBe(200);
     expect($response->getBody()->getContents())->toBe('{}');
 });
+
+it('subscription_succeeded event contains unique list of users', function () {
+    $data = ['user_id' => 1, 'user_info' => ['name' => 'Test User']];
+    subscribe('presence-test-channel', data: $data);
+    $data = ['user_id' => 1, 'user_info' => ['name' => 'Test User']];
+    $response = subscribe('presence-test-channel', data: $data);
+
+    expect($response)->toContain('pusher_internal:subscription_succeeded');
+    expect($response)->toContain('"count\":1');
+    expect($response)->toContain('"ids\":[1]');
+    expect($response)->toContain('"hash\":{\"1\":{\"name\":\"Test User\"}}');
+});

tests/Unit/Protocols/Pusher/Channels/PresenceChannelTest.php

@@ -161,3 +161,35 @@
         'channel' => 'presence-test-channel',
     ]));
 });
+
+it('ensures the "member_added" event is only fired once', function () {
+    $channel = new PresenceChannel('presence-test-channel');
+
+    $connectionOne = collect(factory(data: ['user_info' => ['name' => 'Joe'], 'user_id' => 1]))->first();
+    $connectionTwo = collect(factory(data: ['user_info' => ['name' => 'Joe'], 'user_id' => 1]))->first();
+
+    $this->channelConnectionManager->shouldReceive('all')
+        ->andReturn([$connectionOne, $connectionTwo]);
+
+    $channel->subscribe($connectionOne->connection(), validAuth($connectionOne->id(), 'presence-test-channel', $data = json_encode($connectionOne->data())), $data);
+    $channel->subscribe($connectionTwo->connection(), validAuth($connectionTwo->id(), 'presence-test-channel', $data = json_encode($connectionTwo->data())), $data);
+
+    $connectionOne->connection()->assertNothingReceived();
+});
+
+it('ensures the "member_removed" event is only fired once', function () {
+    $channel = new PresenceChannel('presence-test-channel');
+
+    $connectionOne = collect(factory(data: ['user_info' => ['name' => 'Joe'], 'user_id' => 1]))->first();
+    $connectionTwo = collect(factory(data: ['user_info' => ['name' => 'Joe'], 'user_id' => 1]))->first();
+
+    $this->channelConnectionManager->shouldReceive('find')
+        ->andReturn($connectionOne);
+
+    $this->channelConnectionManager->shouldReceive('all')
+        ->andReturn([$connectionOne, $connectionTwo]);
+
+    $channel->unsubscribe($connectionTwo->connection(), validAuth($connectionTwo->id(), 'presence-test-channel', $data = json_encode($connectionTwo->data())), $data);
+
+    $connectionOne->connection()->assertNothingReceived();
+});