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

Jan Schneider jan at horde.org
Mon Feb 13 09:46:57 UTC 2012


Zitat von Michael J Rubinsky <mrubinsk at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>>
>>> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>>>
>>>> Quoting Michael M Slusarz <slusarz at horde.org>:
>>>>
>>>>> The branch "develop" has been updated.
>>>>> The following is a summary of the commits.
>>>>>
>>>>> from: 7d25aac13b55d57d01aadd7d70b674af04bed9e0
>>>>>
>>>>> fa956a4 Clean up Kronolith_App#_init()
>>>>>
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> commit fa956a4d83a7227e2a5d47bfe87185854e151fbc
>>>>> Author: Michael M Slusarz <slusarz at horde.org>
>>>>> Date:   Sun Feb 12 20:43:32 2012 -0700
>>>>>
>>>>> Clean up Kronolith_App#_init()
>>>>>
>>>>> Get rid of global kronolith_shares variable.
>>>>> Only set timezone/set link tags if kronolith is the current application.
>>>>
>>>>
>>>> This still doesn't solve the rpc.php access issue though. When  
>>>> accessing horde's RPC interface, 'Horde' is *always* the  
>>>> $initialApp. Any application API calls that the RPC request makes  
>>>> occur after appInit() is already called for 'Horde'. This means  
>>>> in Kronolith_Applciation::_init(), Kronolith::initialize() will  
>>>> now *never* be called when accessed via RPC since it is wrapped  
>>>> by the check for kronolith being the $initialApp.
>>>
>>> Actually, thinking this through even more, it will break any API  
>>> access to kronolith when kronolith wasn't the initialApp,  
>>> including loading the initial portal page - the kronolith share  
>>> list would not be able to be built in the sidebar.
>>
>> You can still load shares when creating the sidebar.  But the  
>> sidebar won't be created until later in the page load.  As long as  
>> this doesn't happen in _init(), we are fine.
>
> 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.

>> So that does mean that the timezone stuff needs to go someplace  
>> else.  Maybe Kronolith::initialize()?  But it can't be done in  
>> _init().
>
> Ok, so then the solution is to refactor again to remove  
> Kronolith::initialize() from _init(). It was nice to have it done in  
> one central place, but I'm seeing that this is not going to be  
> possible with the current way apps are initialized.
>
>> The linkTags() can stay as-is for now - it is only used if  
>> Kronolith is the current active application.
>>
>> All of this came to light a few weeks ago when doing some  
>> xdebugging.  In so many words: the SAPO folks were not very happy  
>> at all to see that a large portion of a page access for things like  
>> viewing a message were being spent in kronolith (or, more broadly,  
>> any application), even if NOTHING on the page ever directly  
>> accessed those applications.  I agree.  The two culprits were  
>> application notification handling and alarm checking.
>
> Agreed, though don't the changes in Core border on a BC break since  
> updating Core without updating Kronolith would lead to broken RPC  
> usage, including SyncML and ActiveSync?

Agreed.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/



More information about the dev mailing list