Merge branch 'master' into fix-cache-invalidation

src/Commands/Run.php

@@ -17,7 +17,8 @@ class Run extends Command
     protected $description = 'Run a command for tenant(s)';
 
     protected $signature = 'tenants:run {commandname : The artisan command.}
-                            {--tenants=* : The tenant(s) to run the command for. Default: all}';
+                            {--tenants=* : The tenant(s) to run the command for. Default: all}
+                            {--skip-tenants=* : The tenant(s) to skip}';
 
     public function handle(): int
     {

src/Concerns/HasTenantOptions.php

@@ -10,15 +10,16 @@ use Stancl\Tenancy\Database\Concerns\PendingScope;
 use Symfony\Component\Console\Input\InputOption;
 
 /**
- * Adds 'tenants' and 'with-pending' options.
+ * Adds 'tenants', 'skip-tenants', and 'with-pending' options.
  */
 trait HasTenantOptions
 {
     protected function getOptions()
     {
         return array_merge([
-           new InputOption('tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null),
+            new InputOption('tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null),
-           new InputOption('with-pending', null, InputOption::VALUE_OPTIONAL, 'Include pending tenants in query if true/1, exclude if false/0. Defaults to the tenancy.pending.include_in_queries config value.'),
+            new InputOption('skip-tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to skip when running this command', null),
+            new InputOption('with-pending', null, InputOption::VALUE_OPTIONAL, 'Include pending tenants in query if true/1, exclude if false/0. Defaults to the tenancy.pending.include_in_queries config value.'),
         ], parent::getOptions());
     }
 
@@ -42,6 +43,9 @@ trait HasTenantOptions
             ->when($this->option('tenants'), function ($query) {
                 $query->whereIn(tenancy()->model()->getTenantKeyName(), $this->option('tenants'));
             })
+            ->when($this->option('skip-tenants'), function ($query) {
+                $query->whereNotIn(tenancy()->model()->getTenantKeyName(), $this->option('skip-tenants'));
+            })
             ->when(tenancy()->model()::hasGlobalScope(PendingScope::class), function ($query) {
                 $includePending = $this->input->hasParameterOption('--with-pending')
                     ? filter_var($this->option('with-pending') ?? true, FILTER_VALIDATE_BOOLEAN)

src/Database/Concerns/HasPending.php

@@ -100,27 +100,51 @@ trait HasPending
      */
     public static function pullPendingFromPool(bool $firstOrCreate = false, array $attributes = []): ?Tenant
     {
-        $tenant = DB::transaction(function () use ($attributes): ?Tenant {
+        // Attempt pulling a pending tenant.
-            /** @var (Model&Tenant)|null $tenant */
+        // The loop handles the case where a single tenant is being pulled by multiple processes at the same time.
-            $tenant = static::onlyPending()->first();
+        // If a tenant was pulled by a concurrent process, try pulling the next one in the pool.
+        while (true) {
+            /** @var (Model&Tenant)|null $pullCandidate */
+            $pullCandidate = static::onlyPending()->first();
 
-            if ($tenant !== null) {
+            if ($pullCandidate === null) {
-                event(new PullingPendingTenant($tenant));
+                return $firstOrCreate ? static::create($attributes) : null;
-                $tenant->update(array_merge($attributes, [
-                    'pending_since' => null,
-                ]));
             }
 
+            // Fired before the claim, so it can fire once per attempt, including for a candidate
+            // that ends up being claimed by a different process (in which case the loop retries).
+            // PendingTenantPulled (below) fires exactly once, for the actually pulled tenant.
+            event(new PullingPendingTenant($pullCandidate));
+
+            $tenant = DB::transaction(function () use ($pullCandidate, $attributes): ?Tenant {
+                $tenantWasPulled = static::onlyPending()
+                    ->whereKey($pullCandidate->getKey())
+                    ->update([$pullCandidate->getColumnForQuery('pending_since') => null]) > 0;
+
+                if (! $tenantWasPulled) {
+                    return null;
+                }
+
+                // The tenant's pending_since was just cleared, and a PullingPendingTenant listener
+                // may have made changes to the tenant, so re-fetch it to make sure it's up to date.
+                /** @var Model&Tenant $pulledTenant */
+                $pulledTenant = static::findOrFail($pullCandidate->getKey());
+
+                if (! empty($attributes)) {
+                    $pulledTenant->update($attributes);
+                }
+
+                return $pulledTenant;
+            });
+
+            if ($tenant === null) {
+                // If another pull claimed this tenant first, try claiming the next one
+                continue;
+            }
+
+            event(new PendingTenantPulled($tenant));
+
             return $tenant;
-        });
-
-        if ($tenant === null) {
-            return $firstOrCreate ? static::create($attributes) : null;
         }
-
-        // Only triggered if a tenant that was pulled from the pool is returned
-        event(new PendingTenantPulled($tenant));
-
-        return $tenant;
     }
 }

tests/CommandsTest.php

@@ -515,3 +515,51 @@ test('migrate fresh command only deletes tenant databases if drop_tenant_databas
         expect($tenantHasDatabase($tenant))->toBe($shouldHaveDBAfterMigrateFresh);
     }
 })->with([true, false]);
+
+test('migrate commands can skip specified tenants', function (string $command) {
+    $tenant1 = Tenant::create();
+    $tenant2 = Tenant::create();
+    $tenant3 = Tenant::create();
+
+    pest()->artisan("{$command} --skip-tenants={$tenant1->getTenantKey()} --skip-tenants={$tenant2->getTenantKey()}");
+
+    tenancy()->initialize($tenant1);
+
+    expect(Schema::hasTable('users'))->toBeFalse();
+
+    tenancy()->initialize($tenant2);
+
+    expect(Schema::hasTable('users'))->toBeFalse();
+
+    tenancy()->initialize($tenant3);
+
+    expect(Schema::hasTable('users'))->toBeTrue();
+})->with([
+    'tenants:migrate',
+    'tenants:migrate-fresh',
+]);
+
+test('run command can skip specified tenants', function () {
+    $tenant1 = Tenant::create()->getTenantKey();
+    $tenant2 = Tenant::create()->getTenantKey();
+    $tenant3 = Tenant::create()->getTenantKey();
+
+    pest()->artisan("tenants:run --skip-tenants=$tenant1 --skip-tenants=$tenant2 'bar foo foo@bar foobar arg --option=option'")
+        ->doesntExpectOutputToContain("Tenant: $tenant1")
+        ->doesntExpectOutputToContain("Tenant: $tenant2")
+        ->expectsOutputToContain("Tenant: $tenant3")
+        ->assertExitCode(0);
+});
+
+test('tenants and skip-tenants options can be used together', function () {
+    $tenant1 = Tenant::create()->getTenantKey();
+    $tenant2 = Tenant::create()->getTenantKey();
+    $tenant3 = Tenant::create()->getTenantKey();
+
+    // Scope to tenant1+tenant2, then skip tenant2 — only tenant1 should run
+    pest()->artisan("tenants:run --tenants=$tenant1 --tenants=$tenant2 --skip-tenants=$tenant2 'bar foo foo@bar foobar arg --option=option'")
+        ->expectsOutputToContain("Tenant: $tenant1")
+        ->doesntExpectOutputToContain("Tenant: $tenant2")
+        ->doesntExpectOutputToContain("Tenant: $tenant3")
+        ->assertExitCode(0);
+});

tests/PendingTenantsTest.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 use Illuminate\Database\QueryException;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\Str;
@@ -126,6 +127,52 @@ test('a new tenant gets created while pulling a pending tenant if the pending po
     expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants
 });
 
+test('pulling a pending tenant retries when the tenant is claimed concurrently', function () {
+    Tenant::createPending();
+    Tenant::createPending();
+
+    $stolenId = null;
+
+    Event::listen(PullingPendingTenant::class, function (PullingPendingTenant $event) use (&$stolenId) {
+        if ($stolenId !== null) {
+            return;
+        }
+
+        $stolenId = $event->tenant->id;
+
+        // Steal the tenant like a concurrent process would
+        Tenant::onlyPending()
+            ->whereKey($event->tenant->id)
+            ->update([$event->tenant->getColumnForQuery('pending_since') => null]);
+    });
+
+    $pulled = Tenant::pullPendingFromPool();
+
+    expect($pulled)->not()->toBeNull();
+    expect($pulled->id)->not()->toBe($stolenId); // Stolen tenant was skipped, the next one was claimed by the pull
+    expect(Tenant::onlyPending()->count())->toBe(0); // Both tenants claimed
+});
+
+test('the pull is rolled back and the tenant stays in the pool if setting attributes fails', function () {
+    // Pulling a tenant and setting its attributes happen in one transaction,
+    // so if setting the attributes fails, the whole pull rolls back and the tenant stays in the pool.
+    Schema::table('tenants', function (Blueprint $table) {
+        $table->string('slug')->nullable()->unique();
+    });
+
+    Tenant::$extraCustomColumns = ['slug'];
+
+    Tenant::create(['slug' => 'taken']);
+    Tenant::createPending();
+
+    // During the pull, set slug to 'taken', which is already used by another tenant to make the attribute update throw
+    expect(fn () => Tenant::pullPendingFromPool(false, ['slug' => 'taken']))
+        ->toThrow(QueryException::class);
+
+    // The pull rolled back, so the tenant is still pending
+    expect(Tenant::onlyPending()->count())->toBe(1);
+});
+
 test('withoutPending chained with where clauses returns correct results', function () {
     $tenant = Tenant::create();
     $pendingTenant = Tenant::createPending();