Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
ad7c5f7c by Barry Warsaw at 2016-02-06T17:15:41-05:00
Add a set_preferred() helper.
This refactors the setting of a user's preferred address to the first in
their list of linked addresses.
- - - - -
d8710619 by Barry Warsaw at 2016-02-06T18:00:06-05:00
Fix membership query for preferred addresses.
When multiple users are subscribed to a mailing list via their preferred
address, too many results are returned from a membership query,
resulting in an assertion error. Reported by Darrell Kresge.
(Closes: #190)
- - - - -
11 changed files:
- src/mailman/app/tests/test_subscriptions.py
- src/mailman/docs/NEWS.rst
- src/mailman/model/roster.py
- src/mailman/model/tests/test_mailinglist.py
- src/mailman/model/tests/test_member.py
- src/mailman/model/tests/test_roster.py
- src/mailman/model/tests/test_subscriptions.py
- src/mailman/model/tests/test_user.py
- src/mailman/rest/tests/test_membership.py
- src/mailman/rest/tests/test_moderation.py
- src/mailman/testing/helpers.py
Changes:
=====================================
src/mailman/app/tests/test_subscriptions.py
=====================================
--- a/src/mailman/app/tests/test_subscriptions.py
+++ b/src/mailman/app/tests/test_subscriptions.py
@@ -30,7 +30,8 @@ from mailman.interfaces.bans import IBanManager
from mailman.interfaces.member import MembershipIsBannedError
from mailman.interfaces.pending import IPendings
from mailman.interfaces.subscriptions import TokenOwner
-from mailman.testing.helpers import LogFileMark, get_queue_messages
+from mailman.testing.helpers import (
+ LogFileMark, get_queue_messages, set_preferred)
from mailman.testing.layers import ConfigLayer
from mailman.interfaces.mailinglist import SubscriptionPolicy
from mailman.interfaces.usermanager import IUserManager
@@ -95,9 +96,7 @@ class TestSubscriptionWorkflow(unittest.TestCase):
# Ensure that the sanity check phase, when given an IUser with a
# preferred address, ends up with an address.
anne = self._user_manager.make_user(self._anne)
- address = list(anne.addresses)[0]
- address.verified_on = now()
- anne.preferred_address = address
+ address = set_preferred(anne)
workflow = SubscriptionWorkflow(self._mlist, anne)
# The constructor sets workflow.address because the user has a
# preferred address.
@@ -549,9 +548,7 @@ approval:
# A confirmation step is necessary when a user subscribes with their
# preferred address, and we are not pre-confirming.
anne = self._user_manager.create_user(self._anne)
- address = list(anne.addresses)[0]
- address.verified_on = now()
- anne.preferred_address = address
+ set_preferred(anne)
# Run the workflow to model the confirmation step. There is no
# subscriber attribute yet.
workflow = SubscriptionWorkflow(self._mlist, anne)
=====================================
src/mailman/docs/NEWS.rst
=====================================
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -56,6 +56,8 @@ Bugs
subscribed address that is not linked to a user. Given by Aurélien Bompard.
* Fix a REST server crash when trying to subscribe a user without a preferred
address. (Closes #185)
+ * Fix membership query when multiple users are subscribed to a mailing list.
+ Reported by Darrell Kresge. (Closes: #190)
Configuration
-------------
=====================================
src/mailman/model/roster.py
=====================================
--- a/src/mailman/model/roster.py
+++ b/src/mailman/model/roster.py
@@ -109,8 +109,9 @@ class AbstractRoster:
members_u = store.query(Member).filter(
Member.list_id == self._mlist.list_id,
Member.role == self.role,
- Address.email==email,
- Member.user_id == User.id)
+ Address.email == email,
+ Member.user_id == User.id,
+ User._preferred_address_id == Address.id)
return members_a.union(members_u).all()
def get_member(self, email):
=====================================
src/mailman/model/tests/test_mailinglist.py
=====================================
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -36,7 +36,8 @@ from mailman.interfaces.mailinglist import (
from mailman.interfaces.member import (
AlreadySubscribedError, MemberRole, MissingPreferredAddressError)
from mailman.interfaces.usermanager import IUserManager
-from mailman.testing.helpers import configuration, get_queue_messages
+from mailman.testing.helpers import (
+ configuration, get_queue_messages, set_preferred)
from mailman.testing.layers import ConfigLayer
from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -54,9 +55,7 @@ class TestMailingList(unittest.TestCase):
# list with the same role.
anne = getUtility(IUserManager).create_user('a...@example.com')
# Give the user a preferred address.
- preferred = list(anne.addresses)[0]
- preferred.verified_on = now()
- anne.preferred_address = preferred
+ preferred = set_preferred(anne)
# Subscribe Anne to the mailing list as a regular member.
member = self._mlist.subscribe(anne)
self.assertEqual(member.address, preferred)
=====================================
src/mailman/model/tests/test_member.py
=====================================
--- a/src/mailman/model/tests/test_member.py
+++ b/src/mailman/model/tests/test_member.py
@@ -29,6 +29,7 @@ from mailman.interfaces.member import MemberRole,
MembershipError
from mailman.interfaces.user import UnverifiedAddressError
from mailman.interfaces.usermanager import IUserManager
from mailman.model.member import Member
+from mailman.testing.helpers import set_preferred
from mailman.testing.layers import ConfigLayer
from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -46,9 +47,7 @@ class TestMember(unittest.TestCase):
# A user is subscribed to a mailing list with their preferred address.
# You cannot set the `address` attribute on such IMembers.
anne = self._usermanager.create_user('a...@example.com')
- preferred = list(anne.addresses)[0]
- preferred.verified_on = now()
- anne.preferred_address = preferred
+ set_preferred(anne)
# Subscribe with the IUser object, not the address. This makes Anne a
# member via her preferred address.
member = self._mlist.subscribe(anne)
=====================================
src/mailman/model/tests/test_roster.py
=====================================
--- a/src/mailman/model/tests/test_roster.py
+++ b/src/mailman/model/tests/test_roster.py
@@ -20,6 +20,7 @@
__all__ = [
'TestMailingListRoster',
'TestMembershipsRoster',
+ 'TestUserRoster',
]
@@ -30,8 +31,8 @@ from mailman.interfaces.address import IAddress
from mailman.interfaces.member import DeliveryMode, MemberRole
from mailman.interfaces.user import IUser
from mailman.interfaces.usermanager import IUserManager
+from mailman.testing.helpers import set_preferred
from mailman.testing.layers import ConfigLayer
-from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -140,9 +141,7 @@ class TestMembershipsRoster(unittest.TestCase):
user_manager = getUtility(IUserManager)
self._anne = user_manager.make_user(
'a...@example.com', 'Anne Person')
- preferred = list(self._anne.addresses)[0]
- preferred.verified_on = now()
- self._anne.preferred_address = preferred
+ set_preferred(self._anne)
def test_no_memberships(self):
# An unsubscribed user has no memberships.
@@ -206,3 +205,33 @@ class TestMembershipsRoster(unittest.TestCase):
users = list(self._anne.memberships.users)
self.assertEqual(len(users), 1)
self.assertEqual(users[0], self._anne)
+
+
+
+class TestUserRoster(unittest.TestCase):
+ """Test aspects of rosters when users are subscribed."""
+
+ layer = ConfigLayer
+
+ def setUp(self):
+ self._mlist = create_list('a...@example.com')
+ user_manager = getUtility(IUserManager)
+ self._anne = user_manager.create_user('a...@example.com')
+ self._bart = user_manager.create_user('b...@example.com')
+ self._cris = user_manager.create_user('c...@example.com')
+ self._dave = user_manager.create_user('d...@example.com')
+ set_preferred(self._anne)
+ set_preferred(self._bart)
+ set_preferred(self._cris)
+ set_preferred(self._dave)
+
+ def test_narrow_get_member(self):
+ # Ensure that when multiple users are subscribed to the same mailing
+ # list via their preferred address, only the member in question is
+ # returned from .get_member().
+ self._mlist.subscribe(self._anne)
+ self._mlist.subscribe(self._bart)
+ self._mlist.subscribe(self._cris)
+ self._mlist.subscribe(self._dave)
+ member = self._mlist.members.get_member('b...@example.com')
+ self.assertEqual(member.user, self._bart)
=====================================
src/mailman/model/tests/test_subscriptions.py
=====================================
--- a/src/mailman/model/tests/test_subscriptions.py
+++ b/src/mailman/model/tests/test_subscriptions.py
@@ -30,7 +30,7 @@ from mailman.interfaces.member import MemberRole
from mailman.interfaces.subscriptions import (
ISubscriptionService, TooManyMembersError)
from mailman.interfaces.usermanager import IUserManager
-from mailman.testing.helpers import subscribe
+from mailman.testing.helpers import set_preferred, subscribe
from mailman.testing.layers import ConfigLayer
from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -58,9 +58,7 @@ class TestSubscriptionService(unittest.TestCase):
# Find address-based memberships when a user is linked to the address.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ address = set_preferred(user)
# Subscribe the address.
self._mlist.subscribe(address)
members = self._service.find_members('a...@example.com')
@@ -71,9 +69,7 @@ class TestSubscriptionService(unittest.TestCase):
# Find user-based memberships by address.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ set_preferred(user)
# Subscribe the user.
self._mlist.subscribe(user)
members = self._service.find_members('a...@example.com')
@@ -84,9 +80,7 @@ class TestSubscriptionService(unittest.TestCase):
# Find user-based memberships using a secondary address.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ set_preferred(user)
# Create a secondary address.
address_2 = self._user_manager.create_address(
'an...@example.com', 'Anne User 2')
@@ -104,9 +98,7 @@ class TestSubscriptionService(unittest.TestCase):
# subscription is not returned.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ set_preferred(user)
# Create a secondary address.
address_2 = self._user_manager.create_address(
'an...@example.com', 'Anne User 2')
@@ -122,9 +114,7 @@ class TestSubscriptionService(unittest.TestCase):
# Find user-based memberships by user_id.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ set_preferred(user)
# Subscribe the user.
self._mlist.subscribe(user)
members = self._service.find_members(user.user_id)
@@ -136,9 +126,7 @@ class TestSubscriptionService(unittest.TestCase):
# subscribed.
user = self._user_manager.create_user(
'a...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ set_preferred(user)
# Create a secondary address.
address_2 = self._user_manager.create_address(
'an...@example.com', 'Anne User 2')
@@ -159,9 +147,7 @@ class TestSubscriptionService(unittest.TestCase):
# Check that the memberships are properly sorted.
user = self._user_manager.create_user(
'an...@example.com', 'Anne User')
- address = user.addresses[0]
- address.verified_on = now()
- user.preferred_address = address
+ address = set_preferred(user)
# Create a secondary address.
address_2 = self._user_manager.create_address(
'an...@example.com', 'Anne User 2')
=====================================
src/mailman/model/tests/test_user.py
=====================================
--- a/src/mailman/model/tests/test_user.py
+++ b/src/mailman/model/tests/test_user.py
@@ -32,8 +32,8 @@ from mailman.interfaces.address import (
from mailman.interfaces.user import UnverifiedAddressError
from mailman.interfaces.usermanager import IUserManager
from mailman.model.preferences import Preferences
+from mailman.testing.helpers import set_preferred
from mailman.testing.layers import ConfigLayer
-from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -48,9 +48,7 @@ class TestUser(unittest.TestCase):
self._mlist = create_list('t...@example.com')
self._anne = self._manager.create_user(
'a...@example.com', 'Anne Person')
- preferred = list(self._anne.addresses)[0]
- preferred.verified_on = now()
- self._anne.preferred_address = preferred
+ set_preferred(self._anne)
def test_preferred_address_memberships(self):
self._mlist.subscribe(self._anne)
=====================================
src/mailman/rest/tests/test_membership.py
=====================================
--- a/src/mailman/rest/tests/test_membership.py
+++ b/src/mailman/rest/tests/test_membership.py
@@ -34,7 +34,7 @@ from mailman.interfaces.member import DeliveryMode, MemberRole
from mailman.interfaces.usermanager import IUserManager
from mailman.testing.helpers import (
TestableMaster, call_api, get_lmtp_client, make_testable_runner,
- subscribe, wait_for_webservice)
+ set_preferred, subscribe, wait_for_webservice)
from mailman.runners.incoming import IncomingRunner
from mailman.testing.layers import ConfigLayer, RESTLayer
from mailman.utilities.datetime import now
@@ -42,12 +42,6 @@ from urllib.error import HTTPError
from zope.component import getUtility
-def _set_preferred(user):
- preferred = list(user.addresses)[0]
- preferred.verified_on = now()
- user.preferred_address = preferred
-
-
class TestMembership(unittest.TestCase):
layer = RESTLayer
@@ -207,7 +201,7 @@ class TestMembership(unittest.TestCase):
def test_join_as_user_with_preferred_address(self):
with transaction():
anne = self._usermanager.create_user('a...@example.com')
- _set_preferred(anne)
+ set_preferred(anne)
self._mlist.subscribe(anne)
content, response = call_api('http://localhost:9001/3.0/members')
self.assertEqual(response.status, 200)
@@ -226,7 +220,7 @@ class TestMembership(unittest.TestCase):
def test_member_changes_preferred_address(self):
with transaction():
anne = self._usermanager.create_user('a...@example.com')
- _set_preferred(anne)
+ set_preferred(anne)
self._mlist.subscribe(anne)
# Take a look at Anne's current membership.
content, response = call_api('http://localhost:9001/3.0/members')
@@ -589,7 +583,7 @@ class TestAPI31Members(unittest.TestCase):
def test_create_new_membership_by_hex(self):
with transaction():
user = getUtility(IUserManager).create_user('a...@example.com')
- _set_preferred(user)
+ set_preferred(user)
# Subscribe the user to the mailing list by hex UUID.
response, headers = call_api(
'http://localhost:9001/3.1/members', {
@@ -608,7 +602,7 @@ class TestAPI31Members(unittest.TestCase):
def test_create_new_owner_by_hex(self):
with transaction():
user = getUtility(IUserManager).create_user('a...@example.com')
- _set_preferred(user)
+ set_preferred(user)
# Subscribe the user to the mailing list by hex UUID.
response, headers = call_api(
'http://localhost:9001/3.1/members', {
@@ -625,7 +619,7 @@ class TestAPI31Members(unittest.TestCase):
def test_cannot_create_new_membership_by_int(self):
with transaction():
user = getUtility(IUserManager).create_user('a...@example.com')
- _set_preferred(user)
+ set_preferred(user)
# We can't use the int representation of the UUID with API 3.1.
with self.assertRaises(HTTPError) as cm:
call_api('http://localhost:9001/3.1/members', {
=====================================
src/mailman/rest/tests/test_moderation.py
=====================================
--- a/src/mailman/rest/tests/test_moderation.py
+++ b/src/mailman/rest/tests/test_moderation.py
@@ -34,9 +34,9 @@ from mailman.interfaces.registrar import IRegistrar
from mailman.interfaces.requests import IListRequests, RequestType
from mailman.interfaces.usermanager import IUserManager
from mailman.testing.helpers import (
- call_api, get_queue_messages, specialized_message_from_string as mfs)
+ call_api, get_queue_messages, set_preferred,
+ specialized_message_from_string as mfs)
from mailman.testing.layers import RESTLayer
-from mailman.utilities.datetime import now
from urllib.error import HTTPError
from zope.component import getUtility
@@ -141,9 +141,7 @@ class TestSubscriptionModeration(unittest.TestCase):
'a...@example.com', 'Anne Person')
self._bart = manager.make_user(
'b...@example.com', 'Bart Person')
- preferred = list(self._bart.addresses)[0]
- preferred.verified_on = now()
- self._bart.preferred_address = preferred
+ set_preferred(self._bart)
def test_no_such_list(self):
# Try to get the requests of a nonexistent list.
=====================================
src/mailman/testing/helpers.py
=====================================
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -31,6 +31,7 @@ __all__ = [
'make_digest_messages',
'make_testable_runner',
'reset_the_world',
+ 'set_preferred',
'specialized_message_from_string',
'subscribe',
'temporary_db',
@@ -554,3 +555,13 @@ message triggering a digest
volume=1, digest_number=1)
runner = make_testable_runner(DigestRunner, 'digest')
runner.run()
+
+
+
+def set_preferred(user):
+ # Avoid circular imports.
+ from mailman.utilities.datetime import now
+ preferred = list(user.addresses)[0]
+ preferred.verified_on = now()
+ user.preferred_address = preferred
+ return preferred
View it on GitLab:
https://gitlab.com/mailman/mailman/compare/3a8477dc3d8ce782c36d0043fe4a5521ca5ef8a3...d87106191610b69387feb1e339ecdf6db9727c53
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
