From 5539c8b60f47a1fdc5bdd4fad94f5fbda9250420 Mon Sep 17 00:00:00 2001 From: Mark Shust Date: Sat, 4 Apr 2026 21:46:33 -0400 Subject: [PATCH] feat(container): integrate plugin interception into container resolution Wire PluginInterceptor into Container::resolve() so plugins actually intercept method calls on resolved objects. Previously, plugins were discovered and registered but never applied during resolution. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...001-add-plugin-interceptor-to-container.md | 26 ++ .../002-wrap-resolved-instances-with-proxy.md | 33 ++ .../003-wire-interceptor-in-application.md | 34 ++ .../004-integration-test-end-to-end.md | 29 ++ .../005-prevent-plugins-targeting-plugins.md | 25 ++ .../_devils_advocate.md | 95 +++++ .../container-plugin-integration/_plan.md | 74 ++++ packages/core/src/Application.php | 25 +- packages/core/src/Container/Container.php | 16 + .../core/src/Exceptions/PluginException.php | 11 + packages/core/src/Plugin/PluginProxy.php | 10 + packages/core/src/Plugin/PluginRegistry.php | 10 + packages/core/tests/Unit/ApplicationTest.php | 233 ++++++++++++ .../tests/Unit/Container/ContainerTest.php | 354 ++++++++++++++++++ .../tests/Unit/Plugin/PluginRegistryTest.php | 106 ++++-- packages/routing/src/Router.php | 7 +- 16 files changed, 1056 insertions(+), 32 deletions(-) create mode 100644 .claude/plans/container-plugin-integration/001-add-plugin-interceptor-to-container.md create mode 100644 .claude/plans/container-plugin-integration/002-wrap-resolved-instances-with-proxy.md create mode 100644 .claude/plans/container-plugin-integration/003-wire-interceptor-in-application.md create mode 100644 .claude/plans/container-plugin-integration/004-integration-test-end-to-end.md create mode 100644 .claude/plans/container-plugin-integration/005-prevent-plugins-targeting-plugins.md create mode 100644 .claude/plans/container-plugin-integration/_devils_advocate.md create mode 100644 .claude/plans/container-plugin-integration/_plan.md diff --git a/.claude/plans/container-plugin-integration/001-add-plugin-interceptor-to-container.md b/.claude/plans/container-plugin-integration/001-add-plugin-interceptor-to-container.md new file mode 100644 index 00000000..43e53fc0 --- /dev/null +++ b/.claude/plans/container-plugin-integration/001-add-plugin-interceptor-to-container.md @@ -0,0 +1,26 @@ +# Task 001: Add PluginInterceptor Dependency to Container + +**Status**: completed +**Depends on**: none +**Retry count**: 0 + +## Description +Add a `setPluginInterceptor()` setter method to Container, making plugin interception available during resolution without breaking existing code that creates containers without plugins. A setter is required (not a constructor parameter) because `PluginInterceptor` needs a `ContainerInterface` reference in its own constructor, creating a circular dependency that prevents both from being constructed simultaneously. + +## Context +- Related files: `packages/core/src/Container/Container.php` +- The `PluginInterceptor` class already exists at `packages/core/src/Plugin/PluginInterceptor.php` +- Container is constructed in `Application::initialize()` — PluginInterceptor cannot be created until Container exists (PluginInterceptor's constructor requires `ContainerInterface`) +- `PluginInterceptor` is a `readonly class` — it takes `ContainerInterface` and `PluginRegistry` in its constructor +- Add a `private ?PluginInterceptor $pluginInterceptor = null` property (not constructor-promoted, not readonly since it is set after construction) +- Add a `public function setPluginInterceptor(PluginInterceptor $interceptor): void` method + +## Requirements (Test Descriptions) +- [ ] `it accepts PluginInterceptor via setter method` +- [ ] `it resolves classes without PluginInterceptor when none is set` +- [ ] `it continues to work with PreferenceRegistry when PluginInterceptor is also set` + +## Acceptance Criteria +- All requirements have passing tests +- Existing ContainerTest tests continue to pass unchanged +- Code follows code standards diff --git a/.claude/plans/container-plugin-integration/002-wrap-resolved-instances-with-proxy.md b/.claude/plans/container-plugin-integration/002-wrap-resolved-instances-with-proxy.md new file mode 100644 index 00000000..e373af8a --- /dev/null +++ b/.claude/plans/container-plugin-integration/002-wrap-resolved-instances-with-proxy.md @@ -0,0 +1,33 @@ +# Task 002: Wrap Resolved Instances with Plugin Proxy + +**Status**: completed +**Depends on**: 001 +**Retry count**: 0 + +## Description +Modify `Container::resolve()` to wrap resolved instances with `PluginInterceptor::createProxy()` when a `PluginInterceptor` is available. This is the core change that makes plugins functional. The proxy wrapping should happen for autowired instances and closure bindings, but NOT for pre-registered instances (via `instance()`). Singleton caching must store the proxy, not the raw instance. + +## Context +- Related files: `packages/core/src/Container/Container.php`, `packages/core/src/Plugin/PluginInterceptor.php` +- `PluginInterceptor::createProxy()` already returns the raw target when no plugins exist — no extra guard needed +- `resolve()` has three code paths that create instances: + 1. The `instances[]` early return (pre-registered and cached singletons) — should NOT be re-wrapped + 2. Closure bindings — returns early before end of method, so proxy wrapping must happen INSIDE this block before the return + 3. Autowiring — falls through to end of method, proxy wrapping can happen before the return +- **IMPORTANT**: The closure binding code path has an early `return` statement. Proxy wrapping placed "at the end" of `resolve()` will NEVER execute for closure bindings. You must add proxy wrapping in BOTH the closure binding block (before its return) AND the autowiring path (before its return). Alternatively, restructure to have a single exit point. +- The `$id` variable changes during resolution (preferences/bindings may change it). Pass the resolved concrete class name (post-preference/binding `$id`) as `targetClass` to `createProxy()`, NOT `$originalId` +- Pre-registered instances via `instance()` are returned from the `instances[]` cache before proxy wrapping code runs, so they are naturally excluded +- The `PluginInterceptor` is accessed via the setter added in Task 001 (`$this->pluginInterceptor`), which may be null + +## Requirements (Test Descriptions) +- [ ] `it wraps resolved instance with plugin proxy when plugins are registered` +- [ ] `it returns raw instance when no plugins are registered for the class` +- [ ] `it caches the proxy as the singleton instance on subsequent resolves` +- [ ] `it wraps closure binding results with plugin proxy` +- [ ] `it does not wrap pre-registered instances from instance() method` +- [ ] `it applies plugin proxy after preference resolution` + +## Acceptance Criteria +- All requirements have passing tests +- Existing ContainerTest tests continue to pass +- Code follows code standards diff --git a/.claude/plans/container-plugin-integration/003-wire-interceptor-in-application.md b/.claude/plans/container-plugin-integration/003-wire-interceptor-in-application.md new file mode 100644 index 00000000..ba3c5097 --- /dev/null +++ b/.claude/plans/container-plugin-integration/003-wire-interceptor-in-application.md @@ -0,0 +1,34 @@ +# Task 003: Wire PluginInterceptor into Application Bootstrap + +**Status**: completed +**Depends on**: 001, 002 +**Retry count**: 0 + +## Description +Update `Application::initialize()` to create a `PluginInterceptor` and pass it to the Container. This requires reordering the bootstrap sequence slightly: the PluginRegistry must be created before the Container, and the PluginInterceptor needs both the Container and PluginRegistry. Since PluginInterceptor needs the Container reference and Container needs the PluginInterceptor, we need to use a two-phase approach: create Container first, then create PluginInterceptor, then inject it into Container via a setter or by restructuring initialization order. + +## Context +- Related files: `packages/core/src/Application.php`, `packages/core/src/Container/Container.php` +- Current bootstrap order in `initialize()`: + 1. Container is created with PreferenceRegistry + 2. `discoverPlugins()` creates PluginRegistry and populates it +- The problem: `PluginInterceptor` needs BOTH `ContainerInterface` and `PluginRegistry` in its constructor. Container must exist first. +- Solution: Use the setter approach (Task 001 added `setPluginInterceptor()`): + 1. Create `PluginRegistry` early (empty, mutable) — before `discoverPlugins()` + 2. Create Container as before + 3. Create `PluginInterceptor` with the Container and the empty PluginRegistry + 4. Call `$container->setPluginInterceptor($interceptor)` + 5. `discoverPlugins()` populates the SAME PluginRegistry instance (it's mutable, plugins are added via `register()` and checked at resolve-time) +- The existing `discoverPlugins()` method creates its own `new PluginRegistry()` — this must be changed to use the pre-created instance instead +- Register the PluginInterceptor and PluginRegistry as container instances for other services that may need them + +## Requirements (Test Descriptions) +- [ ] `it creates PluginInterceptor and injects it into Container via setter during initialization` +- [ ] `it uses the same PluginRegistry instance for both discovery and interception` +- [ ] `it resolves objects with plugin interception after full initialization` +- [ ] `it creates PluginRegistry before plugin discovery and reuses it for PluginInterceptor` + +## Acceptance Criteria +- All requirements have passing tests +- Existing ApplicationTest tests continue to pass +- Code follows code standards diff --git a/.claude/plans/container-plugin-integration/004-integration-test-end-to-end.md b/.claude/plans/container-plugin-integration/004-integration-test-end-to-end.md new file mode 100644 index 00000000..8dfb988e --- /dev/null +++ b/.claude/plans/container-plugin-integration/004-integration-test-end-to-end.md @@ -0,0 +1,29 @@ +# Task 004: Integration Test — End-to-End Plugin Interception via Container + +**Status**: completed +**Depends on**: 002 +**Retry count**: 0 + +## Description +Create integration tests that prove the full plugin lifecycle works through the container: register plugins in PluginRegistry, resolve a target class from Container, call a method, and verify before/after plugins fire. These tests validate that the system works as a whole, not just individual pieces. + +## Context +- Related files: `packages/core/tests/Unit/Container/ContainerTest.php` (add tests here alongside existing container tests) +- Patterns to follow: Existing `PluginInterceptorTest.php` fixtures and `ContainerTest.php` style +- Tests should manually wire components: create Container, create PluginRegistry, create PluginInterceptor (with Container + Registry), call `$container->setPluginInterceptor($interceptor)`, register plugins in the registry, then resolve and call methods +- Do NOT rely on Application bootstrap — these are unit/integration tests of the Container+Plugin wiring +- Use `describe('plugin interception')` block to group these tests +- For the preference + plugin test: register a preference (`InterfaceX` -> `ConcreteY`), register a plugin targeting `ConcreteY`, resolve `InterfaceX` from container, call a method, verify plugin fires + +## Requirements (Test Descriptions) +- [ ] `it fires before plugin when calling method on container-resolved object` +- [ ] `it fires after plugin when calling method on container-resolved object` +- [ ] `it passes modified arguments from before plugin to target method` +- [ ] `it passes modified result from after plugin back to caller` +- [ ] `it fires plugins on preference-resolved objects` +- [ ] `it returns same proxied singleton on repeated resolves` + +## Acceptance Criteria +- All requirements have passing tests +- Tests demonstrate real end-to-end behavior (not mocked) +- Code follows code standards diff --git a/.claude/plans/container-plugin-integration/005-prevent-plugins-targeting-plugins.md b/.claude/plans/container-plugin-integration/005-prevent-plugins-targeting-plugins.md new file mode 100644 index 00000000..56558bdf --- /dev/null +++ b/.claude/plans/container-plugin-integration/005-prevent-plugins-targeting-plugins.md @@ -0,0 +1,25 @@ +# Task 005: Prevent Plugins from Targeting Plugin Classes + +**Status**: completed +**Depends on**: none +**Retry count**: 0 + +## Description +Add validation to `PluginRegistry::register()` that prevents registering a plugin whose target class is itself a plugin (has the `#[Plugin]` attribute). This enforces the architectural rule that plugins modify business logic on service classes, not other plugins. If someone needs to alter a plugin's behavior, they should use a Preference to replace the plugin class entirely. + +## Context +- Related files: `packages/core/src/Plugin/PluginRegistry.php`, `packages/core/src/Exceptions/PluginException.php` +- PluginRegistry already validates conflicting sort orders and duplicate hooks during `register()` +- Use reflection to check if the target class has a `#[Plugin]` attribute — this is order-independent (doesn't matter which plugin is registered first) +- Add a new static factory method to `PluginException` for this case (e.g., `PluginException::cannotTargetPlugin()`) +- The error message should be helpful: explain that plugins cannot target other plugin classes, and suggest using a Preference to replace the plugin instead + +## Requirements (Test Descriptions) +- [ ] `it throws PluginException when registering a plugin that targets another plugin class` +- [ ] `it includes helpful message suggesting Preference as alternative` +- [ ] `it allows registering plugins that target non-plugin classes` + +## Acceptance Criteria +- All requirements have passing tests +- Error message follows Marko's "loud errors with helpful messages" principle +- Code follows code standards diff --git a/.claude/plans/container-plugin-integration/_devils_advocate.md b/.claude/plans/container-plugin-integration/_devils_advocate.md new file mode 100644 index 00000000..3cf9b7a7 --- /dev/null +++ b/.claude/plans/container-plugin-integration/_devils_advocate.md @@ -0,0 +1,95 @@ +# Devil's Advocate Review: container-plugin-integration + +## Critical (Must fix before building) + +### C1. Circular dependency makes Option C impossible (Task 003) + +Task 003 describes Option C as "cleanest": create `PluginRegistry` and `PluginInterceptor` before Container, pass to Container constructor. But `PluginInterceptor`'s constructor requires `ContainerInterface`: + +```php +readonly class PluginInterceptor +{ + public function __construct( + private ContainerInterface $container, + private PluginRegistry $registry, + ) {} +} +``` + +You cannot create `PluginInterceptor` before `Container` exists, because it needs the Container. And you cannot pass `PluginInterceptor` to Container's constructor because it does not exist yet. + +**Fix**: Option A (setter method) is the only viable approach. Task 001 must add a `setPluginInterceptor()` method to Container. Task 003 must use Option A: create Container first, create PluginInterceptor with the Container reference, then call `$container->setPluginInterceptor($interceptor)`. This also means Container cannot use `readonly` on the `$pluginInterceptor` property. + +### C2. Closure binding early return bypasses end-of-method proxy wrapping (Task 002) + +Task 002 says "proxy wrapping should happen at the end of `resolve()`, just before returning." But the closure binding code path returns early at line 132: + +```php +if ($binding instanceof Closure) { + $instance = $binding($this); + if (isset($this->shared[$originalId])) { + $this->instances[$originalId] = $instance; + } + return $instance; // <-- EARLY RETURN, skips any code at end of method +} +``` + +Any proxy wrapping placed "at the end" of `resolve()` will never execute for closure bindings. The plan's architecture note ("all code paths get wrapped") is incorrect for the current structure. + +**Fix**: Task 002 must explicitly handle proxy wrapping in BOTH the closure binding return path AND the autowiring return path. The task description must specify that proxy wrapping code needs to be added in two places (or the resolve method must be restructured to have a single exit point). + +### C3. Task 004 missing dependency on Task 003 (Task 004) + +Task 004 says "Depends on: 002" but its description says "These tests validate that the system works as a whole." The integration tests that wire `PluginRegistry` + `PluginInterceptor` + `Container` together require the setter/wiring mechanism from Task 003. Without Task 003, there is no way to inject `PluginInterceptor` into Container (since the constructor approach requires the circular dependency fix). + +Actually, on closer inspection, Task 004 could manually construct all pieces in the test without needing Application wiring. The tests create a Container, create a PluginInterceptor with that Container, and inject it via the setter from Task 001. This works without Task 003. However, the dependency should still be explicit since Task 004 needs the setter method from Task 001 AND the proxy wrapping logic from Task 002. + +**Fix**: Task 004's dependency is correct (depends on 002, which depends on 001), but the task context section must clarify that tests should manually wire components using the setter from Task 001, not rely on Application bootstrap. + +## Important (Should fix before building) + +### I1. Line number references will be stale (Task 002) + +Task 002 references specific line numbers from the current Container.php (line 160-191, line 126-132, line 106-108). After Task 001 adds a new constructor parameter and potentially a setter method, all these line numbers will shift. Workers building Task 002 will be confused by wrong line references. + +**Fix**: Replace line number references with descriptive code path names: "the `instances[]` early return", "the closure binding block", "the autowiring block", "the singleton caching block." + +### I2. Setter method violates `readonly` expectations (Task 001) + +The `PluginInterceptor` property on Container cannot be `readonly` since it must be set after construction via a setter. Container class itself is not `readonly` (it has mutable `$shared`, `$instances`, `$bindings` arrays), so this is fine structurally. But the task must explicitly state this property is nullable and set via setter, not via constructor. + +**Fix**: Task 001 must specify: add a `private ?PluginInterceptor $pluginInterceptor = null` property (not constructor-promoted) and a `setPluginInterceptor(PluginInterceptor $interceptor): void` method. Add a test requirement: "it accepts PluginInterceptor via setter method." + +### I3. No test for plugin on interface-requested class via preference (Task 004) + +The plan's architecture notes discuss preferences + plugins, but the test requirement "it fires plugins on preference-resolved objects" is vague. The specific scenario to test: register a preference `InterfaceA` -> `ConcreteB`, register a plugin targeting `ConcreteB`, resolve `InterfaceA` from container, call a method, verify plugin fires. This is the critical cross-cutting test. + +**Fix**: Add explicit setup instructions to Task 004 for the preference + plugin test case. + +### I4. `createProxy` targetClass parameter needs clarification (Task 002) + +When preferences are in play, `$id` changes from the original interface to the concrete class during resolution. The `createProxy()` call should use `$id` (the resolved concrete class), not `$originalId` (the requested interface). The plan's architecture note says "plugins target concrete classes with public methods" which is correct, but Task 002 doesn't explicitly specify which variable to pass as `targetClass`. + +**Fix**: Task 002 requirements should explicitly state: "pass the resolved concrete class name (post-preference `$id`) as `targetClass` to `createProxy()`, not the original requested `$id`." + +## Minor (Nice to address) + +### M1. No test for plugin class resolution causing recursion + +The plan mentions infinite recursion risk under "Risks & Mitigations" but no test validates that resolving a plugin class from within `PluginProxy::__call()` doesn't cause infinite proxy wrapping. If a plugin class itself has plugins registered against it, the Container would try to proxy-wrap the plugin, which would create another proxy, etc. This is an edge case but worth a test. + +### M2. `PluginProxy` uses `__call()` which code standards forbid + +Code standards say "No magic methods -- Avoid `__get`, `__set`, `__call`, `__callStatic`." `PluginProxy` relies on `__call()` by design. This is pre-existing and out of scope for this plan, but worth noting the tension. + +### M3. No `bind()` method in ContainerInterface + +Container has `bind()` as a public method but it's not on `ContainerInterface`. This means code that only has `ContainerInterface` (like `PluginProxy` which holds `ContainerInterface`) cannot call `bind()`. Not an issue for this plan specifically, but worth noting. + +## Questions for the Team + +1. **Should `setPluginInterceptor()` be on `ContainerInterface`?** Currently, the interface only has `singleton()`, `instance()`, `call()`, and the PSR methods. If the setter is only called during Application bootstrap, it could stay on the concrete `Container` class only. But if other code needs to set interceptors, it should be on the interface. + +2. **Should proxy wrapping apply to `call()` resolved dependencies?** Currently only `resolve()` is being modified. But `call()` also resolves dependencies via `$this->resolve()`. Since `resolve()` is the internal method and `call()` delegates to it, this should be covered. But worth confirming. + +3. **Should there be a guard against double-proxying?** If someone calls `container->get(Foo::class)` and stores the proxy, then later registers it via `container->instance('foo', $proxy)`, and another resolve path hits it -- could it get double-wrapped? The `instance()` exclusion prevents this for pre-registered instances, but worth thinking about. diff --git a/.claude/plans/container-plugin-integration/_plan.md b/.claude/plans/container-plugin-integration/_plan.md new file mode 100644 index 00000000..8b4c6968 --- /dev/null +++ b/.claude/plans/container-plugin-integration/_plan.md @@ -0,0 +1,74 @@ +# Plan: Container Plugin Integration + +## Created +2026-04-04 + +## Status +completed + +## Objective +Wire the plugin interception system into `Container::resolve()` so that resolved objects are automatically wrapped with `PluginProxy` when plugins are registered for their class. This makes the entire plugin system functional end-to-end. + +## Scope + +### In Scope +- Container accepts a `PluginInterceptor` via setter method (not constructor — circular dependency with PluginInterceptor needing ContainerInterface) +- `resolve()` wraps instances with `PluginInterceptor::createProxy()` when plugins exist +- Singleton caching stores the proxy (not the raw instance) +- Closure bindings also get proxy-wrapped +- `instance()` pre-registered objects are NOT proxy-wrapped (they're user-provided) +- Application wires `PluginRegistry` and `PluginInterceptor` into Container +- Integration tests proving end-to-end: container resolve → method call → plugin fires +- Preferences + plugins work together correctly + +### Out of Scope +- Around plugins (architecture doc explicitly excludes these) +- Plugin discovery changes (already working) +- PluginProxy or PluginInterceptor behavioral changes (already working) +- Performance caching/compilation of plugin metadata + +## Success Criteria +- [ ] Container wraps resolved objects with plugin proxies when plugins are registered +- [ ] Singleton instances return the same proxy on subsequent resolves +- [ ] Preferences resolve to the correct class AND apply plugins for the original target +- [ ] Closure bindings are proxy-wrapped +- [ ] Pre-registered instances (via `instance()`) are NOT proxy-wrapped +- [ ] Plugin before/after methods fire when calling methods on container-resolved objects +- [ ] All existing Container and Plugin tests continue to pass +- [ ] All tests passing +- [ ] Code follows project standards + +## Task Overview +| Task | Description | Depends On | Status | +|------|-------------|------------|--------| +| 001 | Add PluginInterceptor setter to Container | - | completed | +| 002 | Wrap resolved instances with plugin proxy | 001 | completed | +| 003 | Wire PluginInterceptor into Application bootstrap | 001, 002 | completed | +| 004 | Integration test: end-to-end plugin interception via container | 002 | completed | +| 005 | Prevent plugins from targeting plugin classes | - | completed | + +## Architecture Notes + +### Where proxy wrapping happens +The proxy wrapping must happen in BOTH return paths of `resolve()`: +1. Inside the closure binding block (before its early `return`) +2. In the autowiring path (before its `return` at end of method) + +The closure binding code path has an early `return` statement — any code placed only at the end of `resolve()` will never execute for closure bindings. Both paths must wrap the instance AND cache the proxy (not the raw instance) for singletons. + +### What class to check plugins for +When preferences are in play, pass the resolved concrete class name (post-preference `$id`) as `targetClass` to `createProxy()`, NOT `$originalId`. Example: if `FooInterface` -> `FooImpl` via preference, and a plugin targets `FooImpl`, pass `FooImpl` to `createProxy()`. Plugins target concrete classes with public methods, not interfaces. + +### PluginInterceptor already handles the "no plugins" case +`PluginInterceptor::createProxy()` already checks `hasPluginsFor()` and returns the raw target if no plugins exist. So the container can unconditionally call `createProxy()` without an extra guard — the interceptor handles the no-op case. + +### instance() exclusion +Objects registered via `instance()` are pre-built by the user (or by Application bootstrap). They should NOT be proxy-wrapped because: +1. They may already be proxied +2. The user explicitly provided them — wrapping would violate "explicit over implicit" +3. Framework services (Container itself, EventDispatcher, etc.) are registered this way + +## Risks & Mitigations +- **Infinite recursion**: Plugin classes are resolved from the container, which could trigger proxy wrapping of the plugin itself. Mitigation: Task 005 adds explicit validation preventing plugins from targeting plugin classes. Additionally, `PluginInterceptor::createProxy()` returns the raw instance if no plugins exist. +- **Existing test breakage**: Container tests don't use PluginInterceptor. Mitigation: The `PluginInterceptor` is injected via setter (not constructor), so existing tests that never call `setPluginInterceptor()` pass unchanged. +- **Type mismatches**: `PluginProxy` uses `__call()` magic, so type checks like `instanceof` on the proxy will fail. This is an existing design characteristic of the proxy system, not new to this change. diff --git a/packages/core/src/Application.php b/packages/core/src/Application.php index 7f6b4366..96176bd4 100644 --- a/packages/core/src/Application.php +++ b/packages/core/src/Application.php @@ -35,12 +35,12 @@ use Marko\Core\Module\ModuleRepositoryInterface; use Marko\Core\Path\ProjectPaths; use Marko\Core\Plugin\PluginDiscovery; +use Marko\Core\Plugin\PluginInterceptor; use Marko\Core\Plugin\PluginRegistry; use Marko\Env\EnvLoader; use Marko\Routing\Exceptions\RouteConflictException; use Marko\Routing\Exceptions\RouteException; use Marko\Routing\Http\Request; -use Marko\Routing\Http\Response; use Marko\Routing\Middleware\MiddlewareInterface; use Marko\Routing\Router; use Marko\Routing\RoutingBootstrapper; @@ -72,7 +72,9 @@ class Application /** @var Router */ public object $router { - get => $this->_router ?? throw new RuntimeException('Router not available. Install marko/routing: composer require marko/routing'); + get => $this->_router ?? throw new RuntimeException( + 'Router not available. Install marko/routing: composer require marko/routing' + ); } private ClassFileParser $classFileParser; @@ -85,11 +87,12 @@ public function __construct( /** * @throws ModuleException|CircularDependencyException|BindingConflictException|BindingException|PluginException|PreferenceConflictException|EventException|ContainerExceptionInterface|RouteException|RouteConflictException|CommandException + * @throws \ReflectionException */ public static function boot(string $basePath): self { if (!is_dir($basePath)) { - throw new RuntimeException("Base path does not exist: {$basePath}"); + throw new RuntimeException("Base path does not exist: $basePath"); } $app = new self( @@ -104,7 +107,7 @@ public static function boot(string $basePath): self } /** - * @throws ModuleException|CircularDependencyException|BindingConflictException|BindingException|PluginException|PreferenceConflictException|EventException|ContainerExceptionInterface|RouteException|RouteConflictException|CommandException + * @throws ModuleException|CircularDependencyException|BindingConflictException|BindingException|PluginException|PreferenceConflictException|EventException|ContainerExceptionInterface|RouteException|RouteConflictException|CommandException|\ReflectionException */ public function initialize(): void { @@ -133,8 +136,13 @@ public function initialize(): void // Initialize container and registries $this->classFileParser = new ClassFileParser(); $this->preferenceRegistry = new PreferenceRegistry(); + $this->pluginRegistry = new PluginRegistry(); $this->container = new Container($this->preferenceRegistry); $this->container->instance(ContainerInterface::class, $this->container); + $interceptor = new PluginInterceptor($this->container, $this->pluginRegistry); + $this->container->setPluginInterceptor($interceptor); + $this->container->instance(PluginInterceptor::class, $interceptor); + $this->container->instance(PluginRegistry::class, $this->pluginRegistry); $bindingRegistry = new BindingRegistry($this->container); // Register ProjectPaths for dependency injection (base path derived from vendor path) @@ -225,6 +233,10 @@ private function registerPsr4Autoloader( }); } + /** + * @throws \ReflectionException + * @throws \Marko\Core\Exceptions\PreferenceConflictException + */ private function discoverPreferences(): void { $preferenceDiscovery = new PreferenceDiscovery(); @@ -256,11 +268,10 @@ private function discoverPreferences(): void } /** - * @throws PluginException + * @throws PluginException|\ReflectionException */ private function discoverPlugins(): void { - $this->pluginRegistry = new PluginRegistry(); $pluginDiscovery = new PluginDiscovery(); foreach ($this->modules as $module) { @@ -330,7 +341,7 @@ private function discoverCommands(): void ]; /** - * @throws RouteException|RouteConflictException + * @throws RouteException|RouteConflictException|\ReflectionException */ private function discoverRoutes(): void { diff --git a/packages/core/src/Container/Container.php b/packages/core/src/Container/Container.php index b2b82dd5..9869a5e5 100644 --- a/packages/core/src/Container/Container.php +++ b/packages/core/src/Container/Container.php @@ -6,6 +6,7 @@ use Closure; use Marko\Core\Exceptions\BindingException; +use Marko\Core\Plugin\PluginInterceptor; use ReflectionClass; use ReflectionException; use ReflectionFunction; @@ -22,10 +23,17 @@ class Container implements ContainerInterface /** @var array */ private array $bindings = []; + private ?PluginInterceptor $pluginInterceptor = null; + public function __construct( private readonly ?PreferenceRegistry $preferenceRegistry = null, ) {} + public function setPluginInterceptor(PluginInterceptor $interceptor): void + { + $this->pluginInterceptor = $interceptor; + } + public function bind( string $interface, string|Closure $implementation, @@ -125,6 +133,10 @@ private function resolve( if ($binding instanceof Closure) { $instance = $binding($this); + if ($this->pluginInterceptor !== null) { + $instance = $this->pluginInterceptor->createProxy($id, $instance); + } + if (isset($this->shared[$originalId])) { $this->instances[$originalId] = $instance; } @@ -191,6 +203,10 @@ private function resolve( $instance = $reflectionClass->newInstanceArgs($dependencies); } + if ($this->pluginInterceptor !== null) { + $instance = $this->pluginInterceptor->createProxy($id, $instance); + } + if (isset($this->shared[$originalId])) { $this->instances[$originalId] = $instance; } diff --git a/packages/core/src/Exceptions/PluginException.php b/packages/core/src/Exceptions/PluginException.php index 71f1c7ba..f4d11a56 100644 --- a/packages/core/src/Exceptions/PluginException.php +++ b/packages/core/src/Exceptions/PluginException.php @@ -61,6 +61,17 @@ public static function duplicatePluginHook( ); } + public static function cannotTargetPlugin( + string $pluginClass, + string $targetClass, + ): self { + return new self( + message: "Plugin '$pluginClass' cannot target '$targetClass' because '$targetClass' is itself a plugin class", + context: "While registering plugin '$pluginClass' targeting '$targetClass'", + suggestion: "Plugins cannot modify other plugins. Use a Preference to replace '$targetClass' entirely if you need to change its behavior.", + ); + } + public static function conflictingSortOrder( string $targetClass, string $targetMethod, diff --git a/packages/core/src/Plugin/PluginProxy.php b/packages/core/src/Plugin/PluginProxy.php index 5fde3be9..e1df53a6 100644 --- a/packages/core/src/Plugin/PluginProxy.php +++ b/packages/core/src/Plugin/PluginProxy.php @@ -22,6 +22,16 @@ public function __construct( private PluginRegistry $registry, ) {} + /** + * Get the underlying target instance. + * + * Useful for reflection-based code that needs to inspect the real object. + */ + public function getPluginTarget(): object + { + return $this->target; + } + /** * Intercept method calls and apply plugins. * diff --git a/packages/core/src/Plugin/PluginRegistry.php b/packages/core/src/Plugin/PluginRegistry.php index 474c5ade..d4c9cb45 100644 --- a/packages/core/src/Plugin/PluginRegistry.php +++ b/packages/core/src/Plugin/PluginRegistry.php @@ -4,7 +4,9 @@ namespace Marko\Core\Plugin; +use Marko\Core\Attributes\Plugin; use Marko\Core\Exceptions\PluginException; +use ReflectionClass; class PluginRegistry { @@ -23,6 +25,14 @@ public function register( ): void { $targetClass = $plugin->targetClass; + $reflection = new ReflectionClass($targetClass); + if ($reflection->getAttributes(Plugin::class) !== []) { + throw PluginException::cannotTargetPlugin( + pluginClass: $plugin->pluginClass, + targetClass: $targetClass, + ); + } + if (!isset($this->plugins[$targetClass])) { $this->plugins[$targetClass] = []; } diff --git a/packages/core/tests/Unit/ApplicationTest.php b/packages/core/tests/Unit/ApplicationTest.php index b3d207d6..e3ed7742 100644 --- a/packages/core/tests/Unit/ApplicationTest.php +++ b/packages/core/tests/Unit/ApplicationTest.php @@ -10,6 +10,7 @@ use Marko\Core\Container\ContainerInterface; use Marko\Core\Event\EventDispatcherInterface; use Marko\Core\Exceptions\CircularDependencyException; +use Marko\Core\Plugin\PluginInterceptor; use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; @@ -1771,3 +1772,235 @@ public function handle(Request $request): Response expect($result)->toBeNull() ->and($reflectionMethod->getReturnType()->getName())->toBe('void'); }); + +it('creates PluginInterceptor and injects it into Container via setter during initialization', function (): void { + $baseDir = sys_get_temp_dir() . '/marko-test-' . uniqid(); + $vendorDir = $baseDir . '/vendor'; + + appTestCreateModule($vendorDir . '/acme/core', 'acme/core'); + + $app = new Application( + vendorPath: $vendorDir, + modulesPath: '', + appPath: '', + ); + + $app->initialize(); + + // Use reflection to verify the container has a PluginInterceptor set + $reflection = new ReflectionClass($app->container); + $property = $reflection->getProperty('pluginInterceptor'); + $property->setAccessible(true); + $interceptor = $property->getValue($app->container); + + expect($interceptor)->toBeInstanceOf(PluginInterceptor::class); + + appTestCleanupDirectory($baseDir); +}); + +it('uses the same PluginRegistry instance for both discovery and interception', function (): void { + $uniqueId = uniqid(); + $baseDir = sys_get_temp_dir() . '/marko-test-' . $uniqueId; + $vendorDir = $baseDir . '/vendor'; + + $modulePath = $vendorDir . '/acme/core'; + appTestCreateModule($modulePath, 'acme/core'); + mkdir($modulePath . '/src', 0755, true); + + $targetCode = <<initialize(); + + // The registry on the app and the one inside the interceptor must be the same instance + $containerReflection = new ReflectionClass($app->container); + $interceptorProp = $containerReflection->getProperty('pluginInterceptor'); + $interceptorProp->setAccessible(true); + $interceptor = $interceptorProp->getValue($app->container); + + $interceptorReflection = new ReflectionClass($interceptor); + $registryProp = $interceptorReflection->getProperty('registry'); + $registryProp->setAccessible(true); + $interceptorRegistry = $registryProp->getValue($interceptor); + + expect($interceptorRegistry)->toBe($app->pluginRegistry); + + appTestCleanupDirectory($baseDir); +}); + +it('resolves objects with plugin interception after full initialization', function (): void { + $uniqueId = uniqid(); + $baseDir = sys_get_temp_dir() . '/marko-test-' . $uniqueId; + $vendorDir = $baseDir . '/vendor'; + + $modulePath = $vendorDir . '/acme/core'; + appTestCreateModule($modulePath, 'acme/core'); + mkdir($modulePath . '/src', 0755, true); + + $targetCode = <<initialize(); + + $targetClass = "AcmeIntercept$uniqueId\\GreetingService$uniqueId"; + $service = $app->container->get($targetClass); + + expect($service->greet())->toBe('hello world'); + + appTestCleanupDirectory($baseDir); +}); + +it('creates PluginRegistry before plugin discovery and reuses it for PluginInterceptor', function (): void { + $uniqueId = uniqid(); + $baseDir = sys_get_temp_dir() . '/marko-test-' . $uniqueId; + $vendorDir = $baseDir . '/vendor'; + + $modulePath = $vendorDir . '/acme/core'; + appTestCreateModule($modulePath, 'acme/core'); + mkdir($modulePath . '/src', 0755, true); + + $targetCode = <<initialize(); + + $targetClass = "AcmePreCreate$uniqueId\\ServiceClass$uniqueId"; + + // The registry used at discovery time must have the plugin, + // and the same instance must power interception at resolve time + expect($app->pluginRegistry->hasPluginsFor($targetClass))->toBeTrue(); + + $service = $app->container->get($targetClass); + expect($service->run())->toBe('original-modified'); + + appTestCleanupDirectory($baseDir); +}); diff --git a/packages/core/tests/Unit/Container/ContainerTest.php b/packages/core/tests/Unit/Container/ContainerTest.php index d23c92ba..b4286a6c 100644 --- a/packages/core/tests/Unit/Container/ContainerTest.php +++ b/packages/core/tests/Unit/Container/ContainerTest.php @@ -3,7 +3,12 @@ declare(strict_types=1); use Marko\Core\Container\Container; +use Marko\Core\Container\PreferenceRegistry; use Marko\Core\Exceptions\BindingException; +use Marko\Core\Plugin\PluginDefinition; +use Marko\Core\Plugin\PluginInterceptor; +use Marko\Core\Plugin\PluginProxy; +use Marko\Core\Plugin\PluginRegistry; use Psr\Container\ContainerInterface as PsrContainerInterface; require_once __DIR__ . '/Fixtures/TestFixtureInterface.php'; @@ -303,3 +308,352 @@ public function __construct(public DependencyClass $dep) {} expect(fn () => $container->get('NonExistentClass')) ->not->toThrow(\Marko\TestFixture\Exceptions\NoDriverException::class); }); + +it('accepts PluginInterceptor via setter method', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + + $container->setPluginInterceptor($interceptor); + + expect($container)->toBeInstanceOf(Container::class); +}); + +it('resolves classes without PluginInterceptor when none is set', function (): void { + $container = new Container(); + + $instance = $container->get(SimpleClass::class); + + expect($instance)->toBeInstanceOf(SimpleClass::class); +}); + +it('continues to work with PreferenceRegistry when PluginInterceptor is also set', function (): void { + $preferenceRegistry = new PreferenceRegistry(); + $container = new Container($preferenceRegistry); + $pluginRegistry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $pluginRegistry); + + $container->setPluginInterceptor($interceptor); + + $instance = $container->get(SimpleClass::class); + + expect($instance)->toBeInstanceOf(SimpleClass::class); +}); + +// Plugin interception test fixtures +class PluggableService +{ + public function getValue(): string + { + return 'original'; + } +} + +class PluggableServicePlugin +{ + public function beforeGetValue(): ?array + { + return null; + } +} + +class PluginPreferredService extends PluggableService {} + +describe('plugin interception', function (): void { + it('wraps resolved instance with plugin proxy when plugins are registered', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: PluggableServicePlugin::class, + targetClass: PluggableService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(PluggableService::class); + + expect($instance)->toBeInstanceOf(PluginProxy::class); + }); + + it('returns raw instance when no plugins are registered for the class', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $instance = $container->get(PluggableService::class); + + expect($instance)->toBeInstanceOf(PluggableService::class) + ->and($instance)->not->toBeInstanceOf(PluginProxy::class); + }); + + it('caches the proxy as the singleton instance on subsequent resolves', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + $container->singleton(PluggableService::class); + + $registry->register(new PluginDefinition( + pluginClass: PluggableServicePlugin::class, + targetClass: PluggableService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $instance1 = $container->get(PluggableService::class); + $instance2 = $container->get(PluggableService::class); + + expect($instance1)->toBeInstanceOf(PluginProxy::class) + ->and($instance1)->toBe($instance2); + }); + + it('wraps closure binding results with plugin proxy', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $container->bind(PluggableService::class, fn () => new PluggableService()); + + $registry->register(new PluginDefinition( + pluginClass: PluggableServicePlugin::class, + targetClass: PluggableService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(PluggableService::class); + + expect($instance)->toBeInstanceOf(PluginProxy::class); + }); + + it('does not wrap pre-registered instances from instance() method', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: PluggableServicePlugin::class, + targetClass: PluggableService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $rawInstance = new PluggableService(); + $container->instance(PluggableService::class, $rawInstance); + + $resolved = $container->get(PluggableService::class); + + expect($resolved)->toBe($rawInstance) + ->and($resolved)->not->toBeInstanceOf(PluginProxy::class); + }); + + it('applies plugin proxy after preference resolution', function (): void { + $preferenceRegistry = new PreferenceRegistry(); + $preferenceRegistry->register( + original: PluggableService::class, + replacement: PluginPreferredService::class, + ); + + $container = new Container($preferenceRegistry); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: PluggableServicePlugin::class, + targetClass: PluginPreferredService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(PluggableService::class); + + expect($instance)->toBeInstanceOf(PluginProxy::class); + }); +}); + +// Fixtures for end-to-end plugin interception tests +class E2eService +{ + public function greet(string $name): string + { + return "Hello, $name"; + } + + public function getValue(): string + { + return 'original'; + } +} + +class E2eBeforePlugin +{ + /** @noinspection PhpUnused - Invoked via reflection */ + public function beforeGreet(string $name): ?array + { + return ['MODIFIED']; + } +} + +class E2eTrackingPlugin +{ + public bool $called = false; + + /** @noinspection PhpUnused - Invoked via reflection */ + public function beforeGetValue(): ?array + { + $this->called = true; + + return null; + } +} + +class E2eAfterPlugin +{ + /** @noinspection PhpUnused - Invoked via reflection */ + public function afterGetValue(string $result): string + { + return strtoupper($result); + } +} + +interface E2eServiceInterface {} + +class E2eConcreteService implements E2eServiceInterface +{ + public function compute(): string + { + return 'computed'; + } +} + +class E2eConcretePlugin +{ + /** @noinspection PhpUnused - Invoked via reflection */ + public function afterCompute(string $result): string + { + return $result . '-plugged'; + } +} + +describe('plugin interception - end to end', function (): void { + it('fires before plugin when calling method on container-resolved object', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $tracking = new E2eTrackingPlugin(); + $container->instance(E2eTrackingPlugin::class, $tracking); + + $registry->register(new PluginDefinition( + pluginClass: E2eTrackingPlugin::class, + targetClass: E2eService::class, + beforeMethods: ['getValue' => ['pluginMethod' => 'beforeGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(E2eService::class); + $instance->getValue(); + + expect($tracking->called)->toBeTrue(); + }); + + it('fires after plugin when calling method on container-resolved object', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: E2eAfterPlugin::class, + targetClass: E2eService::class, + afterMethods: ['getValue' => ['pluginMethod' => 'afterGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(E2eService::class); + $result = $instance->getValue(); + + expect($result)->toBe('ORIGINAL'); + }); + + it('passes modified arguments from before plugin to target method', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: E2eBeforePlugin::class, + targetClass: E2eService::class, + beforeMethods: ['greet' => ['pluginMethod' => 'beforeGreet', 'sortOrder' => 10]], + )); + + $instance = $container->get(E2eService::class); + $result = $instance->greet('World'); + + expect($result)->toBe('Hello, MODIFIED'); + }); + + it('passes modified result from after plugin back to caller', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: E2eAfterPlugin::class, + targetClass: E2eService::class, + afterMethods: ['getValue' => ['pluginMethod' => 'afterGetValue', 'sortOrder' => 10]], + )); + + $instance = $container->get(E2eService::class); + $result = $instance->getValue(); + + expect($result)->toBe('ORIGINAL'); + }); + + it('fires plugins on preference-resolved objects', function (): void { + $preferenceRegistry = new PreferenceRegistry(); + $preferenceRegistry->register( + original: E2eServiceInterface::class, + replacement: E2eConcreteService::class, + ); + + $container = new Container($preferenceRegistry); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + + $registry->register(new PluginDefinition( + pluginClass: E2eConcretePlugin::class, + targetClass: E2eConcreteService::class, + afterMethods: ['compute' => ['pluginMethod' => 'afterCompute', 'sortOrder' => 10]], + )); + + $instance = $container->get(E2eServiceInterface::class); + $result = $instance->compute(); + + expect($result)->toBe('computed-plugged'); + }); + + it('returns same proxied singleton on repeated resolves', function (): void { + $container = new Container(); + $registry = new PluginRegistry(); + $interceptor = new PluginInterceptor($container, $registry); + $container->setPluginInterceptor($interceptor); + $container->singleton(E2eService::class); + + $registry->register(new PluginDefinition( + pluginClass: E2eAfterPlugin::class, + targetClass: E2eService::class, + afterMethods: ['getValue' => ['pluginMethod' => 'afterGetValue', 'sortOrder' => 10]], + )); + + $proxy1 = $container->get(E2eService::class); + $proxy2 = $container->get(E2eService::class); + + expect($proxy1)->toBeInstanceOf(PluginProxy::class) + ->and($proxy1)->toBe($proxy2); + }); +}); diff --git a/packages/core/tests/Unit/Plugin/PluginRegistryTest.php b/packages/core/tests/Unit/Plugin/PluginRegistryTest.php index 89efbc19..25d0cac6 100644 --- a/packages/core/tests/Unit/Plugin/PluginRegistryTest.php +++ b/packages/core/tests/Unit/Plugin/PluginRegistryTest.php @@ -69,12 +69,12 @@ public function processPayment(): void {} $orderPlugins = $registry->getPluginsFor(OrderService::class); + $pluginClasses = array_map(fn (PluginDefinition $p) => $p->pluginClass, $orderPlugins); + expect($orderPlugins) ->toBeArray() - ->toHaveCount(2); - - $pluginClasses = array_map(fn (PluginDefinition $p) => $p->pluginClass, $orderPlugins); - expect($pluginClasses) + ->toHaveCount(2) + ->and($pluginClasses) ->toContain(OrderValidationPlugin::class) ->toContain(OrderLoggingPlugin::class); }); @@ -105,12 +105,12 @@ public function processPayment(): void {} // Get plugins for OrderService $orderPlugins = $registry->getPluginsFor(OrderService::class); + $pluginClasses = array_map(fn (PluginDefinition $p) => $p->pluginClass, $orderPlugins); + expect($orderPlugins) ->toBeArray() - ->toHaveCount(2); - - $pluginClasses = array_map(fn (PluginDefinition $p) => $p->pluginClass, $orderPlugins); - expect($pluginClasses) + ->toHaveCount(2) + ->and($pluginClasses) ->toContain(OrderValidationPlugin::class) ->toContain(OrderLoggingPlugin::class) ->not->toContain(PaymentAuditPlugin::class); @@ -163,12 +163,12 @@ public function doAction(): void {} $beforeMethods = $registry->getBeforeMethodsFor(SortableService::class, 'doAction'); + $sortOrders = array_map(fn (array $method) => $method['sortOrder'], $beforeMethods); + expect($beforeMethods) ->toBeArray() - ->toHaveCount(2); - - $sortOrders = array_map(fn (array $method) => $method['sortOrder'], $beforeMethods); - expect($sortOrders)->toBe([5, 15]); + ->toHaveCount(2) + ->and($sortOrders)->toBe([5, 15]); }); it('sorts plugins by sortOrder (lower runs first)', function (): void { @@ -196,21 +196,19 @@ public function doAction(): void {} // Get sorted before methods for doAction $beforeMethods = $registry->getBeforeMethodsFor(SortableService::class, 'doAction'); - expect($beforeMethods) - ->toBeArray() - ->toHaveCount(3); - // Verify sort order - lower sortOrder should come first $sortOrders = array_map(fn (array $method) => $method['sortOrder'], $beforeMethods); - expect($sortOrders)->toBe([5, 15, 30]); - - // Verify the plugins are in correct order $pluginClasses = array_map(fn (array $method) => $method['pluginClass'], $beforeMethods); - expect($pluginClasses)->toBe([ - HighPriorityPlugin::class, - MediumPriorityPlugin::class, - LowPriorityPlugin::class, - ]); + + expect($beforeMethods) + ->toBeArray() + ->toHaveCount(3) + ->and($sortOrders)->toBe([5, 15, 30]) + ->and($pluginClasses)->toBe([ + HighPriorityPlugin::class, + MediumPriorityPlugin::class, + LowPriorityPlugin::class, + ]); }); // Fixtures for new target-method-keyed registry tests @@ -438,3 +436,63 @@ public function save(): void {} ->and($e->getSuggestion())->not->toBeEmpty(); } }); + +// Fixtures for plugin-targeting-plugin tests +class TargetBusinessService +{ + public function execute(): void {} +} + +#[Plugin(target: TargetBusinessService::class)] +class PluginThatIsTargeted +{ + /** @noinspection PhpUnused - Invoked via reflection */ + #[Before(sortOrder: 10)] + public function execute(): void {} +} + +#[Plugin(target: PluginThatIsTargeted::class)] +class PluginTargetingPlugin +{ + /** @noinspection PhpUnused - Invoked via reflection */ + #[Before(sortOrder: 10)] + public function execute(): void {} +} + +it('throws PluginException when registering a plugin that targets another plugin class', function (): void { + $registry = new PluginRegistry(); + + expect(fn () => $registry->register(new PluginDefinition( + pluginClass: PluginTargetingPlugin::class, + targetClass: PluginThatIsTargeted::class, + beforeMethods: ['execute' => ['pluginMethod' => 'execute', 'sortOrder' => 10]], + )))->toThrow(PluginException::class); +}); + +it('includes helpful message suggesting Preference as alternative', function (): void { + $registry = new PluginRegistry(); + + try { + $registry->register(new PluginDefinition( + pluginClass: PluginTargetingPlugin::class, + targetClass: PluginThatIsTargeted::class, + beforeMethods: ['execute' => ['pluginMethod' => 'execute', 'sortOrder' => 10]], + )); + expect(false)->toBeTrue('Expected PluginException to be thrown'); + } catch (PluginException $e) { + expect($e->getMessage())->toContain('PluginThatIsTargeted') + ->and($e->getSuggestion())->toContain('Preference'); + } +}); + +it('allows registering plugins that target non-plugin classes', function (): void { + $registry = new PluginRegistry(); + + $registry->register(new PluginDefinition( + pluginClass: PluginThatIsTargeted::class, + targetClass: TargetBusinessService::class, + beforeMethods: ['execute' => ['pluginMethod' => 'execute', 'sortOrder' => 10]], + )); + + expect($registry->getPluginsFor(TargetBusinessService::class))->toHaveCount(1); +}); diff --git a/packages/routing/src/Router.php b/packages/routing/src/Router.php index 7e675de0..705f0b73 100644 --- a/packages/routing/src/Router.php +++ b/packages/routing/src/Router.php @@ -5,6 +5,7 @@ namespace Marko\Routing; use Marko\Core\Container\ContainerInterface; +use Marko\Core\Plugin\PluginProxy; use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; use Marko\Routing\Middleware\MiddlewareInterface; @@ -75,7 +76,11 @@ private function resolveParameters( array $routeParams, Request $request, ): array { - $reflection = new ReflectionMethod($controller, $action); + // Unwrap plugin proxy to reflect on the real controller + $reflectionTarget = $controller instanceof PluginProxy + ? $controller->getPluginTarget() + : $controller; + $reflection = new ReflectionMethod($reflectionTarget, $action); $parameters = []; foreach ($reflection->getParameters() as $param) {