Looking good! I have a script that diffs my local, packaged tarball
against the tarball generated via horde-components from the git tree.

Besides the missed $condstore on my side, everything is identical to my code.
Which is even more funny since I grepped for $condstore :o)
Thanks for catching this.

Alright, you convinced me. :)

Please give the commit a final look over and be sure it works for you, 
then I'll close the ticket.

Changes have been made in Git (master):

commit 8a55b3d7e36c2615fd6f7d474a8eaae76f77c716
Author: Thomas Jarosch <thomas.jarosch@intra2net.com>
Date:   Thu Aug 22 10:48:19 2013 +0200

     Simplify CONDSTORE/non-CONDSTORE code paths.

     Horde_Imap_Client already does the abstraction for us.

     Also save the total amount of messages in an IMAP folder
     for all server types again, it's not worth the effort / complexity
     to create seperate code paths for this.

     Request: 12597

  .../lib/Horde/ActiveSync/Imap/Adapter.php          |   59 
++++++++------------
  1 files changed, 23 insertions(+), 36 deletions(-)

http://git.horde.org/horde-git/-/commit/8a55b3d7e36c2615fd6f7d474a8eaae76f77c716

Though, reading through the RFC, it seems that behavior is broken, as 
0 denotes it doesn't support per-mailbox modseq. Hmm.

Sorry, I meant the non-CONDSTORE and changes path, not the initial 
sync path. I had reports from users that their server was returning a 
modseq of zero for these folders (though locally I was seeing a value 
of 1).

What kind of modseq value does a CONDSTORE server report
for a new, empty folder?

The current code enters the modseq code path when the stored
$folder->modseq() is higher than zero.
(same problem if we run into the "initial sync" code path)

If it's higher than zero (as your remark above would imply),
then the new, simplified code would behave the same, won't it?

*sigh* These were still in my tree, but missed them when git adding.
Why? There is already a conditional there around line 364.

I've been of the mindset from the beginning with the EAS email stuff 
that every single bit of optimization we can do in the Horde <-> IMAP 
communication should be done due to the high frequency that EAS 
clients can PING the servers. I understand that it might be relatively 
cheap, but the extra logic involved is also relatively minor.
I'm less concerned with the order the checks are done than I am with 
asking the IMAP server for information we don't *really* need.
For new, empty, folders this would cause the initial sync code to hit 
on every sync even on CONDSTORE servers - after we already know we 
don't have changes. I had code similar to this in place (using only 
modseq == 0 as the final check) once apon a time, but needed to change 
it due to this issue (and others, IIRC though I'm drawing a blank on 
what they were).

ehrm, those tweaks contain little bugs :o)
-> Not all setChanges() calls were updated during "cleanup",
it will still access $status['messages'] for CONDSTORE servers.

Also we would need another if() for CONDSTORE/non-CONDSTORE
for the inital sync.

The code is more complex than it needs to be: We can specify 
Horde_Imap_Client::STATUS_HIGHESTMODSEQ
for non-CONDSTORE servers and the Imap_Client will return 0 for the 
modseq in that case.
This would get rid of the duplicated $status_flags and the 
$imap->queryCapability('CONDSTORE') call.

Also querying the amount of messages is very cheap for a CONDSTORE server,
no need to over-optimize this and create more complex code paths.
-> Let's store the number of messages for all cases.

For the ping() code, I moved the modseq code path first
so it will be the default for modern IMAP servers.
Though I doubt this makes any noticeable performance difference ;)

Please consider the attached patch. Works fine on a non-CONDSTORE server.

Committed, with a few tweaks and changes.

Thanks!

commit 4e05bb38ff570c063999f6895128409bc3e02329
Author: Thomas Jarosch <thomas.jarosch at intra2net.com>
Date:   Wed Aug 21 22:30:54 2013 +0200

     Detect deleted messages in ping() for non-CONDSTORE servers

     We store the total number of messages per folder.
     If a messages is deleted, the total number will change.
     If a DELETE+ADD happens at the same time, UIDNEXT will still increase.

     Note: I didn't increase the folder cache version on purpose.
     Otherwise we lose the cached flags for no good reason.

     Signed-off-by: Michael J Rubinsky <mrubinsk at horde.org>

  framework/ActiveSync/lib/Horde/ActiveSync/Folder/Imap.php  |   27 
+++++++++++-
  framework/ActiveSync/lib/Horde/ActiveSync/Imap/Adapter.php |   18 ++++++--
  2 files changed, 39 insertions(+), 6 deletions(-)

http://git.horde.org/horde-git/-/commit/4e05bb38ff570c063999f6895128409bc3e02329

From the patch:

Detect deleted messages in ping() for non-CONDSTORE servers

We store the total number of messages per folder.
If a messages is deleted, the total number will change.
If a DELETE+ADD happens at the same time, UIDNEXT will still increase.

Note: I didn't increase the folder cache version on purpose.
Otherwise we lose the cached flags for no good reason.