summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mailman/rest/helpers.py20
-rw-r--r--src/mailman/rest/lists.py79
-rw-r--r--src/mailman/rest/root.py9
-rw-r--r--src/mailman/rest/tests/test_lists.py82
-rw-r--r--src/mailman/rest/tests/test_paginate.py37
5 files changed, 152 insertions, 75 deletions
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index 5b51bc436..ab1f5b76a 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -43,7 +43,6 @@ from cStringIO import StringIO
from datetime import datetime, timedelta
from enum import Enum
from lazr.config import as_boolean
-from restish import http
from restish.http import Response
from restish.resource import MethodDecorator
from webob.multidict import MultiDict
@@ -124,22 +123,15 @@ def paginate(method):
arguments.
"""
def wrapper(self, request, *args, **kwargs):
- try:
- count = int(request.GET['count'])
- page = int(request.GET['page'])
- if count < 0 or page < 0:
- return http.bad_request([], b'Invalid parameters')
- # Wrong parameter types or no GET attribute in request object.
- except (AttributeError, ValueError, TypeError):
- return http.bad_request([], b'Invalid parameters')
- # No count/page params.
- except KeyError:
- count = page = None
+ # 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 = int((page - 1) * count)
- list_end = int(page * count)
+ list_start = (page - 1) * count
+ list_end = page * count
return result[list_start:list_end]
return wrapper
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 7f0e6e797..da1a75bd1 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -26,6 +26,7 @@ __all__ = [
'ListArchivers',
'ListConfiguration',
'ListsForDomain',
+ 'Styles',
]
@@ -37,15 +38,17 @@ from restish import http, resource
from zope.component import getUtility
from mailman.app.lifecycle import create_list, remove_list
+from mailman.config import config
from mailman.interfaces.domain import BadDomainSpecificationError
from mailman.interfaces.listmanager import (
IListManager, ListAlreadyExistsError)
from mailman.interfaces.mailinglist import IListArchiverSet
from mailman.interfaces.member import MemberRole
+from mailman.interfaces.styles import IStyleManager
from mailman.interfaces.subscriptions import ISubscriptionService
from mailman.rest.configuration import ListConfiguration
from mailman.rest.helpers import (
- CollectionMixin, GetterSetter, PATCH, etag, no_content, paginate, path_to,
+ CollectionMixin, GetterSetter, NotFound, child, etag, paginate, path_to,
restish_matcher)
from mailman.rest.members import AMember, MemberCollection
from mailman.rest.moderation import HeldMessages, SubscriptionRequests
@@ -140,20 +143,21 @@ class AList(_ListBase):
else:
self._mlist = manager.get_by_list_id(list_identifier)
- @resource.GET()
- def mailing_list(self, request):
+ def on_get(self, request, response):
"""Return a single mailing list end-point."""
if self._mlist is None:
- return http.not_found()
- return http.ok([], self._resource_as_json(self._mlist))
+ falcon.responders.path_not_found(request, response)
+ else:
+ response.status = falcon.HTTP_200
+ response.body = self._resource_as_json(self._mlist)
- @resource.DELETE()
- def delete_list(self, request):
+ def on_delete(self, request, response):
"""Delete the named mailing list."""
if self._mlist is None:
- return http.not_found()
- remove_list(self._mlist)
- return no_content()
+ falcon.responders.path_not_found(request, response)
+ else:
+ remove_list(self._mlist)
+ response.status = falcon.HTTP_204
@resource.child(member_matcher)
def member(self, request, segments, role, email):
@@ -195,11 +199,11 @@ class AList(_ListBase):
return http.not_found()
return SubscriptionRequests(self._mlist)
- @resource.child()
+ @child()
def archivers(self, request, segments):
"""Return a representation of mailing list archivers."""
if self._mlist is None:
- return http.not_found()
+ return NotFound(), []
return ListArchivers(self._mlist)
@@ -229,11 +233,11 @@ class AllLists(_ListBase):
response.status = falcon.HTTP_201
response.location = location
- @resource.GET()
- def collection(self, request):
+ def on_get(self, request, response):
"""/lists"""
resource = self._make_collection(request)
- return http.ok([], etag(resource))
+ response.status = falcon.HTTP_200
+ response.body = etag(resource)
@@ -245,7 +249,7 @@ class MembersOfList(MemberCollection):
self._mlist = mailing_list
self._role = role
- #@paginate
+ @paginate
def _get_collection(self, request):
"""See `CollectionMixin`."""
# Overrides _MemberBase._get_collection() because we only want to
@@ -267,7 +271,7 @@ class ListsForDomain(_ListBase):
response.status = falcon.HTTP_200
response.body = etag(resource)
- #@paginate
+ @paginate
def _get_collection(self, request):
"""See `CollectionMixin`."""
return list(self._domain.mailing_lists)
@@ -291,21 +295,21 @@ class ArchiverGetterSetter(GetterSetter):
archiver.is_enabled = as_boolean(value)
-class ListArchivers(resource.Resource):
+class ListArchivers:
"""The archivers for a list, with their enabled flags."""
def __init__(self, mlist):
self._mlist = mlist
- @resource.GET()
- def statuses(self, request):
+ def on_get(self, request, response):
"""Get all the archiver statuses."""
archiver_set = IListArchiverSet(self._mlist)
resource = {archiver.name: archiver.is_enabled
for archiver in archiver_set.archivers}
- return http.ok([], etag(resource))
+ response.status = falcon.HTTP_200
+ response.body = etag(resource)
- def patch_put(self, request, is_optional):
+ def patch_put(self, request, response, is_optional):
archiver_set = IListArchiverSet(self._mlist)
kws = {archiver.name: ArchiverGetterSetter(self._mlist)
for archiver in archiver_set.archivers}
@@ -315,15 +319,30 @@ class ListArchivers(resource.Resource):
try:
Validator(**kws).update(self._mlist, request)
except ValueError as error:
- return http.bad_request([], str(error))
- return no_content()
+ falcon.responders.bad_request(request, response, body=str(error))
+ else:
+ response.status = falcon.HTTP_204
- @resource.PUT()
- def put_statuses(self, request):
+ def on_put(self, request, response):
"""Update all the archiver statuses."""
- return self.patch_put(request, is_optional=False)
+ self.patch_put(request, response, is_optional=False)
- @PATCH()
- def patch_statuses(self, request):
+ def on_patch(self, request, response):
"""Patch some archiver statueses."""
- return self.patch_put(request, is_optional=True)
+ self.patch_put(request, response, is_optional=True)
+
+
+
+class Styles:
+ """Simple resource representing all list styles."""
+
+ def __init__(self):
+ manager = getUtility(IStyleManager)
+ style_names = sorted(style.name for style in manager.styles)
+ self._resource = dict(
+ style_names=style_names,
+ default=config.styles.default)
+
+ def on_get(self, request, response):
+ response.status = falcon.HTTP_200
+ response.body = etag(self._resource)
diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py
index 9e9f5bfa5..c8dce0eca 100644
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -35,11 +35,10 @@ from mailman.config import config
from mailman.core.constants import system_preferences
from mailman.core.system import system
from mailman.interfaces.listmanager import IListManager
-from mailman.interfaces.styles import IStyleManager
from mailman.rest.addresses import AllAddresses, AnAddress
from mailman.rest.domains import ADomain, AllDomains
from mailman.rest.helpers import BadRequest, NotFound, child, etag, path_to
-from mailman.rest.lists import AList, AllLists
+from mailman.rest.lists import AList, AllLists, Styles
from mailman.rest.members import AMember, AllMembers, FindMembers
from mailman.rest.preferences import ReadOnlyPreferences
from mailman.rest.templates import TemplateFinder
@@ -139,11 +138,7 @@ class TopLevel:
if len(segments) == 0:
return AllLists()
elif len(segments) == 1 and segments[0] == 'styles':
- manager = getUtility(IStyleManager)
- style_names = sorted(style.name for style in manager.styles)
- resource = dict(style_names=style_names,
- default=config.styles.default)
- return http.ok([], etag(resource))
+ return Styles(), []
else:
# list-id is preferred, but for backward compatibility,
# fqdn_listname is also accepted.
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index f4cafa3f3..9426e7f27 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -22,6 +22,7 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
__all__ = [
'TestListArchivers',
+ 'TestListPagination',
'TestLists',
'TestListsMissing',
]
@@ -228,3 +229,84 @@ class TestListArchivers(unittest.TestCase):
method='PATCH')
self.assertEqual(cm.exception.code, 400)
self.assertEqual(cm.exception.reason, 'Invalid boolean value: sure')
+
+
+
+class TestListPagination(unittest.TestCase):
+ """Test mailing list pagination functionality.
+
+ We create a bunch of mailing lists within a domain. When we want to
+ get all the lists in that domain via the REST API, we need to
+ paginate over them, otherwise there could be too many for display.
+ """
+
+ layer = RESTLayer
+
+ def setUp(self):
+ with transaction():
+ # Create a bunch of mailing lists in the example.com domain.
+ create_list('ant@example.com')
+ create_list('bee@example.com')
+ create_list('cat@example.com')
+ create_list('dog@example.com')
+ create_list('emu@example.com')
+ create_list('fly@example.com')
+
+ def test_first_page(self):
+ resource, response = call_api(
+ '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['start'], 0)
+ self.assertEqual(len(resource['entries']), 1)
+ entry = resource['entries'][0]
+ self.assertEqual(entry['fqdn_listname'], 'ant@example.com')
+
+ def test_second_page(self):
+ resource, response = call_api(
+ '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(len(resource['entries']), 1)
+ entry = resource['entries'][0]
+ self.assertEqual(entry['fqdn_listname'], 'bee@example.com')
+
+ def test_last_page(self):
+ resource, response = call_api(
+ '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(len(resource['entries']), 1)
+ entry = resource['entries'][0]
+ self.assertEqual(entry['fqdn_listname'], 'fly@example.com')
+
+ def test_zeroth_page(self):
+ # Page numbers start at one.
+ with self.assertRaises(HTTPError) as cm:
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/domains/example.com/lists'
+ '?count=1&page=0')
+ self.assertEqual(cm.exception.code, 400)
+
+ def test_negative_page(self):
+ # Negative pages are not allowed.
+ with self.assertRaises(HTTPError) as cm:
+ resource, response = call_api(
+ 'http://localhost:9001/3.0/domains/example.com/lists'
+ '?count=1&page=-1')
+ self.assertEqual(cm.exception.code, 400)
+
+ def test_past_last_page(self):
+ # The 7th page doesn't exist so the collection is empty.
+ resource, response = call_api(
+ '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.assertNotIn('entries', resource)
diff --git a/src/mailman/rest/tests/test_paginate.py b/src/mailman/rest/tests/test_paginate.py
index 0774125bb..3a166d9d2 100644
--- a/src/mailman/rest/tests/test_paginate.py
+++ b/src/mailman/rest/tests/test_paginate.py
@@ -27,6 +27,7 @@ __all__ = [
import unittest
+from falcon import InvalidParam, Request
from mailman.app.lifecycle import create_list
from mailman.database.transaction import transaction
from mailman.rest.helpers import paginate
@@ -34,15 +35,15 @@ from mailman.testing.layers import RESTLayer
-class _FakeRequest:
+class _FakeRequest(Request):
"""Fake restish.http.Request object."""
def __init__(self, count=None, page=None):
- self.GET = {}
+ self._params = {}
if count is not None:
- self.GET['count'] = count
+ self._params['count'] = count
if page is not None:
- self.GET['page'] = page
+ self._params['page'] = page
@@ -105,41 +106,29 @@ class TestPaginateHelper(unittest.TestCase):
@paginate
def get_collection(self, request):
return []
- response = get_collection(None, _FakeRequest('two', 1))
- self.assertEqual(response.status, '400 Bad Request')
-
- def test_no_get_attr_returns_bad_request(self):
- # ?count=two&page=2 are not valid values so a bad request is returned.
- @paginate
- def get_collection(self, request):
- return []
- request = _FakeRequest()
- del request.GET
- # The request object has no GET attribute.
- self.assertIsNone(getattr(request, 'GET', None))
- response = get_collection(None, request)
- self.assertEqual(response.status, '400 Bad Request')
+ self.assertRaises(InvalidParam, get_collection,
+ None, _FakeRequest('two', 1))
def test_negative_count(self):
# ?count=-1&page=1
@paginate
def get_collection(self, request):
return ['one', 'two', 'three', 'four', 'five']
- response = get_collection(None, _FakeRequest(-1, 1))
- self.assertEqual(response.status, '400 Bad Request')
+ self.assertRaises(InvalidParam, get_collection,
+ None, _FakeRequest(-1, 1))
def test_negative_page(self):
# ?count=1&page=-1
@paginate
def get_collection(self, request):
return ['one', 'two', 'three', 'four', 'five']
- response = get_collection(None, _FakeRequest(1, -1))
- self.assertEqual(response.status, '400 Bad Request')
+ self.assertRaises(InvalidParam, get_collection,
+ None, _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']
- response = get_collection(None, _FakeRequest(-1, -1))
- self.assertEqual(response.status, '400 Bad Request')
+ self.assertRaises(InvalidParam, get_collection,
+ None, _FakeRequest(-1, -1))
generated by cgit v1.2.3-70-g09d2 (git 2.51.2) at 2025-11-08 15:27:19 +0000