From cfbe1392193796908fe3d8e8c76738bd0a28362e Mon Sep 17 00:00:00 2001 From: Yohan Giarelli Date: Thu, 20 Nov 2025 21:27:57 +0100 Subject: [PATCH 1/4] perf: Small optimization on PropertyAccessor initialization --- .../Bundle/DependencyInjection/FiniteExtension.php | 1 + src/StateMachine.php | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php index a5917d8..46a39c0 100644 --- a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php +++ b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php @@ -20,6 +20,7 @@ public function load(array $configs, ContainerBuilder $container): void [ StateMachine::class => (new Definition(StateMachine::class)) ->setArgument('$dispatcher', new Reference('event_dispatcher')) + ->setArgument('$propertyAccessor', new Reference('property_accessor')) ->setPublic(true), TwigExtension::class => (new Definition(TwigExtension::class)) ->setArgument('$stateMachine', new Reference(StateMachine::class)) diff --git a/src/StateMachine.php b/src/StateMachine.php index e3ecf4c..6674df1 100644 --- a/src/StateMachine.php +++ b/src/StateMachine.php @@ -14,7 +14,8 @@ use Finite\Exception\TransitionNotReachableException; use Finite\Transition\TransitionInterface; use Psr\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Component\PropertyAccess\PropertyAccessor; +use Symfony\Component\PropertyAccess\PropertyAccessorInterface; /** * @api @@ -23,6 +24,7 @@ class StateMachine { public function __construct( private readonly EventDispatcherInterface $dispatcher = new EventDispatcher(), + private readonly ?PropertyAccessorInterface $propertyAccessor = new PropertyAccessor(), ) { } @@ -47,7 +49,7 @@ public function apply(object $object, string $transitionName, ?string $stateClas $this->dispatcher->dispatch(new PreTransitionEvent($object, $transition, $fromState)); $transition->process($object); - PropertyAccess::createPropertyAccessor()->setValue( + $this->propertyAccessor->setValue( $object, $property->getName(), $transition->getTargetState(), @@ -129,7 +131,7 @@ private function extractState(object $object, ?string $stateClass = null): State $property = $this->extractStateProperty($object, $stateClass); /** @psalm-suppress MixedReturnStatement */ - return PropertyAccess::createPropertyAccessor()->getValue($object, $property->getName()); + return $this->propertyAccessor->getValue($object, $property->getName()); } /** From b9f82d9c275a1072dc4d028c12d81d2ad40f0126 Mon Sep 17 00:00:00 2001 From: Yohan Giarelli Date: Thu, 20 Nov 2025 21:37:01 +0100 Subject: [PATCH 2/4] perf: Removed Symfony PropertyAccessor --- composer.json | 1 - .../DependencyInjection/FiniteExtension.php | 1 - src/StateMachine.php | 16 +++++----------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/composer.json b/composer.json index 993fd6f..f04a0a5 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,6 @@ ], "require": { "php": ">=8.1", - "symfony/property-access": ">=5.4,<8", "psr/event-dispatcher": "^1.0" }, "require-dev": { diff --git a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php index 46a39c0..a5917d8 100644 --- a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php +++ b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php @@ -20,7 +20,6 @@ public function load(array $configs, ContainerBuilder $container): void [ StateMachine::class => (new Definition(StateMachine::class)) ->setArgument('$dispatcher', new Reference('event_dispatcher')) - ->setArgument('$propertyAccessor', new Reference('property_accessor')) ->setPublic(true), TwigExtension::class => (new Definition(TwigExtension::class)) ->setArgument('$stateMachine', new Reference(StateMachine::class)) diff --git a/src/StateMachine.php b/src/StateMachine.php index 6674df1..c92a413 100644 --- a/src/StateMachine.php +++ b/src/StateMachine.php @@ -14,8 +14,6 @@ use Finite\Exception\TransitionNotReachableException; use Finite\Transition\TransitionInterface; use Psr\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\PropertyAccess\PropertyAccessor; -use Symfony\Component\PropertyAccess\PropertyAccessorInterface; /** * @api @@ -24,7 +22,6 @@ class StateMachine { public function __construct( private readonly EventDispatcherInterface $dispatcher = new EventDispatcher(), - private readonly ?PropertyAccessorInterface $propertyAccessor = new PropertyAccessor(), ) { } @@ -49,13 +46,8 @@ public function apply(object $object, string $transitionName, ?string $stateClas $this->dispatcher->dispatch(new PreTransitionEvent($object, $transition, $fromState)); $transition->process($object); - $this->propertyAccessor->setValue( - $object, - $property->getName(), - $transition->getTargetState(), - ); + $property->setValue($object, $transition->getTargetState()); - /** @var object $object */ $this->dispatcher->dispatch(new PostTransitionEvent($object, $transition, $fromState)); } @@ -130,8 +122,10 @@ private function extractState(object $object, ?string $stateClass = null): State { $property = $this->extractStateProperty($object, $stateClass); - /** @psalm-suppress MixedReturnStatement */ - return $this->propertyAccessor->getValue($object, $property->getName()); + /** @var State&\BackedEnum $value */ + $value = $property->getValue($object); + + return $value; } /** From 7ed36f0244f47ef4a727ec297a9b7d26c74fcc00 Mon Sep 17 00:00:00 2001 From: Yohan Giarelli Date: Thu, 20 Nov 2025 22:08:16 +0100 Subject: [PATCH 3/4] perf: Added memoized state extractor --- .../DependencyInjection/FiniteExtension.php | 6 ++ .../MemoizedStatePropertyExtractor.php | 29 +++++++ .../ReflectionStatePropertyExtractor.php | 50 +++++++++++ src/Extractor/StatePropertyExtractor.php | 14 +++ src/Extractor/StatePropertyExtractorTrait.php | 47 ++++++++++ src/StateMachine.php | 87 ++----------------- .../FiniteExtensionTest.php | 4 +- .../MemoizedStatePropertyExtractorTest.php | 26 ++++++ .../ReflectionStatePropertyExtractorTest.php | 48 ++++++++++ tests/StateMachineTest.php | 1 + 10 files changed, 231 insertions(+), 81 deletions(-) create mode 100644 src/Extractor/MemoizedStatePropertyExtractor.php create mode 100644 src/Extractor/ReflectionStatePropertyExtractor.php create mode 100644 src/Extractor/StatePropertyExtractor.php create mode 100644 src/Extractor/StatePropertyExtractorTrait.php create mode 100644 tests/Extractor/MemoizedStatePropertyExtractorTest.php create mode 100644 tests/Extractor/ReflectionStatePropertyExtractorTest.php diff --git a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php index a5917d8..3d61cdc 100644 --- a/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php +++ b/src/Extension/Symfony/Bundle/DependencyInjection/FiniteExtension.php @@ -5,6 +5,9 @@ namespace Finite\Extension\Symfony\Bundle\DependencyInjection; use Finite\Extension\Twig\FiniteExtension as TwigExtension; +use Finite\Extractor\MemoizedStatePropertyExtractor; +use Finite\Extractor\ReflectionStatePropertyExtractor; +use Finite\Extractor\StatePropertyExtractor; use Finite\StateMachine; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -18,8 +21,11 @@ public function load(array $configs, ContainerBuilder $container): void { $container->addDefinitions( [ + StatePropertyExtractor::class => (new Definition(MemoizedStatePropertyExtractor::class)) + ->setArgument('$decorated', new Definition(ReflectionStatePropertyExtractor::class)), StateMachine::class => (new Definition(StateMachine::class)) ->setArgument('$dispatcher', new Reference('event_dispatcher')) + ->setArgument('$statePropertyExtractor', new Reference(StatePropertyExtractor::class)) ->setPublic(true), TwigExtension::class => (new Definition(TwigExtension::class)) ->setArgument('$stateMachine', new Reference(StateMachine::class)) diff --git a/src/Extractor/MemoizedStatePropertyExtractor.php b/src/Extractor/MemoizedStatePropertyExtractor.php new file mode 100644 index 0000000..32ddb5a --- /dev/null +++ b/src/Extractor/MemoizedStatePropertyExtractor.php @@ -0,0 +1,29 @@ + + */ + private array $cache = []; + + public function __construct( + private readonly StatePropertyExtractor $decorated = new ReflectionStatePropertyExtractor(), + ) + { + } + + #[\Override] + public function extractAll(object $object): array + { + if (isset($this->cache[get_class($object)])) { + return $this->cache[get_class($object)]; + } + + return $this->cache[get_class($object)] = $this->decorated->extractAll($object); + } +} diff --git a/src/Extractor/ReflectionStatePropertyExtractor.php b/src/Extractor/ReflectionStatePropertyExtractor.php new file mode 100644 index 0000000..a710d0a --- /dev/null +++ b/src/Extractor/ReflectionStatePropertyExtractor.php @@ -0,0 +1,50 @@ +getProperties() as $property) { + /** @var \ReflectionUnionType|\ReflectionIntersectionType|\ReflectionNamedType $reflectionType */ + $reflectionType = $property->getType(); + if (null === $reflectionType) { + continue; + } + + if ($reflectionType instanceof \ReflectionUnionType) { + continue; + } + + if ($reflectionType instanceof \ReflectionIntersectionType) { + continue; + } + + /** @var class-string $name */ + $name = $reflectionType->getName(); + if (!enum_exists($name)) { + continue; + } + + $reflectionEnum = new \ReflectionEnum($name); + /** @psalm-suppress TypeDoesNotContainType */ + if ($reflectionEnum->implementsInterface(State::class)) { + $properties[] = $property; + } + } + } while ($reflectionClass = $reflectionClass->getParentClass()); + + return $properties; + } +} diff --git a/src/Extractor/StatePropertyExtractor.php b/src/Extractor/StatePropertyExtractor.php new file mode 100644 index 0000000..f72e1ec --- /dev/null +++ b/src/Extractor/StatePropertyExtractor.php @@ -0,0 +1,14 @@ + + */ + public function extractAll(object $object): array; +} diff --git a/src/Extractor/StatePropertyExtractorTrait.php b/src/Extractor/StatePropertyExtractorTrait.php new file mode 100644 index 0000000..4c9e769 --- /dev/null +++ b/src/Extractor/StatePropertyExtractorTrait.php @@ -0,0 +1,47 @@ +extractAll($object); + if (null !== $stateClass) { + foreach ($properties as $property) { + if ((string) $property->getType() === $stateClass) { + return $property; + } + } + + throw new BadStateClassException(\sprintf('Found no state on object "%s" with class "%s"', $object::class, $stateClass)); + } + + if (1 === \count($properties)) { + return $properties[0]; + } + + if (\count($properties) > 1) { + throw new NonUniqueStateException('Found multiple states on object '.$object::class); + } + + throw new NoStateFoundException('Found no state on object '.$object::class); + } + + /** + * @return array + */ + abstract public function extractAll(object $object): array; +} diff --git a/src/StateMachine.php b/src/StateMachine.php index c92a413..958d901 100644 --- a/src/StateMachine.php +++ b/src/StateMachine.php @@ -8,10 +8,9 @@ use Finite\Event\EventDispatcher; use Finite\Event\PostTransitionEvent; use Finite\Event\PreTransitionEvent; -use Finite\Exception\BadStateClassException; -use Finite\Exception\NonUniqueStateException; -use Finite\Exception\NoStateFoundException; use Finite\Exception\TransitionNotReachableException; +use Finite\Extractor\MemoizedStatePropertyExtractor; +use Finite\Extractor\StatePropertyExtractor; use Finite\Transition\TransitionInterface; use Psr\EventDispatcher\EventDispatcherInterface; @@ -22,6 +21,7 @@ class StateMachine { public function __construct( private readonly EventDispatcherInterface $dispatcher = new EventDispatcher(), + private readonly StatePropertyExtractor $statePropertyExtractor = new MemoizedStatePropertyExtractor(), ) { } @@ -34,7 +34,7 @@ public function apply(object $object, string $transitionName, ?string $stateClas throw new TransitionNotReachableException('Unable to apply transition '.$transitionName); } - $property = $this->extractStateProperty($object, $stateClass); + $property = $this->statePropertyExtractor->extract($object, $stateClass); $fromState = $this->extractState($object, $stateClass); $transition = array_values( array_filter( @@ -99,7 +99,7 @@ public function getStateClasses(object $object): array return array_filter( array_map( fn (\ReflectionProperty $property): string => (string) $property->getType(), - $this->extractStateProperties($object), + $this->statePropertyExtractor->extractAll($object), ), fn (?string $name): bool => enum_exists($name), ); @@ -107,7 +107,7 @@ public function getStateClasses(object $object): array public function hasState(object $object): bool { - return \count($this->extractStateProperties($object)) > 0; + return \count($this->statePropertyExtractor->extractAll($object)) > 0; } public function getDispatcher(): EventDispatcherInterface @@ -120,84 +120,11 @@ public function getDispatcher(): EventDispatcherInterface */ private function extractState(object $object, ?string $stateClass = null): State&\BackedEnum { - $property = $this->extractStateProperty($object, $stateClass); + $property = $this->statePropertyExtractor->extract($object, $stateClass); /** @var State&\BackedEnum $value */ $value = $property->getValue($object); return $value; } - - /** - * @param class-string|null $stateClass - */ - private function extractStateProperty(object $object, ?string $stateClass = null): \ReflectionProperty - { - if ($stateClass && !enum_exists($stateClass)) { - throw new NoStateFoundException(\sprintf('Enum "%s" does not exists', $stateClass)); - } - - $properties = $this->extractStateProperties($object); - if (null !== $stateClass) { - foreach ($properties as $property) { - if ((string) $property->getType() === $stateClass) { - return $property; - } - } - - throw new BadStateClassException(\sprintf('Found no state on object "%s" with class "%s"', $object::class, $stateClass)); - } - - if (1 === \count($properties)) { - return $properties[0]; - } - - if (\count($properties) > 1) { - throw new NonUniqueStateException('Found multiple states on object '.$object::class); - } - - throw new NoStateFoundException('Found no state on object '.$object::class); - } - - /** - * @return array - */ - private function extractStateProperties(object $object): array - { - $properties = []; - - $reflectionClass = new \ReflectionClass($object); - /** @psalm-suppress DocblockTypeContradiction */ - do { - foreach ($reflectionClass->getProperties() as $property) { - /** @var \ReflectionUnionType|\ReflectionIntersectionType|\ReflectionNamedType $reflectionType */ - $reflectionType = $property->getType(); - if (null === $reflectionType) { - continue; - } - - if ($reflectionType instanceof \ReflectionUnionType) { - continue; - } - - if ($reflectionType instanceof \ReflectionIntersectionType) { - continue; - } - - /** @var class-string $name */ - $name = $reflectionType->getName(); - if (!enum_exists($name)) { - continue; - } - - $reflectionEnum = new \ReflectionEnum($name); - /** @psalm-suppress TypeDoesNotContainType */ - if ($reflectionEnum->implementsInterface(State::class)) { - $properties[] = $property; - } - } - } while ($reflectionClass = $reflectionClass->getParentClass()); - - return $properties; - } } diff --git a/tests/Extension/Symfony/Bundle/DependencyInjection/FiniteExtensionTest.php b/tests/Extension/Symfony/Bundle/DependencyInjection/FiniteExtensionTest.php index a455bed..df3a97a 100644 --- a/tests/Extension/Symfony/Bundle/DependencyInjection/FiniteExtensionTest.php +++ b/tests/Extension/Symfony/Bundle/DependencyInjection/FiniteExtensionTest.php @@ -6,6 +6,7 @@ use Finite\Extension\Symfony\Bundle\DependencyInjection\FiniteExtension; use Finite\Extension\Twig\FiniteExtension as TwigExtension; +use Finite\Extractor\StatePropertyExtractor; use Finite\StateMachine; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -20,7 +21,8 @@ public function testItLoadsServices(): void $container->expects($this->once())->method('addDefinitions')->with( $this->logicalAnd( - $this->countOf(2), + $this->countOf(3), + $this->arrayHasKey(StatePropertyExtractor::class), $this->arrayHasKey(StateMachine::class), $this->arrayHasKey(TwigExtension::class), ), diff --git a/tests/Extractor/MemoizedStatePropertyExtractorTest.php b/tests/Extractor/MemoizedStatePropertyExtractorTest.php new file mode 100644 index 0000000..289c767 --- /dev/null +++ b/tests/Extractor/MemoizedStatePropertyExtractorTest.php @@ -0,0 +1,26 @@ +createMock(StatePropertyExtractor::class); + $extractor = new MemoizedStatePropertyExtractor($decorated); + + $decorated + ->expects($this->once()) + ->method('extractAll') + ->with($this->isInstanceOf(\stdClass::class)) + ->willReturn([$this->createMock(\ReflectionProperty::class)]); + + $object = new \stdClass(); + $extractor->extractAll($object); + $extractor->extractAll($object); + } +} diff --git a/tests/Extractor/ReflectionStatePropertyExtractorTest.php b/tests/Extractor/ReflectionStatePropertyExtractorTest.php new file mode 100644 index 0000000..44f8aa8 --- /dev/null +++ b/tests/Extractor/ReflectionStatePropertyExtractorTest.php @@ -0,0 +1,48 @@ +object = new ReflectionStatePropertyExtractor(); + } + + public function testItExtractSimpleState(): void + { + $property = $this->object->extract(new Article('test')); + + $this->assertSame('state', $property->getName()); + $this->assertSame(SimpleArticleState::class, $property->getType()->getName()); + } + + public function testItThrowsOnAlternativeStateWithoutDetails(): void + { + $this->expectException(NonUniqueStateException::class); + $property = $this->object->extract(new AlternativeArticle('test')); + } + + public function testItExtractAlternativeState(): void + { + $property = $this->object->extract(new AlternativeArticle('test'), AlternativeArticleState::class); + + $this->assertSame('alternativeState', $property->getName()); + $this->assertSame(AlternativeArticleState::class, $property->getType()->getName()); + + $property = $this->object->extract(new AlternativeArticle('test'), SimpleArticleState::class); + + $this->assertSame('state', $property->getName()); + $this->assertSame(SimpleArticleState::class, $property->getType()->getName()); + } +} diff --git a/tests/StateMachineTest.php b/tests/StateMachineTest.php index aa40df5..2aa03b4 100644 --- a/tests/StateMachineTest.php +++ b/tests/StateMachineTest.php @@ -46,6 +46,7 @@ public function testItCanTransition(): void $this->callback(function (CanTransitionEvent $e) use ($object) { $this->assertSame($object, $e->getObject()); $this->assertFalse($e->isPropagationStopped()); + $this->assertSame(SimpleArticleState::DRAFT, $e->getFromState()); return SimpleArticleState::class === $e->getStateClass(); }), From 74b3ef644ec7cf151c07977c868224c56d3030ea Mon Sep 17 00:00:00 2001 From: Yohan Giarelli Date: Thu, 20 Nov 2025 22:18:17 +0100 Subject: [PATCH 4/4] fix: Fixed CS and SA --- src/Extractor/MemoizedStatePropertyExtractor.php | 16 ++++++++++------ .../ReflectionStatePropertyExtractor.php | 2 ++ src/Extractor/StatePropertyExtractor.php | 6 +++++- src/Extractor/StatePropertyExtractorTrait.php | 2 ++ .../MemoizedStatePropertyExtractorTest.php | 2 ++ .../ReflectionStatePropertyExtractorTest.php | 4 +++- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Extractor/MemoizedStatePropertyExtractor.php b/src/Extractor/MemoizedStatePropertyExtractor.php index 32ddb5a..dd2ecae 100644 --- a/src/Extractor/MemoizedStatePropertyExtractor.php +++ b/src/Extractor/MemoizedStatePropertyExtractor.php @@ -1,5 +1,7 @@ + * @var array > */ private array $cache = []; public function __construct( private readonly StatePropertyExtractor $decorated = new ReflectionStatePropertyExtractor(), - ) - { + ) { } #[\Override] public function extractAll(object $object): array { - if (isset($this->cache[get_class($object)])) { - return $this->cache[get_class($object)]; + if (isset($this->cache[$object::class])) { + /** @var array $value */ + $value = $this->cache[$object::class]; + + return $value; } - return $this->cache[get_class($object)] = $this->decorated->extractAll($object); + return $this->cache[$object::class] = $this->decorated->extractAll($object); } } diff --git a/src/Extractor/ReflectionStatePropertyExtractor.php b/src/Extractor/ReflectionStatePropertyExtractor.php index a710d0a..b8379d1 100644 --- a/src/Extractor/ReflectionStatePropertyExtractor.php +++ b/src/Extractor/ReflectionStatePropertyExtractor.php @@ -1,5 +1,7 @@ */ diff --git a/src/Extractor/StatePropertyExtractorTrait.php b/src/Extractor/StatePropertyExtractorTrait.php index 4c9e769..3e81e16 100644 --- a/src/Extractor/StatePropertyExtractorTrait.php +++ b/src/Extractor/StatePropertyExtractorTrait.php @@ -1,5 +1,7 @@ object = new ReflectionStatePropertyExtractor(); }