Merge "objects: added update_objects to OVO framework"

doc/source/devref/objects_usage.rst

@@ -50,11 +50,11 @@ CRUD operations
 ~~~~~~~~~~~~~~~
 Objects support CRUD operations: :code:`create()`, :code:`get_object()` and
 :code:`get_objects()` (equivalent of :code:`read`), :code:`update()`,
-:code:`delete()` and :code:`delete_objects()`. The nature of OVO is, when any
-change is applied, OVO tracks it. After calling :code:`create()` or
-:code:`update()`, OVO detects this and changed fields are saved in the
-database. Please take a look at simple object usage scenarios using example of
-DNSNameServer:
+:code:`delete()`, :code:`update_objects()`, and :code:`delete_objects()`. The
+nature of OVO is, when any change is applied, OVO tracks it. After calling
+:code:`create()` or :code:`update()`, OVO detects this and changed fields are
+saved in the database. Please take a look at simple object usage scenarios
+using example of DNSNameServer:
 
 .. code-block:: Python
 
@@ -76,6 +76,11 @@ DNSNameServer:
     dns.order = 2
     dns.update()
 
+    # if you don't care about keeping the object, you can execute the update
+    # without fetch of the object state from the underlying persistent layer
+    count = DNSNameServer.update_objects(
+        context, {'order': 3}, address='asd', subnet_id='xxx')
+
     # to remove object with filter arguments:
     filters = {'address': 'asd', 'subnet_id': 'xxx'}
     DNSNameServer.delete_objects(context, **filters)
@@ -85,11 +90,12 @@ Filter, sort and paginate
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 The :code:`NeutronDbObject` class has strict validation on which field sorting
 and filtering can happen. When calling :code:`get_objects()`, :code:`count()`,
-:code:`delete_objects()` and :code:`objects_exist()`, :code:`validate_filters()`
-is invoked, to see if it's a supported filter criterion (which is by default
-non-synthetic fields only). Additional filters can be defined using
-:code:`register_filter_hook_on_model()`. This will add the requested string to
-valid filter names in object implementation. It is optional.
+:code:`update_objects()`, :code:`delete_objects()` and :code:`objects_exist()`,
+:code:`validate_filters()` is invoked, to see if it's a supported filter
+criterion (which is by default non-synthetic fields only). Additional filters
+can be defined using :code:`register_filter_hook_on_model()`. This will add the
+requested string to valid filter names in object implementation. It is
+optional.
 
 In order to disable filter validation, :code:`validate_filters=False` needs to
 be passed as an argument in aforementioned methods. It was added because the

neutron/objects/base.py

@@ -182,9 +182,20 @@ class NeutronObject(obj_base.VersionedObject,
                     **kwargs):
         raise NotImplementedError()
 
+    @classmethod
+    def update_objects(cls, context, values, validate_filters=True, **kwargs):
+        objs = cls.get_objects(
+            context, validate_filters=validate_filters, **kwargs)
+        for obj in objs:
+            for k, v in values.items():
+                setattr(obj, k, v)
+            obj.update()
+        return len(objs)
+
     @classmethod
     def delete_objects(cls, context, validate_filters=True, **kwargs):
-        objs = cls.get_objects(context, validate_filters, **kwargs)
+        objs = cls.get_objects(
+            context, validate_filters=validate_filters, **kwargs)
         for obj in objs:
             obj.delete()
         return len(objs)
@@ -201,7 +212,9 @@ class NeutronObject(obj_base.VersionedObject,
     @classmethod
     def count(cls, context, validate_filters=True, **kwargs):
         '''Count the number of objects matching filtering criteria.'''
-        return len(cls.get_objects(context, validate_filters, **kwargs))
+        return len(
+            cls.get_objects(
+                context, validate_filters=validate_filters, **kwargs))
 
 
 def _detach_db_obj(func):
@@ -450,6 +463,33 @@ class NeutronDbObject(NeutronObject):
             )
             return [cls._load_object(context, db_obj) for db_obj in db_objs]
 
