Spanner: untangle options vars and traits

Spanner/src/Batch/BatchClient.php

@@ -187,7 +187,7 @@ public function snapshot(array $options = [])
         $transactionOptions = $this->pluck('transactionOptions', $options);
         $transactionOptions['returnReadTimestamp'] = true;
 
-        $transactionOptions = $this->configureSnapshotOptions($transactionOptions);
+        $transactionOptions = $this->configureReadOnlyTransactionOptions($transactionOptions);
 
         if ($this->databaseRole !== null) {
             $sessionOptions['creator_role'] = $this->databaseRole;

Spanner/src/Database.php

@@ -758,7 +758,20 @@ public function snapshot(array $options = [])
             'singleUse' => false
         ];
 
-        $options['transactionOptions'] = $this->configureSnapshotOptions($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'],
+        );
+
         $options['directedReadOptions'] = $this->configureDirectedReadOptions(
             $options,
             $this->directedReadOptions ?? []
@@ -825,7 +838,7 @@ public function transaction(array $options = [])
             throw new \BadMethodCallException('Nested transactions are not supported by this client.');
         }
 
-        $options['transactionOptions'] = $this->configureTransactionOptions([
+        $options['transactionOptions'] = $this->configureReadWriteTransactionOptions([
             'isolationLevel' => $options['isolationLevel'] ?? $this->isolationLevel
         ]);
 
@@ -935,7 +948,9 @@ public function runTransaction(callable $operation, array $options = [])
         ];
 
         // There isn't anything configurable here.
-        $options['transactionOptions'] = $this->configureTransactionOptions($options['transactionOptions'] ?? []);
+        $options['transactionOptions'] = $this->configureReadWriteTransactionOptions(
+            $options['transactionOptions'] ?? []
+        );
 
         $session = $this->selectSession(
             SessionPoolInterface::CONTEXT_READWRITE,
@@ -1917,8 +1932,7 @@ public function batchWrite(array $mutationGroups, array $options = [])
      *           Please note, if using the `priority` setting you may utilize the constants available
      *           on {@see \Google\Cloud\Spanner\V1\RequestOptions\Priority} to set a value.
      *           Please note, the `transactionTag` setting will be ignored as it is not supported for partitioned DML.
-     *     @type array $transactionOptions Transaction options.
-     *           {@see V1\TransactionOptions}
+     *     @type array $transactionOptions Transaction options ({@see V1\TransactionOptions}).
      * }
      * @return int The number of rows modified.
      */

Spanner/src/Instance.php

@@ -154,8 +154,8 @@ class Instance
      *           {@see \Google\Cloud\Spanner\V1\DirectedReadOptions}
      *           If using the `replicaSelection::type` setting, utilize the constants available in
      *           {@see \Google\Cloud\Spanner\V1\DirectedReadOptions\ReplicaSelection\Type} to set a value.
-     * @param int $isolationLevel The level of Isolation for the transactions executed by this Client's instance.
-     *           **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED
+     *     @type int $isolationLevel The level of Isolation for the transactions executed by this
+     *           Client's instance. **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED.
      * }
      */
     public function __construct(
@@ -524,6 +524,8 @@ public function createDatabaseFromBackup($name, $backup, array $options = [])
      *     @type SessionPoolInterface $sessionPool A pool used to manage
      *           sessions.
      *     @type string $databaseRole The user created database role which creates the session.
+     *     @type int $isolationLevel The level of Isolation for the transactions executed by this
+     *           Client's instance. **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED.
      * }
      * @return Database
      */

Spanner/src/Operation.php

@@ -195,38 +195,37 @@ public function rollback(Session $session, $transactionId, array $options = [])
      */
     public function execute(Session $session, $sql, array $options = [])
     {
-        $options += [
+        $transactionSelector = $options + [
             'parameters' => [],
             'types' => [],
             'transactionContext' => null
         ];
 
-        $parameters = $this->pluck('parameters', $options);
-        $types = $this->pluck('types', $options);
-        $options += $this->mapper->formatParamsForExecuteSql($parameters, $types);
+        $parameters = $this->pluck('parameters', $transactionSelector);
+        $types = $this->pluck('types', $transactionSelector);
+        $transactionSelector += $this->mapper->formatParamsForExecuteSql($parameters, $types);
 
-        $context = $this->pluck('transactionContext', $options);
+        $context = $this->pluck('transactionContext', $transactionSelector);
 
         // Initially with begin, transactionId will be null.
         // Once transaction is generated, even in the case of stream failure,
         // transaction will be passed to this callable by the Result class.
         $call = function ($resumeToken = null, $transaction = null) use (
             $session,
             $sql,
-            $options
+            $transactionSelector
         ) {
             if ($transaction && !empty($transaction->id())) {
-                $options['transaction'] = ['id' => $transaction->id()];
+                $transactionSelector['transaction'] = ['id' => $transaction->id()];
             }
             if ($resumeToken) {
-                $options['resumeToken'] = $resumeToken;
+                $transactionSelector['resumeToken'] = $resumeToken;
             }
-
             return $this->connection->executeStreamingSql([
                 'sql' => $sql,
                 'session' => $session->name(),
                 'database' => $this->getDatabaseNameFromSession($session)
-            ] + $options);
+            ] + $transactionSelector);
         };
 
         return new Result($this, $session, $call, $context, $this->mapper);
