Merge lp:~jskrzeszewska/mailman/enablingArchivers into lp:mailman

Proposed by Joanna Skrzeszewska
Status: Merged
Merge reported by: Barry Warsaw
Merged at revision: not available
Proposed branch: lp:~jskrzeszewska/mailman/enablingArchivers
Merge into: lp:mailman
Diff against target: 279 lines (+137/-19)
5 files modified
src/mailman/interfaces/mailinglist.py (+11/-0)
src/mailman/model/mailinglist.py (+55/-4)
src/mailman/rest/configuration.py (+28/-1)
src/mailman/rest/tests/test_lists.py (+26/-0)
src/mailman/runners/archive.py (+17/-14)
To merge this branch: bzr merge lp:~jskrzeszewska/mailman/enablingArchivers
Reviewer Review Type Date Requested Status
Barry Warsaw Approve
Nicki Hutchens (community) Approve
Nicki Hutchens Pending
Review via email: mp+184461@code.launchpad.net

Description of the change

Changes for enabling/disabling archivers using postorius (bug 1158040).

To post a comment you must log in.
7220. By Joanna Skrzeszewska

Unit tests for enabling/disabling archivers.

Revision history for this message
Nicki Hutchens (nhutch01) :
review: Approve
Revision history for this message
Barry Warsaw (barry) wrote :

Hi Joanna,

Apologies for the long delay, but I've finally managed to review this branch. I decided to make a number of changes to the semantics and API, but your branch was instrumental in getting this new feature landed. Thanks very much for your contribution!

The main change that I made was to the REST API. Rather than put the list of archivers inside the list's 'configuration' resource, I decided to make it a separate sub-resource of the mailing list. This helped map the semantics of getting the list of enabled archivers, and changing their state, better to HTTP commands.

