From 938cb0c5ab11638a07c22e9db2b0d241223d9d9f Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 28 Aug 2015 01:14:02 +0200 Subject: [PATCH] Move get_by_id to CinderObject Currently each Versioned Object needs to implement its own get_by_id, with this patch they don't need anymore, since it will be included in the base class CinderObject and it will work for all the objects. This will help for other things like having a refresh method or conditional updates in the objects. Related-Bug: #1490944 Related-Bug: #1238093 Related-Bug: #1490946 Related-Bug: #1469659 Change-Id: I355dc8eaefed93003533ee083f74acd1315f057e --- cinder/db/api.py | 11 ++++ cinder/db/sqlalchemy/api.py | 56 +++++++++++++++++ cinder/objects/backup.py | 6 -- cinder/objects/base.py | 18 ++++++ cinder/objects/cgsnapshot.py | 5 -- cinder/objects/consistencygroup.py | 6 -- cinder/objects/service.py | 5 -- cinder/objects/snapshot.py | 8 +-- cinder/objects/volume.py | 9 +-- cinder/objects/volume_attachment.py | 5 -- cinder/objects/volume_type.py | 10 +-- cinder/test.py | 6 ++ .../unit/api/contrib/test_snapshot_actions.py | 45 +++++++------ .../unit/api/v1/test_snapshot_metadata.py | 14 +++-- cinder/tests/unit/api/v1/test_volumes.py | 48 +++++++++----- .../unit/api/v2/test_snapshot_metadata.py | 7 ++- cinder/tests/unit/api/v2/test_volumes.py | 63 +++++++++++++------ cinder/tests/unit/objects/test_backup.py | 14 ++++- cinder/tests/unit/objects/test_cgsnapshot.py | 2 +- .../unit/objects/test_consistencygroup.py | 9 +-- cinder/tests/unit/objects/test_objects.py | 34 +++++----- cinder/tests/unit/objects/test_service.py | 2 +- cinder/tests/unit/objects/test_snapshot.py | 11 +++- cinder/tests/unit/objects/test_volume.py | 11 +++- .../unit/objects/test_volume_attachment.py | 2 +- cinder/tests/unit/objects/test_volume_type.py | 2 +- cinder/tests/unit/test_cmd.py | 6 +- cinder/tests/unit/test_service.py | 5 +- cinder/tests/unit/test_volume.py | 48 +++++++------- .../emc/scaleio/test_create_snapshot.py | 3 +- 30 files changed, 294 insertions(+), 177 deletions(-) diff --git a/cinder/db/api.py b/cinder/db/api.py index 258323a45..c086ee828 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -1078,3 +1078,14 @@ def image_volume_cache_get_by_volume_id(context, volume_id): def image_volume_cache_get_all_for_host(context, host): """Query for all image volume cache entry for a host.""" return IMPL.image_volume_cache_get_all_for_host(context, host) + + +################### + + +def get_model_for_versioned_object(versioned_object): + return IMPL.get_model_for_versioned_object(versioned_object) + + +def get_by_id(context, model, id, *args, **kwargs): + return IMPL.get_by_id(context, model, id, *args, **kwargs) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index b3869e0ae..e194c99b8 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -21,6 +21,7 @@ import datetime as dt import functools +import re import sys import threading import time @@ -2595,6 +2596,12 @@ def volume_type_get(context, id, inactive=False, expected_fields=None): expected_fields=expected_fields) +def _volume_type_get_full(context, id): + """Return dict for a specific volume_type with extra_specs and projects.""" + return _volume_type_get(context, id, session=None, inactive=False, + expected_fields=('extra_specs', 'projects')) + + @require_context def _volume_type_ref_get(context, id, session=None, inactive=False): read_deleted = "yes" if inactive else "no" @@ -4126,3 +4133,52 @@ def image_volume_cache_get_all_for_host(context, host): filter_by(host=host).\ order_by(desc(models.ImageVolumeCacheEntry.last_used)).\ all() + + +############################### + + +def get_model_for_versioned_object(versioned_object): + # Exceptions to model mapping, in general Versioned Objects have the same + # name as their ORM models counterparts, but there are some that diverge + VO_TO_MODEL_EXCEPTIONS = { + 'BackupImport': models.Backup, + 'VolumeType': models.VolumeTypes, + 'CGSnapshot': models.Cgsnapshot, + } + + model_name = versioned_object.obj_name() + return (VO_TO_MODEL_EXCEPTIONS.get(model_name) or + getattr(models, model_name)) + + +def _get_get_method(model): + # Exceptions to model to get methods, in general method names are a simple + # conversion changing ORM name from camel case to snake format and adding + # _get to the string + GET_EXCEPTIONS = { + models.ConsistencyGroup: consistencygroup_get, + models.VolumeTypes: _volume_type_get_full, + } + + if model in GET_EXCEPTIONS: + return GET_EXCEPTIONS[model] + + # General conversion + # Convert camel cased model name to snake format + s = re.sub('(.)([A-Z][a-z]+)', r'\1_\2', model.__name__) + # Get method must be snake formatted model name concatenated with _get + method_name = re.sub('([a-z0-9])([A-Z])', r'\1_\2', s).lower() + '_get' + return globals().get(method_name) + + +_GET_METHODS = {} + + +@require_context +def get_by_id(context, model, id, *args, **kwargs): + # Add get method to cache dictionary if it's not already there + if not _GET_METHODS.get(model): + _GET_METHODS[model] = _get_get_method(model) + + return _GET_METHODS[model](context, id, *args, **kwargs) diff --git a/cinder/objects/backup.py b/cinder/objects/backup.py index 98399e8a2..00ebb1db7 100644 --- a/cinder/objects/backup.py +++ b/cinder/objects/backup.py @@ -101,12 +101,6 @@ class Backup(base.CinderPersistentObject, base.CinderObject, backup.obj_reset_changes() return backup - @base.remotable_classmethod - def get_by_id(cls, context, id, read_deleted=None, project_only=None): - db_backup = db.backup_get(context, id, read_deleted=read_deleted, - project_only=project_only) - return cls._from_db_object(context, cls(context), db_backup) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/objects/base.py b/cinder/objects/base.py index cb41dbc34..cb6db1ebd 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -21,7 +21,9 @@ from oslo_log import log as logging from oslo_versionedobjects import base from oslo_versionedobjects import fields +from cinder import db from cinder import exception +from cinder.i18n import _ from cinder import objects @@ -69,6 +71,22 @@ class CinderObject(base.VersionedObject): # Return modified dict return changes + @base.remotable_classmethod + def get_by_id(cls, context, id, *args, **kwargs): + # To get by id we need to have a model and for the model to + # have an id field + if 'id' not in cls.fields: + msg = (_('VersionedObject %s cannot retrieve object by id.') % + (cls.obj_name())) + raise NotImplementedError(msg) + + model = db.get_model_for_versioned_object(cls) + orm_obj = db.get_by_id(context, model, id, *args, **kwargs) + kargs = {} + if hasattr(cls, 'DEFAULT_EXPECTED_ATTR'): + kargs = {'expected_attrs': getattr(cls, 'DEFAULT_EXPECTED_ATTR')} + return cls._from_db_object(context, cls(context), orm_obj, **kargs) + class CinderObjectDictCompat(base.VersionedObjectDictCompat): """Mix-in to provide dictionary key access compat. diff --git a/cinder/objects/cgsnapshot.py b/cinder/objects/cgsnapshot.py index 1defc3146..92657520a 100644 --- a/cinder/objects/cgsnapshot.py +++ b/cinder/objects/cgsnapshot.py @@ -68,11 +68,6 @@ class CGSnapshot(base.CinderPersistentObject, base.CinderObject, cgsnapshot.obj_reset_changes() return cgsnapshot - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_cgsnapshots = db.cgsnapshot_get(context, id) - return cls._from_db_object(context, cls(context), db_cgsnapshots) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/objects/consistencygroup.py b/cinder/objects/consistencygroup.py index 16f3d6830..a252c7f88 100644 --- a/cinder/objects/consistencygroup.py +++ b/cinder/objects/consistencygroup.py @@ -49,12 +49,6 @@ class ConsistencyGroup(base.CinderPersistentObject, base.CinderObject, consistencygroup.obj_reset_changes() return consistencygroup - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_consistencygroup = db.consistencygroup_get(context, id) - return cls._from_db_object(context, cls(context), - db_consistencygroup) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/objects/service.py b/cinder/objects/service.py index 18e3d95bb..bc4f0d848 100644 --- a/cinder/objects/service.py +++ b/cinder/objects/service.py @@ -66,11 +66,6 @@ class Service(base.CinderPersistentObject, base.CinderObject, service.obj_reset_changes() return service - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_service = db.service_get(context, id) - return cls._from_db_object(context, cls(context), db_service) - @base.remotable_classmethod def get_by_host_and_topic(cls, context, host, topic): db_service = db.service_get_by_host_and_topic(context, host, topic) diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index b51b92060..ef5d8bb8c 100644 --- a/cinder/objects/snapshot.py +++ b/cinder/objects/snapshot.py @@ -36,6 +36,8 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, # Version 1.0: Initial version VERSION = '1.0' + DEFAULT_EXPECTED_ATTR = ('metadata',) + fields = { 'id': fields.UUIDField(), @@ -133,12 +135,6 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, snapshot.obj_reset_changes() return snapshot - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_snapshot = db.snapshot_get(context, id) - return cls._from_db_object(context, cls(context), db_snapshot, - expected_attrs=['metadata']) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index e669a45bf..b8790f745 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -37,6 +37,8 @@ class Volume(base.CinderPersistentObject, base.CinderObject, # volume_type VERSION = '1.1' + DEFAULT_EXPECTED_ATTR = ('admin_metadata', 'metadata') + fields = { 'id': fields.UUIDField(), '_name_id': fields.UUIDField(nullable=True), @@ -182,13 +184,6 @@ class Volume(base.CinderPersistentObject, base.CinderObject, volume.obj_reset_changes() return volume - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_volume = db.volume_get(context, id) - expected_attrs = ['admin_metadata', 'metadata'] - return cls._from_db_object(context, cls(context), db_volume, - expected_attrs=expected_attrs) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/objects/volume_attachment.py b/cinder/objects/volume_attachment.py index 213118b34..fcb82ccf4 100644 --- a/cinder/objects/volume_attachment.py +++ b/cinder/objects/volume_attachment.py @@ -57,11 +57,6 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject, attachment.obj_reset_changes() return attachment - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_attach = db.volume_attachment_get(context, id) - return cls._from_db_object(context, cls(context), db_attach) - @base.remotable def save(self): updates = self.cinder_obj_get_changes() diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index 0784947c1..ec1a06f2e 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -33,6 +33,8 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, # Version 1.0: Initial version VERSION = '1.0' + DEFAULT_EXPECTED_ATTR = ('extra_specs', 'projects') + fields = { 'id': fields.UUIDField(), 'name': fields.StringField(nullable=True), @@ -71,14 +73,6 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, type.obj_reset_changes() return type - @base.remotable_classmethod - def get_by_id(cls, context, id): - db_volume_type = volume_types.get_volume_type( - context, id, expected_fields=['extra_specs', 'projects']) - expected_attrs = ['extra_specs', 'projects'] - return cls._from_db_object(context, cls(context), db_volume_type, - expected_attrs=expected_attrs) - @base.remotable def create(self): if self.obj_attr_is_set('id'): diff --git a/cinder/test.py b/cinder/test.py index 75b7d53e0..b068f237d 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -252,6 +252,12 @@ class TestCase(testtools.TestCase): self._disable_osprofiler() + # NOTE(geguileo): This is required because common get_by_id method in + # cinder.db.sqlalchemy.api caches get methods and if we use a mocked + # get method in one test it would carry on to the next test. So we + # clear out the cache. + sqla_api._GET_METHODS = {} + def _restore_obj_registry(self): objects_base.CinderObjectRegistry._registry._obj_classes = \ self._base_test_obj_backup diff --git a/cinder/tests/unit/api/contrib/test_snapshot_actions.py b/cinder/tests/unit/api/contrib/test_snapshot_actions.py index a6b132029..ba00c44a4 100644 --- a/cinder/tests/unit/api/contrib/test_snapshot_actions.py +++ b/cinder/tests/unit/api/contrib/test_snapshot_actions.py @@ -22,16 +22,30 @@ from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs +def stub_snapshot_get(context, snapshot_id): + snapshot = stubs.stub_snapshot(snapshot_id) + if snapshot_id == 3: + snapshot['status'] = 'error' + elif snapshot_id == 1: + snapshot['status'] = 'creating' + elif snapshot_id == 7: + snapshot['status'] = 'available' + else: + snapshot['status'] = 'creating' + + return snapshot + + class SnapshotActionsTest(test.TestCase): def setUp(self): super(SnapshotActionsTest, self).setUp() + @mock.patch('cinder.db.snapshot_update', autospec=True) + @mock.patch('cinder.db.sqlalchemy.api._snapshot_get', + side_effect=stub_snapshot_get) @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict()) - def test_update_snapshot_status(self, metadata_get): - self.stubs.Set(db, 'snapshot_get', stub_snapshot_get) - self.stubs.Set(db, 'snapshot_update', stub_snapshot_update) - + def test_update_snapshot_status(self, metadata_get, *args): body = {'os-update_snapshot_status': {'status': 'available'}} req = webob.Request.blank('/v2/fake/snapshots/1/action') req.method = "POST" @@ -41,9 +55,10 @@ class SnapshotActionsTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(202, res.status_int) + @mock.patch('cinder.db.sqlalchemy.api._snapshot_get', + side_effect=stub_snapshot_get) @mock.patch('cinder.db.snapshot_metadata_get', return_value=dict()) - def test_update_snapshot_status_invalid_status(self, metadata_get): - self.stubs.Set(db, 'snapshot_get', stub_snapshot_get) + def test_update_snapshot_status_invalid_status(self, metadata_get, *args): body = {'os-update_snapshot_status': {'status': 'in-use'}} req = webob.Request.blank('/v2/fake/snapshots/1/action') req.method = "POST" @@ -63,21 +78,3 @@ class SnapshotActionsTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(400, res.status_int) - - -def stub_snapshot_get(context, snapshot_id): - snapshot = stubs.stub_snapshot(snapshot_id) - if snapshot_id == 3: - snapshot['status'] = 'error' - elif snapshot_id == 1: - snapshot['status'] = 'creating' - elif snapshot_id == 7: - snapshot['status'] = 'available' - else: - snapshot['status'] = 'creating' - - return snapshot - - -def stub_snapshot_update(self, context, id, **kwargs): - pass diff --git a/cinder/tests/unit/api/v1/test_snapshot_metadata.py b/cinder/tests/unit/api/v1/test_snapshot_metadata.py index 3fab2c339..65604272f 100644 --- a/cinder/tests/unit/api/v1/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v1/test_snapshot_metadata.py @@ -483,9 +483,10 @@ class SnapshotMetaDataTest(test.TestCase): self.controller.update_all, req, self.req_id, expected) - def test_update_all_malformed_data(self): - self.stubs.Set(cinder.db, 'snapshot_metadata_update', - return_create_snapshot_metadata) + @mock.patch('cinder.db.sqlalchemy.api._snapshot_get') + @mock.patch('cinder.db.snapshot_metadata_update', autospec=True) + def test_update_all_malformed_data(self, metadata_update, snapshot_get): + snapshot_get.return_value = stub_get req = fakes.HTTPRequest.blank(self.url) req.method = 'PUT' req.content_type = "application/json" @@ -554,9 +555,10 @@ class SnapshotMetaDataTest(test.TestCase): self.controller.update, req, self.req_id, 'key1', None) - def test_update_item_empty_key(self): - self.stubs.Set(cinder.db, 'snapshot_metadata_update', - return_create_snapshot_metadata) + @mock.patch('cinder.db.sqlalchemy.api._snapshot_get') + @mock.patch('cinder.db.snapshot_metadata_update', autospec=True) + def test_update_item_empty_key(self, metadata_update, snapshot_get): + snapshot_get.return_value = stub_get req = fakes.HTTPRequest.blank(self.url + '/key1') req.method = 'PUT' body = {"meta": {"": "value1"}} diff --git a/cinder/tests/unit/api/v1/test_volumes.py b/cinder/tests/unit/api/v1/test_volumes.py index df2ec828a..896109d78 100644 --- a/cinder/tests/unit/api/v1/test_volumes.py +++ b/cinder/tests/unit/api/v1/test_volumes.py @@ -73,7 +73,8 @@ class VolumeApiTest(test.TestCase): def test_volume_create(self): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) vol = {"size": 100, "display_name": "Volume Test Name", @@ -161,7 +162,8 @@ class VolumeApiTest(test.TestCase): def test_volume_create_with_image_id(self): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) self.ext_mgr.extensions = {'os-image-create': 'fake'} test_id = "c905cedb-7281-47e4-8a62-f26bc5fc4c77" @@ -241,7 +243,7 @@ class VolumeApiTest(test.TestCase): @mock.patch.object(db, 'volume_admin_metadata_get', return_value={'attached_mode': 'rw', 'readonly': 'False'}) - @mock.patch.object(db, 'volume_type_get', + @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', side_effect=stubs.stub_volume_type_get) @mock.patch.object(volume_api.API, 'get', side_effect=stubs.stub_volume_api_get, autospec=True) @@ -280,7 +282,7 @@ class VolumeApiTest(test.TestCase): return_value={"qos_max_iops": 2000, "readonly": "False", "attached_mode": "rw"}) - @mock.patch.object(db, 'volume_type_get', + @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', side_effect=stubs.stub_volume_type_get) @mock.patch.object(volume_api.API, 'get', side_effect=stubs.stub_volume_api_get, autospec=True) @@ -411,7 +413,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) self.stubs.Set(volume_api.API, 'get_all', stubs.stub_volume_api_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes') res_dict = self.controller.index(req) @@ -489,7 +492,8 @@ class VolumeApiTest(test.TestCase): def test_volume_list_detail(self, *args): self.stubs.Set(volume_api.API, 'get_all', stubs.stub_volume_api_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/detail') res_dict = self.controller.index(req) @@ -566,7 +570,7 @@ class VolumeApiTest(test.TestCase): 'readonly': 'False'}) @mock.patch.object(volume_api.API, 'get', side_effect=stubs.stub_volume_api_get, autospec=True) - @mock.patch.object(db, 'volume_type_get', + @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', side_effect=stubs.stub_volume_type_get, autospec=True) def test_volume_show(self, *args): req = fakes.HTTPRequest.blank('/v1/volumes/1') @@ -599,7 +603,8 @@ class VolumeApiTest(test.TestCase): return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, '1') @@ -629,7 +634,8 @@ class VolumeApiTest(test.TestCase): return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, '1') @@ -679,7 +685,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/detail?limit=2\ &offset=1', @@ -746,7 +753,8 @@ class VolumeApiTest(test.TestCase): return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, 1) @@ -754,14 +762,16 @@ class VolumeApiTest(test.TestCase): def test_volume_show_with_unencrypted_volume(self): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, 1) self.assertEqual(False, res_dict['volume']['encrypted']) def test_volume_delete(self): - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + self.stubs.Set(db.sqlalchemy.api, 'volume_get', + stubs.stub_volume_get_db) req = fakes.HTTPRequest.blank('/v1/volumes/1') resp = self.controller.delete(req, 1) @@ -779,7 +789,8 @@ class VolumeApiTest(test.TestCase): def test_admin_list_volumes_limited_to_project(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes', use_admin_context=True) @@ -789,7 +800,8 @@ class VolumeApiTest(test.TestCase): self.assertEqual(1, len(res['volumes'])) def test_admin_list_volumes_all_tenants(self): - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes?all_tenants=1', use_admin_context=True) res = self.controller.index(req) @@ -800,7 +812,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes?all_tenants=1') res = self.controller.index(req) @@ -811,7 +824,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes') res = self.controller.index(req) diff --git a/cinder/tests/unit/api/v2/test_snapshot_metadata.py b/cinder/tests/unit/api/v2/test_snapshot_metadata.py index a5ac43b49..9261b6349 100644 --- a/cinder/tests/unit/api/v2/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v2/test_snapshot_metadata.py @@ -555,9 +555,10 @@ class SnapshotMetaDataTest(test.TestCase): self.controller.update, req, self.req_id, 'key1', None) - def test_update_item_empty_key(self): - self.stubs.Set(cinder.db, 'snapshot_metadata_update', - return_create_snapshot_metadata) + @mock.patch('cinder.db.sqlalchemy.api._snapshot_get') + @mock.patch('cinder.db.snapshot_metadata_update', autospec=True) + def test_update_item_empty_key(self, metadata_update, snapshot_get): + snapshot_get.return_value = stub_get req = fakes.HTTPRequest.blank(self.url + '/key1') req.method = 'PUT' body = {"meta": {"": "value1"}} diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 0d1e2be48..491ebd5ea 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -72,7 +72,8 @@ class VolumeApiTest(test.TestCase): def test_volume_create(self, mock_validate): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) vol = self._vol_in_request_body() body = {"volume": vol} @@ -120,7 +121,11 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(volume_api.API, 'get_all', lambda *args, **kwargs: objects.VolumeList(objects=[vol_obj])) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + # NOTE(geguileo): This is required because common get_by_id method in + # cinder.db.sqlalchemy.api caches the real get method. + db.sqlalchemy.api._GET_METHODS = {} + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail') res_dict = self.controller.detail(req) self.assertTrue(mock_validate.called) @@ -217,7 +222,8 @@ class VolumeApiTest(test.TestCase): 'multiattach': False, } - @mock.patch.object(db, 'volume_type_get', autospec=True) + @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', + autospec=True) @mock.patch.object(volume_api.API, 'get_snapshot', autospec=True) @mock.patch.object(volume_api.API, 'create', autospec=True) def test_volume_creation_from_snapshot(self, create, get_snapshot, @@ -262,7 +268,8 @@ class VolumeApiTest(test.TestCase): get_snapshot.assert_called_once_with(self.controller.volume_api, context, snapshot_id) - @mock.patch.object(db, 'volume_type_get', autospec=True) + @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', + autospec=True) @mock.patch.object(volume_api.API, 'get_volume', autospec=True) @mock.patch.object(volume_api.API, 'create', autospec=True) def test_volume_creation_from_source_volume(self, create, get_volume, @@ -386,7 +393,8 @@ class VolumeApiTest(test.TestCase): 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_ref(self, mock_validate): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) self.ext_mgr.extensions = {'os-image-create': 'fake'} vol = self._vol_in_request_body( @@ -439,7 +447,8 @@ class VolumeApiTest(test.TestCase): 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_id(self, mock_validate): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) self.ext_mgr.extensions = {'os-image-create': 'fake'} vol = self._vol_in_request_body( @@ -492,7 +501,8 @@ class VolumeApiTest(test.TestCase): 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_name(self, mock_validate): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) self.stubs.Set(fake_image._FakeImageService, "detail", stubs.stub_image_service_detail) @@ -549,7 +559,8 @@ class VolumeApiTest(test.TestCase): def test_volume_update(self, mock_validate): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) updates = { "name": "Updated Test Name", @@ -570,7 +581,8 @@ class VolumeApiTest(test.TestCase): def test_volume_update_deprecation(self, mock_validate): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) updates = { "display_name": "Updated Test Name", @@ -594,7 +606,8 @@ class VolumeApiTest(test.TestCase): """Test current update keys have priority over deprecated keys.""" self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) updates = { "name": "New Name", @@ -619,7 +632,8 @@ class VolumeApiTest(test.TestCase): def test_volume_update_metadata(self, mock_validate): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) updates = { "metadata": {"qos_max_iops": 2000} @@ -717,7 +731,8 @@ class VolumeApiTest(test.TestCase): def test_volume_list_summary(self): self.stubs.Set(volume_api.API, 'get_all', stubs.stub_volume_api_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes') res_dict = self.controller.index(req) @@ -747,7 +762,8 @@ class VolumeApiTest(test.TestCase): def test_volume_list_detail(self): self.stubs.Set(volume_api.API, 'get_all', stubs.stub_volume_api_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail') res_dict = self.controller.detail(req) @@ -910,7 +926,8 @@ class VolumeApiTest(test.TestCase): ] self.stubs.Set(db, 'volume_get_all_by_project', stub_volume_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1') res_dict = self.controller.detail(req) @@ -922,7 +939,8 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_limit(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=1') res_dict = self.controller.detail(req) @@ -953,7 +971,8 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_limit_marker(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1&limit=1') res_dict = self.controller.detail(req) @@ -1150,7 +1169,8 @@ class VolumeApiTest(test.TestCase): def test_volume_show(self): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, '1') @@ -1173,7 +1193,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(volume_api.API, 'get', stub_volume_get) self.stubs.Set(db, 'volume_admin_metadata_get', stub_volume_admin_metadata_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, '1') @@ -1238,7 +1259,8 @@ class VolumeApiTest(test.TestCase): return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, 1) @@ -1246,7 +1268,8 @@ class VolumeApiTest(test.TestCase): def test_volume_show_with_unencrypted_volume(self): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) - self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) + self.stubs.Set(db.sqlalchemy.api, '_volume_type_get_full', + stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, 1) diff --git a/cinder/tests/unit/objects/test_backup.py b/cinder/tests/unit/objects/test_backup.py index e5ba470b7..99eb6e83a 100644 --- a/cinder/tests/unit/objects/test_backup.py +++ b/cinder/tests/unit/objects/test_backup.py @@ -14,6 +14,7 @@ import mock +from cinder.db.sqlalchemy import models from cinder import exception from cinder import objects from cinder.tests.unit import fake_volume @@ -37,10 +38,21 @@ fake_backup = { class TestBackup(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.backup_get', return_value=fake_backup) + @mock.patch('cinder.db.get_by_id', return_value=fake_backup) def test_get_by_id(self, backup_get): backup = objects.Backup.get_by_id(self.context, 1) self._compare(self, fake_backup, backup) + backup_get.assert_called_once_with(self.context, models.Backup, 1) + + @mock.patch('cinder.db.sqlalchemy.api.model_query') + def test_get_by_id_no_existing_id(self, model_query): + query = mock.Mock() + filter_by = mock.Mock() + filter_by.first.return_value = None + query.filter_by.return_value = filter_by + model_query.return_value = query + self.assertRaises(exception.BackupNotFound, objects.Backup.get_by_id, + self.context, 123) @mock.patch('cinder.db.backup_create', return_value=fake_backup) def test_create(self, backup_create): diff --git a/cinder/tests/unit/objects/test_cgsnapshot.py b/cinder/tests/unit/objects/test_cgsnapshot.py index b846df237..f36e53d4c 100644 --- a/cinder/tests/unit/objects/test_cgsnapshot.py +++ b/cinder/tests/unit/objects/test_cgsnapshot.py @@ -33,7 +33,7 @@ fake_cgsnapshot = { class TestCGSnapshot(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.cgsnapshot_get', + @mock.patch('cinder.db.sqlalchemy.api.cgsnapshot_get', return_value=fake_cgsnapshot) def test_get_by_id(self, cgsnapshot_get): cgsnapshot = objects.CGSnapshot.get_by_id(self.context, 1) diff --git a/cinder/tests/unit/objects/test_consistencygroup.py b/cinder/tests/unit/objects/test_consistencygroup.py index 9743e3c3b..6caad6438 100644 --- a/cinder/tests/unit/objects/test_consistencygroup.py +++ b/cinder/tests/unit/objects/test_consistencygroup.py @@ -35,19 +35,16 @@ fake_consistencygroup = { class TestConsistencyGroup(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.consistencygroup_get', + @mock.patch('cinder.db.sqlalchemy.api.consistencygroup_get', return_value=fake_consistencygroup) def test_get_by_id(self, consistencygroup_get): consistencygroup = objects.ConsistencyGroup.get_by_id(self.context, 1) self._compare(self, fake_consistencygroup, consistencygroup) + consistencygroup_get.assert_called_once_with(self.context, 1) @mock.patch('cinder.db.sqlalchemy.api.model_query') def test_get_by_id_no_existing_id(self, model_query): - query = mock.Mock() - filter_by = mock.Mock() - filter_by.first.return_value = None - query.filter_by.return_value = filter_by - model_query.return_value = query + model_query().filter_by().first.return_value = None self.assertRaises(exception.ConsistencyGroupNotFound, objects.ConsistencyGroup.get_by_id, self.context, 123) diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 030229c4a..80a6a2834 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -21,23 +21,23 @@ from cinder import test # NOTE: The hashes in this list should only be changed if they come with a # corresponding version bump in the affected objects. object_data = { - 'Backup': '1.1-f2e7befd20d3bb388700f17c4f386b28', - 'BackupImport': '1.1-f2e7befd20d3bb388700f17c4f386b28', - 'BackupList': '1.0-db44728c8d21bb23bba601a5499550f8', - 'CGSnapshot': '1.0-d50e9480cee2abcb2222997f2bb85656', - 'CGSnapshotList': '1.0-3361be608f396c5ae045b6d94f901346', - 'ConsistencyGroup': '1.0-98714c3d8f83914fd7a17317c3c29e01', - 'ConsistencyGroupList': '1.0-a906318d3e69d847f31df561d12540b3', - 'Service': '1.0-b81a04373ce0ad2d07de525eb534afd6', - 'ServiceList': '1.0-1911175eadd43fb6eafbefd18c802f2c', - 'Snapshot': '1.0-54a2726a282cbdb47ddd326107e821ce', - 'SnapshotList': '1.0-46abf2a1e65ef55dad4f36fe787f9a78', - 'Volume': '1.1-adc26d52b646723bd0633b0771ad2598', - 'VolumeAttachment': '1.0-4fd93dbfa57d048a4859f5bb1ca66bed', - 'VolumeAttachmentList': '1.0-829c18b1d929ea1f8a451b3c4e0a0289', - 'VolumeList': '1.1-d41f3a850be5fbaa94eb4cc955c7ca60', - 'VolumeType': '1.0-8cb7daad27570133543c2c359d85c658', - 'VolumeTypeList': '1.0-980f0b518aed9df0beb55cc533eff632' + 'Backup': '1.1-cd077ec037f5ad1f5409fd660bd59f53', + 'BackupImport': '1.1-cd077ec037f5ad1f5409fd660bd59f53', + 'BackupList': '1.0-24591dabe26d920ce0756fe64cd5f3aa', + 'CGSnapshot': '1.0-190da2a2aa9457edc771d888f7d225c4', + 'CGSnapshotList': '1.0-e8c3f4078cd0ee23487b34d173eec776', + 'ConsistencyGroup': '1.0-b9bad093daee0b259edddb3993c60c31', + 'ConsistencyGroupList': '1.0-09d0aad5491e762ecfdf66bef02ceb8d', + 'Service': '1.0-64baeb4911dbab1153064dd1c87edb9f', + 'ServiceList': '1.0-d242d3384b68e5a5a534e090ff1d5161', + 'Snapshot': '1.0-a6c33eefeadefb324d79f72f66c54e9a', + 'SnapshotList': '1.0-71661e7180ef6cc51501704a9bea4bf1', + 'Volume': '1.1-6037de222b09c330b33b419a0c1b10c6', + 'VolumeAttachment': '1.0-f14a7c03ffc5b93701d496251a5263aa', + 'VolumeAttachmentList': '1.0-307d2b6c8dd55ef854f6386898e9e98e', + 'VolumeList': '1.1-03ba6cb8c546683e64e15c50042cb1a3', + 'VolumeType': '1.0-bf8abbbea2e852ed2e9bac5a9f5f70f2', + 'VolumeTypeList': '1.0-09b01f4526266c1a58cb206ba509d6d2', } diff --git a/cinder/tests/unit/objects/test_service.py b/cinder/tests/unit/objects/test_service.py index 89f073971..937e7ba01 100644 --- a/cinder/tests/unit/objects/test_service.py +++ b/cinder/tests/unit/objects/test_service.py @@ -21,7 +21,7 @@ from cinder.tests.unit import objects as test_objects class TestService(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.service_get') + @mock.patch('cinder.db.sqlalchemy.api.service_get') def test_get_by_id(self, service_get): db_service = fake_service.fake_db_service() service_get.return_value = db_service diff --git a/cinder/tests/unit/objects/test_snapshot.py b/cinder/tests/unit/objects/test_snapshot.py index 3237d26e4..d3933171e 100644 --- a/cinder/tests/unit/objects/test_snapshot.py +++ b/cinder/tests/unit/objects/test_snapshot.py @@ -17,6 +17,7 @@ import mock from oslo_log import log as logging +from cinder.db.sqlalchemy import models from cinder import exception from cinder import objects from cinder.tests.unit import fake_snapshot @@ -48,10 +49,18 @@ fake_snapshot_obj = { class TestSnapshot(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.snapshot_get', return_value=fake_db_snapshot) + @mock.patch('cinder.db.get_by_id', return_value=fake_db_snapshot) def test_get_by_id(self, snapshot_get): snapshot = objects.Snapshot.get_by_id(self.context, 1) self._compare(self, fake_snapshot_obj, snapshot) + snapshot_get.assert_called_once_with(self.context, models.Snapshot, 1) + + @mock.patch('cinder.db.sqlalchemy.api.model_query') + def test_get_by_id_no_existing_id(self, model_query): + query = model_query().options().options().filter_by().first + query.return_value = None + self.assertRaises(exception.SnapshotNotFound, + objects.Snapshot.get_by_id, self.context, 123) def test_reset_changes(self): snapshot = objects.Snapshot() diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index d50f0b79f..feafe2d65 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -15,6 +15,7 @@ import mock from cinder import context +from cinder import exception from cinder import objects from cinder.tests.unit import fake_volume from cinder.tests.unit import objects as test_objects @@ -23,12 +24,20 @@ from cinder.tests.unit import objects as test_objects class TestVolume(test_objects.BaseObjectsTestCase): @mock.patch('cinder.db.volume_glance_metadata_get', return_value={}) - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') def test_get_by_id(self, volume_get, volume_glance_metadata_get): db_volume = fake_volume.fake_db_volume() volume_get.return_value = db_volume volume = objects.Volume.get_by_id(self.context, 1) self._compare(self, db_volume, volume) + volume_get.assert_called_once_with(self.context, 1) + + @mock.patch('cinder.db.sqlalchemy.api.model_query') + def test_get_by_id_no_existing_id(self, model_query): + pf = model_query().options().options().options().options().options() + pf.filter_by().first.return_value = None + self.assertRaises(exception.VolumeNotFound, + objects.Volume.get_by_id, self.context, 123) @mock.patch('cinder.db.volume_create') def test_create(self, volume_create): diff --git a/cinder/tests/unit/objects/test_volume_attachment.py b/cinder/tests/unit/objects/test_volume_attachment.py index 0bd8c2ec4..87c7ffbcf 100644 --- a/cinder/tests/unit/objects/test_volume_attachment.py +++ b/cinder/tests/unit/objects/test_volume_attachment.py @@ -21,7 +21,7 @@ from cinder.tests.unit import objects as test_objects class TestVolumeAttachment(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.volume_attachment_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_attachment_get') def test_get_by_id(self, volume_attachment_get): db_attachment = fake_volume.fake_db_volume_attachment() volume_attachment_get.return_value = db_attachment diff --git a/cinder/tests/unit/objects/test_volume_type.py b/cinder/tests/unit/objects/test_volume_type.py index ad22bd36c..ac3b09ece 100644 --- a/cinder/tests/unit/objects/test_volume_type.py +++ b/cinder/tests/unit/objects/test_volume_type.py @@ -21,7 +21,7 @@ from cinder.tests.unit import objects as test_objects class TestVolumeType(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.volume_type_get') + @mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full') def test_get_by_id(self, volume_type_get): db_volume_type = fake_volume.fake_db_volume_type() volume_type_get.return_value = db_volume_type diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index 704827bc8..3c58a4092 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -462,7 +462,7 @@ class TestCinderManageCmd(test.TestCase): serializer=object_serializer()) self.assertEqual(mock_rpc_client, rpc_client) - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') @mock.patch('cinder.context.get_admin_context') @mock.patch('cinder.rpc.get_client') @mock.patch('cinder.rpc.init') @@ -492,7 +492,7 @@ class TestCinderManageCmd(test.TestCase): volume=volume_obj) @mock.patch('cinder.db.volume_destroy') - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') @mock.patch('cinder.context.get_admin_context') @mock.patch('cinder.rpc.init') def test_volume_commands_delete_no_host(self, rpc_init, get_admin_context, @@ -519,7 +519,7 @@ class TestCinderManageCmd(test.TestCase): self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.db.volume_destroy') - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') @mock.patch('cinder.context.get_admin_context') @mock.patch('cinder.rpc.init') def test_volume_commands_delete_volume_in_use(self, rpc_init, diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py index fee6b60b8..10a0f7f9a 100644 --- a/cinder/tests/unit/test_service.py +++ b/cinder/tests/unit/test_service.py @@ -180,10 +180,11 @@ class ServiceTestCase(test.TestCase): 'report_count': 0, 'availability_zone': 'nova', 'id': 1} - with mock.patch.object(objects.service, 'db') as mock_db: + with mock.patch.object(objects.service, 'db') as mock_db,\ + mock.patch('cinder.db.sqlalchemy.api.get_by_id') as get_by_id: mock_db.service_get_by_args.side_effect = exception.NotFound() mock_db.service_create.return_value = service_ref - mock_db.service_get.return_value = service_ref + get_by_id.return_value = service_ref serv = service.Service( self.host, diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 64887f4db..112f436a3 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -1131,8 +1131,9 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertRaises(exception.NotFound, db.volume_get, self.context, volume['id']) - @mock.patch.object(db, 'volume_get', side_effect=exception.VolumeNotFound( - volume_id='12345678-1234-5678-1234-567812345678')) + @mock.patch.object(db.sqlalchemy.api, 'volume_get', + side_effect=exception.VolumeNotFound( + volume_id='12345678-1234-5678-1234-567812345678')) def test_delete_volume_not_found(self, mock_get_volume): """Test delete volume moves on if the volume does not exist.""" volume_id = '12345678-1234-5678-1234-567812345678' @@ -2026,7 +2027,7 @@ class VolumeTestCase(BaseVolumeTestCase): @mock.patch.object(cinder.volume.targets.iscsi.ISCSITarget, '_get_target_chap_auth') @mock.patch.object(db, 'volume_admin_metadata_get') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') @mock.patch.object(db, 'volume_update') def test_initialize_connection_fetchqos(self, _mock_volume_update, @@ -2085,7 +2086,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertIsNone(conn_info['data']['qos_specs']) @mock.patch.object(fake_driver.FakeISCSIDriver, 'create_export') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') @mock.patch.object(db, 'volume_update') def test_initialize_connection_export_failure(self, _mock_volume_update, @@ -2115,7 +2116,7 @@ class VolumeTestCase(BaseVolumeTestCase): '_get_target_chap_auth') @mock.patch.object(db, 'volume_admin_metadata_get') @mock.patch.object(db, 'volume_update') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') @mock.patch.object(fake_driver.FakeISCSIDriver, 'initialize_connection') @mock.patch.object(db, 'driver_initiator_data_get') @mock.patch.object(db, 'driver_initiator_data_update') @@ -2918,7 +2919,7 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual("detached", vol['attach_status']) @mock.patch.object(cinder.volume.api.API, 'update') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_reserve_volume_success(self, volume_get, volume_update): fake_volume = { 'id': self.FAKE_UUID, @@ -2948,15 +2949,15 @@ class VolumeTestCase(BaseVolumeTestCase): 'status': status } - with mock.patch.object(db, 'volume_get') as mock_volume_get: - mock_volume_get.return_value = fake_volume + with mock.patch.object(db.sqlalchemy.api, 'volume_get') as mock_get: + mock_get.return_value = fake_volume self.assertRaises(exception.InvalidVolume, cinder.volume.api.API().reserve_volume, self.context, fake_volume) - self.assertTrue(mock_volume_get.called) + self.assertTrue(mock_get.called) - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') @mock.patch.object(db, 'volume_attachment_get_used_by_volume_id') @mock.patch.object(cinder.volume.api.API, 'update') def test_unreserve_volume_success(self, volume_get, @@ -3807,7 +3808,7 @@ class VolumeTestCase(BaseVolumeTestCase): 'name', 'description') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_begin_detaching_fails_available(self, volume_get): volume_api = cinder.volume.api.API() volume = tests_utils.create_volume(self.context, **self.volume_params) @@ -4360,7 +4361,7 @@ class VolumeMigrationTestCase(VolumeTestCase): @mock.patch('cinder.compute.API') @mock.patch('cinder.volume.manager.VolumeManager.' 'migrate_volume_completion') - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') def test_migrate_volume_generic(self, volume_get, migrate_volume_completion, nova_api): @@ -4387,7 +4388,7 @@ class VolumeMigrationTestCase(VolumeTestCase): @mock.patch('cinder.compute.API') @mock.patch('cinder.volume.manager.VolumeManager.' 'migrate_volume_completion') - @mock.patch('cinder.db.volume_get') + @mock.patch('cinder.db.sqlalchemy.api.volume_get') def test_migrate_volume_generic_attached_volume(self, volume_get, migrate_volume_completion, nova_api): @@ -4783,8 +4784,8 @@ class VolumeMigrationTestCase(VolumeTestCase): with mock.patch.object(self.volume.driver, 'retype') as _retype,\ mock.patch.object(volume_types, 'volume_types_diff') as _diff,\ mock.patch.object(self.volume, 'migrate_volume') as _mig,\ - mock.patch.object(db, 'volume_get') as get_volume: - get_volume.return_value = volume + mock.patch.object(db.sqlalchemy.api, 'volume_get') as mock_get: + mock_get.return_value = volume _retype.return_value = driver _diff.return_value = ({}, diff_equal) if migrate_exc: @@ -6114,7 +6115,7 @@ class GenericVolumeDriverTestCase(DriverTestCase): @mock.patch('six.moves.builtins.open') @mock.patch.object(os_brick.initiator.connector, 'get_connector_properties') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_backup_volume_available(self, mock_volume_get, mock_get_connector_properties, mock_file_open, @@ -6151,7 +6152,7 @@ class GenericVolumeDriverTestCase(DriverTestCase): @mock.patch('six.moves.builtins.open') @mock.patch.object(os_brick.initiator.connector, 'get_connector_properties') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_backup_volume_inuse_temp_volume(self, mock_volume_get, mock_get_connector_properties, mock_file_open, @@ -6205,7 +6206,7 @@ class GenericVolumeDriverTestCase(DriverTestCase): @mock.patch.object(os_brick.initiator.connector, 'get_connector_properties', return_value={}) - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_backup_volume_inuse_temp_snapshot(self, mock_volume_get, mock_get_connector_properties, mock_check_device, @@ -6809,8 +6810,9 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase): self.stubs.Set(self.volume.driver.vg, 'get_volume', self._get_manage_existing_lvs) - @mock.patch.object(db, 'volume_get', side_effect=exception.VolumeNotFound( - volume_id='d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1')) + @mock.patch.object(db.sqlalchemy.api, 'volume_get', + side_effect=exception.VolumeNotFound( + volume_id='d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1')) def test_lvm_manage_existing_not_found(self, mock_vol_get): self._setup_stubs_for_manage_existing() @@ -6822,7 +6824,7 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase): model_update = self.volume.driver.manage_existing(vol, ref) self.assertIsNone(model_update) - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_lvm_manage_existing_already_managed(self, mock_conf): self._setup_stubs_for_manage_existing() @@ -7070,7 +7072,7 @@ class LVMVolumeDriverTestCase(DriverTestCase): @mock.patch('six.moves.builtins.open') @mock.patch.object(os_brick.initiator.connector, 'get_connector_properties') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_backup_volume(self, mock_volume_get, mock_get_connector_properties, mock_file_open, @@ -7143,7 +7145,7 @@ class LVMVolumeDriverTestCase(DriverTestCase): @mock.patch('six.moves.builtins.open') @mock.patch.object(os_brick.initiator.connector, 'get_connector_properties') - @mock.patch.object(db, 'volume_get') + @mock.patch.object(db.sqlalchemy.api, 'volume_get') def test_backup_volume_inuse(self, mock_volume_get, mock_get_connector_properties, mock_file_open, diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py index 70a01cb68..7c40fa15f 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py @@ -45,7 +45,8 @@ class TestCreateSnapShot(scaleio.TestScaleIODriver): self.snapshot = fake_snapshot.fake_snapshot_obj( ctx, **{'volume': self.fake_volume}) - self.mock_object(db, 'volume_get', self.return_fake_volume) + self.mock_object(db.sqlalchemy.api, 'volume_get', + self.return_fake_volume) snap_vol_id = self.snapshot.volume_id self.volume_name_2x_enc = urllib.parse.quote( -- 2.45.2