-
Notifications
You must be signed in to change notification settings - Fork 21
Add SAVEPOINT (Fixes #56) #111
Changes from 7 commits
7e891bd
cb80c8e
08b697c
e6cfb03
e98d575
cbaf911
c2ddf98
d28d750
37fb625
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -20,20 +20,19 @@ class Transaction | |||||
public function __construct($connection=null) | ||||||
{ | ||||||
$this->_connection = $connection ?: \Pheasant::instance()->connection(); | ||||||
$this->_events = new \Pheasant\Events(); | ||||||
$this->_events = new \Pheasant\Events(array(), $this->_connection->events()); | ||||||
} | ||||||
|
||||||
public function execute() | ||||||
{ | ||||||
$this->results = array(); | ||||||
|
||||||
try { | ||||||
$this->_connection->execute('BEGIN'); | ||||||
$this->_events->trigger('startTransaction', $this->_connection); | ||||||
$this->_connection->execute('COMMIT'); | ||||||
$this->_events->trigger('commitTransaction', $this->_connection); | ||||||
$this->_events->wrap('StartTransaction', $this, function($self) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the capitalisation of this event name correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed that pheasant/lib/Pheasant/DomainObject.php Line 147 in e0f78a1
and pheasant/lib/Pheasant/DomainObject.php Line 91 in e4a90b5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right, because |
||||||
$self->events()->trigger('startTransaction', $self->connection()); | ||||||
$self->events()->trigger('commitTransaction', $self->connection()); | ||||||
}); | ||||||
} catch (\Exception $e) { | ||||||
$this->_connection->execute('ROLLBACK'); | ||||||
$this->_events->trigger('rollbackTransaction', $this->_connection); | ||||||
throw $e; | ||||||
} | ||||||
|
@@ -67,17 +66,26 @@ public function events() | |||||
return $this->_events; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Get the connection object | ||||||
* @return Connection | ||||||
*/ | ||||||
public function connection() | ||||||
{ | ||||||
return $this->_connection; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Links another Events object such that events in it are corked until either commit/rollback and then uncorked | ||||||
* @chainable | ||||||
*/ | ||||||
public function deferEvents($events) | ||||||
{ | ||||||
$this->_events | ||||||
->register('startTransaction', function() use ($events) { | ||||||
->register('beforeStartTransaction', function() use ($events) { | ||||||
$events->cork(); | ||||||
}) | ||||||
->register('commitTransaction', function() use ($events) { | ||||||
->register('afterStartTransaction', function() use ($events) { | ||||||
$events->uncork(); | ||||||
}) | ||||||
->register('rollbackTransaction', function() use ($events) { | ||||||
|
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this actually a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, would be better as |
||
{ | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does MYSQL complain if you re-use savepoint names? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect it's not. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better name for this than
afterStartTransaction
? Isn't it fired just before the transaction completes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I used that even name for the sake of being backwards compatible if someone had bound to it. But I realize that
before/afterStartTransaction
events were never fired.Thoughts on changing the name to just
transaction
. Which would give usbeforeTransaction
andafterTransaction
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds pretty good to me. Thoughts @lox?