From 07ae2bfb888b8aac92176228198700622876d323 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Thu, 22 May 2025 13:18:44 -0700 Subject: [PATCH] untangle options by - explicitly selecting valid fields - better names for variables - documenting missing options in phpdoc - removing pass-by-reference --- Spanner/src/Batch/BatchClient.php | 2 +- Spanner/src/Database.php | 24 +- Spanner/src/Instance.php | 6 +- Spanner/src/Operation.php | 69 +++-- Spanner/src/SnapshotTrait.php | 7 +- Spanner/src/Transaction.php | 48 ++-- Spanner/src/TransactionConfigurationTrait.php | 272 ++++++++++-------- Spanner/src/TransactionalReadTrait.php | 54 ++-- .../TransactionConfigurationTraitTest.php | 85 +++--- 9 files changed, 327 insertions(+), 240 deletions(-) diff --git a/Spanner/src/Batch/BatchClient.php b/Spanner/src/Batch/BatchClient.php index 1c72a60cf7aa..553891bb9a76 100644 --- a/Spanner/src/Batch/BatchClient.php +++ b/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; diff --git a/Spanner/src/Database.php b/Spanner/src/Database.php index c895e7e9e9aa..4f0cb5a2a1f4 100644 --- a/Spanner/src/Database.php +++ b/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. */ diff --git a/Spanner/src/Instance.php b/Spanner/src/Instance.php index 80fdd9a53177..bdd7f7c62878 100644 --- a/Spanner/src/Instance.php +++ b/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 */ diff --git a/Spanner/src/Operation.php b/Spanner/src/Operation.php index e9f4d9748994..be2f8ef0d3a9 100644 --- a/Spanner/src/Operation.php +++ b/Spanner/src/Operation.php @@ -195,17 +195,17 @@ 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, @@ -213,20 +213,19 @@ public function execute(Session $session, $sql, array $options = []) $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 = []) diff --git a/Spanner/src/SnapshotTrait.php b/Spanner/src/SnapshotTrait.php index 760c978fa79d..58538d04e8a2 100644 --- a/Spanner/src/SnapshotTrait.php +++ b/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'] ?? []; } /** diff --git a/Spanner/src/Transaction.php b/Spanner/src/Transaction.php index dd7d3d76d4a0..0aaf25e24408 100644 --- a/Spanner/src/Transaction.php +++ b/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; } diff --git a/Spanner/src/TransactionConfigurationTrait.php b/Spanner/src/TransactionConfigurationTrait.php index f0b6219174bf..729b656934c4 100644 --- a/Spanner/src/TransactionConfigurationTrait.php +++ b/Spanner/src/TransactionConfigurationTrait.php @@ -22,6 +22,8 @@ /** * Configure transaction selection for read, executeSql, rollback and commit. + * + * @internal */ trait TransactionConfigurationTrait { @@ -33,21 +35,23 @@ trait TransactionConfigurationTrait * Depending on given options, can be singleUse or begin, and can be read * or read/write. * + * @see V1\TransactionSelector + * * @param array $options call options. - * @param array $previous Previously given call options (for single-use snapshots). + * @param array $previousReadOnlyOptions Previously given call options (for single-use snapshots). * @return array [(array) transaction selector, (string) context] */ - private function transactionSelector(array &$options, array $previous = []) + private function transactionSelector(array &$options, array $previousReadOnlyOptions = []) { $options += [ 'begin' => false, 'transactionType' => SessionPoolInterface::CONTEXT_READ, ]; - [$transactionOptions, $type, $context] = $this->transactionOptions($options, $previous); + [$transactionOptions, $type, $context] = $this->transactionOptions($options, $previousReadOnlyOptions); // TransactionSelector uses a different key name for singleUseTransaction - // and transactionId than transactionOptions, so we'll rewrite those here + // and transactionId than CommitRequest, so we'll rewrite those here // so transactionOptions works as expected for commitRequest. if ($type === 'singleUseTransaction') { @@ -63,57 +67,24 @@ private function transactionSelector(array &$options, array $previous = []) } /** - * Configure the DirectedReadOptions. + * Return transaction options based on given configuration options. * - * Request level DirectedReadOptions takes precedence over client level DirectedReadOptions. - * Client level DirectedReadOptions apply only to read-only and single-use transactions. + * @see V1\TransactionOptions * - * @param array $requestOptions Request level options. - * @param array $clientOptions Client level Directed Read Options. - * @return array - */ - private function configureDirectedReadOptions(array $requestOptions, array $clientOptions) - { - if (isset($requestOptions['directedReadOptions'])) { - return $requestOptions['directedReadOptions']; - } - - if (isset($requestOptions['transaction']['singleUse']) || ( - isset($requestOptions['transactionContext']) && - $requestOptions['transactionContext'] == SessionPoolInterface::CONTEXT_READ - ) || isset($requestOptions['transactionOptions']['readOnly']) - ) { - if (isset($clientOptions['includeReplicas'])) { - return ['includeReplicas' => $clientOptions['includeReplicas']]; - } elseif (isset($clientOptions['excludeReplicas'])) { - return ['excludeReplicas' => $clientOptions['excludeReplicas']]; - } - } - - return []; - } - - /** - * Return transaction options based on given configuration options. + * @param array $options call options * - * @param array $options call options. - * @param array $previous Previously given call options (for single-use snapshots). + * @param array $previousReadOnlyOptions Previously given call options (for single-use snapshots). * @return array [(array) transaction options, (string) transaction type, (string) context] */ - private function transactionOptions(array &$options, array $previous = []) + private function transactionOptions(array &$options, array $previousReadOnlyOptions = []) { - $options += [ - 'begin' => false, - 'transactionType' => SessionPoolInterface::CONTEXT_READWRITE, - 'transactionId' => null, - ]; + // @TODO: Remove $options being passed by reference $type = null; + $begin = $options['begin'] ?? false; + $context = $options['transactionType'] ?? SessionPoolInterface::CONTEXT_READWRITE; + $id = $options['transactionId'] ?? null; - $context = $this->pluck('transactionType', $options); - $id = $this->pluck('transactionId', $options); - - $begin = $this->pluck('begin', $options); if ($id === null) { if ($begin) { $type = 'begin'; @@ -127,9 +98,10 @@ private function transactionOptions(array &$options, array $previous = []) $type = 'transactionId'; $transactionOptions = $id; } elseif ($context === SessionPoolInterface::CONTEXT_READ) { - $transactionOptions = $this->configureSnapshotOptions($options, $previous); + $options += ['singleUse' => null]; + $transactionOptions = $this->configureReadOnlyTransactionOptions($options, $previousReadOnlyOptions); } elseif ($context === SessionPoolInterface::CONTEXT_READWRITE) { - $transactionOptions = $this->configureTransactionOptions( + $transactionOptions = $this->configureReadWriteTransactionOptions( $type == 'begin' && is_array($begin) ? $begin : [] ); } else { @@ -139,109 +111,165 @@ private function transactionOptions(array &$options, array $previous = []) )); } + // @TODO: Remove this once $options is no longer passed by reference + unset( + $options['begin'], + $options['transactionType'], + $options['transactionId'], + ); + + // For backwards compatibility - remove all PBReadOnly fields + // This was previously being done in configureReadOnlyTransactionOptions + // @TODO: Remove this once $options is no longer passed by reference + unset( + $options['returnReadTimestamp'], + $options['strong'], + $options['readTimestamp'], + $options['exactStaleness'], + $options['minReadTimestamp'], + $options['maxStaleness'], + ); + return [$transactionOptions, $type, $context]; } - private function configureTransactionOptions(array $options = []) + private function configureReadWriteTransactionOptions(array $options = []) { - $transactionOptions = [ - 'readWrite' => [] - ]; - - if (isset($options['excludeTxnFromChangeStreams'])) { - $transactionOptions['excludeTxnFromChangeStreams'] = $options['excludeTxnFromChangeStreams']; - } - - if (isset($options['isolationLevel'])) { - $transactionOptions['isolationLevel'] = $options['isolationLevel']; - } - - return $transactionOptions; + return array_intersect_key($options, array_flip([ + 'excludeTxnFromChangeStreams', + 'isolationLevel' + ])) + ['readWrite' => []]; } /** * Configure a Read-Only transaction. * - * @param array $options Configuration Options. + * @param array $options [optional] { + * Configuration Options + * + * See [ReadOnly](https://cloud.google.com/spanner/reference/rpc/google.spanner.v1#google.spanner.v1.TransactionOptions.ReadOnly) + * for detailed description of available options. + * + * Please note that only one of `$strong`, `$readTimestamp` or + * `$exactStaleness` may be set in a request. + * + * @type bool $returnReadTimestamp If true, the Cloud Spanner-selected + * read timestamp is included in the Transaction message that + * describes the transaction. + * @type bool $strong Read at a timestamp where all previously committed + * transactions are visible. + * @type Timestamp $readTimestamp Executes all reads at the given + * timestamp. + * @type Duration $exactStaleness Represents a number of seconds. Executes + * all reads at a timestamp that is $exactStaleness old. + * @type Timestamp $minReadTimestamp Executes all reads at a + * timestamp >= min_read_timestamp. Only available when + * `$options.singleUse` is true. + * @type Duration $maxStaleness Read data at a timestamp >= NOW - max_staleness + * seconds. Guarantees that all writes that have committed more + * than the specified number of seconds ago are visible. Only + * available when `$options.singleUse` is true. + * @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`. + * } * @param array $previous Previously given call options (for single-use snapshots). * @return array */ - private function configureSnapshotOptions(array &$options, array $previous = []) + private function configureReadOnlyTransactionOptions(array $options, array $previousReadOnlyOptions = []) { - $options += [ - 'singleUse' => false, - 'returnReadTimestamp' => null, - 'strong' => null, - 'readTimestamp' => null, - 'exactStaleness' => null, - 'minReadTimestamp' => null, - 'maxStaleness' => null, - ]; + // select only the PBReadOnly fields from $options + $readOnly = array_intersect_key($options, array_flip([ + 'minReadTimestamp', + 'readTimestamp', + 'returnReadTimestamp', + 'exactStaleness', + 'maxStaleness', + 'strong' + ])); + + // Validate options types + if (isset($readOnly['minReadTimestamp'])) { + $this->validateOptionType($readOnly, 'minReadTimestamp', Timestamp::class); + $readOnly['minReadTimestamp'] = $readOnly['minReadTimestamp']->formatAsString(); + } + + if (isset($readOnly['readTimestamp'])) { + $this->validateOptionType($readOnly, 'readTimestamp', Timestamp::class); + $readOnly['readTimestamp'] = $readOnly['readTimestamp']->formatAsString(); + } - $previousOptions = $previous['transactionOptions']['readOnly'] ?? []; + if (isset($readOnly['exactStaleness'])) { + $this->validateOptionType($readOnly, 'exactStaleness', Duration::class); + $readOnly['exactStaleness'] = $readOnly['exactStaleness']->get(); + } + + if (isset($readOnly['maxStaleness'])) { + $this->validateOptionType($readOnly, 'maxStaleness', Duration::class); + $readOnly['maxStaleness'] = $readOnly['maxStaleness']->get(); + } // These are only available in single-use transactions. - if (!$options['singleUse'] && ($options['maxStaleness'] || $options['minReadTimestamp'])) { + if ( + !($options['singleUse'] ?? false) + && (!empty($readOnly['maxStaleness']) || !empty($readOnly['minReadTimestamp'])) + ) { throw new \BadMethodCallException( 'maxStaleness and minReadTimestamp are only available in single-use transactions.' ); } - $transactionOptions = [ - 'readOnly' => $this->arrayFilterRemoveNull([ - 'returnReadTimestamp' => $this->pluck('returnReadTimestamp', $options), - 'strong' => $this->pluck('strong', $options), - 'minReadTimestamp' => $this->pluck('minReadTimestamp', $options), - 'maxStaleness' => $this->pluck('maxStaleness', $options), - 'readTimestamp' => $this->pluck('readTimestamp', $options), - 'exactStaleness' => $this->pluck('exactStaleness', $options), - ]) + $previousOptions - ]; + $readOnly += $previousReadOnlyOptions; - if (empty($transactionOptions['readOnly'])) { - $transactionOptions['readOnly']['strong'] = true; + if (empty($readOnly)) { + $readOnly['strong'] = true; } - $timestampFields = [ - 'minReadTimestamp', - 'readTimestamp' - ]; - - $durationFields = [ - 'exactStaleness', - 'maxStaleness' - ]; + return ['readOnly' => $readOnly]; + } - foreach ($timestampFields as $tsf) { - if (isset($transactionOptions['readOnly'][$tsf]) && !isset($previousOptions[$tsf])) { - $field = $transactionOptions['readOnly'][$tsf]; - if (!($field instanceof Timestamp)) { - throw new \BadMethodCallException(sprintf( - 'Read Only Transaction Configuration Field %s must be an instance of `%s`.', - $tsf, - Timestamp::class - )); - } - - $transactionOptions['readOnly'][$tsf] = $field->formatAsString(); - } + /** + * Configure the DirectedReadOptions. + * + * Request level DirectedReadOptions takes precedence over client level DirectedReadOptions. + * Client level DirectedReadOptions apply only to read-only and single-use transactions. + * + * @param array $requestOptions Request level options. + * @param array $clientOptions Client level Directed Read Options. + * @return array + */ + private function configureDirectedReadOptions(array $requestOptions, array $clientOptions) + { + if (isset($requestOptions['directedReadOptions'])) { + return $requestOptions['directedReadOptions']; } - foreach ($durationFields as $df) { - if (isset($transactionOptions['readOnly'][$df]) && !isset($previousOptions[$df])) { - $field = $transactionOptions['readOnly'][$df]; - if (!($field instanceof Duration)) { - throw new \BadMethodCallException(sprintf( - 'Read Only Transaction Configuration Field %s must be an instance of `%s`.', - $df, - Duration::class - )); - } - - $transactionOptions['readOnly'][$df] = $field->get(); + if (isset($requestOptions['transaction']['singleUse']) || ( + ($requestOptions['transactionContext'] ?? null) == SessionPoolInterface::CONTEXT_READ + ) || isset($requestOptions['transactionOptions']['readOnly']) + ) { + if (isset($clientOptions['includeReplicas'])) { + return ['includeReplicas' => $clientOptions['includeReplicas']]; + } elseif (isset($clientOptions['excludeReplicas'])) { + return ['excludeReplicas' => $clientOptions['excludeReplicas']]; } } - return $transactionOptions; + return []; + } + + /** + * @throws \BadMethodCallException + */ + private function validateOptionType($options, $field, $type) + { + if (!$options[$field] instanceof $type) { + throw new \BadMethodCallException(sprintf( + 'Read Only Transaction Configuration Field %s must be an instance of `%s`.', + $field, + $type + )); + } } } diff --git a/Spanner/src/TransactionalReadTrait.php b/Spanner/src/TransactionalReadTrait.php index 7186eb5e670c..1b7fe1d71c0f 100644 --- a/Spanner/src/TransactionalReadTrait.php +++ b/Spanner/src/TransactionalReadTrait.php @@ -22,6 +22,8 @@ /** * Shared methods for reads inside a transaction. + * + * @internal */ trait TransactionalReadTrait { @@ -59,9 +61,16 @@ trait TransactionalReadTrait private $state = 0; // TransactionalReadInterface::STATE_ACTIVE /** + * @see V1\TransactionSelector + * @var array + */ + private array $transactionSelector = []; + + /** + * @see V1\TransactionOptions * @var array */ - private $options = []; + private array $transactionOptions = []; /** * @var int @@ -277,35 +286,39 @@ public function execute($sql, array $options = []) $this->singleUseState(); $this->checkReadContext(); - if (empty($this->transactionId) && isset($this->options['begin'])) { - $options['begin'] = $this->options['begin']; + $executeSqlOptions = $options; + if (empty($this->transactionId) && isset($this->transactionSelector['begin'])) { + $executeSqlOptions['begin'] = $this->transactionSelector['begin']; } else { - $options['transactionId'] = $this->transactionId; + $executeSqlOptions['transactionId'] = $this->transactionId; } - $options['transactionType'] = $this->context; - $options['seqno'] = $this->seqno; + $executeSqlOptions['transactionType'] = $this->context; + $executeSqlOptions['seqno'] = $this->seqno; $this->seqno++; - $selector = $this->transactionSelector($options, $this->options); + $selector = $this->transactionSelector( + $executeSqlOptions, + $this->transactionOptions['readOnly'] ?? [] + ); - $options['transaction'] = $selector[0]; + $executeSqlOptions['transaction'] = $selector[0]; - unset($options['requestOptions']['transactionTag']); + unset($executeSqlOptions['requestOptions']['transactionTag']); if (isset($this->tag)) { - $options += [ + $executeSqlOptions += [ 'requestOptions' => [] ]; - $options['requestOptions']['transactionTag'] = $this->tag; + $executeSqlOptions['requestOptions']['transactionTag'] = $this->tag; } - $options['directedReadOptions'] = $this->configureDirectedReadOptions( - $options, + $executeSqlOptions['directedReadOptions'] = $this->configureDirectedReadOptions( + $executeSqlOptions, $this->directedReadOptions ?? [] ); - $options = $this->addLarHeader($options, true, $this->context); + $executeSqlOptions = $this->addLarHeader($executeSqlOptions, true, $this->context); - $result = $this->operation->execute($this->session, $sql, $options); + $result = $this->operation->execute($this->session, $sql, $executeSqlOptions); if (empty($this->id()) && $result->transaction()) { $this->setId($result->transaction()->id()); } @@ -359,14 +372,17 @@ public function read($table, KeySet $keySet, array $columns, array $options = [] $this->singleUseState(); $this->checkReadContext(); - 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; } $options['transactionType'] = $this->context; - $options += $this->options; - $selector = $this->transactionSelector($options, $this->options); + $options += $this->transactionSelector; + $selector = $this->transactionSelector( + $options, + $this->transactionOptions['readOnly'] ?? [] + ); $options['transaction'] = $selector[0]; diff --git a/Spanner/tests/Unit/TransactionConfigurationTraitTest.php b/Spanner/tests/Unit/TransactionConfigurationTraitTest.php index 527d42706c98..df2e9606cd41 100644 --- a/Spanner/tests/Unit/TransactionConfigurationTraitTest.php +++ b/Spanner/tests/Unit/TransactionConfigurationTraitTest.php @@ -61,7 +61,7 @@ public function setUp(): void public function testTransactionSelectorBasicSnapshot() { $args = []; - $res = $this->impl->proxyTransactionSelector($args); + $res = $this->impl->transactionSelector($args); $this->assertEquals(SessionPoolInterface::CONTEXT_READ, $res[1]); $this->assertEquals($res[0]['singleUse']['readOnly'], ['strong' => true]); } @@ -69,7 +69,7 @@ public function testTransactionSelectorBasicSnapshot() public function testTransactionSelectorExistingId() { $args = ['transactionId' => self::TRANSACTION]; - $res = $this->impl->proxyTransactionSelector($args); + $res = $this->impl->transactionSelector($args); $this->assertEquals(SessionPoolInterface::CONTEXT_READ, $res[1]); $this->assertEquals(self::TRANSACTION, $res[0]['id']); } @@ -77,75 +77,75 @@ public function testTransactionSelectorExistingId() public function testTransactionSelectorReadWrite() { $args = ['transactionType' => SessionPoolInterface::CONTEXT_READWRITE]; - $res = $this->impl->proxyTransactionSelector($args); + $res = $this->impl->transactionSelector($args); $this->assertEquals(SessionPoolInterface::CONTEXT_READWRITE, $res[1]); - $this->assertEquals($this->impl->proxyConfigureTransactionOptions(), $res[0]['singleUse']); + $this->assertEquals($this->impl->configureReadWriteTransactionOptions(), $res[0]['singleUse']); } public function testTransactionSelectorReadOnly() { $args = ['transactionType' => SessionPoolInterface::CONTEXT_READ]; - $res = $this->impl->proxyTransactionSelector($args); + $res = $this->impl->transactionSelector($args); $this->assertEquals(SessionPoolInterface::CONTEXT_READ, $res[1]); } public function testBegin() { $args = ['begin' => true]; - $res = $this->impl->proxyTransactionSelector($args); + $res = $this->impl->transactionSelector($args); $this->assertEquals(SessionPoolInterface::CONTEXT_READ, $res[1]); $this->assertEquals($res[0]['begin']['readOnly'], ['strong' => true]); } - public function testConfigureSnapshotOptionsReturnReadTimestamp() + public function testConfigureReadOnlyTransactionOptionsReturnReadTimestamp() { $args = ['returnReadTimestamp' => true]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertTrue($res['readOnly']['returnReadTimestamp']); } - public function testConfigureSnapshotOptionsStrong() + public function testConfigureReadOnlyTransactionOptionsStrong() { $args = ['strong' => true]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertTrue($res['readOnly']['strong']); } /** * @dataProvider timestamps */ - public function testConfigureSnapshotOptionsMinReadTimestamp($timestamp, $expected = null) + public function testConfigureReadOnlyTransactionOptionsMinReadTimestamp($timestamp, $expected = null) { $time = $this->parseTimeString($timestamp); $ts = new Timestamp($time[0], $time[1]); $args = ['minReadTimestamp' => $ts, 'singleUse' => true]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertEquals($expected ?: $timestamp, $res['readOnly']['minReadTimestamp']); } /** * @dataProvider timestamps */ - public function testConfigureSnapshotOptionsReadTimestamp($timestamp) + public function testConfigureReadOnlyTransactionOptionsReadTimestamp($timestamp) { $time = $this->parseTimeString($timestamp); $ts = new Timestamp($time[0], $time[1]); $args = ['readTimestamp' => $ts]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertEquals($timestamp, $res['readOnly']['readTimestamp']); } - public function testConfigureSnapshotOptionsMaxStaleness() + public function testConfigureReadOnlyTransactionOptionsMaxStaleness() { $args = ['maxStaleness' => $this->duration, 'singleUse' => true]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertEquals($this->dur, $res['readOnly']['maxStaleness']); } - public function testConfigureSnapshotOptionsExactStaleness() + public function testConfigureReadOnlyTransactionOptionsExactStaleness() { $args = ['exactStaleness' => $this->duration]; - $res = $this->impl->proxyConfigureSnapshotOptions($args); + $res = $this->impl->configureReadOnlyTransactionOptions($args); $this->assertEquals($this->dur, $res['readOnly']['exactStaleness']); } @@ -154,39 +154,39 @@ public function testTransactionSelectorInvalidContext() $this->expectException(\BadMethodCallException::class); $args = ['transactionType' => 'foo']; - $this->impl->proxyTransactionSelector($args); + $this->impl->transactionSelector($args); } - public function testConfigureSnapshotOptionsInvalidExactStaleness() + public function testConfigureReadOnlyTransactionOptionsInvalidExactStaleness() { $this->expectException(\BadMethodCallException::class); $args = ['exactStaleness' => 'foo']; - $this->impl->proxyConfigureSnapshotOptions($args); + $this->impl->configureReadOnlyTransactionOptions($args); } - public function testConfigureSnapshotOptionsInvalidMaxStaleness() + public function testConfigureReadOnlyTransactionOptionsInvalidMaxStaleness() { $this->expectException(\BadMethodCallException::class); $args = ['maxStaleness' => 'foo']; - $this->impl->proxyConfigureSnapshotOptions($args); + $this->impl->configureReadOnlyTransactionOptions($args); } - public function testConfigureSnapshotOptionsInvalidMinReadTimestamp() + public function testConfigureReadOnlyTransactionOptionsInvalidMinReadTimestamp() { $this->expectException(\BadMethodCallException::class); $args = ['minReadTimestamp' => 'foo']; - $this->impl->proxyConfigureSnapshotOptions($args); + $this->impl->configureReadOnlyTransactionOptions($args); } - public function testConfigureSnapshotOptionsInvalidReadTimestamp() + public function testConfigureReadOnlyTransactionOptionsInvalidReadTimestamp() { $this->expectException(\BadMethodCallException::class); $args = ['readTimestamp' => 'foo']; - $this->impl->proxyConfigureSnapshotOptions($args); + $this->impl->configureReadOnlyTransactionOptions($args); } public function testRequestLevelConfigureDirectedReadOptions() @@ -196,7 +196,7 @@ public function testRequestLevelConfigureDirectedReadOptions() 'directedReadOptions' => $this->directedReadOptionsIncludeReplicas ]; $clientOptions = []; - $res = $this->impl->proxyConfigureDirectedReadOptions($requestOptions, $clientOptions); + $res = $this->impl->configureDirectedReadOptions($requestOptions, $clientOptions); $this->assertEquals($res, $requestOptions['directedReadOptions']); } @@ -204,7 +204,7 @@ public function testClientLevelConfigureDirectedReadOptions() { $requestOptions = ['transaction' => ['singleUse' => true]]; $clientOptions = $this->directedReadOptionsIncludeReplicas; - $res = $this->impl->proxyConfigureDirectedReadOptions($requestOptions, $clientOptions); + $res = $this->impl->configureDirectedReadOptions($requestOptions, $clientOptions); $this->assertEquals($res, $clientOptions); } @@ -221,7 +221,7 @@ public function testPrioritizeRequestLevelConfigureDirectedReadOptions() ] ] ]; - $res = $this->impl->proxyConfigureDirectedReadOptions($requestOptions, $clientOptions); + $res = $this->impl->configureDirectedReadOptions($requestOptions, $clientOptions); $this->assertEquals($res, $requestOptions['directedReadOptions']); } @@ -237,26 +237,11 @@ public function timestamps() //@codingStandardsIgnoreStart class TransactionConfigurationTraitImplementation { - use TransactionConfigurationTrait; - - public function proxyTransactionSelector(array &$options) - { - return $this->transactionSelector($options); - } - - public function proxyConfigureTransactionOptions() - { - return $this->configureTransactionOptions(); - } - - public function proxyConfigureSnapshotOptions(array &$options) - { - return $this->configureSnapshotOptions($options); - } - - public function proxyConfigureDirectedReadOptions(array $args1, array $args2) - { - return $this->configureDirectedReadOptions($args1, $args2); + use TransactionConfigurationTrait { + transactionSelector as public; + configureReadWriteTransactionOptions as public; + configureReadOnlyTransactionOptions as public; + configureDirectedReadOptions as public; } } //@codingStandardsIgnoreEnd