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

[Michael J Rubinsky]

Quoting Michael M Slusarz <slusarz at horde.org>:

> 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.

I stand by my earlier statement - this *IS* a BC break. How can it not  
be? If Core is updated, existing application functionality breaks. It  
doesn't much matter what any documentation does or does not say - a  
user is only going to care that their synchronization, or some custom  
application that uses RPC access is broken. For instance, the Ansel  
uploader plugins for iPhoto and Aperture rely on existing RPC  
behavior. If initializing the share system of applications when  
accessed via RPC is broken (which is currently is) - these uploaders  
will be broken too.

> 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

Which is a change you committed after commenting that you didn't know  
why we were polling unauthenticated apps for notifications. What  
changed with the documentation of the API since then that makes the  
earlier behavior more appropriate?

> 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.

Another issue to consider is that rpc requests initialize Horde with  
authentication => 'none' (we delegate authentication to the Rpc  
classes), and when I attempted to refactor things using this callback,  
it was *never* called from rpc requests.

> 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.

Regarding the AJAX requests, Jan had already made changes that  
prevents Kronolith::initialize from being called during AJAX requests.

> * 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.

A good bit, though I agree not all, of the initialization done there  
may very well change during the session.

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

Again, I say it doesn't matter *what* you call the cause -  
architectural or a result of refactoring - the end result is that  
existing functionality breaks when Core is updated.


-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org