Introduce 'clone' flag for newFromLinkTarget.
authordaniel <dkinzler@wikimedia.org>
Wed, 3 Apr 2019 08:47:57 +0000 (10:47 +0200)
committerdaniel <dkinzler@wikimedia.org>
Fri, 5 Apr 2019 14:52:06 +0000 (16:52 +0200)
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
tests/phpunit/includes/TitleTest.php

index 0c62c4f..256ddaa 100644 (file)
@@ -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() {
index 35b196a..03802a8 100644 (file)
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -564,6 +565,32 @@ class TitleTest extends MediaWikiTestCase {
                $this->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' ],