From 1f8f56c3c7010eb9dd2c583afccee1a63cbf06d6 Mon Sep 17 00:00:00 2001 From: Richard Anabeto Opoku Date: Mon, 6 Jul 2026 01:50:55 +0000 Subject: [PATCH 1/4] Deduplicate aspect classes on repeated registration AspectCollector::setAround() merges class rules with array_merge(), so registering the same aspect again appends duplicate entries. Providers re-register their aspects on every application boot, and each duplicate makes GenerateProxies re-match a longer rule list against the class map on the next boot. In a test suite that boots the application per test, boot time grows linearly with the number of tests already run. Deduplicate the merged class list so repeated registration is idempotent. --- src/di/src/Aop/AspectCollector.php | 7 +++++-- tests/Di/Aop/AspectCollectorTest.php | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/di/src/Aop/AspectCollector.php b/src/di/src/Aop/AspectCollector.php index c14a092bd..54db164d7 100644 --- a/src/di/src/Aop/AspectCollector.php +++ b/src/di/src/Aop/AspectCollector.php @@ -41,13 +41,16 @@ public static function setAround(string $aspect, array $classes, ?int $priority { $priority ??= 0; + // Merge idempotently: providers re-register their aspects on every + // application boot (tests, Octane-style reboots), and duplicate class + // rules make proxy generation cost grow with each boot. $existing = static::get('classes.' . $aspect, []); - static::set('classes.' . $aspect, array_merge($existing, $classes)); + static::set('classes.' . $aspect, array_values(array_unique(array_merge($existing, $classes)))); if (isset(static::$aspectRules[$aspect])) { static::$aspectRules[$aspect] = [ 'priority' => $priority, - 'classes' => array_merge(static::$aspectRules[$aspect]['classes'], $classes), + 'classes' => array_values(array_unique(array_merge(static::$aspectRules[$aspect]['classes'], $classes))), ]; } else { static::$aspectRules[$aspect] = [ diff --git a/tests/Di/Aop/AspectCollectorTest.php b/tests/Di/Aop/AspectCollectorTest.php index 80ccab5fa..2afb28d5c 100644 --- a/tests/Di/Aop/AspectCollectorTest.php +++ b/tests/Di/Aop/AspectCollectorTest.php @@ -43,6 +43,21 @@ public function testSetAroundMergesClassesOnDuplicateRegistration() ); } + public function testSetAroundDeduplicatesClassesOnRepeatedRegistration() + { + AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); + AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); + + $this->assertSame( + ['App\Foo::bar'], + AspectCollector::getRule('App\Aspect\FooAspect')['classes'] + ); + $this->assertSame( + ['App\Foo::bar'], + AspectCollector::get('classes.App\Aspect\FooAspect') + ); + } + public function testGetPriorityReturnsZeroForUnregisteredAspect() { $this->assertSame(0, AspectCollector::getPriority('NonExistent')); From db2c39d8f0b1e8ec2834b999888eb8cf41c74a86 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:25:46 +0000 Subject: [PATCH 2/4] Collapse aspect collector class rules AspectCollector kept class-targeting rules in two stores: the generic dot-key container and the priority-aware aspect rules array. Re-registering an aspect had to keep both stores in sync, which made duplicate class rules easy to accumulate and made the proxy generator scan the same rule repeatedly against the class map. Make the aspect rules array the single source of truth, add a typed getClassRules() reader for consumers that only need aspect => class rules, and remove the old generic get/set/list metadata surface. setAround() now merges and deduplicates class rules once while preserving priority overwrite behavior on repeated registration. Update the AOP parser, proxy manager, and proxy trait to read the typed class-rule map. Also remove the redundant getRule() re-fetch in Aspect::parseClasses() now that the passed map is already the canonical class-rule data. --- src/di/src/Aop/Aspect.php | 12 ++--- src/di/src/Aop/AspectCollector.php | 83 +++++++++++------------------- src/di/src/Aop/ProxyManager.php | 4 +- src/di/src/Aop/ProxyTrait.php | 2 +- 4 files changed, 38 insertions(+), 63 deletions(-) diff --git a/src/di/src/Aop/Aspect.php b/src/di/src/Aop/Aspect.php index 813ad797c..e3d648061 100644 --- a/src/di/src/Aop/Aspect.php +++ b/src/di/src/Aop/Aspect.php @@ -12,10 +12,10 @@ class Aspect public static function parse(string $class): RewriteCollection { $rewriteCollection = new RewriteCollection($class); - $classesCollection = AspectCollector::get('classes', []); + $classRules = AspectCollector::getClassRules(); - if ($classesCollection) { - self::parseClasses($classesCollection, $class, $rewriteCollection); + if ($classRules) { + self::parseClasses($classRules, $class, $rewriteCollection); } return $rewriteCollection; @@ -126,10 +126,8 @@ public static function isMatch(string $class, string $method, string $rule): boo */ private static function parseClasses(array $collection, string $class, RewriteCollection $rewriteCollection): void { - $aspects = array_keys($collection); - foreach ($aspects as $aspect) { - $rules = AspectCollector::getRule($aspect); - foreach ($rules['classes'] ?? [] as $rule) { + foreach ($collection as $rules) { + foreach ($rules as $rule) { [$isMatch, $method] = static::isMatchClassRule($class, $rule); if ($isMatch) { if ($method === null) { diff --git a/src/di/src/Aop/AspectCollector.php b/src/di/src/Aop/AspectCollector.php index 54db164d7..84b27d060 100644 --- a/src/di/src/Aop/AspectCollector.php +++ b/src/di/src/Aop/AspectCollector.php @@ -4,8 +4,6 @@ namespace Hypervel\Di\Aop; -use Hypervel\Support\Arr; - /** * Static registry of aspect class rules and priorities. * @@ -16,13 +14,6 @@ */ class AspectCollector { - /** - * Container indexed by type ('classes') and aspect class name. - * - * @var array>> - */ - protected static array $container = []; - /** * Aspect rules indexed by aspect class name. * @@ -41,23 +32,17 @@ public static function setAround(string $aspect, array $classes, ?int $priority { $priority ??= 0; - // Merge idempotently: providers re-register their aspects on every - // application boot (tests, Octane-style reboots), and duplicate class - // rules make proxy generation cost grow with each boot. - $existing = static::get('classes.' . $aspect, []); - static::set('classes.' . $aspect, array_values(array_unique(array_merge($existing, $classes)))); - - if (isset(static::$aspectRules[$aspect])) { - static::$aspectRules[$aspect] = [ - 'priority' => $priority, - 'classes' => array_values(array_unique(array_merge(static::$aspectRules[$aspect]['classes'], $classes))), - ]; - } else { - static::$aspectRules[$aspect] = [ - 'priority' => $priority, - 'classes' => $classes, - ]; - } + // Merge idempotently: a provider re-registering the same aspect on a repeated + // boot in the same worker must not append duplicate class rules; the proxy + // generator scans every rule against the whole class map, so duplicates make + // each boot slower than the last. + $existing = static::$aspectRules[$aspect]['classes'] ?? []; + $classes = array_values(array_unique(array_merge($existing, $classes))); + + static::$aspectRules[$aspect] = [ + 'priority' => $priority, + 'classes' => $classes, + ]; } /** @@ -69,37 +54,36 @@ public static function hasAspects(): bool } /** - * Retrieve metadata by dot-notated key. - */ - public static function get(string $key, mixed $default = null): mixed - { - return Arr::get(static::$container, $key) ?? $default; - } - - /** - * Set metadata by dot-notated key. + * Remove a specific aspect from the registry. * - * Boot-only. Aspect metadata persists in static properties used by the - * proxy generator and runtime aspect resolver. + * Tests only. Mutates the worker-wide aspect registry; runtime use cannot + * retroactively update already-generated proxies. */ - public static function set(string $key, mixed $value): void + public static function forgetAspect(string $aspect): void { - Arr::set(static::$container, $key, $value); + unset(static::$aspectRules[$aspect]); } /** - * Remove a specific aspect from the registry. + * Get the class-targeting rules for every registered aspect. * - * Tests only. Mutates the worker-wide aspect registry; runtime use cannot - * retroactively update already-generated proxies. + * @return array> */ - public static function forgetAspect(string $aspect): void + public static function getClassRules(): array { - unset(static::$container['classes'][$aspect], static::$aspectRules[$aspect]); + $classRules = []; + + foreach (static::$aspectRules as $aspect => $rule) { + $classRules[$aspect] = $rule['classes']; + } + + return $classRules; } /** * Get the rules for a specific aspect. + * + * @return array{priority: int, classes: array}|array{} */ public static function getRule(string $aspect): array { @@ -116,26 +100,19 @@ public static function getPriority(string $aspect): int /** * Get all aspect rules. + * + * @return array}> */ public static function getRules(): array { return static::$aspectRules; } - /** - * Return all metadata. - */ - public static function list(): array - { - return static::$container; - } - /** * Flush all static state. */ public static function flushState(): void { - static::$container = []; static::$aspectRules = []; } } diff --git a/src/di/src/Aop/ProxyManager.php b/src/di/src/Aop/ProxyManager.php index 71b729a77..88d0fee20 100644 --- a/src/di/src/Aop/ProxyManager.php +++ b/src/di/src/Aop/ProxyManager.php @@ -62,7 +62,7 @@ public function getProxyDir(): string public function getAspectClasses(): array { $aspectClasses = []; - $classesAspects = AspectCollector::get('classes', []); + $classesAspects = AspectCollector::getClassRules(); foreach ($classesAspects as $aspect => $rules) { foreach ($rules as $rule) { if (isset($this->proxies[$rule])) { @@ -172,7 +172,7 @@ protected function initProxiesByReflectionClassMap(array $reflectionClassMap = [ if (! $reflectionClassMap) { return $proxies; } - $classesAspects = AspectCollector::get('classes', []); + $classesAspects = AspectCollector::getClassRules(); foreach ($classesAspects as $aspect => $rules) { foreach ($rules as $rule) { foreach ($reflectionClassMap as $class => $path) { diff --git a/src/di/src/Aop/ProxyTrait.php b/src/di/src/Aop/ProxyTrait.php index eb1b1e5fb..8fadc62f6 100644 --- a/src/di/src/Aop/ProxyTrait.php +++ b/src/di/src/Aop/ProxyTrait.php @@ -75,7 +75,7 @@ protected static function makePipeline(): Pipeline */ protected static function getClassesAspects(string $className, string $method): array { - $aspects = AspectCollector::get('classes', []); + $aspects = AspectCollector::getClassRules(); $matchedAspects = []; foreach ($aspects as $aspect => $rules) { foreach ($rules as $rule) { From e0767fdc55a233ac61f01fe977afa43cb07226d4 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:25:56 +0000 Subject: [PATCH 3/4] Use typed AOP class rules in test helper InteractsWithAop manually resolves matching aspects for tests that need to run the AOP pipeline without generated proxies. That helper was still reading AspectCollector through the removed generic classes metadata store. Switch it to AspectCollector::getClassRules() so the helper follows the same single-source class-rule API as the runtime proxy path. The returned shape is unchanged for the helper: aspect class names keyed to their class-targeting rule lists. --- src/foundation/src/Testing/Concerns/InteractsWithAop.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/foundation/src/Testing/Concerns/InteractsWithAop.php b/src/foundation/src/Testing/Concerns/InteractsWithAop.php index 18d104645..7975cb2c4 100644 --- a/src/foundation/src/Testing/Concerns/InteractsWithAop.php +++ b/src/foundation/src/Testing/Concerns/InteractsWithAop.php @@ -136,7 +136,7 @@ private function buildAopArguments(ReflectionMethod $method, array $provided): a */ private function resolveMatchingAspects(string $className, string $method): array { - $allAspects = AspectCollector::get('classes', []); + $allAspects = AspectCollector::getClassRules(); $matched = []; foreach ($allAspects as $aspect => $rules) { From e84596691e9ecb443ff3685825cccc329c0c27ba Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:26:07 +0000 Subject: [PATCH 4/4] Update AOP collector tests for single store The collector no longer exposes the old generic dot-key metadata helpers, so tests should describe the actual supported AOP registry behavior instead of the removed storage mechanism. Add coverage for duplicate class rules on both initial registration and repeated registration. Assert class-rule reads through getClassRules(), and keep existing merge, priority, forget, and flush behavior covered. Update ProxyTraitTest to register aspects through setAround() instead of seeding the collector internals directly. This exercises the real registration path while preserving the default zero-priority behavior those tests relied on. --- tests/Di/Aop/AspectCollectorTest.php | 50 ++++++++++++---------------- tests/Di/Aop/ProxyTraitTest.php | 24 +++++-------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/tests/Di/Aop/AspectCollectorTest.php b/tests/Di/Aop/AspectCollectorTest.php index 2afb28d5c..8c8235874 100644 --- a/tests/Di/Aop/AspectCollectorTest.php +++ b/tests/Di/Aop/AspectCollectorTest.php @@ -9,12 +9,12 @@ class AspectCollectorTest extends TestCase { - public function testHasAspectsReturnsFalseWhenEmpty() + public function testHasAspectsReturnsFalseWhenEmpty(): void { $this->assertFalse(AspectCollector::hasAspects()); } - public function testSetAroundRegistersAspect() + public function testSetAroundRegistersAspect(): void { AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); @@ -25,14 +25,14 @@ public function testSetAroundRegistersAspect() ], AspectCollector::getRule('App\Aspect\FooAspect')); } - public function testSetAroundDefaultsPriorityToZero() + public function testSetAroundDefaultsPriorityToZero(): void { AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo']); $this->assertSame(0, AspectCollector::getPriority('App\Aspect\FooAspect')); } - public function testSetAroundMergesClassesOnDuplicateRegistration() + public function testSetAroundMergesClassesOnDuplicateRegistration(): void { AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); AspectCollector::setAround('App\Aspect\FooAspect', ['App\Baz::qux'], 5); @@ -43,32 +43,38 @@ public function testSetAroundMergesClassesOnDuplicateRegistration() ); } - public function testSetAroundDeduplicatesClassesOnRepeatedRegistration() + public function testSetAroundDeduplicatesClassesOnInitialRegistration(): void { - AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); - AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); + AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar', 'App\Foo::bar'], 5); $this->assertSame( ['App\Foo::bar'], AspectCollector::getRule('App\Aspect\FooAspect')['classes'] ); + } + + public function testSetAroundDeduplicatesClassesOnRepeatedRegistration(): void + { + AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); + AspectCollector::setAround('App\Aspect\FooAspect', ['App\Foo::bar'], 5); + $this->assertSame( ['App\Foo::bar'], - AspectCollector::get('classes.App\Aspect\FooAspect') + AspectCollector::getRule('App\Aspect\FooAspect')['classes'] ); } - public function testGetPriorityReturnsZeroForUnregisteredAspect() + public function testGetPriorityReturnsZeroForUnregisteredAspect(): void { $this->assertSame(0, AspectCollector::getPriority('NonExistent')); } - public function testGetRuleReturnsEmptyForUnregisteredAspect() + public function testGetRuleReturnsEmptyForUnregisteredAspect(): void { $this->assertSame([], AspectCollector::getRule('NonExistent')); } - public function testForgetAspectRemovesSpecificAspect() + public function testForgetAspectRemovesSpecificAspect(): void { AspectCollector::setAround('Aspect1', ['Class1']); AspectCollector::setAround('Aspect2', ['Class2']); @@ -80,7 +86,7 @@ public function testForgetAspectRemovesSpecificAspect() $this->assertTrue(AspectCollector::hasAspects()); } - public function testFlushStateRemovesAllAspects() + public function testFlushStateRemovesAllAspects(): void { AspectCollector::setAround('Aspect1', ['Class1']); AspectCollector::setAround('Aspect2', ['Class2']); @@ -91,28 +97,14 @@ public function testFlushStateRemovesAllAspects() $this->assertSame([], AspectCollector::getRules()); } - public function testGetReturnsContainerData() + public function testGetClassRulesReturnsClassRules(): void { AspectCollector::setAround('Aspect1', ['Class1', 'Class2']); - $this->assertSame(['Class1', 'Class2'], AspectCollector::get('classes.Aspect1')); - } - - public function testGetReturnsDefaultWhenKeyNotFound() - { - $this->assertSame('default', AspectCollector::get('nonexistent', 'default')); - } - - public function testListReturnsAllContainerData() - { - AspectCollector::setAround('Aspect1', ['Class1']); - - $list = AspectCollector::list(); - $this->assertArrayHasKey('classes', $list); - $this->assertArrayHasKey('Aspect1', $list['classes']); + $this->assertSame(['Class1', 'Class2'], AspectCollector::getClassRules()['Aspect1']); } - public function testGetRulesReturnsAllRules() + public function testGetRulesReturnsAllRules(): void { AspectCollector::setAround('Aspect1', ['Class1'], 5); AspectCollector::setAround('Aspect2', ['Class2'], 10); diff --git a/tests/Di/Aop/ProxyTraitTest.php b/tests/Di/Aop/ProxyTraitTest.php index cbc4a2937..293f842a7 100644 --- a/tests/Di/Aop/ProxyTraitTest.php +++ b/tests/Di/Aop/ProxyTraitTest.php @@ -15,7 +15,7 @@ class ProxyTraitTest extends TestCase { - public function testGetParamsMap() + public function testGetParamsMap(): void { $obj = new ProxyTraitObject; @@ -46,7 +46,7 @@ public function testGetParamsMap() $this->assertEquals(['id' => null, 'variadic' => ['b', 'a' => 'a']], $obj->get4(null, 'b', a: 'a')['keys']); } - public function testGetParamsMapOnTraitAlias() + public function testGetParamsMapOnTraitAlias(): void { $obj = new ProxyTraitObject; @@ -74,45 +74,39 @@ public function testGetParamsMapOnTraitAlias() $this->assertEquals(['id' => null, 'variadic' => ['b', 'a' => 'a']], $obj->get4OnTrait(null, 'b', a: 'a')['keys']); } - public function testProceedingJoinPointGetInstance() + public function testProceedingJoinPointGetInstance(): void { $obj = new ProxyTraitObject; $this->assertSame('HypervelCloud', $obj->getName2()); - AspectCollector::set('classes', [ - GetNameAspect::class => [ProxyTraitObject::class], - ]); + AspectCollector::setAround(GetNameAspect::class, [ProxyTraitObject::class]); $obj = new ProxyTraitObject; $this->assertSame('Hypervel', $obj->getName()); } - public function testProceedingJoinPointGetArguments() + public function testProceedingJoinPointGetArguments(): void { AspectCollector::flushState(); $obj = new ProxyTraitObject; $this->assertEquals(['id' => 1, 'variadic' => ['2', 'foo' => '3'], 'func_get_args' => [1, '2']], $obj->getParams(1, '2', foo: '3')); - AspectCollector::set('classes', [ - GetParamsAspect::class => [ProxyTraitObject::class], - ]); + AspectCollector::setAround(GetParamsAspect::class, [ProxyTraitObject::class]); $obj = new ProxyTraitObject; $this->assertEquals([1, '2', 'foo' => '3'], $obj->getParams2(1, '2', foo: '3')); } - public function testHandleAroundWithClassAspect() + public function testHandleAroundWithClassAspect(): void { - AspectCollector::set('classes', [ - IncrAspect::class => [ProxyTraitObject::class], - ]); + AspectCollector::setAround(IncrAspect::class, [ProxyTraitObject::class]); $obj = new ProxyTraitObject; $this->assertSame(2, $obj->incr()); } - public function testMakePipelineReturnsFreshInstances() + public function testMakePipelineReturnsFreshInstances(): void { $stub = new class { use ProxyTrait;