8 files changed, 302 insertions, 112 deletions

diff --git a/src/mailman/app/bounces.py b/src/mailman/app/bounces.py
index 8d2526a66..a95ee954b 100644
--- a/src/mailman/app/bounces.py
+++ b/src/mailman/app/bounces.py
@@ -21,9 +21,11 @@ from __future__ import absolute_import, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'ProbeVERP',
+    'StandardVERP',
     'bounce_message',
-    'get_verp',
     'scan_message',
+    'send_probe',
     ]
 
 
@@ -33,18 +35,26 @@ import logging
 from email.mime.message import MIMEMessage
 from email.mime.text import MIMEText
 from email.utils import parseaddr
+from string import Template
+from zope.component import getUtility
+from zope.interface import implements
 
 from mailman.app.finder import find_components
 from mailman.config import config
 from mailman.core.i18n import _
 from mailman.email.message import UserNotification
 from mailman.interfaces.bounce import IBounceDetector
+from mailman.interfaces.listmanager import IListManager
+from mailman.interfaces.pending import IPendable, IPendings
 from mailman.utilities.email import split_email
+from mailman.utilities.i18n import make
 from mailman.utilities.string import oneline
 
 log = logging.getLogger('mailman.config')
 elog = logging.getLogger('mailman.error')
 
+DOT = '.'
+
 
 
 def bounce_message(mlist, msg, e=None):
@@ -103,46 +113,126 @@ def scan_message(mlist, msg):
 
 
 
-def get_verp(mlist, msg):
-    """Extract a set of VERP bounce addresses.
+class _BaseVERPParser:
+    """Base class for parsing VERP messages.
 
     Sadly not every MTA bounces VERP messages correctly, or consistently.
     First, the To: header is checked, then Delivered-To: (Postfix),
     Envelope-To: (Exim) and Apparently-To:.  Note that there can be multiple
