From 12eb0ad0ccad0bac728ba962466351a42fb0d6f5 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 01/21] Add SyncedResourceDeleted event and DeleteResourceMapping listener, move pivot record deletion to that listener, improve tests (WIP) --- assets/TenancyServiceProvider.stub.php | 3 ++ .../Events/SyncedResourceDeleted.php | 18 +++++++ .../Listeners/DeleteResourceMapping.php | 52 +++++++++++++++++++ .../Listeners/DeleteResourcesInTenants.php | 6 --- src/ResourceSyncing/ResourceSyncing.php | 7 ++- src/ResourceSyncing/SyncMaster.php | 2 - src/ResourceSyncing/Syncable.php | 2 + tests/Etc/ResourceSyncing/CentralUser.php | 1 + ...05_11_000002_create_tenant_users_table.php | 3 -- tests/ResourceSyncingTest.php | 43 ++++++++++++++- 10 files changed, 123 insertions(+), 14 deletions(-) create mode 100644 src/ResourceSyncing/Events/SyncedResourceDeleted.php create mode 100644 src/ResourceSyncing/Listeners/DeleteResourceMapping.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index e0b69e6e..9d667d2c 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -129,6 +129,9 @@ class TenancyServiceProvider extends ServiceProvider ResourceSyncing\Events\SyncedResourceSaved::class => [ ResourceSyncing\Listeners\UpdateOrCreateSyncedResource::class, ], + ResourceSyncing\Events\SyncedResourceDeleted::class => [ + ResourceSyncing\Listeners\DeleteResourceMapping::class, + ], ResourceSyncing\Events\SyncMasterDeleted::class => [ ResourceSyncing\Listeners\DeleteResourcesInTenants::class, ], diff --git a/src/ResourceSyncing/Events/SyncedResourceDeleted.php b/src/ResourceSyncing/Events/SyncedResourceDeleted.php new file mode 100644 index 00000000..941e1841 --- /dev/null +++ b/src/ResourceSyncing/Events/SyncedResourceDeleted.php @@ -0,0 +1,18 @@ +getCentralResource($event->model); + + if (! $centralResource) { + return; + } + + // Delete pivot records if the central resource doesn't use soft deletes + // or the central resource was deleted using forceDelete() + if ($event->forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { + Pivot::withoutEvents(function() use ($centralResource, $event) { + $centralResource?->tenants()->detach($event->tenant); + }); + } + } + + public function getCentralResource(Syncable&Model $resource): SyncMaster|null + { + if ($resource instanceof SyncMaster) { + return $resource; + } + + $centralResourceClass = $resource->getCentralModelName(); + + /** @var (SyncMaster&Model)|null $centralResource */ + $centralResource = $centralResourceClass::firstWhere( + $resource->getGlobalIdentifierKeyName(), + $resource->getGlobalIdentifierKey() + ); + + return $centralResource; + } +} diff --git a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php index 6876f476..05ae37cc 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php +++ b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php @@ -21,12 +21,6 @@ class DeleteResourcesInTenants extends QueueableListener tenancy()->runForMultiple($centralResource->tenants()->cursor(), function () use ($centralResource, $forceDelete) { $this->deleteSyncedResource($centralResource, $forceDelete); - - // Delete pivot records if the central resource doesn't use soft deletes - // or the central resource was deleted using forceDelete() - if ($forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { - $centralResource->tenants()->detach(tenant()); - } }); } } diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index f0d8cc12..eb028357 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -14,6 +14,7 @@ use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceDetachedFromTenant; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSaved; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterRestored; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; trait ResourceSyncing { @@ -25,8 +26,8 @@ trait ResourceSyncing } }); - static::deleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + static::deleted(function (Syncable&Model $model) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(); } }); @@ -67,6 +68,8 @@ trait ResourceSyncing /** @var SyncMaster&Model $this */ event(new SyncMasterDeleted($this, $forceDelete)); } + + event(new SyncedResourceDeleted($this, tenant(), $forceDelete)); } public function triggerRestoredEvent(): void diff --git a/src/ResourceSyncing/SyncMaster.php b/src/ResourceSyncing/SyncMaster.php index 882aeb54..710bc097 100644 --- a/src/ResourceSyncing/SyncMaster.php +++ b/src/ResourceSyncing/SyncMaster.php @@ -25,7 +25,5 @@ interface SyncMaster extends Syncable public function triggerAttachEvent(TenantWithDatabase&Model $tenant): void; - public function triggerDeleteEvent(bool $forceDelete = false): void; - public function triggerRestoredEvent(): void; } diff --git a/src/ResourceSyncing/Syncable.php b/src/ResourceSyncing/Syncable.php index 3d5288f1..c38b02ea 100644 --- a/src/ResourceSyncing/Syncable.php +++ b/src/ResourceSyncing/Syncable.php @@ -16,6 +16,8 @@ interface Syncable public function triggerSyncEvent(): void; + public function triggerDeleteEvent(bool $forceDelete = false): void; + /** * Get the attributes used for creating the *other* model (i.e. tenant if this is the central one, and central if this is the tenant one). * diff --git a/tests/Etc/ResourceSyncing/CentralUser.php b/tests/Etc/ResourceSyncing/CentralUser.php index 1533bd21..ece09550 100644 --- a/tests/Etc/ResourceSyncing/CentralUser.php +++ b/tests/Etc/ResourceSyncing/CentralUser.php @@ -12,6 +12,7 @@ use Stancl\Tenancy\ResourceSyncing\SyncMaster; class CentralUser extends Model implements SyncMaster { use ResourceSyncing, CentralConnection; + protected $guarded = []; public $timestamps = false; diff --git a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php index 0aafd23c..dcd667a6 100644 --- a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php +++ b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php @@ -16,9 +16,6 @@ class CreateTenantUsersTable extends Migration $table->string('global_user_id'); $table->unique(['tenant_id', 'global_user_id']); - - $table->foreign('tenant_id')->references('id')->on('tenants')->onUpdate('cascade')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onUpdate('cascade')->onDelete('cascade'); }); } diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 3250c37a..d41ed7ee 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -46,6 +46,10 @@ use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSavedInForeignDatabase; use Illuminate\Database\Eloquent\Scope; use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Support\Facades\Schema; +use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { config(['tenancy.bootstrappers' => [ @@ -92,6 +96,7 @@ beforeEach(function () { CentralUser::$creationAttributes = $creationAttributes; Event::listen(SyncedResourceSaved::class, UpdateOrCreateSyncedResource::class); + Event::listen(SyncedResourceDeleted::class, DeleteResourceMapping::class); Event::listen(SyncMasterDeleted::class, DeleteResourcesInTenants::class); Event::listen(SyncMasterRestored::class, RestoreResourcesInTenants::class); Event::listen(CentralResourceAttachedToTenant::class, CreateTenantResource::class); @@ -890,9 +895,13 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function() { +test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { [$tenant] = createTenantsAndRunMigrations(); + if ($dbLevelOnCascadeDelete) { + addFkConstraintsToTenantUsersPivot(); + } + $syncMaster = CentralUser::create([ 'global_id' => 'cascade_user', 'name' => 'Central user', @@ -907,6 +916,30 @@ test('tenant pivot records are deleted along with the tenants to which they belo // Deleting tenant deletes its pivot records expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); +})->with([ + 'db level on cascade delete' => true, + 'event-based on cascade delete' => false, +]); + +test('pivot record is automatically deleted with the tenant resource', function() { + [$tenant] = createTenantsAndRunMigrations(); + + $syncMaster = CentralUser::create([ + 'global_id' => 'cascade_user', + 'name' => 'Central user', + 'email' => 'central@localhost', + 'password' => 'password', + 'role' => 'cascade_user', + ]); + + $syncMaster->tenants()->attach($tenant); + + $tenant->run(function () { + TenantUser::firstWhere('global_id', 'cascade_user')->delete(); + }); + + // Deleting tenant resource deletes its pivot record + expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); test('trashed resources are synced correctly', function () { @@ -1265,6 +1298,14 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); +function addFkConstraintsToTenantUsersPivot(): void +{ + Schema::table('tenant_users', function (Blueprint $table) { + $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); + $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); + }); +} + /** * Create two tenants and run migrations for those tenants. * From 9dd4777a9758f3185552150cad721ce8efb267f8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 3 Nov 2025 16:48:14 +0000 Subject: [PATCH 02/21] Fix code style (php-cs-fixer) --- src/ResourceSyncing/Listeners/DeleteResourceMapping.php | 8 ++++---- .../Listeners/DeleteResourcesInTenants.php | 1 - src/ResourceSyncing/ResourceSyncing.php | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php index 1f777a10..cd836bb5 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php +++ b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php @@ -4,13 +4,13 @@ declare(strict_types=1); namespace Stancl\Tenancy\ResourceSyncing\Listeners; +use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\Pivot; use Illuminate\Database\Eloquent\SoftDeletes; use Stancl\Tenancy\Listeners\QueueableListener; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; -use Stancl\Tenancy\ResourceSyncing\SyncMaster; use Stancl\Tenancy\ResourceSyncing\Syncable; -use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Eloquent\Relations\Pivot; +use Stancl\Tenancy\ResourceSyncing\SyncMaster; class DeleteResourceMapping extends QueueableListener { @@ -27,7 +27,7 @@ class DeleteResourceMapping extends QueueableListener // Delete pivot records if the central resource doesn't use soft deletes // or the central resource was deleted using forceDelete() if ($event->forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { - Pivot::withoutEvents(function() use ($centralResource, $event) { + Pivot::withoutEvents(function () use ($centralResource, $event) { $centralResource?->tenants()->detach($event->tenant); }); } diff --git a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php index 05ae37cc..7b071a27 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php +++ b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\ResourceSyncing\Listeners; -use Illuminate\Database\Eloquent\SoftDeletes; use Stancl\Tenancy\Listeners\QueueableListener; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index eb028357..2b246c4e 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -11,10 +11,10 @@ use Stancl\Tenancy\Contracts\UniqueIdentifierGenerator; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceAttachedToTenant; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceDetachedFromTenant; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSaved; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterRestored; -use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; trait ResourceSyncing { From de6249216a24165cfe4ffc4b2baafc3c5f61e730 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:47:21 +0100 Subject: [PATCH 03/21] Fix DeleteResourceMapping phpstan error, add comments --- .../Listeners/DeleteResourceMapping.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php index cd836bb5..319fd043 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php +++ b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php @@ -12,6 +12,12 @@ use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; use Stancl\Tenancy\ResourceSyncing\Syncable; use Stancl\Tenancy\ResourceSyncing\SyncMaster; +/** + * Deletes pivot records when a synced resource is deleted. + * + * If a SyncMaster (central resource) is deleted, all pivot records for that resource are deleted. + * If a Syncable (tenant resource) is deleted, only delete the pivot record for that tenant. + */ class DeleteResourceMapping extends QueueableListener { public static bool $shouldQueue = false; @@ -28,7 +34,9 @@ class DeleteResourceMapping extends QueueableListener // or the central resource was deleted using forceDelete() if ($event->forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { Pivot::withoutEvents(function () use ($centralResource, $event) { - $centralResource?->tenants()->detach($event->tenant); + // $event->tenant is null when the deleted resource is a SyncMaster - all mappings are deleted in that case + // When $event->tenant is not null (= a Syncable was deleted), only delete the mapping for that tenant + $centralResource->tenants()->detach($event->tenant); }); } } From 41c18cfe148d131be0786781dbb1cea7c2e27eb7 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 04/21] Add and test DeleteAllTenantMappings, use the listener on TenantDeleted in TSP stub by default --- assets/TenancyServiceProvider.stub.php | 1 + .../Listeners/DeleteAllTenantMappings.php | 68 +++++++++++++++++++ tests/ResourceSyncingTest.php | 57 +++++++++++++--- 3 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 9d667d2c..5bd51bca 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -78,6 +78,7 @@ class TenancyServiceProvider extends ServiceProvider Events\TenantDeleted::class => [ JobPipeline::make([ Jobs\DeleteDatabase::class, + ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ])->send(function (Events\TenantDeleted $event) { return $event->tenant; })->shouldBeQueued(false), diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php new file mode 100644 index 00000000..6ff9e6c5 --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,68 @@ +onDelete('cascade') in the constraint definition + * for the pivot records to be deleted automatically. Without ->onDelete('cascade'), + * the constraint will prevent tenant deletion before this listener can clean up the pivot records, + * causing an integrity constraint violation. + * + * With ->onDelete('cascade'), the database will handle the cleanup automatically, + * so there's no need to use this listener (it won't break anything, but it's redundant). + * + * By default, this listener only cleans up the 'tenant_resources' polymorphic pivot table, + * and the records to delete are found by the 'tenant_id' column. + * + * To customize which pivot tables to clean up (or which column has the tenant key), + * set DeleteAllTenantMappings::$pivotTables to an array of table names as the keys, + * and their values should be tenant key column names (e.g. 'tenant_id'). + * + * For example (e.g. in TenancyServiceProvider): + * DeleteAllTenantMappings::$pivotTables = [ + * 'tenant_users' => 'tenant_id', + * ]; + * + * Tables that do not exist will be skipped. + */ +class DeleteAllTenantMappings extends QueueableListener +{ + /** + * Pivot tables to clean up after a tenant is deleted, + * formatted like ['table_name' => 'tenant_key_column']. + * E.g. ['tenant_users' => 'tenant_id']. + * + * If empty, the listener defaults to cleaning only + * the default pivot ('tenant_resources' with 'tenant_id' as the tenant key column). + * + * Set this property, e.g. in your TenancyServiceProvider, + * for this listener to clean up specific pivot tables. + */ + public static array $pivotTables = []; + + public function handle(TenantDeleted $event): void + { + $pivotTables = static::$pivotTables; + + if (! $pivotTables) { + $pivotTables = ['tenant_resources' => 'tenant_id']; + } + + foreach ($pivotTables as $table => $tenantKeyColumn) { + if (Schema::hasTable($table)) { + DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); + } + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index d41ed7ee..6b9b595d 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -48,7 +48,9 @@ use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; use Illuminate\Support\Facades\Schema; use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteAllTenantMappings; use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { @@ -73,6 +75,7 @@ beforeEach(function () { CreateTenantResource::$shouldQueue = false; DeleteResourceInTenant::$shouldQueue = false; UpdateOrCreateSyncedResource::$scopeGetModelQuery = null; + DeleteAllTenantMappings::$pivotTables = []; // Reset global scopes on models (should happen automatically but to make this more explicit) Model::clearBootedModels(); @@ -101,6 +104,7 @@ beforeEach(function () { Event::listen(SyncMasterRestored::class, RestoreResourcesInTenants::class); Event::listen(CentralResourceAttachedToTenant::class, CreateTenantResource::class); Event::listen(CentralResourceDetachedFromTenant::class, DeleteResourceInTenant::class); + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); // Run migrations on central connection pest()->artisan('migrate', [ @@ -895,19 +899,34 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { +test('tenant pivot records are deleted along with the tenants to which they belong to', function (bool $dbLevelOnCascadeDelete, bool $morphPivot) { [$tenant] = createTenantsAndRunMigrations(); - if ($dbLevelOnCascadeDelete) { - addFkConstraintsToTenantUsersPivot(); + if ($morphPivot) { + config(['tenancy.models.tenant' => MorphTenant::class]); + $centralUserModel = BaseCentralUser::class; + + // The default pivot table, no need to configure the listener + $pivotTable = 'tenant_resources'; + } else { + $centralUserModel = CentralUser::class; + + // Custom pivot table + $pivotTable = 'tenant_users'; + + DeleteAllTenantMappings::$pivotTables = [$pivotTable => 'tenant_id']; } - $syncMaster = CentralUser::create([ - 'global_id' => 'cascade_user', + if ($dbLevelOnCascadeDelete) { + addTenantIdConstraintToPivot($pivotTable); + } + + $syncMaster = $centralUserModel::create([ + 'global_id' => 'user', 'name' => 'Central user', 'email' => 'central@localhost', 'password' => 'password', - 'role' => 'cascade_user', + 'role' => 'user', ]); $syncMaster->tenants()->attach($tenant); @@ -915,10 +934,13 @@ test('tenant pivot records are deleted along with the tenants to which they belo $tenant->delete(); // Deleting tenant deletes its pivot records - expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); + expect(DB::select("SELECT * FROM {$pivotTable} WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); })->with([ 'db level on cascade delete' => true, 'event-based on cascade delete' => false, +])->with([ + 'polymorphic pivot' => true, + 'basic pivot' => false, ]); test('pivot record is automatically deleted with the tenant resource', function() { @@ -942,6 +964,22 @@ test('pivot record is automatically deleted with the tenant resource', function( expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); +test('DeleteAllTenantMappings handles incorrect configuration correctly', function() { + [$tenant1, $tenant2] = createTenantsAndRunMigrations(); + + // Existing table, non-existent tenant key column + // The listener should throw an exception + DeleteAllTenantMappings::$pivotTables = ['tenant_users' => 'non_existent_column']; + + // Should throw an exception when tenant is deleted + expect(fn() => $tenant1->delete())->toThrow(QueryException::class, "Unknown column 'non_existent_column' in 'where clause'"); + + // Non-existent table, the listener skips it, no exception to throw + DeleteAllTenantMappings::$pivotTables = ['nonexistent_pivot' => 'non_existent_column']; + + expect(fn() => $tenant2->delete())->not()->toThrow(Exception::class); +}); + test('trashed resources are synced correctly', function () { [$tenant1, $tenant2] = createTenantsAndRunMigrations(); migrateUsersTableForTenants(); @@ -1298,11 +1336,10 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); -function addFkConstraintsToTenantUsersPivot(): void +function addTenantIdConstraintToPivot(string $pivotTable): void { - Schema::table('tenant_users', function (Blueprint $table) { + Schema::table($pivotTable, function (Blueprint $table) { $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); }); } From cb9108302422b71f03ad62a02154fc89f7a2fad0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 4 Nov 2025 15:55:46 +0000 Subject: [PATCH 05/21] Fix code style (php-cs-fixer) --- src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php index 6ff9e6c5..a6c3c932 100644 --- a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -4,10 +4,10 @@ declare(strict_types=1); namespace Stancl\Tenancy\ResourceSyncing\Listeners; -use Stancl\Tenancy\Listeners\QueueableListener; -use Stancl\Tenancy\Events\TenantDeleted; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; +use Stancl\Tenancy\Events\TenantDeleted; +use Stancl\Tenancy\Listeners\QueueableListener; /** * When a tenant is deleted, clean up pivot records related to that tenant. From 9c16b7d53e5be601decfa89e3855181fefe25479 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 5 Nov 2025 09:57:17 +0100 Subject: [PATCH 06/21] Simplify DeleteAllTenantMappings (code and docblocks) --- .../Listeners/DeleteAllTenantMappings.php | 50 ++++++------------- tests/ResourceSyncingTest.php | 2 +- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php index a6c3c932..d9d18ad6 100644 --- a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -10,56 +10,36 @@ use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\Listeners\QueueableListener; /** - * When a tenant is deleted, clean up pivot records related to that tenant. - * Only use this listener for cleaning up tables without tenant foreign key constraints. + * When a tenant is deleted, clean up pivot records related to that tenant + * in the pivot tables specified in the $pivotTables property (see the property for details). * - * If you're using foreign key constraints on the tenant key columns in your pivot tables, - * you still must include ->onDelete('cascade') in the constraint definition - * for the pivot records to be deleted automatically. Without ->onDelete('cascade'), + * Only use this listener for cleaning up tables without tenant foreign key constraints. + * When using foreign key constraints, you'll have to use ->onDelete('cascade') + * on the constraint definition for the cleanup instead of utilizing this listener because * the constraint will prevent tenant deletion before this listener can clean up the pivot records, * causing an integrity constraint violation. - * - * With ->onDelete('cascade'), the database will handle the cleanup automatically, - * so there's no need to use this listener (it won't break anything, but it's redundant). - * - * By default, this listener only cleans up the 'tenant_resources' polymorphic pivot table, - * and the records to delete are found by the 'tenant_id' column. - * - * To customize which pivot tables to clean up (or which column has the tenant key), - * set DeleteAllTenantMappings::$pivotTables to an array of table names as the keys, - * and their values should be tenant key column names (e.g. 'tenant_id'). - * - * For example (e.g. in TenancyServiceProvider): - * DeleteAllTenantMappings::$pivotTables = [ - * 'tenant_users' => 'tenant_id', - * ]; - * - * Tables that do not exist will be skipped. */ class DeleteAllTenantMappings extends QueueableListener { /** * Pivot tables to clean up after a tenant is deleted, * formatted like ['table_name' => 'tenant_key_column']. - * E.g. ['tenant_users' => 'tenant_id']. * - * If empty, the listener defaults to cleaning only - * the default pivot ('tenant_resources' with 'tenant_id' as the tenant key column). + * By default, the listener only cleans up the default pivot + * ('tenant_resources' with 'tenant_id' as the tenant key column). * - * Set this property, e.g. in your TenancyServiceProvider, - * for this listener to clean up specific pivot tables. + * To customize this, set this property, e.g. in TenancyServiceProvider: + * DeleteAllTenantMappings::$pivotTables = [ + * 'tenant_users' => 'tenant_id', + * ]; + * + * Non-existent tables specified in the property will be skipped. */ - public static array $pivotTables = []; + public static array $pivotTables = ['tenant_resources' => 'tenant_id']; public function handle(TenantDeleted $event): void { - $pivotTables = static::$pivotTables; - - if (! $pivotTables) { - $pivotTables = ['tenant_resources' => 'tenant_id']; - } - - foreach ($pivotTables as $table => $tenantKeyColumn) { + foreach (static::$pivotTables as $table => $tenantKeyColumn) { if (Schema::hasTable($table)) { DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); } diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 6b9b595d..ac9e9710 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -75,7 +75,7 @@ beforeEach(function () { CreateTenantResource::$shouldQueue = false; DeleteResourceInTenant::$shouldQueue = false; UpdateOrCreateSyncedResource::$scopeGetModelQuery = null; - DeleteAllTenantMappings::$pivotTables = []; + DeleteAllTenantMappings::$pivotTables = ['tenant_resources' => 'tenant_id']; // Reset global scopes on models (should happen automatically but to make this more explicit) Model::clearBootedModels(); From e78fb46fbe1c718c38ac53ab37c6812c7fa5788c Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 5 Nov 2025 12:45:36 +0100 Subject: [PATCH 07/21] Clarify DeleteAllTenantMappings docblocks --- .../Listeners/DeleteAllTenantMappings.php | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php index d9d18ad6..3a572dd9 100644 --- a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -10,14 +10,15 @@ use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\Listeners\QueueableListener; /** - * When a tenant is deleted, clean up pivot records related to that tenant - * in the pivot tables specified in the $pivotTables property (see the property for details). + * Clean up pivot records related to the deleted tenant. + * The listener only cleans up the pivot tables specified + * in the $pivotTables property (see the property for details), + * and is intended for use with tables that do not have tenant foreign key constraints. * - * Only use this listener for cleaning up tables without tenant foreign key constraints. - * When using foreign key constraints, you'll have to use ->onDelete('cascade') - * on the constraint definition for the cleanup instead of utilizing this listener because - * the constraint will prevent tenant deletion before this listener can clean up the pivot records, - * causing an integrity constraint violation. + * When using foreign key constraints, you'll still have to use ->onDelete('cascade') + * on the constraint (otherwise, deleting a tenant will throw a foreign key constraint violation). + * That way, the cleanup will happen on the database level, and this listener will essentially + * just perform an extra 'where' query. */ class DeleteAllTenantMappings extends QueueableListener { @@ -25,12 +26,16 @@ class DeleteAllTenantMappings extends QueueableListener * Pivot tables to clean up after a tenant is deleted, * formatted like ['table_name' => 'tenant_key_column']. * - * By default, the listener only cleans up the default pivot - * ('tenant_resources' with 'tenant_id' as the tenant key column). + * Since we cannot automatically detect which pivot tables + * you want to clean up, they have to be specified here. + * + * By default, resource syncing uses the tenant_resources table, and the records are associated + * to tenants by the tenant_id column (thus the ['tenant_resources' => 'tenant_id'] default). * * To customize this, set this property, e.g. in TenancyServiceProvider: * DeleteAllTenantMappings::$pivotTables = [ * 'tenant_users' => 'tenant_id', + * // You can also add more pivot tables here * ]; * * Non-existent tables specified in the property will be skipped. From 8deab4041f913099c870fb41af5546f3e1994dc3 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 5 Nov 2025 12:49:12 +0100 Subject: [PATCH 08/21] Use DeleteAllTenantMappings on TenantDeleted only in tests where it matters --- tests/ResourceSyncingTest.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index ac9e9710..ac6b18fd 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -104,7 +104,6 @@ beforeEach(function () { Event::listen(SyncMasterRestored::class, RestoreResourcesInTenants::class); Event::listen(CentralResourceAttachedToTenant::class, CreateTenantResource::class); Event::listen(CentralResourceDetachedFromTenant::class, DeleteResourceInTenant::class); - Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); // Run migrations on central connection pest()->artisan('migrate', [ @@ -913,12 +912,15 @@ test('tenant pivot records are deleted along with the tenants to which they belo // Custom pivot table $pivotTable = 'tenant_users'; - - DeleteAllTenantMappings::$pivotTables = [$pivotTable => 'tenant_id']; } if ($dbLevelOnCascadeDelete) { addTenantIdConstraintToPivot($pivotTable); + } else { + // Event-based cleanup + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + + DeleteAllTenantMappings::$pivotTables = [$pivotTable => 'tenant_id']; } $syncMaster = $centralUserModel::create([ @@ -931,9 +933,9 @@ test('tenant pivot records are deleted along with the tenants to which they belo $syncMaster->tenants()->attach($tenant); + // Pivot records should be deleted along with the tenant $tenant->delete(); - // Deleting tenant deletes its pivot records expect(DB::select("SELECT * FROM {$pivotTable} WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); })->with([ 'db level on cascade delete' => true, @@ -965,10 +967,12 @@ test('pivot record is automatically deleted with the tenant resource', function( }); test('DeleteAllTenantMappings handles incorrect configuration correctly', function() { + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + [$tenant1, $tenant2] = createTenantsAndRunMigrations(); // Existing table, non-existent tenant key column - // The listener should throw an exception + // The listener should throw an 'unknown column' exception DeleteAllTenantMappings::$pivotTables = ['tenant_users' => 'non_existent_column']; // Should throw an exception when tenant is deleted From 65221472c87c209dbb90f28b177c8d2f6f166f15 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 5 Nov 2025 13:25:24 +0100 Subject: [PATCH 09/21] Move DeleteAllTenantMappings listener out of the TenantDeleted JobPipeline in TSP stub --- assets/TenancyServiceProvider.stub.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 5bd51bca..346d7624 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -78,10 +78,11 @@ class TenancyServiceProvider extends ServiceProvider Events\TenantDeleted::class => [ JobPipeline::make([ Jobs\DeleteDatabase::class, - ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ])->send(function (Events\TenantDeleted $event) { return $event->tenant; })->shouldBeQueued(false), + + ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ], Events\TenantMaintenanceModeEnabled::class => [], From 8971c6ecd7cfac7d40f645153777a53063f57dd1 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 5 Nov 2025 14:58:28 +0100 Subject: [PATCH 10/21] Comment out DeleteAllTenantMappings in TSP stub --- assets/TenancyServiceProvider.stub.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 346d7624..c1acb50b 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -82,7 +82,7 @@ class TenancyServiceProvider extends ServiceProvider return $event->tenant; })->shouldBeQueued(false), - ResourceSyncing\Listeners\DeleteAllTenantMappings::class, + // ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ], Events\TenantMaintenanceModeEnabled::class => [], From 947894fa1d40b7f651f55d45bc56746eaacc60d2 Mon Sep 17 00:00:00 2001 From: Hayatunnabi Nabil Date: Sat, 8 Nov 2025 05:52:08 +0600 Subject: [PATCH 11/21] [4.x] Fix dropRLSPolicies() (#1413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `?` parameters are not supported in these statements, so we have to use string interpolation like in other related code. --------- Co-authored-by: Samuel Ć tancl --- src/Concerns/ManagesRLSPolicies.php | 2 +- tests/RLS/PolicyTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Concerns/ManagesRLSPolicies.php b/src/Concerns/ManagesRLSPolicies.php index 6b804fb7..f6329d0e 100644 --- a/src/Concerns/ManagesRLSPolicies.php +++ b/src/Concerns/ManagesRLSPolicies.php @@ -26,7 +26,7 @@ trait ManagesRLSPolicies $policies = static::getRLSPolicies($table); foreach ($policies as $policy) { - DB::statement('DROP POLICY ? ON ?', [$policy, $table]); + DB::statement("DROP POLICY {$policy} ON {$table}"); } return count($policies); diff --git a/tests/RLS/PolicyTest.php b/tests/RLS/PolicyTest.php index ee9bf5cc..b790343e 100644 --- a/tests/RLS/PolicyTest.php +++ b/tests/RLS/PolicyTest.php @@ -17,6 +17,7 @@ use Stancl\Tenancy\Commands\CreateUserWithRLSPolicies; use Stancl\Tenancy\RLS\PolicyManagers\TableRLSManager; use Stancl\Tenancy\RLS\PolicyManagers\TraitRLSManager; use Stancl\Tenancy\Bootstrappers\PostgresRLSBootstrapper; +use Stancl\Tenancy\Tenancy; use function Stancl\Tenancy\Tests\pest; beforeEach(function () { @@ -189,6 +190,22 @@ test('rls command recreates policies if the force option is passed', function (s TraitRLSManager::class, ]); +test('dropRLSPolicies only drops RLS policies', function () { + DB::statement('CREATE POLICY "comments_dummy_rls_policy" ON comments USING (true)'); + DB::statement('CREATE POLICY "comments_foo_policy" ON comments USING (true)'); // non-RLS policy + + $policyCount = fn () => count(DB::select("SELECT policyname FROM pg_policies WHERE tablename = 'comments'")); + + expect($policyCount())->toBe(2); + + $removed = Tenancy::dropRLSPolicies('comments'); + + expect($removed)->toBe(1); + + // Only the non-RLS policy remains + expect($policyCount())->toBe(1); +}); + test('queries will stop working when the tenant session variable is not set', function(string $manager, bool $forceRls) { CreateUserWithRLSPolicies::$forceRls = $forceRls; From 69bf76842496a9b29b48b04dacae38c2e34f1f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 01:07:53 +0100 Subject: [PATCH 12/21] Cloning: remove route context middleware flags during cloning Previously, if a universal route was cloned without a cloneRoutesWithMiddleware(['universal']) call, i.e. it had both 'clone' and 'universal' flags, with only the former triggering cloning, the 'universal' flag would be included in the middleware of the cloned route. Now, we make sure to remove all context flags -- central, tenant, universal -- in the first step of processing middleware, before adding just 'tenant'. --- src/Actions/CloneRoutesAsTenant.php | 6 +++--- tests/CloneActionTest.php | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index f1cb1450..87afe1d7 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -39,7 +39,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * Routes with names that are already prefixed won't be cloned - but that's just the default behavior. * The cloneUsing() method allows you to completely override the default behavior and customize how the cloned routes will be defined. * - * After cloning, only top-level middleware in $cloneRoutesWithMiddleware will be removed + * After cloning, only top-level middleware in $cloneRoutesWithMiddleware (as well as any route context flags) will be removed * from the new route (so by default, 'clone' will be omitted from the new route's MW). * Middleware groups are preserved as-is, even if they contain cloning middleware. * @@ -258,12 +258,12 @@ class CloneRoutesAsTenant return $newRoute; } - /** Removes top-level cloneRoutesWithMiddleware and adds 'tenant' middleware. */ + /** Removes top-level cloneRoutesWithMiddleware and context flags, adds 'tenant' middleware. */ protected function processMiddlewareForCloning(array $middleware): array { $processedMiddleware = array_filter( $middleware, - fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) + fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) && ! in_array($mw, ['central', 'tenant', 'universal']) ); $processedMiddleware[] = 'tenant'; diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 28a8ccd3..b50a1b2f 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -401,3 +401,24 @@ test('tenant parameter addition can be controlled by setting addTenantParameter' $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); } })->with([true, false]); + +test('existing context flags are removed during cloning', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone', 'central']); + RouteFacade::get('/bar', fn () => true)->name('bar')->middleware(['clone', 'universal']); + + $cloneAction = app(CloneRoutesAsTenant::class); + + // Clone foo route + $cloneAction->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName()) + ->toContain('tenant.foo'); + expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) + ->not()->toContain('central'); + + // Clone bar route + $cloneAction->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName()) + ->toContain('tenant.foo', 'tenant.bar'); + expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) + ->not()->toContain('universal'); +}); From 97c5afd2cfe36e4f1e999ea10fd703d858303dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 18:39:28 +0100 Subject: [PATCH 13/21] Cloning: clarify case where neither paths nor domains differ In such a case, the cloned route will actually *override* the original route, rather than being unused as the original docblock claimed. Also adds a static make() function for convenience. --- src/Actions/CloneRoutesAsTenant.php | 7 ++++++- tests/CloneActionTest.php | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index 87afe1d7..120ab0d0 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -71,7 +71,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * // cloned route can be customized using domain(string|null). By default, the cloned route will not be scoped to a domain, * // unless a domain() call is used. It's important to keep in mind that: * // 1. When addTenantParameter(false) is used, the paths will be the same, thus domains must differ. - * // 2. If the original route (with the same path) has no domain, the cloned route will never be used due to registration order. + * // 2. If the original route has no domain, the cloned route will override the original route as they will directly conflict. * $cloneAction->addTenantParameter(false)->cloneRoutesWithMiddleware(['clone'])->cloneRoute('no-tenant-parameter')->handle(); * ``` * @@ -96,6 +96,11 @@ class CloneRoutesAsTenant protected Router $router, ) {} + public static function make(): static + { + return app(static::class); + } + /** Clone routes. This resets routesToClone() but not other config. */ public function handle(): void { diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index b50a1b2f..74625994 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -422,3 +422,18 @@ test('existing context flags are removed during cloning', function () { expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) ->not()->toContain('universal'); }); + +test('cloning a route without a prefix or differing domains overrides the original route', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone']); + + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('foo'); + + $cloneAction = CloneRoutesAsTenant::make(); + $cloneAction->cloneRoute('foo') + ->addTenantParameter(false) + ->tenantParameterBeforePrefix(false) + ->handle(); + + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('foo'); +}); From 197513dd84285c1ce9abe07c0bc59ab6ccb4d597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 18:39:28 +0100 Subject: [PATCH 14/21] Cloning: addTenantMiddleware() for specifying ID MW for cloned route Previously, tenant identification middleware was typically specified for the cloned route by "inheriting" it from the central route, which necessarily meant that the central route had to also be marked as universal so it could continue working in the central context -- despite presumably not being usable in the tenant context, thus being universal for no proper reason. In such cases, universal routes were used mainly as a mechanism for specifying the tenant identification middleware to use on the cloned tenant route. Given that recent refactors of the cloning feature have made it more customizable and a bit nicer to use "multiple times", i.e. run handle() with a few different configurations of the action, letting the developer specify the used tenant middleware using a method like this only makes sense. The feature also becomes more independently usable and not just a "hack for universal routes with path identification". --- src/Actions/CloneRoutesAsTenant.php | 19 ++++++++++++++++--- tests/CloneActionTest.php | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index 120ab0d0..abe2cbcd 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -30,6 +30,8 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * By providing a callback to shouldClone(), you can change how it's determined if a route should be cloned if you don't want to use middleware flags. * * Cloned routes are prefixed with '/{tenant}', flagged with 'tenant' middleware, and have their names prefixed with 'tenant.'. + * The addition of the 'tenant' middleware can be controlled using addTenantMiddleware(array). You can specify the identification + * middleware to be used on the cloned route using that method -- instead of using the approach that "inherits" it from a universal route. * * The addition of the tenant parameter can be controlled using addTenantParameter(true|false). Note that if you decide to disable * tenant parameter addition, the routes MUST differ in domains. This can be controlled using the domain(string|null) method. The @@ -91,6 +93,7 @@ class CloneRoutesAsTenant protected Closure|null $cloneUsing = null; // The callback should accept Route instance or the route name (string) protected Closure|null $shouldClone = null; protected array $cloneRoutesWithMiddleware = ['clone']; + protected array $addTenantMiddleware = ['tenant']; public function __construct( protected Router $router, @@ -148,6 +151,18 @@ class CloneRoutesAsTenant return $this; } + /** + * The tenant middleware to be added to the cloned route. + * + * If used with early identification, make sure to include 'tenant' in this array. + */ + public function addTenantMiddleware(array $middleware): static + { + $this->addTenantMiddleware = $middleware; + + return $this; + } + /** The domain the cloned route should use. Set to null if it shouldn't be scoped to a domain. */ public function domain(string|null $domain): static { @@ -271,9 +286,7 @@ class CloneRoutesAsTenant fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) && ! in_array($mw, ['central', 'tenant', 'universal']) ); - $processedMiddleware[] = 'tenant'; - - return array_unique($processedMiddleware); + return array_unique(array_merge($processedMiddleware, $this->addTenantMiddleware)); } /** Check if route already has tenant parameter or name prefix. */ diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 74625994..8fc66c56 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -437,3 +437,30 @@ test('cloning a route without a prefix or differing domains overrides the origin expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('foo'); }); + +test('addTenantMiddleware can be used to specify the tenant middleware for the cloned route', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone']); + RouteFacade::get('/bar', fn () => true)->name('bar')->middleware(['clone']); + + $cloneAction = app(CloneRoutesAsTenant::class); + + $cloneAction->cloneRoute('foo')->addTenantMiddleware([InitializeTenancyByPath::class])->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + $cloned = RouteFacade::getRoutes()->getByName('tenant.foo'); + expect($cloned->uri())->toBe('{tenant}/foo'); + expect($cloned->getName())->toBe('tenant.foo'); + expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByPath::class]); + + $cloneAction->cloneRoute('bar') + ->addTenantMiddleware([InitializeTenancyByDomain::class]) + ->domain('foo.localhost') + ->addTenantParameter(false) + ->tenantParameterBeforePrefix(false) + ->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.bar'); + $cloned = RouteFacade::getRoutes()->getByName('tenant.bar'); + expect($cloned->uri())->toBe('bar'); + expect($cloned->getName())->toBe('tenant.bar'); + expect($cloned->getDomain())->toBe('foo.localhost'); + expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByDomain::class]); +}); From 6ef4b91744d8745f90ddde3d511bb0c325530351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sun, 9 Nov 2025 01:27:29 +0100 Subject: [PATCH 15/21] Cloning: improve type annotations, add cloneRoutes() for convenience --- src/Actions/CloneRoutesAsTenant.php | 55 +++++++++++++++++++++++++---- tests/CloneActionTest.php | 11 ++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index abe2cbcd..6e988907 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -86,13 +86,27 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; */ class CloneRoutesAsTenant { + /** @var list */ protected array $routesToClone = []; + protected bool $addTenantParameter = true; protected bool $tenantParameterBeforePrefix = true; protected string|null $domain = null; - protected Closure|null $cloneUsing = null; // The callback should accept Route instance or the route name (string) + + /** + * The callback should accept a Route instance or the route name (string). + * + * @var ?Closure(Route|string): void + */ + protected Closure|null $cloneUsing = null; + + /** @var ?Closure(Route): bool */ protected Closure|null $shouldClone = null; + + /** @var list */ protected array $cloneRoutesWithMiddleware = ['clone']; + + /** @var list */ protected array $addTenantMiddleware = ['tenant']; public function __construct( @@ -110,9 +124,12 @@ class CloneRoutesAsTenant // If no routes were specified using cloneRoute(), get all routes // and for each, determine if it should be cloned if (! $this->routesToClone) { - $this->routesToClone = collect($this->router->getRoutes()->get()) + /** @var list */ + $routesToClone = collect($this->router->getRoutes()->get()) ->filter(fn (Route $route) => $this->shouldBeCloned($route)) ->all(); + + $this->routesToClone = $routesToClone; } foreach ($this->routesToClone as $route) { @@ -126,7 +143,9 @@ class CloneRoutesAsTenant if (is_string($route)) { $this->router->getRoutes()->refreshNameLookups(); - $route = $this->router->getRoutes()->getByName($route); + $routeName = $route; + $route = $this->router->getRoutes()->getByName($routeName); + assert(! is_null($route), "Route [{$routeName}] was meant to be cloned but does not exist."); } $this->createNewRoute($route); @@ -155,6 +174,8 @@ class CloneRoutesAsTenant * The tenant middleware to be added to the cloned route. * * If used with early identification, make sure to include 'tenant' in this array. + * + * @param list $middleware */ public function addTenantMiddleware(array $middleware): static { @@ -171,7 +192,11 @@ class CloneRoutesAsTenant return $this; } - /** Provide a custom callback for cloning routes, instead of the default behavior. */ + /** + * Provide a custom callback for cloning routes, instead of the default behavior. + * + * @param ?Closure(Route|string): void $cloneUsing + */ public function cloneUsing(Closure|null $cloneUsing): static { $this->cloneUsing = $cloneUsing; @@ -179,7 +204,11 @@ class CloneRoutesAsTenant return $this; } - /** Specify which middleware should serve as "flags" telling this action to clone those routes. */ + /** + * Specify which middleware should serve as "flags" telling this action to clone those routes. + * + * @param list $middleware + */ public function cloneRoutesWithMiddleware(array $middleware): static { $this->cloneRoutesWithMiddleware = $middleware; @@ -190,7 +219,9 @@ class CloneRoutesAsTenant /** * Provide a custom callback for determining whether a route should be cloned. * Overrides the default middleware-based detection. - * */ + * + * @param Closure(Route): bool $shouldClone + */ public function shouldClone(Closure|null $shouldClone): static { $this->shouldClone = $shouldClone; @@ -213,6 +244,18 @@ class CloneRoutesAsTenant return $this; } + /** + * Clone individual routes. + * + * @param list $routes + */ + public function cloneRoutes(array $routes): static + { + $this->routesToClone = array_merge($this->routesToClone, $routes); + + return $this; + } + protected function shouldBeCloned(Route $route): bool { // Don't clone routes that already have tenant parameter or prefix diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 8fc66c56..ab9c5e9b 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -464,3 +464,14 @@ test('addTenantMiddleware can be used to specify the tenant middleware for the c expect($cloned->getDomain())->toBe('foo.localhost'); expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByDomain::class]); }); + +test('cloneRoutes can be used to clone multiple routes', function () { + RouteFacade::get('/foo', fn () => true)->name('foo'); + $bar = RouteFacade::get('/bar', fn () => true)->name('bar'); + RouteFacade::get('/baz', fn () => true)->name('baz'); + + CloneRoutesAsTenant::make()->cloneRoutes(['foo', $bar])->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.bar'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('tenant.baz'); +}); From 2aca784c0b723606f55a4aeea6bf4b5f4cd6b0d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 10 Nov 2025 17:31:02 +0100 Subject: [PATCH 16/21] Cloning: remove comments in TSP stub in favor of referencing class docs --- assets/TenancyServiceProvider.stub.php | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index e0b69e6e..46f35515 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -242,24 +242,7 @@ class TenancyServiceProvider extends ServiceProvider /** @var CloneRoutesAsTenant $cloneRoutes */ $cloneRoutes = $this->app->make(CloneRoutesAsTenant::class); - // The cloning action has two modes: - // 1. Clone all routes that have the middleware present in the action's $cloneRoutesWithMiddleware property. - // You can customize the middleware that triggers cloning by using cloneRoutesWithMiddleware() on the action. - // - // By default, the middleware is ['clone'], but using $cloneRoutes->cloneRoutesWithMiddleware(['clone', 'universal'])->handle() - // will clone all routes that have either 'clone' or 'universal' middleware (mentioning 'universal' since that's a common use case). - // - // Also, you can use the shouldClone() method to provide a custom closure that determines if a route should be cloned. - // - // 2. Clone only the routes that were manually added to the action using cloneRoute(). - // - // Regardless of the mode, you can provide a custom closure for defining the cloned route, e.g.: - // $cloneRoutesAction->cloneUsing(function (Route $route) { - // RouteFacade::get('/cloned/' . $route->uri(), fn () => 'cloned route')->name('cloned.' . $route->getName()); - // })->handle(); - // This will make all cloned routes use the custom closure to define the cloned route instead of the default behavior. - // See Stancl\Tenancy\Actions\CloneRoutesAsTenant for more details. - + /** See CloneRoutesAsTenant for usage details. */ $cloneRoutes->handle(); } From 0cd0bc44b1c0bdb41d26b7e902755d2daaa4b795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 11 Nov 2025 02:06:03 +0100 Subject: [PATCH 17/21] config: ignore port in default central_domains value Recent Laravel installations often have http://localhost:8000 as APP_URL, so we make sure to strip any port suffix from the default central domain derived from APP_URL. --- assets/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/config.php b/assets/config.php index f15a843a..2a3a07e2 100644 --- a/assets/config.php +++ b/assets/config.php @@ -64,7 +64,7 @@ return [ * Only relevant if you're using the domain or subdomain identification middleware. */ 'central_domains' => [ - str(env('APP_URL'))->after('://')->before('/')->toString(), + str(env('APP_URL'))->after('://')->before('/')->before(':')->toString(), ], /** From 88247462c5f7b4b2539399f084082382a4145418 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 11 Nov 2025 11:19:10 +0100 Subject: [PATCH 18/21] Simplify redundant SoftDeletes checks for calling triggerDeleteEvent/triggerRestoredEvent --- src/ResourceSyncing/ResourceSyncing.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index 2b246c4e..824d909c 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -43,13 +43,13 @@ trait ResourceSyncing if (in_array(SoftDeletes::class, class_uses_recursive(static::class), true)) { static::forceDeleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(true); } }); static::restoring(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerRestoredEvent(); } }); From 45cf7029af2ed4785ed779005fd710c3e74f9b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 14 Nov 2025 10:58:35 +0100 Subject: [PATCH 19/21] globalUrl: useAssetOrigin() instead of setAssetRoot() This change was prompted by a phpstan failure after a recent update. While making this change, I noticed we don't need the macro anymore as useAssetOrigin() was added to the UrlGenerator earlier this year, simplifying our implementation. --- phpstan.neon | 4 ---- .../FilesystemTenancyBootstrapper.php | 14 +++----------- src/TenancyServiceProvider.php | 10 ++++++---- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 2c6e3d69..bb97e3a0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -46,10 +46,6 @@ parameters: message: '#PHPDoc tag \@param has invalid value \(dynamic#' paths: - src/helpers.php - - - message: '#Illuminate\\Routing\\UrlGenerator#' - paths: - - src/Bootstrappers/FilesystemTenancyBootstrapper.php - '#Method Stancl\\Tenancy\\Tenancy::cachedResolvers\(\) should return array#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$tenancy#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$resolver#' diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 2c2d9ec9..faa02de7 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\Bootstrappers; use Illuminate\Foundation\Application; -use Illuminate\Routing\UrlGenerator; use Illuminate\Session\FileSessionHandler; use Illuminate\Support\Facades\Storage; use Stancl\Tenancy\Contracts\TenancyBootstrapper; @@ -22,13 +21,6 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper ) { $this->originalAssetUrl = $this->app['config']['app.asset_url']; $this->originalStoragePath = $app->storagePath(); - - $this->app['url']->macro('setAssetRoot', function ($root) { - /** @var UrlGenerator $this */ - $this->assetRoot = $root; - - return $this; - }); } public function bootstrap(Tenant $tenant): void @@ -107,16 +99,16 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper if ($suffix === false) { $this->app['config']['app.asset_url'] = $this->originalAssetUrl; - $this->app['url']->setAssetRoot($this->originalAssetUrl); + $this->app['url']->useAssetOrigin($this->originalAssetUrl); return; } if ($this->originalAssetUrl) { $this->app['config']['app.asset_url'] = $this->originalAssetUrl . "/$suffix"; - $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']); + $this->app['url']->useAssetOrigin($this->app['config']['app.asset_url']); } else { - $this->app['url']->setAssetRoot($this->app['url']->route('stancl.tenancy.asset', ['path' => ''])); + $this->app['url']->useAssetOrigin($this->app['url']->route('stancl.tenancy.asset', ['path' => ''])); } } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 9b32f088..afd20fb6 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -6,6 +6,7 @@ namespace Stancl\Tenancy; use Closure; use Illuminate\Cache\CacheManager; +use Illuminate\Contracts\Container\Container; use Illuminate\Database\Console\Migrations\FreshCommand; use Illuminate\Routing\Events\RouteMatched; use Illuminate\Support\Facades\Event; @@ -157,12 +158,13 @@ class TenancyServiceProvider extends ServiceProvider $this->loadRoutesFrom(__DIR__ . '/../assets/routes.php'); } - $this->app->singleton('globalUrl', function ($app) { + $this->app->singleton('globalUrl', function (Container $app) { if ($app->bound(FilesystemTenancyBootstrapper::class)) { - $instance = clone $app['url']; - $instance->setAssetRoot($app[FilesystemTenancyBootstrapper::class]->originalAssetUrl); + /** @var \Illuminate\Routing\UrlGenerator */ + $instance = clone $app->make('url'); + $instance->useAssetOrigin($app->make(FilesystemTenancyBootstrapper::class)->originalAssetUrl); } else { - $instance = $app['url']; + $instance = $app->make('url'); } return $instance; From ff95c92134e46cd3de4320e90a6501a3444be49e Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 20/21] Add SyncedResourceDeleted event and DeleteResourceMapping listener Also move pivot record deletion to that listener and improve tests --- assets/TenancyServiceProvider.stub.php | 3 + .../Events/SyncedResourceDeleted.php | 18 ++++++ .../Listeners/DeleteResourceMapping.php | 60 +++++++++++++++++++ .../Listeners/DeleteResourcesInTenants.php | 7 --- src/ResourceSyncing/ResourceSyncing.php | 11 ++-- src/ResourceSyncing/SyncMaster.php | 2 - src/ResourceSyncing/Syncable.php | 2 + tests/Etc/ResourceSyncing/CentralUser.php | 1 + ...05_11_000002_create_tenant_users_table.php | 3 - tests/ResourceSyncingTest.php | 43 ++++++++++++- 10 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 src/ResourceSyncing/Events/SyncedResourceDeleted.php create mode 100644 src/ResourceSyncing/Listeners/DeleteResourceMapping.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 46f35515..2e7819a5 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -129,6 +129,9 @@ class TenancyServiceProvider extends ServiceProvider ResourceSyncing\Events\SyncedResourceSaved::class => [ ResourceSyncing\Listeners\UpdateOrCreateSyncedResource::class, ], + ResourceSyncing\Events\SyncedResourceDeleted::class => [ + ResourceSyncing\Listeners\DeleteResourceMapping::class, + ], ResourceSyncing\Events\SyncMasterDeleted::class => [ ResourceSyncing\Listeners\DeleteResourcesInTenants::class, ], diff --git a/src/ResourceSyncing/Events/SyncedResourceDeleted.php b/src/ResourceSyncing/Events/SyncedResourceDeleted.php new file mode 100644 index 00000000..941e1841 --- /dev/null +++ b/src/ResourceSyncing/Events/SyncedResourceDeleted.php @@ -0,0 +1,18 @@ +getCentralResource($event->model); + + if (! $centralResource) { + return; + } + + // Delete pivot records if the central resource doesn't use soft deletes + // or the central resource was deleted using forceDelete() + if ($event->forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { + Pivot::withoutEvents(function () use ($centralResource, $event) { + // $event->tenant is null when the deleted resource is a SyncMaster - all mappings are deleted in that case + // When $event->tenant is not null (= a Syncable was deleted), only delete the mapping for that tenant + $centralResource->tenants()->detach($event->tenant); + }); + } + } + + public function getCentralResource(Syncable&Model $resource): SyncMaster|null + { + if ($resource instanceof SyncMaster) { + return $resource; + } + + $centralResourceClass = $resource->getCentralModelName(); + + /** @var (SyncMaster&Model)|null $centralResource */ + $centralResource = $centralResourceClass::firstWhere( + $resource->getGlobalIdentifierKeyName(), + $resource->getGlobalIdentifierKey() + ); + + return $centralResource; + } +} diff --git a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php index 6876f476..7b071a27 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php +++ b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\ResourceSyncing\Listeners; -use Illuminate\Database\Eloquent\SoftDeletes; use Stancl\Tenancy\Listeners\QueueableListener; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; @@ -21,12 +20,6 @@ class DeleteResourcesInTenants extends QueueableListener tenancy()->runForMultiple($centralResource->tenants()->cursor(), function () use ($centralResource, $forceDelete) { $this->deleteSyncedResource($centralResource, $forceDelete); - - // Delete pivot records if the central resource doesn't use soft deletes - // or the central resource was deleted using forceDelete() - if ($forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { - $centralResource->tenants()->detach(tenant()); - } }); } } diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index f0d8cc12..824d909c 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -11,6 +11,7 @@ use Stancl\Tenancy\Contracts\UniqueIdentifierGenerator; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceAttachedToTenant; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceDetachedFromTenant; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSaved; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterRestored; @@ -25,8 +26,8 @@ trait ResourceSyncing } }); - static::deleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + static::deleted(function (Syncable&Model $model) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(); } }); @@ -42,13 +43,13 @@ trait ResourceSyncing if (in_array(SoftDeletes::class, class_uses_recursive(static::class), true)) { static::forceDeleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(true); } }); static::restoring(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerRestoredEvent(); } }); @@ -67,6 +68,8 @@ trait ResourceSyncing /** @var SyncMaster&Model $this */ event(new SyncMasterDeleted($this, $forceDelete)); } + + event(new SyncedResourceDeleted($this, tenant(), $forceDelete)); } public function triggerRestoredEvent(): void diff --git a/src/ResourceSyncing/SyncMaster.php b/src/ResourceSyncing/SyncMaster.php index 882aeb54..710bc097 100644 --- a/src/ResourceSyncing/SyncMaster.php +++ b/src/ResourceSyncing/SyncMaster.php @@ -25,7 +25,5 @@ interface SyncMaster extends Syncable public function triggerAttachEvent(TenantWithDatabase&Model $tenant): void; - public function triggerDeleteEvent(bool $forceDelete = false): void; - public function triggerRestoredEvent(): void; } diff --git a/src/ResourceSyncing/Syncable.php b/src/ResourceSyncing/Syncable.php index 3d5288f1..c38b02ea 100644 --- a/src/ResourceSyncing/Syncable.php +++ b/src/ResourceSyncing/Syncable.php @@ -16,6 +16,8 @@ interface Syncable public function triggerSyncEvent(): void; + public function triggerDeleteEvent(bool $forceDelete = false): void; + /** * Get the attributes used for creating the *other* model (i.e. tenant if this is the central one, and central if this is the tenant one). * diff --git a/tests/Etc/ResourceSyncing/CentralUser.php b/tests/Etc/ResourceSyncing/CentralUser.php index 1533bd21..ece09550 100644 --- a/tests/Etc/ResourceSyncing/CentralUser.php +++ b/tests/Etc/ResourceSyncing/CentralUser.php @@ -12,6 +12,7 @@ use Stancl\Tenancy\ResourceSyncing\SyncMaster; class CentralUser extends Model implements SyncMaster { use ResourceSyncing, CentralConnection; + protected $guarded = []; public $timestamps = false; diff --git a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php index 0aafd23c..dcd667a6 100644 --- a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php +++ b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php @@ -16,9 +16,6 @@ class CreateTenantUsersTable extends Migration $table->string('global_user_id'); $table->unique(['tenant_id', 'global_user_id']); - - $table->foreign('tenant_id')->references('id')->on('tenants')->onUpdate('cascade')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onUpdate('cascade')->onDelete('cascade'); }); } diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 3250c37a..d41ed7ee 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -46,6 +46,10 @@ use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSavedInForeignDatabase; use Illuminate\Database\Eloquent\Scope; use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Support\Facades\Schema; +use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { config(['tenancy.bootstrappers' => [ @@ -92,6 +96,7 @@ beforeEach(function () { CentralUser::$creationAttributes = $creationAttributes; Event::listen(SyncedResourceSaved::class, UpdateOrCreateSyncedResource::class); + Event::listen(SyncedResourceDeleted::class, DeleteResourceMapping::class); Event::listen(SyncMasterDeleted::class, DeleteResourcesInTenants::class); Event::listen(SyncMasterRestored::class, RestoreResourcesInTenants::class); Event::listen(CentralResourceAttachedToTenant::class, CreateTenantResource::class); @@ -890,9 +895,13 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function() { +test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { [$tenant] = createTenantsAndRunMigrations(); + if ($dbLevelOnCascadeDelete) { + addFkConstraintsToTenantUsersPivot(); + } + $syncMaster = CentralUser::create([ 'global_id' => 'cascade_user', 'name' => 'Central user', @@ -907,6 +916,30 @@ test('tenant pivot records are deleted along with the tenants to which they belo // Deleting tenant deletes its pivot records expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); +})->with([ + 'db level on cascade delete' => true, + 'event-based on cascade delete' => false, +]); + +test('pivot record is automatically deleted with the tenant resource', function() { + [$tenant] = createTenantsAndRunMigrations(); + + $syncMaster = CentralUser::create([ + 'global_id' => 'cascade_user', + 'name' => 'Central user', + 'email' => 'central@localhost', + 'password' => 'password', + 'role' => 'cascade_user', + ]); + + $syncMaster->tenants()->attach($tenant); + + $tenant->run(function () { + TenantUser::firstWhere('global_id', 'cascade_user')->delete(); + }); + + // Deleting tenant resource deletes its pivot record + expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); test('trashed resources are synced correctly', function () { @@ -1265,6 +1298,14 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); +function addFkConstraintsToTenantUsersPivot(): void +{ + Schema::table('tenant_users', function (Blueprint $table) { + $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); + $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); + }); +} + /** * Create two tenants and run migrations for those tenants. * From 53c7d4988c94e30aca821039a9e2c1a9ae876ad9 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 21/21] Add DeleteAllTenantMappings listener --- assets/TenancyServiceProvider.stub.php | 2 + .../Listeners/DeleteAllTenantMappings.php | 53 ++++++++++++++++ tests/ResourceSyncingTest.php | 63 +++++++++++++++---- 3 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 2e7819a5..a1e681d7 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -81,6 +81,8 @@ class TenancyServiceProvider extends ServiceProvider ])->send(function (Events\TenantDeleted $event) { return $event->tenant; })->shouldBeQueued(false), + + // ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ], Events\TenantMaintenanceModeEnabled::class => [], diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php new file mode 100644 index 00000000..3a572dd9 --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,53 @@ +onDelete('cascade') + * on the constraint (otherwise, deleting a tenant will throw a foreign key constraint violation). + * That way, the cleanup will happen on the database level, and this listener will essentially + * just perform an extra 'where' query. + */ +class DeleteAllTenantMappings extends QueueableListener +{ + /** + * Pivot tables to clean up after a tenant is deleted, + * formatted like ['table_name' => 'tenant_key_column']. + * + * Since we cannot automatically detect which pivot tables + * you want to clean up, they have to be specified here. + * + * By default, resource syncing uses the tenant_resources table, and the records are associated + * to tenants by the tenant_id column (thus the ['tenant_resources' => 'tenant_id'] default). + * + * To customize this, set this property, e.g. in TenancyServiceProvider: + * DeleteAllTenantMappings::$pivotTables = [ + * 'tenant_users' => 'tenant_id', + * // You can also add more pivot tables here + * ]; + * + * Non-existent tables specified in the property will be skipped. + */ + public static array $pivotTables = ['tenant_resources' => 'tenant_id']; + + public function handle(TenantDeleted $event): void + { + foreach (static::$pivotTables as $table => $tenantKeyColumn) { + if (Schema::hasTable($table)) { + DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); + } + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index d41ed7ee..ac6b18fd 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -48,7 +48,9 @@ use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; use Illuminate\Support\Facades\Schema; use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteAllTenantMappings; use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { @@ -73,6 +75,7 @@ beforeEach(function () { CreateTenantResource::$shouldQueue = false; DeleteResourceInTenant::$shouldQueue = false; UpdateOrCreateSyncedResource::$scopeGetModelQuery = null; + DeleteAllTenantMappings::$pivotTables = ['tenant_resources' => 'tenant_id']; // Reset global scopes on models (should happen automatically but to make this more explicit) Model::clearBootedModels(); @@ -895,30 +898,51 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { +test('tenant pivot records are deleted along with the tenants to which they belong to', function (bool $dbLevelOnCascadeDelete, bool $morphPivot) { [$tenant] = createTenantsAndRunMigrations(); - if ($dbLevelOnCascadeDelete) { - addFkConstraintsToTenantUsersPivot(); + if ($morphPivot) { + config(['tenancy.models.tenant' => MorphTenant::class]); + $centralUserModel = BaseCentralUser::class; + + // The default pivot table, no need to configure the listener + $pivotTable = 'tenant_resources'; + } else { + $centralUserModel = CentralUser::class; + + // Custom pivot table + $pivotTable = 'tenant_users'; } - $syncMaster = CentralUser::create([ - 'global_id' => 'cascade_user', + if ($dbLevelOnCascadeDelete) { + addTenantIdConstraintToPivot($pivotTable); + } else { + // Event-based cleanup + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + + DeleteAllTenantMappings::$pivotTables = [$pivotTable => 'tenant_id']; + } + + $syncMaster = $centralUserModel::create([ + 'global_id' => 'user', 'name' => 'Central user', 'email' => 'central@localhost', 'password' => 'password', - 'role' => 'cascade_user', + 'role' => 'user', ]); $syncMaster->tenants()->attach($tenant); + // Pivot records should be deleted along with the tenant $tenant->delete(); - // Deleting tenant deletes its pivot records - expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); + expect(DB::select("SELECT * FROM {$pivotTable} WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); })->with([ 'db level on cascade delete' => true, 'event-based on cascade delete' => false, +])->with([ + 'polymorphic pivot' => true, + 'basic pivot' => false, ]); test('pivot record is automatically deleted with the tenant resource', function() { @@ -942,6 +966,24 @@ test('pivot record is automatically deleted with the tenant resource', function( expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); +test('DeleteAllTenantMappings handles incorrect configuration correctly', function() { + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + + [$tenant1, $tenant2] = createTenantsAndRunMigrations(); + + // Existing table, non-existent tenant key column + // The listener should throw an 'unknown column' exception + DeleteAllTenantMappings::$pivotTables = ['tenant_users' => 'non_existent_column']; + + // Should throw an exception when tenant is deleted + expect(fn() => $tenant1->delete())->toThrow(QueryException::class, "Unknown column 'non_existent_column' in 'where clause'"); + + // Non-existent table, the listener skips it, no exception to throw + DeleteAllTenantMappings::$pivotTables = ['nonexistent_pivot' => 'non_existent_column']; + + expect(fn() => $tenant2->delete())->not()->toThrow(Exception::class); +}); + test('trashed resources are synced correctly', function () { [$tenant1, $tenant2] = createTenantsAndRunMigrations(); migrateUsersTableForTenants(); @@ -1298,11 +1340,10 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); -function addFkConstraintsToTenantUsersPivot(): void +function addTenantIdConstraintToPivot(string $pivotTable): void { - Schema::table('tenant_users', function (Blueprint $table) { + Schema::table($pivotTable, function (Blueprint $table) { $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); }); }