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
This commit is contained in:
parent
81bc51cc29
commit
f24f422373
@ -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()
|
||||
|
@ -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.
|
||||
|
@ -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
|
||||
|
@ -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)
|
||||
|
||||
|
@ -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'})
|
||||
|
||||
|
@ -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):
|
||||
|
@ -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'
|
||||
|
@ -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.
|
Loading…
Reference in New Issue
Block a user