-    Delivered-To: headers so we need to search them all (and we don't worry
-    about false positives for forwarded email, because only one should match
-    `verp_regexp`).
-
-    :param mlist: The mailing list being checked.
-    :type mlist: `IMailingList`
-    :param msg: The message being parsed.
-    :type msg: `email.message.Message`
-    :return: The set of addresses extracted from the VERP headers.
-    :rtype: set of strings
+    headers so we need to search them all
     """
-    cre = re.compile(config.mta.verp_regexp, re.IGNORECASE)
-    blocal, bdomain = split_email(mlist.bounces_address)
-    values = set()
-    verp_matches = set()
-    for header in ('to', 'delivered-to', 'envelope-to', 'apparently-to'):
-        values.update(msg.get_all(header, []))
-    for field in values:
-        address = parseaddr(field)[1]
-        if not address:
-            # This header was empty.
-            continue
-        mo = cre.search(address)
-        if not mo:
-            # This did not match the VERP regexp.
-            continue
-        try:
-            if blocal != mo.group('bounces'):
-                # This was not a bounce to our mailing list.
+
+    def __init__(self, pattern):
+        self._pattern = pattern
+        self._cre = re.compile(pattern, re.IGNORECASE)
+
+    def _get_addresses(self, match_object):
+        raise NotImplementedError
+
+    def get_verp(self, mlist, msg):
+        """Extract a set of VERP bounce addresses.
+
+
+        :param mlist: The mailing list being checked.
+        :type mlist: `IMailingList`
+        :param msg: The message being parsed.
+        :type msg: `email.message.Message`
+        :return: The set of addresses extracted from the VERP headers.
+        :rtype: set of strings
+        """
+        blocal, bdomain = split_email(mlist.bounces_address)
+        values = set()
+        verp_matches = set()
+        for header in ('to', 'delivered-to', 'envelope-to', 'apparently-to'):
+            values.update(msg.get_all(header, []))
+        for field in values:
+            address = parseaddr(field)[1]
+            if not address:
+                # This header was empty.
                 continue
-            original_address = '{0}@{1}'.format(*mo.group('local', 'domain'))
-        except IndexError:
-            elog.error("verp_regexp doesn't yield the right match groups")
-            return set()
-        else:
-            verp_matches.add(original_address)
-    return verp_matches
+            mo = self._cre.search(address)
+            if not mo:
+                # This did not match the VERP regexp.
+                continue
+            try:
+                if blocal != mo.group('bounces'):
+                    # This was not a bounce to our mailing list.
+                    continue
+                original_address = self._get_address(mo)
+            except IndexError:
+                elog.error('Bad VERP pattern: {0}'.format(self._pattern))
+                return set()
+            else:
+                verp_matches.add(original_address)
+        return verp_matches
+
+
+
+class StandardVERP(_BaseVERPParser):
+    def __init__(self):
+        super(StandardVERP, self).__init__(config.mta.verp_regexp)
+
+    def _get_address(self, match_object):
+        return '{0}@{1}'.format(*match_object.group('local', 'domain'))
+
+
+class ProbeVERP(_BaseVERPParser):
+    def __init__(self):
+        super(ProbeVERP, self).__init__(config.mta.verp_probe_regexp)
+
+    def _get_address(self, match_object):
+        # Extract the token and get the matching address.
+        token = match_object.group('token')
+        op, address, bmsg = getUtility(IPendings).confirm(token)
+        return address
+
+
+
+class _ProbePendable(dict):
+    """The pendable dictionary for probe messages."""
+    implements(IPendable)
+
+
+def send_probe(member, msg):
+    """Send a VERP probe to the member.
+
+    :param member: The member to send the probe to.  From this object, both
+        the user and the mailing list can be determined.
+    :type member: IMember
+    :param msg: The bouncing message that caused the probe to be sent.
+    :type msg:
+    :return: The token representing this probe in the pendings database.
+    :rtype: string
+    """
+    mlist = getUtility(IListManager).get(member.mailing_list)
+    text = make('probe.txt', mlist, member.preferred_language.code,
+        listname=mlist.fqdn_listname,
+        address= member.address.email,
+        optionsurl=member.options_url,
+        owneraddr=mlist.owner_address,
+        )
+    pendable = _ProbePendable(
+        member_id=member.member_id,
+        message_id=msg['message-id'],
+        )
+    token = getUtility(IPendings).add(pendable)
+    mailbox, domain_parts = split_email(mlist.bounces_address)
+    probe_sender = Template(config.mta.verp_probe_format).safe_substitute(
+        bounces=mailbox,
+        token=token,
+        domain=DOT.join(domain_parts),
+        )
+    # Calculate the Subject header, in the member's preferred language.
+    with _.using(member.preferred_language.code):
+        subject = _('$mlist.real_name mailing list probe message')
+    # Craft the probe message.  This will be a multipart where the first part
+    # is the probe text and the second part is the message that caused this
+    # probe to be sent.
+    probe = UserNotification(member.address.email, probe_sender,
+                             subject, lang=member.preferred_language)
+    probe.set_type('multipart/mixed')
+    notice = MIMEText(text, _charset=mlist.preferred_language.charset)
+    probe.attach(notice)
+    probe.attach(MIMEMessage(msg))
+    probe.send(mlist, envsender=probe_sender, verp=False, probe_token=token)
+    return token
diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py
index c6a9394ad..7ef78500f 100644
--- a/src/mailman/app/tests/test_bounces.py
+++ b/src/mailman/app/tests/test_bounces.py
@@ -27,19 +27,28 @@ __all__ = [
 
 import unittest
 
-from mailman.app.bounces import get_verp
+from zope.component import getUtility
+
+from mailman.app.bounces import StandardVERP, send_probe
 from mailman.app.lifecycle import create_list
+from mailman.app.membership import add_member
+from mailman.interfaces.member import DeliveryMode
+from mailman.interfaces.pending import IPendings
 from mailman.testing.helpers import (
+    get_queue_messages,
     specialized_message_from_string as message_from_string)
 from mailman.testing.layers import ConfigLayer
 
 
 
 class TestVERP(unittest.TestCase):
+    """Test header VERP detection."""
+
     layer = ConfigLayer
 
     def setUp(self):
         self._mlist = create_list('test@example.com')
+        self._verper = StandardVERP()
 
     def test_no_verp(self):
         # The empty set is returned when there is no VERP headers.
@@ -48,7 +57,7 @@ From: postmaster@example.com
 To: mailman-bounces@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set())
+        self.assertEqual(self._verper.get_verp(self._mlist, msg), set())
 
     def test_verp_in_to(self):
         # A VERP address is found in the To header.
@@ -57,7 +66,8 @@ From: postmaster@example.com
 To: test-bounces+anne=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_verp_in_delivered_to(self):
         # A VERP address is found in the Delivered-To header.
@@ -66,7 +76,8 @@ From: postmaster@example.com
 Delivered-To: test-bounces+anne=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_verp_in_envelope_to(self):
         # A VERP address is found in the Envelope-To header.
@@ -75,7 +86,8 @@ From: postmaster@example.com
 Envelope-To: test-bounces+anne=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_verp_in_apparently_to(self):
         # A VERP address is found in the Apparently-To header.
@@ -84,7 +96,8 @@ From: postmaster@example.com
 Apparently-To: test-bounces+anne=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_verp_with_empty_header(self):
         # A VERP address is found, but there's an empty header.
@@ -94,7 +107,8 @@ To: test-bounces+anne=example.org@example.com
 To:
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_no_verp_with_empty_header(self):
         # There's an empty header, and no VERP address is found.
@@ -103,7 +117,7 @@ From: postmaster@example.com
 To:
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set())
+        self.assertEqual(self._verper.get_verp(self._mlist, msg), set())
 
     def test_verp_with_non_match(self):
         # A VERP address is found, but a header had a non-matching pattern.
@@ -113,7 +127,8 @@ To: test-bounces+anne=example.org@example.com
 To: test-bounces@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_no_verp_with_non_match(self):
         # No VERP address is found, and a header had a non-matching pattern.
@@ -122,7 +137,7 @@ From: postmaster@example.com
 To: test-bounces@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set())
+        self.assertEqual(self._verper.get_verp(self._mlist, msg), set())
 
     def test_multiple_verps(self):
         # More than one VERP address was found in the same header.
@@ -132,7 +147,8 @@ To: test-bounces+anne=example.org@example.com
 To: test-bounces+anne=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
+                         set(['anne@example.org']))
 
     def test_multiple_verps_different_values(self):
         # More than one VERP address was found in the same header with
@@ -143,7 +159,7 @@ To: test-bounces+anne=example.org@example.com
 To: test-bounces+bart=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg),
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
                          set(['anne@example.org', 'bart@example.org']))
 
     def test_multiple_verps_different_values_different_headers(self):
@@ -155,12 +171,110 @@ To: test-bounces+anne=example.org@example.com
 Apparently-To: test-bounces+bart=example.org@example.com
 
 """)
