Re: Module order for hooks (event, theme, etc.)

--===============7884844996866975114==
Content-Type: multipart/alternative; boundary=047d7b6d9da43b8d0e04e00b184b

--047d7b6d9da43b8d0e04e00b184b
Content-Type: text/plain; charset=UTF-8

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 <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> 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
>

--047d7b6d9da43b8d0e04e00b184b
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Quick followup... funny how writing things up can clarify things for you...=
<div><br></div><div>At the end of the email, I proposed two options:</div><=
div><div>- use *reverse* order by default, but give the option of running o=
ppositely if asked. =C2=A0This is the most flexible approach, but I&#39;m u=
nsure if we really need/want this flexibility. =C2=A0Also, it implies that =
we&#39;d run all of the gallery_ready events opposite of all other events, =
which is weird.</div>

<div>- use *reverse* order always, and tell Kohana to look for routes in th=
e opposite order. =C2=A0This is actually pretty easy to do.</div><div><br><=
/div><div>... and now realize that option 2 is definitely the better way to=
 go. =C2=A0In addition to what I said, there&#39;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. =C2=A0This is =
because GalleryEvent::gallery_ready() is what loads the user in the first p=
lace, so it&#39;s gotta go first.</div>

<div><br></div><div>Alright, ramble over. =C2=A0Thanks for listening, or at=
 least pretending to!</div><div><br></div><div>Take care,</div><div>Shad</d=
iv><div><br></div><div><br></div><div><br></div><div><br></div><div class=
=3D"gmail_quote">

On 26 June 2013 10:59, Shad Laws <span dir=3D"ltr">&lt;<a href=3D"mailto:sh=
ad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>&gt;</span> wrote:<=
br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left=
:1px #ccc solid;padding-left:1ex">

Hey everyone,<div><br></div><div>Following on recent discussions of routing=
, init files, hooks, etc., I jumped into unifying Gallery&#39;s hook-callin=
g system. =C2=A0It turns out that the module order is a bit deeper of a rab=
bit hole than I originally thought...</div>


<div><br></div><div>So, the idea of the cascading file system is to have mu=
ltiple modules, ordered by their &quot;priority.&quot; =C2=A0Let&#39;s say =
we have modules A, B, and C, where A is highest and C is lowest priority. =
=C2=A0Then, we&#39;d setup our Kohana paths in the following order:</div>


<div>- APPPATH</div><div>- active theme</div><div>- purifier</div><div>- mo=
dule A</div><div>- module B</div><div>- module C</div><div>- gallery</div><=
div>- (formo, database, orm, etc.)</div><div>- SYSPATH</div><div><br></div>


<div>Then, Kohana uses them like this:</div><div>- autoloading classes: use=
s *forward* order, and stops when it finds something. =C2=A0So, any high-pr=
iority module can override something lower on the list.</div><div>- config =
files: uses *reverse* order, and goes through whole list. =C2=A0So, the hig=
h-priority modules get the last word - whatever variable the low-priority o=
nes set, the high-priority ones can change.</div>


<div><br></div><div>This brings us to Gallery&#39;s hook system. =C2=A0In G=
allery 3.0.x, we used the following order:</div><div><div>- gallery</div><d=
iv>- purifier</div><div>- module A</div><div>- module B</div><div>- module =
C</div>


<div><div>- active theme</div></div></div><div><br></div><div>IMHO, this se=
ems a bit ad hoc and not at all consistent - it shuffles the stack prioriti=
es given elsewhere, simply reversing gallery and the active theme and leavi=
ng the rest alone. =C2=A0It should be either the same as the path search or=
der, or the exact opposite...</div>


<div><br></div><div>... but which one? =C2=A0Some examples from the core co=
de:</div><div>- menu: *reverse* order. =C2=A0Gallery&#39;s menu system (e.g=
. GalleryEvent::site_menu()) relies on the gallery module going first. =C2=
=A0It builds the basic menu, which other modules can modify.</div>


<div>- theme: *reverse* order. =C2=A0Gallery&#39;s theme system builds HTML=
 blocks in the module order. =C2=A0Here it also makes sense for the higher-=
priority modules to go last, as they can add JS that modifies things above =
it.</div>


<div>- routes: *forward* order. =C2=A0Kohana&#39;s routing/request system f=
ollows the init file order, which prioritizes the routes defined first, not=
 last. =C2=A0Since we&#39;re doing away with init files, this doesn&#39;t h=
elp us.</div>


<div><br></div><div>So, it looks like we have a couple options:</div><div>-=
 use *reverse* order by default, but give the option of running oppositely =
if asked. =C2=A0This is the most flexible approach, but I&#39;m unsure if w=
e really need/want this flexibility. =C2=A0Also, it implies that we&#39;d r=
un all of the gallery_ready events opposite of all other events, which is w=
eird.</div>


<div>- use *reverse* order always, and tell Kohana to look for routes in th=
e opposite order. =C2=A0This is actually pretty easy to do:</div><div><br><=
/div><div><font face=3D"courier new, monospace">class Gallery_Request exten=
ds Kohana_Request {</font></div>


<div><font face=3D"courier new, monospace">=C2=A0 public static function pr=
ocess(Request $request, $routes=3Dnull) {</font></div><div><font face=3D"co=
urier new, monospace">=C2=A0 =C2=A0 // Copy first line of Request::process(=
), but add array_reverse().</font></div>


<div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 $routes =3D (empty=
($routes)) ? array_reverse(Route::all()) : array_reverse($routes);</font></=
div><div><font face=3D"courier new, monospace">=C2=A0 =C2=A0 return parent:=
:process($request, $routes);</font></div>


<div><font face=3D"courier new, monospace">=C2=A0 }</font></div><div><font =
face=3D"courier new, monospace">}</font></div><div><font face=3D"courier ne=
w, monospace"><br></font></div><div>My vote is for the second option - it k=
eeps us 100% consistent with the idea that high-priority things get the las=
t word, and cleanly fixes the *one* case where somebody feels otherwise. =
=C2=A0That said, I could understand if people prefer the opposite approach.=
..</div>


<div><br></div><div>One last note: this does have some minor implications f=
or contrib modules. =C2=A0In particular, things that required imposing a mo=
dule order to ensure events happen in sequence (rawphoto and autorotate bef=
ore exif, tags before tag_albums) will be temporarily broken until their or=
der is reversed. =C2=A0This is easily fixed with another stanza in GalleryI=
nstaller::upgrade(), ensuring nothing gets broken.</div>


<div><br></div><div>Thoughts?</div><div><br></div><div>Take care,</div><div=
>Shad</div>
</blockquote></div><br></div>

--047d7b6d9da43b8d0e04e00b184b--


--===============7884844996866975114==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
--===============7884844996866975114==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]
--===============7884844996866975114==--