Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions application/database/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2747,10 +2747,10 @@ def _expand_text_search_aliases(self, text: str) -> List[str]:
deduped.append(entry)
return deduped

def get_root_cres(self):
"""Returns CRES that only have "Contains" links"""
def _root_cres_query(self):
"""CREs that no other CRE contains: the tops of the graph."""
# select name from cre where cre.id not in (select cre from cre_links where type="Contains") and cre.id not in (select "group" from cre_links where type="Is Part OF");
cres = (
return (
self.session.query(CRE)
.filter(
~CRE.id.in_(
Expand All @@ -2766,10 +2766,34 @@ def get_root_cres(self):
)
)
)
.all()
# Without an explicit order the database is free to return rows in
# any order, so page boundaries could shift between requests and a
# CRE could appear on two pages while another was never returned.
# external_id first because that is the order the catalogue reads
# in; id breaks ties and is the primary key, so the order is total.
.order_by(CRE.external_id, CRE.id)
)

def get_root_cres(self):
"""Returns CRES that only have "Contains" links"""
cres = self._root_cres_query().all()
return self._hydrate_cres_batch(list(cres))

def get_root_cres_with_pagination(
self, page: int = 1, per_page: int = 10
) -> Tuple[List[cre_defs.CRE], int, int]:
"""One page of root CREs, in the order get_root_cres returns them.

Mirrors all_cres_with_pagination: a page past the end is empty rather
than an error, and total_pages is always reported.
"""
cres = self._root_cres_query().paginate(
Comment thread
coderabbitai[bot] marked this conversation as resolved.
page=int(page), per_page=per_page, error_out=False
)
total_pages = cres.pages
result = self._hydrate_cres_batch(list(cres.items))
return result, page, total_pages

