Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
f739cda9 by Aurélien Bompard at 2015-12-26T13:41:30Z
Return predictable token IDs in testing mode.
This commits builds upon the mailman.utilities.uid.UniqueIDFactory to
generate predictable tokens when the testing mode is activated. This
will make VCR tapes more diffable between runs.
- - - - -
9e3fb01c by Aurélien Bompard at 2015-12-26T18:04:25Z
Give the predictable ID factories similar APIs.
- - - - -
0e11f68b by Barry Warsaw at 2015-12-26T21:41:45Z
Tweak the API.
- - - - -
5 changed files:
- src/mailman/model/member.py
- src/mailman/model/pending.py
- src/mailman/model/uid.py
- src/mailman/model/user.py
- src/mailman/utilities/uid.py
Changes:
=====================================
src/mailman/model/member.py
=====================================
--- 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):
=====================================
src/mailman/model/pending.py
=====================================
--- a/src/mailman/model/pending.py
+++ b/src/mailman/model/pending.py
@@ -24,9 +24,6 @@ __all__ = [
import json
-import time
-import random
-import hashlib
from lazr.config import as_timedelta
from mailman.config import config
@@ -35,12 +32,16 @@ from mailman.database.transaction import dbconnection
from mailman.interfaces.pending import (
IPendable, IPended, IPendedKeyValue, IPendings)
from mailman.utilities.datetime import now
+from mailman.utilities.uid import TokenFactory
from sqlalchemy import Column, DateTime, ForeignKey, Integer, Unicode, and_
from sqlalchemy.orm import aliased, relationship
from zope.interface import implementer
from zope.interface.verify import verifyObject
+token_factory = TokenFactory()
+
+
@implementer(IPendedKeyValue)
class PendedKeyValue(Model):
@@ -88,17 +89,8 @@ class Pendings:
# Calculate the token and the lifetime.
if lifetime is None:
lifetime = as_timedelta(config.mailman.pending_request_life)
- # Calculate a unique token. Algorithm vetted by the Timbot. time()
- # has high resolution on Linux, clock() on Windows. random gives us
- # about 45 bits in Python 2.2, 53 bits on Python 2.3. The time and
- # clock values basically help obscure the random number generator, as
- # does the hash calculation. The integral parts of the time values
- # are discarded because they're the most predictable bits.
for attempts in range(3):
- right_now = time.time()
- x = random.random() + right_now % 1.0 + time.clock() % 1.0
- # Use sha1 because it produces shorter strings.
- token = hashlib.sha1(repr(x).encode('utf-8')).hexdigest()
+ 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:
=====================================
src/mailman/model/uid.py
=====================================
--- 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.
=====================================
src/mailman/model/user.py
=====================================
--- 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
=====================================
src/mailman/utilities/uid.py
=====================================
--- a/src/mailman/utilities/uid.py
+++ b/src/mailman/utilities/uid.py
@@ -22,14 +22,17 @@ and whatnot. These are better instrumented for testing
purposes.
"""
__all__ = [
- 'UniqueIDFactory',
- 'factory',
+ 'UIDFactory',
+ 'TokenFactory',
]
import os
+import time
import uuid
import errno
+import random
+import hashlib
from flufl.lock import Lock
from mailman.config import config
@@ -38,8 +41,12 @@ from mailman.testing import layers
-class UniqueIDFactory:
- """A factory for unique ids."""
+class _PredictableIDGenerator:
+ """Base class factory.
+
+ This factory provides a base class for unique ids that need to have
+ predictable values in testing mode.
+ """
def __init__(self, context=None):
# We can't call reset() when the factory is created below, because
@@ -63,14 +70,10 @@ 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
+ # When in testing mode we want to produce predictable ids, but we
# need to coordinate this among separate processes. We could use
# the database, but I don't want to add schema just to handle this
# case, and besides transactions could get aborted, causing some
@@ -78,17 +81,26 @@ 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 self._next_uid()
- while True:
- uid = uuid.uuid4()
- try:
- UID.record(uid)
- except ValueError:
- pass
- else:
- return uid
+ return self._next_predictable_id()
+ return self._next_unpredictable_id()
+
+ def _next_unpredictable_id(self):
+ """Generate a unique id when Mailman is not running in testing mode.
+
+ The type of the returned id is intended to be the type that
+ makes sense for the subclass overriding this method.
+ """
+ raise NotImplementedError
- def _next_uid(self):
+ def _next_predictable_id(self):
+ """Generate a predictable id for when Mailman being tested.
+
+ The type of the returned id is intended to be the type that
+ makes sense for the subclass overriding this method.
+ """
+ raise NotImplementedError
+
+ def _next_id(self):
with self._lock:
try:
with open(self._uid_file) as fp:
@@ -96,13 +108,13 @@ class UniqueIDFactory:
next_uid = uid + 1
with open(self._uid_file, 'w') as fp:
fp.write(str(next_uid))
- return uuid.UUID(int=uid)
+ return uid
except IOError as error:
if error.errno != errno.ENOENT:
raise
with open(self._uid_file, 'w') as fp:
fp.write('2')
- return uuid.UUID(int=1)
+ return 1
def reset(self):
with self._lock:
@@ -111,4 +123,50 @@ class UniqueIDFactory:
-factory = UniqueIDFactory()
+class UIDFactory(_PredictableIDGenerator):
+ """A factory for unique ids."""
+
+ def _next_unpredictable_id(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_predictable_id(self):
+ uid = super()._next_id()
+ return uuid.UUID(int=uid)
+
+
+
+class TokenFactory(_PredictableIDGenerator):
+
+ def __init__(self):
+ super().__init__(context='token')
+
+ def _next_unpredictable_id(self):
+ """Calculate a unique token.
+
+ Algorithm vetted by the Timbot. time() has high resolution on
+ Linux, clock() on Windows. random gives us about 45 bits in
+ Python 2.2, 53 bits on Python 2.3. The time and clock values
+ basically help obscure the random number generator, as does the
+ hash calculation. The integral parts of the time values are
+ discarded because they're the most predictable bits.
+ """
+ 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_predictable_id(self):
+ uid = super()._next_id()
+ return str(uid).zfill(40)
View it on GitLab:
https://gitlab.com/mailman/mailman/compare/a11e089cc1e0e5aff2502e584014295a414a43f9...0e11f68b74beef848255272a8010cad3ea96af91
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
