diff options
| -rw-r--r-- | src/mailman/rest/docs/lists.rst | 6 | ||||
| -rw-r--r-- | src/mailman/rest/docs/membership.rst | 4 | ||||
| -rw-r--r-- | src/mailman/rest/docs/users.rst | 6 | ||||
| -rw-r--r-- | src/mailman/rest/helpers.py | 59 | ||||
| -rw-r--r-- | src/mailman/rest/lists.py | 5 | ||||
| -rw-r--r-- | src/mailman/rest/members.py | 3 | ||||
| -rw-r--r-- | src/mailman/rest/queues.py | 4 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_lists.py | 14 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_paginate.py | 97 | ||||
| -rw-r--r-- | src/mailman/rest/users.py | 6 |
10 files changed, 95 insertions, 109 deletions
diff --git a/src/mailman/rest/docs/lists.rst b/src/mailman/rest/docs/lists.rst index aa03de039..1006a8f0c 100644 --- a/src/mailman/rest/docs/lists.rst +++ b/src/mailman/rest/docs/lists.rst @@ -76,7 +76,7 @@ page. volume: 1 http_etag: "..." start: 0 - total_size: 1 + total_size: 2 >>> dump_json('http://localhost:9001/3.0/domains/example.com/lists' ... '?count=1&page=2') @@ -91,8 +91,8 @@ page. self_link: http://localhost:9001/3.0/lists/bird.example.com volume: 1 http_etag: "..." - start: 0 - total_size: 1 + start: 1 + total_size: 2 Creating lists via the API diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index f3a09cfe9..c0f767179 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -267,7 +267,7 @@ page. user: http://localhost:9001/3.0/users/3 http_etag: ... start: 0 - total_size: 1 + total_size: 2 This works with members of a single list as well as with all members. @@ -285,7 +285,7 @@ This works with members of a single list as well as with all members. user: http://localhost:9001/3.0/users/3 http_etag: ... start: 0 - total_size: 1 + total_size: 5 Owners and moderators diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst index 493cf65e2..6c12093ca 100644 --- a/src/mailman/rest/docs/users.rst +++ b/src/mailman/rest/docs/users.rst @@ -84,7 +84,7 @@ page. user_id: 1 http_etag: "..." start: 0 - total_size: 1 + total_size: 2 >>> dump_json('http://localhost:9001/3.0/users?count=1&page=2') entry 0: @@ -94,8 +94,8 @@ page. self_link: http://localhost:9001/3.0/users/2 user_id: 2 http_etag: "..." - start: 0 - total_size: 1 + start: 1 + total_size: 2 Creating users diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index 0bba86521..a78f5b379 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -114,31 +114,6 @@ def etag(resource): return json.dumps(resource, cls=ExtendedEncoder) -def paginate(method): - """Method decorator to paginate through collection result lists. - - Use this to return only a slice of a collection, specified in the request - itself. The request should use query parameters `count` and `page` to - specify the slice they want. The slice will start at index - ``(page - 1) * count`` and end (exclusive) at ``(page * count)``. - - Decorated methods must take ``self`` and ``request`` as the first two - arguments. - """ - def wrapper(self, request, *args, **kwargs): - # Allow falcon's HTTPBadRequest exceptions to percolate up. They'll - # get turned into HTTP 400 errors. - count = request.get_param_as_int('count', min=0) - page = request.get_param_as_int('page', min=1) - result = method(self, request, *args, **kwargs) - if count is None and page is None: - return result - list_start = (page - 1) * count - list_end = page * count - return result[list_start:list_end] - return wrapper - - class CollectionMixin: """Mixin class for common collection-ish things.""" @@ -170,22 +145,38 @@ class CollectionMixin: """ raise NotImplementedError + def _paginate(self, request, collection): + """Method to paginate through collection result lists. + + Use this to return only a slice of a collection, specified in the request + itself. The request should use query parameters `count` and `page` to + specify the slice they want. The slice will start at index + ``(page - 1) * count`` and end (exclusive) at ``(page * count)``. + """ + # Allow falcon's HTTPBadRequest exceptions to percolate up. They'll + # get turned into HTTP 400 errors. + count = request.get_param_as_int('count', min=0) + page = request.get_param_as_int('page', min=1) + total_size = len(collection) + if count is None and page is None: + return 0, total_size, collection + list_start = (page - 1) * count + list_end = page * count + return list_start, total_size, collection[list_start:list_end] + def _make_collection(self, request): """Provide the collection to the REST layer.""" - collection = self._get_collection(request) - if len(collection) == 0: - return dict(start=0, total_size=0) - else: + start, total_size, collection = self._paginate( + request, self._get_collection(request)) + result = dict(start=start, total_size=total_size) + if len(collection) != 0: entries = [self._resource_as_dict(resource) for resource in collection] # Tag the resources but use the dictionaries. [etag(resource) for resource in entries] # Create the collection resource - return dict( - start=0, - total_size=len(collection), - entries=entries, - ) + result['entries'] = entries + return result def path_to(self, resource): return path_to(resource, self.api_version) diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index a17946738..467e71508 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -40,7 +40,7 @@ from mailman.interfaces.subscriptions import ISubscriptionService from mailman.rest.listconf import ListConfiguration from mailman.rest.helpers import ( CollectionMixin, GetterSetter, NotFound, bad_request, child, created, - etag, no_content, not_found, okay, paginate) + etag, no_content, not_found, okay) from mailman.rest.members import AMember, MemberCollection from mailman.rest.post_moderation import HeldMessages from mailman.rest.sub_moderation import SubscriptionRequests @@ -112,7 +112,6 @@ class _ListBase(CollectionMixin): self_link=self.path_to('lists/{0}'.format(mlist.list_id)), ) - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(getUtility(IListManager)) @@ -231,7 +230,6 @@ class MembersOfList(MemberCollection): self._mlist = mailing_list self._role = role - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" # Overrides _MemberBase._get_collection() because we only want to @@ -252,7 +250,6 @@ class ListsForDomain(_ListBase): resource = self._make_collection(request) okay(response, etag(resource)) - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(self._domain.mailing_lists) diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 41cf56e3c..6e72339db 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -38,7 +38,7 @@ from mailman.interfaces.user import IUser, UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( CollectionMixin, NotFound, accepted, bad_request, child, conflict, - created, etag, no_content, not_found, okay, paginate) + created, etag, no_content, not_found, okay) from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) @@ -77,7 +77,6 @@ class _MemberBase(CollectionMixin): 'users/{}'.format(user.user_id.int)) return response - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(getUtility(ISubscriptionService)) diff --git a/src/mailman/rest/queues.py b/src/mailman/rest/queues.py index de0747b84..915a51e3e 100644 --- a/src/mailman/rest/queues.py +++ b/src/mailman/rest/queues.py @@ -28,8 +28,7 @@ from mailman.config import config from mailman.app.inject import inject_text from mailman.interfaces.listmanager import IListManager from mailman.rest.helpers import ( - CollectionMixin, bad_request, created, etag, no_content, not_found, okay, - paginate) + CollectionMixin, bad_request, created, etag, no_content, not_found, okay) from mailman.rest.validator import Validator from zope.component import getUtility @@ -50,7 +49,6 @@ class _QueuesBase(CollectionMixin): self_link=self.path_to('queues/{}'.format(name)), ) - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return sorted(config.switchboards) diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index 8e89f423c..546e431d3 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -287,7 +287,7 @@ class TestListPagination(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com/lists' '?count=1&page=1') # There are 6 total lists, but only the first one in the page. - self.assertEqual(resource['total_size'], 1) + self.assertEqual(resource['total_size'], 6) self.assertEqual(resource['start'], 0) self.assertEqual(len(resource['entries']), 1) entry = resource['entries'][0] @@ -298,8 +298,8 @@ class TestListPagination(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com/lists' '?count=1&page=2') # There are 6 total lists, but only the first one in the page. - self.assertEqual(resource['total_size'], 1) - self.assertEqual(resource['start'], 0) + self.assertEqual(resource['total_size'], 6) + self.assertEqual(resource['start'], 1) self.assertEqual(len(resource['entries']), 1) entry = resource['entries'][0] self.assertEqual(entry['fqdn_listname'], 'bee@example.com') @@ -309,8 +309,8 @@ class TestListPagination(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com/lists' '?count=1&page=6') # There are 6 total lists, but only the first one in the page. - self.assertEqual(resource['total_size'], 1) - self.assertEqual(resource['start'], 0) + self.assertEqual(resource['total_size'], 6) + self.assertEqual(resource['start'], 5) self.assertEqual(len(resource['entries']), 1) entry = resource['entries'][0] self.assertEqual(entry['fqdn_listname'], 'fly@example.com') @@ -337,6 +337,6 @@ class TestListPagination(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com/lists' '?count=1&page=7') # There are 6 total lists, but only the first one in the page. - self.assertEqual(resource['total_size'], 0) - self.assertEqual(resource['start'], 0) + self.assertEqual(resource['total_size'], 6) + self.assertEqual(resource['start'], 6) self.assertNotIn('entries', resource) diff --git a/src/mailman/rest/tests/test_paginate.py b/src/mailman/rest/tests/test_paginate.py index 830af58ff..eeba3ad92 100644 --- a/src/mailman/rest/tests/test_paginate.py +++ b/src/mailman/rest/tests/test_paginate.py @@ -27,7 +27,7 @@ import unittest from falcon import HTTPInvalidParam, Request from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction -from mailman.rest.helpers import paginate +from mailman.rest.helpers import CollectionMixin from mailman.testing.layers import RESTLayer @@ -51,79 +51,84 @@ class TestPaginateHelper(unittest.TestCase): with transaction(): self._mlist = create_list('test@example.com') + def _get_resource(self): + class Resource(CollectionMixin): + def _get_collection(self, request): + return ['one', 'two', 'three', 'four', 'five'] + def _resource_as_dict(self, res): + return {'value': res} + return Resource() + def test_no_pagination(self): # When there is no pagination params in the request, all 5 items in # the collection are returned. - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] + resource = self._get_resource() # Expect 5 items - page = get_collection(None, _FakeRequest()) - self.assertEqual(page, ['one', 'two', 'three', 'four', 'five']) + page = resource._make_collection(_FakeRequest()) + self.assertEqual(page['start'], 0) + self.assertEqual(page['total_size'], 5) + self.assertEqual( + [entry['value'] for entry in page['entries']], + ['one', 'two', 'three', 'four', 'five']) def test_valid_pagination_request_page_one(self): # ?count=2&page=1 returns the first page, with two items in it. - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - page = get_collection(None, _FakeRequest(2, 1)) - self.assertEqual(page, ['one', 'two']) + resource = self._get_resource() + page = resource._make_collection(_FakeRequest(2, 1)) + self.assertEqual(page['start'], 0) + self.assertEqual(page['total_size'], 5) + self.assertEqual( + [entry['value'] for entry in page['entries']], ['one', 'two']) def test_valid_pagination_request_page_two(self): # ?count=2&page=2 returns the second page, where a page has two items # in it. - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - page = get_collection(None, _FakeRequest(2, 2)) - self.assertEqual(page, ['three', 'four']) + resource = self._get_resource() + page = resource._make_collection(_FakeRequest(2, 2)) + self.assertEqual(page['start'], 2) + self.assertEqual(page['total_size'], 5) + self.assertEqual( + [entry['value'] for entry in page['entries']], ['three', 'four']) def test_2nd_index_larger_than_total(self): # ?count=2&page=3 returns the third page with page size 2, but the # last page only has one item in it. - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - page = get_collection(None, _FakeRequest(2, 3)) - self.assertEqual(page, ['five']) + resource = self._get_resource() + page = resource._make_collection(_FakeRequest(2, 3)) + self.assertEqual(page['start'], 4) + self.assertEqual(page['total_size'], 5) + self.assertEqual( + [entry['value'] for entry in page['entries']], ['five']) def test_out_of_range_returns_empty_list(self): # ?count=2&page=4 returns the fourth page, which doesn't exist, so an # empty collection is returned. - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - page = get_collection(None, _FakeRequest(2, 4)) - self.assertEqual(page, []) + resource = self._get_resource() + page = resource._make_collection(_FakeRequest(2, 4)) + self.assertEqual(page['start'], 6) + self.assertEqual(page['total_size'], 5) + self.assertNotIn('entries', page) def test_count_as_string_returns_bad_request(self): # ?count=two&page=2 are not valid values, so a bad request occurs. - @paginate - def get_collection(self, request): - return [] - self.assertRaises(HTTPInvalidParam, get_collection, - None, _FakeRequest('two', 1)) + resource = self._get_resource() + self.assertRaises(HTTPInvalidParam, resource._make_collection, + _FakeRequest('two', 1)) def test_negative_count(self): # ?count=-1&page=1 - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(HTTPInvalidParam, get_collection, - None, _FakeRequest(-1, 1)) + resource = self._get_resource() + self.assertRaises(HTTPInvalidParam, resource._make_collection, + _FakeRequest(-1, 1)) def test_negative_page(self): # ?count=1&page=-1 - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(HTTPInvalidParam, get_collection, - None, _FakeRequest(1, -1)) + resource = self._get_resource() + self.assertRaises(HTTPInvalidParam, resource._make_collection, + _FakeRequest(1, -1)) def test_negative_page_and_count(self): # ?count=1&page=-1 - @paginate - def get_collection(self, request): - return ['one', 'two', 'three', 'four', 'five'] - self.assertRaises(HTTPInvalidParam, get_collection, - None, _FakeRequest(-1, -1)) + resource = self._get_resource() + self.assertRaises(HTTPInvalidParam, resource._make_collection, + _FakeRequest(-1, -1)) diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index 6c352fd8c..2398f5f7e 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -35,8 +35,7 @@ from mailman.interfaces.usermanager import IUserManager from mailman.rest.addresses import UserAddresses from mailman.rest.helpers import ( BadRequest, CollectionMixin, GetterSetter, NotFound, bad_request, child, - conflict, created, etag, forbidden, no_content, not_found, okay, paginate, - path_to) + conflict, created, etag, forbidden, no_content, not_found, okay, path_to) from mailman.rest.preferences import Preferences from mailman.rest.validator import ( PatchValidator, Validator, list_of_strings_validator) @@ -149,7 +148,6 @@ class _UserBase(CollectionMixin): resource['display_name'] = user.display_name return resource - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(getUtility(IUserManager).users) @@ -460,7 +458,6 @@ class OwnersForDomain(_UserBase): self._domain.remove_owner(email) return no_content(response) - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(self._domain.owners) @@ -475,7 +472,6 @@ class ServerOwners(_UserBase): resource = self._make_collection(request) okay(response, etag(resource)) - @paginate def _get_collection(self, request): """See `CollectionMixin`.""" return list(getUtility(IUserManager).server_owners) |
