From 0ba208189575accd1450bad55732c5ea54a1365e Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Tue, 14 Jul 2009 09:10:43 +0000 Subject: [PATCH] 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 git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4315 57a73879-2fb5-44c3-a270-3262357dd7e2 --- CHANGES.txt | 2 + doc/upgrading.txt | 12 +++ roundup/instance.py | 8 ++ roundup/mailgw.py | 18 ++++- roundup/scripts/roundup_mailgw.py | 120 ++++++++++++++---------------- test/test_mailgw.py | 82 ++++++++++++-------- 6 files changed, 144 insertions(+), 98 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 090aa4f..96fc151 100644 --- a/CHANGES.txt +++ b/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) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index a224872..b599602 100644 --- a/doc/upgrading.txt +++ b/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 --------------------------------------------------------- diff --git a/roundup/instance.py b/roundup/instance.py index 127590b..06cff33 100644 --- a/roundup/instance.py +++ b/roundup/instance.py @@ -46,6 +46,10 @@ class Tracker: """ 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 @@ class Tracker: 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 @@ class Tracker: 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) diff --git a/roundup/mailgw.py b/roundup/mailgw.py index a68d92d..683b172 100644 --- a/roundup/mailgw.py +++ b/roundup/mailgw.py @@ -524,9 +524,8 @@ class Message(mimetools.Message): 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 @@ class MailGW: 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 diff --git a/roundup/scripts/roundup_mailgw.py b/roundup/scripts/roundup_mailgw.py index b2f10b8..3325091 100644 --- a/roundup/scripts/roundup_mailgw.py +++ b/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[^:]+)(:(?P.+))?@)?(?P.+)', + 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[^:]+)(:(?P.+))?@)?(?P.+)', - 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)) diff --git a/test/test_mailgw.py b/test/test_mailgw.py index 7d7fba6..60d1164 100644 --- a/test/test_mailgw.py +++ b/test/test_mailgw.py @@ -116,11 +116,11 @@ class MailgwTestCase(unittest.TestCase, DiffHelper): address='chef@bork.bork.bork', realname='Bork, Chef', roles='User') self.richard_id = self.db.user.create(username='richard', address='richard@test.test', roles='User') - self.mary_id = self.db.user.create(username='mary', address='mary@test.test', - roles='User', realname='Contrary, Mary') - self.john_id = self.db.user.create(username='john', address='john@test.test', - alternate_addresses='jondoe@test.test\njohn.doe@test.test', roles='User', - realname='John Doe') + self.mary_id = self.db.user.create(username='mary', + address='mary@test.test', 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='jondoe@test.test\njohn.doe@test.test') def tearDown(self): if os.path.exists(SENDMAILDEBUG): @@ -132,11 +132,15 @@ class MailgwTestCase(unittest.TestCase, DiffHelper): 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 @@ _______________________________________________________________________ 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 @@ Subject: [issue1] Testing... [nosy=-richard] 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 @@ Subject: [issue] Testing... 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 @@ Unknown address: fubar@bork.bork.bork 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 @@ Unknown address: fubar@bork.bork.bork 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 @@ Subject: [issue] Testing... 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] -- 2.30.2