-        self.assertEqual(get_verp(self._mlist, msg),
+        self.assertEqual(self._verper.get_verp(self._mlist, msg),
                          set(['anne@example.org', 'bart@example.org']))
 
 
 
+class TestSendProbe(unittest.TestCase):
+    """Test sending of the probe message."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._member = add_member(self._mlist, 'anne@example.com',
+                                  'Anne Person', 'xxx',
+                                  DeliveryMode.regular, 'en')
+        self._msg = message_from_string("""\
+From: bouncer@example.com
+To: anne@example.com
+Subject: You bounced
+Message-ID: <first>
+
+""")
+
+    def test_token(self):
+        # Show that send_probe() returns a proper token, and that the token
+        # corresponds to a record in the pending database.
+        token = send_probe(self._member, self._msg)
+        pendable = getUtility(IPendings).confirm(token)
+        self.assertEqual(len(pendable.items()), 2)
+        self.assertEqual(set(pendable.keys()),
+                         set(['member_id', 'message_id']))
+        self.assertEqual(pendable['member_id'], self._member.member_id)
+        self.assertEqual(pendable['message_id'], '<first>')
+
+    def test_probe_is_multipart(self):
+        # The probe is a multipart/mixed with two subparts.
+        send_probe(self._member, self._msg)
+        message = get_queue_messages('virgin')[0].msg
+        self.assertEqual(message.get_content_type(), 'multipart/mixed')
+        self.assertTrue(message.is_multipart())
+        self.assertEqual(len(message.get_payload()), 2)
+
+    def test_probe_sends_one_message(self):
+        # send_probe() places one message in the virgin queue.
+        items = get_queue_messages('virgin')
+        self.assertEqual(len(items), 0)
+        send_probe(self._member, self._msg)
+        items = get_queue_messages('virgin')
+        self.assertEqual(len(items), 1)
+
+    def test_probe_contains_original(self):
+        # Show that send_probe() places a properly formatted message in the
+        # virgin queue.
+        send_probe(self._member, self._msg)
+        message = get_queue_messages('virgin')[0].msg
+        rfc822 = message.get_payload(1)
+        self.assertEqual(rfc822.get_content_type(), 'message/rfc822')
+        self.assertTrue(rfc822.is_multipart())
+        self.assertEqual(len(rfc822.get_payload()), 1)
+        self.assertEqual(rfc822.get_payload(0).as_string(),
+                         self._msg.as_string())
+
+    def test_notice(self):
+        # Test that the notice in the first subpart is correct.
+        send_probe(self._member, self._msg)
+        message = get_queue_messages('virgin')[0].msg
+        notice = message.get_payload(0)
+        self.assertEqual(notice.get_content_type(), 'text/plain')
+        # The interesting bits are the parts that have been interpolated into
+        # the message.  For now the best we can do is know that the
+        # interpolation values appear in the message.  When Python 2.7 is our
+        # minimum requirement, we can use assertRegexpMatches().
+        body = notice.get_payload()
+        self.assertTrue('test@example.com' in body)
+        self.assertTrue('anne@example.com' in body)
+        self.assertTrue('http://example.com/anne@example.com' in body)
+        self.assertTrue('test-owner@example.com' in body)
+
+    def test_headers(self):
+        # Check the headers of the outer message.
+        token = send_probe(self._member, self._msg)
+        message = get_queue_messages('virgin')[0].msg
+        self.assertEqual(message['From'],
+                         'test-bounces+{0}@example.com'.format(token))
+        self.assertEqual(message['To'], 'anne@example.com')
+        self.assertEqual(message['Subject'], 'Test mailing list probe message')
+
+
+
+class TestProbe(unittest.TestCase):
+    """Test VERP probing."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+
+    
+
+
+
 def test_suite():
     suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestProbe))
