diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index f42c969722f..fb13ca2034d 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -468,6 +468,8 @@ class mail_thread(osv.AbstractModel): """ assert isinstance(message, Message), 'message must be an email.message.Message at this point' message_id = message.get('Message-Id') + email_from = decode_header(message, 'From') + email_to = decode_header(message, 'To') references = decode_header(message, 'References') in_reply_to = decode_header(message, 'In-Reply-To') @@ -480,8 +482,8 @@ class mail_thread(osv.AbstractModel): model_pool = self.pool.get(model) if thread_id and model and model_pool and model_pool.exists(cr, uid, thread_id) \ and hasattr(model_pool, 'message_update'): - _logger.debug('Routing mail with Message-Id %s: direct reply to model: %s, thread_id: %s, custom_values: %s, uid: %s', - message_id, model, thread_id, custom_values, uid) + _logger.info('Routing mail from %s to %s with Message-Id %s: direct reply to model: %s, thread_id: %s, custom_values: %s, uid: %s', + email_from, email_to, message_id, model, thread_id, custom_values, uid) return [(model, thread_id, custom_values, uid)] # Verify whether this is a reply to a private message @@ -489,8 +491,8 @@ class mail_thread(osv.AbstractModel): message_ids = self.pool.get('mail.message').search(cr, uid, [('message_id', '=', in_reply_to)], limit=1, context=context) if message_ids: message = self.pool.get('mail.message').browse(cr, uid, message_ids[0], context=context) - _logger.debug('Routing mail with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s', - message_id, message.id, custom_values, uid) + _logger.info('Routing mail from %s to %s with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s', + email_from, email_to, message_id, message.id, custom_values, uid) return [(message.model, message.res_id, custom_values, uid)] # 2. Look for a matching mail.alias entry @@ -517,10 +519,11 @@ class mail_thread(osv.AbstractModel): # Note: recognized partners will be added as followers anyway # user_id = self._message_find_user_id(cr, uid, message, context=context) user_id = uid - _logger.debug('No matching user_id for the alias %s', alias.alias_name) + _logger.info('No matching user_id for the alias %s', alias.alias_name) routes.append((alias.alias_model_id.model, alias.alias_force_thread_id, \ eval(alias.alias_defaults), user_id)) - _logger.debug('Routing mail with Message-Id %s: direct alias match: %r', message_id, routes) + _logger.info('Routing mail from %s to %s with Message-Id %s: direct alias match: %r', + email_from, email_to, message_id, routes) return routes # 3. Fallback to the provided parameters, if they work @@ -535,14 +538,14 @@ class mail_thread(osv.AbstractModel): except: thread_id = False assert thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new'), \ - "No possible route found for incoming message with Message-Id %s. " \ - "Create an appropriate mail.alias or force the destination model." % message_id + "No possible route found for incoming message from %s to %s (Message-Id %s:)." \ + "Create an appropriate mail.alias or force the destination model." % (email_from, email_to, message_id) if thread_id and not model_pool.exists(cr, uid, thread_id): _logger.warning('Received mail reply to missing document %s! Ignoring and creating new document instead for Message-Id %s', - thread_id, message_id) + thread_id, message_id) thread_id = None - _logger.debug('Routing mail with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s', - message_id, model, thread_id, custom_values, uid) + _logger.info('Routing mail from %s to %s with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s', + email_from, email_to, message_id, model, thread_id, custom_values, uid) return [(model, thread_id, custom_values, uid)] def message_process(self, cr, uid, model, message, custom_values=None, @@ -592,12 +595,24 @@ class mail_thread(osv.AbstractModel): if isinstance(message, unicode): message = message.encode('utf-8') msg_txt = email.message_from_string(message) - routes = self.message_route(cr, uid, msg_txt, model, - thread_id, custom_values, - context=context) + + # parse the message, verify we are not in a loop by checking message_id is not duplicated msg = self.message_parse(cr, uid, msg_txt, save_original=save_original, context=context) if strip_attachments: msg.pop('attachments', None) + if msg.get('message_id'): # should always be True as message_parse generate one if missing + existing_msg_ids = self.pool.get('mail.message').search(cr, SUPERUSER_ID, [ + ('message_id', '=', msg.get('message_id')), + ], context=context) + if existing_msg_ids: + _logger.info('Ignored mail from %s to %s with Message-Id %s:: found duplicated Message-Id during processing', + msg.get('from'), msg.get('to'), msg.get('message_id')) + return False + + # find possible routes for the message + routes = self.message_route(cr, uid, msg_txt, model, + thread_id, custom_values, + context=context) # postpone setting msg.partner_ids after message_post, to avoid double notifications partner_ids = msg.pop('partner_ids', []) diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index eb6ba22067b..60a08f0c62f 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -116,8 +116,11 @@ class TestMailgateway(TestMailBase): frog_groups = format_and_process(MAIL_TEMPLATE, to='groups@example.com, other@gmail.com') sent_emails = self._build_email_kwargs_list # Test: one group created by mailgateway administrator - self.assertTrue(len(frog_groups) == 1) + self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + res = self.mail_group.perm_read(cr, uid, [frog_group.id], details=False) + self.assertEqual(res[0].get('create_uid'), uid, + 'message_process: group should have been created by uid as alias_user__id is False on the alias') # Test: one message that is the incoming email self.assertEqual(len(frog_group.message_ids), 1, 'message_process: newly created group should have the incoming email in message_ids') @@ -150,9 +153,12 @@ class TestMailgateway(TestMailBase): self._init_mock_build_email() frog_groups = format_and_process(MAIL_TEMPLATE, to='groups@example.com, other@gmail.com') sent_emails = self._build_email_kwargs_list - # Test: one group created by raoul - self.assertTrue(len(frog_groups) == 1) + # Test: one group created by Raoul + self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + res = self.mail_group.perm_read(cr, uid, [frog_group.id], details=False) + self.assertEqual(res[0].get('create_uid'), self.user_raoul_id, + 'message_process: group should have been created by alias_user_id') # Test: one message that is the incoming email self.assertEqual(len(frog_group.message_ids), 1, 'message_process: newly created group should have the incoming email in message_ids') @@ -175,8 +181,8 @@ class TestMailgateway(TestMailBase): # Do: incoming email from a known partner that is also an user that can create a mail.group self.res_users.create(cr, uid, {'partner_id': p1id, 'login': 'sylvie', 'groups_id': [(6, 0, [self.group_employee_id])]}) frog_groups = format_and_process(MAIL_TEMPLATE, to='groups@example.com, other@gmail.com') - # Test: one group created by Sylvie - self.assertTrue(len(frog_groups) == 1) + # Test: one group created by Raoul (or Sylvie maybe, if we implement it) + self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) # Test: one message that is the incoming email self.assertEqual(len(frog_group.message_ids), 1, @@ -195,6 +201,7 @@ class TestMailgateway(TestMailBase): # Do: even with a wrong destination, a reply should end up in the correct thread frog_groups = format_and_process(MAIL_TEMPLATE, email_from='other@gmail.com', + msg_id='<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>', to='erroneous@example.com>', subject='Re: news', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) # Test: no group 'Re: news' created, still only 1 Frogs group @@ -205,12 +212,30 @@ class TestMailgateway(TestMailBase): 'message_process: reply on Frogs should not have created a duplicate group with old subject') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) # Test: one new message - self.assertTrue(len(frog_group.message_ids) == 2, 'message_process: group should contain 2 messages after reply') + self.assertEqual(len(frog_group.message_ids), 2, 'message_process: group should contain 2 messages after reply') # Test: author (and not recipient) added as follower frog_follower_ids = set([p.id for p in frog_group.message_follower_ids]) self.assertEqual(frog_follower_ids, set([p1id, p2id]), 'message_process: after reply, group should have 2 followers') + # Do: due to some issue, same email goes back into the mailgateway + frog_groups = format_and_process(MAIL_TEMPLATE, email_from='other@gmail.com', + msg_id='<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>', + subject='Re: news', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) + # Test: no group 'Re: news' created, still only 1 Frogs group + self.assertEqual(len(frog_groups), 0, + 'message_process: reply on Frogs should not have created a new group with new subject') + frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) + self.assertEqual(len(frog_groups), 1, + 'message_process: reply on Frogs should not have created a duplicate group with old subject') + frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + # Test: no new message + self.assertEqual(len(frog_group.message_ids), 2, 'message_process: message with already existing message_id should not have been duplicated') + # Test: message_id is still unique + msg_ids = self.mail_message.search(cr, uid, [('message_id', 'ilike', '<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>')]) + self.assertEqual(len(msg_ids), 1, + 'message_process: message with already existing message_id should not have been duplicated') + # -------------------------------------------------- # Test3: email_from and partner finding # -------------------------------------------------- @@ -223,6 +248,7 @@ class TestMailgateway(TestMailBase): # Do: post a new message, with a known partner -> duplicate emails -> partner format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (2)', + msg_id='<1198923581.41972151344608186760.JavaMail.new1@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -236,6 +262,7 @@ class TestMailgateway(TestMailBase): self.res_users.write(cr, uid, self.user_raoul_id, {'email': 'test_raoul@email.com'}) format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (3)', + msg_id='<1198923581.41972151344608186760.JavaMail.new2@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -250,6 +277,7 @@ class TestMailgateway(TestMailBase): self.res_users.write(cr, uid, self.user_raoul_id, {'email': 'test_raoul@email.com'}) format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (3)', + msg_id='<1198923581.41972151344608186760.JavaMail.new3@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -265,23 +293,32 @@ class TestMailgateway(TestMailBase): # Do: incoming email with model that does not accepts incoming emails must raise self.assertRaises(AssertionError, - format_and_process, - MAIL_TEMPLATE, to='noone@example.com', subject='spam', extra='', model='res.country') + format_and_process, + MAIL_TEMPLATE, + to='noone@example.com', subject='spam', extra='', model='res.country', + msg_id='<1198923581.41972151344608186760.JavaMail.new4@agrolait.com>') # Do: incoming email without model and without alias must raise self.assertRaises(AssertionError, - format_and_process, - MAIL_TEMPLATE, to='noone@example.com', subject='spam', extra='') + format_and_process, + MAIL_TEMPLATE, + to='noone@example.com', subject='spam', extra='', + msg_id='<1198923581.41972151344608186760.JavaMail.new5@agrolait.com>') # Do: incoming email with model that accepting incoming emails as fallback - frog_groups = format_and_process(MAIL_TEMPLATE, to='noone@example.com', subject='Spammy', extra='', model='mail.group') + frog_groups = format_and_process(MAIL_TEMPLATE, + to='noone@example.com', + subject='Spammy', extra='', model='mail.group', + msg_id='<1198923581.41972151344608186760.JavaMail.new6@agrolait.com>') self.assertEqual(len(frog_groups), 1, 'message_process: erroneous email but with a fallback model should have created a new mail.group') # Do: incoming email in plaintext should be stored as html - frog_groups = format_and_process(MAIL_TEMPLATE_PLAINTEXT, to='groups@example.com', subject='Frogs Return', extra='', msg_id='') + frog_groups = format_and_process(MAIL_TEMPLATE_PLAINTEXT, + to='groups@example.com', subject='Frogs Return', extra='', + msg_id='') # Test: one group created with one message - self.assertTrue(len(frog_groups) == 1) + self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) msg = frog_group.message_ids[0] # Test: plain text content should be wrapped and stored as html @@ -305,19 +342,27 @@ class TestMailgateway(TestMailBase): # Reply to msg1, make sure the reply is properly attached using the various reply identification mechanisms # 0. Direct alias match - reply_msg1 = format(MAIL_TEMPLATE, to='Pretty Pigs ', extra='In-Reply-To: %s' % msg1.message_id) + reply_msg1 = format(MAIL_TEMPLATE, to='Pretty Pigs ', + extra='In-Reply-To: %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.2@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg1) # 1. In-Reply-To header - reply_msg2 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='In-Reply-To: %s' % msg1.message_id) + reply_msg2 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='In-Reply-To: %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.3@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg2) # 2. References header - reply_msg3 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % msg1.message_id) + reply_msg3 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.4@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg3) # 3. Subject contains [] + model passed to message+process -> only attached to group, but not to mail (not in msg1.child_ids) - reply_msg4 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='', subject='Re: [%s] 1' % self.group_pigs_id) + reply_msg4 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='', subject='Re: [%s] 1' % self.group_pigs_id, + msg_id='<1198923581.41972151344608186760.JavaMail.5@agrolait.com>') self.mail_group.message_process(cr, uid, 'mail.group', reply_msg4) group_pigs.refresh()