diff --git a/src/Contracts/Feature.php b/src/Contracts/Feature.php index 74289981..25363cf5 100644 --- a/src/Contracts/Feature.php +++ b/src/Contracts/Feature.php @@ -4,10 +4,8 @@ declare(strict_types=1); namespace Stancl\Tenancy\Contracts; -use Stancl\Tenancy\Tenancy; - /** Additional features, like Telescope tags and tenant redirects. */ interface Feature { - public function bootstrap(Tenancy $tenancy): void; + public function bootstrap(): void; } diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index 64b96fc1..818539c2 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -94,6 +94,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager return false; } + // todo@sqlite we can just respect Laravel config for WAL now if (static::$WAL) { $pdo = new PDO('sqlite:' . $path); $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); @@ -123,6 +124,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager } try { + // todo@sqlite we should also remove any other files for the DB e.g. WAL return unlink($this->getPath($name)); } catch (Throwable) { return false; diff --git a/src/Features/CrossDomainRedirect.php b/src/Features/CrossDomainRedirect.php index a48be6ea..57786274 100644 --- a/src/Features/CrossDomainRedirect.php +++ b/src/Features/CrossDomainRedirect.php @@ -6,11 +6,10 @@ namespace Stancl\Tenancy\Features; use Illuminate\Http\RedirectResponse; use Stancl\Tenancy\Contracts\Feature; -use Stancl\Tenancy\Tenancy; class CrossDomainRedirect implements Feature { - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { RedirectResponse::macro('domain', function (string $domain) { /** @var RedirectResponse $this */ diff --git a/src/Features/DisallowSqliteAttach.php b/src/Features/DisallowSqliteAttach.php index f428a051..16dd47f0 100644 --- a/src/Features/DisallowSqliteAttach.php +++ b/src/Features/DisallowSqliteAttach.php @@ -10,14 +10,13 @@ use Illuminate\Database\SQLiteConnection; use Illuminate\Support\Facades\DB; use PDO; use Stancl\Tenancy\Contracts\Feature; -use Stancl\Tenancy\Tenancy; class DisallowSqliteAttach implements Feature { protected static bool|null $loadExtensionSupported = null; public static string|false|null $extensionPath = null; - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { // Handle any already resolved connections foreach (DB::getConnections() as $connection) { @@ -40,16 +39,20 @@ class DisallowSqliteAttach implements Feature protected function loadExtension(PDO $pdo): bool { if (static::$loadExtensionSupported === null) { + // todo@sqlite refactor to local static static::$loadExtensionSupported = method_exists($pdo, 'loadExtension'); } if (static::$loadExtensionSupported === false) { return false; } + if (static::$extensionPath === false) { return false; } + // todo@sqlite we may want to check for 64 bit + $suffix = match (PHP_OS_FAMILY) { 'Linux' => 'so', 'Windows' => 'dll', diff --git a/src/Features/TelescopeTags.php b/src/Features/TelescopeTags.php index 0a580d23..225049df 100644 --- a/src/Features/TelescopeTags.php +++ b/src/Features/TelescopeTags.php @@ -7,11 +7,10 @@ namespace Stancl\Tenancy\Features; use Laravel\Telescope\IncomingEntry; use Laravel\Telescope\Telescope; use Stancl\Tenancy\Contracts\Feature; -use Stancl\Tenancy\Tenancy; class TelescopeTags implements Feature { - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { if (! class_exists(Telescope::class)) { return; diff --git a/src/Features/TenantConfig.php b/src/Features/TenantConfig.php index 5bc84060..10283da3 100644 --- a/src/Features/TenantConfig.php +++ b/src/Features/TenantConfig.php @@ -12,7 +12,6 @@ use Stancl\Tenancy\Contracts\Feature; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Events\RevertedToCentralContext; use Stancl\Tenancy\Events\TenancyBootstrapped; -use Stancl\Tenancy\Tenancy; class TenantConfig implements Feature { @@ -27,7 +26,7 @@ class TenantConfig implements Feature protected Repository $config, ) {} - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { Event::listen(TenancyBootstrapped::class, function (TenancyBootstrapped $event) { /** @var Tenant $tenant */ diff --git a/src/Features/UserImpersonation.php b/src/Features/UserImpersonation.php index 3db563a4..ac478d07 100644 --- a/src/Features/UserImpersonation.php +++ b/src/Features/UserImpersonation.php @@ -17,9 +17,9 @@ class UserImpersonation implements Feature /** The lifespan of impersonation tokens (in seconds). */ public static int $ttl = 60; - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { - $tenancy->macro('impersonate', function (Tenant $tenant, string $userId, string $redirectUrl, string|null $authGuard = null, bool $remember = false): Model { + Tenancy::macro('impersonate', function (Tenant $tenant, string $userId, string $redirectUrl, string|null $authGuard = null, bool $remember = false): Model { return UserImpersonation::modelClass()::create([ Tenancy::tenantKeyColumn() => $tenant->getTenantKey(), 'user_id' => $userId, diff --git a/src/Features/ViteBundler.php b/src/Features/ViteBundler.php index 987187c7..003984f7 100644 --- a/src/Features/ViteBundler.php +++ b/src/Features/ViteBundler.php @@ -7,19 +7,14 @@ namespace Stancl\Tenancy\Features; use Illuminate\Foundation\Application; use Illuminate\Support\Facades\Vite; use Stancl\Tenancy\Contracts\Feature; -use Stancl\Tenancy\Tenancy; class ViteBundler implements Feature { - /** @var Application */ - protected $app; + public function __construct( + protected Application $app, + ) {} - public function __construct(Application $app) - { - $this->app = $app; - } - - public function bootstrap(Tenancy $tenancy): void + public function bootstrap(): void { Vite::createAssetPathsUsing(function ($path, $secure = null) { return global_asset($path); diff --git a/src/Tenancy.php b/src/Tenancy.php index 86eb6d8d..81e9b1ea 100644 --- a/src/Tenancy.php +++ b/src/Tenancy.php @@ -11,6 +11,7 @@ use Illuminate\Foundation\Bus\PendingDispatch; use Illuminate\Support\Traits\Macroable; use Stancl\Tenancy\Concerns\DealsWithRouteContexts; use Stancl\Tenancy\Concerns\ManagesRLSPolicies; +use Stancl\Tenancy\Contracts\Feature; use Stancl\Tenancy\Contracts\TenancyBootstrapper; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByIdException; @@ -40,7 +41,7 @@ class Tenancy public static array $findWith = []; /** - * A list of bootstrappers that have been initialized. + * List of bootstrappers that have been initialized. * * This is used when reverting tenancy, mainly if an exception * occurs during bootstrapping, to ensure we don't revert @@ -53,6 +54,23 @@ class Tenancy */ public array $initializedBootstrappers = []; + /** + * List of features that have been bootstrapped. + * + * Since features may be bootstrapped multiple times during + * the request cycle (in TSP::boot() and any other times the user calls + * bootstrapFeatures()), we keep track of which features have already + * been bootstrapped so we do not bootstrap them again. Features are + * bootstrapped once and irreversible. + * + * The main point of this is that some features *need* to be bootstrapped + * very early (see #949), so we bootstrap them directly in TSP, but we + * also need the ability to *change* which features are used at runtime + * (mainly tests of this package) and bootstrap features again after making + * changes to config('tenancy.features'). + */ + protected array $bootstrappedFeatures = []; + /** Initialize tenancy for the passed tenant. */ public function initialize(Tenant|int|string $tenant): void { @@ -154,6 +172,27 @@ class Tenancy return in_array($bootstrapper, static::getBootstrappers(), true); } + /** + * Bootstrap configured Tenancy features. + * + * Normally, features are bootstrapped directly in TSP::boot(). However, if + * new features are enabled at runtime (e.g. during tests), this method may + * be called to bootstrap new features. It's idempotent and keeps track of + * which features have already been bootstrapped. Keep in mind that feature + * bootstrapping is irreversible. + */ + public function bootstrapFeatures(): void + { + foreach (config('tenancy.features') ?? [] as $feature) { + /** @var class-string $feature */ + + if (! in_array($feature, $this->bootstrappedFeatures)) { + app($feature)->bootstrap(); + $this->bootstrappedFeatures[] = $feature; + } + } + } + /** * @return Builder */ diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 557306b2..a7f27e63 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -40,15 +40,6 @@ class TenancyServiceProvider extends ServiceProvider // Make sure Tenancy is stateful. $this->app->singleton(Tenancy::class); - // Make sure features are bootstrapped as soon as Tenancy is instantiated. - $this->app->extend(Tenancy::class, function (Tenancy $tenancy) { - foreach ($this->app['config']['tenancy.features'] ?? [] as $feature) { - $this->app[$feature]->bootstrap($tenancy); - } - - return $tenancy; - }); - // Make it possible to inject the current tenant by type hinting the Tenant contract. $this->app->bind(Tenant::class, function ($app) { return $app[Tenancy::class]->tenant; @@ -176,6 +167,11 @@ class TenancyServiceProvider extends ServiceProvider return $instance; }); + // Bootstrap features that are already enabled in the config. + // If more features are enabled at runtime, this method may be called + // multiple times, it keeps track of which features have already been bootstrapped. + $this->app->make(Tenancy::class)->bootstrapFeatures(); + Route::middlewareGroup('clone', []); Route::middlewareGroup('universal', []); Route::middlewareGroup('tenant', []); diff --git a/tests/Features/NoAttachTest.php b/tests/Features/NoAttachTest.php index a1588a24..1ec62f2a 100644 --- a/tests/Features/NoAttachTest.php +++ b/tests/Features/NoAttachTest.php @@ -63,7 +63,7 @@ test('sqlite ATTACH statements can be blocked', function (bool $disallow) { return json_encode(DB::select(request('q2'))); }); - tenancy(); // trigger features: todo@samuel remove after feature refactor + tenancy()->bootstrapFeatures(); if ($disallow) { expect(fn () => pest()->post('/central-sqli', [ diff --git a/tests/Features/RedirectTest.php b/tests/Features/RedirectTest.php index a4102070..a871f529 100644 --- a/tests/Features/RedirectTest.php +++ b/tests/Features/RedirectTest.php @@ -12,6 +12,8 @@ test('tenant redirect macro replaces only the hostname', function () { 'tenancy.features' => [CrossDomainRedirect::class], ]); + tenancy()->bootstrapFeatures(); + Route::get('/foobar', function () { return 'Foo'; })->name('home'); diff --git a/tests/Features/TenantConfigTest.php b/tests/Features/TenantConfigTest.php index b06ddba9..cef5efbf 100644 --- a/tests/Features/TenantConfigTest.php +++ b/tests/Features/TenantConfigTest.php @@ -11,16 +11,21 @@ use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Tests\Etc\Tenant; use function Stancl\Tenancy\Tests\pest; +beforeEach(function () { + config([ + 'tenancy.features' => [TenantConfig::class], + 'tenancy.bootstrappers' => [], + ]); + + tenancy()->bootstrapFeatures(); +}); + afterEach(function () { TenantConfig::$storageToConfigMap = []; }); test('nested tenant values are merged', function () { expect(config('whitelabel.theme'))->toBeNull(); - config([ - 'tenancy.features' => [TenantConfig::class], - 'tenancy.bootstrappers' => [], - ]); Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); @@ -32,6 +37,9 @@ test('nested tenant values are merged', function () { 'whitelabel' => ['config' => ['theme' => 'dark']], ]); + // todo0 one reason why this fails is that tenancy.features was empty + // at register() time + tenancy()->initialize($tenant); expect(config('whitelabel.theme'))->toBe('dark'); tenancy()->end(); @@ -39,10 +47,6 @@ test('nested tenant values are merged', function () { test('config is merged and removed', function () { expect(config('services.paypal'))->toBe(null); - config([ - 'tenancy.features' => [TenantConfig::class], - 'tenancy.bootstrappers' => [], - ]); Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); @@ -68,10 +72,6 @@ test('config is merged and removed', function () { test('the value can be set to multiple config keys', function () { expect(config('services.paypal'))->toBe(null); - config([ - 'tenancy.features' => [TenantConfig::class], - 'tenancy.bootstrappers' => [], - ]); Event::listen(TenancyInitialized::class, BootstrapTenancy::class); Event::listen(TenancyEnded::class, RevertToCentralContext::class); diff --git a/tests/Features/ViteBundlerTest.php b/tests/Features/ViteBundlerTest.php index 3934698f..17ee8e08 100644 --- a/tests/Features/ViteBundlerTest.php +++ b/tests/Features/ViteBundlerTest.php @@ -27,6 +27,7 @@ beforeEach(function () { test('vite bundler ensures vite assets use global_asset when asset_helper_override is enabled', function () { config(['tenancy.features' => [ViteBundler::class]]); + tenancy()->bootstrapFeatures(); withBootstrapping(); diff --git a/tests/TenantUserImpersonationTest.php b/tests/TenantUserImpersonationTest.php index 8c9c4124..48fbe691 100644 --- a/tests/TenantUserImpersonationTest.php +++ b/tests/TenantUserImpersonationTest.php @@ -42,6 +42,8 @@ beforeEach(function () { ], ]); + tenancy()->bootstrapFeatures(); + Event::listen( TenantCreated::class, JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) {