fix: backport #150 and #166 to 5.x branch (#242)

* fix: backport #150 * fix: backport #166 * fix: phpstan error * fix: remove broken test * fix: docs * fix: github workflow * fix release date
colopl · Nov 20, 2024 · d62cae5 · d62cae5
1 parent 3a9880b
commit d62cae5
Show file tree

Hide file tree

Showing 13 changed files with 149 additions and 86 deletions.
diff --git a/.github/workflows/run-test.yml b/.github/workflows/run-test.yml
@@ -11,5 +11,10 @@ jobs:
         uses: actions/checkout@v3
       - run: echo "💡 The ${{ github.repository }} repository has been cloned to the runner."
       - run: echo "🖥️ The workflow is now ready to test your code on the runner."
-      - name: Run test with docker compose
-        run: docker-compose run test
+      - name: Build docker image
+        run: make build
+      - name: Run test
+        run: make test
+      - name: Show failed container logs
+        if: failure()
+        run: make logs
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# v5.4.0 (2024-11-20)
+
+Fixed
+- Match internals so that it lines up with laravel 10.34.0. (#242)
+
 # v5.3.0 (2023-11-17)
 
 Fixed

diff --git a/composer.json b/composer.json
@@ -11,7 +11,7 @@
     "php": "^8.1",
     "ext-grpc": "*",
     "ext-json": "*",
-    "laravel/framework": "^10.32.0",
+    "laravel/framework": "^10.34.2",
     "google/cloud-spanner": "^1.58.4",
     "grpc/grpc": "^1.42",
     "symfony/cache": "~6",

diff --git a/src/Concerns/ManagesTransactions.php b/src/Concerns/ManagesTransactions.php
@@ -141,16 +141,20 @@ protected function performSpannerCommit(): void
             $this->currentTransaction->commit();
         }
 
-        $this->transactionsManager?->stageTransactions($this->getName());
+        [$levelBeingCommitted, $this->transactions] = [
+            $this->transactions,
+            max(0, $this->transactions - 1),
+        ];
 
-        $this->transactions = max(0, $this->transactions - 1);
         if ($this->isTransactionFinished()) {
             $this->currentTransaction = null;
         }
 
-        if ($this->afterCommitCallbacksShouldBeExecuted()) {
-            $this->transactionsManager?->commit($this->getName());
-        }
+        $this->transactionsManager?->commit(
+            $this->getName(),
+            $levelBeingCommitted,
+            $this->transactions
+        );
     }
 
     /**
@@ -164,7 +168,7 @@ protected function performRollBack($toLevel)
 
         if ($this->currentTransaction !== null) {
             try {
-                if ($this->currentTransaction->state() === Transaction::STATE_ACTIVE) {
+                if ($this->currentTransaction->state() === Transaction::STATE_ACTIVE && $this->currentTransaction->id() !== null) {
                     $this->currentTransaction->rollBack();
                 }
             } finally {

diff --git a/src/Connection.php b/src/Connection.php
@@ -440,7 +440,7 @@ protected function escapeArray(array $value, bool $binary): string
     {
         if (array_is_list($value)) {
             $escaped = array_map(function (mixed $v) use ($binary): string {
-                return !is_array($v)
+                return is_scalar($v)
                     ? $this->escape($v, $binary)
                     : throw new LogicException('Nested arrays are not supported by Cloud Spanner');
             }, $value);

diff --git a/src/Query/Concerns/UsesMutations.php b/src/Query/Concerns/UsesMutations.php
@@ -31,7 +31,7 @@ trait UsesMutations
      */
     public function insertUsingMutation(array $values)
     {
-        $this->connection->insertUsingMutation($this->from, $values);
+        $this->connection->insertUsingMutation($this->getTableName(), $values);
     }
 
     /**
@@ -40,7 +40,7 @@ public function insertUsingMutation(array $values)
      */
     public function updateUsingMutation(array $values)
     {
-        $this->connection->updateUsingMutation($this->from, $values);
+        $this->connection->updateUsingMutation($this->getTableName(), $values);
     }
 
     /**
@@ -49,7 +49,7 @@ public function updateUsingMutation(array $values)
      */
     public function insertOrUpdateUsingMutation(array $values)
     {
-        $this->connection->insertOrUpdateUsingMutation($this->from, $values);
+        $this->connection->insertOrUpdateUsingMutation($this->getTableName(), $values);
     }
 
     /**
@@ -58,6 +58,14 @@ public function insertOrUpdateUsingMutation(array $values)
      */
     public function deleteUsingMutation($keys)
     {
-        $this->connection->deleteUsingMutation($this->from, $keys);
+        $this->connection->deleteUsingMutation($this->getTableName(), $keys);
+    }
+
+    /**
+     * @return string
+     */
+    private function getTableName(): string
+    {
+        return (string)$this->getGrammar()->getValue($this->from);
     }
 }
