[dev] BC break IMP_6_2 API.

[Michael M Slusarz]

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

> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>
>> In IMP_6_1, the $registry->mail->mailboxList() used to return the  
>> mailbox attributes in an 'a' element. This is no longer returned in  
>> IMP_6_2, and potentially breaks any code consuming IMP's public  
>> API. Currently, completely breaks ActiveSync mail synchronization.
>> -- 
>> mike
>>
>> The Horde Project (www.horde.org)
>> mrubinsk at horde.org
>
>
> Ping? This is a must-fix, as it is a BC break in a public facing API.

I have no recollection of removing this.  But reviewing the change, I  
can absolutely see why I removed it.

This appears to be the return from that API call:

  * - a: (integer) The mailbox attributes.

First, what are the attributes?  i.e. if 'a' is 3, what does that  
mean?  From an API perspective, you have to know what is being  
returned without looking at any of the code of the application itself.  
  As documented, this looks like a relic of old code.  Regardless,  
this is a worthless API call - calling code can't count on this  
returning anything other than a number that has no meaning, since that  
is what the defined return value is.

Second, by tracing the code these attributes appear to be internal  
representation data of the IMP_Imap_Tree.  Which is very concerning  
because this is internal data for IMP that has now been incorrectly  
leaked outside the application.  These internal constants of the  
IMP_Imap_Tree object never had a guarantee of not changing, since they  
are not tied to any sort of API.  In fact, those constants did change  
in IMP 6.2 - since IMP_Imap_Tree completely disappeared.

 From a theoretical perspective, I have no problems removing this  
code.  The documentation provides no guarantee of what is being  
returned, so the data returned is ambiguous at best.  The actual data  
returned is irrelevant because the only way calling code would know  
what it was is to trace the internal app code which is not acceptable  
for a remote API call.

 From a *practical* perspective, obviously there is some code that is  
using it.  The only thing I could find was in  
Horde_Core_ActiveSync_Imap_Factory.  And my fears were realized - the  
constant MARK_SUBSCRIBED is defined with a value of "8" which  
apparently was the internal integer representation of the  
IMP_Imap_Tree state at some point in time.  But this is not something  
that could have every been determined from the API of that call.

So I would be open to returning only this value in a "hidden" 'a'  
parameter.  The 'a' parameter itself should be removed from the  
documentation, as explained above, for being too vague to be  
practically useful.  It is not an API break since the API was not  
sufficiently documented in the first place to be usable, but allows  
existing code to function until that code can be fixed.

As a replacement, the question becomes: is this data obtainable  
through other existing API means (e.g. through the Imap_Client  
object)?  Or do we need to add an additional array key - such as  
'subscribed' - to the return of mailboxList?

michael

___________________________________
Michael Slusarz [slusarz at horde.org]