From 184301206fcf1951df5b13f07c679254bf76f7c0 Mon Sep 17 00:00:00 2001 From: Andreas Kleemann Date: Sun, 4 Dec 2016 22:08:15 +0000 Subject: [PATCH 1/2] [Workflow] Introduce concept of SupprtStrategyInterface to allow other support checks than class instance --- .../DependencyInjection/Configuration.php | 8 +++- .../FrameworkExtension.php | 11 ++++- .../FrameworkExtensionTest.php | 5 +++ src/Symfony/Component/Workflow/Registry.php | 28 +++++++----- .../ClassInstanceSupportStrategy.php | 23 ++++++++++ .../SupportStrategyInterface.php | 25 +++++++++++ .../Component/Workflow/Tests/RegistryTest.php | 43 ++++++++++++++++--- .../ClassInstanceSupportStrategyTest.php | 37 ++++++++++++++++ 8 files changed, 162 insertions(+), 18 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/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 960007e25a051..a267bcb72ce23 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -280,7 +280,6 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->arrayNode('supports') - ->isRequired() ->beforeNormalization() ->ifString() ->then(function ($v) { return array($v); }) @@ -293,6 +292,9 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->end() + ->scalarNode('support_strategy') + ->cannotBeEmpty() + ->end() ->scalarNode('initial_place')->defaultNull()->end() ->arrayNode('places') ->isRequired() @@ -353,6 +355,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/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 6355908b88688..a4cb08ff181ae 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -36,6 +36,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; /** @@ -475,8 +476,14 @@ 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(ClassInstanceSupportStrategy::class, array($supportedClassName)); + $strategyDefinition->setPublic(false); + $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $strategyDefinition)); + } + } elseif (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 8b9dd97f001fb..9a580d3abf28d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -168,11 +168,16 @@ public function testWorkflows() $this->assertCount(9, $stateMachineDefinition->getArgument(1)); $this->assertSame('start', $stateMachineDefinition->getArgument(2)); + $serviceMarkingStoreWorkflowDefinition = $container->getDefinition('workflow.service_marking_store_workflow'); /** @var Reference $markingStoreRef */ $markingStoreRef = $serviceMarkingStoreWorkflowDefinition->getArgument(1); $this->assertInstanceOf(Reference::class, $markingStoreRef); $this->assertEquals('workflow_service', (string) $markingStoreRef); + + $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/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 53e06fbb5e8e4..787773cf5cf1b 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 @@ -22,12 +23,16 @@ class Registry private $workflows = array(); /** - * @param Workflow $workflow - * @param string $className + * @param Workflow $workflow + * @param string|SupportStrategyInterface $supportStrategy */ - public function add(Workflow $workflow, $className) + public function add(Workflow $workflow, $supportStrategy) { - $this->workflows[] = array($workflow, $className); + if (!$supportStrategy instanceof SupportStrategyInterface) { + @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); } /** @@ -40,8 +45,8 @@ 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.'); } @@ -56,16 +61,19 @@ public function get($subject, $workflowName = null) return $matched; } - private function supports(Workflow $workflow, $className, $subject, $name) + private function supports(Workflow $workflow, $supportStrategy, $subject, $workflowName) { - if (!$subject instanceof $className) { + if (is_string($supportStrategy) && !$subject instanceof $supportStrategy) { + return false; + } + if ($supportStrategy instanceof SupportStrategyInterface && !$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 0000000000000..5e229f0b96545 --- /dev/null +++ b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php @@ -0,0 +1,23 @@ +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 0000000000000..94ec7adc7b6ee --- /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 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 90b537c4b99c1..c9a25d4a7f8cb 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 @@ -14,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'), 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->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() @@ -64,6 +63,40 @@ public function testGetWithNoMatch() $this->assertInstanceOf(Workflow::class, $w1); $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 createSupportStrategy($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 0000000000000..42463d4b2fb1c --- /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 134a58b4c42b2498ce8f431e1641f0fb676fdb84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 18 Jan 2017 15:08:35 +0100 Subject: [PATCH 2/2] [Workflow] Fixed code and tests --- UPGRADE-3.3.md | 6 ++++ UPGRADE-4.0.md | 17 ++++++---- .../Tests/Extension/WorkflowExtensionTest.php | 3 +- .../DependencyInjection/Configuration.php | 14 +++++++-- .../FrameworkExtension.php | 2 +- .../Resources/config/schema/symfony-1.0.xsd | 3 +- ...flow_with_support_and_support_strategy.php | 31 +++++++++++++++++++ ...w_without_support_and_support_strategy.php | 27 ++++++++++++++++ ...flow_with_support_and_support_strategy.xml | 21 +++++++++++++ ...w_without_support_and_support_strategy.xml | 20 ++++++++++++ ...flow_with_support_and_support_strategy.yml | 17 ++++++++++ ...w_without_support_and_support_strategy.yml | 14 +++++++++ .../FrameworkExtensionTest.php | 19 +++++++++++- src/Symfony/Component/Workflow/CHANGELOG.md | 6 ++++ src/Symfony/Component/Workflow/Registry.php | 14 +++------ .../ClassInstanceSupportStrategy.php | 8 ++++- .../SupportStrategyInterface.php | 3 ++ .../ClassInstanceSupportStrategyTest.php | 6 ++-- 18 files changed, 206 insertions(+), 25 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md index a7ad71a426f04..f5c2d756a0eac 100644 --- a/UPGRADE-3.3.md +++ b/UPGRADE-3.3.md @@ -59,3 +59,9 @@ TwigBridge * The `TwigRendererEngine::setEnvironment()` method has been deprecated and will be removed in 4.0. Pass the Twig Environment as second argument of the constructor instead. + +Workflow +-------- + + * Deprecated class name support in `WorkflowRegistry::add()` as second parameter. + Wrap the class name in an instance of ClassInstanceSupportStrategy instead. diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 085f0204d02ff..8047e7ae3c06a 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -154,7 +154,7 @@ FrameworkBundle * The `framework.serializer.cache` option and the services `serializer.mapping.cache.apc` and `serializer.mapping.cache.doctrine.apc` have been removed. APCu should now be automatically used when available. - + * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConsoleCommandPass` has been removed. Use `Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass` instead. SecurityBundle @@ -216,7 +216,7 @@ Serializer * The ability to pass a Doctrine `Cache` instance to the `ClassMetadataFactory` class has been removed. You should use the `CacheClassMetadataFactory` class instead. - + * Not defining the 6th argument `$format = null` of the `AbstractNormalizer::instantiateObject()` method when overriding it is not supported anymore. @@ -294,9 +294,9 @@ Validator // ... } ``` - + * The default value of the strict option of the `Choice` Constraint has been - changed to `true` as of 4.0. If you need the previous behaviour ensure to + changed to `true` as of 4.0. If you need the previous behaviour ensure to set the option to `false`. Yaml @@ -393,5 +393,10 @@ Yaml Ldap ---- - - * The `RenameEntryInterface` has been deprecated, and merged with `EntryManagerInterface` + + * The `RenameEntryInterface` has been deprecated, and merged with `EntryManagerInterface` + +Workflow +-------- + + * Removed class name support in `WorkflowRegistry::add()` as second parameter. diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php index 27a55670a35d2..3193a58dbccc5 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Workflow\Definition; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Registry; +use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy; use Symfony\Component\Workflow\Transition; use Symfony\Component\Workflow\Workflow; @@ -37,7 +38,7 @@ protected function setUp() $workflow = new Workflow($definition); $registry = new Registry(); - $registry->add($workflow, \stdClass::class); + $registry->add($workflow, new ClassInstanceSupportStrategy(\stdClass::class)); $this->extension = new WorkflowExtension($registry); } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index a267bcb72ce23..be604b88e0581 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -295,7 +295,9 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->scalarNode('support_strategy') ->cannotBeEmpty() ->end() - ->scalarNode('initial_place')->defaultNull()->end() + ->scalarNode('initial_place') + ->defaultNull() + ->end() ->arrayNode('places') ->isRequired() ->requiresAtLeastOneElement() @@ -356,9 +358,17 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) ->end() ->end() ->validate() - ->ifTrue(function ($v) { return isset($v['supports']) && isset($v['support_strategy']); }) + ->ifTrue(function ($v) { + return $v['supports'] && isset($v['support_strategy']); + }) ->thenInvalid('"supports" and "support_strategy" cannot be used together.') ->end() + ->validate() + ->ifTrue(function ($v) { + return !$v['supports'] && !isset($v['support_strategy']); + }) + ->thenInvalid('"supports" or "support_strategy" should be configured.') + ->end() ->end() ->end() ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index a4cb08ff181ae..991a535a8266b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -476,7 +476,7 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); // Add workflow to Registry - if (isset($workflow['supports'])) { + if ($workflow['supports']) { foreach ($workflow['supports'] as $supportedClassName) { $strategyDefinition = new Definition(ClassInstanceSupportStrategy::class, array($supportedClassName)); $strategyDefinition->setPublic(false); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 6dce93415e662..ae948d5f85f25 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -232,13 +232,14 @@ - + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php new file mode 100644 index 0000000000000..062fdb9f0dba4 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php @@ -0,0 +1,31 @@ +loadFromExtension('framework', array( + 'workflows' => array( + 'my_workflow' => array( + 'marking_store' => array( + 'type' => 'multiple_state', + ), + 'supports' => array( + FrameworkExtensionTest::class, + ), + 'support_strategy' => 'foobar', + 'places' => array( + 'first', + 'last', + ), + 'transitions' => array( + 'go' => array( + 'from' => array( + 'first', + ), + 'to' => array( + 'last', + ), + ), + ), + ), + ), +)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php new file mode 100644 index 0000000000000..06948785e927c --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php @@ -0,0 +1,27 @@ +loadFromExtension('framework', array( + 'workflows' => array( + 'my_workflow' => array( + 'marking_store' => array( + 'type' => 'multiple_state', + ), + 'places' => array( + 'first', + 'last', + ), + 'transitions' => array( + 'go' => array( + 'from' => array( + 'first', + ), + 'to' => array( + 'last', + ), + ), + ), + ), + ), +)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml new file mode 100644 index 0000000000000..92e26ff327d94 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml @@ -0,0 +1,21 @@ + + + + + + + + Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + first + last + + a + a + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml new file mode 100644 index 0000000000000..14bb287cca489 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml @@ -0,0 +1,20 @@ + + + + + + + + first + last + + a + a + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml new file mode 100644 index 0000000000000..743708485ce65 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml @@ -0,0 +1,17 @@ +framework: + workflows: + my_workflow: + marking_store: + type: multiple_state + supports: + - Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + support_strategy: foobar + places: + - first + - last + transitions: + go: + from: + - first + to: + - last diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml new file mode 100644 index 0000000000000..6dc848d936b21 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml @@ -0,0 +1,14 @@ +framework: + workflows: + my_workflow: + marking_store: + type: multiple_state + places: + - first + - last + transitions: + go: + from: + - first + to: + - last diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 9a580d3abf28d..ec63a505b7c16 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -168,7 +168,6 @@ public function testWorkflows() $this->assertCount(9, $stateMachineDefinition->getArgument(1)); $this->assertSame('start', $stateMachineDefinition->getArgument(2)); - $serviceMarkingStoreWorkflowDefinition = $container->getDefinition('workflow.service_marking_store_workflow'); /** @var Reference $markingStoreRef */ $markingStoreRef = $serviceMarkingStoreWorkflowDefinition->getArgument(1); @@ -189,6 +188,24 @@ public function testWorkflowCannotHaveBothTypeAndService() $this->createContainerFromFile('workflow_with_type_and_service'); } + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage "supports" and "support_strategy" cannot be used together. + */ + public function testWorkflowCannotHaveBothSupportsAndSupportStrategy() + { + $this->createContainerFromFile('workflow_with_support_and_support_strategy'); + } + + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage "supports" or "support_strategy" should be configured. + */ + public function testWorkflowShouldHaveOneOfSupportsAndSupportStrategy() + { + $this->createContainerFromFile('workflow_without_support_and_support_strategy'); + } + /** * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException * @expectedExceptionMessage "arguments" and "service" cannot be used together. diff --git a/src/Symfony/Component/Workflow/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md index c4df4750f73b2..de471f0e2cbe0 100644 --- a/src/Symfony/Component/Workflow/CHANGELOG.md +++ b/src/Symfony/Component/Workflow/CHANGELOG.md @@ -1,2 +1,8 @@ CHANGELOG ========= + +3.3.0 +----- + + * Deprecated class name support in `WorkflowRegistry::add()` as second parameter. + Wrap the class name in an instance of ClassInstanceSupportStrategy instead. diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php index 787773cf5cf1b..4bc806d200ba0 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\ClassInstanceSupportStrategy; use Symfony\Component\Workflow\SupportStrategy\SupportStrategyInterface; /** @@ -30,6 +31,8 @@ public function add(Workflow $workflow, $supportStrategy) { if (!$supportStrategy instanceof SupportStrategyInterface) { @trigger_error('Support of class name string was deprecated after version 3.2 and won\'t work anymore in 4.0.', E_USER_DEPRECATED); + + $supportStrategy = new ClassInstanceSupportStrategy($supportStrategy); } $this->workflows[] = array($workflow, $supportStrategy); @@ -63,17 +66,10 @@ public function get($subject, $workflowName = null) private function supports(Workflow $workflow, $supportStrategy, $subject, $workflowName) { - if (is_string($supportStrategy) && !$subject instanceof $supportStrategy) { - return false; - } - if ($supportStrategy instanceof SupportStrategyInterface && !$supportStrategy->supports($workflow, $subject)) { + if (null !== $workflowName && $workflowName !== $workflow->getName()) { return false; } - if (null === $workflowName) { - return true; - } - - return $workflowName === $workflow->getName(); + return $supportStrategy->supports($workflow, $subject); } } diff --git a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php index 5e229f0b96545..c2aaa27212654 100644 --- a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php +++ b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php @@ -4,10 +4,16 @@ use Symfony\Component\Workflow\Workflow; -class ClassInstanceSupportStrategy implements SupportStrategyInterface +/** + * @author Andreas Kleemann + */ +final class ClassInstanceSupportStrategy implements SupportStrategyInterface { private $className; + /** + * @param string $className a FQCN + */ public function __construct($className) { $this->className = $className; diff --git a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php index 94ec7adc7b6ee..097c6c4d9fe76 100644 --- a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php +++ b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php @@ -13,6 +13,9 @@ use Symfony\Component\Workflow\Workflow; +/** + * @author Andreas Kleemann + */ interface SupportStrategyInterface { /** diff --git a/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php b/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php index 42463d4b2fb1c..a27bfa7f2ce5a 100644 --- a/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php +++ b/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php @@ -11,17 +11,17 @@ public function testSupportsIfClassInstance() { $strategy = new ClassInstanceSupportStrategy('Symfony\Component\Workflow\Tests\SupportStrategy\Subject1'); - $this->assertTrue($strategy->supports($this->getWorkflow(), new Subject1())); + $this->assertTrue($strategy->supports($this->createWorkflow(), new Subject1())); } public function testSupportsIfNotClassInstance() { $strategy = new ClassInstanceSupportStrategy('Symfony\Component\Workflow\Tests\SupportStrategy\Subject2'); - $this->assertFalse($strategy->supports($this->getWorkflow(), new Subject1())); + $this->assertFalse($strategy->supports($this->createWorkflow(), new Subject1())); } - private function getWorkflow() + private function createWorkflow() { return $this->getMockBuilder(Workflow::class) ->disableOriginalConstructor()