Add sorting to listing endpoints (#653)

PGijsbers · web-flow · commit 732238d7eb92 · 2025-11-13T14:08:37.000+01:00
* Add deterministic order for pagination

* Add sorting to list endpoints

* Add test for sorting behavior
diff --git a/src/dependencies/sorting.py b/src/dependencies/sorting.py
@@ -0,0 +1,36 @@
+from enum import StrEnum, auto
+from typing import Annotated
+
+from fastapi import Query, Depends
+from pydantic import BaseModel
+from sqlmodel import Field
+
+
+class SortDirection(StrEnum):
+    ASC = auto()
+    DESC = auto()
+
+
+class Sort(StrEnum):
+    DATE_CREATED = auto()
+    DATE_MODIFIED = auto()
+
+
+class Sorting(BaseModel):
+    """Sorting modes for any AIoDConcept with an AIoD entry."""
+
+    direction: SortDirection = Field(
+        Query(
+            description="The direction of the sort (ascending or descending).",
+            default=SortDirection.DESC,
+        )
+    )
+    sort: Sort = Field(
+        Query(
+            description="The property to sort by.",
+            default=Sort.DATE_MODIFIED,
+        )
+    )
+
+
+SortingParams = Annotated[Sorting, Depends(Sorting)]
diff --git a/src/routers/bookmark_router.py b/src/routers/bookmark_router.py
@@ -48,6 +48,7 @@ def list_bookmarks(
         stmt = (
             select(Bookmark)
             .where(Bookmark.user_identifier == user._subject_identifier)
+            .order_by(Bookmark.created_at)
             .offset(pagination.offset)
             .limit(pagination.limit)
         )
diff --git a/src/routers/resource_router.py b/src/routers/resource_router.py
@@ -28,6 +28,7 @@
 from database.session import DbSession
 from dependencies.filtering import ResourceFilters, ResourceFiltersParams
 from dependencies.pagination import Pagination, PaginationParams
+from dependencies.sorting import SortingParams, Sorting, SortDirection
 from error_handling import as_http_exception
 from database.model.ai_asset.distribution import Distribution
 from database.model.helper_functions import get_asset_type_by_abbreviation
@@ -210,6 +211,7 @@ def get_resources(
         self,
         schema: str,
         pagination: Pagination,
+        sorting: Sorting,
         resource_filters: ResourceFilters,
         user: KeycloakUser | None = None,
         platform: str | None = None,
@@ -227,7 +229,7 @@ def get_resources(
                     else cast(Callable, self.orm_to_read)
                 )
                 resources: Any = self._retrieve_resources_and_post_process(
-                    session, pagination, resource_filters, user, platform
+                    session, pagination, sorting, resource_filters, user, platform
                 )
                 for resource in resources:
                     if not get_image and hasattr(resource, "media"):
@@ -289,13 +291,15 @@ def get_resources_func(self):
 
         def get_resources(
             pagination: PaginationParams,
+            sorting: SortingParams,
             resource_filters: ResourceFiltersParams,
             schema: self._possible_schemas_type = "aiod",  # type:ignore
             user: KeycloakUser | None = Depends(get_user_or_none),
         ):
             resources = self.get_resources(
                 schema=schema,
                 pagination=pagination,
+                sorting=sorting,
                 resource_filters=resource_filters,
                 user=user,
                 platform=None,
@@ -367,13 +371,15 @@ def get_resources(
                 ),
             ],
             pagination: PaginationParams,
+            sorting: SortingParams,
             resource_filters: ResourceFiltersParams,
             schema: self._possible_schemas_type = "aiod",  # type:ignore
             user: KeycloakUser | None = Depends(get_user_or_none),
         ):
             resources = self.get_resources(
                 schema=schema,
                 pagination=pagination,
+                sorting=sorting,
                 resource_filters=resource_filters,
                 user=user,
                 platform=platform,
@@ -725,6 +731,7 @@ def _retrieve_resources(
         self,
         session: Session,
         pagination: Pagination,
+        sorting: Sorting,
         resource_filters: ResourceFilters,
         platform: str | None = None,
     ) -> Sequence[type[RESOURCE_MODEL]]:
@@ -743,10 +750,17 @@ def _retrieve_resources(
             else True,
             AIoDEntryORM.status == EntryStatus.PUBLISHED,
         )
+        sort_attribute = getattr(AIoDEntryORM, sorting.sort.lower())
+        sort = (
+            sort_attribute.asc()
+            if sorting.direction == SortDirection.ASC
+            else sort_attribute.desc()
+        )
         query = (
             select(self.resource_class)
             .join(self.resource_class.aiod_entry, isouter=True)
             .where(where_clause)
+            .order_by(sort, AIoDEntryORM.identifier.asc())  # type: ignore[attr-defined]
             .offset(pagination.offset)
             .limit(pagination.limit)
         )
@@ -773,6 +787,7 @@ def _retrieve_resources_and_post_process(
         self,
         session: Session,
         pagination: Pagination,
+        sorting: Sorting,
         resource_filters: ResourceFilters,
         user: KeycloakUser | None = None,
         platform: str | None = None,
@@ -783,7 +798,7 @@ def _retrieve_resources_and_post_process(
         implement further verification on user access to the resource.
         """
         resources: Sequence[type[RESOURCE_MODEL]] = self._retrieve_resources(
-            session, pagination, resource_filters, platform
+            session, pagination, sorting, resource_filters, platform
         )
         return self._mask_or_filter(resources, session, user)
 
diff --git a/src/routers/resource_routers/organisation_router.py b/src/routers/resource_routers/organisation_router.py
@@ -1,4 +1,5 @@
 from database.model.agent.organisation import Organisation, organisation_versions
+from dependencies.sorting import SortingParams
 from routers.resource_router import ResourceRouter
 from fastapi import UploadFile, File, HTTPException, Query, status, APIRouter, Depends
 from http import HTTPStatus
@@ -43,6 +44,7 @@ def resource_class(self) -> type[Organisation]:
     def get_resources_func(self):
         def get_resources(
             pagination: PaginationParams,
+            sorting: SortingParams,
             resource_filters: ResourceFiltersParams,
             schema: self._possible_schemas_type = "aiod",  # type:ignore
             get_image: bool = Query(False, description="Include image bytes in response?"),
@@ -51,6 +53,7 @@ def get_resources(
             return self.get_resources(
                 schema=schema,
                 pagination=pagination,
+                sorting=sorting,
                 resource_filters=resource_filters,
                 user=user,
                 get_image=get_image,
diff --git a/src/routers/resource_routers/platform_router.py b/src/routers/resource_routers/platform_router.py
@@ -301,7 +301,12 @@ def _retrieve_resources(
         """
         Retrieve a sequence of resources from the database based on the provided identifier.
         """
-        query = select(self.resource_class).offset(pagination.offset).limit(pagination.limit)
+        query = (
+            select(self.resource_class)
+            .order_by(Platform.identifier)
+            .offset(pagination.offset)
+            .limit(pagination.limit)
+        )
         resources: Sequence = session.scalars(query).all()
         return resources
 
diff --git a/src/routers/resource_routers/project_router.py b/src/routers/resource_routers/project_router.py
@@ -4,6 +4,7 @@
 from database.model.project.project import Project, project_versions
 from dependencies.filtering import ResourceFiltersParams
 from dependencies.pagination import PaginationParams
+from dependencies.sorting import SortingParams
 from routers.resource_routers.organisation_router import add_custom_routes
 from routers.resource_router import ResourceRouter
 from versioning import Version
@@ -30,6 +31,7 @@ def resource_class(self) -> type[Project]:
     def get_resources_func(self):
         def get_resources(
             pagination: PaginationParams,
+            sorting: SortingParams,
             resource_filters: ResourceFiltersParams,
             schema: self._possible_schemas_type = "aiod",  # type:ignore
             get_image: bool = Query(False, description="Include image bytes in response?"),
@@ -38,6 +40,7 @@ def get_resources(
             return self.get_resources(
                 schema=schema,
                 pagination=pagination,
+                sorting=sorting,
                 resource_filters=resource_filters,
                 user=user,
                 get_image=get_image,
diff --git a/src/routers/user_router.py b/src/routers/user_router.py
@@ -1,3 +1,4 @@
+import datetime
 from typing import List
 
 from fastapi import APIRouter, Depends
@@ -6,6 +7,7 @@
 from sqlmodel import Session
 
 from dependencies.pagination import PaginationParams
+from dependencies.sorting import SortingParams, SortDirection, Sort
 from routers.resource_routers import versioned_routers
 from authentication import KeycloakUser, get_user_or_raise
 from database.authorization import Permission, PermissionType
@@ -48,6 +50,7 @@ def create(url_prefix: str, version: Version) -> APIRouter:
     )
     def get_versioned_resources_for_user(
         pagination: PaginationParams,
+        sorting: SortingParams,
         user: KeycloakUser = Depends(get_user_or_raise),
         session: Session = Depends(get_session),
     ) -> dict[str, list[AIoDConcept]]:
@@ -60,6 +63,8 @@ def get_versioned_resources_for_user(
             session,
             offset=pagination.offset,
             limit=limit,
+            sort_by=sorting.sort,
+            sort_direction=sorting.direction,
         )
         all_assets = [asset for assets in resources.values() for asset in assets]
         for resource in [a for a in all_assets if hasattr(a, "media")]:
@@ -70,8 +75,17 @@ def get_versioned_resources_for_user(
             r.resource_class.__tablename__: r.orm_to_read
             for r in versioned_routers.get(version, [])
         }
+
+        def sort_function(asset):
+            value = getattr(asset.aiod_entry, sorting.sort)
+            direction = -1 if sorting.direction == SortDirection.DESC else 1
+            return direction * datetime.datetime.timestamp(value)
+
         return {
-            asset_name: [orm_to_read[asset_name](asset) for asset in assets]
+            asset_name: sorted(
+                (orm_to_read[asset_name](asset) for asset in assets),
+                key=sort_function,
+            )
             for asset_name, assets in resources.items()
         }
 
@@ -84,15 +98,20 @@ def _get_resources_for_user(
     *,
     offset: int = 0,
     limit: int | None = None,
+    sort_by: Sort = Sort.DATE_MODIFIED,
+    sort_direction: SortDirection = SortDirection.DESC,
 ) -> dict[str, list[AIoDConcept]]:
     # "Ownership" is currently equivalent to having ADMIN permissions
+    sort_attribute = getattr(AIoDEntryORM, sort_by.lower())
+    sort = sort_attribute.asc() if sort_direction == SortDirection.ASC else sort_attribute.desc()
     stmt = (
         select(AIoDEntryORM)
         .join(Permission.aiod_entry)
         .where(
             Permission.user_identifier == user._subject_identifier,
             Permission.type_ == PermissionType.ADMIN,
         )
+        .order_by(sort, AIoDEntryORM.identifier.asc())  # type: ignore[attr-defined]
         .offset(offset)
     )
     if limit:
diff --git a/src/tests/routers/generic/test_router_get_all.py b/src/tests/routers/generic/test_router_get_all.py
@@ -19,7 +19,7 @@ def test_get_all_happy_path(client_test_resource: TestClient):
     with DbSession() as session:
         session.add_all(resources)
         session.commit()
-    response = client_test_resource.get("/test_resources")
+    response = client_test_resource.get("/test_resources?direction=asc")
     assert response.status_code == 200, response.json()
     response_json = response.json()
 
diff --git a/src/tests/routers/generic/test_router_platform_get_all.py b/src/tests/routers/generic/test_router_platform_get_all.py
@@ -22,7 +22,7 @@ def test_get_all_happy_path(client_test_resource: TestClient, auto_publish):
     with DbSession() as session:
         session.add_all(resources)
         session.commit()
-    response = client_test_resource.get("/platforms/example/test_resources")
+    response = client_test_resource.get("/platforms/example/test_resources?direction=asc")
     assert response.status_code == 200, response.json()
     response_json = response.json()
 
diff --git a/src/tests/routers/generic/test_router_relations.py b/src/tests/routers/generic/test_router_relations.py
@@ -181,7 +181,7 @@ def test_get_happy_path(test_objects: list[TestObject], client_with_testobject:
 
 
 def test_get_all_happy_path(client_with_testobject: TestClient):
-    response = client_with_testobject.get("/test_resources")
+    response = client_with_testobject.get("/test_resources?direction=asc")
     assert response.status_code == 200, response.json()
     response_json = response.json()
     assert "deprecated" not in response.headers
@@ -213,7 +213,7 @@ def test_post_happy_path(client_with_testobject: TestClient, auto_publish: None)
             headers={"Authorization": "Fake token"},
         )
     assert response.status_code == 200, response.json()
-    objects = client_with_testobject.get("/test_resources").json()
+    objects = client_with_testobject.get("/test_resources?direction=asc").json()
     obj = objects[-1]
     assert obj["title"] == "title"
     assert obj["named_string"] == "named_string1"
diff --git a/src/tests/routers/resource_routers/test_resource_router.py b/src/tests/routers/resource_routers/test_resource_router.py
@@ -1,9 +1,14 @@
+import itertools
 from datetime import datetime, timedelta
 from unittest.mock import Mock
 
 import pytest
 from starlette.testclient import TestClient
 
+from dependencies.sorting import SortDirection, Sort
+from tests.testutils.users import register_asset
+
+
 @pytest.mark.parametrize(
     "resource_type",
     [
@@ -67,3 +72,54 @@ def test_happy_path_with_filters(
     response_json = response.json()
     assert isinstance(response_json, list)
     assert len(response_json) == expected_count
+
+
+@pytest.mark.parametrize(
+    "resource_type",
+    [
+        "case_studies",  # generic
+        "organisations",  # currently has custom overrides
+        "projects",  # currently has custom overrides
+    ],
+)
+def test_happy_path_with_sorting(
+        client: TestClient,
+        mocked_privileged_token: Mock,
+        body_asset: dict,
+        resource_type,
+        auto_publish: None,
+):
+    # We set up the resources so that the order of `date_created` and `date_modified` are
+    # different, to assess if the `sort` (by property) parameter works later
+    first = client.post(
+        f"/{resource_type}", json=body_asset, headers={"Authorization": "Fake token"}
+    ).json()["identifier"]
+    second = client.post(
+        f"/{resource_type}", json=body_asset, headers={"Authorization": "Fake token"}
+    ).json()["identifier"]
+    client.put(
+        f"/{resource_type}/{first}",
+        json=body_asset,
+        headers={"Authorization": "Fake token"},
+    )
+
+    for sort, direction in itertools.product(list(Sort), list(SortDirection)):
+        resources = client.get(
+            f"/{resource_type}",
+            params={
+                "direction": str(direction),
+                "sort": str(sort),
+            },
+        ).json()
+
+        match sort, direction:
+            case Sort.DATE_MODIFIED, SortDirection.ASC:
+                assert [r["identifier"] for r in resources] == [second, first]
+            case Sort.DATE_MODIFIED, SortDirection.DESC:
+                assert [r["identifier"] for r in resources] == [first, second]
+            case Sort.DATE_CREATED, SortDirection.ASC:
+                assert [r["identifier"] for r in resources] == [first, second]
+            case Sort.DATE_CREATED, SortDirection.DESC:
+                assert [r["identifier"] for r in resources] == [second, first]
+            case _:
+                assert False, f"Unknown sort strategy: ({sort}, {direction})"
diff --git a/src/tests/routers/resource_routers/test_router_contact.py b/src/tests/routers/resource_routers/test_router_contact.py
@@ -161,7 +161,7 @@ def test_email_mask_for_authenticated_user(
         session.commit()
         session.refresh(contact2)
 
-    response = client.get("/contacts", headers=headers)
+    response = client.get("/contacts?direction=asc", headers=headers)
     response_json = response.json()
     assert response.status_code == 200, response_json
     assert len(response_json) == 2, response_json
@@ -172,8 +172,8 @@ def test_email_mask_for_authenticated_user(
     assert response.status_code == 200, response.json()
     response_json = response.json()
     assert set(response_json["email"]) == {"fake2@email.com", "fake@email.com"}
-    response = client.get("/platforms/aiod/contacts/", headers=headers)
 
+    response = client.get("/platforms/aiod/contacts?direction=asc", headers=headers)
     response_json = response.json()
     assert response.status_code == 200, response_json
 

Original file line number	Diff line number	Diff line change
`@@ -48,6 +48,7 @@ def list_bookmarks(`
`48`	`48`	`stmt = (`
`49`	`49`	`select(Bookmark)`
`50`	`50`	`.where(Bookmark.user_identifier == user._subject_identifier)`
	`51`	`+ .order_by(Bookmark.created_at)`
`51`	`52`	`.offset(pagination.offset)`
`52`	`53`	`.limit(pagination.limit)`
`53`	`54`	`)`