feature #26092 [Workflow] Add a MetadataStore to fetch some metadata …

…(lyrixx)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Workflow] Add a MetadataStore to fetch some metadata

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes (little)
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #23257
| License       | MIT
| Doc PR        | TODO

---

This is an attempt to fix #23257. I first started to implement
`Ẁorkflow::getMetadata()`, `Transition::getMetadata()` and
`Place::getMetadata()`. **BUT**, there are no `Place` class. For now it's just a
`string`. So dealing with BC is a nightmare.

So I tried to find another way to fix the issue. [This
comment](#23257 (comment))
summary well the two options. But this PR is (will be) a mix of theses 2
options.

First it will be possible to configure the workflow/metadata like this:

```yaml
blog_publishing:
    supports:
        - AppBundle\Entity\BlogPost
    metada:
         label: Blog publishing
         description: Manages blog publishing
    places:
        draft:
            metadata:
                 description: Blog has just been created
                 color: grey
        review:
            metadata:
                 description: Blog is waiting for review
                 color: blue
    transitions:
        to_review:
            from: draft
            to: review
            metadata:
                label: Submit for review
                route: admin.blog.review
```

I think is very good for the DX. Simple to understand.

All metadata will live in a `MetadataStoreInterface`. If metadata are set via
the configuration (workflows.yaml), then we will use the
`InMemoryMetadataStore`.

Having a MetadataStoreInterface allow user to get dynamic value for a place /
transitions. It's really flexible. (But is it a valid use case ?)

Then, to retrieve these data, the end user will have to write this code:

```php
public function onReview(Event $event) {
    $metadataStore = $event->getWorkflow()->getMetadataStore();
    foreach ($event->getTransition()->getTos() as $place) {
        $this->flashbag->add('info', $metadataStore->getPlaceMetadata($place)->get('description'));
    }
}
```

Note: I might add some shortcut to the Event class

or in twig:

```jinja
{% for transition in workflow_transitions(post) %}
    <a href="{{ workflow_metadata_transition(post, route) }}">
         {{ workflow_metadata_transition(post, transition) }}
   </a>
{% endfor %}
```

---

WDYT ?

Should I continue this way, or should I introduce a `Place` class (there will be
so many deprecation ...)

Commits
-------

bd1f2c8 [Workflow] Add a MetadataStore

UPGRADE-4.1.md

@@ -104,3 +104,4 @@ Workflow
  * Deprecated the `add` method in favor of the `addWorkflow` method in `Workflow\Registry`.
  * Deprecated `SupportStrategyInterface` in favor of `WorkflowSupportStrategyInterface`.
  * Deprecated the class `ClassInstanceSupportStrategy` in favor of the class `InstanceOfSupportStrategy`.
+ * Deprecated passing the workflow name as 4th parameter of `Event` constructor in favor of the workflow itself.

src/Symfony/Bridge/Twig/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.1.0
+-----
+
+ * add a `workflow_metadata` function
+
 3.4.0
 -----
 

src/Symfony/Bridge/Twig/Extension/WorkflowExtension.php

@@ -37,6 +37,7 @@ public function getFunctions()
             new TwigFunction('workflow_transitions', array($this, 'getEnabledTransitions')),
             new TwigFunction('workflow_has_marked_place', array($this, 'hasMarkedPlace')),
             new TwigFunction('workflow_marked_places', array($this, 'getMarkedPlaces')),
+            new TwigFunction('workflow_metadata', array($this, 'getMetadata')),
         );
     }
 
@@ -101,6 +102,24 @@ public function getMarkedPlaces($subject, $placesNameOnly = true, $name = null)
         return $places;
     }
 
