From 0e91fdaa09abb0a862dc3d5f04908e9100a7217b Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sat, 28 Sep 2024 21:43:08 +0100 Subject: [PATCH 01/11] feat: Add support for tenants with resource keys --- src/Contracts/TenantHasResources.php | 26 ++++++++++ .../Eloquent/Concerns/HasTenantResources.php | 48 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 src/Contracts/TenantHasResources.php create mode 100644 src/Database/Eloquent/Concerns/HasTenantResources.php diff --git a/src/Contracts/TenantHasResources.php b/src/Contracts/TenantHasResources.php new file mode 100644 index 0000000..3d1e58f --- /dev/null +++ b/src/Contracts/TenantHasResources.php @@ -0,0 +1,26 @@ +getAttribute($model->getTenantResourceKeyName()) === null) { + $model->setAttribute( + $model->getTenantResourceKeyName(), + Str::uuid() + ); + } + }); + } + + /** + * Get the resource key used to identify the tenants resources + * + * @return string + */ + public function getTenantResourceKey(): string + { + return (string)$this->getAttribute($this->getTenantResourceKeyName()); + } + + /** + * Gets the name of the resource key used to identify the tenants resources + * + * @return string + */ + public function getTenantResourceKeyName(): string + { + return 'resource_key'; + } +} From 282c205072ea00f4e42bfe376ea338e36305a3da Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sat, 28 Sep 2024 21:43:51 +0100 Subject: [PATCH 02/11] feat: Add Laravel filesystem storage custom driver to tenantise storage disks --- resources/config/sprout.php | 4 ++ src/Listeners/CleanupLaravelServices.php | 51 ++++++++++++++++++++++++ src/SproutServiceProvider.php | 42 +++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 src/Listeners/CleanupLaravelServices.php diff --git a/resources/config/sprout.php b/resources/config/sprout.php index 38ffecf..01fbe89 100644 --- a/resources/config/sprout.php +++ b/resources/config/sprout.php @@ -4,4 +4,8 @@ 'listen_for_routing' => true, + 'services' => [ + 'storage' => true, + ], + ]; diff --git a/src/Listeners/CleanupLaravelServices.php b/src/Listeners/CleanupLaravelServices.php new file mode 100644 index 0000000..e470e32 --- /dev/null +++ b/src/Listeners/CleanupLaravelServices.php @@ -0,0 +1,51 @@ +filesystemManager = $filesystemManager; + } + + /** + * @template TenantClass of \Sprout\Contracts\Tenant + * + * @param \Sprout\Events\CurrentTenantChanged $event + * + * @return void + */ + public function handle(CurrentTenantChanged $event): void + { + $this->purgeFilesystemDisks(); + } + + private function purgeFilesystemDisks(): void + { + // If we're not overriding the storage service we can exit early + if (config('sprout.services.storage', false) === false) { + return; + } + + /** @var array> $diskConfig */ + $diskConfig = config('filesystems.disks', []); + + // If any of the disks have the 'sprout' driver we need to purge them, + // if they exist, so we don't end up leaking tenant information + foreach ($diskConfig as $disk => $config) { + if (($config['driver'] ?? null) === 'sprout') { + $this->filesystemManager->forgetDisk($disk); + } + } + } +} diff --git a/src/SproutServiceProvider.php b/src/SproutServiceProvider.php index cd78efa..9d05ab2 100644 --- a/src/SproutServiceProvider.php +++ b/src/SproutServiceProvider.php @@ -4,13 +4,18 @@ namespace Sprout; use Illuminate\Contracts\Events\Dispatcher; +use Illuminate\Filesystem\FilesystemManager; +use Illuminate\Foundation\Application; use Illuminate\Queue\Events\JobProcessing; use Illuminate\Routing\Events\RouteMatched; use Illuminate\Routing\Router; use Illuminate\Support\ServiceProvider; +use RuntimeException; +use Sprout\Contracts\TenantHasResources; use Sprout\Events\CurrentTenantChanged; use Sprout\Http\Middleware\TenantRoutes; use Sprout\Http\RouterMethods; +use Sprout\Listeners\CleanupLaravelServices; use Sprout\Listeners\IdentifyTenantOnRouting; use Sprout\Listeners\PerformIdentityResolverSetup; use Sprout\Listeners\SetCurrentTenantContext; @@ -86,6 +91,7 @@ public function boot(): void { $this->publishConfig(); $this->registerEventListeners(); + $this->registerServiceOverrides(); } private function publishConfig(): void @@ -105,6 +111,42 @@ private function registerEventListeners(): void $events->listen(CurrentTenantChanged::class, SetCurrentTenantContext::class); $events->listen(CurrentTenantChanged::class, PerformIdentityResolverSetup::class); + $events->listen(CurrentTenantChanged::class, CleanupLaravelServices::class); $events->listen(JobProcessing::class, SetCurrentTenantForJob::class); } + + private function registerServiceOverrides(): void + { + // If we're providing a tenanted override for Laravels filesystem/storage + // service, we'll do that here + if ($this->sprout->config('services.storage', false)) { + $filesystemManager = $this->app->make(FilesystemManager::class); + $filesystemManager->extend('sprout', function (Application $app, array $config) use ($filesystemManager) { + $tenancy = $this->sprout->tenancies()->get($config['tenancy'] ?? null); + + // If there's no tenant, error out + if (! $tenancy->check()) { + // TODO: Better exception + throw new RuntimeException('There isn\'t a current a tenant'); + } + + $tenant = $tenancy->tenant(); + + // If the tenant isn't configured for resources, also error out + if (! ($tenant instanceof TenantHasResources)) { + // TODO: Better exception + throw new RuntimeException('Current tenant isn\t configured for resources'); + } + + /** @var string $pathPrefix */ + $pathPrefix = $this->config['path'] ?? '{tenant}'; + + // Build up the path prefix with the tenant resource key + $config['prefix'] = str_replace('{tenant}', $tenant->getTenantResourceKey(), $pathPrefix); + + // Create a scoped driver for the new path + return $filesystemManager->createScopedDriver($config); + }); + } + } } From 4a14aed988a06876107472bfd5c2533f559cf342 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sun, 29 Sep 2024 22:58:23 +0100 Subject: [PATCH 03/11] refactor: Abstract out the handling of the tenant storage driver --- composer.json | 3 +- src/SproutServiceProvider.php | 28 +---------- src/Support/StorageHelper.php | 90 +++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 27 deletions(-) create mode 100644 src/Support/StorageHelper.php diff --git a/composer.json b/composer.json index a86bb0a..db60d4d 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,8 @@ "type" : "library", "require" : { "php" : "^8.2", - "laravel/framework": "^11.0" + "laravel/framework": "^11.0", + "league/flysystem-path-prefixing": "^3.0" }, "require-dev" : { "phpunit/phpunit" : "^11.0.1", diff --git a/src/SproutServiceProvider.php b/src/SproutServiceProvider.php index 9d05ab2..4f8fdfa 100644 --- a/src/SproutServiceProvider.php +++ b/src/SproutServiceProvider.php @@ -23,6 +23,7 @@ use Sprout\Managers\IdentityResolverManager; use Sprout\Managers\ProviderManager; use Sprout\Managers\TenancyManager; +use Sprout\Support\StorageHelper; class SproutServiceProvider extends ServiceProvider { @@ -121,32 +122,7 @@ private function registerServiceOverrides(): void // service, we'll do that here if ($this->sprout->config('services.storage', false)) { $filesystemManager = $this->app->make(FilesystemManager::class); - $filesystemManager->extend('sprout', function (Application $app, array $config) use ($filesystemManager) { - $tenancy = $this->sprout->tenancies()->get($config['tenancy'] ?? null); - - // If there's no tenant, error out - if (! $tenancy->check()) { - // TODO: Better exception - throw new RuntimeException('There isn\'t a current a tenant'); - } - - $tenant = $tenancy->tenant(); - - // If the tenant isn't configured for resources, also error out - if (! ($tenant instanceof TenantHasResources)) { - // TODO: Better exception - throw new RuntimeException('Current tenant isn\t configured for resources'); - } - - /** @var string $pathPrefix */ - $pathPrefix = $this->config['path'] ?? '{tenant}'; - - // Build up the path prefix with the tenant resource key - $config['prefix'] = str_replace('{tenant}', $tenant->getTenantResourceKey(), $pathPrefix); - - // Create a scoped driver for the new path - return $filesystemManager->createScopedDriver($config); - }); + $filesystemManager->extend('sprout', StorageHelper::creator($this->sprout, $filesystemManager)); } } } diff --git a/src/Support/StorageHelper.php b/src/Support/StorageHelper.php new file mode 100644 index 0000000..ab7dbcc --- /dev/null +++ b/src/Support/StorageHelper.php @@ -0,0 +1,90 @@ +tenancies()->get($config['tenancy'] ?? null); + + // If there's no tenant, error out + if (! $tenancy->check()) { + // TODO: Better exception + throw new RuntimeException('There isn\'t a current a tenant'); + } + + $tenant = $tenancy->tenant(); + + // If the tenant isn't configured for resources, also error out + if (! ($tenant instanceof TenantHasResources)) { + // TODO: Better exception + throw new RuntimeException('Current tenant isn\t configured for resources'); + } + + $tenantConfig = self::getTenantStorageConfig($manager, $tenant, $config); + + // Create a scoped driver for the new path + return $manager->createScopedDriver($tenantConfig); + }; + } + + /** + * @param \Illuminate\Filesystem\FilesystemManager $manager + * @param \Sprout\Contracts\TenantHasResources $tenant + * @param array $config + * + * @return array + */ + private static function getTenantStorageConfig(FilesystemManager $manager, TenantHasResources $tenant, array $config): array + { + /** @var string $pathPrefix */ + $pathPrefix = $config['path'] ?? '{tenant}'; + + // Create the empty tenant config + $tenantConfig = []; + + // Build up the path prefix with the tenant resource key + $tenantConfig['prefix'] = self::createTenantedPrefix($tenant, $pathPrefix); + + // Set the disk config on the newly created tenant config, so that the + // filesystem manager uses this, rather gets it straight from the config + $tenantConfig['disk'] = self::getDiskConfig($config); + + return $tenantConfig; + } + + private static function createTenantedPrefix(TenantHasResources $tenant, string $pathPrefix): string + { + return str_replace('{tenant}', $tenant->getTenantResourceKey(), $pathPrefix); + } + + /** + * @param array $config + * + * @return array + */ + private static function getDiskConfig(array $config): array + { + // Get the disk we're overriding and its config + /** @var string $diskName */ + $diskName = $config['disk'] ?? config('filesystems.default'); + /** @var array $diskConfig */ + $diskConfig = config('filesystems.disks.' . $diskName); + + // This is where we'd do anything like load config overrides for + // the tenant, like say they have their own S3 setup, etc. + + return $diskConfig; + } +} From 48697772d9df278d976aedbdff472d347aa1ed48 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sun, 29 Sep 2024 22:58:52 +0100 Subject: [PATCH 04/11] refactor: Rejig old tests to use factories were appropriate --- tests/Providers/DatabaseTenantProviderTest.php | 17 +++++++++-------- tests/Providers/EloquentTenantProviderTest.php | 4 ++-- tests/Resolvers/CookieResolverTest.php | 2 +- tests/Resolvers/HeaderResolverTest.php | 2 +- tests/Resolvers/PathResolverTest.php | 4 ++-- tests/Resolvers/SessionResolverTest.php | 2 +- tests/Resolvers/SubdomainResolverTest.php | 4 ++-- tests/SproutTest.php | 2 +- workbench/database/seeders/DatabaseSeeder.php | 10 ---------- 9 files changed, 19 insertions(+), 28 deletions(-) diff --git a/tests/Providers/DatabaseTenantProviderTest.php b/tests/Providers/DatabaseTenantProviderTest.php index b84ca2f..f4b6eb9 100644 --- a/tests/Providers/DatabaseTenantProviderTest.php +++ b/tests/Providers/DatabaseTenantProviderTest.php @@ -6,15 +6,14 @@ use Illuminate\Config\Repository; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Str; use Orchestra\Testbench\Concerns\WithWorkbench; use Orchestra\Testbench\TestCase; use PHPUnit\Framework\Attributes\Test; use Sprout\Managers\ProviderManager; use Sprout\Providers\DatabaseTenantProvider; -use Sprout\Providers\EloquentTenantProvider; use Sprout\Sprout; use Sprout\Support\GenericTenant; -use Workbench\App\Models\TenantModel; class DatabaseTenantProviderTest extends TestCase { @@ -47,9 +46,10 @@ public function canRetrieveByIdentifier(): void { $id = DB::table('tenants')->insertGetId( [ - 'name' => 'Test Tenant', - 'identifier' => 'the-big-test-boy', - 'active' => true, + 'name' => 'Test Tenant', + 'identifier' => 'the-big-test-boy', + 'resource_key' => Str::uuid()->toString(), + 'active' => true, ] ); @@ -76,9 +76,10 @@ public function canRetrieveByKey(): void { $id = DB::table('tenants')->insertGetId( [ - 'name' => 'Test Tenant', - 'identifier' => 'the-big-test-boy2', - 'active' => true, + 'name' => 'Test Tenant', + 'identifier' => 'the-big-test-boy2', + 'resource_key' => Str::uuid()->toString(), + 'active' => true, ] ); diff --git a/tests/Providers/EloquentTenantProviderTest.php b/tests/Providers/EloquentTenantProviderTest.php index 7c2f2be..03c0640 100644 --- a/tests/Providers/EloquentTenantProviderTest.php +++ b/tests/Providers/EloquentTenantProviderTest.php @@ -43,7 +43,7 @@ public function isRegisteredCorrectly(): void #[Test] public function canRetrieveByIdentifier(): void { - $newTenant = TenantModel::create( + $newTenant = TenantModel::factory()->createOne( [ 'name' => 'Test Tenant', 'identifier' => 'the-big-test-boy', @@ -70,7 +70,7 @@ public function failsSilentlyWithInvalidIdentifier(): void #[Test] public function canRetrieveByKey(): void { - $newTenant = TenantModel::create( + $newTenant = TenantModel::factory()->createOne( [ 'name' => 'Test Tenant', 'identifier' => 'the-big-test-boy2', diff --git a/tests/Resolvers/CookieResolverTest.php b/tests/Resolvers/CookieResolverTest.php index e2901be..6665bf1 100644 --- a/tests/Resolvers/CookieResolverTest.php +++ b/tests/Resolvers/CookieResolverTest.php @@ -49,7 +49,7 @@ protected function defineRoutes($router) #[Test] public function resolvesFromRoute(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->withUnencryptedCookie('Tenants-Identifier', $tenant->getTenantIdentifier())->get(route('cookie.route')); diff --git a/tests/Resolvers/HeaderResolverTest.php b/tests/Resolvers/HeaderResolverTest.php index 0de95d6..17cc596 100644 --- a/tests/Resolvers/HeaderResolverTest.php +++ b/tests/Resolvers/HeaderResolverTest.php @@ -43,7 +43,7 @@ protected function defineRoutes($router) #[Test] public function resolvesFromRoute(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->get(route('header.route'), ['Tenants-Identifier' => $tenant->getTenantIdentifier()]); diff --git a/tests/Resolvers/PathResolverTest.php b/tests/Resolvers/PathResolverTest.php index 9634f7c..853aad3 100644 --- a/tests/Resolvers/PathResolverTest.php +++ b/tests/Resolvers/PathResolverTest.php @@ -47,7 +47,7 @@ protected function defineRoutes($router) #[Test] public function resolvesFromParameter(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->get(route('path.route', ['tenants_path' => $tenant->getTenantIdentifier()])); @@ -58,7 +58,7 @@ public function resolvesFromParameter(): void #[Test] public function resolvesWithoutParameter(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->get('/' . $tenant->getTenantIdentifier() . '/path-request'); diff --git a/tests/Resolvers/SessionResolverTest.php b/tests/Resolvers/SessionResolverTest.php index ad8a22b..562d454 100644 --- a/tests/Resolvers/SessionResolverTest.php +++ b/tests/Resolvers/SessionResolverTest.php @@ -52,7 +52,7 @@ protected function defineRoutes($router) #[Test] public function resolvesFromRoute(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->withSession(['multitenancy' => ['tenants' => $tenant->getTenantIdentifier()]])->get(route('session.route')); diff --git a/tests/Resolvers/SubdomainResolverTest.php b/tests/Resolvers/SubdomainResolverTest.php index 5e1ffa8..d53edd4 100644 --- a/tests/Resolvers/SubdomainResolverTest.php +++ b/tests/Resolvers/SubdomainResolverTest.php @@ -48,7 +48,7 @@ protected function defineRoutes($router) #[Test] public function resolvesFromParameter(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->get(route('subdomain.route', ['tenants_subdomain' => $tenant->getTenantIdentifier()])); @@ -59,7 +59,7 @@ public function resolvesFromParameter(): void #[Test] public function resolvesWithoutParameter(): void { - $tenant = TenantModel::first(); + $tenant = TenantModel::factory()->createOne(); $result = $this->get('http://' . $tenant->getTenantIdentifier() . '.localhost/subdomain-request'); diff --git a/tests/SproutTest.php b/tests/SproutTest.php index d5aa67b..c04a53a 100644 --- a/tests/SproutTest.php +++ b/tests/SproutTest.php @@ -12,7 +12,7 @@ use Sprout\Sprout; use Workbench\App\Models\TenantModel; -#[Group('core'), Group('services')] +#[Group('core')] class SproutTest extends TestCase { use WithWorkbench, RefreshDatabase; diff --git a/workbench/database/seeders/DatabaseSeeder.php b/workbench/database/seeders/DatabaseSeeder.php index 5683eea..52ba6f1 100644 --- a/workbench/database/seeders/DatabaseSeeder.php +++ b/workbench/database/seeders/DatabaseSeeder.php @@ -20,16 +20,6 @@ class DatabaseSeeder extends Seeder */ public function run(): void { - $tenants = TenantModelFactory::new()->createMany(20); - $tenants->each(function (TenantModel $tenant, int $i) { - TenantChildFactory::new()->afterMaking(function (TenantChild $child) use ($tenant) { - $child->tenant()->associate($tenant); - })->createMany(5); - }); - - TenantChildrenFactory::new()->afterCreating(function (TenantChildren $child) use ($tenants) { - $child->tenants()->saveMany($tenants->random(random_int(1, 4))); - })->createMany(10); } } From 2203f6ac9fa91c1ac8454354314a374db6b0b8a9 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sun, 29 Sep 2024 22:59:17 +0100 Subject: [PATCH 05/11] chore: Make the default tenant model have resources --- workbench/app/Models/TenantModel.php | 11 ++++++++--- .../2024_09_08_194545_create_tenants_table.php | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/workbench/app/Models/TenantModel.php b/workbench/app/Models/TenantModel.php index 384d0ad..c81c91c 100644 --- a/workbench/app/Models/TenantModel.php +++ b/workbench/app/Models/TenantModel.php @@ -6,13 +6,17 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Sprout\Contracts\Tenant; +use Sprout\Contracts\TenantHasResources; +use Sprout\Database\Eloquent\Concerns\HasTenantResources; use Sprout\Database\Eloquent\Concerns\IsTenant; -use Workbench\Database\Factories\TenantChildFactory; use Workbench\Database\Factories\TenantModelFactory; -class TenantModel extends Model implements Tenant +class TenantModel extends Model implements Tenant, TenantHasResources { - use IsTenant, HasFactory; + /** + * @use \Illuminate\Database\Eloquent\Factories\HasFactory + */ + use IsTenant, HasFactory, HasTenantResources; protected $table = 'tenants'; @@ -21,6 +25,7 @@ class TenantModel extends Model implements Tenant protected $fillable = [ 'name', 'identifier', + 'resource_key', 'active', ]; diff --git a/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php b/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php index 3bc6234..fc98bfd 100644 --- a/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php +++ b/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php @@ -15,6 +15,7 @@ public function up(): void $table->string('name'); $table->string('identifier')->unique(); + $table->uuid('resource_key')->unique(); $table->boolean('active')->default(true); $table->timestamps(); From 5149c714eb5d2beadba07ebcd61fdda20be3f301 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Sun, 29 Sep 2024 23:01:28 +0100 Subject: [PATCH 06/11] test(storage): Add base test for filesystem tenant disk --- tests/Services/FilesystemTest.php | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tests/Services/FilesystemTest.php diff --git a/tests/Services/FilesystemTest.php b/tests/Services/FilesystemTest.php new file mode 100644 index 0000000..7ea0c66 --- /dev/null +++ b/tests/Services/FilesystemTest.php @@ -0,0 +1,47 @@ +set('multitenancy.providers.tenants.model', TenantModel::class); + $config->set('filesystems.disks.tenant', [ + 'driver' => 'sprout', + 'disk' => 'local', + 'tenancy' => 'tenants', + ]); + }); + } + + #[Test] + public function canCreateScopedTenantFilesystemDisk(): void + { + $tenant = TenantModel::factory()->createOne(); + + app(TenancyManager::class)->get()->setTenant($tenant); + + $disk = Storage::disk('tenant'); + + $this->assertNotNull($disk); + $this->assertSame($tenant->getTenantResourceKey(), basename($disk->path(''))); + } +} From d161dbc198b33c719bba30a66b41072287f66257 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 30 Sep 2024 10:34:25 +0100 Subject: [PATCH 07/11] refactor: Refactor the TenantMissing exception to not be model-specific --- .../Observers/BelongsToManyTenantsObserver.php | 2 +- .../Eloquent/Observers/BelongsToTenantObserver.php | 2 +- .../Eloquent/Scopes/BelongsToManyTenantsScope.php | 2 +- src/Database/Eloquent/Scopes/BelongsToTenantScope.php | 2 +- src/Exceptions/TenantMissing.php | 6 ++---- tests/Database/Eloquent/BelongsToManyTenantsTest.php | 10 ++-------- tests/Database/Eloquent/BelongsToTenantTest.php | 10 ++-------- 7 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php b/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php index ab9436f..2f58828 100644 --- a/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php +++ b/src/Database/Eloquent/Observers/BelongsToManyTenantsObserver.php @@ -88,7 +88,7 @@ private function passesInitialChecks(Model $model, Tenancy $tenancy, BelongsToMa // If we hit here then there's no tenant, and the model isn't // marked as tenant being optional, so we throw an exception - throw TenantMissing::make($model::class, $tenancy->getName()); + throw TenantMissing::make($tenancy->getName()); } /** diff --git a/src/Database/Eloquent/Observers/BelongsToTenantObserver.php b/src/Database/Eloquent/Observers/BelongsToTenantObserver.php index cd41339..f3749e2 100644 --- a/src/Database/Eloquent/Observers/BelongsToTenantObserver.php +++ b/src/Database/Eloquent/Observers/BelongsToTenantObserver.php @@ -71,7 +71,7 @@ private function passesInitialChecks(Model $model, Tenancy $tenancy, BelongsTo $ // If we hit here then there's no tenant, and the model isn't // marked as tenant being optional, so we throw an exception - throw TenantMissing::make($model::class, $tenancy->getName()); + throw TenantMissing::make($tenancy->getName()); } /** diff --git a/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php b/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php index 38cab63..c5f09a6 100644 --- a/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php +++ b/src/Database/Eloquent/Scopes/BelongsToManyTenantsScope.php @@ -38,7 +38,7 @@ public function apply(Builder $builder, Model $model): void } // We should throw an exception because the tenant is missing - throw TenantMissing::make($model::class, $tenancy->getName()); + throw TenantMissing::make($tenancy->getName()); } // Finally, add the clause so that all queries are scoped to the diff --git a/src/Database/Eloquent/Scopes/BelongsToTenantScope.php b/src/Database/Eloquent/Scopes/BelongsToTenantScope.php index faabcb2..dbf3bf3 100644 --- a/src/Database/Eloquent/Scopes/BelongsToTenantScope.php +++ b/src/Database/Eloquent/Scopes/BelongsToTenantScope.php @@ -38,7 +38,7 @@ public function apply(Builder $builder, Model $model): void } // We should throw an exception because the tenant is missing - throw TenantMissing::make($model::class, $tenancy->getName()); + throw TenantMissing::make($tenancy->getName()); } // Finally, add the clause so that all queries are scoped to the diff --git a/src/Exceptions/TenantMissing.php b/src/Exceptions/TenantMissing.php index 1087f4e..88bc48c 100644 --- a/src/Exceptions/TenantMissing.php +++ b/src/Exceptions/TenantMissing.php @@ -5,12 +5,10 @@ final class TenantMissing extends SproutException { - public static function make(string $model, ?string $tenancy): self + public static function make(string $tenancy): self { return new self( - 'Model [' . $model . '] requires a tenant, and the tenancy' - . ($tenancy ? ' [' . $tenancy . '] ' : ' ') - . 'does not have one' + 'There is no current tenant for tenancy [' . $tenancy . ']' ); } } diff --git a/tests/Database/Eloquent/BelongsToManyTenantsTest.php b/tests/Database/Eloquent/BelongsToManyTenantsTest.php index a2b7443..c3017ad 100644 --- a/tests/Database/Eloquent/BelongsToManyTenantsTest.php +++ b/tests/Database/Eloquent/BelongsToManyTenantsTest.php @@ -87,10 +87,7 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCr { $this->expectException(TenantMissing::class); $this->expectExceptionMessage( - 'Model [' - . TenantChildren::class - . '] requires a tenant, and the tenancy' - . ' [tenants] does not have one' + 'There is no current tenant for tenancy [tenants]' ); TenantChildren::factory()->create(); @@ -169,10 +166,7 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenHy $this->expectException(TenantMissing::class); $this->expectExceptionMessage( - 'Model [' - . TenantChildren::class - . '] requires a tenant, and the tenancy' - . ' [tenants] does not have one' + 'There is no current tenant for tenancy [tenants]' ); TenantChildren::query()->find($child->getKey()); diff --git a/tests/Database/Eloquent/BelongsToTenantTest.php b/tests/Database/Eloquent/BelongsToTenantTest.php index d00f92d..b2ec1e1 100644 --- a/tests/Database/Eloquent/BelongsToTenantTest.php +++ b/tests/Database/Eloquent/BelongsToTenantTest.php @@ -85,10 +85,7 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenCr { $this->expectException(TenantMissing::class); $this->expectExceptionMessage( - 'Model [' - . TenantChild::class - . '] requires a tenant, and the tenancy' - . ' [tenants] does not have one' + 'There is no current tenant for tenancy [tenants]' ); TenantChild::factory()->create(); @@ -201,10 +198,7 @@ public function throwsAnExceptionIfTheresNoTenantAndTheTenantIsNotOptionalWhenHy $this->expectException(TenantMissing::class); $this->expectExceptionMessage( - 'Model [' - . TenantChild::class - . '] requires a tenant, and the tenancy' - . ' [tenants] does not have one' + 'There is no current tenant for tenancy [tenants]' ); TenantChild::query()->find($child->getKey()); From e0e8cccd76cb8d8ba21f6f7d2bebcb51d202d703 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 30 Sep 2024 10:47:19 +0100 Subject: [PATCH 08/11] refactor: Use the TenantMissing exception when creating scoped filesystem disks --- src/Support/StorageHelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Support/StorageHelper.php b/src/Support/StorageHelper.php index ab7dbcc..a8b2958 100644 --- a/src/Support/StorageHelper.php +++ b/src/Support/StorageHelper.php @@ -9,6 +9,7 @@ use Illuminate\Foundation\Application; use RuntimeException; use Sprout\Contracts\TenantHasResources; +use Sprout\Exceptions\TenantMissing; use Sprout\Sprout; final class StorageHelper @@ -20,8 +21,7 @@ public static function creator(Sprout $sprout, FilesystemManager $manager): Clos // If there's no tenant, error out if (! $tenancy->check()) { - // TODO: Better exception - throw new RuntimeException('There isn\'t a current a tenant'); + throw TenantMissing::make($tenancy->getName()); } $tenant = $tenancy->tenant(); From 6d6d52ee8c4d1639b403a20b368ab5ad4ba5563e Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 30 Sep 2024 10:47:41 +0100 Subject: [PATCH 09/11] chore: Allow for custom disk config when creating a scoped tenant disk --- src/Support/StorageHelper.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Support/StorageHelper.php b/src/Support/StorageHelper.php index a8b2958..67162bf 100644 --- a/src/Support/StorageHelper.php +++ b/src/Support/StorageHelper.php @@ -76,11 +76,15 @@ private static function createTenantedPrefix(TenantHasResources $tenant, string */ private static function getDiskConfig(array $config): array { - // Get the disk we're overriding and its config - /** @var string $diskName */ - $diskName = $config['disk'] ?? config('filesystems.default'); + if (is_array($config['disk'])) { + $diskConfig = $config['disk']; + } else { + /** @var string $diskName */ + $diskName = $config['disk'] ?? config('filesystems.default'); + $diskConfig = config('filesystems.disks.' . $diskName); + } + /** @var array $diskConfig */ - $diskConfig = config('filesystems.disks.' . $diskName); // This is where we'd do anything like load config overrides for // the tenant, like say they have their own S3 setup, etc. From 3744f454ddbc7a248e6b2b40415b545595616c7b Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 30 Sep 2024 10:48:07 +0100 Subject: [PATCH 10/11] test(storage): Add tests for other scoped filesystem storage cases --- tests/Services/FilesystemTest.php | 76 +++++++++++++++++++ .../app/Models/NoResourcesTenantModel.php | 35 +++++++++ .../NoResourcesTenantModelFactory.php | 35 +++++++++ ...2024_09_08_194545_create_tenants_table.php | 2 +- 4 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 workbench/app/Models/NoResourcesTenantModel.php create mode 100644 workbench/database/factories/NoResourcesTenantModelFactory.php diff --git a/tests/Services/FilesystemTest.php b/tests/Services/FilesystemTest.php index 7ea0c66..6a5001e 100644 --- a/tests/Services/FilesystemTest.php +++ b/tests/Services/FilesystemTest.php @@ -10,7 +10,10 @@ use Orchestra\Testbench\TestCase; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; +use RuntimeException; +use Sprout\Exceptions\TenantMissing; use Sprout\Managers\TenancyManager; +use Workbench\App\Models\NoResourcesTenantModel; use Workbench\App\Models\TenantModel; #[Group('services'), Group('filesystem')] @@ -44,4 +47,77 @@ public function canCreateScopedTenantFilesystemDisk(): void $this->assertNotNull($disk); $this->assertSame($tenant->getTenantResourceKey(), basename($disk->path(''))); } + + #[Test] + public function canCreateScopedTenantFilesystemDiskWithCustomConfig(): void + { + config()->set('filesystems.disks.tenant.disk', config('filesystems.disks.local')); + + $tenant = TenantModel::factory()->createOne(); + + app(TenancyManager::class)->get()->setTenant($tenant); + + $disk = Storage::disk('tenant'); + + $this->assertNotNull($disk); + $this->assertSame($tenant->getTenantResourceKey(), basename($disk->path(''))); + } + + #[Test] + public function throwsExceptionIfThereIsNoTenant(): void + { + $this->expectException(TenantMissing::class); + $this->expectExceptionMessage('There is no current tenant for tenancy [tenants]'); + + Storage::disk('tenant'); + } + + #[Test] + public function throwsExceptionIfTheTenantDoesNotHaveResources(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Current tenant isn\t configured for resources'); + + config()->set('multitenancy.providers.tenants.model', NoResourcesTenantModel::class); + + app(TenancyManager::class)->get()->setTenant(NoResourcesTenantModel::factory()->createOne()); + + Storage::disk('tenant'); + } + + #[Test] + public function cleansUpStorageDiskAfterTenantChange(): void + { + $tenant = TenantModel::factory()->createOne(); + + app(TenancyManager::class)->get()->setTenant($tenant); + + Storage::disk('tenant'); + + Storage::shouldReceive('forgetDisk')->withArgs(['tenant'])->once(); + + app(TenancyManager::class)->get()->setTenant(null); + } + + #[Test] + public function recreatesStorageDiskPerTenant(): void + { + $tenant1 = TenantModel::factory()->createOne(); + + app(TenancyManager::class)->get()->setTenant($tenant1); + + $disk = Storage::disk('tenant'); + + $this->assertNotNull($disk); + $this->assertSame($tenant1->getTenantResourceKey(), basename($disk->path(''))); + + $tenant2 = TenantModel::factory()->createOne(); + + app(TenancyManager::class)->get()->setTenant($tenant2); + + $disk = Storage::disk('tenant'); + + $this->assertNotNull($disk); + $this->assertSame($tenant2->getTenantResourceKey(), basename($disk->path(''))); + } } diff --git a/workbench/app/Models/NoResourcesTenantModel.php b/workbench/app/Models/NoResourcesTenantModel.php new file mode 100644 index 0000000..8972322 --- /dev/null +++ b/workbench/app/Models/NoResourcesTenantModel.php @@ -0,0 +1,35 @@ + + */ + use IsTenant, HasFactory; + + protected $table = 'tenants'; + + protected static string $factory = NoResourcesTenantModelFactory::class; + + protected $fillable = [ + 'name', + 'identifier', + 'active', + ]; + + protected function casts(): array + { + return [ + 'active' => 'boolean', + ]; + } +} diff --git a/workbench/database/factories/NoResourcesTenantModelFactory.php b/workbench/database/factories/NoResourcesTenantModelFactory.php new file mode 100644 index 0000000..93e1306 --- /dev/null +++ b/workbench/database/factories/NoResourcesTenantModelFactory.php @@ -0,0 +1,35 @@ + + */ +class NoResourcesTenantModelFactory extends Factory +{ + /** + * The name of the factory's corresponding model. + * + * @var class-string + */ + protected $model = NoResourcesTenantModel::class; + + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + return [ + 'name' => $this->faker->company(), + 'identifier' => $this->faker->unique()->slug(), + 'active' => true, + ]; + } +} diff --git a/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php b/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php index fc98bfd..e865de3 100644 --- a/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php +++ b/workbench/database/migrations/2024_09_08_194545_create_tenants_table.php @@ -15,7 +15,7 @@ public function up(): void $table->string('name'); $table->string('identifier')->unique(); - $table->uuid('resource_key')->unique(); + $table->uuid('resource_key')->nullable()->unique(); $table->boolean('active')->default(true); $table->timestamps(); From 14e3bd6a3ea67cab1458a3cccd835285d3449280 Mon Sep 17 00:00:00 2001 From: Ollie Read Date: Mon, 30 Sep 2024 10:52:19 +0100 Subject: [PATCH 11/11] chore: Exit early if storage override is disabled --- src/Support/StorageHelper.php | 6 +++++- tests/Services/FilesystemTest.php | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Support/StorageHelper.php b/src/Support/StorageHelper.php index 67162bf..d83ada7 100644 --- a/src/Support/StorageHelper.php +++ b/src/Support/StorageHelper.php @@ -16,7 +16,11 @@ final class StorageHelper { public static function creator(Sprout $sprout, FilesystemManager $manager): Closure { - return static function (Application $app, array $config) use ($sprout, $manager): Filesystem { + return static function (Application $app, array $config) use ($sprout, $manager): ?Filesystem { + if ($sprout->config('services.storage', false) === false) { + return null; + } + $tenancy = $sprout->tenancies()->get($config['tenancy'] ?? null); // If there's no tenant, error out diff --git a/tests/Services/FilesystemTest.php b/tests/Services/FilesystemTest.php index 6a5001e..ed252ef 100644 --- a/tests/Services/FilesystemTest.php +++ b/tests/Services/FilesystemTest.php @@ -120,4 +120,16 @@ public function recreatesStorageDiskPerTenant(): void $this->assertNotNull($disk); $this->assertSame($tenant2->getTenantResourceKey(), basename($disk->path(''))); } + + #[Test] + public function doesNothingIfStorageServiceIsDisabled(): void + { + config()->set('sprout.services.storage', false); + + app(TenancyManager::class)->get()->setTenant(TenantModel::factory()->createOne()); + + $disk = Storage::disk('tenant'); + + $this->assertNull($disk); + } }