From d50623b175aadca50c49fef3e4831727c5ed7ab0 Mon Sep 17 00:00:00 2001 From: Ostap Brehin Date: Sat, 5 Sep 2026 12:09:31 +0100 Subject: [PATCH 1/5] perf(mapper): cache per-class reflection plans during hydration --- .../src/Mappers/ArrayToObjectMapper.php | 66 ++++++++++++------- packages/reflection/src/ClassReflector.php | 4 +- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/packages/mapper/src/Mappers/ArrayToObjectMapper.php b/packages/mapper/src/Mappers/ArrayToObjectMapper.php index 278a8f04f..53cbb58fa 100644 --- a/packages/mapper/src/Mappers/ArrayToObjectMapper.php +++ b/packages/mapper/src/Mappers/ArrayToObjectMapper.php @@ -135,45 +135,67 @@ private function resolveObject(mixed $objectOrClass): object private function setParentRelations(object $parent, ClassReflector $parentClass): void { - foreach ($parentClass->getPublicProperties() as $property) { - if (! $property->isInitialized($parent)) { - continue; - } + static $plans = []; - if ($property->isVirtual()) { - continue; - } + $plan = $plans[$parentClass->getName()] ??= array_filter(array_map( + function (PropertyReflector $property): ?array { + if ($property->isVirtual()) { + return null; + } + + $type = $property->getIterableType() ?? $property->getType(); + + if (! $type->isClass()) { + return null; + } - $type = $property->getIterableType() ?? $property->getType(); + return [$property, $type->asClass()]; + }, + $parentClass->getPublicProperties(), + )); - if (! $type->isClass()) { + foreach ($plan as [$property, $childClass]) { + if (! $property->isInitialized($parent)) { continue; } $child = $property->getValue($parent); - if ($child === null) { + if ($child === null || $child === []) { continue; } - $this->setChildParentRelation($parent, $child, $type->asClass()); + $this->setChildParentRelation($parent, $child, $childClass); } } private function setChildParentRelation(object $parent, mixed $child, ClassReflector $childClass): void { - foreach ($childClass->getPublicProperties() as $childProperty) { - if ($childProperty->isVirtual()) { - continue; - } + static $plans = []; - if ($childProperty->getType()->equals($parent::class)) { - $valueToSet = $parent; - } elseif ($childProperty->getIterableType()?->equals($parent::class)) { - $valueToSet = [$parent]; - } else { - continue; - } + $key = $childClass->getName() . '|' . $parent::class; + + $plan = $plans[$key] ??= array_filter(array_map( + function (PropertyReflector $childProperty) use ($parent): ?array { + if ($childProperty->isVirtual()) { + return null; + } + + if ($childProperty->getType()->equals($parent::class)) { + return [$childProperty, false]; + } + + if ($childProperty->getIterableType()?->equals($parent::class)) { + return [$childProperty, true]; + } + + return null; + }, + $childClass->getPublicProperties(), + )); + + foreach ($plan as [$childProperty, $wrapInArray]) { + $valueToSet = $wrapInArray ? [$parent] : $parent; if (is_array($child)) { foreach ($child as $childItem) { diff --git a/packages/reflection/src/ClassReflector.php b/packages/reflection/src/ClassReflector.php index 5c0797719..25538e852 100644 --- a/packages/reflection/src/ClassReflector.php +++ b/packages/reflection/src/ClassReflector.php @@ -65,7 +65,9 @@ public function getInterfaces(): array /** @return PropertyReflector[] */ public function getPublicProperties(): array { - return array_map( + static $cache = []; + + return $cache[$this->reflectionClass->getName()] ??= array_map( fn (PHPReflectionProperty $property) => new PropertyReflector($property), $this->reflectionClass->getProperties(PHPReflectionProperty::IS_PUBLIC), ); From 620cbf50d467660aa0b42bbb57d5a9cfb12a83ca Mon Sep 17 00:00:00 2001 From: Ostap Brehin Date: Sat, 5 Sep 2026 12:11:27 +0100 Subject: [PATCH 2/5] perf(database): memoize the resolved primary key property --- packages/database/src/Builder/ModelInspector.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/database/src/Builder/ModelInspector.php b/packages/database/src/Builder/ModelInspector.php index c9a09ac53..e8b04f986 100644 --- a/packages/database/src/Builder/ModelInspector.php +++ b/packages/database/src/Builder/ModelInspector.php @@ -740,6 +740,11 @@ public function hasPrimaryKey(): bool } public function getPrimaryKeyProperty(): ?PropertyReflector + { + return $this->memoize('primary_key_property', fn () => $this->resolvePrimaryKeyProperty()); + } + + private function resolvePrimaryKeyProperty(): ?PropertyReflector { if (! $this->isObjectModel()) { return null; From 1220703b6e2899183d8a6af09e7b3381d4ccffc9 Mon Sep 17 00:00:00 2001 From: Ostap Brehin Date: Sat, 5 Sep 2026 12:22:55 +0100 Subject: [PATCH 3/5] perf(mapper): cache resolved mappers per container and context --- packages/auth/tests/OAuthTest.php | 3 +- packages/mapper/src/MapperCache.php | 35 ++++++++ packages/mapper/src/ObjectFactory.php | 36 ++++---- tests/Integration/Mapper/MapperCacheTest.php | 91 ++++++++++++++++++++ 4 files changed, 146 insertions(+), 19 deletions(-) create mode 100644 packages/mapper/src/MapperCache.php create mode 100644 tests/Integration/Mapper/MapperCacheTest.php diff --git a/packages/auth/tests/OAuthTest.php b/packages/auth/tests/OAuthTest.php index ba8762e07..27e76cb1c 100644 --- a/packages/auth/tests/OAuthTest.php +++ b/packages/auth/tests/OAuthTest.php @@ -26,6 +26,7 @@ use Tempest\Auth\OAuth\OAuthUser; use Tempest\Container\Container; use Tempest\Container\GenericContainer; +use Tempest\Mapper\MapperCache; use Tempest\Mapper\MapperConfig; use Tempest\Mapper\Mappers\ArrayToObjectMapper; use Tempest\Mapper\ObjectFactory; @@ -37,7 +38,7 @@ final class OAuthTest extends TestCase } private ObjectFactory $factory { - get => $this->factory ??= new ObjectFactory(new MapperConfig([ArrayToObjectMapper::class]), $this->container); + get => $this->factory ??= new ObjectFactory(new MapperConfig([ArrayToObjectMapper::class]), $this->container, new MapperCache()); } #[Before] diff --git a/packages/mapper/src/MapperCache.php b/packages/mapper/src/MapperCache.php new file mode 100644 index 000000000..2576186e5 --- /dev/null +++ b/packages/mapper/src/MapperCache.php @@ -0,0 +1,35 @@ + */ + private array $mappers = []; + + /** + * @param callable(): \Tempest\Mapper\Mapper[] $resolve + * @return \Tempest\Mapper\Mapper[] + */ + public function resolve(Context $context, callable $resolve): array + { + return $this->mappers[$context->name] ??= $resolve(); + } + + public function reset(): void + { + $this->mappers = []; + } +} diff --git a/packages/mapper/src/ObjectFactory.php b/packages/mapper/src/ObjectFactory.php index b0f71f5d6..9d2a7d501 100644 --- a/packages/mapper/src/ObjectFactory.php +++ b/packages/mapper/src/ObjectFactory.php @@ -33,14 +33,15 @@ final class ObjectFactory private Context|UnitEnum|string|null $context = null; /** @var \Tempest\Mapper\Mapper[] */ - private array $mappers; + private array $mappers { + get => $this->resolveMappers(); + } public function __construct( private readonly MapperConfig $config, private readonly Container $container, - ) { - $this->mappers = $this->resolveMappers(); - } + private readonly MapperCache $cache, + ) {} /** * Sets the target class for mapping operations. @@ -112,13 +113,9 @@ public function collection(): self */ public function in(Context|UnitEnum|string|null $context): self { - $clone = clone($this, [ + return clone($this, [ 'context' => $context, ]); - - $clone->mappers = $clone->resolveMappers(); - - return $clone; } /** @@ -369,20 +366,23 @@ private function mapWith(mixed $mapper, mixed $from, mixed $to): mixed } /** - * We cache mapper instances within the factory so that we prevent mappers being resolved on every mapping call. - * Whenever a mapping context changes, we'll have to re-resolve the mapper classes with the new context. + * Mapper instances are cached per context in {@see \Tempest\Mapper\MapperCache}, so that they are not resolved + * from the container on every mapping call. Whenever a mapping context changes, we'll have to re-resolve the + * mapper classes with the new context. */ private function resolveMappers(): array { - /** @var Mapper[] $mappers */ - $mappers = []; - $context = MappingContext::from($this->context); - foreach ($this->config->mappers as $mapperClass) { - $mappers[] = $this->container->get($mapperClass, context: $context); - } + return $this->cache->resolve($context, function () use ($context): array { + /** @var Mapper[] $mappers */ + $mappers = []; + + foreach ($this->config->mappers as $mapperClass) { + $mappers[] = $this->container->get($mapperClass, context: $context); + } - return $mappers; + return $mappers; + }); } } diff --git a/tests/Integration/Mapper/MapperCacheTest.php b/tests/Integration/Mapper/MapperCacheTest.php new file mode 100644 index 000000000..7d19a7fda --- /dev/null +++ b/tests/Integration/Mapper/MapperCacheTest.php @@ -0,0 +1,91 @@ +container->get(ArrayToObjectMapper::class, context: MappingContext::default())]; + }; + + $first = $cache->resolve(new MappingContext('default'), $resolve); + $second = $cache->resolve(new MappingContext('default'), $resolve); + + $this->assertSame(1, $resolved); + $this->assertSame($first, $second); + + $cache->resolve(new MappingContext('other'), $resolve); + + $this->assertSame(2, $resolved); + } + + #[Test] + public function is_reset_between_worker_requests(): void + { + $cache = $this->container->get(MapperCache::class); + $resolved = 0; + + $resolve = function () use (&$resolved): array { + $resolved++; + + return [$this->container->get(ArrayToObjectMapper::class, context: MappingContext::default())]; + }; + + $cache->resolve(new MappingContext('default'), $resolve); + $cache->resolve(new MappingContext('default'), $resolve); + + $this->assertSame(1, $resolved); + + $this->container->reset(); + + // A reset clears the cache without dropping the singleton. The mappers must therefore be + // re-resolved through the same instance. + $this->assertSame($cache, $this->container->get(MapperCache::class)); + + $cache->resolve(new MappingContext('default'), $resolve); + + $this->assertSame(2, $resolved); + } + + #[Test] + public function mappers_are_not_shared_between_containers(): void + { + $config = new MapperConfig([ArrayToObjectMapper::class]); + + $factory = fn (GenericContainer $container) => new ObjectFactory($config, $container, new MapperCache()); + + $mappersOf = function (ObjectFactory $factory): array { + $reflection = new ReflectionProperty(ObjectFactory::class, 'mappers'); + + return $reflection->getValue($factory); + }; + + $a = $mappersOf($factory(new GenericContainer())); + $b = $mappersOf($factory(new GenericContainer())); + + $this->assertNotSame($a[0], $b[0]); + } +} From e223c43d29dd7bf1edb819b17f5777f71b58437b Mon Sep 17 00:00:00 2001 From: Ostap Brehin Date: Sat, 5 Sep 2026 21:56:57 +0100 Subject: [PATCH 4/5] chore: mago formatting --- packages/database/src/Builder/ModelInspector.php | 2 +- packages/mapper/src/Mappers/ArrayToObjectMapper.php | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/database/src/Builder/ModelInspector.php b/packages/database/src/Builder/ModelInspector.php index e8b04f986..c3f8e8690 100644 --- a/packages/database/src/Builder/ModelInspector.php +++ b/packages/database/src/Builder/ModelInspector.php @@ -741,7 +741,7 @@ public function hasPrimaryKey(): bool public function getPrimaryKeyProperty(): ?PropertyReflector { - return $this->memoize('primary_key_property', fn () => $this->resolvePrimaryKeyProperty()); + return $this->memoize('primary_key_property', $this->resolvePrimaryKeyProperty(...)); } private function resolvePrimaryKeyProperty(): ?PropertyReflector diff --git a/packages/mapper/src/Mappers/ArrayToObjectMapper.php b/packages/mapper/src/Mappers/ArrayToObjectMapper.php index 53cbb58fa..64e44012f 100644 --- a/packages/mapper/src/Mappers/ArrayToObjectMapper.php +++ b/packages/mapper/src/Mappers/ArrayToObjectMapper.php @@ -137,7 +137,9 @@ private function setParentRelations(object $parent, ClassReflector $parentClass) { static $plans = []; - $plan = $plans[$parentClass->getName()] ??= array_filter(array_map( + $key = $parentClass->getName(); + + $plans[$key] ??= array_filter(array_map( function (PropertyReflector $property): ?array { if ($property->isVirtual()) { return null; @@ -154,7 +156,7 @@ function (PropertyReflector $property): ?array { $parentClass->getPublicProperties(), )); - foreach ($plan as [$property, $childClass]) { + foreach ($plans[$key] as [$property, $childClass]) { if (! $property->isInitialized($parent)) { continue; } @@ -175,7 +177,7 @@ private function setChildParentRelation(object $parent, mixed $child, ClassRefle $key = $childClass->getName() . '|' . $parent::class; - $plan = $plans[$key] ??= array_filter(array_map( + $plans[$key] ??= array_filter(array_map( function (PropertyReflector $childProperty) use ($parent): ?array { if ($childProperty->isVirtual()) { return null; @@ -194,7 +196,7 @@ function (PropertyReflector $childProperty) use ($parent): ?array { $childClass->getPublicProperties(), )); - foreach ($plan as [$childProperty, $wrapInArray]) { + foreach ($plans[$key] as [$childProperty, $wrapInArray]) { $valueToSet = $wrapInArray ? [$parent] : $parent; if (is_array($child)) { From 981b7a0137dcebcf6af41f5ed00bf56b0ff10498 Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 8 Sep 2026 02:03:58 +0200 Subject: [PATCH 5/5] test: add failing tests --- .../reflection/tests/ClassReflectorTest.php | 18 +++++++++++++++ .../Mapper/Fixtures/ContextDialectCaster.php | 22 +++++++++++++++++++ .../Fixtures/ObjectWithContextDialect.php | 15 +++++++++++++ tests/Integration/Mapper/MapperTest.php | 18 +++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 tests/Integration/Mapper/Fixtures/ContextDialectCaster.php create mode 100644 tests/Integration/Mapper/Fixtures/ObjectWithContextDialect.php diff --git a/packages/reflection/tests/ClassReflectorTest.php b/packages/reflection/tests/ClassReflectorTest.php index 543931b55..ecbd04303 100644 --- a/packages/reflection/tests/ClassReflectorTest.php +++ b/packages/reflection/tests/ClassReflectorTest.php @@ -7,7 +7,9 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use ReflectionClass; +use ReflectionObject; use Tempest\Reflection\ClassReflector; +use Tempest\Reflection\PropertyReflector; use Tempest\Reflection\Tests\Fixtures\ChildWithRecursiveAttribute; use Tempest\Reflection\Tests\Fixtures\ClassWithInterfaceWithRecursiveAttribute; use Tempest\Reflection\Tests\Fixtures\RecursiveAttribute; @@ -19,6 +21,22 @@ */ final class ClassReflectorTest extends TestCase { + #[Test] + public function public_properties_are_specific_to_the_reflected_object(): void + { + $first = new ClassReflector(new ReflectionObject((object) ['first' => 1])); + $second = new ClassReflector(new ReflectionObject((object) ['second' => 2])); + + $this->assertSame(['first'], array_map( + fn (PropertyReflector $property) => $property->getName(), + $first->getPublicProperties(), + )); + $this->assertSame(['second'], array_map( + fn (PropertyReflector $property) => $property->getName(), + $second->getPublicProperties(), + )); + } + #[Test] public function getting_underlying_reflection_class(): void { diff --git a/tests/Integration/Mapper/Fixtures/ContextDialectCaster.php b/tests/Integration/Mapper/Fixtures/ContextDialectCaster.php new file mode 100644 index 000000000..b9bb441a9 --- /dev/null +++ b/tests/Integration/Mapper/Fixtures/ContextDialectCaster.php @@ -0,0 +1,22 @@ +context->dialect->name; + } +} diff --git a/tests/Integration/Mapper/Fixtures/ObjectWithContextDialect.php b/tests/Integration/Mapper/Fixtures/ObjectWithContextDialect.php new file mode 100644 index 000000000..e589967cf --- /dev/null +++ b/tests/Integration/Mapper/Fixtures/ObjectWithContextDialect.php @@ -0,0 +1,15 @@ + 'input']) + ->in(new DatabaseContext(DatabaseDialect::MYSQL)) + ->to(ObjectWithContextDialect::class); + + $postgresql = map(['dialect' => 'input']) + ->in(new DatabaseContext(DatabaseDialect::POSTGRESQL)) + ->to(ObjectWithContextDialect::class); + + $this->assertSame('MYSQL', $mysql->dialect); + $this->assertSame('POSTGRESQL', $postgresql->dialect); + } + #[Test] public function make_object_from_class_string(): void {