openstack/neutron
Code Issues Proposed changes

objects: add delete_objects public method

Browse Source 
Sometimes we don't need to construct objects before deleting them, and
it's a waste to do so. It may also be easier to use a single
delete_objects call instead of a loop with get_objects and delete.

This patch introduces a method that will delete all matching resources
without constructing full blown versioned objects.

before:

    for obj in cls.get_objects(...):
        obj.delete()

after:

    cls.delete_objects(...)

delete_objects also returns the number of entries deleted from DB.

The default non-db implementation relies on get_objects and delete
methods, meaning it still constructs objects.

Co-Authored-By: Artur Korzeniewski <artur.korzeniewski@intel.com>
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db
Change-Id: I94a79d244b80421f77b714c94248d8ec55c95946
This commit is contained in:
Ihar Hrachyshka 2016-12-06 13:30:12 +01:00
parent c5dde0c87c
commit 0ec47f9eb4
9 changed files with 117 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
neutron/db/allowedaddresspairs_db.py
View File

@ -58,14 +58,10 @@ class AllowedAddressPairsMixin(object):
return allowed_address_pairs
def get_allowed_address_pairs(self, context, port_id):
pairs = self._get_allowed_address_pairs_objs(context, port_id)
return [self._make_allowed_address_pairs_dict(pair.db_obj)
for pair in pairs]
def _get_allowed_address_pairs_objs(self, context, port_id):
pairs = obj_addr_pair.AllowedAddressPair.get_objects(
context, port_id=port_id)
return pairs
return [self._make_allowed_address_pairs_dict(pair.db_obj)
for pair in pairs]
def _extend_port_dict_allowed_address_pairs(self, port_res, port_db):
# If port_db is provided, allowed address pairs will be accessed via
@ -82,10 +78,8 @@ class AllowedAddressPairsMixin(object):
attr.PORTS, ['_extend_port_dict_allowed_address_pairs'])
def _delete_allowed_address_pairs(self, context, id):
pairs = self._get_allowed_address_pairs_objs(context, port_id=id)
with context.session.begin(subtransactions=True):
for pair in pairs:
pair.delete()
obj_addr_pair.AllowedAddressPair.delete_objects(
context, port_id=id)
@staticmethod
def _make_allowed_address_pairs_dict(allowed_address_pairs,

4
neutron/db/db_base_plugin_common.py
View File

@ -224,10 +224,6 @@ class DbBasePluginCommon(common_db_mixin.CommonDbMixin):
raise n_exc.PortNotFound(port_id=id)
return port
def _get_dns_by_subnet(self, context, subnet_id):
return subnet_obj.DNSNameServer.get_objects(context,
subnet_id=subnet_id)
def _get_route_by_subnet(self, context, subnet_id):
return subnet_obj.Route.get_objects(context,
subnet_id=subnet_id)

10
neutron/db/ipam_backend_mixin.py
View File

@ -146,14 +146,12 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
return new_routes
def _update_subnet_dns_nameservers(self, context, id, s):
old_dns_list = self._get_dns_by_subnet(context, id)
new_dns_addr_list = s["dns_nameservers"]
# NOTE(changzhi) delete all dns nameservers from db
# when update subnet's DNS nameservers. And store new
# nameservers with order one by one.
for dns in old_dns_list:
dns.delete()
subnet_obj.DNSNameServer.delete_objects(context, subnet_id=id)
for order, server in enumerate(new_dns_addr_list):
dns = subnet_obj.DNSNameServer(context,
@ -182,10 +180,8 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
return result_pools
def _update_subnet_service_types(self, context, subnet_id, s):
old_types = subnet_obj.SubnetServiceType.get_objects(
context, subnet_id=subnet_id)
for service_type in old_types:
service_type.delete()
subnet_obj.SubnetServiceType.delete_objects(context,
subnet_id=subnet_id)
updated_types = s.pop('service_types')
for service_type in updated_types:
new_type = subnet_obj.SubnetServiceType(context,

4
neutron/db/servicetype_db.py
View File

@ -103,10 +103,8 @@ class ServiceTypeManager(object):
def del_resource_associations(self, context, resource_ids):
if not resource_ids:
return
objs = servicetype_obj.ProviderResourceAssociation.get_objects(
servicetype_obj.ProviderResourceAssociation.delete_objects(
context, resource_id=resource_ids)
for obj in objs:
obj.delete()
_deprecate._MovedGlobals()

24
neutron/objects/base.py
View File

@ -175,6 +175,13 @@ class NeutronObject(obj_base.VersionedObject,
**kwargs):
raise NotImplementedError()
@classmethod
def delete_objects(cls, context, validate_filters=True, **kwargs):
objs = cls.get_objects(context, validate_filters, **kwargs)
for obj in objs:
obj.delete()
return len(objs)
def create(self):
raise NotImplementedError()
@ -430,6 +437,23 @@ class NeutronDbObject(NeutronObject):
)
return [cls._load_object(context, db_obj) for db_obj in db_objs]
@classmethod
def delete_objects(cls, context, validate_filters=True, **kwargs):
"""
Delete objects that match filtering criteria from DB.
:param context:
: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 deleted
"""
if validate_filters:
cls.validate_filters(**kwargs)
with db_api.autonested_transaction(context.session):
return obj_db_api.delete_objects(
context, cls.db_model, **cls.modify_fields_to_db(kwargs))
@classmethod
def is_accessible(cls, context, db_obj):
return (context.is_admin or

8
neutron/objects/db/api.py
View File

@ -87,13 +87,17 @@ def delete_object(context, model, **kwargs):
context.session.delete(db_obj)
# TODO(ihrachys): expose through objects API
def delete_objects(context, model, **kwargs):
'''Delete matching objects, if any.
'''Delete matching objects, if any. Return number of deleted objects.
This function does not raise exceptions if nothing matches.
:param model: SQL model
:param kwargs: multiple filters defined by key=value pairs
:return: Number of entries deleted
'''
with context.session.begin(subtransactions=True):
db_objs = get_objects(context, model, **kwargs)
for db_obj in db_objs:
context.session.delete(db_obj)
return len(db_objs)

5
neutron/objects/network.py
View File

@ -167,10 +167,7 @@ class Network(rbac_db.NeutronRbacObject):
self.obj_reset_changes(['qos_policy_id'])
def _set_dns_domain(self, dns_domain):
objs = NetworkDNSDomain.get_objects(self.obj_context,
network_id=self.id)
for obj in objs:
obj.delete()
NetworkDNSDomain.delete_objects(self.obj_context, network_id=self.id)
if dns_domain:
NetworkDNSDomain(self.obj_context,
network_id=self.id, dns_domain=dns_domain).create()

6
neutron/objects/subnetpool.py
View File

@ -119,10 +119,8 @@ class SubnetPool(base.NeutronDbObject):
super(SubnetPool, self).update()
if synthetic_changes:
if 'prefixes' in synthetic_changes:
old = SubnetPoolPrefix.get_objects(
self.obj_context, subnetpool_id=self.id)
for prefix in old:
prefix.delete()
SubnetPoolPrefix.delete_objects(self.obj_context,
subnetpool_id=self.id)
for prefix in self.prefixes:
prefix_obj = SubnetPoolPrefix(self.obj_context,
subnetpool_id=self.id,

82
neutron/tests/unit/objects/test_base.py
View File

@ -476,8 +476,11 @@ class _BaseObjectTestCase(object):
for fields in self.obj_fields
]
invalid_fields = (
set(self._test_class.synthetic_fields).union(set(TIMESTAMP_FIELDS))
)
valid_field = [f for f in self._test_class.fields
if f not in self._test_class.synthetic_fields][0]
if f not in invalid_fields][0]
self.valid_field_filter = {valid_field:
self.obj_fields[-1][valid_field]}
self.obj_registry = self.useFixture(
@ -757,6 +760,34 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
[get_obj_db_fields(obj) for obj in self.objs],
[get_obj_db_fields(obj) for obj in objs])
def test_delete_objects(self):
'''Test that delete_objects calls to underlying db_api.'''
with mock.patch.object(
obj_db_api, 'delete_objects', return_value=0
) as delete_objects_mock:
self.assertEqual(0, self._test_class.delete_objects(self.context))
delete_objects_mock.assert_any_call(
self.context, self._test_class.db_model)
def test_delete_objects_valid_fields(self):
'''Test that a valid filter does not raise an error.'''
with mock.patch.object(obj_db_api, 'delete_objects', return_value=0):
self._test_class.delete_objects(self.context,
**self.valid_field_filter)
def test_delete_objects_invalid_fields(self):
with mock.patch.object(obj_db_api, 'delete_objects'):
self.assertRaises(n_exc.InvalidInput,
self._test_class.delete_objects, self.context,
fake_field='xxx')
def test_delete_objects_without_validate_filters(self):
with mock.patch.object(
obj_db_api, 'delete_objects'):
self._test_class.delete_objects(self.context,
validate_filters=False,
unknown_filter='value')
def test_count(self):
if not isinstance(self._test_class, base.NeutronDbObject):
self.skipTest('Class %s does not inherit from NeutronDbObject' %
@ -783,7 +814,10 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
self.assertEqual(expected, self._test_class.count(self.context,
validate_filters=False, fake_field='xxx'))
def test_create(self):
# Adding delete_objects mock because some objects are using delete_objects
# while calling create(), Port for example
@mock.patch.object(obj_db_api, 'delete_objects')
def test_create(self, *mocks):
with mock.patch.object(obj_db_api, 'create_object',
return_value=self.db_objs[0]) as create_mock:
with mock.patch.object(obj_db_api, 'get_objects',
@ -797,7 +831,10 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
self._test_class.modify_fields_to_db(
get_obj_db_fields(self.objs[0])))
def test_create_updates_from_db_object(self):
# Adding delete_objects mock because some objects are using delete_objects
# while calling create(), Port for example
@mock.patch.object(obj_db_api, 'delete_objects')
def test_create_updates_from_db_object(self, *mocks):
with mock.patch.object(obj_db_api, 'create_object',
return_value=self.db_objs[0]):
with mock.patch.object(obj_db_api, 'get_objects',
@ -805,7 +842,10 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
self.objs[1].create()
self._check_equal(self.objs[0], self.objs[1])
def test_create_duplicates(self):
# Adding delete_objects mock because some objects are using delete_objects
# while calling create(), Port for example
@mock.patch.object(obj_db_api, 'delete_objects')
def test_create_duplicates(self, delete_object):
with mock.patch.object(obj_db_api, 'create_object',
side_effect=obj_exc.DBDuplicateEntry):
obj = self._test_class(self.context, **self.obj_fields[0])
@ -876,8 +916,11 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
project_id = self.obj_fields[0]['project_id']
self.assertEqual(project_id, obj.tenant_id)
# Adding delete_objects mock because some objects are using delete_objects
# while calling update(), Port for example
@mock.patch.object(obj_db_api, 'delete_objects')
@mock.patch.object(obj_db_api, 'update_object')
def test_update_changes(self, update_mock):
def test_update_changes(self, update_mock, del_mock):
fields_to_update = self.get_updatable_fields(
self._test_class.modify_fields_from_db(self.db_objs[0]))
if not fields_to_update:
@ -914,7 +957,10 @@ class BaseObjectIfaceTestCase(_BaseObjectTestCase, test_base.BaseTestCase):
obj = self._test_class(self.context, **self.obj_fields[0])
self.assertRaises(o_exc.NeutronObjectUpdateForbidden, obj.update)
def test_update_updates_from_db_object(self):
# Adding delete_objects mock because some objects are using delete_objects
# while calling update(), Port and Network for example
@mock.patch.object(obj_db_api, 'delete_objects')
def test_update_updates_from_db_object(self, *mocks):
with mock.patch.object(obj_db_api, 'update_object',
return_value=self.db_objs[0]):
with mock.patch.object(obj_db_api, 'get_objects',
@ -1515,6 +1561,30 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
self.assertTrue(self._test_class.objects_exist(
self.context, validate_filters=False, fake_filter='xxx'))
def test_delete_objects(self):
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.delete_objects(
self.context, **self.valid_field_filter)
self.assertEqual(len(objs), count)
new_objs = self._test_class.get_objects(self.context)
self.assertEqual(len(self.obj_fields) - len(objs), len(new_objs))
for obj in new_objs:
for k, v in self.valid_field_filter.items():
self.assertNotEqual(v, obj[k])
def test_delete_objects_nothing_to_delete(self):
self.assertEqual(
0, self._test_class.delete_objects(self.context))
def test_db_obj(self):
obj = self._make_object(self.obj_fields[0])
self.assertIsNone(obj.db_obj)