6 files changed, 223 insertions, 112 deletions

diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index eaed35312..e2e3f17e1 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -187,6 +187,9 @@ REST
    With thanks to Anirudh Dahiya.  (Closes #199)
  * Expose the system pipelines and chains via ``<api>/system/pipelines`` and
    ``<api>/system/chains`` respectively.  Given by Simon Hanna.  (Closes #66)
+ * Support mass unsubscription of members via ``DELETE`` on the
+   ``<api>/lists/<list-id>/roster/member`` resource.  Given by Harshit
+   Bansal.  (Closes #171)
  * Port to Falcon 1.0 (Closes #20)
 
 Other
diff --git a/src/mailman/model/subscriptions.py b/src/mailman/model/subscriptions.py
index e399e3285..d3fdc3a8a 100644
--- a/src/mailman/model/subscriptions.py
+++ b/src/mailman/model/subscriptions.py
@@ -161,7 +161,8 @@ class SubscriptionService:
         q_member = store.query(Member).filter(
             Member.list_id == list_id,
             Member.role == MemberRole.member)
-        for email in emails:
+        # De-duplicate.
+        for email in set(emails):
             unsubscribed = False
             # Join with a queries matching the email address and preferred
             # address of any subscribed user.
diff --git a/src/mailman/model/tests/test_subscriptions.py b/src/mailman/model/tests/test_subscriptions.py
index 49dd03625..b0a68e3f9 100644
--- a/src/mailman/model/tests/test_subscriptions.py
+++ b/src/mailman/model/tests/test_subscriptions.py
@@ -397,6 +397,38 @@ class TestSubscriptionService(unittest.TestCase):
             [address.email for address in bee_owners.addresses],
             ['anne_1@example.com'])
 
+    def test_unsubscribe_members_with_duplicates(self):
+        ant = create_list('ant@example.com')
+        ant.admin_immed_notify = False
+        anne = self._user_manager.create_user('anne@example.com')
+        set_preferred(anne)
+        ant.subscribe(anne, MemberRole.member)
+        # Now we try to unsubscribe Anne twice in the same call.  That's okay
+        # because duplicates are ignored.
+        success, fail = self._service.unsubscribe_members(
+            ant.list_id, [
+                'anne@example.com',
+                'anne@example.com',
+                ])
+        self.assertEqual(success, set(['anne@example.com']))
+        self.assertEqual(fail, set())
+
+    def test_unsubscribe_members_with_duplicate_failures(self):
+        ant = create_list('ant@example.com')
+        ant.admin_immed_notify = False
+        anne = self._user_manager.create_user('anne@example.com')
+        set_preferred(anne)
+        ant.subscribe(anne, MemberRole.member)
+        # Now we try to unsubscribe a nonmember twice in the same call.
+        # That's okay because duplicates are ignored.
+        success, fail = self._service.unsubscribe_members(
+            ant.list_id, [
+                'bart@example.com',
+                'bart@example.com',
+                ])
+        self.assertEqual(success, set())
+        self.assertEqual(fail, set(['bart@example.com']))
+
     def test_find_members_issue_227(self):
         # A user is subscribed to a list with their preferred address.  They
         # have a different secondary linked address which is not subscribed.
diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst
index 0a5fb34d9..e19fa4623 100644
--- a/src/mailman/rest/docs/membership.rst
+++ b/src/mailman/rest/docs/membership.rst
@@ -771,53 +771,6 @@ Elly is no longer a member of the mailing list.
     set()
 
 
-Mass Unsubscriptions
-====================
-A batch of users can be unsubscribed from the mailing list via the REST API
-just by supplying their email addresses.
-
-    >>> m_list = create_list('mytest@example.com')
-    >>> subscribe(m_list, 'Joe')
-    <Member: Joe Person <jperson@example.com> on mytest@example.com as MemberRole.member>
-    >>> subscribe(m_list, 'Ken')
-    <Member: Ken Person <kperson@example.com> on mytest@example.com as MemberRole.member>
-    >>> subscribe(m_list, 'Mat')
-    <Member: Mat Person <mperson@example.com> on mytest@example.com as MemberRole.member>
-    >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com'
-    ...           '/roster/member', {'emails': ['kperson@example.com',
-    ...                                         'mperson@example.com',
-    ...                                         'kperson@example.com',
-    ...                                         'tim@example.com']}, 'DELETE')
-    http_etag: "..."
-    kperson@example.com: Member already deleted.
-    tim@example.com: No such member.
-    >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com'
-    ...     '/roster/member')
-    entry 0:
-        address: http://localhost:9001/3.0/addresses/jperson@example.com
-        delivery_mode: regular
-        email: jperson@example.com
-        http_etag: "..."
-        list_id: mytest.example.com
-        member_id: 10
-        role: member
-        self_link: http://localhost:9001/3.0/members/10
-        user: http://localhost:9001/3.0/users/7
-    ...
-    total_size: 1
-
-Ken and Mat have been unsubscribed successfully while tim@example.com can't
-be unsubscribed since he was not a member of the list. Similarly Joe can be
-unsubscribed from the mailing list.
-
-    >>> dump_json('http://localhost:9001/3.0/lists/mytest.example.com'
-    ...     '/roster/member', {'emails': ['jperson@example.com']}, 'DELETE')
-    content-length: 0
-    date: ...
-    server: ...
-    status: 204
-
-
 Changing delivery address
 =========================
 
@@ -856,10 +809,10 @@ addresses.
         email: herb@example.com
         http_etag: "..."
         list_id: ant.example.com
-        member_id: 13
+        member_id: 10
         role: member
-        self_link: http://localhost:9001/3.0/members/13
-        user: http://localhost:9001/3.0/users/10
+        self_link: http://localhost:9001/3.0/members/10
+        user: http://localhost:9001/3.0/users/7
     ...
     entry 9:
         address: http://localhost:9001/3.0/addresses/herb@example.com
@@ -867,10 +820,10 @@ addresses.
         email: herb@example.com
         http_etag: "..."
         list_id: bee.example.com
-        member_id: 14
+        member_id: 11
         role: member
-        self_link: http://localhost:9001/3.0/members/14
-        user: http://localhost:9001/3.0/users/10
+        self_link: http://localhost:9001/3.0/members/11
+        user: http://localhost:9001/3.0/users/7
     http_etag: "..."
     start: 0
     total_size: 10
@@ -884,13 +837,13 @@ Herb must iterate through his memberships explicitly.
     >>> memberships = [entry['self_link'] for entry in content['entries']]
     >>> for url in sorted(memberships):
     ...     print(url)
-    http://localhost:9001/3.0/members/13
-    http://localhost:9001/3.0/members/14
+    http://localhost:9001/3.0/members/10
+    http://localhost:9001/3.0/members/11
 
 For each membership resource, the subscription address is changed by PATCH'ing
 the `address` attribute.
 
-    >>> dump_json('http://localhost:9001/3.0/members/13', {
+    >>> dump_json('http://localhost:9001/3.0/members/10', {
     ...           'address': 'hperson@example.com',
     ...           }, method='PATCH')
     content-length: 0
@@ -898,7 +851,7 @@ the `address` attribute.
     server: ...
     status: 204
 
-    >>> dump_json('http://localhost:9001/3.0/members/14', {
+    >>> dump_json('http://localhost:9001/3.0/members/11', {
     ...           'address': 'hperson@example.com',
     ...           }, method='PATCH')
     content-length: 0
@@ -925,20 +878,20 @@ his membership ids have not changed.
         email: hperson@example.com
         http_etag: "..."
         list_id: ant.example.com
-        member_id: 13
+        member_id: 10
         role: member
-        self_link: http://localhost:9001/3.0/members/13
-        user: http://localhost:9001/3.0/users/10
+        self_link: http://localhost:9001/3.0/members/10
+        user: http://localhost:9001/3.0/users/7
     entry 1:
         address: http://localhost:9001/3.0/addresses/hperson@example.com
         delivery_mode: regular
         email: hperson@example.com
         http_etag: "..."
         list_id: bee.example.com
-        member_id: 14
+        member_id: 11
         role: member
-        self_link: http://localhost:9001/3.0/members/14
-        user: http://localhost:9001/3.0/users/10
+        self_link: http://localhost:9001/3.0/members/11
+        user: http://localhost:9001/3.0/users/7
     http_etag: "..."
     start: 0
     total_size: 2
@@ -947,7 +900,7 @@ When changing his subscription address, Herb may also decide to change his
 mode of delivery.
 ::
 
-    >>> dump_json('http://localhost:9001/3.0/members/14', {
+    >>> dump_json('http://localhost:9001/3.0/members/11', {
     ...           'address': 'herb@example.com',
     ...           'delivery_mode': 'mime_digests',
     ...           }, method='PATCH')
@@ -964,10 +917,10 @@ mode of delivery.
         email: herb@example.com
         http_etag: "..."
         list_id: bee.example.com
-        member_id: 14
+        member_id: 11
         role: member
-        self_link: http://localhost:9001/3.0/members/14
-        user: http://localhost:9001/3.0/users/10
+        self_link: http://localhost:9001/3.0/members/11
+        user: http://localhost:9001/3.0/users/7
     http_etag: "..."
     start: 0
     total_size: 1
@@ -980,22 +933,22 @@ The moderation action for a member can be changed by PATCH'ing the
 `moderation_action` attribute.  When the member action falls back to the list
 default, there is no such attribute in the resource.
 
-    >>> dump_json('http://localhost:9001/3.0/members/13')
+    >>> dump_json('http://localhost:9001/3.0/members/10')
     address: http://localhost:9001/3.0/addresses/hperson@example.com
     delivery_mode: regular
     email: hperson@example.com
     http_etag: "..."
     list_id: ant.example.com
-    member_id: 13
+    member_id: 10
     role: member
-    self_link: http://localhost:9001/3.0/members/13
-    user: http://localhost:9001/3.0/users/10
+    self_link: http://localhost:9001/3.0/members/10
+    user: http://localhost:9001/3.0/users/7
 
 Patching the moderation action both changes it for the given user, and adds
 the attribute to the member's resource.
 ::
 
-    >>> dump_json('http://localhost:9001/3.0/members/13', {
+    >>> dump_json('http://localhost:9001/3.0/members/10', {
     ...           'moderation_action': 'hold',
     ...           }, method='PATCH')
     content-length: 0
@@ -1003,7 +956,7 @@ the attribute to the member's resource.
     server: ...
     status: 204
 
-    >>> dump_json('http://localhost:9001/3.0/members/13')
+    >>> dump_json('http://localhost:9001/3.0/members/10')
     address: http://localhost:9001/3.0/addresses/hperson@example.com
     ...
     moderation_action: hold
@@ -1102,3 +1055,56 @@ As with list-centric bans, you can delete a global ban.
     http_etag: "..."
     start: 0
     total_size: 0
+
+
+Mass Unsubscriptions
+====================
+
+A batch of users can be unsubscribed from the mailing list via the REST API
+just by supplying their email addresses.
+::
+    >>> cat = create_list('cat@example.com')
+    >>> subscribe(cat, 'Isla')
+    <Member: Isla Person <iperson@example.com> on
+             cat@example.com as MemberRole.member>
+    >>> subscribe(cat, 'John')
+    <Member: John Person <jperson@example.com> on
+             cat@example.com as MemberRole.member>
+    >>> subscribe(cat, 'Kate')
+    <Member: Kate Person <kperson@example.com> on
+             cat@example.com as MemberRole.member>
+
+There are three new members of the mailing list.  We try to mass delete them,
+plus one other address that isn't a member of the list.  We get back a
+dictionary mapping email addresses to the success or failure of the removal
+operation.  It's okay that one of the addresses is removed twice.
+
+    >>> dump_json(
+    ...     'http://localhost:9001/3.0/lists/cat.example.com/roster/member', {
+    ...     'emails': ['iperson@example.com',
+    ...                'jperson@example.com',
+    ...                'iperson@example.com',
+    ...                'zperson@example.com',
+    ...                ]},
+    ...     'DELETE')
+    http_etag: "..."
+    iperson@example.com: True
+    jperson@example.com: True
+    zperson@example.com: False
+
+And now only Kate is still a member.
+
+    >>> dump_json(
+    ...     'http://localhost:9001/3.0/lists/cat.example.com/roster/member')
+    entry 0:
+        address: http://localhost:9001/3.0/addresses/kperson@example.com
+        delivery_mode: regular
+        email: kperson@example.com
+        http_etag: "..."
+        list_id: cat.example.com
+        member_id: 14
+        role: member
+        self_link: http://localhost:9001/3.0/members/14
+        user: http://localhost:9001/3.0/users/10
+    ...
+    total_size: 1
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 4f0c56569..69d14e548 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -251,23 +251,19 @@ class MembersOfList(MemberCollection):
     def on_delete(self, request, response):
         """Delete the members of the named mailing list."""
         status = {}
-        success = []
-        fail = []
         try:
             validator = Validator(emails=list_of_strings_validator)
             arguments = validator(request)
-            emails = arguments.pop('emails')
-        except ValueError:
-            return bad_request(response, b'Invalid Input.')
+        except ValueError as error:
+            bad_request(response, str(error))
+            return
+        emails = arguments.pop('emails')
         success, fail = getUtility(ISubscriptionService).unsubscribe_members(
             self._mlist.list_id, emails)
-        if len(fail) == 0:
-            return no_content(response)
-        for email in fail:
-            if email in success:
-                status[email] = 'Member already deleted.'
-            else:
-                status[email] = 'No such member.'
+        # There should be no email in both sets.
+        assert success.isdisjoint(fail), (success, fail)
+        status.update({email: True for email in success})
+        status.update({email: False for email in fail})
         okay(response, etag(status))
 
 
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index 207743e50..891afa889 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -213,38 +213,111 @@ class TestLists(unittest.TestCase):
                      '/owner/nobody@example.com')
         self.assertEqual(cm.exception.code, 404)
 
-    def test_list_mass_unsubscribe(self):
+    def test_list_mass_unsubscribe_all_succeed(self):
         with transaction():
-            aperson = self._usermanager.create_address('aperson@test.com')
-            bperson = self._usermanager.create_address('bperson@test.com')
-            cperson = self._usermanager.create_address('cperson@test.com')
-            mlist = create_list('testlist@example.com')
-            mlist.subscribe(aperson)
-            mlist.subscribe(bperson)
-            mlist.subscribe(cperson)
+            aperson = self._usermanager.create_address('aperson@example.com')
+            bperson = self._usermanager.create_address('bperson@example.com')
+            cperson = self._usermanager.create_address('cperson@example.com')
+            self._mlist.subscribe(aperson)
+            self._mlist.subscribe(bperson)
+            self._mlist.subscribe(cperson)
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test.example.com'
+            '/roster/member', {
+                'emails': ['aperson@example.com',
+                           'bperson@example.com',
+                           ]},
+            'DELETE')
+        self.assertEqual(response.status, 200)
+        # Remove variable data.
+        resource.pop('http_etag')
+        self.assertEqual(resource, {'aperson@example.com': True,
+                                    'bperson@example.com': True,
+                                    })
+
+    def test_list_mass_unsubscribe_all_fail(self):
+        with transaction():
+            aperson = self._usermanager.create_address('aperson@example.com')
+            bperson = self._usermanager.create_address('bperson@example.com')
+            cperson = self._usermanager.create_address('cperson@example.com')
+            self._mlist.subscribe(aperson)
+            self._mlist.subscribe(bperson)
+            self._mlist.subscribe(cperson)
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test.example.com'
+            '/roster/member', {
+                'emails': ['yperson@example.com',
+                           'zperson@example.com',
+                           ]},
+            'DELETE')
+        self.assertEqual(response.status, 200)
+        # Remove variable data.
+        resource.pop('http_etag')
+        self.assertEqual(resource, {'yperson@example.com': False,
+                                    'zperson@example.com': False,
+                                    })
+
+    def test_list_mass_unsubscribe_mixed_success(self):
+        with transaction():
+            aperson = self._usermanager.create_address('aperson@example.com')
+            bperson = self._usermanager.create_address('bperson@example.com')
+            cperson = self._usermanager.create_address('cperson@example.com')
+            self._mlist.subscribe(aperson)
+            self._mlist.subscribe(bperson)
+            self._mlist.subscribe(cperson)
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test.example.com'
+            '/roster/member', {
+                'emails': ['aperson@example.com',
+                           'zperson@example.com',
+                           ]},
+            'DELETE')
+        self.assertEqual(response.status, 200)
+        # Remove variable data.
+        resource.pop('http_etag')
+        self.assertEqual(resource, {'aperson@example.com': True,
+                                    'zperson@example.com': False,
+                                    })
+
+    def test_list_mass_unsubscribe_with_duplicates(self):
+        with transaction():
+            aperson = self._usermanager.create_address('aperson@example.com')
+            bperson = self._usermanager.create_address('bperson@example.com')
+            cperson = self._usermanager.create_address('cperson@example.com')
+            self._mlist.subscribe(aperson)
+            self._mlist.subscribe(bperson)
+            self._mlist.subscribe(cperson)
+        resource, response = call_api(
+            'http://localhost:9001/3.0/lists/test.example.com'
+            '/roster/member', {
+                'emails': ['aperson@example.com',
+                           'aperson@example.com',
+                           'bperson@example.com',
+                           'zperson@example.com',
+                           ]},
+            'DELETE')
+        self.assertEqual(response.status, 200)
+        # Remove variable data.
+        resource.pop('http_etag')
+        self.assertEqual(resource, {'aperson@example.com': True,
+                                    'bperson@example.com': True,
+                                    'zperson@example.com': False,
+                                    })
+
+    def test_list_mass_unsubscribe_bogus_list(self):
         with self.assertRaises(HTTPError) as cm:
             call_api('http://localhost:9001/3.0/lists/bogus.example.com'
-                     '/roster/member', None, 'DELETE')
+                     '/roster/member',
+                     None, 'DELETE')
         self.assertEqual(cm.exception.code, 404)
+
+    def test_list_mass_unsubscribe_with_no_data(self):
         with self.assertRaises(HTTPError) as cm:
-            call_api('http://localhost:9001/3.0/lists/testlist.example.com'
-                     '/roster/member', None, 'DELETE')
+            call_api('http://localhost:9001/3.0/lists/test.example.com'
+                     '/roster/member',
+                     None, 'DELETE')
         self.assertEqual(cm.exception.code, 400)
-        self.assertEqual(cm.exception.reason, b'Invalid Input.')
-        resource, response = call_api(
-            'http://127.0.0.1:9001/3.0/lists/testlist.example.com'
-            '/roster/member', {'emails': ['aperson@test.com']}, 'DELETE')
-        self.assertEqual(response.status, 204)
-        resource, response = call_api(
-            'http://127.0.0.1:9001/3.0/lists/testlist.example.com'
-            '/roster/member', {'emails': ['bperson@test.com',
-                                          'cperson@test.com',
-                                          'bperson@test.com',
-                                          'bogus@test.com']}, 'DELETE')
-        self.assertEqual(response.status, 200)
-        self.assertEqual(resource['bperson@test.com'],
-                         'Member already deleted.')
-        self.assertEqual(resource['bogus@test.com'], 'No such member.')
+        self.assertEqual(cm.exception.reason, b'Missing parameters: emails')
 
 
 class TestListArchivers(unittest.TestCase):