[4.x] Only revert initialized bootstrappers (#1385)

* Only revert initialized bootstrappers (Tenancy::initializedBootstrappers)

* Fix use of @property across the codebase

src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php

@@ -46,7 +46,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
      * tenant instances passed to $closeInMemoryConnectionUsing closures,
      * if you're setting that property as well.
      *
-     * @property Closure(PDO, string)|null
+     * @var Closure(PDO, string)|null
      */
     public static Closure|null $persistInMemoryConnectionUsing = null;
 
@@ -59,7 +59,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
      * NOTE: The parameter provided to the closure is the Tenant
      * instance, not a PDO connection.
      *
-     * @property Closure(Tenant)|null
+     * @var Closure(Tenant)|null
      */
     public static Closure|null $closeInMemoryConnectionUsing = null;
 

src/Listeners/BootstrapTenancy.php

@@ -20,6 +20,10 @@ class BootstrapTenancy
             $tenant = $event->tenancy->tenant;
 
             $bootstrapper->bootstrap($tenant);
+
+            if (! in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) {
+                $event->tenancy->initializedBootstrappers[] = $bootstrapper::class;
+            }
         }
 
         event(new TenancyBootstrapped($event->tenancy));

src/Listeners/RevertToCentralContext.php

@@ -15,7 +15,9 @@ class RevertToCentralContext
         event(new RevertingToCentralContext($event->tenancy));
 
         foreach (array_reverse($event->tenancy->getBootstrappers()) as $bootstrapper) {
-            $bootstrapper->revert();
+            if (in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) {
+                $bootstrapper->revert();
+            }
         }
 
         event(new RevertedToCentralContext($event->tenancy));

src/Tenancy.php

@@ -35,6 +35,20 @@ class Tenancy
      */
     public static array $findWith = [];
 
+    /**
+     * A 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
+     * bootstrappers that haven't been properly initialized
+     * (bootstrapped for the first time) previously.
+     *
+     * @internal
+     *
+     * @var list<class-string<TenancyBootstrapper>>
+     */
+    public array $initializedBootstrappers = [];
+
     /** Initialize tenancy for the passed tenant. */
     public function initialize(Tenant|int|string $tenant): void
     {
@@ -192,7 +206,6 @@ class Tenancy
 
     /**
      * Run a callback for multiple tenants.
-     * More performant than running $tenant->run() one by one.
      *
      * @param array<Tenant>|array<string|int>|\Traversable|string|int|null $tenants
      */

tests/InitializedBootstrappersTest.php

@@ -0,0 +1,87 @@
+<?php
+
+use Illuminate\Support\Facades\Event;
+use Stancl\Tenancy\Contracts\TenancyBootstrapper;
+use Stancl\Tenancy\Contracts\Tenant;
+use Stancl\Tenancy\Events\TenancyEnded;
+use Stancl\Tenancy\Events\TenancyInitialized;
+use Stancl\Tenancy\Listeners\BootstrapTenancy;
+use Stancl\Tenancy\Listeners\RevertToCentralContext;
+use Stancl\Tenancy\Tests\Etc\Tenant as TenantModel;
+
+test('only bootstrappers that have been initialized are reverted', function () {
+    config(['tenancy.bootstrappers' => [
+        Initialized_DummyBootstrapperFoo::class,
+        Initialized_DummyBootstrapperBar::class,
+        Initialized_DummyBootstrapperBaz::class,
+    ]]);
+
+    Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
+    Event::listen(TenancyEnded::class, RevertToCentralContext::class);
+
+    // Only needs to be done in tests
+    app()->singleton(Initialized_DummyBootstrapperFoo::class);
+    app()->singleton(Initialized_DummyBootstrapperBar::class);
+    app()->singleton(Initialized_DummyBootstrapperBaz::class);
+
+    $tenant = TenantModel::create();
+
+    try {
+        $tenant->run(fn() => null);
+        // Should throw an exception
+        expect(true)->toBe(false);
+    } catch (Exception $e) {
+        // NOT 'baz fail in revert' as was the behavior before
+        // the commit that added this test
+        expect($e->getMessage())->toBe('bar fail in bootstrap');
+    }
+
+    expect(tenancy()->initializedBootstrappers)->toBe([
+        Initialized_DummyBootstrapperFoo::class,
+    ]);
+});
+
+class Initialized_DummyBootstrapperFoo implements TenancyBootstrapper
+{
+    public string $bootstrapped = 'uninitialized';
+
+    public function bootstrap(Tenant $tenant): void
+    {
+        $this->bootstrapped = 'bootstrapped';
+    }
+
+    public function revert(): void
+    {
+        $this->bootstrapped = 'reverted';
+    }
+}
+
+class Initialized_DummyBootstrapperBar implements TenancyBootstrapper
+{
+    public string $bootstrapped = 'uninitialized';
+
+    public function bootstrap(Tenant $tenant): void
+    {
+        throw new Exception('bar fail in bootstrap');
+    }
+
+    public function revert(): void
+    {
+        $this->bootstrapped = 'reverted';
+    }
+}
+
+class Initialized_DummyBootstrapperBaz implements TenancyBootstrapper
+{
+    public string $bootstrapped = 'uninitialized';
+
+    public function bootstrap(Tenant $tenant): void
+    {
+        $this->bootstrapped = 'bootstrapped';
+    }
+
+    public function revert(): void
+    {
+        throw new Exception('baz fail in revert');
+    }
+}