[dev] [commits] Horde branch master updated. 807ce308a199bb099eba18e853d2d9f94f3a8780

[Michael M Slusarz]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Michael M Slusarz <slusarz at horde.org>:
>
>> 399111b phpdoc; bump up timeframe to be labeled a 'slow' command

Tweak.

>> cac2e79 [mms] Move IMAP mailboxes sorting into new  
>> Horde_Imap_Client_Mailbox_List class.

API bugfix.  Imap_Client methods that return mailboxes need to return  
this object (instead of an array) so that we can potentially add  
further features **without having to break the API every minor  
release**.

>> 0960738 [mms] Remove Horde_Imap_Client_Utils#removeBareNewlines().

API bugfix.  This should have been removed months ago with the switch  
to Horde_Imap_Client_Data_Format_*.

>> e9f9906 [mms] Remove Horde_Imap_Client_Utils#escape().

Same.

>> f960868 [mms] Move IMAP sequence string generation/parsing to the  
>> Horde_Imap_Client_Ids object.

Bugfix - mailbox information could potentially leak into situations  
where only IMAP sequence string is supported.  Regardless, this fixed  
a hackish/BC way this was implemented in Horde_Imap_Client_Base: Base  
objects needed to store the correct Utils **and** Ids class name, when  
this can be accomplished with a single classname.  And this just makes  
about a million times more sense from an API perspective (see below).

>> 4584d5e [mms] Move base subject parsing to new  
>> Horde_Imap_Client_Data_BaseSubject class.

This is a refactor, but it was the only method left in Utils class.   
Move to a properly named class and implement using proper  
OO-techniques.  Used in *1* location outside of Horde_Imap_Client.   
Allows us to get rid of the confusing (and unnecessary) reliance on  
getUtils()/utils in the base object.

>> fcb84dc [mms] Move IMAP/POP URL parsing to new Horde_Imap_Client_Url object.

API Bugfix.  Was implemented this way in 1.x for BC reasons.  NO  
reason not to switch to an object, especially since we now use this  
code outside of Horde_Imap_Client.  Used in *2* locations outside of  
Horde_Imap_Client.

> Please revert. This is far from what's acceptable for bug fixing.  
> This is refactoring.

See above.  These fixes were made *precisely* because I am in release  
mode: they occurred because I am currently stress testing with every  
IMAP server I can get a hold of.  It is precisely these kind of fixes  
that would NEVER occur during development since I have little/no  
incentive to do this kind of comprehensive testing.  These are exactly  
the kind of things that need to be fixed when preparing for a release.  
  Example: I have forced myself to try to install Horde from a clean  
install; this is the basis for all the cleanup I have been doing in  
the install Test classes.

Not to mention I wrote a bunch of additional unit tests to test these changes.

Additionally, I was reviewing the API documentation of this class and  
noticed it had a bunch of calls that are not needed.  I would really  
like to push the Imap Client more in release 2 - as such, the API  
needs to be as clean as possible.  That needs to happen NOW, before  
the API is frozen, as opposed to later.

And I'll prepare you now: there are two nasty issues in Imap_Client  
with the way we handle EXPUNGE/VANISHED responses and with a race  
condition in QRESYNC (see  
http://tools.ietf.org/id/draft-kundrat-qresync-arrived-00.html).  This  
is requiring a rewrite of how we handle sequence->UID mappings, and it  
is not an easy or trivial fix.  But it needs to be done or else  
messages CAN be lost in the mailbox when using caching - and will  
NEVER be seen unless the cache is deleted.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]