9 files changed, 97 insertions, 24 deletions

diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py
index bc9324971..47548e696 100644
--- a/src/mailman/rest/addresses.py
+++ b/src/mailman/rest/addresses.py
@@ -126,7 +126,7 @@ class AnAddress(_AddressBase):
     def memberships(self, request, segments):
         """/addresses/<email>/memberships"""
         if len(segments) != 0:
-            return BadRequest(), []
+            return NotFound(), []
         if self._address is None:
             return NotFound(), []
         return AddressMemberships(self._address)
@@ -188,19 +188,15 @@ class UserAddresses(_AddressBase):
 
     def on_get(self, request, response):
         """/addresses"""
-        if self._user is None:
-            not_found(response)
-        else:
-            okay(response, etag(self._make_collection(request)))
+        assert self._user is not None
+        okay(response, etag(self._make_collection(request)))
 
     def on_post(self, request, response):
         """POST to /addresses
 
         Add a new address to the user record.
         """
-        if self._user is None:
-            not_found(response)
-            return
+        assert self._user is not None
         user_manager = getUtility(IUserManager)
         validator = Validator(email=str,
                               display_name=str,
diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py
index a1e0d811e..6fc556b23 100644
--- a/src/mailman/rest/domains.py
+++ b/src/mailman/rest/domains.py
@@ -97,7 +97,7 @@ class ADomain(_DomainBase):
                 return NotFound()
             return OwnersForDomain(domain)
         else:
-            return BadRequest(), []
+            return NotFound(), []
 
 
 class AllDomains(_DomainBase):
@@ -122,8 +122,6 @@ class AllDomains(_DomainBase):
             domain = domain_manager.add(**values)
         except BadDomainSpecificationError as error:
             bad_request(response, str(error))
-        except ValueError as error:
-            bad_request(response, str(error))
         else:
             location = self.path_to('domains/{}'.format(domain.mail_host))
             created(response, location)
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index e519e4d7f..d853ae4cf 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -130,7 +130,7 @@ class CollectionMixin:
         :return: The representation of the resource.
         :rtype: dict
         """
-        raise NotImplementedError
+        raise NotImplementedError                   # pragma: no cover
 
     def _resource_as_json(self, resource):
         """Return the JSON formatted representation of the resource."""
@@ -147,7 +147,7 @@ class CollectionMixin:
         :return: The collection
         :rtype: list
         """
-        raise NotImplementedError
+        raise NotImplementedError                   # pragma: no cover
 
     def _paginate(self, request, collection):
         """Method to paginate through collection result lists.
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 2210d41b1..a998585b5 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -217,8 +217,6 @@ class AllLists(_ListBase):
         except BadDomainSpecificationError as error:
             reason = 'Domain does not exist: {}'.format(error.domain)
             bad_request(response, reason.encode('utf-8'))
-        except ValueError as error:
-            bad_request(response, str(error))
         else:
             location = self.path_to('lists/{0}'.format(mlist.list_id))
             created(response, location)
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index 4e9668bb1..1b812fcfa 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -100,7 +100,7 @@ class MemberCollection(_MemberBase):
     """
     def _get_collection(self, request):
         """See `CollectionMixin`."""
-        raise NotImplementedError
+        raise NotImplementedError                   # pragma: no cover
 
     def on_get(self, request, response):
         """roster/[members|owners|moderators]"""
@@ -170,11 +170,7 @@ class AMember(_MemberBase):
             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:
-                not_found(response)
-                return
+            delete_member(mlist, self._member.address.email, False, False)
         else:
             self._member.unsubscribe()
         no_content(response)
diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py
index 8f8dc95e6..3199664d5 100644
--- a/src/mailman/rest/tests/test_addresses.py
+++ b/src/mailman/rest/tests/test_addresses.py
@@ -28,7 +28,7 @@ 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.helpers import call_api, subscribe
 from mailman.testing.layers import RESTLayer
 from mailman.utilities.datetime import now
 from urllib.error import HTTPError
@@ -63,6 +63,13 @@ class TestAddresses(unittest.TestCase):
                      'nobody@example.com/memberships')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_membership_of_address_with_no_user(self):
+        with transaction():
+            getUtility(IUserManager).create_address('anne@example.com')
+        response, content = call_api(
+            'http://localhost:9001/3.0/addresses/anne@example.com/memberships')
+        self.assertEqual(response['total_size'], 0)
+
     def test_verify_a_missing_address(self):
         # POSTing to the 'verify' sub-resource returns a 404.
         with self.assertRaises(HTTPError) as cm:
