From 9d6356aea4ad4b02a420f44f618d752aabcb8c65 Mon Sep 17 00:00:00 2001
From: lukinovec <lukinovec@gmail.com>
Date: Fri, 18 Aug 2023 07:38:07 +0200
Subject: [PATCH] Close tenant connections after jobs (#1128)

* Test that the context is central after jobs get processed

* Revert to central context when jobs get processed, delete `$previousTenant` logic

* Test that the tenant connections get closed after jobs get processed

* Bring back the previous tenant logic

* Adapt tests to the previous tenant logic

* Delete assertion
---
 .../QueueTenancyBootstrapper.php              | 10 +++----
 tests/QueueTest.php                           | 29 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/src/Bootstrappers/QueueTenancyBootstrapper.php b/src/Bootstrappers/QueueTenancyBootstrapper.php
index 92c95ef6..3f46a112 100644
--- a/src/Bootstrappers/QueueTenancyBootstrapper.php
+++ b/src/Bootstrappers/QueueTenancyBootstrapper.php
@@ -41,7 +41,7 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper
      */
     public static function __constructStatic(Application $app): void
     {
-        static::setUpJobListener($app->make(Dispatcher::class), $app->runningUnitTests());
+        static::setUpJobListener($app->make(Dispatcher::class));
     }
 
     public function __construct(Repository $config, QueueManager $queue)
@@ -52,7 +52,7 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper
         $this->setUpPayloadGenerator();
     }
 
-    protected static function setUpJobListener(Dispatcher $dispatcher, bool $runningTests): void
+    protected static function setUpJobListener(Dispatcher $dispatcher): void
     {
         $previousTenant = null;
 
@@ -69,10 +69,8 @@ class QueueTenancyBootstrapper implements TenancyBootstrapper
         });
 
         // 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
diff --git a/tests/QueueTest.php b/tests/QueueTest.php
index f88b3934..00678876 100644
--- a/tests/QueueTest.php
+++ b/tests/QueueTest.php
@@ -110,6 +110,8 @@ test('tenancy is initialized inside queues', function (bool $shouldEndTenancy) {
 
     pest()->artisan('queue:work --once');
 
+    expect(! tenancy()->initialized)->toBe($shouldEndTenancy);
+
     expect(DB::connection('central')->table('failed_jobs')->count())->toBe(0);
 
     expect(pest()->valuestore->get('tenant_id'))->toBe('The current tenant id is: ' . $tenant->id);
@@ -117,7 +119,7 @@ test('tenancy is initialized inside queues', function (bool $shouldEndTenancy) {
     $tenant->run(function () use ($user) {
         expect($user->fresh()->name)->toBe('Bar');
     });
-})->with([true, false]);;
+})->with([true, false]);
 
 test('tenancy is initialized when retrying jobs', function (bool $shouldEndTenancy) {
     withFailedJobs();
@@ -144,12 +146,21 @@ test('tenancy is initialized when retrying jobs', function (bool $shouldEndTenan
 
     pest()->artisan('queue:work --once');
 
+    expect(! tenancy()->initialized)->toBe($shouldEndTenancy);
+
     expect(DB::connection('central')->table('failed_jobs')->count())->toBe(1);
     expect(pest()->valuestore->get('tenant_id'))->toBeNull(); // job failed
 
     pest()->artisan('queue:retry all');
+
+    if ($shouldEndTenancy) {
+        tenancy()->end();
+    }
+
     pest()->artisan('queue:work --once');
 
+    expect(! tenancy()->initialized)->toBe($shouldEndTenancy);
+
     expect(DB::connection('central')->table('failed_jobs')->count())->toBe(0);
 
     expect(pest()->valuestore->get('tenant_id'))->toBe('The current tenant id is: ' . $tenant->id); // job succeeded
@@ -182,6 +193,22 @@ test('the tenant used by the job doesnt change when the current tenant changes',
     expect(pest()->valuestore->get('tenant_id'))->toBe('The current tenant id is: acme');
 });
 
+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());
+});
+
 function createValueStore(): void
 {
     $valueStorePath = __DIR__ . '/Etc/tmp/queuetest.json';