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

[Oleg]

Work done so far: https://github.com/kellanved/phpbb3/com ... ature/9550

[Oleg]

And now for something completely different: https://github.com/p/hooks-poc

This is a from-scratch implementation of proof of concept-level code. It aims to answer the question: What is the smallest practically useful hooks implementation?

To that end, a number of decisions, restrictions and omissions have been made, most importantly:

* Performance is important. See http://tracker.phpbb.com/browse/PHPBB3-9448. Every function call and array lookup, etc. counts. Runtime cost of hook invocation is kept to the minimum.
* Hook signature is the same for all hooks. Hooks accept a single argument which must be a reference. What the reference is to is unspecified. This allows multiple parameters and return values.
* Modifications must supply unique priorities for all hooks. How priorities are calculated is outside of the scope of hook system. In my opinion this is something that mod installer would do. Practically speaking I might add priority calculation to the same repository.
* Code should be straightforward to read and understand. At the moment the entire hook implementation is pretty much https://github.com/p/hooks-poc/blob/mas ... k_impl.php.
* Author info, versions, conflicts and so on are omitted and are outside of the scope. The hook system assumes that some sort of a "mod installer" ensures a consistent set of modifications exists.

Here is how it works. Modifications specify what hooks they define (each hook is a uniquely named function), priority for every hook and whether the hook replaces hooks with lower priority. The hook system uses this information to construct a hook dispatcher tree, which is a tree of classes.

Each class in the hook dispatcher tree corresponds to one priority level. Inside each class are methods corresponding to hooks defined at that priority level (recall that for each hook the priority level may only be defined by one modification). The method calls uniquely named hook function to do the work. If the hook is marked as replacing lower priority hooks, that's all it does. Otherwise before calling the current level's hook the parent method is called, which may call earlier hooks following the same logic.

Code that wants to become hookable essentially invokes the appropriate hook method of the topmost (highest priority) class in the dispatcher tree.

A property that if I'm not mistaken currently holds for the proof of concept project is that the metadata is only needed at build time and actual hook functions are only needed at runtime.

[Oleg]

I compiled a list of top 5 modifications, by download count, with versions for 3.0.7 or 3.0.8:

• advertisement management moddb code
• thanks for posts moddb
• board3 portal moddb code - soon to move to github I've been told
• mchat moddb code?
• last post topic title moddb code

[Oleg]

Looking at these modifications (board3 portal specifically) it is clear that hooks for templates are more than "just a hook function call". This modification adds a conditional around core template code. Therefore, template hooks must be executed during template compilation, before the current operation of compiling phpbb template language into php.

Regardless, from looking at these modifications it is again clear that most edits are in styles, not in php code. So far this RFC does not mention how templates will be hooked at all. Even if it is "trivial" the process still needs to be described.

On the topic of implementation, I think the API that the hook system will expose is more important to get right than the implementation, because the api is going to be fixed and implementation can be changed at any time. Two of the mods have github repositories; they are easiest to fork and modify to take advantage of hooks. I intend to supply modified mods in parallel to any further proposed hook system implementations.

Barring one edit in mchat (the one involving javascript) which needs some help from phpbb, I believe all of these mods can be made editless.

[igorw]

The template hooks (which are really quite different to the PHP hooks) is loosely described in [RFC] hook_template_snippet.

[Oleg]

I did some thinking/prototyping today, mostly having to do with template hooks. The use case I'm thinking of is (from one of the above modifications) adding template fragments around a piece of core template:

Code: Select all

...
(fragment from a modification)
<div ...>
(phpbb code)
</div>
(another fragment from the same modification)
...


The fragments above may contain any template constructs, and in particular they do not need to be complete - e.g., the first fragment opens an IF tag and the second fragment closes it (this is the actual code in the mod).

-----

Modifications have:

- code hooks
- template hooks

Glossary

Hook: code that is part of a modification that is run as if it was placed in a particular location
(Hook) location: a place in code that will invoke modifications' code, also its name which should satisfy ^\w+$ regexp
Hook manager: class that performs hook-related tasks which are not performance sensitive, that is,
those that are not done on every page load
Hook dispatcher: (?) class that performs performance sensitive hook-related tasks, that is,
those that could be done on every page load