+    /**
+     * Returns the metadata for a specific subject.
+     *
+     * @param object                 $subject         A subject
+     * @param null|string|Transition $metadataSubject Use null to get workflow metadata
+     *                                                Use a string (the place name) to get place metadata
+     *                                                Use a Transition instance to get transition metadata
+     */
+    public function getMetadata($subject, string $key, $metadataSubject = null, string $name = null): ?string
+    {
+        return $this
+            ->workflowRegistry
+            ->get($subject, $name)
+            ->getMetadataStore()
+            ->getMetadata($key, $metadataSubject)
+        ;
+    }
+
     public function getName()
     {
         return 'workflow';

src/Symfony/Bridge/Twig/Tests/Extension/WorkflowExtensionTest.php

@@ -14,6 +14,7 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Twig\Extension\WorkflowExtension;
 use Symfony\Component\Workflow\Definition;
+use Symfony\Component\Workflow\Metadata\InMemoryMetadataStore;
 use Symfony\Component\Workflow\Registry;
 use Symfony\Component\Workflow\SupportStrategy\ClassInstanceSupportStrategy;
 use Symfony\Component\Workflow\SupportStrategy\InstanceOfSupportStrategy;
@@ -23,6 +24,7 @@
 class WorkflowExtensionTest extends TestCase
 {
     private $extension;
+    private $t1;
 
     protected function setUp()
     {
@@ -32,10 +34,21 @@ protected function setUp()
 
         $places = array('ordered', 'waiting_for_payment', 'processed');
         $transitions = array(
-            new Transition('t1', 'ordered', 'waiting_for_payment'),
+            $this->t1 = new Transition('t1', 'ordered', 'waiting_for_payment'),
             new Transition('t2', 'waiting_for_payment', 'processed'),
         );
-        $definition = new Definition($places, $transitions);
+
+        $metadataStore = null;
+        if (class_exists(InMemoryMetadataStore::class)) {
+            $transitionsMetadata = new \SplObjectStorage();
+            $transitionsMetadata->attach($this->t1, array('title' => 't1 title'));
+            $metadataStore = new InMemoryMetadataStore(
+                array('title' => 'workflow title'),
+                array('orderer' => array('title' => 'ordered title')),
+                $transitionsMetadata
+            );
+        }
+        $definition = new Definition($places, $transitions, null, $metadataStore);
         $workflow = new Workflow($definition);
 
         $registry = new Registry();
@@ -88,4 +101,19 @@ public function testGetMarkedPlaces()
         $this->assertSame(array('ordered', 'waiting_for_payment'), $this->extension->getMarkedPlaces($subject));
         $this->assertSame($subject->marking, $this->extension->getMarkedPlaces($subject, false));
     }
+
+    public function testGetMetadata()
+    {
+        if (!class_exists(InMemoryMetadataStore::class)) {
+            $this->markTestSkipped('This test requires symfony/workflow:4.1.');
+        }
+        $subject = new \stdClass();
+        $subject->marking = array();
+
+        $this->assertSame('workflow title', $this->extension->getMetadata($subject, 'title'));
+        $this->assertSame('ordered title', $this->extension->getMetadata($subject, 'title', 'orderer'));
+        $this->assertSame('t1 title', $this->extension->getMetadata($subject, 'title', $this->t1));
+        $this->assertNull($this->extension->getMetadata($subject, 'not found'));
+        $this->assertNull($this->extension->getMetadata($subject, 'not found', $this->t1));
+    }
 }

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -31,6 +31,7 @@
  * FrameworkExtension configuration structure.
  *
  * @author Jeremy Mikola <jmikola@gmail.com>
+ * @author Grégoire Pineau <lyrixx@lyrixx.info>
  */
 class Configuration implements ConfigurationInterface
 {
@@ -292,23 +293,61 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                         ->defaultNull()
                                     ->end()
                                     ->arrayNode('places')
+                                        ->beforeNormalization()
+                                            ->always()
+                                            ->then(function ($places) {
+                                                // It's an indexed array of shape  ['place1', 'place2']
+                                                if (isset($places[0]) && is_string($places[0])) {
+                                                    return array_map(function (string $place) {
+                                                        return array('name' => $place);
+                                                    }, $places);
+                                                }
+
+                                                // It's an indexed array, we let the validation occur
+                                                if (isset($places[0]) && is_array($places[0])) {
+                                                    return $places;
+                                                }
+
+                                                foreach ($places as $name => $place) {
+                                                    if (is_array($place) && array_key_exists('name', $place)) {
+                                                        continue;
+                                                    }
+                                                    $place['name'] = $name;
+                                                    $places[$name] = $place;
+                                                }
+
+                                                return array_values($places);
+                                            })
+                                        ->end()
                                         ->isRequired()
                                         ->requiresAtLeastOneElement()
-                                        ->prototype('scalar')
-                                            ->cannotBeEmpty()
+                                        ->prototype('array')
+                                            ->children()
+                                                ->scalarNode('name')
+                                                    ->isRequired()
+                                                    ->cannotBeEmpty()
+                                                ->end()
+                                                ->arrayNode('metadata')
+                                                    ->normalizeKeys(false)
+                                                    ->defaultValue(array())
+                                                    ->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
+                                                    ->prototype('variable')
+                                                    ->end()
+                                                ->end()
+                                            ->end()
                                         ->end()
                                     ->end()
                                     ->arrayNode('transitions')
                                         ->beforeNormalization()
                                             ->always()
                                             ->then(function ($transitions) {
-                                                // It's an indexed array, we let the validation occurs
-                                                if (isset($transitions[0])) {
+                                                // It's an indexed array, we let the validation occur
+                                                if (isset($transitions[0]) && is_array($transitions[0])) {
                                                     return $transitions;
                                                 }
 
                                                 foreach ($transitions as $name => $transition) {
-                                                    if (array_key_exists('name', $transition)) {
+                                                    if (is_array($transition) && array_key_exists('name', $transition)) {
                                                         continue;
                                                     }
                                                     $transition['name'] = $name;
@@ -351,9 +390,23 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
                                                         ->cannotBeEmpty()
                                                     ->end()
                                                 ->end()
+                                                ->arrayNode('metadata')
+                                                    ->normalizeKeys(false)
+                                                    ->defaultValue(array())
+                                                    ->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
+                                                    ->prototype('variable')
+                                                    ->end()
+                                                ->end()
                                             ->end()
                                         ->end()
                                     ->end()
+                                    ->arrayNode('metadata')
+                                        ->normalizeKeys(false)
+                                        ->defaultValue(array())
+                                        ->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
+                                        ->prototype('variable')
+                                        ->end()
+                                    ->end()
                                 ->end()
                                 ->validate()
                                     ->ifTrue(function ($v) {

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

@@ -466,32 +466,68 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
         foreach ($config['workflows'] as $name => $workflow) {
             $type = $workflow['type'];
 
+            // Process Metadata (workflow + places (transition is done in the "create transition" block))
+            $metadataStoreDefinition = new Definition(Workflow\Metadata\InMemoryMetadataStore::class, array(null, null, null));
+            if ($workflow['metadata']) {
+                $metadataStoreDefinition->replaceArgument(0, $workflow['metadata']);
+            }
+            $placesMetadata = array();
+            foreach ($workflow['places'] as $place) {
+                if ($place['metadata']) {
+                    $placesMetadata[$place['name']] = $place['metadata'];
+                }
+            }
+            if ($placesMetadata) {
+                $metadataStoreDefinition->replaceArgument(1, $placesMetadata);
+            }
+
+            // Create transitions
             $transitions = array();
+            $transitionsMetadataDefinition = new Definition(\SplObjectStorage::class);
             foreach ($workflow['transitions'] as $transition) {
                 if ('workflow' === $type) {
-                    $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to']));
+                    $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to']));
+                    $transitions[] = $transitionDefinition;
+                    if ($transition['metadata']) {
+                        $transitionsMetadataDefinition->addMethodCall('attach', array(
+                            $transitionDefinition,
+                            $transition['metadata'],
+                        ));
+                    }
                 } elseif ('state_machine' === $type) {
                     foreach ($transition['from'] as $from) {
                         foreach ($transition['to'] as $to) {
-                            $transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to));
+                            $transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to));
+                            $transitions[] = $transitionDefinition;
+                            if ($transition['metadata']) {
+                                $transitionsMetadataDefinition->addMethodCall('attach', array(
+                                    $transitionDefinition,
+                                    $transition['metadata'],
+                                ));
+                            }
                         }
                     }
                 }
             }
+            $metadataStoreDefinition->replaceArgument(2, $transitionsMetadataDefinition);
+
+            // Create places
+            $places = array_map(function (array $place) {
+                return $place['name'];
+            }, $workflow['places']);
 
             // Create a Definition
             $definitionDefinition = new Definition(Workflow\Definition::class);
             $definitionDefinition->setPublic(false);
-            $definitionDefinition->addArgument($workflow['places']);
+            $definitionDefinition->addArgument($places);
             $definitionDefinition->addArgument($transitions);
+            $definitionDefinition->addArgument($workflow['initial_place'] ?? null);
+            $definitionDefinition->addArgument($metadataStoreDefinition);
             $definitionDefinition->addTag('workflow.definition', array(
                 'name' => $name,
                 'type' => $type,
                 'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null,
             ));
-            if (isset($workflow['initial_place'])) {
-                $definitionDefinition->addArgument($workflow['initial_place']);
-            }
 
             // Create MarkingStore
             if (isset($workflow['marking_store']['type'])) {

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -273,8 +273,9 @@
         <xsd:sequence>
             <xsd:element name="marking-store" type="marking_store" minOccurs="0" maxOccurs="1" />
             <xsd:element name="support" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
-            <xsd:element name="place" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:element name="place" type="place" minOccurs="0" maxOccurs="unbounded" />
             <xsd:element name="transition" type="transition" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
         </xsd:sequence>
         <xsd:attribute name="name" type="xsd:string" />
         <xsd:attribute name="type" type="workflow_type" />
@@ -302,10 +303,24 @@
         <xsd:sequence>
             <xsd:element name="from" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
             <xsd:element name="to" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
+            <xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
         </xsd:sequence>
         <xsd:attribute name="name" type="xsd:string" use="required" />
     </xsd:complexType>
 
+    <xsd:complexType name="place" mixed="true">
+        <xsd:sequence>
+            <xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
+        </xsd:sequence>
+        <xsd:attribute name="name" type="xsd:string" />
+    </xsd:complexType>
+
+    <xsd:complexType name="metadata">
+        <xsd:sequence>
+            <xsd:any minOccurs="0" processContents="lax"/>
+        </xsd:sequence>
+    </xsd:complexType>
+
     <xsd:simpleType name="workflow_type">
         <xsd:restriction base="xsd:string">
             <xsd:enumeration value="state_machine" />

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/workflows.php

@@ -48,18 +48,29 @@
                 FrameworkExtensionTest::class,
             ),
             'initial_place' => 'start',
+            'metadata' => array(
+                'title' => 'workflow title',
+            ),
             'places' => array(
-                'start',
-                'coding',
-                'travis',
-                'review',
-                'merged',
-                'closed',
+                'start_name_not_used' => array(
+                    'name' => 'start',
+                    'metadata' => array(
+                        'title' => 'place start title',
+                    ),
+                ),
+                'coding' => null,
+                'travis' => null,
+                'review' => null,
+                'merged' => null,
+                'closed' => null,
             ),
             'transitions' => array(
                 'submit' => array(
                     'from' => 'start',
                     'to' => 'travis',
+                    'metadata' => array(
+                        'title' => 'transition submit title',
+                    ),
                 ),
                 'update' => array(
                     'from' => array('coding', 'travis', 'review'),
@@ -96,8 +107,8 @@
                 FrameworkExtensionTest::class,
             ),
             'places' => array(
-                'first',
-                'last',
+                array('name' => 'first'),
+                array('name' => 'last'),
             ),
             'transitions' => array(
                 'go' => array(

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_arguments_and_service.xml

@@ -13,8 +13,8 @@
                 <framework:argument>a</framework:argument>
             </framework:marking-store>
             <framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
-            <framework:place>first</framework:place>
-            <framework:place>last</framework:place>
+            <framework:place name="first" />
+            <framework:place name="last" />
             <framework:transition name="foobar">
                 <framework:from>a</framework:from>
                 <framework:to>a</framework:to>

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_multiple_transitions_with_same_name.xml

@@ -13,12 +13,12 @@
                 <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>
-            <framework:place>approved_by_journalist</framework:place>
-            <framework:place>wait_for_spellchecker</framework:place>
-            <framework:place>approved_by_spellchecker</framework:place>
-            <framework:place>published</framework:place>
+            <framework:place name="draft" />
+            <framework:place name="wait_for_journalist" />
+            <framework:place name="approved_by_journalist" />
+            <framework:place name="wait_for_spellchecker" />
+            <framework:place name="approved_by_spellchecker" />
+            <framework:place name="published" />
             <framework:transition name="request_review">
                 <framework:from>draft</framework:from>
                 <framework:to>wait_for_journalist</framework:to>