EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/142399
Change subject: Split WorkflowLoader into multiple classes
......................................................................
Split WorkflowLoader into multiple classes
While trying to figure out how to test blocked users going through
WorkflowLoader I realized the loader does too many disparate things.
This patch splits it into discrete pieces that better match SRP. In
addition previously WorkflowLoader was doing a fair bit of work in the
constructor, going all the way to the database. Constructors should
have minimal setup so moved that into WorkflowLoaderFactory
At this point WorkflowLoader is now just a shell, consider further
refactors.
Note that current intention is to change the forms to use the same
parameters as API requests expect, and change the backend page rendering
process such that it makes an internal API call and passes those api results
to lightncandy. At that point it may make sense to delete the WorkflowLoader
and use the individual pieces within FlowApiBase or some such.
This was started against frontend-rewrite branch, but is now against
the master branch.
Original gerrit change: I22a36a55e9b4704932e93d2f117a79fa1ea146a6
Change-Id: Id38a93bed37fcf8b91b815121f7c0537f25a525e
---
M Flow.php
M container.php
A includes/BlockFactory.php
A includes/SubmissionHandler.php
M includes/WorkflowLoader.php
A includes/WorkflowLoaderFactory.php
A tests/BlockFactoryTest.php
D tests/WorkflowLoaderTest.php
8 files changed, 506 insertions(+), 398 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/99/142399/1
diff --git a/Flow.php b/Flow.php
index 498c468..537f9e1 100755
--- a/Flow.php
+++ b/Flow.php
@@ -76,8 +76,10 @@
$wgAutoloadClasses['Flow\Parsoid\ReferenceExtractor'] = $dir .
'includes/Parsoid/ReferenceExtractor.php';
$wgAutoloadClasses['Flow\UrlGenerator'] = $dir . 'includes/UrlGenerator.php';
$wgAutoloadClasses['Flow\View'] = $dir . 'includes/View.php';
+$wgAutoloadClasses['Flow\BlockFactory'] = $dir . 'includes/BlockFactory.php';
+$wgAutoloadClasses['Flow\SubmissionHandler'] = $dir .
'includes/SubmissionHandler.php';
$wgAutoloadClasses['Flow\WorkflowLoader'] = $dir .
'includes/WorkflowLoader.php';
-$wgAutoloadClasses['Flow\WorkflowLoaderFactory'] = $dir .
'includes/WorkflowLoader.php';
+$wgAutoloadClasses['Flow\WorkflowLoaderFactory'] = $dir .
'includes/WorkflowLoaderFactory.php';
$wgAutoloadClasses['Flow\OccupationController'] = $dir .
'includes/TalkpageManager.php';
$wgAutoloadClasses['Flow\TalkpageManager'] = $dir .
'includes/TalkpageManager.php';
$wgAutoloadClasses['Flow\NotificationFormatter'] = $dir .
'includes/Notifications/Formatter.php';
diff --git a/container.php b/container.php
index 3d4885c..88681c9 100644
--- a/container.php
+++ b/container.php
@@ -581,13 +581,28 @@
);
} );
-$c['factory.loader.workflow'] = $c->share( function( $c ) {
- return new Flow\WorkflowLoaderFactory(
- $c['db.factory'],
- $c['memcache.buffered'],
+$c['submission_handler'] = $c->share( function( $c ) {
+ return new Flow\SubmissionHandler(
$c['storage'],
- $c['loader.root_post'],
- $c['controller.notification']
+ $c['db.factory'],
+ $c['memcache.buffered']
+ );
+} );
+$c['factory.block'] = $c->share( function( $c ) {
+ return new Flow\BlockFactory(
+ $c['storage'],
+ $c['controller.notification'],
+ $c['loader.root_post']
+ );
+} );
+$c['factory.loader.workflow'] = $c->share( function( $c ) {
+ global $wgFlowDefaultWorkflow;
+
+ return new Flow\WorkflowLoaderFactory(
+ $c['storage'],
+ $c['factory.block'],
+ $c['submission_handler'],
+ $wgFlowDefaultWorkflow
);
} );
diff --git a/includes/BlockFactory.php b/includes/BlockFactory.php
new file mode 100644
index 0000000..bdfa291
--- /dev/null
+++ b/includes/BlockFactory.php
@@ -0,0 +1,71 @@
+<?php
+
+namespace Flow;
+
+use Flow\Block\AbstractBlock;
+use Flow\Block\HeaderBlock;
+use Flow\Block\TopicBlock;
+use Flow\Block\TopicListBlock;
+use Flow\Block\TopicSummaryBlock;
+use Flow\Block\BoardHistoryBlock;
+use Flow\Model\Definition;
+use Flow\Model\Workflow;
+use Flow\Data\ManagerGroup;
+use Flow\Data\RootPostLoader;
+use Flow\Exception\InvalidInputException;
+use Flow\Exception\InvalidDataException;
+
+class BlockFactory {
+
+ public function __construct(
+ ManagerGroup $storage,
+ NotificationController $notificationController,
+ RootPostLoader $rootPostLoader
+ ) {
+ $this->storage = $storage;
+ $this->notificationController = $notificationController;
+ $this->rootPostLoader = $rootPostLoader;
+ }
+
+ /**
+ * @param Definition $definition
+ * @param Workflow $workflow
+ * @return AbstractBlock[]
+ * @throws InvalidInputException When the definition type is
unrecognized
+ * @throws InvalidDataException When multiple blocks share the same name
+ */
+ public function createBlocks( Definition $definition, Workflow
$workflow ) {
+ switch( $definition->getType() ) {
+ case 'discussion':
+ $blocks = array(
+ new HeaderBlock( $workflow,
$this->storage, $this->notificationController ),
+ new TopicListBlock( $workflow,
$this->storage, $this->notificationController ),
+ new BoardHistoryBlock( $workflow,
$this->storage, $this->notificationController ),
+ );
+ break;
+
+ case 'topic':
+ $blocks = array(
+ new TopicBlock( $workflow,
$this->storage, $this->notificationController, $this->rootPostLoader ),
+ new TopicSummaryBlock( $workflow,
$this->storage, $this->notificationController, $this->rootPostLoader ),
+ );
+ break;
+
+ default:
+ throw new InvalidInputException( 'Not
Implemented', 'invalid-definition' );
+ break;
+ }
+
+ $return = array();
+ /** @var AbstractBlock[] $blocks */
+ foreach ( $blocks as $block ) {
+ if ( !isset( $return[$block->getName()] ) ) {
+ $return[$block->getName()] = $block;
+ } else {
+ throw new InvalidDataException( 'Multiple
blocks with same name is not yet supported', 'fail-load-data' );
+ }
+ }
+
+ return $return;
+ }
+}
diff --git a/includes/SubmissionHandler.php b/includes/SubmissionHandler.php
new file mode 100644
index 0000000..fcb8ba5
--- /dev/null
+++ b/includes/SubmissionHandler.php
@@ -0,0 +1,143 @@
+<?php
+
+namespace Flow;
+
+use Flow\Block\AbstractBlock;
+use Flow\Model\Workflow;
+use Flow\Data\BufferedCache;
+use Flow\Data\ManagerGroup;
+use Flow\Exception\InvalidDataException;
+use Flow\Exception\InvalidActionException;
+use WebRequest;
+
+class SubmissionHandler {
+
+ public function __construct( ManagerGroup $storage, DbFactory
$dbFactory, BufferedCache $bufferedCache ) {
+ $this->storage = $storage;
+ $this->dbFactory = $dbFactory;
+ $this->bufferedCache = $bufferedCache;
+ }
+
+ /**
+ * @param Workflow $workflow
+ * @param string $action
+ * @param AbstractBlock[] $blocks
+ * @param \User $user
+ * @param WebRequest $request
+ * @return AbstractBlock[]
+ * @throws InvalidActionException
+ * @throws InvalidDataException
+ */
+ public function handleSubmit( Workflow $workflow, $action, array
$blocks, $user, WebRequest $request ) {
+ $success = true;
+ $interestedBlocks = array();
+
+ $params = $this->extractBlockParameters( $request, $blocks );
+ foreach ( $blocks as $block ) {
+ $data = $params[$block->getName()];
+ $result = $block->onSubmit( $action, $user, $data );
+ if ( $result !== null ) {
+ $interestedBlocks[] = $block;
+ $success &= $result;
+ }
+ }
+
+ if ( !$interestedBlocks ) {
+ if ( !$blocks ) {
+ throw new InvalidDataException( 'No Blocks?!?',
'fail-load-data' );
+ }
+ $type = array();
+ foreach ( $blocks as $block ) {
+ $type[] = get_class( $block );
+ }
+ // All blocks returned null, nothing knows how to
handle this action
+ throw new InvalidActionException( "No block accepted
the '$action' action: " . implode( ',', array_unique( $type ) ),
'invalid-action' );
+ }
+
+ // Check permissions before allowing any writes
+ if ( $user->isBlocked() ||
+ !$workflow->getArticleTitle()->userCan( 'edit', $user )
+ ) {
+ reset( $interestedBlocks )->addError( 'permissions',
wfMessage( 'flow-error-not-allowed' ) );
+ $success = false;
+ }
+
+ return $success ? $interestedBlocks : array();
+ }
+
+ /**
+ * @param Workflow $workflow
+ * @param AbstractBlock[] $blocks
+ * @return array
+ * @throws \Exception
+ */
+ public function commit( Workflow $workflow, array $blocks ) {
+ $cache = $this->bufferedCache;
+ $dbw = $this->dbFactory->getDB( DB_MASTER );
+
+ try {
+ $dbw->begin();
+ $cache->begin();
+ // @todo doesn't feel right to have this here
+ $this->storage->getStorage( 'Workflow' )->put(
$workflow );
+ $results = array();
+ foreach ( $blocks as $block ) {
+ $results[$block->getName()] = $block->commit();
+ }
+ $dbw->commit();
+ } catch ( \Exception $e ) {
+ $dbw->rollback();
+ $cache->rollback();
+ throw $e;
+ }
+
+ try {
+ $cache->commit();
+ } catch ( \Exception $e ) {
+ wfWarn( __METHOD__ . ': Commited to database but failed
applying to cache' );
+ \MWExceptionHandler::logException( $e );
+ }
+
+ return $results;
+ }
+
+ /**
+ * Helper function extracts parameters from a WebRequest.
+ s
+ * @param WebRequest $request
+ * @param AbstractBlock[] $blocks
+ * @return array
+ */
+ public function extractBlockParameters( WebRequest $request, array
$blocks ) {
+ $result = array();
+ // BC for old parameters enclosed in square brackets
+ foreach ( $blocks as $block ) {
+ $name = $block->getName();
+ $result[$name] = $request->getArray( $name, array() );
+ }
+ // BC for topic_list renamed to topiclist
+ if ( isset( $result['topiclist'] ) && !$result['topiclist'] ) {
+ $result['topiclist'] = $request->getArray(
'topic_list', array() );
+ }
+ // between urls only allowing [-_.] as unencoded special chars
and
+ // php mangling all of those into '_', we have to split on '_'
+ $globalData = array();
+ foreach ( $request->getValues() as $name => $value ) {
+ if ( false !== strpos( $name, '_' ) ) {
+ list( $block, $var ) = explode( '_', $name, 2 );
+ // flow_xxx is global data for all blocks
+ if ( $block === 'flow' ) {
+ $globalData[$var] = $value;
+ } else {
+ $result[$block][$var] = $value;
+ }
+ }
+ }
+
+ foreach ( $blocks as $block ) {
+ $result[$block->getName()] += $globalData;
+ }
+
+ return $result;
+ }
+}
diff --git a/includes/WorkflowLoader.php b/includes/WorkflowLoader.php
index b5ea745..d55280b 100644
--- a/includes/WorkflowLoader.php
+++ b/includes/WorkflowLoader.php
@@ -3,25 +3,11 @@
namespace Flow;
use Flow\Block\AbstractBlock;
-use Flow\Block\HeaderBlock;
-use Flow\Block\TopicBlock;
-use Flow\Block\TopicListBlock;
-use Flow\Block\TopicSummaryBlock;
-use Flow\Block\BoardHistoryBlock;
use Flow\Model\Definition;
-use Flow\Model\UUID;
use Flow\Model\Workflow;
-use Flow\Data\BufferedCache;
-use Flow\Data\ManagerGroup;
-use Flow\Data\RootPostLoader;
-use Flow\Exception\CrossWikiException;
-use Flow\Exception\InvalidInputException;
-use Flow\Exception\InvalidDataException;
-use Flow\Exception\InvalidActionException;
use WebRequest;
class WorkflowLoader {
- protected $dbFactory, $bufferedCache;
/**
* @var Workflow
*/
@@ -33,67 +19,36 @@
protected $definition;
/**
- * @var ManagerGroup
+ * @var BlockFactory
*/
- protected $storage;
+ protected $blockFactory;
/**
- * @var RootPostLoader
+ * @var SubmissionHandler
*/
- protected $rootPostLoader;
+ protected $submissionHandler;
/**
- * @var NotificationController
+ * @param Definition $definition
+ * @param Workflow $workflow
+ * @param BlockFactory $blockFactory
+ * @param SubmissionHandler $submissionHandler
*/
- protected $notificationController;
-
- /**
- * @var string
- */
- protected $definitionRequest;
-
public function __construct(
- $pageTitle,
- /*UUID or NULL*/ $workflowId,
- $definitionRequest,
- DbFactory $dbFactory,
- BufferedCache $bufferedCache,
- ManagerGroup $storage,
- RootPostLoader $rootPostLoader,
- NotificationController $notificationController
+ Definition $definition,
+ Workflow $workflow,
+ BlockFactory $blockFactory,
+ SubmissionHandler $submissionHandler
) {
- if ( $pageTitle === null ) {
- throw new InvalidInputException( 'Invalid article
requested', 'invalid-title' );
- }
-
- if ( $pageTitle && $pageTitle->isExternal() ) {
- throw new CrossWikiException( 'Interwiki to ' .
$pageTitle->getInterwiki() . ' not implemented ', 'default' );
- }
-
- $this->dbFactory = $dbFactory;
- $this->bufferedCache = $bufferedCache;
- $this->storage = $storage;
- $this->rootPostLoader = $rootPostLoader;
- $this->notificationController = $notificationController;
-
- $this->definitionRequest = $definitionRequest;
-
- $workflow = null;
-
- if ( $workflowId !== null ) {
- list( $workflow, $definition ) =
$this->loadWorkflowById( $pageTitle, $workflowId );
- } else {
- list( $workflow, $definition ) = $this->loadWorkflow(
$pageTitle );
- }
-
- if ( ! $workflow || ! $definition ) {
- throw new InvalidDataException( 'Unable to load
workflow and definition', 'fail-load-data' );
- }
-
- $this->workflow = $workflow;
+ $this->blockFactory = $blockFactory;
+ $this->submissionHandler = $submissionHandler;
$this->definition = $definition;
+ $this->workflow = $workflow;
}
+ /**
+ * @return Definition
+ */
public function getDefinition() {
return $this->definition;
}
@@ -105,253 +60,22 @@
return $this->workflow;
}
- protected function loadWorkflow( \Title $title ) {
- global $wgUser;
- $storage = $this->storage->getStorage( 'Workflow');
-
- $definition = $this->loadDefinition();
- if ( !$definition->getOption( 'unique' ) ) {
- throw new InvalidDataException( 'Workflow is
non-unique, can only fetch object by title + id', 'fail-load-data' );
- }
-
- $found = $storage->find( array(
- 'workflow_definition_id' => $definition->getId(),
- 'workflow_wiki' => $title->isLocal() ? wfWikiId() :
$title->getTransWikiID(),
- 'workflow_namespace' => $title->getNamespace(),
- 'workflow_title_text' => $title->getDBkey(),
- ) );
- if ( $found ) {
- $workflow = reset( $found );
- } else {
- $workflow = Workflow::create( $definition, $wgUser,
$title );
- }
-
- return array( $workflow, $definition );
- }
-
- protected function loadWorkflowById( /* Title or false */ $title,
$workflowId ) {
- $workflow = $this->storage->getStorage( 'Workflow' )->get(
$workflowId );
- if ( !$workflow ) {
- throw new InvalidInputException( 'Invalid workflow
requested by id', 'invalid-input' );
- }
- if ( $title !== false && !$workflow->matchesTitle( $title ) ) {
- throw new InvalidInputException( 'Flow workflow is for
different page', 'invalid-input' );
- }
- $definition = $this->storage->getStorage( 'Definition' )->get(
$workflow->getDefinitionId() );
- if ( !$definition ) {
- throw new InvalidInputException( 'Flow workflow
references unknown definition id: ' .
$workflow->getDefinitionId()->getAlphadecimal(), 'invalid-input' );
- }
-
- return array( $workflow, $definition );
- }
-
- protected function loadDefinition() {
- global $wgFlowDefaultWorkflow;
-
- $repo = $this->storage->getStorage( 'Definition' );
- $id = $this->definitionRequest;
- if ( $id instanceof UUID ) {
- $definition = $repo->get( $id );
- if ( $definition === null ) {
- throw new InvalidInputException( "Unknown flow
id '$id' requested", 'invalid-input' );
- }
- } else {
- $workflowName = $id ? $id : $wgFlowDefaultWorkflow;
- $found = $repo->find( array(
- 'definition_name' => strtolower( $workflowName
),
- 'definition_wiki' => wfWikiId(),
- ) );
- if ( $found ) {
- $definition = reset( $found );
- } else {
- throw new InvalidInputException( "Unknown flow
type '$workflowName' requested", 'invalid-input' );
- }
- }
- return $definition;
- }
-
/**
* @return AbstractBlock[]
- * @throws InvalidInputException When the definition type is
unrecognized
- * @throws InvalidDataException When multiple blocks share the same name
*/
public function createBlocks() {
- switch( $this->definition->getType() ) {
- case 'discussion':
- $blocks = array(
- new HeaderBlock( $this->workflow,
$this->storage, $this->notificationController ),
- new TopicListBlock( $this->workflow,
$this->storage, $this->notificationController ),
- new BoardHistoryBlock( $this->workflow,
$this->storage, $this->notificationController ),
- );
- break;
-
- case 'topic':
- $blocks = array(
- new TopicBlock( $this->workflow,
$this->storage, $this->notificationController, $this->rootPostLoader ),
- new TopicSummaryBlock( $this->workflow,
$this->storage, $this->notificationController, $this->rootPostLoader ),
- );
- break;
-
- default:
- throw new InvalidInputException( 'Not
Implemented', 'invalid-definition' );
- break;
- }
-
- $return = array();
- /** @var AbstractBlock[] $blocks */
- foreach ( $blocks as $block ) {
- if ( !isset( $return[$block->getName()] ) ) {
- $return[$block->getName()] = $block;
- } else {
- throw new InvalidDataException( 'Multiple
blocks with same name is not yet supported', 'fail-load-data' );
- }
- }
-
- return $return;
+ return $this->blockFactory->createBlocks( $this->definition,
$this->workflow );
}
- /**
- * @param string $action
- * @param AbstractBlock[] $blocks
- * @param \User $user
- * @param WebRequest $request
- * @return AbstractBlock[]
- * @throws InvalidActionException
- * @throws InvalidDataException
- */
public function handleSubmit( $action, array $blocks, $user, WebRequest
$request ) {
- $success = true;
- $interestedBlocks = array();
-
- $params = $this->extractBlockParameters( $request, $blocks );
- foreach ( $blocks as $block ) {
- $data = $params[$block->getName()];
- $result = $block->onSubmit( $action, $user, $data );
- if ( $result !== null ) {
- $interestedBlocks[] = $block;
- $success &= $result;
- }
- }
- if ( !$interestedBlocks ) {
- if ( !$blocks ) {
- throw new InvalidDataException( 'No Blocks?!?',
'fail-load-data' );
- }
- $type = array();
- foreach ( $blocks as $block ) {
- $type[] = get_class( $block );
- }
- // All blocks returned null, nothing knows how to
handle this action
- throw new InvalidActionException( "No block accepted
the '$action' action: " . implode( ',', array_unique( $type ) ),
'invalid-action' );
- }
-
- // Check permissions before allowing any writes
- if ( $user->isBlocked() ||
- !$this->workflow->getArticleTitle()->userCan( 'edit',
$user )
- ) {
- reset( $interestedBlocks )->addError( 'permissions',
wfMessage( 'flow-error-not-allowed' ) );
- $success = false;
- }
-
- return $success ? $interestedBlocks : array();
+ return $this->submissionHandler->handleSubmit( $this->workflow,
$action, $blocks, $user, $request );
}
- /**
- * @param Workflow $workflow
- * @param AbstractBlock[] $blocks
- * @return array
- * @throws \Exception
- */
public function commit( Workflow $workflow, array $blocks ) {
- $cache = $this->bufferedCache;
- $dbw = $this->dbFactory->getDB( DB_MASTER );
-
- try {
- $dbw->begin();
- $cache->begin();
- $this->storage->getStorage( 'Workflow' )->put(
$workflow );
- $results = array();
- foreach ( $blocks as $block ) {
- $results[$block->getName()] = $block->commit();
- }
- $dbw->commit();
- } catch ( \Exception $e ) {
- $dbw->rollback();
- $cache->rollback();
- throw $e;
- }
-
- try {
- $cache->commit();
- } catch ( \Exception $e ) {
- wfWarn( __METHOD__ . ': Commited to database but failed
applying to cache' );
- \MWExceptionHandler::logException( $e );
- }
-
- return $results;
+ return $this->submissionHandler->commit( $workflow, $blocks );
}
- /**
- * Helper function extracts something
- *
- * @param WebRequest $request
- * @param AbstractBlock[] $blocks
- * @return array
- */
public function extractBlockParameters( WebRequest $request, array
$blocks ) {
- $result = array();
- // BC for old parameters enclosed in square brackets
- foreach ( $blocks as $block ) {
- $name = $block->getName();
- $result[$name] = $request->getArray( $name, array() );
- }
- // BC for topic_list renamed to topiclist
- if ( isset( $result['topiclist'] ) && !$result['topiclist'] ) {
- $result['topiclist'] = $request->getArray(
'topic_list', array() );
- }
- // between urls only allowing [-_.] as unencoded special chars
and
- // php mangling all of those into '_', we have to split on '_'
- $globalData = array();
- foreach ( $request->getValues() as $name => $value ) {
- if ( false !== strpos( $name, '_' ) ) {
- list( $block, $var ) = explode( '_', $name, 2 );
- // flow_xxx is global data for all blocks
- if ( $block === 'flow' ) {
- $globalData[$var] = $value;
- } else {
- $result[$block][$var] = $value;
- }
- }
- }
-
- foreach ( $blocks as $block ) {
- $result[$block->getName()] += $globalData;
- }
-
- return $result;
- }
-}
-
-class WorkflowLoaderFactory {
- protected $storage, $rootPostLoader, $notificationController;
-
- function __construct( DbFactory $dbFactory, BufferedCache
$bufferedCache, ManagerGroup $storage, RootPostLoader $rootPostLoader,
NotificationController $notificationController ) {
- $this->dbFactory = $dbFactory;
- $this->bufferedCache = $bufferedCache;
- $this->storage = $storage;
- $this->rootPostLoader = $rootPostLoader;
- $this->notificationController = $notificationController;
- }
-
- public function createWorkflowLoader( $pageTitle, $workflowId = null,
$definitionRequest = false ) {
- return new WorkflowLoader(
- $pageTitle,
- $workflowId,
- $definitionRequest,
- $this->dbFactory,
- $this->bufferedCache,
- $this->storage,
- $this->rootPostLoader,
- $this->notificationController
- );
+ return $this->submissionHandler->extractBlockParameters(
$request, $blocks );
}
}
diff --git a/includes/WorkflowLoaderFactory.php
b/includes/WorkflowLoaderFactory.php
new file mode 100644
index 0000000..d6f1168
--- /dev/null
+++ b/includes/WorkflowLoaderFactory.php
@@ -0,0 +1,166 @@
+<?php
+
+namespace Flow;
+
+use Flow\Model\Definition;
+use Flow\Model\UUID;
+use Flow\Model\Workflow;
+use Flow\Data\ManagerGroup;
+use Flow\Exception\CrossWikiException;
+use Flow\Exception\InvalidInputException;
+use Flow\Exception\InvalidDataException;
+use Title;
+
+class WorkflowLoaderFactory {
+ /**
+ * @var ManagerGroup
+ */
+ protected $storage;
+
+ /**
+ * @var BlockFactory
+ */
+ protected $blockFactory;
+
+ /**
+ * @var SubmissionHandler
+ */
+ protected $submissionHandler;
+
+ /**
+ * @var string
+ */
+ protected $defaultWorkflowName;
+
+ /**
+ * @param ManagerGroup $storage
+ * @param BlockFactory $blockFactory
+ * @param SubmissionHandler $submissionHandler
+ * @param string $defaultWorkflowName
+ */
+ function __construct(
+ ManagerGroup $storage,
+ BlockFactory $blockFactory,
+ SubmissionHandler $submissionHandler,
+ $defaultWorkflowName
+ ) {
+ $this->storage = $storage;
+ $this->blockFactory = $blockFactory;
+ $this->submissionHandler = $submissionHandler;
+ $this->defaultWorkflowName = $defaultWorkflowName;
+ }
+
+ /**
+ * @param string $pageTitle
+ * @param UUID|string|null $workflowId
+ * @param string|false $definitionRequest
+ * @return WorkflowLoader
+ * @throws InvalidInputException
+ * @throws CrossWikiException
+ */
+ public function createWorkflowLoader( $pageTitle, $workflowId = null,
$definitionRequest = false ) {
+ if ( $pageTitle === null ) {
+ throw new InvalidInputException( 'Invalid article
requested', 'invalid-title' );
+ }
+
+ if ( $pageTitle && $pageTitle->isExternal() ) {
+ throw new CrossWikiException( 'Interwiki to ' .
$pageTitle->getInterwiki() . ' not implemented ', 'default' );
+ }
+
+ // @todo constructors should just do simple setup, this goes
out and hits the database
+ if ( $workflowId !== null ) {
+ list( $workflow, $definition ) =
$this->loadWorkflowById( $pageTitle, $workflowId );
+ } else {
+ list( $workflow, $definition ) = $this->loadWorkflow(
$pageTitle, $definitionRequest );
+ }
+
+ return new WorkflowLoader(
+ $definition,
+ $workflow,
+ $this->blockFactory,
+ $this->submissionHandler
+ );
+ }
+
+ /**
+ * @param Title $title
+ * @param string $definitionRequest
+ * @return array [Workflow, Definition]
+ * @throws InvalidDataException
+ */
+ protected function loadWorkflow( \Title $title, $definitionRequest ) {
+ global $wgUser;
+ $storage = $this->storage->getStorage( 'Workflow');
+
+ $definition = $this->loadDefinition( $definitionRequest );
+ if ( !$definition->getOption( 'unique' ) ) {
+ throw new InvalidDataException( 'Workflow is
non-unique, can only fetch object by title + id', 'fail-load-data' );
+ }
+
+ $found = $storage->find( array(
+ 'workflow_definition_id' => $definition->getId(),
+ 'workflow_wiki' => $title->isLocal() ? wfWikiId() :
$title->getTransWikiID(),
+ 'workflow_namespace' => $title->getNamespace(),
+ 'workflow_title_text' => $title->getDBkey(),
+ ) );
+ if ( $found ) {
+ $workflow = reset( $found );
+ } else {
+ $workflow = Workflow::create( $definition, $wgUser,
$title );
+ }
+
+ return array( $workflow, $definition );
+ }
+
+ /**
+ * @param Title|false $title
+ * @param string $workflowId
+ * @return array [Workflow, Definition]
+ * @throws InvalidInputException
+ */
+ protected function loadWorkflowById( /* Title or false */ $title,
$workflowId ) {
+ $workflow = $this->storage->getStorage( 'Workflow' )->get(
$workflowId );
+ if ( !$workflow ) {
+ throw new InvalidInputException( 'Invalid workflow
requested by id', 'invalid-input' );
+ }
+ if ( $title !== false && !$workflow->matchesTitle( $title ) ) {
+ throw new InvalidInputException( 'Flow workflow is for
different page', 'invalid-input' );
+ }
+ $definition = $this->storage->getStorage( 'Definition' )->get(
$workflow->getDefinitionId() );
+ if ( !$definition ) {
+ throw new InvalidInputException( 'Flow workflow
references unknown definition id: ' .
$workflow->getDefinitionId()->getAlphadecimal(), 'invalid-input' );
+ }
+
+ return array( $workflow, $definition );
+ }
+
+ /**
+ * @param string $id
+ * @return Definition
+ * @throws InvalidInputException
+ */
+ protected function loadDefinition( $id ) {
+ global $wgFlowDefaultWorkflow;
+
+ $repo = $this->storage->getStorage( 'Definition' );
+ if ( $id instanceof UUID ) {
+ $definition = $repo->get( $id );
+ if ( $definition === null ) {
+ throw new InvalidInputException( "Unknown flow
id '$id' requested", 'invalid-input' );
+ }
+ } else {
+ $workflowName = $id ? $id : $this->defaultWorkflowName;
+ $found = $repo->find( array(
+ 'definition_name' => strtolower( $workflowName
),
+ 'definition_wiki' => wfWikiId(),
+ ) );
+ if ( $found ) {
+ $definition = reset( $found );
+ } else {
+ throw new InvalidInputException( "Unknown flow
type '$workflowName' requested", 'invalid-input' );
+ }
+ }
+ return $definition;
+ }
+
+}
diff --git a/tests/BlockFactoryTest.php b/tests/BlockFactoryTest.php
new file mode 100644
index 0000000..51241c2
--- /dev/null
+++ b/tests/BlockFactoryTest.php
@@ -0,0 +1,80 @@
+<?php
+
+namespace Flow\Tests;
+
+use Flow\BlockFactory;
+use Flow\Container;
+use ReflectionClass;
+
+/**
+ * @group Flow
+ */
+class BlockFactoryTest extends FlowTestCase {
+
+ public function provideDataCreateBlocks() {
+ return array (
+ array( 'discussion', array( 'Flow\Block\HeaderBlock',
'Flow\Block\TopicListBlock', 'Flow\Block\BoardHistoryBlock' ) ),
+ array( 'topic', array( 'Flow\Block\TopicBlock',
'Flow\Block\TopicSummaryBlock' ) ),
+ );
+ }
+
+ /**
+ * @dataProvider provideDataCreateBlocks
+ */
+ public function testCreateBlocks( $definitionType, $expectedResults ) {
+ $factory = $this->createBlockFactory();
+ list( $definition, $workflow ) = $this->mockWorkflow(
$definitionType );
+
+ $blocks = $factory->createBlocks( $definition, $workflow );
+ $this->assertEquals( count( $blocks ), count( $expectedResults
) );
+
+ $results = array();
+ foreach ( $blocks as $obj ) {
+ $results[] = get_class( $obj );
+ }
+ $this->assertEquals( $results, $expectedResults );
+ }
+
+ /**
+ * @expectedException \Flow\Exception\InvalidInputException
+ */
+ public function testCreateBlocksWithInvalidInputException() {
+ $factory = $this->createBlockFactory();
+ list( $definition, $workflow ) = $this->mockWorkflow(
'a-bad-database-flow-definition' );
+ // Trigger InvalidInputException
+ $factory->createBlocks( $definition, $workflow );
+ }
+
+ protected function createBlockFactory() {
+ global $wgLang;
+
+ $storage = $this->getMockBuilder( '\Flow\Data\ManagerGroup' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ // phpunit mocker fails to generate the correct method
definition for
+ // NotificationController::getDefaultNotifiedUsers, just use
the real one
+ $notificationController = new \Flow\NotificationController(
$wgLang );
+
+ $rootPostLoader = $this->getMockBuilder(
'\Flow\Data\RootPostLoader' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ return new BlockFactory( $storage, $notificationController,
$rootPostLoader );
+ }
+
+ protected function mockWorkflow( $type ) {
+ $definition = $this->getMockBuilder( '\Flow\Model\Definition' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $definition->expects( $this->any() )
+ ->method( 'getType' )
+ ->will( $this->returnValue( $type ) );
+
+ $workflow = $this->getMockBuilder( '\Flow\Model\Workflow' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ return array( $definition, $workflow );
+ }
+}
diff --git a/tests/WorkflowLoaderTest.php b/tests/WorkflowLoaderTest.php
deleted file mode 100644
index 78af8df..0000000
--- a/tests/WorkflowLoaderTest.php
+++ /dev/null
@@ -1,93 +0,0 @@
-<?php
-
-namespace Flow\Tests;
-
-use Flow\Container;
-use ReflectionClass;
-
-/**
- * @group Flow
- */
-class WorkflowLoaderTest extends FlowTestCase {
-
- public function provideDataCreateBlocks() {
- return array (
- array( 'discussion', array( 'Flow\Block\HeaderBlock',
'Flow\Block\TopicListBlock', 'Flow\Block\BoardHistoryBlock' ) ),
- array( 'topic', array( 'Flow\Block\TopicBlock',
'Flow\Block\TopicSummaryBlock' ) ),
- );
- }
-
- /**
- * @dataProvider provideDataCreateBlocks
- */
- public function testCreateBlocks( $definitionType, $expectedResults ) {
- $workflowLoader = $this->mockWorkflowLoader( $definitionType );
- $blocks = $workflowLoader->createBlocks();
- $this->assertEquals( count( $blocks ), count( $expectedResults
) );
-
- $results = array();
- foreach ( $blocks as $obj ) {
- $results[] = get_class( $obj );
- }
- $this->assertEquals( $results, $expectedResults );
- }
-
- /**
- * @expectedException \Flow\Exception\InvalidInputException
- */
- public function testCreateBlocksWithInvalidInputException() {
- // Trigger InvalidInputException
- $workflowLoader = $this->mockWorkflowLoader(
'a-bad-database-flow-definition' );
- $workflowLoader->createBlocks();
- }
-
- /**
- * Create a WorkflowLoader mock object since we don't want any query
- * against the database
- *
- * @return \Flow\WorkflowLoader
- */
- protected function mockWorkflowLoader( $type ) {
- $definition = $this->getMockBuilder( '\Flow\Model\Definition' )
- ->disableOriginalConstructor()
- ->getMock();
- $definition->expects( $this->any() )
- ->method( 'getType' )
- ->will( $this->returnValue( $type ) );
-
- $workflow = $this->getMockBuilder( '\Flow\Model\Workflow' )
- ->disableOriginalConstructor()
- ->getMock();
-
- $methods = array_diff( get_class_methods(
'\Flow\WorkflowLoader' ), array( 'createBlocks' ) );
- $loader = $this->getMockBuilder( '\Flow\WorkflowLoader' )
- ->disableOriginalConstructor()
- ->setMethods( $methods )
- ->getMock();
-
- $reflection = new ReflectionClass( $loader );
-
- $property = $reflection->getProperty( 'definition' );
- $property->setAccessible( true );
- $property->setValue( $loader, $definition );
-
- $property = $reflection->getProperty( 'workflow' );
- $property->setAccessible( true );
- $property->setValue( $loader, $workflow );
-
- $property = $reflection->getProperty( 'storage' );
- $property->setAccessible( true );
- $property->setValue( $loader, Container::get( 'storage' ) );
-
- $property = $reflection->getProperty( 'notificationController'
);
- $property->setAccessible( true );
- $property->setValue( $loader, Container::get(
'controller.notification' ) );
-
- $property = $reflection->getProperty( 'rootPostLoader' );
- $property->setAccessible( true );
- $property->setValue( $loader, Container::get(
'loader.root_post' ) );
-
- return $loader;
- }
-
-}
--
To view, visit https://gerrit.wikimedia.org/r/142399
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id38a93bed37fcf8b91b815121f7c0537f25a525e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
