[5.2] fix: queries should be retried when session pool is undefined (#…

…129)
colopl · Aug 22, 2023 · 9bbc8c0 · 9bbc8c0
1 parent 40fe5fa
commit 9bbc8c0
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# v5.2.2 (2023-08-22)
+
+Fixed
+- Fixed a case where queries were not being retried on "Session Not Found" errors when session pool is undefined (#129)
+
 # v5.2.1 (2023-08-16)
 
 Fixed

diff --git a/src/Connection.php b/src/Connection.php
@@ -532,10 +532,7 @@ protected function withSessionNotFoundHandling(Closure $callback): mixed
             throw new InvalidArgumentException("Unsupported sessionNotFoundErrorMode [{$handlerMode}].");
         }
 
-        if ($handlerMode === self::THROW_EXCEPTION
-            || $this->sessionPool === null
-            || $this->inTransaction()
-        ) {
+        if ($handlerMode === self::THROW_EXCEPTION || $this->inTransaction()) {
             // skip handlers, transaction() will handle error by self
             return $callback();
         }

diff --git a/tests/SessionNotFoundTest.php b/tests/SessionNotFoundTest.php
@@ -31,21 +31,34 @@ private function deleteSession(Connection $connection): void
         $connection->getSpannerDatabase()->__debugInfo()['session']->delete();
     }
 
-    private function getSessionNotFoundConnection(string $sessionNotFoundErrorMode): Connection
+    private function getSessionNotFoundConnection(
+        string $sessionNotFoundErrorMode,
+        bool $useSessionPool = true,
+    ): Connection
     {
         $config = $this->app['config']->get('database.connections.main');
 
         // old behavior, just raise QueryException
         $config['sessionNotFoundErrorMode'] = $sessionNotFoundErrorMode;
 
-        $cacheItemPool = new ArrayAdapter();
-        $cacheSessionPool = new CacheSessionPool($cacheItemPool);
-        $conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool);
+        $sessionPool = $useSessionPool
+            ? new CacheSessionPool(new ArrayAdapter())
+            : null;
+
+        $conn = new Connection(
+            $config['instance'],
+            $config['database'],
+            '',
+            $config,
+            null,
+            $sessionPool,
+        );
+
         $this->setUpDatabaseOnce($conn);
         return $conn;
     }
 
-    public function testSessionNotFoundHandledError(): void
+    public function test_session_not_found_handling(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -56,7 +69,18 @@ public function testSessionNotFoundHandledError(): void
         $this->assertEquals([12345], $conn->selectOne('SELECT 12345'));
     }
 
-    public function testInTransactionSessionNotFoundHandledError(): void
+    public function test_session_not_found_without_session_pool(): void
+    {
+        $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL, false);
+
+        $conn->selectOne('SELECT 1');
+
+        $this->deleteSession($conn);
+
+        $this->assertEquals([12345], $conn->selectOne('SELECT 12345'));
+    }
+
+    public function test_session_not_found_in_transaction(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -76,7 +100,7 @@ public function testInTransactionSessionNotFoundHandledError(): void
         $this->assertEquals(2, $passes, 'Transaction should be called twice');
     }
 
-    public function testInTransactionCommitSessionNotFoundHandledError(): void
+    public function test_session_not_found_when_committing(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -94,7 +118,7 @@ public function testInTransactionCommitSessionNotFoundHandledError(): void
         $this->assertEquals(2, $passes, 'Transaction should be called twice');
     }
 
-    public function testInTransactionRollbackSessionNotFoundHandledError(): void
+    public function test_session_not_found_when_rolling_back(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -114,7 +138,7 @@ public function testInTransactionRollbackSessionNotFoundHandledError(): void
         $this->assertEquals(2, $passes, 'Transaction should be called twice');
     }
 
-    public function testNestedTransactionsSessionNotFoundHandledError(): void
+    public function test_session_not_found_on_nested_transaction(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -133,7 +157,7 @@ public function testNestedTransactionsSessionNotFoundHandledError(): void
         $this->assertEquals(2, $passes, 'Transaction should be called twice');
     }
 
-    public function testCursorSessionNotFoundHandledError(): void
+    public function test_session_not_found_on_cursor(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::CLEAR_SESSION_POOL);
 
@@ -154,7 +178,7 @@ public function testCursorSessionNotFoundHandledError(): void
         $this->assertEquals(2, $passes, 'Transaction should be called twice');
     }
 
-    public function testSessionNotFoundUnhandledError(): void
+    public function test_session_not_found_throw_exception_on_query(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);
 
@@ -178,7 +202,7 @@ public function testSessionNotFoundUnhandledError(): void
         }
     }
 
-    public function testCursorSessionNotFoundUnhandledError(): void
+    public function test_session_not_found_throw_exception_on_cursor(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);
 
@@ -202,7 +226,7 @@ public function testCursorSessionNotFoundUnhandledError(): void
         }
     }
 
-    public function testInTransactionSessionNotFoundUnhandledError(): void
+    public function test_session_not_found_throw_exception_in_transaction(): void
     {
         $conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);