diff --git a/nailgun/nailgun/api/v1/handlers/base.py b/nailgun/nailgun/api/v1/handlers/base.py index bb4e8d64c1..e63775b52c 100644 --- a/nailgun/nailgun/api/v1/handlers/base.py +++ b/nailgun/nailgun/api/v1/handlers/base.py @@ -123,6 +123,15 @@ class BaseHandler(object): data=self.message ) + class _range_not_satisfiable(web.HTTPError): + message = 'Requested Range Not Satisfiable' + + def __init__(self): + super(_range_not_satisfiable, self).__init__( + status='416 Range Not Satisfiable', + data=self.message + ) + exc_status_map = { 200: web.ok, 201: web.created, @@ -141,6 +150,7 @@ class BaseHandler(object): 409: web.conflict, 410: web.gone, 415: web.unsupportedmediatype, + 416: _range_not_satisfiable, 500: web.internalerror, } @@ -445,12 +455,86 @@ class SingleHandler(BaseHandler): raise self.http(204) +class Pagination(object): + """Get pagination scope from init or HTTP request arguments""" + + def convert(self, x): + """ret. None if x=None, else ret. x as int>=0; else raise 400""" + + val = x + if val is not None: + if type(val) is not int: + try: + val = int(x) + except ValueError: + raise BaseHandler.http(400, 'Cannot convert "%s" to int' + % x) + # raise on negative values + if val < 0: + raise BaseHandler.http(400, 'Negative limit/offset not \ + allowed') + return val + + def get_order_by(self, order_by): + if order_by: + order_by = [s.strip() for s in order_by.split(',') if s.strip()] + return order_by if order_by else None + + def __init__(self, limit=None, offset=None, order_by=None): + if limit is not None or offset is not None or order_by is not None: + # init with provided arguments + self.limit = self.convert(limit) + self.offset = self.convert(offset) + self.order_by = self.get_order_by(order_by) + else: + # init with HTTP arguments + self.limit = self.convert(web.input(limit=None).limit) + self.offset = self.convert(web.input(offset=None).offset) + self.order_by = self.get_order_by(web.input(order_by=None) + .order_by) + + class CollectionHandler(BaseHandler): collection = None validator = BasicValidator eager = () + def get_scoped_query_and_range(self, pagination=None, filter_by=None): + """Get filtered+paged collection query and collection.ContentRange obj + + Return a scoped query, and if pagination is requested then also return + ContentRange object (see NailgunCollection.content_range) to allow to + set Content-Range header (outside of this functon). + If pagination is not set/requested, return query to all collection's + objects. + Allows getting object count without getting objects - via + content_range if pagination.limit=0. + + :param pagination: Pagination object + :param filter_by: filter dict passed to query.filter_by(\*\*dict) + :type filter_by: dict + :returns: SQLAlchemy query and ContentRange object + """ + pagination = pagination or Pagination() + query = None + content_range = None + if self.collection and self.collection.single.model: + query, content_range = self.collection.scope(pagination, filter_by) + if content_range: + if not content_range.valid: + raise self.http(416, 'Requested range "%s" cannot be ' + 'satisfied' % content_range) + return query, content_range + + def set_content_range(self, content_range): + """Set Content-Range header to indicate partial data + + :param content_range: NailgunCollection.content_range named tuple + """ + txt = 'objects {x.first}-{x.last}/{x.total}'.format(x=content_range) + web.header('Content-Range', txt) + @handle_errors @validate @serialize @@ -458,8 +542,13 @@ class CollectionHandler(BaseHandler): """:returns: Collection of JSONized REST objects. :http: * 200 (OK) + * 400 (Bad Request) + * 406 (requested range not satisfiable) """ - q = self.collection.eager(None, self.eager) + query, content_range = self.get_scoped_query_and_range() + if content_range: + self.set_content_range(content_range) + q = self.collection.eager(query, self.eager) return self.collection.to_list(q) @handle_errors diff --git a/nailgun/nailgun/objects/base.py b/nailgun/nailgun/objects/base.py index 71574d4072..98db8ba0aa 100644 --- a/nailgun/nailgun/objects/base.py +++ b/nailgun/nailgun/objects/base.py @@ -169,6 +169,30 @@ class NailgunCollection(object): #: Single object class single = NailgunObject + @classmethod + def content_range(cls, first, last, total, valid): + """Structure to set Content-Range header + + Defines structure necessary to implement paged requests. + "total" is needed to let client calculate how many pages are available. + "valid" is used to indicate that the requested page is valid + (contains data) or not (outside of data range). + Used in NailgunCollection.scope() + + :param first: first element (row) returned + :param last: last element (row) returned + :param total: total number of elements/rows (before pagination) + :param valid: whether the pagination is within data range or not + :returns: ContentRange object (collections.namedtuple) with 4 fields + """ + rng = collections.namedtuple('ContentRange', + ['first', 'last', 'total', 'valid']) + rng.first = first + rng.last = last + rng.total = total + rng.valid = valid + return rng + @classmethod def _is_iterable(cls, obj): return isinstance( @@ -191,6 +215,53 @@ class NailgunCollection(object): """ return db().query(cls.single.model) + @classmethod + def scope(cls, pagination=None, filter_by=None): + """Return a query to collection's objects and ContentRange object + + Return a filtered and paged query, according to the provided pagination + (see api.v1.handlers.base.Pagination) + Also return ContentRange - object with index of first element, last + element and total count of elements in query(after filtering), and + a 'valid' parameter to indicate that the paging scope (limit + offset) + is valid or not (resulted in no data while there was data to provide) + + :param pagination: Pagination object + :param filter_by: dict to filter objects {field1: value1, ...} + :returns: SQLAlchemy query and ContentRange object + """ + query = cls.all() + content_range = None + if filter_by: + query = query.filter_by(**filter_by) + query_full = query + if pagination: + if pagination.limit > 0 or pagination.limit is None: + if pagination.order_by: + query = cls.order_by(query, pagination.order_by) + if pagination.offset: + query = query.offset(pagination.offset) + if pagination.limit > 0: + query = query.limit(pagination.limit) + else: + # making an empty result + query = query.filter(False) + if pagination.offset or pagination.limit is not None: + total = query_full.count() + selected = query.count() if pagination.limit != 0 else 0 + # first element index=1 + first = pagination.offset + 1 if pagination.offset else 1 + if selected == 0 or pagination.limit == 0: + # no data, report first and last as 0 + first = last = 0 + elif pagination.limit > 0: + last = min(first + pagination.limit - 1, total) + else: + last = total + valid = selected > 0 or pagination.limit == 0 or total == 0 + content_range = cls.content_range(first, last, total, valid) + return query, content_range + @classmethod def _query_order_by(cls, query, order_by): """Adds order by clause into SQLAlchemy query @@ -235,6 +306,23 @@ class NailgunCollection(object): order_by=order_by_fields)) return sorted(iterable, key=key) + @classmethod + def get_iterable(cls, iterable, require=True): + """Return either iterable or cls.all() when possible + + :param iterable: model objects collection + :returns: original iterable or an SQLAlchemy query + """ + if iterable is not None: + if cls._is_iterable(iterable) or cls._is_query(iterable): + return iterable + else: + raise TypeError("'%s' object is not iterable" % type(iterable)) + elif cls.single.model: + return cls.all() + elif require: + raise ValueError('iterable not provided and single.model not set') + @classmethod def order_by(cls, iterable, order_by): """Order given iterable by specified order_by. @@ -244,15 +332,17 @@ class NailgunCollection(object): ORDER BY criterion to SQLAlchemy query. If name starts with '-' desc ordering applies, else asc. :type order_by: tuple of strings or string + :returns: ordered iterable (SQLAlchemy query) """ - if iterable is None or not order_by: + if not iterable or not order_by: return iterable + use_iterable = cls.get_iterable(iterable) if not isinstance(order_by, (list, tuple)): order_by = (order_by,) - if cls._is_query(iterable): - return cls._query_order_by(iterable, order_by) - else: - return cls._iterable_order_by(iterable, order_by) + if cls._is_query(use_iterable): + return cls._query_order_by(use_iterable, order_by) + elif cls._is_iterable(use_iterable): + return cls._iterable_order_by(use_iterable, order_by) @classmethod def filter_by(cls, iterable, **kwargs): @@ -266,10 +356,7 @@ class NailgunCollection(object): else asc. :returns: filtered iterable (SQLAlchemy query) """ - if iterable is not None: - use_iterable = iterable - else: - use_iterable = cls.all() + use_iterable = cls.get_iterable(iterable) if cls._is_query(use_iterable): return use_iterable.filter_by(**kwargs) elif cls._is_iterable(use_iterable): @@ -291,7 +378,7 @@ class NailgunCollection(object): :param iterable: iterable (SQLAlchemy query) :returns: filtered iterable (SQLAlchemy query) """ - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if cls._is_query(use_iterable): conditions = [] for key, value in six.iteritems(kwargs): @@ -306,8 +393,6 @@ class NailgunCollection(object): ), use_iterable ) - else: - raise TypeError("First argument should be iterable") @classmethod def lock_for_update(cls, iterable): @@ -318,15 +403,13 @@ class NailgunCollection(object): :param iterable: iterable (SQLAlchemy query) :returns: filtered iterable (SQLAlchemy query) """ - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if cls._is_query(use_iterable): return use_iterable.with_lockmode('update') elif cls._is_iterable(use_iterable): # we can't lock abstract iterable, so returning as is # for compatibility return use_iterable - else: - raise TypeError("First argument should be iterable") @classmethod def filter_by_list(cls, iterable, field_name, list_of_values, @@ -341,7 +424,7 @@ class NailgunCollection(object): :returns: filtered iterable (SQLAlchemy query) """ field_getter = operator.attrgetter(field_name) - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if cls._is_query(use_iterable): result = use_iterable.filter( field_getter(cls.single.model).in_(list_of_values) @@ -353,8 +436,6 @@ class NailgunCollection(object): lambda i: field_getter(i) in list_of_values, use_iterable ) - else: - raise TypeError("First argument should be iterable") @classmethod def filter_by_id_list(cls, iterable, uid_list): @@ -382,7 +463,7 @@ class NailgunCollection(object): :param options: list of sqlalchemy eagerload types :returns: iterable (SQLAlchemy query) """ - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if options: return use_iterable.options(*options) return use_iterable @@ -404,13 +485,11 @@ class NailgunCollection(object): @classmethod def count(cls, iterable=None): - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if cls._is_query(use_iterable): return use_iterable.count() elif cls._is_iterable(use_iterable): return len(list(iterable)) - else: - raise TypeError("First argument should be iterable") @classmethod def to_list(cls, iterable=None, fields=None, serializer=None): @@ -423,7 +502,7 @@ class NailgunCollection(object): :param serializer: the custom serializer :returns: collection of objects as a list of dicts """ - use_iterable = cls.all() if iterable is None else iterable + use_iterable = cls.get_iterable(iterable) return [ cls.single.to_dict(o, fields=fields, serializer=serializer) for o in use_iterable @@ -465,7 +544,7 @@ class NailgunCollection(object): :param options: list of sqlalchemy mapper options :returns: iterable (SQLAlchemy query) """ - use_iterable = iterable or cls.all() + use_iterable = cls.get_iterable(iterable) if args: return use_iterable.options(*args) return use_iterable diff --git a/nailgun/nailgun/test/unit/test_handlers.py b/nailgun/nailgun/test/unit/test_handlers.py index e53eb06411..6c80f4b351 100644 --- a/nailgun/nailgun/test/unit/test_handlers.py +++ b/nailgun/nailgun/test/unit/test_handlers.py @@ -20,9 +20,13 @@ import urllib import web from nailgun.api.v1.handlers.base import BaseHandler +from nailgun.api.v1.handlers.base import CollectionHandler from nailgun.api.v1.handlers.base import handle_errors +from nailgun.api.v1.handlers.base import Pagination from nailgun.api.v1.handlers.base import serialize +from nailgun.objects import NodeCollection + from nailgun.test.base import BaseIntegrationTest from nailgun.utils import reverse @@ -185,3 +189,89 @@ class TestHandlers(BaseIntegrationTest): def test_get_requested_default(self): self.check_get_requested_mime({}, 'application/json') + + def test_pagination_class(self): + # test empty query + web.ctx.env = {'REQUEST_METHOD': 'GET'} + pagination = Pagination() + self.assertEqual(pagination.limit, None) + self.assertEqual(pagination.offset, None) + self.assertEqual(pagination.order_by, None) + # test value retrieval from web + order_by cleanup + q = 'limit=1&offset=5&order_by=-id, timestamp , somefield ' + web.ctx.env['QUERY_STRING'] = q + pagination = Pagination() + self.assertEqual(pagination.limit, 1) + self.assertEqual(pagination.offset, 5) + self.assertEqual(set(pagination.order_by), + set(['-id', 'timestamp', 'somefield'])) + # test incorrect values raise 400 + web.ctx.env['QUERY_STRING'] = 'limit=qwe' + self.assertRaises(web.HTTPError, Pagination) + web.ctx.env['QUERY_STRING'] = 'offset=asd' + self.assertRaises(web.HTTPError, Pagination) + web.ctx.env['QUERY_STRING'] = 'limit=' + self.assertRaises(web.HTTPError, Pagination) + web.ctx.env['QUERY_STRING'] = 'offset=-2' + self.assertRaises(web.HTTPError, Pagination) + # test constructor, limit = 0 -> 0, offset '0' -> 0, bad order_by + pagination = Pagination(0, '0', ', ,,, ,') + self.assertEqual(pagination.limit, 0) + self.assertEqual(pagination.offset, 0) + self.assertEqual(pagination.order_by, None) + + def test_pagination_of_node_collection(self): + def assert_pagination_and_cont_rng(q, cr, sz, first, last, ttl, valid): + self.assertEqual(q.count(), sz) + self.assertEqual(cr.first, first) + self.assertEqual(cr.last, last) + self.assertEqual(cr.total, ttl) + self.assertEqual(cr.valid, valid) + + self.env.create_nodes(5) + # test pagination limited to 2 first items + pagination = Pagination(limit=2) + q, cr = NodeCollection.scope(pagination) + assert_pagination_and_cont_rng(q, cr, 2, 1, 2, 5, True) + # test invalid pagination + pagination = Pagination(offset=5) + q, cr = NodeCollection.scope(pagination) + assert_pagination_and_cont_rng(q, cr, 0, 0, 0, 5, False) + # test limit=0, offset ignored + pagination = Pagination(limit=0, offset=999) + q, cr = NodeCollection.scope(pagination) + assert_pagination_and_cont_rng(q, cr, 0, 0, 0, 5, True) + # test limit+offset+order_by + pagination = Pagination(limit=3, offset=1, order_by='-id') + q, cr = NodeCollection.scope(pagination) + assert_pagination_and_cont_rng(q, cr, 3, 2, 4, 5, True) + ids = sorted([i.id for i in self.env.nodes]) + n = q.all() + self.assertEqual(n[0].id, ids[3]) + self.assertEqual(n[1].id, ids[2]) + self.assertEqual(n[2].id, ids[1]) + + def test_collection_handler(self): + FakeHandler = CollectionHandler + # setting a collection is mandatory, CollectionHandler is not ready + # to use "as-is" + FakeHandler.collection = NodeCollection + urls = ("/collection_test", "collection_test") + app = web.application(urls, {'collection_test': FakeHandler}) + resp = app.request(urls[0]) + self.assertEqual(resp.status, '200 OK') + + def test_content_range_header(self): + self.env.create_nodes(5) + FakeHandler = CollectionHandler + FakeHandler.collection = NodeCollection + urls = ("/collection_test", "collection_test") + app = web.application(urls, {'collection_test': FakeHandler}) + # test paginated query + resp = app.request("/collection_test?limit=3&offset=1") + self.assertEqual(resp.status, '200 OK') + self.assertIn('Content-Range', resp.headers) + self.assertEqual(resp.headers['Content-Range'], 'objects 2-4/5') + # test invalid range (offset = 6 >= number of nodes ---> no data) + resp = app.request("/collection_test?limit=3&offset=5&order_by=id") + self.assertEqual(resp.status, '416 Range Not Satisfiable')