Re: [Gallery3] Unit test updates
Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> Mon, 24 Jun 2013 12:07:55 +0200
| Newsgroups | gmane.comp.web.gallery.devel |
|---|---|
| Message-ID | <CA+z51A4Vkd0ECR1HpOZs7FLF7L-jEDhvoRF_MM1rsYMmbQ6SMQ@mail.gmail.com> |
--===============5997758129541420715==
Content-Type: multipart/alternative; boundary=047d7b6da78243458e04dfe39791
--047d7b6da78243458e04dfe39791
Content-Type: text/plain; charset=UTF-8
Hey Bharat,
Hmm, I'm not sure I see routing and execution as one inseparable unit. It
seems to me that they decided to remove their event handler and instead
make the cascading file system (which has far less exceptions to rules than
before) and the config file system beefier to pick up the slack. For
example, extending the Controller class as we do is a nice way to split the
two. Alternatively, we can just generate the request and wait a tick
before executing it.
Here's my sketch of the end of index.php:
// Initialize the framework.
require APPPATH . "bootstrap" . EXT;
// Initialize the Gallery modules.
register_shutdown_function("Module::event", "shutdown");
Module::event("gallery_ready");
// Build the initial request.
$request = Request::factory(true, array(), false);
Module::event("initial_request_ready");
// Generate and send the response.
echo $request->execute()
->send_headers(true)
->body();
The functions Gallery::ready() and Gallery::shutdown() are removed, as they
were just one-line wrappers around their respective module events. I like
this more direct approach.
But, this did show me an odd wrinkle with module events: the module order
seems weird.
- active modules list: purifier, high_priority_module, low_priority_module,
gallery (same as Kohana's list)
- current event order: gallery, purifier, high_priority_module,
low_priority_module (put gallery first, leave rest as-is)
- proposed event order: gallery, low_priority_module, high_priority_module,
purifier (reverse of active list, same as Kohana's config file search)
In other words, I think it should be changed to run the events in reverse
module order, thereby giving the high-priority modules the last word. Is
there a reason we don't already do it this way?
Among other advantages, if we can change this, then the routes get loaded
in the right order :-).
Take care,
Shad
On 24 June 2013 00:33, Bharat Mediratta <[email protected]> wrote:
>
>
>
> On Sun, Jun 23, 2013 at 8:33 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>
>> Hey Bharat,
>>
>> Overall, makes sense to me. A couple points:
>> - My guess is that K3 intentionally dropped the postrouting events with
>> the MVC-->HMVC change since routing occurs with every new request <shrug>.
>> - I'm not sure I like "gallery_request_ready" as I feel like it should
>> run with every request (which it doesn't - in K3-speak, it fires only in
>> the initial request and not on sub-requests). Perhaps
>> "initial_request_ready" instead?
>>
>
> That makes total sense. I forgot about the fact that you can go through
> that process multiple times. It's irritating to me that they made the
> routing and execution a single inseparable unit.
>
>
>> - The shutdown function currently uses PHP's register_shutdown_function()
>> in the bootstrap (
>> https://github.com/gallery/gallery3/blob/kohana_3/application/bootstrap.php#L245).
>> I'm in no way committed to its placement... if you'd like it elsewhere,
>> that works for me, too :-).
>>
>
> Ok that makes sense too - that way it gets called even if there's an
> exception or an error. We should keep it that way.
>
>
>> - Your partition of the tasks by event makes sense to me. Is there any
>> reason the bot detection part is in Gallery::ready() instead of
>> Hook_GalleryEvent::gallery_ready()?
>>
>
> No, just arbitrary.
>
>
>> - Agreed - user_homes and sso should still work just fine with the new
>> events.
>>
>> Oh, and re: rambly stream of consciousness, I'm probably not in a great
>> position to critique emails like that as of late... :-)
>>
>> Thoughts?
>>
>
> All sounds good to me!
>
>
>>
>> Take care,
>> Shad
>>
>>
>> On 21 June 2013 19:36, Bharat Mediratta <[email protected]> wrote:
>>
>>> (sorry for the rambly stream of consciousness here, sorry)
>>>
>>> Ok, backing up to a high level - the issue is that we need to reboot our
>>> routes every time there's a module change, right? So by the time we get to
>>> Controller::execute() we need to have initialized our routes already, which
>>> is why we do it in init.php. And we need our own Route::init() function
>>> that goes off and gets routes from all the modules.
>>>
>>> Makes sense to have two events. But I think it's a little weird to say
>>> "before_initial_request" and "after_initial_request" because the after one
>>> implies to me that the request is done. In K2 it was "prerouting" and
>>> "postrouting" but those were Kohana events, not Gallery events.
>>>
>>> There are two ways to think about our events. One is to instruct
>>> modules to take an action, like "please inject your routes now". The other
>>> is to let modules know that we're at a milestone, like "Hey the Gallery is
>>> ready". Kohana takes the milestone approach, but Gallery takes the action
>>> approach (with the exception of "gallery_ready" and "gallery_shutdown").
>>>
>>> It's irritating that K3 doesn't have a "postrouting" style hook for us.
>>>
>>> One quick note is that I think we should put these events in index.php
>>> since they're about the lifecycle of the app, so it'd look like this:
>>>
>>> // Initialize the framework.
>>> require APPPATH . "bootstrap" . EXT;
>>>
>>> *Gallery::ready();*
>>>
>>> // Go!
>>> echo Request::factory(true, array(), false)
>>> ->execute()
>>> ->send_headers(true)->body();
>>>
>>> *Gallery::shutdown();*
>>>
>>> Then in Gallery_Controller::execute() we can have a new event which is "
>>> *gallery_request_ready*" which means "the request is ready but hasn't
>>> been executed yet". That should allow us to load the theme and set the
>>> request locale after we have a request. So in Gallery_Hook_GalleryEvent:
>>>
>>> gallery_ready()
>>> - set date.timezone
>>> - Identity::load_user()
>>>
>>> gallery_request_ready():
>>> - bot detection
>>> - Theme::load_themes()
>>> - Locales::set_request_locale()
>>>
>>>
>>> I think this is generally where you got.. does that sound right to you?
>>>
>>> Notes:
>>> - I don't see where Gallery::shutdown is called in the new code. Did
>>> that get dropped?
>>> - the 3.0 user_homes module uses gallery_ready to redirect the user to a
>>> new location before the request occurs - I think we can still do that in
>>> the 3.1 gallery_ready
>>> - the 3.0 sso module uses gallery_ready to change the active user - I
>>> think the 3.1 gallery_ready works for that as well
>>>
>>>
>>>
>>> On Thu, Jun 20, 2013 at 11:06 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>>
>>>> I tried playing with this, and now remember why we put it in
>>>> Controller::execute() - if we put it before the initial request generation,
>>>> we (unsurprisingly) don't have access to a populated Request object. In
>>>> particular, this screws up Theme::load_themes(), which uses Request to
>>>> figure out if we're in admin mode or have an override. It also screws up
>>>> our check for a robot user_agent - while it doesn't bomb the code,
>>>> Request::user_agent() checks against a still-unset value.
>>>>
>>>> A few options:
>>>> - hack Theme::load_themes() and the robot check to no longer need
>>>> Request. While technically possible, this sounds messy... I don't think
>>>> it's a good approach.
>>>> - move a couple things out of the "ready" event, and leave them in
>>>> Controller::execute(). This gets the job done without major hacks, but the
>>>> fact that we need to do this in the first place hints that a better
>>>> approach would be to...
>>>> - have two events, one right before the initial Request and one right
>>>> after. This is my current favorite approach.
>>>>
>>>> Next question: if we do the two-event option, what should they be
>>>> called? Some possibilities:
>>>> - Before request: init (exact Kohana analog), routes (most common
>>>> application), bootstrap, before_initial_request
>>>> - After request: gallery_ready (current name), ready (shorter name),
>>>> after_initial_request
>>>>
>>>> Thoughts?
>>>> Shad
>>>>
>>>> On 21 June 2013 01:12, Bharat Mediratta <[email protected]> wrote:
>>>>
>>>>>
>>>>> I think that's reasonable. I've never been 100% happy with putting
>>>>> the "ready" event in Controller::execute.
>>>>>
>>>>>
>>>>> On Thu, Jun 20, 2013 at 3:44 PM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
>>>>>
>>>>>> I like the idea of reducing the init.php's down to zero and putting
>>>>>> it in an event, and agree that trying to keep track of this isn't a good
>>>>>> idea. In fact, we could probably just move the execution of the
>>>>>> already-existing "ready" event from Controller::execute() to either
>>>>>> bootstrap.php or index.php (just before the initial request generation). I
>>>>>> wonder - should it be renamed "init" to make its analog more obvious?
>>>>>>
>>>>>> Then, we just add a Route override that clears out the Route list,
>>>>>> and use it in Module to activate/deactivate things as well as in the unit
>>>>>> test.
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>> Take care,
>>>>>> Shad
>>>>>>
>>>>>> On 21 June 2013 00:12, Bharat Mediratta <[email protected]> wrote:
>>>>>>
>>>>>>>
>>>>>>> I've been wondering about that as well, but I wasn't thinking about
>>>>>>> it from the unit test perspective. I was thinking more along the lines of
>>>>>>> activating and deactivating modules during a request's lifetime. Eg, if we
>>>>>>> deactivate and reactivate a module several times, what is a developer
>>>>>>> supposed to do about their code in init.php? Should devs guard against
>>>>>>> their code being called multiple times? It feels messy to me.
>>>>>>>
>>>>>>> I think a better approach would be to minimize what goes into
>>>>>>> init.php, preferably down to zero. We can leave the facility around, but
>>>>>>> since we're only using it for routes I think we could easily have an event
>>>>>>> hook which sets routes, then we can call that from Module whenever we add a
>>>>>>> module.
>>>>>>>
>>>>>>> If we want to be smart about removing routes when a module is
>>>>>>> deactivated (not essential for us to do right now, IMO) we could always
>>>>>>> track which routes were added when we call the hook and remove them when
>>>>>>> the module goes away, similar to how we do Graphics::deactivate_rules
>>>>>>> and BlockManager::deactivate_blocks...
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Jun 20, 2013 at 10:52 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]>wrote:
>>>>>>>
>>>>>>>> Hey Bharat,
>>>>>>>>
>>>>>>>> It looks like, except for the REST-related unit tests I'm still
>>>>>>>> wrapping up, the only tests left are controller auth and XSS - woohoo!
>>>>>>>>
>>>>>>>> I did, however, hit a minor snag. I got everything running well
>>>>>>>> and passing, hit a reset button, and then everything broke with the REST
>>>>>>>> tests - crap! After a bit of digging, I found the problem: the REST tests
>>>>>>>> won't pass on a fresh installation. They only pass if you activate the
>>>>>>>> rest module, *then* run unit tests.
>>>>>>>>
>>>>>>>> The issue is the rest module's init.php file. Like the other
>>>>>>>> init.php files (e.g. in tag), it's designed to run *before* the bootstrap
>>>>>>>> routes are defined. That way, they get precedence. However, if you roll a
>>>>>>>> fresh install (which has rest disabled) straight into a unit test, it bombs
>>>>>>>> - the bootstrap runs first, then rest's init.php later... and it doesn't
>>>>>>>> work :-/
>>>>>>>>
>>>>>>>> One thought I have is to override the Route class so we can clear
>>>>>>>> them and restart from scratch, then make the unit tests re-load them all...
>>>>>>>> but that sounds kinda hacky and flaky (e.g. what happens if an init.php
>>>>>>>> does something besides load routes?). Any better thoughts?
>>>>>>>>
>>>>>>>> Take care,
>>>>>>>> Shad
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
--047d7b6da78243458e04dfe39791
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Hey Bharat,<div><br></div><div>Hmm, I'm not sure I see routing and exec=
ution as one inseparable unit. =C2=A0It seems to me that they decided to re=
move their event handler and instead make the cascading file system (which =
has far less exceptions to rules than before) and the config file system be=
efier to pick up the slack. =C2=A0For example, extending the Controller cla=
ss as we do is a nice way to split the two. =C2=A0Alternatively, we can jus=
t generate the request and wait a tick before executing it.</div>
<div><br></div><div>Here's my sketch of the end of index.php:</div><div=
><br></div><div><div><font face=3D"courier new, monospace">// Initialize th=
e framework.</font></div><div><font face=3D"courier new, monospace">require=
APPPATH . "bootstrap" . EXT;</font></div>
<div><font face=3D"courier new, monospace"><br></font></div><div><font face=
=3D"courier new, monospace">// Initialize the Gallery modules.</font></div>=
<div><font face=3D"courier new, monospace">register_shutdown_function("=
;Module::event", "shutdown");</font></div>
<div><font face=3D"courier new, monospace">Module::event("gallery_read=
y");</font></div><div><font face=3D"courier new, monospace"><br></font=
></div><div><font face=3D"courier new, monospace">// Build the initial requ=
est.</font></div>
<div><font face=3D"courier new, monospace">$request =3D Request::factory(tr=
ue, array(), false);</font></div><div><font face=3D"courier new, monospace"=
>Module::event("initial_request_ready");</font></div><div><font f=
ace=3D"courier new, monospace"><br>
</font></div><div><font face=3D"courier new, monospace">// Generate and sen=
d the response.</font></div><div><font face=3D"courier new, monospace">echo=
$request->execute()</font></div><div><font face=3D"courier new, monospa=
ce">=C2=A0 ->send_headers(true)</font></div>
<div><font face=3D"courier new, monospace">=C2=A0 ->body();</font></div>=
</div><div><br></div><div>The functions Gallery::ready() and Gallery::shutd=
own() are removed, as they were just one-line wrappers around their respect=
ive module events. =C2=A0I like this more direct approach.</div>
<div><br></div><div>But, this did show me an odd wrinkle with module events=
: the module order seems weird.</div><div>- active modules list: purifier, =
high_priority_module, low_priority_module, gallery =C2=A0 (same as Kohana&#=
39;s list)</div>
<div>- current event order: gallery, purifier, high_priority_module, low_pr=
iority_module =C2=A0 (put gallery first, leave rest as-is)</div><div>- prop=
osed event order: gallery, low_priority_module, high_priority_module, purif=
ier =C2=A0 (reverse of active list, same as Kohana's config file search=
)</div>
<div><br></div><div>In other words, I think it should be changed to run the=
events in reverse module order, thereby giving the high-priority modules t=
he last word. =C2=A0Is there a reason we don't already do it this way?<=
/div>
<div><br></div><div>Among other advantages, if we can change this, then the=
routes get loaded in the right order :-).</div><div><br></div><div>Take ca=
re,</div><div>Shad</div><div><br></div><div><br></div><div><div class=3D"gm=
ail_quote">
On 24 June 2013 00:33, Bharat Mediratta <span dir=3D"ltr"><<a href=3D"ma=
ilto:[email protected]" target=3D"_blank">[email protected]</a>></span=
> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bo=
rder-left:1px #ccc solid;padding-left:1ex">
<div dir=3D"ltr"><br><div class=3D"gmail_extra"><br><br><div class=3D"gmail=
_quote">On Sun, Jun 23, 2013 at 8:33 AM, Shad Laws <span dir=3D"ltr"><<a=
href=3D"mailto:shad-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:1p=
x #ccc solid;padding-left:1ex">Hey Bharat,<div><br></div><div>Overall, make=
s sense to me. =C2=A0A couple points:</div><div>- My guess is that K3 inten=
tionally dropped the postrouting events with the MVC-->HMVC change since=
routing occurs with every new request <shrug>.</div>
<div>- I'm not sure I like "gallery_request_ready" as I feel =
like it should run with every request (which it doesn't - in K3-speak, =
it fires only in the initial request and not on sub-requests). =C2=A0Perhap=
s "initial_request_ready" instead?</div>
</blockquote><div><br></div><div>That makes total sense. =C2=A0I forgot abo=
ut the fact that you can go through that process multiple times. =C2=A0It&#=
39;s irritating to me that they made the routing and execution a single ins=
eparable unit.</div>
<div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8=
ex;border-left:1px #ccc solid;padding-left:1ex">
<div>- The shutdown function currently uses PHP's register_shutdown_fun=
ction() in the bootstrap =C2=A0(<a href=3D"https://github.com/gallery/galle=
ry3/blob/kohana_3/application/bootstrap.php#L245" target=3D"_blank">https:/=
/github.com/gallery/gallery3/blob/kohana_3/application/bootstrap.php#L245</=
a>). =C2=A0I'm in no way committed to its placement... if you'd lik=
e it elsewhere, that works for me, too :-).</div>
</blockquote><div><br></div><div>Ok that makes sense too - that way it gets=
called even if there's an exception or an error. =C2=A0We should keep =
it that way.</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=
=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>- Your partition of the tasks by event makes sense to me. =C2=A0Is the=
re any reason the bot detection part is in Gallery::ready() instead of Hook=
_GalleryEvent::gallery_ready()?<br></div></blockquote><div><br></div><div>
No, just arbitrary.</div><div>=C2=A0</div><blockquote class=3D"gmail_quote"=
style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><d=
iv></div><div><div>- Agreed - user_homes and sso should still work just fin=
e with the new events.</div>
</div><div><br></div><div>Oh, and re: rambly stream of consciousness, I'=
;m probably not in a great position to critique emails like that as of late=
... :-)</div><div><br></div><div>Thoughts?</div></blockquote><div><br>
</div><div>All sounds good to me!</div><div>=C2=A0</div><blockquote class=
=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padd=
ing-left:1ex"><div><br></div><div>Take care,</div>
<div>Shad<div><div><br><br><div class=3D"gmail_quote">On 21 June 2013 19:36=
, Bharat Mediratta <span dir=3D"ltr"><<a href=3D"mailto:[email protected]=
om" target=3D"_blank">[email protected]</a>></span> wrote:<br><blockquo=
te class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc so=
lid;padding-left:1ex">
<div dir=3D"ltr">(sorry for the rambly stream of consciousness here, sorry)=
<div><br><div>Ok, backing up to a high level - the issue is that we need to=
reboot our routes every time there's a module change, right? =C2=A0So =
by the time we get to Controller::execute() we need to have initialized our=
routes already, which is why we do it in init.php. =C2=A0And we need our o=
wn Route::init() function that goes off and gets routes from all the module=
s. =C2=A0</div>
<div><br></div><div>Makes sense to have two events. =C2=A0But I think it=
9;s a little weird to say "before_initial_request" and "afte=
r_initial_request" because the after one implies to me that the reques=
t is done. =C2=A0In K2 it was "prerouting" and "postrouting&=
quot; but those were Kohana events, not Gallery events.</div>
</div><div><br></div><div>There are two ways to think about our events. =C2=
=A0One is to instruct modules to take an action, like "please inject y=
our routes now". =C2=A0The other is to let modules know that we're=
at a milestone, like "Hey the Gallery is ready". =C2=A0Kohana ta=
kes the milestone approach, but Gallery takes the action approach (with the=
exception of "gallery_ready" and "gallery_shutdown").<=
/div>
<div><br></div><div>It's irritating that K3 doesn't have a "po=
strouting" style hook for us.</div><div><br></div><div>One quick note =
is that I think we should put these events in index.php since they're a=
bout the lifecycle of the app, so it'd look like this:</div>
<div><br></div><div><div><font face=3D"courier new, monospace">// Initializ=
e the framework.</font></div><div><font face=3D"courier new, monospace">req=
uire APPPATH . "bootstrap" . EXT;</font></div><div><font face=3D"=
courier new, monospace"><br>
</font></div><div><font face=3D"courier new, monospace"><b>Gallery::ready()=
;</b></font></div><div><font face=3D"courier new, monospace"><br></font></d=
iv><div><font face=3D"courier new, monospace">// Go!</font></div><div>
<font face=3D"courier new, monospace">echo Request::factory(true, array(), =
false)</font></div>
<div><span style=3D"font-family:'courier new',monospace">=C2=A0 -&g=
t;execute()</span><br></div><div><font face=3D"courier new, monospace">=C2=
=A0 ->send_headers(true)->body();</font></div><div><font face=3D"cour=
ier new, monospace"><br>
</font></div><div><font face=3D"courier new, monospace"><b>Gallery::shutdow=
n();</b></font></div><div><font face=3D"courier new, monospace"><br></font>=
</div><div>Then in Gallery_Controller::execute() we can have a new event wh=
ich is "<b>gallery_request_ready</b>" which means "the reque=
st is ready but hasn't been executed yet". =C2=A0That should allow=
us to load the theme and set the request locale after we have a request. =
=C2=A0So in Gallery_Hook_GalleryEvent:</div>
<div><br></div><div>gallery_ready()</div><div>- set date.timezone</div><div=
>- Identity::load_user()</div><div><br></div><div>gallery_request_ready():<=
/div><div>- bot detection<br>
</div><div>- Theme::load_themes()</div><div>- Locales::set_request_locale()=
<br></div><div><br></div><div><br></div><div>I think this is generally wher=
e you got.. does that sound right to you?</div>
<div><br></div><div>Notes:</div><div>- =C2=A0I don't see where Gallery:=
:shutdown is called in the new code. =C2=A0Did that get dropped?</div><div>=
- the 3.0 user_homes module uses gallery_ready to redirect the user to a ne=
w location before the request occurs - I think we can still do that in the =
3.1 gallery_ready</div>
<div>- the 3.0 sso module uses gallery_ready to change the active user - I =
think the 3.1 gallery_ready works for that as well</div><div><br></div></di=
v></div><div class=3D"gmail_extra"><br><br><div class=3D"gmail_quote">
On Thu, Jun 20, 2013 at 11:06 PM, Shad Laws <span dir=3D"ltr"><<a href=
=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>></=
span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8e=
x;border-left:1px #ccc solid;padding-left:1ex">
I tried playing with this, and now remember why we put it in Controller::ex=
ecute() - if we put it before the initial request generation, we (unsurpris=
ingly) don't have access to a populated Request object. =C2=A0In partic=
ular, this screws up Theme::load_themes(), which uses Request to figure out=
if we're in admin mode or have an override. =C2=A0It also screws up ou=
r check for a robot user_agent - while it doesn't bomb the code, Reques=
t::user_agent() checks against a still-unset value.<div>
<br></div><div>A few options:</div><div>- hack Theme::load_themes() and the=
robot check to no longer need Request. =C2=A0While technically possible, t=
his sounds messy... I don't think it's a good approach.</div><div>-=
move a couple things out of the "ready" event, and leave them in=
Controller::execute(). =C2=A0This gets the job done without major hacks, b=
ut the fact that we need to do this in the first place hints that a better =
approach would be to...</div>
<div>- have two events, one right before the initial Request and one right =
after. =C2=A0This is my current favorite approach.</div><div><br></div><div=
>Next question: if we do the two-event option, what should they be called? =
=C2=A0Some possibilities:</div>
<div>- Before request: init (exact Kohana analog), routes (most common appl=
ication), bootstrap, before_initial_request</div><div>- After request: gall=
ery_ready (current name), ready (shorter name), after_initial_request</div>
<div><div><br></div><div>Thoughts?</div><span><font color=3D"#888888"><div>=
Shad</div></font></span><div><div><div><br><div class=3D"gmail_quote">On 21=
June 2013 01:12, Bharat Mediratta <span dir=3D"ltr"><<a href=3D"mailto:=
[email protected]" target=3D"_blank">[email protected]</a>></span> wro=
te:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex"><div dir=3D"ltr"><br><div>I think that's=
reasonable. =C2=A0I've never been 100% happy with putting the "re=
ady" event in Controller::execute.</div>
</div><div class=3D"gmail_extra"><br><br><div class=3D"gmail_quote">
On Thu, Jun 20, 2013 at 3:44 PM, Shad Laws <span dir=3D"ltr"><<a href=3D=
"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>></spa=
n> wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;b=
order-left:1px #ccc solid;padding-left:1ex">
I like the idea of reducing the init.php's down to zero and putting it =
in an event, and agree that trying to keep track of this isn't a good i=
dea. =C2=A0In fact, we could probably just move the execution of the alread=
y-existing "ready" event from Controller::execute() to either boo=
tstrap.php or index.php (just before the initial request generation). =C2=
=A0I wonder - should it be renamed "init" to make its analog more=
obvious?<div>
<br></div><div>Then, we just add a Route override that clears out the Route=
list, and use it in Module to activate/deactivate things as well as in the=
unit test.</div><div><br></div><div>Thoughts?</div><div><br></div><div>
Take care,</div><div>Shad</div><div><div><div><div><div><br><div class=3D"g=
mail_quote">On 21 June 2013 00:12, Bharat Mediratta <span dir=3D"ltr"><<=
a href=3D"mailto:[email protected]" target=3D"_blank">[email protected]</=
a>></span> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex"><div dir=3D"ltr"><br><div>I've been wond=
ering about that as well, but I wasn't thinking about it from the unit =
test perspective. =C2=A0I was thinking more along the lines of activating a=
nd deactivating modules during a request's lifetime. =C2=A0Eg, if we de=
activate and reactivate a module several times, what is a developer suppose=
d to do about their code in init.php? =C2=A0Should devs guard against their=
code being called multiple times? =C2=A0It feels messy to me.</div>
<div><br></div><div>I think a better approach would be to minimize what goe=
s into init.php, preferably down to zero. =C2=A0We can leave the facility a=
round, but since we're only using it for routes I think we could easily=
have an event hook which sets routes, then we can call that from Module wh=
enever we add a module.</div>
<div><br></div><div>If we want to be smart about removing routes when a mod=
ule is deactivated (not essential for us to do right now, IMO) we could alw=
ays track which routes were added when we call the hook and remove them whe=
n the module goes away, similar to how we do=C2=A0Graphics::deactivate_rule=
s and=C2=A0BlockManager::deactivate_blocks...</div>
<div><br></div><div><br></div></div><div class=3D"gmail_extra"><br><br><div=
class=3D"gmail_quote">On Thu, Jun 20, 2013 at 10:52 AM, Shad Laws <span di=
r=3D"ltr"><<a href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" target=3D"_blank">shad@s=
hadlaws.com</a>></span> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">Hey Bharat,<div><br></div><div>It looks like=
, except for the REST-related unit tests I'm still wrapping up, the onl=
y tests left are controller auth and XSS - woohoo!</div>
<div><br></div><div>I did, however, hit a minor snag. =C2=A0I got everythin=
g running well and passing, hit a reset button, and then everything broke w=
ith the REST tests - crap! =C2=A0After a bit of digging, I found the proble=
m: the REST tests won't pass on a fresh installation. =C2=A0They only p=
ass if you activate the rest module, *then* run unit tests.</div>
<div><br></div><div>The issue is the rest module's init.php file. =C2=
=A0Like the other init.php files (e.g. in tag), it's designed to run *b=
efore* the bootstrap routes are defined. =C2=A0That way, they get precedenc=
e. =C2=A0However, if you roll a fresh install (which has rest disabled) str=
aight into a unit test, it bombs - the bootstrap runs first, then rest'=
s init.php later... and it doesn't work :-/</div>
<div><br></div><div>One thought I have is to override the Route class so we=
can clear them and restart from scratch, then make the unit tests re-load =
them all... but that sounds kinda hacky and flaky (e.g. what happens if an =
init.php does something besides load routes?). =C2=A0Any better thoughts?</=
div>
<div><br></div><div>Take care,</div><div>Shad</div>
</blockquote></div><br></div>
</blockquote></div><br></div></div></div>
</div></div></blockquote></div><br></div>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div></div>
</blockquote></div><br></div>
--047d7b6da78243458e04dfe39791--
--===============5997758129541420715==
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
--===============5997758129541420715==
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 ]
--===============5997758129541420715==--