feature(events): allows dynamic method callbacks to be unregistered

This adds a class for matching dynamic method callbacks using static
method syntax and uses it to allow unregistering dynamic method callbacks
from events/hooks.

Fixes #7750

docs/design/events.rst

@@ -235,3 +235,30 @@ Parameters:
 -  **$value** The initial value of the plugin hook.
 
 .. warning:: The `$params` and `$value` arguments are reversed between the plugin hook handlers and trigger functions!
+
+
+Unregister Event/Hook Handlers
+------------------------------
+
+The functions ``elgg_unregister_event_handler`` and ``elgg_unregister_plugin_hook_handler`` can be used to remove
+handlers already registered by another plugin or Elgg core. The parameters are in the same order as the registration
+functions, except there's no priority parameter.
+
+.. code:: php
+
+    elgg_unregister_event_handler('login', 'user', 'myPlugin_handle_login');
+
+Anonymous functions or invokable objects cannot be unregistered, but dynamic method callbacks can be unregistered
+by giving the static version of the callback:
+
+.. code:: php
+
+    $obj = new MyPlugin\Handlers();
+    elgg_register_plugin_hook_handler('foo', 'bar', [$obj, 'handleFoo']);
+
+    // ... elsewhere
+
+    elgg_unregister_plugin_hook_handler('foo', 'bar', 'MyPlugin\Handlers::handleFoo');
+
+Even though the event handler references a dynamic method call, the code above will successfully
+remove the handler.

engine/classes/Elgg/HooksRegistrationService.php