@@ -434,6 +441,40 @@ class TestAddresses(unittest.TestCase):
                 method='DELETE')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_bad_memberships_url(self):
+        with transaction():
+            subscribe(self._mlist, 'Anne')
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/addresses/'
+                     'aperson@example.com/memberships/bogus')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_bad_preferences_url(self):
+        with transaction():
+            subscribe(self._mlist, 'Anne')
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/addresses/'
+                     'aperson@example.com/preferences/bogus')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_bad_preferences_address(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/addresses/'
+                     'nobody@example.com/preferences')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_bad_user_address(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/addresses/'
+                     'nobody@example.com/user')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_bad_user_addresses_url(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/users/'
+                     'nobody@example.com/addresses')
+        self.assertEqual(cm.exception.code, 404)
+
 
 
 class TestAPI31Addresses(unittest.TestCase):
diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py
index 9d03859ef..5a01783d3 100644
--- a/src/mailman/rest/tests/test_domains.py
+++ b/src/mailman/rest/tests/test_domains.py
@@ -106,6 +106,13 @@ class TestDomains(unittest.TestCase):
                 'http://localhost:9001/3.0/domains/does-not-exist.com/lists')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_create_existing_domain(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/domains', dict(
+                mail_host='example.com',
+                ))
+        self.assertEqual(cm.exception.code, 400)
+
     def test_double_delete(self):
         # You cannot delete a domain twice.
         content, response = call_api(
@@ -118,7 +125,6 @@ class TestDomains(unittest.TestCase):
         self.assertEqual(cm.exception.code, 404)
 
 
-
 
 class TestDomainOwners(unittest.TestCase):
     layer = RESTLayer
@@ -129,6 +135,12 @@ class TestDomainOwners(unittest.TestCase):
             call_api('http://localhost:9001/3.0/domains/example.net/owners')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_bad_domain_owners_url(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api(
+                'http://localhost:9001/3.0/domains/example.com/owners/bogus')
+        self.assertEqual(cm.exception.code, 404)
+
     def test_post_to_missing_domain_owners(self):
         # Try to add owners to a missing domain.
         with self.assertRaises(HTTPError) as cm:
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index d0eba7c11..4d22c2007 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -218,6 +218,12 @@ class TestLists(unittest.TestCase):
             call_api('http://localhost:9001/3.0/lists/bogus.example.com')
         self.assertEqual(cm.exception.code, 404)
 
+    def test_not_found_member_role(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/lists/test.example.com'
+                     '/owner/nobody@example.com')
+        self.assertEqual(cm.exception.code, 404)
+
 
 
 class TestListArchivers(unittest.TestCase):
diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py
index 1c27c538f..bd7bb9dad 100644
--- a/src/mailman/rest/tests/test_membership.py
+++ b/src/mailman/rest/tests/test_membership.py
@@ -29,7 +29,7 @@ import unittest
 from mailman.app.lifecycle import create_list
 from mailman.config import config
 from mailman.database.transaction import transaction
-from mailman.interfaces.member import DeliveryMode
+from mailman.interfaces.member import DeliveryMode, MemberRole
 from mailman.interfaces.usermanager import IUserManager
 from mailman.testing.helpers import (
     TestableMaster, call_api, get_lmtp_client, make_testable_runner,
@@ -283,6 +283,32 @@ class TestMembership(unittest.TestCase):
         self.assertEqual(cm.exception.reason,
                          b'Cannot convert parameters: moderation_action')
 
+    def test_bad_preferences_url(self):
+        with transaction():
+            subscribe(self._mlist, 'Anne')
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members/1/preferences/bogus')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_not_a_member_preferences(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members/1/preferences')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_not_a_member_all_preferences(self):
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members/1/all/preferences')
+        self.assertEqual(cm.exception.code, 404)
+
+    def test_delete_other_role(self):
+        with transaction():
+            subscribe(self._mlist, 'Anne', MemberRole.moderator)
+        response, headers = call_api(
+            'http://localhost:9001/3.0/members/1',
+            method='DELETE')
+        self.assertEqual(headers.status, 204)
+        self.assertEqual(len(list(self._mlist.moderators.members)), 0)
+
 
 
 class CustomLayer(ConfigLayer):