From ed52c48428250458b1e16cf5670f22bd80bff4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 17 Sep 2024 18:12:33 +0200 Subject: [PATCH] =?UTF-8?q?Always=20revert=20queue=20worker=20to=20the=20c?= =?UTF-8?q?entral=20context=20=E2=80=94=20fix=20#1229=20(#1251)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix #1229 * docker-compose -> docker compose in ci.yml * docker->compose -> docker compose * docker->compose -> docker compose * clarify how dispatchNow() works --- .github/workflows/ci.yml | 2 +- .../QueueTenancyBootstrapper.php | 50 +++++++------------ test | 2 +- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3094e3f8e..b9e2ff554 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: Start docker containers - run: PHP_VERSION=${{ matrix.php }} docker-compose up -d + run: PHP_VERSION=${{ matrix.php }} docker compose up -d - name: Install dependencies run: docker compose exec -T test composer require --no-interaction "laravel/framework:^${{ matrix.laravel }}.0" - name: Run tests diff --git a/src/Bootstrappers/QueueTenancyBootstrapper.php b/src/Bootstrappers/QueueTenancyBootstrapper.php index f747faea9..1cac1893d 100644 --- a/src/Bootstrappers/QueueTenancyBootstrapper.php +++ b/src/Bootstrappers/QueueTenancyBootstrapper.php @@ -31,6 +31,8 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper * 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. * + * @deprecated This now has no effect, tenancy is always ended between queued jobs. + * * @var bool */ public static $forceRefresh = false; @@ -42,7 +44,7 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper */ public static function __constructStatic(Application $app) { - static::setUpJobListener($app->make(Dispatcher::class), $app->runningUnitTests()); + static::setUpJobListener($app->make(Dispatcher::class)); } public function __construct(Repository $config, QueueManager $queue) @@ -53,7 +55,7 @@ public function __construct(Repository $config, QueueManager $queue) $this->setUpPayloadGenerator(); } - protected static function setUpJobListener($dispatcher, $runningTests) + protected static function setUpJobListener($dispatcher) { $previousTenant = null; @@ -69,11 +71,8 @@ protected static function setUpJobListener($dispatcher, $runningTests) 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, $previousTenant); - } + $revertToPreviousState = function ($event) use (&$previousTenant) { + static::revertToPreviousState($event, $previousTenant); }; $dispatcher->listen(JobProcessed::class, $revertToPreviousState); // artisan('queue:work') which succeeds @@ -83,36 +82,14 @@ protected static function setUpJobListener($dispatcher, $runningTests) protected static function initializeTenancyForQueue($tenantId) { if (! $tenantId) { - // The job is not tenant-aware + // The job is not tenant-aware, so we make sure tenancy isn't initialized. 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(); - } - - tenancy()->initialize(tenancy()->find($tenantId)); - - return; - } - - if (tenancy()->initialized) { - // Tenancy is already initialized - if (tenant()->getTenantKey() === $tenantId) { - // It's initialized for the same tenant (e.g. dispatchNow 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. tenancy()->initialize(tenancy()->find($tenantId)); } @@ -120,18 +97,25 @@ protected static function revertToPreviousState($event, &$previousTenant) { $tenantId = $event->job->payload()['tenant_id'] ?? null; - // The job was not tenant-aware if (! $tenantId) { + // The job was not tenant-aware, so there's nothing to revert + return; + } + + if (tenant() && $previousTenant && $previousTenant->is(tenant())) { + // dispatchNow() was used and the tenant in the job is the same as the previous tenant return; } - // Revert back to the previous tenant if (tenant() && $previousTenant && $previousTenant->isNot(tenant())) { + // Revert back to the previous tenant (since Tenancy v3.8.5 this should should *likely* not happen) tenancy()->initialize($previousTenant); + return; } - // End tenancy if (tenant() && (! $previousTenant)) { + // No previous tenant = previous context was central + // NOTE: Since Tenancy v3.8.5 this should *likely* not happen tenancy()->end(); } } diff --git a/test b/test index e064bbfeb..57b7489c1 100755 --- a/test +++ b/test @@ -1,4 +1,4 @@ #!/bin/bash cat vendor/laravel/framework/src/Illuminate/Foundation/Application.php | grep 'const VERSION' -docker-compose exec -T test vendor/bin/phpunit "$@" +docker compose exec -T test vendor/bin/phpunit "$@"