Modified subject line parser in mail gateway.

Tries to be more forgiving and allows both multiple Re/Ang/Sv and
[mailing-list-id].

Author: Erik Forsberg

roundup/mailgw.py

@@ -72,7 +72,7 @@ class node. Any parts of other types are each stored in separate files
 an exception, the original message is bounced back to the sender with the
 explanatory message given in the exception.
 
-$Id: mailgw.py,v 1.182 2007-01-21 18:08:31 forsberg Exp $
+$Id: mailgw.py,v 1.183 2007-01-28 13:49:13 forsberg Exp $
 """
 __docformat__ = 'restructuredtext'
 
@@ -619,57 +619,89 @@ def handle_message(self, message):
 
         # Matches subjects like:
         # Re: "[issue1234] title of issue [status=resolved]"
-        open, close = config['MAILGW_SUBJECT_SUFFIX_DELIMITERS']
-        delim_open = re.escape(open)
+
+        tmpsubject = subject # We need subject untouched for later use
+                             # in error messages
+
+        sd_open, sd_close = config['MAILGW_SUBJECT_SUFFIX_DELIMITERS']
+        delim_open = re.escape(sd_open)
         if delim_open in '[(': delim_open = '\\' + delim_open
-        delim_close = re.escape(close)
+        delim_close = re.escape(sd_close)
         if delim_close in '[(': delim_close = '\\' + delim_close
-        subject_re = r'''
-        (?P<refwd>\s*\W?\s*(fw|fwd|re|aw|sv|ang)\W\s*)*\s*   # Re:
-        (?P<quote>")?                                 # Leading "
-        (%s(?P<classname>[^\d\s]+)                    # [issue..
-           (?P<nodeid>\d+)?                           # ..1234]
-         %s)?\s*
-        (?P<title>[^%s]+)?                             # issue title
-        "?                                            # Trailing "
-        (?P<argswhole>%s(?P<args>.+?)%s)?             # [prop=value]
-        '''%(delim_open, delim_close, delim_open, delim_open, delim_close)
-        subject_re = re.compile(subject_re, re.IGNORECASE|re.VERBOSE)
+
+        matches = dict.fromkeys(['refwd', 'quote', 'classname',
+                                 'nodeid', 'title', 'args',
+                                 'argswhole'])
+
+
+        # Look for Re: et. al. Used later on for MAILGW_SUBJECT_CONTENT_MATCH
+        re_re = r'''(?P<refwd>(\s*\W?\s*(fw|fwd|re|aw|sv|ang)\W)+)\s*'''
+        m = re.match(re_re, tmpsubject, re.IGNORECASE|re.VERBOSE)
+        if m:
+            matches.update(m.groupdict())
+            tmpsubject = tmpsubject[len(matches['refwd']):] # Consume Re:
+
+        # Look for Leading "
+        m = re.match(r'''(?P<quote>\s*")''', tmpsubject,
+                     re.IGNORECASE|re.VERBOSE)
+        if m:
+            matches.update(m.groupdict())
+            tmpsubject = tmpsubject[len(matches['quote']):] # Consume quote
+
+        class_re = r'''%s(?P<classname>(%s))+(?P<nodeid>\d+)?%s''' % \
+                   (delim_open, "|".join(self.db.getclasses()), delim_close)
+        # Note: re.search, not re.match as there might be garbage
+        # (mailing list prefix, etc.) before the class identifier
+        m = re.search(class_re, tmpsubject, re.IGNORECASE|re.VERBOSE)
+        if m:
+            matches.update(m.groupdict())
+            # Skip to the end of the class identifier, including any
+            # garbage before it.
+            
+            tmpsubject = tmpsubject[m.end():]
+
+        m = re.match(r'''(?P<title>[^%s]+)''' % delim_open, tmpsubject,
+                     re.IGNORECASE|re.VERBOSE)
+        if m:
+            matches.update(m.groupdict())
+            tmpsubject = tmpsubject[len(matches['title']):] # Consume title
+
+        args_re = r'''(?P<argswhole>%s(?P<args>.+?)%s)?''' % (delim_open, delim_close)
+        m = re.search(args_re, tmpsubject, re.IGNORECASE|re.VERBOSE)
+        if m:
+            matches.update(m.groupdict())
 
         # figure subject line parsing modes
         pfxmode = config['MAILGW_SUBJECT_PREFIX_PARSING']
         sfxmode = config['MAILGW_SUBJECT_SUFFIX_PARSING']
 
-        # check for well-formed subject line
-        m = subject_re.match(subject)
-        if m:
-            # check for registration OTK
-            # or fallback on the default class
-            if self.db.config['EMAIL_REGISTRATION_CONFIRMATION']:
-                otk_re = re.compile('-- key (?P<otk>[a-zA-Z0-9]{32})')
-                otk = otk_re.search(m.group('title') or '')
-                if otk:
-                    self.db.confirm_registration(otk.group('otk'))
-                    subject = 'Your registration to %s is complete' % \
-                              config['TRACKER_NAME']
-                    sendto = [from_list[0][1]]
-                    self.mailer.standard_message(sendto, subject, '')
-                    return
-            # get the classname
-            if pfxmode == 'none':
-                classname = None
+        # check for registration OTK
+        # or fallback on the default class
+        if self.db.config['EMAIL_REGISTRATION_CONFIRMATION']:
+            otk_re = re.compile('-- key (?P<otk>[a-zA-Z0-9]{32})')
+            otk = otk_re.search(matches['title'] or '')
+            if otk:
+                self.db.confirm_registration(otk.group('otk'))
+                subject = 'Your registration to %s is complete' % \
+                          config['TRACKER_NAME']
+                sendto = [from_list[0][1]]
+                self.mailer.standard_message(sendto, subject, '')
+                return
+        # get the classname
+        if pfxmode == 'none':
+            classname = None
+        else:
+            classname = matches['classname']
+        if classname is None:
+            if self.default_class:
+                classname = self.default_class
             else:
-                classname = m.group('classname')
-            if classname is None:
-                if self.default_class:
-                    classname = self.default_class
-                else:
-                    classname = config['MAILGW_DEFAULT_CLASS']
-                    if not classname:
-                        # fail
-                        m = None
+                classname = config['MAILGW_DEFAULT_CLASS']
+                if not classname:
+                    # fail
+                    m = None
 
-        if not m and pfxmode == 'strict':
+        if not classname and pfxmode == 'strict':
             raise MailUsageError, _("""
 The message you sent to roundup did not contain a properly formed subject
 line. The subject must contain a class name or designator to indicate the
@@ -713,7 +745,7 @@ def handle_message(self, message):
         if pfxmode == 'none':
             nodeid = None
         else:
-            nodeid = m.group('nodeid')
+            nodeid = matches['nodeid']
 
         # try in-reply-to to match the message if there's no nodeid
         inreplyto = message.getheader('in-reply-to') or ''
@@ -723,15 +755,15 @@ def handle_message(self, message):
                 nodeid = cl.filter(None, {'messages':l})[0]
 
         # title is optional too
-        title = m.group('title')
+        title = matches['title']
         if title:
             title = title.strip()
         else:
             title = ''
 
         # strip off the quotes that dumb emailers put around the subject, like
         #      Re: "[issue1] bla blah"
-        if m.group('quote') and title.endswith('"'):
+        if matches['quote'] and title.endswith('"'):
             title = title[:-1]
 
         # but we do need either a title or a nodeid...
@@ -752,7 +784,7 @@ def handle_message(self, message):
         # additional restriction based on the matched node's creation or
         # activity.
         tmatch_mode = config['MAILGW_SUBJECT_CONTENT_MATCH']
-        if tmatch_mode != 'never' and nodeid is None and m.group('refwd'):
+        if tmatch_mode != 'never' and nodeid is None and matches['refwd']:
             l = cl.stringFind(title=title)
             limit = None
             if (tmatch_mode.startswith('creation') or
@@ -905,8 +937,8 @@ def handle_message(self, message):
         # figure what the properties of this Class are
         properties = cl.getprops()
         props = {}
-        args = m.group('args')
-        argswhole = m.group('argswhole')
+        args = matches['args']
+        argswhole = matches['argswhole']
         if args:
             if sfxmode == 'none':
                 title += ' ' + argswhole

test/test_mailgw.py

@@ -8,7 +8,7 @@
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #
-# $Id: test_mailgw.py,v 1.83 2007-01-21 18:14:35 forsberg Exp $
+# $Id: test_mailgw.py,v 1.84 2007-01-28 13:49:21 forsberg Exp $
 
 # TODO: test bcc
 
@@ -403,6 +403,30 @@ def testFollowupTitleMatch(self):
 _______________________________________________________________________
 ''')
 
+    def testFollowupTitleMatchMultiRe(self):
+        nodeid1 = self.doNewIssue()
+        nodeid2 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: richard <richard@test>
+To: [email protected]
+Message-Id: <followup_dummy_id>
+Subject: Re: Testing... [assignedto=mary; nosy=+john]
+
+This is a followup
+''')
+
+        nodeid3 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: richard <richard@test>
+To: [email protected]
+Message-Id: <followup2_dummy_id>
+Subject: Ang: Re: Testing... 
+
+This is a followup
+''')        
+        self.assertEqual(nodeid1, nodeid2)
+        self.assertEqual(nodeid1, nodeid3)
+
     def testFollowupTitleMatchNever(self):
         nodeid = self.doNewIssue()
         self.db.config.MAILGW_SUBJECT_CONTENT_MATCH = 'never'
@@ -1173,6 +1197,40 @@ def testClassLooseOK(self):
         assert not os.path.exists(SENDMAILDEBUG)
         self.assertEqual(self.db.keyword.get('1', 'name'), 'Bar')
 
+    def testClassStrictInvalid(self):
+        self.instance.config.MAILGW_SUBJECT_PREFIX_PARSING = 'strict'
+        self.instance.config.MAILGW_DEFAULT_CLASS = ''
+
+        message = '''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Subject: Testing... 
+Cc: richard@test
+Reply-To: [email protected]
+Message-Id: <dummy_test_message_id>
+
+'''
+        self.assertRaises(MailUsageError, self._handle_mail, message)
+        
+    def testClassStrictValid(self):
+        self.instance.config.MAILGW_SUBJECT_PREFIX_PARSING = 'strict'
+        self.instance.config.MAILGW_DEFAULT_CLASS = ''
+
+        nodeid = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Subject: [issue] Testing... 
+Cc: richard@test
+Reply-To: [email protected]
+Message-Id: <dummy_test_message_id>
+
+''')
+
+        assert not os.path.exists(SENDMAILDEBUG)
+        self.assertEqual(self.db.issue.get(nodeid, 'title'), 'Testing...')
+        
     #
     # TEST FOR INVALID COMMANDS HANDLING
     #
@@ -1325,6 +1383,55 @@ def testHelpSubject(self):
 
 '''
         self.assertRaises(MailUsageHelp, self._handle_mail, message)
+
+    def testMaillistSubject(self):
+        self.instance.config.MAILGW_SUBJECT_SUFFIX_DELIMITERS = '[]'
+        self.db.keyword.create(name='Foo')        
+        self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Subject: [mailinglist-name] [keyword1] Testing.. [name=Bar]
+Cc: richard@test
+Reply-To: [email protected]
+Message-Id: <dummy_test_message_id>
+
+''')
+
+        assert not os.path.exists(SENDMAILDEBUG)
+        self.assertEqual(self.db.keyword.get('1', 'name'), 'Bar')
+
+    def testUnknownPrefixSubject(self):
+        self.db.keyword.create(name='Foo')
+        self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: Chef <[email protected]>
+To: [email protected]
+Subject: VeryStrangeRe: [keyword1] Testing.. [name=Bar]
+Cc: richard@test
+Reply-To: [email protected]
+Message-Id: <dummy_test_message_id>
+
+''')
+
+        assert not os.path.exists(SENDMAILDEBUG)
+        self.assertEqual(self.db.keyword.get('1', 'name'), 'Bar')
+
+    def testIssueidLast(self):
+        nodeid1 = self.doNewIssue()
+        nodeid2 = self._handle_mail('''Content-Type: text/plain;
+  charset="iso-8859-1"
+From: mary <mary@test>
+To: [email protected]
+Message-Id: <followup_dummy_id>
+In-Reply-To: <dummy_test_message_id>
+Subject: New title [issue1]
+
+This is a second followup
+''')
+
+        assert nodeid1 == nodeid2
+        self.assertEqual(self.db.issue.get(nodeid2, 'title'), "Testing...")
 
 
 def test_suite():