openstack
/
keystone
Code Issues Proposed changes

Refactor driver_hints 

Throughout the code-base the Hints class was not actually being used as
a list, as it was previously implemented. Besides this, the limit
"filter" or attribute to filtering, was being included and used in a
completely different way. So what this does is separate the logic to
remove the need of iterating the whole filter list each time we need to
use "limit", and treat it as a separate attribute inside the class.

This in turn also simplifies the use of filters, since now there is no need
to iterate the filters in order to get them and now we just return an
inner variable.

Closes-Bug: #1323770
Change-Id: I0ae2828e85891c9e37f30289af1efc01b9e677c6
This commit is contained in:
Juan Antonio Osorio 2014-05-16 22:22:40 +03:00
parent 455d50e8ae
commit 3dd0c24edf
6 changed files with 56 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
keystone/common/controller.py
View File

@ -381,19 +381,18 @@ class V3Controller(wsgi.Application):
NOT_LIMITED = False
LIMITED = True
if hints is None or hints.get_limit() is None:
if hints is None or hints.limit is None:
# No truncation was requested
return NOT_LIMITED, refs
list_limit = hints.get_limit()
if list_limit.get('truncated', False):
if hints.limit.get('truncated', False):
# The driver did truncate the list
return LIMITED, refs
if len(refs) > list_limit['limit']:
if len(refs) > hints.limit['limit']:
# The driver layer wasn't able to truncate it for us, so we must
# do it here
return LIMITED, refs[:list_limit['limit']]
return LIMITED, refs[:hints.limit['limit']]
return NOT_LIMITED, refs
@ -447,7 +446,7 @@ class V3Controller(wsgi.Application):
return False
for filter in hints.filters():
for filter in hints.filters:
if filter['comparator'] == 'equals':
attr = filter['name']
value = filter['value']

65
keystone/common/driver_hints.py
View File

@ -14,7 +14,7 @@
# under the License.
class Hints(list):
class Hints(object):
"""Encapsulate driver hints for listing entities.
Hints are modifiers that affect the return of entities from a
@ -26,55 +26,40 @@ class Hints(list):
but any filters that it does satisfy must be marked as such by calling
removing the filter from the list.
A Hint object is a list of dicts, initially of type 'filter' or 'limit',
although other types may be added in the future. The list can be enumerated
directly, or by using the filters() method which will guarantee to only
return filters.
A Hint object contains filters, which is a list of dicts that can be
accessed publicly. Also it contains a dict called limit, which will
indicate the amount of data we want to limit our listing to.
Each filter term consists of:
* ``name``: the name of the attribute being matched
* ``value``: the value against which it is being matched
* ``comparator``: the operation, which can be one of ``equals``,
``startswith`` or ``endswith``
* ``case_sensitive``: whether any comparison should take account of
case
* ``type``: will always be 'filter'
"""
def __init__(self):
self.limit = None
self.filters = list()
def add_filter(self, name, value, comparator='equals',
case_sensitive=False):
self.append({'name': name, 'value': value, 'comparator': comparator,
'case_sensitive': case_sensitive, 'type': 'filter'})
def filters(self):
"""Iterate over all unsatisfied filters.
Each filter term consists of:
* ``name``: the name of the attribute being matched
* ``value``: the value against which it is being matched
* ``comparator``: the operation, which can be one of ``equals``,
``startswith`` or ``endswith``
* ``case_sensitive``: whether any comparison should take account of
case
* ``type``: will always be 'filter'
"""
return [x for x in self if x['type'] == 'filter']
"""Adds a filter to the filters list, which is publicly accessible."""
self.filters.append({'name': name, 'value': value,
'comparator': comparator,
'case_sensitive': case_sensitive,
'type': 'filter'})
def get_exact_filter_by_name(self, name):
"""Return a filter key and value if exact filter exists for name."""
for entry in self:
for entry in self.filters:
if (entry['type'] == 'filter' and entry['name'] == name and
entry['comparator'] == 'equals'):
return entry
def set_limit(self, limit, truncated=False):
"""Set a limit to indicate the list should be truncated."""
# We only allow one limit entry in the list, so if it already exists
# we overwrite the old one
for x in self:
if x['type'] == 'limit':
x['limit'] = limit
x['truncated'] = truncated
break
else:
self.append({'limit': limit, 'type': 'limit',
'truncated': truncated})
def get_limit(self):
"""Get the limit to which the list should be truncated."""
for x in self:
if x['type'] == 'limit':
return x
self.limit = {'limit': limit, 'type': 'limit', 'truncated': truncated}

20
keystone/common/sql/core.py
View File

