feat: add Connection methods selectWithOptions and cursorWithOptions (#122)

taka-oyama · halnique · taka-oyama · commit fa17e531b648 · 2023-11-17T12:24:08.000+09:00
Co-authored-by: halnique &lt;shunsuke4dev@gmail.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # v6.0.0 [Not released Yet]
 
+Added
+- Deprecation warnings to `Connection`'s methods `cursorWithTimestampBound` `selectWithTimestampBound` `selectOneWithTimestampBound`. Use `cursorWithOptions` `selectWithOptions` instead. (#122)
+- `Connection` has new methods `selectWithOptions` `cursorWithOptions` which allows spanner specific options to be set for each query. (#122)
+
 Changed
 - [Breaking] Match `Query\Builder::forceIndex()` behavior with laravel's (`forceIndex` property no longer exists). (#114)
 
diff --git a/phpstan.neon b/phpstan.neon
@@ -19,7 +19,7 @@ parameters:
           count: 2
         - message: "#^Parameter \\#1 \\$table of method Illuminate\\\\Database\\\\Query\\\\Builder\\:\\:from\\(\\) expects Closure\\|Illuminate\\\\Database\\\\Eloquent\\\\Builder\\|Illuminate\\\\Database\\\\Query\\\\Builder\\|string, Closure\\|Illuminate\\\\Contracts\\\\Database\\\\Query\\\\Expression\\|Illuminate\\\\Database\\\\Query\\\\Builder\\|string given\\.$#"
           path: src/Connection.php
-        - message: '#^Method Colopl\\Spanner\\Connection::select\(\) should return array but returns mixed\.$#'
+        - message: '#^Method Colopl\\Spanner\\Connection::selectWithOptions\(\) should return array<int, array> but returns mixed\.$#'
           path: src/Connection.php
         - message: '#^Cannot cast mixed to int\.$#'
           path: src/Eloquent/Model.php
diff --git a/src/Concerns/ManagesStaleReads.php b/src/Concerns/ManagesStaleReads.php
@@ -20,56 +20,45 @@
 use Colopl\Spanner\TimestampBound\TimestampBoundInterface;
 use Generator;
 
+/**
+ * @deprecated This trait will be removed in v7.
+ */
 trait ManagesStaleReads
 {
     /**
+     * @deprecated use selectWithOptions() instead. This method will be removed in v7.
      * @param string $query
      * @param array<array-key, mixed> $bindings
      * @param TimestampBoundInterface|null $timestampBound
-     * @return Generator<int, list<mixed>|null>
+     * @return Generator<int, array<array-key, mixed>>
      */
     public function cursorWithTimestampBound($query, $bindings = [], TimestampBoundInterface $timestampBound = null): Generator
     {
-        return $this->run($query, $bindings, function ($query, $bindings) use ($timestampBound) {
-            if ($this->pretending()) {
-                return (static fn() => yield from [])();
-            }
-
-            $options = ['parameters' => $this->prepareBindings($bindings)];
-            if ($timestampBound) {
-                $options = array_merge($options, $timestampBound->transactionOptions());
-            }
-
-            return $this->getSpannerDatabase()
-                ->execute($query, $options)
-                ->rows();
-        });
+        return $this->cursorWithOptions($query, $bindings, $timestampBound?->transactionOptions() ?? []);
     }
 
     /**
-     * @param  string $query
-     * @param  array<array-key, mixed> $bindings
-     * @param  TimestampBoundInterface|null $timestampBound
+     * @deprecated use selectWithOptions() instead. This method will be removed in v7.
+     * @param string $query
+     * @param array<array-key, mixed> $bindings
+     * @param TimestampBoundInterface|null $timestampBound
      * @return list<array<array-key, mixed>|null>
      */
     public function selectWithTimestampBound($query, $bindings = [], TimestampBoundInterface $timestampBound = null): array
     {
-        return $this->withSessionNotFoundHandling(function () use ($query, $bindings, $timestampBound) {
-            return iterator_to_array($this->cursorWithTimestampBound($query, $bindings, $timestampBound));
-        });
+        return $this->selectWithOptions($query, $bindings, $timestampBound?->transactionOptions() ?? []);
     }
 
     /**
+     * @deprecated use selectWithOptions() instead. This method will be removed in v7.
      * @param string $query
      * @param array<array-key, mixed> $bindings
      * @param TimestampBoundInterface|null $timestampBound
      * @return array<array-key, mixed>|null
      */
     public function selectOneWithTimestampBound($query, $bindings = [], TimestampBoundInterface $timestampBound = null): ?array
     {
-        return $this->withSessionNotFoundHandling(function () use ($query, $bindings, $timestampBound) {
-            return $this->cursorWithTimestampBound($query, $bindings, $timestampBound)->current();
-        });
+        return $this->selectWithTimestampBound($query, $bindings, $timestampBound)[0] ?? null;
     }
 }
 
diff --git a/src/Connection.php b/src/Connection.php
@@ -256,32 +256,44 @@ public function query(): QueryBuilder
      */
     public function select($query, $bindings = [], $useReadPdo = true): array
     {
-        return $this->run($query, $bindings, function ($query, $bindings) {
-            if ($this->pretending()) {
-                return [];
-            }
-
-            $generator = $this->getDatabaseContext()
-                ->execute($query, ['parameters' => $this->prepareBindings($bindings)])
-                ->rows();
-
-            return iterator_to_array($generator);
-        });
+        return $this->selectWithOptions($query, $bindings, []);
     }
 
     /**
      * @inheritDoc
      */
     public function cursor($query, $bindings = [], $useReadPdo = true): Generator
     {
-        return $this->run($query, $bindings, function ($query, $bindings) {
-            if ($this->pretending()) {
-                return (static fn() => yield from [])();
-            }
+        return $this->cursorWithOptions($query, $bindings, []);
+    }
 
-            return $this->getDatabaseContext()
-                ->execute($query, ['parameters' => $this->prepareBindings($bindings)])
-                ->rows();
+    /**
+     * @param string $query
+     * @param array<array-key, mixed> $bindings
+     * @param array<string, mixed> $options
+     * @return array<int, array<array-key, mixed>>
+     */
+    public function selectWithOptions(string $query, array $bindings, array $options): array
+    {
+        return $this->run($query, $bindings, function ($query, $bindings) use ($options): array {
+            return !$this->pretending()
+                ? iterator_to_array($this->executeQuery($query, $bindings, $options))
+                : [];
+        });
+    }
+
+    /**
+     * @param string $query
+     * @param array<array-key, mixed> $bindings
+     * @param array<string, mixed> $options
+     * @return Generator<int, array<array-key, mixed>>
+     */
+    public function cursorWithOptions(string $query, array $bindings, array $options): Generator
+    {
+        return $this->run($query, $bindings, function ($query, $bindings) use ($options): Generator {
+            return !$this->pretending()
+                ? $this->executeQuery($query, $bindings, $options)
+                : (static fn() => yield from [])();
         });
     }
 
@@ -566,6 +578,21 @@ protected function withSessionNotFoundHandling(Closure $callback): mixed
         }
     }
 
+    /**
+     * @param string $query
+     * @param array<array-key, mixed> $bindings
+     * @param array<string, mixed> $options
+     * @return Generator<int, array<array-key, mixed>>
+     */
+    protected function executeQuery(string $query, array $bindings, array $options): Generator
+    {
+        $options += ['parameters' => $this->prepareBindings($bindings)];
+
+        return $this->getDatabaseContext()
+            ->execute($query, $options)
+            ->rows();
+    }
+
     /**
      * Check if this is "session not found" error
      *
diff --git a/src/Query/Builder.php b/src/Query/Builder.php
@@ -132,12 +132,14 @@ protected function prepareInsertForDml($values)
      */
     protected function runSelect()
     {
+        $sql = $this->toSql();
+        $bindings = $this->getBindings();
+        $options = [];
+
         if ($this->timestampBound !== null) {
-            return $this->connection->selectWithTimestampBound(
-                $this->toSql(), $this->getBindings(), $this->timestampBound
-            );
+            $options += $this->timestampBound->transactionOptions();
         }
 
-        return parent::runSelect();
+        return $this->connection->selectWithOptions($sql, $bindings, $options); 
     }
 }
diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
@@ -25,9 +25,11 @@
 use Colopl\Spanner\TimestampBound\MinReadTimestamp;
 use Colopl\Spanner\TimestampBound\ReadTimestamp;
 use Colopl\Spanner\TimestampBound\StrongRead;
+use Generator;
 use Google\Auth\FetchAuthTokenInterface;
 use Google\Cloud\Core\Exception\AbortedException;
 use Google\Cloud\Core\Exception\NotFoundException;
+use Google\Cloud\Spanner\Duration;
 use Google\Cloud\Spanner\KeySet;
 use Google\Cloud\Spanner\Session\CacheSessionPool;
 use Google\Cloud\Spanner\SpannerClient;
@@ -38,7 +40,6 @@
 use Illuminate\Database\Events\TransactionCommitted;
 use Illuminate\Support\Carbon;
 use Illuminate\Support\Facades\Event;
-use LogicException;
 use RuntimeException;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 use function dirname;
@@ -59,7 +60,7 @@ public function testReconnect(): void
     {
         $conn = $this->getDefaultConnection();
         $conn->reconnect();
-        $this->assertEquals([12345], $conn->selectOne('SELECT 12345'));
+        $this->assertSame([12345], $conn->selectOne('SELECT 12345'));
     }
 
     public function testQueryLog(): void
@@ -74,6 +75,107 @@ public function testQueryLog(): void
         $this->assertCount(2, $conn->getQueryLog());
     }
 
