sts-sponsors team mailing list archive
-
sts-sponsors team
-
Mailing list archive
-
Message #08530
[Merge] ~r00ta/maas-site-manager:MAASENG-1675 into maas-site-manager:main
Jacopo Rota has proposed merging ~r00ta/maas-site-manager:MAASENG-1675 into maas-site-manager:main.
Commit message:
Add sorting capabilities to GET sites endpoint
Requested reviews:
MAAS Committers (maas-committers)
For more details, see:
https://code.launchpad.net/~r00ta/maas-site-manager/+git/site-manager/+merge/443254
What's included:
- add sorting capabilities to the GET sites endpoint as per https://warthogs.atlassian.net/browse/MAASENG-1675.
What's not included
- Exception/error response handling as the exception middleware is not implemented yet; i.e. the user specifies a parameter that does not exist, the result would be 500 - Internal server error with no more information.
--
Your team MAAS Committers is requested to review the proposed merge of ~r00ta/maas-site-manager:MAASENG-1675 into maas-site-manager:main.
diff --git a/backend/msm/db/queries.py b/backend/msm/db/queries.py
index 77b173e..c63e8a1 100644
--- a/backend/msm/db/queries.py
+++ b/backend/msm/db/queries.py
@@ -5,24 +5,29 @@ from datetime import (
)
from functools import reduce
from operator import or_
+from re import sub
from typing import Any
from uuid import UUID
from sqlalchemy import (
+ asc,
case,
ColumnOperators,
delete,
+ desc,
func,
select,
String,
Table,
Text,
+ UnaryExpression,
update,
)
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.sql.expression import FromClause
from ..schema import MAX_PAGE_SIZE
+from ..user_api._forms import SiteSortParams
from ._tables import (
Site,
SiteData,
@@ -120,10 +125,51 @@ def filters_from_arguments(
]
+def order_by_from_arguments(
+ table: Table, sort_params: SiteSortParams
+) -> list[UnaryExpression]:
+ """Return the list of ORDER BY clauses to sort the sites.
+ This enables to convert query params such as
+
+ ?sort_by=property1-desc,property2-asc
+
+ to a where clause such as
+
+ ORDER BY property1 DESC, property2 ASC
+
+ :param table: the table to create the ORDER BY clauses for
+ :param sort_params: a comma-separated string of parameters matching the
+ table's column name. The column names might have a
+ suffix '-asc' or '-desc' to sort by ascendant or
+ descendant order. If not specified, the sorting is
+ ascendant.
+
+ :returns: a list with clauses to order the sites.
+ """
+
+ def build_expr(parameter: str) -> UnaryExpression:
+ order_by_desc: bool = parameter.endswith("-desc")
+ column_name: str = (
+ sub("-desc$", "", parameter)
+ if order_by_desc
+ else sub("-asc$", "", parameter)
+ )
+ column = table.c[column_name]
+ return desc(column) if order_by_desc else asc(column)
+
+ if sort_params.sort_by:
+ return [
+ build_expr(parameter)
+ for parameter in sort_params.sort_by.split(",")
+ ]
+ return []
+
+
async def get_sites(
session: AsyncSession,
offset: int = 0,
limit: int = MAX_PAGE_SIZE,
+ sort_params: SiteSortParams | None = None,
city: list[str] | None = None,
country: list[str] | None = None,
name: list[str] | None = None,
@@ -145,6 +191,7 @@ async def get_sites(
url=url,
)
filters.append(Site.c.accepted == True) # noqa
+ order_by = order_by_from_arguments(table=Site, sort_params=sort_params)
count = await row_count(session, Site, *filters)
stmt = (
select(
@@ -193,7 +240,7 @@ async def get_sites(
Site.join(SiteData, SiteData.c.site_id == Site.c.id, isouter=True)
)
.where(*filters) # type: ignore[arg-type]
- .order_by(Site.c.id)
+ .order_by(*order_by)
.limit(limit)
.offset(offset)
)
diff --git a/backend/msm/user_api/_forms.py b/backend/msm/user_api/_forms.py
index d6d2cf7..49ebef5 100644
--- a/backend/msm/user_api/_forms.py
+++ b/backend/msm/user_api/_forms.py
@@ -16,6 +16,12 @@ class SiteFilterParams(NamedTuple):
url: list[str] | None
+class SiteSortParams(NamedTuple):
+ """Site sorting parameters."""
+
+ sort_by: str | None
+
+
async def site_filter_parameters(
city: list[str] | None = Query(default=None, title="Filter for cities"),
country: list[str]
@@ -39,3 +45,10 @@ async def site_filter_parameters(
timezone=timezone,
url=url,
)
+
+
+async def site_sort_parameters(
+ sort_by: str | None = Query(default=None, title="Sort by properties"),
+) -> SiteSortParams | None:
+ """Return parameters for site filtering."""
+ return SiteSortParams(sort_by=sort_by)
diff --git a/backend/msm/user_api/_handlers.py b/backend/msm/user_api/_handlers.py
index f7835dc..d1201f6 100644
--- a/backend/msm/user_api/_handlers.py
+++ b/backend/msm/user_api/_handlers.py
@@ -22,7 +22,9 @@ from ..schema import (
from ..settings import SETTINGS
from ._forms import (
site_filter_parameters,
+ site_sort_parameters,
SiteFilterParams,
+ SiteSortParams,
)
from ._jwt import (
authenticate_user,
@@ -52,12 +54,14 @@ async def sites_get(
authenticated_user: Annotated[User, Depends(get_authenticated_user)],
pagination_params: PaginationParams = Depends(pagination_params),
filter_params: SiteFilterParams = Depends(site_filter_parameters),
+ sort_params: SiteSortParams = Depends(site_sort_parameters),
) -> SitesGetResponse:
"""Return accepted sites."""
total, results = await queries.get_sites(
session,
offset=pagination_params.offset,
limit=pagination_params.size,
+ sort_params=sort_params,
**filter_params._asdict(),
)
return SitesGetResponse(
diff --git a/backend/tests/user_api/test_handlers.py b/backend/tests/user_api/test_handlers.py
index b4ae28c..8aea9fb 100644
--- a/backend/tests/user_api/test_handlers.py
+++ b/backend/tests/user_api/test_handlers.py
@@ -4,7 +4,10 @@ from datetime import (
)
from typing import Any
-from httpx import AsyncClient
+from httpx import (
+ AsyncClient,
+ Response,
+)
import pytest
from msm import __version__
@@ -180,6 +183,48 @@ class TestSitesHandler:
"items": [site],
}
+ async def test_get_with_sorting(
+ self, authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
+ ) -> None:
+ def extract_cities(resp: Response):
+ return [site["city"] for site in resp.json()["items"]]
+
+ await fixture.create(
+ "site", [site_details(city="Milan", country="IT")]
+ )
+ await fixture.create(
+ "site", [site_details(city="Paris", country="FR")]
+ )
+ await fixture.create("site", [site_details(city="Rome", country="IT")])
+ await fixture.create(
+ "site", [site_details(city="London", country="GB")]
+ )
+
+ response = await authenticated_user_app_client.get(
+ "/sites", params="sort_by=city-asc"
+ )
+ assert extract_cities(response) == ["London", "Milan", "Paris", "Rome"]
+
+ response = await authenticated_user_app_client.get(
+ "/sites", params="sort_by=city-desc"
+ )
+ assert extract_cities(response) == ["Rome", "Paris", "Milan", "London"]
+
+ response = await authenticated_user_app_client.get(
+ "/sites", params="sort_by=country,city-desc"
+ )
+ assert extract_cities(response) == ["Paris", "London", "Rome", "Milan"]
+
+ response = await authenticated_user_app_client.get(
+ "/sites", params="sort_by=country,city-asc"
+ )
+ assert extract_cities(response) == ["Paris", "London", "Milan", "Rome"]
+
+ response = await authenticated_user_app_client.get(
+ "/sites", params="page=2&size=2&sort_by=country,city-asc"
+ )
+ assert extract_cities(response) == ["Milan", "Rome"]
+
@pytest.mark.asyncio
class TestPendingSitesHandler:
Follow ups