From ee0b45d8006c5f4691abe5c318dfa68ae5e9b7ba Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 1/5] Syncing: SyncedResourceDeleted event and DeleteResourceMapping listener Also move pivot record deletion to that listener and improve tests The 'tenant pivot records are deleted along with the tenants to which they belong to' test is failing in this commit -- the listener for deleting mappings when a *tenant* is deleted is only implemented in the next commit. The only change done here is to re-add FKs (necessary for passing *in this commit* in that specific dataset variant) that were removed from the default test migration as we now have the the DeleteResourceMapping listener that's enabled by default. --- 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 | 45 +++++++++++++- 10 files changed, 135 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..4eaa540c 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,32 @@ 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); + + expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->id]))->toHaveCount(1); + + $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->id]))->toHaveCount(0); }); test('trashed resources are synced correctly', function () { @@ -1265,6 +1300,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 bc06da84eff2d3133a3cbfb239b4862240d01158 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 2/5] 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 3/5] 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 From 2b45a01018b10df09281a07b4b1c8f90a44b8359 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 21 Nov 2025 09:06:52 +0100 Subject: [PATCH 4/5] Move triggerRestoredEvent(), test restoring Syncables Update comments in DeleteResourceMapping to explain the detach() behavior. Move triggerRestoredEvent() from SyncMaster to Syncable interface, test restoring tenant resources. --- .../Listeners/DeleteResourceMapping.php | 4 ++-- src/ResourceSyncing/SyncMaster.php | 2 -- src/ResourceSyncing/Syncable.php | 2 ++ tests/ResourceSyncingTest.php | 11 ++++++++++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php index 319fd043..53754324 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourceMapping.php +++ b/src/ResourceSyncing/Listeners/DeleteResourceMapping.php @@ -34,8 +34,8 @@ 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) { - // $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 + // If detach() is called with null -- if $event->tenant is null -- this means a central resource was deleted and detaches all tenants. + // If detach() is called with a specific tenant, it means the resource was deleted in that tenant, and we only delete that single mapping. $centralResource->tenants()->detach($event->tenant); }); } diff --git a/src/ResourceSyncing/SyncMaster.php b/src/ResourceSyncing/SyncMaster.php index 710bc097..73af0c72 100644 --- a/src/ResourceSyncing/SyncMaster.php +++ b/src/ResourceSyncing/SyncMaster.php @@ -24,6 +24,4 @@ interface SyncMaster extends Syncable public function triggerDetachEvent(TenantWithDatabase&Model $tenant): void; public function triggerAttachEvent(TenantWithDatabase&Model $tenant): void; - - public function triggerRestoredEvent(): void; } diff --git a/src/ResourceSyncing/Syncable.php b/src/ResourceSyncing/Syncable.php index c38b02ea..ec13c485 100644 --- a/src/ResourceSyncing/Syncable.php +++ b/src/ResourceSyncing/Syncable.php @@ -18,6 +18,8 @@ interface Syncable public function triggerDeleteEvent(bool $forceDelete = false): void; + public function triggerRestoredEvent(): 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/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index ebd6ccfd..5687b849 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -1102,7 +1102,16 @@ test('restoring soft deleted resources works', function () { CentralUserWithSoftDeletes::withTrashed()->first()->restore(); tenancy()->runForMultiple([$tenant1, $tenant2], function () { - expect(TenantUserWithSoftDeletes::withTrashed()->first()->trashed())->toBeFalse(); + $tenantResource = TenantUserWithSoftDeletes::withTrashed()->first(); + + expect($tenantResource->trashed())->toBeFalse(); + + $tenantResource->delete(); + + // Restoring a tenant resource works as expected + $tenantResource->restore(); + + expect($tenantResource->trashed())->toBeFalse(); }); }); From ed12b92bd802ee31094190ae392767b420571980 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 21 Nov 2025 15:34:28 +0100 Subject: [PATCH 5/5] Rename and move triggerRestoredEvent Rename triggerRestoredEvent to triggerRestoreEvent, move triggerRestoreEvent from the Syncable interface to SyncMaster, call triggerRestoreEvent only when restoring SyncMaster, delete useless assertions. --- src/ResourceSyncing/ResourceSyncing.php | 6 +++--- src/ResourceSyncing/SyncMaster.php | 2 ++ src/ResourceSyncing/Syncable.php | 2 -- tests/ResourceSyncingTest.php | 11 +---------- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index 0367eb1b..272b7bd7 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -46,8 +46,8 @@ trait ResourceSyncing }); static::restoring(static function (Syncable&Model $model) { - if ($model->shouldSync()) { - $model->triggerRestoredEvent(); + if ($model instanceof SyncMaster && $model->shouldSync()) { + $model->triggerRestoreEvent(); } }); } @@ -69,7 +69,7 @@ trait ResourceSyncing event(new SyncedResourceDeleted($this, tenant(), $forceDelete)); } - public function triggerRestoredEvent(): void + public function triggerRestoreEvent(): void { if ($this instanceof SyncMaster && in_array(SoftDeletes::class, class_uses_recursive($this), true)) { /** @var SyncMaster&Model $this */ diff --git a/src/ResourceSyncing/SyncMaster.php b/src/ResourceSyncing/SyncMaster.php index 73af0c72..290546cb 100644 --- a/src/ResourceSyncing/SyncMaster.php +++ b/src/ResourceSyncing/SyncMaster.php @@ -24,4 +24,6 @@ interface SyncMaster extends Syncable public function triggerDetachEvent(TenantWithDatabase&Model $tenant): void; public function triggerAttachEvent(TenantWithDatabase&Model $tenant): void; + + public function triggerRestoreEvent(): void; } diff --git a/src/ResourceSyncing/Syncable.php b/src/ResourceSyncing/Syncable.php index ec13c485..c38b02ea 100644 --- a/src/ResourceSyncing/Syncable.php +++ b/src/ResourceSyncing/Syncable.php @@ -18,8 +18,6 @@ interface Syncable public function triggerDeleteEvent(bool $forceDelete = false): void; - public function triggerRestoredEvent(): 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/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 5687b849..ebd6ccfd 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -1102,16 +1102,7 @@ test('restoring soft deleted resources works', function () { CentralUserWithSoftDeletes::withTrashed()->first()->restore(); tenancy()->runForMultiple([$tenant1, $tenant2], function () { - $tenantResource = TenantUserWithSoftDeletes::withTrashed()->first(); - - expect($tenantResource->trashed())->toBeFalse(); - - $tenantResource->delete(); - - // Restoring a tenant resource works as expected - $tenantResource->restore(); - - expect($tenantResource->trashed())->toBeFalse(); + expect(TenantUserWithSoftDeletes::withTrashed()->first()->trashed())->toBeFalse(); }); });