diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 5b6c8b447ed7d..9ec02f07228a5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -602,15 +602,44 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ @trigger_error(sprintf('The "type" option of the "framework.workflows.%s" configuration entry must be defined since Symfony 3.3. The default value will be "state_machine" in Symfony 4.0.', $name), E_USER_DEPRECATED); } $type = $workflow['type']; + $workflowId = sprintf('%s.%s', $type, $name); + // Create transitions $transitions = array(); + $guardsConfiguration = array(); + // Global transition counter per workflow + $transitionCounter = 0; foreach ($workflow['transitions'] as $transition) { if ('workflow' === $type) { - $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); + $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to'])); + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } } elseif ('state_machine' === $type) { foreach ($transition['from'] as $from) { foreach ($transition['to'] as $to) { - $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); + $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to)); + $transitionDefinition->setPublic(false); + $transitionId = sprintf('%s.transition.%s', $workflowId, $transitionCounter++); + $container->setDefinition($transitionId, $transitionDefinition); + $transitions[] = new Reference($transitionId); + if (isset($transition['guard'])) { + $configuration = new Definition(Workflow\EventListener\GuardExpression::class); + $configuration->addArgument(new Reference($transitionId)); + $configuration->addArgument($transition['guard']); + $configuration->setPublic(false); + $eventName = sprintf('workflow.%s.guard.%s', $name, $transition['name']); + $guardsConfiguration[$eventName][] = $configuration; + } } } } @@ -641,7 +670,6 @@ 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)) { @@ -677,16 +705,7 @@ 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. Try running "composer require symfony/expression-language".'); } @@ -695,13 +714,11 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ throw new LogicException('Cannot guard workflows as the Security component is not installed. Try running "composer require symfony/security".'); } - $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 = new Definition(Workflow\EventListener\GuardListener::class); + $guard->setPrivate(true); + $guard->setArguments(array( - $configuration, + $guardsConfiguration, new Reference('workflow.security.expression_language'), new Reference('security.token_storage'), new Reference('security.authorization_checker'), @@ -709,6 +726,9 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $ new Reference('security.role_hierarchy'), 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/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 1feb77bba863f..adbf4e5c574ef 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 @@ -300,6 +300,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..a5124d1fe7776 --- /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 + approved_by_journalist + wait_for_spellchecker + approved_by_spellchecker + published + + 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 38420ac1e7672..ae6dc7a75acef 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -302,14 +302,84 @@ 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(array('approved_by_journalist', 'approved_by_spellchecker'), $transitions[3]->getArgument(1)); - $this->assertSame(array('draft'), $transitions[4]->getArgument(1)); + $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('workflow.article.transition.4', (string) $transitions[4]); + $this->assertSame(array( + 'publish', + array( + 'draft', + ), + array( + 'published', + ), + ), $container->getDefinition($transitions[4])->getArguments()); + } + + 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']; + $this->assertSame('workflow.article.transition.3', (string) $transitionGuardExpressions[0]->getArgument(0)); + $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)); } public function testWorkflowServicesCanBeEnabled() diff --git a/src/Symfony/Component/Workflow/EventListener/GuardExpression.php b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php new file mode 100644 index 0000000000000..09ab15086bdff --- /dev/null +++ b/src/Symfony/Component/Workflow/EventListener/GuardExpression.php @@ -0,0 +1,40 @@ + + * + * 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; + +class GuardExpression +{ + private $transition; + + private $expression; + + /** + * @param string $expression + */ + public function __construct(Transition $transition, $expression) + { + $this->transition = $transition; + $this->expression = $expression; + } + + public function getTransition() + { + return $this->transition; + } + + public function getExpression() + { + return $this->expression; + } +} diff --git a/src/Symfony/Component/Workflow/EventListener/GuardListener.php b/src/Symfony/Component/Workflow/EventListener/GuardListener.php index 893f304e47834..4d3cfac57e7dc 100644 --- a/src/Symfony/Component/Workflow/EventListener/GuardListener.php +++ b/src/Symfony/Component/Workflow/EventListener/GuardListener.php @@ -32,7 +32,7 @@ class GuardListener private $roleHierarchy; private $validator; - public function __construct($configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null) + public function __construct(array $configuration, ExpressionLanguage $expressionLanguage, TokenStorageInterface $tokenStorage, AuthorizationCheckerInterface $authenticationChecker, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null, ValidatorInterface $validator = null) { $this->configuration = $configuration; $this->expressionLanguage = $expressionLanguage; @@ -49,7 +49,22 @@ public function onTransition(GuardEvent $event, $eventName) return; } - if (!$this->expressionLanguage->evaluate($this->configuration[$eventName], $this->getVariables($event))) { + $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, $expression) + { + if (!$this->expressionLanguage->evaluate($expression, $this->getVariables($event))) { $event->setBlocked(true); } } diff --git a/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php b/src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php index e224835a07649..8686d74cf6ca9 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; @@ -20,12 +21,17 @@ class GuardListenerTest extends TestCase private $authenticationChecker; private $validator; private $listener; + private $configuration; protected function setUp() { - $configuration = array( + $this->configuration = array( 'test_is_granted' => 'is_granted("something")', 'test_is_valid' => 'is_valid(subject)', + 'test_expression' => array( + new GuardExpression(new Transition('name', 'from', 'to'), '!is_valid(subject)'), + new GuardExpression(new Transition('name', 'from', 'to'), 'is_valid(subject)'), + ), ); $expressionLanguage = new ExpressionLanguage(); $token = $this->getMockBuilder(TokenInterface::class)->getMock(); @@ -35,7 +41,7 @@ protected function setUp() $this->authenticationChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock(); $trustResolver = $this->getMockBuilder(AuthenticationTrustResolverInterface::class)->getMock(); $this->validator = $this->getMockBuilder(ValidatorInterface::class)->getMock(); - $this->listener = new GuardListener($configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); + $this->listener = new GuardListener($this->configuration, $expressionLanguage, $tokenStorage, $this->authenticationChecker, $trustResolver, null, $this->validator); } protected function tearDown() @@ -96,11 +102,38 @@ public function testWithValidatorSupportedEventAndAccept() $this->assertFalse($event->isBlocked()); } - private function createEvent() + public function testWithGuardExpressionWithNotSupportedTransition() + { + $event = $this->createEvent(); + $this->configureValidator(false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testWithGuardExpressionWithSupportedTransition() + { + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); + $this->configureValidator(true, true); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertFalse($event->isBlocked()); + } + + public function testGuardExpressionBlocks() + { + $event = $this->createEvent($this->configuration['test_expression'][1]->getTransition()); + $this->configureValidator(true, false); + $this->listener->onTransition($event, 'test_expression'); + + $this->assertTrue($event->isBlocked()); + } + + private function createEvent(Transition $transition = null) { $subject = new \stdClass(); $subject->marking = new Marking(); - $transition = new Transition('name', 'from', 'to'); + $transition = $transition ?: new Transition('name', 'from', 'to'); return new GuardEvent($subject, $subject->marking, $transition); }