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

[Michael M Slusarz]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Michael J Rubinsky <mrubinsk at horde.org>:
>
>> Quoting Michael M Slusarz <slusarz at horde.org>:
>>
>>> 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 don't disagree with you about ::initialize(), but what I said in  
>> my other email still stands; The changes to notification  
>> initialization in Core will break synchronization to existing  
>> Kronolith installations - or any RPC access that requires access to  
>> a calendar - if Core is updated and Kronolith is not. That's a BC  
>> break.
>>
>> I'll step out of the thread now, and let you and Jan decide if this  
>> is actually to be considered breaking BC or if it can be called a  
>> "bug" in kronolith that necessitates updating kronolith to fix.
>
> To me this is clearly a BC break. It doesn't matter much if the  
> original intention of _init() was different. Fact is it behaved  
> consistently for the complete H4.0 life cycle.
> And I would be surprised if it doesn't affect more applications.  
> Especially if I think of the complex share initialization in Turba.

Re-setting this discussion, since I don't even know where we are at.   
Here's what I see:

* Notifications and Alarms setup HAVE to be cached going forward.   
Nobody has proven to me that these changes broke anything, or are not  
BC.

The only thing that has changed (that I can see) in the  
notification/alarm code is as follows:
   - Applications are only initialized once per session.
   - Non-authenticated apps are initialized (or initialized before the  
entire Horde framework is initialized).

The first shouldn't be an issue (and is the whole reason for the  
change).  The second is NOT A BC BREAK!  Not only was this behavior  
not documented, and thus not part of the API, but this is how we did  
things at the release of Horde 4.  We switched to only checking  
authenticated apps in this commit, a week after Horde 4 was released:

commit dcdd9f52eabc7cd8df5b2ea661097effe039833e
Author: Michael M Slusarz <slusarz at curecanti.org>
Date:   Thu Apr 14 23:48:24 2011 -0600

     Bug #9733:  Don't setup notification handlers in applications  
that are not yet authenticated

Thus, if Kronolith has been initializing things in _init() that aren't  
working because the Horde session itself is not fully initialized,  
KRONOLITH IS BROKEN AND MUST BE FIXED (in 3.0 also).  FYI: this is the  
whole reason for the authAuthenticateCallback() - to do things that  
require the ENTIRE Horde framework to be initialized.

If somebody does prove that they break BC, then I will propose (no,  
demand) that we either allow BC in this instance or we need to move to  
Horde 5.  That is how important these fixes are.  (Especially for  
certain AJAX requests), these fixes demonstrably reduce the load on  
the server by 50% for EVERY page load.

* Horde_Registry_Application#_init() is not being used correctly.  It  
was intended to setup the base necessary to use the application,  
nothing more.  Apparently this is not how it has been used it certain  
applications, but this should be the goal moving forward.

* Even given _init()'s historical usage, and the fact that it MIGHT  
work to initialize everything in there, it shouldn't simply for the  
reason that performance is destroyed anytime that application is  
pushed onto the stack in a page load.

* Kronolith::initialize() needs to be refactored.  GC activities  
should be moved to a login task.  Other data that won't change during  
a page load should be cached in the session.

* Any other bugs that currently exist are a result of refactoring the  
code, not due to architectural changes in the framework.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]