From 5b4e9a9623ed02a4d36e0c026fd3932095f67c20 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] Add support for workflows that need to store many tokens in the marking --- UPGRADE-7.1.md | 1 + src/Symfony/Component/Workflow/CHANGELOG.md | 1 + src/Symfony/Component/Workflow/Marking.php | 49 +++++++++-- .../Component/Workflow/Tests/MarkingTest.php | 54 +++++++++++- .../Workflow/Tests/WorkflowBuilderTrait.php | 39 +++++++++ .../Component/Workflow/Tests/WorkflowTest.php | 85 ++++++++++++++++--- 6 files changed, 207 insertions(+), 22 deletions(-) diff --git a/UPGRADE-7.1.md b/UPGRADE-7.1.md index 6a82eda9b7c9..a2d0b65cebe6 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/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md index 247d5e469b4a..6a8430cd7e77 100644 --- a/src/Symfony/Component/Workflow/CHANGELOG.md +++ b/src/Symfony/Component/Workflow/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * Add method `getEnabledTransition()` to `WorkflowInterface` * Automatically register places from transitions + * Add support for workflows that need to store many tokens in the marking 7.0 --- diff --git a/src/Symfony/Component/Workflow/Marking.php b/src/Symfony/Component/Workflow/Marking.php index de9c8a8d9e1f..58f5ec689a19 100644 --- a/src/Symfony/Component/Workflow/Marking.php +++ b/src/Symfony/Component/Workflow/Marking.php @@ -22,23 +22,60 @@ class Marking private ?array $context = null; /** - * @param int[] $representation Keys are the place name and values should be 1 + * @param int[] $representation Keys are the place name and values should be superior or equals to 1 */ 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 + * + * @psalm-param int<1, max> $nbToken + */ + public function mark(string $place /* , int $nbToken = 1 */): void { - $this->places[$place] = 1; + $nbToken = 1 < \func_num_args() ? func_get_arg(1) : 1; + + if ($nbToken < 1) { + throw new \InvalidArgumentException(sprintf('The number of tokens must be greater than 0, "%s" given.', $nbToken)); + } + + $this->places[$place] ??= 0; + $this->places[$place] += $nbToken; } - public function unmark(string $place): void + /** + * @param int $nbToken + * + * @psalm-param int<1, max> $nbToken + */ + public function unmark(string $place /* , int $nbToken = 1 */): void { - unset($this->places[$place]); + $nbToken = 1 < \func_num_args() ? func_get_arg(1) : 1; + + if ($nbToken < 1) { + throw new \InvalidArgumentException(sprintf('The number of tokens must be greater than 0, "%s" given.', $nbToken)); + } + + if (!$this->has($place)) { + throw new \InvalidArgumentException(sprintf('The place "%s" is not marked.', $place)); + } + + $tokenCount = $this->places[$place] - $nbToken; + + if (0 > $tokenCount) { + throw new \InvalidArgumentException(sprintf('The place "%s" could not contain a negative token number: "%s" (initial) - "%s" (nbToken) = "%s".', $place, $this->places[$place], $nbToken, $tokenCount)); + } + + if (0 === $tokenCount) { + unset($this->places[$place]); + } else { + $this->places[$place] = $tokenCount; + } } public function has(string $place): bool diff --git a/src/Symfony/Component/Workflow/Tests/MarkingTest.php b/src/Symfony/Component/Workflow/Tests/MarkingTest.php index 0a1c22b4cc9d..86a306a46569 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 testUnmarkGuardResultTokenCountIsNotNegative() + { + $marking = new Marking(['a' => 1]); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The place "a" could not contain a negative token number: "1" (initial) - "2" (nbToken) = "-1".'); + $marking->unmark('a', 2); + } + + public function testUnmarkGuardNbTokenIsGreaterThanZero() + { + $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 07a589e47b04..86478bba78c4 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 8e112df60dce..83af790fcac7 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