Skip to content

Commit fbef514

Browse files
authored
Provide assets by asset type alongside easier parsing (#523)
2 parents 375781a + cf89311 commit fbef514

File tree

3 files changed

+56
-42
lines changed

3 files changed

+56
-42
lines changed

src/routers/helper_functions.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,22 @@
33
from database.model.helper_functions import non_abstract_subclasses
44

55

6-
def get_all_asset_schemas():
6+
def get_all_read_classes() -> dict[str, AIoDConcept]:
7+
"""Returns a list of all schema types and a reference to their definition."""
78
available_schemas: list[AIoDConcept] = list(non_abstract_subclasses(AIoDConcept))
89
classes_dict = {clz.__tablename__: clz for clz in available_schemas if clz.__tablename__}
910
resrouters = {
1011
route.resource_name: route
1112
for route in routers.resource_routers.router_list # type: ignore
1213
}
13-
read_classes_dict = {
14+
return {
1415
name: resrouters[name].resource_class_read
1516
for name in classes_dict
1617
if name not in ["testresource", "test_object"]
1718
}
18-
responses = [
19-
{"$ref": f"#/components/schemas/{clz.__name__}"} for clz in read_classes_dict.values()
19+
20+
21+
def get_all_asset_schemas():
22+
return [
23+
{"$ref": f"#/components/schemas/{clz.__name__}"} for clz in get_all_read_classes().values()
2024
]
21-
return responses

src/routers/user_router.py

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
from http import HTTPStatus
2+
from typing import List
23

34
from fastapi import APIRouter, Depends
5+
from pydantic import BaseModel, create_model, Field
46
from sqlalchemy import select
57
from sqlmodel import Session
68

@@ -10,13 +12,24 @@
1012
from database.model.concept.aiod_entry import AIoDEntryORM
1113
from database.model.concept.concept import AIoDConcept
1214
from database.model.helper_functions import non_abstract_subclasses
13-
from routers.helper_functions import get_all_asset_schemas
15+
from routers.helper_functions import get_all_read_classes
1416

1517

1618
def create(url_prefix: str) -> APIRouter:
1719
router = APIRouter()
1820
version = "v1"
1921

22+
# We define a custom response class here to ensure all the asset
23+
# types are included, and the (schema) documentation is generated.
24+
# It also makes sure assets are deserialized the same way as
25+
# direct access would have.
26+
Catalogue = create_model(
27+
"Catalogue",
28+
**{
29+
asset_type: (List[asset_read_class], Field()) # type: ignore[valid-type]
30+
for asset_type, asset_read_class in get_all_read_classes().items()
31+
},
32+
)
2033
for path in [
2134
f"{url_prefix}/user/resources/{version}",
2235
f"{url_prefix}/v2/user/resources",
@@ -26,32 +39,19 @@ def create(url_prefix: str) -> APIRouter:
2639
path,
2740
tags=["User"],
2841
description="Return all assets for which you have administrator rights",
29-
response_model=None, # Required! Otherwise FastAPI infers it from type annotation.
30-
responses={
31-
HTTPStatus.OK: {
32-
"content": {
33-
"application/json": {
34-
"schema": {
35-
"title": "List of assets owned by the user.",
36-
"type": "array",
37-
"items": {"anyOf": get_all_asset_schemas()},
38-
}
39-
}
40-
},
41-
}
42-
},
42+
response_model=Catalogue,
4343
)(get_resources_for_logged_in_user)
4444
return router
4545

4646

4747
def get_resources_for_logged_in_user(
4848
user: KeycloakUser = Depends(get_user_or_raise),
4949
session: Session = Depends(get_session),
50-
) -> list[AIoDConcept]:
50+
) -> dict[str, list[AIoDConcept]]:
5151
return _get_resources_for_user(user, session)
5252

5353

54-
def _get_resources_for_user(user: KeycloakUser, session: Session) -> list[AIoDConcept]:
54+
def _get_resources_for_user(user: KeycloakUser, session: Session) -> dict[str, list[AIoDConcept]]:
5555
# "Ownership" is currently equivalent to having ADMIN permissions
5656
stmt = (
5757
select(AIoDEntryORM)
@@ -66,15 +66,16 @@ def _get_resources_for_user(user: KeycloakUser, session: Session) -> list[AIoDCo
6666
# We have AIoD entries, but want their respective asset information (e.g. publication).
6767
# We lack the information about what the type of the asset is, so unfortunately we
6868
# have to check all tables:
69-
found_assets = []
70-
for asset_type in non_abstract_subclasses(AIoDConcept):
71-
query = select(asset_type).where(asset_type.aiod_entry_identifier.in_(assets_to_fetch))
69+
asset_types = list(non_abstract_subclasses(AIoDConcept))
70+
found_assets: dict[str, list[AIoDConcept]] = {type_.__tablename__: [] for type_ in asset_types}
71+
for asset_type in asset_types:
72+
query = (
73+
select(asset_type)
74+
.where(asset_type.aiod_entry_identifier.in_(assets_to_fetch))
75+
.where(asset_type.date_deleted.is_(None))
76+
)
7277
assets = session.scalars(query).all()
73-
found_assets.extend(assets)
74-
if len(found_assets) == len(assets_to_fetch):
75-
return found_assets # minor optimization since queries may be expensive
76-
77-
raise RuntimeError(
78-
f"Expected to find assets for identifiers {assets_to_fetch}, "
79-
f"but only found {len(found_assets)} in total: {found_assets}."
80-
)
78+
found_assets[asset_type.__tablename__] = list(assets)
79+
if sum(map(len, found_assets.values())) == len(assets_to_fetch):
80+
break
81+
return found_assets # minor optimization since queries may be expensive

src/tests/test_user_endpoints.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,27 @@ def test_my_resources_can_be_empty(client: TestClient) -> None:
1616
with logged_in_user(ALICE):
1717
response = client.get("/user/resources", headers={"Authorization": "fake token"})
1818
assert response.status_code == HTTPStatus.OK
19-
assert response.json() == [], "A user with no resources should get an empty list"
19+
msg = "A user with no resources should get an empty list"
20+
assert all(items == [] for items in response.json().values()), msg
21+
assert len(response.json()) >= 16, "Every asset type should always be included in the response"
2022

2123

2224
def test_my_resources_shows_draft_assets(client: TestClient, publication: Publication) -> None:
2325
register_asset(publication, owner=ALICE, status=EntryStatus.DRAFT)
2426
with logged_in_user(ALICE):
2527
response = client.get("/user/resources", headers={"Authorization": "fake token"})
2628
assert response.status_code == HTTPStatus.OK
27-
assert len(response.json()) == 1, "Draft assets should be included in this view."
29+
assert len(response.json()["publication"]) == 1, "Draft assets should be included in this view."
30+
msg = "Properties should be deserialized"
31+
assert response.json()["publication"][0]["aiod_entry"]["status"] == "draft", msg
2832

2933

3034
def test_my_resources_shows_published_assets(client: TestClient, publication: Publication) -> None:
3135
register_asset(publication, owner=ALICE, status=EntryStatus.PUBLISHED)
3236
with logged_in_user(ALICE):
3337
response = client.get("/user/resources", headers={"Authorization": "fake token"})
3438
assert response.status_code == HTTPStatus.OK
35-
assert len(response.json()) == 1, "Published assets should be included in this view."
39+
assert len(response.json()["publication"]) == 1, "Published assets should be included in this view."
3640

3741

3842
def test_my_resources_shows_mixed_assets(client: TestClient, publication: Publication, organisation: Organisation) -> None:
@@ -42,26 +46,32 @@ def test_my_resources_shows_mixed_assets(client: TestClient, publication: Public
4246
response = client.get("/user/resources", headers={"Authorization": "fake token"})
4347
assert response.status_code == HTTPStatus.OK
4448

45-
pub = next(asset for asset in response.json() if asset["aiod_entry_identifier"] == 1)
46-
org = next(asset for asset in response.json() if asset["aiod_entry_identifier"] == 2)
49+
assert len(response.json()["contact"]) == 0
50+
assert len(response.json()["publication"]) == 1
51+
assert len(response.json()["organisation"]) == 1
52+
53+
pub = response.json()["publication"][0]
54+
org = response.json()["organisation"][0]
55+
4756
dataset_property = "legal_name"
4857
publication_property = "isbn"
4958

5059
assert dataset_property in org and publication_property in pub, "Assets should report properties unique to their type"
5160
assert dataset_property not in pub and publication_property not in org, "Assets should not report properties they do not have"
5261

62+
5363
def test_my_resources_shows_only_own_resources(client: TestClient, publication_factory: Callable[[], Publication]) -> None:
5464
register_asset(publication_factory(), owner=ALICE, status=EntryStatus.DRAFT)
5565
register_asset(publication_factory(), owner=ALICE, status=EntryStatus.PUBLISHED)
5666
register_asset(publication_factory(), owner=BOB, status=EntryStatus.PUBLISHED)
5767

5868
with logged_in_user(ALICE):
5969
response = client.get("/user/resources", headers={"Authorization": "fake token"})
60-
assert len(response.json()) == 2
70+
assert len(response.json()["publication"]) == 2
6171

6272
with logged_in_user(BOB):
6373
response = client.get("/user/resources", headers={"Authorization": "fake token"})
64-
assert len(response.json()) == 1
74+
assert len(response.json()["publication"]) == 1
6575

6676

6777
def test_my_resources_counts_only_if_admin(client: TestClient, publication_factory: Callable[[], Publication]) -> None:
@@ -81,8 +91,8 @@ def test_my_resources_counts_only_if_admin(client: TestClient, publication_facto
8191

8292
with logged_in_user(BOB):
8393
response = client.get("/user/resources", headers={"Authorization": "fake token"})
84-
assert len(response.json()) == 1, "Bob has ADMIN permission to one asset."
85-
assert response.json()[0]["identifier"] == identifier_three
94+
assert len(response.json()["publication"]) == 1, "Bob has ADMIN permission to one asset."
95+
assert response.json()["publication"][0]["identifier"] == identifier_three
8696

8797

8898
def test_my_resources_must_be_authorized(client: TestClient) -> None:

0 commit comments

Comments
 (0)