Commit 58c11674 authored by Nicolas Wavrant's avatar Nicolas Wavrant

Fix activity retry settings for erp5_interface_post emails

In production, the sending of emails by using Internet Message Posts created activity failures : 

```
Node: activities-XX
Failures: 1
User name: 'System Processes'
Uid: 1234567890
Document: /erp5/person_module/XXXX
Method: Entity_sendEmail
Arguments: ()
Named Parameters: {'cc_url': None, 'from_url': None, 'event_relative_url': 'event_module/YYYY'}

Exception:
  Module Products.CMFActivity.Activity.SQLBase, line 607, in dequeueMessage
    transaction.commit()
  Module transaction._manager, line 123, in commit
    return self.get().commit()
  Module transaction._transaction, line 280, in commit
    reraise(t, v, tb)
  Module transaction._transaction, line 271, in commit
    self._commitResources()
  Module Products.TIDStorage.transaction_transaction, line 261, in _commitResources
    result = original__commitResources(self, *args, **kw)
  Module transaction._transaction, line 416, in _commitResources
    reraise(t, v, tb)
  Module transaction._transaction, line 393, in _commitResources
    rm.tpc_vote(self)
  Module ZODB.Connection, line 797, in tpc_vote
    s = vote(transaction)
  Module ZEO.ClientStorage, line 1072, in tpc_vote
    return self._check_serials()
  Module ZEO.ClientStorage, line 902, in _check_serials
    raise s
ReadConflictError: database read conflict error (oid 0x12345 serial this txn started with 0x12345 2018-05-16 06:27:17.471887, serial currently committed 0x12345 2018-05-16 06:53:10.977183)
``` 

This Merge Request proposes to allow Entity_sendEmail to be retried in case of ConflictError, as now Entity_sendEmail is doing more than what it used to be.

For this, we need to isolate the call to sendMailHostMessage to make sur that email won't be sent several times if the activity "Entity_sendEmail" is retried.

We also make sure that the activity created to send a message through MailHost gets the correct parameters to be never retried (retry_max=0 and retry_conflict=False)

/reviewed-on nexedi/erp5!672
parents 893e6ce9 7f486435
......@@ -33,4 +33,9 @@ create_post_message_method = event.getTypeBasedMethod('createPostMessage')
if create_post_message_method:
create_post_message_method(mail_message)
else:
event.sendMailHostMessage(mail_message)
# We do not want to retry those activities, as sending email is not transactional safe
event.activate(
activity='SQLQueue',
conflict_retry=False,
max_retry=0
).sendMailHostMessage(mail_message)
# We do not want to retry those activities, as sending email is not transactional safe
activate_kw = kw.pop('activate_kw', {})
activate_kw['max_retry'] = 0
activate_kw['conflict_retry'] = False
context.getPortalObject().portal_catalog.searchAndActivate(
method_id="Entity_sendEmail",
destination_related_uid=context.getUid(),
method_kw=method_kw,
activate_kw=activate_kw,
activate_kw=kw.pop('activate_kw', {}),
**kw)
# As a general rule, sending messages by MailHost should
# be done in an activity context that never retry.
# We do so because MailHost isn't transactional as the zodb.
# This script should then always be called in an activity
# spawned with parameters :
# conflict_retry=False,
# max_retry=0,
context.getPortalObject().MailHost.send(data)
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>data</string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>ERP5Site_sendMailHostMessage</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
context.getPortalObject().MailHost.send(context.getData())
# calling ERP5Site_sendMailHostMessage should be done in an activity
# which NEVER retries.
# internet_message_post_module because we need an object supporting
# the activate method
context.getPortalObject().internet_message_post_module.activate(
activity='SQLQueue',
conflict_retry=False,
max_retry=0,
).ERP5Site_sendMailHostMessage(context.getData())
......@@ -28,6 +28,7 @@ import email
import time
from Products.ERP5Type.tests.ERP5TypeLiveTestCase import ERP5TypeTestCase
from Products.ERP5Type.tests.utils import createZODBPythonScript, removeZODBPythonScript
from Products.ERP5Type.tests.Sequence import SequenceList
from Products.ZSQLCatalog.SQLCatalog import SimpleQuery
from DateTime import DateTime
......@@ -81,6 +82,14 @@ class TestInterfacePost(ERP5TypeTestCase):
module = getattr(self.portal, module_id)
module.manage_delObjects(list(module.objectIds()))
custom_skin = self.portal.portal_skins.custom
if 'Entity_sendEmail' in custom_skin.objectIds():
removeZODBPythonScript(
custom_skin,
'Entity_sendEmail',
)
self.commit()
def _portal_catalog(self, **kw):
result_list = self.portal.portal_catalog(**kw)
uid_list = [x.uid for x in result_list]
......@@ -333,6 +342,21 @@ class TestInterfacePost(ERP5TypeTestCase):
pdf_document, = pdf_document_list
self.assertEqual(2, int(pdf_document.getContentInformation()['Pages']))
def stepMakeEntitySendEmailFailOnce(self, sequence=None):
createZODBPythonScript(
self.portal.portal_skins.custom,
'Entity_sendEmail',
self.portal.Entity_sendEmail.params(),
"""portal = context.getPortalObject()
for activity in portal.portal_activities.getMessageList():
if activity.method_id == script.id:
if activity.retry == 0:
raise ValueError('Failure on purpose')
else:
return context.skinSuper('custom', script.id)(%s)""" % (self.portal.Entity_sendEmail.params(),)
)
def test_emailSendingIsPilotedByInternetMessagePost(self):
"""
"""
......@@ -392,6 +416,27 @@ class TestInterfacePost(ERP5TypeTestCase):
sequence_list.addSequenceString(sequence_string)
sequence_list.play(self)
def test_Entity_sendEmailCanRaiseOnceWithoutSpammingRecipient(self):
"""
Entity_sendEmail used to be launched in an activity with retry_max=0 and
retry_conflict=False. But now that it creates Internet Message Posts, it
should be able to retry on ConflictError. We should also make sure that
in this case the mail isn't sent (as MailHost isn't transactional)
"""
sequence_list = SequenceList()
sequence_string = """
stepMakeEntitySendEmailFailOnce
stepCreateMailMessage
stepStartMailMessage
stepCheckMailMessage
stepTic
stepCheckInternetMessagePostCreated
stepCheckOnlyOneMessageHasBeenSentFromMailHost
stepCheckLatestMessageListFromMailHost
"""
sequence_list.addSequenceString(sequence_string)
sequence_list.play(self)
def test_ingestedInternetMessagePostCreateMailMessageWithCausality(self):
sequence_list = SequenceList()
sequence_string = """
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment