From 5451ced5f020119c6a10d32a28b54420665a3865 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Tue, 22 Sep 2026 14:24:55 +0600 Subject: [PATCH 1/3] container bindings static to this scope --- src/Phaseolies/Application.php | 13 +- src/Phaseolies/ApplicationBuilder.php | 76 +----------- src/Phaseolies/Config/Config.php | 25 +++- src/Phaseolies/DI/Container.php | 110 +++++++++++----- src/Phaseolies/Support/Router.php | 49 ++++++-- src/Phaseolies/Support/View/View.php | 13 ++ tests/API/Presenter/PresenterBundleTest.php | 9 ++ tests/API/Presenter/SQLitePresenterTest.php | 7 +- tests/Application/ApplicationTest.php | 100 +++++++++++++++ tests/Application/ContainerTest.php | 32 ++--- tests/Application/TimezoneHandlerTest.php | 5 + tests/Builder/BuilderSQLiteTest.php | 5 + .../Builder/Query/EntityBuilderQueryTest.php | 7 +- tests/Builder/QueryBuilderTest.php | 6 + tests/Controller/ControllerTest.php | 12 +- tests/Logger/LoggerHelperTest.php | 9 ++ tests/PaginatorTest.php | 9 ++ tests/Providers/BaseFacadeTest.php | 10 ++ tests/RedirectResponseTest.php | 9 ++ tests/Requests/AllRequestInputTest.php | 13 +- tests/Requests/RequestParserTraitTest.php | 5 + tests/ResponseLifecycleTest.php | 31 +++++ tests/Router/RouterTest.php | 85 +++++++++++++ .../Database/ModelQueryDriverTestCase.php | 7 +- .../Support/View/ViewFetchReentrancyTest.php | 117 ++++++++++++++++++ tests/Validation/FileValidationRuleTest.php | 7 +- tests/Validation/SanitizerTest.php | 12 +- .../ValidationRulesExtendedTest.php | 16 ++- 28 files changed, 643 insertions(+), 156 deletions(-) create mode 100644 tests/Support/View/ViewFetchReentrancyTest.php diff --git a/src/Phaseolies/Application.php b/src/Phaseolies/Application.php index faf41516..1a7902fd 100644 --- a/src/Phaseolies/Application.php +++ b/src/Phaseolies/Application.php @@ -256,9 +256,7 @@ public function langPath($path = ''): string */ public function configure(Application $app): ApplicationBuilder { - return (new ApplicationBuilder($app)) - ->withTimezone() - ->withMiddlewareStack(); + return (new ApplicationBuilder($app))->withTimezone(); } /** @@ -1149,6 +1147,13 @@ protected function cleanupRequestScopedServices(): void foreach (['session', 'request', 'response', 'redirect'] as $abstract) { $this->forgetResolved($abstract); } + + // re-resolves fresh instance instead of reusing it. + $this->forgetRequestScopedInstances(); + + // Undo any config() mutation made while handling + // this request, so it doesn't leak into the next one. + Config::resetRuntimeOverrides(); } /** @@ -1276,6 +1281,8 @@ public function handle(Request $request): Response */ public function dispatch($request): DispatchResult { + $this->snapshotBootBindings(); + try { $this->instance('request', $request); diff --git a/src/Phaseolies/ApplicationBuilder.php b/src/Phaseolies/ApplicationBuilder.php index e976ffba..f42d602d 100644 --- a/src/Phaseolies/ApplicationBuilder.php +++ b/src/Phaseolies/ApplicationBuilder.php @@ -3,24 +3,13 @@ namespace Phaseolies; use Phaseolies\Support\TimezoneHandler; -use Phaseolies\Middleware\Contracts\Middleware as ContractsMiddleware; class ApplicationBuilder { - /** - * Holds the current HTTP request instance - * - * @var \Phaseolies\Http\Request - */ - protected $request; - /** * @param Application $app */ - public function __construct(protected Application $app) - { - $this->request = $this->app->make('request'); - } + public function __construct(protected Application $app) {} /** * Set the application timezone @@ -39,68 +28,7 @@ public function withTimezone(): self } /** - * Configures the application with middleware stack handling - * - * @return self - * @throws \Exception - */ - public function withMiddlewareStack(): self - { - $middlewareStack = $this->buildMiddlewareStack(); - - $handler = $this->processMiddlewareStack($middlewareStack); - - $this->app->router->getGateway()->handle($this->request, $handler); - - return $this; - } - - /** - * Constructs the middleware stack based on request type - * - * @return array - */ - protected function buildMiddlewareStack(): array - { - $gateway = $this->app->router->getGateway(); - - $middlewareStack = $gateway->getGlobalMiddleware(); - - $groupKey = $this->request->isApiRequest() ? 'api' : 'web'; - $groupMiddleware = $gateway->getMiddlewareGroups()[$groupKey] ?? []; - - return array_merge($middlewareStack, $groupMiddleware); - } - - /** - * Processes the middleware stack into a handler pipeline. - * - * @param array $middlewareStack - * @return callable - * @throws \Exception - */ - protected function processMiddlewareStack(array $middlewareStack): callable - { - $response = fn() => $this->app->make('response'); - - foreach ($middlewareStack as $middlewareClass) { - $middlewareInstance = $this->app->make($middlewareClass); - if (!$middlewareInstance instanceof ContractsMiddleware) { - throw new \Exception( - "Failed to register middleware {$middlewareClass}: it must implement " . ContractsMiddleware::class . "." - ); - } - - $response = function ($request) use ($middlewareInstance, $response) { - return $middlewareInstance($request, $response); - }; - } - - return $response; - } - - /** - * Finalizes the builder process and returns the configured application. + * Finalizes the builder process and returns the configured application * * @return Application */ diff --git a/src/Phaseolies/Config/Config.php b/src/Phaseolies/Config/Config.php index 4206ed85..35e72c45 100644 --- a/src/Phaseolies/Config/Config.php +++ b/src/Phaseolies/Config/Config.php @@ -48,6 +48,13 @@ final class Config */ protected static ?array $configFiles = null; + /** + * A snapshot of `$config` exactly as it stood right after boot + * + * @var array + */ + protected static array $bootSnapshot = []; + /** * Initialize the configuration system. * @@ -58,9 +65,20 @@ public static function initialize(): void if (self::$cacheFile === null) { self::$cacheFile = storage_path('framework/cache/config.php'); self::loadFromCache(); + self::$bootSnapshot = self::$config; } } + /** + * Undo any runtime mutation made by `set()` since boot + * + * @return void + */ + public static function resetRuntimeOverrides(): void + { + self::$config = self::$bootSnapshot; + } + /** * Get all config file paths, cached for the request lifetime. * @@ -243,9 +261,6 @@ public static function set(string $key, mixed $value): void } else { $current = $value; } - - self::$configModified = true; - self::cacheConfig(); } /** @@ -278,7 +293,9 @@ public static function has(string $key): bool */ public static function clearCache(): void { - if (file_exists(self::$cacheFile)) @unlink(self::$cacheFile); + if (self::$cacheFile !== null && file_exists(self::$cacheFile)) { + @unlink(self::$cacheFile); + } self::$config = []; self::$fileHashes = []; diff --git a/src/Phaseolies/DI/Container.php b/src/Phaseolies/DI/Container.php index 14e350a8..b9cb72e0 100644 --- a/src/Phaseolies/DI/Container.php +++ b/src/Phaseolies/DI/Container.php @@ -12,14 +12,14 @@ class Container implements ArrayAccess * * @var array */ - private static array $bindings = []; + private array $bindings = []; /** * Array to hold singleton instances. * * @var array */ - private static array $instances = []; + private array $instances = []; /** * Array to track currently resolving classes (for circular dependency detection) @@ -35,6 +35,21 @@ class Container implements ArrayAccess */ private static ?self $instance = null; + /** + * The set of binding keys that existed the moment boot finished — see + * {@see snapshotBootBindings()}. + * + * @var array + */ + private array $bootBindingKeys = []; + + /** + * Whether {@see snapshotBootBindings()} has run yet. + * + * @var bool + */ + private bool $bootBindingsSnapshotted = false; + public function __construct() { $this->resolving = []; @@ -92,7 +107,7 @@ public function offsetSet($offset, $value): void */ public function offsetUnset($offset): void { - unset(self::$bindings[$offset], self::$instances[$offset]); + unset($this->bindings[$offset], $this->instances[$offset]); } /** @@ -109,13 +124,13 @@ public function bind(string $abstract, callable|string|null $concrete = null, bo $concrete = $abstract; } - self::$bindings[$abstract] = [ + $this->bindings[$abstract] = [ 'concrete' => $concrete, 'singleton' => $singleton ]; if ($singleton) { - self::$instances[$abstract] = null; + $this->instances[$abstract] = null; } } @@ -140,9 +155,9 @@ public function singleton(string $abstract, callable|string|null $concrete = nul */ public function instance(string $abstract, mixed $instance): void { - self::$instances[$abstract] = $instance; + $this->instances[$abstract] = $instance; - self::$bindings[$abstract] = [ + $this->bindings[$abstract] = [ 'concrete' => fn() => $instance, 'singleton' => true ]; @@ -164,8 +179,8 @@ public function get(string $abstract, array $parameters = []): mixed } if ( - !isset(self::$bindings[$abstract]) && - !array_key_exists($abstract, self::$instances) && + !isset($this->bindings[$abstract]) && + !array_key_exists($abstract, $this->instances) && method_exists($this, 'loadGhostProvider') ) { $this->loadGhostProvider($abstract); @@ -174,23 +189,23 @@ public function get(string $abstract, array $parameters = []): mixed $this->resolving[$abstract] = true; try { - if (isset(self::$instances[$abstract]) && self::$instances[$abstract] !== null) { - return self::$instances[$abstract]; + if (isset($this->instances[$abstract]) && $this->instances[$abstract] !== null) { + return $this->instances[$abstract]; } - if (isset(self::$bindings[$abstract])) { - $binding = self::$bindings[$abstract]; + if (isset($this->bindings[$abstract])) { + $binding = $this->bindings[$abstract]; $resolved = $this->resolveBinding($abstract, $binding, $parameters); if ($binding['singleton']) { - self::$instances[$abstract] = $resolved; + $this->instances[$abstract] = $resolved; } return $resolved; } // Fallback only: no exact instance and no explicit binding for $abstract. - foreach (self::$instances as $instance) { + foreach ($this->instances as $instance) { if ($instance instanceof $abstract) { return $instance; } @@ -393,7 +408,7 @@ public function when(callable|bool $condition): ?self */ public function has(string $key): bool { - return isset(self::$bindings[$key]) || class_exists($key); + return isset($this->bindings[$key]) || class_exists($key); } /** @@ -404,7 +419,7 @@ public function has(string $key): bool */ public function hasInstance(string $key): bool { - return isset(self::$instances[$key]) && self::$instances[$key] !== null; + return isset($this->instances[$key]) && $this->instances[$key] !== null; } /** @@ -415,7 +430,42 @@ public function hasInstance(string $key): bool */ public function forgetResolved(string $abstract): void { - unset(self::$instances[$abstract]); + unset($this->instances[$abstract]); + } + + /** + * Freeze the current set of binding keys as "boot-time". + * {@see forgetRequestScopedInstances()}, which sweeps exactly this set. + * + * @return void + */ + public function snapshotBootBindings(): void + { + if ($this->bootBindingsSnapshotted) { + return; + } + + $this->bootBindingKeys = array_fill_keys(array_keys($this->bindings), true); + $this->bootBindingsSnapshotted = true; + } + + /** + * Forget the resolved instance of every singleton bound *after* boot + * finished + * + * @return void + */ + public function forgetRequestScopedInstances(): void + { + if (!$this->bootBindingsSnapshotted) { + return; + } + + foreach (array_keys($this->bindings) as $abstract) { + if (!isset($this->bootBindingKeys[$abstract])) { + unset($this->instances[$abstract]); + } + } } /** @@ -425,9 +475,11 @@ public function forgetResolved(string $abstract): void */ public function flush(): void { - self::$bindings = []; - self::$instances = []; + $this->bindings = []; + $this->instances = []; $this->resolving = []; + $this->bootBindingKeys = []; + $this->bootBindingsSnapshotted = false; } /** @@ -437,7 +489,7 @@ public function flush(): void */ public function getBindings(): array { - return self::$bindings; + return $this->bindings; } /** @@ -447,7 +499,7 @@ public function getBindings(): array */ public function getInstances(): array { - return self::$instances; + return $this->instances; } /** @@ -510,9 +562,9 @@ public function extend(string $abstract, callable $extender): void throw new \RuntimeException("Cannot extend unbound abstract [{$abstract}]"); } - $previous = self::$bindings[$abstract]; + $previous = $this->bindings[$abstract]; - self::$bindings[$abstract] = [ + $this->bindings[$abstract] = [ 'concrete' => fn(Container $container, array $parameters = []) => $extender($container->resolveBinding($abstract, $previous, $parameters), $container), 'singleton' => $previous['singleton'] ]; @@ -527,7 +579,7 @@ public function extend(string $abstract, callable $extender): void */ public function alias(string $abstract, string $alias): void { - self::$bindings[$alias] = [ + $this->bindings[$alias] = [ 'concrete' => fn(Container $container) => $container->get($abstract), 'singleton' => false ]; @@ -618,7 +670,7 @@ protected function resolveDependenciesWithAttributes(array $parameters, array $p */ public function isSingleton(string $abstract): bool { - return isset(self::$bindings[$abstract]) && self::$bindings[$abstract]['singleton']; + return isset($this->bindings[$abstract]) && $this->bindings[$abstract]['singleton']; } /** @@ -628,7 +680,7 @@ public function isSingleton(string $abstract): bool */ public function getAliases(): array { - return array_filter(self::$bindings, function ($binding) { + return array_filter($this->bindings, function ($binding) { $concrete = $binding['concrete']; return is_callable($concrete) && !(is_string($concrete) && class_exists($concrete)); }); @@ -700,8 +752,8 @@ public function reset(): void public function resolved(string $abstract): bool { return $this->hasInstance($abstract) || - (isset(self::$bindings[$abstract]) && - self::$bindings[$abstract]['singleton'] && + (isset($this->bindings[$abstract]) && + $this->bindings[$abstract]['singleton'] && $this->hasInstance($abstract)); } } diff --git a/src/Phaseolies/Support/Router.php b/src/Phaseolies/Support/Router.php index bc14e764..7712f17a 100644 --- a/src/Phaseolies/Support/Router.php +++ b/src/Phaseolies/Support/Router.php @@ -11,6 +11,7 @@ use Phaseolies\Support\Router\InteractsWithBundleRouter; use Phaseolies\Support\Router\InteractsWithDynamicControllerBinding; use Phaseolies\Middleware\Contracts\Middleware as ContractsMiddleware; +use Phaseolies\Middleware\Middleware as MiddlewareChain; use Phaseolies\Http\Validation\Contracts\ValidatesWhenResolved; use Phaseolies\Http\Response; use Phaseolies\Http\Request; @@ -969,10 +970,11 @@ public function getCurrentRouteMiddleware($request): ?array * @param Request $request * @param Application $app * @param array $currentMiddleware + * @param MiddlewareChain $chain The request-local chain to apply onto. * @return void * @throws \Exception */ - private function applyRouteMiddleware($request, $app, $currentMiddleware): void + private function applyRouteMiddleware($request, $app, $currentMiddleware, MiddlewareChain $chain): void { $routeMiddleware = $this->gateway->getRouteMiddleware(); @@ -993,13 +995,27 @@ private function applyRouteMiddleware($request, $app, $currentMiddleware): void $middlewareClass = $routeMiddleware['api'][$name]; } - $middlewareInstance = $app->make($middlewareClass); - if (!$middlewareInstance instanceof ContractsMiddleware) { - throw new \Exception("Unresolved dependency $middlewareClass", 1); - } + $chain->applyMiddleware($this->makeGatewayMiddleware($app, $middlewareClass), $params); + } + } - $this->gateway->applyMiddleware($middlewareInstance, $params); + /** + * Resolve a middleware class name to an instance, validating its contract. + * + * @param Application $app + * @param string $middlewareClass + * @return ContractsMiddleware + * @throws \Exception + */ + private function makeGatewayMiddleware($app, string $middlewareClass): ContractsMiddleware + { + $middlewareInstance = $app->make($middlewareClass); + + if (!$middlewareInstance instanceof ContractsMiddleware) { + throw new \Exception("Unresolved dependency $middlewareClass", 1); } + + return $middlewareInstance; } /** @@ -1023,10 +1039,6 @@ public function resolve(Application $app, Request $request): Response } } - if ($currentMiddleware = $this->getCurrentRouteMiddleware($request)) { - $this->applyRouteMiddleware($request, $app, $currentMiddleware); - } - $routeParams = $request->getRouteParams(); $handler = function ($request) use ($callback, $app, $routeParams) { @@ -1037,9 +1049,22 @@ public function resolve(Application $app, Request $request): Response return $result; }; - $response = $this->gateway->handle($request, $handler); + $chain = new MiddlewareChain(); - return $response; + if ($currentMiddleware = $this->getCurrentRouteMiddleware($request)) { + $this->applyRouteMiddleware($request, $app, $currentMiddleware, $chain); + } + + $groupKey = $request->isApiRequest() ? 'api' : 'web'; + foreach ($this->gateway->getMiddlewareGroups()[$groupKey] ?? [] as $middlewareClass) { + $chain->applyMiddleware($this->makeGatewayMiddleware($app, $middlewareClass)); + } + + foreach ($this->gateway->getGlobalMiddleware() as $middlewareClass) { + $chain->applyMiddleware($this->makeGatewayMiddleware($app, $middlewareClass)); + } + + return $chain->handle($request, $handler); } /** diff --git a/src/Phaseolies/Support/View/View.php b/src/Phaseolies/Support/View/View.php index faebe72e..1bbaec82 100644 --- a/src/Phaseolies/Support/View/View.php +++ b/src/Phaseolies/Support/View/View.php @@ -103,6 +103,11 @@ public function fetch($name, array $data = []): string $this->renderStack[] = $name; + $savedParents = $this->parents; + $hadCurrentBlock = array_key_exists('__current_template__', $this->blocks); + $savedCurrentBlock = $this->blocks['__current_template__'] ?? null; + $this->parents = []; + try { $this->parents[] = $name; extract($data, EXTR_SKIP); @@ -117,6 +122,14 @@ public function fetch($name, array $data = []): string return self::$cache[$cacheKey] = $result; } finally { + $this->parents = $savedParents; + + if ($hadCurrentBlock) { + $this->blocks['__current_template__'] = $savedCurrentBlock; + } else { + unset($this->blocks['__current_template__']); + } + if (!empty($this->renderStack)) { array_pop($this->renderStack); } diff --git a/tests/API/Presenter/PresenterBundleTest.php b/tests/API/Presenter/PresenterBundleTest.php index 04f76db3..5c2caf41 100644 --- a/tests/API/Presenter/PresenterBundleTest.php +++ b/tests/API/Presenter/PresenterBundleTest.php @@ -17,10 +17,19 @@ protected function setUp(): void { parent::setUp(); $_SESSION = []; + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // global app()/request() helpers under test to see it. $this->container = new Container(); + Container::setInstance($this->container); $this->container->bind('request', fn() => new Request()); } + protected function tearDown(): void + { + Container::forgetInstance(); + } + protected function createTestPresenterClass() { return new class(['id' => 1]) extends Presenter { diff --git a/tests/API/Presenter/SQLitePresenterTest.php b/tests/API/Presenter/SQLitePresenterTest.php index 635f755b..8ffc5b92 100644 --- a/tests/API/Presenter/SQLitePresenterTest.php +++ b/tests/API/Presenter/SQLitePresenterTest.php @@ -22,8 +22,10 @@ class SQLitePresenterTest extends TestCase protected function setUp(): void { - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('request', fn() => new Request()); $container->bind('url', fn() => UrlGenerator::class); $container->bind('db', fn() => new Database('default')); @@ -39,6 +41,7 @@ protected function tearDown(): void { $this->pdo = null; $this->tearDownDatabaseConnections(); + Container::forgetInstance(); } private function createTestTables(): void diff --git a/tests/Application/ApplicationTest.php b/tests/Application/ApplicationTest.php index 0ee2a634..3384768e 100644 --- a/tests/Application/ApplicationTest.php +++ b/tests/Application/ApplicationTest.php @@ -100,6 +100,16 @@ protected function tearDown(): void GhostableTestProvider::resetState(); $this->app->flush(); Container::forgetInstance(); + + // Config::$configFiles is memoized process-wide and never + // invalidated on its own; a couple of tests in this class touch + // the real Config class against $this->tempBasePath (see + // testTerminateResetsRuntimeConfigOverrides), and without this, + // that memoized file list would dangle once tempBasePath is + // deleted below — corrupting Config for whichever test runs next + // in this same PHPUnit process. + Config::clearCache(); + $this->deleteDirectory($this->tempBasePath); } @@ -648,4 +658,94 @@ public function testTerminateStillCleansRequestScopedServicesWithoutCallbacks(): $this->assertFalse($this->app->hasInstance('session')); $this->assertFalse($this->app->hasInstance('redirect')); } + + /** + * Worker-mode regression: a class bound *while handling a request* + * (mirroring what Router::resolveFormRequestValidationClass() and + * #[Bind]/#[Resolver] attributes do mid-dispatch for FormRequest + * classes and route dependencies) must not silently turn into a + * permanent cross-request singleton. Under a persistent worker + * (Swoole/FrankenPHP/RoadRunner) reusing one Application across many + * requests, the second request must get its own fresh instance rather + * than the first request's — see Container::snapshotBootBindings() + * and Application::cleanupRequestScopedServices(). + */ + public function testDynamicRequestScopedBindingIsNotReusedAcrossDispatches(): void + { + $response = $this->getMockBuilder(Response::class) + ->disableOriginalConstructor() + ->onlyMethods(['prepare', 'send']) + ->getMock(); + $response->method('prepare')->willReturnSelf(); + $response->method('send')->willReturnSelf(); + + $seenInstances = []; + + $router = $this->createMock(Router::class); + $router->method('resolve')->willReturnCallback(function () use ($response, &$seenInstances) { + if (!$this->app->has(WorkerModeDummyFormRequest::class)) { + $this->app->singleton(WorkerModeDummyFormRequest::class, fn() => new WorkerModeDummyFormRequest()); + } + + $seenInstances[] = $this->app->make(WorkerModeDummyFormRequest::class); + + return $response; + }); + $this->app->router = $router; + + $this->app->dispatch(new Request()); + $this->app->terminate(new Request(), $response); + + $this->app->dispatch(new Request()); + $this->app->terminate(new Request(), $response); + + $this->assertCount(2, $seenInstances); + $this->assertNotSame( + $seenInstances[0], + $seenInstances[1], + 'A binding registered while handling one request must not be reused by the next request under a persistent worker.' + ); + + // The binding definition itself is fine to keep (cheap, and lets + // the next request resolve fresh) — only the *resolved instance* + // must not survive. + $this->assertTrue($this->app->has(WorkerModeDummyFormRequest::class)); + } + + /** + * Worker-mode regression: Config::set()/Application::setLocale() calls + * made while handling one request (e.g. from middleware) must not + * leak into whichever request the same persistent worker serves next. + */ + public function testTerminateResetsRuntimeConfigOverrides(): void + { + // Establish a known baseline via the same reset path terminate() + // uses, regardless of what state earlier tests in this process + // left Config in. Config::get() ensures initialize() has run + // (Config::all() alone does not, and will warn on a null cache + // file path if called first in a fresh process). + Config::get('app.locale'); + Config::resetRuntimeOverrides(); + $bootState = Config::all(); + + Config::set('app.locale', '__worker_leak_probe__'); + $this->assertSame('__worker_leak_probe__', Config::get('app.locale')); + + $this->app->terminate(new Request(), new Response('ok')); + + $this->assertSame( + $bootState, + Config::all(), + 'A config mutation made while handling one request must not leak into the next request a persistent worker serves.' + ); + } +} + +/** + * A minimal stand-in for a FormRequest/route-dependency class that the + * router binds the first time a route needs it — see + * testDynamicRequestScopedBindingIsNotReusedAcrossDispatches(). + */ +class WorkerModeDummyFormRequest +{ } diff --git a/tests/Application/ContainerTest.php b/tests/Application/ContainerTest.php index d2be6b87..6a1a4081 100644 --- a/tests/Application/ContainerTest.php +++ b/tests/Application/ContainerTest.php @@ -87,16 +87,11 @@ protected function setUp(): void protected function resetContainer(): void { - $reflection = new \ReflectionClass(Container::class); - - $bindings = $reflection->getProperty('bindings'); - $bindings->setValue(null, []); - - $instances = $reflection->getProperty('instances'); - $instances->setValue(null, []); - - $instance = $reflection->getProperty('instance'); - $instance->setValue(null, null); + // $bindings/$instances are per-instance now (see [[ArchNotes]]), so a + // fresh Container() already starts empty — no reflection needed for + // those. Only the static "active instance" pointer still needs + // clearing between tests. + Container::forgetInstance(); } protected function tearDown(): void @@ -2281,7 +2276,6 @@ public function testFlushClearsResolvingState() $this->assertEquals('value', $result); } - // has issue public function testMultipleContainerInstances() { $container1 = new Container(); @@ -2290,9 +2284,13 @@ public function testMultipleContainerInstances() $container1->bind('service', fn() => 'container1'); $container2->bind('service', fn() => 'container2'); - // creating multiple instances of Container is meaningless - // every instance is just a handle to the same static state. - $this->assertEquals('container2', $container1->get('service')); + // $bindings/$instances are per-instance (see [[ArchNotes]] on the + // property declarations), so each Container is its own isolated + // registry — required for per-request isolation under a persistent + // worker runtime (Swoole/FrankenPHP/RoadRunner), where a fresh + // Container/Application per request must not see another request's + // bindings. + $this->assertEquals('container1', $container1->get('service')); $this->assertEquals('container2', $container2->get('service')); } @@ -2301,8 +2299,12 @@ public function testStaticInstanceIsolation() Container::setInstance($this->container); $this->container->bind('service', fn() => 'value'); + // Container::$instance is still a static "currently active container" + // pointer (intentional — it's what lets `app()`/facades resolve + // without a reference in hand), but a *new* Container() no longer + // inherits its bindings: storage itself is per-instance now. $newContainer = new Container(); - $this->assertTrue($newContainer->has('service')); + $this->assertFalse($newContainer->has('service')); } public function testBindingPriorityOverAutoResolution() diff --git a/tests/Application/TimezoneHandlerTest.php b/tests/Application/TimezoneHandlerTest.php index 3013e93a..8660b19c 100644 --- a/tests/Application/TimezoneHandlerTest.php +++ b/tests/Application/TimezoneHandlerTest.php @@ -13,13 +13,18 @@ class TimezoneHandlerTest extends TestCase protected function setUp(): void { + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // global app() helper under test to see it. $this->container = new Container; + Container::setInstance($this->container); } protected function tearDown(): void { date_default_timezone_set('UTC'); Carbon::setTestNow(); + Container::forgetInstance(); parent::tearDown(); } diff --git a/tests/Builder/BuilderSQLiteTest.php b/tests/Builder/BuilderSQLiteTest.php index 632cce36..5745d8c1 100644 --- a/tests/Builder/BuilderSQLiteTest.php +++ b/tests/Builder/BuilderSQLiteTest.php @@ -20,7 +20,11 @@ class BuilderSQLiteTest extends TestCase protected function setUp(): void { + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must also be activated for + // the global app()/request() helpers under test to see it. $container = new Container(); + Container::setInstance($container); $container->bind('request', fn() => new Request()); $container->bind('url', fn() => new UrlGenerator()); @@ -2204,6 +2208,7 @@ protected function tearDown(): void // Clean up $this->pdo = null; $this->builder = null; + Container::forgetInstance(); } } diff --git a/tests/Builder/Query/EntityBuilderQueryTest.php b/tests/Builder/Query/EntityBuilderQueryTest.php index 6378c351..2c9ed9fb 100644 --- a/tests/Builder/Query/EntityBuilderQueryTest.php +++ b/tests/Builder/Query/EntityBuilderQueryTest.php @@ -18,8 +18,10 @@ class EntityBuilderQueryTest extends TestCase protected function setUp(): void { - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('request', fn() => new Request()); $container->bind('url', fn() => UrlGenerator::class); $container->bind('db', fn() => new Database('default')); @@ -36,6 +38,7 @@ protected function tearDown(): void { $this->pdo = null; $this->tearDownDatabaseConnections(); + Container::forgetInstance(); } private function createTestTables(): void diff --git a/tests/Builder/QueryBuilderTest.php b/tests/Builder/QueryBuilderTest.php index 0d283851..9f6a315b 100644 --- a/tests/Builder/QueryBuilderTest.php +++ b/tests/Builder/QueryBuilderTest.php @@ -19,6 +19,11 @@ class QueryBuilderTest extends TestCase protected function setUp(): void { + $container = new Container(); + Container::setInstance($container); + $container->bind('request', fn() => new Request()); + $container->bind('url', fn() => new UrlGenerator()); + $this->pdo = new PDO('sqlite::memory:'); $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); @@ -1168,5 +1173,6 @@ protected function tearDown(): void { $this->pdo = null; $this->builder = null; + Container::forgetInstance(); } } diff --git a/tests/Controller/ControllerTest.php b/tests/Controller/ControllerTest.php index b59e0a07..d30a0e4a 100644 --- a/tests/Controller/ControllerTest.php +++ b/tests/Controller/ControllerTest.php @@ -14,12 +14,20 @@ class ControllerTest extends TestCase protected function setUp(): void { parent::setUp(); - $container = new Container(); - Container::setInstance(new MockContainer()); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('view', \Phaseolies\Support\View\Factory::class); $this->controller = new Controller(); } + protected function tearDown(): void + { + Container::forgetInstance(); + parent::tearDown(); + } + public function testConstructorInitialization(): void { $this->assertInstanceOf(Controller::class, $this->controller); diff --git a/tests/Logger/LoggerHelperTest.php b/tests/Logger/LoggerHelperTest.php index dbc33abf..2e2fdeb6 100644 --- a/tests/Logger/LoggerHelperTest.php +++ b/tests/Logger/LoggerHelperTest.php @@ -13,13 +13,22 @@ class LoggerHelperTest extends TestCase protected function setUp(): void { + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // Log facade's Container::getInstance() fallback to see it. $container = new Container(); + Container::setInstance($container); $this->logger = new FakeLogger(); $container->instance('log', $this->logger); Log::setFacadeApplication(null); } + protected function tearDown(): void + { + Container::forgetInstance(); + } + #[DataProvider('helperProvider')] public function testLogHelpersForwardPayloadAndContext(string $helper, string $expectedLevel): void { diff --git a/tests/PaginatorTest.php b/tests/PaginatorTest.php index 0b30ff43..cf553937 100644 --- a/tests/PaginatorTest.php +++ b/tests/PaginatorTest.php @@ -22,13 +22,22 @@ class PaginatorTest extends TestCase protected function setUp(): void { $_SESSION = []; + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // global app()/request() helpers under test to see it. $container = new Container; + Container::setInstance($container); $container->singleton('request', Request::class); $this->paginator = new Paginator($this->testData); } + protected function tearDown(): void + { + Container::forgetInstance(); + } + public function testHasPages() { // Test when there are multiple pages diff --git a/tests/Providers/BaseFacadeTest.php b/tests/Providers/BaseFacadeTest.php index 8be14d84..92a21b8f 100644 --- a/tests/Providers/BaseFacadeTest.php +++ b/tests/Providers/BaseFacadeTest.php @@ -45,7 +45,12 @@ class BaseFacadeTest extends TestCase protected function setUp(): void { + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for + // BaseFacade::resolveInstance()'s Container::getInstance() fallback + // to see it. $this->container = new Container(); + Container::setInstance($this->container); $this->app = new MockApplication($this->container); $this->container->bind('test-service', function () { @@ -55,6 +60,11 @@ protected function setUp(): void TestFacade::setFacadeApplication(null); } + protected function tearDown(): void + { + Container::forgetInstance(); + } + protected function callResolveInstance($facadeClass) { $reflection = new \ReflectionClass($facadeClass); diff --git a/tests/RedirectResponseTest.php b/tests/RedirectResponseTest.php index d1d3b813..59fcaa00 100644 --- a/tests/RedirectResponseTest.php +++ b/tests/RedirectResponseTest.php @@ -115,7 +115,11 @@ class RedirectResponseTest extends TestCase protected function setUp(): void { + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // global app()/request() helpers under test to see it. $container = new Container(); + Container::setInstance($container); $container->bind('session', Session::class); $container->bind('str', StringService::class); @@ -136,6 +140,11 @@ protected function setUp(): void $this->replaceMessageBag(); } + protected function tearDown(): void + { + Container::forgetInstance(); + } + private function replaceRouterWithMock() { // Use reflection to replace the Router class reference in RedirectResponse diff --git a/tests/Requests/AllRequestInputTest.php b/tests/Requests/AllRequestInputTest.php index 828bdbc1..069fa376 100644 --- a/tests/Requests/AllRequestInputTest.php +++ b/tests/Requests/AllRequestInputTest.php @@ -21,13 +21,15 @@ class AllRequestInputTest extends TestCase protected function setUp(): void { - $container = new Container(); - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the single container instance we activate — bindings + // are per-instance now (see [[ArchNotes]] in DI/Container.php), so + // reassigning $container to a new Container() (as this setUp used + // to do) silently discarded every earlier bind() call. + $container = new MockContainer(); + Container::setInstance($container); $container->bind('request', fn() => Request::class); - $container = new Container(); - $this->request = Request::createFromGlobals(); $container->bind('str', StringService::class); + $this->request = Request::createFromGlobals(); $this->defaultServerData = [ 'REQUEST_METHOD' => 'GET', @@ -54,6 +56,7 @@ protected function tearDown(): void $this->resetGlobals(); Request::setTrustedProxies([], -1); Request::setTrustedHosts([]); + Container::forgetInstance(); } protected function resetGlobals(): void diff --git a/tests/Requests/RequestParserTraitTest.php b/tests/Requests/RequestParserTraitTest.php index 582fa768..67778945 100644 --- a/tests/Requests/RequestParserTraitTest.php +++ b/tests/Requests/RequestParserTraitTest.php @@ -21,7 +21,11 @@ protected function setUp(): void { parent::setUp(); + // Bindings are per-instance now (see [[ArchNotes]] in + // DI/Container.php), so this container must be activated for the + // global app()/request() helpers under test to see it. $this->container = new Container(); + Container::setInstance($this->container); $this->request = new Request(); $this->container->singleton('request', fn() => $this->request); $this->container->singleton('route', Router::class); @@ -41,6 +45,7 @@ protected function tearDown(): void $_POST = []; $_COOKIE = []; $_FILES = []; + Container::forgetInstance(); parent::tearDown(); } diff --git a/tests/ResponseLifecycleTest.php b/tests/ResponseLifecycleTest.php index 59f0ce5f..5d26af32 100644 --- a/tests/ResponseLifecycleTest.php +++ b/tests/ResponseLifecycleTest.php @@ -50,6 +50,16 @@ public function getPath(): string return $this->testPath; } + public function getRequestUri(): string + { + // Group middleware (CSRF, etc.) is resolved on every dispatch now + // (see [[ArchNotes]] in Support/Router.php) and needs a working + // uri()/isApiRequest(), which the real Request derives from + // $this->server — never initialized here since this stub skips + // Request::__construct(). + return $this->testPath; + } + public function getHost(): string { return $this->testHost; @@ -142,6 +152,16 @@ public function testControllerReturnAndResponseHelperProduceSameJsonLifecycleOut $router->get('/payload', fn() => $payload); $request = new LifecycleRouteRequestStub('GET', '/payload', 'localhost'); + $this->container->instance('request', $request); + + // Group middleware (CSRF, etc.) is resolved on every dispatch now + // (see [[ArchNotes]] in Support/Router.php), so $app->make() needs + // to actually build the 'web' group's CsrfTokenMiddleware, which + // in turn needs the Str facade's 'str' binding. + $this->container->instance('str', new \Phaseolies\Support\StringService()); + $app->method('make')->willReturnCallback( + fn($abstract, $parameters = []) => $this->container->make($abstract, $parameters) + ); $routeResponse = $router->resolve($app, $request); $helperResponse = response($payload); @@ -164,6 +184,17 @@ public function testHeadPreparationStripsBodyForControllerAndHelperJsonResponses $router->get('/payload', fn() => $payload); $routeRequest = new LifecycleRouteRequestStub('GET', '/payload', 'localhost'); + $this->container->instance('request', $routeRequest); + + // Group middleware (CSRF, etc.) is resolved on every dispatch now + // (see [[ArchNotes]] in Support/Router.php), so $app->make() needs + // to actually build the 'web' group's CsrfTokenMiddleware, which + // in turn needs the Str facade's 'str' binding. + $this->container->instance('str', new \Phaseolies\Support\StringService()); + $app->method('make')->willReturnCallback( + fn($abstract, $parameters = []) => $this->container->make($abstract, $parameters) + ); + $routeResponse = $router->resolve($app, $routeRequest); $helperResponse = response($payload); diff --git a/tests/Router/RouterTest.php b/tests/Router/RouterTest.php index c0755c5a..e266d933 100644 --- a/tests/Router/RouterTest.php +++ b/tests/Router/RouterTest.php @@ -42,6 +42,16 @@ public function getPath(): string return $this->testPath; } + public function getRequestUri(): string + { + // Group middleware (CSRF, etc.) is resolved on every dispatch now + // (see [[ArchNotes]] in Support/Router.php) and needs a working + // uri()/isApiRequest(), which the real Request derives from + // $this->server — never initialized here since this stub skips + // Request::__construct(). + return $this->testPath; + } + public function getHost(): string { return $this->testHost; @@ -60,6 +70,22 @@ public function setRouteParams(array $params): self } } +/** + * A route middleware that just counts how many times it actually ran — + * see testRouteMiddlewareDoesNotAccumulateAcrossDispatches(). + */ +class WorkerModeCountingMiddleware implements \Phaseolies\Middleware\Contracts\Middleware +{ + public static int $count = 0; + + public function __invoke($request, $next) + { + self::$count++; + + return $next($request); + } +} + class TestableRouter extends Router { public function handle(Request $request, \Closure $handler): Response @@ -434,6 +460,18 @@ public function testResolveUsesFreshResponseForScalarRouteResults(): void $freshRouter->get('/fresh', fn() => 'fresh body'); $request = new TestRequestStub('GET', '/fresh', 'localhost'); + Container::getInstance()->instance('request', $request); + + // Group middleware (CSRF, etc.) is resolved on every dispatch now + // (see [[ArchNotes]] in Support/Router.php), so $this->app->make() + // needs to actually build the 'web' group's CsrfTokenMiddleware + // rather than return a bare mock default, and CsrfTokenMiddleware + // itself needs the Str facade's 'str' binding. + Container::getInstance()->instance('str', new \Phaseolies\Support\StringService()); + $this->app->method('make')->willReturnCallback( + fn($abstract, $parameters = []) => Container::getInstance()->make($abstract, $parameters) + ); + $response = $freshRouter->resolve($this->app, $request); $this->assertNotSame($sharedResponse, $response); @@ -442,6 +480,53 @@ public function testResolveUsesFreshResponseForScalarRouteResults(): void $this->assertSame(200, $response->getStatusCode()); } + /** + * Worker-mode regression: Router::resolve() used to apply route + * middleware directly onto the shared Gateway singleton's chain + * (`$this->gateway->applyMiddleware()`), which only ever wraps its + * current chain, never resets it. Reusing one Router/Gateway across + * multiple dispatches — exactly what a persistent worker + * (Swoole/FrankenPHP/RoadRunner) does — would make every dispatch's + * middleware permanently stack on top of every dispatch before it, so + * the Nth request would run its middleware N times. resolve() now + * builds a fresh, request-local chain every call instead. + */ + public function testRouteMiddlewareDoesNotAccumulateAcrossDispatches(): void + { + WorkerModeCountingMiddleware::$count = 0; + + $gateway = new Gateway(); + $gateway->routeMiddleware['web']['counter'] = WorkerModeCountingMiddleware::class; + + $router = new TestableRouter($gateway); + $router->get('/counted', fn() => 'ok')->middleware('counter'); + + Container::getInstance()->instance('str', new \Phaseolies\Support\StringService()); + $this->app->method('make')->willReturnCallback( + fn($abstract, $parameters = []) => Container::getInstance()->make($abstract, $parameters) + ); + + $request1 = new TestRequestStub('GET', '/counted', 'localhost'); + Container::getInstance()->instance('request', $request1); + $router->resolve($this->app, $request1); + + $this->assertSame( + 1, + WorkerModeCountingMiddleware::$count, + 'route middleware should run exactly once on the first dispatch' + ); + + $request2 = new TestRequestStub('GET', '/counted', 'localhost'); + Container::getInstance()->instance('request', $request2); + $router->resolve($this->app, $request2); + + $this->assertSame( + 2, + WorkerModeCountingMiddleware::$count, + 'reusing the same Router/Gateway across two dispatches must run route middleware exactly once per dispatch, not accumulate a stacked chain from the previous request' + ); + } + public function testViewHelperReturnsFreshResponseInstance(): void { $sharedResponse = new Response('stale body', 202, ['X-Leaked' => 'yes']); diff --git a/tests/Support/Database/ModelQueryDriverTestCase.php b/tests/Support/Database/ModelQueryDriverTestCase.php index 0a42ce0c..683395df 100644 --- a/tests/Support/Database/ModelQueryDriverTestCase.php +++ b/tests/Support/Database/ModelQueryDriverTestCase.php @@ -42,6 +42,7 @@ final protected function tearDown(): void { $this->tearDownDatabaseConnections(); unset($this->pdo); + Container::forgetInstance(); parent::tearDown(); } @@ -60,8 +61,10 @@ abstract protected function seedData(): array; protected function bootContainer(): void { - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('request', fn() => new Request()); $container->bind('url', fn() => UrlGenerator::class); $container->bind('db', fn() => new Database('default')); diff --git a/tests/Support/View/ViewFetchReentrancyTest.php b/tests/Support/View/ViewFetchReentrancyTest.php new file mode 100644 index 00000000..b495f117 --- /dev/null +++ b/tests/Support/View/ViewFetchReentrancyTest.php @@ -0,0 +1,117 @@ +links()/linkWithJumps() while a matching + * vendor/pagination/*.odo.php template existed. + */ +class ViewFetchReentrancyTest extends TestCase +{ + private Controller $controller; + private string $viewDir; + + protected function setUp(): void + { + parent::setUp(); + + $container = new MockContainer(); + Container::setInstance($container); + $container->bind('view', \Phaseolies\Support\View\Factory::class); + + $this->viewDir = sys_get_temp_dir() . '/doppar_view_reentrancy_' . uniqid(); + mkdir($this->viewDir, 0777, true); + + $this->controller = new Controller(); + $this->controller->setViewFolder($this->viewDir); + + // View::$cache is keyed by view name + data only (not file content + // or mtime), and is static/process-wide — reset it so each test's + // "page"/"partial" names don't collide with another test's cached + // rendering of a same-named-but-different-content view. + $reflection = new \ReflectionClass(\Phaseolies\Support\View\View::class); + $cache = $reflection->getProperty('cache'); + $cache->setValue(null, []); + } + + protected function tearDown(): void + { + Container::forgetInstance(); + + foreach (glob($this->viewDir . '/*') ?: [] as $file) { + unlink($file); + } + rmdir($this->viewDir); + + parent::tearDown(); + } + + private function putView(string $name, string $contents): void + { + file_put_contents($this->viewDir . '/' . $name . '.odo.php', $contents); + } + + public function testNestedFetchDoesNotCorruptAnInFlightExtendsRender(): void + { + // A layout consumed via #extends/#yield — mirrors a real app's + // layouts.app.odo.php wrapping page content. + $this->putView('layout', 'LAYOUT-BEFORE|#yield(\'content\')|LAYOUT-AFTER'); + + // The page being rendered makes a *nested* fetch() call from + // inside its own content section — exactly what + // Paginator::links()/linkWithJumps() does when a matching + // vendor/pagination/*.odo.php view exists. + $this->putView( + 'page', + "#extends('layout')\n#section('content')\nPAGE-BEFORE|[[! \$this->fetch('partial', []) !]]|PAGE-AFTER\n#endsection" + ); + + // A standalone partial — does not itself extend anything. + $this->putView('partial', 'PARTIAL-CONTENT'); + + $html = $this->controller->render('page', [], true); + + $this->assertSame( + 'LAYOUT-BEFORE|PAGE-BEFORE|PARTIAL-CONTENT|PAGE-AFTER|LAYOUT-AFTER', + $this->normalize($html) + ); + } + + public function testDeeplyNestedFetchesAreEachFullyIsolated(): void + { + $this->putView('layout', 'L[#yield(\'content\')]L'); + + $this->putView( + 'page', + "#extends('layout')\n#section('content')\n" . + "P1[[! \$this->fetch('partial-a', []) !]]P2[[! \$this->fetch('partial-b', []) !]]P3\n" . + "#endsection" + ); + + // partial-a itself makes another nested call. + $this->putView('partial-a', "A1[[! \$this->fetch('partial-nested', []) !]]A2"); + $this->putView('partial-nested', 'NESTED'); + $this->putView('partial-b', 'B'); + + $html = $this->controller->render('page', [], true); + + $this->assertSame('L[P1A1NESTEDA2P2BP3]L', trim($html)); + } +} diff --git a/tests/Validation/FileValidationRuleTest.php b/tests/Validation/FileValidationRuleTest.php index 283990ba..a92e470c 100644 --- a/tests/Validation/FileValidationRuleTest.php +++ b/tests/Validation/FileValidationRuleTest.php @@ -21,8 +21,10 @@ protected function setUp(): void { parent::setUp(); - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('translator', function () { $loader = $this->createMock(FileLoader::class); return new Translator($loader, 'en'); @@ -43,6 +45,7 @@ protected function tearDown(): void } rmdir($this->tmpDir); + Container::forgetInstance(); parent::tearDown(); } diff --git a/tests/Validation/SanitizerTest.php b/tests/Validation/SanitizerTest.php index 4e72b0f6..aa0ae4ad 100644 --- a/tests/Validation/SanitizerTest.php +++ b/tests/Validation/SanitizerTest.php @@ -20,8 +20,10 @@ class SanitizerTest extends TestCase protected function setUp(): void { parent::setUp(); - Container::setInstance(new MockContainer()); - $container = new Container(); + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php). + $container = new MockContainer(); + Container::setInstance($container); $container->bind('translator', function () { // Mock the FileLoader dependency $loader = $this->createMock(FileLoader::class); @@ -68,6 +70,12 @@ public function get($key, $replace = [], $default = null) } } + protected function tearDown(): void + { + Container::forgetInstance(); + parent::tearDown(); + } + public function testConstructorAndRequestMethod(): void { $data = ['name' => 'John']; diff --git a/tests/Validation/ValidationRulesExtendedTest.php b/tests/Validation/ValidationRulesExtendedTest.php index 337ee3c8..b0f15ace 100644 --- a/tests/Validation/ValidationRulesExtendedTest.php +++ b/tests/Validation/ValidationRulesExtendedTest.php @@ -16,14 +16,26 @@ class ValidationRulesExtendedTest extends TestCase protected function setUp(): void { parent::setUp(); - Container::setInstance(new MockContainer()); - $container = new Container(); + + // Bind onto the same container instance we activate — bindings are + // per-instance now (see [[ArchNotes]] in DI/Container.php), so a + // binding registered on a throwaway, never-activated Container() + // is no longer visible to the global app()/trans() helpers this + // suite exercises. + $container = new MockContainer(); + Container::setInstance($container); $container->bind('translator', function () { $loader = $this->createMock(FileLoader::class); return new Translator($loader, 'en'); }); } + protected function tearDown(): void + { + Container::forgetInstance(); + parent::tearDown(); + } + private function passes(array $data, array $rules): bool { return (new Sanitizer($data, $rules))->validate(); From c59be7daa3827fed4c8d882a798a19b54e418e86 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Tue, 22 Sep 2026 14:43:24 +0600 Subject: [PATCH 2/3] container bindings static to this scope --- .../Http/Controllers/Controller.php | 2 +- .../Support/View/ViewFetchReentrancyTest.php | 26 +++++++------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/Phaseolies/Http/Controllers/Controller.php b/src/Phaseolies/Http/Controllers/Controller.php index 23bacede..70185fa5 100644 --- a/src/Phaseolies/Http/Controllers/Controller.php +++ b/src/Phaseolies/Http/Controllers/Controller.php @@ -413,7 +413,7 @@ public function prepare(string $view): string $viewKey = str_replace(['/', '\\', DIRECTORY_SEPARATOR], '.', $view); // stronger hash to avoid collisions - $hash = hash('xxh128', $viewKey); + $hash = hash('xxh128', $viewKey . '|' . $actual); $cache = base_path($this->cacheFolder) . DIRECTORY_SEPARATOR . $viewKey . '__' . $hash . '.php'; $needsRecompile = $this->needsRecompilation($cache, $actual); diff --git a/tests/Support/View/ViewFetchReentrancyTest.php b/tests/Support/View/ViewFetchReentrancyTest.php index b495f117..880097ca 100644 --- a/tests/Support/View/ViewFetchReentrancyTest.php +++ b/tests/Support/View/ViewFetchReentrancyTest.php @@ -7,22 +7,6 @@ use Phaseolies\DI\Container; use PHPUnit\Framework\TestCase; -/** - * Regression test for a nested-render corruption bug in View::fetch(). - * - * The Controller/View instance is a singleton for the whole request (see - * RippleLauncher's `Controller::class` binding), and $parents / the - * '__current_template__' block used to be flat instance state shared by - * every fetch() call. A *nested* fetch() — e.g. a pagination widget - * rendering its own partial view from inside an already in-flight - * #extends page — would drain the outer call's still-pending parent - * template (its layout) via the shared $parents queue, then overwrite - * '__current_template__' with its own result, leaving the outer render - * with nothing once it returned. In production this showed up as a - * completely blank page on any view whose layout called - * paginator(...)->links()/linkWithJumps() while a matching - * vendor/pagination/*.odo.php template existed. - */ class ViewFetchReentrancyTest extends TestCase { private Controller $controller; @@ -63,6 +47,14 @@ protected function tearDown(): void parent::tearDown(); } + private function normalize(string $html): string + { + // Collapse the newlines left around #section/#endsection content + // when it's substituted into the layout's #yield, so we can compare + // against the flat pipe-delimited expectation. + return trim(preg_replace('/\R+/', '', $html)); + } + private function putView(string $name, string $contents): void { file_put_contents($this->viewDir . '/' . $name . '.odo.php', $contents); @@ -112,6 +104,6 @@ public function testDeeplyNestedFetchesAreEachFullyIsolated(): void $html = $this->controller->render('page', [], true); - $this->assertSame('L[P1A1NESTEDA2P2BP3]L', trim($html)); + $this->assertSame('L[P1A1NESTEDA2P2BP3]L', $this->normalize($html)); } } From acfc0a2a60869f2a7901f7c91ffa8f70899fef08 Mon Sep 17 00:00:00 2001 From: Arif Hoque Date: Tue, 22 Sep 2026 18:19:43 +0600 Subject: [PATCH 3/3] createFromGlobals only call once not duplicate --- src/Phaseolies/Application.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Phaseolies/Application.php b/src/Phaseolies/Application.php index 1a7902fd..b5aa5463 100644 --- a/src/Phaseolies/Application.php +++ b/src/Phaseolies/Application.php @@ -802,7 +802,8 @@ public function bindApplicationNecessaryPath(): void protected function bindSingletonClasses(): void { $this->bindApplicationNecessaryPath(); - $this->singleton('request', fn() => Request::createFromGlobals()); + + $this->singleton('request', fn() => Request::capture()); $this->bindHttpGateway();