+    suite.addTest(unittest.makeSuite(TestSendProbe))
     suite.addTest(unittest.makeSuite(TestVERP))
     return suite
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index f9f2df03d..329aea412 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -477,7 +477,7 @@ verp_personalized_deliveries: no
 verp_delivery_interval: 0
 
 # VERP format and regexp for probe messages.
-verp_probe_format: %(bounces)s+%(token)s
+verp_probe_format: $bounces+$token@$domain
 verp_probe_regexp: ^(?P<bounces>[^+]+?)\+(?P<token>[^@]+)@.*$
 # Set this 'yes' to activate VERP probe for disabling by bounce.
 verp_probes: no
diff --git a/src/mailman/model/docs/pending.txt b/src/mailman/model/docs/pending.txt
index e85d8e484..707e8a7fc 100644
--- a/src/mailman/model/docs/pending.txt
+++ b/src/mailman/model/docs/pending.txt
@@ -7,11 +7,7 @@ are stored.  These can include email address registration events, held
 messages (but only for user confirmation), auto-approvals, and probe bounces.
 This is not where messages held for administrator approval are kept.
 
-    >>> from zope.interface import implements
-    >>> from zope.interface.verify import verifyObject
-
-In order to pend an event, you first need a pending database, which is
-available by adapting the list manager.
+In order to pend an event, you first need a pending database.
 
     >>> from mailman.interfaces.pending import IPendings
     >>> from zope.component import getUtility
