Merge pull request #5094 from tractorcow/pulls/4.0/invokewithextensions

API Extensible::invokeWithExtension has same method signature as Extensible::extend

core/Extensible.php

@@ -48,7 +48,7 @@ trait Extensible {
 	private static $unextendable_classes = array('Object', 'ViewableData', 'RequestHandler');
 
 	/**
-	 * @var array all current extension instances.
+	 * @var Extension[] all current extension instances.
 	 */
 	protected $extension_instances = array();
 
@@ -56,7 +56,9 @@ trait Extensible {
 	 * List of callbacks to call prior to extensions having extend called on them,
 	 * each grouped by methodName.
 	 *
-	 * @var array[callable]
+	 * Top level array is method names, each of which is an array of callbacks for that name.
+	 *
+	 * @var callable[][]
 	 */
 	protected $beforeExtendCallbacks = array();
 
@@ -64,7 +66,9 @@ trait Extensible {
 	 * List of callbacks to call after extensions having extend called on them,
 	 * each grouped by methodName.
 	 *
-	 * @var array[callable]
+	 * Top level array is method names, each of which is an array of callbacks for that name.
+	 *
+	 * @var callable[][]
 	 */
 	protected $afterExtendCallbacks = array();
 
@@ -364,13 +368,24 @@ trait Extensible {
 	 * all results into an array
 	 *
 	 * @param string $method the method name to call
-	 * @param mixed $argument a single argument to pass
-	 * @return mixed
-	 * @todo integrate inheritance rules
+	 * @param mixed $a1
+	 * @param mixed $a2
+	 * @param mixed $a3
+	 * @param mixed $a4
+	 * @param mixed $a5
+	 * @param mixed $a6
+	 * @param mixed $a7
+	 * @return array List of results with nulls filtered out
 	 */
-	public function invokeWithExtensions($method, $argument = null) {
-		$result = method_exists($this, $method) ? array($this->$method($argument)) : array();
-		$extras = $this->extend($method, $argument);
+	public function invokeWithExtensions($method, &$a1=null, &$a2=null, &$a3=null, &$a4=null, &$a5=null, &$a6=null, &$a7=null) {
+		$result = array();
+		if(method_exists($this, $method)) {
+			$thisResult = $this->$method($a1, $a2, $a3, $a4, $a5, $a6, $a7);
+			if($thisResult !== null) {
+				$result[] = $thisResult;
+			}
+		}
+		$extras = $this->extend($method, $a1, $a2, $a3, $a4, $a5, $a6, $a7);
 
 		return $extras ? array_merge($result, $extras) : $result;
 	}
@@ -387,7 +402,13 @@ trait Extensible {
 	 * The extension methods are defined during {@link __construct()} in {@link defineMethods()}.
 	 *
 	 * @param string $method the name of the method to call on each extension
-	 * @param mixed $a1,... up to 7 arguments to be passed to the method
+	 * @param mixed $a1
+	 * @param mixed $a2
+	 * @param mixed $a3
+	 * @param mixed $a4
+	 * @param mixed $a5
+	 * @param mixed $a6
+	 * @param mixed $a7
 	 * @return array
 	 */
 	public function extend($method, &$a1=null, &$a2=null, &$a3=null, &$a4=null, &$a5=null, &$a6=null, &$a7=null) {

dev/SapphireTest.php

@@ -395,8 +395,8 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	/**
 	 * Get the ID of an object from the fixture.
 	 *
-	 * @param $className The data class, as specified in your fixture file.  Parent classes won't work
-	 * @param $identifier The identifier string, as provided in your fixture file
+	 * @param string $className The data class, as specified in your fixture file.  Parent classes won't work
+	 * @param string $identifier The identifier string, as provided in your fixture file
 	 * @return int
 	 */
 	protected function idFromFixture($className, $identifier) {
@@ -652,10 +652,6 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * Assert that the given {@link SS_List} includes only DataObjects matching the given
 	 * key-value pairs.  Each match must correspond to 1 distinct record.
 	 *
-	 * @param $matches The patterns to match.  Each pattern is a map of key-value pairs.  You can
-	 * either pass a single pattern or an array of patterns.
-	 * @param $dataObjectSet The {@link SS_List} to test.
-	 *
 	 * Example
 	 * --------
 	 * Check that *only* the entries Sam Minnee and Ingo Schommer exist in $members.  Order doesn't
@@ -664,9 +660,13 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 *        array('FirstName' =>'Sam', 'Surname' => 'Minnee'),
 	 *        array('FirstName' => 'Ingo', 'Surname' => 'Schommer'),
 	 *      ), $members);
+	 *
+	 * @param mixed $matches The patterns to match.  Each pattern is a map of key-value pairs.  You can
+	 * either pass a single pattern or an array of patterns.
+	 * @param mixed $dataObjectSet The {@link SS_List} to test.
 	 */
 	public function assertDOSEquals($matches, $dataObjectSet) {
-		if(!$dataObjectSet) return false;
+		if(!$dataObjectSet) return;
 
 		$extracted = array();
 		foreach($dataObjectSet as $item) $extracted[] = $item->toMap();
@@ -704,13 +704,13 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * Assert that the every record in the given {@link SS_List} matches the given key-value
 	 * pairs.
 	 *
-	 * @param $match The pattern to match.  The pattern is a map of key-value pairs.
-	 * @param $dataObjectSet The {@link SS_List} to test.
-	 *
 	 * Example
 	 * --------
 	 * Check that every entry in $members has a Status of 'Active':
 	 *     $this->assertDOSAllMatch(array('Status' => 'Active'), $members);
+	 *
+	 * @param mixed $match The pattern to match.  The pattern is a map of key-value pairs.
+	 * @param mixed $dataObjectSet The {@link SS_List} to test.
 	 */
 	public function assertDOSAllMatch($match, $dataObjectSet) {
 		$extracted = array();

docs/en/04_Changelogs/4.0.0.md

@@ -33,6 +33,7 @@
    Please remove any PHPUnit related `require_once()` calls (e.g. in `FeatureContext` 
    definitions of the [behat-extension](https://github.com/silverstripe-labs/silverstripe-behat-extension) module).
    Run `composer require --dev 'phpunit/phpunit:~4.8' on existing projects to pull in the new dependency.
+ * `Object::invokeWithExtensions` now has the same method signature as `Object::extend` and behaves the same way.
 
 ## New API
 

tests/core/ObjectTest.php

@@ -353,11 +353,18 @@ class ObjectTest extends SapphireTest {
 		$this->assertEquals($object->extend('extendableMethod', $argument), array('ExtendTest2(modified)'));
 		$this->assertEquals($argument, 'modified');
 
-		$this->assertEquals($object->invokeWithExtensions('extendableMethod'), array('ExtendTest()', 'ExtendTest2()'));
-		$this->assertEquals (
-			$object->invokeWithExtensions('extendableMethod', 'test'),
-			array('ExtendTest(test)', 'ExtendTest2(modified)')
+		$this->assertEquals(
+			array('ExtendTest()', 'ExtendTest2()'),
+			$object->invokeWithExtensions('extendableMethod')
 		);
+		$arg1 = 'test';
+		$arg2 = 'bob';
+		$this->assertEquals (
+			array('ExtendTest(test,bob)', 'ExtendTest2(modified,objectmodified)'),
+			$object->invokeWithExtensions('extendableMethod', $arg1, $arg2)
+		);
+		$this->assertEquals('modified', $arg1);
+		$this->assertEquals('objectmodified', $arg2);
 
 		$object2 = new ObjectTest_Extending();
 		$first = 1;
@@ -566,7 +573,11 @@ class ObjectTest_CacheTest extends Object {
 
 class ObjectTest_ExtendTest extends Object {
 	private static $extensions = array('ObjectTest_ExtendTest1', 'ObjectTest_ExtendTest2');
-	public function extendableMethod($argument = null) { return "ExtendTest($argument)"; }
+	public function extendableMethod(&$argument = null, &$argument2 = null) {
+		$args = implode(',', array_filter(func_get_args()));
+		if($argument2) $argument2 = 'objectmodified';
+		return "ExtendTest($args)";
+	}
 }
 
 class ObjectTest_ExtendTest1 extends Extension {
@@ -577,7 +588,10 @@ class ObjectTest_ExtendTest1 extends Extension {
 }
 
 class ObjectTest_ExtendTest2 extends Extension {
-	public function extendableMethod($argument = null) { return "ExtendTest2($argument)"; }
+	public function extendableMethod($argument = null) {
+		$args = implode(',', array_filter(func_get_args()));
+		return "ExtendTest2($args)";
+	}
 }
 
 class ObjectTest_ExtendTest3 extends Extension {