From f24f42237368ae103bdee6a02105ae5b34f3bf69 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Fri, 12 Oct 2018 23:04:17 +0000 Subject: [PATCH] Support fetching specific db column in OVO There is a analysis [1] suggested to run queries against specific columns rather than full ORM entities to optimize the performance. Right now, it is impossible to execute such optimization because OVO doesn't support fetching specific column yet. This commit introduces a new method 'get_values' in the base neutron object class. Subclass of neutron object can leverage this method to fetch specific field of a OVO. It supports fetching non-synthetic fields only as syntheic fields are not directly backed by corresponding DB table columns. neutron-lib patch: https://review.openstack.org/#/c/619047/ [1] https://review.openstack.org/#/c/592361/ Needed-By: https://review.openstack.org/#/c/610184/ Change-Id: Ib90eae7738a5d2e4548fe9fed001d6cdaffddf3b Partial-Implements: blueprint adopt-oslo-versioned-objects-for-db --- neutron/db/_model_query.py | 21 +++++- neutron/objects/base.py | 41 +++++++++++ neutron/objects/db/api.py | 7 ++ neutron/objects/qos/rule_type.py | 5 ++ neutron/tests/unit/objects/db/test_api.py | 60 +++++++++++++++ neutron/tests/unit/objects/test_base.py | 73 ++++++++++++++++++- .../unit/objects/test_port_forwarding.py | 19 ++++- ...ecific-column-in-ovo-69c0b087c8c7ee36.yaml | 6 ++ 8 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/fetch-specific-column-in-ovo-69c0b087c8c7ee36.yaml diff --git a/neutron/db/_model_query.py b/neutron/db/_model_query.py index a55acceab54..e26a991d457 100644 --- a/neutron/db/_model_query.py +++ b/neutron/db/_model_query.py @@ -19,12 +19,15 @@ NOTE: This module shall not be used by external projects. It will be moved from neutron_lib.api import attributes from neutron_lib.db import model_query from neutron_lib.db import utils as db_utils +from neutron_lib import exceptions as n_exc from neutron_lib.objects import utils as obj_utils from neutron_lib.utils import helpers from oslo_db.sqlalchemy import utils as sa_utils from sqlalchemy import sql, or_, and_ from sqlalchemy.ext import associationproxy +from neutron._i18n import _ + # TODO(boden): remove shims _model_query_hooks = model_query._model_query_hooks @@ -32,8 +35,16 @@ register_hook = model_query.register_hook get_hooks = model_query.get_hooks -def query_with_hooks(context, model): - query = context.session.query(model) +def query_with_hooks(context, model, field=None): + if field: + if hasattr(model, field): + field = getattr(model, field) + else: + msg = _("'%s' is not supported as field") % field + raise n_exc.InvalidInput(error_message=msg) + query = context.session.query(field) + else: + query = context.session.query(model) # define basic filter condition for model query query_filter = None if db_utils.model_query_scope_is_project(context, model): @@ -201,5 +212,11 @@ def get_collection(context, model, dict_func, return items +def get_values(context, model, field, filters=None): + query = query_with_hooks(context, model, field=field) + query = apply_filters(query, model, filters, context) + return [c[0] for c in query] + + def get_collection_count(context, model, filters=None): return get_collection_query(context, model, filters).count() diff --git a/neutron/objects/base.py b/neutron/objects/base.py index faf0d28baa8..63d790c790a 100644 --- a/neutron/objects/base.py +++ b/neutron/objects/base.py @@ -259,6 +259,10 @@ class NeutronObject(obj_base.VersionedObject, **kwargs): raise NotImplementedError() + @classmethod + def get_values(cls, context, field, validate_filters=True, **kwargs): + raise NotImplementedError() + @classmethod def _update_objects(cls, objects, values): if not isinstance(objects, collections.Sequence): @@ -583,6 +587,43 @@ class NeutronDbObject(NeutronObject): cls, context, _pager=_pager, **cls.modify_fields_to_db(kwargs)) return [cls._load_object(context, db_obj) for db_obj in db_objs] + @classmethod + def get_values(cls, context, field, validate_filters=True, **kwargs): + """Fetch a list of values of a specific object's field + + Fetch a specific column from DB. + + :param context: + :param field: a specific field of the object + :param validate_filters: Raises an error in case of passing an unknown + filter + :param kwargs: multiple keys defined by key=value pairs + :return: list of objects of NeutronDbObject class or empty list + """ + cls._validate_field(field) + db_field = cls.fields_need_translation.get(field, field) + if validate_filters: + cls.validate_filters(**kwargs) + with cls.db_context_reader(context): + db_values = obj_db_api.get_values( + cls, context, db_field, **cls.modify_fields_to_db(kwargs)) + obj = cls(context) + values = [] + for db_value in db_values: + value = cls.modify_fields_from_db({ + db_field: db_value}).get(field) + value = cls.fields[field].coerce(obj, field, value) + values.append(value) + + return values + + @classmethod + def _validate_field(cls, field): + if field not in cls.fields or cls.is_synthetic(field): + msg = _("Get value of field '%(field)s' is not supported by " + "object '%(object)s'.") % {'field': field, 'object': cls} + raise n_exc.InvalidInput(error_message=msg) + @classmethod def update_object(cls, context, values, validate_filters=True, **kwargs): """Update an object that match filtering criteria from DB. diff --git a/neutron/objects/db/api.py b/neutron/objects/db/api.py index d5004d34371..9ee8b1f0ed6 100644 --- a/neutron/objects/db/api.py +++ b/neutron/objects/db/api.py @@ -53,6 +53,13 @@ def get_objects(obj_cls, context, _pager=None, **kwargs): **(_pager.to_kwargs(context, obj_cls) if _pager else {})) +def get_values(obj_cls, context, field, **kwargs): + with obj_cls.db_context_reader(context): + filters = _kwargs_to_filters(**kwargs) + return model_query.get_values( + context, obj_cls.db_model, field, filters=filters) + + def create_object(obj_cls, context, values, populate_id=True): with obj_cls.db_context_writer(context): if (populate_id and diff --git a/neutron/objects/qos/rule_type.py b/neutron/objects/qos/rule_type.py index 69d6257c331..83b418f0267 100644 --- a/neutron/objects/qos/rule_type.py +++ b/neutron/objects/qos/rule_type.py @@ -68,6 +68,11 @@ class QosRuleType(base.NeutronObject): # TODO(ihrachys): apply filters to returned result return [cls(type=type_) for type_ in rule_types] + # we don't receive context because we don't need db access at all + @classmethod + def get_values(cls, field, **kwargs): + return [getattr(obj, field) for obj in cls.get_objects(**kwargs)] + def obj_make_compatible(self, primitive, target_version): _target_version = versionutils.convert_version_to_tuple(target_version) diff --git a/neutron/tests/unit/objects/db/test_api.py b/neutron/tests/unit/objects/db/test_api.py index 3fbda6b58e8..4491bb69ac3 100644 --- a/neutron/tests/unit/objects/db/test_api.py +++ b/neutron/tests/unit/objects/db/test_api.py @@ -54,6 +54,19 @@ class GetObjectsTestCase(test_base.BaseTestCase): marker_obj=get_object.return_value) +class GetValuesTestCase(test_base.BaseTestCase): + + def test_get_values(self): + ctxt = context.get_admin_context() + fake_field = 'fake_field' + + with mock.patch.object( + model_query, 'get_values') as get_values: + api.get_values(FakeObj, ctxt, fake_field) + get_values.assert_called_with( + ctxt, FakeObj.db_model, fake_field, filters={}) + + class CreateObjectTestCase(test_base.BaseTestCase): def test_populate_id(self, populate_id=True): ctxt = context.get_admin_context() @@ -137,6 +150,53 @@ class CRUDScenarioTestCase(testlib_api.SqlTestCase): self.assertIn(obj2, objs) self.assertNotIn(obj3, objs) + def test_get_values_with_None_value_in_filters(self): + api.create_object(self.obj_cls, self.ctxt, {'name': 'foo'}) + values = api.get_values( + self.obj_cls, self.ctxt, 'name', name='foo', status=None) + self.assertEqual('foo', values[0]) + + def test_get_values_with_string_matching_filters_contains(self): + api.create_object( + self.obj_cls, self.ctxt, {'name': 'obj_con_1'}) + api.create_object( + self.obj_cls, self.ctxt, {'name': 'obj_con_2'}) + api.create_object( + self.obj_cls, self.ctxt, {'name': 'obj_3'}) + + values = api.get_values( + self.obj_cls, self.ctxt, 'name', + name=obj_utils.StringContains('con')) + self.assertEqual(2, len(values)) + self.assertIn('obj_con_1', values) + self.assertIn('obj_con_2', values) + self.assertNotIn('obj_3', values) + + def test_get_values_with_string_matching_filters_starts(self): + api.create_object(self.obj_cls, self.ctxt, {'name': 'pre_obj1'}) + api.create_object(self.obj_cls, self.ctxt, {'name': 'pre_obj2'}) + api.create_object(self.obj_cls, self.ctxt, {'name': 'obj_3'}) + + values = api.get_values( + self.obj_cls, self.ctxt, 'name', + name=obj_utils.StringStarts('pre')) + self.assertEqual(2, len(values)) + self.assertIn('pre_obj1', values) + self.assertIn('pre_obj2', values) + self.assertNotIn('obj_3', values) + + def test_get_values_with_string_matching_filters_ends(self): + api.create_object(self.obj_cls, self.ctxt, {'name': 'obj1_end'}) + api.create_object(self.obj_cls, self.ctxt, {'name': 'obj2_end'}) + api.create_object(self.obj_cls, self.ctxt, {'name': 'obj_3'}) + + values = api.get_values( + self.obj_cls, self.ctxt, 'name', name=obj_utils.StringEnds('end')) + self.assertEqual(2, len(values)) + self.assertIn('obj1_end', values) + self.assertIn('obj2_end', values) + self.assertNotIn('obj_3', values) + def test_get_object_create_update_delete(self): obj = api.create_object(self.obj_cls, self.ctxt, {'name': 'foo'}) diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index 0a9248ba50c..01e5e06a38f 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/neutron/tests/unit/objects/test_base.py @@ -411,6 +411,13 @@ class FakeNeutronObject(base.NeutronObject): for i in range(count) ] + @classmethod + def get_values(cls, context, field, **kwargs): + return [ + getattr(obj, field) + for obj in cls.get_objects(**kwargs) + ] + @base.NeutronObjectRegistry.register_if(False) class FakeNeutronObjectDictOfMiscValues(base.NeutronDbObject): @@ -585,10 +592,10 @@ class _BaseObjectTestCase(object): 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 invalid_fields][0] - self.valid_field_filter = {valid_field: - self.obj_fields[-1][valid_field]} + self.valid_field = [f for f in self._test_class.fields + if f not in invalid_fields][0] + self.valid_field_filter = {self.valid_field: + self.obj_fields[-1][self.valid_field]} self.obj_registry = self.useFixture( NeutronObjectRegistryFixture()) self.obj_registry.register(FakeSmallNeutronObject) @@ -689,6 +696,10 @@ class _BaseObjectTestCase(object): def fake_get_objects(self, obj_cls, context, **kwargs): return self.model_map[obj_cls.db_model] + def fake_get_values(self, obj_cls, context, field, **kwargs): + return [model.get(field) + for model in self.model_map[obj_cls.db_model]] + def _get_object_synthetic_fields(self, objclass): return [field for field in objclass.synthetic_fields if objclass.is_object_field(field)] @@ -888,6 +899,60 @@ 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]) + def test_get_values(self): + field = self.valid_field + db_field = self._test_class.fields_need_translation.get(field, field) + with mock.patch.object( + obj_db_api, 'get_values', + side_effect=self.fake_get_values) as get_values_mock: + values = self._test_class.get_values(self.context, field) + self.assertItemsEqual( + [getattr(obj, field) for obj in self.objs], values) + get_values_mock.assert_any_call( + self._test_class, self.context, db_field + ) + + def test_get_values_with_validate_filters(self): + field = self.valid_field + with mock.patch.object( + obj_db_api, 'get_values', side_effect=self.fake_get_values): + self._test_class.get_values(self.context, field, + **self.valid_field_filter) + + def test_get_values_without_validate_filters(self): + field = self.valid_field + with mock.patch.object( + obj_db_api, 'get_values', + side_effect=self.fake_get_values): + values = self._test_class.get_values(self.context, field, + validate_filters=False, + unknown_filter='value') + self.assertItemsEqual( + [getattr(obj, field) for obj in self.objs], values) + + def test_get_values_mixed_field(self): + synthetic_fields = ( + set(self._test_class.synthetic_fields) - + self._test_class.extra_filter_names + ) + if not synthetic_fields: + self.skipTest('No synthetic fields that are not extra filters ' + 'found in test class %r' % + self._test_class) + + field = synthetic_fields.pop() + with mock.patch.object(obj_db_api, 'get_values', + side_effect=self.fake_get_values): + self.assertRaises(n_exc.InvalidInput, + self._test_class.get_values, self.context, field) + + def test_get_values_invalid_field(self): + field = 'fake_field' + with mock.patch.object(obj_db_api, 'get_values', + side_effect=self.fake_get_values): + self.assertRaises(n_exc.InvalidInput, + self._test_class.get_values, self.context, field) + @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): diff --git a/neutron/tests/unit/objects/test_port_forwarding.py b/neutron/tests/unit/objects/test_port_forwarding.py index 0500fbd565c..855250f50a5 100644 --- a/neutron/tests/unit/objects/test_port_forwarding.py +++ b/neutron/tests/unit/objects/test_port_forwarding.py @@ -31,6 +31,17 @@ class PortForwardingObjectTestCase(obj_test_base.BaseObjectIfaceTestCase): super(PortForwardingObjectTestCase, self).setUp() self.fip_db_fields = self.get_random_db_fields(router.FloatingIP) del self.fip_db_fields['floating_ip_address'] + # 'portforwardings' table will store the 'internal_ip_address' and + # 'internal_port' as a single 'socket' column. + # Port forwarding object accepts 'internal_ip_address' and + # 'internal_port', but can not filter the records in db, so the + # valid filters can not contain them. + not_supported_filter_fields = ['internal_ip_address', 'internal_port'] + invalid_fields = set( + self._test_class.synthetic_fields).union( + set(not_supported_filter_fields)) + self.valid_field = [f for f in self._test_class.fields + if f not in invalid_fields][0] def random_generate_fip_obj(db_fields, **floatingip): if db_fields.get( @@ -67,10 +78,10 @@ class PortForwardingDbObjectTestCase(obj_test_base.BaseDbObjectTestCase, invalid_fields = set( self._test_class.synthetic_fields).union( set(not_supported_filter_fields)) - valid_field = [f for f in self._test_class.fields - if f not in invalid_fields][0] - self.valid_field_filter = {valid_field: - self.obj_fields[-1][valid_field]} + self.valid_field = [f for f in self._test_class.fields + if f not in invalid_fields][0] + self.valid_field_filter = {self.valid_field: + self.obj_fields[-1][self.valid_field]} def _create_test_fip_id_for_port_forwarding(self): fake_fip = '172.23.3.0' diff --git a/releasenotes/notes/fetch-specific-column-in-ovo-69c0b087c8c7ee36.yaml b/releasenotes/notes/fetch-specific-column-in-ovo-69c0b087c8c7ee36.yaml new file mode 100644 index 00000000000..40768da8ae4 --- /dev/null +++ b/releasenotes/notes/fetch-specific-column-in-ovo-69c0b087c8c7ee36.yaml @@ -0,0 +1,6 @@ +--- +other: + - | + Support fetching specific db column in OVO. + A new method ``get_values`` is added to neutron object classes. + This method can be leveraged to fetch specific field of the object.