Merge "Refactor driver_hints"

keystone/common/controller.py

@@ -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 hints.limit.get('truncated', False):
-        if list_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']

keystone/common/driver_hints.py

@@ -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',
+    A Hint object contains filters, which is a list of dicts that can be
-    although other types may be added in the future. The list can be enumerated
+    accessed publicly. Also it contains a dict called limit, which will
-    directly, or by using the filters() method which will guarantee to only
+    indicate the amount of data we want to limit our listing to.
-    return 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'
 
     """
+    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,
+        """Adds a filter to the filters list, which is publicly accessible."""
-                     'case_sensitive': case_sensitive, 'type': 'filter'})
+        self.filters.append({'name': name, 'value': value,
-
+                             'comparator': comparator,
-    def filters(self):
+                             'case_sensitive': case_sensitive,
-        """Iterate over all unsatisfied filters.
+                             'type': 'filter'})
-
-        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']
 
     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
+        self.limit = {'limit': limit, 'type': 'limit', 'truncated': truncated}
-        # 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

keystone/common/sql/core.py

@@ -213,17 +213,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 hints.limit is None:
-        if limit_dict 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)
 
@@ -287,7 +286,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):
@@ -311,12 +310,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)
@@ -343,9 +342,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 hints.limit:
-    if list_limit:
+        query = query.limit(hints.limit['limit'])
-        query = query.limit(list_limit['limit'])
     return query
 
 
@@ -376,7 +374,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

keystone/identity/core.py

@@ -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:

keystone/tests/test_backend.py

@@ -4432,8 +4432,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.assertEqual(hints.limit['limit'], len(entities))
-        self.assertTrue(hints.get_limit()['truncated'])
+        self.assertTrue(hints.limit['truncated'])
         self._match_with_list(entities, self.domain1_entity_lists[entity])
 
         # Override with driver specific limit
@@ -4446,7 +4446,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,

keystone/tests/test_driver_hints.py

@@ -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.assertEqual(10, hints.limit['limit'])
-        self.assertFalse(hints.get_limit()['truncated'])
+        self.assertFalse(hints.limit['truncated'])
         hints.set_limit(11)
-        self.assertEqual(11, hints.get_limit()['limit'])
+        self.assertEqual(11, hints.limit['limit'])
-        self.assertFalse(hints.get_limit()['truncated'])
+        self.assertFalse(hints.limit['truncated'])
         hints.set_limit(10, truncated=True)
-        self.assertEqual(10, hints.get_limit()['limit'])
+        self.assertEqual(10, hints.limit['limit'])
-        self.assertTrue(hints.get_limit()['truncated'])
+        self.assertTrue(hints.limit['truncated'])