Merge pull request #633 from beyondcode/fix/fix-stale-data

[fix] Fix stale data

Author: rennokki

src/Contracts/StatisticsCollector.php

@@ -66,4 +66,13 @@ public function getStatistics(): PromiseInterface;
      * @return PromiseInterface[\BeyondCode\LaravelWebSockets\Statistics\Statistic|null]
      */
     public function getAppStatistics($appId): PromiseInterface;
+
+    /**
+     * Remove all app traces from the database if no connections have been set
+     * in the meanwhile since last save.
+     *
+     * @param  string|int  $appId
+     * @return void
+     */
+    public function resetAppTraces($appId);
 }

src/Statistics/Collectors/MemoryCollector.php

@@ -96,6 +96,12 @@ public function save()
                     continue;
                 }
 
+                if ($statistic->shouldHaveTracesRemoved()) {
+                    $this->resetAppTraces($appId);
+
+                    continue;
+                }
+
                 $this->createRecord($statistic, $appId);
 
                 $this->channelManager
@@ -142,6 +148,18 @@ public function getAppStatistics($appId): PromiseInterface
         );
     }
 
+    /**
+     * Remove all app traces from the database if no connections have been set
+     * in the meanwhile since last save.
+     *
+     * @param  string|int  $appId
+     * @return void
+     */
+    public function resetAppTraces($appId)
+    {
+        unset($this->statistics[$appId]);
+    }
+
     /**
      * Find or create a defined statistic for an app.
      *

src/Statistics/Collectors/RedisCollector.php

@@ -170,6 +170,10 @@ public function save()
                                     $appId, Helpers::redisListToArray($list)
                                 );
 
+                                if ($statistic->shouldHaveTracesRemoved()) {
+                                    return $this->resetAppTraces($appId);
+                                }
+
                                 $this->createRecord($statistic, $appId);
 
                                 $this->channelManager
@@ -265,7 +269,7 @@ public function resetStatistics($appId, int $currentConnectionCount)
             ->getPublishClient()
             ->hset(
                 $this->channelManager->getRedisKey($appId, null, ['stats']),
-                'peak_connections_count', $currentConnectionCount
+                'peak_connections_count', max(0, $currentConnectionCount)
             );
 
         $this->channelManager
@@ -292,6 +296,8 @@ public function resetStatistics($appId, int $currentConnectionCount)
      */
     public function resetAppTraces($appId)
     {
+        parent::resetAppTraces($appId);
+
         $this->channelManager
             ->getPublishClient()
             ->hdel(

src/Statistics/Statistic.php

@@ -178,11 +178,23 @@ public function apiMessage()
     public function reset(int $currentConnectionsCount)
     {
         $this->currentConnectionsCount = $currentConnectionsCount;
-        $this->peakConnectionsCount = $currentConnectionsCount;
+        $this->peakConnectionsCount = max(0, $currentConnectionsCount);
         $this->webSocketMessagesCount = 0;
         $this->apiMessagesCount = 0;
     }
 
+    /**
+     * Check if the current statistic entry is empty. This means
+     * that the statistic entry can be easily deleted if no activity
+     * occured for a while.
+     *
+     * @return bool
+     */
+    public function shouldHaveTracesRemoved(): bool
+    {
+        return $this->currentConnectionsCount === 0 && $this->peakConnectionsCount === 0;
+    }
+
     /**
      * Transform the statistic to array.
      *

tests/StatisticsStoreTest.php

@@ -16,6 +16,21 @@ public function test_store_statistics_on_public_channel()
         $this->assertEquals('2', $records[0]['peak_connections_count']);
         $this->assertEquals('2', $records[0]['websocket_messages_count']);
         $this->assertEquals('0', $records[0]['api_messages_count']);
+
+        $this->pusherServer->onClose($rick);
+        $this->pusherServer->onClose($morty);
+
+        $this->statisticsCollector->save();
+
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
+
+        $this->assertEquals('2', $records[1]['peak_connections_count']);
+
+        $this->statisticsCollector->save();
+
+        // The last one should not generate any more records
+        // since the current state is empty.
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
     }
 
     public function test_store_statistics_on_private_channel()
@@ -30,19 +45,51 @@ public function test_store_statistics_on_private_channel()
         $this->assertEquals('2', $records[0]['peak_connections_count']);
         $this->assertEquals('2', $records[0]['websocket_messages_count']);
         $this->assertEquals('0', $records[0]['api_messages_count']);
+
+        $this->pusherServer->onClose($rick);
+        $this->pusherServer->onClose($morty);
+
+        $this->statisticsCollector->save();
+
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
+
+        $this->assertEquals('2', $records[1]['peak_connections_count']);
+
+        $this->statisticsCollector->save();
+
+        // The last one should not generate any more records
+        // since the current state is empty.
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
     }
 
     public function test_store_statistics_on_presence_channel()
     {
         $rick = $this->newPresenceConnection('presence-channel', ['user_id' => 1]);
         $morty = $this->newPresenceConnection('presence-channel', ['user_id' => 2]);
+        $pickleRick = $this->newPresenceConnection('presence-channel', ['user_id' => 1]);
 
         $this->statisticsCollector->save();
 
         $this->assertCount(1, $records = $this->statisticsStore->getRecords());
 
-        $this->assertEquals('2', $records[0]['peak_connections_count']);
-        $this->assertEquals('2', $records[0]['websocket_messages_count']);
+        $this->assertEquals('3', $records[0]['peak_connections_count']);
+        $this->assertEquals('3', $records[0]['websocket_messages_count']);
         $this->assertEquals('0', $records[0]['api_messages_count']);
+
+        $this->pusherServer->onClose($rick);
+        $this->pusherServer->onClose($morty);
+        $this->pusherServer->onClose($pickleRick);
+
+        $this->statisticsCollector->save();
+
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
+
+        $this->assertEquals('3', $records[1]['peak_connections_count']);
+
+        $this->statisticsCollector->save();
+
+        // The last one should not generate any more records
+        // since the current state is empty.
+        $this->assertCount(2, $records = $this->statisticsStore->getRecords());
     }
 }