From: Shad L. <sh...@sh...> - 2013-06-26 08:59:44
|
Hey everyone, Following on recent discussions of routing, init files, hooks, etc., I jumped into unifying Gallery's hook-calling system. It turns out that the module order is a bit deeper of a rabbit hole than I originally thought... So, the idea of the cascading file system is to have multiple modules, ordered by their "priority." Let's say we have modules A, B, and C, where A is highest and C is lowest priority. Then, we'd setup our Kohana paths in the following order: - APPPATH - active theme - purifier - module A - module B - module C - gallery - (formo, database, orm, etc.) - SYSPATH Then, Kohana uses them like this: - autoloading classes: uses *forward* order, and stops when it finds something. So, any high-priority module can override something lower on the list. - config files: uses *reverse* order, and goes through whole list. So, the high-priority modules get the last word - whatever variable the low-priority ones set, the high-priority ones can change. This brings us to Gallery's hook system. In Gallery 3.0.x, we used the following order: - gallery - purifier - module A - module B - module C - active theme IMHO, this seems a bit ad hoc and not at all consistent - it shuffles the stack priorities given elsewhere, simply reversing gallery and the active theme and leaving the rest alone. It should be either the same as the path search order, or the exact opposite... ... but which one? Some examples from the core code: - menu: *reverse* order. Gallery's menu system (e.g. GalleryEvent::site_menu()) relies on the gallery module going first. It builds the basic menu, which other modules can modify. - theme: *reverse* order. Gallery's theme system builds HTML blocks in the module order. Here it also makes sense for the higher-priority modules to go last, as they can add JS that modifies things above it. - routes: *forward* order. Kohana's routing/request system follows the init file order, which prioritizes the routes defined first, not last. Since we're doing away with init files, this doesn't help us. So, it looks like we have a couple options: - use *reverse* order by default, but give the option of running oppositely if asked. This is the most flexible approach, but I'm unsure if we really need/want this flexibility. Also, it implies that we'd run all of the gallery_ready events opposite of all other events, which is weird. - use *reverse* order always, and tell Kohana to look for routes in the opposite order. This is actually pretty easy to do: class Gallery_Request extends Kohana_Request { public static function process(Request $request, $routes=null) { // Copy first line of Request::process(), but add array_reverse(). $routes = (empty($routes)) ? array_reverse(Route::all()) : array_reverse($routes); return parent::process($request, $routes); } } My vote is for the second option - it keeps us 100% consistent with the idea that high-priority things get the last word, and cleanly fixes the *one* case where somebody feels otherwise. That said, I could understand if people prefer the opposite approach... One last note: this does have some minor implications for contrib modules. In particular, things that required imposing a module order to ensure events happen in sequence (rawphoto and autorotate before exif, tags before tag_albums) will be temporarily broken until their order is reversed. This is easily fixed with another stanza in GalleryInstaller::upgrade(), ensuring nothing gets broken. Thoughts? Take care, Shad |