Skip to content

Commit 2425972

Browse files
lyrixxlyrixx
committed
[Workflow] Move code from ValidateWorkflowsPass to the FrameworkExtension
1 parent 65a81fc commit 2425972

File tree

8 files changed

+279
-14
lines changed

8 files changed

+279
-14
lines changed

‎DependencyInjection/FrameworkExtension.php‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -603,20 +603,15 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
603603

604604
// Create places
605605
$places = array_column($workflow['places'], 'name');
606+
$initialPlace = $workflow['initial_place'] ?? null;
606607

607608
// Create a Definition
608609
$definitionDefinition = new Definition(Workflow\Definition::class);
609610
$definitionDefinition->setPublic(false);
610611
$definitionDefinition->addArgument($places);
611612
$definitionDefinition->addArgument($transitions);
612-
$definitionDefinition->addArgument($workflow['initial_place'] ?? null);
613+
$definitionDefinition->addArgument($initialPlace);
613614
$definitionDefinition->addArgument($metadataStoreDefinition);
614-
$definitionDefinition->addTag('workflow.definition', [
615-
'name' => $name,
616-
'type' => $type,
617-
'marking_store' => $workflow['marking_store']['type'] ?? null,
618-
'single_state' => 'method' === ($workflow['marking_store']['type'] ?? null) && ($workflow['marking_store']['arguments'][0] ?? false),
619-
]);
620615

