minor #29457 [Workflow] Move code from ValidateWorkflowsPass to the FrameworkExtension (lyrixx)
This PR was merged into the 4.3-dev branch.
Discussion
----------
[Workflow] Move code from ValidateWorkflowsPass to the FrameworkExtension
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
---
Just some cleaning. See
https://github.com/symfony/symfony/pull/29146#issuecomment-44418654
Commits
-------
a608797165
[Workflow] Move code from ValidateWorkflowsPass to the FrameworkExtension
This commit is contained in:
commit
f9d3848581
@ -603,20 +603,15 @@ class FrameworkExtension extends Extension
|
|||||||
|
|
||||||
// Create places
|
// Create places
|
||||||
$places = array_column($workflow['places'], 'name');
|
$places = array_column($workflow['places'], 'name');
|
||||||
|
$initialPlace = $workflow['initial_place'] ?? null;
|
||||||
|
|
||||||
// Create a Definition
|
// Create a Definition
|
||||||
$definitionDefinition = new Definition(Workflow\Definition::class);
|
$definitionDefinition = new Definition(Workflow\Definition::class);
|
||||||
$definitionDefinition->setPublic(false);
|
$definitionDefinition->setPublic(false);
|
||||||
$definitionDefinition->addArgument($places);
|
$definitionDefinition->addArgument($places);
|
||||||
$definitionDefinition->addArgument($transitions);
|
$definitionDefinition->addArgument($transitions);
|
||||||
$definitionDefinition->addArgument($workflow['initial_place'] ?? null);
|
$definitionDefinition->addArgument($initialPlace);
|
||||||
$definitionDefinition->addArgument($metadataStoreDefinition);
|
$definitionDefinition->addArgument($metadataStoreDefinition);
|
||||||
$definitionDefinition->addTag('workflow.definition', [
|
|
||||||
'name' => $name,
|
|
||||||
'type' => $type,
|
|
||||||
'marking_store' => $workflow['marking_store']['type'] ?? null,
|
|
||||||
'single_state' => 'method' === ($workflow['marking_store']['type'] ?? null) && ($workflow['marking_store']['arguments'][0] ?? false),
|
|
||||||
]);
|
|
||||||
|
|
||||||
// Create MarkingStore
|
// Create MarkingStore
|
||||||
if (isset($workflow['marking_store']['type'])) {
|
if (isset($workflow['marking_store']['type'])) {
|
||||||
@ -641,6 +636,34 @@ class FrameworkExtension extends Extension
|
|||||||
$container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
|
$container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
|
||||||
$container->registerAliasForArgument($workflowId, WorkflowInterface::class, $name.'.'.$type);
|
$container->registerAliasForArgument($workflowId, WorkflowInterface::class, $name.'.'.$type);
|
||||||
|
|
||||||
|
// Validate Workflow
|
||||||
|
$validator = null;
|
||||||
|
switch (true) {
|
||||||
|
case 'state_machine' === $workflow['type']:
|
||||||
|
$validator = new Workflow\Validator\StateMachineValidator();
|
||||||
|
break;
|
||||||
|
case 'method' === ($workflow['marking_store']['type'] ?? null):
|
||||||
|
$singlePlace = $workflow['marking_store']['arguments'][0] ?? false;
|
||||||
|
$validator = new Workflow\Validator\WorkflowValidator($singlePlace);
|
||||||
|
break;
|
||||||
|
case 'single_state' === ($workflow['marking_store']['type'] ?? null):
|
||||||
|
$validator = new Workflow\Validator\WorkflowValidator(true);
|
||||||
|
break;
|
||||||
|
case 'multiple_state' === ($workflow['marking_store']['type'] ?? false):
|
||||||
|
$validator = new Workflow\Validator\WorkflowValidator(false);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if ($validator) {
|
||||||
|
$realDefinition = (new Workflow\DefinitionBuilder($places))
|
||||||
|
->addTransitions(array_map(function (Reference $ref) use ($container): Workflow\Transition {
|
||||||
|
return $container->get((string) $ref);
|
||||||
|
}, $transitions))
|
||||||
|
->setInitialPlace($initialPlace)
|
||||||
|
->build()
|
||||||
|
;
|
||||||
|
$validator->validate($realDefinition, $name);
|
||||||
|
}
|
||||||
|
|
||||||
// Add workflow to Registry
|
// Add workflow to Registry
|
||||||
if ($workflow['supports']) {
|
if ($workflow['supports']) {
|
||||||
foreach ($workflow['supports'] as $supportedClassName) {
|
foreach ($workflow['supports'] as $supportedClassName) {
|
||||||
|
@ -54,7 +54,6 @@ use Symfony\Component\Translation\DependencyInjection\TranslatorPass;
|
|||||||
use Symfony\Component\Translation\DependencyInjection\TranslatorPathsPass;
|
use Symfony\Component\Translation\DependencyInjection\TranslatorPathsPass;
|
||||||
use Symfony\Component\Validator\DependencyInjection\AddConstraintValidatorsPass;
|
use Symfony\Component\Validator\DependencyInjection\AddConstraintValidatorsPass;
|
||||||
use Symfony\Component\Validator\DependencyInjection\AddValidatorInitializersPass;
|
use Symfony\Component\Validator\DependencyInjection\AddValidatorInitializersPass;
|
||||||
use Symfony\Component\Workflow\DependencyInjection\ValidateWorkflowsPass;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Bundle.
|
* Bundle.
|
||||||
@ -115,7 +114,6 @@ class FrameworkBundle extends Bundle
|
|||||||
$container->addCompilerPass(new DataCollectorTranslatorPass());
|
$container->addCompilerPass(new DataCollectorTranslatorPass());
|
||||||
$container->addCompilerPass(new ControllerArgumentValueResolverPass());
|
$container->addCompilerPass(new ControllerArgumentValueResolverPass());
|
||||||
$container->addCompilerPass(new CachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 32);
|
$container->addCompilerPass(new CachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 32);
|
||||||
$this->addCompilerPassIfExists($container, ValidateWorkflowsPass::class);
|
|
||||||
$container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING);
|
$container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING);
|
||||||
$container->addCompilerPass(new CachePoolPrunerPass(), PassConfig::TYPE_AFTER_REMOVING);
|
$container->addCompilerPass(new CachePoolPrunerPass(), PassConfig::TYPE_AFTER_REMOVING);
|
||||||
$this->addCompilerPassIfExists($container, FormPass::class);
|
$this->addCompilerPassIfExists($container, FormPass::class);
|
||||||
|
@ -0,0 +1,30 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;
|
||||||
|
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'my_workflow' => [
|
||||||
|
'type' => 'state_machine',
|
||||||
|
'supports' => [
|
||||||
|
FrameworkExtensionTest::class,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'first',
|
||||||
|
'middle',
|
||||||
|
'last',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'go' => [
|
||||||
|
'from' => [
|
||||||
|
'first',
|
||||||
|
],
|
||||||
|
'to' => [
|
||||||
|
'middle',
|
||||||
|
'last',
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
@ -0,0 +1,22 @@
|
|||||||
|
<?xml version="1.0" ?>
|
||||||
|
|
||||||
|
<container xmlns="http://symfony.com/schema/dic/services"
|
||||||
|
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||||
|
xmlns:framework="http://symfony.com/schema/dic/symfony"
|
||||||
|
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
|
||||||
|
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
|
||||||
|
|
||||||
|
<framework:config>
|
||||||
|
<framework:workflow name="my_workflow" type="state_machine">
|
||||||
|
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
|
||||||
|
<framework:place name="first" />
|
||||||
|
<framework:place name="middle" />
|
||||||
|
<framework:place name="last" />
|
||||||
|
<framework:transition name="go">
|
||||||
|
<framework:from>first</framework:from>
|
||||||
|
<framework:to>middle</framework:to>
|
||||||
|
<framework:to>last</framework:to>
|
||||||
|
</framework:transition>
|
||||||
|
</framework:workflow>
|
||||||
|
</framework:config>
|
||||||
|
</container>
|
@ -29,12 +29,12 @@
|
|||||||
<framework:to>approved_by_journalist</framework:to>
|
<framework:to>approved_by_journalist</framework:to>
|
||||||
</framework:transition>
|
</framework:transition>
|
||||||
<framework:transition name="spellchecker_approval">
|
<framework:transition name="spellchecker_approval">
|
||||||
<framework:from>wait_for_spellcheker</framework:from>
|
<framework:from>wait_for_spellchecker</framework:from>
|
||||||
<framework:to>approved_by_spellchker</framework:to>
|
<framework:to>approved_by_spellchecker</framework:to>
|
||||||
</framework:transition>
|
</framework:transition>
|
||||||
<framework:transition name="publish">
|
<framework:transition name="publish">
|
||||||
<framework:from>approved_by_journalist</framework:from>
|
<framework:from>approved_by_journalist</framework:from>
|
||||||
<framework:from>approved_by_spellchker</framework:from>
|
<framework:from>approved_by_spellchecker</framework:from>
|
||||||
<framework:to>published</framework:to>
|
<framework:to>published</framework:to>
|
||||||
</framework:transition>
|
</framework:transition>
|
||||||
</framework:workflow>
|
</framework:workflow>
|
||||||
|
@ -0,0 +1,11 @@
|
|||||||
|
framework:
|
||||||
|
workflows:
|
||||||
|
my_workflow:
|
||||||
|
type: state_machine
|
||||||
|
supports:
|
||||||
|
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
|
||||||
|
places: [first, middle, last]
|
||||||
|
transitions:
|
||||||
|
go:
|
||||||
|
from: first
|
||||||
|
to: [last, middle ]
|
@ -215,7 +215,6 @@ abstract class FrameworkExtensionTest extends TestCase
|
|||||||
$workflowDefinition->getArgument(0),
|
$workflowDefinition->getArgument(0),
|
||||||
'Places are passed to the workflow definition'
|
'Places are passed to the workflow definition'
|
||||||
);
|
);
|
||||||
$this->assertSame(['workflow.definition' => [['name' => 'article', 'type' => 'workflow', 'marking_store' => 'multiple_state', 'single_state' => false]]], $workflowDefinition->getTags());
|
|
||||||
$this->assertCount(4, $workflowDefinition->getArgument(1));
|
$this->assertCount(4, $workflowDefinition->getArgument(1));
|
||||||
$this->assertSame('draft', $workflowDefinition->getArgument(2));
|
$this->assertSame('draft', $workflowDefinition->getArgument(2));
|
||||||
|
|
||||||
@ -237,7 +236,6 @@ abstract class FrameworkExtensionTest extends TestCase
|
|||||||
$stateMachineDefinition->getArgument(0),
|
$stateMachineDefinition->getArgument(0),
|
||||||
'Places are passed to the state machine definition'
|
'Places are passed to the state machine definition'
|
||||||
);
|
);
|
||||||
$this->assertSame(['workflow.definition' => [['name' => 'pull_request', 'type' => 'state_machine', 'marking_store' => 'single_state', 'single_state' => false]]], $stateMachineDefinition->getTags());
|
|
||||||
$this->assertCount(9, $stateMachineDefinition->getArgument(1));
|
$this->assertCount(9, $stateMachineDefinition->getArgument(1));
|
||||||
$this->assertSame('start', $stateMachineDefinition->getArgument(2));
|
$this->assertSame('start', $stateMachineDefinition->getArgument(2));
|
||||||
|
|
||||||
@ -272,6 +270,15 @@ abstract class FrameworkExtensionTest extends TestCase
|
|||||||
$this->assertGreaterThan(0, \count($registryDefinition->getMethodCalls()));
|
$this->assertGreaterThan(0, \count($registryDefinition->getMethodCalls()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
|
||||||
|
* @expectedExceptionMessage A transition from a place/state must have an unique name. Multiple transitions named "go" from place/state "first" where found on StateMachine "my_workflow".
|
||||||
|
*/
|
||||||
|
public function testWorkflowAreValidated()
|
||||||
|
{
|
||||||
|
$this->createContainerFromFile('workflow_not_valid');
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
|
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
|
||||||
* @expectedExceptionMessage "type" and "service" cannot be used together.
|
* @expectedExceptionMessage "type" and "service" cannot be used together.
|
||||||
|
@ -56,4 +56,178 @@ class PhpFrameworkExtensionTest extends FrameworkExtensionTest
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
|
||||||
|
* @expectedExceptionMessage A transition from a place/state must have an unique name. Multiple transitions named "a_to_b" from place/state "a" where found on StateMachine "article".
|
||||||
|
*/
|
||||||
|
public function testWorkflowValidationStateMachine()
|
||||||
|
{
|
||||||
|
$this->createContainerFromClosure(function ($container) {
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'article' => [
|
||||||
|
'type' => 'state_machine',
|
||||||
|
'supports' => [
|
||||||
|
__CLASS__,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'a',
|
||||||
|
'b',
|
||||||
|
'c',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'a_to_b' => [
|
||||||
|
'from' => ['a'],
|
||||||
|
'to' => ['b', 'c'],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
|
||||||
|
* @expectedExceptionMessage The marking store of workflow "article" can not store many places. But the transition "a_to_b" has too many output (2). Only one is accepted.
|
||||||
|
*/
|
||||||
|
public function testWorkflowValidationMethodSingle()
|
||||||
|
{
|
||||||
|
$this->createContainerFromClosure(function ($container) {
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'article' => [
|
||||||
|
'type' => 'workflow',
|
||||||
|
'marking_store' => [
|
||||||
|
'type' => 'method',
|
||||||
|
'arguments' => [
|
||||||
|
true,
|
||||||
|
],
|
||||||
|
],
|
||||||
|
'supports' => [
|
||||||
|
__CLASS__,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'a',
|
||||||
|
'b',
|
||||||
|
'c',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'a_to_b' => [
|
||||||
|
'from' => ['a'],
|
||||||
|
'to' => ['b', 'c'],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testWorkflowValidationMethodNotSingle()
|
||||||
|
{
|
||||||
|
$this->createContainerFromClosure(function ($container) {
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'article' => [
|
||||||
|
'type' => 'workflow',
|
||||||
|
'marking_store' => [
|
||||||
|
'type' => 'method',
|
||||||
|
'arguments' => [
|
||||||
|
false,
|
||||||
|
],
|
||||||
|
],
|
||||||
|
'supports' => [
|
||||||
|
__CLASS__,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'a',
|
||||||
|
'b',
|
||||||
|
'c',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'a_to_b' => [
|
||||||
|
'from' => ['a'],
|
||||||
|
'to' => ['b', 'c'],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
|
||||||
|
$this->addToAssertionCount(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testWorkflowValidationMultipleState()
|
||||||
|
{
|
||||||
|
$this->createContainerFromClosure(function ($container) {
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'article' => [
|
||||||
|
'type' => 'workflow',
|
||||||
|
'marking_store' => [
|
||||||
|
'type' => 'multiple_state',
|
||||||
|
],
|
||||||
|
'supports' => [
|
||||||
|
__CLASS__,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'a',
|
||||||
|
'b',
|
||||||
|
'c',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'a_to_b' => [
|
||||||
|
'from' => ['a'],
|
||||||
|
'to' => ['b', 'c'],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
|
||||||
|
$this->addToAssertionCount(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
|
||||||
|
* @expectedExceptionMessage The marking store of workflow "article" can not store many places. But the transition "a_to_b" has too many output (2). Only one is accepted.
|
||||||
|
*/
|
||||||
|
public function testWorkflowValidationSingleState()
|
||||||
|
{
|
||||||
|
$this->createContainerFromClosure(function ($container) {
|
||||||
|
$container->loadFromExtension('framework', [
|
||||||
|
'workflows' => [
|
||||||
|
'article' => [
|
||||||
|
'type' => 'workflow',
|
||||||
|
'marking_store' => [
|
||||||
|
'type' => 'single_state',
|
||||||
|
],
|
||||||
|
'supports' => [
|
||||||
|
__CLASS__,
|
||||||
|
],
|
||||||
|
'places' => [
|
||||||
|
'a',
|
||||||
|
'b',
|
||||||
|
'c',
|
||||||
|
],
|
||||||
|
'transitions' => [
|
||||||
|
'a_to_b' => [
|
||||||
|
'from' => ['a'],
|
||||||
|
'to' => ['b', 'c'],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
],
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
|
||||||
|
$this->addToAssertionCount(1);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -19,6 +19,8 @@ use Symfony\Component\Workflow\Validator\WorkflowValidator;
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
|
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
|
||||||
|
*
|
||||||
|
* @deprecated since Symfony 4.3
|
||||||
*/
|
*/
|
||||||
class ValidateWorkflowsPass implements CompilerPassInterface
|
class ValidateWorkflowsPass implements CompilerPassInterface
|
||||||
{
|
{
|
||||||
|
@ -1,32 +0,0 @@
|
|||||||
<?php
|
|
||||||
|
|
||||||
namespace Symfony\Component\Workflow\Tests\DependencyInjection;
|
|
||||||
|
|
||||||
use PHPUnit\Framework\TestCase;
|
|
||||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
|
||||||
use Symfony\Component\DependencyInjection\Definition;
|
|
||||||
use Symfony\Component\Workflow\Definition as WorkflowDefinition;
|
|
||||||
use Symfony\Component\Workflow\DependencyInjection\ValidateWorkflowsPass;
|
|
||||||
use Symfony\Component\Workflow\Transition;
|
|
||||||
|
|
||||||
class ValidateWorkflowsPassTest extends TestCase
|
|
||||||
{
|
|
||||||
public function testProcess()
|
|
||||||
{
|
|
||||||
$container = new ContainerBuilder();
|
|
||||||
$container->register('definition1', WorkflowDefinition::class)
|
|
||||||
->addArgument(['a', 'b', 'c'])
|
|
||||||
->addArgument([
|
|
||||||
new Definition(Transition::class, ['t1', 'a', 'b']),
|
|
||||||
new Definition(Transition::class, ['t2', 'a', 'c']),
|
|
||||||
])
|
|
||||||
->addTag('workflow.definition', ['name' => 'wf1', 'type' => 'state_machine', 'marking_store' => 'foo']);
|
|
||||||
|
|
||||||
(new ValidateWorkflowsPass())->process($container);
|
|
||||||
|
|
||||||
$workflowDefinition = $container->get('definition1');
|
|
||||||
|
|
||||||
$this->assertSame(['a' => 'a', 'b' => 'b', 'c' => 'c'], $workflowDefinition->getPlaces());
|
|
||||||
$this->assertEquals([new Transition('t1', 'a', 'b'), new Transition('t2', 'a', 'c')], $workflowDefinition->getTransitions());
|
|
||||||
}
|
|
||||||
}
|
|
Reference in New Issue
Block a user