Skip to content

Commit 1684c09

Browse files
feature symfony#60114 [Workflow] Add support for BackedEnum in MethodMarkingStore (tucksaun)
This PR was merged into the 7.4 branch. Discussion ---------- [Workflow] Add support for `BackedEnum` in `MethodMarkingStore` | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Kind of relates to symfony#44211 | License | MIT <details> <summary>Why supporting Backed Enums in Workflow can be useful</summary> 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. </details> <details> <summary>Reducing the supported scope to `MethodMarkingStore`</summary> While trying to implementing BackedEnum support 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). </details> Supporting BackedEnum currently requires some boilerplate in the user project (with dedicated getter and setter for instance) or a complete implementation of 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 method or properties: ```php class MyObject { // ... private ObjectStatus $status = ObjectStatus::Draft; public function getStatus(): ObjectStatus { return $this->status; } public function setStatus(ObjectStatus $status): static { $this->status = $status; return $this; } } ``` Commits ------- bde1977 [Workflow] Add support for `BackedEnum` in `MethodMarkingStore`
2 parents 67d67c1 + bde1977 commit 1684c09

File tree

5 files changed

+111
-21
lines changed

5 files changed

+111
-21
lines changed

src/Symfony/Component/Workflow/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
7.4
5+
---
6+
7+
* Add support for `BackedEnum` in `MethodMarkingStore`
8+
49
7.3
510
---
611

src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,21 @@ final class MethodMarkingStore implements MarkingStoreInterface
3737
private array $setters = [];
3838

3939
/**
40-
* @param string $property Used to determine methods or property to call
41-
* The `getMarking` method will use `$subject->getProperty()` or `$subject->property`
42-
* The `setMarking` method will use `$subject->setProperty(string|array $places, array $context = [])` or `$subject->property = string|array $places`
40+
* @param string $property Used to determine the accessor methods or the property to call
41+
* `getMarking()` will use `$subject->getProperty()` or `$subject->$property`
42+
* `setMarking()` will use `$subject->setProperty(string|array|\BackedEnum $places, array $context = [])` or `$subject->$property = string|array|\BackedEnum $places`
4343
*/
4444
public function __construct(
45-
private bool $singleState = false,
46-
private string $property = 'marking',
45+
private readonly bool $singleState = false,
46+
private readonly string $property = 'marking',
4747
) {
4848
}
4949

