Always revert queue worker to the central context — fix #1229 (#1251)

* Fix #1229 * docker-compose -> docker compose in ci.yml * docker->compose -> docker compose * docker->compose -> docker compose * clarify how dispatchNow() works
archtechx · Sep 17, 2024 · ed52c48 · ed52c48
1 parent ed02943
commit ed52c48
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 35 deletions.
diff --git 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
@@ -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,55 +82,40 @@ 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));
     }
 
     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
@@ -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 "$@"