diff --git a/src/Query/Processor.php b/src/Query/Processor.php
@@ -52,6 +52,27 @@ public function processColumnListing($results)
         }, $results);
     }
 
+    /**
+     * Process the results of a columns query.
+     *
+     * @inheritDoc
+     */
+    public function processColumns($results)
+    {
+        return array_map(static function (array $result) {
+            return [
+                'name' => $result['COLUMN_NAME'],
+                'type_name' => preg_replace("/\([^)]+\)/", "", $result['SPANNER_TYPE']),
+                'type' => $result['SPANNER_TYPE'],
+                'collation' => null,
+                'nullable' => $result['IS_NULLABLE'] !== 'NO',
+                'default' => $result['COLUMN_DEFAULT'],
+                'auto_increment' => false,
+                'comment' => null,
+            ];
+        }, $results);
+    }
+
     /**
      * @param array $results
      * @return array

diff --git a/src/Schema/Builder.php b/src/Schema/Builder.php
@@ -35,6 +35,8 @@ class Builder extends BaseBuilder
 
 
     /**
+     * @deprecated Will be removed in a future Laravel version.
+     *
      * @return list<array{ name: string, type: string }>
      */
     public function getAllTables()
@@ -44,20 +46,6 @@ public function getAllTables()
         );
     }
 
-    /**
-     * @inheritDoc
-     */
-    public function getColumnListing($table)
-    {
-        $table = $this->connection->getTablePrefix().$table;
-
-        $results = $this->connection->select(
-            $this->grammar->compileColumnListing(), [$table]
-        );
-
-        return $this->connection->getPostProcessor()->processColumnListing($results);
-    }
-
     /**
      * @param string $table
      * @return string[]

diff --git a/src/Schema/Grammar.php b/src/Schema/Grammar.php
@@ -40,6 +40,10 @@ class Grammar extends BaseGrammar
     protected $modifiers = ['Nullable', 'Default'];
 
     /**
+     * Compile the query to determine if a table exists.
+     *
+     * @deprecated Will be removed in a future Laravel version.
+     *
      * @return string
      */
     public function compileTableExists()
@@ -48,6 +52,18 @@ public function compileTableExists()
     }
 
     /**
+     * Compile the query to determine the tables.
+     *
+     * @return string
+     */
+    public function compileTables()
+    {
+        return 'select `table_name` as name from information_schema.tables where table_schema = \'\' and table_type = \'BASE TABLE\'';
+    }
+
+    /**
+     * @deprecated Will be removed in a future Laravel version.
+     *
      * @return string
      */
     public function compileGetAllTables()
@@ -56,6 +72,10 @@ public function compileGetAllTables()
     }
 
     /**
+     * Compile the query to determine the list of columns.
+     *
+     * @deprecated Will be removed in a future Laravel version.
+     *
      * @return string
      */
     public function compileColumnListing()
@@ -73,6 +93,20 @@ public function compileIndexListing()
         return 'select index_name as `index_name` from information_schema.indexes where table_schema = \'\' and table_name = ?';
     }
 
