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],