10 files changed, 123 insertions, 74 deletions

diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index d7508a533..7b5e26a7f 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -149,7 +149,7 @@ testing: no
 # Time-outs for starting up various test subprocesses, such as the LMTP and
 # REST servers.  This is only used for the test suite, so if you're seeing
 # test failures, try increasing the wait time.
-wait: 10s
+wait: 60s
 
 
 [passwords]
diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py
index 92772d6f3..6e9eb8872 100644
--- a/src/mailman/rest/addresses.py
+++ b/src/mailman/rest/addresses.py
@@ -27,6 +27,8 @@ __all__ = [
     ]
 
 
+import falcon
+
 from operator import attrgetter
 from restish import http, resource
 from zope.component import getUtility
@@ -34,7 +36,9 @@ from zope.component import getUtility
 from mailman.interfaces.address import (
     ExistingAddressError, InvalidEmailAddressError)
 from mailman.interfaces.usermanager import IUserManager
-from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to
+from mailman.rest.helpers import (
+    BadRequest, CollectionMixin, NotFound, child, etag, path_not_found,
+    path_to)
 from mailman.rest.members import MemberCollection
 from mailman.rest.preferences import Preferences
 from mailman.rest.validator import Validator
@@ -42,7 +46,7 @@ from mailman.utilities.datetime import now
 
 
 
-class _AddressBase(resource.Resource, CollectionMixin):
+class _AddressBase(CollectionMixin):
     """Shared base class for address representations."""
 
     def _resource_as_dict(self, address):
@@ -72,11 +76,11 @@ class _AddressBase(resource.Resource, CollectionMixin):
 class AllAddresses(_AddressBase):
     """The addresses."""
 
-    @resource.GET()
-    def collection(self, request):
+    def on_get(self, request, response):
         """/addresses"""
         resource = self._make_collection(request)
-        return http.ok([], etag(resource))
+        response.status = falcon.HTTP_200
+        response.body = etag(resource)
 
 
 
@@ -88,14 +92,13 @@ class _VerifyResource(resource.Resource):
         self._action = action
         assert action in ('verify', 'unverify')
 
-    @resource.POST()
-    def verify(self, request):
+    def on_post(self, request, response):
         # We don't care about the POST data, just do the action.
         if self._action == 'verify' and self._address.verified_on is None:
             self._address.verified_on = now()
         elif self._action == 'unverify':
             self._address.verified_on = None
-        return no_content()
+        response.status = falcon.HTTP_204
 
 
 class AnAddress(_AddressBase):