5050
public function getMarking(object $subject): Marking
5151
{
5252
$marking = null;
5353
try {
54-
$marking = ($this->getGetter($subject))();
54+
$marking = ($this->getters[$subject::class] ??= $this->getGetter($subject))();
5555
} catch (\Error $e) {
5656
$unInitializedPropertyMessage = \sprintf('Typed property %s::$%s must not be accessed before initialization', get_debug_type($subject), $this->property);
5757
if ($e->getMessage() !== $unInitializedPropertyMessage) {
@@ -64,6 +64,10 @@ public function getMarking(object $subject): Marking
6464
}
6565

6666
if ($this->singleState) {
67+
if ($marking instanceof \BackedEnum) {
68+
$marking = $marking->value;
69+
}
70+
6771
$marking = [(string) $marking => 1];
6872
} elseif (!\is_array($marking)) {
6973
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));
@@ -80,15 +84,15 @@ public function setMarking(object $subject, Marking $marking, array $context = [
8084
$marking = key($marking);
8185
}
8286

83-
($this->getSetter($subject))($marking, $context);
87+
($this->setters[$subject::class] ??= $this->getSetter($subject))($marking, $context);
8488
}
8589

8690
private function getGetter(object $subject): callable
8791
{
8892
$property = $this->property;
8993
$method = 'get'.ucfirst($property);
9094

91-
return match ($this->getters[$subject::class] ??= self::getType($subject, $property, $method)) {
95+
return match (self::getType($subject, $property, $method)) {
9296
MarkingStoreMethod::METHOD => $subject->{$method}(...),
9397
MarkingStoreMethod::PROPERTY => static fn () => $subject->{$property},
9498
};
@@ -99,26 +103,34 @@ private function getSetter(object $subject): callable
99103
$property = $this->property;
100104
$method = 'set'.ucfirst($property);
101105

102-
return match ($this->setters[$subject::class] ??= self::getType($subject, $property, $method)) {
103-
MarkingStoreMethod::METHOD => $subject->{$method}(...),
104-
MarkingStoreMethod::PROPERTY => static fn ($marking) => $subject->{$property} = $marking,
106+
return match (self::getType($subject, $property, $method, $type)) {
107+
MarkingStoreMethod::METHOD => $type ? static fn ($marking, $context) => $subject->{$method}($type::from($marking), $context) : $subject->{$method}(...),
108+
MarkingStoreMethod::PROPERTY => $type ? static fn ($marking) => $subject->{$property} = $type::from($marking) : static fn ($marking) => $subject->{$property} = $marking,
105109
};
106110
}
107111

108-
private static function getType(object $subject, string $property, string $method): MarkingStoreMethod
112+
private static function getType(object $subject, string $property, string $method, ?string &$type = null): MarkingStoreMethod
109113
{
110-
if (method_exists($subject, $method) && (new \ReflectionMethod($subject, $method))->isPublic()) {
111-
return MarkingStoreMethod::METHOD;
112-
}
113-
114114
try {
115-
if ((new \ReflectionProperty($subject, $property))->isPublic()) {
116-
return MarkingStoreMethod::PROPERTY;
115+
if (method_exists($subject, $method) && ($r = new \ReflectionMethod($subject, $method))->isPublic()) {
116+
$type = 0 < $r->getNumberOfRequiredParameters() ? $r->getParameters()[0]->getType() : null;
117+
118+
return MarkingStoreMethod::METHOD;
117119
}
118-
} catch (\ReflectionException) {
119-
}
120120

121-
throw new LogicException(\sprintf('Cannot store marking: class "%s" should have either a public method named "%s()" or a public property named "$%s"; none found.', get_debug_type($subject), $method, $property));
121+
try {
122+
if (($r = new \ReflectionProperty($subject, $property))->isPublic()) {
123+
$type = $r->getType();
124+
125+
return MarkingStoreMethod::PROPERTY;
126+
}
127+
} catch (\ReflectionException) {
128+
}
129+
130+
throw new LogicException(\sprintf('Cannot store marking: class "%s" should have either a public method named "%s()" or a public property named "$%s"; none found.', get_debug_type($subject), $method, $property));
131+
} finally {
132+
$type = $type instanceof \ReflectionNamedType && is_subclass_of($type->getName(), \BackedEnum::class) ? $type->getName() : null;
133+
}
122134
}
123135
}
124136

src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ public function testGetMarkingWithValueObject()
8484
$this->assertSame('first_place', (string) $subject->getMarking());
8585
}
8686

87+
public function testGetMarkingWithBackedEnum()
88+
{
89+
$subject = new SubjectWithEnum(TestEnum::Foo);
90+
91+
$markingStore = new MethodMarkingStore(true);
92+
93+
$marking = $markingStore->getMarking($subject);
94+
95+
$this->assertCount(1, $marking->getPlaces());
96+
$this->assertSame(['foo' => 1], $marking->getPlaces());
97+
98+
$marking->mark('bar');
99+
$marking->unmark('foo');
100+
$markingStore->setMarking($subject, $marking);
101+
102+
$this->assertSame(TestEnum::Bar, $subject->getMarking());
103+
}
104+
87105
public function testGetMarkingWithUninitializedProperty()
88106
{
89107
$subject = new SubjectWithType();
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Workflow\Tests\MarkingStore;
13+
14+
class SubjectWithEnum
15+
{
16+
public function __construct(
17+
private ?TestEnum $marking = null,
18+
private array $context = [],
19+
) {
20+
}
21+
22+
public function getMarking(): ?TestEnum
23+
{
24+
return $this->marking;
25+
}
26+
27+
public function setMarking(TestEnum $marking, array $context = []): void
28+
{
29+
$this->marking = $marking;
30+
$this->context = $context;
31+
}
32+
33+
public function getContext(): array
34+
{
35+
return $this->context;
36+
}
37+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Workflow\Tests\MarkingStore;
13+
14+
enum TestEnum: string
15+
{
16+
case Foo = 'foo';
17+
case Bar = 'bar';
18+
}

0 commit comments

Comments
 (0)