From: Shad L. <sh...@sh...> - 2013-06-30 05:54:10
|
Hey everyone, Okay, final followup: I implemented something slightly different from what I said below. Quick summary: module event order is the same as Gallery 3.0.x, so this should affect approximately 0 people. -- The modules are assigned a weight in the DB (for this example, "module 1" has weight 1, "module 2" has 2, etc.). In Gallery 3.0.x, we sent this module list to Kohana in a way that was inconsistent with the order in which events are fired. 3.0.x class autoloading order (using list sent to Kohana): - APPPATH - active theme - (unit test modules, if applicable) - module 1 - module 2 - module 3 - gallery - (3rd-party modules, e.g. forge) - SYSPATH Result: first modules get their extensions loaded first, so they have priority - module 1 > module 3. 3.0.x module event order: - gallery - module 1 - module 2 - module 3 - active theme Result: last modules get the final say in what happens, so they have priority - module 3 > module 1. In practice, it seems that no module cares about the class autoloading order (so long as gallery is last), but several modules care about the event order. So, let's preserve that. 3.1.x class autoloading order: - APPPATH - active theme - (unit test modules, if applicable) - purifier - module 3 - module 2 - module 1 - gallery - (3rd-party modules, e.g. formo) - SYSPATH Result: first modules get their extensions loaded first, so they have priority - module 3 > module 1. 3.0.x module event order: - gallery - module 1 - module 2 - module 3 - purifier - active theme Result: last modules get the final say in what happens, so they have priority - module 3 > module 1. This gives us a consistent priority/hierarchy setup while maintaining the same event ordering as before. In other words, it makes nit-pickers like me happy while affecting approximately 0 actual users :-) Take care, Shad On 26 June 2013 11:15, Shad Laws <sh...@sh...> wrote: > Quick followup... funny how writing things up can clarify things for you... > > At the end of the email, I proposed two 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. > > ... and now realize that option 2 is definitely the better way to go. In > addition to what I said, there's another reason: running gallery_ready > events in the opposite order of everything else (i.e. gallery module last) > is a surefire way to break sso and user_homes. This is because > GalleryEvent::gallery_ready() is what loads the user in the first place, so > it's gotta go first. > > Alright, ramble over. Thanks for listening, or at least pretending to! > > Take care, > Shad > > > > > On 26 June 2013 10:59, Shad Laws <sh...@sh...> wrote: >> >> 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 > > |