Filesystem logic refactor, improved defaults for cache tenancy (#42)

* refactor FilesystemTenancyBootstrapper

* clean up tests and improve coverage

* minor maintenance mode changes

* Improve tenants:migrate --skip-failing logic

* make tenants:migrate output consistently formatted

* minor RootUrlBootstrapper + misc changes

* cache bootstrapper-related improvements

* Fix code style (php-cs-fixer)

* misc refactor

* Fix code style (php-cs-fixer)

* add %original_storage_path% to fs bootstrapper, improve default config for cache

* rename method

* inject concrete implementations where needed instead of abstracts

* Fix code style (php-cs-fixer)

* refactor DealsWithTenantSymlinks

* remove obsolete phpstan ignore

---------

Co-authored-by: PHP CS Fixer <phpcsfixer@example.com>

src/Bootstrappers/CacheTenancyBootstrapper.php

@@ -17,10 +17,10 @@ use Stancl\Tenancy\Contracts\Tenant;
  */
 class CacheTenancyBootstrapper implements TenancyBootstrapper
 {
-    protected string|null $originalPrefix = null;
-    public static array $tenantCacheStores = []; // E.g. ['redis']
     public static Closure|null $prefixGenerator = null;
 
+    protected string|null $originalPrefix = null;
+
     public function __construct(
         protected ConfigRepository $config,
         protected CacheManager $cacheManager,
@@ -33,7 +33,7 @@ class CacheTenancyBootstrapper implements TenancyBootstrapper
 
         $prefix = $this->generatePrefix($tenant);
 
-        foreach (static::$tenantCacheStores as $store) {
+        foreach ($this->config->get('tenancy.cache.stores') as $store) {
             $this->setCachePrefix($store, $prefix);
 
             // Now that the store uses the passed prefix
@@ -44,7 +44,7 @@ class CacheTenancyBootstrapper implements TenancyBootstrapper
 
     public function revert(): void
     {
-        foreach (static::$tenantCacheStores as $store) {
+        foreach ($this->config->get('tenancy.cache.stores') as $store) {
             $this->setCachePrefix($store, $this->originalPrefix);
         }
     }

src/Bootstrappers/FilesystemTenancyBootstrapper.php

@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Bootstrappers;
 
-use Illuminate\Contracts\Foundation\Application;
+use Illuminate\Foundation\Application;
 use Illuminate\Routing\UrlGenerator;
 use Illuminate\Support\Facades\Storage;
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
@@ -12,20 +12,15 @@ use Stancl\Tenancy\Contracts\Tenant;
 
 class FilesystemTenancyBootstrapper implements TenancyBootstrapper
 {
-    /** @var Application */
-    protected $app;
+    public array $originalDisks = [];
+    public string|null $originalAssetUrl;
+    public string $originalStoragePath;
 
-    /** @var array */
-    public $originalPaths = [];
-
-    public function __construct(Application $app)
-    {
-        $this->app = $app;
-        $this->originalPaths = [
-            'disks' => [],
-            'storage' => $this->app->storagePath(),
-            'asset_url' => $this->app['config']['app.asset_url'],
-        ];
+    public function __construct(
+        protected Application $app,
+    ) {
+        $this->originalAssetUrl = $this->app['config']['app.asset_url'];
+        $this->originalStoragePath = $app->storagePath();
 
         $this->app['url']->macro('setAssetRoot', function ($root) {
             /** @var UrlGenerator $this */
@@ -37,83 +32,128 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper
 
     public function bootstrap(Tenant $tenant): void
     {
-        $suffix = $this->app['config']['tenancy.filesystem.suffix_base'] . $tenant->getTenantKey();
+        $suffix = $this->suffix($tenant);
 
-        // storage_path()
-        if ($this->app['config']['tenancy.filesystem.suffix_storage_path'] ?? true) {
-            $this->app->useStoragePath($this->originalPaths['storage'] . "/{$suffix}");
-        }
-
-        // asset()
-        if ($this->app['config']['tenancy.filesystem.asset_helper_tenancy']) {
-            if ($this->originalPaths['asset_url']) {
-                $this->app['config']['app.asset_url'] = $this->originalPaths['asset_url'] . "/$suffix";
-                $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']);
-            } else {
-                $this->app['url']->setAssetRoot($this->app['url']->route('stancl.tenancy.asset', ['path' => '']));
-            }
-        }
-
-        // Storage facade
-        Storage::forgetDisk($this->app['config']['tenancy.filesystem.disks']);
+        $this->storagePath($suffix);
+        $this->assetHelper($suffix);
+        $this->forgetDisks();
 
         foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) {
-            // todo@v4 \League\Flysystem\PathPrefixer is making this a lot more painful in flysystem v2
-            $diskConfig = $this->app['config']["filesystems.disks.{$disk}"];
-            $originalRoot = $diskConfig['root'] ?? null;
+            $this->diskRoot($disk, $tenant);
 
-            $this->originalPaths['disks']['path'][$disk] = $originalRoot;
-
-            $finalPrefix = str_replace(
-                ['%storage_path%', '%tenant%'],
-                [storage_path(), $tenant->getTenantKey()],
-                $this->app['config']["tenancy.filesystem.root_override.{$disk}"] ?? '',
+            $this->diskUrl(
+                $disk,
+                str($this->app['config']["tenancy.filesystem.url_override.{$disk}"])
+                    ->replace('%tenant%', (string) $tenant->getTenantKey())
+                    ->toString(),
             );
-
-            if (! $finalPrefix) {
-                $finalPrefix = $originalRoot
-                    ? rtrim($originalRoot, '/') . '/' . $suffix
-                    : $suffix;
-            }
-
-            $this->app['config']["filesystems.disks.{$disk}.root"] = $finalPrefix;
-
-            // Storage Url
-            if ($diskConfig['driver'] === 'local') {
-                $this->originalPaths['disks']['url'][$disk] = $diskConfig['url'] ?? null;
-
-                if ($url = str_replace(
-                    '%tenant%',
-                    (string) $tenant->getTenantKey(),
-                    $this->app['config']["tenancy.filesystem.url_override.{$disk}"] ?? ''
-                )) {
-                    $this->app['config']["filesystems.disks.{$disk}.url"] = url($url);
-                }
-            }
         }
     }
 
     public function revert(): void
     {
-        // storage_path()
-        $this->app->useStoragePath($this->originalPaths['storage']);
+        $this->storagePath(false);
+        $this->assetHelper(false);
+        $this->forgetDisks();
 
-        // asset()
-        $this->app['config']['app.asset_url'] = $this->originalPaths['asset_url'];
-        $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']);
+        foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) {
+            $this->diskRoot($disk, false);
+            $this->diskUrl($disk, false);
+        }
+    }
 
-        // Storage facade
+    protected function suffix(Tenant $tenant): string
+    {
+        return $this->app['config']['tenancy.filesystem.suffix_base'] . $tenant->getTenantKey();
+    }
+
+    protected function storagePath(string|false $suffix): void
+    {
+        if ($this->app['config']['tenancy.filesystem.suffix_storage_path'] === false) {
+            return;
+        }
+
+        if ($suffix === false) {
+            $this->app->useStoragePath($this->originalStoragePath);
+        } else {
+            $this->app->useStoragePath($this->originalStoragePath . "/{$suffix}");
+        }
+    }
+
+    protected function assetHelper(string|false $suffix): void
+    {
+        if (! $this->app['config']['tenancy.filesystem.asset_helper_tenancy']) {
+            return;
+        }
+
+        if ($suffix === false) {
+            $this->app['config']['app.asset_url'] = $this->originalAssetUrl;
+            $this->app['url']->setAssetRoot($this->originalAssetUrl);
+
+            return;
+        }
+
+        if ($this->originalAssetUrl) {
+            $this->app['config']['app.asset_url'] = $this->originalAssetUrl . "/$suffix";
+            $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']);
+        } else {
+            $this->app['url']->setAssetRoot($this->app['url']->route('stancl.tenancy.asset', ['path' => '']));
+        }
+    }
+
+    protected function forgetDisks(): void
+    {
         Storage::forgetDisk($this->app['config']['tenancy.filesystem.disks']);
-        foreach ($this->app['config']['tenancy.filesystem.disks'] as $diskName) {
-            $this->app['config']["filesystems.disks.$diskName.root"] = $this->originalPaths['disks']['path'][$diskName];
-            $diskConfig = $this->app['config']['filesystems.disks.' . $diskName];
+    }
 
-            // Storage Url
-            $url = data_get($this->originalPaths, "disks.url.$diskName");
+    protected function diskRoot(string $disk, Tenant|false $tenant): void
+    {
+        if ($tenant === false) {
+            $this->app['config']["filesystems.disks.$disk.root"] = $this->originalDisks[$disk]['root'];
 
-            if ($diskConfig['driver'] === 'local' && ! is_null($url)) {
-                $this->app['config']["filesystems.disks.$diskName.url"] = $url;
-            }
+            return;
+        }
+
+        $suffix = $this->suffix($tenant);
+
+        $diskConfig = $this->app['config']["filesystems.disks.{$disk}"];
+        $originalRoot = $diskConfig['root'] ?? null;
+
+        $this->originalDisks[$disk]['root'] = $originalRoot;
+
+        if ($override = $this->app['config']["tenancy.filesystem.root_override.{$disk}"]) {
+            // This is executed if the disk is in tenancy.filesystem.disks AND has a root_override
+            // This behavior is used for local disks.
+            $newRoot = str($override)
+                ->replace('%storage_path%', storage_path())
+                ->replace('%original_storage_path%', $this->originalStoragePath)
+                ->replace('%tenant%', (string) $tenant->getTenantKey())
+                ->toString();
+        } else {
+            // This is executed if the disk is in tenancy.filesystem.disks but does NOT have a root_override
+            // This behavior is used for disks like S3.
+            $newRoot = $originalRoot
+                ? rtrim($originalRoot, '/') . '/' . $suffix
+                : $suffix;
+        }
+
+        $this->app['config']["filesystems.disks.{$disk}.root"] = $newRoot;
+    }
+
+    protected function diskUrl(string $disk, string|false $override): void
+    {
+        $diskConfig = $this->app['config']["filesystems.disks.{$disk}"];
+
+        if ($diskConfig['driver'] !== 'local' || $this->app['config']["tenancy.filesystem.url_override.{$disk}"] === null) {
+            return;
+        }
+
+        if ($override === false) {
+            $url = data_get($this->originalDisks, "$disk.url");
+            $this->app['config']["filesystems.disks.$disk.url"] = $url;
+        } else {
+            $this->originalDisks[$disk]['url'] ??= $diskConfig['url'] ?? null;
+            $this->app['config']["filesystems.disks.{$disk}.url"] = url($override);
         }
     }
 }

src/Bootstrappers/RootUrlBootstrapper.php

@@ -6,7 +6,7 @@ namespace Stancl\Tenancy\Bootstrappers;
 
 use Closure;
 use Illuminate\Config\Repository;
-use Illuminate\Contracts\Routing\UrlGenerator;
+use Illuminate\Routing\UrlGenerator;
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
 use Stancl\Tenancy\Contracts\Tenant;
 
@@ -26,7 +26,7 @@ class RootUrlBootstrapper implements TenancyBootstrapper
         $this->originalRootUrl = $this->urlGenerator->to('/');
 
         if (static::$rootUrlOverride) {
-            $newRootUrl = (static::$rootUrlOverride)($tenant);
+            $newRootUrl = (static::$rootUrlOverride)($tenant, $this->originalRootUrl);
 
             $this->urlGenerator->forceRootUrl($newRootUrl);
             $this->config->set('app.url', $newRootUrl);

src/Commands/Migrate.php

@@ -30,7 +30,7 @@ class Migrate extends MigrateCommand
     {
         parent::__construct($migrator, $dispatcher);
 
-        $this->addOption('skip-failing');
+        $this->addOption('skip-failing', description: 'Continue execution if migration fails for a tenant');
 
         $this->specifyParameters();
     }
@@ -49,19 +49,21 @@ class Migrate extends MigrateCommand
 
         foreach ($this->getTenants() as $tenant) {
             try {
-                $tenant->run(function ($tenant) {
-                    $this->line("Tenant: {$tenant->getTenantKey()}");
+                $this->components->info("Migrating tenant {$tenant->getTenantKey()}");
 
+                $tenant->run(function ($tenant) {
                     event(new MigratingDatabase($tenant));
                     // Migrate
                     parent::handle();
 
                     event(new DatabaseMigrated($tenant));
                 });
-            } catch (TenantDatabaseDoesNotExistException|QueryException $th) {
+            } catch (TenantDatabaseDoesNotExistException|QueryException $e) {
                 if (! $this->option('skip-failing')) {
-                    throw $th;
+                    throw $e;
                 }
+
+                $this->components->warn("Migration failed for tenant {$tenant->getTenantKey()}: {$e->getMessage()}");
             }
         }
 

src/Concerns/DealsWithTenantSymlinks.php

@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Concerns;
 
-use Illuminate\Support\Collection;
+use Exception;
 use Stancl\Tenancy\Contracts\Tenant;
 
 trait DealsWithTenantSymlinks
@@ -13,31 +13,44 @@ trait DealsWithTenantSymlinks
      * Get all possible tenant symlinks, existing or not (array of ['public path' => 'storage path']).
      *
      * Tenants can have a symlink for each disk registered in the tenancy.filesystem.url_override config.
-     *
      * This is used for creating all possible tenant symlinks and removing all existing tenant symlinks.
+     * The same storage path can be symlinked to multiple public paths, which is why the public path
+     * is the Collection key.
      *
-     * @return Collection<string, string>
+     * @return array<string, string>
      */
-    protected function possibleTenantSymlinks(Tenant $tenant): Collection
+    protected function possibleTenantSymlinks(Tenant $tenant): array
     {
-        $diskUrls = config('tenancy.filesystem.url_override');
-        $disks = config('tenancy.filesystem.root_override');
-        $suffixBase = config('tenancy.filesystem.suffix_base');
+        $disks = config('filesystems.disks');
+        $urlOverrides = config('tenancy.filesystem.url_override');
+        $rootOverrides = config('tenancy.filesystem.root_override');
+
         $tenantKey = $tenant->getTenantKey();
+        $tenantStoragePath = tenancy()->run($tenant, fn () => storage_path());
 
-        /** @var Collection<array<string, string>> $symlinks */
-        $symlinks = collect([]);
+        /** @var array<string, string> $symlinks */
+        $symlinks = [];
+
+        foreach ($urlOverrides as $disk => $publicPath) {
+            if (! isset($disks[$disk])) {
+                continue;
+            }
+
+            if (! isset($rootOverrides[$disk])) {
+                continue;
+            }
+
+            if ($disks[$disk]['driver'] !== 'local') {
+                throw new Exception("Disk $disk is not a local disk. Only local disks can be symlinked.");
+            }
 
-        foreach ($diskUrls as $disk => $publicPath) {
-            $storagePath = str_replace('%storage_path%', $suffixBase . $tenantKey, $disks[$disk]);
             $publicPath = str_replace('%tenant%', (string) $tenantKey, $publicPath);
+            $storagePath = str_replace('%storage_path%', $tenantStoragePath, $rootOverrides[$disk]);
 
-            tenancy()->central(function () use ($symlinks, $publicPath, $storagePath) {
-                $symlinks->push([public_path($publicPath) => storage_path($storagePath)]);
-            });
+            $symlinks[public_path($publicPath)] = $storagePath;
         }
 
-        return $symlinks->mapWithKeys(fn ($item) => $item); // [[a => b], [c => d]] -> [a => b, c => d]
+        return $symlinks;
     }
 
     /** Determine if the provided path is an existing symlink. */

src/Concerns/HasTenantOptions.php

@@ -16,8 +16,8 @@ trait HasTenantOptions
     protected function getOptions()
     {
         return array_merge([
-            ['tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, '', null],
-            ['with-pending', null, InputOption::VALUE_NONE, 'include pending tenants in query'],
+            ['tenants', null, InputOption::VALUE_IS_ARRAY|InputOption::VALUE_OPTIONAL, 'The tenants to run this command for. Leave empty for all tenants', null],
+            ['with-pending', null, InputOption::VALUE_NONE, 'Include pending tenants in query'],
         ], parent::getOptions());
     }
 

src/Middleware/InitializeTenancyByDomainOrSubdomain.php

@@ -27,7 +27,7 @@ class InitializeTenancyByDomainOrSubdomain extends InitializeTenancyBySubdomain
         if ($this->isSubdomain($domain)) {
             $domain = $this->makeSubdomain($domain);
 
-            if (is_object($domain) && $domain instanceof Exception) {
+            if ($domain instanceof Exception) {
                 $onFail = static::$onFail ?? function ($e) {
                     throw $e;
                 };
@@ -36,7 +36,7 @@ class InitializeTenancyByDomainOrSubdomain extends InitializeTenancyBySubdomain
             }
 
             // If a Response instance was returned, we return it immediately.
-            if (is_object($domain) && $domain instanceof Response) {
+            if ($domain instanceof Response) {
                 return $domain;
             }
         }

src/Middleware/InitializeTenancyBySubdomain.php

@@ -37,7 +37,7 @@ class InitializeTenancyBySubdomain extends InitializeTenancyByDomain
 
         $subdomain = $this->makeSubdomain($this->getDomain($request));
 
-        if (is_object($subdomain) && $subdomain instanceof Exception) {
+        if ($subdomain instanceof Exception) {
             $onFail = static::$onFail ?? function ($e) {
                 throw $e;
             };
@@ -46,7 +46,7 @@ class InitializeTenancyBySubdomain extends InitializeTenancyByDomain
         }
 
         // If a Response instance was returned, we return it immediately.
-        if (is_object($subdomain) && $subdomain instanceof Response) {
+        if ($subdomain instanceof Response) {
             return $subdomain;
         }
 

src/Tenancy.php

@@ -31,7 +31,7 @@ class Tenancy
     /** Initialize tenancy for the passed tenant. */
     public function initialize(Tenant|int|string $tenant): void
     {
-        if (! is_object($tenant)) {
+        if (! $tenant instanceof Tenant) {
             $tenantId = $tenant;
             $tenant = $this->find($tenantId);
 

src/TenancyServiceProvider.php

@@ -130,7 +130,7 @@ class TenancyServiceProvider extends ServiceProvider
         $this->app->singleton('globalUrl', function ($app) {
             if ($app->bound(FilesystemTenancyBootstrapper::class)) {
                 $instance = clone $app['url'];
-                $instance->setAssetRoot($app[FilesystemTenancyBootstrapper::class]->originalPaths['asset_url']);
+                $instance->setAssetRoot($app[FilesystemTenancyBootstrapper::class]->originalAssetUrl);
             } else {
                 $instance = $app['url'];
             }