Fixes request with a deleted image as marker

Fixes bug 907461

Change-Id: I5dbcd626ecdbd8dfcb1b1dca80813e07c190b9db

glance/registry/db/api.py

@@ -143,7 +143,7 @@ def image_destroy(context, image_id):
             image_member_delete(context, memb_ref, session=session)
 
 
-def image_get(context, image_id, session=None):
+def image_get(context, image_id, session=None, force_show_deleted=False):
     """Get an image or raise if it does not exist."""
     session = session or get_session()
 
@@ -154,7 +154,7 @@ def image_get(context, image_id, session=None):
                         filter_by(id=image_id)
 
         # filter out deleted images if context disallows it
-        if not can_show_deleted(context):
+        if not force_show_deleted and not can_show_deleted(context):
             query = query.filter_by(deleted=False)
 
         image = query.one()
@@ -219,13 +219,16 @@ def image_get_all(context, filters=None, marker=None, limit=None,
             query = query.filter(the_filter[0])
         del filters['is_public']
 
+    showing_deleted = False
     if 'changes-since' in filters:
         changes_since = filters.pop('changes-since')
         query = query.filter(models.Image.updated_at > changes_since)
+        showing_deleted = True
 
     if 'deleted' in filters:
         deleted_filter = filters.pop('deleted')
         query = query.filter_by(deleted=deleted_filter)
+        showing_deleted = deleted_filter
         # TODO(bcwaldon): handle this logic in registry server
         if not deleted_filter:
             query = query.filter(models.Image.status != 'killed')
@@ -239,7 +242,8 @@ def image_get_all(context, filters=None, marker=None, limit=None,
 
     if marker != None:
         # images returned should be created before the image defined by marker
-        marker_image = image_get(context, marker)
+        marker_image = image_get(context, marker,
+                                 force_show_deleted=showing_deleted)
         marker_value = getattr(marker_image, sort_key)
         if sort_dir == 'desc':
             query = query.filter(

glance/tests/unit/test_api.py

@@ -18,7 +18,6 @@
 import datetime
 import hashlib
 import httplib
-import logging
 import os
 import json
 import unittest

glance/tests/unit/test_db.py

@@ -0,0 +1,156 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010-2011 OpenStack, LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import datetime
+import unittest
+
+import stubout
+
+from glance.common import context
+from glance.common import exception
+from glance.common import utils
+from glance.registry import context as rcontext
+from glance.registry.db import api as db_api
+from glance.registry.db import models as db_models
+from glance.tests import stubs
+from glance.tests import utils as test_utils
+
+
+_gen_uuid = utils.generate_uuid
+
+UUID1 = _gen_uuid()
+UUID2 = _gen_uuid()
+
+
+CONF = {'sql_connection': 'sqlite://',
+        'verbose': False,
+        'debug': False,
+        'registry_host': '0.0.0.0',
+        'registry_port': '9191',
+        'default_store': 'file',
+        'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
+
+FIXTURES = [
+    {'id': UUID1,
+     'name': 'fake image #1',
+     'status': 'active',
+     'disk_format': 'ami',
+     'container_format': 'ami',
+     'is_public': False,
+     'created_at': datetime.datetime.utcnow(),
+     'updated_at': datetime.datetime.utcnow(),
+     'deleted_at': None,
+     'deleted': False,
+     'checksum': None,
+     'min_disk': 0,
+     'min_ram': 0,
+     'size': 13,
+     'location': "swift://user:passwd@acct/container/obj.tar.0",
+     'properties': {'type': 'kernel'}},
+    {'id': UUID2,
+     'name': 'fake image #2',
+     'status': 'active',
+     'disk_format': 'vhd',
+     'container_format': 'ovf',
+     'is_public': True,
+     'created_at': datetime.datetime.utcnow(),
+     'updated_at': datetime.datetime.utcnow(),
+     'deleted_at': None,
+     'deleted': False,
+     'checksum': None,
+     'min_disk': 5,
+     'min_ram': 256,
+     'size': 19,
+     'location': "file:///tmp/glance-tests/2",
+     'properties': {}}]
+
+
+class TestRegistryDb(unittest.TestCase):
+
+    def setUp(self):
+        """Establish a clean test environment"""
+        self.stubs = stubout.StubOutForTesting()
+        stubs.stub_out_registry_and_store_server(self.stubs)
+        stubs.stub_out_filesystem_backend()
+        conf = test_utils.TestConfigOpts(CONF)
+        self.adm_context = rcontext.RequestContext(is_admin=True)
+        self.context = rcontext.RequestContext(is_admin=False)
+        db_api.configure_db(conf)
+        self.destroy_fixtures()
+        self.create_fixtures()
+
+    def tearDown(self):
+        """Clear the test environment"""
+        stubs.clean_out_fake_filesystem_backend()
+        self.stubs.UnsetAll()
+
+    def create_fixtures(self):
+        for fixture in FIXTURES:
+            db_api.image_create(self.adm_context, fixture)
+
+    def destroy_fixtures(self):
+        # Easiest to just drop the models and re-create them...
+        db_models.unregister_models(db_api._ENGINE)
+        db_models.register_models(db_api._ENGINE)
+
+    def test_image_get(self):
+        image = db_api.image_get(self.context, UUID1)
+        self.assertEquals(image['id'], FIXTURES[0]['id'])
+
+    def test_image_get_disallow_deleted(self):
+        db_api.image_destroy(self.adm_context, UUID1)
+        self.assertRaises(exception.NotFound, db_api.image_get,
+                          self.context, UUID1)
+
+    def test_image_get_allow_deleted(self):
+        db_api.image_destroy(self.adm_context, UUID1)
+        image = db_api.image_get(self.adm_context, UUID1)
+        self.assertEquals(image['id'], FIXTURES[0]['id'])
+
+    def test_image_get_force_allow_deleted(self):
+        db_api.image_destroy(self.adm_context, UUID1)
+        image = db_api.image_get(self.context, UUID1, force_show_deleted=True)
+        self.assertEquals(image['id'], FIXTURES[0]['id'])
+
+    def test_image_get_all(self):
+        images = db_api.image_get_all(self.context)
+        self.assertEquals(len(images), 2)
+
+    def test_image_get_all_marker(self):
+        images = db_api.image_get_all(self.context, marker=UUID2)
+        self.assertEquals(len(images), 1)
+
+    def test_image_get_all_marker_deleted(self):
+        """Cannot specify a deleted image as a marker."""
+        db_api.image_destroy(self.adm_context, UUID1)
+        filters = {'deleted': False}
+        self.assertRaises(exception.NotFound, db_api.image_get_all,
+                          self.context, marker=UUID1, filters=filters)
+
+    def test_image_get_all_marker_deleted_showing_deleted_as_admin(self):
+        """Specify a deleted image as a marker if showing deleted images."""
+        db_api.image_destroy(self.adm_context, UUID1)
+        images = db_api.image_get_all(self.adm_context, marker=UUID1)
+        self.assertEquals(len(images), 0)
+
+    def test_image_get_all_marker_deleted_showing_deleted(self):
+        """Specify a deleted image as a marker if showing deleted images."""
+        db_api.image_destroy(self.adm_context, UUID1)
+        filters = {'deleted': True}
+        images = db_api.image_get_all(self.context, marker=UUID1,
+                                      filters=filters)
+        self.assertEquals(len(images), 0)