bug #29141 [Workflow] Fixed bug of buildTransitionBlockerList when many transition are enabled (Tetragramat, lyrixx)
This PR was merged into the 4.1 branch. Discussion ---------- [Workflow] Fixed bug of buildTransitionBlockerList when many transition are enabled | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28429 #28432 #28493 | License | MIT | Doc PR | <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> Commits -------732f343572
[Workflow] Made code simplerdb69ccc185
method buildTransitionBlockerList returns TransitionBlockerList of expected transition
This commit is contained in:
commit
28cd88e261
@ -3,7 +3,10 @@
|
|||||||
namespace Symfony\Component\Workflow\Tests;
|
namespace Symfony\Component\Workflow\Tests;
|
||||||
|
|
||||||
use PHPUnit\Framework\TestCase;
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Component\EventDispatcher\EventDispatcher;
|
||||||
|
use Symfony\Component\Workflow\Event\GuardEvent;
|
||||||
use Symfony\Component\Workflow\StateMachine;
|
use Symfony\Component\Workflow\StateMachine;
|
||||||
|
use Symfony\Component\Workflow\TransitionBlocker;
|
||||||
|
|
||||||
class StateMachineTest extends TestCase
|
class StateMachineTest extends TestCase
|
||||||
{
|
{
|
||||||
@ -38,4 +41,70 @@ class StateMachineTest extends TestCase
|
|||||||
$this->assertTrue($net->can($subject, 't2'));
|
$this->assertTrue($net->can($subject, 't2'));
|
||||||
$this->assertTrue($net->can($subject, 't3'));
|
$this->assertTrue($net->can($subject, 't3'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testBuildTransitionBlockerList()
|
||||||
|
{
|
||||||
|
$definition = $this->createComplexStateMachineDefinition();
|
||||||
|
|
||||||
|
$net = new StateMachine($definition);
|
||||||
|
$subject = new \stdClass();
|
||||||
|
|
||||||
|
$subject->marking = 'a';
|
||||||
|
$this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
$subject->marking = 'd';
|
||||||
|
$this->assertTrue($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
|
||||||
|
$subject->marking = 'b';
|
||||||
|
$this->assertFalse($net->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testBuildTransitionBlockerListWithMultipleTransitions()
|
||||||
|
{
|
||||||
|
$definition = $this->createComplexStateMachineDefinition();
|
||||||
|
|
||||||
|
$net = new StateMachine($definition);
|
||||||
|
$subject = new \stdClass();
|
||||||
|
|
||||||
|
$subject->marking = 'b';
|
||||||
|
$this->assertTrue($net->buildTransitionBlockerList($subject, 't2')->isEmpty());
|
||||||
|
$this->assertTrue($net->buildTransitionBlockerList($subject, 't3')->isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testBuildTransitionBlockerListReturnsExpectedReasonOnBranchMerge()
|
||||||
|
{
|
||||||
|
$definition = $this->createComplexStateMachineDefinition();
|
||||||
|
|
||||||
|
$dispatcher = new EventDispatcher();
|
||||||
|
$net = new StateMachine($definition, null, $dispatcher);
|
||||||
|
|
||||||
|
$dispatcher->addListener('workflow.guard', function (GuardEvent $event) {
|
||||||
|
$event->addTransitionBlocker(new TransitionBlocker(\sprintf('Transition blocker of place %s', $event->getTransition()->getFroms()[0]), 'blocker'));
|
||||||
|
});
|
||||||
|
|
||||||
|
$subject = new \stdClass();
|
||||||
|
|
||||||
|
// There may be multiple transitions with the same name. Make sure that transitions
|
||||||
|
// that are not enabled by the marking are evaluated.
|
||||||
|
// see https://github.com/symfony/symfony/issues/28432
|
||||||
|
|
||||||
|
// Test if when you are in place "a"trying transition "t1" then returned
|
||||||
|
// blocker list contains guard blocker instead blockedByMarking
|
||||||
|
$subject->marking = 'a';
|
||||||
|
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
|
||||||
|
$this->assertCount(1, $transitionBlockerList);
|
||||||
|
$blockers = iterator_to_array($transitionBlockerList);
|
||||||
|
|
||||||
|
$this->assertSame('Transition blocker of place a', $blockers[0]->getMessage());
|
||||||
|
$this->assertSame('blocker', $blockers[0]->getCode());
|
||||||
|
|
||||||
|
// Test if when you are in place "d" trying transition "t1" then
|
||||||
|
// returned blocker list contains guard blocker instead blockedByMarking
|
||||||
|
$subject->marking = 'd';
|
||||||
|
$transitionBlockerList = $net->buildTransitionBlockerList($subject, 't1');
|
||||||
|
$this->assertCount(1, $transitionBlockerList);
|
||||||
|
$blockers = iterator_to_array($transitionBlockerList);
|
||||||
|
|
||||||
|
$this->assertSame('Transition blocker of place d', $blockers[0]->getMessage());
|
||||||
|
$this->assertSame('blocker', $blockers[0]->getCode());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -195,6 +195,32 @@ class WorkflowTest extends TestCase
|
|||||||
$workflow->buildTransitionBlockerList($subject, '404 Not Found');
|
$workflow->buildTransitionBlockerList($subject, '404 Not Found');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testBuildTransitionBlockerList()
|
||||||
|
{
|
||||||
|
$definition = $this->createComplexWorkflowDefinition();
|
||||||
|
$subject = new \stdClass();
|
||||||
|
$subject->marking = null;
|
||||||
|
$workflow = new Workflow($definition, new MultipleStateMarkingStore());
|
||||||
|
|
||||||
|
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
|
||||||
|
|
||||||
|
$subject->marking = array('b' => 1);
|
||||||
|
|
||||||
|
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
|
||||||
|
|
||||||
|
$subject->marking = array('b' => 1, 'c' => 1);
|
||||||
|
|
||||||
|
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't1')->isEmpty());
|
||||||
|
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't2')->isEmpty());
|
||||||
|
|
||||||
|
$subject->marking = array('f' => 1);
|
||||||
|
|
||||||
|
$this->assertFalse($workflow->buildTransitionBlockerList($subject, 't5')->isEmpty());
|
||||||
|
$this->assertTrue($workflow->buildTransitionBlockerList($subject, 't6')->isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking()
|
public function testBuildTransitionBlockerListReturnsReasonsProvidedByMarking()
|
||||||
{
|
{
|
||||||
$definition = $this->createComplexWorkflowDefinition();
|
$definition = $this->createComplexWorkflowDefinition();
|
||||||
|
@ -37,6 +37,17 @@ final class TransitionBlockerList implements \IteratorAggregate, \Countable
|
|||||||
$this->blockers[] = $blocker;
|
$this->blockers[] = $blocker;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function has(string $code): bool
|
||||||
|
{
|
||||||
|
foreach ($this->blockers as $blocker) {
|
||||||
|
if ($code === $blocker->getCode()) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
public function clear(): void
|
public function clear(): void
|
||||||
{
|
{
|
||||||
$this->blockers = array();
|
$this->blockers = array();
|
||||||
|
@ -119,7 +119,15 @@ class Workflow implements WorkflowInterface
|
|||||||
$transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
|
$transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
|
||||||
|
|
||||||
if ($transitionBlockerList->isEmpty()) {
|
if ($transitionBlockerList->isEmpty()) {
|
||||||
continue;
|
return $transitionBlockerList;
|
||||||
|
}
|
||||||
|
|
||||||
|
// We prefer to return transitions blocker by something else than
|
||||||
|
// marking. Because it means the marking was OK. Transitions are
|
||||||
|
// deterministic: it's not possible to have many transitions enabled
|
||||||
|
// at the same time that match the same marking with the same name
|
||||||
|
if (!$transitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) {
|
||||||
|
return $transitionBlockerList;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user