@@ -458,26 +457,34 @@ public function read(
      *           [Refer](https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#transactionoptions)
      *     @type int $isolationLevel The level of Isolation for the transactions executed by this Client's instance.
      *           **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED
+     *     @type array $requestOptions
+     *     @type string $tag
      * }
      * @return Transaction
      */
     public function transaction(Session $session, array $options = [])
     {
-        $options += [
+        // only "singleUse", "requestOptions", "isolationLevel", "transactionOptions", and "begin"
+        // are valid Transaction constructor options
+        $transactionConstructorOptions = array_intersect_key(
+            $options,
+            array_flip(['singleUse', 'requestOptions', 'isolationLevel', 'transactionOptions', 'begin'])
+        ) + [
             'singleUse' => false,
-            'isRetry' => false,
             'requestOptions' => [],
-            'isolationLevel' => IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED
+            'isolationLevel' => IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED,
         ];
-        $transactionTag = $this->pluck('tag', $options, false);
-        if (isset($transactionTag)) {
-            $options['requestOptions']['transactionTag'] = $transactionTag;
+
+        $transactionTag = $options['tag'] ?? null;
+        if ($transactionTag) {
+            $transactionConstructorOptions['requestOptions']['transactionTag'] = $transactionTag;
         }
 
-        if (!$options['singleUse'] && (!isset($options['begin']) ||
-            isset($options['transactionOptions']['partitionedDml']))
-        ) {
-            $res = $this->beginTransaction($session, $options);
+        if (!$transactionConstructorOptions['singleUse'] && (
+            !isset($transactionConstructorOptions['begin'])
+            || isset($transactionConstructorOptions['transactionConstructorOptions']['partitionedDml'])
+        )) {
+            $res = $this->beginTransaction($session, $transactionConstructorOptions);
         } else {
             $res = [];
         }
@@ -487,8 +494,8 @@ public function transaction(Session $session, array $options = [])
             $res,
             [
                 'tag' => $transactionTag,
-                'isRetry' => $options['isRetry'],
-                'transactionOptions' => $options
+                'isRetry' => $options['isRetry'] ?? false,
+                'transactionOptions' => $transactionConstructorOptions
             ]
         );
     }
@@ -498,9 +505,25 @@ public function transaction(Session $session, array $options = [])
      *
      * @param Session $session The session the transaction belongs to.
      * @param array $res [optional] The createTransaction response.
-     * @param array $options [optional] Options for the transaction object.
+     * @param array $options [optional] {
+     *     Configuration Options.
+     *
+     *     @type bool $singleUse If true, a Transaction ID will not be allocated
+     *           up front. Instead, the transaction will be considered
+     *           "single-use", and may be used for only a single operation.
+     *           **Defaults to** `false`.
+     *     @type bool $isRetry If true, the resulting transaction will indicate
+     *           that it is the result of a retry operation. **Defaults to**
+     *           `false`.
      *     @type array $begin The begin transaction options.
      *           [Refer](https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#transactionoptions)
+     *     @type int $isolationLevel The level of Isolation for the transactions executed by this Client's instance.
+     *           **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED
+     *     @type array $requestOptions
+     *     @type string $tag
+     *     @type array $transactionOptions Transaction constructor options. See {@see Transaction}.
+     *           **NOTE**: This is distinct from the TransactionOptions seen in {@see V1\TransactionOptions}.
+     * }
      * @return Transaction
      */
     public function createTransaction(Session $session, array $res = [], array $options = [])

Spanner/src/SnapshotTrait.php

@@ -45,6 +45,7 @@ trait SnapshotTrait
      *           {@see \Google\Cloud\Spanner\V1\DirectedReadOptions}
      *           If using the `replicaSelection::type` setting, utilize the constants available in
      *           {@see \Google\Cloud\Spanner\V1\DirectedReadOptions\ReplicaSelection\Type} to set a value.
+     *     @type array $transactionOptions The Transaction Options
      * }
      */
     private function initialize(
@@ -72,7 +73,11 @@ private function initialize(
 
         $this->context = SessionPoolInterface::CONTEXT_READ;
         $this->directedReadOptions = $options['directedReadOptions'] ?? [];
-        $this->options = $options;
+        $this->transactionSelector = array_intersect_key(
+            (array) $options,
+            array_flip(['singleUse', 'begin'])
+        );
+        $this->transactionOptions = $options['transactionOptions'] ?? [];
     }
 
     /**

Spanner/src/Transaction.php

@@ -21,6 +21,7 @@
 use Google\Cloud\Core\Exception\AbortedException;
 use Google\Cloud\Spanner\Session\Session;
 use Google\Cloud\Spanner\Session\SessionPoolInterface;
+use Google\Cloud\Spanner\V1\TransactionOptions\IsolationLevel;
 
 /**
  * Manages interaction with Cloud Spanner inside a Transaction.
@@ -85,6 +86,9 @@ class Transaction implements TransactionalReadInterface
 
     private ValueMapper $mapper;
 
+    private int $isolationLevel;
+    private array $requestOptions;
+
     /**
      * @param Operation $operation The Operation instance.
      * @param Session $session The session to use for spanner interactions.
@@ -96,10 +100,11 @@ class Transaction implements TransactionalReadInterface
      * @param array $options [optional] {
      *     Configuration Options.
      *
-     *     @type array $begin The begin Transaction options.
-     *           [Refer](https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#transactionoptions)
+     *     @type array $begin The begin Transaction options. See {@see V1\TransactionOptions}.
      *     @type int $isolationLevel level of Isolation for this transaction instance
-     *           **Defaults to** IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED
+     *           **Defaults to** {@see IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED}
+     *     @type array $requestOptions See {@see V1\RequestOptions}.
+     *     @type array $transactionOptions See {@see V1\TransactionOptions}.
      * }
      * @param ValueMapper $mapper Consumed internally for properly map mutation data.
      * @throws \InvalidArgumentException if a tag is specified on a single-use transaction.
@@ -130,7 +135,14 @@ public function __construct(
         $this->tag = $tag;
 
         $this->context = SessionPoolInterface::CONTEXT_READWRITE;
-        $this->options = $options;
+        $this->isolationLevel = $options['isolationLevel'] ?? IsolationLevel::ISOLATION_LEVEL_UNSPECIFIED;
+        $this->transactionSelector = array_intersect_key(
+            (array) $options,
+            array_flip(['singleUse', 'begin'])
+        );
+        $this->requestOptions = $options['requestOptions'] ?? [];
+        $this->transactionOptions = $options['transactionOptions'] ?? [];
+
         if (!is_null($mapper)) {
             $this->mapper = $mapper;
         }
@@ -436,17 +448,19 @@ public function commit(array $options = [])
 
         // For commit, A transaction ID is mandatory for non-single-use transactions,
         // and the `begin` option is not supported.
-        if (empty($this->transactionId) && isset($this->options['begin'])) {
-            // Since the begin option is not supported in commit, unset it.
-            unset($this->options['begin']);
-
-            // A transaction ID is mandatory for non-single-use transactions.
-            if ($this->type !== self::TYPE_SINGLE_USE) {
-                // Execute the beginTransaction RPC.
-                $transaction = $this->operation->transaction($this->session, $this->options);
-                // Set the transaction ID of the current transaction.
-                $this->transactionId = $transaction->id();
-            }
+        // @TODO: Find out why the `begin` option is not supported for calling the `beginTransaction` RPC
+        if (empty($this->transactionId) && isset($this->transactionSelector['begin'])) {
+            $operationTransactionOptions = [
+                'isolationLevel' => $this->isolationLevel,
+                'requestOptions' => $this->requestOptions,
+                'singleUse' => $this->transactionSelector['singleUse'] ?? null,
+                'transactionOptions' => $this->transactionOptions,
+            ];
+            // Execute the beginTransaction RPC.
+            $transaction = $this->operation->transaction($this->session, $operationTransactionOptions);
+
+            // Set the transaction ID of the current transaction.
+            $this->transactionId = $transaction->id();
         }
 
         if (!$this->singleUseState()) {
@@ -536,8 +550,8 @@ private function buildUpdateOptions(array $options): array
         $this->seqno++;
 
         $options['transactionType'] = $this->context;
-        if (empty($this->transactionId) && isset($this->options['begin'])) {
-            $options['begin'] = $this->options['begin'];
+        if (empty($this->transactionId) && isset($this->transactionSelector['begin'])) {
+            $options['begin'] = $this->transactionSelector['begin'];
         } else {
             $options['transactionId'] = $this->transactionId;
         }