@@ -20,6 +16,7 @@ available by adapting the list manager.
 The pending database can add any ``IPendable`` to the database, returning a
 token that can be used in urls and such.
 
+    >>> from zope.interface import implements
     >>> from mailman.interfaces.pending import IPendable
     >>> class SimplePendable(dict):
     ...     implements(IPendable)
@@ -35,24 +32,23 @@ token that can be used in urls and such.
 
 There's not much you can do with tokens except to `confirm` them, which
 basically means returning the ``IPendable`` structure (as a dictionary) from
-the database that matches the token.  If the token isn't in the database,
-``None`` is returned.
+the database that matches the token.  If the token isn't in the database, None
+is returned.
 
     >>> pendable = pendingdb.confirm(bytes('missing'))
     >>> print pendable
     None
     >>> pendable = pendingdb.confirm(token)
-    >>> sorted(pendable.items())
-    [(u'address', u'aperson@example.com'),
-     (u'language', u'en'),
-     (u'password', u'xyz'),
-     (u'realname', u'Anne Person'),
-     (u'type', u'subscription')]
+    >>> dump_msgdata(pendable)
+    address : aperson@example.com
+    language: en
+    password: xyz
+    realname: Anne Person
+    type    : subscription
 
 After confirmation, the token is no longer in the database.
 
-    >>> pendable = pendingdb.confirm(token)
-    >>> print pendable
+    >>> print pendingdb.confirm(token)
     None
 
 There are a few other things you can do with the pending database.  When you
@@ -66,13 +62,12 @@ expunge it.
     >>> event_3 = SimplePendable(type='three')
     >>> token_3 = pendingdb.add(event_3)
     >>> pendable = pendingdb.confirm(token_1, expunge=False)
-    >>> pendable.items()
-    [(u'type', u'one')]
+    >>> dump_msgdata(pendable)
+    type: one
     >>> pendable = pendingdb.confirm(token_1, expunge=True)
-    >>> pendable.items()
-    [(u'type', u'one')]
-    >>> pendable = pendingdb.confirm(token_1)
-    >>> print pendable
+    >>> dump_msgdata(pendable)
+    type: one
+    >>> print pendingdb.confirm(token_1)
     None
 
 An event can be given a lifetime when it is pended, otherwise it just uses a
@@ -86,9 +81,8 @@ default lifetime.
 Every once in a while the pending database is cleared of old records.
 
     >>> pendingdb.evict()
-    >>> pendable = pendingdb.confirm(token_4)
-    >>> print pendable
+    >>> print pendingdb.confirm(token_4)
     None
     >>> pendable = pendingdb.confirm(token_2)
-    >>> pendable.items()
-    [(u'type', u'two')]
+    >>> dump_msgdata(pendable)
+    type: two
diff --git a/src/mailman/queue/bounce.py b/src/mailman/queue/bounce.py
index d6b1576a2..35c27c6d6 100644
--- a/src/mailman/queue/bounce.py
+++ b/src/mailman/queue/bounce.py
@@ -26,7 +26,7 @@ import datetime
 from email.utils import parseaddr
 from lazr.config import as_timedelta
 
-from mailman.app.bounces import get_verp
+from mailman.app.bounces import StandardVERP
 from mailman.config import config
 from mailman.core.i18n import _
 from mailman.interfaces.bounce import Stop
@@ -190,7 +190,7 @@ class BounceRunner(Runner, BounceMixin):
         if not mlist.bounce_processing:
             return
         # Try VERP detection first, since it's quick and easy
-        addrs = get_verp(mlist, msg)
+        addrs = StandardVERP().get_verp(mlist, msg)
         if addrs:
             # We have an address, but check if the message is non-fatal.
             if scan_messages(mlist, msg) is Stop:
