From 826261effa9d74b8ecdf1247e9ebba75fa3b2baa Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 12 Aug 2014 16:42:12 -0400 Subject: First pass at converting to falcon for the REST API layer. Currently, only //system and its subpaths work, but basic auth does work too. Requires a refactoring modification to falcon. --- src/mailman/rest/docs/basic.rst | 70 ++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) (limited to 'src/mailman/rest/docs') diff --git a/src/mailman/rest/docs/basic.rst b/src/mailman/rest/docs/basic.rst index 51b287c90..42e5379ad 100644 --- a/src/mailman/rest/docs/basic.rst +++ b/src/mailman/rest/docs/basic.rst @@ -7,70 +7,48 @@ Mailman exposes a REST HTTP server for administrative control. The server listens for connections on a configurable host name and port. It is always protected by HTTP basic authentication using a single global -username and password. The credentials are set in the webservice section -of the config using the admin_user and admin_pass properties. +user name and password. The credentials are set in the `[webservice]` section +of the configuration using the `admin_user` and `admin_pass` properties. Because the REST server has full administrative access, it should always be run only on localhost, unless you really know what you're doing. In addition -you should set the username and password to secure values and distribute them +you should set the user name and password to secure values and distribute them to any REST clients with reasonable precautions. The Mailman major and minor version numbers are in the URL. -System information can be retrieved from the server. By default JSON is -returned. - >>> dump_json('http://localhost:9001/3.0/system') - http_etag: "..." - mailman_version: GNU Mailman 3.0... (...) - python_version: ... - self_link: http://localhost:9001/3.0/system - - -Non-existent links -================== - -When you try to access a link that doesn't exist, you get the appropriate HTTP -404 Not Found error. - - >>> dump_json('http://localhost:9001/3.0/does-not-exist') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 404: 404 Not Found - - -Invalid credentials -=================== +Credentials +=========== -When you try to access the REST server using invalid credentials you will get -an appropriate HTTP 401 Unauthorized error. -:: +When the `Authorization` header contains the proper creditials, the request +succeeds. >>> from base64 import b64encode - >>> auth = b64encode('baduser:badpass') - - >>> url = 'http://localhost:9001/3.0/system' + >>> from httplib2 import Http + >>> auth = b64encode('{0}:{1}'.format(config.webservice.admin_user, + ... config.webservice.admin_pass)) >>> headers = { ... 'Content-Type': 'application/x-www-form-urlencode', ... 'Authorization': 'Basic ' + auth, ... } - - >>> from httplib2 import Http - >>> response, content = Http().request(url, 'GET', None, headers) - >>> print(content) - 401 Unauthorized - - User is not authorized for the REST API - - -But with the right headers, the request succeeds. - - >>> auth = b64encode('{0}:{1}'.format(config.webservice.admin_user, - ... config.webservice.admin_pass)) - >>> headers['Authorization'] = 'Basic ' + auth + >>> url = 'http://localhost:9001/3.0/system' >>> response, content = Http().request(url, 'GET', None, headers) >>> print(response.status) 200 +Basic information +================= + +System information can be retrieved from the server, in the form of a JSON +encoded response. + + >>> dump_json('http://localhost:9001/3.0/system') + http_etag: "..." + mailman_version: GNU Mailman 3.0... (...) + python_version: ... + self_link: http://localhost:9001/3.0/system + + .. _REST: http://en.wikipedia.org/wiki/REST -- cgit v1.2.3-70-g09d2 From 72dd28e26b1fa369de93652bd51869cebd79c0a3 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 13 Aug 2014 00:18:23 -0400 Subject: Make preferences work. --- src/mailman/rest/addresses.py | 2 +- src/mailman/rest/docs/preferences.rst | 20 +-------- src/mailman/rest/members.py | 8 ++-- src/mailman/rest/preferences.py | 30 ++++++------- src/mailman/rest/tests/test_preferences.py | 67 ++++++++++++++++++++++++++++++ src/mailman/rest/users.py | 2 +- 6 files changed, 87 insertions(+), 42 deletions(-) create mode 100644 src/mailman/rest/tests/test_preferences.py (limited to 'src/mailman/rest/docs') diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py index d42a66aa7..f9a08b157 100644 --- a/src/mailman/rest/addresses.py +++ b/src/mailman/rest/addresses.py @@ -129,7 +129,7 @@ class AnAddress(_AddressBase): return NotFound(), [] return AddressMemberships(self._address) - @resource.child() + @child() def preferences(self, request, segments): """/addresses//preferences""" if len(segments) != 0: diff --git a/src/mailman/rest/docs/preferences.rst b/src/mailman/rest/docs/preferences.rst index 8694364a4..b8a0af500 100644 --- a/src/mailman/rest/docs/preferences.rst +++ b/src/mailman/rest/docs/preferences.rst @@ -199,21 +199,12 @@ Preferences accessed through this interface are always read only. receive_own_postings: False self_link: http://localhost:9001/3.0/members/1/all/preferences -These preferences cannot be changed. - - >>> dump_json('http://localhost:9001/3.0/members/1/all/preferences', { - ... 'delivery_status': 'enabled', - ... }, method='PATCH') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 405: 405 Method Not Allowed - System preferences ================== The Mailman system itself has a default set of preference. All preference -lookups fall back to these values, which are read-only. +look ups fall back to these values, which are read-only. >>> dump_json('http://localhost:9001/3.0/system/preferences') acknowledge_posts: False @@ -225,12 +216,3 @@ lookups fall back to these values, which are read-only. receive_list_copy: True receive_own_postings: True self_link: http://localhost:9001/3.0/system/preferences - -These preferences cannot be changed. - - >>> dump_json('http://localhost:9001/3.0/system/preferences', { - ... 'delivery_status': 'enabled', - ... }, method='PATCH') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 405: 405 Method Not Allowed diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index ae44ee2d5..b28a0d3e4 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -45,14 +45,14 @@ from mailman.interfaces.subscriptions import ISubscriptionService from mailman.interfaces.user import UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( - CollectionMixin, PATCH, etag, no_content, paginate, path_to) + CollectionMixin, PATCH, child, etag, no_content, paginate, path_to) from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) -class _MemberBase(resource.Resource, CollectionMixin): +class _MemberBase(CollectionMixin): """Shared base class for member representations.""" def _resource_as_dict(self, member): @@ -126,7 +126,7 @@ class AMember(_MemberBase): return http.not_found() return http.ok([], self._resource_as_json(self._member)) - @resource.child() + @child() def preferences(self, request, segments): """/members//preferences""" if len(segments) != 0: @@ -138,7 +138,7 @@ class AMember(_MemberBase): 'members/{0}'.format(self._member.member_id.int)) return child, [] - @resource.child() + @child() def all(self, request, segments): """/members//all/preferences""" if len(segments) == 0: diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py index d501ef865..fdd1f65fb 100644 --- a/src/mailman/rest/preferences.py +++ b/src/mailman/rest/preferences.py @@ -29,11 +29,8 @@ __all__ = [ import falcon from lazr.config import as_boolean -from restish import http, resource - from mailman.interfaces.member import DeliveryMode, DeliveryStatus -from mailman.rest.helpers import ( - GetterSetter, PATCH, etag, no_content, path_to) +from mailman.rest.helpers import GetterSetter, etag, path_to from mailman.rest.validator import ( Validator, enum_validator, language_validator) @@ -81,9 +78,9 @@ class ReadOnlyPreferences: class Preferences(ReadOnlyPreferences): """Preferences which can be changed.""" - def patch_put(self, request, is_optional): + def patch_put(self, request, response, is_optional): if self._parent is None: - return http.not_found() + response.status = falcon.HTTP_404 kws = dict( acknowledge_posts=GetterSetter(as_boolean), hide_address = GetterSetter(as_boolean), @@ -99,23 +96,22 @@ class Preferences(ReadOnlyPreferences): try: Validator(**kws).update(self._parent, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + falcon.responders.bad_request( + request, response, body=str(error)) + else: + response.status = falcon.HTTP_204 - @PATCH() - def patch_preferences(self, request): + def on_patch(self, request, response): """Patch the preferences.""" - return self.patch_put(request, is_optional=True) + self.patch_put(request, response, is_optional=True) - @resource.PUT() - def put_preferences(self, request): + def on_put(self, request, response): """Change all preferences.""" - return self.patch_put(request, is_optional=False) + self.patch_put(request, response, is_optional=False) - @resource.DELETE() - def delete_preferences(self, request): + def on_delete(self, request, response): """Delete all preferences.""" for attr in PREFERENCES: if hasattr(self._parent, attr): setattr(self._parent, attr, None) - return no_content() + response.status = falcon.HTTP_204 diff --git a/src/mailman/rest/tests/test_preferences.py b/src/mailman/rest/tests/test_preferences.py new file mode 100644 index 000000000..91a066cff --- /dev/null +++ b/src/mailman/rest/tests/test_preferences.py @@ -0,0 +1,67 @@ +# Copyright (C) 2014 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 . + +"""Test various preference functionality.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'TestPreferences', + ] + + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.database.transaction import transaction +from mailman.interfaces.usermanager import IUserManager +from mailman.testing.helpers import call_api +from mailman.testing.layers import RESTLayer +from urllib2 import HTTPError +from zope.component import getUtility + + +class TestPreferences(unittest.TestCase): + """Test various preference functionality.""" + + layer = RESTLayer + + def setUp(self): + user_manager = getUtility(IUserManager) + with transaction(): + self._mlist = create_list('test@example.com') + anne = user_manager.create_address('anne@example.com') + self._member = self._mlist.subscribe(anne) + + def test_read_only_member_all_preferences(self): + # A member's combined preferences are read-only. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members/1/all/preferences', { + 'delivery_status': 'enabled', + }, method='PATCH') + # The resource at this endpoint doesn't even have a PATCH method. + self.assertEqual(cm.exception.code, 405) + + def test_read_only_system_preferences(self): + # The system preferences are read-only. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/system/preferences', { + 'delivery_status': 'enabled', + }, method='PATCH') + # The resource at this endpoint doesn't even have a PATCH method. + self.assertEqual(cm.exception.code, 405) diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index ca38d72dc..019d02aa7 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -185,7 +185,7 @@ class AUser(_UserBase): user_manager.delete_user(self._user) return no_content() - @resource.child() + @child() def preferences(self, request, segments): """/addresses//preferences""" if len(segments) != 0: -- cgit v1.2.3-70-g09d2 From 45691d23d4fb4dca8e7a3d7442186a333e7f9663 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 13 Aug 2014 10:30:13 -0400 Subject: Domains are ported to falcon. --- src/mailman/rest/docs/domains.rst | 7 ----- src/mailman/rest/domains.py | 55 ++++++++++++++++++---------------- src/mailman/rest/lists.py | 34 +++++++++++---------- src/mailman/rest/tests/test_domains.py | 14 +++++++++ 4 files changed, 63 insertions(+), 47 deletions(-) (limited to 'src/mailman/rest/docs') diff --git a/src/mailman/rest/docs/domains.rst b/src/mailman/rest/docs/domains.rst index 92c73ffbf..b28326f73 100644 --- a/src/mailman/rest/docs/domains.rst +++ b/src/mailman/rest/docs/domains.rst @@ -110,13 +110,6 @@ The information for a single domain is available by following one of the self_link: http://localhost:9001/3.0/domains/lists.example.net url_host: example.net -But we get a 404 for a non-existent domain. - - >>> dump_json('http://localhost:9001/3.0/domains/does-not-exist') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 404: 404 Not Found - You can also list all the mailing lists for a given domain. At first, the example.com domain does not contain any mailing lists. :: diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py index 1a260ea3d..23b1c6b3c 100644 --- a/src/mailman/rest/domains.py +++ b/src/mailman/rest/domains.py @@ -26,18 +26,19 @@ __all__ = [ ] -from restish import http, resource -from zope.component import getUtility +import falcon from mailman.interfaces.domain import ( BadDomainSpecificationError, IDomainManager) -from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to +from mailman.rest.helpers import ( + BadRequest, CollectionMixin, NotFound, child, etag, path_to) from mailman.rest.lists import ListsForDomain from mailman.rest.validator import Validator +from zope.component import getUtility -class _DomainBase(resource.Resource, CollectionMixin): +class _DomainBase(CollectionMixin): """Shared base class for domain representations.""" def _resource_as_dict(self, domain): @@ -62,41 +63,42 @@ class ADomain(_DomainBase): def __init__(self, domain): self._domain = domain - @resource.GET() - def domain(self, request): + def on_get(self, request, response): """Return a single domain end-point.""" domain = getUtility(IDomainManager).get(self._domain) if domain is None: - return http.not_found() - return http.ok([], self._resource_as_json(domain)) + falcon.responders.path_not_found(request, response) + else: + response.status = falcon.HTTP_200 + response.body = self._resource_as_json(domain) - @resource.DELETE() - def delete(self, request): + def on_delete(self, request, response): """Delete the domain.""" try: getUtility(IDomainManager).remove(self._domain) except KeyError: # The domain does not exist. - return http.not_found() - return no_content() + falcon.responders.path_not_found( + request, response, '404 Not Found') + else: + response.status = falcon.HTTP_204 - @resource.child() + @child() def lists(self, request, segments): """/domains//lists""" if len(segments) == 0: domain = getUtility(IDomainManager).get(self._domain) if domain is None: - return http.not_found() + return NotFound() return ListsForDomain(domain) else: - return http.bad_request() + return BadRequest(), [] class AllDomains(_DomainBase): """The domains.""" - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new domain.""" domain_manager = getUtility(IDomainManager) try: @@ -108,15 +110,18 @@ class AllDomains(_DomainBase): 'contact_address')) domain = domain_manager.add(**validator(request)) except BadDomainSpecificationError: - return http.bad_request([], b'Domain exists') + falcon.responders.bad_request( + request, response, body=b'Domain exists') except ValueError as error: - return http.bad_request([], str(error)) - location = path_to('domains/{0}'.format(domain.mail_host)) - # Include no extra headers or body. - return http.created(location, [], None) + falcon.responders.bad_request( + request, response, body=str(error)) + else: + location = path_to('domains/{0}'.format(domain.mail_host)) + response.status = falcon.HTTP_201 + response.location = location - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/domains""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + response.status = falcon.HTTP_200 + response.body = etag(resource) diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index 552824927..7f0e6e797 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -29,6 +29,8 @@ __all__ = [ ] +import falcon + from lazr.config import as_boolean from operator import attrgetter from restish import http, resource @@ -103,7 +105,7 @@ def config_matcher(request, segments): -class _ListBase(resource.Resource, CollectionMixin): +class _ListBase(CollectionMixin): """Shared base class for mailing list representations.""" def _resource_as_dict(self, mlist): @@ -205,8 +207,7 @@ class AList(_ListBase): class AllLists(_ListBase): """The mailing lists.""" - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new mailing list.""" try: validator = Validator(fqdn_listname=unicode, @@ -214,16 +215,19 @@ class AllLists(_ListBase): _optional=('style_name',)) mlist = create_list(**validator(request)) except ListAlreadyExistsError: - return http.bad_request([], b'Mailing list exists') + falcon.responders.bad_request( + request, response, body=b'Mailing list exists') except BadDomainSpecificationError as error: - return http.bad_request([], b'Domain does not exist: {0}'.format( + falcon.responders.bad_request( + request, response, body=b'Domain does not exist: {0}'.format( error.domain)) except ValueError as error: - return http.bad_request([], str(error)) - # wsgiref wants headers to be bytes, not unicodes. - location = path_to('lists/{0}'.format(mlist.list_id)) - # Include no extra headers or body. - return http.created(location, [], None) + falcon.responders.bad_request( + request, response, body=str(error)) + else: + location = path_to('lists/{0}'.format(mlist.list_id)) + response.status = falcon.HTTP_201 + response.location = location @resource.GET() def collection(self, request): @@ -241,7 +245,7 @@ class MembersOfList(MemberCollection): self._mlist = mailing_list self._role = role - @paginate + #@paginate def _get_collection(self, request): """See `CollectionMixin`.""" # Overrides _MemberBase._get_collection() because we only want to @@ -257,13 +261,13 @@ class ListsForDomain(_ListBase): def __init__(self, domain): self._domain = domain - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/domains//lists""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + response.status = falcon.HTTP_200 + response.body = etag(resource) - @paginate + #@paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(self._domain.mailing_lists) diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py index ec08b749d..44cf11ef3 100644 --- a/src/mailman/rest/tests/test_domains.py +++ b/src/mailman/rest/tests/test_domains.py @@ -66,3 +66,17 @@ class TestDomains(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com', method='DELETE') self.assertEqual(response.status, 204) self.assertEqual(getUtility(IListManager).get('ant@example.com'), None) + + def test_missing_domain(self): + # You get a 404 if you try to access a nonexisting domain. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/domains/does-not-exist.com') + self.assertEqual(cm.exception.code, 404) + + def test_missing_domain_lists(self): + # You get a 404 if you try to access the mailing lists of a + # nonexisting domain. + with self.assertRaises(HTTPError) as cm: + call_api( + 'http://localhost:9001/3.0/domains/does-not-exist.com/lists') + self.assertEqual(cm.exception.code, 404) -- cgit v1.2.3-70-g09d2 From 4236657a07af91b1a247ca3fecc3838a875d127f Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 14 Aug 2014 13:43:35 -0400 Subject: Complete the conversion from restish to falcon, modulo a clean up pass. All REST tests pass. This requires an as yet unmerged internal change to falcon. --- setup.py | 2 +- src/mailman/rest/addresses.py | 7 +- src/mailman/rest/configuration.py | 65 ++++++++++-------- src/mailman/rest/docs/configuration.rst | 3 +- src/mailman/rest/docs/helpers.rst | 38 +++++------ src/mailman/rest/helpers.py | 69 ++------------------ src/mailman/rest/lists.py | 32 ++++----- src/mailman/rest/members.py | 68 ++++++++++--------- src/mailman/rest/moderation.py | 98 +++++++++++++++------------- src/mailman/rest/root.py | 7 +- src/mailman/rest/templates.py | 16 ++--- src/mailman/rest/tests/test_configuration.py | 94 ++++++++++++++++++++++++++ src/mailman/rest/tests/test_lists.py | 18 +++++ src/mailman/rest/tests/test_membership.py | 3 - src/mailman/rest/tests/test_paginate.py | 2 - src/mailman/rest/users.py | 8 +-- src/mailman/rest/validator.py | 6 +- src/mailman/rest/wsgiapp.py | 75 +++++++++++++++------ 18 files changed, 353 insertions(+), 258 deletions(-) create mode 100644 src/mailman/rest/tests/test_configuration.py (limited to 'src/mailman/rest/docs') diff --git a/setup.py b/setup.py index 79018f19d..cc6fa4b6b 100644 --- a/setup.py +++ b/setup.py @@ -94,6 +94,7 @@ case second `m'. Any other spelling is incorrect.""", }, install_requires = [ 'enum34', + 'falcon', 'flufl.bounce', 'flufl.i18n', 'flufl.lock', @@ -103,7 +104,6 @@ case second `m'. Any other spelling is incorrect.""", 'mock', 'nose2', 'passlib', - 'restish', 'storm', 'zope.component', 'zope.configuration', diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py index f9a08b157..a1ed31781 100644 --- a/src/mailman/rest/addresses.py +++ b/src/mailman/rest/addresses.py @@ -30,7 +30,6 @@ __all__ = [ import falcon from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.interfaces.address import ( @@ -83,7 +82,7 @@ class AllAddresses(_AddressBase): -class _VerifyResource(resource.Resource): +class _VerifyResource: """A helper resource for verify/unverify POSTS.""" def __init__(self, address, action): @@ -133,9 +132,9 @@ class AnAddress(_AddressBase): def preferences(self, request, segments): """/addresses//preferences""" if len(segments) != 0: - return http.bad_request() + return NotFound(), [] if self._address is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._address.preferences, 'addresses/{0}'.format(self._address.email)) diff --git a/src/mailman/rest/configuration.py b/src/mailman/rest/configuration.py index db0918703..532f7bc4d 100644 --- a/src/mailman/rest/configuration.py +++ b/src/mailman/rest/configuration.py @@ -25,9 +25,9 @@ __all__ = [ ] -from lazr.config import as_boolean, as_timedelta -from restish import http, resource +import falcon +from lazr.config import as_boolean, as_timedelta from mailman.config import config from mailman.core.errors import ( ReadOnlyPATCHRequestError, UnknownPATCHRequestError) @@ -35,7 +35,7 @@ from mailman.interfaces.action import Action from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.mailinglist import IAcceptableAliasSet, ReplyToMunging -from mailman.rest.helpers import GetterSetter, PATCH, etag, no_content +from mailman.rest.helpers import GetterSetter, etag from mailman.rest.validator import PatchValidator, Validator, enum_validator @@ -156,15 +156,14 @@ for attribute, gettersetter in VALIDATORS.items(): -class ListConfiguration(resource.Resource): +class ListConfiguration: """A mailing list configuration resource.""" def __init__(self, mailing_list, attribute): self._mlist = mailing_list self._attribute = attribute - @resource.GET() - def get_configuration(self, request): + def on_get(self, request, response): """Get a mailing list configuration.""" resource = {} if self._attribute is None: @@ -173,16 +172,18 @@ class ListConfiguration(resource.Resource): value = ATTRIBUTES[attribute].get(self._mlist, attribute) resource[attribute] = value elif self._attribute not in ATTRIBUTES: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(self._attribute)) + falcon.responders.bad_request( + request, response, + body=b'Unknown attribute: {0}'.format(self._attribute)) + return else: attribute = self._attribute value = ATTRIBUTES[attribute].get(self._mlist, attribute) resource[attribute] = value - return http.ok([], etag(resource)) + response.status = falcon.HTTP_200 + response.body = etag(resource) - @resource.PUT() - def put_configuration(self, request): + def on_put(self, request, response): """Set a mailing list configuration.""" attribute = self._attribute if attribute is None: @@ -190,34 +191,46 @@ class ListConfiguration(resource.Resource): try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) + falcon.responders.bad_request( + request, response, body=str(error)) + return elif attribute not in ATTRIBUTES: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(attribute)) + falcon.responders.bad_request( + request, response, + body=b'Unknown attribute: {0}'.format(attribute)) + return elif ATTRIBUTES[attribute].decoder is None: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(attribute)) + falcon.responders.bad_request( + request, response, + body=b'Read-only attribute: {0}'.format(attribute)) + return else: validator = Validator(**{attribute: VALIDATORS[attribute]}) try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + falcon.responders.bad_request( + request, response, body=str(error)) + return + response.status = falcon.HTTP_204 - @PATCH() - def patch_configuration(self, request): + def on_patch(self, request, response): """Patch the configuration (i.e. partial update).""" try: validator = PatchValidator(request, ATTRIBUTES) except UnknownPATCHRequestError as error: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(error.attribute)) + falcon.responders.bad_request( + request, response, + body=b'Unknown attribute: {0}'.format(error.attribute)) + return except ReadOnlyPATCHRequestError as error: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(error.attribute)) + falcon.responders.bad_request( + request, response, + body=b'Read-only attribute: {0}'.format(error.attribute)) + return try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + falcon.responders.bad_request(request, response, body=str(error)) + else: + response.status = falcon.HTTP_204 diff --git a/src/mailman/rest/docs/configuration.rst b/src/mailman/rest/docs/configuration.rst index 53ad172e9..841ab3c27 100644 --- a/src/mailman/rest/docs/configuration.rst +++ b/src/mailman/rest/docs/configuration.rst @@ -73,9 +73,8 @@ Not all of the readable attributes can be set through the web interface. The ones that can, can either be set via ``PUT`` or ``PATCH``. ``PUT`` changes all the writable attributes in one request. -When using PUT, all writable attributes must be included. +When using ``PUT``, all writable attributes must be included. - >>> from mailman.interfaces.action import Action >>> dump_json('http://localhost:9001/3.0/lists/' ... 'ant@example.com/config', ... dict( diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst index 7405fc98f..1a2a244ac 100644 --- a/src/mailman/rest/docs/helpers.rst +++ b/src/mailman/rest/docs/helpers.rst @@ -62,19 +62,19 @@ dictionary after tagging, since that's almost always what you want. neil : drums -POST unpacking -============== +POST and PUT unpacking +====================== -Another helper unpacks ``POST`` request variables, validating and converting -their values. +Another helper unpacks ``POST`` and ``PUT`` request variables, validating and +converting their values. :: >>> from mailman.rest.validator import Validator >>> validator = Validator(one=int, two=unicode, three=bool) >>> class FakeRequest: - ... POST = {} - >>> FakeRequest.POST = dict(one='1', two='two', three='yes') + ... params = {} + >>> FakeRequest.params = dict(one='1', two='two', three='yes') On valid input, the validator can be used as a ``**keyword`` argument. @@ -85,7 +85,7 @@ On valid input, the validator can be used as a ``**keyword`` argument. On invalid input, an exception is raised. - >>> FakeRequest.POST['one'] = 'hello' + >>> FakeRequest.params['one'] = 'hello' >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -93,7 +93,7 @@ On invalid input, an exception is raised. On missing input, an exception is raised. - >>> del FakeRequest.POST['one'] + >>> del FakeRequest.params['one'] >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -101,7 +101,7 @@ On missing input, an exception is raised. If more than one key is missing, it will be reflected in the error message. - >>> del FakeRequest.POST['two'] + >>> del FakeRequest.params['two'] >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -109,8 +109,8 @@ If more than one key is missing, it will be reflected in the error message. Extra keys are also not allowed. - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='', five='') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='', five='') >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -123,25 +123,25 @@ However, if optional keys are missing, it's okay. ... four=int, five=int, ... _optional=('four', 'five')) - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='4', five='5') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='4', five='5') >>> def print_request(one, two, three, four=None, five=None): ... print(repr(one), repr(two), repr(three), repr(four), repr(five)) >>> print_request(**validator(FakeRequest)) 1 u'two' True 4 5 - >>> del FakeRequest.POST['four'] + >>> del FakeRequest.params['four'] >>> print_request(**validator(FakeRequest)) 1 u'two' True None 5 - >>> del FakeRequest.POST['five'] + >>> del FakeRequest.params['five'] >>> print_request(**validator(FakeRequest)) 1 u'two' True None None But if the optional values are present, they must of course also be valid. - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='no', five='maybe') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='no', five='maybe') >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -181,7 +181,7 @@ This is a validation function that ensure the value is *not* a list. And a validator to pull it all together. >>> validator = Validator(one=must_be_scalar, many=must_be_list) - >>> FakeRequest.POST = form_data + >>> FakeRequest.params = form_data >>> values = validator(FakeRequest) >>> print(values['one']) 1 @@ -195,7 +195,7 @@ form data. >>> form_data = MultiDict(one='1', many='3') >>> form_data.add('many', '5') >>> form_data.add('many', '4') - >>> FakeRequest.POST = form_data + >>> FakeRequest.params = form_data >>> values = validator(FakeRequest) >>> print(values['one']) 1 diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index ab1f5b76a..74c66958d 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -25,28 +25,19 @@ __all__ = [ 'ChildError', 'GetterSetter', 'NotFound', - 'PATCH', 'child', 'etag', - 'no_content', 'path_to', - 'restish_matcher', ] -import cgi import json import falcon import hashlib -from cStringIO import StringIO from datetime import datetime, timedelta from enum import Enum from lazr.config import as_boolean -from restish.http import Response -from restish.resource import MethodDecorator -from webob.multidict import MultiDict - from mailman.config import config @@ -161,14 +152,14 @@ class CollectionMixin: This must be implemented by subclasses. - :param request: A restish request. + :param request: An http request. :return: The collection :rtype: list """ raise NotImplementedError def _make_collection(self, request): - """Provide the collection to restish.""" + """Provide the collection to the REST layer.""" collection = self._get_collection(request) if len(collection) == 0: return dict(start=0, total_size=0) @@ -186,59 +177,6 @@ class CollectionMixin: -# XXX 2010-02-24 barry Seems like contrary to the documentation, matchers -# cannot be plain functions, because matchers must have a .score attribute. -# OTOH, I think they support regexps, so that might be a better way to go. -def restish_matcher(function): - """Decorator for restish matchers.""" - function.score = () - return function - - -# restish doesn't support HTTP response code 204. -def no_content(): - """204 No Content.""" - return Response('204 No Content', [], None) - - -# These two classes implement an ugly, dirty hack to work around the fact that -# neither WebOb nor really the stdlib cgi module support non-standard HTTP -# verbs such as PATCH. Note that restish handles it just fine in the sense -# that the right method gets called, but without the following kludge, the -# body of the request will never get decoded, so the method won't see any -# data. -# -# Stuffing the MultiDict on request.PATCH is pretty ugly, but it mirrors -# WebOb's use of request.POST and request.PUT for those standard verbs. -# Besides, WebOb refuses to allow us to set request.POST. This does make -# validators.py a bit more complicated. :( - -class PATCHWrapper: - """Hack to decode the request body for PATCH.""" - def __init__(self, func): - self.func = func - - def __call__(self, resource, request): - # We can't use request.body_file because that's a socket that's - # already had its data read off of. IOW, if we use that directly, - # we'll block here. - field_storage = cgi.FieldStorage( - fp=StringIO(request.body), - # Yes, lie about the method so cgi will do the right thing. - environ=dict(REQUEST_METHOD='POST'), - keep_blank_values=True) - request.PATCH = MultiDict.from_fieldstorage(field_storage) - return self.func(resource, request) - - -class PATCH(MethodDecorator): - method = 'PATCH' - - def __call__(self, func): - really_wrapped_func = PATCHWrapper(func) - return super(PATCH, self).__call__(really_wrapped_func) - - class GetterSetter: """Get and set attributes on an object. @@ -321,6 +259,9 @@ class ChildError: on_get = _oops on_post = _oops + on_put = _oops + on_patch = _oops + on_delete = _oops class BadRequest(ChildError): diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index da1a75bd1..0aaa6298f 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -34,7 +34,6 @@ import falcon from lazr.config import as_boolean from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.app.lifecycle import create_list, remove_list @@ -48,15 +47,13 @@ from mailman.interfaces.styles import IStyleManager from mailman.interfaces.subscriptions import ISubscriptionService from mailman.rest.configuration import ListConfiguration from mailman.rest.helpers import ( - CollectionMixin, GetterSetter, NotFound, child, etag, paginate, path_to, - restish_matcher) + CollectionMixin, GetterSetter, NotFound, child, etag, paginate, path_to) from mailman.rest.members import AMember, MemberCollection from mailman.rest.moderation import HeldMessages, SubscriptionRequests from mailman.rest.validator import Validator -@restish_matcher def member_matcher(request, segments): """A matcher of member URLs inside mailing lists. @@ -69,13 +66,9 @@ def member_matcher(request, segments): except KeyError: # Not a valid role. return None - # No more segments. - # XXX 2010-02-25 barry Matchers are undocumented in restish; they return a - # 3-tuple of (match_args, match_kws, segments). return (), dict(role=role, email=segments[1]), () -@restish_matcher def roster_matcher(request, segments): """A matcher of all members URLs inside mailing lists. @@ -90,7 +83,6 @@ def roster_matcher(request, segments): return None -@restish_matcher def config_matcher(request, segments): """A matcher for a mailing list's configuration resource. @@ -159,44 +151,44 @@ class AList(_ListBase): remove_list(self._mlist) response.status = falcon.HTTP_204 - @resource.child(member_matcher) + @child(member_matcher) def member(self, request, segments, role, email): """Return a single member representation.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] members = getUtility(ISubscriptionService).find_members( email, self._mlist.list_id, role) if len(members) == 0: - return http.not_found() + return NotFound(), [] assert len(members) == 1, 'Too many matches' return AMember(members[0].member_id) - @resource.child(roster_matcher) + @child(roster_matcher) def roster(self, request, segments, role): """Return the collection of all a mailing list's members.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return MembersOfList(self._mlist, role) - @resource.child(config_matcher) + @child(config_matcher) def config(self, request, segments, attribute=None): """Return a mailing list configuration object.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return ListConfiguration(self._mlist, attribute) - @resource.child() + @child() def held(self, request, segments): """Return a list of held messages for the mailing list.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return HeldMessages(self._mlist) - @resource.child() + @child() def requests(self, request, segments): """Return a list of subscription/unsubscription requests.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return SubscriptionRequests(self._mlist) @child() diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 766185c3e..afc4b302d 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -32,7 +32,6 @@ import falcon from uuid import UUID from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.app.membership import delete_member @@ -45,7 +44,7 @@ from mailman.interfaces.subscriptions import ISubscriptionService from mailman.interfaces.user import UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( - CollectionMixin, PATCH, child, etag, no_content, paginate, path_to) + CollectionMixin, NotFound, child, etag, paginate, path_to) from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) @@ -119,20 +118,21 @@ class AMember(_MemberBase): service = getUtility(ISubscriptionService) self._member = service.get_member(member_id) - @resource.GET() - def member(self, request): + def on_get(self, request, response): """Return a single member end-point.""" if self._member is None: - return http.not_found() - return http.ok([], self._resource_as_json(self._member)) + falcon.responders.path_not_found(request, response) + else: + response.status = falcon.HTTP_200 + response.body = self._resource_as_json(self._member) @child() def preferences(self, request, segments): """/members//preferences""" if len(segments) != 0: - return http.bad_request() + return NotFound(), [] if self._member is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._member.preferences, 'members/{0}'.format(self._member.member_id.int)) @@ -142,59 +142,65 @@ class AMember(_MemberBase): def all(self, request, segments): """/members//all/preferences""" if len(segments) == 0: - return http.not_found() + return NotFound(), [] if self._member is None: - return http.not_found() + return NotFound(), [] child = ReadOnlyPreferences( self._member, 'members/{0}/all'.format(self._member.member_id.int)) return child, [] - @resource.DELETE() - def delete(self, request): + def on_delete(self, request, response): """Delete the member (i.e. unsubscribe).""" # Leaving a list is a bit different than deleting a moderator or # owner. Handle the former case first. For now too, we will not send # an admin or user notification. if self._member is None: - return http.not_found() + falcon.responders.path_not_found(request, response) + return mlist = getUtility(IListManager).get_by_list_id(self._member.list_id) if self._member.role is MemberRole.member: try: delete_member(mlist, self._member.address.email, False, False) except NotAMemberError: - return http.not_found() + falcon.responders.path_not_found(request, response) + return else: self._member.unsubscribe() - return no_content() + response.status = falcon.HTTP_204 - @PATCH() - def patch_membership(self, request): + def on_patch(self, request, response): """Patch the membership. This is how subscription changes are done. """ if self._member is None: - return http.not_found() + falcon.responders.path_not_found(request, response) + return try: values = Validator( address=unicode, delivery_mode=enum_validator(DeliveryMode), _optional=('address', 'delivery_mode'))(request) except ValueError as error: - return http.bad_request([], str(error)) + falcon.responders.bad_request(request, response, body=str(error)) + return if 'address' in values: email = values['address'] address = getUtility(IUserManager).get_address(email) if address is None: - return http.bad_request([], b'Address not registered') + falcon.responders.bad_request( + request, response, body=b'Address not registered') + return try: self._member.address = address except (MembershipError, UnverifiedAddressError) as error: - return http.bad_request([], str(error)) + falcon.responders.bad_request( + request, response, body=str(error)) + return if 'delivery_mode' in values: self._member.preferences.delivery_mode = values['delivery_mode'] - return no_content() + response.status = falcon.HTTP_204 @@ -256,8 +262,7 @@ class _FoundMembers(MemberCollection): class FindMembers(_MemberBase): """/members/find""" - @resource.POST() - def find(self, request): + def on_post(self, request, response): """Find a member""" service = getUtility(ISubscriptionService) validator = Validator( @@ -265,10 +270,11 @@ class FindMembers(_MemberBase): subscriber=unicode, role=enum_validator(MemberRole), _optional=('list_id', 'subscriber', 'role')) - members = service.find_members(**validator(request)) - # We can't just return the _FoundMembers instance, because - # CollectionMixins have only a GET method, which is incompatible with - # this POSTed resource. IOW, without doing this here, restish would - # throw a 405 Method Not Allowed. - resource = _FoundMembers(members)._make_collection(request) - return http.ok([], etag(resource)) + try: + members = service.find_members(**validator(request)) + except ValueError as error: + falcon.responders.bad_request(response, body=str(error)) + else: + resource = _FoundMembers(members)._make_collection(request) + response.status = falcon.HTTP_200 + response.body = etag(resource) diff --git a/src/mailman/rest/moderation.py b/src/mailman/rest/moderation.py index 1a302ff3c..97371168d 100644 --- a/src/mailman/rest/moderation.py +++ b/src/mailman/rest/moderation.py @@ -28,16 +28,16 @@ __all__ = [ ] -from restish import http, resource -from zope.component import getUtility +import falcon from mailman.app.moderator import ( handle_message, handle_subscription, handle_unsubscription) from mailman.interfaces.action import Action from mailman.interfaces.messages import IMessageStore from mailman.interfaces.requests import IListRequests, RequestType -from mailman.rest.helpers import CollectionMixin, etag, no_content +from mailman.rest.helpers import CollectionMixin, child, etag from mailman.rest.validator import Validator, enum_validator +from zope.component import getUtility HELD_MESSAGE_REQUESTS = (RequestType.held_message,) @@ -101,45 +101,50 @@ class _HeldMessageBase(_ModerationBase): return resource -class HeldMessage(_HeldMessageBase, resource.Resource): +class HeldMessage(_HeldMessageBase): """Resource for moderating a held message.""" def __init__(self, mlist, request_id): self._mlist = mlist self._request_id = request_id - @resource.GET() - def details(self, request): + def on_get(self, request, response): try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + falcon.responders.bad_request(request, response) + return resource = self._make_resource(request_id) if resource is None: - return http.not_found() - return http.ok([], etag(resource)) + falcon.responders.path_not_found( + request, response, body=b'404 Not Found') + else: + response.status = falcon.HTTP_200 + response.body = etag(resource) - @resource.POST() - def moderate(self, request): + def on_post(self, request, response): try: validator = Validator(action=enum_validator(Action)) arguments = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + falcon.responders.bad_request(request, response, body=str(error)) + return requests = IListRequests(self._mlist) try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + falcon.responders.bad_request(request, response) + return results = requests.get_request(request_id, RequestType.held_message) if results is None: - return http.not_found() - handle_message(self._mlist, request_id, **arguments) - return no_content() + falcon.responders.path_not_found(request, response) + else: + handle_message(self._mlist, request_id, **arguments) + response.status = falcon.HTTP_204 -class HeldMessages(_HeldMessageBase, resource.Resource, CollectionMixin): +class HeldMessages(_HeldMessageBase, CollectionMixin): """Resource for messages held for moderation.""" def __init__(self, mlist): @@ -155,70 +160,74 @@ class HeldMessages(_HeldMessageBase, resource.Resource, CollectionMixin): self._requests = requests return list(requests.of_type(RequestType.held_message)) - @resource.GET() - def requests(self, request): + def on_get(self, request, response): """/lists/listname/held""" - # `request` is a restish.http.Request object. resource = self._make_collection(request) - return http.ok([], etag(resource)) + response.status = falcon.HTTP_200 + response.body = etag(resource) - @resource.child('{id}') + @child(r'^(?P[^/]+)') def message(self, request, segments, **kw): return HeldMessage(self._mlist, kw['id']) -class MembershipChangeRequest(resource.Resource, _ModerationBase): +class MembershipChangeRequest(_ModerationBase): """Resource for moderating a membership change.""" def __init__(self, mlist, request_id): self._mlist = mlist self._request_id = request_id - @resource.GET() - def details(self, request): + def on_get(self, request, response): try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + falcon.responders.bad_request(request, response) + return resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) if resource is None: - return http.not_found() - # Remove unnecessary keys. - del resource['key'] - return http.ok([], etag(resource)) + falcon.responders.path_not_found(request, response) + else: + # Remove unnecessary keys. + del resource['key'] + response.status = falcon.HTTP_200 + response.body = etag(resource) - @resource.POST() - def moderate(self, request): + def on_post(self, request, response): try: validator = Validator(action=enum_validator(Action)) arguments = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + falcon.responders.bad_request(request, response, body=str(error)) + return requests = IListRequests(self._mlist) try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + falcon.responders.bad_request(request, response) + return results = requests.get_request(request_id) if results is None: - return http.not_found() + falcon.responders.path_not_found(request, response) + return key, data = results try: request_type = RequestType[data['_request_type']] except ValueError: - return http.bad_request() + falcon.responders.bad_request(request, response) + return if request_type is RequestType.subscription: handle_subscription(self._mlist, request_id, **arguments) elif request_type is RequestType.unsubscription: handle_unsubscription(self._mlist, request_id, **arguments) else: - return http.bad_request() - return no_content() + falcon.responders.bad_request(request, response) + return + response.status = falcon.HTTP_204 -class SubscriptionRequests( - _ModerationBase, resource.Resource, CollectionMixin): +class SubscriptionRequests(_ModerationBase, CollectionMixin): """Resource for membership change requests.""" def __init__(self, mlist): @@ -241,13 +250,12 @@ class SubscriptionRequests( items.append(request) return items - @resource.GET() - def requests(self, request): + def on_get(self, request, response): """/lists/listname/requests""" - # `request` is a restish.http.Request object. resource = self._make_collection(request) - return http.ok([], etag(resource)) + response.status = falcon.HTTP_200 + response.body = etag(resource) - @resource.child('{id}') + @child(r'^(?P[^/]+)') def subscription(self, request, segments, **kw): return MembershipChangeRequest(self._mlist, kw['id']) diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py index c8dce0eca..5db59ad7d 100644 --- a/src/mailman/rest/root.py +++ b/src/mailman/rest/root.py @@ -28,7 +28,6 @@ __all__ = [ import falcon from base64 import b64decode -from restish import http from zope.component import getUtility from mailman.config import config @@ -77,7 +76,7 @@ class Root: raise falcon.HTTPUnauthorized( b'401 Unauthorized', b'User is not authorized for the REST API') - return TopLevel(), segments + return TopLevel() class System: @@ -179,10 +178,10 @@ class TopLevel: fqdn_listname, template = segments language = 'en' else: - return http.bad_request() + return BadRequest(), [] mlist = getUtility(IListManager).get(fqdn_listname) if mlist is None: - return http.not_found() + return NotFound(), [] # XXX dig out content-type from request content_type = None return TemplateFinder( diff --git a/src/mailman/rest/templates.py b/src/mailman/rest/templates.py index bee21d7de..85c698f07 100644 --- a/src/mailman/rest/templates.py +++ b/src/mailman/rest/templates.py @@ -25,11 +25,8 @@ __all__ = [ ] -import os +import falcon -from restish import http, resource - -from mailman.config import config from mailman.utilities.i18n import TemplateNotFoundError, find @@ -41,7 +38,7 @@ EXTENSIONS = { -class TemplateFinder(resource.Resource): +class TemplateFinder: """Template finder resource.""" def __init__(self, mlist, template, language, content_type): @@ -50,19 +47,20 @@ class TemplateFinder(resource.Resource): self.language = language self.content_type = content_type - @resource.GET() - def find_template(self, request): + def on_get(self, request, response): # XXX We currently only support .txt and .html files. extension = EXTENSIONS.get(self.content_type) if extension is None: - return http.not_found() + falcon.responders.path_not_found(request, response) + return template = self.template + extension fp = None try: try: path, fp = find(template, self.mlist, self.language) except TemplateNotFoundError: - return http.not_found() + falcon.responders.path_not_found(request, response) + return else: return fp.read() finally: diff --git a/src/mailman/rest/tests/test_configuration.py b/src/mailman/rest/tests/test_configuration.py new file mode 100644 index 000000000..93171ec4b --- /dev/null +++ b/src/mailman/rest/tests/test_configuration.py @@ -0,0 +1,94 @@ +# Copyright (C) 2014 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 . + +"""Test list configuration via the REST API.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'TestConfiguration', + ] + + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.database.transaction import transaction +from mailman.interfaces.mailinglist import IAcceptableAliasSet +from mailman.testing.helpers import call_api +from mailman.testing.layers import RESTLayer + + + +class TestConfiguration(unittest.TestCase): + """Test list configuration via the REST API.""" + + layer = RESTLayer + + def setUp(self): + with transaction(): + self._mlist = create_list('test@example.com') + + def test_put_configuration(self): + aliases = [ + 'ant@example.com', + 'bee@example.com', + 'cat@example.com', + ] + # When using PUT, all writable attributes must be included. + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@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='bee@example.com', + send_welcome_message=False, + subject_prefix='[ant]', + welcome_message_uri='mailman:///welcome.txt', + default_member_action='hold', + default_nonmember_action='discard', + ), + '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)) diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index 9426e7f27..ba6f6ea59 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -162,6 +162,24 @@ class TestLists(unittest.TestCase): method='DELETE') self.assertEqual(cm.exception.code, 404) + def test_roster(self): + # Lists have rosters which can be accessed by role. + with transaction(): + anne = self._usermanager.create_address('anne@example.com') + bart = self._usermanager.create_address('bart@example.com') + self._mlist.subscribe(anne) + self._mlist.subscribe(bart) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@example.com/roster/member') + self.assertEqual(resource['start'], 0) + self.assertEqual(resource['total_size'], 2) + member = resource['entries'][0] + self.assertEqual(member['email'], 'anne@example.com') + self.assertEqual(member['role'], 'member') + member = resource['entries'][1] + self.assertEqual(member['email'], 'bart@example.com') + self.assertEqual(member['role'], 'member') + class TestListArchivers(unittest.TestCase): diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 503d68a94..3c7d0520b 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -69,14 +69,12 @@ class TestMembership(unittest.TestCase): '/member/nobody@example.com', method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_list_with_bogus_address(self): # Try to leave a mailing list using an invalid membership address. with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/1', method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_a_list_twice(self): with transaction(): @@ -91,7 +89,6 @@ class TestMembership(unittest.TestCase): with self.assertRaises(HTTPError) as cm: call_api(url, method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_join_a_list_twice(self): with transaction(): diff --git a/src/mailman/rest/tests/test_paginate.py b/src/mailman/rest/tests/test_paginate.py index 3a166d9d2..a710307ca 100644 --- a/src/mailman/rest/tests/test_paginate.py +++ b/src/mailman/rest/tests/test_paginate.py @@ -36,8 +36,6 @@ from mailman.testing.layers import RESTLayer class _FakeRequest(Request): - """Fake restish.http.Request object.""" - def __init__(self, count=None, page=None): self._params = {} if count is not None: diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index e77987b45..cccd5ab98 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -29,7 +29,6 @@ __all__ = [ import falcon from passlib.utils import generate_password as generate -from restish import http from uuid import UUID from zope.component import getUtility @@ -40,7 +39,8 @@ from mailman.interfaces.address import ExistingAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.addresses import UserAddresses from mailman.rest.helpers import ( - CollectionMixin, GetterSetter, NotFound, child, etag, paginate, path_to) + BadRequest, CollectionMixin, GetterSetter, NotFound, child, etag, + paginate, path_to) from mailman.rest.preferences import Preferences from mailman.rest.validator import PatchValidator, Validator @@ -195,9 +195,9 @@ class AUser(_UserBase): def preferences(self, request, segments): """/addresses//preferences""" if len(segments) != 0: - return http.bad_request() + return BadRequest(), [] if self._user is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._user.preferences, 'users/{0}'.format(self._user.user_id.int)) diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index 94067fab0..ceb57e0fc 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -90,11 +90,7 @@ class Validator: # in the pre-converted dictionary. All keys which show up more than # once get a list value. missing = object() - # This is a gross hack to allow PATCH. See helpers.py for details. - try: - items = request.PATCH.items() - except AttributeError: - items = request.params.items() + items = request.params.items() for key, new_value in items: old_value = form_data.get(key, missing) if old_value is missing: diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py index 24ba6db3b..341a4cbb4 100644 --- a/src/mailman/rest/wsgiapp.py +++ b/src/mailman/rest/wsgiapp.py @@ -26,6 +26,7 @@ __all__ = [ ] +import re import falcon import logging @@ -42,6 +43,7 @@ from mailman.rest.root import Root log = logging.getLogger('mailman.http') _missing = object() +SLASH = '/' @@ -89,25 +91,60 @@ class RootedAPI(API): matcher = getattr(attribute, '__matcher__', _missing) if matcher is _missing: continue - if matcher == this_segment: - result = attribute(req, path_segments) - if isinstance(result, tuple): - resource, path_segments = result - else: - resource = result - # The method could have truncated the remaining segments, - # meaning, it's consumed all the path segments, or this is - # the last path segment. In that case the resource we're - # left at is the responder. - if len(path_segments) == 0: - # We're at the end of the path, so the root must be the - # responder. - method_map = create_http_method_map( - resource, None, None, None) - responder = method_map[method] - return responder, {}, resource - this_segment = path_segments.pop(0) - break + result = None + if isinstance(matcher, basestring): + # Is the matcher string a regular expression or plain + # string? If it starts with a caret, it's a regexp. + if matcher.startswith('^'): + cre = re.compile(matcher) + # Search against the entire remaining path. + tmp_path_segments = path_segments[:] + tmp_path_segments.insert(0, this_segment) + remaining_path = SLASH.join(tmp_path_segments) + mo = cre.match(remaining_path) + if mo: + result = attribute( + req, path_segments, **mo.groupdict()) + elif matcher == this_segment: + result = attribute(req, path_segments) + else: + # The matcher is a callable. It returns None if it + # doesn't match, and if it does, it returns a 3-tuple + # containing the positional arguments, the keyword + # arguments, and the remaining segments. The attribute is + # then called with these arguments. Note that the matcher + # wants to see the full remaining path components, which + # includes the current hop. + tmp_path_segments = path_segments[:] + tmp_path_segments.insert(0, this_segment) + matcher_result = matcher(req, tmp_path_segments) + if matcher_result is not None: + positional, keyword, path_segments = matcher_result + result = attribute( + req, path_segments, *positional, **keyword) + # The attribute could return a 2-tuple giving the resource and + # remaining path segments, or it could just return the + # result. Of course, if the result is None, then the matcher + # did not match. + if result is None: + continue + elif isinstance(result, tuple): + resource, path_segments = result + else: + resource = result + # The method could have truncated the remaining segments, + # meaning, it's consumed all the path segments, or this is the + # last path segment. In that case the resource we're left at + # is the responder. + if len(path_segments) == 0: + # We're at the end of the path, so the root must be the + # responder. + method_map = create_http_method_map( + resource, None, None, None) + responder = method_map[method] + return responder, {}, resource + this_segment = path_segments.pop(0) + break else: # None of the attributes matched this path component, so the # response is a 404. -- cgit v1.2.3-70-g09d2 From 9b19d1a9d77e71afc0783f22496bd623eda3024b Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 2 Nov 2014 15:29:10 -0500 Subject: Port to current Falcon git head. --- .bzrignore | 1 + src/mailman/rest/docs/helpers.rst | 30 ++++++++++++++++++------------ src/mailman/rest/tests/test_paginate.py | 10 +++++----- 3 files changed, 24 insertions(+), 17 deletions(-) (limited to 'src/mailman/rest/docs') diff --git a/.bzrignore b/.bzrignore index 862dd0b10..b348eb336 100644 --- a/.bzrignore +++ b/.bzrignore @@ -21,3 +21,4 @@ distribute-*.egg distribute-*.tar.gz .coverage htmlcov +.tox diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst index 1a2a244ac..68dfda1be 100644 --- a/src/mailman/rest/docs/helpers.rst +++ b/src/mailman/rest/docs/helpers.rst @@ -73,7 +73,7 @@ converting their values. >>> validator = Validator(one=int, two=unicode, three=bool) >>> class FakeRequest: - ... params = {} + ... params = None >>> FakeRequest.params = dict(one='1', two='two', three='yes') On valid input, the validator can be used as a ``**keyword`` argument. @@ -157,12 +157,17 @@ such form data. Specifically, when a key shows up multiple times in the form data, a list is given to the validator. :: - # Of course we can't use a normal dictionary, but webob has a useful data - # type we can use. - >>> from webob.multidict import MultiDict - >>> form_data = MultiDict(one='1', many='3') - >>> form_data.add('many', '4') - >>> form_data.add('many', '5') + # We can't use a normal dictionary because we'll have multiple keys, but + # the validator only wants to call .items() on the object. + >>> class MultiDict: + ... def __init__(self, *params): self.values = list(params) + ... def items(self): return iter(self.values) + >>> form_data = MultiDict( + ... ('one', '1'), + ... ('many', '3'), + ... ('many', '4'), + ... ('many', '5'), + ... ) This is a validation function that ensures the value is a list. @@ -191,11 +196,12 @@ And a validator to pull it all together. The list values are guaranteed to be in the same order they show up in the form data. - >>> from webob.multidict import MultiDict - >>> form_data = MultiDict(one='1', many='3') - >>> form_data.add('many', '5') - >>> form_data.add('many', '4') - >>> FakeRequest.params = form_data + >>> FakeRequest.params = MultiDict( + ... ('one', '1'), + ... ('many', '3'), + ... ('many', '5'), + ... ('many', '4'), + ... ) >>> values = validator(FakeRequest) >>> print(values['one']) 1 diff --git a/src/mailman/rest/tests/test_paginate.py b/src/mailman/rest/tests/test_paginate.py index a710307ca..e267100c7 100644 --- a/src/mailman/rest/tests/test_paginate.py +++ b/src/mailman/rest/tests/test_paginate.py @@ -27,7 +27,7 @@ __all__ = [ import unittest -from falcon import InvalidParam, Request +from falcon import HTTPInvalidParam, Request from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction from mailman.rest.helpers import paginate @@ -104,7 +104,7 @@ class TestPaginateHelper(unittest.TestCase): @paginate def get_collection(self, request): return [] - self.assertRaises(InvalidParam, get_collection, + self.assertRaises(HTTPInvalidParam, get_collection, None, _FakeRequest('two', 1)) def test_negative_count(self): @@ -112,7 +112,7 @@ class TestPaginateHelper(unittest.TestCase): @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(InvalidParam, get_collection, + self.assertRaises(HTTPInvalidParam, get_collection, None, _FakeRequest(-1, 1)) def test_negative_page(self): @@ -120,7 +120,7 @@ class TestPaginateHelper(unittest.TestCase): @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(InvalidParam, get_collection, + self.assertRaises(HTTPInvalidParam, get_collection, None, _FakeRequest(1, -1)) def test_negative_page_and_count(self): @@ -128,5 +128,5 @@ class TestPaginateHelper(unittest.TestCase): @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(InvalidParam, get_collection, + self.assertRaises(HTTPInvalidParam, get_collection, None, _FakeRequest(-1, -1)) -- cgit v1.2.3-70-g09d2 From 59f980abc3aa10e0ff8a3d38ed91d69f68b194ed Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 6 Nov 2014 20:39:25 -0500 Subject: When we switch to tox, we'll get hash randomization, which breaks many tests. While I'm shipping a tox.ini, do not yet use tox to run the tests, unless you want to help fix the randomizations. In the meantime, I'm also adding a few randomization fixes, and updating to the latest falcon git trunk. --- src/mailman/rest/docs/helpers.rst | 4 ++-- src/mailman/rest/docs/preferences.rst | 2 +- src/mailman/rest/helpers.py | 7 +++++-- src/mailman/rest/wsgiapp.py | 3 +-- tox.ini | 12 ++++++++++++ 5 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 tox.ini (limited to 'src/mailman/rest/docs') diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst index 68dfda1be..5bcf5cad4 100644 --- a/src/mailman/rest/docs/helpers.rst +++ b/src/mailman/rest/docs/helpers.rst @@ -45,7 +45,7 @@ gets modified to contain the etag under the ``http_etag`` key. >>> resource = dict(geddy='bass', alex='guitar', neil='drums') >>> json_data = etag(resource) >>> print(resource['http_etag']) - "43942176d8d5bb4414ccf35e2720ccd5251e66da" + "96e036d66248cab746b7d97047e08896fcfb2493" For convenience, the etag function also returns the JSON representation of the dictionary after tagging, since that's almost always what you want. @@ -58,7 +58,7 @@ dictionary after tagging, since that's almost always what you want. >>> dump_msgdata(data) alex : guitar geddy : bass - http_etag: "43942176d8d5bb4414ccf35e2720ccd5251e66da" + http_etag: "96e036d66248cab746b7d97047e08896fcfb2493" neil : drums diff --git a/src/mailman/rest/docs/preferences.rst b/src/mailman/rest/docs/preferences.rst index b8a0af500..b9332c954 100644 --- a/src/mailman/rest/docs/preferences.rst +++ b/src/mailman/rest/docs/preferences.rst @@ -162,7 +162,7 @@ deleted. >>> dump_json('http://localhost:9001/3.0/addresses/anne@example.com' ... '/preferences') acknowledge_posts: True - http_etag: "5219245d1eea98bc107032013af20ef91bfb5c51" + http_etag: "1ff07b0367bede79ade27d217e12df3915aaee2b" preferred_language: ja self_link: http://localhost:9001/3.0/addresses/anne@example.com/preferences diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index acc5106be..f67d9d448 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -46,6 +46,7 @@ from datetime import datetime, timedelta from enum import Enum from lazr.config import as_boolean from mailman.config import config +from pprint import pformat @@ -103,8 +104,10 @@ def etag(resource): :rtype string """ assert 'http_etag' not in resource, 'Resource already etagged' - etag = hashlib.sha1(repr(resource)).hexdigest() - + # Calculate the tag from a predictable (i.e. sorted) representation of the + # dictionary. The actual details aren't so important. pformat() is + # guaranteed to sort the keys. + etag = hashlib.sha1(pformat(resource)).hexdigest() resource['http_etag'] = '"{0}"'.format(etag) return json.dumps(resource, cls=ExtendedEncoder) diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py index d95926cfd..82f2a3a8f 100644 --- a/src/mailman/rest/wsgiapp.py +++ b/src/mailman/rest/wsgiapp.py @@ -30,7 +30,6 @@ import re import logging from falcon import API -from falcon.api_helpers import create_http_method_map from falcon.responders import path_not_found from mailman.config import config from mailman.database.transaction import transactional @@ -132,7 +131,7 @@ class RootedAPI(API): if len(path_segments) == 0: # We're at the end of the path, so the root must be the # responder. - method_map = create_http_method_map( + method_map = self._create_http_method_map( resource, None, None, None) responder = method_map[method] return responder, {}, resource diff --git a/tox.ini b/tox.ini new file mode 100644 index 000000000..881c90136 --- /dev/null +++ b/tox.ini @@ -0,0 +1,12 @@ +[tox] +envlist = py27 +recreate = True + +[testenv] +commands = python -m nose2 -v +#sitepackages = True +usedevelop = True +deps = + git+https://github.com/racker/falcon.git#egg=falcon +setenv = + PYTHONHASHSEED=100 -- cgit v1.2.3-70-g09d2