[RFC|Merged] General Hook Architecture of phpBB3.1

[Kellanved]

Version 2

Diff for events: https://github.com/p/phpbb3/compare/dev ... re%2Fhooks
Diff for hook locations: https://github.com/p/phpbb3/compare/fea ... 2Fledges-2
Pull request and comments: https://github.com/phpbb/phpbb3/pull/551

Version 1
General Hook Architecture of phpBB3.1
This is an early draft and subject to change
“Hooks” are a popular concept in contemporary web applications. Generally speaking, a “hook” is a place where extensions can register functions to be called when certain events occur.

While phpBB 3.0 had a powerful hook system, it is obvious that few people ever bothered to use it. The new system thus has to meet the challenge: it has to be easier to use, more powerful and extremely scalable.


The architecture of a phpBB MODule
MODs are stored in the mod/ directory, each MOD using its own subdirectory.

Functions to be called by a hook are identified by naming convention. The hook “foo” automatically invokes all MOD methods named “<MODNAME>_foo”.

Every MOD using the hook system has to provide a class <MODNAME>_class.php implementing the phpBB\MOD interface. The interface declares four methods:

function MOD_hooks
returns an associative array mapping hook names to an array holding a flag whether the method is static and the hook’s priority. Hooks not mentioned here are used statically and with default priority.

function MOD_install
Called on installation of the MOD

function MOD_uninstall
Called on de-installation of the MOD

function MOD_update
Called when the MOD is updated to a newer version

Additonally, a info file with the following information is required:
s an associative array with information about the MOD. Required keys:

