From 197ca18566161d251d0144202f7e994fab397f30 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 7 Sep 2026 12:32:24 +0200 Subject: [PATCH] Simplify baseDiskName() Refrain from dealing with the impossible "self-referencing" scoped disk case. Instead of that, test the inline parent behavior. Also update the exception message in forgetDisks() so that it's a bit less vague. --- .../FilesystemTenancyBootstrapper.php | 24 +++------ .../FilesystemTenancyBootstrapperTest.php | 53 +++++-------------- 2 files changed, 19 insertions(+), 58 deletions(-) diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index eb9138c9..0a4f9992 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -132,12 +132,10 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper continue; } - $baseDisk = static::baseDiskName($name); - - if (in_array($baseDisk, $tenantDisks, true)) { + if (in_array(static::baseDiskName($name), $tenantDisks, true)) { $scopedDisks[] = $name; } elseif (in_array($name, $tenantDisks, true)) { - throw new Exception("A disk using the 'scoped' driver cannot be tenant-aware. List its base disk in tenancy.filesystem.disks instead."); + 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."); } } @@ -359,27 +357,17 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper * 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 chain doesn't end with a named disk, i.e. when a parent disk is - * configured inline or when the disks reference each other. + * 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 { - // Keep track of visited disks to avoid infinite loops in case of disks referencing each other - $visited = []; - while (config("filesystems.disks.$disk.driver") === 'scoped') { - if (in_array($disk, $visited, true)) { - // The disk and its parents reference each other, invalid - return null; - } - - $visited[] = $disk; - $disk = config("filesystems.disks.$disk.disk"); - - if (! is_string($disk)) { + 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; diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index b7f91907..26307674 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -341,35 +341,33 @@ test('adding a scoped disk to tenancy.filesystem.disks throws an exception if it 'disk' => 'foo', 'prefix' => 'bar', ], - // Disks referencing each other (neither has a base disk) - 'filesystems.disks.abc' => [ + // Scoped disk with an inline parent + 'filesystems.disks.inline_parent' => [ 'driver' => 'scoped', - 'disk' => 'def', - 'prefix' => 'abc', - ], - 'filesystems.disks.def' => [ - 'driver' => 'scoped', - 'disk' => 'abc', - 'prefix' => 'def', + 'disk' => [ + 'driver' => 'local', + 'root' => storage_path('app/inline'), + ], + 'prefix' => 'inline_parent', ], 'tenancy.filesystem.disks' => [$disk], ]); expect(fn () => tenancy()->initialize(Tenant::create())) - ->toThrow(Exception::class, "List its base disk in tenancy.filesystem.disks instead"); + ->toThrow(Exception::class, "Disk [$disk] uses the 'scoped' driver, so it has no root to make tenant-aware."); - // Parent of 'abc' is 'def', whose parent is 'abc' -- there's no base disk for these, so these are - // still invalid and the exception will still be thrown. - if ($disk !== 'abc') { + // 'inline_parent' has no base disk name to list, so there's no way to make it tenant-aware + // and the exception is thrown regardless of what's listed. + if ($disk !== 'inline_parent') { config(['tenancy.filesystem.disks' => ['public', $disk]]); expect(fn () => tenancy()->initialize(Tenant::create())) - ->not()->toThrow(Exception::class, "List its base disk in tenancy.filesystem.disks instead"); + ->not()->toThrow(Exception::class, "Disk [$disk] uses the 'scoped' driver, so it has no root to make tenant-aware."); } })->with([ 'scoped disk' => 'foo', 'nested scoped disk' => 'bar', - 'scoped disk with no base disk' => 'abc', + 'scoped disk with an inline base disk' => 'inline_parent', ]); test('adding a scoped disk to tenancy.filesystem.disks has no effect on the disk when its base disk is listed too', function () { @@ -401,31 +399,6 @@ test('adding a scoped disk to tenancy.filesystem.disks has no effect on the disk ]); }); -test('scoped disks referencing each other do not make bootstrapper hang', function () { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - ], - 'filesystems.disks.foo' => [ - 'driver' => 'scoped', - 'disk' => 'bar', - 'prefix' => 'foo', - ], - 'filesystems.disks.bar' => [ - 'driver' => 'scoped', - 'disk' => 'foo', - 'prefix' => 'bar', - ], - ]); - - expect(FilesystemTenancyBootstrapper::baseDiskName('foo'))->toBeNull(); - expect(FilesystemTenancyBootstrapper::baseDiskName('bar'))->toBeNull(); - - tenancy()->initialize(Tenant::create()); - - expect(tenant())->not()->toBeNull(); -}); - test('file cache stores get their paths scoped on bootstrap and restored back on revert', function () { $fooPath = storage_path('framework/cache/foo_file'); $barPath = storage_path('framework/cache/bar_file');