[MINOR BC] [4.x] Make storage features not depend on the suffix_storage_path config

assets/config.php

@@ -321,11 +321,12 @@
 
     /**
      * Filesystem tenancy config. Used by FilesystemTenancyBootstrapper.
-     * https://tenancyforlaravel.com/docs/v3/tenancy-bootstrappers/#filesystem-tenancy-boostrapper.
+     * https://v4.tenancyforlaravel.com/bootstrappers/filesystem
      */
     'filesystem' => [
         /**
-         * Each disk listed in the 'disks' array will be suffixed by the suffix_base, followed by the tenant_id.
+         * Each disk listed in the 'disks' array will have its root
+         * suffixed by the suffix_base, followed by the tenant_id.
          */
         'suffix_base' => 'tenant',
         'disks' => [
@@ -337,10 +338,18 @@
         /**
          * Use this for local disks.
          *
-         * See https://tenancyforlaravel.com/docs/v3/tenancy-bootstrappers/#filesystem-tenancy-boostrapper
+         * Customizes how the disk's root is scoped, instead of the default behavior
+         * which simply appends the tenant suffix to the original root.
+         *
+         * The overrides can reference the following placeholders:
+         * - %storage_path% -- the tenant's storage directory
+         * - %original_storage_path% -- the central storage directory.
+         * - %tenant% -- the tenant key.
+         *
+         * See https://v4.tenancyforlaravel.com/bootstrappers/filesystem
          */
         'root_override' => [
-            // Disks whose roots should be overridden after storage_path() is suffixed.
+            // Disks whose roots should be overridden in tenant context.
             'local' => '%storage_path%/app/',
             'public' => '%storage_path%/app/public/',
         ],
@@ -357,7 +366,8 @@
          * Use `php artisan tenants:link` to create a symbolic link from the tenant's storage to its public directory.
          */
         'url_override' => [
-            // Note that the local disk you add must exist in the tenancy.filesystem.root_override config
+            // Note that the local disk you add must exist in the tenancy.filesystem.disks config,
+            // and it must have a non-falsy root (not null nor an empty string).
             'public' => 'public-%tenant%',
         ],
 
@@ -378,11 +388,7 @@
         /**
          * Should storage_path() be suffixed.
          *
-         * Note: Disabling this will likely break local disk tenancy. Only disable this if you're using an external file storage service like S3.
-         *
-         * For the vast majority of applications, this feature should be enabled. But in some
-         * edge cases, it can cause issues (like using Passport with Vapor - see #196), so
-         * you may want to disable this if you are experiencing these edge case issues.
+         * Only affects the storage_path() helper, other features use the tenant storage directory regardless.
          */
         'suffix_storage_path' => true,
 

src/Actions/CreateStorageSymlinksAction.php

@@ -47,6 +47,12 @@ protected function createLink(string $publicPath, string $storagePath, Tenant $t
             mkdir($storagePath, 0777, true);
         }
 
+        // The public path of a prefixed disk includes the prefix,
+        // and its parent directories may not exist yet.
+        if (! is_dir($publicParent = dirname($publicPath))) {
+            mkdir($publicParent, 0777, true);
+        }
+
         if ($relativeLink) {
             app()->make('files')->relativeLink($storagePath, $publicPath);
         } else {

src/Actions/RemoveStorageSymlinksAction.php

@@ -15,6 +15,16 @@ class RemoveStorageSymlinksAction
 {
     use DealsWithTenantSymlinks;
 
+    /**
+     * Should the directories created for nested symlinks be removed along with the symlink.
+     *
+     * Before enabling this, make sure you understand the removeLink() method
+     * and the high stakes of recursively removing parent directories (even if the logic should be sound).
+     *
+     * @see CreateStorageSymlinksAction
+     */
+    public static bool $removeNestedDirectories = false;
+
     /**
      * @param Tenant|Collection<covariant int|string, Tenant&\Illuminate\Database\Eloquent\Model>|LazyCollection<covariant int|string, Tenant&\Illuminate\Database\Eloquent\Model> $tenants
      */
@@ -32,12 +42,38 @@ public function __invoke(Tenant|Collection|LazyCollection $tenants): void
 
     protected function removeLink(string $publicPath, Tenant $tenant): void
     {
-        if ($this->symlinkExists($publicPath)) {
-            event(new RemovingStorageSymlink($tenant));
+        if (! $this->symlinkExists($publicPath)) {
+            return;
+        }
+
+        $files = app()->make('files');
+
+        event(new RemovingStorageSymlink($tenant));
+
+        $files->delete($publicPath);
 
-            app()->make('files')->delete($publicPath);
+        event(new StorageSymlinkRemoved($tenant));
+
+        if (! static::$removeNestedDirectories) {
+            return;
+        }
+
+        $publicRoot = realpath(public_path());
+        $directory = realpath(dirname($publicPath));
+
+        if ($publicRoot === false || $directory === false) {
+            return;
+        }
+
+        // Remove the directories CreateStorageSymlinksAction created for the symlink
+        // until a non-empty one is reached.
+        while (str_starts_with($directory, rtrim($publicRoot, '/\\') . DIRECTORY_SEPARATOR) && $files->isEmptyDirectory($directory)) {
+            if (! @rmdir($directory)) {
+                // Stop the loop if the directory couldn't be removed
+                break;
+            }
 
-            event(new StorageSymlinkRemoved($tenant));
+            $directory = dirname($directory);
         }
     }
 }

src/Bootstrappers/FilesystemTenancyBootstrapper.php

@@ -128,10 +128,14 @@ protected function forgetDisks(): void
         $scopedDisks = [];
 
         foreach ($this->app['config']['filesystems.disks'] as $name => $disk) {
-            if (isset($disk['driver'], $disk['disk'])
-                && $disk['driver'] === 'scoped'
-                && in_array($disk['disk'], $tenantDisks, true)) {
+            if (($disk['driver'] ?? null) !== 'scoped') {
+                continue;
+            }
+
+            if (in_array(static::baseDiskName($name), $tenantDisks, true)) {
                 $scopedDisks[] = $name;
+            } elseif (in_array($name, $tenantDisks, true)) {
+                throw new Exception("Disk [$name] uses the 'scoped' driver, so it has no root to make tenant-aware. List its base disk in tenancy.filesystem.disks instead.");
             }
         }
 
@@ -140,6 +144,12 @@ protected function forgetDisks(): void
 
     protected function diskRoot(string $disk, Tenant|false $tenant): void
     {
+        if ($this->app['config']["filesystems.disks.$disk.driver"] === 'scoped') {
+            // Skip scoped disks since they have no root to override
+            // (reachable when a scoped disk is listed in tenancy.filesystem.disks alongside its base disk).
+            return;
+        }
+
         if ($tenant === false) {
             $this->app['config']["filesystems.disks.$disk.root"] = $this->originalDisks[$disk]['root'];
 
@@ -176,7 +186,7 @@ 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) {
+        if ($diskConfig['driver'] !== 'local' || ! $this->app['config']["tenancy.filesystem.url_override.{$disk}"]) {
             return;
         }
 
@@ -327,4 +337,39 @@ public static function getBoundCentralStoragePath(): string
     {
         return app(static::class)->originalStoragePath;
     }
+
+    /**
+     * Get the storage path of the passed tenant (independent of the current context).
+     *
+     * Note that the returned path doesn't depend on suffix_storage_path.
+     * That config option only affects the storage_path() helper.
+     */
+    public static function getTenantStoragePath(Tenant $tenant): string
+    {
+        $bootstrapper = app(static::class);
+
+        return $bootstrapper->tenantStoragePath($bootstrapper->suffix($tenant));
+    }
+
+    /**
+     * Name of the disk whose root the passed disk uses.
+     *
+     * Disks using the 'scoped' driver have no root or url of their own -- they inherit those from their parent disk,
+     * which can be scoped as well, so only the final/base parent has to be tenant-aware.
+     *
+     * Returns null if the base disk has no name, i.e. when the disk is configured inline as an array.
+     */
+    public static function baseDiskName(string $disk): string|null
+    {
+        while (config("filesystems.disks.$disk.driver") === 'scoped') {
+            if (! is_string($parent = config("filesystems.disks.$disk.disk"))) {
+                // Laravel allows configuring the parent disk inline as an array, and such a disk has no name
+                return null;
+            }
+
+            $disk = $parent;
+        }
+
+        return $disk;
+    }
 }

src/Bootstrappers/LogChannelBootstrapper.php

@@ -21,11 +21,8 @@
  * Laravel's 'single' and 'daily' channels by default. To customize it,
  * see the property's docblock.
  *
- * For the storage path channels to be scoped correctly:
- * - this bootstrapper must run *after* FilesystemTenancyBootstrapper,
- *   since FilesystemTenancyBootstrapper adjusts storage_path() for the tenant
- * - storage path suffixing has to be enabled (= config('tenancy.filesystem.suffix_storage_path')
- *   must be true), since the storage path suffix is what separates filesystem-based logs
+ * Note that since the tenant's storage path is resolved using FilesystemTenancyBootstrapper::getTenantStoragePath(),
+ * which is a public static method, FilesystemTenancyBootstrapper does not have to be enabled.
  *
  * For logging channels that are not filesystem-based, see the $channelOverrides logic.
  *
@@ -40,14 +37,10 @@ class LogChannelBootstrapper implements TenancyBootstrapper
     /**
      * Logging channels whose path is built using storage_path() (e.g. Laravel's 'single' and 'daily').
      *
-     * Channels included here will be configured to use tenant-specific storage paths
-     * created using storage_path() in the tenant context. Overrides in the $channelOverrides
-     * property take precedence over $storagePathChannels when a channel is included in both.
+     * Channels included here will be configured to use tenant-specific storage paths.
      *
-     * Requires FilesystemTenancyBootstrapper to run before this bootstrapper,
-     * and storage path suffixing to be enabled.
-     *
-     * @see Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper
+     * Overrides in the $channelOverrides property take precedence over
+     * $storagePathChannels when a channel is included in both.
      */
     public static array $storagePathChannels = ['single', 'daily'];
 
@@ -158,11 +151,12 @@ protected function configureChannels(array $channels, Tenant $tenant): void
                 // The tenant log will be located at e.g. "storage/tenant{$tenantKey}/logs/laravel.log".
                 $originalChannelPath = $this->config->get("logging.channels.{$channel}.path");
                 $centralStoragePath = FilesystemTenancyBootstrapper::getBoundCentralStoragePath();
+                $tenantStoragePath = FilesystemTenancyBootstrapper::getTenantStoragePath($tenant);
 
                 // The tenant log will inherit the segment that follows the storage path from the central channel path config.
                 // For example, if a channel's path is configured to storage_path('logs/foo.log') (storage/logs/foo.log),
-                // the 'logs/foo.log' segment will be passed to storage_path() in the tenant context (storage/tenant123/logs/foo.log).
-                $this->config->set("logging.channels.{$channel}.path", storage_path(Str::after($originalChannelPath, $centralStoragePath)));
+                // the '/logs/foo.log' segment will be placed within the tenant storage path (so the log will be located at storage/tenant123/logs/foo.log).
+                $this->config->set("logging.channels.{$channel}.path", $tenantStoragePath . Str::after($originalChannelPath, rtrim($centralStoragePath, '/\\')));
             }
         }
     }

src/Concerns/DealsWithTenantSymlinks.php

@@ -7,47 +7,69 @@
 use Exception;
 use Stancl\Tenancy\Contracts\Tenant;
 
+/**
+ * Requires FilesystemTenancyBootstrapper to be enabled, since the tenant symlinks
+ * point to the disk roots scoped by the bootstrapper.
+ *
+ * @see \Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper
+ */
 trait DealsWithTenantSymlinks
 {
     /**
-     * Get all possible tenant symlinks, existing or not (array of ['public path' => 'storage path']).
+     * Get all possible tenant symlinks, existing or not (array of ['public path' => 'disk root with the disk's prefix appended']).
+     *
+     * Tenants can have a symlink for each local disk that is listed
+     * in both tenancy.filesystem.disks and tenancy.filesystem.url_override.
      *
-     * 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.
+     * The same disk root can be symlinked to multiple public paths, which is why the public path
+     * is the array key.
      *
      * @return array<string, string>
      */
     protected function possibleTenantSymlinks(Tenant $tenant): array
     {
         $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());
+        $tenantDisks = tenancy()->run($tenant, fn () => config('filesystems.disks'));
 
         /** @var array<string, string> $symlinks */
         $symlinks = [];
 
         foreach ($urlOverrides as $disk => $publicPath) {
-            if (! isset($disks[$disk])) {
+            if (! $publicPath) {
                 continue;
             }
 
-            if (! isset($rootOverrides[$disk])) {
+            if (! isset($disks[$disk])) {
                 continue;
             }
 
             if ($disks[$disk]['driver'] !== 'local') {
                 throw new Exception("Disk $disk is not a local disk. Only local disks can be symlinked.");
             }
 
+            if (! in_array($disk, config('tenancy.filesystem.disks'), true)) {
+                // The bootstrapper only scopes disks listed in tenancy.filesystem.disks.
+                // Without that, the root stays central and the symlink of every tenant would point to it.
+                throw new Exception("Disk $disk is not tenant-aware. Add it to the tenancy.filesystem.disks config to make its root tenant-specific.");
+            }
+
             $publicPath = str_replace('%tenant%', (string) $tenantKey, $publicPath);
-            $storagePath = str_replace('%storage_path%', $tenantStoragePath, $rootOverrides[$disk]);
+            $diskRoot = $tenantDisks[$disk]['root'];
+
+            if ($prefix = trim($disks[$disk]['prefix'] ?? '', '/\\')) {
+                // Append the disk's prefix to the disk root
+                $diskRoot = rtrim($diskRoot, '/\\') . DIRECTORY_SEPARATOR . $prefix;
+
+                // Append the same prefix to the public path.
+                // Storage::url() appends the disk's prefix to the url, so the prefix has to be in the public path as well.
+                $publicPath .= DIRECTORY_SEPARATOR . $prefix;
+            }
 
-            $symlinks[public_path($publicPath)] = $storagePath;
+            $symlinks[public_path($publicPath)] = $diskRoot;
         }
 
         return $symlinks;