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 c14a092bd..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,20 +32,17 @@ public static function setAround(string $aspect, array $classes, ?int $priority { $priority ??= 0; - $existing = static::get('classes.' . $aspect, []); - static::set('classes.' . $aspect, array_merge($existing, $classes)); - - if (isset(static::$aspectRules[$aspect])) { - static::$aspectRules[$aspect] = [ - 'priority' => $priority, - 'classes' => 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, + ]; } /** @@ -66,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 { @@ -113,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) { 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) { diff --git a/tests/Di/Aop/AspectCollectorTest.php b/tests/Di/Aop/AspectCollectorTest.php index 80ccab5fa..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,17 +43,38 @@ public function testSetAroundMergesClassesOnDuplicateRegistration() ); } - public function testGetPriorityReturnsZeroForUnregisteredAspect() + public function testSetAroundDeduplicatesClassesOnInitialRegistration(): void + { + 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::getRule('App\Aspect\FooAspect')['classes'] + ); + } + + 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']); @@ -65,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']); @@ -76,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;