Transaction() method: no rollback() counterpart

[rr-]

Right now we have:

R::begin()
...
R::commit()

We can also use closures like this:

R::transaction(function()
{
    ...
});

However, when we want to create transactions that we know are always going to rollback by design, there's no available construct that will allow us to use closures:

R::transaction(function()
{
    R::exec('CREATE TEMPORARY TABLE ...');
    #this shouldn't be committed, really
});

We can use following workarounds to achieve this, but it's kind of clumsy:

try
{
    R::transaction(function()
    {
        R::exec('CREATE TEMPORARY TABLE ...');
        #this shouldn't be committed, really
        throw new MyPredefinedException();
    });
}
catch (MyPredefinedException $e)
{
    #okay, rollback happened
}

[gabordemooij]

The transaction is already wrapped - no need for a try-catch. You can use your own catch to handle a specific exception but the transaction will have been rolled back already.

[rr-]

The point is that I want to make a transaction that will always rollback - even if it's successful (i.e. no exceptions were thrown). Think unit tests that do test operations on database - they shouldn't ever be commited. Throwing exceptions just to achieve rollback on given closure is a workaround.

I could of course use R::begin() and R::rollback() methods. But the point is that since we got mechanism like R::transaction(closure) that wraps R::begin() and R::commit() methods, it'd be nice to have some method like R::transactionFail(closure) that would wrap R::begin() and R::rollback() methods.

[rr-]

Added pull request in #286

[daviddeutsch]

Sorry, I'm with Gabor on this one. Your original workaround doesn't make that much sense - a "rollback by design" is not an exception. It's a rollback by design.

The function is called transaction() and within the logic of the function, a rollback is an exception - so something that you don't want to happen. Making a parameter where you say that you DO want it to happen... I dunno, that's simply confusing.

I do understand how this would clean up things for you, I guess I'm just struggling to see a use beyond your own use case.

[rr-]

We use unit tests at work. They test classes that put stuff into database. They cannot do any commits to database, because this would pollute it for other tests. So we wrap everything in a rollback transaction - once we're done with specific test, we perform rollback and end up with clean database, without "rubbish" from that test.

Also sometimes I insert temporary stuff to database just to make JOIN with other table, instead of doing SELECT ... WHERE ... IN (?,?,?). I use temporary tables for that. Although temporary tables will be deleted at the end of transaction whether you commit your transaction or not, I find performing rollback explicitly to be more readable for other coders - they know the data you inserted is not going to be commited into DB regardless of their knowledge about temporary tables.

[daviddeutsch]

But you do see how this is inherently a "not in the real world" case then? You are talking specifically about Testing. Well, in RB, there is unit testing as well and it just nuke()s the database over and over again. Would it make sense to put in an option to add a parameter that nukes the database after a certain command? No - because we do not want to impose the testing case on every other case. (And actually, simply nuking the database might be faster anyhow.)

As for your example with the JOIN/SELECT - Well now we're finally getting somewhere that is a little more interesting, but I disagree that your solution is a good one that should be incorporated that way into the Facade. Right now, you're talking about this:

R::transaction( function(){
   // Stuff that should be permanent
} );

// and:

R::transaction( function(){
   // Stuff that should be temporary
}, false );

I disagree that this is more readable. You actively have to read the database class to understand that the "false" makes it temporary. It would make far more sense to have a R::temporary() function instead of that one parameter that you would probably have to comment every single time to make sure coders get it. That would necessitate that we move the stack concept into a separate class, though, since the $depth static is particular to the function.

To sum up: The transaction() class has one purpose and one purpose only right now: Do a thing and if that is successful, call a function. If it is not successful, roll back. The parameter you propose violates that simple contract. If we want to have anything like this at all, my vote would be that it should go into a separate function.

---

For me, the most interesting solution would be a stack handler class, maybe referenced with an $s in the Facade, for "stack". This would help us get that bit of business logic out of the Facade and it would be a neat tool for people to use. Then, you could do R::$s->push(func...); and R::$s->temp(func...) to your hearts content.

Anyhow, that's my two cents - We need to have Gabor weigh in on this.

[rr-]

Nuking DB is not an option in solutions, where your DB might contain X tables populated by your client. Performing rollback is simpler than loading dump in every test.

I understand that my pull request isn't anywhere near perfect, I'm just saying that such functionality would be welcome by at least one user.

[gabordemooij]

Hi,

This thread needed a somewhat longer answer so I decided to write this the next morning, sorry for the delay.

I already have some issues with the R::transaction() method. This method has been contributed by another developer but it pollutes the façade, it should be moved to the adapter. However that's another issue.

I understand your case, but we need to be careful not to add to much functionality to RedBeanPHP, otherwise we end up with a very bloated library - and nobody wants that. I try to make the façade thinner as well.

At Organic Software we also use a lot of unit testing. Our approach is to use a special sandbox database and test on that. I would not recommend to perform rollbacks at all - you're not testing the 'exact' same scenarios in that case (in real life you won't do the rollback part). Also it's very useful to review the state of the database after a unit test has failed - this often speeds up debugging. Anyway - it's your choice of course how you want to implement your tests.

I don't think this is core functionality though. I have features I really would like to have myself and I don't even add these simply because I want the façade to remain small and simple. It's very hard not to add a feature - but it's also necessary. But, I also see why you would like to have something like: R::simulate(); which would perform the database operation and rollback immediately. Luckily there is a solution for this:

You can write a plugin for the facade. It's really simple. Just write a plugin that implements this 'do-and-rollback' method. Now after writing the plugin add the following comment to your file header:

@plugin public static function simulate($callback) { return RedBean_Plugin_Simulator::simulate($callback); }

Keep that on one line. Now if you run: php replica.php - this plugin will be added to the facade. Now you have your simulate metod as R::simulate() if you install the plugin. This way we can all have what we want:

• You have your auto rollback method (I prefer to call it simulate, but that's of course personal taste)
• I will be happy to proudly add your plugin to the plugin page (please write more plugins!)
• You get traffic to your website because I provide a link to your 'plugin page'
• We keep the façade clean
• We don't add confusing method signature to R::transaction

Please let me know what you think about this solution.

[daviddeutsch]

Ah true. So what about the R::$stack idea? Would that be something for you?

[gabordemooij]

For more details concerning the Replica solution, see:

http://www.redbeanphp.com/replica#replica_and_plugins