jenkins-bot has submitted this change and it was merged.

Change subject: Make PostRevisionTestCase store all objects
......................................................................


Make PostRevisionTestCase store all objects

It should mimick a real storage setup as much as possible (see
TopicList.php)
Only storing part of the data will get us in trouble when testing
functionality that needs to reach out to other objects that are
assumed to exist (e.g. the board, to check if it's deleted or we
have permissions)

Change-Id: I43a23e85f4edc2dba6bea6ec050ad44942d12b87
---
M tests/phpunit/Collection/RevisionCollectionPermissionsTest.php
M tests/phpunit/LinksTableTest.php
M tests/phpunit/Notifications/NotifiedUsersTest.php
M tests/phpunit/PostRevisionTestCase.php
4 files changed, 148 insertions(+), 42 deletions(-)

Approvals:
  Sbisson: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/tests/phpunit/Collection/RevisionCollectionPermissionsTest.php 
b/tests/phpunit/Collection/RevisionCollectionPermissionsTest.php
index e95fa58..597a16e 100644
--- a/tests/phpunit/Collection/RevisionCollectionPermissionsTest.php
+++ b/tests/phpunit/Collection/RevisionCollectionPermissionsTest.php
@@ -17,11 +17,6 @@
  */
 class RevisionCollectionPermissionsTest extends PostRevisionTestCase {
        /**
-        * @var array
-        */
-       protected $tablesUsed = array( 'flow_revision', 'flow_tree_revision' );
-
-       /**
         * @var FlowActions
         */
        protected $actions;
@@ -186,6 +181,11 @@
                        $this->blockedUser->addToDatabase();
                        // note: the block will be added in setUp & deleted in 
tearDown;
                        // otherwise this is just any regular user
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->blockedUser->mRights = array_merge( 
$this->blockedUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->blockedUser;
@@ -194,6 +194,11 @@
        protected function anonUser() {
                if ( !$this->anonUser ) {
                        $this->anonUser = new User;
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->anonUser->mRights = array_merge( 
$this->anonUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->anonUser;
@@ -204,6 +209,11 @@
                        $this->unconfirmedUser = User::newFromName( 
'UTFlowUnconfirmed' );
                        $this->unconfirmedUser->addToDatabase();
                        $this->unconfirmedUser->addGroup( 'user' );
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->unconfirmedUser->mRights = array_merge( 
$this->unconfirmedUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->unconfirmedUser;
@@ -214,6 +224,11 @@
                        $this->confirmedUser = User::newFromName( 
'UTFlowConfirmed' );
                        $this->confirmedUser->addToDatabase();
                        $this->confirmedUser->addGroup( 'autoconfirmed' );
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->confirmedUser->mRights = array_merge( 
$this->confirmedUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->confirmedUser;
@@ -224,6 +239,11 @@
                        $this->sysopUser = User::newFromName( 'UTFlowSysop' );
                        $this->sysopUser->addToDatabase();
                        $this->sysopUser->addGroup( 'sysop' );
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->sysopUser->mRights = array_merge( 
$this->sysopUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->sysopUser;
@@ -234,6 +254,11 @@
                        $this->oversightUser = User::newFromName( 
'UTFlowOversight' );
                        $this->oversightUser->addToDatabase();
                        $this->oversightUser->addGroup( 'oversight' );
+
+                       // this test is not checking board-level permissions 
(which is currently
+                       // broken & will be fixed in follow-up patch); let's 
just bypass it for
+                       // now by making sure this user has the required 
permissions
+                       $this->oversightUser->mRights = array_merge( 
$this->oversightUser->getRights(), array( 'deletedtext', 'deletedhistory' ) );
                }
 
                return $this->oversightUser;
diff --git a/tests/phpunit/LinksTableTest.php b/tests/phpunit/LinksTableTest.php
index afdaaad..484213b 100644
--- a/tests/phpunit/LinksTableTest.php
+++ b/tests/phpunit/LinksTableTest.php
@@ -8,6 +8,7 @@
 use Flow\Exception\WikitextException;
 use Flow\LinksTableUpdater;
 use Flow\Model\AbstractRevision;
+use Flow\Model\PostRevision;
 use Flow\Model\Workflow;
 use Flow\Parsoid\ReferenceExtractor;
 use Flow\Parsoid\ReferenceFactory;
@@ -23,7 +24,18 @@
        /**
         * @var array
         */
-       protected $tablesUsed = array( 'flow_ext_ref', 'flow_wiki_ref', 
'flow_revision', 'flow_tree_revision', 'flow_workflow' );
+       protected $tablesUsed = array(
+               'flow_ext_ref',
+               'flow_revision',
+               'flow_topic_list',
+               'flow_tree_node',
+               'flow_tree_revision',
+               'flow_wiki_ref',
+               'flow_workflow',
+               'page',
+               'revision',
+               'text',
+       );
 
        /**
         * @var ManagerGroup
@@ -45,8 +57,22 @@
         */
        protected $updater;
 
+       /**
+        * @var Workflow
+        */
+       protected $workflow;
+
+       /**
+        * @var PostRevision
+        */
+       protected $revision;
+
        public function setUp() {
                parent::setUp();
+
+               // create a workflow & revision associated with it
+               $this->revision = $this->generateObject();
+               $this->workflow = 
$this->workflows[$this->revision->getCollectionId()->getAlphadecimal()];
                $this->storage = Container::get( 'storage' );
                $this->extractor = Container::get( 'reference.extractor' );
                $this->recorder = Container::get( 'reference.recorder' );
@@ -152,7 +178,12 @@
         */
        public function testGetReferencesFromRevisionContent( $content, 
$expectedReferences ) {
                $content = Utils::convert( 'wikitext', 'html', $content, 
$this->workflow->getOwnerTitle() );
-               $revision = $this->generatePost( array( 'rev_content' => 
$content ) );
+               $revision = $this->generatePost( array(
+                       'rev_content' => $content,
+                       // make sure it's associated with $this->workflow
+                       'rev_type_id' => $this->workflow->getId()->getBinary(),
+                       'tree_rev_descendant_id' => 
$this->workflow->getId()->getBinary(),
+               ) );
 
                $expectedReferences = $this->expandReferences( $this->workflow, 
$revision, $expectedReferences );
 
@@ -166,7 +197,12 @@
         */
        public function testGetReferencesAfterRevisionInsert( $content, 
$expectedReferences ) {
                $content = Utils::convert( 'wikitext', 'html', $content, 
$this->workflow->getOwnerTitle() );
-               $revision = $this->generatePost( array( 'rev_content' => 
$content ) );
+               $revision = $this->generatePost( array(
+                       'rev_content' => $content,
+                       // make sure it's associated with $this->workflow
+                       'rev_type_id' => $this->workflow->getId()->getBinary(),
+                       'tree_rev_descendant_id' => 
$this->workflow->getId()->getBinary(),
+               ) );
 
                // Save to storage to test if ReferenceRecorder listener picks 
this up
                $this->store( $revision );
diff --git a/tests/phpunit/Notifications/NotifiedUsersTest.php 
b/tests/phpunit/Notifications/NotifiedUsersTest.php
index 6ec7082..8aea3ac 100644
--- a/tests/phpunit/Notifications/NotifiedUsersTest.php
+++ b/tests/phpunit/Notifications/NotifiedUsersTest.php
@@ -95,8 +95,7 @@
         * }
         */
        protected function getTestData() {
-               $this->generateWorkflowForPost();
-               $topicWorkflow = $this->workflow;
+               $topicWorkflow = $this->generateWorkflow();
                $post = $this->generateObject( array(), array(), 1 );
                $topic = $this->generateObject( array(), array( $post ) );
                $user = User::newFromName( 'Flow Test User' );
diff --git a/tests/phpunit/PostRevisionTestCase.php 
b/tests/phpunit/PostRevisionTestCase.php
index 7229113..f36fa7b 100644
--- a/tests/phpunit/PostRevisionTestCase.php
+++ b/tests/phpunit/PostRevisionTestCase.php
@@ -4,15 +4,15 @@
 
 use DeferredUpdates;
 use Flow\Container;
-use Flow\Data\Index\BoardHistoryIndex;
-use Flow\Data\Listener\NotificationListener;
-use Flow\Data\Listener\RecentChangesListener;
-use Flow\Data\ObjectManager;
+use Flow\Data\ManagerGroup;
+use Flow\Exception\FlowException;
 use Flow\Model\AbstractRevision;
 use Flow\Model\PostRevision;
+use Flow\Model\TopicListEntry;
 use Flow\Model\Workflow;
 use Flow\Model\UserTuple;
 use Flow\Model\UUID;
+use Flow\OccupationController;
 use SplQueue;
 use User;
 
@@ -21,10 +21,16 @@
  * @group Database
  */
 class PostRevisionTestCase extends FlowTestCase {
-       /**
-        * @var PostRevision
-        */
-       protected $revision;
+       protected $tablesUsed = array(
+               'flow_revision',
+               'flow_topic_list',
+               'flow_tree_node',
+               'flow_tree_revision',
+               'flow_workflow',
+               'page',
+               'revision',
+               'text',
+       );
 
        /**
         * @var PostRevision[]
@@ -32,14 +38,13 @@
        protected $revisions = array();
 
        /**
-        * @var Workflow
+        * @var Workflow[]
         */
-       protected $workflow;
+       protected $workflows = array();
 
        protected function setUp() {
                parent::setUp();
-               $this->generateWorkflowForPost();
-               $this->revision = $this->generateObject();
+
                // Revisions must be blanked here otherwise phpunit run with 
--repeat will remember
                // ths revision list between multiple invocations of the test 
causing issues.
                $this->revisions = array();
@@ -53,10 +58,23 @@
 
                foreach ( $this->revisions as $revision ) {
                        try {
-                               $this->getStorage()->remove( $revision );
+                               $this->getStorage()->multiRemove( array( 
$revision ) );
                        } catch ( \MWException $e ) {
                                // ignore - lifecyclehandlers may cause issues 
with tests, where
                                // not all related stuff is loaded
+                       }
+               }
+
+               foreach ( $this->workflows as $workflow ) {
+                       try {
+                               $this->getStorage()->multiRemove( array( 
$workflow ) );
+
+                               $found = $this->getStorage()->find( 
'TopicListEntry', array( 'topic_id' => $workflow->getId() ) );
+                               if ( $found ) {
+                                       $this->getStorage()->multiRemove( 
$found );
+                               }
+                       } catch ( FlowException $e ) {
+                               // nothing, was probably never stored...
                        }
                }
 
@@ -65,10 +83,10 @@
        }
 
        /**
-        * @return ObjectManager
+        * @return ManagerGroup
         */
        protected function getStorage() {
-               return Container::get( 'storage.post' );
+               return Container::get( 'storage' );
        }
 
        /**
@@ -83,7 +101,7 @@
         * @return array
         */
        protected function generateRow( array $row = array() ) {
-               $this->generateWorkflowForPost();
+               $workflow = $this->generateWorkflow( array( 'workflow_type' => 
'topic' ) );
                $uuidRevision = UUID::create();
 
                $user = User::newFromName( 'UTSysop' );
@@ -93,7 +111,7 @@
                        // flow_revision
                        'rev_id' => $uuidRevision->getBinary(),
                        'rev_type' => 'post',
-                       'rev_type_id' => $this->workflow->getId()->getBinary(),
+                       'rev_type_id' => $workflow->getId()->getBinary(),
                        'rev_user_wiki' => $tuple->wiki,
                        'rev_user_id' => $tuple->id,
                        'rev_user_ip' => $tuple->ip,
@@ -115,7 +133,7 @@
                        'rev_previous_content_length' => 0,
 
                        // flow_tree_revision
-                       'tree_rev_descendant_id' => 
$this->workflow->getId()->getBinary(),
+                       'tree_rev_descendant_id' => 
$workflow->getId()->getBinary(),
                        'tree_rev_id' => $uuidRevision->getBinary(),
                        'tree_orig_user_wiki' => $tuple->wiki,
                        'tree_orig_user_id' => $tuple->id,
@@ -127,14 +145,11 @@
        /**
         * Populate a fake workflow in the unittest database
         *
+        * @param array $row
         * @return Workflow
         */
-       protected function generateWorkflowForPost() {
-               if ( $this->workflow ) {
-                       return $this->workflow;
-               }
-
-               $row = array(
+       protected function generateWorkflow( $row = array() ) {
+               $row = $row + array(
                        'workflow_id' => UUID::create()->getBinary(),
                        'workflow_type' => 'topic',
                        'workflow_wiki' => wfWikiId(),
@@ -146,9 +161,14 @@
                        'workflow_lock_state' => 0,
                        'workflow_last_update_timestamp' => wfTimestampNow(),
                );
-               $this->workflow = Workflow::fromStorageRow( $row );
+               $workflow = Workflow::fromStorageRow( $row );
 
-               return $this->workflow;
+               // store workflow:
+               // * so we can retrieve it should we want to store it (see 
store())
+               // * so we can remove it on tearDown
+               $this->workflows[$workflow->getId()->getAlphadecimal()] = 
$workflow;
+
+               return $workflow;
        }
 
        /**
@@ -181,14 +201,40 @@
         * @param PostRevision $revision
         */
        protected function store( PostRevision $revision ) {
-               $this->getStorage()->put(
-                       $revision,
-                       array(
-                               'workflow' => $this->generateWorkflowForPost(),
-                               // @todo: Topic.php also adds 'topic-title'
-                       )
+               $topicWorkflow = 
$this->workflows[$revision->getCollectionId()->getAlphadecimal()];
+               $boardWorkflow = Container::get( 'factory.loader.workflow' )
+                       ->createWorkflowLoader( $topicWorkflow->getOwnerTitle() 
)
+                       ->getWorkflow();
+
+               $metadata = array(
+                       'workflow' => $topicWorkflow,
+                       'board-workflow' => $boardWorkflow,
+                       // @todo: Topic.php also adds 'topic-title'
                );
 
+               // check if this topic (+ workflow + board workflow + board 
page) have
+               // already been inserted or do so now
+               $found = $this->getStorage()->find( 'TopicListEntry', array( 
'topic_id' => $topicWorkflow->getId() ) );
+               if ( !$found ) {
+                       $title = $boardWorkflow->getOwnerTitle();
+                       $user = User::newFromName( '127.0.0.1', false );
+
+                       /** @var OccupationController $occupationController */
+                       $occupationController = Container::get( 
'occupation_controller' );
+                       // make sure user has rights to create board
+                       $user->mRights = array_merge( $user->getRights(), 
array( 'flow-create-board' ) );
+                       $occupationController->allowCreation( $title, $user );
+                       $occupationController->ensureFlowRevision( new 
\Article( $title ), $boardWorkflow );
+
+                       $topicListEntry = TopicListEntry::create( 
$boardWorkflow, $topicWorkflow );
+
+                       $this->getStorage()->put( $boardWorkflow, $metadata );
+                       $this->getStorage()->put( $topicWorkflow, $metadata );
+                       $this->getStorage()->put( $topicListEntry, $metadata );
+               }
+
+               $this->getStorage()->put( $revision, $metadata );
+
                /** @var SplQueue $deferredQueue */
                $deferredQueue = Container::get( 'deferred_queue' );
                while( !$deferredQueue->isEmpty() ) {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I43a23e85f4edc2dba6bea6ec050ad44942d12b87
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to