From ff95c92134e46cd3de4320e90a6501a3444be49e Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 1/4] 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 14d40b3a28875dec09deeff185a7a66e455effab Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 2/4] Add DeleteAllTenantMappings listener --- assets/TenancyServiceProvider.stub.php | 2 + .../Listeners/DeleteAllTenantMappings.php | 48 ++++++++++++++ tests/ResourceSyncingTest.php | 63 +++++++++++++++---- 3 files changed, 102 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..6ccf51be --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,48 @@ +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 + * ]; + */ + public static array $pivotTables = ['tenant_resources' => 'tenant_id']; + + public function handle(TenantDeleted $event): void + { + foreach (static::$pivotTables as $table => $tenantKeyColumn) { + DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index d41ed7ee..a4736ce7 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 + DeleteAllTenantMappings::$pivotTables = ['nonexistent_pivot' => 'non_existent_column']; + + expect(fn() => $tenant2->delete())->toThrow(QueryException::class, "Table 'main.nonexistent_pivot' doesn't exist"); +}); + 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'); }); } From cd08becf3df4a11d8bcc8d45670f2bf8d99d0f71 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 3/4] Syncing: 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 a75528e228afe6cd75f50d73794e31f1b49a9700 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 4/4] Syncing: Add DeleteAllTenantMappings listener --- assets/TenancyServiceProvider.stub.php | 2 + .../Listeners/DeleteAllTenantMappings.php | 48 ++++++++++++++ tests/ResourceSyncingTest.php | 63 +++++++++++++++---- 3 files changed, 102 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..6ccf51be --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,48 @@ +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 + * ]; + */ + public static array $pivotTables = ['tenant_resources' => 'tenant_id']; + + public function handle(TenantDeleted $event): void + { + foreach (static::$pivotTables as $table => $tenantKeyColumn) { + DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index d41ed7ee..a4736ce7 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 + DeleteAllTenantMappings::$pivotTables = ['nonexistent_pivot' => 'non_existent_column']; + + expect(fn() => $tenant2->delete())->toThrow(QueryException::class, "Table 'main.nonexistent_pivot' doesn't exist"); +}); + 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'); }); }