From: Ihar Hrachyshka <ihrachys@redhat.com>
Date: Thu, 9 Jul 2015 12:12:56 +0000 (+0200)
Subject: objects.base: fixed object.delete()
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=954e9de8b3b697b39e087f2d03b49f0856c44c32;p=openstack-build%2Fneutron-build.git

objects.base: fixed object.delete()

It was using wrong call to delete an object from a session.

Also, expanded test_create() test to check all basic operations:
get_by_id, create, update, and delete.

To facilitate update() check, introduced a new field for objects called
fields_to_update that stores names of fields that are not expected to be
updated in an object. This allows us to keep the above mentioned test
untangled from specific object type.

Also made get_by_id() behave correctly (returning None) if the object
does not exist.

Change-Id: I1aecb2e7c4d8cb8f239072d1cb9df3db29dcedde
---

diff --git a/neutron/db/api.py b/neutron/db/api.py
index f3cb3a84a..2bada2f6e 100644
--- a/neutron/db/api.py
+++ b/neutron/db/api.py
@@ -104,7 +104,7 @@ def get_object(context, model, id):
     with context.session.begin(subtransactions=True):
         return (common_db_mixin.model_query(context, model)
                 .filter_by(id=id)
-                .one())
+                .first())
 
 
 def get_objects(context, model):
@@ -132,4 +132,4 @@ def update_object(context, model, id, values):
 def delete_object(context, model, id):
     with context.session.begin(subtransactions=True):
         db_obj = get_object(context, model, id)
-        db_obj.delete()
+        context.session.delete(db_obj)
diff --git a/neutron/objects/base.py b/neutron/objects/base.py
index d3f75c20d..cba387c36 100644
--- a/neutron/objects/base.py
+++ b/neutron/objects/base.py
@@ -29,6 +29,9 @@ class NeutronObject(obj_base.VersionedObject,
     # should be overridden for all persistent objects
     db_model = None
 
+    # fields that are not allowed to update
+    fields_no_update = []
+
     def from_db_object(self, *objs):
         for field in self.fields:
             for db_obj in objs:
@@ -40,9 +43,10 @@ class NeutronObject(obj_base.VersionedObject,
     @classmethod
     def get_by_id(cls, context, id):
         db_obj = db_api.get_object(context, cls.db_model, id)
-        obj = cls(context, **db_obj)
-        obj.obj_reset_changes()
-        return obj
+        if db_obj:
+            obj = cls(context, **db_obj)
+            obj.obj_reset_changes()
+            return obj
 
     @classmethod
     def get_objects(cls, context):
@@ -58,6 +62,7 @@ class NeutronObject(obj_base.VersionedObject,
         self.from_db_object(db_obj)
 
     def update(self):
+        # TODO(QoS): enforce fields_no_update
         updates = self.obj_get_changes()
         if updates:
             db_obj = db_api.update_object(self._context, self.db_model,
diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py
index 1e34c6809..e421023bd 100644
--- a/neutron/objects/qos/policy.py
+++ b/neutron/objects/qos/policy.py
@@ -42,6 +42,8 @@ class QosPolicy(base.NeutronObject):
         'shared': obj_fields.BooleanField()
     }
 
+    fields_no_update = ['id', 'tenant_id']
+
     @classmethod
     def _get_object_policy(cls, context, model, **kwargs):
         # TODO(QoS): we should make sure we use public functions
diff --git a/neutron/objects/qos/rule.py b/neutron/objects/qos/rule.py
index 55189c628..1f3b26a26 100644
--- a/neutron/objects/qos/rule.py
+++ b/neutron/objects/qos/rule.py
@@ -36,6 +36,8 @@ class QosRule(base.NeutronObject):
         'qos_policy_id': obj_fields.UUIDField()
     }
 
+    fields_no_update = ['id', 'tenant_id', 'qos_policy_id']
+
     _core_fields = list(fields.keys())
 
     _common_fields = ['id']
diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py
index 6e6541c75..a56d6cb3f 100644
--- a/neutron/tests/unit/objects/test_base.py
+++ b/neutron/tests/unit/objects/test_base.py
@@ -93,6 +93,11 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
             get_object_mock.assert_called_once_with(
                 self.context, self._test_class.db_model, 'fake_id')
 
+    def test_get_by_id_missing_object(self):
+        with mock.patch.object(db_api, 'get_object', return_value=None):
+            obj = self._test_class.get_by_id(self.context, id='fake_id')
+            self.assertIsNone(obj)
+
     def test_get_objects(self):
         with mock.patch.object(db_api, 'get_objects',
                                return_value=self.db_objs) as get_objects_mock:
@@ -173,9 +178,24 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
 
 class BaseDbObjectTestCase(_BaseObjectTestCase):
 
-    def test_create(self):
+    def test_get_by_id_create_update_delete(self):
         obj = self._test_class(self.context, **self.db_obj)
         obj.create()
 
         new = self._test_class.get_by_id(self.context, id=obj.id)
         self.assertEqual(obj, new)
+
+        obj = new
+        for key, val in self.db_objs[1].items():
+            if key not in self._test_class.fields_no_update:
+                setattr(obj, key, val)
+        obj.update()
+
+        new = self._test_class.get_by_id(self.context, id=obj.id)
+        self.assertEqual(obj, new)
+
+        obj = new
+        new.delete()
+
+        new = self._test_class.get_by_id(self.context, id=obj.id)
+        self.assertIsNone(new)