[dev] [commits] Horde branch develop updated. fa956a4d83a7227e2a5d47bfe87185854e151fbc

[Michael M Slusarz]

Quoting Michael J Rubinsky <mrubinsk at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>> Quoting Jan Schneider <jan at horde.org>:
>>
>>>> This is currently broken though, because Kronolith::initialize()  
>>>> is where we determine what shares (and other, non-share based  
>>>> calendars) are available. I just tested this, and in fact the  
>>>> sidebar does not contain the calendar list when viewing the horde  
>>>> portal.
>>>>
>>>>> _init() is for bootstrapping only.  You can't call  
>>>>> registry/horde-wide things in there due to the chicken/egg  
>>>>> problem.  _init() is not designed to set up the full application  
>>>>> environment.
>>>
>>> But we have been using it for exactly that since Horde 4. We  
>>> cannot simply change the semantics of an API method because it  
>>> doesn't work anymore after some refactoring in Horde or Horde_Core.
>>> _init() has always been the place to set up the app environment  
>>> since we dropped base.php.
>>
>> Except this is *exactly* how we've been using it.  And it is  
>> exactly what the API says it should be used for:
>>
>> Does any necessary init needed to setup the full environment for  
>> the application.
>>
>> Full environment meaning enough to load the application.  Not to  
>> load **EVERYTHING** the application may eventually need to possibly  
>> use on a page.  See, e.g., IMP which does nothing in _init() except  
>> load the basic environment.
>
> Well, I would argue that determining what calendars are available  
> for use in Kronolith is a pretty fundamental requirement of any  
> kronolith action, so even if we buy in to what you say in the above  
> paragraph, this still belongs in _init().

I stand by what I said before - but no need for me to continue  
discussing my position.

I agree that Kronolith will most likely need a more thorough base  
initialization as opposed to an application like IMP that doesn't need  
to worry about either guest access and stores its pertinent data  
(mail) remotely instead of on the Horde installation.  But on the flip  
side, there is no reason for about 90% of that stuff in  
Kronolith::initialize().

Almost everything in there can be initialized once per session.  E.g.  
- is there really a need to initialize the holiday list on every page  
load?  And does any given page use *everything* that is initialized in  
that method?  I would guess not.  So I disagree with the statement  
that all of this stuff is "fundamental".

Maybe what this boils down to is that the initialize() code needs to  
be cleaned up immediately.  Most of the stuff in there can be  
refactored to either load-on-demand or be initialized once and cached  
in the session.  You are going to see a giant speed boost when this  
happens.  I would even say this is the single most important feature  
that Kronolith 3.1 can offer.

I added back the Kronolith::initialize() call to _init() for now until  
this can be cleaned up.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]