[Zope3-checkins] SVN: Zope3/branches/3.2/ Backport the bugfix of
issue 590 to the Zope 3.2 branch:
Marius Gedminas
marius at pov.lt
Fri Apr 21 13:58:20 EDT 2006
Log message for revision 67244:
Backport the bugfix of issue 590 to the Zope 3.2 branch:
svn merge -r 67222:67241 svn+ssh://svn.zope.org/repos/main/Zope3/trunk/src/zope/app/mail src/zope/app/mail
svn merge -r 67241:67242 svn+ssh://svn.zope.org/repos/main/Zope3/trunk/doc doc
Changed:
U Zope3/branches/3.2/doc/CHANGES.txt
U Zope3/branches/3.2/src/zope/app/mail/delivery.py
U Zope3/branches/3.2/src/zope/app/mail/interfaces.py
U Zope3/branches/3.2/src/zope/app/mail/maildir.py
U Zope3/branches/3.2/src/zope/app/mail/metaconfigure.py
U Zope3/branches/3.2/src/zope/app/mail/metadirectives.py
U Zope3/branches/3.2/src/zope/app/mail/tests/test_delivery.py
U Zope3/branches/3.2/src/zope/app/mail/tests/test_maildir.py
U Zope3/branches/3.2/src/zope/app/mail/tests/test_mailer.py
-=-
Modified: Zope3/branches/3.2/doc/CHANGES.txt
===================================================================
--- Zope3/branches/3.2/doc/CHANGES.txt 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/doc/CHANGES.txt 2006-04-21 17:58:19 UTC (rev 67244)
@@ -10,6 +10,10 @@
Bug fixes
+ - Fixed issue 590: zope.app.mail could send multiple copies of emails on
+ ConflictErrors, and it was also rate limited to 1 email per second when
+ you used QueuedMailDelivery.
+
- SimpleInputWidget used to clear the widget's error as a side effect
sometimes.
Modified: Zope3/branches/3.2/src/zope/app/mail/delivery.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/delivery.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/delivery.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -24,7 +24,6 @@
import logging
import atexit
import time
-
from os import unlink, getpid
from cStringIO import StringIO
from random import randrange
@@ -37,6 +36,7 @@
from transaction.interfaces import IDataManager
import transaction
+
class MailDataManager(object):
implements(IDataManager)
@@ -48,7 +48,7 @@
self.transaction_manager = transaction.manager
def commit(self, transaction):
- self.callable(*self.args)
+ pass
def abort(self, transaction):
if self.onAbort:
@@ -60,6 +60,7 @@
# No subtransaction support.
def abort_sub(self, transaction):
pass
+
commit_sub = abort_sub
def beforeCompletion(self, transaction):
@@ -73,8 +74,12 @@
def tpc_vote(self, transaction):
pass
- tpc_finish = tpc_abort = tpc_vote
+ def tpc_finish(self, transaction):
+ self.callable(*self.args)
+ tpc_abort = abort
+
+
class AbstractMailDelivery(object):
def newMessageId(self):
@@ -131,10 +136,12 @@
msg.write(message)
return MailDataManager(msg.commit, onAbort=msg.abort)
+
class QueueProcessorThread(threading.Thread):
"""This thread is started at configuration time from the
`mail:queuedDelivery` directive handler.
"""
+
log = logging.getLogger("QueueProcessorThread")
__stopped = False
@@ -202,11 +209,11 @@
if fromaddr != '' or toaddrs != ():
self.log.error(
"Error while sending mail from %s to %s.",
- fromaddr, ", ".join(toaddrs), exc_info=1)
+ fromaddr, ", ".join(toaddrs), exc_info=True)
else:
self.log.error(
"Error while sending mail : %s ",
- filename, exc_info=1)
+ filename, exc_info=True)
else:
if forever:
time.sleep(3)
Modified: Zope3/branches/3.2/src/zope/app/mail/interfaces.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/interfaces.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/interfaces.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -42,7 +42,7 @@
- An `IMailQueueProcessor` or `IDirectMailDelivery` actually delivers the
messages by using a mailer (`IMailer`) component that encapsulates the
- delivery process. There are currently two mailers:
+ delivery process. There currently is only one mailer:
- `ISMTPMailer` sends all messages to a relay host using SMTP
@@ -77,8 +77,8 @@
Returns the message ID.
- You can subscribe to `IMailEvent` events for notification about problems
- or successful delivery.
+ You can subscribe to `IMailEvent` events for notification about
+ problems or successful delivery.
Messages are actually sent during transaction commit.
"""
@@ -259,3 +259,4 @@
Calling ``abort()`` more than once is allowed.
"""
+
Modified: Zope3/branches/3.2/src/zope/app/mail/maildir.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/maildir.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/maildir.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -20,12 +20,14 @@
import os
import socket
import time
+import random
from zope.interface import implements, classProvides
from zope.app.mail.interfaces import \
IMaildirFactory, IMaildir, IMaildirMessageWriter
+
class Maildir(object):
"""See `zope.app.interfaces.mail.IMaildir`"""
@@ -80,21 +82,28 @@
subdir_new = join(self.path, 'new')
pid = os.getpid()
host = socket.gethostname()
+ randmax = 0x7fffffff
counter = 0
- while 1:
+ while True:
timestamp = int(time.time())
- unique = '%d.%d.%s' % (timestamp, pid, host)
+ unique = '%d.%d.%s.%d' % (timestamp, pid, host,
+ random.randrange(randmax))
filename = join(subdir_tmp, unique)
- if not os.path.exists(filename):
+ try:
+ fd = os.open(filename, os.O_CREAT|os.O_EXCL|os.O_WRONLY, 0600)
+ except OSError:
+ # File exists
+ counter += 1
+ if counter >= 1000:
+ raise RuntimeError("Failed to create unique file name"
+ " in %s, are we under a DoS attack?"
+ % subdir_tmp)
+ # NOTE: maildir.html (see above) says I should sleep for 2
+ time.sleep(0.1)
+ else:
break
- counter += 1
- if counter >= 1000:
- raise RuntimeError("Failed to create unique file name in %s,"
- " are we under a DoS attack?" % subdir_tmp)
- # NOTE: maildir.html (see above) says I should sleep for 2
- # seconds, not 1
- time.sleep(1)
- return MaildirMessageWriter(filename, join(subdir_new, unique))
+ return MaildirMessageWriter(os.fdopen(fd, 'w'), filename,
+ join(subdir_new, unique))
class MaildirMessageWriter(object):
@@ -102,13 +111,10 @@
implements(IMaildirMessageWriter)
- # A hook for unit tests
- open = open
-
- def __init__(self, filename, new_filename):
+ def __init__(self, fd, filename, new_filename):
self._filename = filename
self._new_filename = new_filename
- self._fd = self.open(filename, 'w')
+ self._fd = fd
self._closed = False
self._aborted = False
Modified: Zope3/branches/3.2/src/zope/app/mail/metaconfigure.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/metaconfigure.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/metaconfigure.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -36,8 +36,8 @@
checker = InterfaceChecker(interfaces, permission)
return proxify(component, checker)
-
+
def queuedDelivery(_context, permission, queuePath, mailer, name="Mail"):
def createQueuedDelivery():
Modified: Zope3/branches/3.2/src/zope/app/mail/metadirectives.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/metadirectives.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/metadirectives.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -22,10 +22,11 @@
from zope.schema import TextLine, ASCII, BytesLine, Int
from zope.app.security.fields import Permission
+
class IDeliveryDirective(Interface):
"""This abstract directive describes a generic mail delivery utility
registration."""
-
+
name = TextLine(
title=u"Name",
description=u'Specifies the Delivery name of the mail utility. '\
@@ -37,7 +38,7 @@
title=u"Permission",
description=u"Defines the permission needed to use this service.",
required=True)
-
+
mailer = TextLine(
title=u"Mailer",
description=u"Defines the mailer to be used for sending mail.",
@@ -66,8 +67,8 @@
title=u"Name",
description=u"Name of the Mailer.",
required=True)
-
+
class ISMTPMailerDirective(IMailerDirective):
"""Registers a new SMTP mailer."""
Modified: Zope3/branches/3.2/src/zope/app/mail/tests/test_delivery.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/tests/test_delivery.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/tests/test_delivery.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -17,11 +17,13 @@
$Id$
"""
+
import os.path
from tempfile import mktemp
from unittest import TestCase, TestSuite, makeSuite
+
import transaction
-
+from zope.testing import doctest
from zope.interface import implements
from zope.interface.verify import verifyObject
from zope.app.mail.interfaces import IMailer
@@ -48,6 +50,47 @@
self.assertEqual(manager.args, (1, 2))
+def print_success(*args):
+ print "message successfully sent, args: %s" % (args, )
+
+def print_abort():
+ print "message aborted"
+
+
+def doctest_successful_commit():
+ """Regression test for http://www.zope.org/Collectors/Zope3-dev/590
+
+ Let's do a full two-phase commit.
+
+ >>> from zope.app.mail.delivery import MailDataManager
+ >>> manager = MailDataManager(print_success, ('foo', 'bar'),
+ ... onAbort=print_abort)
+ >>> transaction = object()
+ >>> manager.tpc_begin(transaction)
+ >>> manager.commit(transaction)
+ >>> manager.tpc_vote(transaction)
+ >>> manager.tpc_finish(transaction)
+ message successfully sent, args: ('foo', 'bar')
+
+ """
+
+
+def doctest_unsuccessful_commit():
+ """Regression test for http://www.zope.org/Collectors/Zope3-dev/590
+
+ Let's start a two-phase commit, then abort it.
+
+ >>> from zope.app.mail.delivery import MailDataManager
+ >>> manager = MailDataManager(print_success, onAbort=print_abort)
+ >>> manager.tpc_begin(transaction)
+ >>> manager.commit(transaction)
+ >>> manager.tpc_vote(transaction)
+ >>> manager.tpc_abort(transaction)
+ message aborted
+
+ """
+
+
class TestDirectMailDelivery(TestCase):
def testInterface(self):
@@ -243,14 +286,11 @@
self.thread.log = LoggerStub()
def test_parseMessage(self):
-
hdr = ('X-Zope-From: foo at example.com\n'
'X-Zope-To: bar at example.com, baz at example.com\n')
msg = ('Header: value\n'
'\n'
'Body\n')
-
-
f, t, m = self.thread._parseMessage(hdr + msg)
self.assertEquals(f, 'foo at example.com')
self.assertEquals(t, ('bar at example.com', 'baz at example.com'))
@@ -276,7 +316,6 @@
'bar at example.com, baz at example.com'),
{})])
-
def test_error_logging(self):
self.thread.setMailer(BrokenMailerStub())
self.filename = mktemp()
@@ -294,13 +333,13 @@
{'exc_info': 1})])
-
def test_suite():
return TestSuite((
makeSuite(TestMailDataManager),
makeSuite(TestDirectMailDelivery),
makeSuite(TestQueuedMailDelivery),
makeSuite(TestQueueProcessorThread),
+ doctest.DocTestSuite(),
))
if __name__ == '__main__':
Modified: Zope3/branches/3.2/src/zope/app/mail/tests/test_maildir.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/tests/test_maildir.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/tests/test_maildir.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -15,8 +15,10 @@
$Id$
"""
+
import unittest
import stat
+import os
from zope.interface.verify import verifyObject
@@ -42,20 +44,20 @@
self.files = files
self.dirs = dirs
- _exists_never_fails = False
-
def join(self, *args):
return '/'.join(args)
def isdir(self, dir):
return dir in self.dirs
- def exists(self, p):
- return self._exists_never_fails or p in self.files
class FakeOsModule(object):
F_OK = 0
+ O_CREAT = os.O_CREAT
+ O_EXCL = os.O_EXCL
+ O_WRONLY = os.O_WRONLY
+
_stat_mode = {
'/path/to/maildir': stat.S_IFDIR,
'/path/to/maildir/new': stat.S_IFDIR,
@@ -67,8 +69,8 @@
'/path/to/maildir/tmp': stat.S_IFDIR,
'/path/to/maildir/tmp/1': stat.S_IFREG,
'/path/to/maildir/tmp/2': stat.S_IFREG,
- '/path/to/maildir/tmp/1234500000.4242.myhostname': stat.S_IFREG,
- '/path/to/maildir/tmp/1234500001.4242.myhostname': stat.S_IFREG,
+ '/path/to/maildir/tmp/1234500000.4242.myhostname.*': stat.S_IFREG,
+ '/path/to/maildir/tmp/1234500001.4242.myhostname.*': stat.S_IFREG,
'/path/to/regularfile': stat.S_IFREG,
'/path/to/emptydirectory': stat.S_IFDIR,
}
@@ -84,8 +86,19 @@
_removed_files = ()
_renamed_files = ()
+ _all_files_exist = False
+
+ def __init__(self):
+ self._descriptors = {}
+
def access(self, path, mode):
- return path in self._stat_mode
+ if self._all_files_exist:
+ return True
+ if path in self._stat_mode:
+ return True
+ if path.rsplit('.', 1)[0] + '.*' in self._stat_mode:
+ return True
+ return False
def stat(self, path):
if path in self._stat_mode:
@@ -107,6 +120,37 @@
def rename(self, old, new):
self._renamed_files += ((old, new), )
+ def open(self, filename, flags, mode=0777):
+ if (flags & os.O_EXCL and flags & os.O_CREAT
+ and self.access(filename, 0)):
+ raise OSError('file already exists')
+ if not flags & os.O_CREAT and not self.access(filename, 0):
+ raise OSError('file not found')
+ fd = max(self._descriptors.keys() + [2]) + 1
+ self._descriptors[fd] = filename, flags, mode
+ return fd
+
+ def fdopen(self, fd, mode='r'):
+ try:
+ filename, flags, permissions = self._descriptors[fd]
+ except KeyError:
+ raise AssertionError('os.fdopen() called with an unknown'
+ ' file descriptor')
+ if mode == 'r':
+ assert not flags & os.O_WRONLY
+ assert not flags & os.O_RDWR
+ elif mode == 'w':
+ assert flags & os.O_WRONLY
+ assert not flags & os.O_RDWR
+ elif mode == 'r+':
+ assert not flags & os.O_WRONLY
+ assert flags & os.O_RDWR
+ else:
+ raise AssertionError("don't know how to verify if flags match"
+ " mode %r" % mode)
+ return FakeFile(filename, mode)
+
+
class FakeFile(object):
def __init__(self, filename, mode):
@@ -124,10 +168,7 @@
def writelines(self, lines):
self._written += ''.join(lines)
-def fake_open(self, filename, mode):
- return FakeFile(filename, mode)
-
class TestMaildir(unittest.TestCase):
def setUp(self):
@@ -139,15 +180,13 @@
maildir_module.os = self.fake_os_module = FakeOsModule()
maildir_module.time = FakeTimeModule()
maildir_module.socket = FakeSocketModule()
- maildir_module.MaildirMessageWriter.open = fake_open
def tearDown(self):
self.maildir_module.os = self.old_os_module
self.maildir_module.time = self.old_time_module
self.maildir_module.socket = self.old_socket_module
- self.maildir_module.MaildirMessageWriter.open = open
self.fake_os_module._stat_never_fails = False
- self.fake_os_module.path._exists_never_fails = False
+ self.fake_os_module._all_files_exist = False
def test_factory(self):
from zope.app.mail.interfaces import IMaildirFactory, IMaildir
@@ -160,7 +199,7 @@
# Case 2a: directory does not exist, create = False
self.assertRaises(ValueError, Maildir, '/path/to/nosuchfolder', False)
-
+
# Case 2b: directory does not exist, create = True
m = Maildir('/path/to/nosuchfolder', True)
verifyObject(IMaildir, m)
@@ -195,13 +234,13 @@
m = Maildir('/path/to/maildir')
fd = m.newMessage()
verifyObject(IMaildirMessageWriter, fd)
- self.assertEquals(fd._filename,
- '/path/to/maildir/tmp/1234500002.4242.myhostname')
+ self.assert_(fd._filename.startswith(
+ '/path/to/maildir/tmp/1234500002.4242.myhostname.'))
def test_newMessage_never_loops(self):
from zope.app.mail.maildir import Maildir
from zope.app.mail.interfaces import IMaildirMessageWriter
- self.fake_os_module.path._exists_never_fails = True
+ self.fake_os_module._all_files_exist = True
m = Maildir('/path/to/maildir')
self.assertRaises(RuntimeError, m.newMessage)
@@ -209,8 +248,8 @@
from zope.app.mail.maildir import MaildirMessageWriter
filename1 = '/path/to/maildir/tmp/1234500002.4242.myhostname'
filename2 = '/path/to/maildir/new/1234500002.4242.myhostname'
- writer = MaildirMessageWriter(filename1, filename2)
- # writer._fd should be a FakeFile instance because we stubbed open()
+ fd = FakeFile(filename1, 'w')
+ writer = MaildirMessageWriter(fd, filename1, filename2)
self.assertEquals(writer._fd._filename, filename1)
self.assertEquals(writer._fd._mode, 'w') # TODO or 'wb'?
print >> writer, 'fee',
@@ -233,7 +272,8 @@
from zope.app.mail.maildir import MaildirMessageWriter
filename1 = '/path/to/maildir/tmp/1234500002.4242.myhostname'
filename2 = '/path/to/maildir/new/1234500002.4242.myhostname'
- writer = MaildirMessageWriter(filename1, filename2)
+ fd = FakeFile(filename1, 'w')
+ writer = MaildirMessageWriter(fd, filename1, filename2)
writer.commit()
self.assertEquals(writer._fd._closed, True)
self.assert_((filename1, filename2)
Modified: Zope3/branches/3.2/src/zope/app/mail/tests/test_mailer.py
===================================================================
--- Zope3/branches/3.2/src/zope/app/mail/tests/test_mailer.py 2006-04-21 17:53:53 UTC (rev 67243)
+++ Zope3/branches/3.2/src/zope/app/mail/tests/test_mailer.py 2006-04-21 17:58:19 UTC (rev 67244)
@@ -90,7 +90,6 @@
self.assert_(self.smtp.quit)
-
def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestSMTPMailer))
More information about the Zope3-Checkins
mailing list