feature symfony#21334 [Workflow] Introduce concept of SupportStrategyInterface (andesk, lyrixx)

fabpot · fabpot · commit 4491eb64633f · 2017-01-18T11:12:00.000-08:00
This PR was merged into the 3.3-dev branch. Discussion ---------- [Workflow] Introduce concept of SupportStrategyInterface | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony#20751 | License | MIT | Doc PR | - Commits ------- 134a58b [Workflow] Fixed code and tests 1843012 [Workflow] Introduce concept of SupprtStrategyInterface to allow other support checks than class instance
diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md
@@ -59,3 +59,9 @@ TwigBridge
 
  * The `TwigRendererEngine::setEnvironment()` method has been deprecated and will be removed
    in 4.0. Pass the Twig Environment as second argument of the constructor instead.
+
+Workflow
+--------
+
+ * Deprecated class name support in `WorkflowRegistry::add()` as second parameter.
+   Wrap the class name in an instance of ClassInstanceSupportStrategy instead.
diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -154,7 +154,7 @@ FrameworkBundle
  * The `framework.serializer.cache` option and the services
    `serializer.mapping.cache.apc` and `serializer.mapping.cache.doctrine.apc`
    have been removed. APCu should now be automatically used when available.
-   
+
  * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConsoleCommandPass` has been removed. Use `Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass` instead.
 
 SecurityBundle
@@ -216,7 +216,7 @@ Serializer
  * The ability to pass a Doctrine `Cache` instance to the `ClassMetadataFactory`
    class has been removed. You should use the `CacheClassMetadataFactory` class
    instead.
-   
+
  * Not defining the 6th argument `$format = null` of the
    `AbstractNormalizer::instantiateObject()` method when overriding it is not
    supported anymore.
@@ -294,9 +294,9 @@ Validator
        // ...
    }
    ```
-   
+
  * The default value of the strict option of the `Choice` Constraint has been
-   changed to `true` as of 4.0. If you need the previous behaviour ensure to 
+   changed to `true` as of 4.0. If you need the previous behaviour ensure to
    set the option to `false`.
 
 Yaml
@@ -393,5 +393,10 @@ Yaml
 
 Ldap
 ----
- 
-  * The `RenameEntryInterface` has been deprecated, and merged with `EntryManagerInterface`
+
+ * The `RenameEntryInterface` has been deprecated, and merged with `EntryManagerInterface`
+
+Workflow
+--------
+
+ * Removed class name support in `WorkflowRegistry::add()` as second parameter.
diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php
@@ -15,6 +15,7 @@
 use Symfony\Component\Workflow\Definition;
 use Symfony\Component\Workflow\Marking;
 use Symfony\Component\Workflow\Registry;
+use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy;
 use Symfony\Component\Workflow\Transition;
 use Symfony\Component\Workflow\Workflow;
 
@@ -37,7 +38,7 @@ protected function setUp()
         $workflow = new Workflow($definition);
 
         $registry = new Registry();
-        $registry->add($workflow, \stdClass::class);
+        $registry->add($workflow, new ClassInstanceSupportStrategy(\stdClass::class));
 
         $this->extension = new WorkflowExtension($registry);
     }
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -280,7 +280,6 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                 ->end()
                             ->end()
                             ->arrayNode('supports')
-                                ->isRequired()
                                 ->beforeNormalization()
                                     ->ifString()
                                     ->then(function ($v) { return array($v); })
@@ -293,7 +292,12 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                     ->end()
                                 ->end()
                             ->end()
-                            ->scalarNode('initial_place')->defaultNull()->end()
+                            ->scalarNode('support_strategy')
+                                ->cannotBeEmpty()
+                            ->end()
+                            ->scalarNode('initial_place')
+                                ->defaultNull()
+                            ->end()
                             ->arrayNode('places')
                                 ->isRequired()
                                 ->requiresAtLeastOneElement()
@@ -353,6 +357,18 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                 ->end()
                             ->end()
                         ->end()
+                        ->validate()
+                            ->ifTrue(function ($v) {
+                                return $v['supports'] && isset($v['support_strategy']);
+                            })
+                            ->thenInvalid('"supports" and "support_strategy" cannot be used together.')
+                        ->end()
+                        ->validate()
+                            ->ifTrue(function ($v) {
+                                return !$v['supports'] && !isset($v['support_strategy']);
+                            })
+                            ->thenInvalid('"supports" or "support_strategy" should be configured.')
+                        ->end()
                     ->end()
                 ->end()
             ->end()
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -36,6 +36,7 @@
 use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;
 use Symfony\Component\Serializer\Normalizer\JsonSerializableNormalizer;
 use Symfony\Component\Workflow;
