From 5d3b3d3c21375aabd86d653824955a1e13dad36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Thu, 16 Jan 2025 10:30:06 +0100 Subject: [PATCH] [4.x] Improve RootUrl and UrlGenerator bootstrappers (#1294) * Make RootUrlBootstrapper run ONLY in CLI by default (add $rootUrlOverrideInTests), work with resolved UrlGenerator * Make resolving 'url' return a pre-created generator instance instead of creating it on every app('url') call * Take care of doubling tenant keys in TenancyUrlGenerator, add regression test for using UrlGenerator and RootUrl bootstrappers together * Fix code style (php-cs-fixer) * refactor RootUrlBootstrapper * add docblock * clarify docblock * simplify test: use concrete values instead of overly dynamic code * Fix bootstrapper order in test, add url('/') assertion * Use $this->app instead of app() * Improve TenancyUrlGenerator and RootUrlBootstrapperTest clarity * Revert attempt to maintain compatibility between the two bootstrappers * Delete bootstrapper combining test * Fix code style (php-cs-fixer) --------- Co-authored-by: lukinovec Co-authored-by: PHP CS Fixer --- src/Bootstrappers/RootUrlBootstrapper.php | 37 ++++++++++++++----- .../UrlGeneratorBootstrapper.php | 32 ++++++++-------- .../Bootstrappers/RootUrlBootstrapperTest.php | 7 +++- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/Bootstrappers/RootUrlBootstrapper.php b/src/Bootstrappers/RootUrlBootstrapper.php index 6a523673..3a650169 100644 --- a/src/Bootstrappers/RootUrlBootstrapper.php +++ b/src/Bootstrappers/RootUrlBootstrapper.php @@ -7,7 +7,6 @@ namespace Stancl\Tenancy\Bootstrappers; use Closure; use Illuminate\Config\Repository; use Illuminate\Contracts\Foundation\Application; -use Illuminate\Routing\UrlGenerator; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; @@ -36,28 +35,46 @@ class RootUrlBootstrapper implements TenancyBootstrapper protected string|null $originalRootUrl = null; + /** + * You may want to selectively enable or disable this bootstrapper in specific tests. + * For instance, when using `Livewire::test()` this bootstrapper can cause problems, + * due to an internal Livewire route, so you may want to disable it, while in tests + * that are generating URLs in things like mails, the bootstrapper should be used + * just like in any queued job. + */ + public static bool $rootUrlOverrideInTests = false; + public function __construct( - protected UrlGenerator $urlGenerator, protected Repository $config, protected Application $app, ) {} public function bootstrap(Tenant $tenant): void { - if ($this->app->runningInConsole() && static::$rootUrlOverride) { - $this->originalRootUrl = $this->urlGenerator->to('/'); - - $newRootUrl = (static::$rootUrlOverride)($tenant, $this->originalRootUrl); - - $this->urlGenerator->forceRootUrl($newRootUrl); - $this->config->set('app.url', $newRootUrl); + if (static::$rootUrlOverride === null) { + return; } + + if (! $this->app->runningInConsole()) { + return; + } + + if ($this->app->runningUnitTests() && ! static::$rootUrlOverrideInTests) { + return; + } + + $this->originalRootUrl = $this->app['url']->to('/'); + + $newRootUrl = (static::$rootUrlOverride)($tenant, $this->originalRootUrl); + + $this->app['url']->forceRootUrl($newRootUrl); + $this->config->set('app.url', $newRootUrl); } public function revert(): void { if ($this->originalRootUrl) { - $this->urlGenerator->forceRootUrl($this->originalRootUrl); + $this->app['url']->forceRootUrl($this->originalRootUrl); $this->config->set('app.url', $this->originalRootUrl); } } diff --git a/src/Bootstrappers/UrlGeneratorBootstrapper.php b/src/Bootstrappers/UrlGeneratorBootstrapper.php index e3bb4a99..15116760 100644 --- a/src/Bootstrappers/UrlGeneratorBootstrapper.php +++ b/src/Bootstrappers/UrlGeneratorBootstrapper.php @@ -37,7 +37,7 @@ class UrlGeneratorBootstrapper implements TenancyBootstrapper public function revert(): void { - $this->app->bind('url', fn () => $this->originalUrlGenerator); + $this->app->extend('url', fn () => $this->originalUrlGenerator); } /** @@ -47,24 +47,22 @@ class UrlGeneratorBootstrapper implements TenancyBootstrapper */ protected function useTenancyUrlGenerator(): void { - $this->app->extend('url', function (UrlGenerator $urlGenerator, Application $app) { - $newGenerator = new TenancyUrlGenerator( - $app['router']->getRoutes(), - $urlGenerator->getRequest(), - $app['config']->get('app.asset_url'), - ); + $newGenerator = new TenancyUrlGenerator( + $this->app['router']->getRoutes(), + $this->originalUrlGenerator->getRequest(), + $this->app['config']->get('app.asset_url'), + ); - $newGenerator->defaults($urlGenerator->getDefaultParameters()); + $newGenerator->defaults($this->originalUrlGenerator->getDefaultParameters()); - $newGenerator->setSessionResolver(function () { - return $this->app['session'] ?? null; - }); - - $newGenerator->setKeyResolver(function () { - return $this->app->make('config')->get('app.key'); - }); - - return $newGenerator; + $newGenerator->setSessionResolver(function () { + return $this->app['session'] ?? null; }); + + $newGenerator->setKeyResolver(function () { + return $this->app->make('config')->get('app.key'); + }); + + $this->app->extend('url', fn () => $newGenerator); } } diff --git a/tests/Bootstrappers/RootUrlBootstrapperTest.php b/tests/Bootstrappers/RootUrlBootstrapperTest.php index ee17a802..c25a8bae 100644 --- a/tests/Bootstrappers/RootUrlBootstrapperTest.php +++ b/tests/Bootstrappers/RootUrlBootstrapperTest.php @@ -10,18 +10,23 @@ use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\RootUrlBootstrapper; use Stancl\Tenancy\Middleware\InitializeTenancyBySubdomain; +use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper; +use Stancl\Tenancy\Middleware\InitializeTenancyByPath; +use Stancl\Tenancy\Overrides\TenancyUrlGenerator; beforeEach(function () { Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); RootUrlBootstrapper::$rootUrlOverride = null; + RootUrlBootstrapper::$rootUrlOverrideInTests = true; }); afterEach(function () { RootUrlBootstrapper::$rootUrlOverride = null; + RootUrlBootstrapper::$rootUrlOverrideInTests = false; }); -test('root url bootstrapper overrides the root url when tenancy gets initialized and reverts the url to the central one after tenancy ends', function() { +test('root url bootstrapper overrides the root url when tenancy gets initialized and reverts the url to the central one when ending tenancy', function() { config(['tenancy.bootstrappers' => [RootUrlBootstrapper::class]]); Route::group([