diff options
Diffstat (limited to 'src')
31 files changed, 388 insertions, 135 deletions
diff --git a/src/mailman/app/bounces.py b/src/mailman/app/bounces.py index 611ab48b9..bd8b465ba 100644 --- a/src/mailman/app/bounces.py +++ b/src/mailman/app/bounces.py @@ -30,6 +30,7 @@ __all__ = [ import re +import uuid import logging from email.mime.message import MIMEMessage @@ -168,7 +169,8 @@ class ProbeVERP(_BaseVERPParser): # The token must have already been confirmed, or it may have been # evicted from the database already. return None - member_id = pendable['member_id'] + # We had to pend the uuid as a unicode. + member_id = uuid.UUID(hex=pendable['member_id']) member = getUtility(ISubscriptionService).get_member(member_id) if member is None: return None @@ -200,7 +202,8 @@ def send_probe(member, msg): owneraddr=mlist.owner_address, ) pendable = _ProbePendable( - member_id=member.member_id, + # We can only pend unicodes. + member_id=member.member_id.hex, message_id=msg['message-id'], ) token = getUtility(IPendings).add(pendable) diff --git a/src/mailman/app/docs/lifecycle.txt b/src/mailman/app/docs/lifecycle.txt index b421d1800..4a8b732e1 100644 --- a/src/mailman/app/docs/lifecycle.txt +++ b/src/mailman/app/docs/lifecycle.txt @@ -27,7 +27,7 @@ bogus posting address, you get an exception. >>> create_list('not a valid address') Traceback (most recent call last): ... - InvalidEmailAddressError: u'not a valid address' + InvalidEmailAddressError: not a valid address If the posting address is valid, but the domain has not been registered with Mailman yet, you get an exception. diff --git a/src/mailman/app/docs/subscriptions.rst b/src/mailman/app/docs/subscriptions.rst index 8291132ce..f897d219e 100644 --- a/src/mailman/app/docs/subscriptions.rst +++ b/src/mailman/app/docs/subscriptions.rst @@ -17,10 +17,15 @@ membership role. At first, there are no memberships. [] >>> sum(1 for member in service) 0 - >>> print service.get_member(801) + >>> from uuid import UUID + >>> print service.get_member(UUID(int=801)) None -The service can be used to subscribe new members, but only with the `member` + +Adding new members +================== + +The service can be used to subscribe new members, by default with the `member` role. At a minimum, a mailing list and an address for the new user is required. @@ -37,14 +42,13 @@ The real name of the new member can be given. <Member: Bart Person <bart@example.com> on test@example.com as MemberRole.member> -Other roles can be subscribed using the more traditional interfaces. +Other roles can also be subscribed. >>> from mailman.interfaces.member import MemberRole - >>> from mailman.utilities.datetime import now - >>> address = list(anne.user.addresses)[0] - >>> address.verified_on = now() - >>> anne.user.preferred_address = address - >>> anne_owner = mlist.subscribe(anne.user, MemberRole.owner) + >>> anne_owner = service.join('test@example.com', 'anne@example.com', + ... role=MemberRole.owner) + >>> anne_owner + <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> And all the subscribed members can now be displayed. @@ -56,18 +60,61 @@ And all the subscribed members can now be displayed. as MemberRole.member>] >>> sum(1 for member in service) 3 - >>> print service.get_member(3) + >>> print service.get_member(UUID(int=3)) <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> +New members can also be added by providing an existing user id instead of an +email address. However, the user must have a preferred email address. +:: + + >>> service.join('test@example.com', bart.user.user_id, + ... role=MemberRole.owner) + Traceback (most recent call last): + ... + MissingPreferredAddressError: User must have a preferred address: + <User "Bart Person" (2) at ...> + + >>> from mailman.utilities.datetime import now + >>> address = list(bart.user.addresses)[0] + >>> address.verified_on = now() + >>> bart.user.preferred_address = address + >>> service.join('test@example.com', bart.user.user_id, + ... role=MemberRole.owner) + <Member: Bart Person <bart@example.com> + on test@example.com as MemberRole.owner> + + +Removing members +================ + Regular members can also be removed. - >>> service.leave('test@example.com', 'anne@example.com') + >>> cris = service.join('test@example.com', 'cris@example.com') >>> service.get_members() - [<Member: anne <anne@example.com> on test@example.com as MemberRole.owner>, + [<Member: anne <anne@example.com> on test@example.com + as MemberRole.owner>, + <Member: Bart Person <bart@example.com> on test@example.com + as MemberRole.owner>, + <Member: anne <anne@example.com> on test@example.com + as MemberRole.member>, + <Member: Bart Person <bart@example.com> on test@example.com + as MemberRole.member>, + <Member: cris <cris@example.com> on test@example.com + as MemberRole.member>] + >>> sum(1 for member in service) + 5 + >>> service.leave('test@example.com', 'cris@example.com') + >>> service.get_members() + [<Member: anne <anne@example.com> on test@example.com + as MemberRole.owner>, + <Member: Bart Person <bart@example.com> on test@example.com + as MemberRole.owner>, + <Member: anne <anne@example.com> on test@example.com + as MemberRole.member>, <Member: Bart Person <bart@example.com> on test@example.com as MemberRole.member>] >>> sum(1 for member in service) - 2 + 4 Finding members @@ -75,17 +122,18 @@ Finding members If you know the member id for a specific member, you can get that member. - >>> service.get_member(3) + >>> service.get_member(UUID(int=3)) <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> If you know the member's address, you can find all their memberships, based on -specific search criteria. -:: +specific search criteria. We start by subscribing Anne to a couple of new +mailing lists. >>> mlist2 = create_list('foo@example.com') >>> mlist3 = create_list('bar@example.com') - >>> mlist.subscribe(anne.user, MemberRole.member) - <Member: anne <anne@example.com> on test@example.com as MemberRole.member> + >>> address = list(anne.user.addresses)[0] + >>> address.verified_on = now() + >>> anne.user.preferred_address = address >>> mlist.subscribe(anne.user, MemberRole.moderator) <Member: anne <anne@example.com> on test@example.com as MemberRole.moderator> @@ -94,6 +142,8 @@ specific search criteria. >>> mlist3.subscribe(anne.user, MemberRole.owner) <Member: anne <anne@example.com> on bar@example.com as MemberRole.owner> +And now we can find all of Anne's memberships. + >>> service.find_members('anne@example.com') [<Member: anne <anne@example.com> on bar@example.com as MemberRole.owner>, <Member: anne <anne@example.com> on foo@example.com as MemberRole.member>, @@ -111,7 +161,7 @@ There may be no matching memberships. Memberships can also be searched for by user id. - >>> service.find_members(1) + >>> service.find_members(UUID(int=1)) [<Member: anne <anne@example.com> on bar@example.com as MemberRole.owner>, <Member: anne <anne@example.com> on foo@example.com as MemberRole.member>, <Member: anne <anne@example.com> on test@example.com @@ -130,7 +180,9 @@ You can find all the memberships for a specific mailing list. <Member: anne <anne@example.com> on test@example.com as MemberRole.moderator>, <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.member>] + as MemberRole.member>, + <Member: Bart Person <bart@example.com> on test@example.com + as MemberRole.owner>] You can find all the memberships for an address on a specific mailing list. diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index ee4aa1769..88c0751f0 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -35,16 +35,15 @@ from mailman.email.message import OwnerNotification from mailman.interfaces.address import IEmailValidator from mailman.interfaces.bans import IBanManager from mailman.interfaces.member import ( - AlreadySubscribedError, MemberRole, MembershipIsBannedError, - NotAMemberError) + MemberRole, MembershipIsBannedError, NotAMemberError) from mailman.interfaces.usermanager import IUserManager from mailman.utilities.i18n import make from mailman.utilities.passwords import encrypt_password -def add_member(mlist, email, realname, password, delivery_mode, language, - role=MemberRole.member): +def add_member(mlist, email, realname, password, delivery_mode, language, + role=MemberRole.member): """Add a member right now. The member's subscription must be approved by whatever policy the list @@ -62,6 +61,8 @@ def add_member(mlist, email, realname, password, delivery_mode, language, :type delivery_mode: DeliveryMode :param language: The language that the subscriber is going to use. :type language: str + :param role: The membership role for this subscription. + :type role: `MemberRole` :return: The just created member. :rtype: `IMember` :raises AlreadySubscribedError: if the user is already subscribed to @@ -115,13 +116,13 @@ def add_member(mlist, email, realname, password, delivery_mode, language, -def delete_member(mlist, address, admin_notif=None, userack=None): +def delete_member(mlist, email, admin_notif=None, userack=None): """Delete a member right now. - :param mlist: The mailing list to add the member to. + :param mlist: The mailing list to remove the member from. :type mlist: `IMailingList` - :param address: The address to subscribe. - :type address: string + :param email: The email address to unsubscribe. + :type email: string :param admin_notif: Whether the list administrator should be notified that this member was deleted. :type admin_notif: bool, or None to let the mailing list's @@ -134,23 +135,23 @@ def delete_member(mlist, address, admin_notif=None, userack=None): if admin_notif is None: admin_notif = mlist.admin_notify_mchanges # Delete a member, for which we know the approval has been made. - member = mlist.members.get_member(address) + member = mlist.members.get_member(email) if member is None: - raise NotAMemberError(mlist, address) + raise NotAMemberError(mlist, email) language = member.preferred_language member.unsubscribe() # And send an acknowledgement to the user... if userack: - send_goodbye_message(mlist, address, language) + send_goodbye_message(mlist, email, language) # ...and to the administrator. if admin_notif: - user = getUtility(IUserManager).get_user(address) + user = getUtility(IUserManager).get_user(email) realname = user.real_name subject = _('$mlist.real_name unsubscription notification') text = make('adminunsubscribeack.txt', mailing_list=mlist, listname=mlist.real_name, - member=formataddr((realname, address)), + member=formataddr((realname, email)), ) msg = OwnerNotification(mlist, subject, text, roster=mlist.administrators) diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 6f9399520..9eba89d8b 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -22,19 +22,20 @@ from __future__ import absolute_import, unicode_literals __metaclass__ = type __all__ = [ 'SubscriptionService', - 'handle_ListDeleteEvent', + 'handle_ListDeletedEvent', ] from operator import attrgetter from storm.expr import And, Or +from uuid import UUID from zope.component import getUtility from zope.interface import implements from mailman.app.membership import add_member, delete_member from mailman.config import config from mailman.core.constants import system_preferences -from mailman.interfaces.address import InvalidEmailAddressError +from mailman.interfaces.address import IEmailValidator from mailman.interfaces.listmanager import ( IListManager, ListDeletedEvent, NoSuchListError) from mailman.interfaces.member import DeliveryMode, MemberRole @@ -88,7 +89,7 @@ class SubscriptionService: """See `ISubscriptionService`.""" members = config.db.store.find( Member, - Member._member_id == unicode(member_id)) + Member._member_id == member_id) if members.count() == 0: return None else: @@ -118,7 +119,7 @@ class SubscriptionService: Member.user_id == user.id)) else: # subscriber is a user id. - user = user_manager.get_user_by_id(unicode(subscriber)) + user = user_manager.get_user_by_id(subscriber) address_ids = list(address.id for address in user.addresses if address.id is not None) if len(address_ids) == 0 or user is None: @@ -138,20 +139,22 @@ class SubscriptionService: for member in self.get_members(): yield member - def join(self, fqdn_listname, subscriber, real_name= None, - delivery_mode=DeliveryMode.regular, role=MemberRole.member): + def join(self, fqdn_listname, subscriber, + real_name=None, + delivery_mode=DeliveryMode.regular, + role=MemberRole.member): """See `ISubscriptionService`.""" mlist = getUtility(IListManager).get(fqdn_listname) if mlist is None: raise NoSuchListError(fqdn_listname) - # Is the subscriber a user or email address? - if '@' in subscriber: - # It's an email address, so we'll want a real name. + # Is the subscriber an email address or user id? + if isinstance(subscriber, basestring): + # It's an email address, so we'll want a real name. Make sure + # it's a valid email address, and let InvalidEmailAddressError + # propagate up. + getUtility(IEmailValidator).validate(subscriber) if real_name is None: real_name, at, domain = subscriber.partition('@') - if len(at) == 0: - # It can't possibly be a valid email address. - raise InvalidEmailAddressError(subscriber) # Because we want to keep the REST API simple, there is no # password or language given to us. We'll use the system's # default language for the user's default language. We'll set the @@ -159,23 +162,24 @@ class SubscriptionService: # it can't be retrieved. Note that none of these are used unless # the address is completely new to us. password = make_user_friendly_password() - return add_member(mlist, subscriber, real_name, password, - delivery_mode, + return add_member(mlist, subscriber, real_name, password, + delivery_mode, system_preferences.preferred_language, role) else: - # We have to assume it's a user id. + # We have to assume it's a UUID. + assert isinstance(subscriber, UUID), 'Not a UUID' user = getUtility(IUserManager).get_user_by_id(subscriber) if user is None: raise MissingUserError(subscriber) return mlist.subscribe(user, role) - def leave(self, fqdn_listname, address): + def leave(self, fqdn_listname, email): """See `ISubscriptionService`.""" mlist = getUtility(IListManager).get(fqdn_listname) if mlist is None: raise NoSuchListError(fqdn_listname) - # XXX for now, no notification or user acknowledgement. - delete_member(mlist, address, False, False) + # XXX for now, no notification or user acknowledgment. + delete_member(mlist, email, False, False) diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py index 954b5fc05..3a39b756a 100644 --- a/src/mailman/app/tests/test_bounces.py +++ b/src/mailman/app/tests/test_bounces.py @@ -26,6 +26,7 @@ __all__ = [ import os +import uuid import shutil import tempfile import unittest @@ -211,7 +212,9 @@ Message-ID: <first> 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) + # member_ids are pended as unicodes. + self.assertEqual(uuid.UUID(hex=pendable['member_id']), + self._member.member_id) self.assertEqual(pendable['message_id'], '<first>') def test_probe_is_multipart(self): diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py index df2f73134..0e8225f3a 100644 --- a/src/mailman/app/tests/test_membership.py +++ b/src/mailman/app/tests/test_membership.py @@ -34,8 +34,8 @@ from mailman.app.membership import add_member from mailman.config import config from mailman.core.constants import system_preferences from mailman.interfaces.bans import IBanManager -from mailman.interfaces.member import (DeliveryMode, MembershipIsBannedError, - MemberRole) +from mailman.interfaces.member import ( + AlreadySubscribedError, DeliveryMode, MemberRole, MembershipIsBannedError) from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import reset_the_world from mailman.testing.layers import ConfigLayer @@ -127,7 +127,7 @@ class AddMemberTest(unittest.TestCase): self.assertEqual(member.address.email, 'anne@example.com') def test_add_member_moderator(self): - # Test adding a moderator to a mailing list + # Test adding a moderator to a mailing list. member = add_member(self._mlist, 'aperson@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language, @@ -135,6 +135,42 @@ class AddMemberTest(unittest.TestCase): self.assertEqual(member.address.email, 'aperson@example.com') self.assertEqual(member.mailing_list, 'test@example.com') self.assertEqual(member.role, MemberRole.moderator) + + def test_add_member_twice(self): + # Adding a member with the same role twice causes an + # AlreadySubscribedError to be raised. + add_member(self._mlist, 'aperson@example.com', + 'Anne Person', '123', DeliveryMode.regular, + system_preferences.preferred_language, + MemberRole.member) + try: + add_member(self._mlist, 'aperson@example.com', + 'Anne Person', '123', DeliveryMode.regular, + system_preferences.preferred_language, + MemberRole.member) + except AlreadySubscribedError as exc: + self.assertEqual(exc.fqdn_listname, 'test@example.com') + self.assertEqual(exc.email, 'aperson@example.com') + self.assertEqual(exc.role, MemberRole.member) + else: + raise AssertionError('AlreadySubscribedError expected') + + def test_add_member_with_different_roles(self): + # Adding a member twice with different roles is okay. + member_1 = add_member(self._mlist, 'aperson@example.com', + 'Anne Person', '123', DeliveryMode.regular, + system_preferences.preferred_language, + MemberRole.member) + member_2 = add_member(self._mlist, 'aperson@example.com', + 'Anne Person', '123', DeliveryMode.regular, + system_preferences.preferred_language, + MemberRole.owner) + self.assertEqual(member_1.mailing_list, member_2.mailing_list) + self.assertEqual(member_1.address, member_2.address) + self.assertEqual(member_1.user, member_2.user) + self.assertNotEqual(member_1.member_id, member_2.member_id) + self.assertEqual(member_1.role, MemberRole.member) + self.assertEqual(member_2.role, MemberRole.owner) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py new file mode 100644 index 000000000..861551a03 --- /dev/null +++ b/src/mailman/app/tests/test_subscriptions.py @@ -0,0 +1,76 @@ +# Copyright (C) 2011 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""Tests for the subscription service.""" + +from __future__ import absolute_import, unicode_literals + +__metaclass__ = type +__all__ = [ + 'test_suite', + ] + + +import uuid +import unittest + +from zope.component import getUtility + +from mailman.app.lifecycle import create_list +from mailman.interfaces.address import InvalidEmailAddressError +from mailman.interfaces.subscriptions import ( + MissingUserError, ISubscriptionService) +from mailman.testing.helpers import reset_the_world +from mailman.testing.layers import ConfigLayer + + + +class TestJoin(unittest.TestCase): + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + self._service = getUtility(ISubscriptionService) + + def tearDown(self): + reset_the_world() + + def test_join_user_with_bogus_id(self): + # When `subscriber` is a missing user id, an exception is raised. + try: + self._service.join('test@example.com', uuid.UUID(int=99)) + except MissingUserError as exc: + self.assertEqual(exc.user_id, uuid.UUID(int=99)) + else: + raise AssertionError('MissingUserError expected') + + def test_join_user_with_invalid_email_address(self): + # When `subscriber` is a string that is not an email address, an + # exception is raised. + try: + self._service.join('test@example.com', 'bogus') + except InvalidEmailAddressError as exc: + self.assertEqual(exc.address, 'bogus') + else: + raise AssertionError('InvalidEmailAddressError expected') + + + +def test_suite(): + suite = unittest.TestSuite() + suite.addTest(unittest.makeSuite(TestJoin)) + return suite diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 402e194a1..54b05f028 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -29,6 +29,7 @@ Architecture deleted, as well as all held message requests (but not the held messages themselves). (LP: 827036) * IDomain.email_host -> .mail_host (LP: #831660) + * User and Member ids are now proper UUIDs. REST ---- @@ -43,6 +44,8 @@ REST * Fixed incorrect error code for /members/<bogus> (LP: #821020). Given by Stephen A. Goss. * DELETE users via the REST API. (LP: #820660) + * Moderators and owners can be added via REST (LP: #834130). Given by + Stephen A. Goss. Commands -------- @@ -65,12 +68,13 @@ Testing * Handle SIGTERM in the REST server so that the test suite always shuts down correctly. (LP: #770328) -Other bugs ----------- +Other bugs and changes +---------------------- * Moderating a message with Action.accept now sends the message. (LP: #827697) * Fix AttributeError triggered by i18n call in autorespond_to_sender() (LP: #827060) * Local timezone in X-Mailman-Approved-At caused test failure. (LP: #832404) + * InvalidEmailAddressError no longer repr()'s its value. 3.0 alpha 7 -- "Mission" diff --git a/src/mailman/email/validate.py b/src/mailman/email/validate.py index 97b4b9e5f..e21bb1de5 100644 --- a/src/mailman/email/validate.py +++ b/src/mailman/email/validate.py @@ -66,4 +66,4 @@ class Validator: :raise InvalidEmailAddressError: when the address is deemed invalid. """ if not self.is_valid(email): - raise InvalidEmailAddressError(repr(email)) + raise InvalidEmailAddressError(email) diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 5470f0b5f..9ae779409 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -245,7 +245,10 @@ class IMailingList(Interface): :return: The member object representing the subscription. :rtype: `IMember` :raises AlreadySubscribedError: If the address or user is already - subscribed to the mailing list with the given role. + subscribed to the mailing list with the given role. Note however + that it is possible to subscribe an address to a mailing list with + a particular role, and also subscribe a user with a matching + preferred address that is explicitly subscribed with the same role. """ # Posting history. diff --git a/src/mailman/interfaces/member.py b/src/mailman/interfaces/member.py index 0676bfe78..0a8a5cfd5 100644 --- a/src/mailman/interfaces/member.py +++ b/src/mailman/interfaces/member.py @@ -81,15 +81,15 @@ class MembershipError(MailmanError): class AlreadySubscribedError(MembershipError): """The member is already subscribed to the mailing list with this role.""" - def __init__(self, fqdn_listname, address, role): + def __init__(self, fqdn_listname, email, role): super(AlreadySubscribedError, self).__init__() - self._fqdn_listname = fqdn_listname - self._address = address - self._role = role + self.fqdn_listname = fqdn_listname + self.email = email + self.role = role def __str__(self): return '{0} is already a {1} of mailing list {2}'.format( - self._address, self._role, self._fqdn_listname) + self.email, self.role, self.fqdn_listname) class MembershipIsBannedError(MembershipError): @@ -134,7 +134,7 @@ class IMember(Interface): """A member of a mailing list.""" member_id = Attribute( - """The member's unique, random identifier (sha1 hex digest).""") + """The member's unique, random identifier as a UUID.""") mailing_list = Attribute( """The mailing list subscribed to.""") diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index a10b7dab8..006249922 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -92,8 +92,9 @@ class ISubscriptionService(Interface): def __iter__(): """See `get_members()`.""" - def join(fqdn_listname, subscriber, real_name=None, - delivery_mode=DeliveryMode.regular, role=MemberRole.member): + def join(fqdn_listname, subscriber, real_name=None, + delivery_mode=DeliveryMode.regular, + role=MemberRole.member): """Subscribe to a mailing list. A user for the address is created if it is not yet known to Mailman, @@ -107,7 +108,7 @@ class ISubscriptionService(Interface): :type fqdn_listname: string :param subscriber: The email address or user id of the user getting subscribed. - :type subscriber: string + :type subscriber: string or int :param real_name: The name of the user. This is only used if a new user is created, and it defaults to the local part of the email address if not given. @@ -116,6 +117,8 @@ class ISubscriptionService(Interface): can be one of the enum values of `DeliveryMode`. If not given, regular delivery is assumed. :type delivery_mode: string + :param role: The membership role for this subscription. + :type role: `MemberRole` :return: The just created member. :rtype: `IMember` :raises AlreadySubscribedError: if the user is already subscribed to @@ -127,14 +130,14 @@ class ISubscriptionService(Interface): :raises ValueError: when `delivery_mode` is invalid. """ - def leave(fqdn_listname, address): + def leave(fqdn_listname, email): """Unsubscribe from a mailing list. :param fqdn_listname: The posting address of the mailing list to - subscribe the user to. + unsubscribe the user from. :type fqdn_listname: string - :param address: The address of the user getting subscribed. - :type address: string + :param email: The email address of the user getting unsubscribed. + :type email: string :raises InvalidEmailAddressError: if the email address is not valid. :raises NoSuchListError: if the named mailing list does not exist. :raises NotAMemberError: if the given address is not a member of the diff --git a/src/mailman/interfaces/user.py b/src/mailman/interfaces/user.py index 46dd3ed63..16dc1b0ea 100644 --- a/src/mailman/interfaces/user.py +++ b/src/mailman/interfaces/user.py @@ -54,7 +54,7 @@ class IUser(Interface): """This user's password information.""") user_id = Attribute( - """The user's unique, random identifier (sha1 hex digest).""") + """The user's unique, random identifier as a UUID.""") created_on = Attribute( """The date and time at which this user was created.""") diff --git a/src/mailman/interfaces/usermanager.py b/src/mailman/interfaces/usermanager.py index 59895af7b..c6486dfaf 100644 --- a/src/mailman/interfaces/usermanager.py +++ b/src/mailman/interfaces/usermanager.py @@ -66,7 +66,7 @@ class IUserManager(Interface): """Get the user associated with the given id. :param user_id: The user id. - :type user_id: unicode + :type user_id: `uuid.UUID` :return: The user found or None. :rtype: `IUser`. """ diff --git a/src/mailman/model/docs/membership.txt b/src/mailman/model/docs/membership.rst index 8435e8097..f070b4d40 100644 --- a/src/mailman/model/docs/membership.txt +++ b/src/mailman/model/docs/membership.rst @@ -282,8 +282,8 @@ though that the address their changing to must be verified. >>> gwen = user_manager.create_user('gwen@example.com') >>> gwen_address = list(gwen.addresses)[0] >>> gwen_member = bee.subscribe(gwen_address) - >>> for member in bee.members.members: - ... print member.member_id, member.mailing_list, member.address.email + >>> for m in bee.members.members: + ... print m.member_id.int, m.mailing_list, m.address.email 7 bee@example.com gwen@example.com Gwen gets a email address. @@ -300,8 +300,8 @@ address, but the address is not yet verified. Her membership has not changed. - >>> for member in bee.members.members: - ... print member.member_id, member.mailing_list, member.address.email + >>> for m in bee.members.members: + ... print m.member_id.int, m.mailing_list, m.address.email 7 bee@example.com gwen@example.com Gwen verifies her email address, and updates her membership. @@ -312,6 +312,6 @@ Gwen verifies her email address, and updates her membership. Now her membership reflects the new address. - >>> for member in bee.members.members: - ... print member.member_id, member.mailing_list, member.address.email + >>> for m in bee.members.members: + ... print m.member_id.int, m.mailing_list, m.address.email 7 bee@example.com gperson@example.com diff --git a/src/mailman/model/docs/registration.txt b/src/mailman/model/docs/registration.txt index 0e80bfa14..9605fcbea 100644 --- a/src/mailman/model/docs/registration.txt +++ b/src/mailman/model/docs/registration.txt @@ -49,27 +49,27 @@ addresses are rejected outright. >>> registrar.register(mlist, '') Traceback (most recent call last): ... - InvalidEmailAddressError: u'' + InvalidEmailAddressError >>> registrar.register(mlist, 'some name@example.com') Traceback (most recent call last): ... - InvalidEmailAddressError: u'some name@example.com' + InvalidEmailAddressError: some name@example.com >>> registrar.register(mlist, '<script>@example.com') Traceback (most recent call last): ... - InvalidEmailAddressError: u'<script>@example.com' + InvalidEmailAddressError: <script>@example.com >>> registrar.register(mlist, '\xa0@example.com') Traceback (most recent call last): ... - InvalidEmailAddressError: u'\xa0@example.com' + InvalidEmailAddressError: \xa0@example.com >>> registrar.register(mlist, 'noatsign') Traceback (most recent call last): ... - InvalidEmailAddressError: u'noatsign' + InvalidEmailAddressError: noatsign >>> registrar.register(mlist, 'nodom@ain') Traceback (most recent call last): ... - InvalidEmailAddressError: u'nodom@ain' + InvalidEmailAddressError: nodom@ain Register an email address diff --git a/src/mailman/model/docs/usermanager.txt b/src/mailman/model/docs/usermanager.rst index e427eb63a..12528bedf 100644 --- a/src/mailman/model/docs/usermanager.txt +++ b/src/mailman/model/docs/usermanager.rst @@ -143,5 +143,6 @@ Users can also be found by their unique user id. If a non-existent user id is given, None is returned. - >>> print user_manager.get_user_by_id('missing') + >>> from uuid import UUID + >>> print user_manager.get_user_by_id(UUID(int=801)) None diff --git a/src/mailman/model/docs/users.txt b/src/mailman/model/docs/users.rst index 1813ef636..6d0c9a5b0 100644 --- a/src/mailman/model/docs/users.txt +++ b/src/mailman/model/docs/users.rst @@ -41,12 +41,11 @@ Basic user identification ========================= Although rarely visible to users, every user has a unique ID in Mailman, which -never changes. This ID is generated randomly at the time the user is -created. +never changes. This ID is generated randomly at the time the user is created, +and is represented by a UUID. - # The test suite uses a predictable user id. >>> print user_1.user_id - 1 + 00000000-0000-0000-0000-000000000001 The user id cannot change. diff --git a/src/mailman/model/member.py b/src/mailman/model/member.py index 095e6fae7..c7e9713dd 100644 --- a/src/mailman/model/member.py +++ b/src/mailman/model/member.py @@ -25,6 +25,7 @@ __all__ = [ ] from storm.locals import Int, Reference, Unicode +from storm.properties import UUID from zope.component import getUtility from zope.interface import implements @@ -49,7 +50,7 @@ class Member(Model): implements(IMember) id = Int(primary=True) - _member_id = Unicode() + _member_id = UUID() role = Enum() mailing_list = Unicode() moderation_action = Enum() diff --git a/src/mailman/model/tests/test_uid.py b/src/mailman/model/tests/test_uid.py index 6f5cde4d2..c8ba4b770 100644 --- a/src/mailman/model/tests/test_uid.py +++ b/src/mailman/model/tests/test_uid.py @@ -25,6 +25,7 @@ __all__ = [ ] +import uuid import unittest from mailman.model.uid import UID @@ -36,9 +37,16 @@ class TestUID(unittest.TestCase): layer = ConfigLayer def test_record(self): - UID.record('abc') - UID.record('def') - self.assertRaises(ValueError, UID.record, 'abc') + # Test that the .record() method works. + UID.record(uuid.UUID(int=11)) + UID.record(uuid.UUID(int=99)) + self.assertRaises(ValueError, UID.record, uuid.UUID(int=11)) + + def test_longs(self): + # In a non-test environment, the uuid will be a long int. + my_uuid = uuid.uuid4() + UID.record(my_uuid) + self.assertRaises(ValueError, UID.record, my_uuid) diff --git a/src/mailman/model/uid.py b/src/mailman/model/uid.py index 5b8f027d6..df99f497a 100644 --- a/src/mailman/model/uid.py +++ b/src/mailman/model/uid.py @@ -25,7 +25,8 @@ __all__ = [ ] -from storm.locals import Int, Unicode +from storm.locals import Int +from storm.properties import UUID from mailman.config import config from mailman.database.model import Model @@ -43,12 +44,9 @@ class UID(Model): There is no interface for this class, because it's purely an internal implementation detail. - - Since it's a hash, it's overwhelmingly more common for a uid to be - unique. """ id = Int(primary=True) - uid = Unicode() + uid = UUID() def __init__(self, uid): super(UID, self).__init__() diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py index a29837bb4..53c96c3a2 100644 --- a/src/mailman/model/user.py +++ b/src/mailman/model/user.py @@ -26,6 +26,7 @@ __all__ = [ from storm.locals import ( DateTime, Int, RawStr, Reference, ReferenceSet, Unicode) +from storm.properties import UUID from zope.interface import implements from mailman.config import config @@ -52,7 +53,7 @@ class User(Model): id = Int(primary=True) real_name = Unicode() password = RawStr() - _user_id = Unicode() + _user_id = UUID() _created_on = DateTime() addresses = ReferenceSet(id, 'Address.user_id') @@ -73,8 +74,9 @@ class User(Model): config.db.store.add(self) def __repr__(self): - return '<User "{0.real_name}" ({0.user_id}) at {1:#x}>'.format( - self, id(self)) + short_user_id = self.user_id.int + return '<User "{0.real_name}" ({2}) at {1:#x}>'.format( + self, id(self), short_user_id) @property def user_id(self): diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py index d6817021d..fc1830b70 100644 --- a/src/mailman/model/usermanager.py +++ b/src/mailman/model/usermanager.py @@ -40,6 +40,7 @@ class UserManager: implements(IUserManager) def create_user(self, email=None, real_name=None): + """See `IUserManager`.""" user = User(real_name, Preferences()) if email: address = self.create_address(email, real_name) @@ -47,15 +48,18 @@ class UserManager: return user def delete_user(self, user): + """See `IUserManager`.""" config.db.store.remove(user) def get_user(self, email): + """See `IUserManager`.""" addresses = config.db.store.find(Address, email=email.lower()) if addresses.count() == 0: return None return addresses.one().user def get_user_by_id(self, user_id): + """See `IUserManager`.""" users = config.db.store.find(User, _user_id=user_id) if users.count() == 0: return None @@ -63,10 +67,12 @@ class UserManager: @property def users(self): + """See `IUserManager`.""" for user in config.db.store.find(User): yield user def create_address(self, email, real_name=None): + """See `IUserManager`.""" addresses = config.db.store.find(Address, email=email.lower()) if addresses.count() == 1: found = addresses[0] @@ -82,6 +88,7 @@ class UserManager: return address def delete_address(self, address): + """See `IUserManager`.""" # If there's a user controlling this address, it has to first be # unlinked before the address can be deleted. if address.user: @@ -89,6 +96,7 @@ class UserManager: config.db.store.remove(address) def get_address(self, email): + """See `IUserManager`.""" addresses = config.db.store.find(Address, email=email.lower()) if addresses.count() == 0: return None @@ -96,5 +104,6 @@ class UserManager: @property def addresses(self): + """See `IUserManager`.""" for address in config.db.store.find(Address): yield address diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index 974ba6517..c09cd10a1 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -478,6 +478,7 @@ Elly is now a known user, and a member of the mailing list. Gwen is a user with a preferred address. She subscribes to the `ant` mailing list with her preferred address. +:: >>> from mailman.utilities.datetime import now >>> gwen = user_manager.create_user('gwen@example.com', 'Gwen Person') @@ -487,8 +488,10 @@ list with her preferred address. # Note that we must extract the user id before we commit the transaction. # This is because accessing the .user_id attribute will lock the database - # in the testing process, breaking the REST queue process. - >>> user_id = gwen.user_id + # in the testing process, breaking the REST queue process. Also, the + # user_id is a UUID internally, but an integer (represented as a string) + # is required by the REST API. + >>> user_id = gwen.user_id.int >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/members', { diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst index 4f73e12e7..43df35b94 100644 --- a/src/mailman/rest/docs/users.rst +++ b/src/mailman/rest/docs/users.rst @@ -37,7 +37,7 @@ When there are users in the database, they can be retrieved as a collection. The user ids match. >>> json = call_http('http://localhost:9001/3.0/users') - >>> json['entries'][0]['user_id'] == anne.user_id + >>> json['entries'][0]['user_id'] == anne.user_id.int True A user might not have a real name, in which case, the attribute will not be diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index dd1e01d56..e7d0a095f 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -28,6 +28,7 @@ __all__ = [ ] +from uuid import UUID from operator import attrgetter from restish import http, resource from zope.component import getUtility @@ -43,7 +44,8 @@ from mailman.interfaces.user import UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( CollectionMixin, PATCH, etag, no_content, path_to) -from mailman.rest.validator import Validator, enum_validator +from mailman.rest.validator import ( + Validator, enum_validator, subscriber_validator) @@ -53,12 +55,16 @@ class _MemberBase(resource.Resource, CollectionMixin): def _resource_as_dict(self, member): """See `CollectionMixin`.""" enum, dot, role = str(member.role).partition('.') + # Both the user_id and the member_id are UUIDs. We need to use the + # integer equivalent in the URL. + user_id = member.user.user_id.int + member_id = member.member_id.int return dict( fqdn_listname=member.mailing_list, address=member.address.email, role=role, - user=path_to('users/{0}'.format(member.user.user_id)), - self_link=path_to('members/{0}'.format(member.member_id)), + user=path_to('users/{0}'.format(user_id)), + self_link=path_to('members/{0}'.format(member_id)), ) def _get_collection(self, request): @@ -89,9 +95,17 @@ class MemberCollection(_MemberBase): class AMember(_MemberBase): """A member.""" - def __init__(self, member_id): - self._member_id = member_id - self._member = getUtility(ISubscriptionService).get_member(member_id) + def __init__(self, member_id_string): + # REST gives us the member id as the string of an int; we have to + # convert it to a UUID. + try: + member_id = UUID(int=int(member_id_string)) + except ValueError: + # The string argument could not be converted to an integer. + self._member = None + else: + service = getUtility(ISubscriptionService) + self._member = service.get_member(member_id) @resource.GET() def member(self, request): @@ -124,6 +138,8 @@ class AMember(_MemberBase): This is how subscription changes are done. """ + if self._member is None: + return http.not_found() # Currently, only the `address` parameter can be patched. values = Validator(address=unicode)(request) assert len(values) == 1, 'Unexpected values' @@ -147,12 +163,13 @@ class AllMembers(_MemberBase): """Create a new member.""" service = getUtility(ISubscriptionService) try: - validator = Validator(fqdn_listname=unicode, - subscriber=unicode, - real_name=unicode, - delivery_mode=enum_validator(DeliveryMode), - role=enum_validator(MemberRole), - _optional=('delivery_mode', 'real_name', 'role')) + validator = Validator( + fqdn_listname=unicode, + subscriber=subscriber_validator, + real_name=unicode, + delivery_mode=enum_validator(DeliveryMode), + role=enum_validator(MemberRole), + _optional=('delivery_mode', 'real_name', 'role')) member = service.join(**validator(request)) except AlreadySubscribedError: return http.conflict([], b'Member already subscribed') @@ -162,7 +179,10 @@ class AllMembers(_MemberBase): return http.bad_request([], b'Invalid email address') except ValueError as error: return http.bad_request([], str(error)) - location = path_to('members/{0}'.format(member.member_id)) + # The member_id are UUIDs. We need to use the integer equivalent in + # the URL. + member_id = member.member_id.int + location = path_to('members/{0}'.format(member_id)) # Include no extra headers or body. return http.created(location, [], None) diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index 29a18a45f..3fdf5d7e8 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -27,6 +27,7 @@ __all__ = [ from restish import http, resource +from uuid import UUID from zope.component import getUtility from mailman.interfaces.address import ExistingAddressError @@ -46,11 +47,13 @@ class _UserBase(resource.Resource, CollectionMixin): """See `CollectionMixin`.""" # The canonical URL for a user is their unique user id, although we # can always look up a user based on any registered and validated - # email address associated with their account. + # email address associated with their account. The user id is a UUID, + # but we serialize its integer equivalent. + user_id = user.user_id.int resource = dict( - user_id=user.user_id, + user_id=user_id, created_on=user.created_on, - self_link=path_to('users/{0}'.format(user.user_id)), + self_link=path_to('users/{0}'.format(user_id)), ) # Add the password attribute, only if the user has a password. Same # with the real name. These could be None or the empty string. @@ -99,7 +102,7 @@ class AllUsers(_UserBase): # This will have to be reset since it cannot be retrieved. password = make_user_friendly_password() user.password = encrypt_password(password) - location = path_to('users/{0}'.format(user.user_id)) + location = path_to('users/{0}'.format(user.user_id.int)) return http.created(location, [], None) @@ -115,13 +118,20 @@ class AUser(_UserBase): controlled by the user. The type of identifier is auto-detected by looking for an `@` symbol, in which case it's taken as an email address, otherwise it's assumed to be an integer. - :type user_identifier: str + :type user_identifier: string """ user_manager = getUtility(IUserManager) if '@' in user_identifier: self._user = user_manager.get_user(user_identifier) else: - self._user = user_manager.get_user_by_id(user_identifier) + # The identifier is the string representation of an integer that + # must be converted to a UUID. + try: + user_id = UUID(int=int(user_identifier)) + except ValueError: + self._user = None + else: + self._user = user_manager.get_user_by_id(user_id) @resource.GET() def user(self, request): diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index d8297e519..cf32ca838 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -23,9 +23,13 @@ __metaclass__ = type __all__ = [ 'Validator', 'enum_validator', + 'subscriber_validator', ] +from uuid import UUID + + COMMASPACE = ', ' @@ -42,6 +46,17 @@ class enum_validator: return self._enum_class[enum_value] + + +def subscriber_validator(subscriber): + """Convert an email-or-int to an email-or-UUID.""" + try: + return UUID(int=int(subscriber)) + except ValueError: + return subscriber + + + class Validator: """A validator of parameter input.""" diff --git a/src/mailman/runners/outgoing.py b/src/mailman/runners/outgoing.py index e771d8be3..1ce668391 100644 --- a/src/mailman/runners/outgoing.py +++ b/src/mailman/runners/outgoing.py @@ -22,6 +22,7 @@ import logging from datetime import datetime from lazr.config import as_boolean, as_timedelta +from uuid import UUID from zope.component import getUtility from mailman.config import config @@ -122,8 +123,9 @@ class OutgoingRunner(Runner): # It's possible the token has been confirmed out of the # database. Just ignore that. if pended is not None: + # The UUID had to be pended as a unicode. member = getUtility(ISubscriptionService).get_member( - pended['member_id']) + UUID(hex=pended['member_id'])) processor.register( mlist, member.address.email, msg, BounceContext.probe) diff --git a/src/mailman/utilities/uid.py b/src/mailman/utilities/uid.py index 7ef50ace0..dfbff0ae6 100644 --- a/src/mailman/utilities/uid.py +++ b/src/mailman/utilities/uid.py @@ -31,10 +31,10 @@ __all__ = [ import os +import uuid import errno from flufl.lock import Lock -from uuid import uuid4 from mailman.config import config from mailman.model.uid import UID @@ -71,7 +71,7 @@ class UniqueIDFactory: """Return a new UID. :return: The new uid - :rtype: unicode + :rtype: int """ if layers.is_testing(): # When in testing mode we want to produce predictable id, but we @@ -84,7 +84,7 @@ class UniqueIDFactory: # tests) that it will not be a problem. Maybe. return self._next_uid() while True: - uid = unicode(uuid4().hex, 'us-ascii') + uid = uuid.uuid4() try: UID.record(uid) except ValueError: @@ -96,17 +96,17 @@ class UniqueIDFactory: with self._lock: try: with open(self._uid_file) as fp: - uid = fp.read().strip() - next_uid = int(uid) + 1 + uid = int(fp.read().strip()) + next_uid = uid + 1 with open(self._uid_file, 'w') as fp: fp.write(str(next_uid)) + return uuid.UUID(int=uid) except IOError as error: if error.errno != errno.ENOENT: raise with open(self._uid_file, 'w') as fp: fp.write('2') - return '1' - return unicode(uid, 'us-ascii') + return uuid.UUID(int=1) def reset(self): with self._lock: |