From ffaeba39fc0013539be4bd969e29540b1817dceb Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 8 Nov 2016 20:01:05 +0100 Subject: [PATCH] [Workflow] Added Definition builder --- .../Command/WorkflowDumpCommand.php | 14 +-- .../FrameworkExtension.php | 33 ++++--- src/Symfony/Component/Workflow/Definition.php | 36 ++++--- .../Component/Workflow/DefinitionBuilder.php | 93 +++++++++++++++++++ .../Workflow/Tests/DefinitionBuilderTest.php | 54 +++++++++++ .../Workflow/Tests/DefinitionTest.php | 12 +-- .../Tests/Dumper/GraphvizDumperTest.php | 36 +++---- .../Component/Workflow/Tests/RegistryTest.php | 6 +- .../Component/Workflow/Tests/WorkflowTest.php | 25 ++--- src/Symfony/Component/Workflow/Workflow.php | 8 ++ 10 files changed, 237 insertions(+), 80 deletions(-) create mode 100644 src/Symfony/Component/Workflow/DefinitionBuilder.php create mode 100644 src/Symfony/Component/Workflow/Tests/DefinitionBuilderTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.php index 14bb2ced1a..ddfb987ff1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.php +++ b/src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.php @@ -66,23 +66,13 @@ EOF throw new \InvalidArgumentException(sprintf('No service found for "workflow.%1$s" nor "state_machine.%1$s".', $serviceId)); } - $definition = $this->getProperty($workflow, 'definition'); - $dumper = new GraphvizDumper(); - $marking = new Marking(); + foreach ($input->getArgument('marking') as $place) { $marking->mark($place); } - $output->writeln($dumper->dump($definition, $marking)); - } - - private function getProperty($object, $property) - { - $reflectionProperty = new \ReflectionProperty(Workflow::class, $property); - $reflectionProperty->setAccessible(true); - - return $reflectionProperty->getValue($object); + $output->writeln($dumper->dump($workflow->getDefinition(), $marking)); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index b7cdb631db..732106c845 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -406,20 +406,32 @@ class FrameworkExtension extends Extension foreach ($workflows as $name => $workflow) { $type = $workflow['type']; - $definitionDefinition = new Definition(Workflow\Definition::class); - $definitionDefinition->addMethodCall('addPlaces', array($workflow['places'])); + // Create a DefinitionBuilder + $definitionBuilderDefinition = new Definition(Workflow\DefinitionBuilder::class); + $definitionBuilderDefinition->addMethodCall('addPlaces', array($workflow['places'])); foreach ($workflow['transitions'] as $transitionName => $transition) { if ($type === 'workflow') { - $definitionDefinition->addMethodCall('addTransition', array(new Definition(Workflow\Transition::class, array($transitionName, $transition['from'], $transition['to'])))); + $definitionBuilderDefinition->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)))); + $definitionBuilderDefinition->addMethodCall('addTransition', array(new Definition(Workflow\Transition::class, array($transitionName, $from, $to)))); } } } } + // Create a Definition + $definitionDefinition = new Definition(Workflow\Definition::class); + $definitionDefinition->setPublic(false); + $definitionDefinition->setFactory(array($definitionBuilderDefinition, 'build')); + $definitionDefinition->addTag('workflow.definition', array( + 'name' => $name, + 'type' => $type, + 'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null, + )); + + // Create MarkingStore if (isset($workflow['marking_store']['type'])) { $markingStoreDefinition = new DefinitionDecorator('workflow.marking_store.'.$workflow['marking_store']['type']); foreach ($workflow['marking_store']['arguments'] as $argument) { @@ -429,13 +441,7 @@ class FrameworkExtension extends Extension $markingStoreDefinition = new Reference($workflow['marking_store']['service']); } - $definitionDefinition->addTag('workflow.definition', array( - 'name' => $name, - 'type' => $type, - 'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null, - )); - $definitionDefinition->setPublic(false); - + // Create Workflow $workflowDefinition = new DefinitionDecorator(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, $definitionDefinition); if (isset($markingStoreDefinition)) { @@ -443,11 +449,12 @@ class FrameworkExtension extends Extension } $workflowDefinition->replaceArgument(3, $name); + // Store to container $workflowId = sprintf('%s.%s', $type, $name); - - $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); $container->setDefinition($workflowId, $workflowDefinition); + $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); + // Add workflow to Registry foreach ($workflow['supports'] as $supportedClass) { $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $supportedClass)); } diff --git a/src/Symfony/Component/Workflow/Definition.php b/src/Symfony/Component/Workflow/Definition.php index 52e1d1ee4c..6250bf57d2 100644 --- a/src/Symfony/Component/Workflow/Definition.php +++ b/src/Symfony/Component/Workflow/Definition.php @@ -17,8 +17,9 @@ use Symfony\Component\Workflow\Exception\LogicException; /** * @author Fabien Potencier * @author Grégoire Pineau + * @author Tobias Nyholm */ -class Definition +final class Definition { private $places = array(); private $transitions = array(); @@ -29,18 +30,28 @@ class Definition * * @param string[] $places * @param Transition[] $transitions + * @param string|null $initialPlace */ - public function __construct(array $places = array(), array $transitions = array()) + public function __construct(array $places, array $transitions, $initialPlace = null) { $this->addPlaces($places); - $this->addTransitions($transitions); + $this->setInitialPlace($initialPlace); + foreach ($transitions as $transition) { + $this->addTransition($transition); + } } + /** + * @return string|null + */ public function getInitialPlace() { return $this->initialPlace; } + /** + * @return string[] + */ public function getPlaces() { return $this->places; @@ -54,8 +65,12 @@ class Definition return $this->transitions; } - public function setInitialPlace($place) + private function setInitialPlace($place) { + if (null === $place) { + return; + } + if (!isset($this->places[$place])) { throw new LogicException(sprintf('Place "%s" cannot be the initial place as it does not exist.', $place)); } @@ -63,7 +78,7 @@ class Definition $this->initialPlace = $place; } - public function addPlace($place) + private function addPlace($place) { if (!preg_match('{^[\w\d_-]+$}', $place)) { throw new InvalidArgumentException(sprintf('The place "%s" contains invalid characters.', $place)); @@ -76,21 +91,14 @@ class Definition $this->places[$place] = $place; } - public function addPlaces(array $places) + private function addPlaces(array $places) { foreach ($places as $place) { $this->addPlace($place); } } - public function addTransitions(array $transitions) - { - foreach ($transitions as $transition) { - $this->addTransition($transition); - } - } - - public function addTransition(Transition $transition) + private function addTransition(Transition $transition) { $name = $transition->getName(); diff --git a/src/Symfony/Component/Workflow/DefinitionBuilder.php b/src/Symfony/Component/Workflow/DefinitionBuilder.php new file mode 100644 index 0000000000..447d67337d --- /dev/null +++ b/src/Symfony/Component/Workflow/DefinitionBuilder.php @@ -0,0 +1,93 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow; + +use Symfony\Component\Workflow\Exception\InvalidArgumentException; + +/** + * Builds a definition. + * + * @author Fabien Potencier + * @author Grégoire Pineau + * @author Tobias Nyholm + */ +class DefinitionBuilder +{ + private $places = array(); + private $transitions = array(); + private $initialPlace; + + /** + * @param string[] $places + * @param Transition[] $transitions + */ + public function __construct(array $places = array(), array $transitions = array()) + { + $this->addPlaces($places); + $this->addTransitions($transitions); + } + + /** + * @return Definition + */ + public function build() + { + return new Definition($this->places, $this->transitions, $this->initialPlace); + } + + /** + * Clear all data in the builder. + */ + public function reset() + { + $this->places = array(); + $this->transitions = array(); + $this->initialPlace = null; + } + + public function setInitialPlace($place) + { + $this->initialPlace = $place; + } + + public function addPlace($place) + { + if (!preg_match('{^[\w\d_-]+$}', $place)) { + throw new InvalidArgumentException(sprintf('The place "%s" contains invalid characters.', $place)); + } + + if (!$this->places) { + $this->initialPlace = $place; + } + + $this->places[$place] = $place; + } + + public function addPlaces(array $places) + { + foreach ($places as $place) { + $this->addPlace($place); + } + } + + public function addTransitions(array $transitions) + { + foreach ($transitions as $transition) { + $this->addTransition($transition); + } + } + + public function addTransition(Transition $transition) + { + $this->transitions[] = $transition; + } +} diff --git a/src/Symfony/Component/Workflow/Tests/DefinitionBuilderTest.php b/src/Symfony/Component/Workflow/Tests/DefinitionBuilderTest.php new file mode 100644 index 0000000000..fa4803b83d --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/DefinitionBuilderTest.php @@ -0,0 +1,54 @@ +setInitialPlace('b'); + $definition = $builder->build(); + + $this->assertEquals('b', $definition->getInitialPlace()); + } + + public function testAddTransition() + { + $places = range('a', 'b'); + + $transition0 = new Transition('name0', $places[0], $places[1]); + $transition1 = new Transition('name1', $places[0], $places[1]); + $builder = new DefinitionBuilder($places, array($transition0)); + $builder->addTransition($transition1); + + $definition = $builder->build(); + + $this->assertCount(2, $definition->getTransitions()); + $this->assertSame($transition0, $definition->getTransitions()[0]); + $this->assertSame($transition1, $definition->getTransitions()[1]); + } + + public function testAddPlace() + { + $builder = new DefinitionBuilder(array('a'), array()); + $builder->addPlace('b'); + + $definition = $builder->build(); + + $this->assertCount(2, $definition->getPlaces()); + $this->assertEquals('a', $definition->getPlaces()['a']); + $this->assertEquals('b', $definition->getPlaces()['b']); + } +} diff --git a/src/Symfony/Component/Workflow/Tests/DefinitionTest.php b/src/Symfony/Component/Workflow/Tests/DefinitionTest.php index 09e594233e..a0e7c9ed45 100644 --- a/src/Symfony/Component/Workflow/Tests/DefinitionTest.php +++ b/src/Symfony/Component/Workflow/Tests/DefinitionTest.php @@ -10,7 +10,7 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase public function testAddPlaces() { $places = range('a', 'e'); - $definition = new Definition($places); + $definition = new Definition($places, array()); $this->assertCount(5, $definition->getPlaces()); @@ -23,15 +23,13 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase public function testAddPlacesInvalidArgument() { $places = array('a"', 'e"'); - $definition = new Definition($places); + $definition = new Definition($places, array()); } public function testSetInitialPlace() { $places = range('a', 'e'); - $definition = new Definition($places); - - $definition->setInitialPlace($places[3]); + $definition = new Definition($places, array(), $places[3]); $this->assertEquals($places[3], $definition->getInitialPlace()); } @@ -42,9 +40,7 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase */ public function testSetInitialPlaceAndPlaceIsNotDefined() { - $definition = new Definition(); - - $definition->setInitialPlace('d'); + $definition = new Definition(array(), array(), 'd'); } public function testAddTransition() diff --git a/src/Symfony/Component/Workflow/Tests/Dumper/GraphvizDumperTest.php b/src/Symfony/Component/Workflow/Tests/Dumper/GraphvizDumperTest.php index d6e9cd30f3..f24bb003a8 100644 --- a/src/Symfony/Component/Workflow/Tests/Dumper/GraphvizDumperTest.php +++ b/src/Symfony/Component/Workflow/Tests/Dumper/GraphvizDumperTest.php @@ -2,7 +2,7 @@ namespace Symfony\Component\Workflow\Tests\Dumper; -use Symfony\Component\Workflow\Definition; +use Symfony\Component\Workflow\DefinitionBuilder; use Symfony\Component\Workflow\Dumper\GraphvizDumper; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Transition; @@ -39,7 +39,7 @@ class GraphvizDumperTest extends \PHPUnit_Framework_TestCase public function provideWorkflowDefinitionWithMarking() { yield array( - $this->createprovideComplexWorkflowDefinition(), + $this->provideComplexWorkflowDefinition(), new Marking(array('b' => 1)), $this->createComplexWorkflowDumpWithMarking(), ); @@ -53,36 +53,36 @@ class GraphvizDumperTest extends \PHPUnit_Framework_TestCase public function provideWorkflowDefinitionWithoutMarking() { - yield array($this->createprovideComplexWorkflowDefinition(), $this->provideComplexWorkflowDumpWithoutMarking()); + yield array($this->provideComplexWorkflowDefinition(), $this->provideComplexWorkflowDumpWithoutMarking()); yield array($this->provideSimpleWorkflowDefinition(), $this->provideSimpleWorkflowDumpWithoutMarking()); } - public function createprovideComplexWorkflowDefinition() + public function provideComplexWorkflowDefinition() { - $definition = new Definition(); + $builder = new DefinitionBuilder(); - $definition->addPlaces(range('a', 'g')); + $builder->addPlaces(range('a', 'g')); - $definition->addTransition(new Transition('t1', 'a', array('b', 'c'))); - $definition->addTransition(new Transition('t2', array('b', 'c'), 'd')); - $definition->addTransition(new Transition('t3', 'd', 'e')); - $definition->addTransition(new Transition('t4', 'd', 'f')); - $definition->addTransition(new Transition('t5', 'e', 'g')); - $definition->addTransition(new Transition('t6', 'f', 'g')); + $builder->addTransition(new Transition('t1', 'a', array('b', 'c'))); + $builder->addTransition(new Transition('t2', array('b', 'c'), 'd')); + $builder->addTransition(new Transition('t3', 'd', 'e')); + $builder->addTransition(new Transition('t4', 'd', 'f')); + $builder->addTransition(new Transition('t5', 'e', 'g')); + $builder->addTransition(new Transition('t6', 'f', 'g')); - return $definition; + return $builder->build(); } public function provideSimpleWorkflowDefinition() { - $definition = new Definition(); + $builder = new DefinitionBuilder(); - $definition->addPlaces(range('a', 'c')); + $builder->addPlaces(range('a', 'c')); - $definition->addTransition(new Transition('t1', 'a', 'b')); - $definition->addTransition(new Transition('t2', 'b', 'c')); + $builder->addTransition(new Transition('t1', 'a', 'b')); + $builder->addTransition(new Transition('t2', 'b', 'c')); - return $definition; + return $builder->build(); } public function createComplexWorkflowDumpWithMarking() diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 42fa8c4ac7..90b537c4b9 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->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); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), Subject1::class); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), Subject2::class); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), Subject2::class); } protected function tearDown() diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 4a7ea56e21..c926695270 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -4,6 +4,7 @@ namespace Symfony\Component\Workflow\Tests; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Workflow\Definition; +use Symfony\Component\Workflow\DefinitionBuilder; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; @@ -21,7 +22,7 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase { $subject = new \stdClass(); $subject->marking = null; - $workflow = new Workflow(new Definition(), $this->getMockBuilder(MarkingStoreInterface::class)->getMock()); + $workflow = new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock()); $workflow->getMarking($subject); } @@ -34,7 +35,7 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase { $subject = new \stdClass(); $subject->marking = null; - $workflow = new Workflow(new Definition(), new PropertyAccessorMarkingStore()); + $workflow = new Workflow(new Definition(array(), array()), new PropertyAccessorMarkingStore()); $workflow->getMarking($subject); } @@ -48,7 +49,7 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase $subject = new \stdClass(); $subject->marking = null; $subject->marking = array('nope' => true); - $workflow = new Workflow(new Definition(), new PropertyAccessorMarkingStore()); + $workflow = new Workflow(new Definition(array(), array()), new PropertyAccessorMarkingStore()); $workflow->getMarking($subject); } @@ -211,18 +212,18 @@ class WorkflowTest extends \PHPUnit_Framework_TestCase protected function createComplexWorkflow() { - $definition = new Definition(); + $builder = new DefinitionBuilder(); - $definition->addPlaces(range('a', 'g')); + $builder->addPlaces(range('a', 'g')); - $definition->addTransition(new Transition('t1', 'a', array('b', 'c'))); - $definition->addTransition(new Transition('t2', array('b', 'c'), 'd')); - $definition->addTransition(new Transition('t3', 'd', 'e')); - $definition->addTransition(new Transition('t4', 'd', 'f')); - $definition->addTransition(new Transition('t5', 'e', 'g')); - $definition->addTransition(new Transition('t6', 'f', 'g')); + $builder->addTransition(new Transition('t1', 'a', array('b', 'c'))); + $builder->addTransition(new Transition('t2', array('b', 'c'), 'd')); + $builder->addTransition(new Transition('t3', 'd', 'e')); + $builder->addTransition(new Transition('t4', 'd', 'f')); + $builder->addTransition(new Transition('t5', 'e', 'g')); + $builder->addTransition(new Transition('t6', 'f', 'g')); - return $definition; + return $builder->build(); // The graph looks like: // diff --git a/src/Symfony/Component/Workflow/Workflow.php b/src/Symfony/Component/Workflow/Workflow.php index 348b3b95a9..72dc07d73c 100644 --- a/src/Symfony/Component/Workflow/Workflow.php +++ b/src/Symfony/Component/Workflow/Workflow.php @@ -159,6 +159,14 @@ class Workflow return $this->name; } + /** + * @return Definition + */ + public function getDefinition() + { + return $this->definition; + } + /** * @param object $subject * @param Marking $marking