Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Add SAVEPOINT (Fixes #56) #111

Closed
wants to merge 9 commits into from
Closed

Add SAVEPOINT (Fixes #56) #111

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7e891bd
Initial Savepoint Concept
Jud May 9, 2014
cb80c8e
Adding comments to the transaction stack
Jud May 9, 2014
08b697c
Changing tests
Jud May 9, 2014
e6cfb03
Making assertions consistent
Jud May 9, 2014
e98d575
Fixing tests
Jud May 9, 2014
cbaf911
Make test more complex
Jud May 9, 2014
c2ddf98
Move transaction sql to the Connection object
Jud Jul 19, 2014
d28d750
Changed TransactionStack to SavePointStack,
Jud Sep 20, 2014
37fb625
Adding the `registerOne` method to Event class.
Jud Sep 20, 2014
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
12 changes: 12 additions & 0 deletions lib/Pheasant/Database/Mysqli/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Pheasant\Database\Dsn;
use Pheasant\Database\FilterChain;
use Pheasant\Database\MysqlPlatform;
use Pheasant\Database\Mysqli\TransactionStack;

/**
* A connection to a MySql database
Expand All @@ -19,6 +20,7 @@ class Connection
$_sequencePool,
$_strict,
$_selectedDatabase,
$_transactionStack,
$_debug=false
;

Expand All @@ -44,6 +46,7 @@ public function __construct(Dsn $dsn)
$this->_selectedDatabase = $this->_dsn->database;

$this->_debug = getenv('PHEASANT_DEBUG');
$this->_transactionStack = new TransactionStack();
}

/**
Expand Down Expand Up @@ -248,4 +251,13 @@ public function selectedDatabase()
{
return $this->_selectedDatabase;
}

/**
* Returns the transaction stack
* @return TransactionStack
*/
public function transactionStack()
{
return $this->_transactionStack;
}
}
15 changes: 12 additions & 3 deletions lib/Pheasant/Database/Mysqli/Transaction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,21 @@ public function execute()
$this->results = array();

try {
$this->_connection->execute('BEGIN');
$this->_connection->execute(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to see what's going on in this method at a glance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed -- I think I wanted the changes to be as minimal as possible, but this cuts readability. I'll refactor it a bit.

($sp = $this->_connection->transactionStack()->descend()) === null
? 'BEGIN'
: "SAVEPOINT {$sp}");
$this->_events->trigger('startTransaction', $this->_connection);
$this->_connection->execute('COMMIT');
$this->_connection->execute(
($sp = $this->_connection->transactionStack()->pop()) === null
? 'COMMIT'
: "RELEASE SAVEPOINT {$sp}");
$this->_events->trigger('commitTransaction', $this->_connection);
} catch (\Exception $e) {
$this->_connection->execute('ROLLBACK');
$this->_connection->execute(
($sp = $this->_connection->transactionStack()->pop()) === null
? 'ROLLBACK'
: "ROLLBACK TO {$sp}");
$this->_events->trigger('rollbackTransaction', $this->_connection);
throw $e;
}
Expand Down
45 changes: 45 additions & 0 deletions lib/Pheasant/Database/Mysqli/TransactionStack.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace Pheasant\Database\Mysqli;

/**
* A Transaction Stack that keeps track of open savepoints
*/
class TransactionStack
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually a SavePointStack, or something like that? The current name implies that we push/pop Transaction objects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, would be better as SavePointStack

{
private
$_transactionStack = array()
;

/**
* Get the depth of the stack
* @return integer
*/
public function depth()
{
return count($this->_transactionStack);
}

/**
* Decend deeper into the transaction stack and return a unique
* transaction savepoint name
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does MYSQL complain if you re-use savepoint names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you reuse savepoint names, the old savepoint is replaced with the new one. I haven't tested, but I believe that would prevent rolling back transactions arbitrary levels > 1 and continuing with a parent transaction.

Relevant docs:
http://dev.mysql.com/doc/refman/5.0/en/savepoint.html

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I meant say if you end up using the TransactionStack object a second time in a request, is it a problem that you reuse savepoint_1, etc.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it's not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would only have unintended consequences if you could simultaneously create two open transactions on the same connection. Though, I can't think of a way that would happen. Reuse after the close of another transaction is okay. And reuse across connections is okay as well.

* @return string
*/
public function descend()
{
$this->_transactionStack[] = current($this->_transactionStack) === false
? null
: 'savepoint_'.$this->depth();

return end($this->_transactionStack);
}

/**
* Pop off the last savepoint
* @return string
*/
public function pop()
{
return array_pop($this->_transactionStack);
}
}
118 changes: 95 additions & 23 deletions tests/Pheasant/Tests/TransactionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,77 +2,102 @@

namespace Pheasant\Tests\Transaction;
use \Pheasant\Database\Mysqli\Transaction;
use \Pheasant\Tests\Examples\Animal;

class TransactionTest extends \Pheasant\Tests\MysqlTestCase
{
public function setUp()
{
parent::setUp();

$migrator = new \Pheasant\Migrate\Migrator();
$migrator
->create('animal', Animal::schema())
;

$this->queries = array();
$test = $this;
$this->connection()->filterChain()->onQuery(function($sql) use($test) {
$test->queries []= $sql;
return $sql;
});
}

public function testBasicSuccessfulTransaction()
{
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with('COMMIT')->once();
$connection = $this->connection();

$transaction = new Transaction($connection);
$transaction->callback(function(){
return 'blargh';
});

$transaction->execute();

$this->assertEquals(count($this->queries), 2);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'COMMIT');

$this->assertEquals(count($transaction->results), 1);
$this->assertEquals($transaction->results[0], 'blargh');
}

public function testExceptionsCauseRollback()
{
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with('ROLLBACK')->once();

$connection = $this->connection();
$transaction = new Transaction($connection);
$transaction->callback(function(){
throw new \Exception('Eeeek!');
});

$this->setExpectedException('\Exception');
$transaction->execute();

$this->assertEquals(count($this->queries), 2);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'ROLLBACK');
}

