From d9bb5284d999ce89563265389d9bf67b6f30c24a Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 18 Nov 2024 09:45:51 +0000 Subject: [PATCH 1/5] chore: Have observers exit early if outside of multitenanted context --- .../Eloquent/Observers/BelongsToManyTenantsObserver.php | 9 +++++++++ .../Eloquent/Observers/BelongsToTenantObserver.php | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php b/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php index a293084..d65f4b0 100644 --- a/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php +++ b/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php @@ -10,6 +10,7 @@ use Sprout\Exceptions\TenantMismatch; use Sprout\Exceptions\TenantMissing; use Sprout\TenancyOptions; +use function Sprout\sprout; /** * Belongs to Many Tenants Observer @@ -151,6 +152,10 @@ private function passesInitialChecks(Model $model, Tenancy $tenancy, BelongsToMa */ public function created(Model $model): void { + if (! sprout()->withinContext()) { + return; + } + /** * @var \Illuminate\Database\Eloquent\Relations\BelongsToMany $relation * @phpstan-ignore-next-line @@ -199,6 +204,10 @@ public function created(Model $model): void */ public function retrieved(Model $model): void { + if (! sprout()->withinContext()) { + return; + } + /** * @var \Illuminate\Database\Eloquent\Relations\BelongsToMany $relation * @phpstan-ignore-next-line diff --git a/src/Database/Eloquent/Observers/BelongsToTenantObserver.php b/src/Database/Eloquent/Observers/BelongsToTenantObserver.php index 47a1fd3..e8224ae 100644 --- a/src/Database/Eloquent/Observers/BelongsToTenantObserver.php +++ b/src/Database/Eloquent/Observers/BelongsToTenantObserver.php @@ -10,6 +10,7 @@ use Sprout\Exceptions\TenantMismatch; use Sprout\Exceptions\TenantMissing; use Sprout\TenancyOptions; +use function Sprout\sprout; /** * Belongs to Tenant Observer @@ -134,6 +135,10 @@ private function passesInitialChecks(Model $model, Tenancy $tenancy, BelongsTo $ */ public function creating(Model $model): bool { + if (! sprout()->withinContext()) { + return true; + } + /** * @var \Illuminate\Database\Eloquent\Relations\BelongsTo $relation * @phpstan-ignore-next-line @@ -175,6 +180,10 @@ public function creating(Model $model): bool */ public function retrieved(Model $model): void { + if (! sprout()->withinContext()) { + return; + } + /** * @var \Illuminate\Database\Eloquent\Relations\BelongsTo $relation * @phpstan-ignore-next-line From 11834a81b6fadbe2af757fd83f1d8be144851424 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 18 Nov 2024 09:48:08 +0000 Subject: [PATCH 2/5] chore: Add support for additional extensions --- src/Database/Eloquent/Scopes/TenantChildScope.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Database/Eloquent/Scopes/TenantChildScope.php b/src/Database/Eloquent/Scopes/TenantChildScope.php index b183770..75a6bb6 100644 --- a/src/Database/Eloquent/Scopes/TenantChildScope.php +++ b/src/Database/Eloquent/Scopes/TenantChildScope.php @@ -19,6 +19,11 @@ */ abstract class TenantChildScope implements Scope { + /** + * @var array + */ + protected array $extensions = []; + /** * Extend the query builder with the necessary macros * @@ -34,5 +39,9 @@ public function extend(Builder $builder): void /** @phpstan-ignore-next-line */ return $builder->withoutGlobalScope($this); }); + + foreach ($this->extensions as $macro => $method) { + $builder->macro($macro, $this->$method(...)); + } } } From 4329fa8a7afc24b25b10aa5d8486166a8885b8b7 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 18 Nov 2024 09:50:48 +0000 Subject: [PATCH 3/5] chore: Do not apply condition from scopes when outside multitenanted context --- src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php | 5 +++++ src/Database/Eloquent/Scopes/BelongsToTenantScope.php | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php b/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php index 3d23f82..0422c43 100644 --- a/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php +++ b/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Sprout\Exceptions\TenantMissing; +use function Sprout\sprout; /** * Belongs to many Tenants Scope @@ -38,6 +39,10 @@ final class BelongsToManyTenantsScope extends TenantChildScope */ public function apply(Builder $builder, Model $model): void { + if (! sprout()->withinContext()) { + return; + } + /** @phpstan-ignore-next-line */ $tenancy = $model->getTenancy(); diff --git a/src/Database/Eloquent/Scopes/BelongsToTenantScope.php b/src/Database/Eloquent/Scopes/BelongsToTenantScope.php index c166ba1..b3ec890 100644 --- a/src/Database/Eloquent/Scopes/BelongsToTenantScope.php +++ b/src/Database/Eloquent/Scopes/BelongsToTenantScope.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Sprout\Exceptions\TenantMissing; +use function Sprout\sprout; /** * Belongs to Tenant Scope @@ -38,6 +39,10 @@ final class BelongsToTenantScope extends TenantChildScope */ public function apply(Builder $builder, Model $model): void { + if (! sprout()->withinContext()) { + return; + } + /** @phpstan-ignore-next-line */ $tenancy = $model->getTenancy(); From 61100059feb18bbdd68ca3fb873a97f02d0c1bcd Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 18 Nov 2024 09:59:44 +0000 Subject: [PATCH 4/5] fix: Fix tests by setting the current tenancy --- .../Eloquent/BelongsToManyTenantsTest.php | 25 +++++++++++++++-- .../Database/Eloquent/BelongsToTenantTest.php | 28 +++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/tests/Database/Eloquent/BelongsToManyTenantsTest.php b/tests/Database/Eloquent/BelongsToManyTenantsTest.php index 2b39c40..5e2e9a0 100644 --- a/tests/Database/Eloquent/BelongsToManyTenantsTest.php +++ b/tests/Database/Eloquent/BelongsToManyTenantsTest.php @@ -23,6 +23,7 @@ use Workbench\App\Models\TenantChildren; use Workbench\App\Models\TenantChildrenOptional; use Workbench\App\Models\TenantModel; +use function Sprout\sprout; #[Group('database'), Group('eloquent')] class BelongsToManyTenantsTest extends TestCase @@ -73,7 +74,11 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void { $tenant = TenantModel::factory()->create(); - app(TenancyManager::class)->get()->setTenant($tenant); + $tenancy = app(TenancyManager::class)->get(); + + sprout()->setCurrentTenancy($tenancy); + + $tenancy->setTenant($tenant); $child = TenantChildren::factory()->create(); @@ -85,6 +90,8 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void #[Test] public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreating(): void { + sprout()->setCurrentTenancy(app(TenancyManager::class)->get()); + $this->expectException(TenantMissing::class); $this->expectExceptionMessage( 'There is no current tenant for tenancy [tenants]' @@ -142,7 +149,11 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void { $tenant = TenantModel::factory()->create(); - app(TenancyManager::class)->get()->setTenant($tenant); + $tenancy = app(TenancyManager::class)->get(); + + sprout()->setCurrentTenancy($tenancy); + + $tenancy->setTenant($tenant); $child = TenantChildren::query()->find(TenantChildren::factory()->create()->getKey()); @@ -158,6 +169,8 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenHy $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); $child = TenantChildren::factory()->create(); @@ -223,7 +236,10 @@ public function throwsAnExceptionIfTheTenantIsAlreadySetOnTheModelAndItIsDiffere $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); + $tenancy->addOption(TenancyOptions::throwIfNotRelated()); $child = TenantChildren::factory()->create(); @@ -248,7 +264,10 @@ public function doesNotThrowAnExceptionForTenantMismatchIfNotSetToWhenHydrating( $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); + $tenancy->removeOption(TenancyOptions::throwIfNotRelated()); $child = TenantChildren::factory()->create(); @@ -270,6 +289,8 @@ public function onlyReturnsModelsForTheCurrentTenant(): void $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); $original = TenantChildren::factory()->create(); diff --git a/tests/Database/Eloquent/BelongsToTenantTest.php b/tests/Database/Eloquent/BelongsToTenantTest.php index 2b187da..df1b3d1 100644 --- a/tests/Database/Eloquent/BelongsToTenantTest.php +++ b/tests/Database/Eloquent/BelongsToTenantTest.php @@ -21,6 +21,7 @@ use Workbench\App\Models\TenantChild; use Workbench\App\Models\TenantChildOptional; use Workbench\App\Models\TenantModel; +use function Sprout\sprout; #[Group('database'), Group('eloquent')] class BelongsToTenantTest extends TestCase @@ -71,7 +72,11 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void { $tenant = TenantModel::factory()->create(); - app(TenancyManager::class)->get()->setTenant($tenant); + $tenancy = app(TenancyManager::class)->get(); + + sprout()->setCurrentTenancy($tenancy); + + $tenancy->setTenant($tenant); $child = TenantChild::factory()->create(); @@ -83,6 +88,8 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void #[Test] public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreating(): void { + sprout()->setCurrentTenancy(app(TenancyManager::class)->get()); + $this->expectException(TenantMissing::class); $this->expectExceptionMessage( 'There is no current tenant for tenancy [tenants]' @@ -138,7 +145,10 @@ public function throwsAnExceptionIfTheTenantIsAlreadySetOnTheModelAndItIsDiffere $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); + $tenancy->addOption(TenancyOptions::throwIfNotRelated()); $this->expectException(TenantMismatch::class); @@ -174,7 +184,11 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void { $tenant = TenantModel::factory()->create(); - app(TenancyManager::class)->get()->setTenant($tenant); + $tenancy = app(TenancyManager::class)->get(); + + sprout()->setCurrentTenancy($tenancy); + + $tenancy->setTenant($tenant); $child = TenantChild::query()->find(TenantChild::factory()->create()->getKey()); @@ -208,6 +222,8 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenHy $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); $child = TenantChild::factory()->create(); @@ -273,7 +289,10 @@ public function throwsAnExceptionIfTheTenantIsAlreadySetOnTheModelAndItIsDiffere $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); + $tenancy->addOption(TenancyOptions::throwIfNotRelated()); $child = TenantChild::factory()->create(); @@ -298,7 +317,10 @@ public function doesNotThrowAnExceptionForTenantMismatchIfNotSetToWhenHydrating( $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); + $tenancy->removeOption(TenancyOptions::throwIfNotRelated()); $child = TenantChild::factory()->create(); @@ -320,6 +342,8 @@ public function onlyReturnsModelsForTheCurrentTenant(): void $tenancy = app(TenancyManager::class)->get(); + sprout()->setCurrentTenancy($tenancy); + $tenancy->setTenant($tenant); $original = TenantChild::factory()->create(); From 0a39cd4157d9e1133071b078e0cfaff1a44e8a6a Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 18 Nov 2024 10:16:01 +0000 Subject: [PATCH 5/5] test: Add a few tests to account for new restrictions on eloquent models --- .../Eloquent/BelongsToManyTenantsTest.php | 29 +++++++++++++++++++ .../Database/Eloquent/BelongsToTenantTest.php | 29 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/tests/Database/Eloquent/BelongsToManyTenantsTest.php b/tests/Database/Eloquent/BelongsToManyTenantsTest.php index 5e2e9a0..2c4891a 100644 --- a/tests/Database/Eloquent/BelongsToManyTenantsTest.php +++ b/tests/Database/Eloquent/BelongsToManyTenantsTest.php @@ -87,6 +87,16 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void $this->assertNotNull($child->tenants->first(fn (Model $model) => $model->is($tenant))); } + #[Test] + public function doesNotAutomaticallyAssociateWithTenantWhenCreatingWhenOutsideMultitenantedContext(): void + { + $child = TenantChildren::factory()->create(); + + $this->assertTrue($child->exists); + $this->assertFalse($child->relationLoaded('tenants')); + $this->assertTrue($child->tenants->isEmpty()); + } + #[Test] public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreating(): void { @@ -100,6 +110,15 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCr TenantChildren::factory()->create(); } + #[Test] + public function doesNotThrowAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreatingWhenOutsideMultitenantedContext(): void + { + $model = TenantChildren::factory()->create(); + + $this->assertNotNull($model); + $this->assertTrue($model->exists); + } + #[Test] public function doesNothingIfTheresNoTenantAndTheTenantIsOptionalWithInterfaceWhenCreating(): void { @@ -162,6 +181,16 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void $this->assertNotNull($child->getRelation('tenants')->first(fn (Model $model) => $model->is($tenant))); } + #[Test] + public function doesNotAutomaticallyPopulateTheTenantRelationWhenHydratingWhenOutsideMultitenantedContext(): void + { + + $child = TenantChildren::query()->find(TenantChildren::factory()->create()->getKey()); + + $this->assertTrue($child->exists); + $this->assertFalse($child->relationLoaded('tenants')); + } + #[Test] public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenHydrating(): void { diff --git a/tests/Database/Eloquent/BelongsToTenantTest.php b/tests/Database/Eloquent/BelongsToTenantTest.php index df1b3d1..7bb857e 100644 --- a/tests/Database/Eloquent/BelongsToTenantTest.php +++ b/tests/Database/Eloquent/BelongsToTenantTest.php @@ -85,6 +85,16 @@ public function automaticallyAssociatesWithTenantWhenCreating(): void $this->assertTrue($child->tenant->is($tenant)); } + #[Test] + public function doesNotAutomaticallyAssociateWithTenantWhenCreatingWhenOutsideMultitenantedContext(): void + { + $child = TenantChild::factory()->create(); + + $this->assertTrue($child->exists); + $this->assertFalse($child->relationLoaded('tenant')); + $this->assertNull($child->tenant); + } + #[Test] public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreating(): void { @@ -98,6 +108,16 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCr TenantChild::factory()->create(); } + #[Test] + public function doesNotThrowAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCreatingWhenOutsideMultitenantedContext(): void + { + $child = TenantChild::factory()->create(); + + $this->assertTrue($child->exists); + $this->assertFalse($child->relationLoaded('tenant')); + $this->assertNull($child->tenant); + } + #[Test] public function doesNothingIfTheresNoTenantAndTheTenantIsOptionalWithInterfaceWhenCreating(): void { @@ -198,6 +218,15 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void $this->assertTrue($child->getRelation('tenant')->is($tenant)); } + #[Test] + public function doesNotAutomaticallyPopulateTheTenantRelationWhenHydratingWhenOutsideMultitenantedCContext(): void + { + $child = TenantChild::query()->find(TenantChild::factory()->create()->getKey()); + + $this->assertTrue($child->exists); + $this->assertFalse($child->relationLoaded('tenant')); + } + #[Test] public function doNotHydrateWhenHydrateTenantRelationIsMissing(): void {