8000 feature #46000 [Workflow] Mark registry as internal and deprecate the… · enumag/symfony@15ee67e · GitHub
[go: up one dir, main page]
Skip to content

Commit 15ee67e

Browse files
fabpotfabpot
committed
feature symfony#46000 [Workflow] Mark registry as internal and deprecate the service (lyrixx)
This PR was merged into the 6.2 branch. Discussion ---------- [Workflow] Mark registry as internal and deprecate the service | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Tickets | Fix symfony#45315 | License | MIT | Doc PR | --- The Registry was added to be able to get a workflow easily from twig templates without having to specify the workflow name. Unfortunately, many people thought this was the only way to get a workflow and we even add some flexibility to retrieve the workflow from the registry. I think it's a bad idea to inject the registry, it has the same default as injecting the Container: It's a black box, does not respect SOLID principle and the demeter law, and make testing harder. So I decided to mark the registry as internal. Instead people have to inject the "right" workflow where they need it. It can be legit to need all workflows, for documentation or for testing. So, all workflow services are tagged and can be injected with the following YAML syntax: ```yaml !tagged_locator { tag: workflow, index_by: name } ``` or PHP syntax: ```php tagged_locator('workflow', 'name') ``` Also, two others tags exists for each workflow types * `workflow.workflow` * `workflow.state_machine` Commits ------- f873074 [Workflow] Mark registry as internal and deprecate the service
2 parents 4a22bcb + f873074 commit 15ee67e

File tree

13 files changed

+87
-21
lines changed

13 files changed

+87
-21
lines changed

‎UPGRADE-6.2.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,13 @@ Validator
3737
---------
3838

3939
* Deprecate the `loose` e-mail validation mode, use `html5` instead
40+
41+
Workflow
42+
--------
43+
44+
* The `Registry` is marked as internal and should not be used directly. use a tagged locator instead
45+
```
46+
tagged_locator('workflow', 'name')
47+
```
48+
* The first argument of `WorkflowDumpCommand` should be a `ServiceLocator` of
49+
all workflows indexed by names

‎UPGRADE-7.0.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
UPGRADE FROM 6.4 to 7.0
2+
=======================
3+
4+
Workflow
5+
--------
6+
7+
* The first argument of `WorkflowDumpCommand` must be a `ServiceLocator` of all
8+
workflows indexed by names

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ CHANGELOG
1212
`Symfony\Component\Serializer\Normalizer\NormalizerInterface` or implement `NormalizerAwareInterface` instead
1313
* Add service usages list to the `debug:container` command output
1414
* Add service and alias deprecation message to `debug:container [<name>]` output
15+
* Tag all workflows services with `workflow`, those with type=workflow are
16+
tagged with `workflow.workflow`, and those with type=state_machine with
17+
`workflow.state_machine`
1518

1619
6.1
1720
---

‎src/Symfony/Bundle/FrameworkBundle/Command/WorkflowDumpCommand.php

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@
2020
use Symfony\Component\Console\Input\InputInterface;
2121
use Symfony\Component\Console\Input\InputOption;
2222
use Symfony\Component\Console\Output\OutputInterface;
23+
use Symfony\Component\DependencyInjection\ServiceLocator;
2324
use Symfony\Component\Workflow\Definition;
2425
use Symfony\Component\Workflow\Dumper\GraphvizDumper;
2526
use Symfony\Component\Workflow\Dumper\MermaidDumper;
2627
use Symfony\Component\Workflow\Dumper\PlantUmlDumper;
2728
use Symfony\Component\Workflow\Dumper\StateMachineGraphvizDumper;
2829
use Symfony\Component\Workflow\Marking;
30+
use Symfony\Component\Workflow\StateMachine;
2931

3032
/**
3133
* @author Grégoire Pineau <lyrixx@lyrixx.info>
@@ -40,19 +42,28 @@ class WorkflowDumpCommand extends Command
4042
*
4143
* @var array<string, Definition>
4244
*/
43-
private array $workflows = [];
45+
private array $definitions = [];
46+
47+
private ServiceLocator $workflows;
4448

4549
private const DUMP_FORMAT_OPTIONS = [
4650
'puml',
4751
'mermaid',
4852
'dot',
4953
];
5054

51-
public function __construct(array $workflows)
55+
public function __construct($workflows)
5256
{
5357
parent::__construct();
5458

55-
$this->workflows = $workflows;
59+
if ($workflows instanceof ServiceLocator) {
60+
$this->workflows = $workflows;
61+
} elseif (\is_array($workflows)) {
62+
$this->definitions = $workflows;
63+
trigger_deprecation('symfony/framework-bundle', '6.2', 'Passing an array of definitions in "%s()" is deprecated. Inject a ServiceLocator filled with all workflows instead.', __METHOD__);
64+
} else {
65+
throw new \TypeError(sprintf('Argument 1 passed to "%s()" must be an array or a ServiceLocator, "%s" given.', __METHOD__, \gettype($workflows)));
66+
}
5667
}
5768