• Description : a String describing the MOD
• Target core version : version of phpBB this MOD is intended for
• Version: Version of the MOD
• Author : a string listing the author name
• Installation: a flag describing whether this is an editless MOD or not.
• Requires: An array of other MODs required by this one.
• Plugins: an associative array listing all plugins (captcha/search/db/*cp modules/…) the MOD provides

Optional features of a MOD:

• A modx installation file for core changes (discouraged)
• Language files in the language subdirectory
• Template, javascript and theme files
• Plugin files

Technical aspects
The hook functions of a MOD are enumerated during install and recorded in a table. This enumeration is repeated every time a new MOD is installed or uninstalled. The hook table is cached and stores all MODs to invoke for a given hook.
MOD classes are instantiated on page load, if the MOD declares any non-static hooks. Only one instance per MOD will be created, stored in an array of the static PHPBB class. MODs that are exclusively static are not instantiated automatically.

Debug: Unless DEBUG is defined, phpBB will disable any MODs that are triggering errors. MODs are not to use trigger error, but have to call the phpBB_message method instead. This behavior can be overridden.

Core Hooks (Note, this is an incomplete list subject to changes):

• MOD_start: Called on page load, after the database connection is established
• MOD_exit: Called when the php instance exits, before the database connection is closed
• MOD_cron: Called in cron runs
• MOD_sessionStart: Called when a session is created
• MOD_sessionExit: Called when an user logs out
• MOD_register: Called on all steps of the registration process, with a parameter indicating the current step (‘ToS’, ‘form’, ‘validation’, ‘save’)
• MOD_templateInject: Can return additional template snippets that are added to the template file prior to compilation. A parameter lists the current template region where the injection will take place. Only called when template files are (re-)compiled.
• MOD_css: Css files to add, loaded from the MOD directory. Called on theme compile.
• MOD_js: js files to add, loaded from the MOD directory. Called on template compile
• MOD_post: Called on all steps of the posting process, with a parameter indicating the current step
• MOD_message: Called on all steps of the PM process, with a parameter indicating the current step
• MOD_page_<x>: where <x> is the current phpBB page (index, viewtopic, viewforum, …). Called several times during each page, with a parameter stating the current position in the page.
• MOD_url: all urls created by phpBB are run through this hook.
• MOD_path: Called when an URL is requested – for URL re-writing
• MOD_page: Called when an URL is requested that does not lead to one of the phpBB pages - for adding pages.
• All hooks supported by the 3.0 hook system
• ...

Adding hooks
MODules can introduce their own hooks simply by calling phpBB’s “invoke_hook” method. If the hook to be called is unknown, it will be added to the hook table and all installed MODs will be queried for this hook during the next cron run.

Requesting Hooks
Once the hook infrastructure is committed, we will accept RFCs for placing hooks in the code. Such RFCs should include:

• Reason for adding the hook
• Where to call the hook
• Input values
• Return values
• Actions to be performed on/with the return values (if any)

[naderman]

That sounds pretty good.

Functions to be called by a hook are identified by naming convention. The hook “foo” automatically invokes all MOD methods named “<MODNAME>_foo”.

I'm not sure I get that, if it is a method, then why have the prefix?

Every MOD using the hook system has to provide a class <MODNAME>_class.php implementing the phpBB\MOD interface.

I'd strip the _class suffix from the filename. No namespaces in PHP5.2 yet, so phpbb_mod_interface is what I would suggest

function MOD_hooks

Again, why the MOD prefix? Same for all the other methods, names etc.

returns an associative array mapping hook names to an array holding a flag whether the method is static and the hook’s priority. Hooks not mentioned here are used statically and with default priority.

Why not return an array of name => callable, rather than some static flag? Do we even need to support static methods at all here?

function MOD_info

• Incompatible: An array of other MODs known to be incompatible with this one

Should this kind of meta information really go into the code?

[Kellanved]

I'm not sure I get that, if it is a method, then why have the prefix?

To avoid having methods accidentally registered as hook, especially if a MOD adds custom hooks - the actual naming convention can be changed, naturally.

I'd strip the _class suffix from the filename. No namespaces in PHP5.2 yet, so phpbb_mod_interface is what I would suggest

Sure, those are arbitrary

Again, why the MOD prefix? Same for all the other methods, names etc.

The naming prefix is arbitrary; it was chosen to make it easy to identify the methods required by the interface, vs the methods defined in the actual MOD.

Why not return an array of name => callable, rather than some static flag? Do we even need to support static methods at all here?

The hook info method is invoked on install, so the priority is required (dependencies between MODs might require one hook implementation to be called before another etc). The callable wouldn't help us, as the actual call will be done with call_user_func. The idea is that static calls are much faster than instance calls; moreover, many MODs will be stateless and not require an instance at all.
We could move the static flag to the general MOD info, though - for the method, the static keyword is more important than some flag to be set.

Should this kind of meta information really go into the code?

I just like to have everything in one place. It is entirely feasible to move all the meta info into its own file.

[naderman]

I guess I'll just wait for the implementation on the prefixes, changing those should be trivial if I still don't like them then

Why not return an array of name => callable, rather than some static flag? Do we even need to support static methods at all here?

The hook is invoked on install, so the priority is required (dependencies between MODs might require one hook implementation to be called before another etc). The callable wouldn't help us, as the actual call will be done with call_user_func.

A PHP callable is exactly what you pass to call_user_func(_array) as the first argument, that's actually why I would have done this. To be more clear, a PHP callable is one of the following 3 (in PHP5.2, 5.3 adds closures): 'function_name', array('class_name', 'static_method_name'), array($object, 'method_name').

The idea is that static calls are much faster than instance calls; moreover, many MODs will be stateless and not require an instance at all.
We could move the static flag to the general MOD info, though - for the method, the static keyword is more important than some flag to be set.

In PHP there isn't really any speed difference between static and non-static calls, so that's not a reason to make them static. From a testability point of view static methods are to be avoided because people will start using global scope / static class attributes. You also break inheritance, since late static binding was only added to PHP in version 5.3.

[Kellanved]

Technically we can do without static methods - the idea is to avoid creating unneeded instances.

I'm well aware of the definition of a callable, but a callable is not a priority. We need a priority value, not a callable. Again: The callable wouldn't help us in either case, as we need the priority value to be stored in the database. The callable thus would be rather worthless.

[naderman]

Oh right of course you can't quite cache the callable, my mistake I'd personally prefer an implementation without the option of making something static, because I see more potential for abuse than actual benefit.

[Oleg]

I think the idea is solid. Some considerations:

Please go easy on the use of all caps. They are painful to type. Function names do not need capital letters at all. Remember also that php is case insensitive for function names.

With regard to priorities, experiences with Unix initscripts demonstrate that a simple priority-based system is inadequate. Priorities, which are really an implementation detail, become part of mods' public interface, and resist changing which will inevitably happen once mods begin depending on other mods.

Instead I propose each hook to have a list of mods/hooks that must be run before it, and another list of mods/hooks that it must run before. Priorities then would be computed by phpbb during mod installation.

Target core version : version of phpBB this MOD is intended for

It should probably be a range. In the real world people delay updates for all kinds of reasons, one of which is the lag of styles and modifications behind phpbb releases, sometimes substantial. Version range ensures that, for example, if I'm on phpbb 3.0.6 and I use FooMod 1.0 which is compatible with 3.0.5-3.0.6, I can safely install FooMod 1.1 which is compatible with 3.0.5-3.0.7 while remaining on phpbb 3.0.6, and then safely update phpbb to 3.0.7. This should generally ease the update process and may well have the effect of people staying more current. Naturally, if a mod author wants to test on a single version it is trivial to accomplish with a range as well.

Requires: An array of other MODs required by this one.

Version numbers of required mods should be included, either as "I require mod Foo, version 1.0 through 1.99" or "I require mod Foo, and I know that versions 1.0 through 1.2 work".

Incompatible: An array of other MODs known to be incompatible with this one

Also needs version specification.

Maybe version specification can use wildcards: 1.* means 1.0 <= x < 2.0. Then all boundaries can be made inclusive.

Perhaps it is obvious, but all hooks should go through "requesting hooks" process, in particular explaining the use case for the hook, incuding the core hooks listed.

Specifically on the core hooks:

MOD_register: Called on all steps of the registration process, with a parameter indicating the current step (‘ToS’, ‘form’, ‘validation’, ‘save’)

"Called on all steps" sounds like it would unnecessarily make hook implementations more convoluted than necessary. Why can't separate hooks be used for each step?

It would be beneficial for the hook architecture discussion itself to take a popular mod (or several) and consider how it would look if the architecture was implemented. In fact I strongly suggest writing client code for the hook architecture covering all important functionality points before writing any code for the hook architecture itself.

It may further be a good idea to solicit feedback for the hook architecture from mod writers (via main phpbb forums). A successful system must take its users' needs into account, and the needs can only be discovered by asking users.

Adding hooks
MODules can introduce their own hooks simply by calling phpBB’s “invoke_hook” method. If the hook to be called is unknown, it will be added to the hook table and all installed MODs will be queried for this hook during the next cron run.

Since mods must be installed anyway in order for their hooks to be registered, why not analyze the mods during installation for any hook additions as well? Surely even asking mod authors to list new hooks in a text file would not be too big of an inconvenience.

Whether a hook is invoked statically could be indicated by the hook implementing a (dummy) interface.

[Kellanved]

Please go easy on the use of all caps. They are painful to type. Function names do not need capital letters at all. Remember also that php is case insensitive for function names.

I am not aware of an "all caps". The caps used in the example are mostly placeholders for names; the MOD_ prefix (subject to change) for the escriptor methods being the sole exception.

With regard to priorities, experiences with Unix initscripts demonstrate that a simple priority-based system is inadequate. Priorities, which are really an implementation detail, become part of mods' public interface, and resist changing which will inevitably happen once mods begin depending on other mods.

Instead I propose each hook to have a list of mods/hooks that must be run before it, and another list of mods/hooks that it must run before. Priorities then would be computed by phpbb during mod installation.

That's much more complicated than assigning a priority and can be easily contradictory. Also, the priority approach is much more flexible, as it operates on a per-hook and not on a per-module basis.

Target core version : version of phpBB this MOD is intended for

3.1 would be an example. The granularity won't extend down to releases.

Requires: An array of other MODs required by this one.

All MODs should be kept updated.

Incompatible: An array of other MODs known to be incompatible with this one

I'll remove it.

The core hooks are not set in stone and will see much more specification. Generally, the system will be implemented as described here within the next two weeks.

"Called on all steps" sounds like it would unnecessarily make hook implementations more convoluted than necessary. Why can't separate hooks be used for each step?

Mainly because it makes matters much simpler when looking up the modules to call and is - at least in my experience with hook systems - more readable.
This is a perfect example for something that might and probably will change.

It may further be a good idea to solicit feedback for the hook architecture from mod writers (via main phpbb forums). A successful system must take its users' needs into account, and the needs can only be discovered by asking users.

Area51 is the forum we'll use. Feedback is most welcome.

Since mods must be installed anyway in order for their hooks to be registered, why not analyze the mods during installation for any hook additions as well? Surely even asking mod authors to list new hooks in a text file would not be too big of an inconvenience.

Because it is much simpler to handle such things on the fly rather than implementing yet another proprietary file format.

[Oleg]

Please go easy on the use of all caps. They are painful to type. Function names do not need capital letters at all. Remember also that php is case insensitive for function names.

I am not aware of an "all caps". The caps used in the example are mostly placeholders for names; the MOD_ prefix (subject to change) for the escriptor methods being the sole exception.

Those are the caps I'm referring to. There is no reason to have more than one capital letter sequentially.

With regard to priorities, experiences with Unix initscripts demonstrate that a simple priority-based system is inadequate. Priorities, which are really an implementation detail, become part of mods' public interface, and resist changing which will inevitably happen once mods begin depending on other mods.

Instead I propose each hook to have a list of mods/hooks that must be run before it, and another list of mods/hooks that it must run before. Priorities then would be computed by phpbb during mod installation.

That's much more complicated than assigning a priority and can be easily contradictory. Also, the priority approach is much more flexible, as it operates on a per-hook and not on a per-module basis.

The fact is that a simple priority specification will not work. If you think dependency specification can be contradictory, what makes you think the global mod development community will be able to coordinate priority numbers?

Target core version : version of phpBB this MOD is intended for

3.1 would be an example. The granularity won't extend down to releases.

Is this because a modification written for 3.0.0 is guaranteed to work correctly on 3.0.7, and a modification written for 3.0.7 is guaranteed to work on 3.0.0?

Requires: An array of other MODs required by this one.

All MODs should be kept updated.

Should, yes. Are, no. Every single board in existence goes through periods of time when it is not up to date. Ignoring reality like this will only make updates harder for everyone.

Incompatible: An array of other MODs known to be incompatible with this one

I'll remove it.

Because, of course, all mods are compatible with all other mods?

The core hooks are not set in stone and will see much more specification. Generally, the system will be implemented as described here within the next two weeks.

Does this mean implemented according to your idea of how a hook system should behave, or implemented according to reality?

"Called on all steps" sounds like it would unnecessarily make hook implementations more convoluted than necessary. Why can't separate hooks be used for each step?

Mainly because it makes matters much simpler when looking up the modules to call and is - at least in my experience with hook systems - more readable.
This is a perfect example for something that might and probably will change.

No point in arguing this now, I only wanted to point out that flatter hooks should be considered.

It may further be a good idea to solicit feedback for the hook architecture from mod writers (via main phpbb forums). A successful system must take its users' needs into account, and the needs can only be discovered by asking users.

Area51 is the forum we'll use. Feedback is most welcome.

Are you building a system for users of phpbb or for yourself? It has already been pointed out that existing hook system falls short of being great. The proposal here sounds much better already, but whether the final result is just passable or great depends in part on whether the users' voices are heard. Locking yourself up in an ivory tower is a path to mediocrity.

Since mods must be installed anyway in order for their hooks to be registered, why not analyze the mods during installation for any hook additions as well? Surely even asking mod authors to list new hooks in a text file would not be too big of an inconvenience.

Because it is much simpler to handle such things on the fly rather than implementing yet another proprietary file format.

"Handling things on the fly" also enlarges the critical path, which, if this hook system is successful, will be very often used. One of your stated goals is scalability; your proposal here directly contradicts that goal.

[Kellanved]

Those are the caps I'm referring to. There is no reason to have more than one capital letter sequentially.

This is rather particular; I don't see a problem with having a capital-letter convention for three interface methods. However, as discussed above, this will change.

The fact is that a simple priority specification will not work. If you think dependency specification can be contradictory, what makes you think the global mod development community will be able to coordinate priority numbers?

As there is no requirement for global synchronization, but just for MODs that happen to depend on each other, I'm very optimistic that this approach is sufficient.

Is this because a modification written for 3.0.0 is guaranteed to work correctly on 3.0.7, and a modification written for 3.0.7 is guaranteed to work on 3.0.0?

No, it is because the point of modules is to be independent from changes to the core.

Should, yes. Are, no. Every single board in existence goes through periods of time when it is not up to date. Ignoring reality like this will only make updates harder for everyone.

It's a rather pointless exercise; the general assumption is that all MODs are at their newest version. We are thinking about adding MOD updates to the update warning in the ACP.

Because, of course, all mods are compatible with all other mods?

No, because it is pointless trying to come up with a list of all incompatibilities and side effects.

Does this mean implemented according to your idea of how a hook system should behave, or implemented according to reality?

Interesting, where do you see the mismatch? Care to expand on the subject? Feedback is the sole reason for this topic being here and if you see a pressing problem, please share.

No point in arguing this now, I only wanted to point out that flatter hooks should be considered.

This is more about the design of particular hooks than the system; please post RFCs for the hooks once we are in that phase.

Are you building a system for users of phpbb or for yourself? It has already been pointed out that existing hook system falls short of being great. The proposal here sounds much better already, but whether the final result is just passable or great depends in part on whether the users' voices are heard. Locking yourself up in an ivory tower is a path to mediocrity.

We will certainly post about the developments on .com as well, but this is the forum to use for the discussion.

"Handling things on the fly" also enlarges the critical path, which, if this hook system is successful, will be very often used. One of your stated goals is scalability; your proposal here directly contradicts that goal.

There is no conflict with scalability at all, quite the contrary. The "it just works" factor is extremely important for hook systems, if they are to be adopted - I won't introduce any more descriptors than absolutely required. The creation of a hook is nothing more than the insertion of one row into a table in case of an key not being defined - I don't see any scalability issues at all.