Features refactor

Features are now *always* bootstrapped, even if Tenancy is not resolved
from the container.

Previous implementations include
https://github.com/tenancy-for-laravel/v4/pull/19
https://github.com/archtechx/tenancy/pull/1021

Bug originally reported here
https://github.com/archtechx/tenancy/issues/949

This implementation is much simpler, we do not distinguish between
features that should be "always bootstrapped" and features that should
only be bootstrapped after Tenancy is resolved. All features should work
without issues if they're bootstrapped when TSP::boot() is called. We
also add a Tenancy::bootstrapFeatures() method that can be used to
bootstrap any features dynamically added at runtime that weren't
bootstrapped in TSP::boot(). The function keeps track of which features
were already bootstrapped so it doesn't bootstrap them again.

The only potentialy risky thing in this implementation is that we're now
resolving Tenancy in TSP::boot() (previously Tenancy was not being
resolved) but that shouldn't be causing any issues.

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;
 }

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;

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 */

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',

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;

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 */

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,

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);

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> $feature */
+
+            if (! in_array($feature, $this->bootstrappedFeatures)) {
+                app($feature)->bootstrap();
+                $this->bootstrappedFeatures[] = $feature;
+            }
+        }
+    }
+
     /**
      * @return Builder<Tenant&Model>
      */

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', []);

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', [

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');

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);
 

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();
 

tests/TenantUserImpersonationTest.php

@@ -42,6 +42,8 @@ beforeEach(function () {
         ],
     ]);
 
+    tenancy()->bootstrapFeatures();
+
     Event::listen(
         TenantCreated::class,
         JobPipeline::make([CreateDatabase::class])->send(function (TenantCreated $event) {