Cleanup for SnapshotObject

Mostly switched from dictionary notation (obj['prop']) to object
notation (obj.prop).
This patch doesn't contain changes for drivers. Drivers need to be
fixed in other patch.

Partial-Implements: blueprint cinder-objects
Change-Id: I98dd519e6dc84315df6caa768ce3f5fc4274431c

cinder/consistencygroup/api.py

@@ -250,7 +250,7 @@ def _create_cg_from_cgsnapshot(self, context, group, cgsnapshot):
                 kwargs['cgsnapshot'] = cgsnapshot
                 kwargs['consistencygroup'] = group
                 kwargs['snapshot'] = snapshot
-                volume_type_id = snapshot.get('volume_type_id')
+                volume_type_id = snapshot.volume_type_id
                 if volume_type_id:
                     kwargs['volume_type'] = volume_types.get_volume_type(
                         context, volume_type_id)
@@ -262,7 +262,7 @@ def _create_cg_from_cgsnapshot(self, context, group, cgsnapshot):
                 # and removal of volume entry in the db.
                 try:
                     self.volume_api.create(context,
-                                           snapshot['volume_size'],
+                                           snapshot.volume_size,
                                            None,
                                            None,
                                            **kwargs)

cinder/objects/snapshot.py

@@ -87,11 +87,11 @@ def obj_reset_changes(self, fields=None):
     def _reset_metadata_tracking(self, fields=None):
         if fields is None or 'metadata' in fields:
             self._orig_metadata = (dict(self.metadata)
-                                   if 'metadata' in self else {})
+                                   if self.obj_attr_is_set('metadata') else {})
 
     def obj_what_changed(self):
         changes = super(Snapshot, self).obj_what_changed()
-        if 'metadata' in self and self.metadata != self._orig_metadata:
+        if hasattr(self, 'metadata') and self.metadata != self._orig_metadata:
             changes.add('metadata')
 
         return changes
@@ -111,7 +111,7 @@ def _from_db_object(context, snapshot, db_snapshot, expected_attrs=None):
             value = db_snapshot.get(name)
             if isinstance(field, fields.IntegerField):
                 value = value if value is not None else 0
-            snapshot[name] = value
+            setattr(snapshot, name, value)
 
         if 'volume' in expected_attrs:
             volume = objects.Volume(context)
@@ -220,8 +220,7 @@ def get_all(cls, context, search_opts, marker=None, limit=None,
         snapshots = db.snapshot_get_all(context, search_opts, marker, limit,
                                         sort_keys, sort_dirs, offset)
         return base.obj_make_list(context, cls(), objects.Snapshot,
-                                  snapshots,
-                                  expected_attrs=['metadata'])
+                                  snapshots, expected_attrs=['metadata'])
 
     @base.remotable_classmethod
     def get_by_host(cls, context, host, filters=None):

cinder/tests/unit/api/contrib/test_admin_actions.py

@@ -85,7 +85,7 @@ def _issue_volume_reset(self, ctx, volume, updated_status):
 
     def _issue_snapshot_reset(self, ctx, snapshot, updated_status):
         req = webob.Request.blank('/v2/fake/snapshots/%s/action' %
-                                  snapshot['id'])
+                                  snapshot.id)
         req.method = 'POST'
         req.headers['content-type'] = 'application/json'
         req.body = \
@@ -353,10 +353,9 @@ def test_snapshot_reset_status(self):
         }
         snapshot = objects.Snapshot(context=ctx, **kwargs)
         snapshot.create()
+        self.addCleanup(snapshot.destroy)
 
-        resp = self._issue_snapshot_reset(ctx,
-                                          snapshot,
-                                          {'status': 'error'})
+        resp = self._issue_snapshot_reset(ctx, snapshot, {'status': 'error'})
 
         self.assertEqual(202, resp.status_int)
         snapshot = objects.Snapshot.get_by_id(ctx, snapshot['id'])
@@ -366,16 +365,16 @@ def test_invalid_status_for_snapshot(self):
         ctx = context.RequestContext('admin', 'fake', True)
         volume = db.volume_create(ctx, {'status': 'available', 'host': 'test',
                                         'provider_location': '', 'size': 1})
-        snapshot = db.snapshot_create(ctx, {'status': 'available',
-                                            'volume_id': volume['id']})
+        snapshot = objects.Snapshot(ctx, status='available',
+                                    volume_id=volume['id'])
+        snapshot.create()
+        self.addCleanup(snapshot.destroy)
 
