Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
a5b65681 by Aurélien Bompard at 2015-11-04T11:33:07Z
Rework pagination to fix the 'start' and 'total_size' values
- - - - -
10 changed files:
- src/mailman/rest/docs/lists.rst
- src/mailman/rest/docs/membership.rst
- src/mailman/rest/docs/users.rst
- src/mailman/rest/helpers.py
- src/mailman/rest/lists.py
- src/mailman/rest/members.py
- src/mailman/rest/queues.py
- src/mailman/rest/tests/test_lists.py
- src/mailman/rest/tests/test_paginate.py
- src/mailman/rest/users.py
Changes:
=====================================
src/mailman/rest/docs/lists.rst
=====================================
--- 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
=====================================
src/mailman/rest/docs/membership.rst
=====================================
--- 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
=====================================
src/mailman/rest/docs/users.rst
=====================================
--- 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
=====================================
src/mailman/rest/helpers.py
=====================================
--- 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)
=====================================
src/mailman/rest/lists.py
=====================================
--- 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)
=====================================
src/mailman/rest/members.py
=====================================
--- 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))
=====================================
src/mailman/rest/queues.py
=====================================
--- 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)
=====================================
src/mailman/rest/tests/test_lists.py
=====================================
--- 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'], 'b...@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'], 'f...@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)
=====================================
src/mailman/rest/tests/test_paginate.py
=====================================
--- 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('t...@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))
=====================================
src/mailman/rest/users.py
=====================================
--- 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)
View it on GitLab:
https://gitlab.com/mailman/mailman/commit/a5b6568179b9b17b2dd14b43bcccded15bf5025d
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
