Make EditPage fail on non-textual content.

Change-Id: Ida542340cab75de2540632c8116d8ebe074522cb

diff --git a/includes/EditPage.php b/includes/EditPage.php
index be815a2..5102f7a 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -239,6 +239,13 @@ class EditPage {
 
        public $suppressIntro = false;
 
+       /**
+        * Set to true to allow editing of non-text content types.
+        *
+        * @var bool
+        */
+       public $allowNonTextContent = false;
+
        /**
         * @param $article Article
         */
@@ -481,7 +488,7 @@ class EditPage {
                        $text = $this->textbox1;
                        $wgOut->addWikiMsg( 'viewyourtext' );
                } else {
-                       $text = $content->serialize( $this->content_format );
+                       $text = $this->toEditText( $content );
                        $wgOut->addWikiMsg( 'viewsourcetext' );
                }
 
@@ -768,7 +775,7 @@ class EditPage {
                $this->edittime = $this->mArticle->getTimestamp();
 
                $content = $this->getContentObject( false ); #TODO: track content object?!
-               $this->textbox1 = $content->serialize( $this->content_format );
+               $this->textbox1 = $this->toEditText( $content );
 
                // activate checkboxes if user wants them to be always active
                # Sort out the "watch" checkbox
@@ -805,7 +812,7 @@ class EditPage {
                wfDeprecated( __METHOD__, '1.WD' );
 
                if ( $def_text !== null && $def_text !== false && $def_text !== '' ) {
-                       $def_content = ContentHandler::makeContent( $def_text, $this->getTitle() );
+                       $def_content = $this->toEditContent( $def_text );
                } else {
                        $def_content = false;
                }
@@ -813,7 +820,7 @@ class EditPage {
                $content = $this->getContentObject( $def_content );
 
                // Note: EditPage should only be used with text based content anyway.
-               return $content->serialize( $this->content_format );
+               return $this->toEditText( $content );
        }
 
        private function getContentObject( $def_content = null ) {
@@ -830,7 +837,7 @@ class EditPage {
                                # If this is a system message, get the default text.
                                $msg = $this->mTitle->getDefaultMessageText();
 
-                               $content = ContentHandler::makeContent( $msg, $this->mTitle );
+                               $content = $this->toEditContent( $msg );
                        }
                        if ( $content === false ) {
                                # If requested, preload some text.
@@ -980,7 +987,7 @@ class EditPage {
        public function setPreloadedText( $text ) {
                wfDeprecated( __METHOD__, "1.WD" );
 
-               $content = ContentHandler::makeContent( $text, $this->getTitle() );
+               $content = $this->toEditContent( $text );
 
                $this->setPreloadedContent( $content );
        }
@@ -1010,7 +1017,7 @@ class EditPage {
                wfDeprecated( __METHOD__, "1.WD" );
 
                $content = $this->getPreloadedContent( $preload );
-               $text = $content->serialize( $this->content_format );
+               $text = $this->toEditText( $content );
 
                return $text;
        }
@@ -1203,8 +1210,7 @@ class EditPage {
 
                try {
                        # Construct Content object
-                       $textbox_content = ContentHandler::makeContent( $this->textbox1, $this->getTitle(),
-                                                                                                                       $this->content_model, $this->content_format );
+                       $textbox_content = $this->toEditContent( $this->textbox1 );
                } catch (MWContentSerializationException $ex) {
                        $status->fatal( 'content-failed-to-parse', $this->content_model, $this->content_format, $ex->getMessage() );
                        $status->value = self::AS_PARSE_ERROR;
@@ -1557,14 +1563,14 @@ class EditPage {
                        // merged the section into full text. Clear the section field
                        // so that later submission of conflict forms won't try to
                        // replace that into a duplicated mess.
-                       $this->textbox1 = $content->serialize( $this->content_format );
+                       $this->textbox1 = $this->toEditText( $content );
                        $this->section = '';
 
                        $status->value = self::AS_SUCCESS_UPDATE;
                }
 
                // Check for length errors again now that the section is merged in
-                       $this->kblength = (int)( strlen( $content->serialize( $this->content_format ) ) / 1024 );
+                       $this->kblength = (int)( strlen( $this->toEditText( $content ) ) / 1024 );
                if ( $this->kblength > $wgMaxArticleSize ) {
                        $this->tooBig = true;
                        $status->setResult( false, self::AS_MAX_ARTICLE_SIZE_EXCEEDED );
@@ -1660,13 +1666,12 @@ class EditPage {
        function mergeChangesInto( &$editText ){
                wfDebug( __METHOD__, "1.WD" );
 
-               $editContent = ContentHandler::makeContent( $editText, $this->getTitle(),
-                                                                                                       $this->content_model, $this->content_format );
+               $editContent = $this->toEditContent( $editText );
 
                $ok = $this->mergeChangesIntoContent( $editContent );
 
                if ( $ok ) {
-                       $editText = $editContent->serialize( $this->content_format );
+                       $editText = $this->toEditText( $editContent );
                        return true;
                } else {
                        return false;
@@ -1907,6 +1912,54 @@ class EditPage {
                }
        }
 
+       /**
+        * Gets an editable textual representation of the given Content object.
+        * The textual representation can be turned by into a Content object by the
+        * toEditContent() method.
+        *
+        * If the given Content object is not of a type that can be edited using the text base EditPage,
+        * an exception will be raised. Set $this->allowNonTextContent to true to allow editing of non-textual
+        * content.
+        *
+        * @param Content $content
+        * @return String the editable text form of the content.
+        *
+        * @throws MWException if $content is not an instance of TextContent and $this->allowNonTextContent is not true.
+        */
+       protected function toEditText( Content $content ) {
+               if ( !$this->allowNonTextContent && !( $content instanceof TextContent ) ) {
+                       throw new MWException( "This content model can not be edited as text: "
+                                                               . ContentHandler::getLocalizedName( $content->getModel() ) );
+               }
+
+               return $content->serialize( $this->content_format );
+       }
+
+       /**
+        * Turns the given text into a Content object by unserializing it.
+        *
+        * If the resulting Content object is not of a type that can be edited using the text base EditPage,
+        * an exception will be raised. Set $this->allowNonTextContent to true to allow editing of non-textual
+        * content.
+        *
+        * @param String $text Text to unserialize
+        * @return Content the content object created from $text
+        *
+        * @throws MWException if unserializing the text results in a Content object that is not an instance of TextContent
+        *          and $this->allowNonTextContent is not true.
+        */
+       protected function toEditContent( $text ) {
+               $content = ContentHandler::makeContent( $text, $this->getTitle(),
+                       $this->content_model, $this->content_format );
+
+               if ( !$this->allowNonTextContent && !( $content instanceof TextContent ) ) {
+                       throw new MWException( "This content model can not be edited as text: "
+                               . ContentHandler::getLocalizedName( $content->getModel() ) );
+               }
+
+               return $content;
+       }
+
        /**
         * Send the edit form and related headers to $wgOut
         * @param $formCallback Callback that takes an OutputPage parameter; will be called
@@ -2043,7 +2096,7 @@ class EditPage {
                        $this->textbox2 = $this->textbox1;
 
                        $content = $this->getCurrentContent();
-                       $this->textbox1 = $content->serialize( $this->content_format );
+                       $this->textbox1 = $this->toEditText( $content );
 
                        $this->showTextbox1();
                } else {
@@ -2548,7 +2601,7 @@ HTML
                        $oldtext = $this->mTitle->getDefaultMessageText();
                        if( $oldtext !== false ) {
                                $oldtitlemsg = 'defaultmessagetext';
-                               $oldContent = ContentHandler::makeContent( $oldtext, $this->mTitle );
+                               $oldContent = $this->toEditContent( $oldtext );
                        } else {
                                $oldContent = null;
                        }
@@ -2556,8 +2609,7 @@ HTML
                        $oldContent = $this->getOriginalContent();
                }
 
-               $textboxContent = ContentHandler::makeContent( $this->textbox1, $this->getTitle(),
-                                                                                                               $this->content_model, $this->content_format );
+               $textboxContent = $this->toEditContent( $this->textbox1 );
 
                $newContent = $this->mArticle->replaceSectionContent(
                                                                                        $this->section, $textboxContent,
@@ -2692,8 +2744,8 @@ HTML
                if ( wfRunHooks( 'EditPageBeforeConflictDiff', array( &$this, &$wgOut ) ) ) {
                        $wgOut->wrapWikiMsg( '<h2>$1</h2>', "yourdiff" );
 
-                       $content1 = ContentHandler::makeContent( $this->textbox1, $this->getTitle(), $this->content_model, $this->content_format );
-                       $content2 = ContentHandler::makeContent( $this->textbox2, $this->getTitle(), $this->content_model, $this->content_format );
+                       $content1 = $this->toEditContent( $this->textbox1 );
+                       $content2 = $this->toEditContent( $this->textbox2 );
 
                        $handler = ContentHandler::getForModelID( $this->content_model );
                        $de = $handler->createDifferenceEngine( $this->mArticle->getContext() );
@@ -2823,8 +2875,7 @@ HTML
                $note = '';
 
                try {
-                       $content = ContentHandler::makeContent( $this->textbox1, $this->getTitle(),
-                                                                                                       $this->content_model, $this->content_format );
+                       $content = $this->toEditContent( $this->textbox1 );
 
                        if ( $this->mTriedSave && !$this->mTokenOk ) {
                                if ( $this->mTokenOkExceptSuffix ) {

diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php
index 378ee91..dbcf29c 100644 (file)
--- a/includes/api/ApiEditPage.php
+++ b/includes/api/ApiEditPage.php
@@ -300,6 +300,10 @@ class ApiEditPage extends ApiBase {
                $articleObject = new Article( $titleObj );
                $ep = new EditPage( $articleObject );
 
+               // allow editing of non-textual content.
+               //@todo: let the ContentHandler decide whether direct editing is OK
+               $ep->allowNonTextContent = true;
+
                $ep->setContextTitle( $titleObj );
                $ep->importFormData( $req );
 

diff --git a/tests/phpunit/includes/ContentHandlerTest.php b/tests/phpunit/includes/ContentHandlerTest.php
index 0dc1c1b..633f72d 100644 (file)
--- a/tests/phpunit/includes/ContentHandlerTest.php
+++ b/tests/phpunit/includes/ContentHandlerTest.php
@@ -10,7 +10,9 @@
  */
 class ContentHandlerTest extends MediaWikiTestCase {
 
-       public function setUp() {
+       public function setup() {
+               parent::setup();
+
                global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
 
                $wgExtraNamespaces[ 12312 ] = 'Dummy';
@@ -23,7 +25,7 @@ class ContentHandlerTest extends MediaWikiTestCase {
                $wgContLang->resetNamespaces(); # reset namespace cache
        }
 
-       public function tearDown() {
+       public function teardown() {
                global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
 
                unset( $wgExtraNamespaces[ 12312 ] );
@@ -34,6 +36,8 @@ class ContentHandlerTest extends MediaWikiTestCase {
 
                MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache
                $wgContLang->resetNamespaces(); # reset namespace cache
+
+               parent::teardown();
        }
 
        public function dataGetDefaultModelFor() {

diff --git a/tests/phpunit/includes/api/ApiEditPageTest.php b/tests/phpunit/includes/api/ApiEditPageTest.php
index 5297d6d..f02c6d7 100644 (file)
--- a/tests/phpunit/includes/api/ApiEditPageTest.php
+++ b/tests/phpunit/includes/api/ApiEditPageTest.php
@@ -10,11 +10,38 @@
  */
 class ApiEditPageTest extends ApiTestCase {
 
-       function setUp() {
-               parent::setUp();
+       public function setup() {
+               global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
+
+               parent::setup();
+
+               $wgExtraNamespaces[ 12312 ] = 'Dummy';
+               $wgExtraNamespaces[ 12313 ] = 'Dummy_talk';
+
+               $wgNamespaceContentModels[ 12312 ] = "testing";
+               $wgContentHandlers[ "testing" ] = 'DummyContentHandlerForTesting';
+
+               MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache
+               $wgContLang->resetNamespaces(); # reset namespace cache
+
                $this->doLogin();
        }
 
+       public function teardown() {
+               global $wgExtraNamespaces, $wgNamespaceContentModels, $wgContentHandlers, $wgContLang;
+
+               unset( $wgExtraNamespaces[ 12312 ] );
+               unset( $wgExtraNamespaces[ 12313 ] );
+
+               unset( $wgNamespaceContentModels[ 12312 ] );
+               unset( $wgContentHandlers[ "testing" ] );
+
+               MWNamespace::getCanonicalNamespaces( true ); # reset namespace cache
+               $wgContLang->resetNamespaces(); # reset namespace cache
+
+               parent::teardown();
+       }
+
        function testEdit( ) {
                $name = 'ApiEditPageTest_testEdit';
 
@@ -25,7 +52,7 @@ class ApiEditPageTest extends ApiTestCase {
                                'text' => 'some text', ) );
                $apiResult = $apiResult[0];
 
-               # Validate API result data
+               // Validate API result data
                $this->assertArrayHasKey( 'edit', $apiResult );
                $this->assertArrayHasKey( 'result', $apiResult['edit'] );
                $this->assertEquals( 'Success', $apiResult['edit']['result'] );
@@ -66,6 +93,33 @@ class ApiEditPageTest extends ApiTestCase {
                );
        }
 
+       function testNonTextEdit( ) {
+               $name = 'Dummy:ApiEditPageTest_testNonTextEdit';
+               $data = serialize( 'some bla bla text' );
+
+               // -- test new page --------------------------------------------
+               $apiResult = $this->doApiRequestWithToken( array(
+                       'action' => 'edit',
+                       'title' => $name,
+                       'text' => $data, ) );
+               $apiResult = $apiResult[0];
+
+               // Validate API result data
+               $this->assertArrayHasKey( 'edit', $apiResult );
+               $this->assertArrayHasKey( 'result', $apiResult['edit'] );
+               $this->assertEquals( 'Success', $apiResult['edit']['result'] );
+
+               $this->assertArrayHasKey( 'new', $apiResult['edit'] );
+               $this->assertArrayNotHasKey( 'nochange', $apiResult['edit'] );
+
+               $this->assertArrayHasKey( 'pageid', $apiResult['edit'] );
+
+               // validate resulting revision
+               $page = WikiPage::factory( Title::newFromText( $name ) );
+               $this->assertEquals( "testing", $page->getContentModel() );
+               $this->assertEquals( $data, $page->getContent()->serialize() );
+       }
+
        function testEditAppend() {
                $this->markTestIncomplete( "not yet implemented" );
        }