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

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
Jhong
Registered User
Posts: 50
Joined: Tue Dec 26, 2006 3:28 pm

Re: General Hook Architecture of phpBB3.1

Post by Jhong »

yes, integrating with other packages. The first hook is not as important as the last hook, but both are nice to have.

For the early hook, it is nice to be able to set defines to override error reporting settings, etc. as they might not be compatible with other packages.

If the hook system is DB driven, fine -- but perhaps allow to auto-include any files in the hooks folder very early, before the hook system is initialised, so we can at least run some procedural code early.

Nelsaidi
Registered User
Posts: 122
Joined: Tue Nov 11, 2008 5:44 pm

Re: General Hook Architecture of phpBB3.1

Post by Nelsaidi »

Whats the latest on hooks in 3.1?

Given the OOPless format of phpBB hooks should be documented (typically you'd just pass $this to the callback) - And there should be a good bundle of hooks/event triggers in 3.1 to cover the absolute basics at the very least:

user sign up; user login; page render; warning issue; new topic; new post; post delete;

Are all things I'd like to see, whilst I understand area 51 is to discuss the needs of functionality forthe general public, I do feel these being the very basics could be used by many (ie, blog integration, membership integration).

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: General Hook Architecture of phpBB3.1

Post by igorw »

Nelsaidi wrote:Given the OOPless format of phpBB hooks should be documented (typically you'd just pass $this to the callback) - And there should be a good bundle of hooks/event triggers in 3.1 to cover the absolute basics at the very least:
Re-read the first post. The new hooks will use OOP.

User avatar
EXreaction
Registered User
Posts: 1555
Joined: Sat Sep 10, 2005 2:15 am

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by EXreaction »

In addition to the proposed design in the first post, but this goes somewhat against the current design implementation in the development branch for it.

Modify the existing 3.0 hooks system to fit our needs and wants.

Notes:
  • Hook means a single point in the code where additional code can be run.
  • Plugin means a package which may use one or more hooks to modify the phpBB system without editing files.
Change the hook class to use static methods
  • No more declaring global $phpbb_hook in order to use it
Remove the requirement of sending the list (array) of valid hooks to the hook class
  • It's simply not reasonable to have a list of valid hooks for 3.1, as there will be too many hooks.
  • Adding hooks becomes a real PITA with requiring the declaration of valid hooks before allowing the new hook to be called
  • Any invalid registers by plugins (registering a hook that does not exist) is not a problem (it'll just never be called)
Simplify the use model for hooks, three common needs are satisfied by hooks:
  1. Taking over a function completely to replace it with your own (should only rarely occur)
  2. Modifying data (input and output)
  3. Outputing additional data to the template
Two main styles of hooks can accomplish that:
  • Taking over a function; this looks similar, though simplified, to what we use already:

    Code: Select all

    		// If boolean false is returned, that means something has taken over the function
    		if (phpbb_hook::call_hook(__FUNCTION__, $url, $params, $is_amp, $session_id) === false)
    		{
    			return;
    		}
  • Modifying data could look like this:

    Code: Select all

    		// Here we can modify some variables
    		extract(phpbb_hook:call_hook(array('posting', 'submit'), compact('sql_data', 'poll_data', ...)));
    
    		// Or a simpler version for sending a single variable for modification:
    		extract(phpbb_hook:call_hook(array('posting', 'test'), $test_variable));
  • Outputting additional data to the template (or for other reasons) in key areas without the need for data modification:

    Code: Select all

    		// This is easy
    		phpbb_hook::call_hook(array('foo', 'bar'));
Expanding hooks into templates:
  • Option one: simply add extra template variables for plugins to use to the template
    This is a bit ugly and requires a bunch of unique template variable names.
    Probably requires that we create our own output function in the hooks class to prevent overwritting of others and enforce it's use
  • Option two: Add a new kind of template variable for hooks
    It could look like:
    {HOOK posting, userdata}
    This is not as ugly, does not require long unique variable names, and is not prone to overwriting
    Requires we create our own output function in the hooks class to output what the plugin wants
Allowing templates to act like plugins too only for when that template is loaded:
  • When a template is used, a check for a plugins.php file would be made by the hooks system.
    This greatly increases the flexibility for tweaking some data sent to the user, allowing styles to become even more customised.
  • A warning during installation of a style if the plugins file exists and a global on/off for template plugins (like the PHP include)
Plugins (packages):
  • The current method of just dropping a plugin file in a directory and it automatically starting up does not fit the user's needs
    Users should be able to install, enable, disable, and uninstall plugins
    • Install:
      The installation method is what is required first, it allows plugins to easily handle any DB alterations required before installation
    • Enable/Disable:
      Enables or disables the plugin without uninstalling (losing) what is stored in the database.
    • Uninstall:
      Removes any changes made to the database as well (the plugin handles it through an uninstall method)
  • A new ACP page is created to list installed/uninstalled hooks and manage them.
  • A database table holds the list of currently installed plugins which are to be loaded at runtime.
  • At runtime the plugin files are included and register the locations that they need.
  • There is a slight problem that could come up here however. In the instance where a change to an existing DB table is made and a plugin is disabled.
    • This may cause SQL errors in some cases.
    • In order to prevent this we can do two things:
    • Plugins can specify that they are not able to be disabled (requires uninstallation to disable)
    • This is the easiest method, but data will be lost for temporary disables
    • Plugins can specify some specific items to be called if they are installed but disabled
    • A bit more difficult, but data will be saved without SQL errors
If something here isn't clear just ask, I wrote this up on little sleep after a long day. :P

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by igorw »

EXreaction wrote:Change the hook class to use static methods
This is a bad idea. Static methods lead to monolithic design and make testing hard. Plus you don't really get much benefit, except not having to global your instance.

Are you suggesting to re-start the implementation of the hooks system based on the existing hooks in 3.0?

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by igorw »

There's two things I'd like to suggest this system to support.

Namespacing

Some MODs may want to provide their own hook locations. To avoid naming clashes it would be great to be able to namespace these. Something simple like:

Code: Select all

$hook_handler->add_hook('blog.add_post', $callable);
The core hooks don't need a 'core' namespace, they can just use the global namespace instead.

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Berlin, Germany
Contact:

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by naderman »

I'd say that your "dynamic proxies" are really quite independent of what this topic is discussing. Using __call is quite a performance hit so you wouldn't normally want to do that for methods called tons of times on every request. It's also something a MOD could easily do by itself, we could still provide such a proxy but it's really little code. The concept also doesn't fit into the automatically registered hooks concept.

As for the namespaces I agree that that is a good idea but don't really see any sensible way to enforce it, so it'll be up to MOD authors to do that. Could be mentioned in documentation.

User avatar
A_Jelly_Doughnut
Registered User
Posts: 1780
Joined: Wed Jun 04, 2003 4:23 pm

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by A_Jelly_Doughnut »

naderman wrote: As for the namespaces I agree that that is a good idea but don't really see any sensible way to enforce it, so it'll be up to MOD authors to do that. Could be mentioned in documentation.
No way to enforce in MODs, but we could come up with examples in the core code where namespacing might help too
A_Jelly_Doughnut

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] General Hook Architecture of phpBB3.1

Post by igorw »

Yes, this should definitely be highlighted in the hook docs.

James78
Registered User
Posts: 2
Joined: Sat Oct 22, 2005 4:53 pm

Re: General Hook Architecture of phpBB3.1

Post by James78 »

nn- wrote:
Kellanved wrote: 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.
I don't know if this changed or not yet, or if my input is even worth anything as I'm not a development team member, but I would like to add here, on the topic of version specification.. If you're going to have a feature to allow wildcards for the versions, you might as well just allow the authors to use a regex; that way specifying the version(s) are truly in the hands of the MOD author. I would much rather be able to (possibly) have many options regarding this. E.g., you can use a simple string '3.0.8', a range '3.0.6-3.0.8' *, or a regex, which is ultimately so much more power in the hands of the modder (if they ever need it). :)

* Not sure how easy or feasible that actually is, as version strings may not always end up matching what you want, e.g. What if my version string is '0.9-RC-PL1'? How then can a range work on that really?

Edit: Arggg.. My mistake. I'm pretty sure the general hook architecture has already been finalized.. :oops:

Post Reply