The job of the hook dispatcher is to:
1. Determine which hooks are defined for a particular location,
2. Invoke those hooks
as efficiently as possible. In particular, this means expending as little effort as possible processing hook locations that do not have defined hooks.

The job of the hook manager is to:
1. Determine (or delegate to another component) which modifications are installed,
2. Determine which hooks each modification has defined,
(thus determining which hooks are defined system-wide)
3. Organize the defined hooks into a data structure that may be efficiently queried by the hook dispatcher.

Hook definition

Hooks may be declared by modifications or comply with a convention that would allow them to be automatically discovered.

A requirement is that hooks for any particular modification should be determinable without running any of the modification's code. Why this requirement? It would make it possible to remove broken modifications and probably make it easier to repair the board should something go wrong. At the same time it is not absolutely needed, maybe it will end up deferred or removed.

Hook ordering

Hooks need to run in a well-defined order. Going back to the template example above if two modifications wrap the same core fragment the order in which the before and after hooks are invoked is important.

Because the set of hooks could be different for each board the hook ordering is, generally speaking, specific to a particular board as well.

The hook manager is responsible for figuring out some usable ordering during installation of board, modifications or hooks or some similar process.

Hook priority

It seems useful to give modifications some control over where their hooks end up as far as ordering goes. This could be an absolute numeric value as proposed earlier in this topic or some kind of relationship with other hooks/modifications which was my proposal. The exact details are not necessary for implementing a functional hook manager. It is actually possible that an arbitrary ordering would work for a lot of modifications (here arbitrary is with respect to different modifications, nesting must be done correctly which requires a certain measure of internal consistency).

------

Template construct to be added:

Code: Select all

<!-- RUNHOOKS location_name -->


It is evaluated when the template is compiled.

Needed from hooks system:

- ability to get all template fragments for a given hook location, in correct order

Needed from template system:

- reentrant template compilation

Hooks api:

phpbb_hook_manager::get_all_template_fragments_for_location($location)

Possibly have the order flag in RUNHOOKS call.
But is order resolution not orthogonal concept?
Could be simpler if we have a limited before/after concept instead of arbitrary ordering.

<!-- RUNHOOKS before_something -->
<!-- RUNHOOKS -after_something -->

Whatever order modifications are arranged in, - before location will invert it and use the inverted order.

[Oleg]

Rebased on current develop: https://github.com/p/phpbb3/compare/dev ... re%2Fhooks

With the extension manager merged, my todo list can actually be implemented now

[Oleg]

Progress: https://github.com/p/phpbb3/compare/tic ... re%2Fhooks

Surprisingly, it did not take mountains of code to implement a minimally useful template hook: https://github.com/p/phpbb3/commit/59f4 ... 9c323d481d

From that hooks seem to need the following from the extension manager:

1. Ability to specify extension basename instead of extension prefix+suffix. The idea is that each extension may provide 0 or 1 implementation of each template hook.

And the following questions are open:

1. Extension ordering. Which order does extension finder return files in? Looks like this is not documented, and possibly not even defined.
2. Some template hooks are style-specific, some are not (i.e. they should be invoked for all styles, they are providing sitewide markup). Specifying style name over and over in hooks is suboptimal, but currently the template engine does not know which style it is rendering.
   
   1. Should we make the template engine aware of the style it is compiling/rendering?
   2. Should we somehow inject this from outside the template engine and forward this all the way to template filter?
   3. In case of inheritance, if a style is FooStyle derived from BarStyle, and a hook is defined in a template in BarStyle, should we look in FooStyle or BarStyle under the extensions to find the template files to use with this hook?
3. Should we compile extensions' templates when core templates are compiled, or when templates are executed? In the first approach, which is what I have so far, if an extension is enabled or disabled all templates must be recompiled. Naturally the first approach is faster.

[Oleg]

I created a pull request for comments: https://github.com/phpbb/phpbb3/pull/491

Be warned that I am likely to rebase the code every day I change something. There are already changes to the template engine that are going in before hooks, there are likely changes needed to the extension manager which will also go before hooks, then there are hooks and I want to keep hook locations separate from hooks. New commits will be getting spliced into the diff in appropriate places.

[Oleg]

The template hooks (which are really quite different to the PHP hooks) is loosely described in [RFC] hook_template_snippet.

I read that again and I think I will rename RUNHOOKS to REGION.