From 12eb0ad0ccad0bac728ba962466351a42fb0d6f5 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 01/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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 88247462c5f7b4b2539399f084082382a4145418 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 11 Nov 2025 11:19:10 +0100 Subject: [PATCH 11/11] 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(); } });