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

You are still wrong.  It is NOT a BC break.  Just because KRONOLITH is  
incorrectly using the API does not make anything in Core wrong.

There are only 2 ways you can break backward compatibility:

1. You explicitly change an API element.  We all agree that is not the  
case here (these changes don't change what a method either accepts or  
returns).
2. Alternatively, it could be argued that in the absence of a  
clearly-defined API, you change some critical system as it was  
performed hen 4.0 was released.  As already described, in 4.0 the  
defined behavior is that non-authenticated applications would have  
init() called BEFORE the full Horde environment was created.

To reiterate: init() is solely a bootstrapping function to allow  
enough of the application environment to be setup so that it could be  
accessed by the Registry.  That's all it was ever designed to do.  As  
amply shown, it was never guaranteed to have the full Horde  
environment (this is why authAuthenticateCallback() exists).

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

If Ansel is not following the limitations described above, then the  
correct fix is to fix Ansel.  There's nothing wrong with Core - just  
how Ansel uses Core.

As far as versions - this is the whole point of package.xml.  Will the  
next version of Kronolith 3.x require Horde_Core 1.6.0 (or whatever  
the next release is)?  Yes.  That's just the reality of the situation.

Can you upgrade Horde_Core without upgrading Kronolith?  No.  But  
that's also the reality of the situation.

Imagine this scenario: MIME encoding of an address.  Let's say an  
application parsed the results of an encode for the quoted-printable  
signature to determine if the message was encoded, and performed an  
action based on the encoding status.  In a future version of the MIME  
library, it was decided to change quoted-printable encoding into  
binary encoding.

This change would "break" the existing application if the library is  
updated but not the Application.  But this is not a BC break.  This is  
an example of an application making incorrect assumptions about the  
API.  Nobody could possibly argue that we would have to maintain the  
old way of MIME encoding just because a downstream application won't  
be able to handle it properly.

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

Because this was a broken assumption.  This prevents non-authenticated  
applications from setting notification handlers, which is perfectly  
acceptable under the original API (as it should be).  So the original  
bug-fix was flat wrong.

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

This seems like a limitation in the Registry_Application API then that  
was not contemplated before Horde 4 came out.  Sounds like you will  
have to manually do Authentication checking in Kronolith.  Or add a  
new method that is called in this situation.  But you can't hijack the  
init() function to do something it wasn't supposed to do.


I'll be honest: revisiting this kind of BC discussion/argument is  
exactly what I was fearful of a year ago.  This adherence to BC is the  
single biggest problem of Horde, as it has been for the last 10 years.  
  I have had preliminary thoughts of forking IMP for precisely this  
reason so I could avoid all this headache.  I spend way too much time  
worrying about these sorts of issues - something like releasing  
versions of IMP that require the latest version of all required  
components would eliminate a tremendous amount of wasted time for me.   
There's a bunch of stuff that I have wanted to do, but have not been  
able to because of these limitations.  Not my intention to sound  
combative, but it accurately portrays the frustration I have felt the  
last few months.  Or maybe my goals are simply diverging from others'.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]