@ -214,17 +214,16 @@ def truncated(f):
"""
@functools.wraps(f)
def wrapper(self, hints, *args, **kwargs):
if not hasattr(hints, 'get_limit'):
if not hasattr(hints, 'limit'):
raise exception.UnexpectedError(
_('Cannot truncate a driver call without hints list as '
'first parameter after self '))
limit_dict = hints.get_limit()
if limit_dict is None:
if hints.limit is None:
return f(self, hints, *args, **kwargs)
# A limit is set, so ask for one more entry than we need
list_limit = limit_dict['limit']
list_limit = hints.limit['limit']
hints.set_limit(list_limit + 1)
ref_list = f(self, hints, *args, **kwargs)
@ -288,7 +287,7 @@ def _filter(model, query, hints):
# work out if they need to do something with it.
return query
hints.remove(filter_)
hints.filters.remove(filter_)
return query.filter(query_term)
def exact_filter(model, filter_, cumulative_filter_dict, hints):
@ -312,12 +311,12 @@ def _filter(model, query, hints):
utils.attr_as_boolean(filter_['value']))
else:
cumulative_filter_dict[key] = filter_['value']
hints.remove(filter_)
hints.filters.remove(filter_)
return cumulative_filter_dict
filter_dict = {}
for filter_ in hints.filters():
for filter_ in hints.filters:
# TODO(henry-nash): Check if name is valid column, if not skip
if filter_['comparator'] == 'equals':
filter_dict = exact_filter(model, filter_, filter_dict, hints)
@ -344,9 +343,8 @@ def _limit(query, hints):
# we would expand this method to support pagination and limiting.
# If we satisfied all the filters, set an upper limit if supplied
list_limit = hints.get_limit()
if list_limit:
query = query.limit(list_limit['limit'])
if hints.limit:
query = query.limit(hints.limit['limit'])
return query
@ -377,7 +375,7 @@ def filter_limit_query(model, query, hints):
# unsatisfied filters, we have to leave any limiting to the controller
# as well.
if not hints.filters():
if not hints.filters:
return _limit(query, hints)
else:
return query

4
keystone/identity/core.py
View File

@ -258,10 +258,10 @@ class Manager(manager.Manager):
def _mark_domain_id_filter_satisfied(self, hints):
if hints:
for filter in hints.filters():
for filter in hints.filters:
if (filter['name'] == 'domain_id' and
filter['comparator'] == 'equals'):
hints.remove(filter)
hints.filters.remove(filter)
# The actual driver calls - these are pre/post processed here as
# part of the Manager layer to make sure we:

6
keystone/tests/test_backend.py
View File

@ -4351,8 +4351,8 @@ class LimitTests(filtering.FilterTests):
hints = driver_hints.Hints()
hints.add_filter('domain_id', self.domain1['id'])
entities = self._list_entities(entity)(hints=hints)
self.assertEqual(hints.get_limit()['limit'], len(entities))
self.assertTrue(hints.get_limit()['truncated'])
self.assertEqual(hints.limit['limit'], len(entities))
self.assertTrue(hints.limit['truncated'])
self._match_with_list(entities, self.domain1_entity_lists[entity])
# Override with driver specific limit
@ -4365,7 +4365,7 @@ class LimitTests(filtering.FilterTests):
hints = driver_hints.Hints()
hints.add_filter('domain_id', self.domain1['id'])
entities = self._list_entities(entity)(hints=hints)
self.assertEqual(hints.get_limit()['limit'], len(entities))
self.assertEqual(hints.limit['limit'], len(entities))
self._match_with_list(entities, self.domain1_entity_lists[entity])
# Finally, let's pretend we want to get the full list of entities,

24
keystone/tests/test_driver_hints.py
View File

@ -24,16 +24,16 @@ class ListHintsTests(test.TestCase):
hints = driver_hints.Hints()
hints.add_filter('t1', 'data1')
hints.add_filter('t2', 'data2')
self.assertEqual(2, len(hints.filters()))
self.assertEqual(2, len(hints.filters))
filter = hints.get_exact_filter_by_name('t1')
self.assertEqual('t1', filter['name'])
self.assertEqual('data1', filter['value'])
self.assertEqual('equals', filter['comparator'])
self.assertEqual(False, filter['case_sensitive'])
hints.remove(filter)
hints.filters.remove(filter)
filter_count = 0
for filter in hints.filters():
for filter in hints.filters:
filter_count += 1
self.assertEqual('t2', filter['name'])
self.assertEqual(1, filter_count)
@ -42,21 +42,21 @@ class ListHintsTests(test.TestCase):
hints = driver_hints.Hints()
hints.add_filter('t1', 'data1')
hints.add_filter('t2', 'data2')
self.assertEqual(2, len(hints.filters()))
self.assertEqual(2, len(hints.filters))
hints2 = driver_hints.Hints()
hints2.add_filter('t4', 'data1')
hints2.add_filter('t5', 'data2')
self.assertEqual(2, len(hints.filters()))
self.assertEqual(2, len(hints.filters))
def test_limits(self):
hints = driver_hints.Hints()
self.assertIsNone(hints.get_limit())
self.assertIsNone(hints.limit)
hints.set_limit(10)
self.assertEqual(10, hints.get_limit()['limit'])
self.assertFalse(hints.get_limit()['truncated'])
self.assertEqual(10, hints.limit['limit'])
self.assertFalse(hints.limit['truncated'])
hints.set_limit(11)
self.assertEqual(11, hints.get_limit()['limit'])
self.assertFalse(hints.get_limit()['truncated'])
self.assertEqual(11, hints.limit['limit'])
self.assertFalse(hints.limit['truncated'])
hints.set_limit(10, truncated=True)
self.assertEqual(10, hints.get_limit()['limit'])
self.assertTrue(hints.get_limit()['truncated'])
self.assertEqual(10, hints.limit['limit'])
self.assertTrue(hints.limit['truncated'])