Addshore has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/383554 )

Change subject: Revision, test and fix __construct exceptions
......................................................................

Revision, test and fix __construct exceptions

This adds tests for each exception that can be thrown
in the Revision constructor.
It also fixes where the exception for the content part of a row
not containing a content object is thrown.
Prior to this ->getModel() could be called on the content row
element before the check had actually occoured.

Change-Id: Ia2d2cfdca01871fc6dbb96707d781db33d7d0a40
---
M includes/Revision.php
M tests/phpunit/includes/RevisionTest.php
2 files changed, 49 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/54/383554/1

diff --git a/includes/Revision.php b/includes/Revision.php
index bcfbe63..8855441 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -645,6 +645,10 @@
 
                        # if we have a content object, use it to set the model 
and type
                        if ( !empty( $row['content'] ) ) {
+                               if ( !( $row['content'] instanceof Content ) ) {
+                                       throw new MWException( '`content` field 
must contain a Content object.' );
+                               }
+
                                // @todo when is that set? test with external 
store setup! check out insertOn() [dk]
                                if ( !empty( $row['text_id'] ) ) {
                                        throw new MWException( "Text already 
stored in external store (id {$row['text_id']}), " .
@@ -685,10 +689,6 @@
 
                        // if we have a Content object, override mText and 
mContentModel
                        if ( !empty( $row['content'] ) ) {
-                               if ( !( $row['content'] instanceof Content ) ) {
-                                       throw new MWException( '`content` field 
must contain a Content object.' );
-                               }
-
                                $handler = $this->getContentHandler();
                                $this->mContent = $row['content'];
 
diff --git a/tests/phpunit/includes/RevisionTest.php 
b/tests/phpunit/includes/RevisionTest.php
index 8022d8e..9c71354 100644
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -66,9 +66,9 @@
                ];
                yield 'with content' => [
                        [
-                               'content' => ContentHandler::makeContent(
+                               'content' => $this->makeContent(
                                        'hello world.',
-                                       Title::newFromText( 
'RevisionTest_testConstructWithContent' ),
+                                       'RevisionTest_testConstructWithContent',
                                        CONTENT_MODEL_JAVASCRIPT
                                ),
                        ],
@@ -85,6 +85,49 @@
                $this->assertEquals( CONTENT_MODEL_JAVASCRIPT, 
$rev->getContentModel() );
        }
 
+       public function provideConstructThrowsExceptions() {
+               yield 'content and text_id both not empty' => [
+                       [
+                               'content' => $this->makeContent(),
+                               'text_id' => 'someid',
+                               ],
+                       new MWException( "Text already stored in external store 
(id someid), " .
+                               "can't serialize content object" )
+               ];
+               yield 'with bad content object (class)' => [
+                       [ 'content' => new stdClass() ],
+                       new MWException( '`content` field must contain a 
Content object.' )
+               ];
+               yield 'with bad content object (string)' => [
+                       [ 'content' => 'ImAGoat' ],
+                       new MWException( '`content` field must contain a 
Content object.' )
+               ];
+               yield 'bad row format' => [
+                       'imastring, not a row',
+                       new MWException( 'Revision constructor passed invalid 
row format.' )
+               ];
+       }
+
+       /**
+        * @dataProvider provideConstructThrowsExceptions
+        */
+       public function testConstructThrowsExceptions( $rowArray, Exception 
$expectedException ) {
+               $this->setExpectedException(
+                       get_class( $expectedException ),
+                       $expectedException->getMessage(),
+                       $expectedException->getCode()
+               );
+               new Revision( $rowArray );
+       }
+
+       private function makeContent( $text = 'text', $titleString = 'title', 
$modelid = null ) {
+               return ContentHandler::makeContent(
+                       'hello world.',
+                       Title::newFromText( $titleString ),
+                       $modelid
+               );
+       }
+
        public function provideGetRevisionText() {
                yield 'Generic test' => [
                        'This is a goat of revision text.',

-- 
To view, visit https://gerrit.wikimedia.org/r/383554
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia2d2cfdca01871fc6dbb96707d781db33d7d0a40
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to