diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 0a864fbd..78204fd9 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -14,6 +14,8 @@ use Stancl\Tenancy\Contracts\Tenant; class FilesystemTenancyBootstrapper implements TenancyBootstrapper { public array $originalDisks = []; + protected array $originalCachePaths = []; + protected array $originalCacheLockPaths = []; public string|null $originalAssetUrl; public string $originalStoragePath; @@ -191,32 +193,63 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper return; } - $storagePath = $suffix - ? $this->tenantStoragePath($suffix) - : $this->originalStoragePath; - - $stores = array_filter($this->app['config']['tenancy.cache.stores'], function ($name) { - $store = $this->app['config']["cache.stores.{$name}"]; - - if ($store === null) { - return false; - } - - return $store['driver'] === 'file'; - }); + // On revert, restore exactly the stores captured during bootstrap -- not the current + // (possibly mutated) tenancy.cache.stores. Otherwise, removing a store from that + // config in tenant context would make revert() skip it (so the store would be stuck with a tenant-scoped path). + $stores = $suffix !== false + ? $this->app['config']['tenancy.cache.stores'] + : array_keys($this->originalCachePaths); foreach ($stores as $name) { - $path = $storagePath . '/framework/cache/data'; + $store = $this->app['config']["cache.stores.{$name}"]; + + // Only file stores have a path to scope. Skip stores that don't exist (null) or use another driver. + if ($store === null || $store['driver'] !== 'file') { + continue; + } + + if ($suffix !== false && ! isset($this->originalCachePaths[$name])) { + $this->originalCachePaths[$name] = $store['path']; + $this->originalCacheLockPaths[$name] = $store['lock_path'] ?? null; + } + + $path = $suffix ? $this->tenantCachePath($this->originalCachePaths[$name], $suffix) : $this->originalCachePaths[$name]; + + // Unlike path, lock_path is optional -- if it's not set, FileStore::lock() falls back to path + // itself (see `$this->lockDirectory ?? $this->directory` in FileStore). Leave it null here rather + // than hardcoding it to $path ourselves, so a store that didn't configure a separate lock_path + // doesn't end up with one. + $lockPath = $this->originalCacheLockPaths[$name]; + if ($suffix && $lockPath !== null) { + $lockPath = $this->tenantCachePath($lockPath, $suffix); + } + $this->app['config']["cache.stores.{$name}.path"] = $path; - $this->app['config']["cache.stores.{$name}.lock_path"] = $path; + $this->app['config']["cache.stores.{$name}.lock_path"] = $lockPath; /** @var \Illuminate\Cache\FileStore $store */ $store = $this->app['cache']->store($name)->getStore(); $store->setDirectory($path); - $store->setLockDirectory($path); + $store->setLockDirectory($lockPath); } } + /** Scope a configured cache path (path or lock_path) to the tenant identified by $suffix. */ + protected function tenantCachePath(string $configuredPath, string $suffix): string + { + if (str_starts_with($configuredPath, $this->originalStoragePath . '/')) { + // Swap the central storage path prefix for the tenant's. + // For example, storage_path('framework/cache/data') becomes storage_path('tenant1/framework/cache/data'). + return str($configuredPath) + ->after($this->originalStoragePath . '/') + ->prepend($this->tenantStoragePath($suffix) . '/') + ->toString(); + } + + // Otherwise $configuredPath isn't necessarily storage_path()-based, so just append the suffix directly. + return rtrim($configuredPath, '/') . '/' . $suffix; + } + public function scopeSessions(string|false $suffix): void { if (! $this->app['config']['tenancy.filesystem.scope_sessions']) { diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 4e834917..39d5cef7 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -1,6 +1,7 @@ toBe('central2'); expect(file_get_contents(storage_path() . "/tenant{$tenant->id}/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); }); + +test('file cache stores are separated per tenant', function () { + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + // Only stores that use the 'file' driver are scoped. + // The 'redis' store won't be scoped since it doesn't use the 'file' driver, + // and 'nonexistent_store' won't be scoped since it just doesn't exist. + 'tenancy.cache.stores' => ['file', 'redis', 'nonexistent_store'], + // Laravel's default 'file' store config (set explicitly here for clarity) + 'cache.stores.file' => [ + 'driver' => 'file', + 'path' => storage_path('framework/cache/data'), + 'lock_path' => storage_path('framework/cache/data'), + ], + ]); + + $tenant1 = Tenant::create(); + $tenant2 = Tenant::create(); + + Cache::store('file')->put('key', 'central'); + Cache::store('redis')->put('key', 'central'); + + // 'redis' and 'nonexistent_store' are skipped by the driver check in scopeCache(), before it reads their path. + // Without the skipping logic, the `$this->originalCachePaths[$name] = $store['path']` + // line in scopeCache() would throw an ErrorException (with 'redis', we'd get an + // 'Undefined array key "path"' exception, and with 'nonexistent_store', + // we'd get 'Trying to access array offset on null'). + expect(fn () => tenancy()->initialize($tenant1))->not()->toThrow(ErrorException::class); + + expect(Cache::store('file')->get('key'))->toBeNull(); + Cache::store('file')->put('key', 'tenant1'); + + // The 'redis' store doesn't use the file driver (no path to scope), + // so FilesystemTenancyBootstrapper skips it in scopeCache(). + // The central value is retained in the tenant context. + expect(Cache::store('redis')->get('key'))->toBe('central'); + + tenancy()->initialize($tenant2); + + expect(Cache::store('file')->get('key'))->toBeNull(); + Cache::store('file')->put('key', 'tenant2'); + + tenancy()->initialize($tenant1); + + expect(Cache::store('file')->get('key'))->toBe('tenant1'); + + tenancy()->end(); + + expect(Cache::store('file')->get('key'))->toBe('central'); + + // Turn FilesystemTenancyBootstrapper's cache scoping off + config(['tenancy.filesystem.scope_cache' => false]); + + tenancy()->initialize($tenant1); + + expect(Cache::store('file')->get('key'))->toBe('central'); + Cache::store('file')->put('key', 'written in tenant context'); + + tenancy()->end(); + + expect(Cache::store('file')->get('key'))->toBe('written in tenant context'); +}); + +test('file cache stores get their path scoped on bootstrap and restored back on revert', function () { + $fooPath = storage_path('framework/cache/foo_file'); + $barPath = storage_path('framework/cache/bar_file'); + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); + + // Use separate 'foo_file'/'bar_file' stores rather than reconfiguring 'file'. + // TestCase::setUp() calls `cache:clear file`, which resolves the 'file' store and leaves it + // in the CacheManager with the default path. A later config() call can't mutate that, + // so in central context, cache would be written to 'storage/framework/cache/data' no matter how the config changes. + // The same applies to the other tests below that configure separate cache stores rather than using 'file'. + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file', 'bar_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + 'cache.stores.bar_file' => [ + 'driver' => 'file', + 'path' => $barPath, + ], + ]); + + Cache::store('foo_file')->put('key', 'central foo'); + Cache::store('bar_file')->put('key', 'central bar'); + + tenancy()->initialize(Tenant::create()); + + Cache::store('foo_file')->put('key', 'tenant foo'); + Cache::store('bar_file')->put('key', 'tenant bar'); + + // Each store uses its own scoped path. + // The stores don't read or overwrite each other's entries. + expect(Cache::store('foo_file')->get('key'))->toBe('tenant foo'); + expect(Cache::store('bar_file')->get('key'))->toBe('tenant bar'); + + Cache::store('bar_file')->flush(); + + // Only bar_file was flushed + expect(Cache::store('bar_file')->get('key'))->toBeNull(); + expect(Cache::store('foo_file')->get('key'))->toBe('tenant foo'); + + tenancy()->end(); + + // revert() points each store back at its original configured path + expect(Cache::store('foo_file')->get('key'))->toBe('central foo'); + expect(Cache::store('bar_file')->get('key'))->toBe('central bar'); + + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); +}); + +test('scopeCache ignores changes to tenancy.cache.stores made in tenant context', function () { + $fooPath = storage_path('framework/cache/foo_file'); + $barPath = storage_path('framework/cache/bar_file'); + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $fooPath, + ], + 'cache.stores.bar_file' => [ + 'driver' => 'file', + 'path' => $barPath, + ], + // Only foo_file is scoped during bootstrap() + 'tenancy.cache.stores' => ['foo_file'], + ]); + + Cache::store('foo_file')->put('key', 'central'); + Cache::store('bar_file')->put('key', 'central'); + + tenancy()->initialize(Tenant::create()); + + Cache::store('foo_file')->put('key', 'tenant'); + + // Mutate tenancy.cache.stores in tenant context (remove 'foo_file', add 'bar_file') + config(['tenancy.cache.stores' => ['bar_file']]); + // 'bar_file' wasn't in tenancy.cache.stores during bootstrap. + // No original path is captured for that store, so it continues to use its central path. + expect(Cache::store('bar_file')->get('key'))->toBe('central'); + Cache::store('bar_file')->put('key', 'written in tenant context'); + + tenancy()->end(); + + // revert() restores the paths for the stores that were scoped during bootstrap + expect(Cache::store('foo_file')->get('key'))->toBe('central'); + // 'bar_file' was never scoped, it still reads from the same path it was writing to in tenant context + expect(Cache::store('bar_file')->get('key'))->toBe('written in tenant context'); + + File::deleteDirectory($fooPath); + File::deleteDirectory($barPath); +}); + +test('a configured lock_path is scoped separately from path', function () { + // A store can configure path and lock_path as two different directories + $centralStoragePath = storage_path(); + $path = "{$centralStoragePath}/framework/cache/foo"; + $lockPath = "{$centralStoragePath}/framework/cache/foo_locks"; + + // Delete the directories before running assertions, not just after. + // Cache::store('foo_file')->lock(...) below defaults to a lock that never expires, + // and this test never releases it, so a stale lock left over from + // a previous run would make lock() fail forever. + File::deleteDirectory($path); + File::deleteDirectory($lockPath); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + 'lock_path' => $lockPath, + ], + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $tenantPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo"; + $tenantLockPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo_locks"; + + // lock('foo')->get() acquires the lock and creates the lock file at $tenantLockPath + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + + expect(File::isDirectory($tenantLockPath))->toBeTrue(); + // The lock file was created at $tenantLockPath, not $tenantPath + expect(File::isDirectory($tenantPath))->toBeFalse(); + + tenancy()->end(); + + // The lock was only acquired in the tenant context, so in the central context, + // it's free (and the lock file doesn't exist in the central context). + expect(File::isDirectory($lockPath))->toBeFalse(); + // Acquire the lock in the central context + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + // The lock file was created at the original (central) lock_path + expect(File::isDirectory($lockPath))->toBeTrue(); + + File::deleteDirectory($path); + File::deleteDirectory($lockPath); +}); + +test('a file cache store without a configured lock_path defaults to using its scoped path for locks', function () { + // 'lock_path' is optional in the file store config. + // FileStore falls back to using 'path' for locks when 'lock_path' is not specified in the config (or set to null). + // scopeCache() respects the original config and leaves lock_path unset/null. + $centralStoragePath = storage_path(); + $path = "{$centralStoragePath}/framework/cache/foo"; + + // $path is hardcoded here. The directory created at $path in a previous run can persist, + // so delete the directory at $path first. + File::deleteDirectory($path); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + // 'lock_path' not set (same behavior as if it were set to null) + ], + ]); + + $tenant = Tenant::create(); + tenancy()->initialize($tenant); + + $tenantPath = "{$centralStoragePath}/tenant{$tenant->id}/framework/cache/foo"; + + // Initializing tenancy scopes the store's 'path', but respects the configured + // (unset) 'lock_path' and leaves it alone. + expect(config('cache.stores.foo_file.path'))->toBe($tenantPath); + expect(config('cache.stores.foo_file.lock_path'))->toBeNull(); + + // With no 'lock_path' configured, the lock file will be created in the scoped 'path' directory + expect(File::isDirectory($tenantPath))->toBeFalse(); + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + expect(File::isDirectory($tenantPath))->toBeTrue(); + + // The lock is still held in the tenant context, so acquiring it again fails + expect(Cache::store('foo_file')->lock('foo')->get())->toBeFalse(); + + tenancy()->end(); + + // The same 'foo' lock is free in central context. + // revert() points the 'path' back to the original. + expect(File::isDirectory($path))->toBeFalse(); + expect(Cache::store('foo_file')->lock('foo')->get())->toBeTrue(); + expect(File::isDirectory($path))->toBeTrue(); + + // Clean up directory created at the hardcoded path + File::deleteDirectory($path); +}); + +test('a cache store using a path not based on storage_path() has the tenant suffix appended', function () { + // tenantCachePath() has no central storage path prefix to swap for the tenant's here, + // so it appends the tenant suffix to $path instead. + $path = '/tmp/tenancy-cache-test'; + File::deleteDirectory($path); + + config([ + 'tenancy.bootstrappers' => [ + FilesystemTenancyBootstrapper::class, + ], + 'tenancy.cache.stores' => ['foo_file'], + 'cache.stores.foo_file' => [ + 'driver' => 'file', + 'path' => $path, + ], + ]); + + $tenant1 = Tenant::create(); + $tenant2 = Tenant::create(); + + tenancy()->initialize($tenant1); + Cache::store('foo_file')->put('key', 'tenant1'); + + tenancy()->initialize($tenant2); + expect(Cache::store('foo_file')->get('key'))->toBeNull(); + + tenancy()->initialize($tenant1); + expect(Cache::store('foo_file')->get('key'))->toBe('tenant1'); + + // Tenant's 'foo_file' cache directory is created at $path with + // the tenant suffix appended, not at the tenant-scoped storage_path(). + expect(File::isDirectory("{$path}/tenant{$tenant1->id}"))->toBeTrue(); + expect(File::isDirectory(storage_path('framework/cache/data')))->toBeFalse(); + + tenancy()->end(); + + File::deleteDirectory($path); +});