Ensure repeated member deletion fails with 404

Fixes bug 1157427

If an image membership is deleted more than once, the second and
subsequent deletions would be expected to return a 404 Not Found
response code, as opposed to 204 No Content.

Previously, the can_show_deleted logic got in the way, such that
for an admin caller the previously deleted membership was considered
still extant.

We now override the show_deleted logic for deletion, as it makes no
sense to consider the existence of previously deleted database rows
when another deletion request is issued.

Note that there is no corresponding proposal to master as this logic
has been refactored and this no longer affects master.

Change-Id: If8200bbb74e46c814c9f8fa6b4c636e7b022ab6c

glance/db/sqlalchemy/api.py

@@ -710,11 +710,15 @@ def image_member_delete(context, memb_ref, session=None):
     return memb_ref
 
 
-def image_member_find(context, image_id=None, member=None, session=None):
+def image_member_find(context, image_id=None, member=None, session=None,
+                      show_deleted=True):
     """Find all members that meet the given criteria
 
     :param image_id: identifier of image entity
     :param member: tenant to which membership has been granted
+    :param session: an existing sqlalchemy session to use
+    :param show_deleted: set to False to override use of the context to
+                         determine if deleted memberships are included
     """
     session = session or get_session()
 
@@ -726,7 +730,7 @@ def image_member_find(context, image_id=None, member=None, session=None):
         query = query.filter_by(image_id=image_id)
     if member is not None:
         query = query.filter_by(member=member)
-    if not can_show_deleted(context):
+    if not (show_deleted and can_show_deleted(context)):
         query = query.filter_by(deleted=False)
 
     return query.all()

glance/registry/api/v1/members.py

@@ -278,7 +278,8 @@ class Controller(object):
         members = self.db_api.image_member_find(req.context,
                                                 image_id=image_id,
                                                 member=id,
-                                                session=session)
+                                                session=session,
+                                                show_deleted=False)
         if members:
             self.db_api.image_member_delete(req.context,
                                             members[0],

glance/tests/unit/v1/test_api.py

@@ -1918,6 +1918,28 @@ class TestRegistryAPI(base.IsolatedUnitTest):
         self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code)
         self.assertTrue('Membership could not be found' in res.body)
 
+    def test_delete_member_existing(self):
+        """
+        Tests deleting an existing member is handled correctly
+        """
+        member = dict(image_id=UUID2, member='pattieblack', can_share=False)
+        db_api.image_member_create(self.context, member)
+
+        self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper),
+                                                 is_admin=True)
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
+        req.method = 'DELETE'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNoContent.code)
+
+        req = webob.Request.blank('/images/%s/members/pattieblack' % UUID2)
+        req.method = 'DELETE'
+
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code)
+        self.assertTrue('Membership could not be found' in res.body)
+
 
 class TestGlanceAPI(base.IsolatedUnitTest):
     def setUp(self):