Skip to content

Commit

Permalink
feat(page): add page param to support pagination without knowing dids (
Browse files Browse the repository at this point in the history
…#254)

* feat(page): add page param to support pagination without knowing dids

* fix(page): don't default page value

* fix(page): check if none

* feat(page): order by udpated date when paginating with page

* fix(page): fix order by when page is zero

* feat(pagination): always order by date, add unit test for pagination
  • Loading branch information
Avantol13 authored Dec 2, 2019
1 parent 8313c86 commit d3c3d3e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 9 deletions.
2 changes: 1 addition & 1 deletion indexd/alias/drivers/alchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def session(self):
finally:
session.close()

def aliases(self, limit=100, start=None, size=None, hashes=None):
def aliases(self, limit=100, start=None, size=None, hashes=None, page=None):
"""
Returns list of records stored by the backend.
"""
Expand Down
11 changes: 10 additions & 1 deletion indexd/index/blueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,12 @@ def get_index():
"""
limit = flask.request.args.get("limit")
start = flask.request.args.get("start")
page = flask.request.args.get("page")

ids = flask.request.args.get("ids")
if ids:
ids = ids.split(",")
if start is not None or limit is not None:
if start is not None or limit is not None or page is not None:
raise UserError("pagination is not supported when ids is provided")
try:
limit = 100 if limit is None else int(limit)
Expand All @@ -71,6 +72,12 @@ def get_index():
if limit < 0 or limit > 1024:
raise UserError("limit must be between 0 and 1024")

if page is not None:
try:
page = int(page)
except ValueError as err:
raise UserError("page must be an integer")

size = flask.request.args.get("size")
try:
size = size if size is None else int(size)
Expand Down Expand Up @@ -123,6 +130,7 @@ def get_index():
records = blueprint.index_driver.ids(
start=start,
limit=limit,
page=page,
size=size,
file_name=file_name,
version=version,
Expand All @@ -142,6 +150,7 @@ def get_index():
"records": records,
"limit": limit,
"start": start,
"page": page,
"size": size,
"file_name": file_name,
"version": version,
Expand Down
12 changes: 11 additions & 1 deletion indexd/index/drivers/alchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ def session(self):
def ids(
self,
limit=100,
page=None,
start=None,
size=None,
urls=None,
Expand Down Expand Up @@ -432,14 +433,23 @@ def ids(
if urls_metadata or negate_params:
query = query.distinct(IndexRecord.did)

query = query.order_by(IndexRecord.did)
# order by updated date so newly added stuff is
# at the end (reduce risk that a new records ends up in a page
# earlier on) and allows for some logic to check for newly added records
# (e.g. parallelly processing from beginning -> middle and ending -> middle
# and as a final step, checking the "ending"+1 to see if there are
# new records).
query = query.order_by(IndexRecord.updated_date)

if ids:
query = query.filter(IndexRecord.did.in_(ids))
else:
# only apply limit when ids is not provided
query = query.limit(limit)

if page is not None:
query = query.offset(limit * page)

return [i.to_document_dict() for i in query]

@staticmethod
Expand Down
17 changes: 11 additions & 6 deletions openapis/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ paths:
description: number of records to return for this page, default to 100
required: false
type: integer
- name: page
in: query
description: pagination support without relying on dids. offsets results by limit*page
required: false
type: integer
produces:
- application/json
responses:
Expand Down Expand Up @@ -423,7 +428,7 @@ paths:
parameters:
- name: GUID
in: path
description: The GUID to associate the new aliases with.
description: The GUID to associate the new aliases with.
required: true
type: string
- in: body
Expand Down Expand Up @@ -495,7 +500,7 @@ paths:
parameters:
- name: GUID
in: path
description: The GUID to delete these aliases from.
description: The GUID to delete these aliases from.
required: true
type: string
responses:
Expand All @@ -506,7 +511,7 @@ paths:
schema:
$ref: '#/definitions/ErrorResponse'
security:
- basic_auth: []
- basic_auth: []
'/index/{GUID}/aliases/{ALIAS}':
delete:
tags:
Expand All @@ -519,7 +524,7 @@ paths:
parameters:
- name: GUID
in: path
description: The GUID to delete this alias from.
description: The GUID to delete this alias from.
required: true
type: string
- name: ALIAS
Expand All @@ -535,7 +540,7 @@ paths:
schema:
$ref: '#/definitions/ErrorResponse'
security:
- basic_auth: []
- basic_auth: []

'/index/{GUID}/latest':
get:
Expand Down Expand Up @@ -1022,7 +1027,7 @@ definitions:
properties:
aliases:
type: array
items:
items:
type: object
properties:
value:
Expand Down
40 changes: 40 additions & 0 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,46 @@ def test_index_list_with_start(client, user):
assert rec3["did"] in dids


def test_index_list_with_page(client, user):
data = {
"did": "testprefix:11111111-1111-1111-1111-111111111111",
"form": "object",
"size": 123,
"urls": ["s3://endpointurl/bucket/key"],
"hashes": {"md5": "8b9942cf415384b27cadf1f4d2d682e5"},
}
res = client.post("/index/", json=data, headers=user)
assert res.status_code == 200
rec1 = res.json

data["did"] = "testprefix:22222222-2222-2222-2222-222222222222"
res = client.post("/index/", json=data, headers=user)
assert res.status_code == 200
rec2 = res.json

data["did"] = "testprefix:33333333-3333-3333-3333-333333333333"
res = client.post("/index/", json=data, headers=user)
assert res.status_code == 200
rec3 = res.json

res = client.get("/index/?page=0&limit=2")
assert res.status_code == 200
rec = res.json

dids = [record["did"] for record in rec["records"]]
assert len(rec["records"]) == 2
assert rec1["did"] in dids
assert rec2["did"] in dids

res = client.get("/index/?page=1&limit=2")
assert res.status_code == 200
rec = res.json

dids = [record["did"] for record in rec["records"]]
assert len(rec["records"]) == 1
assert rec3["did"] in dids


def test_unauthorized_create(client):
# test that unauthorized post throws 403 error
data = get_doc()
Expand Down

0 comments on commit d3c3d3e

Please sign in to comment.