Skip to content

Commit 7cf3f98

Browse files
purvavasudeopurvavasudeo
committed
feat: Support for Spanner ReadLockMode
tests: Add in unit tests for ReadLockMode option chore: Fix lint issues chore: Fix lint issues chore: Fix lint issues feat: Add ability to set this ReadLockMode through operations chore: Cleanup code and remove debug logging chore: Lint fixes chore: Fix style warnings chore: Review comment resolution
1 parent 4571aaa commit 7cf3f98

5 files changed

Lines changed: 220 additions & 21 deletions

File tree

‎Spanner/src/Connection/Grpc.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,12 @@ public function beginTransaction(array $args)
11071107
$readWrite = new ReadWrite();
11081108
$options->setReadWrite($readWrite);
11091109
$args = $this->addLarHeader($args, $this->larEnabled);
1110+
1111+
if (isset($transactionOptions['readWrite']['readLockMode'])) {
1112+
// Nested option `readLockMode` inside `readWrite` transactions
1113+
$readLockModeOption = $transactionOptions['readWrite']['readLockMode'];
1114+
$options->getReadWrite()->setReadLockMode($readLockModeOption);
1115+
}
11101116
} elseif (isset($transactionOptions['partitionedDml'])) {
11111117
$pdml = new PartitionedDml();
11121118
$options->setPartitionedDml($pdml);
@@ -1576,6 +1582,32 @@ private function formatTransactionOptions(array $transactionOptions)
15761582
$transactionOptions['readOnly'] = $ro;
15771583
}
15781584

1585+
if (isset($transactionOptions['readLockMode'])) {
1586+
if (isset($transactionOptions['readOnly'])) {
1587+
throw new ValidationException(
1588+
'The readLockMode option is only valid for readWrite transactions.'
1589+
);
1590+
}
1591+
1592+
if (!isset($transactionOptions['readWrite'])) {
1593+
$transactionOptions['readWrite'] = [];
1594+
}
1595+
}
1596+
1597+
if (isset($transactionOptions['readWrite'])) {
1598+
$rw = $transactionOptions['readWrite'];
1599+
1600+
// Format nested options inside readWrite transaction
1601+
if (isset($transactionOptions['readLockMode'])) {
1602+
$rw['readLockMode'] = $transactionOptions['readLockMode'];
1603+
1604+
// Unset the readLockMode key on the base options array.
1605+
// If we don't do this it causes issues in the serializer for TransactionOptions
1606+
unset($transactionOptions['readLockMode']);
1607+
}
1608+
1609+
$transactionOptions['readWrite'] = $rw;
1610+
}
15791611
return $transactionOptions;
15801612
}
15811613

‎Spanner/src/Database.php‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -775,10 +775,8 @@ public function snapshot(array $options = [])
775775
* If you wish Google Cloud PHP to handle retry logic for you (recommended
776776
* for most cases), use {@see \Google\Cloud\Spanner\Database::runTransaction()}.
777777
*
778-
* Please note that once a transaction reads data, it will lock the read
779-
* data, preventing other users from modifying that data. For this reason,
780-
* it is important that every transaction commits or rolls back as early as
781-
* possible. Do not hold transactions open longer than necessary.
778+
* Please note for locking semantics and defaults for the transactions
779+
* use {@see \Google\Cloud\Spanner\V1\TransactionOptions\ReadWrite\ReadLockMode}
782780
*
783781
* Example:
784782
* ```
@@ -812,8 +810,8 @@ public function transaction(array $options = [])
812810
throw new \BadMethodCallException('Nested transactions are not supported by this client.');
813811
}
814812

815-
// There isn't anything configurable here.
816-
$options['transactionOptions'] = $this->configureTransactionOptions();
813+
// Configure readWrite options here. Any nested options for readWrite should be added to this call
814+
$options['transactionOptions'] = $this->configureTransactionOptions($options['transactionOptions'] ?? []);
817815

818816
$session = $this->selectSession(
819817
SessionPoolInterface::CONTEXT_READWRITE,
@@ -840,10 +838,8 @@ public function transaction(array $options = [])
840838
* exception types will immediately bubble up and will interrupt the retry
841839
* operation.
842840
*
843-
* Please note that once a transaction reads data, it will lock the read
844-
* data, preventing other users from modifying that data. For this reason,
845-
* it is important that every transaction commits or rolls back as early as
846-
* possible. Do not hold transactions open longer than necessary.
841+
* Please note for locking semantics and defaults for the transactions
842+
* use {@see \Google\Cloud\Spanner\V1\TransactionOptions\ReadWrite\ReadLockMode}
847843
*
848844
* Please also note that nested transactions are NOT supported by this client.
849845
* Attempting to call `runTransaction` inside a transaction callable will
@@ -920,7 +916,6 @@ public function runTransaction(callable $operation, array $options = [])
920916
'maxRetries' => self::MAX_RETRIES,
921917
];
922918

923-
// There isn't anything configurable here.
924919
$options['transactionOptions'] = $this->configureTransactionOptions($options['transactionOptions'] ?? []);
925920

926921
$session = $this->selectSession(

‎Spanner/src/TransactionConfigurationTrait.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
use Google\Cloud\Core\ArrayTrait;
2121
use Google\Cloud\Spanner\Session\SessionPoolInterface;
22+
use Google\Cloud\Spanner\V1\TransactionOptions\ReadWrite\ReadLockMode as ReadLockMode;
2223

2324
/**
2425
* Configure transaction selection for read, executeSql, rollback and commit.
@@ -152,6 +153,15 @@ private function configureTransactionOptions(array $options = [])
152153
$transactionOptions['excludeTxnFromChangeStreams'] = $options['excludeTxnFromChangeStreams'];
153154
}
154155

156+
// Allow for proper configuring of the `readLockMode` if it's set as a base or nested option
157+
if (isset($options['readLockMode'])) {
158+
$transactionOptions['readWrite']['readLockMode'] = $options['readLockMode'];
159+
}
160+
161+
if (isset($options['readWrite']['readLockMode'])) {
162+
$transactionOptions['readWrite']['readLockMode'] = $options['readWrite']['readLockMode'];
163+
}
164+
155165
return $transactionOptions;
156166
}
157167

‎Spanner/tests/Unit/DatabaseTest.php‎

Lines changed: 91 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Google\Cloud\Core\Exception\AbortedException;
2222
use Google\Cloud\Core\Exception\NotFoundException;
2323
use Google\Cloud\Core\Exception\ServerException;
24+
use Google\Cloud\Core\Exception\ServiceException;
2425
use Google\Cloud\Core\Iam\Iam;
2526
use Google\Cloud\Core\Iterator\ItemIterator;
2627
use Google\Cloud\Core\LongRunning\LongRunningConnectionInterface;
@@ -45,18 +46,18 @@
4546
use Google\Cloud\Spanner\Tests\StubCreationTrait;
4647
use Google\Cloud\Spanner\Timestamp;
4748
use Google\Cloud\Spanner\Transaction;
49+
use Google\Cloud\Spanner\V1\DirectedReadOptions\ReplicaSelection\Type as ReplicaType;
4850
use Google\Cloud\Spanner\V1\ResultSet;
4951
use Google\Cloud\Spanner\V1\ResultSetStats;
50-
use Google\Cloud\Spanner\V1\DirectedReadOptions\ReplicaSelection\Type as ReplicaType;
5152
use Google\Cloud\Spanner\V1\Session as SessionProto;
5253
use Google\Cloud\Spanner\V1\SpannerClient;
5354
use Google\Cloud\Spanner\V1\Transaction as TransactionProto;
5455
use Google\Cloud\Spanner\V1\TransactionOptions;
56+
use Google\Cloud\Spanner\V1\TransactionOptions\ReadWrite\ReadLockMode as ReadLockMode;
5557
use Google\Rpc\Code;
5658
use PHPUnit\Framework\TestCase;
5759
use Prophecy\Argument;
5860
use Prophecy\PhpUnit\ProphecyTrait;
59-
use Google\Cloud\Core\Exception\ServiceException;
6061

6162
/**
6263
* @group spanner
@@ -92,7 +93,6 @@ class DatabaseTest extends TestCase
9293
private $directedReadOptionsIncludeReplicas;
9394
private $directedReadOptionsExcludeReplicas;
9495

95-
9696
public function setUp(): void
9797
{
9898
$this->checkAndSkipGrpcTests();
@@ -251,7 +251,7 @@ public function testBackups()
251251
]
252252
];
253253

254-
$expectedFilter = "database:".$this->database->name();
254+
$expectedFilter = 'database:' . $this->database->name();
255255
$this->connection->listBackups(Argument::withEntry('filter', $expectedFilter))
256256
->shouldBeCalled()
257257
->willReturn(['backups' => $backups]);
@@ -279,8 +279,8 @@ public function testBackupsWithCustomFilter()
279279
'name' => DatabaseAdminClient::backupName(self::PROJECT, self::INSTANCE, 'backup2'),
280280
]
281281
];
282-
$defaultFilter = "database:" . $this->database->name();
283-
$customFilter = "customFilter";
282+
$defaultFilter = 'database:' . $this->database->name();
283+
$customFilter = 'customFilter';
284284
$expectedFilter = sprintf('(%1$s) AND (%2$s)', $defaultFilter, $customFilter);
285285

286286
$this->connection->listBackups(Argument::withEntry('filter', $expectedFilter))
@@ -410,7 +410,7 @@ public function testCreatePostgresDialect()
410410
$this->database->___setProperty('connection', $this->connection->reveal());
411411

412412
$op = $this->database->create([
413-
'databaseDialect'=> DatabaseDialect::POSTGRESQL
413+
'databaseDialect' => DatabaseDialect::POSTGRESQL
414414
]);
415415

416416
$this->assertInstanceOf(LongRunningOperation::class, $op);
@@ -708,7 +708,6 @@ public function testBatchWrite()
708708
Argument::withEntry('mutationGroups', [$expectedMutationGroup])
709709
))->shouldBeCalled()->willReturn(['foo result']);
710710

711-
712711
$mutationGroups = [
713712
($this->database->mutationGroup(false))
714713
->insertOrUpdate(
@@ -2139,6 +2138,90 @@ public function testBatchWriteWithExcludeTxnFromChangeStreams()
21392138
]);
21402139
}
21412140

2141+
public function testRunTransactionWithReadLockMode()
2142+
{
2143+
$expectedReadLockMode = ReadLockMode::OPTIMISTIC;
2144+
2145+
$gapic = $this->prophesize(SpannerClient::class);
2146+
2147+
$sessName = SpannerClient::sessionName(self::PROJECT, self::INSTANCE, self::DATABASE, self::SESSION);
2148+
$session = new SessionProto(['name' => $sessName]);
2149+
$resultSet = new ResultSet(['stats' => new ResultSetStats(['row_count_exact' => 0])]);
2150+
$gapic->createSession(Argument::cetera())->shouldBeCalled()->willReturn($session);
2151+
$gapic->deleteSession(Argument::cetera())->shouldBeCalled();
2152+
2153+
$sql = 'SELECT example FROM sql_query';
2154+
$stream = $this->prophesize(ServerStream::class);
2155+
$stream->readAll()->shouldBeCalledOnce()->willReturn([$resultSet]);
2156+
$gapic->executeStreamingSql(
2157+
$sessName,
2158+
$sql,
2159+
Argument::that(function (array $options) use ($expectedReadLockMode) {
2160+
$this->assertArrayHasKey('transaction', $options);
2161+
$this->assertNotNull($transactionOptions = $options['transaction']->getBegin());
2162+
$this->assertNotNull($readWriteTxnOptions = $transactionOptions->getReadWrite());
2163+
$this->assertNotNull($readLockModeOption = $readWriteTxnOptions->getReadLockMode());
2164+
$this->assertEquals(
2165+
$expectedReadLockMode,
2166+
$readLockModeOption
2167+
);
2168+
return true;
2169+
})
2170+
)
2171+
->shouldBeCalledOnce()
2172+
->willReturn($stream->reveal());
2173+
2174+
$database = new Database(
2175+
new Grpc(['gapicSpannerClient' => $gapic->reveal()]),
2176+
$this->instance,
2177+
$this->lro->reveal(),
2178+
$this->lroCallables,
2179+
self::PROJECT,
2180+
self::DATABASE
2181+
);
2182+
2183+
// Test TransactionOption array format with base level property set for readLockMode
2184+
// This helps test proper formating by the library to the format expected by Spanner backend
2185+
// (i.e. readLockMode should be inside readWrite)
2186+
$database->runTransaction(
2187+
function (Transaction $t) use ($sql) {
2188+
// Run a fake query
2189+
$t->executeUpdate($sql);
2190+
2191+
// Simulate calling Transaction::commmit()
2192+
$prop = new \ReflectionProperty($t, 'state');
2193+
$prop->setAccessible(true);
2194+
$prop->setValue($t, Transaction::STATE_COMMITTED);
2195+
},
2196+
['transactionOptions' => ['readLockMode' => $expectedReadLockMode, ] ]
2197+
);
2198+
}
2199+
2200+
public function testTransactionWithReadLockMode()
2201+
{
2202+
$expectedReadLockMode = ReadLockMode::OPTIMISTIC;
2203+
2204+
$this->connection->beginTransaction(
2205+
Argument::that(function (array $args) use ($expectedReadLockMode) {
2206+
$this->assertArrayHasKey('transactionOptions', $args);
2207+
$this->assertArrayHasKey('readWrite', $args['transactionOptions']);
2208+
$this->assertArrayHasKey('readLockMode', $args['transactionOptions']['readWrite']);
2209+
$this->assertEquals(
2210+
$expectedReadLockMode,
2211+
$args['transactionOptions']['readWrite']['readLockMode'],
2212+
"The read lock mode received was {$args['transactionOptions']['readWrite']['readLockMode']} " .
2213+
"does not match expected {$expectedReadLockMode}"
2214+
);
2215+
return true;
2216+
})
2217+
)
2218+
->shouldBeCalled()
2219+
->willReturn(['id' => self::TRANSACTION]);
2220+
2221+
$t = $this->database->transaction(['transactionOptions' => ['readLockMode' => $expectedReadLockMode, ]]);
2222+
$this->assertInstanceOf(Transaction::class, $t);
2223+
}
2224+
21422225
private function createStreamingAPIArgs()
21432226
{
21442227
$row = ['id' => 1];

‎Spanner/tests/Unit/OperationTest.php‎

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Google\ApiCore\ServerStream;
2121
use Google\Cloud\Core\Testing\GrpcTestTrait;
2222
use Google\Cloud\Core\Testing\TestHelpers;
23-
use Google\Cloud\Spanner\Admin\Database\V1\DatabaseAdminClient;
2423
use Google\Cloud\Spanner\Batch\QueryPartition;
2524
use Google\Cloud\Spanner\Batch\ReadPartition;
2625
use Google\Cloud\Spanner\Connection\Grpc;
@@ -36,12 +35,12 @@
3635
use Google\Cloud\Spanner\Tests\StubCreationTrait;
3736
use Google\Cloud\Spanner\Timestamp;
3837
use Google\Cloud\Spanner\Transaction;
39-
use Google\Cloud\Spanner\V1\CommitResponse;
4038
use Google\Cloud\Spanner\V1\ResultSet;
4139
use Google\Cloud\Spanner\V1\ResultSetStats;
4240
use Google\Cloud\Spanner\V1\SpannerClient;
4341
use Google\Cloud\Spanner\V1\Transaction as TransactionProto;
4442
use Google\Cloud\Spanner\V1\TransactionOptions;
43+
use Google\Cloud\Spanner\V1\TransactionOptions\ReadWrite\ReadLockMode as ReadLockMode;
4544
use PHPUnit\Framework\TestCase;
4645
use Prophecy\Argument;
4746
use Prophecy\PhpUnit\ProphecyTrait;
@@ -421,6 +420,86 @@ public function testExecuteAndExecuteUpdateWithExcludeTxnFromChangeStreams()
421420
]);
422421
}
423422

423+
public function testTransactionWithReadLockMode()
424+
{
425+
$expectedReadLockMode = ReadLockMode::OPTIMISTIC;
426+
$gapic = $this->prophesize(SpannerClient::class);
427+
$gapic->beginTransaction(
428+
self::SESSION,
429+
Argument::that(function (TransactionOptions $options) use ($expectedReadLockMode) {
430+
$this->assertNotNull($readWriteTxnOptions = $options->getReadWrite());
431+
$this->assertNotNull($readLockModeOption = $readWriteTxnOptions->getReadLockMode());
432+
$this->assertEquals(
433+
$expectedReadLockMode,
434+
$readLockModeOption
435+
);
436+
return true;
437+
}),
438+
Argument::type('array')
439+
)
440+
->shouldBeCalled()
441+
->willReturn(new TransactionProto(['id' => 'foo']));
442+
443+
$operation = new Operation(
444+
new Grpc(['gapicSpannerClient' => $gapic->reveal()]),
445+
true
446+
);
447+
448+
$transaction = $operation->transaction($this->session, [
449+
'transactionOptions' => ['readLockMode' => $expectedReadLockMode, ]
450+
]);
451+
452+
$this->assertEquals('foo', $transaction->id());
453+
}
454+
455+
public function testExecuteAndExecuteUpdateWithReadLockMode()
456+
{
457+
$expectedReadLockMode = ReadLockMode::OPTIMISTIC;
458+
$sql = 'SELECT example FROM sql_query';
459+
460+
$resultSet = new ResultSet(['stats' => new ResultSetStats(['row_count_exact' => 0])]);
461+
$stream = $this->prophesize(ServerStream::class);
462+
$stream->readAll()->shouldBeCalledTimes(2)->willReturn([$resultSet]);
463+
464+
$gapic = $this->prophesize(SpannerClient::class);
465+
$gapic->executeStreamingSql(
466+
self::SESSION,
467+
$sql,
468+
Argument::that(function (array $options) use ($expectedReadLockMode) {
469+
$this->assertArrayHasKey('transaction', $options);
470+
$this->assertNotNull($transactionOptions = $options['transaction']->getBegin());
471+
$this->assertNotNull($readWriteTxnOptions = $transactionOptions->getReadWrite());
472+
$this->assertNotNull($readLockModeOption = $readWriteTxnOptions->getReadLockMode());
473+
$this->assertEquals(
474+
$expectedReadLockMode,
475+
$readLockModeOption
476+
);
477+
return true;
478+
})
479+
)
480+
->shouldBeCalledTimes(2)
481+
->willReturn($stream->reveal());
482+
483+
$operation = new Operation(
484+
new Grpc(['gapicSpannerClient' => $gapic->reveal()]),
485+
true
486+
);
487+
488+
$lockModeOptions = [
489+
'transaction' => [
490+
'begin' => [
491+
'readLockMode' => $expectedReadLockMode,
492+
]
493+
]
494+
];
495+
496+
$operation->execute($this->session, $sql, $lockModeOptions);
497+
498+
$transaction = $this->prophesize(Transaction::class)->reveal();
499+
500+
$operation->executeUpdate($this->session, $transaction, $sql, $lockModeOptions);
501+
}
502+
424503
public function testSnapshot()
425504
{
426505
$this->connection->beginTransaction(Argument::allOf(

0 commit comments

Comments
 (0)