+    public function test_select(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $values = $conn->select('SELECT 12345');
+        $this->assertCount(1, $values);
+        $this->assertSame(12345, $values[0][0]);
+    }
+
+    public function test_selectWithOptions(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $conn->table(self::TABLE_NAME_USER)->insert(['userId' => $this->generateUuid(), 'name' => __FUNCTION__]);
+        $values = $conn->selectWithOptions('SELECT * FROM ' . self::TABLE_NAME_USER, [], ['exactStaleness' => new Duration(10)]);
+        $this->assertEmpty($values);
+    }
+
+    public function test_cursorWithOptions(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $conn->table(self::TABLE_NAME_USER)->insert(['userId' => $this->generateUuid(), 'name' => __FUNCTION__]);
+        $cursor = $conn->cursorWithOptions('SELECT * FROM ' . self::TABLE_NAME_USER, [], ['exactStaleness' => new Duration(10)]);
+        $this->assertInstanceOf(Generator::class, $cursor);
+        $this->assertNull($cursor->current());
+    }
+
+    public function test_statement_with_select(): void
+    {
+        $executedCount = 0;
+        $this->app['events']->listen(QueryExecuted::class, function () use (&$executedCount) { $executedCount++; });
+
+        $conn = $this->getDefaultConnection();
+        $res = $conn->statement('SELECT ?', ['12345']);
+
+        $this->assertTrue($res);
+        $this->assertSame(1, $executedCount);
+    }
+
+    public function test_statement_with_dml(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $userId = $this->generateUuid();
+        $executedCount = 0;
+        $this->app['events']->listen(QueryExecuted::class, function () use (&$executedCount) { $executedCount++; });
+
+        $res[] = $conn->statement('INSERT '.self::TABLE_NAME_USER.' (`userId`, `name`) VALUES (?,?)', [$userId, __FUNCTION__]);
+        $res[] = $conn->statement('UPDATE '.self::TABLE_NAME_USER.' SET `name`=? WHERE `userId`=?', [__FUNCTION__.'2', $userId]);
+        $res[] = $conn->statement('DELETE '.self::TABLE_NAME_USER.' WHERE `userId`=?', [$this->generateUuid()]);
+
+        $this->assertTrue($res[0]);
+        $this->assertTrue($res[1]);
+        $this->assertTrue($res[2]);
+        $this->assertSame(3, $executedCount);
+    }
+
+    public function test_unprepared_with_select(): void
+    {
+        $executedCount = 0;
+        $this->app['events']->listen(QueryExecuted::class, function () use (&$executedCount) { $executedCount++; });
+
+        $conn = $this->getDefaultConnection();
+        $res = $conn->unprepared('SELECT 12345');
+
+        $this->assertTrue($res);
+        $this->assertSame(1, $executedCount);
+    }
+
+    public function test_unprepared_with_dml(): void
+    {
+        $conn = $this->getDefaultConnection();
+        $userId = $this->generateUuid();
+        $executedCount = 0;
+        $this->app['events']->listen(QueryExecuted::class, function () use (&$executedCount) { $executedCount++; });
+
+        $res[] = $conn->unprepared('INSERT '.self::TABLE_NAME_USER.' (`userId`, `name`) VALUES (\''.$userId.'\',\''.__FUNCTION__.'\')');
+        $res[] = $conn->unprepared('UPDATE '.self::TABLE_NAME_USER.' SET `name`=\''.__FUNCTION__.'2'.'\' WHERE `userId`=\''.$userId.'\'');
+        $res[] = $conn->unprepared('DELETE '.self::TABLE_NAME_USER.' WHERE `userId`=\''.$userId.'\'');
+
+        $this->assertTrue($res[0]);
+        $this->assertTrue($res[1]);
+        $this->assertTrue($res[2]);
+        $this->assertSame(3, $executedCount);
+    }
+
+    public function test_pretend(): void
+    {
+        $executedCount = 0;
+        $this->app['events']->listen(QueryExecuted::class, function () use (&$executedCount) { $executedCount++; });
+
+        $resSelect = null;
+        $resInsert = null;
+        $conn = $this->getDefaultConnection();
+        $conn->pretend(function(Connection $conn) use (&$resSelect, &$resInsert) {
+            $resSelect = $conn->select('SELECT 12345');
+            $resInsert = $conn->table(self::TABLE_NAME_USER)->insert(['userId' => $this->generateUuid(), 'name' => __FUNCTION__]);
+        });
+
+        $this->assertSame([], $resSelect);
+        $this->assertTrue($resInsert);
+        $this->assertSame(2, $executedCount);
+    }
+
     public function testInsertUsingMutationWithTransaction(): void
     {
         Event::fake();
diff --git a/tests/Query/BuilderTest.php b/tests/Query/BuilderTest.php

Original file line number	Diff line number	Diff line change
`@@ -132,12 +132,14 @@ protected function prepareInsertForDml($values)`
`132`	`132`	`*/`
`133`	`133`	`protected function runSelect()`
`134`	`134`	`{`
	`135`	`+ $sql = $this->toSql();`
	`136`	`+ $bindings = $this->getBindings();`
	`137`	`+ $options = [];`
	`138`	`+`
`135`	`139`	`if ($this->timestampBound !== null) {`
`136`		`- return $this->connection->selectWithTimestampBound(`
`137`		`- $this->toSql(), $this->getBindings(), $this->timestampBound`
`138`		`- );`
	`140`	`+ $options += $this->timestampBound->transactionOptions();`
`139`	`141`	`}`
`140`	`142`
`141`		`- return parent::runSelect();`
	`143`	`+ return $this->connection->selectWithOptions($sql, $bindings, $options);`
`142`	`144`	`}`
`143`	`145`	`}`