[Workflow] Apply the same logic of precedence between the apply() and the buildTransitionBlockerList() method

This commit is contained in:
Grégoire Pineau 2019-11-24 18:18:02 +01:00
parent 2743e259c6
commit 2ff3496d62
2 changed files with 62 additions and 22 deletions

View File

@ -5,6 +5,7 @@ namespace Symfony\Component\Workflow\Tests;
use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\Workflow\Event\GuardEvent;
use Symfony\Component\Workflow\Exception\NotEnabledTransitionException;
use Symfony\Component\Workflow\StateMachine;
use Symfony\Component\Workflow\TransitionBlocker;
@ -84,27 +85,52 @@ class StateMachineTest extends TestCase
$subject = new Subject();
// There may be multiple transitions with the same name. Make sure that transitions
// that are not enabled by the marking are evaluated.
// that are 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
// Test if when you are in place "a" and trying to apply "t1" then it returns
// blocker list contains guard blocker instead blockedByMarking
$subject->setMarking('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
// Test if when you are in place "d" and trying to apply "t1" then
// it returns blocker list contains guard blocker instead blockedByMarking
$subject->setMarking('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());
}
public function testApplyReturnsExpectedReasonOnBranchMerge()
{
$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 Subject();
// There may be multiple transitions with the same name. Make sure that all transitions
// that are enabled by the marking are evaluated.
// see https://github.com/symfony/symfony/issues/34489
try {
$net->apply($subject, 't1');
$this->fail();
} catch (NotEnabledTransitionException $e) {
$blockers = iterator_to_array($e->getTransitionBlockerList());
$this->assertSame('Transition blocker of place a', $blockers[0]->getMessage());
$this->assertSame('blocker', $blockers[0]->getCode());
}
}
}

View File

@ -159,25 +159,47 @@ class Workflow implements WorkflowInterface
$marking = $this->getMarking($subject);
$transitionBlockerList = null;
$applied = false;
$approvedTransitionQueue = [];
$transitionExist = false;
$approvedTransitions = [];
$bestTransitionBlockerList = null;
foreach ($this->definition->getTransitions() as $transition) {
if ($transition->getName() !== $transitionName) {
continue;
}
$transitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
if (!$transitionBlockerList->isEmpty()) {
$transitionExist = true;
$tmpTransitionBlockerList = $this->buildTransitionBlockerListForTransition($subject, $marking, $transition);
if ($tmpTransitionBlockerList->isEmpty()) {
$approvedTransitions[] = $transition;
continue;
}
$approvedTransitionQueue[] = $transition;
if (!$bestTransitionBlockerList) {
$bestTransitionBlockerList = $tmpTransitionBlockerList;
continue;
}
// 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 (!$tmpTransitionBlockerList->has(TransitionBlocker::BLOCKED_BY_MARKING)) {
$bestTransitionBlockerList = $tmpTransitionBlockerList;
}
}
foreach ($approvedTransitionQueue as $transition) {
$applied = true;
if (!$transitionExist) {
throw new UndefinedTransitionException($subject, $transitionName, $this);
}
if (!$approvedTransitions) {
throw new NotEnabledTransitionException($subject, $transitionName, $this, $bestTransitionBlockerList);
}
foreach ($approvedTransitions as $transition) {
$this->leave($subject, $transition, $marking);
$context = $this->transition($subject, $transition, $marking, $context);
@ -193,14 +215,6 @@ class Workflow implements WorkflowInterface
$this->announce($subject, $transition, $marking);
}
if (!$transitionBlockerList) {
throw new UndefinedTransitionException($subject, $transitionName, $this);
}
if (!$applied) {
throw new NotEnabledTransitionException($subject, $transitionName, $this, $transitionBlockerList);
}
return $marking;
}