Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
c22d7895 by Yashu Seth at 2015-12-16T22:54:51Z
adds digest_send_periodic, digest_volume_frequency attributes
- - - - -
e5422f53 by Yashu Seth at 2015-12-16T22:54:51Z
adds tests for new digest attributes
- - - - -
21062456 by Barry Warsaw at 2015-12-17T17:41:49Z
Many improvements to listconf.py.
Closes #182
* Improve the documentation, especially in describing how to PUT and
PATCH to list configuration subresources.
* Improve the return codes for many error corner cases. Specifically,
this makes more consistent when a 400 error is returned or a 404 error
is returned.
* Improve the handling of some weird corner cases, and add tests.
* Fix the setting of error response reasons by not trying to .format()
into a bytes object (which isn't allowed in Python 3).
* Add lots of comments to the code, which improves the readability of
all the twisty little turns.
* 100% code coverage for listconf.py!
- - - - -
bb1d8bfb by Barry Warsaw at 2015-12-17T17:49:31Z
Remove code accidentally pulled in from another branch.
- - - - -
3fb07280 by Barry Warsaw at 2015-12-17T18:04:26Z
Fix the error code for a corner case.
I.e. when the URL is good, but the request dictionary contains a bogus
attribute, you should get a 400 error, not a 404.
Also, fix some comments.
- - - - -
3 changed files:
- src/mailman/rest/docs/listconf.rst
- src/mailman/rest/listconf.py
- src/mailman/rest/tests/test_listconf.py
Changes:
=====================================
src/mailman/rest/docs/listconf.rst
=====================================
--- a/src/mailman/rest/docs/listconf.rst
+++ b/src/mailman/rest/docs/listconf.rst
@@ -186,10 +186,56 @@ These values are changed permanently.
Sub-resources
=============
-Many of the mailing list configuration variables are actually available as
-sub-resources on the mailing list. This is because they are collections,
-sequences, and other complex configuration types. Their values can be
-retrieved and set through the sub-resource.
+Mailing list configuration variables are actually available as sub-resources
+on the mailing list. Their values can be retrieved and set through the
+sub-resource.
+
+
+Simple resources
+----------------
+
+You can view the current value of the sub-resource.
+
+ >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
+ ... '/config/display_name')
+ display_name: My List
+ http_etag: ...
+
+The resource can be changed by PUTting to it. Note that the value still
+requires a dictionary, and that dictionary must have a single key matching the
+name of the resource.
+::
+
+ >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
+ ... '/config/display_name',
+ ... dict(display_name='Your List'),
+ ... 'PUT')
+ content-length: 0
+ date: ...
+ server: ...
+ status: 204
+
+ >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
+ ... '/config/display_name')
+ display_name: Your List
+ http_etag: ...
+
+PATCH works the same way, with the same effect, so you can choose to use
+either method.
+
+ >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
+ ... '/config/display_name',
+ ... dict(display_name='Their List'),
+ ... 'PATCH')
+ content-length: 0
+ date: ...
+ server: ...
+ status: 204
+
+ >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
+ ... '/config/display_name')
+ display_name: Their List
+ http_etag: ...
Acceptable aliases
=====================================
src/mailman/rest/listconf.py
=====================================
--- a/src/mailman/rest/listconf.py
+++ b/src/mailman/rest/listconf.py
@@ -32,7 +32,7 @@ from mailman.interfaces.autorespond import ResponseAction
from mailman.interfaces.mailinglist import (
IAcceptableAliasSet, ReplyToMunging, SubscriptionPolicy)
from mailman.rest.helpers import (
- GetterSetter, bad_request, etag, no_content, okay)
+ GetterSetter, bad_request, etag, no_content, not_found, okay)
from mailman.rest.validator import (
PatchValidator, Validator, enum_validator, list_of_strings_validator)
@@ -44,7 +44,7 @@ class AcceptableAliases(GetterSetter):
def get(self, mlist, attribute):
"""Return the mailing list's acceptable aliases."""
assert attribute == 'acceptable_aliases', (
- 'Unexpected attribute: {}'.format(attribute))
+ 'Unexpected attribute: {}'.format(attribute)) # pragma: no cover
aliases = IAcceptableAliasSet(mlist)
return sorted(aliases.aliases)
@@ -56,7 +56,7 @@ class AcceptableAliases(GetterSetter):
ignored.
"""
assert attribute == 'acceptable_aliases', (
- 'Unexpected attribute: {}'.format(attribute))
+ 'Unexpected attribute: {}'.format(attribute)) # pragma: no cover
alias_set = IAcceptableAliasSet(mlist)
alias_set.clear()
for alias in value:
@@ -161,15 +161,18 @@ class ListConfiguration:
"""Get a mailing list configuration."""
resource = {}
if self._attribute is None:
- # Return all readable attributes.
+ # This is a requst for all the mailing list's configuration
+ # variables. Return all readable attributes.
for attribute in ATTRIBUTES:
value = ATTRIBUTES[attribute].get(self._mlist, attribute)
resource[attribute] = value
elif self._attribute not in ATTRIBUTES:
- bad_request(
- response, b'Unknown attribute: {}'.format(self._attribute))
+ # This is a request for a specific, nonexistent attribute.
+ not_found(
+ response, 'Unknown attribute: {}'.format(self._attribute))
return
else:
+ # This is a request for a specific attribute.
attribute = self._attribute
value = ATTRIBUTES[attribute].get(self._mlist, attribute)
resource[attribute] = value
@@ -179,20 +182,31 @@ class ListConfiguration:
"""Set a mailing list configuration."""
attribute = self._attribute
if attribute is None:
+ # This is a request to update all the list's writable
+ # configuration variables. All must be provided in the request.
validator = Validator(**VALIDATORS)
try:
validator.update(self._mlist, request)
except ValueError as error:
+ # Unlike the case where we're PUTting to a specific
+ # configuration sub-resource, if we're PUTting to the list's
+ # entire configuration, but the request has a bogus attribute,
+ # the entire request is considered bad. We can also get here
+ # if one of the attributes is read-only. The error will
+ # contain sufficient details, so just return it as the reason.
bad_request(response, str(error))
return
elif attribute not in ATTRIBUTES:
- bad_request(response, b'Unknown attribute: {}'.format(attribute))
+ # Here we're PUTting to a specific resource, but that attribute is
+ # bogus so the URL is considered pointing to a missing resource.
+ not_found(response, 'Unknown attribute: {}'.format(attribute))
return
elif ATTRIBUTES[attribute].decoder is None:
bad_request(
- response, b'Read-only attribute: {}'.format(attribute))
+ response, 'Read-only attribute: {}'.format(attribute))
return
else:
+ # We're PUTting to a specific configuration sub-resource.
validator = Validator(**{attribute: VALIDATORS[attribute]})
try:
validator.update(self._mlist, request)
@@ -203,15 +217,42 @@ class ListConfiguration:
def on_patch(self, request, response):
"""Patch the configuration (i.e. partial update)."""
+ if self._attribute is None:
+ # We're PATCHing one or more of the attributes on the list's
+ # configuration resource, so all the writable attributes are valid
+ # candidates for updating.
+ converters = ATTRIBUTES
+ else:
+ # We're PATCHing a specific list configuration attribute
+ # sub-resource. Because the request data must be a dictionary, we
+ # restrict it to containing only a single key, which must match
+ # the attribute name. First, check for any extra attributes in
+ # the request.
+ keys = [key for key, value in request.params.items()]
+ if len(keys) > 1:
+ bad_request(response, 'Expected 1 attribute, got {}'.format(
+ len(keys)))
+ return
+ converter = ATTRIBUTES.get(self._attribute)
+ if converter is None:
+ # This is the case where the URL points to a nonexisting list
+ # configuration attribute sub-resource.
+ not_found(response, 'Unknown attribute: {}'.format(
+ self._attribute))
+ return
+ converters = {self._attribute: converter}
try:
- validator = PatchValidator(request, ATTRIBUTES)
+ validator = PatchValidator(request, converters)
except UnknownPATCHRequestError as error:
+ # This is the case where the URL points to the list's entire
+ # configuration resource, but the request dictionary contains a
+ # nonexistent attribute.
bad_request(
- response, b'Unknown attribute: {}'.format(error.attribute))
+ response, 'Unknown attribute: {}'.format(error.attribute))
return
except ReadOnlyPATCHRequestError as error:
bad_request(
- response, b'Read-only attribute: {}'.format(error.attribute))
+ response, 'Read-only attribute: {}'.format(error.attribute))
return
try:
validator.update(self._mlist, request)
=====================================
src/mailman/rest/tests/test_listconf.py
=====================================
--- a/src/mailman/rest/tests/test_listconf.py
+++ b/src/mailman/rest/tests/test_listconf.py
@@ -30,6 +30,51 @@ from mailman.interfaces.mailinglist import (
IAcceptableAliasSet, SubscriptionPolicy)
from mailman.testing.helpers import call_api
from mailman.testing.layers import RESTLayer
+from urllib.error import HTTPError
+
+
+# The representation of the listconf resource as a dictionary. This is used
+# when PUTting to the list's configuration resource.
+RESOURCE = dict(
+ acceptable_aliases=[
+ 'a...@example.com',
+ 'b...@example.com',
+ 'c...@example.com',
+ ],
+ admin_immed_notify=False,
+ admin_notify_mchanges=True,
+ administrivia=False,
+ advertised=False,
+ anonymous_list=True,
+ archive_policy='never',
+ autorespond_owner='respond_and_discard',
+ autorespond_postings='respond_and_continue',
+ autorespond_requests='respond_and_discard',
+ autoresponse_grace_period='45d',
+ autoresponse_owner_text='the owner',
+ autoresponse_postings_text='the mailing list',
+ autoresponse_request_text='the robot',
+ display_name='Fnords',
+ description='This is my mailing list',
+ include_rfc2369_headers=False,
+ allow_list_posts=False,
+ #digest_send_periodic='yes',
+ digest_size_threshold=10.5,
+ #digest_volume_frequency=1,
+ posting_pipeline='virgin',
+ filter_content=True,
+ first_strip_reply_to=True,
+ convert_html_to_plaintext=True,
+ collapse_alternatives=False,
+ reply_goes_to_list='point_to_list',
+ reply_to_address='b...@example.com',
+ send_welcome_message=False,
+ subject_prefix='[ant]',
+ subscription_policy='confirm_then_moderate',
+ welcome_message_uri='mailman:///welcome.txt',
+ default_member_action='hold',
+ default_nonmember_action='discard',
+ )
@@ -40,71 +85,173 @@ class TestConfiguration(unittest.TestCase):
def setUp(self):
with transaction():
- self._mlist = create_list('t...@example.com')
+ self._mlist = create_list('a...@example.com')
+
+ def test_get_missing_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config/bogus')
+ self.assertEqual(cm.exception.code, 404)
+ self.assertEqual(cm.exception.reason, b'Unknown attribute: bogus')
def test_put_configuration(self):
- aliases = [
- 'a...@example.com',
- 'b...@example.com',
- 'c...@example.com',
- ]
# When using PUT, all writable attributes must be included.
resource, response = call_api(
- 'http://localhost:9001/3.0/lists/t...@example.com/config',
- dict(
- acceptable_aliases=aliases,
- admin_immed_notify=False,
- admin_notify_mchanges=True,
- administrivia=False,
- advertised=False,
- anonymous_list=True,
- archive_policy='never',
- autorespond_owner='respond_and_discard',
- autorespond_postings='respond_and_continue',
- autorespond_requests='respond_and_discard',
- autoresponse_grace_period='45d',
- autoresponse_owner_text='the owner',
- autoresponse_postings_text='the mailing list',
- autoresponse_request_text='the robot',
- display_name='Fnords',
- description='This is my mailing list',
- include_rfc2369_headers=False,
- allow_list_posts=False,
- digest_size_threshold=10.5,
- posting_pipeline='virgin',
- filter_content=True,
- first_strip_reply_to=True,
- convert_html_to_plaintext=True,
- collapse_alternatives=False,
- reply_goes_to_list='point_to_list',
- reply_to_address='b...@example.com',
- send_welcome_message=False,
- subject_prefix='[ant]',
- subscription_policy='confirm_then_moderate',
- welcome_message_uri='mailman:///welcome.txt',
- default_member_action='hold',
- default_nonmember_action='discard',
- ),
+ 'http://localhost:9001/3.0/lists/ant.example.com/config',
+ RESOURCE,
'PUT')
self.assertEqual(response.status, 204)
self.assertEqual(self._mlist.display_name, 'Fnords')
# All three acceptable aliases were set.
self.assertEqual(set(IAcceptableAliasSet(self._mlist).aliases),
- set(aliases))
+ set(RESOURCE['acceptable_aliases']))
+
+ def test_put_attribute(self):
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address')
+ self.assertEqual(resource['reply_to_address'], '')
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address',
+ dict(reply_to_address='b...@ant.example.com'),
+ 'PUT')
+ self.assertEqual(response.status, 204)
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address')
+ self.assertEqual(resource['reply_to_address'], 'b...@ant.example.com')
+
+ def test_put_extra_attribute(self):
+ bogus_resource = RESOURCE.copy()
+ bogus_resource['bogus'] = 'yes'
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config',
+ bogus_resource,
+ 'PUT')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason, b'Unexpected parameters: bogus')
+
+ def test_put_attribute_mismatch(self):
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address')
+ self.assertEqual(resource['reply_to_address'], '')
+ with self.assertRaises(HTTPError) as cm:
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address',
+ dict(display_name='b...@ant.example.com'),
+ 'PUT')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Unexpected parameters: display_name')
+
+ def test_put_attribute_double(self):
+ with self.assertRaises(HTTPError) as cm:
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address',
+ dict(display_name='b...@ant.example.com',
+ reply_to_address='f...@example.com'),
+ 'PUT')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Unexpected parameters: display_name')
+
+ def test_put_read_only_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api('http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/mail_host',
+ dict(mail_host='foo.example.com'),
+ 'PUT')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Read-only attribute: mail_host')
+
+ def test_put_missing_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config/bogus',
+ dict(bogus='no matter'),
+ 'PUT')
+ self.assertEqual(cm.exception.code, 404)
+ self.assertEqual(cm.exception.reason, b'Unknown attribute: bogus')
def test_patch_subscription_policy(self):
# The new subscription_policy value can be patched.
#
# To start with, the subscription policy is confirm by default.
resource, response = call_api(
- 'http://localhost:9001/3.0/lists/t...@example.com/config')
+ 'http://localhost:9001/3.0/lists/ant.example.com/config')
self.assertEqual(resource['subscription_policy'], 'confirm')
# Let's patch it to do some moderation.
resource, response = call_api(
- 'http://localhost:9001/3.0/lists/t...@example.com/config', dict(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config', dict(
subscription_policy='confirm_then_moderate'),
method='PATCH')
self.assertEqual(response.status, 204)
# And now we verify that it has the requested setting.
self.assertEqual(self._mlist.subscription_policy,
SubscriptionPolicy.confirm_then_moderate)
+
+ def test_patch_attribute_double(self):
+ with self.assertRaises(HTTPError) as cm:
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/reply_to_address',
+ dict(display_name='b...@ant.example.com',
+ reply_to_address='foo'),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason, b'Expected 1 attribute, got 2')
+
+ def test_unknown_patch_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api('http://localhost:9001/3.0/lists/ant.example.com/config',
+ dict(bogus=1),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason, b'Unknown attribute: bogus')
+
+ def test_read_only_patch_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api('http://localhost:9001/3.0/lists/ant.example.com'
+ '/config/mail_host',
+ dict(mail_host='foo.example.com'),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Read-only attribute: mail_host')
+
+ def test_patch_missing_attribute(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config/bogus',
+ dict(bogus='no matter'),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 404)
+ self.assertEqual(cm.exception.reason, b'Unknown attribute: bogus')
+
+ def test_patch_bad_value(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config'
+ '/archive_policy',
+ dict(archive_policy='not a valid archive policy'),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Cannot convert parameters: archive_policy')
+
+ def test_bad_pipeline_name(self):
+ with self.assertRaises(HTTPError) as cm:
+ call_api(
+ 'http://localhost:9001/3.0/lists/ant.example.com/config'
+ '/posting_pipeline',
+ dict(posting_pipeline='not a valid pipeline'),
+ 'PATCH')
+ self.assertEqual(cm.exception.code, 400)
+ self.assertEqual(cm.exception.reason,
+ b'Cannot convert parameters: posting_pipeline')
View it on GitLab:
https://gitlab.com/mailman/mailman/compare/1ebb7654978939d3ae549956b7b45ef0ea0a983a...3fb072801ffe4990146caa0a7c5afa26f300229f
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