@@ -109,20 +112,21 @@ class AnAddress(_AddressBase):
         """
         self._address = getUtility(IUserManager).get_address(email)
 
-    @resource.GET()
-    def address(self, request):
+    def on_get(self, request, response):
         """Return a single address."""
         if self._address is None:
-            return http.not_found()
-        return http.ok([], self._resource_as_json(self._address))
+            path_not_found(request, response)
+        else:
+            response.status = falcon.HTTP_200
+            response.body = self._resource_as_json(self._address)
 
-    @resource.child()
+    @child()
     def memberships(self, request, segments):
         """/addresses/<email>/memberships"""
         if len(segments) != 0:
-            return http.bad_request()
+            return BadRequest(), []
         if self._address is None:
-            return http.not_found()
+            return NotFound(), []
         return AddressMemberships(self._address)
 
     @resource.child()
@@ -137,23 +141,23 @@ class AnAddress(_AddressBase):
             'addresses/{0}'.format(self._address.email))
         return child, []
 
-    @resource.child()
+    @child()
     def verify(self, request, segments):
         """/addresses/<email>/verify"""
         if len(segments) != 0:
-            return http.bad_request()
+            return BadRequest(), []
         if self._address is None:
-            return http.not_found()
+            return NotFound(), []
         child = _VerifyResource(self._address, 'verify')
         return child, []
 
-    @resource.child()
+    @child()
     def unverify(self, request, segments):
         """/addresses/<email>/verify"""
         if len(segments) != 0:
-            return http.bad_request()
+            return BadRequest(), []
         if self._address is None:
-            return http.not_found()
+            return NotFound(), []
         child = _VerifyResource(self._address, 'unverify')
         return child, []
 
@@ -171,20 +175,23 @@ class UserAddresses(_AddressBase):
         return sorted(self._user.addresses,
                       key=attrgetter('original_email'))
 
-    @resource.GET()
-    def collection(self, request):
+    def on_get(self, request, response):
         """/addresses"""
-        resource = self._make_collection(request)
-        return http.ok([], etag(resource))
+        if self._user is None:
+            path_not_found(request, response)
+        else:
+            response.status = falcon.HTTP_200
+            resource = self._make_collection(request)
+            response.body = etag(resource)
 
-    @resource.POST()
-    def create(self, request):
+    def on_post(self, request, response):
         """POST to /addresses
 
         Add a new address to the user record.
         """
         if self._user is None:
-            return http.not_found()
+            path_not_found(request, response)
+            return
         user_manager = getUtility(IUserManager)
         validator = Validator(email=unicode,
                               display_name=unicode,
@@ -201,7 +208,8 @@ class UserAddresses(_AddressBase):
             # Link the address to the current user and return it.
             address.user = self._user
             location = path_to('addresses/{0}'.format(address.email))
-            return http.created(location, [], None)
+            response.status = falcon.HTTP_201
+            response.location = location
 
 
 
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index 025ad1779..af7392f5a 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -21,8 +21,12 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'BadRequest',
+    'ChildError',
     'GetterSetter',
+    'NotFound',
     'PATCH',
+    'child',
     'etag',
     'no_content',
     'path_to',
@@ -32,6 +36,7 @@ __all__ = [
 
 import cgi
 import json
+import falcon
 import hashlib
 
 from cStringIO import StringIO
@@ -300,3 +305,40 @@ class GetterSetter:
         if self.decoder is None:
             return value
         return self.decoder(value)
+
+
+
+# Falcon REST framework add-ons.
+
+def child(matcher=None):
+    def decorator(func):
+        if matcher is None:
+            func.__matcher__ = func.__name__
+        else:
+            func.__matcher__ = matcher
+        return func
+    return decorator
+
+
+class ChildError:
+    def __init__(self, status):
+        self._status = status
+
+    def on_get(self, request, response):
+        raise falcon.HTTPError(self._status, None)
+
+
+class BadRequest(ChildError):
+    def __init__(self):
+        super(BadRequest, self).__init__(falcon.HTTP_400)
+
+
+class NotFound(ChildError):
+    def __init__(self):
+        super(NotFound, self).__init__(falcon.HTTP_404)
+
+
+def path_not_found(request, response, **kws):
+    # Like falcon.responders.path_not_found() but sets the body.
+    response.status = falcon.HTTP_404
+    response.body = b'404 Not Found'
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index 51cbbd888..ae44ee2d5 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -28,6 +28,8 @@ __all__ = [
     ]
 
 
+import falcon
+
 from uuid import UUID
 from operator import attrgetter
 from restish import http, resource
@@ -94,11 +96,11 @@ class MemberCollection(_MemberBase):
         """See `CollectionMixin`."""
         raise NotImplementedError
 
-    @resource.GET()
-    def container(self, request):
+    def on_get(self, request, response):
         """roster/[members|owners|moderators]"""
         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/root.py b/src/mailman/rest/root.py
index 6b13d8246..9e9f5bfa5 100644
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -38,7 +38,7 @@ from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.styles import IStyleManager
 from mailman.rest.addresses import AllAddresses, AnAddress
 from mailman.rest.domains import ADomain, AllDomains
-from mailman.rest.helpers import etag, path_to
+from mailman.rest.helpers import BadRequest, NotFound, child, etag, path_to
 from mailman.rest.lists import AList, AllLists
 from mailman.rest.members import AMember, AllMembers, FindMembers
 from mailman.rest.preferences import ReadOnlyPreferences
@@ -46,23 +46,6 @@ from mailman.rest.templates import TemplateFinder
 from mailman.rest.users import AUser, AllUsers
 
 
-def child(matcher=None):
-    def decorator(func):
-        if matcher is None:
-            func.__matcher__ = func.__name__
-        else:
-            func.__matcher__ = matcher
-        return func
-    return decorator
-
-
-class BadRequest:
-    def on_get(self, request, response):
-        raise falcon.HTTPError(falcon.HTTP_400, None)
-
-STOP = []
-
-
 
 class Root:
     """The RESTful root resource.
@@ -117,13 +100,13 @@ class TopLevel:
     def system(self, request, segments):
         """/<api>/system"""
         if len(segments) == 0:
-            return System(), STOP
+            return System()
         elif len(segments) > 1:
-            return BadRequest(), STOP
+            return BadRequest(), []
         elif segments[0] == 'preferences':
-            return ReadOnlyPreferences(system_preferences, 'system'), STOP
+            return ReadOnlyPreferences(system_preferences, 'system'), []
         else:
-            return BadRequest(), STOP
+            return NotFound(), []
 
     @child()
     def addresses(self, request, segments):
diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py
index 198674b22..8f43ffc9a 100644
--- a/src/mailman/rest/tests/test_addresses.py
+++ b/src/mailman/rest/tests/test_addresses.py
@@ -46,6 +46,13 @@ class TestAddresses(unittest.TestCase):
         with transaction():
             self._mlist = create_list('test@example.com')
 
