diff --git a/keystone/common/driver_hints.py b/keystone/common/driver_hints.py index 0361e314f0..ff0a774cef 100644 --- a/keystone/common/driver_hints.py +++ b/keystone/common/driver_hints.py @@ -30,6 +30,10 @@ class Hints(object): accessed publicly. Also it contains a dict called limit, which will indicate the amount of data we want to limit our listing to. + If the filter is discovered to never match, then `cannot_match` can be set + to indicate that there will not be any matches and the backend work can be + short-circuited. + Each filter term consists of: * ``name``: the name of the attribute being matched @@ -44,6 +48,7 @@ class Hints(object): def __init__(self): self.limit = None self.filters = list() + self.cannot_match = False def add_filter(self, name, value, comparator='equals', case_sensitive=False): diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index 26c5e3416d..ebd61bb77d 100644 --- a/keystone/common/sql/core.py +++ b/keystone/common/sql/core.py @@ -239,6 +239,39 @@ def truncated(f): return wrapper +class _WontMatch(Exception): + """Raised to indicate that the filter won't match. + + This is raised to short-circuit the computation of the filter as soon as + it's discovered that the filter requested isn't going to match anything. + + A filter isn't going to match anything if the value is too long for the + field, for example. + + """ + + @classmethod + def check(cls, value, col_attr): + """Check if the value can match given the column attributes. + + Raises this class if the value provided can't match any value in the + column in the table given the column's attributes. For example, if the + column is a string and the value is longer than the column then it + won't match any value in the column in the table. + + """ + col = col_attr.property.columns[0] + if isinstance(col.type, sql.types.Boolean): + # The column is a Boolean, we should have already validated input. + return + if not col.type.length: + # The column doesn't have a length so can't validate anymore. + return + if len(value) > col.type.length: + raise cls() + # Otherwise the value could match a value in the column. + + def _filter(model, query, hints): """Applies filtering to a query. @@ -276,10 +309,13 @@ def _filter(model, query, hints): return query if filter_['comparator'] == 'contains': + _WontMatch.check(filter_['value'], column_attr) query_term = column_attr.ilike('%%%s%%' % filter_['value']) elif filter_['comparator'] == 'startswith': + _WontMatch.check(filter_['value'], column_attr) query_term = column_attr.ilike('%s%%' % filter_['value']) elif filter_['comparator'] == 'endswith': + _WontMatch.check(filter_['value'], column_attr) query_term = column_attr.ilike('%%%s' % filter_['value']) else: # It's a filter we don't understand, so let the caller @@ -299,33 +335,40 @@ def _filter(model, query, hints): """ key = filter_['name'] - if isinstance(getattr(model, key).property.columns[0].type, - sql.types.Boolean): + + col = getattr(model, key) + if isinstance(col.property.columns[0].type, sql.types.Boolean): cumulative_filter_dict[key] = ( utils.attr_as_boolean(filter_['value'])) else: + _WontMatch.check(filter_['value'], col) cumulative_filter_dict[key] = filter_['value'] - filter_dict = {} - satisfied_filters = [] - for filter_ in hints.filters: - if filter_['name'] not in model.attributes: - continue - if filter_['comparator'] == 'equals': - exact_filter(model, filter_, filter_dict) - satisfied_filters.append(filter_) - else: - query = inexact_filter(model, query, filter_, satisfied_filters) + try: + filter_dict = {} + satisfied_filters = [] + for filter_ in hints.filters: + if filter_['name'] not in model.attributes: + continue + if filter_['comparator'] == 'equals': + exact_filter(model, filter_, filter_dict) + satisfied_filters.append(filter_) + else: + query = inexact_filter(model, query, filter_, + satisfied_filters) - # Apply any exact filters we built up - if filter_dict: - query = query.filter_by(**filter_dict) + # Apply any exact filters we built up + if filter_dict: + query = query.filter_by(**filter_dict) - # Remove satisfied filters, then the caller will know remaining filters - for filter_ in satisfied_filters: - hints.filters.remove(filter_) + # Remove satisfied filters, then the caller will know remaining filters + for filter_ in satisfied_filters: + hints.filters.remove(filter_) - return query + return query + except _WontMatch: + hints.cannot_match = True + return def _limit(query, hints): @@ -366,6 +409,10 @@ def filter_limit_query(model, query, hints): # First try and satisfy any filters query = _filter(model, query, hints) + if hints.cannot_match: + # Nothing's going to match, so don't bother with the query. + return [] + # NOTE(henry-nash): Any unsatisfied filters will have been left in # the hints list for the controller to handle. We can only try and # limit here if all the filters are already satisfied since, if not, diff --git a/keystone/tests/unit/test_backend.py b/keystone/tests/unit/test_backend.py index b06ff59949..8188513826 100644 --- a/keystone/tests/unit/test_backend.py +++ b/keystone/tests/unit/test_backend.py @@ -5804,6 +5804,40 @@ class FilterTests(filtering.FilterTests): self._delete_test_data('user', user_list) self._delete_test_data('group', group_list) + def _get_user_name_field_size(self): + """Return the size of the user name field for the backend. + + Subclasses can override this method to indicate that the user name + field is limited in length. The user name is the field used in the test + that validates that a filter value works even if it's longer than a + field. + + If the backend doesn't limit the value length then return None. + + """ + return None + + def test_filter_value_wider_than_field(self): + # If a filter value is given that's larger than the field in the + # backend then no values are returned. + + user_name_field_size = self._get_user_name_field_size() + + if user_name_field_size is None: + # The backend doesn't limit the size of the user name, so pass this + # test. + return + + # Create some users just to make sure would return something if the + # filter was ignored. + self._create_test_data('user', 2) + + hints = driver_hints.Hints() + value = 'A' * (user_name_field_size + 1) + hints.add_filter('name', value) + users = self.identity_api.list_users(hints=hints) + self.assertEqual([], users) + class LimitTests(filtering.FilterTests): ENTITIES = ['user', 'group', 'project'] diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index 54879543f3..0fa18b0011 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -743,6 +743,9 @@ class SqlTokenCacheInvalidation(SqlTests, test_backend.TokenCacheInvalidation): class SqlFilterTests(SqlTests, test_backend.FilterTests): + def _get_user_name_field_size(self): + return identity_sql.User.name.type.length + def clean_up_entities(self): """Clean up entity test data from Filter Test Cases."""