From 9e491982cfa156c129b4e8f93f50c9c66c45fe9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Mon, 7 Nov 2016 18:57:54 +0100 Subject: [PATCH] [Workflow] Made the code more robbust and ease on-boarding --- .../Compiler/ValidateWorkflowsPass.php | 6 ++-- .../FrameworkExtension.php | 6 ++-- .../Resources/config/workflow.xml | 4 +-- .../Component/Workflow/Tests/WorkflowTest.php | 8 +++-- .../Validator/StateMachineValidator.php | 2 +- src/Symfony/Component/Workflow/Workflow.php | 31 ++++++++----------- 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php index 4f1024e2c4..09ad3dd60e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php @@ -35,13 +35,13 @@ class ValidateWorkflowsPass implements CompilerPassInterface foreach ($taggedServices as $id => $tags) { $definition = $container->get($id); foreach ($tags as $tag) { - if (empty($tag['name'])) { + if (!array_key_exists('name', $tag)) { throw new RuntimeException(sprintf('The "name" for the tag "workflow.definition" of service "%s" must be set.', $id)); } - if (empty($tag['type'])) { + if (!array_key_exists('type', $tag)) { throw new RuntimeException(sprintf('The "type" for the tag "workflow.definition" of service "%s" must be set.', $id)); } - if (empty($tag['marking_store'])) { + if (!array_key_exists('marking_store', $tag)) { throw new RuntimeException(sprintf('The "marking_store" for the tag "workflow.definition" of service "%s" must be set.', $id)); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 3a1f0f029a..b7cdb631db 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -425,7 +425,7 @@ class FrameworkExtension extends Extension foreach ($workflow['marking_store']['arguments'] as $argument) { $markingStoreDefinition->addArgument($argument); } - } else { + } elseif (isset($workflow['marking_store']['service'])) { $markingStoreDefinition = new Reference($workflow['marking_store']['service']); } @@ -438,7 +438,9 @@ class FrameworkExtension extends Extension $workflowDefinition = new DefinitionDecorator(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, $definitionDefinition); - $workflowDefinition->replaceArgument(1, $markingStoreDefinition); + if (isset($markingStoreDefinition)) { + $workflowDefinition->replaceArgument(1, $markingStoreDefinition); + } $workflowDefinition->replaceArgument(3, $name); $workflowId = sprintf('%s.%s', $type, $name); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml index cb8c132fb0..1314be8b9f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml @@ -7,13 +7,13 @@ - + null - + null diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 6144de1166..4a7ea56e21 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -113,7 +113,9 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase $subject = new \stdClass(); $subject->marking = null; $eventDispatcher = new EventDispatcher(); - $eventDispatcher->addListener('workflow.workflow_name.guard.t1', function (GuardEvent $event) { $event->setBlocked(true); }); + $eventDispatcher->addListener('workflow.workflow_name.guard.t1', function (GuardEvent $event) { + $event->setBlocked(true); + }); $workflow = new Workflow($definition, new PropertyAccessorMarkingStore(), $eventDispatcher, 'workflow_name'); $this->assertFalse($workflow->can($subject, 't1')); @@ -188,7 +190,9 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase $subject = new \stdClass(); $subject->marking = null; $eventDispatcher = new EventDispatcher(); - $eventDispatcher->addListener('workflow.workflow_name.guard.t1', function (GuardEvent $event) { $event->setBlocked(true); }); + $eventDispatcher->addListener('workflow.workflow_name.guard.t1', function (GuardEvent $event) { + $event->setBlocked(true); + }); $workflow = new Workflow($definition, new PropertyAccessorMarkingStore(), $eventDispatcher, 'workflow_name'); $this->assertEmpty($workflow->getEnabledTransitions($subject)); diff --git a/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php b/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php index 57dc6322dc..6f34c1f2f4 100644 --- a/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php +++ b/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php @@ -53,7 +53,7 @@ class StateMachineValidator implements DefinitionValidatorInterface if (isset($transitionFromNames[$from][$transition->getName()])) { throw new InvalidDefinitionException( sprintf( - 'A transition from a place/state must have an unique name. Multiple transition named "%s" from place/state "%s" where found on StateMachine "%s". ', + 'A transition from a place/state must have an unique name. Multiple transitions named "%s" from place/state "%s" where found on StateMachine "%s". ', $transition->getName(), $from, $name diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 4e83778cf1..348b3b95a9 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -16,6 +16,7 @@ use Symfony\Component\Workflow\Event\Event; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\Exception\LogicException; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; +use Symfony\Component\Workflow\MarkingStore\PropertyAccessorMarkingStore; /** * @author Fabien Potencier @@ -29,10 +30,10 @@ class Workflow private $dispatcher; private $name; - public function __construct(Definition $definition, MarkingStoreInterface $markingStore, EventDispatcherInterface $dispatcher = null, $name = 'unnamed') + public function __construct(Definition $definition, MarkingStoreInterface $markingStore = null, EventDispatcherInterface $dispatcher = null, $name = 'unnamed') { $this->definition = $definition; - $this->markingStore = $markingStore; + $this->markingStore = $markingStore ?: new PropertyAccessorMarkingStore(); $this->dispatcher = $dispatcher; $this->name = $name; } @@ -163,7 +164,7 @@ class Workflow * @param Marking $marking * @param Transition $transition * - * @return bool|void boolean true if this transition is guarded, ie you cannot use it. + * @return bool|void boolean true if this transition is guarded, ie you cannot use it */ private function guardTransition($subject, Marking $marking, Transition $transition) { @@ -253,25 +254,21 @@ class Workflow { $transitions = $this->definition->getTransitions(); - $namedTransitions = array_filter( - $transitions, - function (Transition $transition) use ($transitionName) { - return $transitionName === $transition->getName(); - } - ); + $transitions = array_filter($transitions, function (Transition $transition) use ($transitionName) { + return $transitionName === $transition->getName(); + }); - if (empty($namedTransitions)) { - throw new LogicException( - sprintf('Transition "%s" does not exist for workflow "%s".', $transitionName, $this->name) - ); + if (!$transitions) { + throw new LogicException(sprintf('Transition "%s" does not exist for workflow "%s".', $transitionName, $this->name)); } - return $namedTransitions; + return $transitions; } /** - * Return the first Transition in $transitions that is valid for the $subject and $marking. null is returned when - * you cannot do any Transition in $transitions on the $subject. + * Return the first Transition in $transitions that is valid for the + * $subject and $marking. null is returned when you cannot do any Transition + * in $transitions on the $subject. * * @param object $subject * @param Marking $marking @@ -292,7 +289,5 @@ class Workflow return $transition; } } - - return; } }