+    def test_no_addresses(self):
+        # At first, there are no addresses.
+        url = 'http://localhost:9001/3.0/addresses'
+        json, response = call_api(url)
+        self.assertEqual(json['start'], 0)
+        self.assertEqual(json['total_size'], 0)
+
     def test_membership_of_missing_address(self):
         # Try to get the memberships of a missing address.
         with self.assertRaises(HTTPError) as cm:
@@ -111,7 +118,7 @@ class TestAddresses(unittest.TestCase):
         self.assertEqual(cm.exception.code, 400)
 
     def test_address_added_to_user(self):
-        # Address is added to a user record.
+        # An  address is added to a user record.
         user_manager = getUtility(IUserManager)
         with transaction():
             anne = user_manager.create_user('anne@example.com')
@@ -184,6 +191,13 @@ class TestAddresses(unittest.TestCase):
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.reason, 'Missing parameters: email')
 
+    def test_get_addresses_of_missing_user(self):
+        # There is no user associated with the given address.
+        with self.assertRaises(HTTPError) as cm:
+            call_api(
+                'http://localhost:9001/3.0/users/anne@example.com/addresses')
+        self.assertEqual(cm.exception.code, 404)
+
     def test_add_address_to_missing_user(self):
         # The user that the address is being added to must exist.
         with self.assertRaises(HTTPError) as cm:
diff --git a/src/mailman/rest/tests/test_root.py b/src/mailman/rest/tests/test_root.py
index 7d9124e02..d4d25ede0 100644
--- a/src/mailman/rest/tests/test_root.py
+++ b/src/mailman/rest/tests/test_root.py
@@ -66,7 +66,7 @@ class TestRoot(unittest.TestCase):
         # /system/foo where `foo` is not `preferences`.
         with self.assertRaises(HTTPError) as cm:
             call_api('http://localhost:9001/3.0/system/foo')
-        self.assertEqual(cm.exception.code, 400)
+        self.assertEqual(cm.exception.code, 404)
 
     def test_system_preferences_are_read_only(self):
         # /system/preferences are read-only.
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index bf9668564..ca38d72dc 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -38,7 +38,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, PATCH, etag, no_content, paginate, path_to)
+    CollectionMixin, GetterSetter, NotFound, PATCH, child, etag, no_content,
+    paginate, path_to)
 from mailman.rest.preferences import Preferences
 from mailman.rest.validator import PatchValidator, Validator
 
@@ -63,7 +64,7 @@ ATTRIBUTES = dict(
 
 
 
-class _UserBase(resource.Resource, CollectionMixin):
+class _UserBase(CollectionMixin):
     """Shared base class for user representations."""
 
     def _resource_as_dict(self, user):
@@ -164,11 +165,11 @@ class AUser(_UserBase):
             return http.not_found()
         return http.ok([], self._resource_as_json(self._user))
 
-    @resource.child()
+    @child()
     def addresses(self, request, segments):
         """/users/<uid>/addresses"""
         if self._user is None:
-            return http.not_found()
+            return NotFound(), []
         return UserAddresses(self._user)
 
     @resource.DELETE()
diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py
index 90d0334e9..9f15289c9 100644
--- a/src/mailman/rest/validator.py
+++ b/src/mailman/rest/validator.py
@@ -94,7 +94,7 @@ class Validator:
         try:
             items = request.PATCH.items()
         except AttributeError:
-            items = request.POST.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 8c2ee1758..3bd77bbcc 100644
--- a/src/mailman/rest/wsgiapp.py
+++ b/src/mailman/rest/wsgiapp.py
@@ -30,23 +30,18 @@ import logging
 
 from falcon import API
 from falcon.api_helpers import create_http_method_map
-from falcon.status_codes import HTTP_404
 
 from wsgiref.simple_server import WSGIRequestHandler
 from wsgiref.simple_server import make_server as wsgi_server
 
 from mailman.config import config
 from mailman.database.transaction import transactional
+from mailman.rest.helpers import path_not_found
 from mailman.rest.root import Root
 
 
 log = logging.getLogger('mailman.http')
-
-
-def path_not_found(request, response, **kws):
-    # Like falcon.responders.path_not_found() but sets the body.
-    response.status = HTTP_404
-    response.body = b'404 Not Found'
+_missing = object()
 
 
 
@@ -89,13 +84,17 @@ class RootedAPI(API):
             for name in dir(resource):
                 if name.startswith('__') and name.endswith('__'):
                     continue
-                attribute = getattr(resource, name)
-                assert attribute is not None, name
-                matcher = getattr(attribute, '__matcher__', None)
-                if matcher is None:
+                attribute = getattr(resource, name, _missing)
+                assert attribute is not _missing, name
+                matcher = getattr(attribute, '__matcher__', _missing)
+                if matcher is _missing:
                     continue
                 if matcher == this_segment:
-                    resource, path_segments = attribute(req, path_segments)
+                    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