[RFC|Merged] Modular cron

[Oleg]

rxu, eviL3: there is no ultimatum here. I am merely explaining the situation (namely, lack of my activity here) for those who might be following this topic. The development team is free to do whatever they want, just as I am free to do whatever I want. If anyone wants me to continue working on the modular cron and/or other things I explained what has to happen.

bantu: thank you for your comments, I will make the requested changes after my other tickets are attended to (per #2 above).

[Oleg]

Testing via system cron:

1. Apply the patch.
2. Go to acp, I believe it's under server settings, change cron type from phpbb to system.
3. Invoke cron.php. To test in correct environment, do it from console. With that, the code should still work if invoked via curl or web browser.

[bantu]

Although PHP lacks try/finally, wouldn't it be possible to unlock cron using register_shutdown_function() if available?

This seems to be what the current 3.0.x code is doing.

[Oleg]

I wrote the code in question a while ago obviously but if I remember correctly the issue is this bit from php manual:

Multiple calls to register_shutdown_function() can be made, and each will be called in the same order as they were registered.

DB connection is closed in a shutdown function registered before lock release, so if lock is not released by the end of normal script execution it will not be released in the shutdown function either.

[Oleg]

As requested I cleaned up the history: removed merge commits, squashed fix-up commits, also changed one instance of intval() call to (int) cast.

http://github.com/p/phpbb3/compare/deve ... ystem-cron

[igorw]

Reviewed it, looks good. I tested it a little and it appears to work.

Debug information on using system_cron when DEBUG_EXTRA is enabled. Telling which cron tasks were executed.

viewforum.php:

Code: Select all

if ($task && $task->is_ready()) {

Needs to be on a new line.

cron_manager:

Code: Select all

				if (!class_exists($class))
				{
					include($phpbb_root_path . "includes/cron/tasks/$mod/$filename.$phpEx");
				}

Should use the new autoloading. This means that tasks needs to be renamed to task. That should be all to get it working.

http://github.com/evil3/phpbb3/commit/4 ... 63d1c4dff9

[igorw]

Now we need some documentation on the wiki...

http://wiki.phpbb.com/display/DEV/Modular+cron

[ToonArmy]

Really liking the look of this, few things I've noticed:

• Some class methods don't have a visibility defined.
• A number of classes are included manually
• The cron_task interface file has another interface in the file

[igorw]

Should use the new autoloading. This means that tasks needs to be renamed to task. That should be all to get it working.

Bumping this.

includes/cron/cron_manager.php would become includes/cron/manager.php, the class name would become phpbb_cron_manager. And you could get rid of all of the includes.

[Oleg]

I am addressing reported issues/requests as time permits.

Outstanding requests are currently as follows:

- Use new naming standards (naderman, bantu)
- prefix classes with phpbb_
- rename tasks to task
- cron task interface has another interface in file (toonarmy)
- Use autoloading (naderman, bantu, evil3, toonarmy) http://tracker.phpbb.com/browse/PHPBB3- ... tion_34289
- Tests for new and/or changed code (naderman)
- Add @return and @param documentation to every function (bantu)
- Add documentation to wiki (evil3)

Some of this is simply mechanical work, although there is a fair amount of it due to the size of the diff.

As far as renaming classes goes, if that is done the code that loads tasks may need to be adjusted since it loads both phpbb classes and non-phpbb classes.

Last time I tried running unit tests I did not succeed.

Already addressed:

- Updated install/schema_data.sql (bantu)
- Use integer casting instead of intval (bantu)
- Added debug information (evil3)
- Fixed braces (evil3)
- Added missing visibility specifiers (toonarmy)