Implement changes from review on GH

src/Contracts/UniqueIdentifierGenerator.php

@@ -4,12 +4,10 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Contracts;
 
-use Illuminate\Database\Eloquent\Model;
-
 interface UniqueIdentifierGenerator
 {
     /**
      * Generate a unique identifier.
      */
-    public static function generate(Model $model): string;
+    public static function generate($resource): string;
 }

src/Database/Models/TenantPivot.php

@@ -3,13 +3,18 @@
 namespace Stancl\Tenancy\Database\Models;
 
 use Illuminate\Database\Eloquent\Relations\Pivot;
+use Stancl\Tenancy\Contracts\Syncable;
 
 class TenantPivot extends Pivot
 {
     public static function booted()
     {
         static::saved(function (self $pivot) {
-            $pivot->pivotParent->triggerSyncEvent();
+            $parent = $pivot->pivotParent;
+
+            if ($parent instanceof Syncable) {
+                $parent->triggerSyncEvent();
+            }
         });
     }
 }

src/Events/Listeners/UpdateSyncedResource.php

@@ -5,6 +5,7 @@ namespace Stancl\Tenancy\Events\Listeners;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\Pivot;
 use Stancl\Tenancy\Contracts\SyncMaster;
+use Stancl\Tenancy\Events\SyncedResourceChangedInForeignDatabase;
 use Stancl\Tenancy\Events\SyncedResourceSaved;
 use Stancl\Tenancy\Exceptions\ModelNotSyncMaster;
 
@@ -18,39 +19,16 @@ class UpdateSyncedResource extends QueueableListener
 
         // We update the central record only if the event comes from tenant context.
         if ($event->tenant) {
-            /** @var Model|SyncMaster $centralModel */
+            $tenants = $this->updateResourceInCentralDatabaseAndGetTenants($event, $syncedAttributes);
-            $centralModel = $event->model->getCentralModelName()
-                ::where($event->model->getGlobalIdentifierKeyName(), $event->model->getGlobalIdentifierKey())
-                ->first();
-
-            // We disable events for this call, to avoid triggering this event & listener again.
-            $event->model->getCentralModelName()::withoutEvents(function () use (&$centralModel, $syncedAttributes, $event) {
-                if ($centralModel) {
-                    $centralModel->update($syncedAttributes);
         } else {
-                    // If the resource doesn't exist at all in the central DB,we create
+            $tenants = $this->getTenantsForCentralModel($event->model);
-                    // the record with all attributes, not just the synced ones.
-                    $centralModel = $event->model->getCentralModelName()::create($event->model->getAttributes());
-                }
-            });
-
-            // If the model was just created, the mapping of the tenant to the user likely doesn't exist, so we create it.
-            $mappingExists = $centralModel->tenants->contains(function ($model) use ($event) {
-                return $model->tenant_id === $event->tenant->getTenantKey();
-            });
-
-            if (! $mappingExists) {
-                // Here we should call TenantPivot, but we call general Pivot, so that this works
-                // even if people use their own pivot model that is not based on our TenantPivot
-                Pivot::withoutEvents(function () use ($centralModel, $event) {
-                    $centralModel->tenants()->attach($event->tenant->getTenantKey());
-                });
         }
 
-            $tenants = $centralModel->tenants->except($event->tenant->getTenantKey());
+        $this->updateResourceInTenantDatabases($tenants, $event, $syncedAttributes);
-        } else {
+    }
-            $centralModel = $event->model;
 
