Revision::getContent must return clone if mutable

Revision::getContent must return a cloned instance of the Content object
if the Content object is mutable to avoid confusion.

Content::copy is used to achieve this, which is specified to return $this
for immutable Content.

Change-Id: Iace17b6ae8aa85a3500624441b69bc067c1ade00

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 9c18ced..6b3b27a 100644 (file)
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -293,6 +293,7 @@ $wgAutoloadLocalClasses = array(
        'AbstractContent' => 'includes/Content.php',
        'ContentHandler' => 'includes/ContentHandler.php',
        'CssContent' => 'includes/Content.php',
+       'TextContentHandler' => 'includes/ContentHandler.php',
        'CssContentHandler' => 'includes/ContentHandler.php',
        'JavaScriptContent' => 'includes/Content.php',
        'JavaScriptContentHandler' => 'includes/ContentHandler.php',

diff --git a/includes/Revision.php b/includes/Revision.php
index 30ffbea..d5d1e0b 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -970,7 +970,7 @@ class Revision implements IDBAccessObject {
                        $this->mContent = is_null( $this->mText ) ? null : $handler->unserializeContent( $this->mText, $format );
                }
 
-               return $this->mContent;
+               return $this->mContent->copy(); // NOTE: copy() will return $this for immutable content objects
        }
 
        /**

diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index f9a400c..5a98355 100644 (file)
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -31,6 +31,7 @@ class RevisionTest extends MediaWikiTestCase {
 
                $wgNamespaceContentModels[ 12312 ] = "testing";
                $wgContentHandlers[ "testing" ] = 'DummyContentHandlerForTesting';
+               $wgContentHandlers[ "RevisionTestModifyableContent" ] = 'RevisionTestModifyableContentHandler';
 
                MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache
                $wgContLang->resetNamespaces(); # reset namespace cache
@@ -346,6 +347,87 @@ class RevisionTest extends MediaWikiTestCase {
                $this->assertEquals( CONTENT_MODEL_JAVASCRIPT, $rev->getContentModel() );
        }
 
+       /**
+        * Tests whether $rev->getContent() returns a clone when needed.
+        *
+        * @group Database
+        */
+       function testGetContentClone( ) {
+               $content = new RevisionTestModifyableContent( "foo" );
+
+               $rev = new Revision(
+                       array(
+                               'id'         => 42,
+                               'page'       => 23,
+                               'title'      => Title::newFromText( "testGetContentClone_dummy" ),
+
+                               'content'    => $content,
+                               'length'     => $content->getSize(),
+                               'comment'    => "testing",
+                               'minor_edit' => false,
+                       )
+               );
+
+               $content = $rev->getContent( Revision::RAW );
+               $content->setText( "bar" );
+
+               $content2 = $rev->getContent( Revision::RAW );
+               $this->assertNotSame( $content, $content2, "expected a clone" ); // content is mutable, expect clone
+               $this->assertEquals( "foo", $content2->getText() ); // clone should contain the original text
+
+               $content2->setText( "bla bla" );
+               $this->assertEquals( "bar", $content->getText() ); // clones should be independent
+       }
+
+
+       /**
+        * Tests whether $rev->getContent() returns the same object repeatedly if appropriate.
+        *
+        * @group Database
+        */
+       function testGetContentUncloned() {
+               $rev = $this->newTestRevision( "hello", "testGetContentUncloned_dummy", CONTENT_MODEL_WIKITEXT );
+               $content = $rev->getContent( Revision::RAW );
+               $content2 = $rev->getContent( Revision::RAW );
+
+               // for immutable content like wikitext, this should be the same object
+               $this->assertSame( $content, $content2 );
+       }
+
+}
+
+class RevisionTestModifyableContent extends TextContent {
+       public function __construct( $text ) {
+               parent::__construct( $text, "RevisionTestModifyableContent" );
+       }
+
+       public function copy( ) {
+               return new RevisionTestModifyableContent( $this->mText );
+       }
+
+       public function getText() {
+               return $this->mText;
+       }
+
+       public function setText( $text ) {
+               $this->mText = $text;
+       }
+
 }
 
+class RevisionTestModifyableContentHandler extends TextContentHandler {
+
+       public function __construct( ) {
+               parent::__construct( "RevisionTestModifyableContent", array( CONTENT_FORMAT_TEXT ) );
+       }
 
+       public function unserializeContent( $text, $format = null ) {
+               $this->checkFormat( $format );
+
+               return new RevisionTestModifyableContent( $text );
+       }
+
+       public function makeEmptyContent() {
+               return new RevisionTestModifyableContent( '' );
+       }
+}