From ad2db375e1009ab5b30810cf24543c319456c2c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Wed, 13 Mar 2024 16:04:20 +0100 Subject: [PATCH] [Workflow] Fix Marking when it must contains more than one tokens --- UPGRADE-7.1.md | 1 + src/Symfony/Component/Workflow/Marking.php | 53 ++++++++++-- .../Component/Workflow/Tests/MarkingTest.php | 54 +++++++++++- .../Workflow/Tests/WorkflowBuilderTrait.php | 39 +++++++++ .../Component/Workflow/Tests/WorkflowTest.php | 85 ++++++++++++++++--- 5 files changed, 210 insertions(+), 22 deletions(-) diff --git a/UPGRADE-7.1.md b/UPGRADE-7.1.md index 6a82eda9b7c95..a2d0b65cebe64 100644 --- a/UPGRADE-7.1.md +++ b/UPGRADE-7.1.md @@ -46,3 +46,4 @@ Workflow -------- * Add method `getEnabledTransition()` to `WorkflowInterface` + * Add `$nbToken` argument to `Marking::mark()` and `Marking::unmark()` diff --git a/src/Symfony/Component/Workflow/Marking.php b/src/Symfony/Component/Workflow/Marking.php index de9c8a8d9e1fa..8712ff616233e 100644 --- a/src/Symfony/Component/Workflow/Marking.php +++ b/src/Symfony/Component/Workflow/Marking.php @@ -27,23 +27,64 @@ class Marking public function __construct(array $representation = []) { foreach ($representation as $place => $nbToken) { - $this->mark($place); + $this->mark($place, $nbToken); } } - public function mark(string $place): void + /** + * @param int $nbToken + */ + public function mark(string $place /*, int $nbToken = 1 */): void { - $this->places[$place] = 1; + if (1 === \func_num_args()) { + $nbToken = 1; + } else { + $nbToken = func_get_arg(1); + } + + if ($nbToken < 1) { + throw new \LogicException(sprintf('The number of tokens must be greater than 0, "%s" given.', $nbToken)); + } + + if (!\array_key_exists($place, $this->places)) { + $this->places[$place] = 0; + } + $this->places[$place] += $nbToken; } - public function unmark(string $place): void + /** + * @param int $nbToken + */ + public function unmark(string $place /*, int $nbToken = 1 */): void { - unset($this->places[$place]); + if (1 === \func_num_args()) { + $nbToken = 1; + } else { + $nbToken = func_get_arg(1); + } + + if ($nbToken < 1) { + throw new \LogicException(sprintf('The number of tokens must be greater than 0, "%s" given.', $nbToken)); + } + + if (!$this->has($place)) { + throw new \LogicException(sprintf('The place "%s" is not marked.', $place)); + } + + $this->places[$place] -= $nbToken; + + if (0 > $this->places[$place]) { + throw new \LogicException(sprintf('The place "%s" could not contain a negative token number.', $place)); + } + + if (0 === $this->places[$place]) { + unset($this->places[$place]); + } } public function has(string $place): bool { - return isset($this->places[$place]); + return \array_key_exists($place, $this->places); } public function getPlaces(): array diff --git a/src/Symfony/Component/Workflow/Tests/MarkingTest.php b/src/Symfony/Component/Workflow/Tests/MarkingTest.php index 0a1c22b4cc9d7..fc06b725d158e 100644 --- a/src/Symfony/Component/Workflow/Tests/MarkingTest.php +++ b/src/Symfony/Component/Workflow/Tests/MarkingTest.php @@ -22,24 +22,70 @@ public function testMarking() $this->assertTrue($marking->has('a')); $this->assertFalse($marking->has('b')); - $this->assertSame(['a' => 1], $marking->getPlaces()); + $this->assertPlaces(['a' => 1], $marking); $marking->mark('b'); $this->assertTrue($marking->has('a')); $this->assertTrue($marking->has('b')); - $this->assertSame(['a' => 1, 'b' => 1], $marking->getPlaces()); + $this->assertPlaces(['a' => 1, 'b' => 1], $marking); $marking->unmark('a'); $this->assertFalse($marking->has('a')); $this->assertTrue($marking->has('b')); - $this->assertSame(['b' => 1], $marking->getPlaces()); + $this->assertPlaces(['b' => 1], $marking); $marking->unmark('b'); $this->assertFalse($marking->has('a')); $this->assertFalse($marking->has('b')); - $this->assertSame([], $marking->getPlaces()); + $this->assertPlaces([], $marking); + + $marking->mark('a'); + $this->assertPlaces(['a' => 1], $marking); + + $marking->mark('a'); + $this->assertPlaces(['a' => 2], $marking); + + $marking->unmark('a'); + $this->assertPlaces(['a' => 1], $marking); + + $marking->unmark('a'); + $this->assertPlaces([], $marking); + } + + public function testGuardNotMarked() + { + $marking = new Marking([]); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The place "a" is not marked.'); + $marking->unmark('a'); + } + + public function testGuardNotNbTokenLowerThanZero() + { + $marking = new Marking(['a' => 1]); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The place "a" could not contain a negative token number.'); + $marking->unmark('a', 2); + } + + public function testGuardNotNbTokenEquals0() + { + $marking = new Marking(['a' => 1]); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The number of tokens must be greater than 0, "0" given.'); + $marking->unmark('a', 0); + } + + private function assertPlaces(array $expected, Marking $marking) + { + $places = $marking->getPlaces(); + ksort($places); + $this->assertSame($expected, $places); } } diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowBuilderTrait.php b/src/Symfony/Component/Workflow/Tests/WorkflowBuilderTrait.php index 07a589e47b04b..86478bba78c47 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowBuilderTrait.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowBuilderTrait.php @@ -158,4 +158,43 @@ private static function createComplexStateMachineDefinition(): Definition // | d | -------------+ // +-----+ } + + private static function createWorkflowWithSameNameBackTransition(): Definition + { + $places = range('a', 'c'); + + $transitions = []; + $transitions[] = new Transition('a_to_bc', 'a', ['b', 'c']); + $transitions[] = new Transition('back1', 'b', 'a'); + $transitions[] = new Transition('back1', 'c', 'b'); + $transitions[] = new Transition('back2', 'c', 'b'); + $transitions[] = new Transition('back2', 'b', 'a'); + $transitions[] = new Transition('c_to_cb', 'c', ['b', 'c']); + + return new Definition($places, $transitions); + + // The graph looks like: + // +-----------------------------------------------------------------+ + // | | + // | | + // | +---------------------------------------------+ | + // v | v | + // +---+ +---------+ +-------+ +---------+ +---+ +-------+ + // | a | --> | a_to_bc | --> | | --> | back2 | --> | | --> | back2 | + // +---+ +---------+ | | +---------+ | | +-------+ + // ^ | | | | + // | | c | <-----+ | b | + // | | | | | | + // | | | +---------+ | | +-------+ + // | | | --> | c_to_cb | --> | | --> | back1 | + // | +-------+ +---------+ +---+ +-------+ + // | | ^ | + // | | | | + // | v | | + // | +-------+ | | + // | | back1 | ----------------------+ | + // | +-------+ | + // | | + // +-----------------------------------------------------------------+ + } } diff --git a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php index 8e112df60dce5..83af790fcac7d 100644 --- a/src/Symfony/Component/Workflow/Tests/WorkflowTest.php +++ b/src/Symfony/Component/Workflow/Tests/WorkflowTest.php @@ -319,28 +319,32 @@ public function testApplyWithSameNameTransition() $marking = $workflow->apply($subject, 'a_to_bc'); - $this->assertFalse($marking->has('a')); - $this->assertTrue($marking->has('b')); - $this->assertTrue($marking->has('c')); + $this->assertPlaces([ + 'b' => 1, + 'c' => 1, + ], $marking); $marking = $workflow->apply($subject, 'to_a'); - $this->assertTrue($marking->has('a')); - $this->assertFalse($marking->has('b')); - $this->assertFalse($marking->has('c')); + // Two tokens in "a" + $this->assertPlaces([ + 'a' => 2, + ], $marking); $workflow->apply($subject, 'a_to_bc'); $marking = $workflow->apply($subject, 'b_to_c'); - $this->assertFalse($marking->has('a')); - $this->assertFalse($marking->has('b')); - $this->assertTrue($marking->has('c')); + $this->assertPlaces([ + 'a' => 1, + 'c' => 2, + ], $marking); $marking = $workflow->apply($subject, 'to_a'); - $this->assertTrue($marking->has('a')); - $this->assertFalse($marking->has('b')); - $this->assertFalse($marking->has('c')); + $this->assertPlaces([ + 'a' => 2, + 'c' => 1, + ], $marking); } public function testApplyWithSameNameTransition2() @@ -776,6 +780,63 @@ public function testGetEnabledTransitionsWithSameNameTransition() $this->assertSame('to_a', $transitions[1]->getName()); $this->assertSame('to_a', $transitions[2]->getName()); } + + /** + * @@testWith ["back1"] + * ["back2"] + */ + public function testApplyWithSameNameBackTransition(string $transition) + { + $definition = $this->createWorkflowWithSameNameBackTransition(); + $workflow = new Workflow($definition, new MethodMarkingStore()); + + $subject = new Subject(); + + $marking = $workflow->apply($subject, 'a_to_bc'); + $this->assertPlaces([ + 'b' => 1, + 'c' => 1, + ], $marking); + + $marking = $workflow->apply($subject, $transition); + $this->assertPlaces([ + 'a' => 1, + 'b' => 1, + ], $marking); + + $marking = $workflow->apply($subject, $transition); + $this->assertPlaces([ + 'a' => 2, + ], $marking); + + $marking = $workflow->apply($subject, 'a_to_bc'); + $this->assertPlaces([ + 'a' => 1, + 'b' => 1, + 'c' => 1, + ], $marking); + + $marking = $workflow->apply($subject, 'c_to_cb'); + $this->assertPlaces([ + 'a' => 1, + 'b' => 2, + 'c' => 1, + ], $marking); + + $marking = $workflow->apply($subject, 'c_to_cb'); + $this->assertPlaces([ + 'a' => 1, + 'b' => 3, + 'c' => 1, + ], $marking); + } + + private function assertPlaces(array $expected, Marking $marking) + { + $places = $marking->getPlaces(); + ksort($places); + $this->assertSame($expected, $places); + } } class EventDispatcherMock implements \Symfony\Contracts\EventDispatcher\EventDispatcherInterface