From 29d658813244e377507ea900dc1b5ee565a72b5e Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 19 Jul 2018 20:09:12 +0300 Subject: [PATCH 01/11] [FrameworkBundle] fixed guard event names for transitions --- .../DependencyInjection/FrameworkExtension.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index a29a0ceadcc5d..736e98e306acb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -606,7 +606,9 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $guard = new Definition(Workflow\EventListener\GuardListener::class); $guard->setPrivate(true); $configuration = array(); - foreach ($workflow['transitions'] as $transitionName => $config) { + foreach ($workflow['transitions'] as $config) { + $transitionName = $config['name']; + if (!isset($config['guard'])) { continue; } From 49ceb2cf99a73d1cba172524bcade69e56aadb55 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Fri, 20 Jul 2018 16:19:51 +0300 Subject: [PATCH 02/11] [FrameworkBundle][Workflow] register guard expression per every transition in configuration --- .../FrameworkExtension.php | 40 ++++++++++------ .../EventListener/GuardExpression.php | 46 +++++++++++++++++++ .../Workflow/EventListener/GuardListener.php | 15 +++++- 3 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 src/Symfony/Component/Workflow/EventListener/GuardExpression.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 736e98e306acb..26d98d2633076 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -510,6 +510,7 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ // Create transitions $transitions = array(); + $guardsConfiguration = array(); $transitionsMetadataDefinition = new Definition(\SplObjectStorage::class); foreach ($workflow['transitions'] as $transition) { if ('workflow' === $type) { @@ -521,6 +522,16 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $transition['metadata'], )); } + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument($transitionDefinition); + $configuration->addArgument($transition['guard']); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + if (!isset($guardsConfiguration[$eventName])) { + $guardsConfiguration[$eventName] = array(); + } + $guardsConfiguration[$eventName][] = $configuration; + } } elseif ('state_machine' === $type) { foreach ($transition['from'] as $from) { foreach ($transition['to'] as $to) { @@ -532,6 +543,16 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $transition['metadata'], )); } + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument($transitionDefinition); + $configuration->addArgument($transition['guard']); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + if (!isset($guardsConfiguration[$eventName])) { + $guardsConfiguration[$eventName] = array(); + } + $guardsConfiguration[$eventName][] = $configuration; + } } } } @@ -605,14 +626,8 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ // Add Guard Listener $guard = new Definition(Workflow\EventListener\GuardListener::class); $guard->setPrivate(true); - $configuration = array(); - foreach ($workflow['transitions'] as $config) { - $transitionName = $config['name']; - - if (!isset($config['guard'])) { - continue; - } + if ($guardsConfiguration) { if (!class_exists(ExpressionLanguage::class)) { throw new LogicException('Cannot guard workflows as the ExpressionLanguage component is not installed.'); } @@ -621,13 +636,8 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ throw new LogicException('Cannot guard workflows as the Security component is not installed.'); } - $eventName = sprintf('workflow.%s.guard.%s', $name, $transitionName); - $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); - $configuration[$eventName] = $config['guard']; - } - if ($configuration) { $guard->setArguments(array( - $configuration, + $guardsConfiguration, new Reference('workflow.security.expression_language'), new Reference('security.token_storage'), new Reference('security.authorization_checker'), @@ -636,6 +646,10 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ new Reference('validator', ContainerInterface::NULL_ON_INVALID_REFERENCE), )); + foreach ($guardsConfiguration as $eventName => $config) { + $guard->addTag('kernel.event_listener', array('event' => $eventName, 'method' => 'onTransition')); + } + $container->setDefinition(sprintf('%s.listener.guard', $workflowId), $guard); $container->setParameter('workflow.has_guard_listeners', true); } diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php new file mode 100644 index 0000000000000..9e87d3f8c4fc4 --- /dev/null +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -0,0 +1,46 @@ +transition; + } + + /** + * @return string + */ + public function getExpression(): string + { + return $this->expression; + } + + /** + * GuardConfiguration constructor. + * @param Transition $transition + * @param string $expression + */ + public function __construct(Transition $transition, string $expression) + { + $this->transition = $transition; + $this->expression = $expression; + } +} \ No newline at end of file diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index 912dc5dada540..4f1c229e51b6f 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardListener.php @@ -50,8 +50,21 @@ public function onTransition(GuardEvent $event, $eventName) return; } - $expression = $this->configuration[$eventName]; + $eventConfiguration = (array) $this->configuration[$eventName]; + foreach ($eventConfiguration as $guard) { + if ($guard instanceof GuardExpression) { + if ($guard->getTransition() !== $event->getTransition()) { + continue; + } + $this->validateGuardExpression($event, $guard->getExpression()); + } else { + $this->validateGuardExpression($event, $guard); + } + } + } + private function validateGuardExpression(GuardEvent $event, string $expression) + { if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { $blocker = TransitionBlocker::createBlockedByExpressionGuardListener($expression); $event->addTransitionBlocker($blocker); From 5d5f2fb2faa03e957c95030a0cd69b0169127b46 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Fri, 20 Jul 2018 16:20:35 +0300 Subject: [PATCH 03/11] [FrameworkBundle] workflow guard expressions tests --- .../Resources/config/schema/symfony-1.0.xsd | 1 + .../php/workflow_with_guard_expression.php | 51 +++++++++++++++++++ .../xml/workflow_with_guard_expression.xml | 48 +++++++++++++++++ .../yml/workflow_with_guard_expression.yml | 35 +++++++++++++ .../FrameworkExtensionTest.php | 25 +++++++++ 5 files changed, 160 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml 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 eeebffb1d3ad7..4be4d7d670313 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 @@ -311,6 +311,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php new file mode 100644 index 0000000000000..89c86339afe15 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_guard_expression.php @@ -0,0 +1,51 @@ +loadFromExtension('framework', array( + 'workflows' => array( + 'article' => array( + 'type' => 'workflow', + 'marking_store' => array( + 'type' => 'multiple_state', + ), + 'supports' => array( + FrameworkExtensionTest::class, + ), + 'initial_place' => 'draft', + 'places' => array( + 'draft', + 'wait_for_journalist', + 'approved_by_journalist', + 'wait_for_spellchecker', + 'approved_by_spellchecker', + 'published', + ), + 'transitions' => array( + 'request_review' => array( + 'from' => 'draft', + 'to' => array('wait_for_journalist', 'wait_for_spellchecker'), + ), + 'journalist_approval' => array( + 'from' => 'wait_for_journalist', + 'to' => 'approved_by_journalist', + ), + 'spellchecker_approval' => array( + 'from' => 'wait_for_spellchecker', + 'to' => 'approved_by_spellchecker', + ), + 'publish' => array( + 'from' => array('approved_by_journalist', 'approved_by_spellchecker'), + 'to' => 'published', + 'guard' => '!!true', + ), + 'publish_editor_in_chief' => array( + 'name' => 'publish', + 'from' => 'draft', + 'to' => 'published', + 'guard' => '!!false', + ), + ), + ), + ), +)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml new file mode 100644 index 0000000000000..cf129e45c33c0 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_guard_expression.xml @@ -0,0 +1,48 @@ + + + + + + + + a + a + + Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + + + + + + + + draft + wait_for_journalist + wait_for_spellchecker + + + wait_for_journalist + approved_by_journalist + + + wait_for_spellchecker + approved_by_spellchecker + + + approved_by_journalist + approved_by_spellchecker + published + !!true + + + draft + published + !!false + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml new file mode 100644 index 0000000000000..458cb4ae1ee77 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_guard_expression.yml @@ -0,0 +1,35 @@ +framework: + workflows: + article: + type: workflow + marking_store: + type: multiple_state + supports: + - Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest + initial_place: draft + places: + - draft + - wait_for_journalist + - approved_by_journalist + - wait_for_spellchecker + - approved_by_spellchecker + - published + transitions: + request_review: + from: [draft] + to: [wait_for_journalist, wait_for_spellchecker] + journalist_approval: + from: [wait_for_journalist] + to: [approved_by_journalist] + spellchecker_approval: + from: [wait_for_spellchecker] + to: [approved_by_spellchecker] + publish: + from: [approved_by_journalist, approved_by_spellchecker] + to: [published] + guard: "!!true" + publish_editor_in_chief: + name: publish + from: [draft] + to: [published] + guard: "!!false" diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 69d5647e2fec4..dcd677b22bfae 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -332,6 +332,31 @@ public function testWorkflowMultipleTransitionsWithSameName() $this->assertSame(array('draft'), $transitions[4]->getArgument(1)); } + public function testGuardExpressions() + { + $container = $this->createContainerFromFile('workflow_with_guard_expression'); + + $this->assertTrue($container->hasDefinition('workflow.article.listener.guard'), 'Workflow guard listener is registered as a service'); + $this->assertTrue($container->hasParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter exists'); + $this->assertTrue(true === $container->getParameter('workflow.has_guard_listeners'), 'Workflow guard listeners parameter is enabled'); + $guardDefinition = $container->getDefinition('workflow.article.listener.guard'); + $this->assertSame(array( + array( + 'event' => 'workflow.article.guard.publish', + 'method' => 'onTransition' + ) + ), $guardDefinition->getTag('kernel.event_listener')); + $guardsConfiguration = $guardDefinition->getArgument(0); + $this->assertTrue(1 === count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); + $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; + $workflowDefinition = $container->getDefinition('workflow.article.definition'); + $transitions = $workflowDefinition->getArgument(1); + $this->assertSame($transitions[3], $transitionGuardExpressions[0]->getArgument(0)); + $this->assertSame("!!true", $transitionGuardExpressions[0]->getArgument(1)); + $this->assertSame($transitions[4], $transitionGuardExpressions[1]->getArgument(0)); + $this->assertSame("!!false", $transitionGuardExpressions[1]->getArgument(1)); + } + public function testWorkflowServicesCanBeEnabled() { $container = $this->createContainerFromFile('workflows_enabled'); From 5a44ca2f7f6d4ee21475e817fdbfe64f8c9a335c Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Fri, 20 Jul 2018 16:21:13 +0300 Subject: [PATCH 04/11] [Workflow] test with guard expression bound to exact transitions --- .../Tests/EventListener/GuardListenerTest.php | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index f0cc707220cbf..1a15be494d390 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -11,6 +11,7 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Workflow\Event\GuardEvent; use Symfony\Component\Workflow\EventListener\ExpressionLanguage; +use Symfony\Component\Workflow\EventListener\GuardExpression; use Symfony\Component\Workflow\EventListener\GuardListener; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Transition; @@ -21,12 +22,17 @@ class GuardListenerTest extends TestCase private $authenticationChecker; private $validator; private $listener; + private $transition; protected function setUp() { $configuration = array( 'test_is_granted' => 'is_granted("something")', 'test_is_valid' => 'is_valid(subject)', + 'test_expression' => array( + new GuardExpression($this->getTransition(true), '!is_valid(subject)'), + new GuardExpression($this->getTransition(true), 'is_valid(subject)'), + ), ); $expressionLanguage = new ExpressionLanguage(); $token = $this->getMockBuilder(TokenInterface::class)->getMock(); @@ -97,11 +103,49 @@ public function testWithValidatorSupportedEventAndAccept() $this->assertFalse($event->isBlocked()); } - private function createEvent() + public function testWithGuardExpression() + { + $event = $this->createEvent(); + $this->configureValidator(true, true); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testWithGuardExpressionWithNotSupportedTransition() + { + $event = $this->createEvent(true); + $this->configureValidator(false, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testWithGuardExpressionWithSupportedTransition() + { + $event = $this->createEvent(); + $this->configureValidator(true, true); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testGuardExpressionBlocks() + { + $event = $this->createEvent(); + $this->configureValidator(true, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertTrue($event->isBlocked()); + } + + + + private function createEvent($newTransition = false) { $subject = new \stdClass(); $subject->marking = new Marking(); - $transition = new Transition('name', 'from', 'to'); + $transition = $this->getTransition($newTransition); $workflow = $this->getMockBuilder(WorkflowInterface::class)->getMock(); @@ -143,4 +187,12 @@ private function configureValidator($isUsed, $valid = true) ->willReturn($valid ? array() : array('a violation')) ; } + + private function getTransition($new = false) + { + if ($new || !$this->transition) { + $this->transition = new Transition('name', 'from', 'to'); + } + return $this->transition; + } } From 3dda5ee5e61cb90e2334e4f20895521d85f609e1 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 13:35:49 +0300 Subject: [PATCH 05/11] [FrameworkBundle][Workflow] Add transitions to service container and pass them by reference to guard expressions --- .../FrameworkExtension.php | 32 ++++----- .../FrameworkExtensionTest.php | 67 ++++++++++++++++--- 2 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 26d98d2633076..37830d8f183d1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -492,6 +492,7 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ foreach ($config['workflows'] as $name => $workflow) { $type = $workflow['type']; + $workflowId = sprintf('%s.%s', $type, $name); // Process Metadata (workflow + places (transition is done in the "create transition" block)) $metadataStoreDefinition = new Definition(Workflow\Metadata\InMemoryMetadataStore::class, array(array(), array(), null)); @@ -512,10 +513,14 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $transitions = array(); $guardsConfiguration = array(); $transitionsMetadataDefinition = new Definition(\SplObjectStorage::class); - foreach ($workflow['transitions'] as $transition) { + // Global transition counter per workflow + $transitionCounter = 0; + foreach (array_values($workflow['transitions']) as $number => $transition) { if ('workflow' === $type) { $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); - $transitions[] = $transitionDefinition; + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, @@ -524,19 +529,18 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ } if (isset($transition['guard'])) { $configuration = new Definition(Workflow\EventListener\GuardExpression::class); - $configuration->addArgument($transitionDefinition); + $configuration->addArgument(new Reference($transitionId)); $configuration->addArgument($transition['guard']); $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); - if (!isset($guardsConfiguration[$eventName])) { - $guardsConfiguration[$eventName] = array(); - } $guardsConfiguration[$eventName][] = $configuration; } } elseif ('state_machine' === $type) { - foreach ($transition['from'] as $from) { - foreach ($transition['to'] as $to) { + foreach ($transition['from'] as $keyFrom => $from) { + foreach ($transition['to'] as $keyTo => $to) { $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); - $transitions[] = $transitionDefinition; + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, @@ -544,13 +548,10 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ )); } if (isset($transition['guard'])) { - $configuration = new Definition(Workflow\EventListener\GuardExpression::class); - $configuration->addArgument($transitionDefinition); + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); $configuration->addArgument($transition['guard']); $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); - if (!isset($guardsConfiguration[$eventName])) { - $guardsConfiguration[$eventName] = array(); - } $guardsConfiguration[$eventName][] = $configuration; } } @@ -563,7 +564,6 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $places = array_map(function (array $place) { return $place['name']; }, $workflow['places']); - // Create a Definition $definitionDefinition = new Definition(Workflow\Definition::class); $definitionDefinition->setPublic(false); @@ -588,7 +588,7 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ } // Create Workflow - $workflowId = sprintf('%s.%s', $type, $name); + $workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type)); $workflowDefinition->replaceArgument(0, new Reference(sprintf('%s.definition', $workflowId))); if (isset($markingStoreDefinition)) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index dcd677b22bfae..428bd50f43b37 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -322,14 +322,61 @@ public function testWorkflowMultipleTransitionsWithSameName() $this->assertCount(5, $transitions); - $this->assertSame('request_review', $transitions[0]->getArgument(0)); - $this->assertSame('journalist_approval', $transitions[1]->getArgument(0)); - $this->assertSame('spellchecker_approval', $transitions[2]->getArgument(0)); - $this->assertSame('publish', $transitions[3]->getArgument(0)); - $this->assertSame('publish', $transitions[4]->getArgument(0)); + $this->assertSame('workflow.article.transition.0', (string) $transitions[0]); + $this->assertSame(array( + 'request_review', + array( + 'draft', + ), + array( + 'wait_for_journalist', 'wait_for_spellchecker', + ), + ), $container->getDefinition($transitions[0])->getArguments()); + + $this->assertSame('workflow.article.transition.1', (string) $transitions[1]); + $this->assertSame(array( + 'journalist_approval', + array( + 'wait_for_journalist', + ), + array( + 'approved_by_journalist', + ), + ), $container->getDefinition($transitions[1])->getArguments()); + + $this->assertSame('workflow.article.transition.2', (string) $transitions[2]); + $this->assertSame(array( + 'spellchecker_approval', + array( + 'wait_for_spellchecker', + ), + array( + 'approved_by_spellchecker', + ), + ), $container->getDefinition($transitions[2])->getArguments()); + + $this->assertSame('workflow.article.transition.3', (string) $transitions[3]); + $this->assertSame(array( + 'publish', + array( + 'approved_by_journalist', + 'approved_by_spellchecker', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[3])->getArguments()); - $this->assertSame(array('approved_by_journalist', 'approved_by_spellchecker'), $transitions[3]->getArgument(1)); - $this->assertSame(array('draft'), $transitions[4]->getArgument(1)); + $this->assertSame('workflow.article.transition.4', (string) $transitions[4]); + $this->assertSame(array( + 'publish', + array( + 'draft', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[4])->getArguments()); } public function testGuardExpressions() @@ -349,11 +396,9 @@ public function testGuardExpressions() $guardsConfiguration = $guardDefinition->getArgument(0); $this->assertTrue(1 === count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; - $workflowDefinition = $container->getDefinition('workflow.article.definition'); - $transitions = $workflowDefinition->getArgument(1); - $this->assertSame($transitions[3], $transitionGuardExpressions[0]->getArgument(0)); + $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); $this->assertSame("!!true", $transitionGuardExpressions[0]->getArgument(1)); - $this->assertSame($transitions[4], $transitionGuardExpressions[1]->getArgument(0)); + $this->assertSame('workflow.article.transition.4', (string) $transitionGuardExpressions[1]->getArgument(0)); $this->assertSame("!!false", $transitionGuardExpressions[1]->getArgument(1)); } From 1ef1e3b8594ffd7ef05fa10b176cc4801cee9c46 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 13:42:48 +0300 Subject: [PATCH 06/11] [CS] remove docblocks in guard expression class --- .../EventListener/GuardExpression.php | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php index 9e87d3f8c4fc4..b634b37129173 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -6,41 +6,23 @@ class GuardExpression { - - /** - * @var Transition - */ private $transition; - /** - * @var string - */ private $expression; - /** - * @return Transition - */ public function getTransition(): Transition { return $this->transition; } - /** - * @return string - */ public function getExpression(): string { return $this->expression; } - /** - * GuardConfiguration constructor. - * @param Transition $transition - * @param string $expression - */ public function __construct(Transition $transition, string $expression) { $this->transition = $transition; $this->expression = $expression; } -} \ No newline at end of file +} From f93381d3e4ecccc0fb96faef8c4571df7dce53d0 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 13:45:25 +0300 Subject: [PATCH 07/11] [CS] remove unused keys --- .../DependencyInjection/FrameworkExtension.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 37830d8f183d1..6ebfa2ab5f5ad 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -521,12 +521,14 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); $container->setDefinition($transitionId, $transitionDefinition); $transitions[] = new Reference($transitionId); + if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, $transition['metadata'], )); } + if (isset($transition['guard'])) { $configuration = new Definition(Workflow\EventListener\GuardExpression::class); $configuration->addArgument(new Reference($transitionId)); @@ -535,18 +537,20 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ $guardsConfiguration[$eventName][] = $configuration; } } elseif ('state_machine' === $type) { - foreach ($transition['from'] as $keyFrom => $from) { - foreach ($transition['to'] as $keyTo => $to) { + foreach ($transition['from'] as $from) { + foreach ($transition['to'] as $to) { $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); $container->setDefinition($transitionId, $transitionDefinition); $transitions[] = new Reference($transitionId); + if ($transition['metadata']) { $transitionsMetadataDefinition->addMethodCall('attach', array( $transitionDefinition, $transition['metadata'], )); } + if (isset($transition['guard'])) { $configuration = new Definition(Workflow\EventListener\GuardExpression::class); $configuration->addArgument(new Reference($transitionId)); From 1f2b8d7ec7abd9fb07f6008088fa9bc2d17b61f3 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 13:52:56 +0300 Subject: [PATCH 08/11] [CS] apply coding standards recommendations --- .../DependencyInjection/FrameworkExtension.php | 2 +- .../Tests/DependencyInjection/FrameworkExtensionTest.php | 8 ++++---- .../Workflow/Tests/EventListener/GuardListenerTest.php | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 6ebfa2ab5f5ad..bf529ab5c8bc3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -530,7 +530,7 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ } if (isset($transition['guard'])) { - $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); $configuration->addArgument(new Reference($transitionId)); $configuration->addArgument($transition['guard']); $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 428bd50f43b37..ae94325d71ec2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -390,16 +390,16 @@ public function testGuardExpressions() $this->assertSame(array( array( 'event' => 'workflow.article.guard.publish', - 'method' => 'onTransition' - ) + 'method' => 'onTransition', + ), ), $guardDefinition->getTag('kernel.event_listener')); $guardsConfiguration = $guardDefinition->getArgument(0); $this->assertTrue(1 === count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); - $this->assertSame("!!true", $transitionGuardExpressions[0]->getArgument(1)); + $this->assertSame('!!true', $transitionGuardExpressions[0]->getArgument(1)); $this->assertSame('workflow.article.transition.4', (string) $transitionGuardExpressions[1]->getArgument(0)); - $this->assertSame("!!false", $transitionGuardExpressions[1]->getArgument(1)); + $this->assertSame('!!false', $transitionGuardExpressions[1]->getArgument(1)); } public function testWorkflowServicesCanBeEnabled() diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index 1a15be494d390..23289515e0414 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -139,8 +139,6 @@ public function testGuardExpressionBlocks() $this->assertTrue($event->isBlocked()); } - - private function createEvent($newTransition = false) { $subject = new \stdClass(); @@ -193,6 +191,7 @@ private function getTransition($new = false) if ($new || !$this->transition) { $this->transition = new Transition('name', 'from', 'to'); } + return $this->transition; } } From 05506c4cd9292ef31631960b8487326cb689fc8f Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 14:04:59 +0300 Subject: [PATCH 09/11] [CS] add license header --- .../Component/Workflow/EventListener/GuardExpression.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php index b634b37129173..cf3b8c7e18694 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Component\Workflow\EventListener; use Symfony\Component\Workflow\Transition; From 720f943fb7645f5c615348c0e2bdf2446da18a9e Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 14:13:03 +0300 Subject: [PATCH 10/11] [Workflow] remove duplicate test --- .../Workflow/Tests/EventListener/GuardListenerTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index 23289515e0414..253f21743b9b9 100644 --- a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php +++ b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php @@ -103,15 +103,6 @@ public function testWithValidatorSupportedEventAndAccept() $this->assertFalse($event->isBlocked()); } - public function testWithGuardExpression() - { - $event = $this->createEvent(); - $this->configureValidator(true, true); - $this->listener->onTransition($event, 'test_expression'); - - $this->assertFalse($event->isBlocked()); - } - public function testWithGuardExpressionWithNotSupportedTransition() { $event = $this->createEvent(true); From ac835a83eac0d5534eefdbd5aea2e8c88e6d8be0 Mon Sep 17 00:00:00 2001 From: Ivan Nikolaev Date: Thu, 26 Jul 2018 14:51:48 +0300 Subject: [PATCH 11/11] [CS] apply cs recommendations --- .../Tests/DependencyInjection/FrameworkExtensionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index ae94325d71ec2..185a1bcd3c84e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -394,7 +394,7 @@ public function testGuardExpressions() ), ), $guardDefinition->getTag('kernel.event_listener')); $guardsConfiguration = $guardDefinition->getArgument(0); - $this->assertTrue(1 === count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); + $this->assertTrue(1 === \count($guardsConfiguration), 'Workflow guard configuration contains one element per transition name'); $transitionGuardExpressions = $guardsConfiguration['workflow.article.guard.publish']; $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); $this->assertSame('!!true', $transitionGuardExpressions[0]->getArgument(1));