621616
// Create MarkingStore
622617
if (isset($workflow['marking_store']['type'])) {
@@ -641,6 +636,34 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
641636
$container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
642637
$container->registerAliasForArgument($workflowId, WorkflowInterface::class, $name.'.'.$type);
643638

639+
// Validate Workflow
640+
$validator = null;
641+
switch (true) {
642+
case 'state_machine' === $workflow['type']:
643+
$validator = new Workflow\Validator\StateMachineValidator();
644+
break;
645+
case 'method' === ($workflow['marking_store']['type'] ?? null):
646+
$singlePlace = $workflow['marking_store']['arguments'][0] ?? false;
647+
$validator = new Workflow\Validator\WorkflowValidator($singlePlace);
648+
break;
649+
case 'single_state' === ($workflow['marking_store']['type'] ?? null):
650+
$validator = new Workflow\Validator\WorkflowValidator(true);
651+
break;
652+
case 'multiple_state' === ($workflow['marking_store']['type'] ?? false):
653+
$validator = new Workflow\Validator\WorkflowValidator(false);
654+
break;
655+
}
656+
if ($validator) {
657+
$realDefinition = (new Workflow\DefinitionBuilder($places))
658+
->addTransitions(array_map(function (Reference $ref) use ($container): Workflow\Transition {
659+
return $container->get((string) $ref);
660+
}, $transitions))
661+
->setInitialPlace($initialPlace)
662+
->build()
663+
;
664+
$validator->validate($realDefinition, $name);
665+
}
666+
644667
// Add workflow to Registry
645668
if ($workflow['supports']) {
646669
foreach ($workflow['supports'] as $supportedClassName) {

‎FrameworkBundle.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
use Symfony\Component\Translation\DependencyInjection\TranslatorPathsPass;
5555
use Symfony\Component\Validator\DependencyInjection\AddConstraintValidatorsPass;
5656
use Symfony\Component\Validator\DependencyInjection\AddValidatorInitializersPass;
57-
use Symfony\Component\Workflow\DependencyInjection\ValidateWorkflowsPass;
5857

5958
/**
6059
* Bundle.
@@ -115,7 +114,6 @@ public function build(ContainerBuilder $container)
115114
$container->addCompilerPass(new DataCollectorTranslatorPass());
116115
$container->addCompilerPass(new ControllerArgumentValueResolverPass());
117116
$container->addCompilerPass(new CachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 32);
118-
$this->addCompilerPassIfExists($container, ValidateWorkflowsPass::class);
119117
$container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING);
120118
$container->addCompilerPass(new CachePoolPrunerPass(), PassConfig::TYPE_AFTER_REMOVING);
121119
$this->addCompilerPassIfExists($container, FormPass::class);

‎Tests/DependencyInjection/Fixtures/php/workflow_not_valid.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;
4+
5+
$container->loadFromExtension('framework', [
6+
'workflows' => [
7+
'my_workflow' => [
8+
'type' => 'state_machine',
9+
'supports' => [
10+
FrameworkExtensionTest::class,
11+
],
12+
'places' => [
13+
'first',
14+
'middle',
15+
'last',
16+
],
17+
'transitions' => [
18+
'go' => [
19+
'from' => [
20+
'first',
21+
],
22+
'to' => [
23+
'middle',
24+
'last',
25+
],
26+
],
27+
],
28+
],
29+
],
30+
]);

‎Tests/DependencyInjection/Fixtures/xml/workflow_not_valid.xml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?xml version="1.0" ?>
2+
3+
<container xmlns="http://symfony.com/schema/dic/services"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xmlns:framework="http://symfony.com/schema/dic/symfony"
6+
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
7+
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
8+
9+
<framework:config>
10+
<framework:workflow name="my_workflow" type="state_machine">
11+
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
12+
<framework:place name="first" />
13+
<framework:place name="middle" />
14+
<framework:place name="last" />
15+
<framework:transition name="go">
16+
<framework:from>first</framework:from>
17+
<framework:to>middle</framework:to>
18+
<framework:to>last</framework:to>
19+
</framework:transition>
20+
</framework:workflow>
21+
</framework:config>
22+
</container>

‎Tests/DependencyInjection/Fixtures/xml/workflows.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@
2929
<framework:to>approved_by_journalist</framework:to>
3030
</framework:transition>
3131
<framework:transition name="spellchecker_approval">
32-
<framework:from>wait_for_spellcheker</framework:from>
33-
<framework:to>approved_by_spellchker</framework:to>
32+
<framework:from>wait_for_spellchecker</framework:from>
33+
<framework:to>approved_by_spellchecker</framework:to>
3434
</framework:transition>
3535
<framework:transition name="publish">
3636
<framework:from>approved_by_journalist</framework:from>
37-
<framework:from>approved_by_spellchker</framework:from>
37+
<framework:from>approved_by_spellchecker</framework:from>
3838
<framework:to>published</framework:to>
3939
</framework:transition>
4040
</framework:workflow>

‎Tests/DependencyInjection/Fixtures/yml/workflow_not_valid.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
framework:
2+
workflows:
3+
my_workflow:
4+
type: state_machine
5+
supports:
6+
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
7+
places: [first, middle, last]
8+
transitions:
9+
go:
10+
from: first
11+
to: [last, middle ]

‎Tests/DependencyInjection/FrameworkExtensionTest.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ public function testWorkflows()
215215
$workflowDefinition->getArgument(0),
216216
'Places are passed to the workflow definition'
217217
);
218-
$this->assertSame(['workflow.definition' => [['name' => 'article', 'type' => 'workflow', 'marking_store' => 'multiple_state', 'single_state' => false]]], $workflowDefinition->getTags());
219218
$this->assertCount(4, $workflowDefinition->getArgument(1));
220219
$this->assertSame('draft', $workflowDefinition->getArgument(2));
221220

@@ -237,7 +236,6 @@ public function testWorkflows()
237236
$stateMachineDefinition->getArgument(0),
238237
'Places are passed to the state machine definition'
239238
);
240-
$this->assertSame(['workflow.definition' => [['name' => 'pull_request', 'type' => 'state_machine', 'marking_store' => 'single_state', 'single_state' => false]]], $stateMachineDefinition->getTags());
241239
$this->assertCount(9, $stateMachineDefinition->getArgument(1));
242240
$this->assertSame('start', $stateMachineDefinition->getArgument(2));
243241

@@ -272,6 +270,15 @@ public function testWorkflows()
272270
$this->assertGreaterThan(0, \count($registryDefinition->getMethodCalls()));
273271
}
274272

273+
/**
274+
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
275+
* @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".
276+
*/
277+
public function testWorkflowAreValidated()
278+
{
279+
$this->createContainerFromFile('workflow_not_valid');
280+
}
281+
275282
/**
276283
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
277284
* @expectedExceptionMessage "type" and "service" cannot be used together.

‎Tests/DependencyInjection/PhpFrameworkExtensionTest.php‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,178 @@ public function testAssetPackageCannotHavePathAndUrl()
5656
]);
5757
});
5858
}
59+
60+
/**
61+
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
62+
* @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".
63+
*/
64+
public function testWorkflowValidationStateMachine()
65+
{
66+
$this->createContainerFromClosure(function ($container) {
67+
$container->loadFromExtension('framework', [
68+
'workflows' => [
69+
'article' => [
70+
'type' => 'state_machine',
71+
'supports' => [
72+
__CLASS__,
73+
],
74+
'places' => [
75+
'a',
76+
'b',
77+
'c',
78+
],
79+
'transitions' => [
80+
'a_to_b' => [
81+
'from' => ['a'],
82+
'to' => ['b', 'c'],
83+
],
84+
],
85+
],
86+
],
87+
]);
88+
});
89+
}
90+
91+
/**
92+
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
93+
* @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.
94+
*/
95+
public function testWorkflowValidationMethodSingle()
96+
{
97+
$this->createContainerFromClosure(function ($container) {
98+
$container->loadFromExtension('framework', [
99+
'workflows' => [
100+
'article' => [
101+
'type' => 'workflow',
102+
'marking_store' => [
103+
'type' => 'method',
104+
'arguments' => [
105+
true,
106+
],
107+
],
108+
'supports' => [
109+
__CLASS__,
110+
],
111+
'places' => [
112+
'a',
113+
'b',
114+
'c',
115+
],
116+
'transitions' => [
117+
'a_to_b' => [
118+
'from' => ['a'],
119+
'to' => ['b', 'c'],
120+
],
121+
],
122+
],
123+
],
124+
]);
125+
});
126+
}
127+
128+
public function testWorkflowValidationMethodNotSingle()
129+
{
130+
$this->createContainerFromClosure(function ($container) {
131+
$container->loadFromExtension('framework', [
132+
'workflows' => [
133+
'article' => [
134+
'type' => 'workflow',
135+
'marking_store' => [
136+
'type' => 'method',
137+
'arguments' => [
138+
false,
139+
],
140+
],
141+
'supports' => [
142+
__CLASS__,
143+
],
144+
'places' => [
145+
'a',
146+
'b',
147+
'c',
148+
],
149+
'transitions' => [
150+
'a_to_b' => [
151+
'from' => ['a'],
152+
'to' => ['b', 'c'],
153+
],
154+
],
155+
],
156+
],
157+
]);
158+
});
159+
160+
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
161+
$this->addToAssertionCount(1);
162+
}
163+
164+
public function testWorkflowValidationMultipleState()
165+
{
166+
$this->createContainerFromClosure(function ($container) {
167+
$container->loadFromExtension('framework', [
168+
'workflows' => [
169+
'article' => [
170+
'type' => 'workflow',
171+
'marking_store' => [
172+
'type' => 'multiple_state',
173+
],
174+
'supports' => [
175+
__CLASS__,
176+
],
177+
'places' => [
178+
'a',
179+
'b',
180+
'c',
181+
],
182+
'transitions' => [
183+
'a_to_b' => [
184+
'from' => ['a'],
185+
'to' => ['b', 'c'],
186+
],
187+
],
188+
],
189+
],
190+
]);
191+
});
192+
193+
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
194+
$this->addToAssertionCount(1);
195+
}
196+
197+
/**
198+
* @expectedException \Symfony\Component\Workflow\Exception\InvalidDefinitionException
199+
* @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.
200+
*/
201+
public function testWorkflowValidationSingleState()
202+
{
203+
$this->createContainerFromClosure(function ($container) {
204+
$container->loadFromExtension('framework', [
205+
'workflows' => [
206+
'article' => [
207+
'type' => 'workflow',
208+
'marking_store' => [
209+
'type' => 'single_state',
210+
],
211+
'supports' => [
212+
__CLASS__,
213+
],
214+
'places' => [
215+
'a',
216+
'b',
217+
'c',
218+
],
219+
'transitions' => [
220+
'a_to_b' => [
221+
'from' => ['a'],
222+
'to' => ['b', 'c'],
223+
],
224+
],
225+
],
226+
],
227+
]);
228+
});
229+
230+
// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
231+
$this->addToAssertionCount(1);
232+
}
59233
}

0 commit comments

Comments
 (0)