Merge pull request #3 from Renegade334/devel

elazar · elazar · commit 5d2154102574 · 2015-04-06T08:39:08.000-05:00
Add a "pass-through" return value for filters
diff --git a/README.md b/README.md
@@ -257,8 +257,10 @@ Filters are merely classes that implement
 [`FilterInterface`](https://github.com/phergie/phergie-irc-plugin-react-eventfilter/blob/master/src/FilterInterface.php).
 This interface has a single method, `filter()`, which accepts an event object
 that implements [`EventInterface`](https://github.com/phergie/phergie-irc-event/blob/master/src/EventInterface.php)
-as its only parameter and returns `true` if the event should be allowed or
-`false` if it should not.
+as its only parameter and returns one of the following values:
+* `true` if the event should be allowed
+* `false` if the event should be denied
+* `null` if the event should be ignored by the filter ("pass-through")
 
 ## Tests
 
diff --git a/src/AndFilter.php b/src/AndFilter.php
@@ -24,16 +24,21 @@ class AndFilter extends CompositeFilter
      * Filters events that pass all contained filters.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event passes all contained filters, FALSE
-     *         otherwise
+     * @return boolean|null TRUE if the event passes all contained filters, FALSE
+     *         if it fails any filter, or NULL if all filters return NULL.
      */
     public function filter(EventInterface $event)
     {
+        $output = null;
         foreach ($this->filters as $filter) {
-            if (!$filter->filter($event)) {
+            $result = $filter->filter($event);
+            if ($result === false) {
                 return false;
             }
+            if ($result === true) {
+                $output = true;
+            }
         }
-        return true;
+        return $output;
     }
 }
diff --git a/src/ChannelFilter.php b/src/ChannelFilter.php
@@ -40,7 +40,7 @@ class ChannelFilter implements FilterInterface
     protected $parameters = array(
         'JOIN' => 'channels',
         'PART' => 'channels',
-        'MODE' => 'target',
+        'MODE' => 'channel',
         'TOPIC' => 'channel',
         'KICK' => 'channel',
         'PRIVMSG' => 'receivers',
@@ -88,17 +88,21 @@ function($value) {
      * channels.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event is not channel-specific or originated
-     *         from a matching channel associated with this filter, FALSE
-     *         otherwise
+     * @return boolean|null TRUE if the event originated from a matching channel
+     *         associated with this filter, FALSE if it originated from a non-matching
+     *         channel, or NULL if it is not associated with a channel.
      */
     public function filter(EventInterface $event)
     {
         if (!$event instanceof UserEventInterface) {
-            return true;
+            return null;
         }
 
         $channels = $this->getChannels($event);
+        if (empty($channels)) {
+            return null;
+        }
+
         $commonChannels = array_intersect($channels, $this->channels);
         if ($commonChannels) {
             return true;
diff --git a/src/FilterInterface.php b/src/FilterInterface.php
@@ -24,7 +24,9 @@ interface FilterInterface
      * Evaluates an event for forwarding.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event should be forwarded, FALSE otherwise
+     * @return boolean|null TRUE if the event should be allowed, FALSE if it should be rejected,
+     *         or NULL if it should be ignored (ie. the event does not fall under the scope of
+     *         this filter)
      */
     public function filter(EventInterface $event);
 }
diff --git a/src/NotFilter.php b/src/NotFilter.php
@@ -41,10 +41,15 @@ public function __construct(FilterInterface $filter)
      * Filters events that do not pass the contained filter.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the contained filter fails, FALSE if it passes
+     * @return boolean|null TRUE if the contained filter fails, FALSE if it passes,
+     *         or NULL if it returns NULL.
      */
     public function filter(EventInterface $event)
     {
-        return !$this->filter->filter($event);
+        $result = $this->filter->filter($event);
+        if ($result === null) {
+            return null;
+        }
+        return !$result;
     }
 }
diff --git a/src/OrFilter.php b/src/OrFilter.php
@@ -24,16 +24,21 @@ class OrFilter extends CompositeFilter
      * Filters events that pass any contained filters.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event passes any contained filters, FALSE
-     *         otherwise
+     * @return boolean|null TRUE if the event passes any contained filters, FALSE
+     *         if it fails all filters, or NULL if all filters return NULL.
      */
     public function filter(EventInterface $event)
     {
+        $output = null;
         foreach ($this->filters as $filter) {
-            if ($filter->filter($event)) {
+            $result = $filter->filter($event);
+            if ($result === true) {
                 return true;
             }
+            if ($result === false) {
+                $output = false;
+            }
         }
-        return false;
+        return $output;
     }
 }
diff --git a/src/Plugin.php b/src/Plugin.php
@@ -165,7 +165,7 @@ public function handleEvent($event, array $args)
             ));
         } else {
             $eventObject = reset($eventObjects);
-            if (!$this->filter->filter($eventObject)) {
+            if ($this->filter->filter($eventObject) === false) {
                 $logger->info('Event did not pass filter, skipping', array(
                     'event' => $event,
                     'args' => $args,
diff --git a/src/UserFilter.php b/src/UserFilter.php
@@ -45,18 +45,23 @@ public function __construct(array $masks)
      * Filters events that are not user-specific or are from the specified users.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event is not user-specific or originated
-     *         from a user with a matching mask associated with this filter,
-     *         FALSE otherwise
+     * @return boolean|null TRUE if the event originated from a user with a matching mask
+     *         associated with this filter, FALSE if it originated from a user without a
+     *         matching mask, or NULL if it did not originate from a user.
      */
     public function filter(EventInterface $event)
     {
         if (!$event instanceof UserEventInterface) {
-            return true;
+            return null;
+        }
+
+        $nick = $event->getNick();
+        if ($nick === null) {
+            return null;
         }
 
         $userMask = sprintf('%s!%s@%s',
-            $event->getNick(),
+            $nick,
             $event->getUsername(),
             $event->getHost()
         );
diff --git a/src/UserModeFilter.php b/src/UserModeFilter.php
@@ -57,19 +57,23 @@ public function __construct(UserModePlugin $userMode, array $modes)
      * specified modes.
      *
      * @param \Phergie\Irc\Event\EventInterface $event
-     * @return boolean TRUE if the event is not user-specific or originated
-     *         from a user with a matching mode associated with this filter,
-     *         FALSE otherwise
+     * @return boolean|null TRUE if the event originated from a user with a matching mode
+     *         associated with this filter, FALSE if the event originated from a user
+     *         without a matching mode, or NULL if the event did not originate from a user.
      */
     public function filter(EventInterface $event)
     {
         if (!$event instanceof UserEventInterface) {
-            return true;
+            return null;
         }
 
-        $connection = $event->getConnection();
         $channels = $this->getChannels($event);
         $nick = $event->getNick();
+        if (empty($channels) || $nick === null) {
+            return null;
+        }
+
+        $connection = $event->getConnection();
 
         foreach ($channels as $channel) {
             $userModes = $this->userMode->getUserModes($connection, $channel, $nick);
diff --git a/tests/AndFilterTest.php b/tests/AndFilterTest.php
@@ -35,6 +35,9 @@ public function dataProviderFilter()
             array(true, false, false),
             array(false, true, false),
             array(true, true, true),
+            array(true, null, true),
+            array(false, null, false),
+            array(null, null, null),
         );
 
         foreach ($returns as $return) {
diff --git a/tests/ChannelFilterTest.php b/tests/ChannelFilterTest.php
@@ -47,14 +47,14 @@ public function dataProviderFilterPasses()
         $parameters = array(
             'JOIN' => 'channels',
             'PART' => 'channels',
-            'MODE' => 'target',
+            'MODE' => 'channel',
             'TOPIC' => 'channel',
             'KICK' => 'channel',
             'PRIVMSG' => 'receivers',
         );
 
         // Not an instance of UserEventInterface
-        $data[] = array(Phake::mock('\Phergie\Irc\Event\EventInterface'), true);
+        $data[] = array(Phake::mock('\Phergie\Irc\Event\EventInterface'), null);
 
         // Supported events in same channels as filter
         foreach (array('#channel1', '&channel2') as $channel) {
@@ -70,7 +70,7 @@ public function dataProviderFilterPasses()
         // Unsupported event
         $event = $this->getMockUserEvent();
         Phake::when($event)->getCommand()->thenReturn('QUIT');
-        $data[] = array($event, false);
+        $data[] = array($event, null);
 
         // Supported events in a different channel from the filter
         foreach ($parameters as $command => $parameter) {
diff --git a/tests/NotFilterTest.php b/tests/NotFilterTest.php
@@ -36,5 +36,8 @@ public function testFilter()
 
         Phake::when($nestedFilter)->filter($event)->thenReturn(false);
         $this->assertTrue($filter->filter($event));
+
+        Phake::when($nestedFilter)->filter($event)->thenReturn(null);
+        $this->assertNull($filter->filter($event));
     }
 }
diff --git a/tests/OrFilterTest.php b/tests/OrFilterTest.php
@@ -37,6 +37,9 @@ public function dataProviderFilter()
             array(true, false, true),
             array(false, true, true),
             array(true, true, true),
+            array(true, null, true),
+            array(false, null, false),
+            array(null, null, null),
         );
 
         foreach ($returns as $return) {
diff --git a/tests/PluginTest.php b/tests/PluginTest.php
@@ -57,10 +57,10 @@ class PluginTest extends \PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
-        foreach (range(1, 3) as $index) {
+        foreach (range(1, 4) as $index) {
             $this->mockPlugins[$index] = $this->getMockPlugin();
         }
-        $this->mockPlugins[4] = new CallbackPlugin;
+        $this->mockPlugins[5] = new CallbackPlugin;
         $this->mockFilter = $this->getMockFilter();
         $this->plugin = new Plugin(array(
             'plugins' => $this->mockPlugins,
@@ -168,6 +168,11 @@ public function testHandleEventForwardsEventsPerFilter()
             ->filter($this->identicalTo($passEvent))
             ->thenReturn(true);
 
+        $neutralEvent = $this->getMockEvent();
+        Phake::when($this->mockFilter)
+            ->filter($this->identicalTo($neutralEvent))
+            ->thenReturn(null);
+
         $failEvent = $this->getMockEvent();
         Phake::when($this->mockFilter)
             ->filter($this->identicalTo($failEvent))
@@ -192,16 +197,28 @@ public function testHandleEventForwardsEventsPerFilter()
         };
         Phake::when($this->mockPlugins[3])
             ->getSubscribedEvents()
-            ->thenReturn(array('fail' => $callbacks[3]));
+            ->thenReturn(array('neutral' => $callbacks[3]));
+
+        $neutralArgs = array($neutralEvent, 'bar');
+        $this->plugin->handleEvent('neutral', $neutralArgs);
+
+        $called[4] = false;
+        $callbacks[4] = function() use (&$called) {
+            $called[4] = true;
+        };
+        Phake::when($this->mockPlugins[4])
+            ->getSubscribedEvents()
+            ->thenReturn(array('fail' => $callbacks[4]));
 
         $this->plugin->handleEvent('pass', $passArgs);
 
-        $failArgs = array($failEvent, 'bar');
+        $failArgs = array($failEvent, 'baz');
         $this->plugin->handleEvent('fail', $failArgs);
 
         $this->assertTrue($called[1]);
         $this->assertTrue($called[2]);
-        $this->assertFalse($called[3]);
+        $this->assertTrue($called[3]);
+        $this->assertFalse($called[4]);
     }
 
     /**
@@ -235,10 +252,10 @@ public function testGetSubscribedEvents()
         $barArgs = array('barArg');
         $this->assertArrayHasKey('bar', $events);
         $this->assertInternalType('callable', $events['bar']);
-        $this->assertFalse($this->mockPlugins[4]->called);
+        $this->assertFalse($this->mockPlugins[5]->called);
         call_user_func_array($events['bar'], $barArgs);
-        $this->assertTrue($this->mockPlugins[4]->called);
-        $this->assertSame($barArgs, $this->mockPlugins[4]->args);
+        $this->assertTrue($this->mockPlugins[5]->called);
+        $this->assertSame($barArgs, $this->mockPlugins[5]->args);
     }
 
     /**
diff --git a/tests/UserFilterTest.php b/tests/UserFilterTest.php
@@ -32,7 +32,12 @@ public function dataProviderFilter()
         $data = array();
 
         // Not an instance of UserEventInterface
-        $data[] = array(Phake::mock('\Phergie\Irc\Event\EventInterface'), true);
+        $data[] = array(Phake::mock('\Phergie\Irc\Event\EventInterface'), null);
+
+        // Not originating from a user
+        $event = Phake::mock('\Phergie\Irc\Event\UserEventInterface');
+        Phake::when($event)->getNick()->thenReturn(null);
+        $data[] = array($event, null);
 
         // Non-matching user mask
         $event = $this->getMockUserEvent('nick3', 'user3', 'host3');
diff --git a/tests/UserModeFilterTest.php b/tests/UserModeFilterTest.php
@@ -35,7 +35,18 @@ public function dataProviderFilter()
 
         // Not an instance of UserEventInterface
         $userMode = $this->getMockUserModePlugin(array());
-        $data[] = array($userMode, Phake::mock('\Phergie\Irc\Event\EventInterface'), true);
+        $data[] = array($userMode, Phake::mock('\Phergie\Irc\Event\EventInterface'), null);
+
+        // Not originating from a user
+        $event = $this->getMockUserEvent();
+        Phake::when($event)->getNick()->thenReturn(null);
+        $data[] = array($userMode, $event, null);
+
+        // Not originating within a channel
+        $event = $this->getMockUserEvent();
+        Phake::when($event)->getCommand()->thenReturn('QUIT');
+        Phake::when($event)->getParams()->thenReturn([]);
+        $data[] = array($userMode, $event, null);
 
         // User does not have needed modes
         $event = $this->getMockUserEvent();

Original file line number	Diff line number	Diff line change
`@@ -24,16 +24,21 @@ class AndFilter extends CompositeFilter`
`24`	`24`	`* Filters events that pass all contained filters.`
`25`	`25`	`*`
`26`	`26`	`* @param \Phergie\Irc\Event\EventInterface $event`
`27`		`- * @return boolean TRUE if the event passes all contained filters, FALSE`
`28`		`- * otherwise`
	`27`	`+ * @return boolean\|null TRUE if the event passes all contained filters, FALSE`
	`28`	`+ * if it fails any filter, or NULL if all filters return NULL.`
`29`	`29`	`*/`
`30`	`30`	`public function filter(EventInterface $event)`
`31`	`31`	`{`
	`32`	`+ $output = null;`
`32`	`33`	`foreach ($this->filters as $filter) {`
`33`		`- if (!$filter->filter($event)) {`
	`34`	`+ $result = $filter->filter($event);`
	`35`	`+ if ($result === false) {`
`34`	`36`	`return false;`
`35`	`37`	`}`
	`38`	`+ if ($result === true) {`
	`39`	`+ $output = true;`
	`40`	`+ }`
`36`	`41`	`}`
`37`		`- return true;`
	`42`	`+ return $output;`
`38`	`43`	`}`
`39`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,9 @@ interface FilterInterface`
`24`	`24`	`* Evaluates an event for forwarding.`
`25`	`25`	`*`
`26`	`26`	`* @param \Phergie\Irc\Event\EventInterface $event`
`27`		`- * @return boolean TRUE if the event should be forwarded, FALSE otherwise`
	`27`	`+ * @return boolean\|null TRUE if the event should be allowed, FALSE if it should be rejected,`
	`28`	`+ * or NULL if it should be ignored (ie. the event does not fall under the scope of`
	`29`	`+ * this filter)`
`28`	`30`	`*/`
`29`	`31`	`public function filter(EventInterface $event);`
`30`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,10 +41,15 @@ public function __construct(FilterInterface $filter)`
`41`	`41`	`* Filters events that do not pass the contained filter.`
`42`	`42`	`*`
`43`	`43`	`* @param \Phergie\Irc\Event\EventInterface $event`
`44`		`- * @return boolean TRUE if the contained filter fails, FALSE if it passes`
	`44`	`+ * @return boolean\|null TRUE if the contained filter fails, FALSE if it passes,`
	`45`	`+ * or NULL if it returns NULL.`
`45`	`46`	`*/`
`46`	`47`	`public function filter(EventInterface $event)`
`47`	`48`	`{`
`48`		`- return !$this->filter->filter($event);`
	`49`	`+ $result = $this->filter->filter($event);`
	`50`	`+ if ($result === null) {`
	`51`	`+ return null;`
	`52`	`+ }`
	`53`	`+ return !$result;`
`49`	`54`	`}`
`50`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,16 +24,21 @@ class OrFilter extends CompositeFilter`
`24`	`24`	`* Filters events that pass any contained filters.`
`25`	`25`	`*`
`26`	`26`	`* @param \Phergie\Irc\Event\EventInterface $event`
`27`		`- * @return boolean TRUE if the event passes any contained filters, FALSE`
`28`		`- * otherwise`
	`27`	`+ * @return boolean\|null TRUE if the event passes any contained filters, FALSE`
	`28`	`+ * if it fails all filters, or NULL if all filters return NULL.`
`29`	`29`	`*/`
`30`	`30`	`public function filter(EventInterface $event)`
`31`	`31`	`{`
	`32`	`+ $output = null;`
`32`	`33`	`foreach ($this->filters as $filter) {`
`33`		`- if ($filter->filter($event)) {`
	`34`	`+ $result = $filter->filter($event);`
	`35`	`+ if ($result === true) {`
`34`	`36`	`return true;`
`35`	`37`	`}`
	`38`	`+ if ($result === false) {`
	`39`	`+ $output = false;`
	`40`	`+ }`
`36`	`41`	`}`
`37`		`- return false;`
	`42`	`+ return $output;`
`38`	`43`	`}`
`39`	`44`	`}`