Skip to content

Commit e16f6dd

Browse files
nicolas-grekasnicolas-grekas
committed
[FrameworkBundle] Perform-no-deep-merging on workflow transitions' from/to configs
1 parent ef9988b commit e16f6dd

File tree

10 files changed

+207
-28
lines changed

10 files changed

+207
-28
lines changed

‎DependencyInjection/Configuration.php‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode): void
358358
->arrayNode('workflows')
359359
->canBeEnabled()
360360
->beforeNormalization()
361-
->always(function ($v) {
361+
->always(static function ($v) {
362362
if (\is_array($v) && true === $v['enabled']) {
363363
$workflows = $v;
364364
unset($workflows['enabled']);
@@ -367,7 +367,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode): void
367367
$workflows = [];
368368
}
369369

370-
if (1 === \count($workflows) && isset($workflows['workflows']) && !array_is_list($workflows['workflows']) && array_diff(array_keys($workflows['workflows']), ['audit_trail', 'type', 'marking_store', 'supports', 'support_strategy', 'initial_marking', 'places', 'transitions'])) {
370+
if (1 === \count($workflows) && isset($workflows['workflows']) && !array_is_list($workflows['workflows']) && array_diff_key($workflows['workflows'], ['audit_trail' => 1, 'type' => 1, 'marking_store' => 1, 'supports' => 1, 'support_strategy' => 1, 'initial_marking' => 1, 'places' => 1, 'transitions' => 1])) {
371371
$workflows = $workflows['workflows'];
372372
}
373373

@@ -505,26 +505,26 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode): void
505505
->end()
506506
->arrayNode('transitions')
507507
->beforeNormalization()
508-
->always()
509-
->then(function ($transitions) {
508+
->always(static function ($transitions) {
510509
if (!\is_array($transitions)) {
511510
throw new InvalidConfigurationException('The "transitions" option must be an array in workflow configuration.');
512511
}
513512

514-
// It's an indexed array, we let the validation occur
515-
if (isset($transitions[0]) && \is_array($transitions[0])) {
516-
return $transitions;
517-
}
518-
519-
foreach ($transitions as $name => $transition) {
520-
if (\is_array($transition) && \array_key_exists('name', $transition)) {
521-
continue;
513+
$normalizedTransitions = [];
514+
foreach ($transitions as $key => $transition) {
515+
if (\is_array($transition)) {
516+
if (\is_string($key = $transition['key'] ?? $key)) {
517+
$transition['name'] ??= $key;
518+
}
519+
if (!($transition['name'] ?? false)) {
520+
throw new InvalidConfigurationException('The "name" option is required for each transition in workflow configuration.');
521+
}
522+
unset($transition['key']);
522523
}
523-
$transition['name'] = $name;
524-
$transitions[$name] = $transition;
524+
$normalizedTransitions[$key] = $transition;
525525
}
526526

527-
return $transitions;
527+
return $normalizedTransitions;
528528
})
529529
->end()
530530
->isRequired()
@@ -541,6 +541,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode): void
541541
->example('is_fully_authenticated() and is_granted(\'ROLE_JOURNALIST\') and subject.getTitle() == \'My first article\'')
542542
->end()
543543
->arrayNode('from')
544+
->performNoDeepMerging()
544545
->beforeNormalization()
545546
->ifString()
546547
->then(fn ($v) => [$v])
@@ -551,6 +552,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode): void
551552
->end()
552553
->end()
553554
->arrayNode('to')
555+
->performNoDeepMerging()
554556
->beforeNormalization()
555557
->ifString()
556558
->then(fn ($v) => [$v])

‎DependencyInjection/FrameworkExtension.php‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -972,16 +972,16 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
972972
$transitionCounter = 0;
973973
foreach ($workflow['transitions'] as $transition) {
974974
if ('workflow' === $type) {
975-
$transitionDefinition = new Definition(Workflow\Transition::class, [$transition['name'], $transition['from'], $transition['to']]);
976975
$transitionId = \sprintf('.%s.transition.%s', $workflowId, $transitionCounter++);
977-
$container->setDefinition($transitionId, $transitionDefinition);
976+
$container->register($transitionId, Workflow\Transition::class)
977+
->setArguments([$transition['name'], $transition['from'], $transition['to']]);
978978
$transitions[] = new Reference($transitionId);
979979
if (isset($transition['guard'])) {
980-
$configuration = new Definition(Workflow\EventListener\GuardExpression::class);
981-
$configuration->addArgument(new Reference($transitionId));
982-
$configuration->addArgument($transition['guard']);
983980
$eventName = \sprintf('workflow.%s.guard.%s', $name, $transition['name']);
984-
$guardsConfiguration[$eventName][] = $configuration;
981+
$guardsConfiguration[$eventName][] = new Definition(
982+
Workflow\EventListener\GuardExpression::class,
983+
[new Reference($transitionId), $transition['guard']]
984+
);
985985
}
986986
if ($transition['metadata']) {
987987
$transitionsMetadataDefinition->addMethodCall('offsetSet', [
@@ -992,16 +992,16 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
992992
} elseif ('state_machine' === $type) {
993993
foreach ($transition['from'] as $from) {
994994
foreach ($transition['to'] as $to) {
995-
$transitionDefinition = new Definition(Workflow\Transition::class, [$transition['name'], $from, $to]);
996995
$transitionId = \sprintf('.%s.transition.%s', $workflowId, $transitionCounter++);
997-
$container->setDefinition($transitionId, $transitionDefinition);
996+
$container->register($transitionId, Workflow\Transition::class)
997+
->setArguments([$transition['name'], $from, $to]);
998998
$transitions[] = new Reference($transitionId);
999999
if (isset($transition['guard'])) {
1000-
$configuration = new Definition(Workflow\EventListener\GuardExpression::class);
1001-
$configuration->addArgument(new Reference($transitionId));
1002-
$configuration->addArgument($transition['guard']);
10031000
$eventName = \sprintf('workflow.%s.guard.%s', $name, $transition['name']);
1004-
$guardsConfiguration[$eventName][] = $configuration;
1001+
$guardsConfiguration[$eventName][] = new Definition(
1002+
Workflow\EventListener\GuardExpression::class,
1003+
[new Reference($transitionId), $transition['guard']]
1004+
);
10051005
}
10061006
if ($transition['metadata']) {
10071007
$transitionsMetadataDefinition->addMethodCall('offsetSet', [

‎Resources/config/schema/symfony-1.0.xsd‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,8 @@
471471
<xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
472472
<xsd:element name="guard" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
473473
</xsd:sequence>
474-
<xsd:attribute name="name" type="xsd:string" use="required" />
474+
<xsd:attribute name="name" type="xsd:string" />
475+
<xsd:attribute name="key" type="xsd:string" />
475476
</xsd:complexType>
476477

477478
<xsd:complexType name="place" mixed="true">

‎Tests/DependencyInjection/Fixtures/php/workflow_base_config.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
return function (Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator $container) {
4+
$container->services()->alias('test_workflow', 'workflow.test_workflow')->public();
5+
$container->extension('framework', [
6+
'http_method_override' => false,
7+
'handle_all_throwables' => true,
8+
'annotations' => [
9+
'enabled' => false,
10+
],
11+
'php_errors' => [
12+
'log' => true,
13+
],
14+
'workflows' => [
15+
'test_workflow' => [
16+
'type' => 'workflow',
17+
'supports' => [
18+
'Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTestCase',
19+
],
20+
'initial_marking' => ['start'],
21+
'places' => [
22+
'start',
23+
'middle',
24+
'end',
25+
'alternative',
26+
],
27+
'transitions' => [
28+
'base_transition' => [
29+
'from' => ['start'],
30+
'to' => ['end'],
31+
],
32+
],
33+
],
34+
],
35+
]);
36+
};

‎Tests/DependencyInjection/Fixtures/php/workflow_override_config.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
return function (Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator $container) {
4+
$container->extension('framework', [
5+
'http_method_override' => false,
6+
'handle_all_throwables' => true,
7+
'annotations' => [
8+
'enabled' => false,
9+
],
10+
'php_errors' => [
11+
'log' => true,
12+
],
13+
'workflows' => [
14+
'test_workflow' => [
15+
'type' => 'workflow',
16+
'supports' => [
17+
'Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTestCase',
18+
],
19+
'initial_marking' => ['start'],
20+
'places' => [
21+
'start',
22+
'middle',
23+
'end',
24+
'alternative',
25+
],
26+
'transitions' => [
27+
'base_transition' => [
28+
'from' => ['middle'],
29+
'to' => ['alternative'],
30+
],
31+
],
32+
],
33+
],
34+
]);
35+
};

‎Tests/DependencyInjection/Fixtures/xml/workflow_base_config.xml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<container xmlns="http://symfony.com/schema/dic/services"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xmlns:framework="http://symfony.com/schema/dic/symfony"
5+
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
6+
http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
7+
8+
<services>
9+
<service id="test_workflow" alias="workflow.test_workflow" public="true" />
10+
</services>
11+
<framework:config http-method-override="false" handle-all-throwables="true">
12+
<framework:annotations enabled="false" />
13+
<framework:php-errors log="true" />
14+
<framework:workflow name="test_workflow" type="workflow">
15+
<framework:initial-marking>start</framework:initial-marking>
16+
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTestCase</framework:support>
17+
<framework:place>start</framework:place>
18+
<framework:place>middle</framework:place>
19+
<framework:place>end</framework:place>
20+
<framework:place>alternative</framework:place>
21+
<framework:transition key="base_transition">
22+
<framework:from>start</framework:from>
23+
<framework:to>end</framework:to>
24+
</framework:transition>
25+
</framework:workflow>
26+
</framework:config>
27+
</container>

‎Tests/DependencyInjection/Fixtures/xml/workflow_override_config.xml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<container xmlns="http://symfony.com/schema/dic/services"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xmlns:framework="http://symfony.com/schema/dic/symfony"
5+
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
6+
http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
7+
8+
<framework:config http-method-override="false" handle-all-throwables="true">
9+
<framework:annotations enabled="false" />
10+
<framework:php-errors log="true" />
11+
<framework:workflow name="test_workflow" type="workflow">
12+
<framework:transition key="base_transition">
13+
<framework:from>middle</framework:from>
14+
<framework:to>alternative</framework:to>
15+
</framework:transition>
16+
</framework:workflow>
17+
</framework:config>
18+
</container>

‎Tests/DependencyInjection/Fixtures/yml/workflow_base_config.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
services:
2+
test_workflow:
3+
alias: workflow.test_workflow
4+
public: true
5+
framework:
6+
http_method_override: false
7+
handle_all_throwables: true
8+
annotations:
9+
enabled: false
10+
php_errors:
11+
log: true
12+
workflows:
13+
test_workflow:
14+
type: workflow
15+
supports:
16+
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTestCase
17+
initial_marking: [start]
18+
places:
19+
- start
20+
- middle
21+
- end
22+
- alternative
23+
transitions:
24+
base_transition:
25+
from: [start]
26+
to: [end]

‎Tests/DependencyInjection/Fixtures/yml/workflow_override_config.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
framework:
2+
http_method_override: false
3+
handle_all_throwables: true
4+
annotations:
5+
enabled: false
6+
php_errors:
7+
log: true
8+
workflows:
9+
test_workflow:
10+
transitions:
11+
base_transition:
12+
from: [middle]
13+
to: [alternative]

‎Tests/DependencyInjection/FrameworkExtensionTestCase.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,27 @@ public function testWorkflowsWithSpecifiedDispatchedEvents()
561561
$this->assertSame([WorkflowEvents::LEAVE, WorkflowEvents::COMPLETED], $eventsToDispatch);
562562
}
563563

564+
public function testWorkflowTransitionsPerformNoDeepMerging()
565+
{
566+
$container = $this->createContainer(['kernel.charset' => 'UTF-8', 'kernel.secret' => 'secret', 'kernel.runtime_environment' => 'test']);
567+
$container->registerExtension(new FrameworkExtension());
568+
569+
$this->loadFromFile($container, 'workflow_base_config');
570+
571+
$this->loadFromFile($container, 'workflow_override_config');
572+
573+
$container->compile();
574+
575+
$transitions = [];
576+
577+
foreach ($container->getDefinition('test_workflow')->getArgument(0)->getArgument(1) as $transitionDefinition) {
578+
$transitions[] = $transitionDefinition->getArguments();
579+
}
580+
581+
$this->assertCount(1, $transitions);
582+
$this->assertSame(['base_transition', ['middle'], ['alternative']], $transitions[0]);
583+
}
584+
564585
public function testEnabledPhpErrorsConfig()
565586
{
566587
$container = $this->createContainerFromFile('php_errors_enabled');

0 commit comments

Comments
 (0)