diff --git a/src/mailman/templates/en/probe.txt b/src/mailman/templates/en/probe.txt
index e0ae4ff57..98eaa310d 100644
--- a/src/mailman/templates/en/probe.txt
+++ b/src/mailman/templates/en/probe.txt
@@ -1,25 +1,20 @@
 This is a probe message.  You can ignore this message.
 
-The %(listname)s mailing list has received a number of bounces from you,
-indicating that there may be a problem delivering messages to %(address)s.
-A bounce sample is attached below.  Please examine this message to make sure
-there are no problems with your email address.  You may want to check with
-your mail administrator for more help.
-
-If you are reading this, you don't need to do anything to remain an enabled
-member of the mailing list.  If this message had bounced, you would not be
-reading it, and your membership would have been disabled.  Normally when you
-are disabled, you receive occasional messages asking you to re-enable your
-subscription.
+The $listname mailing list has received a number of bounces from you,
+indicating that there may be a problem delivering messages to $address.  A
+sample is attached below.  Please examine this message to make sure there are
+no problems with your email address.  You may want to check with your mail
+administrator for more help.
 
+You don't need to do anything to remain an enabled member of the mailing list.
 You can also visit your membership page at
 
-    %(optionsurl)s
+    $optionsurl
 
 On your membership page, you can change various delivery options such
 as your email address and whether you get digests or not.
 
-If you have any questions or problems, you can contact the list owner
+If you have any questions or problems, you can contact the mailing list owner
 at
 
-    %(owneraddr)s
+    $owneraddr
diff --git a/src/mailman/utilities/i18n.py b/src/mailman/utilities/i18n.py
index 8e769329c..000e74ac6 100644
--- a/src/mailman/utilities/i18n.py
+++ b/src/mailman/utilities/i18n.py
@@ -174,9 +174,8 @@ def make(template_file, mailing_list=None, language=None, wrap=True, **kw):
     :param wrap: When True, wrap the text.
     :type wrap: bool
     :param **kw: Keyword arguments for template interpolation.
-    :return: A tuple of the file system path to the first matching template,
-        and an open file object allowing reading of the file.
-    :rtype: (string, file)
+    :return: The interpolated text.
+    :rtype: string
     :raises TemplateNotFoundError: when the template could not be found.
     """
     path, fp = find(template_file, mailing_list, language)
diff --git a/src/mailman/utilities/uid.py b/src/mailman/utilities/uid.py
index e44ed2983..7ef50ace0 100644
--- a/src/mailman/utilities/uid.py
+++ b/src/mailman/utilities/uid.py
@@ -31,16 +31,14 @@ __all__ = [
 
 
 import os
-import time
 import errno
-import hashlib
 
 from flufl.lock import Lock
+from uuid import uuid4
 
 from mailman.config import config
 from mailman.model.uid import UID
 from mailman.testing import layers
-from mailman.utilities.passwords import SALT_LENGTH
 
 
 
@@ -69,7 +67,12 @@ class UniqueIDFactory:
             self._lockobj = Lock(self._lock_file)
         return self._lockobj
 
-    def new_uid(self, bytes=None):
+    def new_uid(self):
+        """Return a new UID.
+
+        :return: The new uid
+        :rtype: unicode
+        """
         if layers.is_testing():
             # When in testing mode we want to produce predictable id, but we
             # need to coordinate this among separate processes.  We could use
@@ -80,19 +83,14 @@ class UniqueIDFactory:
             # tests will be serialized enough (and the ids reset between
             # tests) that it will not be a problem.  Maybe.
             return self._next_uid()
-        salt = os.urandom(SALT_LENGTH)
         while True:
-            h = hashlib.sha1(repr(time.time()))
-            h.update(salt)
-            if bytes is not None:
-                h.update(bytes)
-                uid = unicode(h.hexdigest(), 'us-ascii')
-                try:
-                    UID.record(uid)
-                except ValueError:
-                    pass
-                else:
-                    return uid
+            uid = unicode(uuid4().hex, 'us-ascii')
+            try:
+                UID.record(uid)
+            except ValueError:
+                pass
+            else:
+                return uid
 
     def _next_uid(self):
         with self._lock: