caching has problems with skip_deleted

[ziba]

Issues

Issue 1: The cache may populate with the wrong message range
Issue 2: After building the cache it may be cleaned from the wrong position up
Issue 3: The cache check (good/bad) may compare the wrong uids
Issue 4: Cache syncing compares the cache against the wrong selection of messages

Causes

Three places assume the return value of _messagecount is the highest message id.

    rcube_imap.php:562 (issues 1,2):  $max = $this->_messagecount($mailbox);
    rcube_imap.php:2211 (issue 3):    $msg_count = $this->_messagecount($mailbox);
    rcube_imap.php:896 (issue 4):     $msg_count = $this->_messagecount($mailbox);

If skip_deleted is enabled _messagecount does not include deleted messages in the count.
This means that the largest message id and the message count for a given mailbox are not necessarily equivalent.

That makes these calls which use $max or $msg_count unpredictable:

    rcube_imap.php:566  (issue 1):  list($begin, $end) = $this->_get_message_range($max, $page);
    rcube_imap.php:608  (issue 2):  $this->clear_message_cache($cache_key, $max + 1);
    rcube_imap.php:2219 (issue 3):  $header = iil_C_FetchHeader($this->conn, $mailbox, "$msg_count");
    rcube_imap.php:899  (issue 4):  $a_message_index = iil_C_FetchHeaderIndex($this->conn, $mailbox, "1:$msg_count", 'UID');

Possible Fixes

For issues 1 and 2, the true highest message id could possibly be obtained this way:

    if ($this->skip_deleted){
        $max = max($this->_search_index($mailbox, 'ALL UNDELETED'));
    }

For issue 3, the highest message uid could be obtained this way:

rcube_imap.php:2220:
    if ($this->skip_deleted){
        $max = max($this->_search_index($mailbox, 'ALL UNDELETED'));
        $a_index = iil_C_FetchHeaderIndex($this->conn, $mailbox, "$max:$max", 'UID');
        $header->uid = array_pop($a_index);
    }

Issue 4 is a little messier. sync_header_index makes two assumptions that aren't true
when skip_deleted is enabled. First, like the other issues it assumes the messagecount
is equivalent to the highest message id. Second, it assumes that the cache is supposed to
contain every message in the mailbox, but the rest of RC doesn't expect deleted headers
to be in the cache when skip_deleted is enabled.

Perhaps sync_header_index should be modified to be deleted message aware or perhaps
the rest of RC should be modified to expect the cache to have deleted messages. Maybe we
could even add a deleted flag to the messages table so they can be easily queried.

Thoughts?

Sorry this ticket is so long. Just trying to be helpful.

[ziba]

I've added a patch which addresses 1,2 and 3 as described and addresses 4 by making syncing iterate only over undeleted messages if skip_deleted is enabled.

[alec]

Fixed in [2dd7ee34].