From 5bb76e1421673d63dfabcab8da7c2a347b77db80 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Mon, 20 Apr 2026 18:22:04 +0200 Subject: [PATCH] refactor 1. Remove the CreateTenantStorage job altogether since as the docblock says the class should be redundant now that FilesystemTenancyBootstrapper creates this path automatically when storage_path suffixing is enabled 2. Remove docblock on the DeleteTenantStorage job - a class's docblock should describe what it does, not how it handles edge cases. Here the former isn't even necessary and the latter is well explained by comments in the implementation. 3. Remove the CreateTenantStorage test following the class's removal, the test would pass on its own even without the job with just tenancy initialization as mentioned above. 4. Slightly improve the structure of the DeleteTenantStorage job and deprecated listener 5. Improve deprecation notices so they include full steps for upgrading to the new approach. --- assets/TenancyServiceProvider.stub.php | 1 - src/Jobs/CreateTenantStorage.php | 39 ------------------- src/Jobs/DeleteTenantStorage.php | 24 +++++------- src/Listeners/CreateTenantStorage.php | 2 +- src/Listeners/DeleteTenantStorage.php | 19 +++++---- .../FilesystemTenancyBootstrapperTest.php | 22 ----------- 6 files changed, 22 insertions(+), 85 deletions(-) delete mode 100644 src/Jobs/CreateTenantStorage.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 050a2ffc..915e80c2 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -46,7 +46,6 @@ class TenancyServiceProvider extends ServiceProvider Jobs\CreateDatabase::class, Jobs\MigrateDatabase::class, // Jobs\SeedDatabase::class, - // Jobs\CreateTenantStorage::class, // Jobs\CreateStorageSymlinks::class, // Your own jobs to prepare the tenant. diff --git a/src/Jobs/CreateTenantStorage.php b/src/Jobs/CreateTenantStorage.php deleted file mode 100644 index 8e4e15a9..00000000 --- a/src/Jobs/CreateTenantStorage.php +++ /dev/null @@ -1,39 +0,0 @@ -run($this->tenant, fn () => storage_path()); - $cache_path = "$storage_path/framework/cache"; - - if (! is_dir($cache_path)) { - // Create the tenant's storage directory and /framework/cache within (used for e.g. real-time facades) - mkdir($cache_path, 0750, true); - } - } -} diff --git a/src/Jobs/DeleteTenantStorage.php b/src/Jobs/DeleteTenantStorage.php index f31ddbd2..36a0d326 100644 --- a/src/Jobs/DeleteTenantStorage.php +++ b/src/Jobs/DeleteTenantStorage.php @@ -12,13 +12,6 @@ use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\File; use Stancl\Tenancy\Contracts\Tenant; -/** - * Only delete the tenant storage if storage path suffixing is enabled - * and the tenant's storage path is different from the central storage path. - * - * This is to prevent accidental deletion of the central storage when - * a tenant's storage path is not properly suffixed. - */ class DeleteTenantStorage implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; @@ -29,19 +22,22 @@ class DeleteTenantStorage implements ShouldQueue public function handle(): void { - // Skip storage deletion if path suffixing is disabled if (config('tenancy.filesystem.suffix_storage_path') === false) { + // Skip storage deletion if path suffixing is disabled return; } - $centralPath = tenancy()->central(fn () => storage_path()); - $path = tenancy()->run($this->tenant, fn () => storage_path()); + $centralStoragePath = tenancy()->central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($this->tenant, fn () => storage_path()); - // Skip storage deletion if tenant's storage path is the same as central storage path - $tenantPathIsCentral = realpath($path) === realpath($centralPath); + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } - if (is_dir($path) && ! $tenantPathIsCentral) { - File::deleteDirectory($path); + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); } } } diff --git a/src/Listeners/CreateTenantStorage.php b/src/Listeners/CreateTenantStorage.php index 3210a050..0ffdef60 100644 --- a/src/Listeners/CreateTenantStorage.php +++ b/src/Listeners/CreateTenantStorage.php @@ -7,7 +7,7 @@ namespace Stancl\Tenancy\Listeners; use Stancl\Tenancy\Events\Contracts\TenantEvent; /** - * @deprecated use Stancl\Tenancy\Jobs\CreateTenantStorage instead. + * @deprecated FilesystemTenancyBootstrapper creates the path automatically when suffix_storage_path is enabled. */ class CreateTenantStorage { diff --git a/src/Listeners/DeleteTenantStorage.php b/src/Listeners/DeleteTenantStorage.php index c32e14ba..06f20454 100644 --- a/src/Listeners/DeleteTenantStorage.php +++ b/src/Listeners/DeleteTenantStorage.php @@ -8,25 +8,28 @@ use Illuminate\Support\Facades\File; use Stancl\Tenancy\Events\Contracts\TenantEvent; /** - * @deprecated use Stancl\Tenancy\Jobs\DeleteTenantStorage instead. + * @deprecated Use Stancl\Tenancy\Jobs\DeleteTenantStorage in a job pipeline instead. */ class DeleteTenantStorage { public function handle(TenantEvent $event): void { - // Skip storage deletion if path suffixing is disabled if (config('tenancy.filesystem.suffix_storage_path') === false) { + // Skip storage deletion if path suffixing is disabled return; } - $centralPath = tenancy()->central(fn () => storage_path()); - $path = tenancy()->run($event->tenant, fn () => storage_path()); + $centralStoragePath = tenancy()->central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($event->tenant, fn () => storage_path()); - // Skip storage deletion if tenant's storage path is the same as central storage path - $tenantPathIsCentral = realpath($path) === realpath($centralPath); + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } - if (is_dir($path) && ! $tenantPathIsCentral) { - File::deleteDirectory($path); + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); } } } diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index d0c8af59..449ee1f4 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -13,7 +13,6 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Jobs\CreateStorageSymlinks; use Stancl\Tenancy\Jobs\RemoveStorageSymlinks; use Stancl\Tenancy\Listeners\BootstrapTenancy; -use Stancl\Tenancy\Jobs\CreateTenantStorage; use Stancl\Tenancy\Jobs\DeleteTenantStorage; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; @@ -185,27 +184,6 @@ test('create and delete storage symlinks jobs work', function() { $this->assertDirectoryDoesNotExist(public_path("public-$tenantKey")); }); -test('tenant storage gets created when TenantCreated listens to CreateTenantStorage', function() { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - ], - ]); - - Event::listen(TenantCreated::class, - JobPipeline::make([CreateTenantStorage::class])->send(function (TenantCreated $event) { - return $event->tenant; - })->shouldBeQueued(false)->toListener() - ); - - $centralStoragePath = storage_path(); - $tenant = Tenant::create(); - $suffixBase = config('tenancy.filesystem.suffix_base', 'tenant'); - $tenantStoragePath = $centralStoragePath . '/' . $suffixBase . $tenant->getTenantKey(); - - $this->assertDirectoryExists($tenantStoragePath . '/framework/cache'); -}); - test('tenant storage can get deleted after the tenant when DeletingTenant listens to DeleteTenantStorage', function() { Event::listen(DeletingTenant::class, JobPipeline::make([DeleteTenantStorage::class])->send(function (DeletingTenant $event) {