public function testCallbacksWithConnectionCalls()
{
$sql = "SELECT * FROM table";
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with($sql)->once();
$connection->shouldReceive('execute')->with('COMMIT')->once();
$sql = "SELECT * FROM animal";
$connection = $this->connection();

$transaction = new Transaction($connection);
$transaction->callback(function() use ($connection, $sql) {
$connection->execute($sql);
});

$transaction->execute();

$this->assertEquals(count($this->queries), 3);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], $sql);
$this->assertEquals($this->queries[2], 'COMMIT');
}

public function testCallbacksWithParams()
{
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with('COMMIT')->once();
$connection = $this->connection();

$transaction = new Transaction($connection);
$transaction->callback(function($param) {
return $param;
}, 'blargh');

$transaction->execute();

$this->assertEquals(count($transaction->results), 1);
$this->assertEquals($transaction->results[0], 'blargh');

$this->assertEquals(count($this->queries), 2);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'COMMIT');
}

public function testDeferEventsFireOnCommit()
{
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with('COMMIT')->once();
$connection = $this->connection();

$events = \Mockery::mock();
$events->shouldReceive('cork')->once();
Expand All @@ -85,13 +110,15 @@ public function testDeferEventsFireOnCommit()
});

$transaction->execute();

$this->assertEquals(count($this->queries), 2);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'COMMIT');
}

public function testDeferEventsFireOnRollback()
{
$connection = \Mockery::mock('\Pheasant\Database\Mysqli\Connection');
$connection->shouldReceive('execute')->with('BEGIN')->once();
$connection->shouldReceive('execute')->with('ROLLBACK')->once();
$connection = $this->connection();

$events = \Mockery::mock();
$events->shouldReceive('cork')->once()->andReturn($events);
Expand All @@ -104,7 +131,52 @@ public function testDeferEventsFireOnRollback()
throw new \Exception("Llamas :( :)");
});

$this->setExpectedException('\Exception');
$transaction->execute();
try {
$transaction->execute();
} catch(\Exception $e) {
$exception = $e;
}

$this->assertInstanceOf('\Exception', $exception);
$this->assertEquals(count($this->queries), 2);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'ROLLBACK');
}

public function testNestedDeferEventsFireOnRollback()
{
$connection = $this->connection();

$events = \Mockery::mock();
$events->shouldReceive('cork')->once()->andReturn($events);
$events->shouldReceive('discard')->once()->andReturn($events);
$events->shouldReceive('uncork')->once()->andReturn($events);

$transaction = new Transaction($connection);
$transaction->deferEvents($events);
$transaction->callback(function() use($connection){
$t = new Transaction($connection);
$t->callback(function() use($connection){
$t = new Transaction($connection);
$t->callback(function() use($connection){
throw new \Exception("Llamas :( :)");
})->execute();
})->execute();
});

try {
$transaction->execute();
} catch(\Exception $e) {
$exception = $e;
}

$this->assertInstanceOf('\Exception', $exception);
$this->assertEquals(count($this->queries), 6);
$this->assertEquals($this->queries[0], 'BEGIN');
$this->assertEquals($this->queries[1], 'SAVEPOINT savepoint_1');
$this->assertEquals($this->queries[2], 'SAVEPOINT savepoint_2');
$this->assertEquals($this->queries[3], 'ROLLBACK TO savepoint_2');
$this->assertEquals($this->queries[4], 'ROLLBACK TO savepoint_1');
$this->assertEquals($this->queries[5], 'ROLLBACK');
}
}