5869
/**
@@ -88,15 +99,22 @@ protected function execute(InputInterface $input, OutputInterface $output): int
8899

89100
$workflow = null;
90101

91-
if (isset($this->workflows['workflow.'.$workflowName])) {
92-
$workflow = $this->workflows['workflow.'.$workflowName];
102+
if (isset($this->workflows)) {
103+
if (!$this->workflows->has($workflowName)) {
104+
throw new InvalidArgumentException(sprintf('The workflow named "%s" cannot be found.', $workflowName));
105+
}
106+
$workflow = $this->workflows->get($workflowName);
107+
$type = $workflow instanceof StateMachine ? 'state_machine' : 'workflow';
108+
$definition = $workflow->getDefinition();
109+
} elseif (isset($this->definitions['workflow.'.$workflowName])) {
110+
$definition = $this->definitions['workflow.'.$workflowName];
93111
$type = 'workflow';
94-
} elseif (isset($this->workflows['state_machine.'.$workflowName])) {
95-
$workflow = $this->workflows['state_machine.'.$workflowName];
112+
} elseif (isset($this->definitions['state_machine.'.$workflowName])) {
113+
$definition = $this->definitions['state_machine.'.$workflowName];
96114
$type = 'state_machine';
97115
}
98116

99-
if (null === $workflow) {
117+
if (null === $definition) {
100118
throw new InvalidArgumentException(sprintf('No service found for "workflow.%1$s" nor "state_machine.%1$s".', $workflowName));
101119
}
102120

@@ -129,15 +147,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
129147
'label' => $input->getOption('label'),
130148
],
131149
];
132-
$output->writeln($dumper->dump($workflow, $marking, $options));
150+
$output->writeln($dumper->dump($definition, $marking, $options));
133151

134152
return 0;
135153
}
136154

137155
public function complete(CompletionInput $input, CompletionSuggestions $suggestions): void
138156
{
139157
if ($input->mustSuggestArgumentValuesFor('name')) {
140-
$suggestions->suggestValues(array_keys($this->workflows));
158+
if (isset($this->workflows)) {
159+
$suggestions->suggestValues(array_keys($this->workflows->getProvidedServices()));
160+
} else {
161+
$suggestions->suggestValues(array_keys($this->definitions));
162+
}
141163
}
142164

143165
if ($input->mustSuggestOptionValuesFor('dump-format')) {

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ class UnusedTagsPass implements CompilerPassInterface
9797
'validator.auto_mapper',
9898
'validator.constraint_validator',
9999
'validator.initializer',
100+
'workflow',
100101
];
101102

102103
public function process(ContainerBuilder $container)

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -883,9 +883,9 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
883883

884884
$loader->load('workflow.php');
885885

886-
$registryDefinition = $container->getDefinition('workflow.registry');
886+
$registryDefinition = $container->getDefinition('.workflow.registry');
887887

888-
$workflows = [];
888+
$workflow = [];
889889

890890
foreach ($config['workflows'] as $name => $workflow) {
891891
$type = $workflow['type'];
@@ -994,6 +994,13 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
994994
$workflowDefinition->replaceArgument(3, $name);
995995
$workflowDefinition->replaceArgument(4, $workflow['events_to_dispatch']);
996996

997+
$workflowDefinition->addTag('workflow', ['name' => $name]);
998+
if ('workflow' === $type) {
999+
$workflowDefinition->addTag('workflow.workflow', ['name' => $name]);
1000+
} elseif ('state_machine' === $type) {
1001+
$workflowDefinition->addTag('workflow.state_machine', ['name' => $name]);
1002+
}
1003+
9971004
// Store to container
9981005
$container->setDefinition($workflowId, $workflowDefinition);
9991006
$container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
@@ -1063,9 +1070,6 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
10631070
$container->setParameter('workflow.has_guard_listeners', true);
10641071
}
10651072
}
1066-
1067-
$commandDumpDefinition = $container->getDefinition('console.command.workflow_dump');
1068-
$commandDumpDefinition->setArgument(0, $workflows);
10691073
}
10701074

10711075
private function registerDebugConfiguration(array $config, ContainerBuilder $container, PhpFileLoader $loader)

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/console.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@
282282
->tag('console.command', ['command' => 'translation:push'])
283283

284284
->set('console.command.workflow_dump', WorkflowDumpCommand::class)
285+
->args([
286+
tagged_locator('workflow', 'name'),
287+
])
285288
->tag('console.command')
286289

287290
->set('console.command.xliff_lint', XliffLintCommand::class)

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@
3939
->abstract()
4040
->set('workflow.marking_store.method', MethodMarkingStore::class)
4141
->abstract()
42-
->set('workflow.registry', Registry::class)
43-
->alias(Registry::class, 'workflow.registry')
42+
->set('.workflow.registry', Registry::class)
43+
->alias(Registry::class, '.workflow.registry')
44+
->deprecate('symfony/workflow', '6.2', 'The "%alias_id%" alias is deprecated since Symfony 6.2 and will be removed in Symfony 7.0. Inject the workflow directly.')
45+
->alias('workflow.registry', '.workflow.registry')
46+
->deprecate('symfony/workflow', '6.2', 'The "%alias_id%" service is deprecated since Symfony 6.2 and will be removed in Symfony 7.0. Inject the workflow directly.')
4447
->set('workflow.security.expression_language', ExpressionLanguage::class)
4548
;
4649
};

‎src/Symfony/Bundle/FrameworkBundle/Tests/Command/WorkflowDumpCommandTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Bundle\FrameworkBundle\Command\WorkflowDumpCommand;
1616
use Symfony\Component\Console\Application;
1717
use Symfony\Component\Console\Tester\CommandCompletionTester;
18+
use Symfony\Component\DependencyInjection\ServiceLocator;
1819

1920
class WorkflowDumpCommandTest extends TestCase
2021
{
@@ -24,7 +25,7 @@ class WorkflowDumpCommandTest extends TestCase
2425
public function testComplete(array $input, array $expectedSuggestions)
2526
{
2627
$application = new Application();
27-
$application->add(new WorkflowDumpCommand([]));
28+
$application->add(new WorkflowDumpCommand(new ServiceLocator([])));
2829

2930
$tester = new CommandCompletionTester($application->find('workflow:dump'));
3031
$suggestions = $tester->complete($input, 2);

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ public function testWorkflows()
295295
$this->assertArrayHasKey('index_4', $args);
296296
$this->assertNull($args['index_4'], 'Workflows has eventsToDispatch=null');
297297

298+
$this->assertSame(['workflow' => [['name' => 'article']], 'workflow.workflow' => [['name' => 'article']]], $container->getDefinition('workflow.article')->getTags());
299+
298300
$this->assertTrue($container->hasDefinition('workflow.article.definition'), 'Workflow definition is registered as a service');
299301

300302
$workflowDefinition = $container->getDefinition('workflow.article.definition');
@@ -324,6 +326,8 @@ public function testWorkflows()
324326
$this->assertSame('state_machine.abstract', $container->getDefinition('state_machine.pull_request')->getParent());
325327
$this->assertTrue($container->hasDefinition('state_machine.pull_request.definition'), 'State machine definition is registered as a service');
326328

329+
$this->assertSame(['workflow' => [['name' => 'pull_request']], 'workflow.state_machine' => [['name' => 'pull_request']]], $container->getDefinition('state_machine.pull_request')->getTags());
330+
327331
$stateMachineDefinition = $container->getDefinition('state_machine.pull_request.definition');
328332

329333
$this->assertSame(
@@ -371,8 +375,8 @@ public function testWorkflows()
371375
$this->assertInstanceOf(Reference::class, $markingStoreRef);
372376
$this->assertEquals< 10000 /span>('workflow_service', (string) $markingStoreRef);
373377

374-
$this->assertTrue($container->hasDefinition('workflow.registry'), 'Workflow registry is registered as a service');
375-
$registryDefinition = $container->getDefinition('workflow.registry');
378+
$this->assertTrue($container->hasDefinition('.workflow.registry'), 'Workflow registry is registered as a service');
379+
$registryDefinition = $container->getDefinition('.workflow.registry');
376380
$this->assertGreaterThan(0, \count($registryDefinition->getMethodCalls()));
377381
}
378382

‎src/Symfony/Bundle/TwigBundle/Resources/config/twig.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@
141141
->tag('translation.extractor', ['alias' => 'twig'])
142142

143143
->set('workflow.twig_extension', WorkflowExtension::class)
144-
->args([service('workflow.registry')])
144+
->args([service('.workflow.registry')])
145145

146146
->set('twig.configurator.environment', EnvironmentConfigurator::class)
147147
->args([

‎src/Symfony/Component/Workflow/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
6.2
5+
---
6+
7+
* Mark `Symfony\Component\Workflow\Registry` as internal
8+
49
6.0
510
---
611

‎src/Symfony/Component/Workflow/Registry.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
/**
1818
* @author Fabien Potencier <fabien@symfony.com>
1919
* @author Grégoire Pineau <lyrixx@lyrixx.info>
20+
*
21+
* @internal since Symfony 6.2. Inject the workflow where you need it.
2022
*/
2123
class Registry
2224
{

0 commit comments

Comments
 (0)
0