diff --git a/jupyterhub/apihandlers/base.py b/jupyterhub/apihandlers/base.py index d439d582..713fbfa9 100644 --- a/jupyterhub/apihandlers/base.py +++ b/jupyterhub/apihandlers/base.py @@ -359,8 +359,8 @@ class APIHandler(BaseHandler): ) def get_api_pagination(self): - default_limit = self.settings["app"].api_page_default_limit - max_limit = self.settings["app"].api_page_max_limit + default_limit = self.settings["api_page_default_limit"] + max_limit = self.settings["api_page_max_limit"] if not self.accepts_pagination: # if new pagination Accept header is not used, # default to the higher max page limit to reduce likelihood diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 927aba6b..deb6cf4b 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -2596,6 +2596,8 @@ class JupyterHub(Application): activity_resolution=self.activity_resolution, admin_users=self.authenticator.admin_users, admin_access=self.admin_access, + api_page_default_limit=self.api_page_default_limit, + api_page_max_limit=self.api_page_max_limit, authenticator=self.authenticator, spawner_class=self.spawner_class, base_url=self.base_url, diff --git a/jupyterhub/handlers/pages.py b/jupyterhub/handlers/pages.py index e3be65bf..fa654a6b 100644 --- a/jupyterhub/handlers/pages.py +++ b/jupyterhub/handlers/pages.py @@ -467,7 +467,7 @@ class AdminHandler(BaseHandler): allow_named_servers=self.allow_named_servers, named_server_limit_per_user=self.named_server_limit_per_user, server_version='{} {}'.format(__version__, self.version_hash), - api_page_limit=self.settings["app"].api_page_default_limit, + api_page_limit=self.settings["api_page_default_limit"], ) self.finish(html) diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 723e7bd1..36e7adba 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -46,12 +46,10 @@ from .. import orm from .. import roles from ..app import JupyterHub from ..auth import PAMAuthenticator -from ..objects import Server from ..singleuser import SingleUserNotebookApp -from ..spawner import LocalProcessSpawner from ..spawner import SimpleLocalProcessSpawner from ..utils import random_port -from ..utils import url_path_join +from ..utils import utcnow from .utils import async_requests from .utils import public_host from .utils import public_url @@ -329,6 +327,8 @@ class MockHub(JupyterHub): user = self.db.query(orm.User).filter(orm.User.name == 'user').first() if user is None: user = orm.User(name='user') + # avoid initial state inconsistency by setting initial activity + user.last_activity = utcnow() self.db.add(user) self.db.commit() metrics.TOTAL_USERS.inc() diff --git a/jupyterhub/tests/test_api.py b/jupyterhub/tests/test_api.py index ad58652a..7597c985 100644 --- a/jupyterhub/tests/test_api.py +++ b/jupyterhub/tests/test_api.py @@ -10,13 +10,17 @@ from unittest import mock from urllib.parse import quote from urllib.parse import urlparse +from pytest import fixture from pytest import mark +from tornado.httputil import url_concat import jupyterhub from .. import orm +from ..apihandlers.base import PAGINATION_MEDIA_TYPE from ..objects import Server from ..utils import url_path_join as ujoin from ..utils import utcnow +from .conftest import new_username from .mocking import public_host from .mocking import public_url from .utils import add_user @@ -168,6 +172,7 @@ TIMESTAMP = normalize_timestamp(datetime.now().isoformat() + 'Z') @mark.role async def test_get_users(app): db = app.db + r = await api_request(app, 'users', headers=auth_header(db, 'admin')) assert r.status_code == 200 @@ -177,7 +182,6 @@ async def test_get_users(app): 'name': 'user', 'admin': False, 'roles': ['user'], - 'last_activity': None, 'auth_state': None, } assert users == [ @@ -189,37 +193,117 @@ async def test_get_users(app): r = await api_request(app, 'users', headers=auth_header(db, 'user')) assert r.status_code == 403 - # Tests offset for pagination - r = await api_request(app, 'users?offset=1') + +@fixture +def default_page_limit(app): + """Set and return low default page size for testing""" + n = 10 + with mock.patch.dict(app.tornado_settings, {"api_page_default_limit": n}): + yield n + + +@fixture +def max_page_limit(app): + """Set and return low max page size for testing""" + n = 20 + with mock.patch.dict(app.tornado_settings, {"api_page_max_limit": n}): + yield n + + +@mark.user +@mark.role +@mark.parametrize( + "n, offset, limit, accepts_pagination, expected_count", + [ + (10, None, None, False, 10), + (10, None, None, True, 10), + (10, 5, None, True, 5), + (10, 5, None, False, 5), + (10, 5, 1, True, 1), + (10, 10, 10, True, 0), + ( # default page limit, pagination expected + 30, + None, + None, + True, + 'default', + ), + ( + # default max page limit, pagination not expected + 30, + None, + None, + False, + 'max', + ), + ( + # limit exceeded + 30, + None, + 500, + False, + 'max', + ), + ], +) +async def test_get_users_pagination( + app, + n, + offset, + limit, + accepts_pagination, + expected_count, + default_page_limit, + max_page_limit, +): + db = app.db + + if expected_count == 'default': + expected_count = default_page_limit + elif expected_count == 'max': + expected_count = max_page_limit + # populate users + usernames = [] + + existing_users = db.query(orm.User).order_by(orm.User.id.asc()) + usernames.extend(u.name for u in existing_users) + + for i in range(n - existing_users.count()): + name = new_username() + usernames.append(name) + add_user(db, app, name=name) + print(f"{db.query(orm.User).count()} total users") + + url = 'users' + params = {} + if offset: + params['offset'] = offset + if limit: + params['limit'] = limit + url = url_concat(url, params) + headers = auth_header(db, 'admin') + if accepts_pagination: + headers['Accept'] = PAGINATION_MEDIA_TYPE + r = await api_request(app, url, headers=headers) assert r.status_code == 200 + response = r.json() + if accepts_pagination: + assert set(response) == { + "items", + "_pagination", + } + pagination = response["_pagination"] + users = response["items"] + else: + users = response + assert len(users) == expected_count + expected_usernames = usernames + if offset: + expected_usernames = expected_usernames[offset:] + expected_usernames = expected_usernames[:expected_count] - users = sorted(r.json(), key=lambda d: d['name']) - users = [normalize_user(u) for u in users] - assert users == [ - fill_user( - {'name': 'user', 'admin': False, 'auth_state': None, 'roles': ['user']} - ) - ] - - r = await api_request(app, 'users?offset=20') - assert r.status_code == 200 - assert r.json() == [] - - # Test limit for pagination - r = await api_request(app, 'users?limit=1') - assert r.status_code == 200 - - users = sorted(r.json(), key=lambda d: d['name']) - users = [normalize_user(u) for u in users] - assert users == [ - fill_user( - {'name': 'admin', 'admin': True, 'auth_state': None, 'roles': ['admin']} - ) - ] - - r = await api_request(app, 'users?limit=0') - assert r.status_code == 200 - assert r.json() == [] + got_usernames = [u['name'] for u in users] + assert got_usernames == expected_usernames @mark.user