[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