diff --git a/CHANGELOG.md b/CHANGELOG.md index 434f0ba4..fff1f77a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ See [UPGRADE-4.0.md](UPGRADE-4.0.md) for the migration guide. * [Feature] [BC BREAK] **Interface-keyed registration listeners** — new `AspectContainer::onRegistration(string $interfaceFQCN, Closure $listener)`: the listener receives the id (class-name) and the container whenever a deferred service whose id implements the interface is registered via `addLazyService()`; matching never autoloads, so an id whose class is not loaded yet reaches the listener as a candidate. Values are never touched, so laziness is preserved; with no listeners armed (production) registration stays a pure array write. The kernel arms one `Aspect::class` listener in debug mode to track aspect source files as freshness resources at registration time, resolving them through composer without loading the aspects (previously hardwired into `registerAspect()`). `AspectContainer::addResource()` is now part of the interface and `final public` on `Container` (was `final protected`). * [Feature] [BC BREAK] **PSR-11 container** — `AspectContainer` extends `Psr\Container\ContainerInterface`: `getValue()` is renamed to `get()`, `has()` is inherited from the PSR interface. `ServiceNotFoundException` implements `NotFoundExceptionInterface`, and a container entry of the wrong type in `getService()` throws the new `Go\Aop\Exception\ContainerException` (an `InvalidConfigurationException` implementing `ContainerExceptionInterface`). * [Feature] **Aspects from the application container** — `AspectContainer::setDelegate(ContainerInterface $delegate)` attaches the PSR-11 container of the application: `get()`/`getService()`/`has()` fall back to it for unknown ids. Aspects are registered in `configureAop()` with `addLazyService()` as before, and their factories resolve dependencies from the delegate on first use, once the application has built its container; weaving and cache warm-up never need it. The delegate is never enumerated (PSR-11 has no lookup by interface or tag), framework bridges can collect tagged services and register them. +* [Feature] [BC BREAK] **Weaving never creates aspects** — the aspect loader reads aspects from their classes and builds every aspect-method advice in the lazy static-data form of the `Interceptor` facade, which exposes the aspect class, the advice method, the order and the expression without resolving the aspect. An aspect is created on the first call of one of its advices, also one without instance properties, whose factory used to run as soon as the weaver collected the aspects (before an application container attached with `setDelegate()` existed). New `AspectContainer::getServiceIdsByInterface()` lists service ids without creating the services. `AspectLoaderExtension::load()` takes the `ReflectionClass` of the aspect only, `AbstractAspectLoaderExtension::parsePointcut()` the `ReflectionClass` of the aspect instead of the instance. * [Feature] Console commands declare their name, description and help with `#[AsCommand]`, `bin/aspect` loads them lazily, and every command returns `Command::SUCCESS`/`Command::FAILURE` (`debug:weaving` used to return the raw error count). `cache:warmup:aop` stops cleanly on SIGINT/SIGTERM. * [Feature] [BC BREAK] **Doctrine bridge rewritten for the trait-based engine (#671)** — new `Go\Bridge\Doctrine\WovenEntityClassLocator` lets Doctrine mapping drivers (ORM 3.6+) discover entities through the autoloader, so the woven class is mapped instead of the original source, and entities loaded before discovery are no longer missed. `MetadataLoadInterceptor` no longer rewrites metadata (the old mapped-superclass workaround only matched the extend-based engine); it now reports woven entities with intercepted mapped properties when Doctrine native lazy objects are disabled. It is registered as a plain listener: `$eventManager->addEventListener(Events::loadClassMetadata, new MetadataLoadInterceptor())` (it is no longer an `EventSubscriber`). diff --git a/docs/configuration.md b/docs/configuration.md index 58513e91..9d3a64bd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -66,7 +66,7 @@ ApplicationAspectKernel::getInstance()->getContainer()->setDelegate($psrContaine ``` Weaving and the cache warm-up only need the class of the aspect, so they run before `setDelegate()`, even in -`bin/aspect`. The factory runs on the first advice call; a dependency that is still missing then throws +`bin/aspect`, and never create an aspect. The factory runs on the first advice call; a dependency that is still missing then throws `ServiceNotFoundException`. - A factory never asks the aspect container for its own id: that id resolves to the aspect being built. @@ -75,8 +75,9 @@ Weaving and the cache warm-up only need the class of the aspect, so they run bef (some containers can, each in its own way). Aspects are therefore never discovered in the delegate, every aspect is registered in `configureAop()`. An integration bridge for a specific framework can collect the services by tag or interface from its container and register them with `addLazyService()`. -- An aspect without instance properties cannot be created lazily, so its factory runs when the aspects are - collected for weaving, before the delegate exists. Such an aspect has no dependencies to inject. +- Weaving never creates an aspect: it reads the aspect class, and the advices refer to the aspect by class and + method name. Every aspect, also one without instance properties (which PHP can not make lazy), is created by its + factory on the first call of one of its advices, once the application container is attached. ## Extending the transformation pipeline diff --git a/phpstan-baseline-strict.php b/phpstan-baseline-strict.php index 1a815e11..ad580c3a 100644 --- a/phpstan-baseline-strict.php +++ b/phpstan-baseline-strict.php @@ -11,7 +11,7 @@ 'message' => '#^Variable method call on Go\\\\Aop\\\\Aspect\\.$#', 'identifier' => 'method.dynamicName', 'count' => 1, - 'path' => __DIR__ . '/src/Aop/Framework/Interceptor.php', + 'path' => __DIR__ . '/src/Aop/Framework/AbstractInterceptor.php', ]; $ignoreErrors[] = [ 'message' => '#^Only booleans are allowed in &&, list\\\\|\\(T of object \\= object\\)\\|null\\>\\|null given on the right side\\.$#', diff --git a/src/Aop/AGENTS.md b/src/Aop/AGENTS.md index 20dc5c08..ccc76c0f 100644 --- a/src/Aop/AGENTS.md +++ b/src/Aop/AGENTS.md @@ -48,7 +48,7 @@ when `$level > 0` and pops it in `finally`. Check every change with `composer te ## Advice wiring (src/Aop/Framework/) - The — proxy-code accessor: aspect(X::class) fetches aspect from container; advice('advisorId') resolves container-backed closure advice (unwraps Advisor/AbstractInterceptor to raw Closure) -- Interceptor — @internal factory facade with TWO construction modes: before()/after()/around()/afterThrowing(class-string|Closure, ?string $methodName=null, int $order=0, string $expression=''). With aspect class + method name it returns a native PHP 8.4 lazy proxy (via Go\Core\NativeLazyProxy::create) — interceptor construction, The::aspect() resolution and FCC creation all defer until first real use (invocation/ordering), so unmatched advices never instantiate their aspect; ONLY compiled advisor cache files use this lazy form. A ready Closure constructs eagerly, and generated PROXY classes deliberately use the eager `The::aspect(X::class)->method(...)` FCC form (InterceptorListGenerator): the proxy method/hook is already executing, so the interceptor is needed right now and a lazy detour would be pure overhead; The::advice('') stays eager too. Free to change between releases +- Interceptor — @internal factory facade, before()/after()/around()/afterThrowing(Closure|class-string, ?methodName, order, expression). Aspect class + method name → native lazy proxy whose order, expression and aspectMethod are readable without initialization: the aspect loader and advisor cache files use it, so weaving never creates an aspect. A Closure constructs eagerly; generated proxies use the eager `The::aspect(X::class)->method(...)` form on purpose (reasons in the class docblock) - GeneratedInterceptor — internal descriptor built by AbstractJoinpoint::flatAndSortAdvices() via fromAdvice(); usesContainerAdvice=true when advice closure isn't scoped to an Aspect class - AdviceTypeEnum — Advice::getType() kind + sorting priority (before → after/afterThrowing → around → introduction); replaced AdviceBefore/AdviceAfter/AdviceAround marker interfaces - Advice methods MUST be public (FCC calls them on the aspect instance from generated code) diff --git a/src/Aop/Framework/AbstractInterceptor.php b/src/Aop/Framework/AbstractInterceptor.php index cb200fae..ccac61c1 100644 --- a/src/Aop/Framework/AbstractInterceptor.php +++ b/src/Aop/Framework/AbstractInterceptor.php @@ -55,11 +55,30 @@ */ abstract class AbstractInterceptor implements InterceptorInterface, OrderedAdvice { + /** + * Advice closure, called with the joinpoint + */ + protected readonly Closure $adviceMethod; + + /** + * Aspect class and advice method of an advice given by name, null for a closure advice + * + * @var array{class-string, string}|null + * + * @internal Read by the weaver and the advisor cache: the lazy {@see Interceptor} facade sets it without + * initializing the interceptor, so the aspect is not created + */ + public private(set) ?array $aspectMethod = null; + /** * Default constructor for interceptor + * + * @param Closure|array{class-string, string} $adviceMethod Advice closure, or the aspect class and the + * name of its advice method: the aspect is then resolved from the container ({@see The::aspect()}). + * The array is not a callable for PHP (the advice method is not static), so the type can not be callable. */ public function __construct( - protected readonly Closure $adviceMethod, + Closure|array $adviceMethod, /** Order of advice invocation, lower values are invoked first */ public readonly int $adviceOrder = 0, /** @@ -68,7 +87,15 @@ public function __construct( * @internal Framework-internal introspection point (e.g. debug:advisor command), not a public API */ public private(set) string $pointcutExpression = '', - ) {} + ) { + if ($adviceMethod instanceof Closure) { + $this->adviceMethod = $adviceMethod; + } else { + [$aspectClass, $methodName] = $adviceMethod; + $this->adviceMethod = The::aspect($aspectClass)->$methodName(...); + $this->aspectMethod = $adviceMethod; + } + } /** * Getter for extracting the advice closure from Interceptor @@ -90,10 +117,17 @@ public function getRawAdvice(): Closure */ final public function compileToPhp(): Expr { - $reflectionAdvice = new ReflectionFunction($this->adviceMethod); - $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); - if (!isset($scopeReflectionClass) || !is_subclass_of($scopeReflectionClass->name, Aspect::class)) { - throw NotCompilableException::interceptorWithoutAspect(); + // An advice given by name is compiled without reading the closure: that would create the aspect + if ($this->aspectMethod !== null) { + [$aspectClass, $methodName] = $this->aspectMethod; + } else { + $reflectionAdvice = new ReflectionFunction($this->adviceMethod); + $scopeReflectionClass = $reflectionAdvice->getClosureScopeClass(); + if (!isset($scopeReflectionClass) || !is_subclass_of($scopeReflectionClass->name, Aspect::class)) { + throw NotCompilableException::interceptorWithoutAspect(); + } + $aspectClass = $scopeReflectionClass->name; + $methodName = $reflectionAdvice->name; } $factoryMethod = match ($this::class) { @@ -105,8 +139,8 @@ final public function compileToPhp(): Expr }; $args = [ - new Arg(new ClassConstFetch(new FullyQualified($scopeReflectionClass->name), 'class')), - new Arg(new String_($reflectionAdvice->name)), + new Arg(new ClassConstFetch(new FullyQualified($aspectClass), 'class')), + new Arg(new String_($methodName)), ]; if ($this->adviceOrder !== 0) { $args[] = new Arg(new Int_($this->adviceOrder), name: new Identifier('order')); diff --git a/src/Aop/Framework/GeneratedInterceptor.php b/src/Aop/Framework/GeneratedInterceptor.php index 4ca0e4b5..270be79b 100644 --- a/src/Aop/Framework/GeneratedInterceptor.php +++ b/src/Aop/Framework/GeneratedInterceptor.php @@ -39,14 +39,22 @@ public static function fromAdvice(string $advisorId, Advice $advice): self throw AspectException::unsupportedAdvice($advisorId, $advice); } - $reflectionAdvice = new ReflectionFunction($advice->getRawAdvice()); - $scopeClass = $reflectionAdvice->getClosureScopeClass(); - $usesContainerAdvice = $scopeClass === null || !is_subclass_of($scopeClass->name, Aspect::class); + if ($advice->aspectMethod !== null) { + // An advice given by name: reading its closure would create the aspect + [$aspectClass, $adviceMethod] = $advice->aspectMethod; + $usesContainerAdvice = false; + } else { + $reflectionAdvice = new ReflectionFunction($advice->getRawAdvice()); + $scopeClass = $reflectionAdvice->getClosureScopeClass(); + $usesContainerAdvice = $scopeClass === null || !is_subclass_of($scopeClass->name, Aspect::class); + $aspectClass = $usesContainerAdvice ? null : $scopeClass->name; + $adviceMethod = $usesContainerAdvice ? null : $reflectionAdvice->name; + } return new self( $advice->getType()->value, - $usesContainerAdvice ? null : $scopeClass->name, - $usesContainerAdvice ? null : $reflectionAdvice->name, + $aspectClass, + $adviceMethod, $advice->adviceOrder, $advisorId, $usesContainerAdvice, diff --git a/src/Aop/Framework/Interceptor.php b/src/Aop/Framework/Interceptor.php index c8ed9a7a..aa2778fc 100644 --- a/src/Aop/Framework/Interceptor.php +++ b/src/Aop/Framework/Interceptor.php @@ -93,15 +93,15 @@ private static function createLazily( throw AspectException::adviceMethodNameRequired(); } + $aspectMethod = [$aspectClassOrAdvice, $methodName]; + // Interceptor classes are framework-owned and lazy-compatible by construction, // so they take the probe-free trusted path. return NativeLazyProxy::create( $interceptorClass, - static fn(): AbstractInterceptor => new $interceptorClass( - The::aspect($aspectClassOrAdvice)->$methodName(...), - $order, - $expression, - ), + static fn(): AbstractInterceptor => new $interceptorClass($aspectMethod, $order, $expression), + // What weaving, ordering and cache compilation read: the aspect is only created when the advice runs + ['adviceOrder' => $order, 'pointcutExpression' => $expression, 'aspectMethod' => $aspectMethod], ); } } diff --git a/src/Console/Command/DebugAspectCommand.php b/src/Console/Command/DebugAspectCommand.php index 820c1175..46ed566f 100644 --- a/src/Console/Command/DebugAspectCommand.php +++ b/src/Console/Command/DebugAspectCommand.php @@ -16,7 +16,7 @@ use Go\Aop\Aspect; use Go\Aop\Pointcut; use Go\Core\AspectLoader; -use ReflectionObject; +use ReflectionClass; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -61,16 +61,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int $aspectName = $input->getOption('aspect'); if (!is_string($aspectName) || $aspectName === '') { $io->text('' . $this->aspectKernel::class . ' has following enabled aspects:'); - $aspects = $container->getServicesByInterface(Aspect::class); + $aspects = $container->getServiceIdsByInterface(Aspect::class); } else { $aspectName = ltrim($aspectName, '\\'); if (!$container->has($aspectName) || !is_subclass_of($aspectName, Aspect::class)) { - $registeredAspects = array_keys($container->getServicesByInterface(Aspect::class)); + $registeredAspects = $container->getServiceIdsByInterface(Aspect::class); $io->error(sprintf('Aspect "%s" is not registered in the kernel.', $aspectName) . self::suggestAlternative($aspectName, $registeredAspects)); return Command::FAILURE; } - $aspects[] = $container->getService($aspectName); + $aspects[] = $aspectName; } $this->showRegisteredAspectsInfo($io, $aspects); @@ -78,23 +78,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int } /** - * Shows an information about registered aspects + * Shows an information about registered aspects, read from their classes: no aspect is created * - * @param Aspect[] $aspects List of aspects + * @param list $aspects Classes of the aspects */ private function showRegisteredAspectsInfo(SymfonyStyle $io, array $aspects): void { - foreach ($aspects as $aspect) { - $this->showAspectInfo($io, $aspect); + foreach ($aspects as $aspectClass) { + if (is_subclass_of($aspectClass, Aspect::class)) { + $this->showAspectInfo($io, $aspectClass); + } } } /** * Displays an information about single aspect + * + * @param class-string $aspectClass */ - private function showAspectInfo(SymfonyStyle $io, Aspect $aspect): void + private function showAspectInfo(SymfonyStyle $io, string $aspectClass): void { - $refAspect = new ReflectionObject($aspect); + $refAspect = new ReflectionClass($aspectClass); $aspectName = $refAspect->getName(); $io->section($aspectName); $io->writeln('Defined in: ' . $refAspect->getFileName() . ''); @@ -102,13 +106,15 @@ private function showAspectInfo(SymfonyStyle $io, Aspect $aspect): void if ($docComment) { $io->writeln($this->getPrettyText($docComment)); } - $this->showAspectPointcutsAndAdvisors($io, $aspect); + $this->showAspectPointcutsAndAdvisors($io, $refAspect); } /** * Shows an information about aspect pointcuts and advisors + * + * @param ReflectionClass $aspect */ - private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, Aspect $aspect): void + private function showAspectPointcutsAndAdvisors(SymfonyStyle $io, ReflectionClass $aspect): void { $container = $this->aspectKernel->getContainer(); diff --git a/src/Core/AGENTS.md b/src/Core/AGENTS.md index 969c4a64..14e126ce 100644 --- a/src/Core/AGENTS.md +++ b/src/Core/AGENTS.md @@ -1,7 +1,7 @@ # src/Core — Container and aspect loading ## Container (Container.php) -- Generic, AOP-unaware DI container, PSR-11 (AspectContainer extends Psr\Container\ContainerInterface): add(by class-string|key), get(), has(), getService(), addLazyService(Closure), setDelegate(), onRegistration(interfaceFQCN, listener), addResource() +- Generic, AOP-unaware DI container, PSR-11 (AspectContainer extends Psr\Container\ContainerInterface): add(by class-string|key), get(), has(), getService(), addLazyService(Closure), setDelegate(), onRegistration(interfaceFQCN, listener), addResource(), getServiceIdsByInterface() (ids of eager/materialized entries, then matching deferred ids; never materializes) - Delegate (app's PSR-11 container, attached after init via setDelegate()): get()/has() fall back to it on a miss only (hot path unchanged, delegated values are not stored); no dedicated registration API: addLazyService() factories resolve dependencies through get()/getService() on first use, so registration/weaving/warm-up never need the delegate. getServicesByInterface() never enumerates it (PSR-11 has no lookup by interface/tag; bridges collect tagged services and register them) - Automatic tagging by interface; deferred services materialize as native lazy proxies via NativeLazyProxy (engine probe, no userland compatibility predicate) - add() runs on every request: tags via class_implements(), the class file of an added object becomes a resource only on the next isFreshSince() (no reflection or stat calls at registration) @@ -11,6 +11,7 @@ ## Aspect loading - AspectLoader — scans aspect classes for pointcut/advice attributes → Advisor[]; CachedAspectLoader decorates it (both implement AspectLoaderInterface) +- Aspects are loaded BY CLASS, never by instance: getUnloadedAspects() returns a ReflectionClass per unloaded id (AspectContainer::getServiceIdsByInterface(), no factory runs), which the loaders and extensions share; advices are built with the lazy Interceptor facade. Weaving, warm-up and the advisor cache never create an aspect, also not one PHP can't make lazy (no instance properties). Never reintroduce `getServicesByInterface(Aspect::class)` or `getClosure($aspect)` on the weaving path - AdvisorIndex (@internal) — loads unloaded aspects, returns advisors, hasPointcutKind(Pointcut::KIND_*) = union of advisor pointcut kinds, recollected whenever getServicesByInterface(Advisor) returns another array (after any registration). Gates kind-specific machinery instead of feature flags (`new` rewriting ← KIND_INIT); function advices need no gate (AdviceMatcher skips non-KIND_FUNCTION advisors) ### Compiled advisor cache diff --git a/src/Core/AbstractAspectLoaderExtension.php b/src/Core/AbstractAspectLoaderExtension.php index 31ded029..2ae8f752 100644 --- a/src/Core/AbstractAspectLoaderExtension.php +++ b/src/Core/AbstractAspectLoaderExtension.php @@ -41,11 +41,12 @@ public function __construct( * General method for parsing pointcuts * * @throws PointcutSyntaxException if there was an error during parsing + * @param ReflectionClass $aspect Aspect the expression belongs to, `$this` in it * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect */ final protected function parsePointcut( - Aspect $aspect, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { @@ -57,18 +58,19 @@ final protected function parsePointcut( /** * Performs lexical analyze of pointcut * + * @param ReflectionClass $aspect * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection * @template T of Aspect * * @throws PointcutSyntaxException */ private function makeLexicalAnalyze( - Aspect $aspect, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): TokenStream { try { - $resolvedThisPointcut = str_replace('$this', $aspect::class, $pointcutExpression); + $resolvedThisPointcut = str_replace('$this', $aspect->name, $pointcutExpression); $stream = $this->pointcutLexer->lex($resolvedThisPointcut); } catch (RecognitionException $e) { throw PointcutSyntaxException::unrecognizedLexicalStructure($pointcutExpression, $reflection, $e); diff --git a/src/Core/AspectContainer.php b/src/Core/AspectContainer.php index 84894e23..cddda32a 100644 --- a/src/Core/AspectContainer.php +++ b/src/Core/AspectContainer.php @@ -99,6 +99,19 @@ public function getService(string $className): object; */ public function getServicesByInterface(string $interfaceTagClassName): array; + /** + * Returns the ids of the services tagged with the marker interface, without creating any of them + * + * Unlike {@see getServicesByInterface()}, a deferred service is not materialized: its id (a class-name) is matched + * against the interface, which autoloads the class but never runs its factory. The weaver reads aspects this way, + * by class, so an aspect is only created when one of its advices runs. + * + * @param class-string $interfaceTagClassName Interface name of services to look up + * + * @return list Ids of eagerly added and materialized services first, then the deferred ones + */ + public function getServiceIdsByInterface(string $interfaceTagClassName): array; + /** * Returns a value from the container * diff --git a/src/Core/AspectLoader.php b/src/Core/AspectLoader.php index 1e236903..6b130cce 100644 --- a/src/Core/AspectLoader.php +++ b/src/Core/AspectLoader.php @@ -49,15 +49,16 @@ public function __construct( * * @see loadAndRegister() method for registration * + * @param ReflectionClass $aspect + * * @return array */ - public function load(Aspect $aspect): array + public function load(ReflectionClass $aspect): array { - $refAspect = new ReflectionClass($aspect); $loadedItems = []; foreach ($this->aspectLoaders as $loader) { - $loadedItems += $loader->load($aspect, $refAspect); + $loadedItems += $loader->load($aspect); } return $loadedItems; @@ -65,28 +66,30 @@ public function load(Aspect $aspect): array /** * Loads and register all items of aspect in the container + * + * @param ReflectionClass $aspect */ - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(ReflectionClass $aspect): void { $loadedItems = $this->load($aspect); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspect::class] = $aspect::class; + $this->loadedAspects[$aspect->name] = $aspect->name; } /** - * Returns list of unloaded aspects in the container + * Returns the aspects registered in the container that are not loaded yet * - * @return list + * @return list> */ public function getUnloadedAspects(): array { $unloadedAspects = []; - foreach ($this->container->getServicesByInterface(Aspect::class) as $aspect) { - if (!isset($this->loadedAspects[$aspect::class])) { - $unloadedAspects[] = $aspect; + foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { + if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { + $unloadedAspects[] = new ReflectionClass($aspectClass); } } diff --git a/src/Core/AspectLoaderExtension.php b/src/Core/AspectLoaderExtension.php index 6af6b8b0..5e48c2ea 100644 --- a/src/Core/AspectLoaderExtension.php +++ b/src/Core/AspectLoaderExtension.php @@ -25,12 +25,12 @@ interface AspectLoaderExtension /** * Loads definition from specific point of aspect into the container * - * @param T $aspect Instance of aspect - * @param ReflectionClass $reflectionAspect Reflection of aspect + * Only the class is read, the aspect is never created here: advices refer to it by class and method name, the + * aspect is resolved from the container when one of them runs for the first time. * - * @return array + * @param ReflectionClass $reflectionAspect Reflection of aspect * - * @template T of Aspect + * @return array */ - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array; + public function load(ReflectionClass $reflectionAspect): array; } diff --git a/src/Core/AspectLoaderInterface.php b/src/Core/AspectLoaderInterface.php index f0b6ce5a..4e2635c2 100644 --- a/src/Core/AspectLoaderInterface.php +++ b/src/Core/AspectLoaderInterface.php @@ -15,6 +15,7 @@ use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use ReflectionClass; /** * Contract for loading aspects into the container @@ -26,21 +27,36 @@ interface AspectLoaderInterface /** * Loads an aspect with the help of aspect loaders, but don't register it in the container * + * Only the class is read: the aspect itself is never created, its advices resolve it from the container when + * they run for the first time. + * * @see loadAndRegister() method for registration * + * @internal + * + * @param ReflectionClass $aspect + * * @return array */ - public function load(Aspect $aspect): array; + public function load(ReflectionClass $aspect): array; /** * Loads and register all items of aspect in the container + * + * @internal + * + * @param ReflectionClass $aspect */ - public function loadAndRegister(Aspect $aspect): void; + public function loadAndRegister(ReflectionClass $aspect): void; /** - * Returns list of unloaded aspects in the container + * Returns the aspects registered in the container that are not loaded yet + * + * The reflection of each aspect is created once here and passed on to the loaders. + * + * @internal * - * @return list + * @return list> */ public function getUnloadedAspects(): array; } diff --git a/src/Core/AttributeAspectLoaderExtension.php b/src/Core/AttributeAspectLoaderExtension.php index 5aeac85a..994ac596 100644 --- a/src/Core/AttributeAspectLoaderExtension.php +++ b/src/Core/AttributeAspectLoaderExtension.php @@ -14,11 +14,8 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; -use Go\Aop\Framework\AfterInterceptor; -use Go\Aop\Framework\AfterThrowingInterceptor; -use Go\Aop\Framework\AroundInterceptor; -use Go\Aop\Framework\BeforeInterceptor; -use Go\Aop\Intercept\Interceptor; +use Go\Aop\Framework\Interceptor; +use Go\Aop\Intercept\Interceptor as InterceptorInterface; use Go\Aop\Support\GenericPointcutAdvisor; use Go\Lang\Attribute; use Go\Lang\Attribute\After; @@ -41,7 +38,7 @@ final class AttributeAspectLoaderExtension extends AbstractAspectLoaderExtension */ public const string ADVISOR_ID_SUFFIX = '.advice'; - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { $loadedItems = []; foreach ($reflectionAspect->getMethods() as $aspectMethod) { @@ -56,12 +53,12 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array foreach ($pointcutAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $loadedItems[$methodId] = $this->parsePointcut($aspect, $aspectMethod, $attribute->expression); + $loadedItems[$methodId] = $this->parsePointcut($reflectionAspect, $aspectMethod, $attribute->expression); } foreach ($adviceAttributes as $reflectionAttribute) { $attribute = $reflectionAttribute->newInstance(); - $pointcut = $this->parsePointcut($aspect, $aspectMethod, $attribute->expression); - $interceptor = $this->getAdvice($attribute, $aspect, $aspectMethod); + $pointcut = $this->parsePointcut($reflectionAspect, $aspectMethod, $attribute->expression); + $interceptor = $this->getAdvice($attribute, $reflectionAspect, $aspectMethod); $advisorId = $pointcutAttributes === [] ? $methodId : $methodId . self::ADVISOR_ID_SUFFIX; $loadedItems[$advisorId] = new GenericPointcutAdvisor($pointcut, $interceptor); @@ -72,27 +69,34 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array } /** - * Returns an advice (interceptor) instance by meta-type attribute and closure + * Returns an advice (interceptor) for the advice method, without creating the aspect + * + * The interceptor is the lazy form of the {@see Interceptor} facade: it carries the aspect class, the method name, + * the order and the expression as plain data, and resolves the aspect from the container on first use only. + * + * @param ReflectionClass $aspect * * @throws AspectException If the advice method is not public or the attribute is unsupported */ protected function getAdvice( AbstractInterceptor $interceptorAttribute, - Aspect $aspect, + ReflectionClass $aspect, ReflectionMethod $aspectMethod, - ): Interceptor { + ): InterceptorInterface { if (!$aspectMethod->isPublic()) { throw AspectException::nonPublicAdviceMethod($aspectMethod); } - $adviceCallback = $aspectMethod->getClosure($aspect); - $adviceOrder = $interceptorAttribute->order; - $pointcutExpression = $interceptorAttribute->expression; + $aspectClass = $aspect->name; + $methodName = $aspectMethod->getName(); + $order = $interceptorAttribute->order; + $expression = $interceptorAttribute->expression; + return match (true) { - $interceptorAttribute instanceof Before => new BeforeInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof After => new AfterInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof Around => new AroundInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), - $interceptorAttribute instanceof AfterThrowing => new AfterThrowingInterceptor($adviceCallback, $adviceOrder, $pointcutExpression), + $interceptorAttribute instanceof Before => Interceptor::before($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof After => Interceptor::after($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof Around => Interceptor::around($aspectClass, $methodName, $order, $expression), + $interceptorAttribute instanceof AfterThrowing => Interceptor::afterThrowing($aspectClass, $methodName, $order, $expression), default => throw AspectException::unsupportedAdviceAttribute($interceptorAttribute), }; } diff --git a/src/Core/Cache/CachedAspectLoader.php b/src/Core/Cache/CachedAspectLoader.php index c7e0c88c..da04515a 100644 --- a/src/Core/Cache/CachedAspectLoader.php +++ b/src/Core/Cache/CachedAspectLoader.php @@ -94,14 +94,13 @@ public function __construct( $this->advisorCacheCompiler = new AdvisorCacheCompiler(); } - public function load(Aspect $aspect): array + public function load(ReflectionClass $aspect): array { if ($this->cacheDir === null || $this->cacheDir === '') { return $this->loader->load($aspect); } - $refAspect = new ReflectionClass($aspect); - $aspectFileName = $refAspect->getFileName(); + $aspectFileName = $aspect->getFileName(); $cacheFileName = $aspectFileName !== false ? $this->resolveCacheFileName($aspectFileName) : null; if ($aspectFileName === false || $cacheFileName === null) { // Aspects without a resolvable source file below the application root are not cached @@ -127,7 +126,7 @@ public function load(Aspect $aspect): array } $loadedItems = $this->loader->load($aspect); - $this->saveToCache($refAspect->getName(), $loadedItems, $cacheFileName); + $this->saveToCache($aspect->name, $loadedItems, $cacheFileName); return $loadedItems; } @@ -135,27 +134,27 @@ public function load(Aspect $aspect): array /** * Loads and register all items of aspect in the container */ - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(ReflectionClass $aspect): void { $loadedItems = $this->load($aspect); foreach ($loadedItems as $itemId => $item) { $this->container->add($itemId, $item); } - $this->loadedAspects[$aspect::class] = $aspect::class; + $this->loadedAspects[$aspect->name] = $aspect->name; } /** - * Returns list of unloaded aspects in the container + * Returns the aspects registered in the container that are not loaded yet * - * @return list + * @return list> */ public function getUnloadedAspects(): array { $unloadedAspects = []; - foreach ($this->container->getServicesByInterface(Aspect::class) as $aspect) { - if (!isset($this->loadedAspects[$aspect::class])) { - $unloadedAspects[] = $aspect; + foreach ($this->container->getServiceIdsByInterface(Aspect::class) as $aspectClass) { + if (!isset($this->loadedAspects[$aspectClass]) && is_subclass_of($aspectClass, Aspect::class)) { + $unloadedAspects[] = new ReflectionClass($aspectClass); } } diff --git a/src/Core/Container.php b/src/Core/Container.php index 5c143678..e6a97331 100644 --- a/src/Core/Container.php +++ b/src/Core/Container.php @@ -224,6 +224,19 @@ public function getServicesByInterface(string $interfaceTagClassName): array return $values; } + public function getServiceIdsByInterface(string $interfaceTagClassName): array + { + $ids = $this->tags[$interfaceTagClassName] ?? []; + // A snapshot of the ids: the is_subclass_of() autoload can re-enter the container (see above) + foreach (array_keys($this->factories) as $id) { + if (is_subclass_of($id, $interfaceTagClassName) && !in_array($id, $ids, true)) { + $ids[] = $id; + } + } + + return $ids; + } + /** * Materializes a deferred service into a container entry and tags it by its interfaces. * diff --git a/src/Core/IntroductionAspectExtension.php b/src/Core/IntroductionAspectExtension.php index ebe8fdea..0be6d422 100644 --- a/src/Core/IntroductionAspectExtension.php +++ b/src/Core/IntroductionAspectExtension.php @@ -29,7 +29,7 @@ */ class IntroductionAspectExtension extends AbstractAspectLoaderExtension { - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { $loadedItems = []; foreach ($reflectionAspect->getProperties() as $aspectProperty) { @@ -43,13 +43,13 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array ? $reflectionAttribute->newInstance() : null; if ($attribute instanceof DeclareParents) { - $pointcut = $this->parsePointcut($aspect, $aspectProperty, $attribute->expression); + $pointcut = $this->parsePointcut($reflectionAspect, $aspectProperty, $attribute->expression); // Introduction doesn't have own syntax and uses any suitable class-filter $pointcut = new Pointcut\AndPointcut( Pointcut::KIND_INTRODUCTION | Pointcut::KIND_CLASS, $pointcut, ); - $advice = $this->getAdvice($attribute, $aspect, $aspectProperty); + $advice = $this->getAdvice($attribute, $aspectProperty); $advisor = new GenericPointcutAdvisor($pointcut, $advice); $loadedItems[$propertyId] = $advisor; @@ -69,7 +69,6 @@ public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array */ protected function getAdvice( AbstractAttribute $interceptorAttribute, - Aspect $aspect, ReflectionProperty $aspectProperty, ): Advice { return match (true) { diff --git a/src/Core/NativeLazyProxy.php b/src/Core/NativeLazyProxy.php index 226ece8f..f1a6797c 100644 --- a/src/Core/NativeLazyProxy.php +++ b/src/Core/NativeLazyProxy.php @@ -16,6 +16,7 @@ use Error; use ReflectionClass; use ReflectionException; +use ReflectionProperty; /** * Creates native lazy proxies ({@see ReflectionClass::newLazyProxy()}) through a single @@ -46,6 +47,11 @@ private function __construct() {} */ private static array $unsupported = []; + /** + * @var array> Properties set by {@see create()}, by class and name + */ + private static array $rawProperties = []; + /** * Creates a lazy proxy of the class, or returns null when PHP cannot make the class * lazy - the caller is expected to fall back to eager construction @@ -86,15 +92,33 @@ public static function tryCreate(string $className, Closure $initializer): ?obje * Creates a lazy proxy of a class known to be proxy-compatible by construction * (framework-owned classes), skipping the probe and the memo * + * The properties given in $rawValues are set on the proxy without initializing it, so they can be read + * without running the initializer. The initializer must give the real instance the same values: once the + * proxy is initialized, reads go to the real instance. At least one property has to stay unset, or PHP + * marks the proxy as initialized and the initializer never runs. + * * @template T of object * - * @param class-string $className - * @param Closure(T): T $initializer + * @param class-string $className + * @param Closure(T): T $initializer + * @param array $rawValues Values of properties, by property name * * @return T */ - public static function create(string $className, Closure $initializer): object + public static function create(string $className, Closure $initializer, array $rawValues = []): object { - return new ReflectionClass($className)->newLazyProxy($initializer); + $reflection = new ReflectionClass($className); + $proxy = $reflection->newLazyProxy($initializer); + foreach ($rawValues as $propertyName => $value) { + // Taken from the declaring class: PHP refuses to set an inherited private(set) property in the scope + // of a child class + $property = self::$rawProperties[$className][$propertyName] ??= new ReflectionProperty( + $reflection->getProperty($propertyName)->class, + $propertyName, + ); + $property->setRawValueWithoutLazyInitialization($proxy, $value); + } + + return $proxy; } } diff --git a/src/Instrument/Transformer/WeavingTransformer.php b/src/Instrument/Transformer/WeavingTransformer.php index eb7c1fcf..d9550ef1 100644 --- a/src/Instrument/Transformer/WeavingTransformer.php +++ b/src/Instrument/Transformer/WeavingTransformer.php @@ -1103,7 +1103,7 @@ private static function removeCachePath(string $path): void /** * Utility method to load and register unloaded aspects * - * @param Aspect[] $unloadedAspects List of unloaded aspects + * @param list<\ReflectionClass> $unloadedAspects Reflections of the unloaded aspects */ private function loadAndRegisterAspects(array $unloadedAspects): void { diff --git a/tests/Aop/Framework/InterceptorTest.php b/tests/Aop/Framework/InterceptorTest.php index 6d722ff9..9dad9dbe 100644 --- a/tests/Aop/Framework/InterceptorTest.php +++ b/tests/Aop/Framework/InterceptorTest.php @@ -44,6 +44,25 @@ public function testCreatesBeforeInterceptor(): void $this->assertSame(10, $interceptor->adviceOrder); } + public function testLazyInterceptorDescribesItsAdviceWithoutCreatingTheAspect(): void + { + $interceptor = Interceptor::around(InterceptorTestAspect::class, 'recordInvocation', 7, 'execution(public Foo->bar(*))'); + $reflection = new ReflectionClass(AroundInterceptor::class); + + $this->assertSame([InterceptorTestAspect::class, 'recordInvocation'], $interceptor->aspectMethod); + $this->assertSame(7, $interceptor->adviceOrder); + $this->assertSame('execution(public Foo->bar(*))', $interceptor->pointcutExpression); + + // What the weaver and the advisor cache read about the advice does not initialize the interceptor + $descriptor = GeneratedInterceptor::fromAdvice('advisor.id', $interceptor); + $this->assertSame(InterceptorTestAspect::class, $descriptor->aspectClass); + $this->assertSame('recordInvocation', $descriptor->adviceMethod); + $this->assertSame(7, $descriptor->order); + $this->assertFalse($descriptor->usesContainerAdvice); + $interceptor->compileToPhp(); + $this->assertTrue($reflection->isUninitializedLazyObject($interceptor), 'No aspect may be resolved'); + } + public function testAspectClassRequiresAdviceMethodName(): void { try { diff --git a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php index a14efc69..b9bcb7d3 100644 --- a/tests/Console/Command/DebugAdvisorCommandInProcessTest.php +++ b/tests/Console/Command/DebugAdvisorCommandInProcessTest.php @@ -75,7 +75,7 @@ public function testExecuteListsRegisteredAdvisors(): void public function testShowAdvisorsListRegistersUnloadedAspects(): void { - $aspect = $this->createStub(\Go\Aop\Aspect::class); + $aspect = new ReflectionClass($this->createStub(\Go\Aop\Aspect::class)); $aspectLoader = $this->createMock(CachedAspectLoader::class); $aspectLoader->method('getUnloadedAspects')->willReturn([$aspect]); diff --git a/tests/Console/Command/DebugAspectCommandInProcessTest.php b/tests/Console/Command/DebugAspectCommandInProcessTest.php index f4a10319..ddfe5961 100644 --- a/tests/Console/Command/DebugAspectCommandInProcessTest.php +++ b/tests/Console/Command/DebugAspectCommandInProcessTest.php @@ -20,7 +20,6 @@ use Go\Core\AspectLoader; use Go\Tests\TestProject\Aspect\LoggingAspect; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; use stdClass; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -49,7 +48,7 @@ public function testExecuteListsEnabledAspects(): void { $container = $this->createMock(AspectContainer::class); $container->expects($this->once()) - ->method('getServicesByInterface') + ->method('getServiceIdsByInterface') ->with(Aspect::class) ->willReturn([]); @@ -70,7 +69,7 @@ public function testExecuteWithUnknownAspectOptionFailsWithSuggestion(): void { $container = $this->createStub(AspectContainer::class); $container->method('has')->willReturn(false); - $container->method('getServicesByInterface')->willReturn([LoggingAspect::class => new LoggingAspect($this->createStub(LoggerInterface::class))]); + $container->method('getServiceIdsByInterface')->willReturn([LoggingAspect::class]); $kernel = $this->createStub(AspectKernel::class); $kernel->method('getContainer')->willReturn($container); @@ -89,9 +88,6 @@ public function testExecuteWithUnknownAspectOptionFailsWithSuggestion(): void public function testExecuteFiltersByAspectOptionAndShowsPointcutsAndAdvisors(): void { - $logger = $this->createStub(LoggerInterface::class); - $aspect = new LoggingAspect($logger); - $aspectLoader = $this->createStub(AspectLoader::class); $aspectLoader->method('load')->willReturn([ 'a.pointcut' => $this->createStub(Pointcut::class), @@ -101,9 +97,9 @@ public function testExecuteFiltersByAspectOptionAndShowsPointcutsAndAdvisors(): $container = $this->createStub(AspectContainer::class); $container->method('has')->willReturn(true); + // The aspect is read from its class, never created $container->method('getService')->willReturnMap([ [AspectLoader::class, $aspectLoader], - [LoggingAspect::class, $aspect], ]); $kernel = $this->createStub(AspectKernel::class); diff --git a/tests/Core/AbstractAspectLoaderExtensionTest.php b/tests/Core/AbstractAspectLoaderExtensionTest.php index 89e268be..fb7efb26 100644 --- a/tests/Core/AbstractAspectLoaderExtensionTest.php +++ b/tests/Core/AbstractAspectLoaderExtensionTest.php @@ -40,7 +40,7 @@ public function testParsesValidPointcutExpressionForMethodReflection(): void $aspect = new AbstractAspectLoaderExtensionTestAspect(); $reflection = new ReflectionMethod($aspect, 'someMethod'); - $pointcut = $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->bar(*))'); + $pointcut = $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->bar(*))'); // @phpstan-ignore method.alreadyNarrowedType (runtime double-check of the declared return type) $this->assertInstanceOf(Pointcut::class, $pointcut); @@ -53,7 +53,7 @@ public function testResolvesThisInPointcutExpressionToAspectClassName(): void // "$this" must be replaced with the aspect's own class name before lexing $pointcut = $this->extension->doParsePointcut( - $aspect, + new ReflectionClass($aspect), $reflection, 'execution(public $this->someMethod(*))', ); @@ -70,7 +70,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForMethodReflect $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyReflection(): void @@ -81,7 +81,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForPropertyRefle $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflection(): void @@ -92,7 +92,7 @@ public function testThrowsUnexpectedValueExceptionOnLexicalErrorForClassReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Can not recognize the lexical structure/'); - $this->extension->doParsePointcut($aspect, $reflection, 'execution(public Foo->@#$%^bar(*))'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'execution(public Foo->@#$%^bar(*))'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflection(): void @@ -103,7 +103,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForMethodReflecti $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflection(): void @@ -114,7 +114,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForPropertyReflec $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflection(): void @@ -125,7 +125,7 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflectio $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessageMatches('/Unexpected token/'); - $this->extension->doParsePointcut($aspect, $reflection, 'public execution(*)'); + $this->extension->doParsePointcut(new ReflectionClass($aspect), $reflection, 'public execution(*)'); } } @@ -134,17 +134,18 @@ public function testThrowsUnexpectedValueExceptionOnParserErrorForClassReflectio */ final class AbstractAspectLoaderExtensionTestExtension extends AbstractAspectLoaderExtension implements AspectLoaderExtension { - public function load(Aspect $aspect, ReflectionClass $reflectionAspect): array + public function load(ReflectionClass $reflectionAspect): array { return []; } /** * @template T of Aspect + * @param ReflectionClass $aspect * @param ReflectionMethod|ReflectionProperty|ReflectionClass $reflection */ public function doParsePointcut( - Aspect $aspect, + ReflectionClass $aspect, ReflectionMethod|ReflectionProperty|ReflectionClass $reflection, string $pointcutExpression, ): Pointcut { diff --git a/tests/Core/AdvisorIndexTest.php b/tests/Core/AdvisorIndexTest.php index d59dcf2f..6815725d 100644 --- a/tests/Core/AdvisorIndexTest.php +++ b/tests/Core/AdvisorIndexTest.php @@ -15,9 +15,15 @@ use Go\Aop\Advice; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use Go\Aop\Pointcut\PointcutGrammar; +use Go\Aop\Pointcut\PointcutLexer; +use Go\Aop\Pointcut\PointcutParser; use Go\Aop\Pointcut\TruePointcut; use Go\Aop\Support\GenericPointcutAdvisor; +use Go\Lang\Attribute\Before; +use LogicException; use PHPUnit\Framework\TestCase; +use ReflectionClass; final class AdvisorIndexTest extends TestCase { @@ -63,21 +69,22 @@ public function testLoadsUnloadedAspectsBeforeCollectingPointcutKinds(): void $container = new Container(); $aspect = new class implements Aspect {}; $advisor = $this->createAdvisor(Pointcut::KIND_INIT | Pointcut::KIND_CLASS); - $loader = new class ($container, $aspect, $advisor) implements AspectLoaderInterface { + $loader = new class ($container, $aspect::class, $advisor) implements AspectLoaderInterface { private bool $isLoaded = false; public function __construct( private readonly AspectContainer $container, - private readonly Aspect $aspect, + /** @var class-string */ + private readonly string $aspectClass, private readonly GenericPointcutAdvisor $advisor, ) {} - public function load(Aspect $aspect): array + public function load(ReflectionClass $aspect): array { return ['advisor.lazy' => $this->advisor]; } - public function loadAndRegister(Aspect $aspect): void + public function loadAndRegister(ReflectionClass $aspect): void { foreach ($this->load($aspect) as $itemId => $item) { $this->container->add($itemId, $item); @@ -87,7 +94,7 @@ public function loadAndRegister(Aspect $aspect): void public function getUnloadedAspects(): array { - return $this->isLoaded ? [] : [$this->aspect]; + return $this->isLoaded ? [] : [new ReflectionClass($this->aspectClass)]; } }; $index = new AdvisorIndex($container, $loader); @@ -96,6 +103,27 @@ public function getUnloadedAspects(): array $this->assertSame(['advisor.lazy' => $advisor], $index->getAdvisors()); } + /** + * Weaving reads aspects by class: even an aspect PHP can not make lazy (no instance properties) is not created, + * its factory runs on the first call of one of its advices only + */ + public function testLoadsDeferredAspectsWithoutRunningTheirFactories(): void + { + $container = new Container(); + $container->addLazyService( + AdvisorIndexTestStatelessAspect::class, + static fn(): never => throw new LogicException('Weaving must not create the aspect'), + ); + $loader = new AspectLoader( + $container, + new AttributeAspectLoaderExtension(new PointcutLexer(), new PointcutParser(new PointcutGrammar())), + ); + $index = new AdvisorIndex($container, $loader); + + $this->assertTrue($index->hasPointcutKind(Pointcut::KIND_METHOD)); + $this->assertArrayHasKey(AdvisorIndexTestStatelessAspect::class . '->beforeMethod', $index->getAdvisors()); + } + private function createAdvisor(int $pointcutKind): GenericPointcutAdvisor { return new GenericPointcutAdvisor(new TruePointcut($pointcutKind), $this->createStub(Advice::class)); @@ -109,3 +137,9 @@ private function createLoader(): AspectLoaderInterface return $loader; } } + +final class AdvisorIndexTestStatelessAspect implements Aspect +{ + #[Before('execution(public NonExistent\\**->*(*))')] + public function beforeMethod(): void {} +} diff --git a/tests/Core/AspectLoaderTest.php b/tests/Core/AspectLoaderTest.php index cbf26ce7..b3d7e74f 100644 --- a/tests/Core/AspectLoaderTest.php +++ b/tests/Core/AspectLoaderTest.php @@ -15,7 +15,9 @@ use Go\Aop\Advisor; use Go\Aop\Aspect; use Go\Aop\Pointcut; +use LogicException; use PHPUnit\Framework\TestCase; +use ReflectionClass; #[\PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations] class AspectLoaderTest extends TestCase @@ -25,7 +27,7 @@ public function testLoadReturnsEmptyArrayWithoutRegisteredLoaders(): void $container = new Container(); $loader = new AspectLoader($container); - $items = $loader->load(new AspectLoaderTestAspectOne()); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame([], $items); } @@ -45,8 +47,7 @@ public function testLoadCombinesItemsFromAllLoaderExtensions(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $aspect = new AspectLoaderTestAspectOne(); - $items = $loader->load($aspect); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame( ['item.one' => $pointcutOne, 'item.two' => $pointcutTwo], @@ -69,7 +70,7 @@ public function testLoadKeepsFirstLoaderItemWhenKeysCollide(): void $loader = new AspectLoader($container, $extensionOne, $extensionTwo); - $items = $loader->load(new AspectLoaderTestAspectOne()); + $items = $loader->load(new ReflectionClass(AspectLoaderTestAspectOne::class)); // array union (+=) semantics: the first loader's item for a colliding key wins $this->assertSame($pointcutFromFirst, $items['shared.key']); @@ -84,7 +85,7 @@ public function testLoadAndRegisterAddsLoadedItemsToContainer(): void $extension->method('load')->willReturn(['aspect.advisor' => $advisor]); $loader = new AspectLoader($container, $extension); - $loader->loadAndRegister(new AspectLoaderTestAspectOne()); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); $this->assertSame($advisor, $container->get('aspect.advisor')); } @@ -92,23 +93,47 @@ public function testLoadAndRegisterAddsLoadedItemsToContainer(): void public function testGetUnloadedAspectsReturnsOnlyAspectsNotYetLoaded(): void { $container = new Container(); - $aspectOne = new AspectLoaderTestAspectOne(); - $aspectTwo = new AspectLoaderTestAspectTwo(); - - $container->add(AspectLoaderTestAspectOne::class, $aspectOne); - $container->add(AspectLoaderTestAspectTwo::class, $aspectTwo); + $container->add(AspectLoaderTestAspectOne::class, new AspectLoaderTestAspectOne()); + $container->add(AspectLoaderTestAspectTwo::class, new AspectLoaderTestAspectTwo()); $loader = new AspectLoader($container); // Before anything is loaded, both aspects are unloaded $this->assertSame( - [$aspectOne, $aspectTwo], - $loader->getUnloadedAspects(), + [AspectLoaderTestAspectOne::class, AspectLoaderTestAspectTwo::class], + self::names($loader->getUnloadedAspects()), ); - $loader->loadAndRegister($aspectOne); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); + + $this->assertSame([AspectLoaderTestAspectTwo::class], self::names($loader->getUnloadedAspects())); + } - $this->assertSame([$aspectTwo], $loader->getUnloadedAspects()); + public function testDeferredAspectsAreLoadedWithoutCreatingThem(): void + { + $container = new Container(); + $container->addLazyService( + AspectLoaderTestAspectOne::class, + static fn(): never => throw new LogicException('The aspect must not be created to load it'), + ); + $extension = $this->createMock(AspectLoaderExtension::class); + $extension->method('load')->willReturn([]); + $loader = new AspectLoader($container, $extension); + + $this->assertSame([AspectLoaderTestAspectOne::class], self::names($loader->getUnloadedAspects())); + $loader->loadAndRegister(new ReflectionClass(AspectLoaderTestAspectOne::class)); + + $this->assertSame([], $loader->getUnloadedAspects()); + } + + /** + * @param list> $aspects + * + * @return list> + */ + private static function names(array $aspects): array + { + return array_map(static fn(ReflectionClass $aspect): string => $aspect->name, $aspects); } } diff --git a/tests/Core/AttributeAspectLoaderExtensionTest.php b/tests/Core/AttributeAspectLoaderExtensionTest.php index eeadfa47..12c39dae 100644 --- a/tests/Core/AttributeAspectLoaderExtensionTest.php +++ b/tests/Core/AttributeAspectLoaderExtensionTest.php @@ -8,16 +8,19 @@ use Go\Aop\Aspect; use Go\Aop\Exception\AspectException; use Go\Aop\Exception\PointcutSyntaxException; +use Go\Aop\Framework\AbstractInterceptor as AbstractInterceptorClass; use Go\Aop\Pointcut; use Go\Aop\Pointcut\PointcutGrammar; use Go\Aop\Pointcut\PointcutLexer; use Go\Aop\Pointcut\PointcutParser; use Go\Aop\Support\GenericPointcutAdvisor; +use Go\Lang\Attribute\AbstractInterceptor as AbstractInterceptorAttribute; use Go\Lang\Attribute\After; use Go\Lang\Attribute\Before; use Go\Lang\Attribute\Pointcut as PointcutAttribute; use Go\Stubs\AttributeAspectLoaderExtensionTestPrivateAspect; use Go\Stubs\AttributeAspectLoaderExtensionTestPublicAspect; +use LogicException; use PHPUnit\Framework\TestCase; use ReflectionClass; @@ -33,11 +36,37 @@ protected function setUp(): void public function testLoadsAdvisorForPublicAdviceMethod(): void { $aspect = new AttributeAspectLoaderExtensionTestPublicAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertArrayHasKey($aspect::class . '->publicAdvice', $loadedItems); } + public function testLoadsAdvicesWithoutCreatingTheAspect(): void + { + $loadedItems = $this->extension->load(new ReflectionClass(AttributeAspectLoaderExtensionTestUncreatableAspect::class)); + + $advisor = $loadedItems[AttributeAspectLoaderExtensionTestUncreatableAspect::class . '->advice'] ?? null; + $this->assertInstanceOf(GenericPointcutAdvisor::class, $advisor); + $interceptor = $advisor->getAdvice(); + $this->assertInstanceOf(AbstractInterceptorClass::class, $interceptor); + $this->assertSame([AttributeAspectLoaderExtensionTestUncreatableAspect::class, 'advice'], $interceptor->aspectMethod); + $this->assertSame(3, $interceptor->adviceOrder); + $this->assertTrue(new ReflectionClass($interceptor::class)->isUninitializedLazyObject($interceptor)); + } + + public function testUnsupportedAdviceAttributeIsReported(): void + { + try { + $this->extension->load(new ReflectionClass(AttributeAspectLoaderExtensionTestUnknownAdviceAspect::class)); + $this->fail('An advice attribute no interceptor handles must be rejected'); + } catch (AspectException $exception) { + $this->assertSame( + 'Unsupported method meta class: ' . AttributeAspectLoaderExtensionTestUnknownAdvice::class, + $exception->getMessage(), + ); + } + } + public function testRejectsNonPublicAdviceMethod(): void { $aspect = new AttributeAspectLoaderExtensionTestPrivateAspect(); @@ -45,13 +74,13 @@ public function testRejectsNonPublicAdviceMethod(): void $this->expectException(AspectException::class); $this->expectExceptionMessage('first-class advice callables require all advice methods to be public'); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testForeignAttributeOnAdviceMethodIsIgnored(): void { $aspect = new AttributeAspectLoaderExtensionTestForeignAttributeAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertSame([$aspect::class . '->advice'], array_keys($loadedItems)); } @@ -59,7 +88,7 @@ public function testForeignAttributeOnAdviceMethodIsIgnored(): void public function testPointcutAndAdviceOnOneMethodAreBothLoaded(): void { $aspect = new AttributeAspectLoaderExtensionTestPointcutAndAdviceAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $methodId = $aspect::class . '->pointcutAndAdvice'; $this->assertInstanceOf(Pointcut::class, $loadedItems[$methodId] ?? null); @@ -78,7 +107,7 @@ public function testSecondAdviceAttributeOnOneMethodFailsLoudly(): void 'Advice method ' . AttributeAspectLoaderExtensionTestTwoAdvicesAspect::class . '::twoAdvices() declares 2 advice attributes', ); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testPointcutSyntaxErrorCitesTheMethod(): void @@ -88,13 +117,39 @@ public function testPointcutSyntaxErrorCitesTheMethod(): void $this->expectException(PointcutSyntaxException::class); $this->expectExceptionMessage(AttributeAspectLoaderExtensionTestBrokenPointcutAspect::class . '->brokenAdvice'); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } } #[Attribute(Attribute::TARGET_METHOD)] final class AttributeAspectLoaderExtensionTestForeignAttribute {} +/** + * An advice attribute of the framework hierarchy that no interceptor type handles + */ +#[Attribute(Attribute::TARGET_METHOD)] +final class AttributeAspectLoaderExtensionTestUnknownAdvice extends AbstractInterceptorAttribute {} + +final class AttributeAspectLoaderExtensionTestUnknownAdviceAspect implements Aspect +{ + #[AttributeAspectLoaderExtensionTestUnknownAdvice('execution(public NonExistent\\**->*(*))')] + public function advice(): void {} +} + +/** + * Aspect without instance properties (PHP can not make it lazy) that must never be created by loading it + */ +final class AttributeAspectLoaderExtensionTestUncreatableAspect implements Aspect +{ + public function __construct() + { + throw new LogicException('Loading an aspect must not create it'); + } + + #[Before('execution(public NonExistent\**->*(*))', order: 3)] + public function advice(): void {} +} + final class AttributeAspectLoaderExtensionTestForeignAttributeAspect implements Aspect { #[AttributeAspectLoaderExtensionTestForeignAttribute] diff --git a/tests/Core/Cache/CachedAspectLoaderTest.php b/tests/Core/Cache/CachedAspectLoaderTest.php index 1f34d2a6..b1cfe9bf 100644 --- a/tests/Core/Cache/CachedAspectLoaderTest.php +++ b/tests/Core/Cache/CachedAspectLoaderTest.php @@ -24,6 +24,7 @@ use Go\Core\Container; use Go\VirtualFileSystem\FileSystem; use PHPUnit\Framework\TestCase; +use ReflectionClass; #[\PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations] class CachedAspectLoaderTest extends TestCase @@ -38,7 +39,8 @@ class CachedAspectLoaderTest extends TestCase private string $cacheDir; - private Aspect $aspect; + /** @var ReflectionClass */ + private ReflectionClass $aspect; /** * Path of the fake aspect source file below the application root @@ -71,9 +73,8 @@ protected function setUp(): void ); require $this->aspectFileName; $className = 'Go\\Core\\TestFixture\\' . $shortClassName; - $aspect = new $className(); - assert($aspect instanceof Aspect); - $this->aspect = $aspect; + assert(is_subclass_of($className, Aspect::class)); + $this->aspect = new ReflectionClass($className); // The naming scheme: app-root prefix replaced by the cache dir, '.php' by '.cache.php' $this->cacheFileName = $this->cacheDir . '/src/Aspect/' . $shortClassName . '.cache.php'; @@ -313,7 +314,7 @@ public function testAspectOutsideApplicationRootIsLoadedDirectlyWithoutCaching() $loadedItems = ['pc' => new TruePointcut()]; $this->innerLoader->expects($this->once())->method('load')->willReturn($loadedItems); - $this->assertSame($loadedItems, $loader->load($foreignAspect)); + $this->assertSame($loadedItems, $loader->load(new ReflectionClass($foreignAspect))); // No shadow file may appear anywhere below the cache directory $cacheEntries = scandir($this->cacheDir); diff --git a/tests/Core/ContainerTest.php b/tests/Core/ContainerTest.php index f0806f97..a2bf7cdd 100644 --- a/tests/Core/ContainerTest.php +++ b/tests/Core/ContainerTest.php @@ -432,6 +432,28 @@ function () use (&$constructed): StatefulTestAspect { $this->assertFalse((new \ReflectionClass(StatefulTestAspect::class))->isUninitializedLazyObject($aspect)); } + public function testServiceIdsByInterfaceNeverCreateServices(): void + { + $constructed = false; + $this->container->add(StatefulTestAspect::class, new StatefulTestAspect(1)); + // A property-less class: materializing it would run the factory right away + $this->container->addLazyService( + DoSomethingAspect::class, + function () use (&$constructed): DoSomethingAspect { + $constructed = true; + + return new DoSomethingAspect(); + }, + ); + + $this->assertSame( + [StatefulTestAspect::class, DoSomethingAspect::class], + $this->container->getServiceIdsByInterface(Aspect::class), + ); + $this->assertFalse($constructed, 'Listing the ids must not run any factory'); + $this->assertSame([], $this->container->getServiceIdsByInterface(\Countable::class)); + } + public function testPropertylessServiceFallsBackToEagerConstruction(): void { // PHP creates lazy objects of property-less classes as already initialized, diff --git a/tests/Core/IntroductionAspectExtensionTest.php b/tests/Core/IntroductionAspectExtensionTest.php index 88938e06..5a7538dd 100644 --- a/tests/Core/IntroductionAspectExtensionTest.php +++ b/tests/Core/IntroductionAspectExtensionTest.php @@ -44,7 +44,7 @@ protected function setUp(): void public function testLoadsIntroductionAdvisorForDeclareParentsProperty(): void { $aspect = new IntroductionAspectExtensionTestAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $propertyId = IntroductionAspectExtensionTestAspect::class . '->introduction'; $this->assertArrayHasKey($propertyId, $loadedItems); @@ -68,7 +68,7 @@ public function testForeignAttributeOnPropertyIsIgnored(): void { $aspect = new IntroductionAspectExtensionTestForeignAttributeAspect(); - $this->assertSame([], $this->extension->load($aspect, new ReflectionClass($aspect))); + $this->assertSame([], $this->extension->load(new ReflectionClass($aspect))); } public function testThrowsForAdviceAttributeOnProperty(): void @@ -81,13 +81,13 @@ public function testThrowsForAdviceAttributeOnProperty(): void . IntroductionAspectExtensionTestInvalidAspect::class . '::$invalidIntroduction', ); - $this->extension->load($aspect, new ReflectionClass($aspect)); + $this->extension->load(new ReflectionClass($aspect)); } public function testReturnsEmptyArrayForAspectWithoutAnnotatedProperties(): void { $aspect = new IntroductionAspectExtensionTestEmptyAspect(); - $loadedItems = $this->extension->load($aspect, new ReflectionClass($aspect)); + $loadedItems = $this->extension->load(new ReflectionClass($aspect)); $this->assertSame([], $loadedItems); } @@ -97,10 +97,9 @@ public function testGetAdviceThrowsForUnsupportedAttribute(): void $extension = new class (new PointcutLexer(), new PointcutParser(new PointcutGrammar())) extends IntroductionAspectExtension { public function doGetAdvice( AbstractAttribute $interceptorAttribute, - Aspect $aspect, ReflectionProperty $aspectProperty, ): Advice { - return $this->getAdvice($interceptorAttribute, $aspect, $aspectProperty); + return $this->getAdvice($interceptorAttribute, $aspectProperty); } }; @@ -112,7 +111,7 @@ public function doGetAdvice( $this->expectException(AspectException::class); $this->expectExceptionMessage('Unsupported attribute class: ' . $unsupportedAttribute::class); - $extension->doGetAdvice($unsupportedAttribute, $aspect, $reflection); + $extension->doGetAdvice($unsupportedAttribute, $reflection); } } diff --git a/tests/Core/NativeLazyProxyTest.php b/tests/Core/NativeLazyProxyTest.php index 020f711b..5805aaf3 100644 --- a/tests/Core/NativeLazyProxyTest.php +++ b/tests/Core/NativeLazyProxyTest.php @@ -87,6 +87,22 @@ public function testTrustedCreateSkipsProbe(): void $this->assertTrue((new ReflectionClass(LazyProxyFixture::class))->isUninitializedLazyObject($proxy)); $this->assertSame(7, $proxy->getValue()); } + + public function testTrustedCreateSetsRawValuesWithoutInitializing(): void + { + $proxy = NativeLazyProxy::create( + RawValueChildFixture::class, + static fn(): RawValueChildFixture => new RawValueChildFixture('described', 7), + ['description' => 'described'], + ); + $reflection = new ReflectionClass(RawValueChildFixture::class); + + // An inherited private(set) property, which PHP refuses to set in the scope of the child class + $this->assertSame('described', $proxy->description); + $this->assertTrue($reflection->isUninitializedLazyObject($proxy)); + $this->assertSame(7, $proxy->getValue()); + $this->assertFalse($reflection->isUninitializedLazyObject($proxy)); + } } class LazyProxyFixture @@ -99,6 +115,18 @@ public function getValue(): int } } +abstract class RawValueParentFixture +{ + public function __construct(public private(set) string $description, private readonly int $value) {} + + public function getValue(): int + { + return $this->value; + } +} + +final class RawValueChildFixture extends RawValueParentFixture {} + class PropertylessFixture { public function doNothing(): void {} diff --git a/tests/Instrument/Transformer/WeavingTransformerTest.php b/tests/Instrument/Transformer/WeavingTransformerTest.php index 0d9fd39f..d61356a3 100644 --- a/tests/Instrument/Transformer/WeavingTransformerTest.php +++ b/tests/Instrument/Transformer/WeavingTransformerTest.php @@ -244,7 +244,7 @@ public function testUnadvisedClassWithUnknownParentIsNotTransformed(): void */ public function testLoadsUnloadedAspectsBeforeMatching(): void { - $aspect = $this->createStub(Aspect::class); + $aspect = new ReflectionClass($this->createStub(Aspect::class)); $loader = $this->createMock(AspectLoader::class); $loader->method('getUnloadedAspects')->willReturn([$aspect]); $loader->expects($this->once())->method('loadAndRegister')->with($aspect);