From 2d721e69e885fb7ee6afbe9bd87d30972a00efd3 Mon Sep 17 00:00:00 2001 From: daniel Date: Wed, 3 Apr 2019 10:47:57 +0200 Subject: [PATCH] Introduce 'clone' flag for newFromLinkTarget. The "new" part of some of the newXXX pseudo-constructors in the Title class is a lie: these methods do not always return a new instance. This patch ensures that this fact is documented, and we have a safe way to get a fresh Title from a LinkTarget. Needed by I94479b44afb30 Change-Id: I4d561ef7d7447d3d6e35079cf656bd564882d25e --- includes/Title.php | 43 ++++++++++++++++++++++++---- tests/phpunit/includes/TitleTest.php | 27 +++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/includes/Title.php b/includes/Title.php index 0c62c4ff3f..256ddaa92f 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -53,6 +53,13 @@ class Title implements LinkTarget, IDBAccessObject { */ const GAID_FOR_UPDATE = 1; + /** + * Flag for use with factory methods like newFromLinkTarget() that have + * a $forceClone parameter. If set, the method must return a new instance. + * Without this flag, some factory methods may return existing instances. + */ + const NEW_CLONE = 'clone'; + /** * @name Private member variables * Please use the accessor functions instead. @@ -231,27 +238,39 @@ class Title implements LinkTarget, IDBAccessObject { } /** - * Create a new Title from a TitleValue + * Returns a Title given a TitleValue. + * If the given TitleValue is already a Title instance, that instance is returned, + * unless $forceClone is "clone". If $forceClone is "clone" and the given TitleValue + * is already a Title instance, that instance is copied using the clone operator. * * @param TitleValue $titleValue Assumed to be safe. + * @param string $forceClone set to NEW_CLONE to ensure a fresh instance is returned. * * @return Title */ - public static function newFromTitleValue( TitleValue $titleValue ) { - return self::newFromLinkTarget( $titleValue ); + public static function newFromTitleValue( TitleValue $titleValue, $forceClone = '' ) { + return self::newFromLinkTarget( $titleValue, $forceClone ); } /** - * Create a new Title from a LinkTarget + * Returns a Title given a LinkTarget. + * If the given LinkTarget is already a Title instance, that instance is returned, + * unless $forceClone is "clone". If $forceClone is "clone" and the given LinkTarget + * is already a Title instance, that instance is copied using the clone operator. * * @param LinkTarget $linkTarget Assumed to be safe. + * @param string $forceClone set to NEW_CLONE to ensure a fresh instance is returned. * * @return Title */ - public static function newFromLinkTarget( LinkTarget $linkTarget ) { + public static function newFromLinkTarget( LinkTarget $linkTarget, $forceClone = '' ) { if ( $linkTarget instanceof Title ) { // Special case if it's already a Title object - return $linkTarget; + if ( $forceClone === self::NEW_CLONE ) { + return clone $linkTarget; + } else { + return $linkTarget; + } } return self::makeTitle( $linkTarget->getNamespace(), @@ -268,6 +287,10 @@ class Title implements LinkTarget, IDBAccessObject { * Title objects returned by this method are guaranteed to be valid, and * thus return true from the isValid() method. * + * @note The Title instance returned by this method is not guaranteed to be a fresh instance. + * It may instead be a cached instance created previously, with references to it remaining + * elsewhere. + * * @param string|int|null $text The link text; spaces, prefixes, and an * initial ':' indicating the main namespace are accepted. * @param int $defaultNamespace The namespace to use if none is specified @@ -302,6 +325,10 @@ class Title implements LinkTarget, IDBAccessObject { * Title objects returned by this method are guaranteed to be valid, and * thus return true from the isValid() method. * + * @note The Title instance returned by this method is not guaranteed to be a fresh instance. + * It may instead be a cached instance created previously, with references to it remaining + * elsewhere. + * * @see Title::newFromText * * @since 1.25 @@ -593,6 +620,10 @@ class Title implements LinkTarget, IDBAccessObject { /** * Create a new Title for the Main Page * + * @note The Title instance returned by this method is not guaranteed to be a fresh instance. + * It may instead be a cached instance created previously, with references to it remaining + * elsewhere. + * * @return Title The new object */ public static function newMainPage() { diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index 35b196a9ba..03802a85ec 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -1,5 +1,6 @@ assertEquals( $value->getFragment(), $title->getFragment() ); } + /** + * @covers Title::newFromLinkTarget + * @dataProvider provideNewFromTitleValue + */ + public function testNewFromLinkTarget( LinkTarget $value ) { + $title = Title::newFromLinkTarget( $value ); + + $dbkey = str_replace( ' ', '_', $value->getText() ); + $this->assertEquals( $dbkey, $title->getDBkey() ); + $this->assertEquals( $value->getNamespace(), $title->getNamespace() ); + $this->assertEquals( $value->getFragment(), $title->getFragment() ); + } + + /** + * @covers Title::newFromLinkTarget + */ + public function testNewFromLinkTarget_clone() { + $title = Title::newFromText( __METHOD__ ); + $this->assertSame( $title, Title::newFromLinkTarget( $title ) ); + + // The Title::NEW_CLONE flag should ensure that a fresh instance is returned. + $clone = Title::newFromLinkTarget( $title, Title::NEW_CLONE ); + $this->assertNotSame( $title, $clone ); + $this->assertTrue( $clone->equals( $title ) ); + } + public static function provideGetTitleValue() { return [ [ 'Foo' ], -- 2.20.1