[PATCH 1/2] Re: Another way of locating UID of just saved message

[Nicolas Sebrecht]

On Wed, Aug 03, 2011 at 04:59:21PM +0200, Vladimir.Marek at oracle.com wrote:
> From: Vladimir Marek <vlmarek at volny.cz>
> 
> It works by fetching all headers from new messages from IMAP server an searching
> for our X-OfflineIMAP marker by using reular expression.
                                        ^^^^^^
                                        regular

> Signed-off-by: Vladimir Marek <vlmarek at volny.cz>
> ---
>  offlineimap/folder/IMAP.py |   67 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 67 insertions(+), 0 deletions(-)
> 
> diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py
> index 3c702f4..9104faa 100644
> --- a/offlineimap/folder/IMAP.py
> +++ b/offlineimap/folder/IMAP.py
> @@ -317,6 +317,70 @@ class IMAPFolder(BaseFolder):
>          matchinguids.sort()
>          return long(matchinguids[0])
>  
> +    def savemessage_fetchheaders(self, imapobj, headername, headervalue):
> +        """ We fetch all new mail headers and search for the right
> +        X-OfflineImap line by hand. The response from the server has form:
> +        (
> +          'OK',
> +          [
> +            (
> +              '185 (RFC822.HEADER {1789}',
> +              '... mail headers ...'
> +            ),
> +            ' UID 2444)',
> +            (
> +              '186 (RFC822.HEADER {1789}',
> +              '... 2nd mail headers ...'
> +            ),
> +            ' UID 2445)'
> +          ]
> +        )
> +        We need to locate the UID just after mail headers containing our
> +        X-OfflineIMAP line.
> +
> +        Returns UID when found, 0 when not found.
> +        """
> +        self.ui.debug('imap', 'savemessage_fetchheaders called for %s: %s' % \
> +                 (headername, headervalue))
> +
> +        # run "fetch X:* rfc822.header"
> +        # since we stored the mail we are looking for just recently, it would
> +        # not be optimal to fetch all messages. So we'll find highest message
> +        # UID in our local messagelist and search from there (exactly from
> +        # UID+1). That works because UIDs are guaranteed to be unique and
> +        # ascending.

The logic looks quiet good to me but I'm still missing why we would need
to fetch all headers of new mails. Is there any way to get the first or
two messages headers of new mails?

> +
> +        if self.getmessagelist():
> +            start = 1+max(self.getmessagelist().keys())
> +        else:
> +            # Folder was empty - start from 1
> +            start = 1
> +
> +        # The problem is that imapobj quotes all parameters. That must not happen
> +        # with range X:*. So we use bytearray for that
> +
> +        result = imapobj.uid('FETCH', bytearray('%d:*' % start), 'rfc822.header')
> +        assert result[0] == 'OK', 'Error with fetching mail headers: ' + '. '.join(result[1])

assert sucks. I'd rather much raise an OfflineImapError.

> +        result = result[1]
> +
> +        found = 0
> +        for item in result:
> +            if found == 0 and type(item) == type( () ):
> +                # Walk just tuples
> +                if re.search("(?:^|\\r|\\n)%s:\s*%s(?:\\r|\\n)" % (headername, headervalue), item[1], flags=re.IGNORECASE):
> +                    found = 1
> +            elif found == 1:
> +                if type(item) == type (""):

                   if type(item) == str:

> +                    uid = re.search("UID\s+(\d+)", item, flags=re.IGNORECASE)
> +                    if uid:
> +                        return int(uid.group(1))
> +                    else:
> +                        self.ui.warn("Can't parse FETCH response, can't find UID: %s", result.__repr__())
> +                else:
> +                    self.ui.warn("Can't parse FETCH response, we awaited string: %s", result.__repr__())
> +
> +        return 0
>  
>      def getmessageinternaldate(self, content, rtime=None):
>          """Parses mail and returns an INTERNALDATE string
> @@ -485,6 +549,9 @@ class IMAPFolder(BaseFolder):
>                      assert(imapobj.noop()[0] == 'OK')
>                      uid = self.savemessage_searchforheader(imapobj, headername,
>                                                         headervalue)
> +                    if uid == 0:
> +                        self.ui.debug('imap', 'savemessage: second attempt to get new UID failed. Going to try search headers manually')

Not that I'm against long lines in code (the 80 chars is obsolete IMHO),
I think you should limit to 120 characters, at least.

> +                        uid = self.savemessage_fetchheaders(imapobj, headername, headervalue)
>  
>          finally:
>              self.imapserver.releaseconnection(imapobj)
-- 
Nicolas Sebrecht