From 6421c10c5911ae2c6745c5c089bc58022cd5e63e Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sun, 4 Dec 2016 22:08:15 +0000 Subject: [PATCH 01/10] [Workflow] Introduce concept of SupprtStrategyInterface to allow other support checks than class instance --- .../Compiler/ValidateWorkflowsPass.php | 3 ++ .../DependencyInjection/Configuration.php | 4 +- .../FrameworkExtension.php | 15 ++++++-- .../FrameworkExtensionTest.php | 4 ++ .../Component/Workflow/Event/Event.php | 9 +++++ .../Component/Workflow/Event/GuardEvent.php | 6 +++ src/Symfony/Component/Workflow/Registry.php | 34 ++++++++++++----- .../ClassInstanceSupportStrategy.php | 27 ++++++++++++++ .../SupportStrategyInterface.php | 25 +++++++++++++ .../Component/Workflow/Tests/RegistryTest.php | 18 +++++++-- .../ClassInstanceSupportStrategyTest.php | 37 +++++++++++++++++++ 11 files changed, 166 insertions(+), 16 deletions(-) create mode 100644 src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php create mode 100644 src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php create mode 100644 src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php index a2f695992926..58789863f5ea 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php @@ -38,6 +38,9 @@ public function process(ContainerBuilder $container) 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)); } + if (!array_key_exists('supports', $tag) && !array_key_exists('support_strategy', $tag)) { + throw new RuntimeException(sprintf('Either "supports" or "support_strategy" for the tag "workflow.definition" of service "%s" must be set.', $id)); + } $this->createValidator($tag)->validate($definition, $tag['name']); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 5c6208e6e46f..b8c9f4546749 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -273,7 +273,6 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->arrayNode('supports') - ->isRequired() ->beforeNormalization() ->ifString() ->then(function ($v) { return array($v); }) @@ -286,6 +285,9 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->end() + ->scalarNode('support_strategy') + ->cannotBeEmpty() + ->end() ->scalarNode('initial_place')->defaultNull()->end() ->arrayNode('places') ->isRequired() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 61a7d7a9170d..1bf582e31485 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -436,7 +436,7 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde // Create MarkingStore if (isset($workflow['marking_store']['type'])) { - $markingStoreDefinition = new DefinitionDecorator('workflow.marking_store.'.$workflow['marking_store']['type']); + $markingStoreDefinition = new DefinitionDecorator('workflow.marking_store.' . $workflow['marking_store']['type']); foreach ($workflow['marking_store']['arguments'] as $argument) { $markingStoreDefinition->addArgument($argument); } @@ -458,8 +458,17 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); // Add workflow to Registry - foreach ($workflow['supports'] as $supportedClass) { - $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $supportedClass)); + if (isset($workflow['supports'])) { + foreach ($workflow['supports'] as $supportedClassName) { + $strategyDefinition = new Definition( + Workflow\SupportStrategy\ClassInstanceSupportStrategy::class, array($supportedClassName) + ); + $strategyDefinition->setPublic(false); + $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $supportedClassName)); + } + } + if (isset($workflow['support_strategy'])) { + $registryDefinition->addMethodCall('add', array(new Reference($workflowId), new Reference($workflow['support_strategy']))); } } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index c1ee84685654..4ca3aa97a3ff 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -165,6 +165,10 @@ public function testWorkflows() $this->assertSame(array('workflow.definition' => array(array('name' => 'pull_request', 'type' => 'state_machine', 'marking_store' => 'single_state'))), $stateMachineDefinition->getTags()); $this->assertCount(9, $stateMachineDefinition->getArgument(1)); $this->assertSame('start', $stateMachineDefinition->getArgument(2)); + + $this->assertTrue($container->hasDefinition('workflow.registry', 'Workflow registry is registered as a service')); + $registryDefinition = $container->getDefinition('workflow.registry'); + $this->assertGreaterThan(0, count($registryDefinition->getMethodCalls())); } /** diff --git a/src/Symfony/Component/Workflow/Event/Event.php b/src/Symfony/Component/Workflow/Event/Event.php index dce6a6f5df8f..d26d012d0011 100644 --- a/src/Symfony/Component/Workflow/Event/Event.php +++ b/src/Symfony/Component/Workflow/Event/Event.php @@ -37,16 +37,25 @@ public function __construct($subject, Marking $marking, Transition $transition) $this->transition = $transition; } + /** + * @return Marking + */ public function getMarking() { return $this->marking; } + /** + * @return object + */ public function getSubject() { return $this->subject; } + /** + * @return Transition + */ public function getTransition() { return $this->transition; diff --git a/src/Symfony/Component/Workflow/Event/GuardEvent.php b/src/Symfony/Component/Workflow/Event/GuardEvent.php index bf4b6f3971e7..9529b59a40e6 100644 --- a/src/Symfony/Component/Workflow/Event/GuardEvent.php +++ b/src/Symfony/Component/Workflow/Event/GuardEvent.php @@ -19,11 +19,17 @@ class GuardEvent extends Event { private $blocked = false; + /** + * @return bool + */ public function isBlocked() { return $this->blocked; } + /** + * @param bool $blocked + */ public function setBlocked($blocked) { $this->blocked = (bool) $blocked; diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 619ed5f66a5b..30134badf6ab 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Workflow; use Symfony\Component\Workflow\Exception\InvalidArgumentException; +use Symfony\Component\Workflow\SupportStrategy\SupportStrategyInterface; /** * @author Fabien Potencier @@ -23,19 +24,26 @@ class Registry /** * @param Workflow $workflow - * @param string $className + * @param SupportStrategyInterface $supportStrategy */ - public function add(Workflow $workflow, $className) + public function add(Workflow $workflow, SupportStrategyInterface $supportStrategy) { - $this->workflows[] = array($workflow, $className); + $this->workflows[] = array($workflow, $supportStrategy); } + /** + * @param object $subject + * @param null|string $workflowName + * + * @return Workflow + * @throws InvalidArgumentException + */ public function get($subject, $workflowName = null) { $matched = null; - foreach ($this->workflows as list($workflow, $className)) { - if ($this->supports($workflow, $className, $subject, $workflowName)) { + foreach ($this->workflows as list($workflow, $supportStrategy)) { + if ($this->supports($workflow, $supportStrategy, $subject, $workflowName)) { if ($matched) { throw new InvalidArgumentException('At least two workflows match this subject. Set a different name on each and use the second (name) argument of this method.'); } @@ -50,16 +58,24 @@ public function get($subject, $workflowName = null) return $matched; } - private function supports(Workflow $workflow, $className, $subject, $name) + /** + * @param Workflow $workflow + * @param SupportStrategyInterface $supportStrategy + * @param object $subject + * @param null|string $workflowName + * + * @return bool + */ + private function supports(Workflow $workflow, SupportStrategyInterface $supportStrategy, $subject, $workflowName) { - if (!$subject instanceof $className) { + if (!$supportStrategy->supports($workflow, $subject)) { return false; } - if (null === $name) { + if (null === $workflowName) { return true; } - return $name === $workflow->getName(); + return $workflowName === $workflow->getName(); } } diff --git a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php new file mode 100644 index 000000000000..5b2e527069f9 --- /dev/null +++ b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php @@ -0,0 +1,27 @@ +className = $className; + } + + /** + * {@inheritdoc} + */ + public function supports(Workflow $workflow, $subject) + { + return $subject instanceof $this->className; + } +} diff --git a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php new file mode 100644 index 000000000000..2a1219c25cf3 --- /dev/null +++ b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\SupportStrategy; + +use Symfony\Component\Workflow\Workflow; + +interface SupportStrategyInterface +{ + /** + * @param \Symfony\Component\Workflow\Workflow $workflow + * @param object $subject + * + * @return bool + */ + public function supports(Workflow $workflow, $subject); +} diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 90b537c4b99c..2729384fb2e6 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -6,6 +6,7 @@ use Symfony\Component\Workflow\Definition; use Symfony\Component\Workflow\MarkingStore\MarkingStoreInterface; use Symfony\Component\Workflow\Registry; +use Symfony\Component\Workflow\SupportStrategy\SupportStrategyInterface; use Symfony\Component\Workflow\Workflow; class RegistryTest extends \PHPUnit_Framework_TestCase @@ -18,9 +19,9 @@ protected function setUp() $this->registry = new Registry(); - $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); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), $this->getSupportStrategy(Subject1::class)); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), $this->getSupportStrategy(Subject2::class)); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), $this->getSupportStrategy(Subject2::class)); } protected function tearDown() @@ -64,6 +65,17 @@ public function testGetWithNoMatch() $this->assertInstanceOf(Workflow::class, $w1); $this->assertSame('workflow1', $w1->getName()); } + + private function getSupportStrategy($supportedClassName) + { + $strategy = $this->getMockBuilder(SupportStrategyInterface::class)->getMock(); + $strategy->expects($this->any())->method('supports') + ->will($this->returnCallback(function($workflow, $subject) use ($supportedClassName) { + return $subject instanceof $supportedClassName; + })); + + return $strategy; + } } class Subject1 diff --git a/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php b/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php new file mode 100644 index 000000000000..42463d4b2fb1 --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php @@ -0,0 +1,37 @@ +assertTrue($strategy->supports($this->getWorkflow(), new Subject1())); + } + + public function testSupportsIfNotClassInstance() + { + $strategy = new ClassInstanceSupportStrategy('Symfony\Component\Workflow\Tests\SupportStrategy\Subject2'); + + $this->assertFalse($strategy->supports($this->getWorkflow(), new Subject1())); + } + + private function getWorkflow() + { + return $this->getMockBuilder(Workflow::class) + ->disableOriginalConstructor() + ->getMock(); + } +} + +class Subject1 +{ +} +class Subject2 +{ +} From bfd361198cd749b2f7a72e47a880c888882824f9 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Mon, 5 Dec 2016 15:24:28 +0000 Subject: [PATCH 02/10] [Workflow] Refactoring to resolve BC break and adding deprecated warning --- .../Compiler/ValidateWorkflowsPass.php | 3 --- .../DependencyInjection/Configuration.php | 4 ++++ src/Symfony/Component/Workflow/Registry.php | 21 ++++++++++++++----- .../Component/Workflow/Tests/RegistryTest.php | 13 +++++++++--- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php index 58789863f5ea..a2f695992926 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/ValidateWorkflowsPass.php @@ -38,9 +38,6 @@ public function process(ContainerBuilder $container) 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)); } - if (!array_key_exists('supports', $tag) && !array_key_exists('support_strategy', $tag)) { - throw new RuntimeException(sprintf('Either "supports" or "support_strategy" for the tag "workflow.definition" of service "%s" must be set.', $id)); - } $this->createValidator($tag)->validate($definition, $tag['name']); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index b8c9f4546749..9756ffa85006 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -326,6 +326,10 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->end() + ->validate() + ->ifTrue(function ($v) { return isset($v['supports']) && isset($v['support_strategy']); }) + ->thenInvalid('"supports" and "support_strategy" cannot be used together.') + ->end() ->end() ->end() ->end() diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 30134badf6ab..cd4e84109e08 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -24,10 +24,18 @@ class Registry /** * @param Workflow $workflow - * @param SupportStrategyInterface $supportStrategy + * @param string|SupportStrategyInterface $supportStrategy */ - public function add(Workflow $workflow, SupportStrategyInterface $supportStrategy) + public function add(Workflow $workflow, $supportStrategy) { + if (!$supportStrategy instanceof SupportStrategyInterface) { + if (!is_string($supportStrategy)) { + throw new InvalidArgumentException('Expecting instance of SupportStrategyInterface or class name as a string.'); + } + + @trigger_error('Support of class name string was deprecated after version 3.2 and won\'t work anymore in 4.0.', E_USER_DEPRECATED); + } + $this->workflows[] = array($workflow, $supportStrategy); } @@ -60,15 +68,18 @@ public function get($subject, $workflowName = null) /** * @param Workflow $workflow - * @param SupportStrategyInterface $supportStrategy + * @param string|SupportStrategyInterface $supportStrategy * @param object $subject * @param null|string $workflowName * * @return bool */ - private function supports(Workflow $workflow, SupportStrategyInterface $supportStrategy, $subject, $workflowName) + private function supports(Workflow $workflow, $supportStrategy, $subject, $workflowName) { - if (!$supportStrategy->supports($workflow, $subject)) { + if (is_string($supportStrategy) && !$subject instanceof $supportStrategy) { + return false; + } + if ($supportStrategy instanceof SupportStrategyInterface && !$supportStrategy->supports($workflow, $subject)) { return false; } diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 2729384fb2e6..ffa8286e1baf 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -15,13 +15,11 @@ class RegistryTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $workflows = array(); - $this->registry = new Registry(); $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), $this->getSupportStrategy(Subject1::class)); $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), $this->getSupportStrategy(Subject2::class)); - $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), $this->getSupportStrategy(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() @@ -29,6 +27,15 @@ protected function tearDown() $this->registry = null; } + /** + * @expectedException \Symfony\Component\Workflow\Exception\InvalidArgumentException + * @expectedExceptionMessage Expecting instance of SupportStrategyInterface or class name as a string. + */ + public function testAddUnsupportedSupportStrategy() + { + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow4'), array(Subject1::class, Subject2::class)); + } + public function testGetWithSuccess() { $workflow = $this->registry->get(new Subject1()); From c0cbc85ab8b5098e1382a2a496b2eb130972fbce Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Tue, 6 Dec 2016 11:45:55 +0000 Subject: [PATCH 03/10] [Workflow] Moving and tagging legacy unit tests --- .../Component/Workflow/Tests/RegistryTest.php | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index ffa8286e1baf..e30efb5363e0 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -19,7 +19,7 @@ protected function setUp() $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), $this->getSupportStrategy(Subject1::class)); $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), $this->getSupportStrategy(Subject2::class)); - $this->registry->add(new Workflow(new Definition(array(), array()), $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(), 'workflow3'), $this->getSupportStrategy(Subject2::class)); } protected function tearDown() @@ -73,6 +73,29 @@ public function testGetWithNoMatch() $this->assertSame('workflow1', $w1->getName()); } + /** + * @group legacy + */ + public function testGetWithSuccessLegacyStrategy() + { + $registry = new Registry(); + + $registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), Subject1::class); + $registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), Subject2::class); + + $workflow = $registry->get(new Subject1()); + $this->assertInstanceOf(Workflow::class, $workflow); + $this->assertSame('workflow1', $workflow->getName()); + + $workflow = $registry->get(new Subject1(), 'workflow1'); + $this->assertInstanceOf(Workflow::class, $workflow); + $this->assertSame('workflow1', $workflow->getName()); + + $workflow = $registry->get(new Subject2(), 'workflow2'); + $this->assertInstanceOf(Workflow::class, $workflow); + $this->assertSame('workflow2', $workflow->getName()); + } + private function getSupportStrategy($supportedClassName) { $strategy = $this->getMockBuilder(SupportStrategyInterface::class)->getMock(); From fa448af630c8251ecc0ec2cfc8a41592f922af9a Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sat, 10 Dec 2016 19:15:16 +0000 Subject: [PATCH 04/10] [Workflow] Some CS fixes --- src/Symfony/Component/Workflow/Registry.php | 10 +++++----- .../SupportStrategy/SupportStrategyInterface.php | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index cd4e84109e08..c74b5054cba5 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -23,7 +23,7 @@ class Registry private $workflows = array(); /** - * @param Workflow $workflow + * @param Workflow $workflow * @param string|SupportStrategyInterface $supportStrategy */ public function add(Workflow $workflow, $supportStrategy) @@ -40,7 +40,7 @@ public function add(Workflow $workflow, $supportStrategy) } /** - * @param object $subject + * @param object $subject * @param null|string $workflowName * * @return Workflow @@ -67,10 +67,10 @@ public function get($subject, $workflowName = null) } /** - * @param Workflow $workflow + * @param Workflow $workflow * @param string|SupportStrategyInterface $supportStrategy - * @param object $subject - * @param null|string $workflowName + * @param object $subject + * @param null|string $workflowName * * @return bool */ diff --git a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php index 2a1219c25cf3..94ec7adc7b6e 100644 --- a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php +++ b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php @@ -16,8 +16,8 @@ interface SupportStrategyInterface { /** - * @param \Symfony\Component\Workflow\Workflow $workflow - * @param object $subject + * @param Workflow $workflow + * @param object $subject * * @return bool */ From 19fb676c72972c67cee64a25bd4090f0b7bf3d70 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sat, 10 Dec 2016 19:34:49 +0000 Subject: [PATCH 05/10] [Workflow] Removing some docblocks --- src/Symfony/Component/Workflow/Event/Event.php | 9 --------- src/Symfony/Component/Workflow/Event/GuardEvent.php | 6 ------ src/Symfony/Component/Workflow/Registry.php | 8 -------- 3 files changed, 23 deletions(-) diff --git a/src/Symfony/Component/Workflow/Event/Event.php b/src/Symfony/Component/Workflow/Event/Event.php index d26d012d0011..dce6a6f5df8f 100644 --- a/src/Symfony/Component/Workflow/Event/Event.php +++ b/src/Symfony/Component/Workflow/Event/Event.php @@ -37,25 +37,16 @@ public function __construct($subject, Marking $marking, Transition $transition) $this->transition = $transition; } - /** - * @return Marking - */ public function getMarking() { return $this->marking; } - /** - * @return object - */ public function getSubject() { return $this->subject; } - /** - * @return Transition - */ public function getTransition() { return $this->transition; diff --git a/src/Symfony/Component/Workflow/Event/GuardEvent.php b/src/Symfony/Component/Workflow/Event/GuardEvent.php index 9529b59a40e6..bf4b6f3971e7 100644 --- a/src/Symfony/Component/Workflow/Event/GuardEvent.php +++ b/src/Symfony/Component/Workflow/Event/GuardEvent.php @@ -19,17 +19,11 @@ class GuardEvent extends Event { private $blocked = false; - /** - * @return bool - */ public function isBlocked() { return $this->blocked; } - /** - * @param bool $blocked - */ public function setBlocked($blocked) { $this->blocked = (bool) $blocked; diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index c74b5054cba5..89f734718dff 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -66,14 +66,6 @@ public function get($subject, $workflowName = null) return $matched; } - /** - * @param Workflow $workflow - * @param string|SupportStrategyInterface $supportStrategy - * @param object $subject - * @param null|string $workflowName - * - * @return bool - */ private function supports(Workflow $workflow, $supportStrategy, $subject, $workflowName) { if (is_string($supportStrategy) && !$subject instanceof $supportStrategy) { From d22fe018bb141476a3435e2de22522cfb6c27fc8 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Wed, 14 Dec 2016 23:54:47 +0000 Subject: [PATCH 06/10] [Workflow] fixing service definition of workflow registry in framework bundle --- .../FrameworkBundle/DependencyInjection/FrameworkExtension.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 1bf582e31485..256141c1a36a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -464,7 +464,7 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde Workflow\SupportStrategy\ClassInstanceSupportStrategy::class, array($supportedClassName) ); $strategyDefinition->setPublic(false); - $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $supportedClassName)); + $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $strategyDefinition)); } } if (isset($workflow['support_strategy'])) { From 047a2e75fa96aa64078080a6e81b8625f55ec1b6 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sat, 17 Dec 2016 10:54:24 +0000 Subject: [PATCH 07/10] [Workflow] fixing code style mostly, removing exception --- .../DependencyInjection/FrameworkExtension.php | 8 ++++---- src/Symfony/Component/Workflow/Registry.php | 7 ++----- .../SupportStrategy/ClassInstanceSupportStrategy.php | 4 ---- src/Symfony/Component/Workflow/Tests/RegistryTest.php | 2 +- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 256141c1a36a..84cfc25c6c24 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -35,6 +35,7 @@ use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer; use Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer; use Symfony\Component\Workflow; +use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy; use Symfony\Component\Yaml\Yaml; /** @@ -436,7 +437,7 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde // Create MarkingStore if (isset($workflow['marking_store']['type'])) { - $markingStoreDefinition = new DefinitionDecorator('workflow.marking_store.' . $workflow['marking_store']['type']); + $markingStoreDefinition = new DefinitionDecorator('workflow.marking_store.'.$workflow['marking_store']['type']); foreach ($workflow['marking_store']['arguments'] as $argument) { $markingStoreDefinition->addArgument($argument); } @@ -461,13 +462,12 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde if (isset($workflow['supports'])) { foreach ($workflow['supports'] as $supportedClassName) { $strategyDefinition = new Definition( - Workflow\SupportStrategy\ClassInstanceSupportStrategy::class, array($supportedClassName) + ClassInstanceSupportStrategy::class, array($supportedClassName) ); $strategyDefinition->setPublic(false); $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $strategyDefinition)); } - } - if (isset($workflow['support_strategy'])) { + } elseif (isset($workflow['support_strategy'])) { $registryDefinition->addMethodCall('add', array(new Reference($workflowId), new Reference($workflow['support_strategy']))); } } diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 89f734718dff..16a4fe201a76 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -29,10 +29,6 @@ class Registry public function add(Workflow $workflow, $supportStrategy) { if (!$supportStrategy instanceof SupportStrategyInterface) { - if (!is_string($supportStrategy)) { - throw new InvalidArgumentException('Expecting instance of SupportStrategyInterface or class name as a string.'); - } - @trigger_error('Support of class name string was deprecated after version 3.2 and won\'t work anymore in 4.0.', E_USER_DEPRECATED); } @@ -41,9 +37,10 @@ public function add(Workflow $workflow, $supportStrategy) /** * @param object $subject - * @param null|string $workflowName + * @param string|null $workflowName * * @return Workflow + * * @throws InvalidArgumentException */ public function get($subject, $workflowName = null) diff --git a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php index 5b2e527069f9..5e229f0b9654 100644 --- a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php +++ b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php @@ -6,12 +6,8 @@ class ClassInstanceSupportStrategy implements SupportStrategyInterface { - /** @var string */ private $className; - /** - * @param string $className - */ public function __construct($className) { $this->className = $className; diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index e30efb5363e0..4e707e3ec5e9 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -100,7 +100,7 @@ private function getSupportStrategy($supportedClassName) { $strategy = $this->getMockBuilder(SupportStrategyInterface::class)->getMock(); $strategy->expects($this->any())->method('supports') - ->will($this->returnCallback(function($workflow, $subject) use ($supportedClassName) { + ->will($this->returnCallback(function ($workflow, $subject) use ($supportedClassName) { return $subject instanceof $supportedClassName; })); From a34e4afdc69fa681c9c021840e19c0d86f6d0c09 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sat, 17 Dec 2016 10:57:13 +0000 Subject: [PATCH 08/10] [Workflow] tiny cs fix in docblock... --- src/Symfony/Component/Workflow/Registry.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 16a4fe201a76..4a0a5b1ad43b 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -40,7 +40,7 @@ public function add(Workflow $workflow, $supportStrategy) * @param string|null $workflowName * * @return Workflow - * + * * @throws InvalidArgumentException */ public function get($subject, $workflowName = null) From 1e65b12db8692ae754036ac208ff44d37d6a9bcb Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sat, 17 Dec 2016 11:08:29 +0000 Subject: [PATCH 09/10] [Workflow] removing deprecated test case for removed InvalidArgumentException --- src/Symfony/Component/Workflow/Tests/RegistryTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 4e707e3ec5e9..9a425a25f3ab 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -27,15 +27,6 @@ protected function tearDown() $this->registry = null; } - /** - * @expectedException \Symfony\Component\Workflow\Exception\InvalidArgumentException - * @expectedExceptionMessage Expecting instance of SupportStrategyInterface or class name as a string. - */ - public function testAddUnsupportedSupportStrategy() - { - $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow4'), array(Subject1::class, Subject2::class)); - } - public function testGetWithSuccess() { $workflow = $this->registry->get(new Subject1()); From 2d11004af46f4a955f769419f8f079ac29eb7eed Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Wed, 4 Jan 2017 19:13:21 +0100 Subject: [PATCH 10/10] [Workflow] Some more adaptions after code review comments, not finished yet --- .../DependencyInjection/FrameworkExtension.php | 4 +--- src/Symfony/Component/Workflow/Registry.php | 2 -- src/Symfony/Component/Workflow/Tests/RegistryTest.php | 8 ++++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index a0af623a8db8..380c96301a8a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -461,9 +461,7 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde // Add workflow to Registry if (isset($workflow['supports'])) { foreach ($workflow['supports'] as $supportedClassName) { - $strategyDefinition = new Definition( - ClassInstanceSupportStrategy::class, array($supportedClassName) - ); + $strategyDefinition = new Definition(ClassInstanceSupportStrategy::class, array($supportedClassName)); $strategyDefinition->setPublic(false); $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $strategyDefinition)); } diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 4a0a5b1ad43b..787773cf5cf1 100644 --- a/src/Symfony/Component/Workflow/Registry.php +++ b/src/Symfony/Component/Workflow/Registry.php @@ -40,8 +40,6 @@ public function add(Workflow $workflow, $supportStrategy) * @param string|null $workflowName * * @return Workflow - * - * @throws InvalidArgumentException */ public function get($subject, $workflowName = null) { diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php index 9a425a25f3ab..c9a25d4a7f8c 100644 --- a/src/Symfony/Component/Workflow/Tests/RegistryTest.php +++ b/src/Symfony/Component/Workflow/Tests/RegistryTest.php @@ -17,9 +17,9 @@ protected function setUp() { $this->registry = new Registry(); - $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), $this->getSupportStrategy(Subject1::class)); - $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), $this->getSupportStrategy(Subject2::class)); - $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), $this->getSupportStrategy(Subject2::class)); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow1'), $this->createSupportStrategy(Subject1::class)); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow2'), $this->createSupportStrategy(Subject2::class)); + $this->registry->add(new Workflow(new Definition(array(), array()), $this->getMockBuilder(MarkingStoreInterface::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock(), 'workflow3'), $this->createSupportStrategy(Subject2::class)); } protected function tearDown() @@ -87,7 +87,7 @@ public function testGetWithSuccessLegacyStrategy() $this->assertSame('workflow2', $workflow->getName()); } - private function getSupportStrategy($supportedClassName) + private function createSupportStrategy($supportedClassName) { $strategy = $this->getMockBuilder(SupportStrategyInterface::class)->getMock(); $strategy->expects($this->any())->method('supports')