+use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy;
 use Symfony\Component\Yaml\Yaml;
 
 /**
@@ -475,8 +476,14 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde
             $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition);
 
             // Add workflow to Registry
-            foreach ($workflow['supports'] as $supportedClass) {
-                $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $supportedClass));
+            if ($workflow['supports']) {
+                foreach ($workflow['supports'] as $supportedClassName) {
+                    $strategyDefinition = new Definition(ClassInstanceSupportStrategy::class, array($supportedClassName));
+                    $strategyDefinition->setPublic(false);
+                    $registryDefinition->addMethodCall('add', array(new Reference($workflowId), $strategyDefinition));
+                }
+            } elseif (isset($workflow['support_strategy'])) {
+                $registryDefinition->addMethodCall('add', array(new Reference($workflowId), new Reference($workflow['support_strategy'])));
             }
         }
     }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
@@ -232,13 +232,14 @@
     <xsd:complexType name="workflow">
         <xsd:sequence>
             <xsd:element name="marking-store" type="marking_store" />
-            <xsd:element name="support" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
+            <xsd:element name="support" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
             <xsd:element name="place" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
             <xsd:element name="transition" type="transition" minOccurs="1" maxOccurs="unbounded" />
         </xsd:sequence>
         <xsd:attribute name="name" type="xsd:string" use="required" />
         <xsd:attribute name="type" type="workflow_type" />
         <xsd:attribute name="initial-place" type="xsd:string" />
+        <xsd:attribute name="support-strategy" type="xsd:string" />
     </xsd:complexType>
 
     <xsd:complexType name="marking_store">
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_support_and_support_strategy.php
@@ -0,0 +1,31 @@
+<?php
+
+use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;
+
+$container->loadFromExtension('framework', array(
+    'workflows' => array(
+        'my_workflow' => array(
+            'marking_store' => array(
+                'type' => 'multiple_state',
+            ),
+            'supports' => array(
+                FrameworkExtensionTest::class,
+            ),
+            'support_strategy' => 'foobar',
+            'places' => array(
+                'first',
+                'last',
+            ),
+            'transitions' => array(
+                'go' => array(
+                    'from' => array(
+                        'first',
+                    ),
+                    'to' => array(
+                        'last',
+                    ),
+                ),
+            ),
+        ),
+    ),
+));
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_without_support_and_support_strategy.php
@@ -0,0 +1,27 @@
+<?php
+
+use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;
+
+$container->loadFromExtension('framework', array(
+    'workflows' => array(
+        'my_workflow' => array(
+            'marking_store' => array(
+                'type' => 'multiple_state',
+            ),
+            'places' => array(
+                'first',
+                'last',
+            ),
+            'transitions' => array(
+                'go' => array(
+                    'from' => array(
+                        'first',
+                    ),
+                    'to' => array(
+                        'last',
+                    ),
+                ),
+            ),
+        ),
+    ),
+));
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml
@@ -0,0 +1,21 @@
+<?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" support-strategy="foobar">
+            <framework:marking-store type="multiple_state"/>
+            <framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
+            <framework:place>first</framework:place>
+            <framework:place>last</framework:place>
+            <framework:transition name="foobar">
+                <framework:from>a</framework:from>
+                <framework:to>a</framework:to>
+            </framework:transition>
+        </framework:workflow>
+    </framework:config>
+</container>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml
@@ -0,0 +1,20 @@
+<?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">
+            <framework:marking-store type="multiple_state"/>
+            <framework:place>first</framework:place>
+            <framework:place>last</framework:place>
+            <framework:transition name="foobar">
+                <framework:from>a</framework:from>
+                <framework:to>a</framework:to>
+            </framework:transition>
+        </framework:workflow>
+    </framework:config>
+</container>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_support_and_support_strategy.yml
@@ -0,0 +1,17 @@
+framework:
+    workflows:
+        my_workflow:
+            marking_store:
+                type: multiple_state
+            supports:
+                - Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
+            support_strategy: foobar
+            places:
+                - first
+                - last
+            transitions:
+                go:
+                    from:
+                        - first
+                    to:
+                        - last
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_without_support_and_support_strategy.yml
@@ -0,0 +1,14 @@
+framework:
+    workflows:
+        my_workflow:
+            marking_store:
+                type: multiple_state
+            places:
+                - first
+                - last
+            transitions:
+                go:
+                    from:
+                        - first
+                    to:
+                        - last
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -173,6 +173,10 @@ public function testWorkflows()
         $markingStoreRef = $serviceMarkingStoreWorkflowDefinition->getArgument(1);
         $this->assertInstanceOf(Reference::class, $markingStoreRef);
         $this->assertEquals('workflow_service', (string) $markingStoreRef);
+
+        $this->assertTrue($container->hasDefinition('workflow.registry', 'Workflow registry is registered as a service'));
+        $registryDefinition = $container->getDefinition('workflow.registry');
+        $this->assertGreaterThan(0, count($registryDefinition->getMethodCalls()));
     }
 
     /**
@@ -184,6 +188,24 @@ public function testWorkflowCannotHaveBothTypeAndService()
         $this->createContainerFromFile('workflow_with_type_and_service');
     }
 
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage "supports" and "support_strategy" cannot be used together.
+     */
+    public function testWorkflowCannotHaveBothSupportsAndSupportStrategy()
+    {
+        $this->createContainerFromFile('workflow_with_support_and_support_strategy');
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage "supports" or "support_strategy" should be configured.
+     */
+    public function testWorkflowShouldHaveOneOfSupportsAndSupportStrategy()
+    {
+        $this->createContainerFromFile('workflow_without_support_and_support_strategy');
+    }
+
     /**
      * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
      * @expectedExceptionMessage "arguments" and "service" cannot be used together.
diff --git a/src/Symfony/Component/Workflow/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md
@@ -1,2 +1,8 @@
 CHANGELOG
 =========
+
+3.3.0
+-----
+
+ * Deprecated class name support in `WorkflowRegistry::add()` as second parameter.
+   Wrap the class name in an instance of ClassInstanceSupportStrategy instead.
diff --git a/src/Symfony/Component/Workflow/Registry.php b/src/Symfony/Component/Workflow/Registry.php
@@ -12,6 +12,8 @@
 namespace Symfony\Component\Workflow;
 
 use Symfony\Component\Workflow\Exception\InvalidArgumentException;
+use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy;
+use Symfony\Component\Workflow\SupportStrategy\SupportStrategyInterface;
 
 /**
  * @author Fabien Potencier <fabien@symfony.com>
@@ -22,12 +24,18 @@ class Registry
     private $workflows = array();
 
     /**
-     * @param Workflow $workflow
-     * @param string   $className
+     * @param Workflow                        $workflow
+     * @param string|SupportStrategyInterface $supportStrategy
      */
-    public function add(Workflow $workflow, $className)
+    public function add(Workflow $workflow, $supportStrategy)
     {
-        $this->workflows[] = array($workflow, $className);
+        if (!$supportStrategy instanceof SupportStrategyInterface) {
+            @trigger_error('Support of class name string was deprecated after version 3.2 and won\'t work anymore in 4.0.', E_USER_DEPRECATED);
+
+            $supportStrategy = new ClassInstanceSupportStrategy($supportStrategy);
+        }
+
+        $this->workflows[] = array($workflow, $supportStrategy);
     }
 
     /**
@@ -40,8 +48,8 @@ public function get($subject, $workflowName = null)
     {
         $matched = null;
 
-        foreach ($this->workflows as list($workflow, $className)) {
-            if ($this->supports($workflow, $className, $subject, $workflowName)) {
+        foreach ($this->workflows as list($workflow, $supportStrategy)) {
+            if ($this->supports($workflow, $supportStrategy, $subject, $workflowName)) {
                 if ($matched) {
                     throw new InvalidArgumentException('At least two workflows match this subject. Set a different name on each and use the second (name) argument of this method.');
                 }
@@ -56,16 +64,12 @@ public function get($subject, $workflowName = null)
         return $matched;
     }
 
-    private function supports(Workflow $workflow, $className, $subject, $name)
+    private function supports(Workflow $workflow, $supportStrategy, $subject, $workflowName)
     {
-        if (!$subject instanceof $className) {
+        if (null !== $workflowName && $workflowName !== $workflow->getName()) {
             return false;
         }
 
-        if (null === $name) {
-            return true;
-        }
-
-        return $name === $workflow->getName();
+        return $supportStrategy->supports($workflow, $subject);
     }
 }
diff --git a/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php b/src/Symfony/Component/Workflow/SupportStrategy/ClassInstanceSupportStrategy.php
diff --git a/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php b/src/Symfony/Component/Workflow/SupportStrategy/SupportStrategyInterface.php
diff --git a/src/Symfony/Component/Workflow/Tests/RegistryTest.php b/src/Symfony/Component/Workflow/Tests/RegistryTest.php
diff --git a/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php b/src/Symfony/Component/Workflow/Tests/SupportStrategy/ClassInstanceSupportStrategyTest.php
