Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
03bb57c8 by Barry Warsaw at 2016-01-11T22:52:33Z
Several optimizations:
* Use `yield from` wherever appropriate.
* Use SA's .one_or_none() where appropriate.
- Fix a bug in MailingList.pass_extensions.
- Use ValueError in other places for consistency.
- Remove unreached/nonsense code.
- Simplify the SubscriptionService.find_member() and .find_members()
implementations.
- Boost coverage.
- - - - -
20 changed files:
- src/mailman/chains/base.py
- src/mailman/chains/tests/test_base.py
- src/mailman/config/config.py
- src/mailman/config/tests/test_configuration.py
- src/mailman/core/pipelines.py
- src/mailman/model/bounce.py
- src/mailman/model/domain.py
- src/mailman/model/listmanager.py
- src/mailman/model/mailinglist.py
- src/mailman/model/requests.py
- src/mailman/model/roster.py
- src/mailman/model/subscriptions.py
- src/mailman/model/tests/test_mailinglist.py
- src/mailman/model/tests/test_roster.py
- src/mailman/model/tests/test_subscriptions.py
- src/mailman/model/usermanager.py
- src/mailman/mta/aliases.py
- src/mailman/testing/helpers.py
- src/mailman/testing/mta.py
- src/mailman/utilities/modules.py
Changes:
=====================================
src/mailman/chains/base.py
=====================================
--- a/src/mailman/chains/base.py
+++ b/src/mailman/chains/base.py
@@ -114,8 +114,7 @@ class Chain:
"""Return an iterator over the links."""
# We do it this way in order to preserve a separation of interfaces,
# and allows .get_links() to be overridden.
- for link in self._links:
- yield link
+ yield from self._links
=====================================
src/mailman/chains/tests/test_base.py
=====================================
--- a/src/mailman/chains/tests/test_base.py
+++ b/src/mailman/chains/tests/test_base.py
@@ -19,15 +19,22 @@
__all__ = [
'TestMiscellaneous',
+ 'TestTerminalChainBase',
]
import unittest
from mailman.chains.accept import AcceptChain
-from mailman.chains.base import Chain, Link
+from mailman.chains.base import Chain, Link, TerminalChainBase
from mailman.interfaces.chain import LinkAction
from mailman.rules.any import Any
+from mailman.testing.layers import ConfigLayer
+
+
+class SimpleChain(TerminalChainBase):
+ def _process(self, mlist, msg, msgdata):
+ pass
@@ -75,3 +82,12 @@ class TestMiscellaneous(unittest.TestCase):
chain.flush()
count = sum(1 for link in chain.get_iterator())
self.assertEqual(count, 0)
+
+
+class TestTerminalChainBase(unittest.TestCase):
+ layer = ConfigLayer
+
+ def test_terminal_chain_iterator(self):
+ chain = SimpleChain()
+ self.assertEqual([link.action for link in chain],
+ [LinkAction.run, LinkAction.stop])
=====================================
src/mailman/config/config.py
=====================================
--- a/src/mailman/config/config.py
+++ b/src/mailman/config/config.py
@@ -245,8 +245,7 @@ class Configuration:
@property
def runner_configs(self):
"""Iterate over all the runner configuration sections."""
- for section in self._config.getByCategory('runner', []):
- yield section
+ yield from self._config.getByCategory('runner', [])
@property
def archivers(self):
@@ -262,8 +261,7 @@ class Configuration:
@property
def language_configs(self):
"""Iterate over all the language configuration sections."""
- for section in self._config.getByCategory('language', []):
- yield section
+ yield from self._config.getByCategory('language', [])
=====================================
src/mailman/config/tests/test_configuration.py
=====================================
--- a/src/mailman/config/tests/test_configuration.py
+++ b/src/mailman/config/tests/test_configuration.py
@@ -58,6 +58,21 @@ class TestConfiguration(unittest.TestCase):
pass
self.assertEqual(events, ['first', 'second', 'first'])
+ def test_config_template_dir_is_source(self):
+ fd, filename = tempfile.mkstemp()
+ self.addCleanup(os.remove, filename)
+ os.close(fd)
+ with open(filename, 'w') as fp:
+ print("""\
+[paths.here]
+template_dir: :source:
+""", file=fp)
+ config = Configuration()
+ config.load(filename)
+ import mailman.templates
+ self.assertEqual(config.TEMPLATE_DIR,
+ os.path.dirname(mailman.templates.__file__))
+
class TestExternal(unittest.TestCase):
=====================================
src/mailman/core/pipelines.py
=====================================
--- a/src/mailman/core/pipelines.py
+++ b/src/mailman/core/pipelines.py
@@ -85,8 +85,7 @@ class BasePipeline:
def __iter__(self):
"""See `IPipeline`."""
- for handler in self._handlers:
- yield handler
+ yield from self._handlers
=====================================
src/mailman/model/bounce.py
=====================================
--- a/src/mailman/model/bounce.py
+++ b/src/mailman/model/bounce.py
@@ -54,8 +54,6 @@ class BounceEvent(Model):
self.email = email
self.timestamp = now()
msgid = msg['message-id']
- if isinstance(msgid, bytes):
- msgid = msgid.decode('ascii')
self.message_id = msgid
self.context = (BounceContext.normal if context is None else context)
self.processed = False
@@ -77,12 +75,10 @@ class BounceProcessor:
@dbconnection
def events(self, store):
"""See `IBounceProcessor`."""
- for event in store.query(BounceEvent).all():
- yield event
+ yield from store.query(BounceEvent).all()
@property
@dbconnection
def unprocessed(self, store):
"""See `IBounceProcessor`."""
- for event in store.query(BounceEvent).filter_by(processed=False):
- yield event
+ yield from store.query(BounceEvent).filter_by(processed=False)
=====================================
src/mailman/model/domain.py
=====================================
--- a/src/mailman/model/domain.py
+++ b/src/mailman/model/domain.py
@@ -94,11 +94,9 @@ class Domain(Model):
@dbconnection
def mailing_lists(self, store):
"""See `IDomain`."""
- mailing_lists = store.query(MailingList).filter(
+ yield from store.query(MailingList).filter(
MailingList.mail_host == self.mail_host
).order_by(MailingList._list_id)
- for mlist in mailing_lists:
- yield mlist
def confirm_url(self, token=''):
"""See `IDomain`."""
@@ -194,8 +192,7 @@ class DomainManager:
@dbconnection
def __iter__(self, store):
"""See `IDomainManager`."""
- for domain in store.query(Domain).order_by(Domain.mail_host).all():
- yield domain
+ yield from store.query(Domain).order_by(Domain.mail_host).all()
@dbconnection
def __contains__(self, store, mail_host):
=====================================
src/mailman/model/listmanager.py
=====================================
--- a/src/mailman/model/listmanager.py
+++ b/src/mailman/model/listmanager.py
@@ -88,15 +88,13 @@ class ListManager:
@dbconnection
def mailing_lists(self, store):
"""See `IListManager`."""
- for mlist in store.query(MailingList).order_by(
- MailingList._list_id).all():
- yield mlist
+ yield from store.query(MailingList).order_by(
+ MailingList._list_id).all()
@dbconnection
def __iter__(self, store):
"""See `IListManager`."""
- for mlist in store.query(MailingList).all():
- yield mlist
+ yield from store.query(MailingList).all()
@property
@dbconnection
@@ -105,7 +103,7 @@ class ListManager:
result_set = store.query(MailingList)
for mail_host, list_name in result_set.values(MailingList.mail_host,
MailingList.list_name):
- yield '{0}@{1}'.format(list_name, mail_host)
+ yield '{}@{}'.format(list_name, mail_host)
@property
@dbconnection
=====================================
src/mailman/model/mailinglist.py
=====================================
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -431,7 +431,7 @@ class MailingList(Model):
ContentFilter.mailing_list == self,
ContentFilter.filter_type == FilterType.pass_extension)
for content_filter in results:
- yield content_filter.pass_pattern
+ yield content_filter.filter_pattern
@pass_extensions.setter
@dbconnection
@@ -459,7 +459,7 @@ class MailingList(Model):
elif role is MemberRole.nonmember:
return self.nonmembers
else:
- raise TypeError('Undefined MemberRole: {}'.format(role))
+ raise ValueError('Undefined MemberRole: {}'.format(role))
@dbconnection
def subscribe(self, store, subscriber, role=MemberRole.member):
@@ -576,10 +576,10 @@ class ListArchiver(Model):
@property
def system_archiver(self):
- for archiver in config.archivers:
+ for archiver in config.archivers: # pragma: no branch
if archiver.name == self.name:
return archiver
- return None
+ raise AssertionError('Archiver not found: {}'.format(self.name))
@property
def is_enabled(self):
@@ -613,8 +613,7 @@ class ListArchiverSet:
def archivers(self, store):
entries = store.query(ListArchiver).filter(
ListArchiver.mailing_list == self._mailing_list)
- for entry in entries:
- yield entry
+ yield from entries
@dbconnection
def get(self, store, archiver_name):
=====================================
src/mailman/model/requests.py
=====================================
--- a/src/mailman/model/requests.py
+++ b/src/mailman/model/requests.py
@@ -83,8 +83,7 @@ class ListRequests:
def held_requests(self, store):
results = store.query(_Request).filter_by(
mailing_list=self.mailing_list)
- for request in results:
- yield request
+ yield from results
@dbconnection
def of_type(self, store, request_type):
=====================================
src/mailman/model/roster.py
=====================================
--- a/src/mailman/model/roster.py
+++ b/src/mailman/model/roster.py
@@ -68,8 +68,7 @@ class AbstractRoster:
@property
def members(self):
"""See `IRoster`."""
- for member in self._query():
- yield member
+ yield from self._query()
@property
def member_count(self):
@@ -84,9 +83,7 @@ class AbstractRoster:
# keep a set of unique users. It's possible for the same user to be
# subscribed to a mailing list multiple times with different
# addresses.
- users = set(member.address.user for member in self.members)
- for user in users:
- yield user
+ yield from set(member.address.user for member in self.members)
@property
def addresses(self):
@@ -189,19 +186,12 @@ class AdministratorRoster(AbstractRoster):
@dbconnection
def get_member(self, store, email):
"""See `IRoster`."""
- results = store.query(Member).filter(
+ return store.query(Member).filter(
Member.list_id == self._mlist.list_id,
or_(Member.role == MemberRole.moderator,
Member.role == MemberRole.owner),
Address.email == email,
- Member.address_id == Address.id)
- if results.count() == 0:
- return None
- elif results.count() == 1:
- return results[0]
- else:
- raise AssertionError(
- 'Too many matching member results: {0}'.format(results))
+ Member.address_id == Address.id).one_or_none()
@@ -243,8 +233,7 @@ class RegularMemberRoster(DeliveryMemberRoster):
@property
def members(self):
"""See `IRoster`."""
- for member in self._get_members(DeliveryMode.regular):
- yield member
+ yield from self._get_members(DeliveryMode.regular)
@@ -256,10 +245,10 @@ class DigestMemberRoster(DeliveryMemberRoster):
@property
def members(self):
"""See `IRoster`."""
- for member in self._get_members(DeliveryMode.plaintext_digests,
- DeliveryMode.mime_digests,
- DeliveryMode.summary_digests):
- yield member
+ yield from self._get_members(
+ DeliveryMode.plaintext_digests,
+ DeliveryMode.mime_digests,
+ DeliveryMode.summary_digests)
@@ -301,8 +290,7 @@ class Memberships:
@property
def members(self):
"""See `IRoster`."""
- for member in self._query():
- yield member
+ yield from self._query()
@property
def users(self):
@@ -312,27 +300,14 @@ class Memberships:
@property
def addresses(self):
"""See `IRoster`."""
- for address in self._user.addresses:
- yield address
+ yield from self._user.addresses
@dbconnection
def get_member(self, store, email):
"""See `IRoster`."""
- results = store.query(Member).filter(
- Member.address_id == Address.id,
- Address.user_id == self._user.id)
- if results.count() == 0:
- return None
- elif results.count() == 1:
- return results[0]
- else:
- raise AssertionError(
- 'Too many matching member results: {0}'.format(
- results.count()))
+ raise NotImplementedError
@dbconnection
def get_memberships(self, store, address):
"""See `IRoster`."""
- # 2015-04-14 BAW: See LP: #1444055 -- this currently exists just to
- # pass a test.
raise NotImplementedError
=====================================
src/mailman/model/subscriptions.py
=====================================
--- a/src/mailman/model/subscriptions.py
+++ b/src/mailman/model/subscriptions.py
@@ -82,7 +82,7 @@ class SubscriptionService:
# which are controlled by the user, otherwise we'll just search for
# the given address.
if subscriber is None and list_id is None and role is None:
- raise NoResultFound
+ return None
order = (Member.list_id, Address.email, Member.role)
# Querying for the subscriber is the most complicated part, because
# the parameter can either be an email address or a user id. Start by
@@ -116,24 +116,23 @@ class SubscriptionService:
def find_members(self, subscriber=None, list_id=None, role=None):
"""See `ISubscriptionService`."""
- try:
- query = self._find_members(subscriber, list_id, role)
- except NoResultFound:
- query = None
- return QuerySequence(query)
+ return QuerySequence(self._find_members(subscriber, list_id, role))
def find_member(self, subscriber=None, list_id=None, role=None):
"""See `ISubscriptionService`."""
try:
- return self._find_members(subscriber, list_id, role).one()
+ result = self._find_members(subscriber, list_id, role)
+ return (result if result is None else result.one())
except NoResultFound:
return None
except MultipleResultsFound:
+ # Coerce the exception into a Mailman-layer exception so call
+ # sites don't have to import from SQLAlchemy, resulting in a layer
+ # violation.
raise TooManyMembersError(subscriber, list_id, role)
def __iter__(self):
- for member in self.get_members():
- yield member
+ yield from self.get_members()
def leave(self, list_id, email):
"""See `ISubscriptionService`."""
=====================================
src/mailman/model/tests/test_mailinglist.py
=====================================
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -75,6 +75,17 @@ class TestMailingList(unittest.TestCase):
self.assertRaises(MissingPreferredAddressError,
self._mlist.subscribe, anne)
+ def test_pass_extensions(self):
+ self._mlist.pass_extensions = ('foo', 'bar', 'baz')
+ self.assertEqual(list(self._mlist.pass_extensions),
+ ['foo', 'bar', 'baz'])
+
+ def test_get_roster_argument(self):
+ self.assertRaises(ValueError, self._mlist.get_roster, 'members')
+
+ def test_subscribe_argument(self):
+ self.assertRaises(ValueError, self._mlist.subscribe, 'anne')
+
class TestListArchiver(unittest.TestCase):
@@ -218,3 +229,11 @@ class TestHeaderMatch(unittest.TestCase):
('header', 'pattern', None),
('subject', 'patt.*', None),
])
+
+ def test_clear(self):
+ header_matches = IHeaderMatchSet(self._mlist)
+ header_matches.add('Header', 'pattern')
+ self.assertEqual(len(self._mlist.header_matches), 1)
+ with transaction():
+ header_matches.clear()
+ self.assertEqual(len(self._mlist.header_matches), 0)
=====================================
src/mailman/model/tests/test_roster.py
=====================================
--- a/src/mailman/model/tests/test_roster.py
+++ b/src/mailman/model/tests/test_roster.py
@@ -200,3 +200,9 @@ class TestMembershipsRoster(unittest.TestCase):
self.assertEqual(
[record.address.email for record in memberships],
['a...@example.com', 'a...@example.com'])
+
+ def test_memberships_users(self):
+ self._ant.subscribe(self._anne)
+ users = list(self._anne.memberships.users)
+ self.assertEqual(len(users), 1)
+ self.assertEqual(users[0], self._anne)
=====================================
src/mailman/model/tests/test_subscriptions.py
=====================================
--- a/src/mailman/model/tests/test_subscriptions.py
+++ b/src/mailman/model/tests/test_subscriptions.py
@@ -234,7 +234,7 @@ class TestSubscriptionService(unittest.TestCase):
('test3', 'anne3', MemberRole.moderator),
])
- def test_find_members_shortcut(self):
+ def test_find_no_members(self):
members = self._service.find_members()
self.assertEqual(len(members), 0)
=====================================
src/mailman/model/usermanager.py
=====================================
--- a/src/mailman/model/usermanager.py
+++ b/src/mailman/model/usermanager.py
@@ -103,8 +103,7 @@ class UserManager:
@dbconnection
def users(self, store):
"""See `IUserManager`."""
- for user in store.query(User).all():
- yield user
+ yield from store.query(User).all()
@dbconnection
def create_address(self, store, email, display_name=None):
@@ -152,19 +151,16 @@ class UserManager:
@dbconnection
def addresses(self, store):
"""See `IUserManager`."""
- for address in store.query(Address).all():
- yield address
+ yield from store.query(Address).all()
@property
@dbconnection
def members(self, store):
"""See `IUserManager."""
- for member in store.query(Member).all():
- yield member
+ yield from store.query(Member).all()
@property
@dbconnection
def server_owners(self, store):
""" See `IUserManager."""
- users = store.query(User).filter_by(is_server_owner=True)
- yield from users
+ yield from store.query(User).filter_by(is_server_owner=True)
=====================================
src/mailman/mta/aliases.py
=====================================
--- a/src/mailman/mta/aliases.py
+++ b/src/mailman/mta/aliases.py
@@ -48,7 +48,7 @@ class MailTransportAgentAliases:
# Always return
yield mlist.posting_address
for destination in sorted(SUBDESTINATIONS):
- yield '{0}-{1}@{2}'.format(
+ yield '{}-{}@{}'.format(
mlist.list_name, destination, mlist.mail_host)
def destinations(self, mlist):
@@ -56,4 +56,4 @@ class MailTransportAgentAliases:
# Always return
yield mlist.list_name
for destination in sorted(SUBDESTINATIONS):
- yield '{0}-{1}'.format(mlist.list_name, destination)
+ yield '{}-{}'.format(mlist.list_name, destination)
=====================================
src/mailman/testing/helpers.py
=====================================
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -220,8 +220,7 @@ class TestableMaster(Master):
@property
def runner_pids(self):
"""The pids of all the child runner processes."""
- for pid in self._started_kids:
- yield pid
+ yield from self._started_kids
=====================================
src/mailman/testing/mta.py
=====================================
--- a/src/mailman/testing/mta.py
+++ b/src/mailman/testing/mta.py
@@ -251,8 +251,7 @@ class ConnectionCountingController(QueueController):
@property
def messages(self):
"""Return all the messages received by the SMTP server."""
- for message in self:
- yield message
+ yield from self
def clear(self):
"""Clear all the messages from the queue."""
=====================================
src/mailman/utilities/modules.py
=====================================
--- a/src/mailman/utilities/modules.py
+++ b/src/mailman/utilities/modules.py
@@ -106,8 +106,7 @@ def find_components(package, interface):
module = sys.modules[module_name]
if not hasattr(module, '__all__'):
continue
- for component in scan_module(module, interface):
- yield component
+ yield from scan_module(module, interface)
View it on GitLab:
https://gitlab.com/mailman/mailman/commit/03bb57c8c2a47a08e19b20975622ebb2ef2b81c6
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
