chore(Spanner): TransactionOptions refactor (#8681)

Author: bshaffer

Spanner/src/Batch/BatchClient.php

@@ -17,12 +17,12 @@
 
 namespace Google\Cloud\Spanner\Batch;
 
+use Google\Cloud\Core\ArrayTrait;
 use Google\Cloud\Core\TimeTrait;
 use Google\Cloud\Spanner\Operation;
 use Google\Cloud\Spanner\Session\SessionCache;
 use Google\Cloud\Spanner\Timestamp;
-use Google\Cloud\Spanner\TransactionConfigurationTrait;
-use Google\Protobuf\Duration;
+use Google\Cloud\Spanner\TransactionOptionsBuilder;
 
 /**
  * Provides Batch APIs used to read data from a Cloud Spanner database.
@@ -102,7 +102,7 @@
 class BatchClient
 {
     use TimeTrait;
-    use TransactionConfigurationTrait;
+    use ArrayTrait;
 
     const PARTITION_TYPE_KEY = '__partitionTypeName';
     private const ALLOWED_PARTITION_TYPES = [
@@ -145,22 +145,17 @@ public function __construct(
      */
     public function snapshot(array $options = [])
     {
-        $options += [
-            'transactionOptions' => [],
-        ];
-
         // Single Use transactions are not supported in batch mode.
         $options['transactionOptions']['singleUse'] = false;
+        $options['transactionOptions']['returnReadTimestamp'] = true;
 
-        $transactionOptions = $this->pluck('transactionOptions', $options);
-        $transactionOptions['returnReadTimestamp'] = true;
-
-        $transactionOptions = $this->configureReadOnlyTransactionOptions($transactionOptions);
+        $txnOptions = (new TransactionOptionsBuilder())
+            ->configureReadOnlyTransactionOptions($options['transactionOptions']);
 
         /** @var BatchSnapshot */
         return $this->operation->snapshot($this->session, [
             'className' => BatchSnapshot::class,
-            'transactionOptions' => $transactionOptions
+            'transactionOptions' => $txnOptions
         ] + $options);
     }
 

Spanner/src/Database.php

@@ -88,7 +88,6 @@
 class Database
 {
     use ApiHelperTrait;
-    use TransactionConfigurationTrait;
     use RequestTrait;
 
     public const CONTEXT_READ = 'r';
@@ -128,6 +127,7 @@ class Database
     private CacheItemPoolInterface $cacheItemPool;
     private array $info;
     private int $isolationLevel;
+    private TransactionOptionsBuilder $transactionOptionsBuilder;
 
     /**
      * Create an object representing a Database.
@@ -185,6 +185,7 @@ public function __construct(
         );
 
         $this->optionsValidator = new OptionsValidator($serializer);
+        $this->transactionOptionsBuilder = new TransactionOptionsBuilder();
         $this->directedReadOptions = $instance->directedReadOptions();
     }
 
@@ -747,25 +748,13 @@ public function snapshot(array $options = []): TransactionalReadInterface
             throw new BadMethodCallException('Nested transactions are not supported by this client.');
         }
 
-        $options += [
-            'singleUse' => false
+        $snapshotOptions = [
+            'singleUse' => $options['singleUse'] ?? false,
+            'transactionOptions' => $this->transactionOptionsBuilder
+                ->configureReadOnlyTransactionOptions($options),
         ];
 
-        $options['transactionOptions'] = $this->configureReadOnlyTransactionOptions($options);
-
-        // For backwards compatibility - remove all PBReadOnly fields
-        // This was previously being done in configureReadOnlyTransactionOptions
-        // @TODO: clean this up
-        unset(
-            $options['returnReadTimestamp'],
-            $options['strong'],
-            $options['readTimestamp'],
-            $options['exactStaleness'],
-            $options['minReadTimestamp'],
-            $options['maxStaleness'],
-        );
-
-        return $this->operation->snapshot($this->session, $options);
+        return $this->operation->snapshot($this->session, $snapshotOptions);
     }
 
     /**
@@ -814,9 +803,11 @@ public function transaction(array $options = []): Transaction
         }
 
         // Configure readWrite options here. Any nested options for readWrite should be added to this call
-        $options['transactionOptions'] = $this->configureReadWriteTransactionOptions(
-            ($options['transactionOptions'] ?? []) + ['isolationLevel' => $this->isolationLevel]
-        );
+        $txnOptions = $options['transactionOptions'] ?? [];
+        $options['transactionOptions'] = $this->transactionOptionsBuilder
+            ->configureReadWriteTransactionOptions($txnOptions + [
+                'isolationLevel' => $this->isolationLevel
+            ]);
 
         return $this->operation->transaction($this->session, $options);
     }
@@ -920,10 +911,11 @@ public function runTransaction(callable $operation, array $options = []): mixed
             : $retrySettings['maxRetries'];
 
         // Configure necessary readWrite nested and base options
-        $transactionOptions = $options['transactionOptions'] ?? [];
-        $options['transactionOptions'] = $this->configureReadWriteTransactionOptions(
-            $transactionOptions + ['isolationLevel' => $this->isolationLevel]
-        );
+        $txnOptions = $options['transactionOptions'] ?? [];
+        $options['transactionOptions'] = $this->transactionOptionsBuilder
+            ->configureReadWriteTransactionOptions($txnOptions + [
+                'isolationLevel' => $this->isolationLevel
+            ]);
 
         $attempt = 0;
         $startTransactionFn = function ($options) use (&$attempt) {
@@ -1666,28 +1658,23 @@ public function delete(string $table, KeySet $keySet, array $options = []): Time
      */
     public function execute($sql, array $options = []): Result
     {
-        unset($options['requestOptions']['transactionTag']);
-        $session = $this->pluck('session', $options, false)
-            ?: $this->session;
-
-        list(
-            $options['transaction'],
-            $options['transactionContext']
-        ) = $this->transactionSelector($options);
-
         if (isset($options['transaction']['readWrite'])) {
             $options['transaction']['begin']['isolationLevel'] ??= $this->isolationLevel;
         }
 
-        $options['directedReadOptions'] = $this->configureDirectedReadOptions(
-            $options,
-            $this->directedReadOptions
+        [$txnOptions, $txnContext] = $this->transactionOptionsBuilder->transactionSelector($options);
+        $directedReadOptions = $this->transactionOptionsBuilder->configureDirectedReadOptions(
+            ['transaction' => $txnOptions] + $options,
+            $this->directedReadOptions,
         );
 
-        // Unset the internal flag.
-        unset($options['singleUse']);
-        return $this->operation->execute($session, $sql, $options + [
-            'route-to-leader' => $options['transactionContext'] === Database::CONTEXT_READWRITE
+        $session = $options['session'] ?? $this->session;
+        $executeOptions = $this->pluckArray(['parameters', 'types'], $options);
+        return $this->operation->execute($session, $sql, $executeOptions + [
+            'transaction' => $txnOptions,
+            'transactionContext' => $txnContext,
+            'directedReadOptions' => $directedReadOptions,
+            'route-to-leader' => $txnContext === Database::CONTEXT_READWRITE
         ]);
     }
 
@@ -1903,8 +1890,6 @@ public function batchWrite(array $mutationGroups, array $options = []): Generato
      */
     public function executePartitionedUpdate($statement, array $options = []): int
     {
-        unset($options['requestOptions']['transactionTag']);
-
         if (isset($options['transactionOptions']['isolationLevel'])) {
             throw new ValidationException('Partitioned DML cannot be configured with an isolation level');
         }
@@ -1915,11 +1900,13 @@ public function executePartitionedUpdate($statement, array $options = []): int
             ]
         ];
 
+        unset($options['requestOptions']['transactionTag']);
         if (isset($options['transactionOptions']['excludeTxnFromChangeStreams'])) {
             $beginTransactionOptions['transactionOptions']['excludeTxnFromChangeStreams'] =
                 $options['transactionOptions']['excludeTxnFromChangeStreams'];
             unset($options['transactionOptions']);
         }
+
         $transaction = $this->operation->transaction($this->session, $beginTransactionOptions);
 
         return $this->operation->executeUpdate($this->session, $transaction, $statement, [
@@ -2053,21 +2040,23 @@ public function executePartitionedUpdate($statement, array $options = []): int
      */
     public function read($table, KeySet $keySet, array $columns, array $options = []): Result
     {
-        unset($options['requestOptions']['transactionTag']);
-
-        list($transactionOptions, $context) = $this->transactionSelector($options);
-        $options['transaction'] = $transactionOptions;
-        $options['transactionContext'] = $context;
+        [$txnOptions, $txnContext] = $this->transactionOptionsBuilder->transactionSelector($options);
 
-        $options['directedReadOptions'] = $this->configureDirectedReadOptions(
-            $options,
-            $this->directedReadOptions
+        $readOptions = $this->pluckArray(
+            ['index', 'limit', 'orderBy', 'lockHint', 'directedReadOptions'],
+            $options
         );
+        $readOptions += [
+            'transactionContext' => $txnContext,
+            'directedReadOptions' => $this->transactionOptionsBuilder->configureDirectedReadOptions(
+                ['transaction' => $txnOptions] + $readOptions,
+                $this->directedReadOptions
+            ),
+            'transaction' => $txnOptions,
+        ];
 
-        // Unset the internal flag.
-        unset($options['singleUse']);
-        return $this->operation->read($this->session, $table, $keySet, $columns, $options + [
-            'route-to-leader' => $context === Database::CONTEXT_READ
+        return $this->operation->read($this->session, $table, $keySet, $columns, $readOptions + [
+            'route-to-leader' => $txnContext === Database::CONTEXT_READ
         ]);
     }
 
@@ -2146,7 +2135,7 @@ public function createDatabaseFromBackup($name, $backup, array $options = []): L
     {
         $options += [
             'parent' => $this->instance->name(),
-            'databaseId' => $this->databaseIdOnly($name),
+            'databaseId' => $this->databaseId($name),
             'backup' => $backup instanceof Backup ? $backup->name() : $backup
         ];
         /**
@@ -2368,7 +2357,7 @@ private function getCreateDbStatement(int $dialect): string
      * @param string $name The database name or id.
      * @return string
      */
-    private function databaseIdOnly(string $name): string
+    private function databaseId(string $name): string
     {
         try {
             return DatabaseAdminClient::parseName($name)['database'];

Spanner/src/Instance.php

@@ -119,7 +119,7 @@ public function __construct(
         private string $name,
         array $options = [],
     ) {
-        $this->name = $this->fullyQualifiedInstanceName($name, $projectId);
+        $this->name = $this->fullyQualifiedInstanceName($name);
         $this->directedReadOptions = $options['directedReadOptions'] ?? [];
         $this->isolationLevel = $options['isolationLevel'] ?? IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED;
         $this->routeToLeader = $options['routeToLeader'] ?? true;
@@ -545,21 +545,10 @@ public function database(string $name, array $options = []): Database
             'isolationLevel',
         ]);
 
-        try {
-            $instance = DatabaseAdminClient::parseName($this->name())['instance'];
-            $databaseName = GapicSpannerClient::databaseName(
-                $this->projectId,
-                $instance,
-                $name
-            );
-        } catch (ValidationException $e) {
-            $databaseName = $name;
-        }
-
         if (!$session = $options['session'] ?? null) {
             $session = new SessionCache(
                 $this->spannerClient,
-                $databaseName,
+                $this->fullyQualifiedDatabaseName($name),
                 [
                     'databaseRole' => $options['databaseRole'] ?? '',
                     'lock' => $options['lock'] ?? null,
@@ -837,17 +826,36 @@ public function iam(): IamManager
      * Convert the simple instance name to a fully qualified name.
      *
      * @param string $name The instance name.
-     * @param string $project The project ID.
      * @return string
      */
-    private function fullyQualifiedInstanceName(string $name, string $project): string
+    private function fullyQualifiedInstanceName(string $name): string
     {
         return InstanceAdminClient::instanceName(
-            $project,
+            $this->projectId,
             $name
         );
     }
 
+    /**
+     * Convert the simple database name to a fully qualified name.
+     *
+     * @param string $databaseName The database name.
+     * @return string
+     */
+    private function fullyQualifiedDatabaseName(string $databaseName): string
+    {
+        try {
+            $instance = DatabaseAdminClient::parseName($this->name())['instance'];
+            return GapicSpannerClient::databaseName(
+                $this->projectId,
+                $instance,
+                $databaseName
+            );
+        } catch (ValidationException $e) {
+            return $databaseName;
+        }
+    }
+
     /**
      * Return the directed read options.
      *