From bdd3f95da64e6ed1279e530381dc51fe6eeae673 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 16 Aug 2016 13:13:16 +0200 Subject: [PATCH] Make the Workflow support State Machines --- .../Compiler/ValidateWorkflowsPass.php | 77 ++++++++++++ .../DependencyInjection/Configuration.php | 4 + .../FrameworkExtension.php | 24 +++- .../FrameworkBundle/FrameworkBundle.php | 2 + .../Resources/config/workflow.xml | 6 + src/Symfony/Component/Workflow/Definition.php | 5 +- .../Workflow/Dumper/GraphvizDumper.php | 11 +- .../Exception/InvalidDefinitionException.php | 21 ++++ .../MarkingStore/ScalarMarkingStore.php | 2 +- .../UniqueTransitionOutputInterface.php | 21 ---- .../Component/Workflow/StateMachine.php | 18 +++ .../Workflow/Tests/DefinitionTest.php | 2 +- .../Component/Workflow/Tests/RegistryTest.php | 8 +- .../Workflow/Tests/StateMachineTest.php | 75 ++++++++++++ .../SinglePlaceWorkflowValidatorTest.php | 33 ++++++ .../Validator/StateMachineValidatorTest.php | 108 +++++++++++++++++ .../Component/Workflow/Tests/WorkflowTest.php | 31 +---- .../DefinitionValidatorInterface.php | 31 +++++ .../SinglePlaceWorkflowValidator.php | 41 +++++++ .../Validator/StateMachineValidator.php | 68 +++++++++++ .../Workflow/Validator/WorkflowValidator.php | 24 ++++ src/Symfony/Component/Workflow/Workflow.php | 111 +++++++++++------- 22 files changed, 619 insertions(+), 104 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php create mode 100644 src/Symfony/Component/Workflow/Exception/InvalidDefinitionException.php delete mode 100644 src/Symfony/Component/Workflow/MarkingStore/UniqueTransitionOutputInterface.php create mode 100644 src/Symfony/Component/Workflow/StateMachine.php create mode 100644 src/Symfony/Component/Workflow/Tests/StateMachineTest.php create mode 100644 src/Symfony/Component/Workflow/Tests/Validator/SinglePlaceWorkflowValidatorTest.php create mode 100644 src/Symfony/Component/Workflow/Tests/Validator/StateMachineValidatorTest.php create mode 100644 src/Symfony/Component/Workflow/Validator/DefinitionValidatorInterface.php create mode 100644 src/Symfony/Component/Workflow/Validator/SinglePlaceWorkflowValidator.php create mode 100644 src/Symfony/Component/Workflow/Validator/StateMachineValidator.php create mode 100644 src/Symfony/Component/Workflow/Validator/WorkflowValidator.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php new file mode 100644 index 0000000000..4f1024e2c4 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php @@ -0,0 +1,77 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; +use Symfony\Component\Workflow\Validator\DefinitionValidatorInterface; +use Symfony\Component\Workflow\Validator\SinglePlaceWorkflowValidator; +use Symfony\Component\Workflow\Validator\StateMachineValidator; +use Symfony\Component\Workflow\Validator\WorkflowValidator; + +/** + * @author Tobias Nyholm + */ +class ValidateWorkflowsPass implements CompilerPassInterface +{ + /** + * @var DefinitionValidatorInterface[] + */ + private $validators = array(); + + public function process(ContainerBuilder $container) + { + $taggedServices = $container->findTaggedServiceIds('workflow.definition'); + foreach ($taggedServices as $id => $tags) { + $definition = $container->get($id); + foreach ($tags as $tag) { + if (empty($tag['name'])) { + throw new RuntimeException(sprintf('The "name" for the tag "workflow.definition" of service "%s" must be set.', $id)); + } + if (empty($tag['type'])) { + throw new RuntimeException(sprintf('The "type" for the tag "workflow.definition" of service "%s" must be set.', $id)); + } + if (empty($tag['marking_store'])) { + throw new RuntimeException(sprintf('The "marking_store" for the tag "workflow.definition" of service "%s" must be set.', $id)); + } + + $this->getValidator($tag)->validate($definition, $tag['name']); + } + } + } + + /** + * @param array $tag + * + * @return DefinitionValidatorInterface + */ + private function getValidator($tag) + { + if ($tag['type'] === 'state_machine') { + $name = 'state_machine'; + $class = StateMachineValidator::class; + } elseif ($tag['marking_store'] === 'scalar') { + $name = 'single_place'; + $class = SinglePlaceWorkflowValidator::class; + } else { + $name = 'workflow'; + $class = WorkflowValidator::class; + } + + if (empty($this->validators[$name])) { + $this->validators[$name] = new $class(); + } + + return $this->validators[$name]; + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index c17822a3bf..460e2a860f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -236,6 +236,10 @@ class Configuration implements ConfigurationInterface ->useAttributeAsKey('name') ->prototype('array') ->children() + ->enumNode('type') + ->values(array('workflow', 'state_machine')) + ->defaultValue('workflow') + ->end() ->arrayNode('marking_store') ->isRequired() ->children() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 1ea7441fad..3a1f0f029a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -404,10 +404,20 @@ class FrameworkExtension extends Extension $registryDefinition = $container->getDefinition('workflow.registry'); foreach ($workflows as $name => $workflow) { + $type = $workflow['type']; + $definitionDefinition = new Definition(Workflow\Definition::class); $definitionDefinition->addMethodCall('addPlaces', array($workflow['places'])); foreach ($workflow['transitions'] as $transitionName => $transition) { - $definitionDefinition->addMethodCall('addTransition', array(new Definition(Workflow\Transition::class, array($transitionName, $transition['from'], $transition['to'])))); + if ($type === 'workflow') { + $definitionDefinition->addMethodCall('addTransition', array(new Definition(Workflow\Transition::class, array($transitionName, $transition['from'], $transition['to'])))); + } elseif ($type === 'state_machine') { + foreach ($transition['from'] as $from) { + foreach ($transition['to'] as $to) { + $definitionDefinition->addMethodCall('addTransition', array(new Definition(Workflow\Transition::class, array($transitionName, $from, $to)))); + } + } + } } if (isset($workflow['marking_store']['type'])) { @@ -419,13 +429,21 @@ class FrameworkExtension extends Extension $markingStoreDefinition = new Reference($workflow['marking_store']['service']); } - $workflowDefinition = new DefinitionDecorator('workflow.abstract'); + $definitionDefinition->addTag('workflow.definition', array( + 'name' => $name, + 'type' => $type, + 'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null, + )); + $definitionDefinition->setPublic(false); + + $workflowDefinition = new DefinitionDecorator(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, $definitionDefinition); $workflowDefinition->replaceArgument(1, $markingStoreDefinition); $workflowDefinition->replaceArgument(3, $name); - $workflowId = 'workflow.'.$name; + $workflowId = sprintf('%s.%s', $type, $name); + $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); $container->setDefinition($workflowId, $workflowDefinition); foreach ($workflow['supports'] as $supportedClass) { diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index a736c091a1..72873aa6e9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -35,6 +35,7 @@ use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\TranslationDumpe use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\SerializerPass; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\UnusedTagsPass; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ConfigCachePass; +use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ValidateWorkflowsPass; use Symfony\Component\Debug\ErrorHandler; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\PassConfig; @@ -93,6 +94,7 @@ class FrameworkBundle extends Bundle $container->addCompilerPass(new PropertyInfoPass()); $container->addCompilerPass(new ControllerArgumentValueResolverPass()); $container->addCompilerPass(new CachePoolPass()); + $container->addCompilerPass(new ValidateWorkflowsPass()); $container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING); if ($container->getParameter('kernel.debug')) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml index d37b5d3e06..cb8c132fb0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.xml @@ -11,6 +11,12 @@ + + + + + + diff --git a/src/Symfony/Component/Workflow/Definition.php b/src/Symfony/Component/Workflow/Definition.php index 11c04ef524..52e1d1ee4c 100644 --- a/src/Symfony/Component/Workflow/Definition.php +++ b/src/Symfony/Component/Workflow/Definition.php @@ -46,6 +46,9 @@ class Definition return $this->places; } + /** + * @return Transition[] + */ public function getTransitions() { return $this->transitions; @@ -103,6 +106,6 @@ class Definition } } - $this->transitions[$name] = $transition; + $this->transitions[] = $transition; } } diff --git a/src/Symfony/Component/Workflow/Dumper/GraphvizDumper.php b/src/Symfony/Component/Workflow/Dumper/GraphvizDumper.php index 56bbef64ad..916c4bd460 100644 --- a/src/Symfony/Component/Workflow/Dumper/GraphvizDumper.php +++ b/src/Symfony/Component/Workflow/Dumper/GraphvizDumper.php @@ -83,9 +83,10 @@ class GraphvizDumper implements DumperInterface { $transitions = array(); - foreach ($definition->getTransitions() as $name => $transition) { - $transitions[$name] = array( + foreach ($definition->getTransitions() as $transition) { + $transitions[] = array( 'attributes' => array('shape' => 'box', 'regular' => true), + 'name' => $transition->getName(), ); } @@ -111,10 +112,10 @@ class GraphvizDumper implements DumperInterface { $code = ''; - foreach ($transitions as $id => $place) { + foreach ($transitions as $place) { $code .= sprintf(" transition_%s [label=\"%s\", shape=box%s];\n", - $this->dotize($id), - $id, + $this->dotize($place['name']), + $place['name'], $this->addAttributes($place['attributes']) ); } diff --git a/src/Symfony/Component/Workflow/Exception/InvalidDefinitionException.php b/src/Symfony/Component/Workflow/Exception/InvalidDefinitionException.php new file mode 100644 index 0000000000..1f55a49ab3 --- /dev/null +++ b/src/Symfony/Component/Workflow/Exception/InvalidDefinitionException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Exception; + +/** + * Thrown by the DefinitionValidatorInterface when the definition is invalid. + * + * @author Tobias Nyholm + */ +class InvalidDefinitionException extends \LogicException implements ExceptionInterface +{ +} diff --git a/src/Symfony/Component/Workflow/MarkingStore/ScalarMarkingStore.php b/src/Symfony/Component/Workflow/MarkingStore/ScalarMarkingStore.php index c76fb4081c..1c7a36093b 100644 --- a/src/Symfony/Component/Workflow/MarkingStore/ScalarMarkingStore.php +++ b/src/Symfony/Component/Workflow/MarkingStore/ScalarMarkingStore.php @@ -20,7 +20,7 @@ use Symfony\Component\Workflow\Marking; * * @author Grégoire Pineau */ -class ScalarMarkingStore implements MarkingStoreInterface, UniqueTransitionOutputInterface +class ScalarMarkingStore implements MarkingStoreInterface { private $property; private $propertyAccessor; diff --git a/src/Symfony/Component/Workflow/MarkingStore/UniqueTransitionOutputInterface.php b/src/Symfony/Component/Workflow/MarkingStore/UniqueTransitionOutputInterface.php deleted file mode 100644 index 35c00eb58d..0000000000 --- a/src/Symfony/Component/Workflow/MarkingStore/UniqueTransitionOutputInterface.php +++ /dev/null @@ -1,21 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Workflow\MarkingStore; - -/** - * UniqueTransitionOutputInterface. - * - * @author Grégoire Pineau - */ -interface UniqueTransitionOutputInterface -{ -} diff --git a/src/Symfony/Component/Workflow/StateMachine.php b/src/Symfony/Component/Workflow/StateMachine.php new file mode 100644 index 0000000000..0c4e3edc0a --- /dev/null +++ b/src/Symfony/Component/Workflow/StateMachine.php @@ -0,0 +1,18 @@ + + */ +class StateMachine extends Workflow +{ + public function __construct(Definition $definition, MarkingStoreInterface $markingStore = null, EventDispatcherInterface $dispatcher = null, $name = 'unnamed') + { + parent::__construct($definition, $markingStore ?: new ScalarMarkingStore(), $dispatcher, $name); + } +} diff --git a/src/Symfony/Component/Workflow/Tests/DefinitionTest.php b/src/Symfony/Component/Workflow/Tests/DefinitionTest.php index 9693d01fa3..09e594233e 100644 --- a/src/Symfony/Component/Workflow/Tests/DefinitionTest.php +++ b/src/Symfony/Component/Workflow/Tests/DefinitionTest.php @@ -55,7 +55,7 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase $definition = new Definition($places, array($transition)); $this->assertCount(1, $definition->getTransitions()); - $this->assertSame($transition, $definition->getTransitions()['name']); + $this->assertSame($transition, $definition->getTransitions()[0]); } /** diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 41bfbab8b9..42fa8c4ac7 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -18,9 +18,9 @@ class RegistryTest extends \PHPUnit_Framework_TestCase $this->registry = new Registry(); - $this->registry->add(new Workflow(new Definition(), $this->getMock(MarkingStoreInterface::class), $this->getMock(EventDispatcherInterface::class), 'workflow1'), Subject1::class); - $this->registry->add(new Workflow(new Definition(), $this->getMock(MarkingStoreInterface::class), $this->getMock(EventDispatcherInterface::class), 'workflow2'), Subject2::class); - $this->registry->add(new Workflow(new Definition(), $this->getMock(MarkingStoreInterface::class), $this->getMock(EventDispatcherInterface::class), 'workflow3'), Subject2::class); + $this->registry->add(new Workflow(new Definition(), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), Subject1::class); + $this->registry->add(new Workflow(new Definition(), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), Subject2::class); + $this->registry->add(new Workflow(new Definition(), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), Subject2::class); } protected function tearDown() @@ -55,7 +55,7 @@ class RegistryTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException Symfony\Component\Workflow\Exception\InvalidArgumentException + * @expectedException \Symfony\Component\Workflow\Exception\InvalidArgumentException * @expectedExceptionMessage Unable to find a workflow for class "stdClass". */ public function testGetWithNoMatch() diff --git a/src/Symfony/Component/Workflow/Tests/StateMachineTest.php b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php new file mode 100644 index 0000000000..6aa3c60fc1 --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/StateMachineTest.php @@ -0,0 +1,75 @@ +marking = 'a'; + $this->assertTrue($net->can($subject, 't1')); + $subject->marking = 'd'; + $this->assertTrue($net->can($subject, 't1')); + + $subject->marking = 'b'; + $this->assertFalse($net->can($subject, 't1')); + + // The graph looks like: + // + // +-------------------------------+ + // v | + // +---+ +----+ +----+ +----+ +---+ +----+ + // | a | --> | t1 | --> | b | --> | t3 | --> | d | --> | t1 | + // +---+ +----+ +----+ +----+ +---+ +----+ + // | + // | + // v + // +----+ +----+ + // | t2 | --> | c | + // +----+ +----+ + } + + public function testCanWithMultipleTransition() + { + $places = array('a', 'b', 'c'); + $transitions[] = new Transition('t1', 'a', 'b'); + $transitions[] = new Transition('t2', 'a', 'c'); + $definition = new Definition($places, $transitions); + + $net = new StateMachine($definition); + $subject = new \stdClass(); + + // If you are in place "a" you should be able to apply "t1" and "t2" + $subject->marking = 'a'; + $this->assertTrue($net->can($subject, 't1')); + $this->assertTrue($net->can($subject, 't2')); + + // The graph looks like: + // + // +----+ +----+ +---+ + // | a | --> | t1 | --> | b | + // +----+ +----+ +---+ + // | + // | + // v + // +----+ +----+ + // | t2 | --> | c | + // +----+ +----+ + } +} diff --git a/src/Symfony/Component/Workflow/Tests/Validator/SinglePlaceWorkflowValidatorTest.php b/src/Symfony/Component/Workflow/Tests/Validator/SinglePlaceWorkflowValidatorTest.php new file mode 100644 index 0000000000..db433ff0b6 --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/Validator/SinglePlaceWorkflowValidatorTest.php @@ -0,0 +1,33 @@ +createComplexWorkflow(); + + (new SinglePlaceWorkflowValidator())->validate($definition, 'foo'); + } + + public function testSinglePlaceWorkflowValidatorAndSimpleWorkflow() + { + $places = array('a', 'b'); + $transition = new Transition('t1', 'a', 'b'); + $definition = new Definition($places, array($transition)); + + (new SinglePlaceWorkflowValidator())->validate($definition, 'foo'); + } +} diff --git a/src/Symfony/Component/Workflow/Tests/Validator/StateMachineValidatorTest.php b/src/Symfony/Component/Workflow/Tests/Validator/StateMachineValidatorTest.php new file mode 100644 index 0000000000..17a790f063 --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/Validator/StateMachineValidatorTest.php @@ -0,0 +1,108 @@ +validate($definition, 'foo'); + + // The graph looks like: + // + // +----+ +----+ +---+ + // | a | --> | t1 | --> | b | + // +----+ +----+ +---+ + // | + // | + // v + // +----+ +----+ + // | t1 | --> | c | + // +----+ +----+ + } + + /** + * @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException + * @expectedExceptionMessage A transition in StateMachine can only have one output. + */ + public function testWithMultipleTos() + { + $places = array('a', 'b', 'c'); + $transitions[] = new Transition('t1', 'a', array('b', 'c')); + $definition = new Definition($places, $transitions); + + (new StateMachineValidator())->validate($definition, 'foo'); + + // The graph looks like: + // + // +---+ +----+ +---+ + // | a | --> | t1 | --> | b | + // +---+ +----+ +---+ + // | + // | + // v + // +----+ + // | c | + // +----+ + } + + /** + * @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException + * @expectedExceptionMessage A transition in StateMachine can only have one input. + */ + public function testWithMultipleFroms() + { + $places = array('a', 'b', 'c'); + $transitions[] = new Transition('t1', array('a', 'b'), 'c'); + $definition = new Definition($places, $transitions); + + (new StateMachineValidator())->validate($definition, 'foo'); + + // The graph looks like: + // + // +---+ +----+ +---+ + // | a | --> | t1 | --> | c | + // +---+ +----+ +---+ + // ^ + // | + // | + // +----+ + // | b | + // +----+ + } + + public function testValid() + { + $places = array('a', 'b', 'c'); + $transitions[] = new Transition('t1', 'a', 'b'); + $transitions[] = new Transition('t2', 'a', 'c'); + $definition = new Definition($places, $transitions); + + (new StateMachineValidator())->validate($definition, 'foo'); + + // The graph looks like: + // + // +----+ +----+ +---+ + // | a | --> | t1 | --> | b | + // +----+ +----+ +---+ + // | + // | + // v + // +----+ +----+ + // | t2 | --> | c | + // +----+ +----+ + } +} diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 22dc1927b8..6144de1166 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -8,32 +8,11 @@ use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; use Symfony\Component\Workflow\MarkingStore\PropertyAccessorMarkingStore; -use Symfony\Component\Workflow\MarkingStore\ScalarMarkingStore; use Symfony\Component\Workflow\Transition; use Symfony\Component\Workflow\Workflow; class WorkflowTest extends \PHPUnit_Framework_TestCase { - /** - * @expectedException \Symfony\Component\Workflow\Exception\LogicException - * @expectedExceptionMessage The marking store (Symfony\Component\Workflow\MarkingStore\ScalarMarkingStore) of workflow "unnamed" can not store many places. But the transition "t1" has too many output (2). Only one is accepted. - */ - public function testConstructorWithUniqueTransitionOutputInterfaceAndComplexWorkflow() - { - $definition = $this->createComplexWorkflow(); - - new Workflow($definition, new ScalarMarkingStore()); - } - - public function testConstructorWithUniqueTransitionOutputInterfaceAndSimpleWorkflow() - { - $places = array('a', 'b'); - $transition = new Transition('t1', 'a', 'b'); - $definition = new Definition($places, array($transition)); - - new Workflow($definition, new ScalarMarkingStore()); - } - /** * @expectedException \Symfony\Component\Workflow\Exception\LogicException * @expectedExceptionMessage The value returned by the MarkingStore is not an instance of "Symfony\Component\Workflow\Marking" for workflow "unnamed". @@ -42,7 +21,7 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase { $subject = new \stdClass(); $subject->marking = null; - $workflow = new Workflow(new Definition(), $this->getMock(MarkingStoreInterface::class)); + $workflow = new Workflow(new Definition(), $this->getMockBuilder(MarkingStoreInterface::class)->getMock()); $workflow->getMarking($subject); } @@ -217,16 +196,16 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase $subject->marking = array('d' => true); $transitions = $workflow->getEnabledTransitions($subject); $this->assertCount(2, $transitions); - $this->assertSame('t3', $transitions['t3']->getName()); - $this->assertSame('t4', $transitions['t4']->getName()); + $this->assertSame('t3', $transitions[0]->getName()); + $this->assertSame('t4', $transitions[1]->getName()); $subject->marking = array('c' => true, 'e' => true); $transitions = $workflow->getEnabledTransitions($subject); $this->assertCount(1, $transitions); - $this->assertSame('t5', $transitions['t5']->getName()); + $this->assertSame('t5', $transitions[0]->getName()); } - private function createComplexWorkflow() + protected function createComplexWorkflow() { $definition = new Definition(); diff --git a/src/Symfony/Component/Workflow/Validator/DefinitionValidatorInterface.php b/src/Symfony/Component/Workflow/Validator/DefinitionValidatorInterface.php new file mode 100644 index 0000000000..09fbc706c7 --- /dev/null +++ b/src/Symfony/Component/Workflow/Validator/DefinitionValidatorInterface.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Validator; + +use Symfony\Component\Workflow\Definition; +use Symfony\Component\Workflow\Exception\InvalidDefinitionException; + +/** + * @author Tobias Nyholm + */ +interface DefinitionValidatorInterface +{ + /** + * @param Definition $definition + * @param string $name + * + * @return bool + * + * @throws InvalidDefinitionException on invalid definition + */ + public function validate(Definition $definition, $name); +} diff --git a/src/Symfony/Component/Workflow/Validator/SinglePlaceWorkflowValidator.php b/src/Symfony/Component/Workflow/Validator/SinglePlaceWorkflowValidator.php new file mode 100644 index 0000000000..6b59db507d --- /dev/null +++ b/src/Symfony/Component/Workflow/Validator/SinglePlaceWorkflowValidator.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Validator; + +use Symfony\Component\Workflow\Definition; +use Symfony\Component\Workflow\Exception\InvalidDefinitionException; + +/** + * If the marking can contain only one place, we should control the definition. + * + * @author Tobias Nyholm + */ +class SinglePlaceWorkflowValidator extends WorkflowValidator +{ + public function validate(Definition $definition, $name) + { + foreach ($definition->getTransitions() as $transition) { + if (1 < count($transition->getTos())) { + throw new InvalidDefinitionException( + sprintf( + 'The marking store of workflow "%s" can not store many places. But the transition "%s" has too many output (%d). Only one is accepted.', + $name, + $transition->getName(), + count($transition->getTos()) + ) + ); + } + } + + return parent::validate($definition, $name); + } +} diff --git a/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php b/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php new file mode 100644 index 0000000000..57dc6322dc --- /dev/null +++ b/src/Symfony/Component/Workflow/Validator/StateMachineValidator.php @@ -0,0 +1,68 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Validator; + +use Symfony\Component\Workflow\Definition; +use Symfony\Component\Workflow\Exception\InvalidDefinitionException; + +/** + * @author Tobias Nyholm + */ +class StateMachineValidator implements DefinitionValidatorInterface +{ + public function validate(Definition $definition, $name) + { + $transitionFromNames = array(); + foreach ($definition->getTransitions() as $transition) { + // Make sure that each transition has exactly one TO + if (1 !== count($transition->getTos())) { + throw new InvalidDefinitionException( + sprintf( + 'A transition in StateMachine can only have one output. But the transition "%s" in StateMachine "%s" has %d outputs.', + $transition->getName(), + $name, + count($transition->getTos()) + ) + ); + } + + // Make sure that each transition has exactly one FROM + $froms = $transition->getFroms(); + if (1 !== count($froms)) { + throw new InvalidDefinitionException( + sprintf( + 'A transition in StateMachine can only have one input. But the transition "%s" in StateMachine "%s" has %d inputs.', + $transition->getName(), + $name, + count($transition->getTos()) + ) + ); + } + + // Enforcing uniqueness of the names of transitions starting at each node + $from = reset($froms); + 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". ', + $transition->getName(), + $from, + $name + ) + ); + } + $transitionFromNames[$from][$transition->getName()] = true; + } + + return true; + } +} diff --git a/src/Symfony/Component/Workflow/Validator/WorkflowValidator.php b/src/Symfony/Component/Workflow/Validator/WorkflowValidator.php new file mode 100644 index 0000000000..7d4366fdfb --- /dev/null +++ b/src/Symfony/Component/Workflow/Validator/WorkflowValidator.php @@ -0,0 +1,24 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Validator; + +use Symfony\Component\Workflow\Definition; + +/** + * @author Tobias Nyholm + */ +class WorkflowValidator implements DefinitionValidatorInterface +{ + public function validate(Definition $definition, $name) + { + } +} diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 641fc9e8d8..4e83778cf1 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -16,11 +16,11 @@ 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\UniqueTransitionOutputInterface; /** * @author Fabien Potencier * @author Grégoire Pineau + * @author Tobias Nyholm */ class Workflow { @@ -35,15 +35,6 @@ class Workflow $this->markingStore = $markingStore; $this->dispatcher = $dispatcher; $this->name = $name; - - // If the marking can contain only one place, we should control the definition - if ($markingStore instanceof UniqueTransitionOutputInterface) { - foreach ($definition->getTransitions() as $transition) { - if (1 < count($transition->getTos())) { - throw new LogicException(sprintf('The marking store (%s) of workflow "%s" can not store many places. But the transition "%s" has too many output (%d). Only one is accepted.', get_class($markingStore), $this->name, $transition->getName(), count($transition->getTos()))); - } - } - } } /** @@ -102,16 +93,10 @@ class Workflow */ public function can($subject, $transitionName) { - $transitions = $this->definition->getTransitions(); - - if (!isset($transitions[$transitionName])) { - throw new LogicException(sprintf('Transition "%s" does not exist for workflow "%s".', $transitionName, $this->name)); - } - - $transition = $transitions[$transitionName]; + $transitions = $this->getTransitions($transitionName); $marking = $this->getMarking($subject); - return $this->doCan($subject, $marking, $transition); + return null !== $this->getTransitionForSubject($subject, $marking, $transitions); } /** @@ -127,15 +112,13 @@ class Workflow */ public function apply($subject, $transitionName) { - if (!$this->can($subject, $transitionName)) { + $transitions = $this->getTransitions($transitionName); + $marking = $this->getMarking($subject); + + if (null === $transition = $this->getTransitionForSubject($subject, $marking, $transitions)) { throw new LogicException(sprintf('Unable to apply transition "%s" for workflow "%s".', $transitionName, $this->name)); } - // We can shortcut the getMarking method in order to boost performance, - // since the "can" method already checks the Marking state - $marking = $this->markingStore->getMarking($subject); - $transition = $this->definition->getTransitions()[$transitionName]; - $this->leave($subject, $transition, $marking); $this->transition($subject, $transition, $marking); @@ -162,8 +145,8 @@ class Workflow $marking = $this->getMarking($subject); foreach ($this->definition->getTransitions() as $transition) { - if ($this->doCan($subject, $marking, $transition)) { - $enabled[$transition->getName()] = $transition; + if (null !== $this->getTransitionForSubject($subject, $marking, array($transition))) { + $enabled[] = $transition; } } @@ -175,21 +158,13 @@ class Workflow return $this->name; } - private function doCan($subject, Marking $marking, Transition $transition) - { - foreach ($transition->getFroms() as $place) { - if (!$marking->has($place)) { - return false; - } - } - - if (true === $this->guardTransition($subject, $marking, $transition)) { - return false; - } - - return true; - } - + /** + * @param object $subject + * @param Marking $marking + * @param Transition $transition + * + * @return bool|void boolean true if this transition is guarded, ie you cannot use it. + */ private function guardTransition($subject, Marking $marking, Transition $transition) { if (null === $this->dispatcher) { @@ -263,9 +238,61 @@ class Workflow $event = new Event($subject, $marking, $initialTransition); foreach ($this->definition->getTransitions() as $transition) { - if ($this->doCan($subject, $marking, $transition)) { + if (null !== $this->getTransitionForSubject($subject, $marking, array($transition))) { $this->dispatcher->dispatch(sprintf('workflow.%s.announce.%s', $this->name, $transition->getName()), $event); } } } + + /** + * @param $transitionName + * + * @return Transition[] + */ + private function getTransitions($transitionName) + { + $transitions = $this->definition->getTransitions(); + + $namedTransitions = 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) + ); + } + + return $namedTransitions; + } + + /** + * 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 + * @param Transition[] $transitions + * + * @return Transition|null + */ + private function getTransitionForSubject($subject, Marking $marking, array $transitions) + { + foreach ($transitions as $transition) { + foreach ($transition->getFroms() as $place) { + if (!$marking->has($place)) { + continue 2; + } + } + + if (true !== $this->guardTransition($subject, $marking, $transition)) { + return $transition; + } + } + + return; + } }