def health_check(self) -> Dict[str, Any]:
"""Lightweight liveness/readiness probe for the serving database.

Expand Down
37 changes: 37 additions & 0 deletions application/tests/db_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,43 @@ def test_get_root_cres(self):
self.maxDiff = None
self.assertCountEqual(root_cres, [cres[0], cres[1], cres[7]])

def test_get_root_cres_with_pagination(self):
"""Pages carry only root CREs, in the same order as get_root_cres,
and a page past the end is empty rather than an error."""
sqla.session.remove()
sqla.drop_all()
sqla.create_all()
collection = db.Node_collection().with_graph()

dbcres = []
for i in range(7):
dbcres.append(
collection.add_cre(defs.CRE(name=f"C{i}", id=f"{i}{i}{i}-{i}{i}{i}"))
)
# C5 is contained by C0 and C6 is contained by C1: neither is a root.
collection.add_internal_link(
higher=dbcres[0], lower=dbcres[5], ltype=defs.LinkTypes.Contains
)
collection.add_internal_link(
higher=dbcres[1], lower=dbcres[6], ltype=defs.LinkTypes.Contains
)
collection.session.commit()

all_roots = collection.get_root_cres()
self.assertEqual(5, len(all_roots))

page1, page, total_pages = collection.get_root_cres_with_pagination(1, 2)
self.assertEqual((1, 3), (page, total_pages))
self.assertEqual(all_roots[0:2], page1)

page3, page, total_pages = collection.get_root_cres_with_pagination(3, 2)
self.assertEqual((3, 3), (page, total_pages))
self.assertEqual(all_roots[4:5], page3)

page4, page, total_pages = collection.get_root_cres_with_pagination(4, 2)
self.assertEqual([], page4)
self.assertEqual(3, total_pages)

@patch.object(db.NEO_DB, "gap_analysis")
def test_gap_analysis_disconnected(self, gap_mock):
collection = db.Node_collection()
Expand Down
180 changes: 180 additions & 0 deletions application/tests/web_main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,186 @@ def test_find_root_cres(self) -> None:
self.assertEqual(json.loads(response.data.decode()), expected)
self.assertEqual(200, response.status_code)

@patch.object(db, "Node_collection")
def test_find_root_cres_paginates_only_when_asked(self, db_mock) -> None:
"""Without page/per_page the endpoint keeps its original shape: every
root CRE and no pagination keys. The CLI import, the Explorer tree and
the MCP tool all read the whole list, so that path must not change."""
cres = [defs.CRE(name=f"root{i}", id=f"{i}{i}{i}-{i}{i}{i}") for i in range(3)]
db_mock.return_value.get_root_cres.return_value = cres

with self.app.test_client() as client:
response = client.get(
"/rest/v1/root_cres",
headers={"Content-Type": "application/json"},
)
self.assertEqual(200, response.status_code)
self.assertEqual(
{"data": [c.todict() for c in cres]}, json.loads(response.data)
)
db_mock.return_value.get_root_cres.assert_called_once_with()
db_mock.return_value.get_root_cres_with_pagination.assert_not_called()

@patch.object(db, "Node_collection")
def test_find_root_cres_caps_per_page(self, db_mock) -> None:
db_mock.return_value.get_root_cres_with_pagination.return_value = ([], 1, 1)

with self.app.test_client() as client:
client.get(
"/rest/v1/root_cres?per_page=1000",
headers={"Content-Type": "application/json"},
)

db_mock.return_value.get_root_cres_with_pagination.assert_called_once_with(
1, 100
)
db_mock.return_value.get_root_cres.assert_not_called()

def test_find_root_cres_pagination_integration(self) -> None:
"""root_cres pages through root CREs against a real database, with the
same metadata all_cres returns, and non-root CREs never appear."""
collection = db.Node_collection().with_graph()
roots = []
for i in range(25):
roots.append(
collection.add_cre(defs.CRE(name=f"root-{i}", id=f"{i:03d}-{i:03d}"))
)
# A child of the first root: reachable through a Contains link, so it
# is not a root and must be excluded from every page.
child = collection.add_cre(defs.CRE(name="child", id="999-999"))
collection.add_internal_link(
higher=roots[0], lower=child, ltype=defs.LinkTypes.Contains
)
collection.session.commit()

with self.app.test_client() as client:
response = client.get(
"/rest/v1/root_cres?per_page=10&page=1",
headers={"Content-Type": "application/json"},
)
self.assertEqual(200, response.status_code)
body = json.loads(response.data)
self.assertEqual(10, len(body["data"]))
self.assertEqual(1, body["page"])
self.assertEqual(3, body["total_pages"])

response = client.get(
"/rest/v1/root_cres?per_page=10&page=3",
headers={"Content-Type": "application/json"},
)
self.assertEqual(200, response.status_code)
body = json.loads(response.data)
self.assertEqual(5, len(body["data"]))
self.assertEqual(3, body["page"])
self.assertEqual(3, body["total_pages"])

seen = set()
for page in (1, 2, 3):
response = client.get(
f"/rest/v1/root_cres?per_page=10&page={page}",
headers={"Content-Type": "application/json"},
)
seen.update(doc["id"] for doc in json.loads(response.data)["data"])
self.assertEqual(25, len(seen))
self.assertNotIn("999-999", seen)

# Only `page` given: per_page falls back to the default page size.
response = client.get(
"/rest/v1/root_cres?page=1",
headers={"Content-Type": "application/json"},
)
body = json.loads(response.data)
self.assertEqual(web_main.ITEMS_PER_PAGE, len(body["data"]))
self.assertEqual(2, body["total_pages"])

# A page past the end is an empty page, not everything.
response = client.get(
"/rest/v1/root_cres?per_page=10&page=4",
headers={"Content-Type": "application/json"},
)
self.assertEqual(404, response.status_code)

def test_find_root_cres_orders_duplicate_external_ids_by_primary_key(self) -> None:
"""external_id is not unique on its own: the constraint is on
(name, external_id), so two root CREs may carry the same one. Ordering
by external_id alone would leave those ties undefined, which is what the
CRE.id half of _root_cres_query's sort is for. The primary keys are
fixed here rather than generated, so the expected order cannot drift."""
collection = db.Node_collection().with_graph()
# Inserted in the opposite order to their primary keys.
for pk, name in (
("bbbbbbbb-0000-0000-0000-000000000000", "tie-second"),
("aaaaaaaa-0000-0000-0000-000000000000", "tie-first"),
):
collection.session.add(
db.CRE(id=pk, external_id="500-500", name=name, description="")
)
collection.session.commit()

with self.app.test_client() as client:
response = client.get(
"/rest/v1/root_cres?per_page=10&page=1",
headers={"Content-Type": "application/json"},
)
self.assertEqual(200, response.status_code)
names = [doc["name"] for doc in json.loads(response.data)["data"]]

self.assertEqual(["tie-first", "tie-second"], names)

def test_find_root_cres_rejects_malformed_pagination(self) -> None:
"""A non-integer page or per_page is a client error, not a 500."""
collection = db.Node_collection().with_graph()
collection.add_cre(defs.CRE(name="root", id="111-111"))
collection.session.commit()

with self.app.test_client() as client:
for query in ("page=abc", "per_page=abc", "page=1.5"):
response = client.get(
f"/rest/v1/root_cres?{query}",
headers={"Content-Type": "application/json"},
)
self.assertEqual(400, response.status_code, query)

def test_find_root_cres_pages_deterministically(self) -> None:
"""Paging twice returns the same rows in the same places. Without an
ORDER BY the database may return rows in any order, which lets a CRE
appear on two pages while another is never returned at all."""
collection = db.Node_collection().with_graph()
# Insert in an order that is deliberately not the expected one. SQLite
# hands back a small table in insertion order, so a fixture inserted
# already-sorted would pass even with the ORDER BY removed.
external_ids = [f"{i:03d}-{i:03d}" for i in range(30)]
for external_id in reversed(external_ids):
collection.add_cre(defs.CRE(name=f"root-{external_id}", id=external_id))
collection.session.commit()

with self.app.test_client() as client:

def ids(page: int) -> list:
response = client.get(
f"/rest/v1/root_cres?per_page=10&page={page}",
headers={"Content-Type": "application/json"},
)
return [doc["id"] for doc in json.loads(response.data)["data"]]

first = [ids(p) for p in (1, 2, 3)]
second = [ids(p) for p in (1, 2, 3)]
self.assertEqual(first, second)

flat = [cre_id for page in first for cre_id in page]
self.assertEqual(30, len(set(flat)), "a CRE appeared on two pages")
# Inserted descending, returned ascending: this is what fails if the
# ORDER BY goes away.
self.assertEqual(external_ids, flat)

# The paginated pages are exactly the unpaginated list, in order.
response = client.get(
"/rest/v1/root_cres", headers={"Content-Type": "application/json"}
)
self.assertEqual(
[doc["id"] for doc in json.loads(response.data)["data"]], flat
)

def test_smartlink(self) -> None:
self.maxDiff = None
collection = db.Node_collection().with_graph()
Expand Down
11 changes: 8 additions & 3 deletions application/web/openapi_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,14 @@ def __init__(
"/rest/v1/root_cres",
"find_root_cres",
tags=["CRE"],
summary="Get root CREs",
query_schema=schemas.FormatQuerySchema,
response_schema=schemas.DataListResponseSchema,
summary="Get root CREs (optionally paginated)",
description=(
"Returns every root CRE by default. Pass page and/or per_page to "
"page through them; the response then also carries page and "
"total_pages, as /rest/v1/all_cres does."
),
query_schema=schemas.RootCREsQuerySchema,
response_schema=schemas.RootCREsResponseSchema,
),
PathSpec(
"/rest/v1/deeplink/{name}",
Expand Down
21 changes: 21 additions & 0 deletions application/web/openapi_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ class HealthResponseSchema(Schema):
message = fields.Str(required=False)


class RootCREsQuerySchema(FormatQuerySchema):
page = fields.Int(
required=False,
metadata={
"description": "Page number. Supplying page or per_page switches "
"the response to paginated form; without either, every root CRE "
"is returned."
},
)
per_page = fields.Int(
required=False,
metadata={"description": "Items per page (default 20, maximum 100)"},
)


class RootCREsResponseSchema(Schema):
data = fields.List(fields.Dict(keys=fields.Str(), values=fields.Raw()))
page = fields.Int(required=False)
total_pages = fields.Int(required=False)


class AllCREsQuerySchema(Schema):
page = fields.Int(required=False)
per_page = fields.Int(required=False)
Expand Down
41 changes: 39 additions & 2 deletions application/web/web_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pathlib
import re
import urllib.parse
from typing import Any
from typing import Any, Dict

from rq import job, exceptions
from rq import Queue
Expand Down Expand Up @@ -670,10 +670,47 @@ def find_root_cres() -> Any:
database = db.Node_collection()
# opt_osib = request.args.get("osib")
opt_format = request.args.get("format")
documents = database.get_root_cres()

# Pagination is opt-in. The CLI import (cre_main.download_cre_from_upstream),
# the Explorer tree and the MCP tool all read this endpoint as the complete
# list of roots, so without page/per_page the response keeps its original
# shape. With either present it pages like /rest/v1/all_cres does.
paginated = (
request.args.get("page") is not None or request.args.get("per_page") is not None
)
result: Dict[str, Any]
if paginated:
page = 1
per_page = ITEMS_PER_PAGE
# Parse once: int() on a non-integer raises ValueError, which has no
# handler and would surface as a 500 for what is a client mistake.
# A value of zero or less falls back to the default, as in all_cres.
try:
if request.args.get("page") is not None:
requested_page = int(request.args.get("page"))
if requested_page > 0:
page = requested_page

if request.args.get("per_page") is not None:
requested_per_page = int(request.args.get("per_page"))
if requested_per_page > 0:
per_page = requested_per_page
except ValueError:
abort(400, "page and per_page must be integers")
per_page = min(per_page, MAX_ITEMS_PER_PAGE)

documents, page, total_pages = database.get_root_cres_with_pagination(
page, per_page
)
else:
documents = database.get_root_cres()

if documents:
res = [doc.todict() for doc in documents]
result = {"data": res}
if paginated:
result["page"] = page
result["total_pages"] = total_pages
# if opt_osib:
# result["osib"] = odefs.cre2osib(documents).todict()
if opt_format == SupportedFormats.Markdown.value:
Expand Down
Loading