+    /**
+     * Compile the query to determine the columns.
+     *
+     * @param  string  $table
+     * @return string
+     */
+    public function compileColumns($table)
+    {
+        return sprintf(
+            'select * from information_schema.columns where table_schema = \'\' and table_name = %s',
+            $this->quoteString($table),
+        );
+    }
+
     /**
      * Compile a create table command.
      *

diff --git a/tests/Query/BuilderTest.php b/tests/Query/BuilderTest.php
@@ -768,7 +768,7 @@ public function test_whereLike(): void
             $caughtException = $ex;
         }
         if (getenv('SPANNER_EMULATOR_HOST')) {
-            $this->assertStringContainsString('INTERNAL', $caughtException?->getMessage());
+            $this->assertStringContainsString('Invalid UTF-8', $caughtException?->getMessage());
         } else {
             $this->assertStringContainsString('Invalid request proto: an error was encountered during deserialization of the request proto.', $caughtException?->getMessage());
         }

diff --git a/tests/Schema/BuilderTest.php b/tests/Schema/BuilderTest.php
@@ -187,23 +187,67 @@ public function testCreateInterleavedTable(): void
         $this->assertTrue($sb->hasTable(self::TABLE_NAME_RELATION_CHILD_INTERLEAVED));
     }
 
+    public function test_getTables(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $sb = $conn->getSchemaBuilder();
+        $table = $this->generateTableName(class_basename(__CLASS__));
+
+        $sb->create($table, function (Blueprint $table) {
+            $table->uuid('id');
+            $table->primary('id');
+        });
+
+        /** @var array{ name: string, type: string } $row */
+        $row = Arr::first(
+            $sb->getTables(),
+            static fn (array $row): bool => $row['name'] === $table,
+        );
+
+        $this->assertSame($table, $row['name']);
+    }
+
+    public function test_getColumns(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $sb = $conn->getSchemaBuilder();
+        $table = $this->generateTableName(class_basename(__CLASS__));
+
+        $sb->create($table, function (Blueprint $table) {
+            $table->uuid('id');
+            $table->primary('id');
+        });
+
+        $this->assertSame([
+            'name' => 'id',
+            'type_name' => 'STRING',
+            'type' => 'STRING(36)',
+            'collation' => null,
+            'nullable' => false,
+            'default' => null,
+            'auto_increment' => false,
+            'comment' => null,
+        ], Arr::first($sb->getColumns($table)));
+    }
+
     public function test_getAllTables(): void
     {
         $conn = $this->getDefaultConnection();
         $sb = $conn->getSchemaBuilder();
+        $table = $this->generateTableName(class_basename(__CLASS__));
 
-        $sb->create(self::TABLE_NAME_CREATED, function (Blueprint $table) {
+        $sb->create($table, function (Blueprint $table) {
             $table->uuid('id');
             $table->primary('id');
         });
 
         /** @var array{ name: string, type: string } $row */
         $row = Arr::first(
             $sb->getAllTables(),
-            static fn (array $row): bool => $row['name'] === self::TABLE_NAME_CREATED,
+            static fn (array $row): bool => $row['name'] === $table,
         );
 
-        $this->assertSame(self::TABLE_NAME_CREATED, $row['name']);
+        $this->assertSame($table, $row['name']);
         $this->assertSame('BASE TABLE', $row['type']);
     }
 }
diff --git a/tests/SessionNotFoundTest.php b/tests/SessionNotFoundTest.php
@@ -164,13 +164,13 @@ public function test_session_not_found_on_cursor(): void
         $passes = 0;
 
         $conn->transaction(function () use ($conn, &$passes) {
-            $cursor = $conn->cursor('SELECT 12345');
-
             if ($passes === 0) {
                 $this->deleteSession($conn);
                 $passes++;
             }
 
+            $cursor = $conn->cursor('SELECT 12345');
+
             $this->assertEquals([12345], $cursor->current());
 
             $passes++;
@@ -201,56 +201,4 @@ public function test_session_not_found_throw_exception_on_query(): void
             throw $e;
         }
     }
-
-    public function test_session_not_found_throw_exception_on_cursor(): void
-    {
-        $conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);
-
-        $cursor = $conn->cursor('SELECT 1');
-
-        // deliberately delete session on spanner side
-        $this->deleteSession($conn);
-
-        $this->expectException(NotFoundException::class);
-
-        // the string is used in sessionNotFoundWrapper() to catch 'session not found' error,
-        // if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
-        $this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);
-
-        try {
-            iterator_to_array($cursor);
-        } catch (NotFoundException $e) {
-            $conn->disconnect();
-            $conn->clearSessionPool();
-            throw $e;
-        }
-    }
-
-    public function test_session_not_found_throw_exception_in_transaction(): void
-    {
-        $conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);
-
-        $this->expectException(NotFoundException::class);
-
-        // the string is used in sessionNotFoundWrapper() to catch 'session not found' error,
-        // if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
-        $this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);
-
-        $passes = 0;
-
-        try {
-            $conn->transaction(function () use ($conn, &$passes) {
-                if ($passes === 0) {
-                    $this->deleteSession($conn);
-                    $passes++;
-                }
-
-                $conn->selectOne('SELECT 12345');
-            });
-        } catch (NotFoundException $e) {
-            $conn->disconnect();
-            $conn->clearSessionPool();
-            throw $e;
-        }
-    }
 }