From bc06da84eff2d3133a3cbfb239b4862240d01158 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 1/2] Syncing: Add DeleteAllTenantMappings listener --- assets/TenancyServiceProvider.stub.php | 2 + .../Listeners/DeleteAllTenantMappings.php | 38 +++++++++++ tests/ResourceSyncingTest.php | 63 +++++++++++++++---- 3 files changed, 92 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..9a206872 --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,38 @@ + 'tenant_key_column'] format. + * + * Since we cannot automatically detect which pivot tables + * are being used, they have to be specified here manually. + * + * The default value follows the polymorphic table used by default. + */ + 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->getKey())->delete(); + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 4eaa540c..ebd6ccfd 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', 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() { @@ -944,6 +968,24 @@ test('pivot record is automatically deleted with the tenant resource', function( expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->id]))->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(); @@ -1300,11 +1342,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 f53fcb279fe247d184212022e170cac09d63b99a Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Tue, 18 Nov 2025 04:02:48 +0100 Subject: [PATCH 2/2] Syncing: move global ID generation logic to an overridable method Also make all resource syncing-related listener closures static. Also correct return type for getGlobalIdentifierKey to string|int. (We intentionally do not support returning null like many other "get x key" methods would since such a case might break resource syncing logic. This is also why we use inline getAttribute() in the creating listener instead of calling the method.) --- src/ResourceSyncing/ResourceSyncing.php | 29 ++++++++++++-------- src/ResourceSyncing/TriggerSyncingEvents.php | 6 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index 824d909c..0367eb1b 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -20,35 +20,32 @@ trait ResourceSyncing { public static function bootResourceSyncing(): void { - static::saved(function (Syncable&Model $model) { + static::saved(static function (Syncable&Model $model) { if ($model->shouldSync() && ($model->wasRecentlyCreated || $model->wasChanged($model->getSyncedAttributeNames()))) { $model->triggerSyncEvent(); } }); - static::deleted(function (Syncable&Model $model) { + static::deleted(static function (Syncable&Model $model) { if ($model->shouldSync()) { $model->triggerDeleteEvent(); } }); - static::creating(function (Syncable&Model $model) { - if (! $model->getAttribute($model->getGlobalIdentifierKeyName()) && app()->bound(UniqueIdentifierGenerator::class)) { - $model->setAttribute( - $model->getGlobalIdentifierKeyName(), - app(UniqueIdentifierGenerator::class)->generate($model) - ); + static::creating(static function (Syncable&Model $model) { + if (! $model->getAttribute($model->getGlobalIdentifierKeyName())) { + $model->generateGlobalIdentifierKey(); } }); if (in_array(SoftDeletes::class, class_uses_recursive(static::class), true)) { - static::forceDeleting(function (Syncable&Model $model) { + static::forceDeleting(static function (Syncable&Model $model) { if ($model->shouldSync()) { $model->triggerDeleteEvent(true); } }); - static::restoring(function (Syncable&Model $model) { + static::restoring(static function (Syncable&Model $model) { if ($model->shouldSync()) { $model->triggerRestoredEvent(); } @@ -119,8 +116,18 @@ trait ResourceSyncing return 'global_id'; } - public function getGlobalIdentifierKey(): string + public function getGlobalIdentifierKey(): string|int { return $this->getAttribute($this->getGlobalIdentifierKeyName()); } + + protected function generateGlobalIdentifierKey(): void + { + if (! app()->bound(UniqueIdentifierGenerator::class)) return; + + $this->setAttribute( + $this->getGlobalIdentifierKeyName(), + app(UniqueIdentifierGenerator::class)->generate($this), + ); + } } diff --git a/src/ResourceSyncing/TriggerSyncingEvents.php b/src/ResourceSyncing/TriggerSyncingEvents.php index eec1b13d..da79df3a 100644 --- a/src/ResourceSyncing/TriggerSyncingEvents.php +++ b/src/ResourceSyncing/TriggerSyncingEvents.php @@ -20,14 +20,14 @@ trait TriggerSyncingEvents { public static function bootTriggerSyncingEvents(): void { - static::saving(function (self $pivot) { + static::saving(static function (self $pivot) { // Try getting the central resource to see if it is available // If it is not available, throw an exception to interrupt the saving process // And prevent creating a pivot record without a central resource $pivot->getCentralResourceAndTenant(); }); - static::saved(function (self $pivot) { + static::saved(static function (self $pivot) { /** * @var static&Pivot $pivot * @var SyncMaster|null $centralResource @@ -40,7 +40,7 @@ trait TriggerSyncingEvents } }); - static::deleting(function (self $pivot) { + static::deleting(static function (self $pivot) { /** * @var static&Pivot $pivot * @var SyncMaster|null $centralResource