[Workflow] Finished integration of initial_marking + deprecated support for workflow + single state markin store

lyrixx · lyrixx · commit 87839cfaf94e · 2019-04-06T15:38:51.000+02:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -282,8 +282,15 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                         ->fixXmlConfig('argument')
                                         ->children()
                                             ->enumNode('type')
-                                                ->setDeprecated('The "%path%.%node%" configuration key has been deprecated in Symfony 4.3. Use "method" instead as it will be the only option in Symfony 5.0.')
                                                 ->values(['multiple_state', 'single_state', 'method'])
+                                                ->validate()
+                                                    ->ifTrue(function ($v) { return 'method' !== $v; })
+                                                    ->then(function ($v) {
+                                                        @trigger_error('Passing something else than "method" has been deprecated in Symfony 4.3.', E_USER_DEPRECATED);
+
+                                                        return $v;
+                                                    })
+                                                ->end()
                                             ->end()
                                             ->arrayNode('arguments')
                                                 ->setDeprecated('The "%path%.%node%" configuration key has been deprecated in Symfony 4.3. Use "property" instead.')
@@ -296,7 +303,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                                 ->end()
                                             ->end()
                                             ->scalarNode('property')
-                                                ->defaultNull()
+                                                ->defaultValue('marking')
                                             ->end()
                                             ->scalarNode('service')
                                                 ->cannotBeEmpty()
@@ -310,10 +317,6 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                             ->ifTrue(function ($v) { return !empty($v['arguments']) && isset($v['service']); })
                                             ->thenInvalid('"arguments" and "service" cannot be used together.')
                                         ->end()
