jenkins-bot has submitted this change and it was merged.
Change subject: Refactor out calls to RequestContext::getMain()
......................................................................
Refactor out calls to RequestContext::getMain()
While trying to write unit tests against the API i ran into a problem,
namely that RequestContext::getMain()->getUser()->getName() always returned
127.0.0.1 regardless of the user provided to the api in the unit
test. This is because the main request context is the wrong context.
Thankfully during the api output refactors we used the context throughout, the
only changes necessary were to pass the correct context from the Action and API
wrappers into the block instances, so they could pass it on to the formatters.
This additionally adds a POC test for ApiFlowLockTopic demonstrating that we
can now pass in a priveleged user and perform priveleged actions.
There is still some mess left behind, namely the value user value in the
container which is used in some classes. I think the best solution here is
going to be to remove all static calls to Container, and instead create the
container on entry from API or Action specifically for that context. One
container per context, so everything inside points to the right values. But
that will require several patches removing calls to Container::get, and probably
passing containers directly into the block instances.
For now this appears to work, and this isn't actually a problem we've ever seen
in production, its a theoretical problem that is affecting testability.
Change-Id: Ia22eb12f2bfc5c5e3356a021b6df960fd26290cf
---
M Hooks.php
M includes/Block/Block.php
M includes/Block/BoardHistory.php
M includes/Block/Header.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
M includes/Block/TopicSummary.php
M includes/SubmissionHandler.php
M includes/View.php
M includes/WorkflowLoader.php
M includes/api/ApiFlowBaseGet.php
M includes/api/ApiFlowBasePost.php
M includes/api/ApiQueryFlow.php
A tests/phpunit/api/ApiFlowLockTopicTest.php
14 files changed, 321 insertions(+), 138 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Hooks.php b/Hooks.php
index 8360cdd..c35ab45 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -93,6 +93,15 @@
}
/**
+ * Reset anything that happened in self::initFlowExtension for
+ * unit tests
+ */
+ public static function resetFlowExtension() {
+ self::$abuseFilter = null;
+ self::$occupationController = null;
+ }
+
+ /**
* Hook: LoadExtensionSchemaUpdates
*
* @param $updater DatabaseUpdater object
diff --git a/includes/Block/Block.php b/includes/Block/Block.php
index a521aba..044c442 100644
--- a/includes/Block/Block.php
+++ b/includes/Block/Block.php
@@ -9,20 +9,24 @@
use Flow\Model\AbstractRevision;
use Flow\Model\UUID;
use Flow\Model\Workflow;
+use Flow\RevisionActionPermissions;
use Flow\SpamFilter\Controller as SpamFilterController;
use Flow\Templating;
-use User;
+use IContextSource;
interface Block {
+ /**
+ * @param IContextSource $context
+ */
+ function init( IContextSource $context, $action );
+
/**
* Perform validation of data model
*
* @param string $action
- * @param User $user
- * @param array $data
* @return boolean True if data model is valid
*/
- function onSubmit( $action, User $user, array $data );
+ function onSubmit( array $data );
/**
* Write updates to storage
@@ -51,14 +55,26 @@
abstract class AbstractBlock implements Block {
+ /** @var Workflow */
protected $workflow;
+ /** @var ManagerGroup */
protected $storage;
- /** @var User $user */
- protected $user;
+ /** @var IContextSource */
+ protected $context;
+ /** @var array|null */
protected $submitted = null;
+ /** @var array */
protected $errors = array();
+
+ /**
+ * @var string|null The commitable action being submitted, or null
+ * for read-only actions.
+ */
protected $action;
+
+ /** @var RevisionActionPermissions */
+ protected $permissions;
/**
* A list of supported post actions
@@ -72,6 +88,7 @@
*/
protected $supportedGetActions = array();
+ /** @var array */
protected $requiresWikitext = array();
/**
@@ -85,15 +102,33 @@
$this->storage = $storage;
}
+ /**
+ * Called by $this->onSubmit to populate $this->errors based
+ * on $this->action and $this->submitted.
+ */
abstract protected function validate();
- // These methods exist in the Block interface and as such cannot be
abstract
+
+ // This method exists in the Block interface and as such cannot be
abstract
// until php 5.3.9, but MediaWiki requires PHP version 5.3.2 or later
(and
// some of our test machines are on 5.3.3).
//abstract public function commit();
- public function init( $action, $user ) {
- $this->action = $this->getActionName( $action );
- $this->user = $user;
+ /**
+ * @var IContextSource $context
+ * @var string $action
+ */
+ public function init( IContextSource $context, $action ) {
+ $this->context = $context;
+ $this->action = $action;
+ // @todo not guaranteed that $this->permissions->getUser() ===
$context->getUser();
+ $this->permissions = Container::get( 'permissions' );
+ }
+
+ /**
+ * @return IContextSource
+ */
+ public function getContext() {
+ return $this->context;
}
/**
@@ -142,16 +177,19 @@
return $this->templates[$action];
}
- public function onSubmit( $action, User $user, array $data ) {
+ /**
+ * @param array $data
+ * @return bool|null true when accepted, false when not accepted.
+ * null when this action does not support submission.
+ */
+ public function onSubmit( array $data ) {
/** @noinspection PhpUnusedLocalVariableInspection */
$section = new \ProfileSection( __METHOD__ );
- if ( !$this->canSubmit( $action ) ) {
+ if ( !$this->canSubmit( $this->action ) ) {
return null;
}
- $this->user = $user;
$this->submitted = $data;
-
$this->validate();
return !$this->hasErrors();
@@ -269,7 +307,7 @@
}
public function getEditToken() {
- return $this->user->getEditToken();
+ return $this->context->getUser()->getEditToken();
}
public function unsetRequiresWikitext( $action ) {
diff --git a/includes/Block/BoardHistory.php b/includes/Block/BoardHistory.php
index 251f835..9e8ae94 100644
--- a/includes/Block/BoardHistory.php
+++ b/includes/Block/BoardHistory.php
@@ -2,30 +2,18 @@
namespace Flow\Block;
-use Flow\RevisionActionPermissions;
use Flow\Container;
use Flow\Formatter\BoardHistoryQuery;
use Flow\Formatter\RevisionFormatter;
use Flow\Exception\DataModelException;
class BoardHistoryBlock extends AbstractBlock {
-
- /**
- * @var RevisionActionPermissions $permissions Allows or denies actions
to be performed
- */
- protected $permissions;
-
protected $supportedGetActions = array( 'history' );
// @Todo - fill in the template names
protected $templates = array(
'history' => '',
);
-
- public function init( $action, $user ) {
- parent::init( $action, $user );
- $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
- }
/**
* Board history is read-only block which should not invoke write action
@@ -57,11 +45,10 @@
/** @var RevisionFormatter $formatter */
$formatter = Container::get( 'formatter.revision' );
$formatter->setIncludeHistoryProperties( true );
- $ctx = \RequestContext::getMain();
$revisions = array();
foreach ( $history as $row ) {
- $serialized = $formatter->formatApi( $row, $ctx );
+ $serialized = $formatter->formatApi( $row,
$this->context );
if ( $serialized ) {
$revisions[$serialized['revisionId']] =
$serialized;
}
diff --git a/includes/Block/Header.php b/includes/Block/Header.php
index 27d25dc..a40dda0 100644
--- a/includes/Block/Header.php
+++ b/includes/Block/Header.php
@@ -12,9 +12,9 @@
use Flow\Model\Header;
use Flow\RevisionActionPermissions;
use Flow\UrlGenerator;
+use IContextSource;
class HeaderBlock extends AbstractBlock {
-
/**
* @var Header|null
*/
@@ -60,10 +60,8 @@
*/
protected $permissions;
- public function init( $action, $user ) {
- parent::init( $action, $user );
-
- $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
+ public function init( IContextSource $context, $action ) {
+ parent::init( $context, $action );
// Basic initialisation done -- now, load data if applicable
if ( $this->workflow->isNew() ) {
@@ -85,7 +83,9 @@
protected function validate() {
// @todo some sort of restriction along the lines of article
protection
- if ( !$this->user->isAllowed( 'edit' ) ) {
+ // @todo this is superseeded by SubmissionHandler::onSubmit
checking
+ // Title::userCan() ?
+ if ( !$this->context->getUser()->isAllowed( 'edit' ) ) {
$this->addError( 'permissions', wfMessage(
'flow-error-not-allowed' ) );
return;
}
@@ -117,13 +117,21 @@
// handing user back to specific dialog indicating race
condition
$this->addError(
'prev_revision',
- wfMessage( 'flow-error-prev-revision-mismatch'
)->params( $this->submitted['prev_revision'],
$this->header->getRevisionId()->getAlphadecimal(), $this->user->getName() ),
+ wfMessage( 'flow-error-prev-revision-mismatch'
)->params(
+ $this->submitted['prev_revision'],
+
$this->header->getRevisionId()->getAlphadecimal(),
+ $this->context->getUser()->getName()
+ ),
array( 'revision_id' =>
$this->header->getRevisionId()->getAlphadecimal() ) // save current revision ID
);
}
// this isn't really part of validate, but we want the
error-rendering template to see the users edited header
- $this->newRevision = $this->header->newNextRevision(
$this->user, $this->submitted['content'], 'edit-header' );
+ $this->newRevision = $this->header->newNextRevision(
+ $this->context->getUser(),
+ $this->submitted['content'],
+ 'edit-header'
+ );
if ( !$this->checkSpamFilters( $this->header,
$this->newRevision ) ) {
return;
@@ -144,7 +152,12 @@
return;
}
- $this->newRevision = Header::create( $this->workflow,
$this->user, $this->submitted['content'], 'create-header' );
+ $this->newRevision = Header::create(
+ $this->workflow,
+ $this->context->getUser(),
+ $this->submitted['content'],
+ 'create-header'
+ );
if ( !$this->checkSpamFilters( null, $this->newRevision ) ) {
return;
@@ -229,7 +242,7 @@
list( $new, $old ) = $query->getDiffViewResult(
$options['newRevision'], $oldRevision );
/** @var RevisionDiffViewFormatter $formatter */
$formatter = Container::get( 'formatter.revision.diff.view' );
- $output['revision'] = $formatter->formatApi( $new, $old,
\RequestContext::getMain() );
+ $output['revision'] = $formatter->formatApi( $new, $old,
$this->context );
return $output;
}
@@ -240,7 +253,7 @@
$row = $query->getSingleViewResult( $revId );
/** @var RevisionViewFormatter $formatter */
$formatter = Container::get( 'formatter.revisionview' );
- $output['revision'] = $formatter->formatApi( $row,
\RequestContext::getMain() );
+ $output['revision'] = $formatter->formatApi( $row,
$this->context );
return $output;
}
@@ -259,7 +272,6 @@
),
);
} else {
- $ctx = \RequestContext::getMain();
$row = new FormatterRow;
$row->workflow = $this->workflow;
$row->revision = $this->header;
@@ -270,7 +282,7 @@
$serializer->setContentFormat( 'wikitext' );
}
- $output['revision'] = $serializer->formatApi( $row,
$ctx );
+ $output['revision'] = $serializer->formatApi( $row,
$this->context );
}
return $output;
}
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index 6f7eac9..fa8dcae 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -16,7 +16,6 @@
use Flow\Model\UUID;
use Flow\Model\Workflow;
use Flow\Repository\RootPostLoader;
-use Flow\RevisionActionPermissions;
use Flow\Templating;
use Message;
@@ -87,11 +86,6 @@
protected $requiresWikitext = array( 'edit-post', 'edit-title',
'lock-topic' );
- /**
- * @var RevisionActionPermissions $permissions Allows or denies actions
to be performed
- */
- protected $permissions;
-
public function __construct( Workflow $workflow, ManagerGroup $storage,
$root ) {
parent::__construct( $workflow, $storage );
if ( $root instanceof PostRevision ) {
@@ -103,11 +97,6 @@
'Expected PostRevision or RootPostLoader,
received: ' . is_object( $root ) ? get_class( $root ) : gettype( $root ),
'invalid-input'
);
}
- }
-
- public function init( $action, $user ) {
- parent::init( $action, $user );
- $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
}
protected function validate() {
@@ -200,13 +189,21 @@
// handing user back to specific dialog indicating race
condition
$this->addError(
'prev_revision',
- wfMessage( 'flow-error-prev-revision-mismatch'
)->params( $this->submitted['prev_revision'],
$topicTitle->getRevisionId()->getAlphadecimal(), $this->user->getName() ),
+ wfMessage( 'flow-error-prev-revision-mismatch'
)->params(
+ $this->submitted['prev_revision'],
+
$topicTitle->getRevisionId()->getAlphadecimal(),
+ $this->context->getUser()->getName()
+ ),
array( 'revision_id' =>
$topicTitle->getRevisionId()->getAlphadecimal() ) // save current revision ID
);
return;
}
- $this->newRevision = $topicTitle->newNextRevision( $this->user,
$this->submitted['content'], 'edit-title' );
+ $this->newRevision = $topicTitle->newNextRevision(
+ $this->context->getUser(),
+ $this->submitted['content'],
+ 'edit-title'
+ );
if ( !$this->checkSpamFilters( $topicTitle, $this->newRevision
) ) {
return;
}
@@ -230,7 +227,11 @@
$this->addError( 'permissions', wfMessage(
'flow-error-not-allowed' ) );
return;
}
- $this->newRevision = $post->reply( $this->workflow,
$this->user, $this->submitted['content'] );
+ $this->newRevision = $post->reply(
+ $this->workflow,
+ $this->context->getUser(),
+ $this->submitted['content']
+ );
if ( !$this->checkSpamFilters( null, $this->newRevision ) ) {
return;
}
@@ -330,7 +331,7 @@
$reason = $this->submitted['reason'];
- $this->newRevision = $post->moderate( $this->user, $newState,
$action, $reason );
+ $this->newRevision = $post->moderate(
$this->context->getUser(), $newState, $action, $reason );
if ( !$this->newRevision ) {
$this->addError( 'moderate', wfMessage(
'flow-error-not-allowed' ) );
return;
@@ -359,20 +360,32 @@
return;
}
if ( $post->getRevisionId()->getAlphadecimal() !==
$this->submitted['prev_revision'] ) {
- // This is a reasonably effective way to ensure prev
revision matches, but for guarantees against race
- // conditions there also exists a unique index on
rev_prev_revision in mysql, meaning if someone else inserts against the
- // parent we and the submitter think is the latest, our
insert will fail.
- // TODO: Catch whatever exception happens there, make
sure the most recent revision is the one in the cache before
- // handing user back to specific dialog indicating race
condition
+ // This is a reasonably effective way to ensure prev
revision
+ // matches, but for guarantees against race conditions
there
+ // also exists a unique index on rev_prev_revision in
mysql,
+ // meaning if someone else inserts against the parent
we and
+ // the submitter think is the latest, our insert will
fail.
+ //
+ // TODO: Catch whatever exception happens there, make
sure the
+ // most recent revision is the one in the cache before
handing
+ // user back to specific dialog indicating race
condition
$this->addError(
'prev_revision',
- wfMessage( 'flow-error-prev-revision-mismatch'
)->params( $this->submitted['prev_revision'],
$post->getRevisionId()->getAlphadecimal(), $this->user->getName() ),
+ wfMessage( 'flow-error-prev-revision-mismatch'
)->params(
+ $this->submitted['prev_revision'],
+
$post->getRevisionId()->getAlphadecimal(),
+ $this->context->getUser()->getName()
+ ),
array( 'revision_id' =>
$post->getRevisionId()->getAlphadecimal() ) // save current revision ID
);
return;
}
- $this->newRevision = $post->newNextRevision( $this->user,
$this->submitted['content'], 'edit-post' );
+ $this->newRevision = $post->newNextRevision(
+ $this->context->getUser(),
+ $this->submitted['content'],
+ 'edit-post'
+ );
if ( !$this->checkSpamFilters( $post, $this->newRevision ) ) {
return;
}
@@ -525,14 +538,17 @@
$oldRevision = $options['newRevision'];
}
list( $new, $old ) = Container::get( 'query.post.view'
)->getDiffViewResult( $options['newRevision'], $oldRevision );
- $output['revision'] = Container::get(
'formatter.revision.diff.view' )->formatApi( $new, $old,
\RequestContext::getMain() );
+ $output['revision'] = Container::get(
'formatter.revision.diff.view' )
+ ->formatApi( $new, $old, $this->context );
return $output;
}
// @Todo - duplicated logic in other single view block
protected function renderSingleViewAPI( $revId ) {
$row = Container::get( 'query.post.view'
)->getSingleViewResult( $revId );
- $output['revision'] = Container::get( 'formatter.revisionview'
)->formatApi( $row, \RequestContext::getMain() );
+ $output['revision'] = Container::get( 'formatter.revisionview' )
+ ->formatApi( $row, $this->context );
+
return $output;
}
@@ -562,7 +578,7 @@
return $serializer->formatApi(
$this->workflow,
Container::get( 'query.topiclist' )->getResults( array(
$workflowId ) ),
- \RequestContext::getMain()
+ $this->context
);
}
@@ -592,7 +608,7 @@
if ( isset( $options['contentFormat'] ) ) {
$serializer->setContentFormat(
$options['contentFormat'] );
}
- $serialized = $serializer->formatApi( $row,
\RequestContext::getMain() );
+ $serialized = $serializer->formatApi( $row, $this->context );
if ( !$serialized ) {
return null;
}
@@ -646,11 +662,10 @@
$serializer->setContentFormat(
$options['contentFormat'] );
}
$serializer->setIncludeHistoryProperties( true );
- $ctx = \RequestContext::getMain();
$result = array();
foreach ( $found as $row ) {
- $serialized = $serializer->formatApi( $row, $ctx );
+ $serialized = $serializer->formatApi( $row,
$this->context );
// if the user is not allowed to see this row it will
return empty
if ( $serialized ) {
$result[$serialized['revisionId']] =
$serialized;
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index 3c451ae..dd29ecb 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -11,7 +11,6 @@
use Flow\Model\TopicListEntry;
use Flow\Model\UUID;
use Flow\Model\Workflow;
-use Flow\RevisionActionPermissions;
use Flow\Templating;
use Flow\Exception\FailCommitException;
@@ -53,16 +52,6 @@
*/
protected $firstPost;
- /**
- * @var RevisionActionPermissions
- */
- protected $permissions;
-
- public function init( $action, $user ) {
- parent::init( $action, $user );
- $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
- }
-
protected function validate() {
// for now, new topic is considered a new post; perhaps some
day topic creation should get it's own permissions?
if ( !$this->permissions->isAllowed( null, 'new-post' ) ) {
@@ -83,9 +72,7 @@
return;
}
- if (
- trim( $this->submitted['content'] === '' )
- ) {
+ if ( trim( $this->submitted['content'] ) === '' ) {
$this->addError( 'content', wfMessage(
'flow-error-missing-content' ) );
return;
}
@@ -114,13 +101,14 @@
*/
protected function create() {
$title = $this->workflow->getArticleTitle();
- $topicWorkflow = Workflow::create( 'topic', $this->user, $title
);
+ $user = $this->context->getUser();
+ $topicWorkflow = Workflow::create( 'topic', $user, $title );
$topicListEntry = TopicListEntry::create( $this->workflow,
$topicWorkflow );
$topicTitle = PostRevision::create( $topicWorkflow,
$this->submitted['topic'] );
$firstPost = null;
if ( !empty( $this->submitted['content'] ) ) {
- $firstPost = $topicTitle->reply( $topicWorkflow,
$this->user, $this->submitted['content'] );
+ $firstPost = $topicTitle->reply( $topicWorkflow, $user,
$this->submitted['content'] );
$topicTitle->setChildren( array( $firstPost ) );
}
@@ -177,7 +165,6 @@
return $response + $serializer->buildEmptyResult(
$this->workflow );
}
- $ctx = \RequestContext::getMain();
// @todo remove the 'api' => true, its always api
$findOptions = $this->getFindOptions( $options + array( 'api'
=> true ) );
$page = $this->getPage( $findOptions );
@@ -204,7 +191,7 @@
return $id->getAlphadecimal();
}, $workflowIds ) ) );
- return $response + $serializer->formatApi( $this->workflow,
$workflows, $found, $page, $ctx );
+ return $response + $serializer->formatApi( $this->workflow,
$workflows, $found, $page, $this->context );
}
public function getName() {
@@ -248,7 +235,7 @@
// Only support sortby = updated now, fall back to creation
time by default otherwise.
// To clear the sortby user preference, pass sortby with an
empty value
$sortByOption = '';
- $user = $this->user;
+ $user = $this->context->getUser();
if ( isset( $requestOptions['sortby'] ) ) {
if ( $requestOptions['sortby'] === 'updated' ) {
$sortByOption = 'updated';
diff --git a/includes/Block/TopicSummary.php b/includes/Block/TopicSummary.php
index cc7a481..f87b7e2 100644
--- a/includes/Block/TopicSummary.php
+++ b/includes/Block/TopicSummary.php
@@ -14,11 +14,10 @@
use Flow\Model\PostRevision;
use Flow\Model\PostSummary;
use Flow\Templating;
-use Flow\RevisionActionPermissions;
+use IContextSource;
use Message;
class TopicSummaryBlock extends AbstractBlock {
-
/**
* @var PostSummary|null
*/
@@ -28,13 +27,6 @@
* @var FormatterRow
*/
protected $formatterRow;
-
- /**
- * Allows or denies actions to be performed
- *
- * @var RevisionActionPermissions
- */
- protected $permissions;
/**
* @var PostSummary|null
@@ -71,9 +63,8 @@
* @param string
* @param User
*/
- public function init( $action, $user ) {
- parent::init( $action, $user );
- $this->permissions = new RevisionActionPermissions(
Container::get( 'flow_actions' ), $user );
+ public function init( IContextSource $context, $action ) {
+ parent::init( $context, $action );
if ( !$this->workflow->isNew() ) {
/** @var PostSummaryQuery $query */
@@ -126,7 +117,12 @@
return;
}
- $this->nextRevision = PostSummary::create(
$this->findTopicTitle(), $this->user, $this->submitted['summary'],
'create-topic-summary' );
+ $this->nextRevision = PostSummary::create(
+ $this->findTopicTitle(),
+ $this->context->getUser(),
+ $this->submitted['summary'],
+ 'create-topic-summary'
+ );
// Edit topic summary
} else {
if ( !$this->permissions->isAllowed(
$this->topicSummary, 'edit-topic-summary' ) ) {
@@ -143,14 +139,18 @@
wfMessage(
'flow-error-prev-revision-mismatch' )->params(
$this->submitted['prev_revision'],
$this->topicSummary->getRevisionId()->getAlphadecimal(),
- $this->user->getName()
+
$this->context->getUser()->getName()
),
array( 'revision_id' =>
$this->topicSummary->getRevisionId()->getAlphadecimal() )
);
return;
}
- $this->nextRevision =
$this->topicSummary->newNextRevision( $this->user, $this->submitted['summary'],
'edit-topic-summary' );
+ $this->nextRevision =
$this->topicSummary->newNextRevision(
+ $this->context->getUser(),
+ $this->submitted['summary'],
+ 'edit-topic-summary'
+ );
}
if ( !$this->checkSpamFilters( $this->topicSummary,
$this->nextRevision ) ) {
@@ -252,7 +252,7 @@
$row = $query->getSingleViewResult(
$options['revId'] );
/** @var RevisionViewFormatter
$formatter */
$formatter = Container::get(
'formatter.revisionview' );
- $output['revision'] =
$formatter->formatApi( $row, \RequestContext::getMain() );
+ $output['revision'] =
$formatter->formatApi( $row, $this->context );
} else {
if ( isset( $options['contentFormat'] )
&& $options['contentFormat'] === 'wikitext' ) {
$this->requiresWikitext[] =
'view-topic-summary';
@@ -273,7 +273,7 @@
$oldRevision = $options['newRevision'];
}
list( $new, $old ) = Container::get(
'query.postsummary.view' )->getDiffViewResult( $options['newRevision'],
$oldRevision );
- $output['revision'] = Container::get(
'formatter.revision.diff.view' )->formatApi( $new, $old,
\RequestContext::getMain() );
+ $output['revision'] = Container::get(
'formatter.revision.diff.view' )->formatApi( $new, $old, $this->context );
break;
}
@@ -289,7 +289,8 @@
}
if ( $this->formatterRow ) {
$output['revision'] = $formatter->formatApi(
- $this->formatterRow, \RequestContext::getMain()
+ $this->formatterRow,
+ $this->context
);
} else {
$urlGenerator = Container::get( 'url_generator' );
diff --git a/includes/SubmissionHandler.php b/includes/SubmissionHandler.php
index cb72c99..8046531 100644
--- a/includes/SubmissionHandler.php
+++ b/includes/SubmissionHandler.php
@@ -2,14 +2,15 @@
namespace Flow;
+use DeferredUpdates;
use Flow\Block\AbstractBlock;
use Flow\Block\Block;
-use Flow\Model\Workflow;
use Flow\Data\BufferedCache;
use Flow\Data\ManagerGroup;
use Flow\Exception\InvalidDataException;
use Flow\Exception\InvalidActionException;
-use DeferredUpdates;
+use Flow\Model\Workflow;
+use IContextSource;
use SplQueue;
class SubmissionHandler {
@@ -48,26 +49,31 @@
/**
* @param Workflow $workflow
- * @param string $action
+ * @param IContextSource $context
* @param AbstractBlock[] $blocks
- * @param \User $user
- * @param array $params
+ * @param string $action
+ * @param array $parameters
* @return AbstractBlock[]
* @throws InvalidActionException
* @throws InvalidDataException
*/
- public function handleSubmit( Workflow $workflow, $action, array
$blocks, $user, array $params ) {
- $success = true;
- /** @var Block[] $interestedBlocks */
- $interestedBlocks = array();
-
+ public function handleSubmit(
+ Workflow $workflow,
+ IContextSource $context,
+ array $blocks,
+ $action,
+ array $parameters
+ ) {
// since this is a submit force dbFactory to always return
master
$this->dbFactory->forceMaster();
+ /** @var Block[] $interestedBlocks */
+ $interestedBlocks = array();
foreach ( $blocks as $block ) {
// This is just a check whether the block understands
the action,
// Doesn't consider permissions
if ( $block->canSubmit( $action ) ) {
+ $block->init( $context, $action );
$interestedBlocks[] = $block;
}
}
@@ -86,15 +92,16 @@
// Check mediawiki core permissions for title protection,
blocked
// status, etc.
- if ( !$workflow->userCan( 'edit', $user ) ) {
+ if ( !$workflow->userCan( 'edit', $context->getUser() ) ) {
reset( $interestedBlocks )->addError( 'block',
wfMessage( 'blockedtitle' ) );
return array();
}
+ $success = true;
foreach ( $interestedBlocks as $block ) {
$name = $block->getName();
- $data = isset( $params[$name] ) ? $params[$name] :
array();
- $success &= $block->onSubmit( $action, $user, $data );
+ $data = isset( $parameters[$name] ) ?
$parameters[$name] : array();
+ $success &= $block->onSubmit( $data );
}
return $success ? $interestedBlocks : array();
diff --git a/includes/View.php b/includes/View.php
index 604964d..dc3d6e2 100644
--- a/includes/View.php
+++ b/includes/View.php
@@ -81,9 +81,6 @@
$user = $this->getUser();
$blocks = $loader->createBlocks();
- foreach ( $blocks as $block ) {
- $block->init( $action, $user );
- }
wfProfileOut( __CLASS__ . '-init' );
$parameters = $this->extractBlockParameters( $action, $request,
$blocks );
@@ -91,7 +88,7 @@
$wasPosted = $request->wasPosted();
if ( $wasPosted ) {
wfProfileIn( __CLASS__ . '-submit' );
- $blocksToCommit = $loader->handleSubmit( $action,
$blocks, $user, $parameters );
+ $blocksToCommit = $loader->handleSubmit( $this,
$blocks, $action, $parameters );
if ( $blocksToCommit ) {
if ( !$user->matchEditToken( $request->getVal(
'wpEditToken' ) ) ) {
// only render the failed blocks
@@ -107,6 +104,10 @@
}
}
wfProfileOut( __CLASS__ . '-submit' );
+ } else {
+ foreach ( $blocks as $block ) {
+ $block->init( $this, $action );
+ }
}
wfProfileIn( __CLASS__ . '-serialize' );
diff --git a/includes/WorkflowLoader.php b/includes/WorkflowLoader.php
index 7d09501..dcf652e 100644
--- a/includes/WorkflowLoader.php
+++ b/includes/WorkflowLoader.php
@@ -4,6 +4,7 @@
use Flow\Block\AbstractBlock;
use Flow\Model\Workflow;
+use IContextSource;
class WorkflowLoader {
/**
@@ -51,14 +52,15 @@
}
/**
- * @param string $action
+ * @param IContextSource $context
* @param array $blocks
- * @param mixed $user
- * @param array $params
+ * @param string $action
+ * @param array $parameters
* @return Block\AbstractBlock[]
*/
- public function handleSubmit( $action, array $blocks, $user, array
$params ) {
- return $this->submissionHandler->handleSubmit( $this->workflow,
$action, $blocks, $user, $params );
+ public function handleSubmit( IContextSource $context, array $blocks,
$action, array $parameters ) {
+ return $this->submissionHandler
+ ->handleSubmit( $this->workflow, $context, $blocks,
$action, $parameters );
}
public function commit( Workflow $workflow, array $blocks ) {
diff --git a/includes/api/ApiFlowBaseGet.php b/includes/api/ApiFlowBaseGet.php
index 606647a..2eca645 100644
--- a/includes/api/ApiFlowBaseGet.php
+++ b/includes/api/ApiFlowBaseGet.php
@@ -7,8 +7,8 @@
public function execute() {
$loader = $this->getLoader();
$blocks = $loader->createBlocks();
+ $context = $this->getContext();
$action = $this->getAction();
- $user = $this->getUser();
$passedParams = $this->getBlockParams();
$output[$action] = array(
@@ -18,7 +18,7 @@
/** @var AbstractBlock $block */
foreach( $blocks as $block ) {
- $block->init( $action, $user );
+ $block->init( $context, $action );
if ( $block->canRender( $action ) ) {
$blockParams = array();
diff --git a/includes/api/ApiFlowBasePost.php b/includes/api/ApiFlowBasePost.php
index cc899c1..dd22fce 100644
--- a/includes/api/ApiFlowBasePost.php
+++ b/includes/api/ApiFlowBasePost.php
@@ -12,14 +12,14 @@
$action = $this->getAction();
$user = $this->getUser();
- /** @var AbstractBlock $block */
- foreach ( $blocks as $block ) {
- $block->init( $action, $user );
- }
$result = $this->getResult();
-
$params = $this->getBlockParams();
- $blocksToCommit = $loader->handleSubmit( $action, $blocks,
$user, $params );
+ $blocksToCommit = $loader->handleSubmit(
+ $this->getContext(),
+ $blocks,
+ $action,
+ $params
+ );
// See if any of the blocks generated an error (in which case
the
// request will terminate with an the error message)
diff --git a/includes/api/ApiQueryFlow.php b/includes/api/ApiQueryFlow.php
index 264da66..4951f68 100644
--- a/includes/api/ApiQueryFlow.php
+++ b/includes/api/ApiQueryFlow.php
@@ -49,7 +49,7 @@
'workflow' =>
$this->loader->getWorkflow()->getId()->getAlphadecimal()
);
foreach( $this->loader->createBlocks() as $block ) {
- $block->init( $params['action'], $this->getUser() );
+ $block->init( $this->getContext(), $params['action'] );
$blockParams = array();
if ( isset( $passedParams[$block->getName()] ) ) {
diff --git a/tests/phpunit/api/ApiFlowLockTopicTest.php
b/tests/phpunit/api/ApiFlowLockTopicTest.php
new file mode 100644
index 0000000..8337d6f
--- /dev/null
+++ b/tests/phpunit/api/ApiFlowLockTopicTest.php
@@ -0,0 +1,124 @@
+<?php
+
+namespace Flow\Tests\Api;
+
+use Flow\Container;
+use FlowHooks;
+use User;
+
+/**
+ * @group Flow
+ * @group medium
+ */
+class ApiFlowLockTopicTest extends \ApiTestCase {
+ /**
+ * @var array
+ */
+ protected $tablesUsed = array( 'flow_revision', 'flow_tree_revision' );
+
+ protected function setUp() {
+ $this->setMwGlobals( 'wgFlowOccupyPages', array( 'Talk:Flow QA'
) );
+ parent::setUp();
+ }
+
+ protected function getEditToken() {
+ $tokens = $this->getTokenList( self::$users['sysop'] );
+ return $tokens['edittoken'];
+ }
+
+ protected function doApiRequest(
+ array $params,
+ array $session = null,
+ $appendModule = false,
+ User $user = null
+ ) {
+ // reset flow state before each request
+ FlowHooks::resetFlowExtension();
+ Container::reset();
+ $container = Container::getContainer();
+ $container['user'] = $user ?: self::$users['sysop']->user;
+ return parent::doApiRequest( $params, $session, $appendModule,
$user );
+ }
+
+ public function testLockTopic() {
+ $workflowId = $this->createTopic();
+ $data = $this->doApiRequest( array(
+ 'page' => "Topic:$workflowId",
+ 'token' => $this->getEditToken(),
+ 'action' => 'flow',
+ 'submodule' => 'lock-topic',
+ 'cotmoderationState' => 'lock',
+ 'cotreason' => 'fiddle faddle',
+ 'cotprev_revision' => null,
+ ) );
+
+ $result = $data[0]['flow']['lock-topic']['result']['topic'];
+ $this->assertArrayHasKey( 'errors', $result );
+ $this->assertCount( 0, $result['errors'] );
+ $this->assertArrayHasKey( 'workflowId', $result );
+ $this->assertEquals( $workflowId, $result['workflowId'] );
+ $this->assertArrayHasKey( 'changeType', $result );
+ $this->assertEquals( 'lock-topic', $result['changeType'] );
+ $this->assertArrayHasKey( 'isModerated', $result );
+ $this->assertTrue( $result['isModerated'] );
+ $this->assertArrayHasKey( 'actions', $result );
+ $this->assertArrayHasKey( 'unlock', $result['actions'] );
+ $this->assertArrayHasKey( 'moderateReason', $result );
+ $this->assertEquals( 'fiddle faddle',
$result['moderateReason']['content'] );
+ $this->assertEquals( 'plaintext',
$result['moderateReason']['format'] );
+ }
+
+ public function testUnlockTopic() {
+ $workflowId = $this->createTopic();
+ $data = $this->doApiRequest( array(
+ 'page' => "Topic:$workflowId",
+ 'token' => $this->getEditToken(),
+ 'action' => 'flow',
+ 'submodule' => 'lock-topic',
+ 'cotmoderationState' => 'lock',
+ 'cotreason' => 'fiddle faddle',
+ ) );
+ $result = $data[0]['flow']['lock-topic']['result']['topic'];
+ $this->assertCount( 0, $result['errors'] );
+
+ $data = $this->doApiRequest( array(
+ 'page' => "Topic:$workflowId",
+ 'token' => $this->getEditToken(),
+ 'action' => 'flow',
+ 'submodule' => 'lock-topic',
+ 'cotmoderationState' => 'unlock',
+ 'cotreason' => 'Ether',
+ ) );
+
+ $result = $data[0]['flow']['lock-topic']['result']['topic'];
+ $this->assertArrayHasKey( 'errors', $result );
+ $this->assertCount( 0, $result['errors'] );
+ $this->assertArrayHasKey( 'changeType', $result );
+ $this->assertEquals( 'restore-topic', $result['changeType'] );
+ $this->assertArrayHasKey( 'isModerated', $result );
+ $this->assertFalse( $result['isModerated'] );
+ $this->assertArrayHasKey( 'actions', $result );
+ $this->assertArrayHasKey( 'lock', $result['actions'] );
+ // Is this intentional? We don't display it by default
+ // but perhaps it should still be in the api output.
+ $this->assertArrayNotHasKey( 'moderateReason', $result );
+ }
+
+ protected function createTopic() {
+ $data = $this->doApiRequest( array(
+ 'page' => 'Talk:Flow QA',
+ 'token' => $this->getEditToken(),
+ 'action' => 'flow',
+ 'submodule' => 'new-topic',
+ 'nttopic' => 'Hi there!',
+ 'ntcontent' => '...',
+ ) );
+ $this->assertTrue(
+ // @todo we should return the new id much more directly
than this
+ isset(
$data[0]['flow']['new-topic']['result']['topiclist']['roots'][0] ),
+ 'Api response must contain new topic id'
+ );
+
+ return
$data[0]['flow']['new-topic']['result']['topiclist']['roots'][0];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/164661
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia22eb12f2bfc5c5e3356a021b6df960fd26290cf
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: SG <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
