-

symfony · Jul 28, 2023 · cd15e71 · cd15e71
1 parent bd42133
commit cd15e71
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 53 deletions.
diff --git a/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php b/src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php
@@ -15,23 +15,25 @@
 use Symfony\Component\Workflow\Marking;
 
 /**
- * MethodMarkingStore stores the marking with a subject's public method if
- * exist, then to a property.
+ * MethodMarkingStore stores the marking with a subject's public method
+ * or public property.
  *
- * This store deals with a "single state" or "multiple state" Marking.
+ * This store deals with a "single state" or "multiple state" marking.
  *
- * "single state" Marking means a subject can be in one and only one state at
+ * "single state" marking means a subject can be in one and only one state at
  * the same time. Use it with state machine. It uses a string to store the
  * marking.
  *
- * "multiple state" Marking means a subject can be in many states at the same
+ * "multiple state" marking means a subject can be in many states at the same
  * time. Use it with workflow. It uses an array of strings to store the marking.
  *
  * @author Grégoire Pineau <lyrixx@lyrixx.info>
  */
 final class MethodMarkingStore implements MarkingStoreInterface
 {
+    /** @var array<class-string, MarkingStoreMethod> */
     private array $getters = [];
+    /** @var array<class-string, MarkingStoreMethod> */
     private array $setters = [];
 
     /**
@@ -64,7 +66,7 @@ public function getMarking(object $subject): Marking
         if ($this->singleState) {
             $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));
+            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));
         }
 
         return new Marking($marking);
@@ -83,62 +85,40 @@ public function setMarking(object $subject, Marking $marking, array $context = [
 
     private function getGetter(object $subject): callable
     {
-        $propertyName = $this->property;
-        $methodName = 'get'.ucfirst($propertyName);
+        $property = $this->property;
+        $method = 'get'.ucfirst($property);
 
-        return match ($this->getters[$subject::class] ??= $this->getType($subject, $propertyName, $methodName)) {
-            MarkingStoreMethod::METHOD => static fn () => $subject->{$methodName}(),
-            MarkingStoreMethod::PROPERTY => static fn () => $subject->{$propertyName},
+        return match ($this->getters[$subject::class] ??= $this->getType($subject, $property, $method)) {
+            MarkingStoreMethod::METHOD => $subject->{$method}(...),
+            MarkingStoreMethod::PROPERTY => static fn () => $subject->{$property},
         };
     }
 
     private function getSetter(object $subject): callable
     {
-        $propertyName = $this->property;
-        $methodName = 'set'.ucfirst($propertyName);
+        $property = $this->property;
+        $method = 'set'.ucfirst($property);
 
-        return match ($this->setters[$subject::class] ??= $this->getType($subject, $propertyName, $methodName)) {
-            MarkingStoreMethod::METHOD => static fn ($marking, $context) => $subject->{$methodName}($marking, $context),
-            MarkingStoreMethod::PROPERTY => static fn ($marking) => $subject->{$propertyName} = $marking,
+        return match ($this->setters[$subject::class] ??= $this->getType($subject, $property, $method)) {
+            MarkingStoreMethod::METHOD => $subject->{$method}(...),
+            MarkingStoreMethod::PROPERTY => static fn ($marking) => $subject->{$property} = $marking,
         };
     }
 
-    private function getType(object $subject, string $propertyName, string $method): MarkingStoreMethod
+    private static function getType(object $subject, string $property, string $method): MarkingStoreMethod
     {
-        if (!method_exists($subject, $method)) {
-            goto property;
+        if (method_exists($subject, $method) && (new \ReflectionMethod($subject, $method))->isPublic()) {
+            return MarkingStoreMethod::METHOD;
         }
 
         try {
-            $r = new \ReflectionMethod($subject, $method);
-        } catch (\ReflectionException) {
-            property:
-            try {
-                $r = new \ReflectionProperty($subject, $propertyName);
-            } catch (\ReflectionException) {
-                throw new LogicException(sprintf('The public property "%1$s::%2$s" nor the public method "%1$s::%3$s()" exist. At least one must be declared.', get_debug_type($subject), $propertyName, $method));
-            }
-
-            if (!$r->isPublic()) {
-                throw new LogicException(sprintf('The public method "%1$s::%3$s()" must be declared, or the property "%1$s::%2$s" must be public.', get_debug_type($subject), $propertyName, $method));
+            if ((new \ReflectionProperty($subject, $property))->isPublic()) {
+                return MarkingStoreMethod::PROPERTY;
             }
-
-            return MarkingStoreMethod::PROPERTY;
-        }
-
-        if (!$r->isPublic()) {
-            try {
-                $r = new \ReflectionProperty($subject, $propertyName);
-                if ($r->isPublic()) {
-                    return MarkingStoreMethod::PROPERTY;
-                }
-            } catch (\ReflectionException) {
-            }
-
-            throw new LogicException(sprintf('The method "%s::%s()" must be public.', get_debug_type($subject), $method));
+        } catch (\ReflectionException) {
         }
 
-        return MarkingStoreMethod::METHOD;
+        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));
     }
 }
 

diff --git a/src/Symfony/Component/Workflow/Tests/MarkingStore/PropertiesMarkingStoreTest.php b/src/Symfony/Component/Workflow/Tests/MarkingStore/PropertiesMarkingStoreTest.php
@@ -19,7 +19,6 @@ class PropertiesMarkingStoreTest extends TestCase
     public function testGetSetMarkingWithMultipleState()
     {
         $subject = new SubjectWithProperties();
-
         $markingStore = new MethodMarkingStore(false);
 
         $marking = $markingStore->getMarking($subject);
@@ -40,7 +39,6 @@ public function testGetSetMarkingWithMultipleState()
     public function testGetSetMarkingWithSingleState()
     {
         $subject = new SubjectWithProperties();
-
         $markingStore = new MethodMarkingStore(true, 'place', 'placeContext');
 
         $marking = $markingStore->getMarking($subject);
@@ -97,12 +95,9 @@ public function testGetMarkingWithUninitializedProperty()
     private function createValueObject(string $markingValue): object
     {
         return new class($markingValue) {
-            /** @var string */
-            private $markingValue;
-
-            public function __construct(string $markingValue)
-            {
-                $this->markingValue = $markingValue;
+            public function __construct(
+                private string $markingValue,
+            ) {
             }
 
             public function __toString(): string