From b309e12a5d6d7a53c86e98d719967c0a3bb07e14 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Fri, 31 Mar 2017 17:17:47 +0000 Subject: [PATCH] objects: added update_objects to OVO framework This method allows to update matching resources with requested values without doing full fetch/set/update cycle. This is also handy to "lock" records in database with UPDATE WHERE. Change-Id: I2347fedbecef823babe3d8038f5a74b21fc0a602 Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db --- doc/source/devref/objects_usage.rst | 26 +++++--- neutron/objects/base.py | 44 ++++++++++++- neutron/objects/db/api.py | 17 +++++ neutron/objects/subnetpool.py | 9 ++- neutron/tests/unit/objects/test_base.py | 77 +++++++++++++++++++++++ neutron/tests/unit/objects/test_subnet.py | 13 ++++ 6 files changed, 169 insertions(+), 17 deletions(-) diff --git a/doc/source/devref/objects_usage.rst b/doc/source/devref/objects_usage.rst index 70c5a93a593..4ccbd3cf869 100644 --- a/doc/source/devref/objects_usage.rst +++ b/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 diff --git a/neutron/objects/base.py b/neutron/objects/base.py index e708b080daf..aab0601e3d2 100644 --- a/neutron/objects/base.py +++ b/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): """ diff --git a/neutron/objects/db/api.py b/neutron/objects/db/api.py index 80a514eca8a..f37878fcd7a 100644 --- a/neutron/objects/db/api.py +++ b/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. diff --git a/neutron/objects/subnetpool.py b/neutron/objects/subnetpool.py index 17073799079..797c9153f36 100644 --- a/neutron/objects/subnetpool.py +++ b/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): diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index 71af64f0517..b2eb71664c6 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/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() diff --git a/neutron/tests/unit/objects/test_subnet.py b/neutron/tests/unit/objects/test_subnet.py index d5430dc69e7..09d6d1671b6 100644 --- a/neutron/tests/unit/objects/test_subnet.py +++ b/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,