From 2ef1ea94ed2a4cf0245ef60dc20807f748e5fc83 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 25 Aug 2026 13:20:50 +0200 Subject: [PATCH] Make LogChannelBootstrapper not depend on suffixed storage_path() Since the tenant storage path can now be grabbed using FilesystemTenancyBootstrapper::getBoundTenantStoragePath(), the log bootstrapper doesn't need to depend on the FSBootstrapper being enabled and storage_path() being suffixed. Instead of adding a regression test, just delete FilesystemTenancyBootstrapper from the config settings in the log bootstrapper tests (and in tests that did use storage_path() in tenant context assertions, use explicitly "hardcoded" paths instead). --- src/Bootstrappers/LogChannelBootstrapper.php | 22 ++--- .../LogChannelBootstrapperTest.php | 85 +++++-------------- 2 files changed, 30 insertions(+), 77 deletions(-) diff --git a/src/Bootstrappers/LogChannelBootstrapper.php b/src/Bootstrappers/LogChannelBootstrapper.php index 293028e3..af30733a 100644 --- a/src/Bootstrappers/LogChannelBootstrapper.php +++ b/src/Bootstrappers/LogChannelBootstrapper.php @@ -21,11 +21,8 @@ use Stancl\Tenancy\Contracts\Tenant; * 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::getBoundTenantStoragePath(), + * 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 @@ class LogChannelBootstrapper implements TenancyBootstrapper // 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::getBoundTenantStoragePath($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 appended to 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, $centralStoragePath)); } } } diff --git a/tests/Bootstrappers/LogChannelBootstrapperTest.php b/tests/Bootstrappers/LogChannelBootstrapperTest.php index c37e7b54..009a0d66 100644 --- a/tests/Bootstrappers/LogChannelBootstrapperTest.php +++ b/tests/Bootstrappers/LogChannelBootstrapperTest.php @@ -9,7 +9,6 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\LogChannelBootstrapper; -use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use Illuminate\Support\Facades\Log; afterEach($cleanup = function () { @@ -42,15 +41,6 @@ beforeEach(function () use ($cleanup) { }); test('storage path channels get tenant-specific paths by default', function () { - // Note that for LogChannelBootstrapper to change the paths correctly by default, - // the bootstrapper MUST run after FilesystemTenancyBootstrapper. - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - $centralStoragePath = storage_path(); $tenant = Tenant::create(); @@ -76,10 +66,6 @@ test('storage path channels get tenant-specific paths by default', function () { test('all channels included in a stack get processed correctly', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], @@ -176,30 +162,24 @@ test('channel config keys remain unchanged if the specified tenant override attr }); test('channel overrides take precedence over the default storage path channel updating logic', function () { + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'tenant1']); LogChannelBootstrapper::$storagePathChannels = ['single']; LogChannelBootstrapper::$channelOverrides = [ - 'single' => function (Tenant $tenant, array $channel) { - return array_merge($channel, ['path' => storage_path("logs/override-{$tenant->id}.log")]); + 'single' => function (Tenant $tenant, array $channel) use ($centralStoragePath) { + return array_merge($channel, ['path' => "{$centralStoragePath}/logs/override-{$tenant->id}.log"]); }, ]; tenancy()->initialize($tenant); // Should use channel override, not the storage path updating behavior - expect(config('logging.channels.single.path'))->toEndWith('storage/logs/override-tenant1.log'); + expect(config('logging.channels.single.path'))->toBe("{$centralStoragePath}/logs/override-tenant1.log"); }); test('channels are forgotten and re-resolved during bootstrap and revert', function () { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - $logManager = app('log'); $originalChannel = $logManager->channel('single'); $originalSinglePath = config('logging.channels.single.path'); @@ -229,14 +209,8 @@ test('channels are forgotten and re-resolved during bootstrap and revert', funct // Test real usage test('logs are written to tenant-specific files and do not leak between contexts', function () { - config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], - ]); - - $centralLogPath = storage_path('logs/laravel.log'); + $centralStoragePath = storage_path(); + $centralLogPath = "{$centralStoragePath}/logs/laravel.log"; Log::channel('single')->info('central'); @@ -244,15 +218,11 @@ test('logs are written to tenant-specific files and do not leak between contexts [$tenant1, $tenant2] = [Tenant::create(['id' => 'tenant1']), Tenant::create(['id' => 'tenant2'])]; - tenancy()->runForMultiple([$tenant1, $tenant2], function (Tenant $tenant) use ($centralLogPath) { + tenancy()->runForMultiple([$tenant1, $tenant2], function (Tenant $tenant) use ($centralStoragePath) { Log::channel('single')->info($tenant->id); - $tenantLogPath = storage_path('logs/laravel.log'); - // The log gets saved to the tenant's storage directory (default behavior) - expect($tenantLogPath) - ->not()->toBe($centralLogPath) - ->toEndWith("storage/tenant{$tenant->id}/logs/laravel.log"); + $tenantLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; expect(file_get_contents($tenantLogPath)) ->toContain($tenant->id) @@ -268,14 +238,14 @@ test('logs are written to tenant-specific files and do not leak between contexts // Tenant log messages didn't leak to logs of other tenants tenancy()->initialize($tenant1); - expect(file_get_contents(storage_path('logs/laravel.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant1->id}/logs/laravel.log")) ->toContain('tenant1') ->not()->toContain('central') ->not()->toContain('tenant2'); tenancy()->initialize($tenant2); - expect(file_get_contents(storage_path('logs/laravel.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant2->id}/logs/laravel.log")) ->toContain('tenant2') ->not()->toContain('central') ->not()->toContain('tenant1'); @@ -285,9 +255,8 @@ test('logs are written to tenant-specific files and do not leak between contexts $tenant = Tenant::create(['id' => 'override-tenant']); LogChannelBootstrapper::$channelOverrides = [ - 'single' => function (Tenant $tenant, array $channel) { - // The tenant log path will be set to storage/tenantoverride-tenant/logs/custom-override-tenant.log - return array_merge($channel, ['path' => storage_path("logs/custom-{$tenant->id}.log")]); + 'single' => function (Tenant $tenant, array $channel) use ($centralStoragePath) { + return array_merge($channel, ['path' => "{$centralStoragePath}/tenant{$tenant->id}/logs/custom-{$tenant->id}.log"]); }, ]; @@ -296,21 +265,18 @@ test('logs are written to tenant-specific files and do not leak between contexts Log::channel('single')->info('tenant-override'); - expect(file_get_contents(storage_path('logs/custom-override-tenant.log')))->toContain('tenant-override'); + expect(file_get_contents("{$centralStoragePath}/tenantoverride-tenant/logs/custom-override-tenant.log"))->toContain('tenant-override'); }); test('stack logs are written to all configured channels with tenant-specific paths', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], ], ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'stack-tenant']); $today = now()->format('Y-m-d'); @@ -327,8 +293,8 @@ test('stack logs are written to all configured channels with tenant-specific pat // Tenant context stack log tenancy()->initialize($tenant); Log::channel('stack')->info('tenant'); - $tenantSingleLogPath = storage_path('logs/laravel.log'); - $tenantDailyLogPath = storage_path("logs/laravel-{$today}.log"); + $tenantSingleLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; + $tenantDailyLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel-{$today}.log"; expect(file_get_contents($tenantSingleLogPath))->toContain('tenant'); expect(file_get_contents($tenantDailyLogPath))->toContain('tenant'); @@ -351,18 +317,15 @@ test('stack logs are written to all configured channels with tenant-specific pat test('stack channels that include any configured channel are re-resolved', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.custom_stack' => [ 'driver' => 'stack', 'channels' => ['single'], ], ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(['id' => 'stack-tenant']); - $centralLogPath = storage_path('logs/laravel.log'); + $centralLogPath = "{$centralStoragePath}/logs/laravel.log"; $logManager = app('log'); @@ -387,7 +350,7 @@ test('stack channels that include any configured channel are re-resolved', funct ->toContain('central log message') ->not()->toContain('tenant log message'); - $tenantLogPath = storage_path('logs/laravel.log'); + $tenantLogPath = "{$centralStoragePath}/tenant{$tenant->id}/logs/laravel.log"; expect(file_exists($tenantLogPath))->toBeTrue(); expect(file_get_contents($tenantLogPath)) ->toContain('tenant log message'); @@ -454,10 +417,6 @@ test('slack channel uses correct webhook urls', function () { test('tenant logs inherit the path from the central log path config', function () { config([ - 'tenancy.bootstrappers' => [ - FilesystemTenancyBootstrapper::class, - LogChannelBootstrapper::class, - ], 'logging.channels.stack' => [ 'driver' => 'stack', 'channels' => ['single', 'daily'], @@ -466,6 +425,7 @@ test('tenant logs inherit the path from the central log path config', function ( 'logging.channels.daily.path' => storage_path('logs/daily/custom-name.log'), ]); + $centralStoragePath = storage_path(); $tenant = Tenant::create(); $today = now()->format('Y-m-d'); @@ -476,18 +436,17 @@ test('tenant logs inherit the path from the central log path config', function ( tenancy()->initialize($tenant); - // Tenant log is located at storage/tenantX/logs/custom-name.log Log::channel('stack')->info($tenant->id); // The filename from the central config is preserved in tenant context expect(config('logging.channels.single.path'))->toEndWith('logs/single/custom-name.log'); expect(config('logging.channels.daily.path'))->toEndWith('logs/daily/custom-name.log'); - expect(file_get_contents(storage_path('logs/single/custom-name.log'))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant->id}/logs/single/custom-name.log")) ->toContain($tenant->id) ->not()->toContain('central'); - expect(file_get_contents(storage_path("logs/daily/custom-name-{$today}.log"))) + expect(file_get_contents("{$centralStoragePath}/tenant{$tenant->id}/logs/daily/custom-name-{$today}.log")) ->toContain($tenant->id) ->not()->toContain('central'); });