Commit f98eb03f authored by David Sikter's avatar David Sikter
Browse files

Renamed to page_size, page_after_value, next_page_after_value, next_page_url....

Renamed to page_size, page_after_value, next_page_after_value, next_page_url. Added prev_page_url and first_page_url
parent 2bc4daab
Pipeline #97188 passed with stages
in 24 minutes and 25 seconds
......@@ -438,10 +438,10 @@ metadata_required_parameters = parameter_dependency_from_model(
class Pagination(BaseModel):
''' Defines the order, size, and page of results. '''
size: Optional[int] = Field(
page_size: Optional[int] = Field(
10, description=strip('''
The page size, e.g. the maximum number of items contained in one response.
A `size` of 0 will return no results.
A `page_size` of 0 will return no results.
'''))
order_by: Optional[str] = Field(
None, # type: ignore
......@@ -454,27 +454,28 @@ class Pagination(BaseModel):
The ordering direction of the results based on `order_by`. Its either
ascending `asc` or decending `desc`. Default is `asc`.
'''))
after: Optional[str] = Field(
page_after_value: Optional[str] = Field(
None, description=strip('''
This attribute defines the position after which the page begins, and is used
to navigate through the total list of results.
When requesting the first page, no value should be provided for `after`. Each
response will contain a value `next_after`, which can be used to obtain the
next page (by setting `after` in your next request to this value).
When requesting the first page, no value should be provided for
`page_after_value`. Each response will contain a value `next_page_after_value`,
which can be used to obtain the next page (by setting `page_after_value` in
your next request to this value).
The field is encoded as a string, and the format of `after` and `next_after`
depends on which API method is used.
The field is encoded as a string, and the format of `page_after_value` and
`next_page_after_value` depends on which API method is used.
Some API functions additionally allows a simplified navigation, by specifying
the page number in the key `page`. It is however always possible to use `after`
and `next_after` to iterate through the results.
the page number in the key `page`. It is however always possible to use
`page_after_value` and `next_page_after_value` to iterate through the results.
'''))
@validator('size')
def validate_size(cls, size): # pylint: disable=no-self-argument
assert size >= 0, 'size must be >= 0'
return size
@validator('page_size')
def validate_page_size(cls, page_size): # pylint: disable=no-self-argument
assert page_size >= 0, 'page_size must be >= 0'
return page_size
@validator('order_by')
def validate_order_by(cls, order_by): # pylint: disable=no-self-argument
......@@ -484,13 +485,14 @@ class Pagination(BaseModel):
'''
raise NotImplementedError('Validation of `order_by` not implemented!')
@validator('after')
def validate_after(cls, after, values): # pylint: disable=no-self-argument
@validator('page_after_value')
def validate_page_after_value(cls, page_after_value, values): # pylint: disable=no-self-argument
'''
Override this in your Pagination class to implement validation of the `after` value.
Override this in your Pagination class to implement validation of the
`page_after_value` value.
This method has to be implemented!
'''
raise NotImplementedError('Validation of `after` not implemented!')
raise NotImplementedError('Validation of `page_after_value` not implemented!')
class IndexBasedPagination(Pagination):
......@@ -498,14 +500,15 @@ class IndexBasedPagination(Pagination):
None, description=strip('''
For simple, index-based pagination, this should contain the number of the
requested page (1-based). When provided in a request, this attribute can be
used instead of `after` to jump to a particular results page. However, if you
specify both `after` *and* `page` in your request, they need to be consistent.
used instead of `page_after_value` to jump to a particular results page.
However, if you specify both `page_after_value` *and* `page` in your request,
they need to be consistent.
'''))
@validator('after')
def validate_after(cls, after, values): # pylint: disable=no-self-argument
@validator('page_after_value')
def validate_page_after_value(cls, page_after_value, values): # pylint: disable=no-self-argument
# This is validated in the root validator instead
return after
return page_after_value
@validator('page')
def validate_page(cls, page, values): # pylint: disable=no-self-argument
......@@ -515,41 +518,42 @@ class IndexBasedPagination(Pagination):
@root_validator(skip_on_failure=True)
def validate_values(cls, values): # pylint: disable=no-self-argument
'''
Ensure that both page and after are filled in consistently. This requires us to
look at `page`, `after` and `size` (whichever is set). If inconsistent
information is provided, an exception will be thrown.
Ensure that both `page` and `page_after_value` are filled in consistently. This
requires us to look at `page`, `page_after_value` and `page_size` (whichever is set).
If inconsistent information is provided, an exception will be thrown.
'''
page = values.get('page')
after = values.get('after')
size = values.get('size')
if after is not None:
page_after_value = values.get('page_after_value')
page_size = values.get('page_size')
if page_after_value is not None:
try:
after_int = int(after)
page_after_value_int = int(page_after_value)
except ValueError:
raise ValueError('Invalid value for `after` - could not convert to integer.')
if page is None and after is None:
# Neither page nor after provided - default to first page
raise ValueError(
'Invalid value for `page_after_value` - could not convert to integer.')
if page is None and page_after_value is None:
# Neither page nor page_after_value provided - default to first page
page = 1
after = None
elif page is not None and after is not None:
page_after_value = None
elif page is not None and page_after_value is not None:
# Both provided - check that they are consistent.
assert page != 1, '`after` should not be set for the first page'
assert size, '`size` cannot be zero or unspecified when `page` != 1'
assert after_int == (page - 1) * size - 1, 'inconsistent page/after values provided'
assert page != 1, '`page_after_value` should not be set for the first page'
assert page_size, '`page_size` cannot be zero or unspecified when `page` != 1'
assert page_after_value_int == (page - 1) * page_size - 1, 'inconsistent page/page_after_value values provided'
elif page is not None:
# Only page provided - calculate after
# Only page provided - calculate page_after_value
if page == 1:
after = None
page_after_value = None
else:
after = str((page - 1) * size - 1)
elif after is not None:
# Only after provided - calculate page
assert size, '`after` should not be set when `size` is zero'
assert (after_int + 1) % size == 0, 'illegal value for `after` provided'
page = (after_int + 1) // size + 1
page_after_value = str((page - 1) * page_size - 1)
elif page_after_value is not None:
# Only page_after_value provided - calculate page
assert page_size, '`page_after_value` should not be set when `page_size` is zero'
assert (page_after_value_int + 1) % page_size == 0, 'illegal value for `page_after_value` provided'
page = (page_after_value_int + 1) // page_size + 1
assert page >= 1, 'negative paging is not allowed'
values['page'] = page
values['after'] = after
values['page_after_value'] = page_after_value
return values
......@@ -559,18 +563,19 @@ class PaginationResponse(Pagination):
The total number of results that fit the given query. This is independent of
any pagination and aggregations.
'''))
next_after: Optional[str] = Field(
page: Optional[int] = Field(
None, description=strip('''
The *next* value to be used as `after` in a follow up requests, to get the next
page of results. If no more results are available, `next_after` will not be set.
The returned page number. Only applicable for some API methods.
'''))
next_url: Optional[str] = Field(
next_page_after_value: Optional[str] = Field(
None, description=strip('''
The url to get the next page.
The *next* value to be used as `page_after_value` in a follow up requests, to get
the next page of results. If no more results are available, `next_page_after_value`
will not be set.
'''))
page: Optional[int] = Field(
next_page_url: Optional[str] = Field(
None, description=strip('''
The returned page number. Only applicable for some API methods.
The url to get the next page.
'''))
@validator('order_by')
......@@ -578,20 +583,31 @@ class PaginationResponse(Pagination):
# No validation - behaviour of this field depends on api method
return order_by
@validator('after')
def validate_after(cls, after, values): # pylint: disable=no-self-argument
@validator('page_after_value')
def validate_page_after_value(cls, page_after_value, values): # pylint: disable=no-self-argument
# No validation - behaviour of this field depends on api method
return after
return page_after_value
@validator('page')
def validate_page(cls, page, values): # pylint: disable=no-self-argument
# No validation - behaviour of this field depends on api method
return page
@validator('next_after')
def validate_next_after(cls, next_after, values): # pylint: disable=no-self-argument
@validator('next_page_after_value')
def validate_next_page_after_value(cls, next_page_after_value, values): # pylint: disable=no-self-argument
# No validation - behaviour of this field depends on api method
return next_after
return next_page_after_value
class IndexBasedPaginationResponse(PaginationResponse):
prev_page_url: Optional[str] = Field(
None, description=strip('''
The url to get the previous page.
'''))
first_page_url: Optional[str] = Field(
None, description=strip('''
The url to get the first page.
'''))
class EntryBasedPagination(Pagination):
......@@ -612,12 +628,13 @@ class EntryBasedPagination(Pagination):
assert quantity.definition.is_scalar, 'the order_by quantity must be a scalar'
return order_by
@validator('after')
def validate_after(cls, after, values): # pylint: disable=no-self-argument
@validator('page_after_value')
def validate_page_after_value(cls, page_after_value, values): # pylint: disable=no-self-argument
order_by = values.get('order_by', calc_id)
if after is not None and order_by is not None and order_by != calc_id and ':' not in after:
after = '%s:' % after
return after
if page_after_value is not None and order_by is not None and order_by != calc_id:
if ':' not in page_after_value:
page_after_value = '%s:' % page_after_value
return page_after_value
class EntryPagination(EntryBasedPagination):
......@@ -625,18 +642,18 @@ class EntryPagination(EntryBasedPagination):
None, description=strip('''
For simple, index-based pagination, this should contain the number of the
requested page (1-based). When provided in a request, this attribute can be
used instead of `after` to jump to a particular results page.
used instead of `page_after_value` to jump to a particular results page.
However, you can only retreive up to the 10.000th entry with a page number.
Only one, `after` *or* `page` can be provided.
Only one, `page_after_value` *or* `page` can be provided.
'''))
@validator('page')
def validate_page(cls, page, values): # pylint: disable=no-self-argument
if page is not None:
assert values['after'] is None, 'There can only be one, a page number or an after value.'
assert values['page_after_value'] is None, 'There can only be one, a page number or an page_after_value value.'
assert page > 0, 'Page has to be larger than 1.'
assert page * values.get('size', 10) < 10000, 'Pagination by page is limited to 10.000 entries.'
assert page * values.get('page_size', 10) < 10000, 'Pagination by page is limited to 10.000 entries.'
return page
......@@ -744,7 +761,7 @@ class WithQueryAndPagination(WithQuery):
pagination: Optional[EntryPagination] = Body(
None,
example={
'size': 5,
'page_size': 5,
'order_by': 'upload_time'
})
......@@ -776,7 +793,7 @@ class EntriesMetadata(WithQueryAndPagination):
'uploads': {
'quantity': 'upload_id',
'pagination': {
'size': 10,
'page_size': 10,
'order_by': 'upload_time'
},
'entries': {
......
......@@ -33,8 +33,8 @@ from .auth import get_required_user
from .entries import _do_exaustive_search
from ..utils import create_responses, parameter_dependency_from_model
from ..models import (
IndexBasedPagination, PaginationResponse, Query, HTTPExceptionModel, User, Direction,
Owner, Any_)
IndexBasedPagination, IndexBasedPaginationResponse, Query, HTTPExceptionModel, User,
Direction, Owner, Any_)
router = APIRouter()
......@@ -93,7 +93,7 @@ dataset_pagination_parameters = parameter_dependency_from_model(
class DatasetsResponse(BaseModel):
pagination: PaginationResponse = Field(None)
pagination: IndexBasedPaginationResponse = Field(None)
data: List[Dataset] = Field(None) # type: ignore
......@@ -140,12 +140,12 @@ async def get_datasets(
mongodb_query = mongodb_query.order_by(order_by)
start = (pagination.page - 1) * pagination.size
end = start + pagination.size
start = (pagination.page - 1) * pagination.page_size
end = start + pagination.page_size
pagination_response = PaginationResponse(
pagination_response = IndexBasedPaginationResponse(
total=mongodb_query.count(),
next_after=str(end - 1) if pagination.page != 1 and end < mongodb_query.count() else None,
next_page_after_value=str(end - 1) if pagination.page != 1 and end < mongodb_query.count() else None,
**pagination.dict()) # type: ignore
return {
......
......@@ -162,20 +162,20 @@ async def get_entries_metadata(
def _do_exaustive_search(owner: Owner, query: Query, include: List[str], user: User) -> Iterator[Dict[str, Any]]:
after = None
page_after_value = None
while True:
response = perform_search(
owner=owner, query=query,
pagination=EntryPagination(size=100, after=after, order_by='upload_id'),
pagination=EntryPagination(size=100, page_after_value=page_after_value, order_by='upload_id'),
required=MetadataRequired(include=include),
user_id=user.user_id if user is not None else None)
after = response.pagination.next_after
page_after_value = response.pagination.next_page_after_value
for result in response.data:
yield result
if after is None or len(response.data) == 0:
if page_after_value is None or len(response.data) == 0:
break
......@@ -256,7 +256,7 @@ def _answer_entries_raw_download_request(owner: Owner, query: Query, files: File
response = perform_search(
owner=owner, query=query,
pagination=EntryPagination(size=0),
pagination=EntryPagination(page_size=0),
required=MetadataRequired(include=[]),
user_id=user.user_id if user is not None else None)
......@@ -566,7 +566,7 @@ def _answer_entries_archive_download_request(
response = perform_search(
owner=owner, query=query,
pagination=EntryPagination(size=0),
pagination=EntryPagination(page_size=0),
required=MetadataRequired(include=[]),
user_id=user.user_id if user is not None else None)
......
......@@ -995,17 +995,17 @@ def _api_to_es_aggregation(es_search: Search, name: str, agg: Aggregation) -> A:
# 'after' feature that allows to scan through all aggregation values.
order_by = agg.pagination.order_by
if order_by is None:
composite = dict(sources={name: terms}, size=agg.pagination.size)
composite = dict(sources={name: terms}, size=agg.pagination.page_size)
else:
order_quantity = search_quantities[order_by]
sort_terms = A('terms', field=order_quantity.search_field, order=agg.pagination.order.value)
composite = dict(sources=[{order_by: sort_terms}, {quantity.name: terms}], size=agg.pagination.size)
composite = dict(sources=[{order_by: sort_terms}, {quantity.name: terms}], size=agg.pagination.page_size)
if agg.pagination.after is not None:
if agg.pagination.page_after_value is not None:
if order_by is None:
composite['after'] = {name: agg.pagination.after}
composite['after'] = {name: agg.pagination.page_after_value}
else:
order_value, quantity_value = agg.pagination.after.split(':')
order_value, quantity_value = agg.pagination.page_after_value.split(':')
composite['after'] = {quantity.name: quantity_value, order_quantity.name: order_value}
composite_agg = es_search.aggs.bucket('agg:%s' % name, 'composite', **composite)
......@@ -1056,10 +1056,10 @@ def _es_to_api_aggregation(es_response, name: str, agg: Aggregation) -> Aggregat
if 'after_key' in es_agg:
after_key = es_agg['after_key']
if order_by is None:
pagination.next_after = after_key[name]
pagination.next_page_after_value = after_key[name]
else:
str_values = [str(v) for v in after_key.to_dict().values()]
pagination.next_after = ':'.join(str_values)
pagination.next_page_after_value = ':'.join(str_values)
return AggregationResponse(data=agg_data, pagination=pagination, **aggregation_dict)
......@@ -1094,12 +1094,12 @@ def search(
if order_field != 'calc_id':
sort['calc_id'] = pagination.order.value
search = search.sort(sort)
search = search.extra(size=pagination.size)
if pagination.after:
search = search.extra(search_after=pagination.after.rsplit(':', 1))
search = search.extra(size=pagination.page_size)
if pagination.page_after_value:
search = search.extra(search_after=pagination.page_after_value.rsplit(':', 1))
if pagination.page:
search = search[(pagination.page - 1) * pagination.size: pagination.page * pagination.size]
search = search[(pagination.page - 1) * pagination.page_size: pagination.page * pagination.page_size]
# required
if required:
......@@ -1126,19 +1126,19 @@ def search(
more_response_data = {}
# pagination
next_after = None
next_page_after_value = None
if 0 < len(es_response.hits) < es_response.hits.total:
last = es_response.hits[-1]
if order_field == 'calc_id':
next_after = last['calc_id']
next_page_after_value = last['calc_id']
else:
after_value = last
for order_field_segment in order_field.split('.'):
after_value = after_value[order_field_segment]
next_after = '%s:%s' % (after_value, last['calc_id'])
next_page_after_value = '%s:%s' % (after_value, last['calc_id'])
pagination_response = PaginationResponse(
total=es_response.hits.total,
next_after=next_after,
next_page_after_value=next_page_after_value,
**pagination.dict())
# statistics
......
......@@ -306,7 +306,7 @@ def assert_aggregations(response_json, name, agg, total: int, size: int):
assert_at_least(agg, agg_response)
n_data = len(agg_response['data'])
assert agg.get('pagination', {}).get('size', 10) >= n_data
assert agg.get('pagination', {}).get('page_size', 10) >= n_data
assert agg_response['pagination']['total'] >= n_data
for item in agg_response['data'].values():
for key in ['size']:
......@@ -338,7 +338,7 @@ def assert_aggregations(response_json, name, agg, total: int, size: int):
def assert_pagination(pagination, pagination_response, data, order_by=None, order=None):
assert_at_least(pagination, pagination_response)
assert len(data) <= pagination_response['size']
assert len(data) <= pagination_response['page_size']
assert len(data) <= pagination_response['total']
if order is None:
......@@ -523,8 +523,8 @@ def test_entries_all_statistics(client, data):
pytest.param({'quantity': 'upload_id', 'pagination': {'order_by': 'upload_time'}}, 3, 3, 200, id='order-date'),
pytest.param({'quantity': 'upload_id', 'pagination': {'order_by': 'dft.n_calculations'}}, 3, 3, 200, id='order-int'),
pytest.param({'quantity': 'dft.labels_springer_classification'}, 0, 0, 200, id='no-results'),
pytest.param({'quantity': 'upload_id', 'pagination': {'after': 'id_published'}}, 3, 1, 200, id='after'),
pytest.param({'quantity': 'upload_id', 'pagination': {'order_by': 'uploader', 'after': 'Sheldon Cooper:id_published'}}, 3, 1, 200, id='after-order'),
pytest.param({'quantity': 'upload_id', 'pagination': {'page_after_value': 'id_published'}}, 3, 1, 200, id='after'),
pytest.param({'quantity': 'upload_id', 'pagination': {'order_by': 'uploader', 'page_after_value': 'Sheldon Cooper:id_published'}}, 3, 1, 200, id='after-order'),
pytest.param({'quantity': 'upload_id', 'entries': {'size': 10}}, 3, 3, 200, id='entries'),
pytest.param({'quantity': 'upload_id', 'entries': {'size': 1}}, 3, 3, 200, id='entries-size'),
pytest.param({'quantity': 'upload_id', 'entries': {'size': 0}}, -1, -1, 422, id='bad-entries'),
......@@ -536,7 +536,7 @@ def test_entries_aggregations(client, data, test_user_auth, aggregation, total,
aggregations = {'test_agg_name': aggregation}
response_json = perform_entries_metadata_test(
client, headers=headers, owner='visible', aggregations=aggregations,
pagination=dict(size=0),
pagination=dict(page_size=0),
status_code=status_code, http_method='post')
if response_json is None:
......@@ -558,7 +558,7 @@ def test_entries_aggregations(client, data, test_user_auth, aggregation, total,
@pytest.mark.parametrize('http_method', ['post', 'get'])
def test_entries_required(client, data, required, status_code, http_method):
response_json = perform_entries_metadata_test(
client, required=required, pagination={'size': 1}, status_code=status_code, http_method=http_method)
client, required=required, pagination={'page_size': 1}, status_code=status_code, http_method=http_method)
if response_json is None:
return
......@@ -751,10 +751,10 @@ def test_entries_post_query(client, data, query, status_code, total, test_method
pagination = response_json['pagination']
assert pagination['total'] == total
assert pagination['size'] == 10
assert pagination['page_size'] == 10
assert pagination['order_by'] == 'calc_id'
assert pagination['order'] == 'asc'
assert ('next_after' in pagination) == (total > 10)
assert ('next_page_after_value' in pagination) == (total > 10)
@pytest.mark.parametrize('query, status_code, total', [
......@@ -808,10 +808,10 @@ def test_entries_get_query(client, data, query, status_code, total, test_method)
pagination = response_json['pagination']
assert pagination['total'] == total
assert pagination['size'] == 10
assert pagination['page_size'] == 10
assert pagination['order_by'] == 'calc_id'
assert pagination['order'] == 'asc'
assert ('next_after' in pagination) == (total > 10)
assert ('next_page_after_value' in pagination) == (total > 10)
@pytest.mark.parametrize('owner, user, status_code, total', [
......@@ -859,22 +859,22 @@ def test_entries_owner(
@pytest.mark.parametrize('pagination, response_pagination, status_code', [
pytest.param({}, {'total': 23, 'size': 10, 'next_after': 'id_10'}, 200, id='empty'),
pytest.param({'size': 1}, {'total': 23, 'size': 1, 'next_after': 'id_01'}, 200, id='size'),
pytest.param({'size': 0}, {'total': 23, 'size': 0}, 200, id='size-0'),
pytest.param({'size': 1, 'after': 'id_01'}, {'after': 'id_01', 'next_after': 'id_02'}, 200, id='after'),
pytest.param({'size': 1, 'after': 'id_02', 'order': 'desc'}, {'next_after': 'id_01'}, 200, id='after-desc'),
pytest.param({'size': 1, 'order_by': 'n_atoms'}, {'next_after': '2:id_01'}, 200, id='order-by-after-int'),
pytest.param({'size': 1, 'order_by': 'dft.code_name'}, {'next_after': 'VASP:id_01'}, 200, id='order-by-after-nested'),
pytest.param({'size': -1}, None, 422, id='bad-size'),
pytest.param({}, {'total': 23, 'page_size': 10, 'next_page_after_value': 'id_10'}, 200, id='empty'),
pytest.param({'page_size': 1}, {'total': 23, 'page_size': 1, 'next_page_after_value': 'id_01'}, 200, id='size'),
pytest.param({'page_size': 0}, {'total': 23, 'page_size': 0}, 200, id='size-0'),
pytest.param({'page_size': 1, 'page_after_value': 'id_01'}, {'page_after_value': 'id_01', 'next_page_after_value': 'id_02'}, 200, id='after'),
pytest.param({'page_size': 1, 'page_after_value': 'id_02', 'order': 'desc'}, {'next_page_after_value': 'id_01'}, 200, id='after-desc'),
pytest.param({'page_size': 1, 'order_by': 'n_atoms'}, {'next_page_after_value': '2:id_01'}, 200, id='order-by-after-int'),
pytest.param({'page_size': 1, 'order_by': 'dft.code_name'}, {'next_page_after_value': 'VASP:id_01'}, 200, id='order-by-after-nested'),
pytest.param({'page_size': -1}, None, 422, id='bad-size'),
pytest.param({'order': 'misspelled'}, None, 422, id='bad-order'),
pytest.param({'order_by': 'misspelled'}, None, 422, id='bad-order-by'),
pytest.param({'order_by': 'atoms', 'after': 'H:id_01'}, None, 422, id='order-by-list'),
pytest.param({'order_by': 'n_atoms', 'after': 'some'}, None, 400, id='order-by-bad-after'),
pytest.param({'page': 1, 'size': 1}, {'total': 23, 'size': 1, 'next_after': 'id_02', 'page': 1}, 200, id='page-1'),
pytest.param({'page': 2, 'size': 1}, {'total': 23, 'size': 1, 'next_after': 'id_03', 'page': 2}, 200, id='page-2'),
pytest.param({'page': 1000, 'size': 10}, None, 422, id='page-too-large'),
pytest.param({'page': 9999, 'size': 1}, None, 200, id='page-just-small-enough'),
pytest.param({'order_by': 'atoms', 'page_after_value': 'H:id_01'}, None, 422, id='order-by-list'),
pytest.param({'order_by': 'n_atoms', 'page_after_value': 'some'}, None, 400, id='order-by-bad-after'),
pytest.param({'page': 1, 'page_size': 1}, {'total': 23, 'page_size': 1, 'next_page_after_value': 'id_02', 'page': 1}, 200, id='page-1'),
pytest.param({'page': 2, 'page_size': 1}, {'total': 23, 'page_size': 1, 'next_page_after_value': 'id_03', 'page': 2}, 200, id='page-2'),
pytest.param({'page': 1000, 'page_size': 10}, None, 422, id='page-too-large'),
pytest.param({'page': 9999, 'page_size': 1}, None, 200, id='page-just-small-enough'),
])
@pytest.mark.parametrize('http_method', ['post', 'get'])
@pytest.mark.parametrize('test_method', [
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment