Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent nested transactions #533

Merged
merged 3 commits into from
Jun 28, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions src/Spanner/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ class Database
*/
private $sessionPool;

/**
* @var bool
*/
private $isRunningTransaction = false;

/**
* Create an object representing a Database.
*
Expand Down Expand Up @@ -511,6 +516,10 @@ public function iam()
*/
public function snapshot(array $options = [])

This comment was marked as spam.

{
if ($this->isRunningTransaction) {
throw new \BadMethodCallException('Nested transactions are not supported by this client.');
}

$options += [
'singleUse' => false
];
Expand Down Expand Up @@ -563,6 +572,10 @@ public function snapshot(array $options = [])
*/
public function transaction(array $options = [])
{
if ($this->isRunningTransaction) {
throw new \BadMethodCallException('Nested transactions are not supported by this client.');
}

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

Expand Down Expand Up @@ -593,6 +606,10 @@ public function transaction(array $options = [])
* it is important that every transaction commits or rolls back as early as
* possible. Do not hold transactions open longer than necessary.
*
* Please also note that nested transactions are NOT supported by this client.
* Attempting to call `runTransaction` inside a transaction callable will
* raise a `BadMethodCallException`.
*
* If a callable finishes executing without invoking
* {@see Google\Cloud\Spanner\Transaction::commit()} or
* {@see Google\Cloud\Spanner\Transaction::rollback()}, the transaction will
Expand Down Expand Up @@ -643,10 +660,15 @@ public function transaction(array $options = [])
* `false`.
* }
* @return mixed The return value of `$operation`.
* @throws \RuntimeException
* @throws \RuntimeException If a transaction is not committed or rolled back.
* @throws \BadMethodCallException If a nested transaction is created.
*/
public function runTransaction(callable $operation, array $options = [])
{
if ($this->isRunningTransaction) {

This comment was marked as spam.

This comment was marked as spam.

throw new \BadMethodCallException('Nested transactions are not supported by this client.');
}

$options += [
'maxRetries' => self::MAX_RETRIES,
];
Expand Down Expand Up @@ -683,7 +705,13 @@ public function runTransaction(callable $operation, array $options = [])
$options
]);

$res = call_user_func($operation, $transaction);
// Prevent nested transactions.
$this->isRunningTransaction = true;
try {
$res = call_user_func($operation, $transaction);
} finally {
$this->isRunningTransaction = false;
}

$active = $transaction->state() === Transaction::STATE_ACTIVE;
$singleUse = $transaction->type() === Transaction::TYPE_SINGLE_USE;
Expand Down
57 changes: 57 additions & 0 deletions tests/unit/Spanner/DatabaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,25 @@ public function testSnapshotMaxStaleness()
$this->database->snapshot(['maxStaleness' => 'foo']);
}

/**
* @expectedException BadMethodCallException
*/
public function testSnapshotNestedTransaction()
{
$this->connection->beginTransaction(Argument::any())
->shouldBeCalled()
->willReturn(['id' => self::TRANSACTION]);

$this->connection->rollback(Argument::any())
->shouldNotBeCalled();

$this->refreshOperation();

$this->database->runTransaction(function ($t) {
$this->database->snapshot();
});
}

public function testRunTransaction()
{
$this->connection->beginTransaction(Argument::any())
Expand Down Expand Up @@ -340,6 +359,25 @@ public function testRunTransactionNoCommit()
$this->database->runTransaction(function (Transaction $t) {});
}

/**
* @expectedException BadMethodCallException
*/
public function testRunTransactionNestedTransaction()
{
$this->connection->beginTransaction(Argument::any())
->shouldBeCalled()
->willReturn(['id' => self::TRANSACTION]);

$this->connection->rollback(Argument::any())
->shouldNotBeCalled();

$this->refreshOperation();

$this->database->runTransaction(function ($t) {
$this->database->runTransaction(function ($t) {});
});
}

public function testRunTransactionRetry()
{
$abort = new AbortedException('foo', 409, null, [
Expand Down Expand Up @@ -427,6 +465,25 @@ public function testTransaction()
$this->assertInstanceOf(Transaction::class, $t);
}

/**
* @expectedException BadMethodCallException
*/
public function testTransactionNestedTransaction()
{
$this->connection->beginTransaction(Argument::any())
->shouldBeCalled()
->willReturn(['id' => self::TRANSACTION]);

$this->connection->rollback(Argument::any())
->shouldNotBeCalled();

$this->refreshOperation();

$this->database->runTransaction(function ($t) {
$this->database->transaction();
});
}

public function testInsert()
{
$table = 'foo';
Expand Down