+    @classmethod
+    def update_objects(cls, context, values, validate_filters=True, **kwargs):
+        """
+        Update objects that match filtering criteria from DB.
+
+        :param context:
+        :param values: multiple keys to update in matching objects
+        :param validate_filters: Raises an error in case of passing an unknown
+                                 filter
+        :param kwargs: multiple keys defined by key=value pairs
+        :return: Number of entries updated
+        """
+        if validate_filters:
+            cls.validate_filters(**kwargs)
+
+        # if we have standard attributes, we will need to fetch records to
+        # update revision numbers
+        if cls.has_standard_attributes():
+            return super(NeutronDbObject, cls).update_objects(
+                context, values, validate_filters=False, **kwargs)
+
+        with db_api.autonested_transaction(context.session):
+            return obj_db_api.update_objects(
+                context, cls.db_model,
+                cls.modify_fields_to_db(values),
+                **cls.modify_fields_to_db(kwargs))
+
     @classmethod
     def delete_objects(cls, context, validate_filters=True, **kwargs):
         """

neutron/objects/db/api.py

@@ -83,6 +83,23 @@ def delete_object(context, model, **kwargs):
         context.session.delete(db_obj)
 
 
+def update_objects(context, model, values, **kwargs):
+    '''Update matching objects, if any. Return number of updated objects.
+
+    This function does not raise exceptions if nothing matches.
+
+    :param model: SQL model
+    :param values: values to update in matching objects
+    :param kwargs: multiple filters defined by key=value pairs
+    :return: Number of entries updated
+    '''
+    with context.session.begin(subtransactions=True):
+        if not values:
+            return count(context, model, **kwargs)
+        q = _get_filter_query(context, model, **kwargs)
+        return q.update(values, synchronize_session=False)
+
+
 def delete_objects(context, model, **kwargs):
     '''Delete matching objects, if any. Return number of deleted objects.
 

neutron/objects/subnetpool.py

@@ -53,11 +53,10 @@ class SubnetPool(base.NeutronDbObject):
     def from_db_object(self, db_obj):
         super(SubnetPool, self).from_db_object(db_obj)
         self.prefixes = []
-        if db_obj['prefixes']:
-            self.prefixes = [
-                prefix.cidr
-                for prefix in db_obj.prefixes
-            ]
+        self.prefixes = [
+            prefix.cidr
+            for prefix in db_obj.get('prefixes', [])
+        ]
         self.obj_reset_changes(['prefixes'])
 
     def _attach_prefixes(self, prefixes):

neutron/tests/unit/objects/test_base.py

@@ -528,6 +528,10 @@ class _BaseObjectTestCase(object):
             self._test_class.db_model: self.db_objs,
             ObjectFieldsModel: [ObjectFieldsModel(**synthetic_obj_fields)]}
 
+    def _get_random_update_fields(self):
+        return self.get_updatable_fields(
+            self.get_random_object_fields(self._test_class))
+
     def get_random_object_fields(self, obj_cls=None):
         obj_cls = obj_cls or self._test_class
         fields = {}
@@ -650,6 +654,9 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
             mock.patch.object(
                 rbac_db.RbacNeutronDbObjectMixin,
                 'is_shared_with_tenant', return_value=False).start()
+            mock.patch.object(
+                rbac_db.RbacNeutronDbObjectMixin,
+                'get_shared_with_tenant').start()
 
     def fake_get_object(self, context, model, **kwargs):
         objs = self.model_map[model]
@@ -806,6 +813,27 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
                 [get_obj_persistent_fields(obj) for obj in self.objs],
                 [get_obj_persistent_fields(obj) for obj in objs])
 
+    @mock.patch.object(obj_db_api, 'update_object', return_value={})
+    @mock.patch.object(obj_db_api, 'update_objects', return_value=0)
+    def test_update_objects_valid_fields(self, *mocks):
+        '''Test that a valid filter does not raise an error.'''
+        self._test_class.update_objects(
+            self.context, {},
+            **self.valid_field_filter)
+
+    def test_update_objects_invalid_fields(self):
+        with mock.patch.object(obj_db_api, 'update_objects'):
+            self.assertRaises(n_exc.InvalidInput,
+                              self._test_class.update_objects,
+                              self.context, {}, fake_field='xxx')
+
+    @mock.patch.object(obj_db_api, 'update_objects')
+    @mock.patch.object(obj_db_api, 'update_object', return_value={})
+    def test_update_objects_without_validate_filters(self, *mocks):
+            self._test_class.update_objects(
+                self.context, {'unknown_filter': 'new_value'},
+                validate_filters=False, unknown_filter='value')
+
     def test_delete_objects(self):
         '''Test that delete_objects calls to underlying db_api.'''
         with mock.patch.object(
@@ -1678,6 +1706,55 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
         self.assertTrue(self._test_class.objects_exist(
             self.context, validate_filters=False, fake_filter='xxx'))
 
+    def test_update_objects(self):
+        fields_to_update = self.get_updatable_fields(
+            self.obj_fields[1])
+        if not fields_to_update:
+            self.skipTest('No updatable fields found in test '
+                          'class %r' % self._test_class)
+        for fields in self.obj_fields:
+            self._make_object(fields).create()
+
+        objs = self._test_class.get_objects(
+            self.context, **self.valid_field_filter)
+        for k, v in self.valid_field_filter.items():
+            self.assertEqual(v, objs[0][k])
+
+        count = self._test_class.update_objects(
+            self.context, {}, **self.valid_field_filter)
+
+        # we haven't updated anything, but got the number of matching records
+        self.assertEqual(len(objs), count)
+
+        # and the request hasn't changed the number of matching records
+        new_objs = self._test_class.get_objects(
+            self.context, **self.valid_field_filter)
+        self.assertEqual(len(objs), len(new_objs))
+
+        # now update an object with new values
+        new_values = self._get_random_update_fields()
+        keys = self.objs[0]._get_composite_keys()
+        count_updated = self._test_class.update_objects(
+            self.context, new_values, **keys)
+        self.assertEqual(1, count_updated)
+
+        new_filter = keys.copy()
+        new_filter.update(new_values)
+
+        # check that we can fetch using new values
+        new_objs = self._test_class.get_objects(
+            self.context, **new_filter)
+        self.assertEqual(1, len(new_objs))
+
+    def test_update_objects_nothing_to_update(self):
+        fields_to_update = self.get_updatable_fields(
+            self.obj_fields[1])
+        if not fields_to_update:
+            self.skipTest('No updatable fields found in test '
+                          'class %r' % self._test_class)
+        self.assertEqual(
+            0, self._test_class.update_objects(self.context, {}))
+
     def test_delete_objects(self):
         for fields in self.obj_fields:
             self._make_object(fields).create()

neutron/tests/unit/objects/test_subnet.py

@@ -10,12 +10,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 from neutron_lib import context
 from oslo_utils import uuidutils
 
 from neutron.db import rbac_db_models
 from neutron.objects import base as obj_base
 from neutron.objects.db import api as obj_db_api
+from neutron.objects import rbac_db
 from neutron.objects import subnet
 from neutron.tests.unit.objects import test_base as obj_test_base
 from neutron.tests.unit import testlib_api
@@ -127,6 +129,17 @@ class SubnetObjectIfaceTestCase(obj_test_base.BaseObjectIfaceTestCase):
         super(SubnetObjectIfaceTestCase, self).setUp()
         self.pager_map[subnet.DNSNameServer.obj_name()] = (
             obj_base.Pager(sorts=[('order', True)]))
+        # Base class will mock those out only when rbac_db_model is set for the
+        # object. Since subnets don't have their own models but only derive
+        # shared value from networks, we need to unconditionally mock those
+        # entry points out here, otherwise they will trigger database access,
+        # which is not allowed in 'Iface' test classes.
+        mock.patch.object(
+            rbac_db.RbacNeutronDbObjectMixin,
+            'is_shared_with_tenant', return_value=False).start()
+        mock.patch.object(
+            rbac_db.RbacNeutronDbObjectMixin,
+            'get_shared_with_tenant').start()
 
 
 class SubnetDbObjectTestCase(obj_test_base.BaseDbObjectTestCase,