From d5563db9247c78c3b4c4503021d437a6a90f0774 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Wed, 5 Nov 2014 14:29:43 -0800 Subject: [PATCH] Deprecate MWFunction::newObj() in favor of ObjectFactory Change-Id: Iaa803311409cf7b649f64f69bafe2935a418d31c --- includes/StubObject.php | 6 +- includes/libs/ObjectFactory.php | 23 +++++--- includes/specialpage/SpecialPageFactory.php | 6 +- includes/utils/MWFunction.php | 13 +++-- tests/phpunit/includes/MWFunctionTest.php | 1 + .../includes/libs/ObjectFactoryTest.php | 55 +++++++++++++++++++ 6 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 tests/phpunit/includes/libs/ObjectFactoryTest.php diff --git a/includes/StubObject.php b/includes/StubObject.php index 8878660b73..7f12c1689d 100644 --- a/includes/StubObject.php +++ b/includes/StubObject.php @@ -110,7 +110,11 @@ class StubObject { * @return object */ public function _newObject() { - return MWFunction::newObj( $this->class, $this->params ); + return ObjectFactory::getObjectFromSpec( array( + 'class' => $this->class, + 'args' => $this->params, + 'closure_expansion' => false, + ) ); } /** diff --git a/includes/libs/ObjectFactory.php b/includes/libs/ObjectFactory.php index ee696c3aa2..73e76f7129 100644 --- a/includes/libs/ObjectFactory.php +++ b/includes/libs/ObjectFactory.php @@ -47,7 +47,8 @@ class ObjectFactory { * expanded by invoking them with no arguments before passing the * resulting value on to the constructor/callable. This can be used to * pass DatabaseBase instances or other live objects to the - * constructor/callable. + * constructor/callable. This behavior can be suppressed by adding + * closure_expansion => false to the specification. * * @param array $spec Object specification * @return object @@ -59,14 +60,18 @@ class ObjectFactory { public static function getObjectFromSpec( $spec ) { $args = isset( $spec['args'] ) ? $spec['args'] : array(); - $args = array_map( function ( $value ) { - if ( is_object( $value ) && $value instanceof Closure ) { - // If an argument is a Closure, call it. - return $value(); - } else { - return $value; - } - }, $args ); + if ( !isset( $spec['closure_expansion'] ) || + $spec['closure_expansion'] === true + ) { + $args = array_map( function ( $value ) { + if ( is_object( $value ) && $value instanceof Closure ) { + // If an argument is a Closure, call it. + return $value(); + } else { + return $value; + } + }, $args ); + } if ( isset( $spec['class'] ) ) { $clazz = $spec['class']; diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index b110bdabef..1531f69771 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -414,7 +414,11 @@ class SpecialPageFactory { // @deprecated, officially since 1.18, unofficially since forever wfDeprecated( "Array syntax for \$wgSpecialPages is deprecated ($className), " . "define a subclass of SpecialPage instead.", '1.18' ); - $page = MWFunction::newObj( $className, $rec ); + $page = ObjectFactory::getObjectFromSpec( array( + 'class' => $className, + 'args' => $rec, + 'closure_expansion' => false, + ) ); } elseif ( $rec instanceof SpecialPage ) { $page = $rec; //XXX: we should deep clone here } else { diff --git a/includes/utils/MWFunction.php b/includes/utils/MWFunction.php index 9fb4c198e5..fa7eebe825 100644 --- a/includes/utils/MWFunction.php +++ b/includes/utils/MWFunction.php @@ -26,14 +26,15 @@ class MWFunction { * @param string $class * @param array $args * @return object + * @deprecated 1.25 Use ObjectFactory::getObjectFromSpec() instead */ public static function newObj( $class, $args = array() ) { - if ( !count( $args ) ) { - return new $class; - } + wfDeprecated( __METHOD__, '1.25' ); - $ref = new ReflectionClass( $class ); - - return $ref->newInstanceArgs( $args ); + return ObjectFactory::getObjectFromSpec( array( + 'class' => $class, + 'args' => $args, + 'closure_expansion' => false, + ) ); } } diff --git a/tests/phpunit/includes/MWFunctionTest.php b/tests/phpunit/includes/MWFunctionTest.php index f2a720e829..f4d17999a0 100644 --- a/tests/phpunit/includes/MWFunctionTest.php +++ b/tests/phpunit/includes/MWFunctionTest.php @@ -13,6 +13,7 @@ class MWFunctionTest extends MediaWikiTestCase { $args = array( $arg1, $arg2, $arg3, $arg4 ); $newObject = new MWBlankClass( $arg1, $arg2, $arg3, $arg4 ); + $this->hideDeprecated( 'MWFunction::newObj' ); $this->assertEquals( MWFunction::newObj( 'MWBlankClass', $args )->args, $newObject->args diff --git a/tests/phpunit/includes/libs/ObjectFactoryTest.php b/tests/phpunit/includes/libs/ObjectFactoryTest.php new file mode 100644 index 0000000000..1f88b12cf5 --- /dev/null +++ b/tests/phpunit/includes/libs/ObjectFactoryTest.php @@ -0,0 +1,55 @@ + 'ObjectFactoryTest_Fixture', + 'args' => array( function (){ return 'unwrapped'; }, ), + 'closure_expansion' => false, + ) ); + $this->assertInstanceOf( 'Closure', $obj->args[0] ); + $this->assertSame( 'unwrapped', $obj->args[0]() ); + } + + public function testClosureExpansionEnabled() { + $obj = ObjectFactory::getObjectFromSpec( array( + 'class' => 'ObjectFactoryTest_Fixture', + 'args' => array( function (){ return 'unwrapped'; }, ), + 'closure_expansion' => true, + ) ); + $this->assertInternalType( 'string', $obj->args[0] ); + $this->assertSame( 'unwrapped', $obj->args[0] ); + + $obj = ObjectFactory::getObjectFromSpec( array( + 'class' => 'ObjectFactoryTest_Fixture', + 'args' => array( function (){ return 'unwrapped'; }, ), + ) ); + $this->assertInternalType( 'string', $obj->args[0] ); + $this->assertSame( 'unwrapped', $obj->args[0] ); + } +} + +class ObjectFactoryTest_Fixture { + public $args; + public function __construct( /*...*/ ) { $this->args = func_get_args(); } +} -- 2.20.1