@@ -13,7 +13,7 @@
  * @since      1.9.0
  */
 abstract class HooksRegistrationService {
-	
+
 	private $handlers = array();
 
 	/**
@@ -78,11 +78,21 @@ public function registerHandler($name, $type, $callback, $priority = 500) {
 	 */
 	public function unregisterHandler($name, $type, $callback) {
 		if (isset($this->handlers[$name]) && isset($this->handlers[$name][$type])) {
-			foreach ($this->handlers[$name][$type] as $key => $name_callback) {
-				if ($name_callback == $callback) {
-					unset($this->handlers[$name][$type][$key]);
-					return true;
+			$matcher = $this->getMatcher($callback);
+
+			foreach ($this->handlers[$name][$type] as $key => $handler) {
+				if ($matcher) {
+					if (!$matcher->matches($handler)) {
+						continue;
+					}
+				} else {
+					if ($handler != $callback) {
+						continue;
+					}
 				}
+
+				unset($this->handlers[$name][$type][$key]);
+				return true;
 			}
 		}
 
@@ -149,4 +159,32 @@ public function getOrderedHandlers($name, $type) {
 
 		return $handlers;
 	}
+
+	/**
+	 * Create a matcher for the given callable (if it's for a static or dynamic method)
+	 *
+	 * @param callable $spec Callable we're creating a matcher for
+	 *
+	 * @return MethodMatcher|null
+	 */
+	protected function getMatcher($spec) {
+		if (is_string($spec) && false !== strpos($spec, '::')) {
+			list ($type, $method) = explode('::', $spec, 2);
+			return new MethodMatcher($type, $method);
+		}
+
+		if (!is_array($spec) || empty($spec[0]) || empty($spec[1]) || !is_string($spec[1])) {
+			return null;
+		}
+
+		if (is_object($spec[0])) {
+			$spec[0] = get_class($spec[0]);
+		}
+
+		if (!is_string($spec[0])) {
+			return null;
+		}
+
+		return new MethodMatcher($spec[0], $spec[1]);
+	}
 }

engine/classes/Elgg/MethodMatcher.php

@@ -0,0 +1,70 @@
+<?php
+namespace Elgg;
+
+/**
+ * Identify a static/dynamic method callable, even if contains an object to which you don't have a reference.
+ *
+ * @access private
+ * @since 1.11.0
+ */
+class MethodMatcher {
+
+	/**
+	 * @var string
+	 */
+	private $type;
+
+	/**
+	 * @var string
+	 */
+	private $method;
+
+	/**
+	 * Constructor
+	 *
+	 * @param string $type   Class to match
+	 * @param string $method Method name to match
+	 */
+	public function __construct($type, $method) {
+		$this->type = strtolower(ltrim($type, '\\'));
+		$this->method = strtolower($method);
+	}
+
+	/**
+	 * Does the given callable match the specification?
+	 *
+	 * @param callable $subject Callable to test
+	 * @return bool
+	 */
+	public function matches($subject) {
+		// We don't use the callable type-hint because it unnecessarily autoloads for static methods.
+
+		if (is_string($subject)) {
+			if (false === strpos($subject, '::')) {
+				return false;
+			}
+
+			$subject = explode('::', $subject, 2);
+		}
+
+		if (!is_array($subject) || empty($subject[0]) || empty($subject[1]) || !is_string($subject[1])) {
+			return false;
+		}
+
+		if (strtolower($subject[1]) !== $this->method) {
+			return false;
+		}
+
+		if (is_object($subject[0])) {
+			$subject[0] = get_class($subject[0]);
+		}
+
+		if (!is_string($subject[0])) {
+			return false;
+		}
+
+		return (strtolower(ltrim($subject[0], '\\')) === $this->type);
+	}
+
+
+}

engine/lib/elgglib.php

@@ -525,7 +525,7 @@ function elgg_register_event_handler($event, $object_type, $callback, $priority
  *
  * @param string $event       The event type
  * @param string $object_type The object type
- * @param string $callback    The callback
+ * @param string $callback    The callback. Since 1.11, static method callbacks will match dynamic methods
  *
  * @return bool true if a handler was found and removed
  * @since 1.7
@@ -709,7 +709,8 @@ function elgg_register_plugin_hook_handler($hook, $type, $callback, $priority =
  *
  * @param string   $hook        The name of the hook
  * @param string   $entity_type The name of the type of entity (eg "user", "object" etc)
- * @param callable $callback    The PHP callback to be removed
+ * @param callable $callback    The PHP callback to be removed. Since 1.11, static method
+ *                              callbacks will match dynamic methods
  *
  * @return void
  * @since 1.8.0

engine/tests/phpunit/Elgg/HooksRegistrationServiceTest.php

@@ -15,15 +15,17 @@ public function setUp() {
 	}
 
 	public function testCanRegisterHandlers() {
+		$f = function () {};
+
 		$this->assertTrue($this->mock->registerHandler('foo', 'bar', 'callback1'));
-		$this->assertTrue($this->mock->registerHandler('foo', 'bar', 'callback2'));
+		$this->assertTrue($this->mock->registerHandler('foo', 'bar', $f));
 		$this->assertTrue($this->mock->registerHandler('foo', 'baz', 'callback3', 100));
 
 		$expected = array(
 			'foo' => array(
 				'bar' => array(
 					500 => 'callback1',
-					501 => 'callback2'
+					501 => $f,
 				),
 				'baz' => array(
 					100 => 'callback3'
@@ -38,12 +40,35 @@ public function testCanRegisterHandlers() {
 	}
 
 	public function testCanUnregisterHandlers() {
+		$o = new HooksRegistrationServiceTest_invokable();
+
 		$this->mock->registerHandler('foo', 'bar', 'callback1');
 		$this->mock->registerHandler('foo', 'bar', 'callback2', 100);
+		$this->mock->registerHandler('foo', 'bar', 'callback2', 150);
+		$this->mock->registerHandler('foo', 'bar', [$o, '__invoke'], 300);
+		$this->mock->registerHandler('foo', 'bar', [$o, '__invoke'], 300);
+		$this->mock->registerHandler('foo', 'bar', [$o, '__invoke'], 300);
+
+		$this->assertTrue($this->mock->unregisterHandler(
+			'foo', 'bar', 'callback2'));
+		$this->assertTrue($this->mock->unregisterHandler(
+			'foo', 'bar', HooksRegistrationServiceTest_invokable::KLASS . '::__invoke'));
+		$this->assertTrue($this->mock->unregisterHandler(
+			'foo', 'bar', [HooksRegistrationServiceTest_invokable::KLASS, '__invoke']));
+		$this->assertTrue($this->mock->unregisterHandler(
+			'foo', 'bar', [$o, '__invoke']));
 
-		$this->assertTrue($this->mock->unregisterHandler('foo', 'bar', 'callback2'));
+		$expected = array(
+			'foo' => array(
+				'bar' => array(
+					// only one removed
+					150 => 'callback2',
 
-		$this->assertSame([500 => 'callback1'], $this->mock->getAllHandlers()['foo']['bar']);
+					500 => 'callback1',
+				)
+			)
+		);
+		$this->assertSame($expected, $this->mock->getAllHandlers());
 
 		// check unregistering things that aren't registered
 		$this->assertFalse($this->mock->unregisterHandler('foo', 'bar', 'not_valid'));
@@ -76,3 +101,8 @@ public function testGetOrderedHandlers() {
 		$this->assertSame($expected_foo_baz, $this->mock->getOrderedHandlers('foo', 'baz'));
 	}
 }
+
+class HooksRegistrationServiceTest_invokable {
+	const KLASS = __CLASS__;
+	function __invoke() {}
+}

engine/tests/phpunit/Elgg/MethodMatcherTest.php

@@ -0,0 +1,32 @@
+<?php
+namespace Elgg;
+
+class MethodMatcherTest extends \PHPUnit_Framework_TestCase {
+
+	public function testMatchesStrings() {
+		$matcher = new MethodMatcher('stdClass', 'bar');
+
+		$this->assertTrue($matcher->matches('stdClass::bar'));
+		$this->assertTrue($matcher->matches('\STDClass::BAR'));
+		$this->assertFalse($matcher->matches('foooo::bar'));
+		$this->assertFalse($matcher->matches('foo\bar'));
+	}
+
+	public function testMatchesStaticArrays() {
+		$matcher = new MethodMatcher('stdClass', 'bar');
+
+		$this->assertTrue($matcher->matches(['stdClass', 'bar']));
+		$this->assertTrue($matcher->matches(['\STDClass', 'BAR']));
+		$this->assertFalse($matcher->matches(['foooo', 'bar']));
+	}
+
+	public function testMatchesDynamicArrays() {
+		$matcher = new MethodMatcher('stdClass', 'bar');
+
+		$this->assertTrue($matcher->matches([new \stdClass(), 'bar']));
+		$this->assertTrue($matcher->matches([new \stdClass(), 'BAR']));
+		$this->assertFalse($matcher->matches([new MethodMatcherTestObject, 'bar']));
+	}
+}
+
+class MethodMatcherTestObject {}