sts-sponsors team mailing list archive

[Alberto Donato <mp+442480@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas-site-manager:api-naming-consistency into maas-site-manager:main.

Commit message:
consistent naming in API handlers and their schemas



Requested reviews:
  MAAS Committers (maas-committers)

For more details, see:
https://code.launchpad.net/~ack/maas-site-manager/+git/site-manager/+merge/442480
-- 
Your team MAAS Committers is requested to review the proposed merge of ~ack/maas-site-manager:api-naming-consistency into maas-site-manager:main.

diff --git a/backend/msm/user_api/_handlers.py b/backend/msm/user_api/_handlers.py
index 04d1e4d..f7835dc 100644
--- a/backend/msm/user_api/_handlers.py
+++ b/backend/msm/user_api/_handlers.py
@@ -4,11 +4,11 @@ from typing import Annotated
 from fastapi import (
     Depends,
     HTTPException,
+    Request,
     status,
 )
 from sqlalchemy.ext.asyncio import AsyncSession
 
-from .. import __version__
 from ..db import (
     db_session,
     queries,
@@ -30,28 +30,29 @@ from ._jwt import (
     get_authenticated_user,
 )
 from ._schema import (
-    CreateTokensRequest,
-    CreateTokensResponse,
-    JSONWebToken,
-    PaginatedPendingSites,
-    PaginatedSites,
-    PaginatedTokens,
-    PendingSitesActionRequest,
-    UserLoginRequest,
+    LoginPostRequest,
+    LoginPostResponse,
+    PendingSitesGetResponse,
+    PendingSitesPostRequest,
+    RootGetResponse,
+    SitesGetResponse,
+    TokensGetResponse,
+    TokensPostRequest,
+    TokensPostResponse,
 )
 
 
-async def root() -> dict[str, str]:
+async def root_get(request: Request) -> RootGetResponse:
     """Root endpoint."""
-    return {"version": __version__}
+    return RootGetResponse(version=request.app.version)
 
 
-async def sites(
+async def sites_get(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
     pagination_params: PaginationParams = Depends(pagination_params),
     filter_params: SiteFilterParams = Depends(site_filter_parameters),
-) -> PaginatedSites:
+) -> SitesGetResponse:
     """Return accepted sites."""
     total, results = await queries.get_sites(
         session,
@@ -59,7 +60,7 @@ async def sites(
         limit=pagination_params.size,
         **filter_params._asdict(),
     )
-    return PaginatedSites(
+    return SitesGetResponse(
         total=total,
         page=pagination_params.page,
         size=pagination_params.size,
@@ -67,18 +68,18 @@ async def sites(
     )
 
 
-async def pending_sites(
+async def pending_sites_get(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
     pagination_params: PaginationParams = Depends(pagination_params),
-) -> PaginatedPendingSites:
+) -> PendingSitesGetResponse:
     """Return pending sites."""
     total, results = await queries.get_pending_sites(
         session,
         offset=pagination_params.offset,
         limit=pagination_params.size,
     )
-    return PaginatedPendingSites(
+    return PendingSitesGetResponse(
         total=total,
         page=pagination_params.page,
         size=pagination_params.size,
@@ -89,7 +90,7 @@ async def pending_sites(
 async def pending_sites_post(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
-    action: PendingSitesActionRequest,
+    action: PendingSitesPostRequest,
 ) -> None:
     """Accept or reject pending sites."""
     try:
@@ -107,16 +108,16 @@ async def pending_sites_post(
     return None
 
 
-async def tokens(
+async def tokens_get(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
     pagination_params: PaginationParams = Depends(pagination_params),
-) -> PaginatedTokens:
+) -> TokensGetResponse:
     """Return all tokens"""
     total, results = await queries.get_tokens(
         session, pagination_params.offset, pagination_params.size
     )
-    return PaginatedTokens(
+    return TokensGetResponse(
         total=total,
         page=pagination_params.page,
         size=pagination_params.size,
@@ -127,8 +128,8 @@ async def tokens(
 async def tokens_post(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
-    create_request: CreateTokensRequest,
-) -> CreateTokensResponse:
+    create_request: TokensPostRequest,
+) -> TokensPostResponse:
     """
     Create one or more tokens.
     Token duration (TTL) is expressed in seconds.
@@ -138,13 +139,13 @@ async def tokens_post(
         create_request.duration,
         count=create_request.count,
     )
-    return CreateTokensResponse(expired=expired, tokens=tokens)
+    return TokensPostResponse(expired=expired, tokens=tokens)
 
 
-async def login_for_access_token(
+async def login_post(
     session: Annotated[AsyncSession, Depends(db_session)],
-    user_login: UserLoginRequest,
-) -> JSONWebToken:
+    user_login: LoginPostRequest,
+) -> LoginPostResponse:
     user = await authenticate_user(
         session, user_login.username, user_login.password
     )
@@ -160,11 +161,12 @@ async def login_for_access_token(
     access_token = create_access_token(
         data={"sub": user.email}, expires_delta=access_token_expires
     )
-    return JSONWebToken(access_token=access_token, token_type="bearer")
+    return LoginPostResponse(access_token=access_token, token_type="bearer")
 
 
-async def read_users_me(
+async def users_me_get(
     session: Annotated[AsyncSession, Depends(db_session)],
     authenticated_user: Annotated[User, Depends(get_authenticated_user)],
 ) -> User:
+    """Render info about the authenticated user."""
     return authenticated_user
diff --git a/backend/msm/user_api/_jwt.py b/backend/msm/user_api/_jwt.py
index ecf2a26..44284c3 100644
--- a/backend/msm/user_api/_jwt.py
+++ b/backend/msm/user_api/_jwt.py
@@ -24,7 +24,6 @@ from ..db import db_session
 from ..db.models import UserWithPassword
 from ..db.queries import get_user
 from ..settings import SETTINGS
-from ._schema import JSONWebTokenData
 
 pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
 oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token")
@@ -80,13 +79,12 @@ async def get_authenticated_user(
         payload = jwt.decode(
             token, SETTINGS.secret_key, algorithms=[SETTINGS.algorithm]
         )
-        email: str | None = payload.get("sub")
+        email = payload.get("sub")
         if email is None:
             raise credentials_exception
-        token_data = JSONWebTokenData(email=email)
     except JWTError:
         raise credentials_exception
-    user = await get_user(session, email=token_data.email)
+    user = await get_user(session, email=email)
     if user is None:
         raise credentials_exception
     return user
diff --git a/backend/msm/user_api/_schema.py b/backend/msm/user_api/_schema.py
index f481522..f513eb5 100644
--- a/backend/msm/user_api/_schema.py
+++ b/backend/msm/user_api/_schema.py
@@ -14,7 +14,19 @@ from ..db.models import (
 from ..schema import PaginatedResults
 
 
-class CreateTokensRequest(BaseModel):
+class RootGetResponse(BaseModel):
+    """Root handler response."""
+
+    version: str
+
+
+class TokensGetResponse(PaginatedResults):
+    """List of existing tokens."""
+
+    items: list[Token]
+
+
+class TokensPostRequest(BaseModel):
     """
     Request to create one or more tokens, with a certain validity,
     expressed in seconds.
@@ -24,51 +36,37 @@ class CreateTokensRequest(BaseModel):
     duration: timedelta
 
 
-class CreateTokensResponse(BaseModel):
+class TokensPostResponse(BaseModel):
     """List of created tokens, along with their duration."""
 
     expired: datetime
     tokens: list[UUID]
 
 
-class PendingSitesActionRequest(BaseModel):
-    """Request to accept/reject sites."""
-
-    ids: list[int]
-    accept: bool
-
-
-class PaginatedSites(PaginatedResults):
+class SitesGetResponse(PaginatedResults):
     items: list[Site]
 
 
-class PaginatedPendingSites(PaginatedResults):
+class PendingSitesGetResponse(PaginatedResults):
     items: list[PendingSite]
 
 
-class PaginatedTokens(PaginatedResults):
-    items: list[Token]
+class PendingSitesPostRequest(BaseModel):
+    """Request to accept/reject sites."""
+
+    ids: list[int]
+    accept: bool
 
 
-class UserLoginRequest(BaseModel):
+class LoginPostRequest(BaseModel):
     """User login request schema."""
 
     username: str
     password: str
 
 
-class JSONWebToken(BaseModel):
-    """
-    A JSON Web Token for authenticating users.
-    """
+class LoginPostResponse(BaseModel):
+    """User login response with JSON Web Token."""
 
     access_token: str
     token_type: str
-
-
-class JSONWebTokenData(BaseModel):
-    """
-    The payload data for a JWT Token
-    """
-
-    email: str
diff --git a/backend/msm/user_api/_setup.py b/backend/msm/user_api/_setup.py
index 8a7f332..02aee09 100644
--- a/backend/msm/user_api/_setup.py
+++ b/backend/msm/user_api/_setup.py
@@ -36,12 +36,10 @@ def create_app(db_dsn: str | None = None) -> FastAPI:
         allow_headers=["*"],
     )
     app.state.db = db
-    app.router.add_api_route("/", _handlers.root, methods=["GET"])
+    app.router.add_api_route("/", _handlers.root_get, methods=["GET"])
+    app.router.add_api_route("/login", _handlers.login_post, methods=["POST"])
     app.router.add_api_route(
-        "/login", _handlers.login_for_access_token, methods=["POST"]
-    )
-    app.router.add_api_route(
-        "/requests", _handlers.pending_sites, methods=["GET"]
+        "/requests", _handlers.pending_sites_get, methods=["GET"]
     )
     app.router.add_api_route(
         "/requests",
@@ -49,12 +47,12 @@ def create_app(db_dsn: str | None = None) -> FastAPI:
         methods=["POST"],
         status_code=204,
     )
-    app.router.add_api_route("/sites", _handlers.sites, methods=["GET"])
-    app.router.add_api_route("/tokens", _handlers.tokens, methods=["GET"])
+    app.router.add_api_route("/sites", _handlers.sites_get, methods=["GET"])
+    app.router.add_api_route("/tokens", _handlers.tokens_get, methods=["GET"])
     app.router.add_api_route(
         "/tokens", _handlers.tokens_post, methods=["POST"]
     )
     app.router.add_api_route(
-        "/users/me", _handlers.read_users_me, methods=["GET"]
+        "/users/me", _handlers.users_me_get, methods=["GET"]
     )
     return app
diff --git a/backend/tests/user_api/test_handlers.py b/backend/tests/user_api/test_handlers.py
index 4ed19a1..bf62306 100644
--- a/backend/tests/user_api/test_handlers.py
+++ b/backend/tests/user_api/test_handlers.py
@@ -48,7 +48,7 @@ async def test_root_as_auth(
 
 
 @pytest.mark.asyncio
-async def test_list_sites(
+async def test_sites_get(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site1 = {
@@ -108,7 +108,7 @@ async def test_list_sites(
 
 
 @pytest.mark.asyncio
-async def test_list_sites_only_accepted(
+async def test_sites_get_only_accepted(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site1 = {
@@ -150,7 +150,7 @@ async def test_list_sites_only_accepted(
 
 
 @pytest.mark.asyncio
-async def test_list_sites_filter_timezone(
+async def test_sites_get_filter_timezone(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site1 = {
@@ -192,7 +192,7 @@ async def test_list_sites_filter_timezone(
 
 
 @pytest.mark.asyncio
-async def test_list_sites_with_stats(
+async def test_sites_get_with_stats(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     [site] = await fixture.create(
@@ -244,7 +244,7 @@ async def test_list_sites_with_stats(
 
 
 @pytest.mark.asyncio
-async def test_list_pending_sites(
+async def test_pending_sites_get(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site1 = {
@@ -290,7 +290,7 @@ async def test_list_pending_sites(
 
 
 @pytest.mark.asyncio
-async def test_accept_pending_sites(
+async def test_pending_sites_post_accept(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site = {
@@ -310,7 +310,7 @@ async def test_accept_pending_sites(
 
 
 @pytest.mark.asyncio
-async def test_reject_pending_sites(
+async def test_pending_sites_post_reject(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site = {
@@ -329,7 +329,7 @@ async def test_reject_pending_sites(
 
 
 @pytest.mark.asyncio
-async def test_post_pending_sites_invalid_ids(
+async def test_pending_sites_post_invalid_ids(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     site = {
@@ -371,7 +371,7 @@ async def test_token_time_format(
 
 
 @pytest.mark.asyncio
-async def test_list_tokens(
+async def test_tokens_get(
     authenticated_user_app_client: AuthAsyncClient, fixture: Fixture
 ) -> None:
     tokens = await fixture.create(
@@ -410,7 +410,7 @@ async def test_list_tokens(
 
 
 @pytest.mark.asyncio
-async def test_login_fails_with_wrong_password(
+async def test_login_post_fails_with_wrong_password(
     user_app_client: AsyncClient, fixture: Fixture
 ) -> None:
     phash = "$2b$12$F5sgrhRNtWAOehcoVO.XK.oSvupmcg8.0T2jCHOTg15M8N8LrpRwS"
@@ -435,7 +435,7 @@ async def test_login_fails_with_wrong_password(
 
 
 @pytest.mark.asyncio
-async def test_sites_fails_without_login(
+async def test_sites_get_unauthenticated(
     user_app_client: AsyncClient, fixture: Fixture
 ) -> None:
     site = [
@@ -459,7 +459,7 @@ async def test_sites_fails_without_login(
 
 
 @pytest.mark.asyncio
-async def test_token_fails_without_login(user_app_client: AsyncClient) -> None:
+async def test_token_get_unauthenticated(user_app_client: AsyncClient) -> None:
     seconds = 100
 
     response = await user_app_client.post(