pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mail: revert the unlink in batch of the <mail.mail>

Browse Source 
Bug
===
The unlink of the <mail.mail> in the CRON is problematic because we
accumulate a lot of records, and the CRON timeout.

In particular, when we sent a mailing, we receive the "opened" event
(blank image in the email), and so we need to update the mailing trace.
But, if we unlink the mail at the same time, it locked the mailing trace
table and we couldn't write the new value.

The reason for that is that before, the unlink took more queries, but
it was done one record at a time, so we could commit the change and
release the lock between each unlink.

Task-3179157
See odoo/odoo/pull/73271

closes odoo/odoo#112703

X-original-commit: 57ae1b9b8b61f5f4719a8a81e9d0d21fab58cfda
Related: odoo/enterprise#37069
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Signed-off-by: Stéphane Debauche (std) <std@odoo.com>
This commit is contained in:
std-odoo
2023-02-15 10:14:53 +01:00
parent e5aee1fcbf
commit dab3d0a282
11 changed files with 43 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/auth_signup/tests/test_reset_password.py
+1 -1
View File
@@ -18,7 +18,7 @@ class TestResetPassword(HttpCase):
'email': 'noop@example.com',
})
self.assertEqual(test_user.email, url_parse(test_user.signup_url).decode_query()["signup_email"], "query must contain 'signup_email'")
self.assertEqual(test_user.email, url_parse(test_user.with_context(create_user=True).signup_url).decode_query()["signup_email"], "query must contain 'signup_email'")
# Invalidate signup_url to skip signup process
self.env.invalidate_all()
addons/hr_holidays_attendance/models/hr_leave.py
+1 -1
View File
@@ -56,7 +56,7 @@ class HRLeave(models.Model):
if employee.user_id == self.env.user:
raise ValidationError(_('You do not have enough extra hours to request this leave'))
raise ValidationError(_('The employee does not have enough extra hours to request this leave.'))
if not leave.overtime_id:
if not leave.sudo().overtime_id:
leave.sudo().overtime_id = self.env['hr.attendance.overtime'].sudo().create({
'employee_id': employee.id,
'date': fields.Date.today(),
addons/mail/models/mail_mail.py
+3 -17
View File
@@ -84,6 +84,7 @@ class MailMail(models.Model):
auto_delete = fields.Boolean(
'Auto Delete',
help="This option permanently removes any track of email after it's been sent, including from the Technical menu in the Settings, in order to preserve storage space of your Odoo database.")
# Unused since v16, to remove in master.
to_delete = fields.Boolean('To Delete', help='If set, the mail will be deleted during the next Email Queue CRON run.')
scheduled_date = fields.Datetime('Scheduled Send Date',
help="If set, the queue manager will send the email after the date. If not set, the email will be send as soon as possible. Unless a timezone is specified, it is considered as being in UTC timezone.")
@@ -111,19 +112,6 @@ class MailMail(models.Model):
restricted_attaments = mail_sudo.attachment_ids - IrAttachment._filter_attachment_access(mail_sudo.attachment_ids.ids)
mail_sudo.attachment_ids = restricted_attaments | mail.unrestricted_attachment_ids
def init(self):
"""Create a partial index on "to_delete" to make the search on those records fast.
The benefit on this partial index is to not have a big impact on the
update / insert of other records in the database in comparison to a standard
index.
"""
self._cr.execute("""
CREATE INDEX IF NOT EXISTS mail_mail_to_delete_idx
ON mail_mail(id)
WHERE to_delete = TRUE;
""")
@api.model_create_multi
def create(self, values_list):
# notification field: if not set, set if mail comes from an existing mail.message
@@ -137,7 +125,7 @@ class MailMail(models.Model):
values['scheduled_date'] = False # void string crashes
new_mails = super(MailMail, self).create(values_list)
new_mails_w_attach = self
new_mails_w_attach = self.env['mail.mail']
for mail, values in zip(new_mails, values_list):
if values.get('attachment_ids'):
new_mails_w_attach += mail
@@ -239,8 +227,6 @@ class MailMail(models.Model):
except Exception:
_logger.exception("Failed processing mail queue")
# Remove all the <mail.mail> marked as "to delete"
self.env['mail.mail'].sudo().search([('to_delete', '=', True)]).unlink()
return res
def _postprocess_sent_message(self, success_pids, failure_reason=False, failure_type=None):
@@ -278,7 +264,7 @@ class MailMail(models.Model):
# TDE TODO: could be great to notify message-based, not notifications-based, to lessen number of notifs
messages._notify_message_notification_update() # notify user that we have a failure
if not failure_type or failure_type in ['mail_email_invalid', 'mail_email_missing']: # if we have another error, we want to keep the mail.
self.filtered(lambda mail: mail.auto_delete).to_delete = True
self.filtered(lambda mail: mail.auto_delete).unlink()
return True
addons/mail/tests/common.py
-5
View File
@@ -77,11 +77,6 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
self.mail_mail_create_mocked = mail_mail_create_mocked
yield
if mail_unlink_sent:
# Remove all the <mail.mail> marked as to_delete to simulate the CRON
# Make tests easier and keep backward compatibility before this patch
self.env['mail.mail'].sudo().search([('to_delete', '=', True)]).unlink()
def _init_mail_mock(self):
self._mails = []
self._mails_args = []
addons/mail/views/mail_mail_views.xml
+2 -3
View File
@@ -46,8 +46,7 @@
<group string="Status">
<field name="auto_delete"
attrs="{'invisible': [('state', '!=', 'outgoing'), ('state', '!=', 'exception')]}"/>
<field name="to_delete"
attrs="{'invisible': [('state', '=', 'outgoing')]}"/>
<field name="to_delete" invisible="1"/>
<field name="is_notification"/>
<field name="message_type"/>
<field name="mail_server_id"/>
@@ -99,7 +98,7 @@
<field name="message_type" invisible="1"/>
<field name="state" widget="badge" decoration-muted="state in ('sent', 'cancel')"
decoration-info="state=='outgoing'" decoration-danger="state=='exception'"/>
<field name="to_delete"/>
<field name="to_delete" invisible="1"/>
<button name="send" string="Send Now" type="object" icon="fa-paper-plane" states='outgoing'/>
<button name="mark_outgoing" string="Retry" type="object" icon="fa-repeat" states='exception,cancel'/>
<button name="cancel" string="Cancel Email" type="object" icon="fa-times-circle" states='outgoing'/>
addons/project/models/project.py
+1
View File
@@ -2291,6 +2291,7 @@ class Task(models.Model):
record_name=task.display_name,
email_layout_xmlid='mail.mail_notification_layout',
model_description=task_model_description,
mail_auto_delete=False,
)
def _message_auto_subscribe_followers(self, updated_values, default_subtype_ids):
addons/test_mail/tests/test_mail_activity.py
-2
View File
@@ -589,8 +589,6 @@ class TestActivityMixin(TestActivityCommon):
origin_2_activity_4 = origin_2_activity_1.copy()
origin_2_activity_4.date_deadline = datetime(2020, 1, 2, 0, 0, 0)
self.env['mail.test.activity'].flush_model()
self.assertEqual(origin_2_activity_1.state, 'planned')
self.assertEqual(origin_2_activity_2.state, 'today')
self.assertEqual(origin_2_activity_3.state, 'today')
addons/test_mail/tests/test_message_post.py
-2
View File
@@ -1194,7 +1194,6 @@ class TestMessagePostHelpers(TestMessagePostCommon):
'message_type': 'email',
'model': test_record._name,
'notified_partner_ids': self.env['res.partner'],
'to_delete': True,
'subtype_id': self.env['mail.message.subtype'],
'reply_to': formataddr((f'{self.company_admin.name} {test_record.name}', f'{self.alias_catchall}@{self.alias_domain}')),
'res_id': test_record.id,
@@ -1236,7 +1235,6 @@ class TestMessagePostHelpers(TestMessagePostCommon):
'model': test_record._name,
'notified_partner_ids': self.env['res.partner'],
'recipient_ids': test_record.customer_id,
'to_delete': False,
'subtype_id': self.env['mail.message.subtype'],
'reply_to': formataddr((f'{self.company_admin.name} {test_record.name}', f'{self.alias_catchall}@{self.alias_domain}')),
'res_id': test_record.id,
addons/test_mail/tests/test_performance.py
+30 -22
View File
@@ -2,6 +2,7 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from contextlib import nullcontext
from unittest.mock import patch
from odoo.addons.base.tests.common import TransactionCaseWithUserDemo
from odoo.addons.mail.tests.common import MailCommon
@@ -347,7 +348,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'partner_ids': [(4, customer_id)],
})
with self.assertQueryCount(admin=29, employee=29):
with self.assertQueryCount(admin=35, employee=35):
composer._action_send_mail()
@users('admin', 'employee')
@@ -368,7 +369,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'partner_ids': [(4, customer.id)],
})
with self.assertQueryCount(admin=33, employee=33):
with self.assertQueryCount(admin=39, employee=39):
composer._action_send_mail()
@users('admin', 'employee')
@@ -392,7 +393,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
composer_form.attachment_ids.add(attachment)
composer = composer_form.save()
with self.assertQueryCount(admin=43, employee=43): # tm+com 42/42
with self.assertQueryCount(admin=49, employee=49): # tm+com 48/48
composer._action_send_mail()
# notifications
@@ -436,7 +437,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'partner_ids': [(4, customer_id)],
})
with self.assertQueryCount(admin=29, employee=29):
with self.assertQueryCount(admin=35, employee=35):
composer._action_send_mail()
@users('admin', 'employee')
@@ -454,7 +455,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'default_template_id': test_template.id,
}).create({})
with self.assertQueryCount(admin=28, employee=28):
with self.assertQueryCount(admin=34, employee=34):
composer._action_send_mail()
# notifications
@@ -478,7 +479,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'default_template_id': test_template.id,
}).create({})
with self.assertQueryCount(admin=39, employee=39):
with self.assertQueryCount(admin=45, employee=45):
composer._action_send_mail()
# notifications
@@ -510,7 +511,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
)
composer = composer_form.save()
with self.assertQueryCount(admin=38, employee=38):
with self.assertQueryCount(admin=44, employee=44):
composer._action_send_mail()
# notifications
@@ -540,7 +541,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
)
composer = composer_form.save()
with self.assertQueryCount(admin=59, employee=59):
with self.assertQueryCount(admin=65, employee=65):
composer._action_send_mail()
# notifications
@@ -567,7 +568,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
# use another user already pre-defined with the email notification type,
# so the ormcache is preserved.
record = self.env['mail.test.track'].create({'name': 'Test'})
with self.assertQueryCount(admin=25, employee=25):
with self.assertQueryCount(admin=37, employee=37):
record.write({
'user_id': self.user_test_email.id,
})
@@ -650,7 +651,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
def test_message_post_one_email_notification(self):
record = self.env['mail.test.simple'].create({'name': 'Test'})
with self.assertQueryCount(admin=24, employee=24):
with self.assertQueryCount(admin=30, employee=30):
record.message_post(
body='<p>Test Post Performances with an email ping</p>',
partner_ids=self.customer.ids,
@@ -825,16 +826,23 @@ class TestMailComplexPerformance(BaseMailPerformance):
} for idx in range(12)])
mails[-2].write({'email_cc': False, 'email_to': 'strange@example¢¡.com', 'recipient_ids': [(5, 0)]})
mails[-1].write({'email_cc': False, 'email_to': 'void', 'recipient_ids': [(5, 0)]})
with self.assertQueryCount(admin=43, employee=43):
def _patched_unlink(records):
nonlocal unlinked_mails
unlinked_mails |= set(records.ids)
unlinked_mails = set()
with (self.assertQueryCount(admin=43, employee=43),
patch.object(type(self.env['mail.mail']), 'unlink', _patched_unlink)):
self.env['mail.mail'].sudo().browse(mails.ids).send()
for mail in mails[:-2]:
self.assertEqual(mail.state, 'sent')
self.assertTrue(mail.to_delete, 'Mail: sent mails are to be unlinked')
self.assertIn(mail.id, unlinked_mails, 'Mail: sent mails are to be unlinked')
self.assertEqual(mails[-2].state, 'exception')
self.assertTrue(mails[-2].to_delete, 'Mail: mails with invalid recipient are also to be unlinked')
self.assertIn(mails[-2].id, unlinked_mails, 'Mail: mails with invalid recipient are also to be unlinked')
self.assertEqual(mails[-1].state, 'exception')
self.assertTrue(mails[-1].to_delete, 'Mail: mails with invalid recipient are also to be unlinked')
self.assertIn(mails[-1].id, unlinked_mails, 'Mail: mails with invalid recipient are also to be unlinked')
@mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink')
@users('admin', 'employee')
@@ -844,7 +852,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
record = self.container.with_user(self.env.user)
# about 20 (19?) queries per additional customer group
with self.assertQueryCount(admin=35, employee=34):
with self.assertQueryCount(admin=53, employee=52):
record.message_post(
body='<p>Test Post Performances</p>',
message_type='comment',
@@ -862,7 +870,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
template = self.env.ref('test_mail.mail_test_container_tpl')
# about 20 (19 ?) queries per additional customer group
with self.assertQueryCount(admin=42, employee=41):
with self.assertQueryCount(admin=60, employee=59):
record.message_post_with_source(
template,
message_type='comment',
@@ -952,7 +960,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
})
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id)
with self.assertQueryCount(admin=23, employee=23):
with self.assertQueryCount(admin=37, employee=37):
rec.write({'user_id': self.user_portal.id})
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
# write tracking message
@@ -972,7 +980,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
customer_id = self.customer.id
user_id = self.user_portal.id
with self.assertQueryCount(admin=52, employee=52):
with self.assertQueryCount(admin=88, employee=88):
rec = self.env['mail.test.ticket'].create({
'name': 'Test',
'container_id': container_id,
@@ -1001,7 +1009,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
self.assertEqual(len(rec1.message_ids), 1)
with self.assertQueryCount(admin=33, employee=33):
with self.assertQueryCount(admin=56, employee=56):
rec.write({
'name': 'Test2',
'container_id': self.container.id,
@@ -1038,7 +1046,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
with self.assertQueryCount(admin=33, employee=33):
with self.assertQueryCount(admin=62, employee=62):
rec.write({
'name': 'Test2',
'container_id': container_id,
@@ -1071,7 +1079,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
with self.assertQueryCount(admin=21, employee=21):
with self.assertQueryCount(admin=32, employee=32):
rec.write({
'name': 'Test2',
'customer_id': customer_id,
@@ -1301,7 +1309,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance):
attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals)
# enable_logging = self.cr._enable_logging() if self.warm else nullcontext()
# with self.assertQueryCount(employee=49), enable_logging:
with self.assertQueryCount(employee=45):
with self.assertQueryCount(employee=64):
record_container.with_context({}).message_post(
body='<p>Test body <img src="cid:cid1"> <img src="cid:cid2"></p>',
subject='Test Subject',
addons/test_mail_full/tests/test_mail_performance.py
+1 -1
View File
@@ -81,7 +81,7 @@ class TestMailPerformance(BaseMailPerformance):
record_ticket = self.env['mail.test.ticket.mc'].browse(self.record_ticket.ids)
attachments = self.env['ir.attachment'].create(self.test_attachments_vals)
with self.assertQueryCount(employee=61): # tmf: 60
with self.assertQueryCount(employee=91): # tmf: 90
new_message = record_ticket.message_post(
attachment_ids=attachments.ids,
body='<p>Test Content</p>',
addons/test_mass_mailing/tests/test_performance.py
+4 -12
View File
@@ -46,17 +46,13 @@ class TestMassMailPerformance(TestMassMailPerformanceBase):
'mailing_domain': [('id', 'in', self.mm_recs.ids)],
})
# runbot needs +2 compared to local
with self.assertQueryCount(__system__=427, marketing=428): # tm 425/426
# runbot needs +51 compared to local
with self.assertQueryCount(__system__=1523, marketing=1524):
mailing.action_send_mail()
self.assertEqual(mailing.sent, 50)
self.assertEqual(mailing.delivered, 50)
# runbot needs +3 compared to local
with self.assertQueryCount(__system__=18, marketing=17): # tm 15/15
self.env['mail.mail'].sudo().search([('to_delete', '=', True)]).unlink()
mails = self.env['mail.mail'].sudo().search([('mailing_id', '=', mailing.id)])
self.assertFalse(mails, 'Should have auto-deleted the <mail.mail>')
@@ -93,16 +89,12 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase):
'mailing_domain': [('id', 'in', self.mm_recs.ids)],
})
# runbot needs +2 compared to local
with self.assertQueryCount(__system__=489, marketing=490): # tm 487/488
# runbot needs +51 compared to local
with self.assertQueryCount(__system__=1597, marketing=1598):
mailing.action_send_mail()
self.assertEqual(mailing.sent, 50)
self.assertEqual(mailing.delivered, 50)
# runbot needs +3 compared to local
with self.assertQueryCount(__system__=18, marketing=17): # tm 15/15
self.env['mail.mail'].sudo().search([('to_delete', '=', True)]).unlink()
cancelled_mail_count = self.env['mail.mail'].sudo().search([('mailing_id', '=', mailing.id)])
self.assertEqual(len(cancelled_mail_count), 12, 'Should not have auto deleted the blacklisted emails')