r7229 in trunk contains the motified and merged branch.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/interfaces/mailinglist.py'
2--- src/mailman/interfaces/mailinglist.py 2013-06-19 02:43:40 +0000
3+++ src/mailman/interfaces/mailinglist.py 2013-09-18 10:39:39 +0000
4@@ -24,6 +24,7 @@
5 'IAcceptableAlias',
6 'IAcceptableAliasSet',
7 'IMailingList',
8+ 'IArchiverList',
9 'Personalization',
10 'ReplyToMunging',
11 ]
12@@ -54,6 +55,16 @@
13 # An explicit Reply-To header is added
14 explicit_header = 2
15
16+class IArchiverList(Interface):
17+ mailing_list_id = Attribute("""List id""")
18+ archiver_name = Attribute("""Archiver name""")
19+ archiver_enabled = Attribute("""If is enabled.""")
20+
21+class IListArchiverSet(Interface):
22+ def getAll():
23+ """Return dict containing all archivers and their settings."""
24+ def set(archiver, is_enabled):
25+ """Set archiver for this list."""
26
27
28
29 class IMailingList(Interface):
30
31=== modified file 'src/mailman/model/mailinglist.py'
32--- src/mailman/model/mailinglist.py 2013-01-01 14:05:42 +0000
33+++ src/mailman/model/mailinglist.py 2013-09-18 10:39:39 +0000
34@@ -28,8 +28,8 @@
35 import os
36
37 from storm.locals import (
38- And, Bool, DateTime, Float, Int, Pickle, RawStr, Reference, Store,
39- TimeDelta, Unicode)
40+ And, Bool, DateTime, Float, Int, Pickle, RawStr, Reference, ReferenceSet,
41+ Store, TimeDelta, Unicode)
42 from urlparse import urljoin
43 from zope.component import getUtility
44 from zope.event import notify
45@@ -47,8 +47,8 @@
46 from mailman.interfaces.domain import IDomainManager
47 from mailman.interfaces.languages import ILanguageManager
48 from mailman.interfaces.mailinglist import (
49- IAcceptableAlias, IAcceptableAliasSet, IMailingList, Personalization,
50- ReplyToMunging)
51+ IAcceptableAlias, IAcceptableAliasSet, IArchiverList, IListArchiverSet,
52+ IMailingList, Personalization, ReplyToMunging)
53 from mailman.interfaces.member import (
54 AlreadySubscribedError, MemberRole, MissingPreferredAddressError,
55 SubscriptionEvent)
56@@ -67,6 +67,19 @@
57 SPACE = ' '
58 UNDERSCORE = '_'
59
60+@implementer(IArchiverList)
61+class ArchiverList(Model):
62+ __storm_primary__ = "mailing_list_id", "archiver_name"
63+ mailing_list_id = Int()
64+ archiver_name = Unicode()
65+ archiver_enabled = Bool()
66+
67+ def __init__(self, mailing_list_id, archiver_name):
68+ self.mailing_list_id = mailing_list_id
69+ self.archiver_name = archiver_name
70+ self.archiver_enabled = False
71+
72+
73
74
75
76 @implementer(IMailingList)
77@@ -78,6 +91,7 @@
78 # XXX denotes attributes that should be part of the public interface but
79 # are currently missing.
80
81+ archivers = ReferenceSet(id, ArchiverList.mailing_list_id)
82 # List identity
83 list_name = Unicode()
84 mail_host = Unicode()
85@@ -538,3 +552,40 @@
86 AcceptableAlias.mailing_list == self._mailing_list)
87 for alias in aliases:
88 yield alias.alias
89+
90+@implementer(IListArchiverSet)
91+class ListArchiverSet:
92+ def __init__(self, mailing_list):
93+ self._mailing_list = mailing_list
94+ self.lazyAdd()
95+
96+ def getAll(self):
97+ entries = Store.of(self._mailing_list).find(ArchiverList, ArchiverList.mailing_list_id == self._mailing_list.id)
98+ all_in_config = {archiver.name for archiver in config.archivers}
99+ ret = {}
100+ for entry in entries:
101+ if entry.archiver_name in all_in_config:
102+ ret[entry.archiver_name] = int(entry.archiver_enabled)
103+ return ret
104+
105+ def set(self, archiver, is_enabled):
106+ bool_enabled = (int(is_enabled) != 0)
107+ self.get(archiver).set(archiver_enabled=bool_enabled)
108+
109+ def isEnabled(self, archiverName):
110+ return self.get(archiverName).one().archiver_enabled
111+
112+ def get(self, archiverName):
113+ return Store.of(self._mailing_list).find(ArchiverList,
114+ (ArchiverList.mailing_list_id == self._mailing_list.id) & (ArchiverList.archiver_name == archiverName))
115+
116+ def lazyAdd(self):
117+ names = []
118+ for archiver in config.archivers:
119+ count = self.get(archiver.name).count()
120+ names.append((archiver.name, count))
121+ if not count:
122+ entry = ArchiverList(self._mailing_list.id, archiver.name)
123+ Store.of(self._mailing_list).add(entry)
124+ Store.of(self._mailing_list).commit()
125+
126
127=== modified file 'src/mailman/rest/configuration.py'
128--- src/mailman/rest/configuration.py 2013-03-20 21:01:57 +0000
129+++ src/mailman/rest/configuration.py 2013-09-18 10:39:39 +0000
130@@ -34,9 +34,10 @@
131 from mailman.interfaces.action import Action
132 from mailman.interfaces.archiver import ArchivePolicy
133 from mailman.interfaces.autorespond import ResponseAction
134-from mailman.interfaces.mailinglist import IAcceptableAliasSet, ReplyToMunging
135+from mailman.interfaces.mailinglist import IAcceptableAliasSet, IListArchiverSet, ReplyToMunging
136 from mailman.rest.helpers import GetterSetter, PATCH, etag, no_content
137 from mailman.rest.validator import PatchValidator, Validator, enum_validator
138+from mailman.model.mailinglist import ListArchiverSet
139
140
141
142
143@@ -64,6 +65,22 @@
144 for alias in value:
145 alias_set.add(unicode(alias))
146
147+class ListArchivers(GetterSetter):
148+
149+ def get(self, mlist, attribute):
150+ """Return the mailing list's acceptable aliases."""
151+ assert attribute == 'archivers', (
152+ 'Unexpected attribute: {0}'.format(attribute))
153+ archivers = ListArchiverSet(mlist)
154+ return archivers.getAll()
155+
156+ def put(self, mlist, attribute, value):
157+ assert attribute == 'archivers', (
158+ 'Unexpected attribute: {0}'.format(attribute))
159+ archivers = ListArchiverSet(mlist)
160+ for key, value in value.iteritems():
161+ archivers.set(key, value)
162+
163
164
165
166 # Additional validators for converting from web request strings to internal
167@@ -80,6 +97,15 @@
168 """Turn a list of things into a list of unicodes."""
169 return [unicode(value) for value in values]
170
171+def list_of_pairs_to_dict(pairs):
172+ dict = {}
173+ # If pairs has only one element then it is not a list but a string.
174+ if not isinstance(pairs, list):
175+ pairs = [pairs]
176+ for key_value in pairs:
177+ parts = key_value.split('|')
178+ dict[parts[0]] = parts[1]
179+ return dict
180
181
182
183 # This is the list of IMailingList attributes that are exposed through the
184@@ -98,6 +124,7 @@
185
186 ATTRIBUTES = dict(
187 acceptable_aliases=AcceptableAliases(list_of_unicode),
188+ archivers=ListArchivers(list_of_pairs_to_dict),
189 admin_immed_notify=GetterSetter(as_boolean),
190 admin_notify_mchanges=GetterSetter(as_boolean),
191 administrivia=GetterSetter(as_boolean),
192
193=== modified file 'src/mailman/rest/tests/test_lists.py'
194--- src/mailman/rest/tests/test_lists.py 2013-01-01 14:05:42 +0000
195+++ src/mailman/rest/tests/test_lists.py 2013-09-18 10:39:39 +0000
196@@ -28,12 +28,16 @@
197
198 import unittest
199
200+from zope.component import getUtility
201 from urllib2 import HTTPError
202 from zope.component import getUtility
203
204 from mailman.app.lifecycle import create_list
205+from mailman.config import config
206 from mailman.database.transaction import transaction
207 from mailman.interfaces.usermanager import IUserManager
208+from mailman.interfaces.listmanager import IListManager
209+from mailman.model.mailinglist import ListArchiverSet
210 from mailman.testing.helpers import call_api
211 from mailman.testing.layers import RESTLayer
212
213@@ -159,3 +163,25 @@
214 call_api('http://localhost:9001/3.0/lists/ant.example.com',
215 method='DELETE')
216 self.assertEqual(cm.exception.code, 404)
217+
218+ def test_prototype_in_list_archivers(self):
219+ resource, response = call_api(
220+ 'http://localhost:9001/3.0/lists/test@example.com/config')
221+ self.assertEqual(response.status, 200)
222+ self.assertEqual(resource['archivers']['prototype'], 0)
223+
224+ def test_lazy_add_archivers(self):
225+ call_api('http://localhost:9001/3.0/lists', {
226+ 'fqdn_listname': 'new_list@example.com',
227+ })
228+ resource, response = call_api(
229+ 'http://localhost:9001/3.0/lists/new_list@example.com/config')
230+ self.assertEqual(response.status, 200)
231+ self.assertEqual(resource['archivers']['prototype'], 0)
232+
233+ def test_set_archiver_enabled(self):
234+ mlist = getUtility(IListManager).create('newest_list@example.com')
235+ lset = ListArchiverSet(mlist)
236+ lset.set('prototype', 1)
237+ self.assertEqual(lset.isEnabled('prototype'), 1)
238+
239
240=== modified file 'src/mailman/runners/archive.py'
241--- src/mailman/runners/archive.py 2013-01-01 14:05:42 +0000
242+++ src/mailman/runners/archive.py 2013-09-18 10:39:39 +0000
243@@ -36,6 +36,7 @@
244 from mailman.core.runner import Runner
245 from mailman.interfaces.archiver import ClobberDate
246 from mailman.utilities.datetime import RFC822_DATE_FMT, now
247+from mailman.model.mailinglist import ListArchiverSet
248
249
250 log = logging.getLogger('mailman.error')
251@@ -91,17 +92,19 @@
252 def _dispose(self, mlist, msg, msgdata):
253 received_time = msgdata.get('received_time', now(strip_tzinfo=False))
254 for archiver in config.archivers:
255- msg_copy = copy.deepcopy(msg)
256- if _should_clobber(msg, msgdata, archiver.name):
257- original_date = msg_copy['date']
258- del msg_copy['date']
259- del msg_copy['x-original-date']
260- msg_copy['Date'] = received_time.strftime(RFC822_DATE_FMT)
261- if original_date:
262- msg_copy['X-Original-Date'] = original_date
263- # A problem in one archiver should not prevent other archivers
264- # from running.
265- try:
266- archiver.archive_message(mlist, msg_copy)
267- except Exception:
268- log.exception('Broken archiver: %s' % archiver.name)
269+ archSet = ListArchiverSet(mlist)
270+ if archSet.isEnabled(archiver.name):
271+ msg_copy = copy.deepcopy(msg)
272+ if _should_clobber(msg, msgdata, archiver.name):
273+ original_date = msg_copy['date']
274+ del msg_copy['date']
275+ del msg_copy['x-original-date']
276+ msg_copy['Date'] = received_time.strftime(RFC822_DATE_FMT)
277+ if original_date:
278+ msg_copy['X-Original-Date'] = original_date
279+ # A problem in one archiver should not prevent other archivers
280+ # from running.
281+ try:
282+ archiver.archive_message(mlist, msg_copy)
283+ except Exception:
284+ log.exception('Broken archiver: %s' % archiver.name)