From 54af6fa40ee3dbf21eb668ab33c5251f69de206b Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Tue, 1 Apr 2025 16:56:18 +0200 Subject: [PATCH 1/2] [Workflow] Add support for Backed Enum in `MethodMarkingStore` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Kind of relates to #44211 | License | MIT Supporting Enums in Workflow (and overall in Symfony) is a highly debated topic. While using Enums is not always a good choice, I personally found that Backed Enums are really suited for status and places because they ensure type safety in your model and naturally validate the set of allowed values. This is why I think supporting them in Workflow could be nice if not too complicated. While trying to implementing this in my current project using custom code, I figured we could go with a really narrow scope to support them out-of-the-box (but only for Single State Machine): in such case only `MethodMarkingStore` has to support Enums. In my mind, the way the workflow uses strings internally in the `Marking` is an implementation detail and what really matters to users is being able to control the values stored in their objects. Also, I don’t believe supporting enums for transitions is necessary as they are mostly configuration and validating transitions is already part of the component’s job (and in such case I would recommend using constants anyway). Finally, supporting Enum values in the configuration is not a target at the moment either because it can already be done as one can use `!php/enum` if they are using yaml files or use PHP config and directly use the enums. And improving this experience can be done later on if deemed necessary. Supporting this use case currently requires some boilerplate in the user project (with a dedicated getter for instance). Or completely reimplementing a marking store. Therefore I’m sharing what is in my mind a small patch that can greatly improve the situation for users as it allows them to use Backed Enums in their model right away with their getters (ideally we would not need this patch but as PHP will not allow Enums to implement `\Stringable` soon we need to do it ourself). The complicated part could be the setters but now that PHP supports union types we can only document how users can support them: ```php class MyObject { // ... private ObjectStatus $status = ObjectStatus::Draft; public function getStatus(): ObjectStatus { return $this->status; } public function setStatus(ObjectStatus|string $status): static { if (\is_string($status)) { $status = ObjectStatus::from($status); } $this->status = $status; return $this; } } ``` --- src/Symfony/Component/Workflow/CHANGELOG.md | 4 ++++ .../MarkingStore/MethodMarkingStore.php | 4 ++++ .../MarkingStore/MethodMarkingStoreTest.php | 19 +++++++++++++++++++ .../Component/Workflow/Tests/Subject.php | 7 +++++-- .../Component/Workflow/Tests/TestEnum.php | 18 ++++++++++++++++++ 5 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Workflow/Tests/TestEnum.php diff --git a/src/Symfony/Component/Workflow/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md index 2926da4e6428d..b96224946737c 100644 --- a/src/Symfony/Component/Workflow/CHANGELOG.md +++ b/src/Symfony/Component/Workflow/CHANGELOG.md @@ -1,6 +1,10 @@ CHANGELOG ========= +7.3 +--- +* Add support for Backed Enum in `MethodMarkingStore` + 7.1 --- diff --git a/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php b/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php index a2844b7b8ede7..5806c68298b84 100644 --- a/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php +++ b/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php @@ -64,6 +64,10 @@ public function getMarking(object $subject): Marking } if ($this->singleState) { + if ($marking instanceof \BackedEnum) { + $marking = $marking->value; + } + $marking = [(string) $marking => 1]; } elseif (!\is_array($marking)) { throw new LogicException(\sprintf('The marking stored in "%s::$%s" is not an array and the Workflow\'s Marking store is instantiated with $singleState=false.', get_debug_type($subject), $this->property)); diff --git a/src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php b/src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php index af0be682329be..54311e78621b2 100644 --- a/src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php +++ b/src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Workflow\MarkingStore\MethodMarkingStore; use Symfony\Component\Workflow\Tests\Subject; +use Symfony\Component\Workflow\Tests\TestEnum; class MethodMarkingStoreTest extends TestCase { @@ -84,6 +85,24 @@ public function testGetMarkingWithValueObject() $this->assertSame('first_place', (string) $subject->getMarking()); } + public function testGetMarkingWithBackedEnum() + { + $subject = new Subject(TestEnum::Foo); + + $markingStore = new MethodMarkingStore(true); + + $marking = $markingStore->getMarking($subject); + + $this->assertCount(1, $marking->getPlaces()); + $this->assertSame(['foo' => 1], $marking->getPlaces()); + + $marking->mark('bar'); + $marking->unmark('foo'); + $markingStore->setMarking($subject, $marking); + + $this->assertSame(TestEnum::Bar, $subject->getMarking()); + } + public function testGetMarkingWithUninitializedProperty() { $subject = new SubjectWithType(); diff --git a/src/Symfony/Component/Workflow/Tests/Subject.php b/src/Symfony/Component/Workflow/Tests/Subject.php index d68d430035f5d..82ea499a45164 100644 --- a/src/Symfony/Component/Workflow/Tests/Subject.php +++ b/src/Symfony/Component/Workflow/Tests/Subject.php @@ -13,7 +13,7 @@ final class Subject { - private string|array|null $marking; + private string|array|\BackedEnum|null $marking; private array $context = []; public function __construct($marking = null) @@ -21,13 +21,16 @@ public function __construct($marking = null) $this->marking = $marking; } - public function getMarking(): string|array|null + public function getMarking(): string|array|\BackedEnum|null { return $this->marking; } public function setMarking($marking, array $context = []): void { + if (\is_string($marking) && $newMarking = TestEnum::tryFrom($marking)) { + $marking = $newMarking; + } $this->marking = $marking; $this->context = $context; } diff --git a/src/Symfony/Component/Workflow/Tests/TestEnum.php b/src/Symfony/Component/Workflow/Tests/TestEnum.php new file mode 100644 index 0000000000000..e476f8b63b7ae --- /dev/null +++ b/src/Symfony/Component/Workflow/Tests/TestEnum.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Workflow\Tests; + +enum TestEnum: string +{ + case Foo = 'foo'; + case Bar = 'bar'; +} From a14d1860339df82e45dd8a01932ded5590ffc1fc Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Wed, 2 Apr 2025 09:52:43 +0200 Subject: [PATCH 2/2] Update src/Symfony/Component/Workflow/CHANGELOG.md Co-authored-by: Alexandre Daubois <2144837+alexandre-daubois@users.noreply.github.com> --- src/Symfony/Component/Workflow/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Workflow/CHANGELOG.md b/src/Symfony/Component/Workflow/CHANGELOG.md index b96224946737c..93bc0eaed9663 100644 --- a/src/Symfony/Component/Workflow/CHANGELOG.md +++ b/src/Symfony/Component/Workflow/CHANGELOG.md @@ -3,7 +3,8 @@ CHANGELOG 7.3 --- -* Add support for Backed Enum in `MethodMarkingStore` + + * Add support for Backed Enum in `MethodMarkingStore` 7.1 ---