test pagination limits on users endpoint

This commit is contained in:
Min RK
2021-08-23 11:04:48 +02:00
parent 911d1b5081
commit d0c2bc051a
5 changed files with 122 additions and 36 deletions

View File

@@ -359,8 +359,8 @@ class APIHandler(BaseHandler):
) )
def get_api_pagination(self): def get_api_pagination(self):
default_limit = self.settings["app"].api_page_default_limit default_limit = self.settings["api_page_default_limit"]
max_limit = self.settings["app"].api_page_max_limit max_limit = self.settings["api_page_max_limit"]
if not self.accepts_pagination: if not self.accepts_pagination:
# if new pagination Accept header is not used, # if new pagination Accept header is not used,
# default to the higher max page limit to reduce likelihood # default to the higher max page limit to reduce likelihood

View File

@@ -2596,6 +2596,8 @@ class JupyterHub(Application):
activity_resolution=self.activity_resolution, activity_resolution=self.activity_resolution,
admin_users=self.authenticator.admin_users, admin_users=self.authenticator.admin_users,
admin_access=self.admin_access, 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, authenticator=self.authenticator,
spawner_class=self.spawner_class, spawner_class=self.spawner_class,
base_url=self.base_url, base_url=self.base_url,

View File

@@ -467,7 +467,7 @@ class AdminHandler(BaseHandler):
allow_named_servers=self.allow_named_servers, allow_named_servers=self.allow_named_servers,
named_server_limit_per_user=self.named_server_limit_per_user, named_server_limit_per_user=self.named_server_limit_per_user,
server_version='{} {}'.format(__version__, self.version_hash), 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) self.finish(html)

View File

@@ -46,12 +46,10 @@ from .. import orm
from .. import roles from .. import roles
from ..app import JupyterHub from ..app import JupyterHub
from ..auth import PAMAuthenticator from ..auth import PAMAuthenticator
from ..objects import Server
from ..singleuser import SingleUserNotebookApp from ..singleuser import SingleUserNotebookApp
from ..spawner import LocalProcessSpawner
from ..spawner import SimpleLocalProcessSpawner from ..spawner import SimpleLocalProcessSpawner
from ..utils import random_port from ..utils import random_port
from ..utils import url_path_join from ..utils import utcnow
from .utils import async_requests from .utils import async_requests
from .utils import public_host from .utils import public_host
from .utils import public_url from .utils import public_url
@@ -329,6 +327,8 @@ class MockHub(JupyterHub):
user = self.db.query(orm.User).filter(orm.User.name == 'user').first() user = self.db.query(orm.User).filter(orm.User.name == 'user').first()
if user is None: if user is None:
user = orm.User(name='user') user = orm.User(name='user')
# avoid initial state inconsistency by setting initial activity
user.last_activity = utcnow()
self.db.add(user) self.db.add(user)
self.db.commit() self.db.commit()
metrics.TOTAL_USERS.inc() metrics.TOTAL_USERS.inc()

View File

@@ -10,13 +10,17 @@ from unittest import mock
from urllib.parse import quote from urllib.parse import quote
from urllib.parse import urlparse from urllib.parse import urlparse
from pytest import fixture
from pytest import mark from pytest import mark
from tornado.httputil import url_concat
import jupyterhub import jupyterhub
from .. import orm from .. import orm
from ..apihandlers.base import PAGINATION_MEDIA_TYPE
from ..objects import Server from ..objects import Server
from ..utils import url_path_join as ujoin from ..utils import url_path_join as ujoin
from ..utils import utcnow from ..utils import utcnow
from .conftest import new_username
from .mocking import public_host from .mocking import public_host
from .mocking import public_url from .mocking import public_url
from .utils import add_user from .utils import add_user
@@ -168,6 +172,7 @@ TIMESTAMP = normalize_timestamp(datetime.now().isoformat() + 'Z')
@mark.role @mark.role
async def test_get_users(app): async def test_get_users(app):
db = app.db db = app.db
r = await api_request(app, 'users', headers=auth_header(db, 'admin')) r = await api_request(app, 'users', headers=auth_header(db, 'admin'))
assert r.status_code == 200 assert r.status_code == 200
@@ -177,7 +182,6 @@ async def test_get_users(app):
'name': 'user', 'name': 'user',
'admin': False, 'admin': False,
'roles': ['user'], 'roles': ['user'],
'last_activity': None,
'auth_state': None, 'auth_state': None,
} }
assert users == [ assert users == [
@@ -189,37 +193,117 @@ async def test_get_users(app):
r = await api_request(app, 'users', headers=auth_header(db, 'user')) r = await api_request(app, 'users', headers=auth_header(db, 'user'))
assert r.status_code == 403 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 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']) got_usernames = [u['name'] for u in users]
users = [normalize_user(u) for u in users] assert got_usernames == expected_usernames
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() == []
@mark.user @mark.user