diff --git a/.github/workflows/queue.yml b/.github/workflows/queue.yml index 0f3ec82e..4bd30f02 100644 --- a/.github/workflows/queue.yml +++ b/.github/workflows/queue.yml @@ -25,3 +25,5 @@ jobs: cd tenancy-queue-tester TENANCY_VERSION=${VERSION_PREFIX}#${GITHUB_SHA} ./setup.sh ./test.sh + ./alternative_config.sh + PERSISTENT=1 ./test.sh diff --git a/src/Bootstrappers/PersistentQueueTenancyBootstrapper.php b/src/Bootstrappers/PersistentQueueTenancyBootstrapper.php new file mode 100644 index 00000000..90bb3d08 --- /dev/null +++ b/src/Bootstrappers/PersistentQueueTenancyBootstrapper.php @@ -0,0 +1,146 @@ +make(Dispatcher::class), $app->runningUnitTests()); + } + + public function __construct(Repository $config, QueueManager $queue) + { + $this->config = $config; + $this->queue = $queue; + + $this->setUpPayloadGenerator(); + } + + protected static function setUpJobListener(Dispatcher $dispatcher, bool $runningTests): void + { + $previousTenant = null; + + $dispatcher->listen(JobProcessing::class, function ($event) use (&$previousTenant) { + $previousTenant = tenant(); + + static::initializeTenancyForQueue($event->job->payload()['tenant_id'] ?? null); + }); + + $dispatcher->listen(JobRetryRequested::class, function ($event) use (&$previousTenant) { + $previousTenant = tenant(); + + static::initializeTenancyForQueue($event->payload()['tenant_id'] ?? null); + }); + + // If we're running tests, we make sure to clean up after any artisan('queue:work') calls + $revertToPreviousState = function ($event) use (&$previousTenant, $runningTests) { + if ($runningTests) { + static::revertToPreviousState($event->job->payload()['tenant_id'] ?? null, $previousTenant); + + // We don't need to reset $previousTenant since the value will be set again when a job is processed. + } + + // If we're not running tests, we remain in the tenant's context. This makes other JobProcessed + // listeners able to deserialize the job, including with SerializesModels, since the tenant connection + // remains open. + }; + + $dispatcher->listen(JobProcessed::class, $revertToPreviousState); // artisan('queue:work') which succeeds + $dispatcher->listen(JobFailed::class, $revertToPreviousState); // artisan('queue:work') which fails + } + + protected static function initializeTenancyForQueue(string|int|null $tenantId): void + { + if (! $tenantId) { + // The job is not tenant-aware + if (tenancy()->initialized) { + // Tenancy was initialized, so we revert back to the central context + tenancy()->end(); + } + + return; + } + + // Re-initialize tenancy between all jobs even if the tenant is the same + // so that we don't work with an outdated tenant() instance in case it + // was updated outside the queue worker. + tenancy()->end(); + + /** @var Tenant $tenant */ + $tenant = tenancy()->find($tenantId); + tenancy()->initialize($tenant); + } + + protected static function revertToPreviousState(string|int|null $tenantId, ?Tenant $previousTenant): void + { + // The job was not tenant-aware + if (! $tenantId) { + return; + } + + // Revert back to the previous tenant + if (tenant() && $previousTenant && $previousTenant->isNot(tenant())) { + tenancy()->initialize($previousTenant); + } + + // End tenancy + if (tenant() && (! $previousTenant)) { + tenancy()->end(); + } + } + + protected function setUpPayloadGenerator(): void + { + $bootstrapper = &$this; + + if (! $this->queue instanceof QueueFake) { + $this->queue->createPayloadUsing(function ($connection) use (&$bootstrapper) { + return $bootstrapper->getPayload($connection); + }); + } + } + + public function getPayload(string $connection): array + { + if (! tenancy()->initialized) { + return []; + } + + if ($this->config["queue.connections.$connection.central"]) { + return []; + } + + return [ + 'tenant_id' => tenant()->getTenantKey(), + ]; + } + + public function bootstrap(Tenant $tenant): void {} + public function revert(): void {} +} diff --git a/src/Bootstrappers/QueueTenancyBootstrapper.php b/src/Bootstrappers/QueueTenancyBootstrapper.php index 16ae043b..2ed6b7f1 100644 --- a/src/Bootstrappers/QueueTenancyBootstrapper.php +++ b/src/Bootstrappers/QueueTenancyBootstrapper.php @@ -24,16 +24,6 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper /** @var QueueManager */ protected $queue; - /** - * Don't persist the same tenant across multiple jobs even if they have the same tenant ID. - * - * This is useful when you're changing the tenant's state (e.g. properties in the `data` column) and want the next job to initialize tenancy again - * with the new data. Features like the Tenant Config are only executed when tenancy is initialized, so the re-initialization is needed in some cases. - * - * @var bool - */ - public static $forceRefresh = false; - /** * The normal constructor is only executed after tenancy is bootstrapped. * However, we're registering a hook to initialize tenancy. Therefore, @@ -68,9 +58,12 @@ protected static function setUpJobListener(Dispatcher $dispatcher): void static::initializeTenancyForQueue($event->payload()['tenant_id'] ?? null); }); - // If we're running tests, we make sure to clean up after any artisan('queue:work') calls $revertToPreviousState = function ($event) use (&$previousTenant) { - static::revertToPreviousState($event, $previousTenant); + // In queue worker context, this reverts to the central context. + // In dispatchSync context, this reverts to the previous tenant's context. + // There's no need to reset $previousTenant here since it's always first + // set in the above listeners and the app is reverted back to that context. + static::revertToPreviousState($event->job->payload()['tenant_id'] ?? null, $previousTenant); }; $dispatcher->listen(JobProcessed::class, $revertToPreviousState); // artisan('queue:work') which succeeds @@ -79,61 +72,25 @@ protected static function setUpJobListener(Dispatcher $dispatcher): void protected static function initializeTenancyForQueue(string|int|null $tenantId): void { - if ($tenantId === null) { - // The job is not tenant-aware - if (tenancy()->initialized) { - // Tenancy was initialized, so we revert back to the central context - tenancy()->end(); - } - - return; - } - - if (static::$forceRefresh) { - // Re-initialize tenancy between all jobs - if (tenancy()->initialized) { - tenancy()->end(); - } - - /** @var Tenant $tenant */ - $tenant = tenancy()->find($tenantId); - tenancy()->initialize($tenant); - + if (! $tenantId) { return; } - if (tenancy()->initialized) { - // Tenancy is already initialized - if (tenant()->getTenantKey() === $tenantId) { - // It's initialized for the same tenant (e.g. dispatchSync was used, or the previous job also ran for this tenant) - return; - } - } - - // Tenancy was either not initialized, or initialized for a different tenant. - // Therefore, we initialize it for the correct tenant. - /** @var Tenant $tenant */ $tenant = tenancy()->find($tenantId); tenancy()->initialize($tenant); } - protected static function revertToPreviousState(JobProcessed|JobFailed $event, ?Tenant &$previousTenant): void + protected static function revertToPreviousState(string|int|null $tenantId, ?Tenant $previousTenant): void { - $tenantId = $event->job->payload()['tenant_id'] ?? null; - - // The job was not tenant-aware + // The job was not tenant-aware so no context switch was done if (! $tenantId) { return; } - // Revert back to the previous tenant - if (tenant() && $previousTenant?->isNot(tenant())) { - tenancy()->initialize($previousTenant); - } - - // End tenancy - if (tenant() && (! $previousTenant)) { + // End tenancy when there's no previous tenant + // (= when running in a queue worker, not dispatchSync) + if (tenant() && ! $previousTenant) { tenancy()->end(); } } @@ -149,16 +106,6 @@ protected function setUpPayloadGenerator(): void } } - public function bootstrap(Tenant $tenant): void - { - // - } - - public function revert(): void - { - // - } - public function getPayload(string $connection): array { if (! tenancy()->initialized) { @@ -169,10 +116,11 @@ public function getPayload(string $connection): array return []; } - $id = tenant()->getTenantKey(); - return [ - 'tenant_id' => $id, + 'tenant_id' => tenant()->getTenantKey(), ]; } + + public function bootstrap(Tenant $tenant): void {} + public function revert(): void {} } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 29caf7a7..4059479e 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -4,6 +4,7 @@ namespace Stancl\Tenancy; +use Closure; use Illuminate\Cache\CacheManager; use Illuminate\Database\Console\Migrations\FreshCommand; use Illuminate\Routing\Events\RouteMatched; @@ -18,9 +19,15 @@ class TenancyServiceProvider extends ServiceProvider { + public static Closure|null $configure = null; + /* Register services. */ public function register(): void { + if (static::$configure) { + (static::$configure)(); + } + $this->mergeConfigFrom(__DIR__ . '/../assets/config.php', 'tenancy'); $this->app->singleton(Database\DatabaseManager::class); diff --git a/tests/Features/NoAttachTest.php b/tests/Features/NoAttachTest.php index 9ba6079d..e82f3eb4 100644 --- a/tests/Features/NoAttachTest.php +++ b/tests/Features/NoAttachTest.php @@ -20,12 +20,6 @@ use Stancl\Tenancy\Tests\Etc\Tenant; test('sqlite ATTACH statements can be blocked', function (bool $disallow) { - try { - readlink(base_path('vendor')); - } catch (\Throwable) { - symlink(base_path('vendor'), '/var/www/html/vendor'); - } - if (php_uname('m') == 'aarch64') { // Escape testbench prison. Can't hardcode /var/www/html/extensions/... here // since GHA doesn't mount the filesystem on the container's workdir diff --git a/tests/QueueTest.php b/tests/QueueTest.php index 4d3f5ce0..2095cc84 100644 --- a/tests/QueueTest.php +++ b/tests/QueueTest.php @@ -22,16 +22,12 @@ use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\QueueTenancyBootstrapper; use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\PersistentQueueTenancyBootstrapper; use Stancl\Tenancy\Listeners\QueueableListener; beforeEach(function () { - $this->mockConsoleOutput = false; - config([ - 'tenancy.bootstrappers' => [ - QueueTenancyBootstrapper::class, - DatabaseTenancyBootstrapper::class, - ], + 'tenancy.bootstrappers' => [DatabaseTenancyBootstrapper::class], 'queue.default' => 'redis', ]); @@ -45,7 +41,22 @@ pest()->valuestore->flush(); }); -test('tenant id is passed to tenant queues', function () { +dataset('queue_bootstrappers', [ + QueueTenancyBootstrapper::class, + PersistentQueueTenancyBootstrapper::class, +]); + +function withQueueBootstrapper(string $class) { + config(['tenancy.bootstrappers' => [ + DatabaseTenancyBootstrapper::class, + $class, + ]]); + + $class::__constructStatic(app()); +} + +test('tenant id is passed to tenant queues', function (string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withTenantDatabases(); config(['queue.default' => 'sync']); @@ -61,9 +72,10 @@ Event::assertDispatched(JobProcessing::class, function ($event) { return $event->job->payload()['tenant_id'] === tenant('id'); }); -}); +})->with('queue_bootstrappers'); -test('tenant id is not passed to central queues', function () { +test('tenant id is not passed to central queues', function (string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withTenantDatabases(); $tenant = Tenant::create(); @@ -82,9 +94,10 @@ Event::assertDispatched(JobProcessing::class, function ($event) { return ! isset($event->job->payload()['tenant_id']); }); -}); +})->with('queue_bootstrappers'); -test('tenancy is initialized inside queues', function (bool $shouldEndTenancy) { +test('tenancy is initialized inside queues', function (bool $shouldEndTenancy, string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withTenantDatabases(); withFailedJobs(); @@ -117,7 +130,7 @@ $tenant->run(function () use ($user) { expect($user->fresh()->name)->toBe('Bar'); }); -})->with([true, false]); +})->with([true, false])->with('queue_bootstrappers'); test('changing the shouldQueue static property in parent class affects child classes unless the property is redefined', function () { // Parent – $shouldQueue is true @@ -142,7 +155,8 @@ expect(app(ShouldNotQueueListener::class)->shouldQueue(new stdClass()))->toBeFalse(); }); -test('tenancy is initialized when retrying jobs', function (bool $shouldEndTenancy) { +test('tenancy is initialized when retrying jobs', function (bool $shouldEndTenancy, string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withFailedJobs(); withTenantDatabases(); @@ -189,9 +203,10 @@ $tenant->run(function () use ($user) { expect($user->fresh()->name)->toBe('Bar'); }); -})->with([true, false]); +})->with([true, false])->with('queue_bootstrappers'); -test('the tenant used by the job doesnt change when the current tenant changes', function () { +test('the tenant used by the job doesnt change when the current tenant changes', function (string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withTenantDatabases(); $tenant1 = Tenant::create(); @@ -208,26 +223,11 @@ pest()->artisan('queue:work --once'); expect(pest()->valuestore->get('tenant_id'))->toBe('The current tenant id is: ' . $tenant1->getTenantKey()); -}); - -test('tenant connections do not persist after tenant jobs get processed', function() { - withTenantDatabases(); - - $tenant = Tenant::create(); - - tenancy()->initialize($tenant); - - dispatch(new TestJob(pest()->valuestore)); - - tenancy()->end(); - - pest()->artisan('queue:work --once'); - - expect(collect(DB::select('SHOW FULL PROCESSLIST'))->pluck('db'))->not()->toContain($tenant->database()->getName()); -}); +})->with('queue_bootstrappers'); // Regression test for #1277 -test('dispatching a job from a tenant run arrow function dispatches it immediately', function () { +test('dispatching a job from a tenant run arrow function dispatches it immediately', function (string $bootstrapper) { + withQueueBootstrapper($bootstrapper); withTenantDatabases(); $tenant = Tenant::create(); @@ -241,7 +241,7 @@ expect(tenant())->toBe(null); expect(pest()->valuestore->get('tenant_id'))->toBe('The current tenant id is: ' . $tenant->getTenantKey()); -}); +})->with('queue_bootstrappers'); function createValueStore(): void { diff --git a/tests/TestCase.php b/tests/TestCase.php index f9a2d357..7fc1813b 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -22,6 +22,7 @@ use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper; use Stancl\Tenancy\Bootstrappers\BroadcastingConfigBootstrapper; use Stancl\Tenancy\Bootstrappers\BroadcastChannelPrefixBootstrapper; +use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; abstract class TestCase extends \Orchestra\Testbench\TestCase { @@ -85,11 +86,8 @@ protected function setUp(): void '--realpath' => true, ]); - // Laravel 6.x support todo@refactor clean up - $testResponse = class_exists('Illuminate\Testing\TestResponse') ? 'Illuminate\Testing\TestResponse' : 'Illuminate\Foundation\Testing\TestResponse'; - $testResponse::macro('assertContent', function ($content) { - $assertClass = class_exists('Illuminate\Testing\Assert') ? 'Illuminate\Testing\Assert' : 'Illuminate\Foundation\Testing\Assert'; - $assertClass::assertSame($content, $this->baseResponse->getContent()); + \Illuminate\Testing\TestResponse::macro('assertContent', function ($content) { + \Illuminate\Testing\Assert::assertSame($content, $this->baseResponse->getContent()); return $this; }); @@ -175,18 +173,25 @@ protected function getEnvironmentSetUp($app) 'tenancy.models.tenant' => Tenant::class, // Use test tenant w/ DBs & domains ]); - $app->singleton(RedisTenancyBootstrapper::class); // todo@samuel use proper approach eg config for singleton registration - $app->singleton(CacheTenancyBootstrapper::class); // todo@samuel use proper approach eg config for singleton registration + // Since we run the TSP with no bootstrappers enabled, we need + // to manually register bootstrappers as singletons here. + $app->singleton(RedisTenancyBootstrapper::class); + $app->singleton(CacheTenancyBootstrapper::class); $app->singleton(BroadcastingConfigBootstrapper::class); $app->singleton(BroadcastChannelPrefixBootstrapper::class); $app->singleton(PostgresRLSBootstrapper::class); $app->singleton(MailConfigBootstrapper::class); $app->singleton(RootUrlBootstrapper::class); $app->singleton(UrlGeneratorBootstrapper::class); + $app->singleton(FilesystemTenancyBootstrapper::class); } protected function getPackageProviders($app) { + TenancyServiceProvider::$configure = function () { + config(['tenancy.bootstrappers' => []]); + }; + return [ TenancyServiceProvider::class, ];