Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feature #21334 [Workflow] Introduce concept of SupportStrategyInterfa…
…ce (andesk, lyrixx)

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 | #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
  • Loading branch information
fabpot committed Jan 18, 2017
2 parents d0da74e + 134a58b commit 4491eb6
Show file tree
Hide file tree
Showing 19 changed files with 355 additions and 30 deletions.
6 changes: 6 additions & 0 deletions UPGRADE-3.3.md
Expand Up @@ -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.
17 changes: 11 additions & 6 deletions UPGRADE-4.0.md
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Up @@ -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;

Expand All @@ -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);
}
Expand Down
Expand Up @@ -280,7 +280,6 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
->end()
->end()
->arrayNode('supports')
->isRequired()
->beforeNormalization()
->ifString()
->then(function ($v) { return array($v); })
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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'])));
}
}
}
Expand Down
Expand Up @@ -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">
Expand Down
@@ -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',
),
),
),
),
),
));
@@ -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',
),
),
),
),
),
));
@@ -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>
@@ -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>
@@ -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
@@ -0,0 +1,14 @@
framework:
workflows:
my_workflow:
marking_store:
type: multiple_state
places:
- first
- last
transitions:
go:
from:
- first
to:
- last
Expand Up @@ -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()));
}

/**
Expand All @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions 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.
30 changes: 17 additions & 13 deletions src/Symfony/Component/Workflow/Registry.php
Expand Up @@ -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>
Expand All @@ -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);
}

/**
Expand All @@ -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.');
}
Expand All @@ -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);
}
}

0 comments on commit 4491eb6

Please sign in to comment.