From 652bc987cedb19d345bcb1a948ad02a6ce40e238 Mon Sep 17 00:00:00 2001 From: Jimish Gamit Date: Mon, 8 Jun 2026 03:48:38 +0530 Subject: [PATCH 1/2] Add --skip-tenants option to HasTenantOptions (#1436) Adds a --skip-tenants option to all tenant artisan commands (`tenants:run`, `tenants:migrate`, `tenants:rollback`, `tenants:seed`, `tenants:up`, `tenants:down`). The option is the complement of the existing `--tenants` option instead of specifying which tenants to include, you specify which to exclude. --------- Co-authored-by: Jimish Gamit Co-authored-by: Samuel Stancl Co-authored-by: lukinovec --- src/Commands/Run.php | 3 +- src/Concerns/HasTenantOptions.php | 10 +++++-- tests/CommandsTest.php | 48 +++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/Commands/Run.php b/src/Commands/Run.php index 7dd69e0f..d3435ca2 100644 --- a/src/Commands/Run.php +++ b/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 { diff --git a/src/Concerns/HasTenantOptions.php b/src/Concerns/HasTenantOptions.php index 3933c469..b10d7bd4 100644 --- a/src/Concerns/HasTenantOptions.php +++ b/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('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('tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null), + 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) diff --git a/tests/CommandsTest.php b/tests/CommandsTest.php index a5b3b856..bda3eea9 100644 --- a/tests/CommandsTest.php +++ b/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); +}); From 04da9c896b4e948097cb414781673b720dca47f8 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 26 Jun 2026 04:51:39 +0200 Subject: [PATCH 2/2] [MINOR BC] Fix pending tenant pull race conditions (#1463) > Minor breaking change: clearing pending_since no longer fires Eloquent events, PullingPendingTenant is now fired at a different point in the lifecycle and does not guarantee the tenant will actually be pulled. `pullPendingFromPool` had a race condition when user A attempted to pull a tenant at the same time as user B. Both could end up grabbing the same tenant, and the result was unexpected, e.g. one of them ending up with no pending tenant pulled at all even though there was a pending tenant in the pool. instead of selecting a pending tenant and updating the same model, we now run `update()` conditionally -- it clears `pending_since` _only_ if the tenant is still pending, and we check the affected row count. Only one process can get a row back, the other gets 0 and retries with the next pending candidate in the pool. The loop always terminates since every lost claim means the pool shrank by one. Eventually it's empty and we create a new tenant (or return null). The claim and the attribute update happen in a single transaction now, so if updating `$attributes` fails, the claim rolls back and the tenant stays in the pool. Added a regression test that simulates a concurrent "steal" synchronously via a PullingPendingTenant listener. Fails with the old code, passes with the HasPending changes. Very minor BC: - Clearing `pending_since` no longer fires model updating/updated events (since the update goes through query builder). `PendingTenantPulled` still fires the same as before and is the listener you'd want to use anyway. - `PullingPendingTenant` now fires before the claim (and outside the transaction), so it can fire more than once with concurrent pulls (e.g. when a tenant gets claimed by someone else). `PendingTenantPulled` is still the one that fires exactly once for the actually pulled tenant. --------- Co-authored-by: Samuel Stancl --- src/Database/Concerns/HasPending.php | 58 ++++++++++++++++++++-------- tests/PendingTenantsTest.php | 47 ++++++++++++++++++++++ 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 04fcccc1..e3d8a6fb 100644 --- a/src/Database/Concerns/HasPending.php +++ b/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 { - /** @var (Model&Tenant)|null $tenant */ - $tenant = static::onlyPending()->first(); + // Attempt pulling a pending tenant. + // The loop handles the case where a single tenant is being pulled by multiple processes at the same time. + // 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) { - event(new PullingPendingTenant($tenant)); - $tenant->update(array_merge($attributes, [ - 'pending_since' => null, - ])); + if ($pullCandidate === null) { + return $firstOrCreate ? static::create($attributes) : 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; } } diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index b04f8bc4..c9960728 100644 --- a/tests/PendingTenantsTest.php +++ b/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();