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 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