[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

User avatar
rxu
Registered User
Posts: 126
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

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

Post by rxu » Mon Jan 02, 2012 5:22 am

Oleg
Could you please provide an extended description of your alternative ("version 2") hooks architecture?
That probably would help to contribute in developing it.
Image

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

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

Post by Oleg » Tue Jan 03, 2012 6:27 am

The problem is I don't really have a design even so much as some rough ideas about going about getting it designed.

I believe last time I looked into hooks I got the template insertion working.

Also, some of my earlier posts in this topic have been superseded by the extensions.

I still need answers for viewtopic.php?f=84&t=32805&start=30#p231831 so you can possibly help with that or poke Nils.

User avatar
rxu
Registered User
Posts: 126
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

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

Post by rxu » Tue Jan 03, 2012 2:05 pm

Oleg wrote:so you can possibly help with that or poke Nils.
Well, all the Dev team should participate, I just can through my own opinion here:
Oleg wrote:Extension ordering. Which order does extension finder return files in? Looks like this is not documented, and possibly not even defined.
Currently, extensions info is stored in the special table:

Code: Select all

# Table: 'phpbb_ext'
CREATE TABLE phpbb_ext (
	ext_name varchar(255) DEFAULT '' NOT NULL,
	ext_active tinyint(1) UNSIGNED DEFAULT '0' NOT NULL,
	ext_state text NOT NULL,
	UNIQUE ext_name (ext_name)
) CHARACTER SET `utf8` COLLATE `utf8_bin`;
As we can see, it does not have ext_id column. I'd suggest to create that to be able to sort extensions by id. Why is it needed? Imagine you have 2 or more extensions which exploit the same hook. Having id, we can 1) state what extension would overwrite previous ones results if needed, and 2) the order of extensions were installed (for example, for debugging purposes/etc). Probably, there're some more pros with the id.
So, generally, my suggestion is to sort by id. As for the order id is assigned, I assume it's the same PHP reads files from directory, that means "The entries are returned in the order in which they are stored by the filesystem."
Oleg wrote: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.
Personally, I think (and it seems to be apparent thing) template hooks are style-unique. You can invoke template hook for all styles, but it should be a user choice. Generally speaking, one template hook, being invoked for some custom/modified/etc-non-default style, can break the page layout completely.
As for inheritance, I think it should be user configured as well if the hook should be inherited by every style.
Speaking about implementation, I'm not sure to be honest. Probably, it's depending on the answer for the question #3:
Oleg wrote:Should we compile extensions' templates when core templates are compiled, or when templates are executed?
For my taste, it could be the latter way, it looks like it should let to control things dynamically.

Please note that my comments can be irrelevant in some parts since even you
Oleg wrote:don't really have a design even so much as some rough ideas about going about getting it designed.
;)
Image

User avatar
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 » Wed Jan 04, 2012 11:05 am

rxu wrote:As we can see, it does not have ext_id column. I'd suggest to create that to be able to sort extensions by id. Why is it needed? Imagine you have 2 or more extensions which exploit the same hook. Having id, we can 1) state what extension would overwrite previous ones results if needed, and 2) the order of extensions were installed (for example, for debugging purposes/etc). Probably, there're some more pros with the id.
So, generally, my suggestion is to sort by id. As for the order id is assigned, I assume it's the same PHP reads files from directory, that means "The entries are returned in the order in which they are stored by the filesystem."
I don't think this is a good idea. We already have the unique ext name, it's probably best to sort by that. Installing the same N MODs should always have the same result in terms of hook execution order. Hooks should be able to define their own priority, higher priority hooks get executed first.

sajaki
Registered User
Posts: 86
Joined: Mon Jun 21, 2010 8:28 pm

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

Post by sajaki » Mon Jan 09, 2012 7:40 am

What about mod dependencies, say mod C requires mod A & B to be installed, can hooks manage that order ?
if not I'd like to be able to sort by install timestamp, id and name.
Automod has had this same problem, and there was even a sorting mod developed for it.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

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

Post by Oleg » Fri Feb 03, 2012 8:47 am

I was making good progress today until php ruined everything, as it tends to do.

First, I like https://github.com/phpbb/phpbb3/pull/529. I rebased my work on top of event dispatcher and replaced run_hooks calls with event dispatcher stuff.

At this point https://github.com/p/phpbb3-ext-overall-header-addition dropped in the phpbb directory produced a good-looking (?) yellow background everywhere.

Procedure to reproduce:

1. QI-install phpbb.
2. rsync https://github.com/p/phpbb3/compare/dev ... 2Fledges-2 over the install and run database updater.
3. rsync https://github.com/p/phpbb3-ext-overall-header-addition over the install.
4. enable the extension (currently needs to be done by creating an appropriate entry in the ext table?)
5. view the board.

To make #4 more reasonable I did https://github.com/phpbb/phpbb3/pull/546 but when I got to enabling and disabling of extensions php decided to rear its ugly head, which is where I stopped. Someone fix that script please.

User avatar
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 » Fri Feb 03, 2012 12:11 pm

Thank you. Everything is in feature/ledges-2? No PR for this?

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

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

Post by Oleg » Fri Feb 03, 2012 6:07 pm

https://github.com/phpbb/phpbb3/pull/551

Pull requests are messy because of all the different branches that are going on.

User avatar
naderman
Product Manager
Product Manager
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

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

Post by naderman » Wed Feb 22, 2012 11:38 pm

It would be great if we can resolve extensions at compile time and at runtime based on maybe a define in config.php. Projects like debian, run multiple boards with a single code base, they rely on the template cache being usable by all boards. If extensions are compiled into templates, this will no longer be the case. However compiling them in might be faster. So having them resolved at compile time would be the default, but a define would allow moving all extension related calculations to runtime.

Post Reply