[dev] BC break IMP_6_2 API.

[Michael J Rubinsky]

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

> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>
>> I hear what you are saying, but this is still an API break. I could  
>> easily add phpdoc to that API method explaining the value of the  
>> bitmask in IMP 6.1.x before IMP 6.2 is released. Regardless if it  
>> was the correct way to originally implement it or that it wasn't  
>> documented fully doesn't make it NOT a BC break.
>
> This is probably a fundamental disagreement between you and I when  
> it comes to what makes an API then.  For me, the API is what is  
> defined in the documentation.  It doesn't matter what was actually  
> returned as of 6.0.0.  Because it most certainly is not the job of  
> someone using the API to have to go back and trace the code manually  
> to figure this value out.

I agree. The point I'm making though is that instead of you saying it  
wasn't really part of the API because it wasn't documented,  I could  
just as easily say that this is a "bug" in the documentation of the  
API (and fix it in the next bug fix release). Either way, the point is  
moot now.

>>> 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?
>>
>> I need to be able to determine if the mailbox is subscribed or not.  
>> Reading the phpdoc of Horde_Imap_Client_Mailbox I don't see any way  
>> to get that data from the object. Besides, we shouldn't be relying  
>> on objects being passed via the API when at all possible. So, how  
>> about a 's' array key?
>
> I'm not talking about Horde_Imap_Client_Mailbox.  I'm talking about  
> Horde_Imap_Client.  This object is used elsewhere in the ActiveSync  
> code (which, btw, sort of goes against the whole "APIs shouldn't be  
> returning objects" argument.  How is ActiveSync supposed to work if  
> IMP's API doesn't return this object?  But that's another thread...)

Well, as someone said in that other thread, over the years some of  
these objects slipped in. Perhaps it was because (at least in the case  
of HIC) asking for these objects over RPC makes little sense anyway.  
Regardless, I agree that in H6 we need to look at this and perhaps  
implement another php/internal (or RPC/external) interface. To address  
your point though, I used HIC since it was already available. If it  
wasn't already returned from the stable API I would not have added it  
and would have had to create a HIC locally in  
Horde_Core_ActiveSync_Driver:: using IMP's configuration (or more  
accurately, the 'mail' application's configuration) read via the  
registry.

> I'm wary of returning subscribed information directly from IMP,  
> since the subscribe status can be influenced by UI specific IMP  
> preferences that.  i.e IMP has the 'subscribe' preference that  
> determines whether unsubscribed mailboxes are displayed by default.   
> But I'm not sure that's the purpose of mailboxList.  Is it supposed  
> to be returning the mailbox listing as defined by IMP or is it  
> supposed to be returning the mailbox listing as defined on the IMAP  
> server?  (That being said, I think it is the former, so I'm not sure  
> whether that is correct.)

The whole point I used mailboxList() was so that any IMP  
configuration/hooks/preferences WOULD be taken into account so that  
what the user saw on the ActiveSync client matched what they saw in  
IMP. I assumed that this would be the case. The ActiveSync client  
should represent what the UI of the paired account looks like.

When building the folderlist to send to the EAS client, we only take  
subscribed folders into account. So if the folders are subscribed in  
IMP, they should appear on the client. If they are not subscribed in  
IMP, the don't appear in the client. I could do this manually by using  
HIC, but (1) I don't want to duplicate all that code, and, (2) I want  
to utilize any prefs/hooks/configuration in IMP that may affect the  
mailbox listing.

So, to your point, my opinion would be that mailboxList() should  
return the mailbox listing according to IMP. If some application wants  
the "raw" information from the IMAP server, that's what HIC is for.

> If we are going to add an entry to the return, I would rather make  
> it 'subscribed' rather than 's'.

Ok.
-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5849 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/dev/attachments/20131023/e9fcd380/attachment.bin>