+    protected function getTenantsForCentralModel($centralModel)
+    {
         if (! $centralModel instanceof SyncMaster) {
             // If we're trying to use a tenant User model instead of the central User model, for example.
             throw new ModelNotSyncMaster(get_class($centralModel));
@@ -61,10 +39,54 @@ class UpdateSyncedResource extends QueueableListener
         // Since this model is "dirty" (taken by reference from the event), it might have the tenants
         // relationship already loaded and cached. For this reason, we refresh the relationship.
         $centralModel->load('tenants');
-            $tenants = $centralModel->tenants;
+
+        return $centralModel->tenants;
     }
 
-        tenancy()->runForMultiple($tenants, function () use ($event, $syncedAttributes) {
+    protected function updateResourceInCentralDatabaseAndGetTenants($event, $syncedAttributes)
+    {
+        /** @var Model|SyncMaster $centralModel */
+        $centralModel = $event->model->getCentralModelName()
+            ::where($event->model->getGlobalIdentifierKeyName(), $event->model->getGlobalIdentifierKey())
+            ->first();
+
+        // We disable events for this call, to avoid triggering this event & listener again.
+        $event->model->getCentralModelName()::withoutEvents(function () use (&$centralModel, $syncedAttributes, $event) {
+            if ($centralModel) {
+                $centralModel->update($syncedAttributes);
+                event(new SyncedResourceChangedInForeignDatabase($event->model, null));
+            } else {
+                // If the resource doesn't exist at all in the central DB,we create
+                // the record with all attributes, not just the synced ones.
+                $centralModel = $event->model->getCentralModelName()::create($event->model->getAttributes());
+                event(new SyncedResourceChangedInForeignDatabase($event->model, null));
+            }
+        });
+
+        // If the model was just created, the mapping of the tenant to the user likely doesn't exist, so we create it.
+        $currentTenantMapping = function ($model) use ($event) {
+            return ((string) $model->pivot->tenant_id) === ((string) $event->tenant->getTenantKey());
+        };
+
+        $mappingExists = $centralModel->tenants->contains($currentTenantMapping);
+
+        if (!$mappingExists) {
+            // Here we should call TenantPivot, but we call general Pivot, so that this works
+            // even if people use their own pivot model that is not based on our TenantPivot
+            Pivot::withoutEvents(function () use ($centralModel, $event) {
+                $centralModel->tenants()->attach($event->tenant->getTenantKey());
+            });
+        }
+
+        return $centralModel->tenants->filter(function ($model) use ($currentTenantMapping) {
+            // Remove the mapping for the current tenant.
+            return !$currentTenantMapping($model);
+        });
+    }
+
+    protected function updateResourceInTenantDatabases($tenants, $event, $syncedAttributes)
+    {
+        tenancy()->runForMultiple($tenants, function ($tenant) use ($event, $syncedAttributes) {
             // Forget instance state and find the model,
             // again in the current tenant's context.
 
@@ -84,13 +106,15 @@ class UpdateSyncedResource extends QueueableListener
             // why we're using Eloquent instead of direct DB queries.
 
             // We disable events for this call, to avoid triggering this event & listener again.
-            $localModelClass::withoutEvents(function () use ($localModelClass, $localModel, $syncedAttributes, $eventModel) {
+            $localModelClass::withoutEvents(function () use ($localModelClass, $localModel, $syncedAttributes, $eventModel, $tenant) {
                 if ($localModel) {
                     $localModel->update($syncedAttributes);
                 } else {
                     // When creating, we use all columns, not just the synced ones.
-                    $localModelClass::create($eventModel->getAttributes());
+                    $localModel = $localModelClass::create($eventModel->getAttributes());
                 }
+
+                event(new SyncedResourceChangedInForeignDatabase($localModel, $tenant));
             });
         });
     }

src/Events/SyncedResourceChangedInForeignDatabase.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace Stancl\Tenancy\Events;
+
+use Stancl\Tenancy\Contracts\Syncable;
+use Stancl\Tenancy\Contracts\TenantWithDatabase;
+
+class SyncedResourceChangedInForeignDatabase
+{
+    /** @var Syncable */
+    public $model;
+
+    /** @var TenantWithDatabase|null */
+    public $tenant;
+
+    public function __construct(Syncable $model, ?TenantWithDatabase $tenant)
+    {
+        $this->model = $model;
+        $this->tenant = $tenant;
+    }
+}

src/Events/SyncedResourceSaved.php

@@ -4,17 +4,17 @@ namespace Stancl\Tenancy\Events;
 
 use Illuminate\Database\Eloquent\Model;
 use Stancl\Tenancy\Contracts\Syncable;
-use Stancl\Tenancy\Contracts\Tenant;
+use Stancl\Tenancy\Contracts\TenantWithDatabase;
 
 class SyncedResourceSaved
 {
     /** @var Syncable|Model */
     public $model;
 
-    /** @var Tenant|Model|null */
+    /** @var TenantWithDatabase|Model|null */
     public $tenant;
 
-    public function __construct(Syncable $model, ?Tenant $tenant)
+    public function __construct(Syncable $model, ?TenantWithDatabase $tenant)
     {
         $this->model = $model;
         $this->tenant = $tenant;

src/UniqueIDGenerators/UUIDGenerator.php

@@ -4,13 +4,12 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\UniqueIDGenerators;
 
-use Illuminate\Database\Eloquent\Model;
 use Ramsey\Uuid\Uuid;
 use Stancl\Tenancy\Contracts\UniqueIdentifierGenerator;
 
 class UUIDGenerator implements UniqueIdentifierGenerator
 {
-    public static function generate(Model $model): string
+    public static function generate($resource): string
     {
         return Uuid::uuid4()->toString();
     }

tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php

@@ -20,6 +20,8 @@ class CreateTenantUsersTable extends Migration
             $table->string('tenant_id');
             $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');
         });

tests/v3/ResourceSyncingTest.php

@@ -11,12 +11,13 @@ use Stancl\Tenancy\Contracts\Syncable;
 use Stancl\Tenancy\Contracts\SyncMaster;
 use Stancl\Tenancy\Database\Models\Concerns\CentralConnection;
 use Stancl\Tenancy\Database\Models\Concerns\ResourceSyncing;
-use Stancl\Tenancy\Database\Models\Tenant;
+use Stancl\Tenancy\Database\Models;
 use Stancl\Tenancy\Database\Models\TenantPivot;
 use Stancl\Tenancy\Events\Listeners\BootstrapTenancy;
 use Stancl\Tenancy\Events\Listeners\JobPipeline;
 use Stancl\Tenancy\Events\Listeners\RevertToCentralContext;
 use Stancl\Tenancy\Events\Listeners\UpdateSyncedResource;
+use Stancl\Tenancy\Events\SyncedResourceChangedInForeignDatabase;
 use Stancl\Tenancy\Events\SyncedResourceSaved;
 use Stancl\Tenancy\Events\TenancyEnded;
 use Stancl\Tenancy\Events\TenancyInitialized;
@@ -211,6 +212,35 @@ class ResourceSyncingTest extends TestCase
         });
     }
 
+    /** @test */
+    public function attaching_users_to_tenants_DOES_NOT_DO_ANYTHING()
+    {
+        $centralUser = CentralUser::create([
+            'global_id' => 'acme',
+            'name' => 'John Doe',
+            'email' => 'john@localhost',
+            'password' => 'secret',
+            'role' => 'commenter', // unsynced
+        ]);
+
+        $tenant = Tenant::create([
+            'id' => 't1',
+        ]);
+        $this->migrateTenants();
+
+        $tenant->run(function () {
+            $this->assertCount(0, User::all());
+        });
+
+        // The child model is inaccessible in the Pivot Model, so we can't fire any events.
+        $tenant->users()->attach($centralUser);
+
+        $tenant->run(function () {
+            // Still zero
+            $this->assertCount(0, User::all());
+        });
+    }
+
     /** @test */
     public function resources_are_synced_only_to_workspaces_that_have_the_resource()
     {
@@ -439,6 +469,93 @@ class ResourceSyncingTest extends TestCase
             return $job->class === UpdateSyncedResource::class;
         });
     }
+
+    /** @test */
+    public function an_event_is_fired_for_all_touched_resources()
+    {
+        Event::fake([SyncedResourceChangedInForeignDatabase::class]);
+
+        // create shared resource
+        $centralUser = CentralUser::create([
+            'global_id' => 'acme',
+            'name' => 'John Doe',
+            'email' => 'john@localhost',
+            'password' => 'secret',
+            'role' => 'commenter', // unsynced
+        ]);
+
+        $t1 = Tenant::create([
+            'id' => 't1',
+        ]);
+        $t2 = Tenant::create([
+            'id' => 't2',
+        ]);
+        $t3 = Tenant::create([
+            'id' => 't3',
+        ]);
+        $this->migrateTenants();
+
+        // Copy (cascade) user to t1 DB
+        $centralUser->tenants()->attach('t1');
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return $event->tenant->getTenantKey() === 't1';
+        });
+        
+        $centralUser->tenants()->attach('t2');
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return $event->tenant->getTenantKey() === 't2';
+        });
+
+        $centralUser->tenants()->attach('t3');
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return $event->tenant->getTenantKey() === 't3';
+        });
+
+        // Assert no event for central
+        Event::assertNotDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return $event->tenant === null;
+        });
+
+        // Flush
+        Event::fake([SyncedResourceChangedInForeignDatabase::class]);
+
+        $t3->run(function () {
+            User::first()->update([
+                'name' => 'John 3',
+                'role' => 'employee', // unsynced
+            ]);
+
+            $this->assertSame('employee', User::first()->role);
+        });
+
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return optional($event->tenant)->getTenantKey() === 't1';
+        });
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return optional($event->tenant)->getTenantKey() === 't2';
+        });
+
+        // Assert NOT dispatched in t3
+        Event::assertNotDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return optional($event->tenant)->getTenantKey() === 't3';
+        });
+
+        // Assert dispatched in central
+        Event::assertDispatched(SyncedResourceChangedInForeignDatabase::class, function (SyncedResourceChangedInForeignDatabase $event) {
+            return $event->tenant === null;
+        });
+
+        // todo update in global
+    }
+}
+
+class Tenant extends Models\Tenant
+{
+    public function users()
+    {
+        return $this->belongsToMany(CentralUser::class, 'tenant_users', 'tenant_id', 'global_user_id')
+            ->using(TenantPivot::class);
+    }
 }
 
 class CentralUser extends Model implements SyncMaster
@@ -451,7 +568,7 @@ class CentralUser extends Model implements SyncMaster
 
     public function tenants(): BelongsToMany
     {
-        return $this->belongsToMany(Tenant::class, 'tenant_users', 'tenant_id', 'global_user_id')
+        return $this->belongsToMany(Tenant::class, 'tenant_users', 'global_user_id', 'tenant_id')
             ->using(TenantPivot::class);
     }