Give the predictable ID factories similar APIs.

5 files changed, 54 insertions, 34 deletions

diff --git a/src/mailman/model/member.py b/src/mailman/model/member.py
index d21943919..811a7ec18 100644
--- a/src/mailman/model/member.py
+++ b/src/mailman/model/member.py
@@ -33,7 +33,7 @@ from mailman.interfaces.member import (
     IMember, MemberRole, MembershipError, UnsubscriptionEvent)
 from mailman.interfaces.user import IUser, UnverifiedAddressError
 from mailman.interfaces.usermanager import IUserManager
-from mailman.utilities.uid import UniqueIDFactory
+from mailman.utilities.uid import UIDFactory
 from sqlalchemy import Column, ForeignKey, Integer, Unicode
 from sqlalchemy.orm import relationship
 from zope.component import getUtility
@@ -41,7 +41,7 @@ from zope.event import notify
 from zope.interface import implementer
 
 
-uid_factory = UniqueIDFactory(context='members')
+uid_factory = UIDFactory(context='members')
 
 
 
@@ -65,7 +65,7 @@ class Member(Model):
     _user = relationship('User')
 
     def __init__(self, role, list_id, subscriber):
-        self._member_id = uid_factory.new_uid()
+        self._member_id = uid_factory.new()
         self.role = role
         self.list_id = list_id
         if IAddress.providedBy(subscriber):
diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py
index b8dbb1932..5b56f5418 100644
--- a/src/mailman/model/pending.py
+++ b/src/mailman/model/pending.py
@@ -90,7 +90,7 @@ class Pendings:
         if lifetime is None:
             lifetime = as_timedelta(config.mailman.pending_request_life)
         for attempts in range(3):
-            token = token_factory.new_token()
+            token = token_factory.new()
             # In practice, we'll never get a duplicate, but we'll be anal
             # about checking anyway.
             if store.query(Pended).filter_by(token=token).count() == 0:
diff --git a/src/mailman/model/uid.py b/src/mailman/model/uid.py
index 0ff22438c..40d137ee0 100644
--- a/src/mailman/model/uid.py
+++ b/src/mailman/model/uid.py
@@ -36,8 +36,8 @@ class UID(Model):
     This is used so that unique ids don't have to be tracked by each
     individual model object that uses them.  So for example, when a user is
     deleted, we don't have to keep separate track of its uid to prevent it
-    from ever being used again.  This class, hooked up to the
-    `UniqueIDFactory` serves that purpose.
+    from ever being used again.  This class, hooked up to the `UIDFactory`
+    serves that purpose.
 
     There is no interface for this class, because it's purely an internal
     implementation detail.
diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py
index f6aedd132..ebd6d611b 100644
--- a/src/mailman/model/user.py
+++ b/src/mailman/model/user.py
@@ -34,14 +34,14 @@ from mailman.model.address import Address
 from mailman.model.preferences import Preferences
 from mailman.model.roster import Memberships
 from mailman.utilities.datetime import factory as date_factory
-from mailman.utilities.uid import UniqueIDFactory
+from mailman.utilities.uid import UIDFactory
 from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer, Unicode
 from sqlalchemy.orm import relationship, backref
 from zope.event import notify
 from zope.interface import implementer
 
 
-uid_factory = UniqueIDFactory(context='users')
+uid_factory = UIDFactory(context='users')
 
 
 
@@ -80,7 +80,7 @@ class User(Model):
     def __init__(self, store, display_name=None, preferences=None):
         super(User, self).__init__()
         self._created_on = date_factory.now()
-        user_id = uid_factory.new_uid()
+        user_id = uid_factory.new()
         assert store.query(User).filter_by(_user_id=user_id).count() == 0, (
             'Duplicate user id {0}'.format(user_id))
         self._user_id = user_id
diff --git a/src/mailman/utilities/uid.py b/src/mailman/utilities/uid.py
index d20360ca0..a5bc8850e 100644
--- a/src/mailman/utilities/uid.py
+++ b/src/mailman/utilities/uid.py
@@ -22,8 +22,8 @@ and whatnot.  These are better instrumented for testing purposes.
 """
 
 __all__ = [
-    'UniqueIDFactory',
-    'factory',
+    'UIDFactory',
+    'TokenFactory',
     ]
 
 
@@ -41,8 +41,11 @@ from mailman.testing import layers
 
 
 
-class UniqueIDFactory:
-    """A factory for unique ids."""
+class _PredictableIDGenerator:
+    """
+    A base class factory for unique ids that can have predictable values in
+    testing mode.
+    """
 
     def __init__(self, context=None):
         # We can't call reset() when the factory is created below, because
@@ -66,12 +69,8 @@ class UniqueIDFactory:
             self._lockobj = Lock(self._lock_file)
         return self._lockobj
 
-    def new_uid(self):
-        """Return a new UID.
-
-        :return: The new uid
-        :rtype: int
-        """
+    def new(self):
+        """Return a new unique ID or a predictable one if in testing mode."""
         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
@@ -81,15 +80,12 @@ class UniqueIDFactory:
             # may still not be ideal due to race conditions, but I think the
             # tests will be serialized enough (and the ids reset between
             # tests) that it will not be a problem.  Maybe.
-            return uuid.UUID(int=self._next_uid())
-        while True:
-            uid = uuid.uuid4()
-            try:
-                UID.record(uid)
-            except ValueError:
-                pass
-            else:
-                return uid
+            return self._next_uid()
+        return self._generate()
+
+    def _generate(self):
+        """Generate a unique id when Mailman is not running in testing mode."""
+        raise NotImplementedError
 
     def _next_uid(self):
         with self._lock:
@@ -114,12 +110,36 @@ class UniqueIDFactory:
 
 
 
-class TokenFactory(UniqueIDFactory):
+class UIDFactory(_PredictableIDGenerator):
+    """A factory for unique ids."""
+
+    def _generate(self):
+        """Return a new UID.
+
+        :return: The new uid
+        :rtype: uuid.UUID
+        """
+        while True:
+            uid = uuid.uuid4()
+            try:
+                UID.record(uid)
+            except ValueError:
+                pass
+            else:
+                return uid
+
+    def _next_uid(self):
+        uid = super(UIDFactory, self)._next_uid()
+        return uuid.UUID(int=uid)
+
+
+
+class TokenFactory(_PredictableIDGenerator):
 
     def __init__(self):
         super(TokenFactory, self).__init__(context='token')
 
-    def new_token(self):
+    def _generate(self):
         """
         Calculate a unique token.  Algorithm vetted by the Timbot.  time()
         has high resolution on Linux, clock() on Windows.  random gives us
@@ -128,11 +148,11 @@ class TokenFactory(UniqueIDFactory):
         does the hash calculation.  The integral parts of the time values
         are discarded because they're the most predictable bits.
         """
-        if layers.is_testing():
-            # When in testing mode we want to produce predictable tokens, see
-            # UniqueIDFactory for a similar use case.
-            return str(self._next_uid()).zfill(40)
         right_now = time.time()
         x = random.random() + right_now % 1.0 + time.clock() % 1.0
         # Use sha1 because it produces shorter strings.
         return hashlib.sha1(repr(x).encode('utf-8')).hexdigest()
+
+    def _next_uid(self):
+        uid = super(TokenFactory, self)._next_uid()
+        return str(uid).zfill(40)