Process each message through the mail gateway as a separate transaction.

The mail-gateway used to process messages fetched, e.g., via imap in a
single big transaction.  Now we process each message coming in via the
mail-gateway in its own transaction. Regression-tests passed.
See also discussion:
http://thread.gmane.org/gmane.comp.bug-tracking.roundup.user/9500

Author: Ralf Schlatterbeck

CHANGES.txt

@@ -8,6 +8,8 @@ Fixes:
 - fixed action taken in response to invalid GET request
 - fixed classic tracker template to submit POST requests when appropriate
 - fix problems with french and german locale files (issue 2550546)
+- Run each message of the mail-gateway in a separate transaction,
+  see http://thread.gmane.org/gmane.comp.bug-tracking.roundup.user/9500
 
 
 2009-03-18 1.4.8 (r4209)

doc/upgrading.txt

@@ -16,6 +16,18 @@ steps.
 Migrating from 1.4.x to 1.4.9
 =============================
 
+Customized MailGW Class
+-----------------------
+
+If you have customized the MailGW class in your tracker: The new MailGW
+class opens the database for each message in the method handle_message
+(instance.open) instead of passing the opened database as a parameter to
+the MailGW constructor. The old handle_message has been renamed to
+_handle_message. The new method opens the database and wraps the call to
+the old method into a try/finally.
+
+Your customized MailGW class needs to mirror this behavior.
+
 Fix the "remove" button in issue files and messages lists
 ---------------------------------------------------------
 

roundup/instance.py

@@ -46,6 +46,10 @@ def __init__(self, tracker_home, optimize=0):
         """
         self.tracker_home = tracker_home
         self.optimize = optimize
+        # if set, call schema_hook after executing schema.py will get
+        # same variables (in particular db) as schema.py main purpose is
+        # for regression tests
+        self.schema_hook = None
         self.config = configuration.CoreConfig(tracker_home)
         self.actions = {}
         self.cgi_actions = {}
@@ -106,6 +110,8 @@ def open(self, name=None):
         if self.optimize:
             # execute preloaded schema object
             exec(self.schema, vars)
+            if callable (self.schema_hook):
+                self.schema_hook(**vars)
             # use preloaded detectors
             detectors = self.detectors
         else:
@@ -114,6 +120,8 @@ def open(self, name=None):
                 sys.path.insert(1, libdir)
             # execute the schema file
             self._load_python('schema.py', vars)
+            if callable (self.schema_hook):
+                self.schema_hook(**vars)
             # reload extensions and detectors
             for extension in self.get_extensions('extensions'):
                 extension(self)

roundup/mailgw.py

@@ -524,9 +524,8 @@ def verify_signature(self, author):
 
 class MailGW:
 
-    def __init__(self, instance, db, arguments=()):
+    def __init__(self, instance, arguments=()):
         self.instance = instance
-        self.db = db
         self.arguments = arguments
         self.default_class = None
         for option, value in self.arguments:
@@ -802,6 +801,21 @@ def handle_message(self, message):
 
         Parse the message as per the module docstring.
         '''
+        # get database handle for handling one email
+        self.db = self.instance.open ('admin')
+        try:
+            return self._handle_message (message)
+        finally:
+            self.db.close()
+
+    def _handle_message(self, message):
+        ''' message - a Message instance
+
+        Parse the message as per the module docstring.
+
+        The implementation expects an opened database and a try/finally
+        that closes the database.
+        '''
         # detect loops
         if message.getheader('x-roundup-loop', ''):
             raise IgnoreLoop

roundup/scripts/roundup_mailgw.py

@@ -138,73 +138,65 @@ def main(argv):
     import roundup.instance
     instance = roundup.instance.open(instance_home)
 
-    # get a mail handler
-    db = instance.open('admin')
+    if hasattr(instance, 'MailGW'):
+        handler = instance.MailGW(instance, optionsList)
+    else:
+        handler = mailgw.MailGW(instance, optionsList)
+
+    # if there's no more arguments, read a single message from stdin
+    if len(args) == 1:
+        return handler.do_pipe()
+
+    # otherwise, figure what sort of mail source to handle
+    if len(args) < 3:
+        return usage(argv, _('Error: not enough source specification information'))
+    source, specification = args[1:3]
+
+    # time out net connections after a minute if we can
+    if source not in ('mailbox', 'imaps'):
+        if hasattr(socket, 'setdefaulttimeout'):
+            socket.setdefaulttimeout(60)
+
+    if source == 'mailbox':
+        return handler.do_mailbox(specification)
 
-    # now wrap in try/finally so we always close the database
+    # the source will be a network server, so obtain the credentials to
+    # use in connecting to the server
     try:
-        if hasattr(instance, 'MailGW'):
-            handler = instance.MailGW(instance, db, optionsList)
+        # attempt to obtain credentials from a ~/.netrc file
+        authenticator = netrc.netrc().authenticators(specification)
+        username = authenticator[0]
+        password = authenticator[2]
+        server = specification
+        # IOError if no ~/.netrc file, TypeError if the hostname
+        # not found in the ~/.netrc file:
+    except (IOError, TypeError):
+        match = re.match(r'((?P<user>[^:]+)(:(?P<pass>.+))?@)?(?P<server>.+)',
+                         specification)
+        if match:
+            username = match.group('user')
+            password = match.group('pass')
+            server = match.group('server')
         else:
-            handler = mailgw.MailGW(instance, db, optionsList)
-
-        # if there's no more arguments, read a single message from stdin
-        if len(args) == 1:
-            return handler.do_pipe()
-
-        # otherwise, figure what sort of mail source to handle
-        if len(args) < 3:
-            return usage(argv, _('Error: not enough source specification information'))
-        source, specification = args[1:3]
-
-        # time out net connections after a minute if we can
-        if source not in ('mailbox', 'imaps'):
-            if hasattr(socket, 'setdefaulttimeout'):
-                socket.setdefaulttimeout(60)
-
-        if source == 'mailbox':
-            return handler.do_mailbox(specification)
-
-        # the source will be a network server, so obtain the credentials to
-        # use in connecting to the server
-        try:
-            # attempt to obtain credentials from a ~/.netrc file
-            authenticator = netrc.netrc().authenticators(specification)
-            username = authenticator[0]
-            password = authenticator[2]
-            server = specification
-            # IOError if no ~/.netrc file, TypeError if the hostname
-            # not found in the ~/.netrc file:
-        except (IOError, TypeError):
-            match = re.match(r'((?P<user>[^:]+)(:(?P<pass>.+))?@)?(?P<server>.+)',
-                             specification)
-            if match:
-                username = match.group('user')
-                password = match.group('pass')
-                server = match.group('server')
-            else:
-                return usage(argv, _('Error: %s specification not valid') % source)
-
-        # now invoke the mailgw handler depending on the server handler requested
-        if source.startswith('pop'):
-            ssl = source.endswith('s')
-            if ssl and sys.version_info<(2,4):
-                return usage(argv, _('Error: a later version of python is required'))
-            return handler.do_pop(server, username, password, ssl)
-        elif source == 'apop':
-            return handler.do_apop(server, username, password)
-        elif source.startswith('imap'):
-            ssl = source.endswith('s')
-            mailbox = ''
-            if len(args) > 3:
-                mailbox = args[3]
-            return handler.do_imap(server, username, password, mailbox, ssl)
-
-        return usage(argv, _('Error: The source must be either "mailbox",'
-            ' "pop", "pops", "apop", "imap" or "imaps"'))
-    finally:
-        # handler might have closed the initial db and opened a new one
-        handler.db.close()
+            return usage(argv, _('Error: %s specification not valid') % source)
+
+    # now invoke the mailgw handler depending on the server handler requested
+    if source.startswith('pop'):
+        ssl = source.endswith('s')
+        if ssl and sys.version_info<(2,4):
+            return usage(argv, _('Error: a later version of python is required'))
+        return handler.do_pop(server, username, password, ssl)
+    elif source == 'apop':
+        return handler.do_apop(server, username, password)
+    elif source.startswith('imap'):
+        ssl = source.endswith('s')
+        mailbox = ''
+        if len(args) > 3:
+            mailbox = args[3]
+        return handler.do_imap(server, username, password, mailbox, ssl)
+
+    return usage(argv, _('Error: The source must be either "mailbox",'
+        ' "pop", "pops", "apop", "imap" or "imaps"'))
 
 def run():
     sys.exit(main(sys.argv))

test/test_mailgw.py

@@ -116,11 +116,11 @@ def setUp(self):
             address='[email protected]', realname='Bork, Chef', roles='User')
         self.richard_id = self.db.user.create(username='richard',
             address='[email protected]', roles='User')
-        self.mary_id = self.db.user.create(username='mary', address='[email protected]',
-            roles='User', realname='Contrary, Mary')
-        self.john_id = self.db.user.create(username='john', address='[email protected]',
-            alternate_addresses='jondoe@test.test\n[email protected]', roles='User',
-            realname='John Doe')
+        self.mary_id = self.db.user.create(username='mary',
+            address='[email protected]', roles='User', realname='Contrary, Mary')
+        self.john_id = self.db.user.create(username='john',
+            address='john@test.test', roles='User', realname='John Doe',
+            alternate_addresses='[email protected]\n[email protected]')
 
     def tearDown(self):
         if os.path.exists(SENDMAILDEBUG):
@@ -132,11 +132,15 @@ def tearDown(self):
             if error.errno not in (errno.ENOENT, errno.ESRCH): raise
 
     def _handle_mail(self, message):
-        handler = self.instance.MailGW(self.instance, self.db)
+        # handler will open a new db handle. On single-threaded
+        # databases we'll have to close our current connection
+        self.db.commit()
+        self.db.close()
+        handler = self.instance.MailGW(self.instance)
         handler.trapExceptions = 0
         ret = handler.main(StringIO(message))
-        # handler can close the db on us and open a new one
-        self.db = handler.db
+        # handler had its own database, open new connection
+        self.db = self.instance.open('admin')
         return ret
 
     def _get_mail(self):
@@ -549,6 +553,11 @@ def testPropertyChangeOnly(self):
         self.doNewIssue()
         oldvalues = self.db.getnode('issue', '1').copy()
         oldvalues['assignedto'] = None
+        # reconstruct old behaviour: This would reuse the
+        # database-handle from the doNewIssue above which has committed
+        # as user "Chef". So we close and reopen the db as that user.
+        self.db.close()
+        self.db = self.instance.open('Chef')
         self.db.issue.set('1', assignedto=self.chef_id)
         self.db.commit()
         self.db.issue.nosymessage('1', None, oldvalues)
@@ -990,11 +999,6 @@ def testNosyRemove(self):
         assert not os.path.exists(SENDMAILDEBUG)
 
     def testNewUserAuthor(self):
-        # first without the permission
-        # heh... just ignore the API for a second ;)
-        self.db.security.role['anonymous'].permissions=[]
-        anonid = self.db.user.lookup('anonymous')
-        self.db.user.set(anonid, roles='Anonymous')
 
         l = self.db.user.list()
         l.sort()
@@ -1007,6 +1011,12 @@ def testNewUserAuthor(self):
 
 This is a test submission of a new issue.
 '''
+        def hook (db, **kw):
+            ''' set up callback for db open '''
+            db.security.role['anonymous'].permissions=[]
+            anonid = db.user.lookup('anonymous')
+            db.user.set(anonid, roles='Anonymous')
+        self.instance.schema_hook = hook
         try:
             self._handle_mail(message)
         except Unauthorized, value:
@@ -1021,15 +1031,17 @@ def testNewUserAuthor(self):
         else:
             raise AssertionError, "Unathorized not raised when handling mail"
 
-        # Add Web Access role to anonymous, and try again to make sure
-        # we get a "please register at:" message this time.
-        p = [
-            self.db.security.getPermission('Create', 'user'),
-            self.db.security.getPermission('Web Access', None),
-        ]
-
-        self.db.security.role['anonymous'].permissions=p
 
+        def hook (db, **kw):
+            ''' set up callback for db open '''
+            # Add Web Access role to anonymous, and try again to make sure
+            # we get a "please register at:" message this time.
+            p = [
+                db.security.getPermission('Create', 'user'),
+                db.security.getPermission('Web Access', None),
+            ]
+            db.security.role['anonymous'].permissions=p
+        self.instance.schema_hook = hook
         try:
             self._handle_mail(message)
         except Unauthorized, value:
@@ -1053,12 +1065,15 @@ def testNewUserAuthor(self):
         m.sort()
         self.assertEqual(l, m)
 
-        # now with the permission
-        p = [
-            self.db.security.getPermission('Create', 'user'),
-            self.db.security.getPermission('Email Access', None),
-        ]
-        self.db.security.role['anonymous'].permissions=p
+        def hook (db, **kw):
+            ''' set up callback for db open '''
+            # now with the permission
+            p = [
+                db.security.getPermission('Create', 'user'),
+                db.security.getPermission('Email Access', None),
+            ]
+            db.security.role['anonymous'].permissions=p
+        self.instance.schema_hook = hook
         self._handle_mail(message)
         m = self.db.user.list()
         m.sort()
@@ -1076,11 +1091,14 @@ def testNewUserAuthorHighBit(self):
 
 This is a test submission of a new issue.
 '''
-        p = [
-            self.db.security.getPermission('Create', 'user'),
-            self.db.security.getPermission('Email Access', None),
-        ]
-        self.db.security.role['anonymous'].permissions=p
+        def hook (db, **kw):
+            ''' set up callback for db open '''
+            p = [
+                db.security.getPermission('Create', 'user'),
+                db.security.getPermission('Email Access', None),
+            ]
+            db.security.role['anonymous'].permissions=p
+        self.instance.schema_hook = hook
         self._handle_mail(message)
         m = set(self.db.user.list())
         new = list(m - l)[0]