From bc3d39e1f4f3e5b89a42bb63cb3a6dc4f12cf72e Mon Sep 17 00:00:00 2001 From: Olivier Gorzalka Date: Mon, 12 Oct 2026 17:30:54 +0200 Subject: [PATCH] refactor: let the WpRest module run without Laravel's application WpRestDiscovery resolves controllers through the discovery's instance pool and hands custom HandlesAttributes attributes the PSR-11 container it was given, instead of app(); the wrapper takes the instance instead of building it with resolve() for its logger. An architecture test keeps the module to attributes, discovery, PSR contracts and WordPress. Fixes a REST controller with a constructor dependency never being built. --- CHANGELOG.md | 2 + .../Services/WpRestAttributableWrapper.php | 39 ++----------- .../Services/WpRestDiscovery.php | 11 +++- tests/Unit/WpRest/PackageBoundaryTest.php | 31 ++++++++++ tests/Unit/WpRest/WpRestDiscoveryTest.php | 58 +++++++++++++++++++ 5 files changed, 104 insertions(+), 37 deletions(-) create mode 100644 tests/Unit/WpRest/PackageBoundaryTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 93b92baf..489ebf97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - One `DiscoveryCacheManager` is shared by the engine and the `pollora:doctor` discovery cache check, which built its own - The discovery benchmarks measure memory as PHP allocates it, from a reset peak: they read 2 MB whatever the load - `#[WpRestRoute\Method]` no longer registers its route: `WpRestRouteRegistrar`, in the WpRest module, does, with the same arguments, permissions and request injection. Default slugs, meta keys and command names come from `Pollora\Attributes\Support\Naming`, which gives the same results as `Str::kebab()` and `Str::snake()`; `Pollora\Support\Slug` uses it +- The WpRest module no longer reaches for Laravel's application, the first step towards `pollora/wp-rest`: custom `HandlesAttributes` attributes receive the PSR-11 container given to `WpRestDiscovery`, and `WpRestAttributableWrapper` takes the controller instance instead of building it. An architecture test keeps the module to the attributes, discovery, PSR contracts and WordPress ### Deprecated - `DiscoveryCacheInterface` and `LaravelDiscoveryCache`, removed in 15.0: nothing reads this cache, scans are cached by the Spatie driver given to `DiscoveryCacheManager` @@ -28,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - A public `#[Action]` or `#[Filter]` method in a `#[PostType]` or `#[Taxonomy]` class is registered once: the post type and taxonomy discoveries also handed it to `Action::handle()`, which registered it a second time on the discovery's configuration object, so the page firing that hook failed with `call_user_func(): Argument #1 ($callback) must be a valid callback, class Pollora\PostType\Infrastructure\Services\PostTypeConfiguration does not have a method …` +- A REST controller declared with `#[WpRestRoute]` gets its constructor dependencies: it is resolved through the discovery's instance pool (Laravel's container in a Pollora application) instead of `new` with no argument, so a controller with a required dependency answered every request with `Trying to invoke non static method … without an object` ## [v13.35.6](https://github.com/Pollora/framework/compare/v13.35.5...v13.35.6) - 2026-10-12 diff --git a/src/WpRest/Infrastructure/Services/WpRestAttributableWrapper.php b/src/WpRest/Infrastructure/Services/WpRestAttributableWrapper.php index cca261ca..3d276ab2 100644 --- a/src/WpRest/Infrastructure/Services/WpRestAttributableWrapper.php +++ b/src/WpRest/Infrastructure/Services/WpRestAttributableWrapper.php @@ -6,57 +6,28 @@ use Pollora\Attributes\Attributable; use Pollora\Attributes\WpRestRoute\Permission; -use Pollora\Discovery\Domain\Contracts\ReflectionCacheInterface; -use Psr\Log\LoggerInterface; /** * Wrapper class to make any class compatible with Attributable interface. * * This class acts as a bridge between regular classes that use WpRestRoute attributes - * and the Attributable interface required by the Method attribute handlers. + * and the Attributable interface required by the Method attribute handlers. It carries + * the controller instance the discovery resolved, dependencies included. */ final readonly class WpRestAttributableWrapper implements Attributable { - private mixed $realInstance; - public function __construct( - private string $className, + private object $realInstance, public string $namespace, public string $route, public string|Permission|null $classPermission = null, - private ?ReflectionCacheInterface $reflectionCache = null - ) { - $this->realInstance = $this->createRealInstance(); - } + ) {} /** * Get the real instance of the wrapped class. */ - public function getRealInstance(): mixed + public function getRealInstance(): object { return $this->realInstance; } - - /** - * Create an instance of the real class. - */ - private function createRealInstance(): mixed - { - try { - $reflectionClass = $this->reflectionCache->getClassReflection($this->className); - - if ($reflectionClass->isInstantiable()) { - return $reflectionClass->newInstance(); - } - } catch (\Throwable $throwable) { - try { - resolve(LoggerInterface::class)->error( - sprintf('Failed to create instance of %s: ', $this->className).$throwable->getMessage() - ); - } catch (\Throwable) { - } - } - - return null; - } } diff --git a/src/WpRest/Infrastructure/Services/WpRestDiscovery.php b/src/WpRest/Infrastructure/Services/WpRestDiscovery.php index f8781370..0f69a7e1 100644 --- a/src/WpRest/Infrastructure/Services/WpRestDiscovery.php +++ b/src/WpRest/Infrastructure/Services/WpRestDiscovery.php @@ -12,6 +12,7 @@ use Pollora\Discovery\Domain\Contracts\ReflectionCacheInterface; use Pollora\Discovery\Domain\Services\HasInstancePool; use Pollora\Discovery\Domain\Services\IsDiscovery; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use ReflectionMethod; use Spatie\StructureDiscoverer\Data\DiscoveredClass; @@ -34,9 +35,13 @@ final class WpRestDiscovery implements DiscoveryInterface */ private array $wrapperCache = []; + /** + * @param ContainerInterface|null $container Handed to custom attributes implementing HandlesAttributes + */ public function __construct( private readonly ?LoggerInterface $logger = null, private readonly WpRestRouteRegistrar $registrar = new WpRestRouteRegistrar, + private readonly ?ContainerInterface $container = null, ) {} /** @@ -137,12 +142,12 @@ private function processWpRestRoute(string $className, ?ReflectionCacheInterface // Create wrapper once for the class (use cache to avoid recreating) $wrapperKey = md5($className.$wpRestRoute->namespace.$wpRestRoute->route); + // The controller comes from the instance pool, which gives it its dependencies $this->wrapperCache[$wrapperKey] ??= new WpRestAttributableWrapper( - $className, + $this->getInstanceFromPool($className), $wpRestRoute->namespace, $wpRestRoute->route, $wpRestRoute->permissionCallback, - $reflectionCache ); $attributableWrapper = $this->wrapperCache[$wrapperKey]; @@ -218,7 +223,7 @@ private function processMethodAttribute( if ($attributeInstance instanceof HandlesAttributes) { add_action('rest_api_init', function () use ($attributeInstance, $attributableWrapper, $method): void { - $attributeInstance->handle(app(), $attributableWrapper, $method, $attributeInstance); + $attributeInstance->handle($this->container, $attributableWrapper, $method, $attributeInstance); }); } } catch (\Throwable $throwable) { diff --git a/tests/Unit/WpRest/PackageBoundaryTest.php b/tests/Unit/WpRest/PackageBoundaryTest.php new file mode 100644 index 00000000..2486a4fc --- /dev/null +++ b/tests/Unit/WpRest/PackageBoundaryTest.php @@ -0,0 +1,31 @@ +expect(['Pollora\WpRest', WpGlobals::class]) + ->toOnlyUse([ + 'Pollora\WpRest', + 'Pollora\Attributes', + 'Pollora\Discovery', + WpGlobals::class, + 'Psr\Container', + 'Psr\Log', + 'Spatie\StructureDiscoverer', + // WordPress + '__', + 'add_action', + 'current_user_can', + 'is_user_logged_in', + 'register_rest_route', + 'rest_authorization_required_code', + ]) + ->ignoring([WpRestAttributeServiceProvider::class, 'WP_REST_Request', 'WP_Error']); diff --git a/tests/Unit/WpRest/WpRestDiscoveryTest.php b/tests/Unit/WpRest/WpRestDiscoveryTest.php index ed292d5b..5f0a4ff5 100644 --- a/tests/Unit/WpRest/WpRestDiscoveryTest.php +++ b/tests/Unit/WpRest/WpRestDiscoveryTest.php @@ -2,12 +2,14 @@ declare(strict_types=1); +use Brain\Monkey\Functions; use Pollora\Attributes\Attributable; use Pollora\Attributes\WpRestRoute; use Pollora\Attributes\WpRestRoute\Method; use Pollora\Discovery\Domain\Models\DiscoveryLocation; use Pollora\Discovery\Infrastructure\Services\ReflectionCache; use Pollora\WpRest\Infrastructure\Services\WpRestDiscovery; +use Spatie\StructureDiscoverer\Data\DiscoveredClass; // Mock WordPress functions if they don't exist if (! function_exists('register_rest_route')) { @@ -30,6 +32,31 @@ function add_action($hook, $callback, $priority = 10, $accepted_args = 1): bool } } +if (! class_exists('WP_REST_Request')) { + eval('class WP_REST_Request {}'); +} + +final class DocumentRepository +{ + public function title(): string + { + return 'Annual report'; + } +} + +// A controller with a dependency, given by the container +#[WpRestRoute('api/v1', '/reports')] +final readonly class ReportsController +{ + public function __construct(private DocumentRepository $documents) {} + + #[Method('GET')] + public function index(): array + { + return ['title' => $this->documents->title()]; + } +} + // Test class that doesn't implement Attributable (real-world scenario) #[WpRestRoute('api/v1', '/documents/(?P\\d+)')] class TestDocumentAPI @@ -186,6 +213,37 @@ class: WpRestRoute::class, expect(fn () => $discovery->apply())->not->toThrow(Exception::class); }); + test('registers each #[Method] on rest_api_init and calls the controller with its dependencies', function (): void { + $hooks = []; + $routes = []; + Functions\when('add_action')->alias(function (string $hook, callable $callback) use (&$hooks): bool { + $hooks[$hook][] = $callback; + + return true; + }); + Functions\when('register_rest_route')->alias(function (string $namespace, string $route, array $args) use (&$routes): bool { + $routes[] = ['namespace' => $namespace, 'route' => $route, 'args' => $args]; + + return true; + }); + + $discovery = new WpRestDiscovery; + $discovery->discover(new DiscoveryLocation('', __DIR__), DiscoveredClass::fromReflection(new ReflectionClass(ReportsController::class)), new ReflectionCache); + $discovery->apply(); + + expect($routes)->toBe([]); + + foreach ($hooks['rest_api_init'] ?? [] as $callback) { + $callback(); + } + + expect($routes)->toHaveCount(1) + ->and($routes[0]['namespace'])->toBe('api/v1') + ->and($routes[0]['route'])->toBe('/reports') + ->and($routes[0]['args']['methods'])->toBe(['GET']) + ->and($routes[0]['args']['callback'](Mockery::mock(WP_REST_Request::class)))->toBe(['title' => 'Annual report']); + }); + test('returns correct identifier', function (): void { $discovery = new WpRestDiscovery; expect($discovery->getIdentifier())->toBe('wp_rest_routes');