From 2a0464d694dc12b819824394fa8b3f1918121314 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 10 Jul 2026 15:06:03 +0200 Subject: [PATCH] Resolve (and cache) tenant broadcasters once per tenancy initialization instead of re-resolving them every time TenancyBroadcastManager now overrides resolve() (only to pass the central channels to the newly resolved broadcasters) instead of get(), so broadcasters stay cached for the duration of the tenant's context. Remove TenancyBroadcastManager::$tenantBroadcasters -- all broadcasters now inherit the central channels, so even custom drivers work without any additional configuration/registration. Channels registered via Broadcast::channel() in tenant context are no longer lost on subsequent broadcaster retrievals (e.g. during /broadcasting/auth). Added a dedicated test for this, --- .../BroadcastingConfigBootstrapper.php | 14 ++-- src/Overrides/TenancyBroadcastManager.php | 55 +++++----------- .../BroadcastingConfigBootstrapperTest.php | 64 ++++++++++++++----- 3 files changed, 71 insertions(+), 62 deletions(-) diff --git a/src/Bootstrappers/BroadcastingConfigBootstrapper.php b/src/Bootstrappers/BroadcastingConfigBootstrapper.php index 93f4366f..a421e85f 100644 --- a/src/Bootstrappers/BroadcastingConfigBootstrapper.php +++ b/src/Bootstrappers/BroadcastingConfigBootstrapper.php @@ -15,7 +15,7 @@ use Stancl\Tenancy\Overrides\TenancyBroadcastManager; /** * Maps tenant properties to broadcasting config and overrides - * the BroadcastManager binding with TenancyBroadcastManager. + * the BroadcastManager and Broadcaster bindings with tenant-aware instances. * * @see TenancyBroadcastManager */ @@ -73,8 +73,9 @@ class BroadcastingConfigBootstrapper implements TenancyBootstrapper $this->setConfig($tenant); - // Make BroadcastManager resolve to TenancyBroadcastManager. The manager: - // - resolves fresh broadcasters so that the updated (tenant) broadcasting config is used while broadcasting + // Make BroadcastManager resolve to a fresh TenancyBroadcastManager. The new manager: + // - has no cached broadcasters, so its broadcasters get resolved using the updated (tenant) + // broadcasting config and stay cached for the duration of the tenant's context // - makes the tenant broadcasters inherit the channels of the original (central) broadcaster // (since newly resolved broadcasters don't receive any channels by default, broadcasting on // channels registered in central context, e.g. in routes/channels.php, would otherwise not @@ -93,11 +94,8 @@ class BroadcastingConfigBootstrapper implements TenancyBootstrapper }); // Swap the currently bound Broadcaster singleton (resolved earlier with the central credentials) - // for one resolved through the tenant BroadcastManager, so that anything resolving the Broadcaster - // contract gets a broadcaster that uses the tenant's credentials instead of the stale central one. - // Unlike broadcasters resolved through the manager (re-resolved on each call), this instance is - // resolved once, so credential changes made later in tenant context don't affect it until tenancy - // is reinitialized. + // for the tenant BroadcastManager's default broadcaster, so that anything resolving the Broadcaster + // contract gets the same tenant broadcaster that the manager uses, instead of the stale central one. $this->app->extend(Broadcaster::class, function () { return $this->app->make(BroadcastManager::class)->connection(); }); diff --git a/src/Overrides/TenancyBroadcastManager.php b/src/Overrides/TenancyBroadcastManager.php index 3d77cc4a..97383cf0 100644 --- a/src/Overrides/TenancyBroadcastManager.php +++ b/src/Overrides/TenancyBroadcastManager.php @@ -9,57 +9,36 @@ use Illuminate\Broadcasting\BroadcastManager; use Illuminate\Contracts\Broadcasting\Broadcaster as BroadcasterContract; /** - * BroadcastManager override that always re-resolves the broadcasters in static::$tenantBroadcasters - * when attempting to retrieve them so that they use the updated tenant-specific config - * and passes the channels of the original (central) broadcaster - * to the newly resolved (tenant) broadcasters. + * BroadcastManager override that makes the newly resolved (tenant) broadcasters + * inherit the channels of the original (central) broadcaster. * - * Affects calls that use BroadcastManager's get() method. + * BroadcastingConfigBootstrapper binds a new instance of this manager on each tenancy + * initialization, so the broadcasters get resolved using the tenant's broadcasting config + * and stay cached (like in the parent manager) for the duration of the tenant's context. * * @see Stancl\Tenancy\Bootstrappers\BroadcastingConfigBootstrapper */ class TenancyBroadcastManager extends BroadcastManager { /** - * Names of broadcasters that - * - should always be recreated using $this->resolve(), even when they're cached and available - * in $this->drivers so that when you update broadcasting config in the tenant context, - * the updated config/credentials will be used for broadcasting immediately. - * Note that in cases like this, only direct config changes are reflected right away. - * For the broadcasters to reflect tenant property changes made in tenant context, - * you still have to reinitialize tenancy after updating the tenant properties intended - * to be mapped to broadcasting config, since the properties are only mapped to config - * on BroadcastingConfigBootstrapper::bootstrap(). - * - should inherit the original broadcaster's channels (= the channels registered in - * the central context, e.g. in routes/channels.php, before this manager overrides the bound BroadcastManager). + * Resolve the broadcaster and pass it the channels of the currently bound broadcaster + * (the central one, when the default driver is resolved during bootstrap). */ - public static array $tenantBroadcasters = ['pusher', 'ably', 'reverb']; - - /** - * Override the get method so that the broadcasters in static::$tenantBroadcasters - * - receive the original (central) broadcaster's channels - * - always get freshly resolved. - */ - protected function get($name) + protected function resolve($name) { - if (in_array($name, static::$tenantBroadcasters)) { - /** @var Broadcaster|null $originalBroadcaster */ - $originalBroadcaster = $this->app->make(BroadcasterContract::class); - $newBroadcaster = $this->resolve($name); + $newBroadcaster = parent::resolve($name); - // Give the channels of the original (central) broadcaster to the newly resolved one. - // - // Broadcasters only have to implement the Illuminate\Contracts\Broadcasting\Broadcaster contract - // which doesn't require the channels property, so we only pass the channels to - // Illuminate\Broadcasting\Broadcasters\Broadcaster instances (= all the default broadcasters, e.g. PusherBroadcaster). - if ($originalBroadcaster instanceof Broadcaster && $newBroadcaster instanceof Broadcaster) { - $this->passChannelsFromOriginalBroadcaster($originalBroadcaster, $newBroadcaster); - } + /** @var Broadcaster|null $originalBroadcaster */ + $originalBroadcaster = $this->app->make(BroadcasterContract::class); - return $newBroadcaster; + // Broadcasters only have to implement the Illuminate\Contracts\Broadcasting\Broadcaster contract + // which doesn't require the channels property, so we only pass the channels to + // Illuminate\Broadcasting\Broadcasters\Broadcaster instances (= all the default broadcasters, e.g. PusherBroadcaster). + if ($originalBroadcaster instanceof Broadcaster && $newBroadcaster instanceof Broadcaster) { + $this->passChannelsFromOriginalBroadcaster($originalBroadcaster, $newBroadcaster); } - return parent::get($name); + return $newBroadcaster; } /** diff --git a/tests/Bootstrappers/BroadcastingConfigBootstrapperTest.php b/tests/Bootstrappers/BroadcastingConfigBootstrapperTest.php index 77f153cf..a58014c6 100644 --- a/tests/Bootstrappers/BroadcastingConfigBootstrapperTest.php +++ b/tests/Bootstrappers/BroadcastingConfigBootstrapperTest.php @@ -34,7 +34,6 @@ $cleanup = function () { 'broadcasting.connections.ably.public' => 'ably_public', ], ]; - TenancyBroadcastManager::$tenantBroadcasters = ['pusher', 'ably', 'reverb']; }; beforeEach(function () use ($cleanup) { @@ -70,7 +69,6 @@ test('ending tenancy reverts the bound broadcaster to the original instance', fu 'broadcasting.default' => 'testing', 'broadcasting.connections.testing.driver' => 'testing', ]); - TenancyBroadcastManager::$tenantBroadcasters = ['testing']; app(BroadcastManager::class)->extend('testing', fn ($app, $config) => new TestingBroadcaster('testing', $config)); @@ -81,6 +79,9 @@ test('ending tenancy reverts the bound broadcaster to the original instance', fu // BroadcastingConfigBootstrapper binds a freshly resolved broadcaster expect(app(BroadcasterContract::class))->not()->toBe($originalBroadcaster); + // The bound broadcaster is the same instance as the tenant BroadcastManager's default driver + expect(app(BroadcasterContract::class))->toBe(app(BroadcastManager::class)->driver()); + tenancy()->end(); // Ending tenancy reverts the binding back to the original broadcaster instance @@ -98,9 +99,6 @@ test('BroadcastingConfigBootstrapper maps tenant properties to broadcaster crede if ($driver === 'custom') { config(['broadcasting.connections.custom.driver' => 'custom']); - - // Custom driver, not included in TenancyBroadcastManager::$tenantBroadcasters by default - TenancyBroadcastManager::$tenantBroadcasters = ['custom']; } BroadcastingConfigBootstrapper::$credentialsMap["broadcasting.connections.{$driver}.key"] = 'testing_key'; @@ -147,14 +145,15 @@ test('BroadcastingConfigBootstrapper maps tenant properties to broadcaster crede tenancy()->initialize($tenant1); - // When updating tenant properties without reinitializing, the tenant property update doesn't update the config, - // so the config has to be modified manually. Only methods that use TenancyBroadcastManager::get() - // will use the updated credentials without needing to reinitialize tenancy (e.g. the bound - // BroadcasterContract instance will still use the original credentials, even after config gets updated directly). + // Direct config changes aren't picked up by the broadcasters -- they get resolved + // using the config mapped from tenant properties at initialization and stay cached + // until tenancy is reinitialized. config(["broadcasting.connections.{$driver}.key" => 'new_tenant1_key']); - expect(app(BroadcastManager::class)->driver()->config['key'])->toBe('new_tenant1_key'); - expect(Broadcast::driver()->config['key'])->toBe('new_tenant1_key'); + expect(config("broadcasting.connections.{$driver}.key"))->toBe('new_tenant1_key'); + expect(app(BroadcastManager::class)->driver()->config['key'])->toBe('tenant1_key'); + expect(app(BroadcasterContract::class)->config['key'])->toBe('tenant1_key'); + expect(Broadcast::driver()->config['key'])->toBe('tenant1_key'); tenancy()->end(); @@ -167,7 +166,7 @@ test('BroadcastingConfigBootstrapper maps tenant properties to broadcaster crede 'pusher', 'ably', 'reverb', - 'custom', // Except for this custom driver, assume that the drivers are included in TenancyBroadcastManager::$tenantBroadcasters by default + 'custom', ]); test('tenant broadcast manager receives the custom driver creators of the central broadcast manager', function() { @@ -217,9 +216,6 @@ test('tenant broadcasters receive the channels from the broadcaster bound in cen if ($driver === 'custom') { config(['broadcasting.connections.custom.driver' => 'custom']); - - // Custom driver, not included in TenancyBroadcastManager::$tenantBroadcasters by default - TenancyBroadcastManager::$tenantBroadcasters = ['custom']; } $tenant1 = Tenant::create(); @@ -256,9 +252,45 @@ test('tenant broadcasters receive the channels from the broadcaster bound in cen 'pusher', 'ably', 'reverb', - 'custom', // Except for this custom driver, assume that the drivers are included in TenancyBroadcastManager::$tenantBroadcasters by default + 'custom', ]); +test('channels registered in tenant context persist within that context but do not leak into other contexts', function() { + config([ + 'tenancy.bootstrappers' => [BroadcastingConfigBootstrapper::class], + 'broadcasting.default' => 'testing', + 'broadcasting.connections.testing.driver' => 'testing', + ]); + + app(BroadcastManager::class)->extend('testing', fn($app, $config) => new TestingBroadcaster('testing', $config)); + + Broadcast::channel('central-channel', fn() => true); + + tenancy()->initialize(Tenant::create()); + + Broadcast::channel('tenant-channel', fn() => true); + + // Retrieving the broadcaster again (e.g. on Broadcast::auth() during a /broadcasting/auth request) + // returns the cached broadcaster, so the channel registered in tenant context is still available + expect(array_keys(invade(Broadcast::driver())->channels)) + ->toContain('central-channel') + ->toContain('tenant-channel'); + + // The channel registered in the previous tenant's context doesn't leak to another tenant's broadcaster + tenancy()->initialize(Tenant::create()); + + expect(array_keys(invade(Broadcast::driver())->channels)) + ->toContain('central-channel') + ->not()->toContain('tenant-channel'); + + tenancy()->end(); + + // The channel registered in tenant context doesn't leak to the central broadcaster + expect(array_keys(invade(Broadcast::driver())->channels)) + ->toContain('central-channel') + ->not()->toContain('tenant-channel'); +}); + test('mappings specified in credentialsMap override default mapPresets', function($driver) { config([ 'tenancy.bootstrappers' => [BroadcastingConfigBootstrapper::class],