Merge pull request #63 from clue-labs/monitor

Remove support for MONITOR command for performance and consistency reasons
clue · Sep 19, 2017 · aa41eb9 · aa41eb9
2 parents 2dfe566 + b634de1
commit aa41eb9
Show file tree

Hide file tree

Showing 6 changed files with 7 additions and 105 deletions.
diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ It enables you to set and query its data or use its PubSub topics to react to in
   process their responses as soon as results come in.
   The Promise-based design provides a *sane* interface to working with async responses.
 * **Event-driven core** -
-  Register your event handler callbacks to react to incoming events, such as an incoming PubSub message or a MONITOR event.
+  Register your event handler callbacks to react to incoming events, such as an incoming PubSub message event.
 * **Lightweight, SOLID design** -
   Provides a thin abstraction that is [*just good enough*](http://en.wikipedia.org/wiki/Principle_of_good_enough)
   and does not get in your way.
@@ -236,11 +236,6 @@ $client->on('unsubscribe', function ($channel, $total) {
 $client->on('punsubscribe', function ($pattern, $total) {
     // unsubscribed from matching given $pattern
 });
-
-// monitor events:
-$client->on('monitor', function (StatusReply $message) {
-    // somebody executed a command
-});
 ```
 
 #### close()

diff --git a/examples/monitor.php b/examples/monitor.php
diff --git a/src/Client.php b/src/Client.php
@@ -18,8 +18,6 @@
  * @event pmessage($pattern, $channel, $message)
  * @event psubscribe($channel, $numberOfChannels)
  * @event punsubscribe($channel, $numberOfChannels)
- *
- * @event monitor(ModelInterface $statusModel)
  */
 interface Client extends EventEmitterInterface
 {

diff --git a/src/StreamingClient.php b/src/StreamingClient.php
@@ -14,7 +14,6 @@
 use Clue\Redis\Protocol\Model\ErrorReply;
 use Clue\Redis\Protocol\Model\ModelInterface;
 use Clue\Redis\Protocol\Model\MultiBulkReply;
-use Clue\Redis\Protocol\Model\StatusReply;
 use React\Stream\DuplexStreamInterface;
 
 /**
@@ -31,7 +30,6 @@ class StreamingClient extends EventEmitter implements Client
 
     private $subscribed = 0;
     private $psubscribed = 0;
-    private $monitoring = false;
 
     public function __construct(DuplexStreamInterface $stream, ParserInterface $parser = null, SerializerInterface $serializer = null)
     {
@@ -89,17 +87,14 @@ public function __call($name, $args)
             $request->reject(new RuntimeException('Connection closed'));
         } elseif (count($args) !== 1 && in_array($name, $pubsubs)) {
             $request->reject(new InvalidArgumentException('PubSub commands limited to single argument'));
+        } elseif ($name === 'monitor') {
+            $request->reject(new \BadMethodCallException('MONITOR command explicitly not supported'));
         } else {
             $this->stream->write($this->serializer->getRequestMessage($name, $args));
             $this->requests []= $request;
         }
 
-        if ($name === 'monitor') {
-            $monitoring =& $this->monitoring;
-            $promise->then(function () use (&$monitoring) {
-                $monitoring = true;
-            });
-        } elseif (in_array($name, $pubsubs)) {
+        if (in_array($name, $pubsubs)) {
             $that = $this;
             $subscribed =& $this->subscribed;
             $psubscribed =& $this->psubscribed;
@@ -124,11 +119,6 @@ public function __call($name, $args)
 
     public function handleMessage(ModelInterface $message)
     {
-        if ($this->monitoring && $this->isMonitorMessage($message)) {
-            $this->emit('monitor', array($message));
-            return;
-        }
-
         if (($this->subscribed !== 0 || $this->psubscribed !== 0) && $message instanceof MultiBulkReply) {
             $array = $message->getValueNative();
             $first = array_shift($array);
@@ -192,10 +182,4 @@ public function close()
             $request->reject(new RuntimeException('Connection closing'));
         }
     }
-
-    private function isMonitorMessage(ModelInterface $message)
-    {
-        // Check status '1409172115.207170 [0 127.0.0.1:58567] "ping"' contains otherwise uncommon '] "'
-        return ($message instanceof StatusReply && strpos($message->getValueNative(), '] "') !== false);
-    }
 }
diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php
@@ -94,18 +94,6 @@ public function testMultiExecQueuedExecHasValues()
         $this->waitFor($client);
     }
 
-    public function testMonitorPing()
-    {
-        $client = $this->client;
-
-        $client->on('monitor', $this->expectCallableOnce());
-
-        $client->monitor()->then($this->expectCallableOnce('OK'));
-        $client->ping()->then($this->expectCallableOnce('PONG'));
-
-        $this->waitFor($client);
-    }
-
     public function testPubSub()
     {
         $consumer = $this->client;

diff --git a/tests/StreamingClientTest.php b/tests/StreamingClientTest.php
@@ -7,7 +7,6 @@
 use Clue\Redis\Protocol\Model\ErrorReply;
 use Clue\Redis\Protocol\Model\MultiBulkReply;
 use Clue\React\Redis\Client;
-use Clue\Redis\Protocol\Model\StatusReply;
 
 class StreamingClientTest extends TestCase
 {
@@ -81,52 +80,14 @@ public function testPingPong()
         $promise->then($this->expectCallableOnce('PONG'));
     }
 
-    /**
-     * @expectedException UnderflowException
-     */
-    public function testInvalidMonitor()
-    {
-        $this->client->handleMessage(new StatusReply('+1409171800.312243 [0 127.0.0.1:58542] "ping"'));
-    }
-
-    public function testMonitor()
+    public function testMonitorCommandIsNotSupported()
     {
-        $this->serializer->expects($this->once())->method('getRequestMessage')->with($this->equalTo('monitor'));
-
         $promise = $this->client->monitor();
 
-        $this->client->handleMessage(new StatusReply('OK'));
-
-        $this->expectPromiseResolve($promise);
-        $promise->then($this->expectCallableOnce('OK'));
-    }
-
-    public function testMonitorEventFromOtherConnection()
-    {
-        // enter MONITOR mode
-        $client = $this->client;
-        $client->monitor();
-        $client->handleMessage(new StatusReply('OK'));
-
-        // expect a single "monitor" event when a matching status reply comes in
-        $client->on('monitor', $this->expectCallableOnce());
-        $client->handleMessage(new StatusReply('1409171800.312243 [0 127.0.0.1:58542] "ping"'));
+        $this->expectPromiseReject($promise);
+        $this->assertFalse($this->client->isBusy());
     }
 
-    public function testMonitorEventFromPingMessage()
-    {
-        // enter MONITOR mode
-        $client = $this->client;
-        $client->monitor();
-        $client->handleMessage(new StatusReply('OK'));
-
-        // expect a single "monitor" event when a matching status reply comes in
-        // ignore the status reply for the command executed (ping)
-        $client->on('monitor', $this->expectCallableOnce());
-        $client->ping();
-        $client->handleMessage(new StatusReply('1409171800.312243 [0 127.0.0.1:58542] "ping"'));
-        $client->handleMessage(new StatusReply('PONG'));
-    }
 
     public function testErrorReply()
     {