search

lox / pheasant

A lightweight data mapper designed to take advantage of PHP 5.3+
http://getpheasant.com
MIT License
101 stars 21 forks source link

Add SAVEPOINT (Fixes #56) #111

Closed Jud closed 9 years ago

Jud commented 10 years ago

We needed this so I hacked it in and added a test case. I ran into trouble getting Mockery to correctly mock the object, so I had to change the tests up a bit.

Jud commented 10 years ago

Ping @lox

lox commented 10 years ago

Hey, sorry for the delay @Jud. Reviewing this and other patches now.

lox commented 10 years ago

What would you think of actually removing responsibility for issuing SQL entirely from the Transaction object? It already fires events, we could just have the Connection object listening for those events and converting them to SQL. That's probably a cleaner separation of responsibility. Then the API for transactions could remain the same and we could have a Strategy in the Connection to switch between plain old transactions or SAVEPOINTS.

Jud commented 10 years ago

@lox moved the transaction sql into the Connection object. I'm not entirely sure I see the need for multiple transaction strategies. Using SAVEPOINTS with unnested Pheasant Transactions gives the same behavior as unnested 'plain old transactions'. Nesting 'plain old transactions' doesn't work, and if it is being used, would probably benefit from a working strategy.

lox commented 10 years ago

I'm pretty happy with this, @harto any thoughts before merge?

harto commented 10 years ago

Looks good. Just a couple of questions about naming, etc.

Jud commented 10 years ago

@harto @lox Finished up the renaming and also added an event method registerOne that adds an event listener and removes it after execution.

In this case it was useful for binding to the connection events commit message to defer callbacks.

harto commented 9 years ago

@lox might leave it to you to give this the final :+1:

Jud commented 9 years ago

@lox fwiw, we've been using this in production since October without issue. This was the foundation for allowing all afterSave/afterDelete events to run inside transactions.

https://github.com/judsonco/pheasant/blob/52d4a22a208f5b8b5346e0caa8102d3cb352dda8/lib/Pheasant/DomainObject.php#L89-L109

Jud commented 9 years ago

Closing in favor of #142