-        resp = self._issue_snapshot_reset(ctx,
-                                          snapshot,
+        resp = self._issue_snapshot_reset(ctx, snapshot,
                                           {'status': 'attaching'})
 
         self.assertEqual(400, resp.status_int)
-        snapshot = db.snapshot_get(ctx, snapshot['id'])
-        self.assertEqual('available', snapshot['status'])
+        self.assertEqual('available', snapshot.status)
 
     def test_force_delete(self):
         # admin context
@@ -862,7 +861,9 @@ def test_migrate_volume_with_snap(self):
         host = 'test2'
         ctx = context.RequestContext('admin', 'fake', True)
         volume = self._migrate_volume_prep()
-        db.snapshot_create(ctx, {'volume_id': volume['id']})
+        snap = objects.Snapshot(ctx, volume_id=volume['id'])
+        snap.create()
+        self.addCleanup(snap.destroy)
         self._migrate_volume_exec(ctx, volume, host, expected_status)
 
     def test_migrate_volume_bad_force_host_copy(self):
@@ -900,8 +901,8 @@ def test_migrate_volume_comp_as_non_admin(self):
         expected_status = 403
         expected_id = None
         ctx = context.RequestContext('fake', 'fake')
-        volume = self._migrate_volume_comp_exec(ctx, volume, new_volume, False,
-                                                expected_status, expected_id)
+        self._migrate_volume_comp_exec(ctx, volume, new_volume, False,
+                                       expected_status, expected_id)
 
     def test_migrate_volume_comp_no_mig_status(self):
         admin_ctx = context.get_admin_context()
@@ -958,8 +959,8 @@ def test_migrate_volume_comp_from_nova(self):
         expected_status = 200
         expected_id = 'fake2'
         ctx = context.RequestContext('admin', 'fake', True)
-        volume = self._migrate_volume_comp_exec(ctx, volume, new_volume, False,
-                                                expected_status, expected_id)
+        self._migrate_volume_comp_exec(ctx, volume, new_volume, False,
+                                       expected_status, expected_id)
 
     def test_backup_reset_valid_updates(self):
         vac = admin_actions.BackupAdminController()

cinder/tests/unit/api/contrib/test_consistencygroups.py

@@ -42,7 +42,8 @@ class ConsistencyGroupsAPITestCase(test.TestCase):
     def setUp(self):
         super(ConsistencyGroupsAPITestCase, self).setUp()
         self.cg_api = cinder.consistencygroup.API()
-        self.ctxt = context.RequestContext('fake', 'fake', auth_token=True)
+        self.ctxt = context.RequestContext('fake', 'fake', auth_token=True,
+                                           is_admin=True)
 
     def _create_consistencygroup(
             self,
@@ -489,6 +490,7 @@ def test_update_consistencygroup_success(self, mock_validate):
         volume_type_id = '123456'
         consistencygroup = self._create_consistencygroup(status='available',
                                                          host='test_host')
+
         remove_volume_id = utils.create_volume(
             self.ctxt,
             volume_type_id=volume_type_id,
@@ -732,11 +734,11 @@ def test_create_consistencygroup_from_src(self, mock_validate):
         cgsnapshot_id = utils.create_cgsnapshot(
             self.ctxt,
             consistencygroup_id=consistencygroup.id)['id']
-        snapshot_id = utils.create_snapshot(
+        snapshot = utils.create_snapshot(
             self.ctxt,
             volume_id,
             cgsnapshot_id=cgsnapshot_id,
-            status='available')['id']
+            status='available')
 
         test_cg_name = 'test cg'
         body = {"consistencygroup-from-src": {"name": test_cg_name,
@@ -759,7 +761,7 @@ def test_create_consistencygroup_from_src(self, mock_validate):
             self.ctxt.elevated(), res_dict['consistencygroup']['id'])
 
         cg_ref.destroy()
-        db.snapshot_destroy(self.ctxt.elevated(), snapshot_id)
+        snapshot.destroy()
         db.cgsnapshot_destroy(self.ctxt.elevated(), cgsnapshot_id)
         db.volume_destroy(self.ctxt.elevated(), volume_id)
         consistencygroup.destroy()
@@ -804,11 +806,11 @@ def test_create_consistencygroup_from_src_both_snap_cg(self):
         cgsnapshot_id = utils.create_cgsnapshot(
             self.ctxt,
             consistencygroup_id=consistencygroup.id)['id']
-        snapshot_id = utils.create_snapshot(
+        snapshot = utils.create_snapshot(
             self.ctxt,
             volume_id,
             cgsnapshot_id=cgsnapshot_id,
-            status='available')['id']
+            status='available')
 
         test_cg_name = 'test cg'
         body = {"consistencygroup-from-src": {"name": test_cg_name,
@@ -828,7 +830,7 @@ def test_create_consistencygroup_from_src_both_snap_cg(self):
         self.assertEqual(400, res_dict['badRequest']['code'])
         self.assertIsNotNone(res_dict['badRequest']['message'])
 
-        db.snapshot_destroy(self.ctxt.elevated(), snapshot_id)
+        snapshot.destroy()
         db.cgsnapshot_destroy(self.ctxt.elevated(), cgsnapshot_id)
         db.volume_destroy(self.ctxt.elevated(), volume_id)
         consistencygroup.destroy()
@@ -874,11 +876,11 @@ def test_create_consistencygroup_from_src_no_host(self):
         cgsnapshot_id = utils.create_cgsnapshot(
             self.ctxt,
             consistencygroup_id=consistencygroup.id)['id']
-        snapshot_id = utils.create_snapshot(
+        snapshot = utils.create_snapshot(
             self.ctxt,
             volume_id,
             cgsnapshot_id=cgsnapshot_id,
-            status='available')['id']
+            status='available')
 
         test_cg_name = 'test cg'
         body = {"consistencygroup-from-src": {"name": test_cg_name,
@@ -898,7 +900,7 @@ def test_create_consistencygroup_from_src_no_host(self):
                 'group')
         self.assertIn(msg, res_dict['badRequest']['message'])
 
-        db.snapshot_destroy(self.ctxt.elevated(), snapshot_id)
+        snapshot.destroy()
         db.cgsnapshot_destroy(self.ctxt.elevated(), cgsnapshot_id)
         db.volume_destroy(self.ctxt.elevated(), volume_id)
         consistencygroup.destroy()
@@ -1007,11 +1009,11 @@ def test_create_consistencygroup_from_src_cgsnapshot_create_volume_failed(
         cgsnapshot_id = utils.create_cgsnapshot(
             self.ctxt,
             consistencygroup_id=consistencygroup.id)['id']
-        snapshot_id = utils.create_snapshot(
+        snapshot = utils.create_snapshot(
             self.ctxt,
             volume_id,
             cgsnapshot_id=cgsnapshot_id,
-            status='available')['id']
+            status='available')
 
         test_cg_name = 'test cg'
         body = {"consistencygroup-from-src": {"name": test_cg_name,
@@ -1030,7 +1032,7 @@ def test_create_consistencygroup_from_src_cgsnapshot_create_volume_failed(
         msg = _("Create volume failed.")
         self.assertEqual(msg, res_dict['badRequest']['message'])
 
-        db.snapshot_destroy(self.ctxt.elevated(), snapshot_id)
+        snapshot.destroy()
         db.cgsnapshot_destroy(self.ctxt.elevated(), cgsnapshot_id)
         db.volume_destroy(self.ctxt.elevated(), volume_id)
         consistencygroup.destroy()

cinder/tests/unit/test_quota.py

@@ -29,6 +29,7 @@
 from cinder.db.sqlalchemy import api as sqa_api
 from cinder.db.sqlalchemy import models as sqa_models
 from cinder import exception
+from cinder import objects
 from cinder import quota
 from cinder import test
 import cinder.tests.unit.image.fake
@@ -41,6 +42,7 @@
 class QuotaIntegrationTestCase(test.TestCase):
 
     def setUp(self):
+        objects.register_all()
         super(QuotaIntegrationTestCase, self).setUp()
         self.volume_type_name = CONF.default_volume_type
         self.volume_type = db.volume_type_create(
@@ -79,14 +81,15 @@ def _create_volume(self, size=1):
         return db.volume_create(self.context, vol)
 
     def _create_snapshot(self, volume):
-        snapshot = {}
-        snapshot['user_id'] = self.user_id
-        snapshot['project_id'] = self.project_id
-        snapshot['volume_id'] = volume['id']
-        snapshot['volume_size'] = volume['size']
-        snapshot['host'] = volume['host']
-        snapshot['status'] = 'available'
-        return db.snapshot_create(self.context, snapshot)
+        snapshot = objects.Snapshot(self.context)
+        snapshot.user_id = self.user_id or 'fake_user_id'
+        snapshot.project_id = self.project_id or 'fake_project_id'
+        snapshot.volume_id = volume['id']
+        snapshot.volume_size = volume['size']
+        snapshot.host = volume['host']
+        snapshot.status = 'available'
+        snapshot.create()
+        return snapshot
 
     def _create_backup(self, volume):
         backup = {}
@@ -156,7 +159,7 @@ def test_too_many_snapshots_of_type(self):
         self.assertRaises(exception.SnapshotLimitExceeded,
                           volume.API().create_snapshot,
                           self.context, vol_ref, '', '')
-        db.snapshot_destroy(self.context, snap_ref['id'])
+        snap_ref.destroy()
         db.volume_destroy(self.context, vol_ref['id'])
 
     def test_too_many_backups(self):
@@ -206,7 +209,7 @@ def test_too_many_combined_gigabytes(self):
         usages = db.quota_usage_get_all_by_project(self.context,
                                                    self.project_id)
         self.assertEqual(20, usages['gigabytes']['in_use'])
-        db.snapshot_destroy(self.context, snap_ref['id'])
+        snap_ref.destroy()
         db.volume_destroy(self.context, vol_ref['id'])
 
     def test_too_many_combined_backup_gigabytes(self):
@@ -244,8 +247,8 @@ def test_no_snapshot_gb_quota_flag(self):
         self.assertEqual(20, usages['gigabytes']['in_use'])
         self.assertEqual(0, usages['gigabytes']['reserved'])
 
-        db.snapshot_destroy(self.context, snap_ref['id'])
-        db.snapshot_destroy(self.context, snap_ref2['id'])
+        snap_ref.destroy()
+        snap_ref2.destroy()
         db.volume_destroy(self.context, vol_ref['id'])
         db.volume_destroy(self.context, vol_ref2['id'])