Shutdown Functions in QCubed.

[vaibhav]

PHP Offers us to create shutdown functions. QApplication does not seem to do anything about it. I think we can create a constant in configuration.inc.php which will contain the address of a file which would be 'included' during the script shutdown procedure. This will help users write code more easily. I think something like this:

in configuration.inc.php, user defines the path to the file:

define('__SHUTDOWN_PROCEDURE_FILE__', __APP_INCLUDES__ . '/function/shutdown.inc.php');

Then in QApplicationBase::Initialize():

register_shutdown_function('QApplicationBase::Shutdown'); 

And create a public static function in QApplicationBase as:

public static function Shutdown() {
include(__SHUTDOWN_PROCEDURE_FILE__);
}

This way any one can write custom shutdown code with QCubed anywhere he wants to without fearing that on an upgrade things will change. Also, all the shutdown code management would not be responsibility of user. This would be a fair addition to functionality!

Regards,
Vaibhav

[vakopian]

vaibhav, This is a nice addition. But I'd like to keep 2.1.1 primarily for bug fixes. Unless you really want it ;-).
So if it's not too critical, let's move this to 2.2?

[vaibhav]

Well, I was trying to develop (and my mind has been running over this since last 3-4 days) about creating a class for myself (that could benefit others as well) which could leverage the awesome 'SELECT FOR UPDATE' feature in PostgreSQL. I have about 3 functions which can provide help to users on PG.

A little description about what SELECT FOR UPDATE is (although I think you might already be knowing, but in case):

SELECT FOR UPDATE retrieves a row from a table and LOCKS it so that others could not modify. This statement can only be run inside a TRANSACTION. As such, if the transaction never ends (Commits or Rollbacks), the row gets locked forever and all updates to it will keep waiting forever as well! To prevent that, it is necessary that we END the transaction. If we write a function which helps the user do the "SELECT FOR UPDATE", it is important that we check if the transaction was closed or not. For that, we will need shutdown function.

It can also help at other places where the user initiates any action which must end before the script finishes! So I would like it, but then, I leave the good will upon you :)

Regards,
Vaibhav

[vakopian]

I'm not sure if the shutdown is the correct hook for a transaction commit/rollback.
I'm guessing, you just want to call a "rollback" there, just in case. But then you would need to know the status of your transaction, maybe it has actually committed. Would you reset the shutdown hook if commit was successful?
If that's the idea, I think you can do this better with error handlers already.
Something like this:

function rollbackErrorHandler($objDatabase) {
  $objDatabase->TransactionRollback();
}

// code that needs transaction
$objDB->TransactionBegin();
QApplication::SetErrorHandler(array('rollbackErrorHandler', $objDB));
try {
  // do things in transaction
  // ...
  // all is well, commit
  $objDB->TransactionCommit();
  QApplication::RestoreErrorHandler();
} catch (Exception $ex) {
  // error
  $objDB->TransactionRollback();
  QApplication::RestoreErrorHandler();
  throw $ex;
}

Basically, PHP is missing the equivalent of the finally construct in Java. The combination of try/catch and error handlers tries to cover this shortcoming.

[vaibhav]

Well, it is not just this much, there are parts in my codebase which do things with the $_SESSION. And those things need to be completed before the scripts complete or else the user does get logged out. Now, some of those procedures run through a set of queries which would not fail normally but there is a fair 5% chance that it can fail. One thing is for sure that yes, I can write a try-catch at all those points or at least an if-else. The other option which looks better to me is that since I already know what state of $_SESSION meant what went wrong, why not move all those possibilities at once place and clean-up any mess so that the next page access does not get screwed.

Again, I wanted to run some analysis code on all page accesses which would usually be done after the render is complete. All such things need some sort of mechanism which is done before the script ends. There are obviously my personal needs from the project and I can implement them easily on my side. Also, I have already done the changes proposed in #811 on my setup already so it would not be difficult for me at all to go ahead and implement this little feature without calling in the possibility of breaking things on an upgrade (given that #811 is accepted) by messing up the core. It's just that if QCubed provided the feature, it would be a little more neat. Nothing more.

So in case you think that this feature is not implemented, at least have a look at #811 . It is one change which has made my application very very portable. I am literally playing with the directory structure of app_includes without a worry and my code is simpler. There is just one single include on the top of every file: require_once ('qcubed.inc.php'); and things are not only simpler but also faster (no more pre-loading of classes which need not be) and more secure (since there are just no includes in the page, there is no way you are letting in a RFI or LFI on the page). This patch just keep things a little cleaner. :)

Regards,
Vaibhav

[vaibhav]

And about that code on the transactions: let me tell you one thing frankly - " I am an idiot! " I have been writing code for months using QCubed, reading all the core code and I did not have a look on that SetErrorHandler? function till now.

Frankly speaking, I noticed it after writing the above post (I thought to just let you the other uses of having a shutdown register and my use case so that you might see the need). My jaw almost dropped after seeing that code and I am feeling like an idiot - there are at least a few dozen places in my code where that function would have worked like a charm and saved me a lot of work and I just didn't know it existed!

Seriously I need to look at all the things autocomplete shows me everytime!

[vakopian]

Can we close this or move it to 2.2?
If you absolutely want it for 2.1.1, please provide a patch.

[vaibhav]

I do not absolutely want it for 2.1.1 or even for 2.2. The thing is - if #811 is accepted, there is no need to this one I guess coz then the shutdown functions become a responsibility of user as #811 offers a lot of bootstrapping customization.

So in case you are going to commit #811, I would say it can be closed without committing, or if you want to take time to decide over #811, then move this to 2.2. I hope I am clear now.

[vaibhav]

Why is that half my sentences sound as if I am being rude *wondering*. I dont intend that but this happens. Guess I have to be more alert when writing things.

[vakopian]

Ok, #811 is now committed, closing this.

[vaibhav]

No problems. That one was good enough to let users do their own shutdown stuff. F/W should do only the core jobs :)