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 f3cb3a84a4d50eee7c544c6ce515aadf48a3bf24..2bada2f6e98493a903b4fd81d608b319bf0aae0f 100644 (file)
--- 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 d3f75c20deabab4f372f2c67f1ca671d1ffc0954..cba387c362be613563492e8149677c7a98943f98 100644 (file)
--- 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 1e34c6809e91cfc0b1ed3bdd6cf0b8003259d674..e421023bdb541352c2178b388650622357b1a50c 100644 (file)
--- 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 55189c628647cecfbd9b638d840b3fb49721a5d6..1f3b26a26717f7a56847f81901b4314f582342bf 100644 (file)
--- 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 6e6541c75ff1cf84eec43e9bf0a8fefc4c4017cc..a56d6cb3fd74fbd18b76fa957347147b99275f96 100644 (file)
--- 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)