-                                        ->validate()
-                                            ->ifTrue(function ($v) { return !empty($v['property']) && isset($v['service']); })
-                                            ->thenInvalid('"property" and "service" cannot be used together.')
-                                        ->end()
                                     ->end()
                                     ->arrayNode('supports')
                                         ->beforeNormalization()
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -644,14 +644,15 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
             // Create MarkingStore
             if (isset($workflow['marking_store']['type'])) {
                 $markingStoreDefinition = new ChildDefinition('workflow.marking_store.'.$workflow['marking_store']['type']);
-                foreach ($workflow['marking_store']['arguments'] as $argument) {
-                    $markingStoreDefinition->addArgument($argument);
-                }
                 if ('method' === $workflow['marking_store']['type']) {
                     $markingStoreDefinition->setArguments([
                         'state_machine' === $type, //single state
-                        $workflow['marking_store']['property'] ?? 'marking',
+                        $workflow['marking_store']['property'],
                     ]);
+                } else {
+                    foreach ($workflow['marking_store']['arguments'] as $argument) {
+                        $markingStoreDefinition->addArgument($argument);
+                    }
                 }
             } elseif (isset($workflow['marking_store']['service'])) {
                 $markingStoreDefinition = new Reference($workflow['marking_store']['service']);
@@ -676,17 +677,14 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
                 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 {
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow-legacy.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow-legacy.php
@@ -3,7 +3,13 @@
 $container->loadFromExtension('framework', [
     'workflows' => [
         'legacy' => [
-            'type' => 'workflow',
+            'type' => 'state_machine',
+            'marking_store' => [
+                'type' => 'single_state',
+                'arguments' => [
+                    'state',
+                ],
+            ],
             'supports' => [
                 stdClass::class,
             ],
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_property_and_service.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_property_and_service.php
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow-legacy.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow-legacy.xml
@@ -7,7 +7,10 @@
                         http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
 
     <framework:config>
-        <framework:workflow name="legacy" type="workflow" initial-place="draft">
+        <framework:workflow name="legacy" type="state_machine" initial-place="draft">
+            <framework:marking-store type="single_state">
+                <framework:argument>state</framework:argument>
+            </framework:marking-store>
             <framework:support>stdClass</framework:support>
             <framework:place name="draft" />
             <framework:place name="published" />
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
@@ -9,10 +9,6 @@
     <framework:config>
         <framework:workflow name="article" type="workflow">
             <framework:initial-marking>draft</framework:initial-marking>
-            <framework:marking-store>
-                <framework:argument>a</framework:argument>
-                <framework:argument>a</framework:argument>
-            </framework:marking-store>
             <framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
             <framework:place>draft</framework:place>
             <framework:place>wait_for_journalist</framework:place>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_multiple_transitions_with_same_name.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_multiple_transitions_with_same_name.xml
@@ -9,10 +9,6 @@
     <framework:config>
         <framework:workflow name="article" type="workflow">
             <framework:initial-marking>draft</framework:initial-marking>
-            <framework:marking-store type="multiple_state">
-                <framework:argument>a</framework:argument>
-                <framework:argument>a</framework:argument>
-            </framework:marking-store>
             <framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
             <framework:place name="draft" />
             <framework:place name="wait_for_journalist" />
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_property_and_service.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_property_and_service.xml
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflows.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflows.xml
@@ -9,10 +9,7 @@
     <framework:config>
         <framework:workflow name="article" type="workflow">
             <framework:initial-marking>draft</framework:initial-marking>
-            <framework:marking-store type="multiple_state">
-                <framework:argument>a</framework:argument>
-                <framework:argument>a</framework:argument>
-            </framework:marking-store>
+            <framework:marking-store type="method" property="state" />
             <framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
             <framework:place name="draft" />
             <framework:place name="wait_for_journalist" />
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow-legacy.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow-legacy.yml
@@ -1,7 +1,11 @@
 framework:
     workflows:
         legacy:
-            type: workflow
+            type: state_machine
+            marking_store:
+                type: single_state
+                arguments:
+                    - state
             initial_place: draft
             supports:
                 - stdClass
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_property_and_service.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_property_and_service.yml
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -275,15 +275,18 @@ public function testWorkflows()
         $this->assertGreaterThan(0, \count($registryDefinition->getMethodCalls()));
     }
 
+    /**
+     * @group legacy
+     */
     public function testWorkflowLegacy()
     {
         $container = $this->createContainerFromFile('workflow-legacy');
 
-        $this->assertTrue($container->hasDefinition('workflow.legacy'), 'Workflow is registered as a service');
-        $this->assertSame('workflow.abstract', $container->getDefinition('workflow.legacy')->getParent());
-        $this->assertTrue($container->hasDefinition('workflow.legacy.definition'), 'Workflow definition is registered as a service');
+        $this->assertTrue($container->hasDefinition('state_machine.legacy'), 'Workflow is registered as a service');
+        $this->assertSame('state_machine.abstract', $container->getDefinition('state_machine.legacy')->getParent());
+        $this->assertTrue($container->hasDefinition('state_machine.legacy.definition'), 'Workflow definition is registered as a service');
 
-        $workflowDefinition = $container->getDefinition('workflow.legacy.definition');
+        $workflowDefinition = $container->getDefinition('state_machine.legacy.definition');
 
         $this->assertSame(['draft'], $workflowDefinition->getArgument(2));
 
@@ -307,17 +310,6 @@ public function testWorkflowAreValidated()
     }
 
     /**
-     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
-     * @expectedExceptionMessage "property" and "service" cannot be used together.
-     */
-    public function testWorkflowCannotHaveBothPropertyAndService()
-    {
-        $this->createContainerFromFile('workflow_with_property_and_service');
-    }
-
-    /**
-     * @legacy
-     *
      * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
      * @expectedExceptionMessage "type" and "service" cannot be used together.
      */
@@ -347,6 +339,7 @@ public function testWorkflowShouldHaveOneOfSupportsAndSupportStrategy()
     /**
      * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
      * @expectedExceptionMessage "arguments" and "service" cannot be used together.
+     * @group legacy
      */
     public function testWorkflowCannotHaveBothArgumentsAndService()
     {
@@ -423,7 +416,7 @@ public function testWorkflowMultipleTransitionsWithSameName()
         ], $container->getDefinition($transitions[4])->getArguments());
     }
 
-    public function testGuardExpressions()
+    public function testWorkflowGuardExpressions()
     {
         $container = $this->createContainerFromFile('workflow_with_guard_expression');
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php
@@ -89,78 +89,8 @@ public function testWorkflowValidationStateMachine()
     }
 
     /**
-     * @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.
+     * @group legacy
      */
-    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) {
@@ -197,6 +127,7 @@ public function testWorkflowValidationMultipleState()
     /**
      * @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.
+     * @group legacy
      */
     public function testWorkflowValidationSingleState()
     {
