Reduce DB calls when getting an image
Right now if you want to get a single image, two queries need to be executed: - Get image + associated locations and properties - Get associated tags for image Proposed solution combines these two queries into single one which gets image and all related data - locations, properties and tags. Similar pattern has been already implemented when querying for list of images. Change-Id: Ic4424bb1eb1769d8f621ebe111d95961ecf08479 Closes-Bug: 1434578
This commit is contained in:
parent
0d7b292b82
commit
4734d83513
@ -66,13 +66,14 @@ class ImageRepo(object):
|
|||||||
|
|
||||||
def get(self, image_id):
|
def get(self, image_id):
|
||||||
try:
|
try:
|
||||||
db_api_image = dict(self.db_api.image_get(self.context, image_id))
|
db_api_image = dict(self.db_api.image_get(self.context,
|
||||||
|
image_id,
|
||||||
|
return_tag=True))
|
||||||
assert not db_api_image['deleted']
|
assert not db_api_image['deleted']
|
||||||
except (exception.NotFound, exception.Forbidden, AssertionError):
|
except (exception.NotFound, exception.Forbidden, AssertionError):
|
||||||
msg = _("No image found with ID %s") % image_id
|
msg = _("No image found with ID %s") % image_id
|
||||||
raise exception.NotFound(msg)
|
raise exception.NotFound(msg)
|
||||||
tags = self.db_api.image_tag_get_all(self.context, image_id)
|
image = self._format_image_from_db(db_api_image, db_api_image['tags'])
|
||||||
image = self._format_image_from_db(db_api_image, tags)
|
|
||||||
return ImageProxy(image, self.context, self.db_api)
|
return ImageProxy(image, self.context, self.db_api)
|
||||||
|
|
||||||
def list(self, marker=None, limit=None, sort_key=None,
|
def list(self, marker=None, limit=None, sort_key=None,
|
||||||
|
@ -83,9 +83,10 @@ def image_destroy(client, image_id):
|
|||||||
|
|
||||||
|
|
||||||
@_get_client
|
@_get_client
|
||||||
def image_get(client, image_id, force_show_deleted=False):
|
def image_get(client, image_id, force_show_deleted=False, return_tag=False):
|
||||||
return client.image_get(image_id=image_id,
|
return client.image_get(image_id=image_id,
|
||||||
force_show_deleted=force_show_deleted)
|
force_show_deleted=force_show_deleted,
|
||||||
|
return_tag=return_tag)
|
||||||
|
|
||||||
|
|
||||||
def is_image_visible(context, image, status=None):
|
def is_image_visible(context, image, status=None):
|
||||||
|
@ -380,7 +380,8 @@ def _sort_images(images, sort_key, sort_dir):
|
|||||||
return images
|
return images
|
||||||
|
|
||||||
|
|
||||||
def _image_get(context, image_id, force_show_deleted=False, status=None):
|
def _image_get(context, image_id, force_show_deleted=False, status=None,
|
||||||
|
return_tag=False):
|
||||||
try:
|
try:
|
||||||
image = DATA['images'][image_id]
|
image = DATA['images'][image_id]
|
||||||
except KeyError:
|
except KeyError:
|
||||||
@ -396,12 +397,18 @@ def _image_get(context, image_id, force_show_deleted=False, status=None):
|
|||||||
LOG.warn(_LW('Unable to get unowned image'))
|
LOG.warn(_LW('Unable to get unowned image'))
|
||||||
raise exception.Forbidden("Image not visible to you")
|
raise exception.Forbidden("Image not visible to you")
|
||||||
|
|
||||||
|
if return_tag:
|
||||||
|
image['tags'] = image_tag_get_all(context, image_id)
|
||||||
|
|
||||||
return image
|
return image
|
||||||
|
|
||||||
|
|
||||||
@log_call
|
@log_call
|
||||||
def image_get(context, image_id, session=None, force_show_deleted=False):
|
def image_get(context, image_id, session=None, force_show_deleted=False,
|
||||||
image = _image_get(context, image_id, force_show_deleted)
|
return_tag=False):
|
||||||
|
image = _image_get(context, image_id, force_show_deleted,
|
||||||
|
return_tag=return_tag)
|
||||||
|
|
||||||
return _normalize_locations(context, copy.deepcopy(image),
|
return _normalize_locations(context, copy.deepcopy(image),
|
||||||
force_show_deleted=force_show_deleted)
|
force_show_deleted=force_show_deleted)
|
||||||
|
|
||||||
|
@ -196,11 +196,17 @@ def _normalize_tags(image):
|
|||||||
return image
|
return image
|
||||||
|
|
||||||
|
|
||||||
def image_get(context, image_id, session=None, force_show_deleted=False):
|
def image_get(context, image_id, session=None, force_show_deleted=False,
|
||||||
|
return_tag=False):
|
||||||
image = _image_get(context, image_id, session=session,
|
image = _image_get(context, image_id, session=session,
|
||||||
force_show_deleted=force_show_deleted)
|
force_show_deleted=force_show_deleted,
|
||||||
|
return_tag=return_tag)
|
||||||
image = _normalize_locations(context, image.to_dict(),
|
image = _normalize_locations(context, image.to_dict(),
|
||||||
force_show_deleted=force_show_deleted)
|
force_show_deleted=force_show_deleted)
|
||||||
|
|
||||||
|
if return_tag:
|
||||||
|
image = _normalize_tags(image)
|
||||||
|
|
||||||
return image
|
return image
|
||||||
|
|
||||||
|
|
||||||
@ -218,7 +224,8 @@ def _check_image_id(image_id):
|
|||||||
raise exception.NotFound()
|
raise exception.NotFound()
|
||||||
|
|
||||||
|
|
||||||
def _image_get(context, image_id, session=None, force_show_deleted=False):
|
def _image_get(context, image_id, session=None, force_show_deleted=False,
|
||||||
|
return_tag=False):
|
||||||
"""Get an image or raise if it does not exist."""
|
"""Get an image or raise if it does not exist."""
|
||||||
_check_image_id(image_id)
|
_check_image_id(image_id)
|
||||||
session = session or get_session()
|
session = session or get_session()
|
||||||
@ -233,6 +240,9 @@ def _image_get(context, image_id, session=None, force_show_deleted=False):
|
|||||||
if not force_show_deleted and not context.can_see_deleted:
|
if not force_show_deleted and not context.can_see_deleted:
|
||||||
query = query.filter_by(deleted=False)
|
query = query.filter_by(deleted=False)
|
||||||
|
|
||||||
|
if return_tag:
|
||||||
|
query = query.options(sa_orm.joinedload(models.Image.tags))
|
||||||
|
|
||||||
image = query.one()
|
image = query.one()
|
||||||
|
|
||||||
except sa_orm.exc.NoResultFound:
|
except sa_orm.exc.NoResultFound:
|
||||||
|
@ -382,6 +382,28 @@ class DriverTests(object):
|
|||||||
force_show_deleted=True)
|
force_show_deleted=True)
|
||||||
self.assertEqual(self.fixtures[0]['id'], image['id'])
|
self.assertEqual(self.fixtures[0]['id'], image['id'])
|
||||||
|
|
||||||
|
def test_image_get_with_tag_returning(self):
|
||||||
|
expected_tags = {UUID1: ['foo'], UUID2: ['bar'], UUID3: ['baz']}
|
||||||
|
|
||||||
|
self.db_api.image_tag_create(self.context, UUID1,
|
||||||
|
expected_tags[UUID1][0])
|
||||||
|
self.db_api.image_tag_create(self.context, UUID2,
|
||||||
|
expected_tags[UUID2][0])
|
||||||
|
self.db_api.image_tag_create(self.context, UUID3,
|
||||||
|
expected_tags[UUID3][0])
|
||||||
|
|
||||||
|
image = self.db_api.image_get(self.context, UUID1, return_tag=True)
|
||||||
|
self.assertEqual(self.fixtures[0]['id'], image['id'])
|
||||||
|
self.assertEqual(expected_tags[image['id']], image['tags'])
|
||||||
|
|
||||||
|
self.db_api.image_tag_delete(self.context, UUID1,
|
||||||
|
expected_tags[UUID1][0])
|
||||||
|
expected_tags[UUID1] = []
|
||||||
|
|
||||||
|
image = self.db_api.image_get(self.context, UUID1, return_tag=True)
|
||||||
|
self.assertEqual(self.fixtures[0]['id'], image['id'])
|
||||||
|
self.assertEqual(expected_tags[image['id']], image['tags'])
|
||||||
|
|
||||||
def test_image_get_not_owned(self):
|
def test_image_get_not_owned(self):
|
||||||
TENANT1 = str(uuid.uuid4())
|
TENANT1 = str(uuid.uuid4())
|
||||||
TENANT2 = str(uuid.uuid4())
|
TENANT2 = str(uuid.uuid4())
|
||||||
|
Loading…
Reference in New Issue
Block a user