Skip to content

Commit a713e41

Browse files
authored
refactor(identity): layer the module into router/service/repository/github (#61)
- repository.py owns every query against github_users; the service no longer builds SQL - github.py is a typed boundary: GitHubOAuthToken and GitHubUserProfile replace the bare dicts that used to cross the domain, so a GitHub shape change fails at the edge instead of as a KeyError in a service - service.py exposes use cases (authenticate_with_code, sync_user, refresh_tokens) and returns a TokenPair object instead of an untyped 2-tuple; get_user_stats returns UserStatsResponse instead of a dict - container/repository.py is introduced to own the ContainerSession aggregates the dashboard needs, so identity stops writing SQL over another module's tables - router.py is thin: validate, call one use case, shape the response. Inline UnauthorizedError imports hoisted, cookie policy factored into one helper - logout now clears the cookie with the same attributes used to set it (it was hardcoded secure=True, so it silently failed over plain HTTP) and is rate-limited like its neighbours - dead RefreshRequest schema removed - tests rewritten off unittest.mock onto httpx.MockTransport: 17 -> 26 tests covering the boundary parsing, token claims and the stats path 317 tests pass, ruff and mypy clean.
1 parent 380bc65 commit a713e41

9 files changed

Lines changed: 695 additions & 449 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
"""Persistence for container sessions and their events.
2+
3+
Container sessions belong to this module, so every query against them lives
4+
here — including the aggregates other modules need. That keeps the identity
5+
dashboard from writing SQL over tables it does not own.
6+
"""
7+
8+
import uuid
9+
from dataclasses import dataclass
10+
from datetime import date, datetime
11+
12+
from sqlalchemy import case, cast, func, select
13+
from sqlalchemy.ext.asyncio import AsyncSession
14+
from sqlalchemy.types import Date
15+
16+
from helprs.modules.container.models import ContainerSession, ContainerStatus
17+
18+
19+
@dataclass(frozen=True)
20+
class StatusCounts:
21+
"""Session totals per terminal status, for one set of installations."""
22+
23+
completed: int
24+
failed: int
25+
timeout: int
26+
total: int
27+
28+
@classmethod
29+
def empty(cls) -> "StatusCounts":
30+
return cls(completed=0, failed=0, timeout=0, total=0)
31+
32+
33+
@dataclass(frozen=True)
34+
class DailyCount:
35+
"""How many sessions ran on a given day."""
36+
37+
day: date
38+
count: int
39+
40+
41+
async def count_by_status(session: AsyncSession, installation_ids: list[uuid.UUID]) -> StatusCounts:
42+
"""Count sessions per terminal status across the given installations."""
43+
if not installation_ids:
44+
return StatusCounts.empty()
45+
46+
result = await session.execute(
47+
select(
48+
func.count(ContainerSession.id).label("total"),
49+
func.count(case((ContainerSession.status == ContainerStatus.COMPLETED, 1))).label("completed"),
50+
func.count(case((ContainerSession.status == ContainerStatus.FAILED, 1))).label("failed"),
51+
func.count(case((ContainerSession.status == ContainerStatus.TIMEOUT, 1))).label("timeout"),
52+
).where(ContainerSession.installation_id.in_(installation_ids))
53+
)
54+
row = result.one()
55+
return StatusCounts(
56+
completed=row.completed,
57+
failed=row.failed,
58+
timeout=row.timeout,
59+
total=row.total,
60+
)
61+
62+
63+
async def count_per_day(
64+
session: AsyncSession,
65+
installation_ids: list[uuid.UUID],
66+
*,
67+
since: datetime,
68+
) -> list[DailyCount]:
69+
"""Count sessions per calendar day since ``since``, oldest first."""
70+
if not installation_ids:
71+
return []
72+
73+
result = await session.execute(
74+
select(
75+
cast(ContainerSession.created_at, Date).label("day"),
76+
func.count(ContainerSession.id).label("count"),
77+
)
78+
.where(
79+
ContainerSession.installation_id.in_(installation_ids),
80+
ContainerSession.created_at >= since,
81+
)
82+
.group_by("day")
83+
.order_by("day")
84+
)
85+
return [DailyCount(day=row.day, count=row.count) for row in result.all()]
86+
87+
88+
async def count_grouped_by_installation(
89+
session: AsyncSession,
90+
installation_ids: list[uuid.UUID],
91+
) -> dict[uuid.UUID, int]:
92+
"""Session count per installation — one GROUP BY, never a query per row."""
93+
if not installation_ids:
94+
return {}
95+
96+
result = await session.execute(
97+
select(ContainerSession.installation_id, func.count(ContainerSession.id))
98+
.where(ContainerSession.installation_id.in_(installation_ids))
99+
.group_by(ContainerSession.installation_id)
100+
)
101+
return {installation_id: count for installation_id, count in result.all()}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
"""Typed boundary to GitHub's OAuth and user APIs.
2+
3+
Every GitHub response is parsed into a model here, so a shape change surfaces
4+
as a validation error at the edge instead of a ``KeyError`` deep in a service.
5+
Nothing downstream of this module handles raw JSON.
6+
"""
7+
8+
import httpx
9+
from pydantic import BaseModel, Field, ValidationError
10+
11+
from helprs.core.config import Settings
12+
from helprs.core.exceptions import ExternalServiceError, UnauthorizedError
13+
14+
GITHUB_TOKEN_URL = "https://github.com/login/oauth/access_token"
15+
GITHUB_USER_URL = "https://api.github.com/user"
16+
17+
_TIMEOUT_SECONDS = 10.0
18+
19+
20+
class GitHubOAuthToken(BaseModel):
21+
"""The credential GitHub hands back for an authorization code."""
22+
23+
access_token: str
24+
token_type: str = "bearer"
25+
scope: str = ""
26+
27+
28+
class GitHubUserProfile(BaseModel):
29+
"""The subset of GitHub's user payload the application stores."""
30+
31+
github_id: int = Field(alias="id")
32+
login: str
33+
email: str | None = None
34+
avatar_url: str | None = None
35+
36+
37+
def _parse[ModelT: BaseModel](model: type[ModelT], payload: object, what: str) -> ModelT:
38+
try:
39+
return model.model_validate(payload)
40+
except ValidationError as e:
41+
raise ExternalServiceError(f"Unexpected {what} payload from GitHub") from e
42+
43+
44+
async def exchange_code_for_token(code: str, settings: Settings) -> GitHubOAuthToken:
45+
"""Exchange an OAuth authorization code for a user access token."""
46+
try:
47+
async with httpx.AsyncClient(timeout=_TIMEOUT_SECONDS) as client:
48+
resp = await client.post(
49+
GITHUB_TOKEN_URL,
50+
data={
51+
"client_id": settings.GITHUB_CLIENT_ID,
52+
"client_secret": settings.GITHUB_CLIENT_SECRET,
53+
"code": code,
54+
},
55+
headers={"Accept": "application/json"},
56+
)
57+
resp.raise_for_status()
58+
except httpx.TimeoutException as e:
59+
raise ExternalServiceError("GitHub is temporarily unavailable") from e
60+
except httpx.HTTPStatusError as e:
61+
raise ExternalServiceError("GitHub token exchange failed") from e
62+
63+
payload = resp.json()
64+
# GitHub answers 200 with an ``error`` key for a bad or expired code.
65+
if isinstance(payload, dict) and "error" in payload:
66+
raise UnauthorizedError(f"GitHub OAuth error: {payload['error']}")
67+
68+
return _parse(GitHubOAuthToken, payload, "OAuth token")
69+
70+
71+
async def fetch_user_profile(access_token: str) -> GitHubUserProfile:
72+
"""Fetch the profile of the user owning ``access_token``."""
73+
try:
74+
async with httpx.AsyncClient(timeout=_TIMEOUT_SECONDS) as client:
75+
resp = await client.get(
76+
GITHUB_USER_URL,
77+
headers={
78+
"Authorization": f"Bearer {access_token}",
79+
"Accept": "application/json",
80+
},
81+
)
82+
resp.raise_for_status()
83+
except httpx.TimeoutException as e:
84+
raise ExternalServiceError("GitHub is temporarily unavailable") from e
85+
except httpx.HTTPStatusError as e:
86+
if e.response.status_code == 401:
87+
raise UnauthorizedError("GitHub token is invalid or revoked") from e
88+
raise ExternalServiceError("GitHub API error") from e
89+
90+
return _parse(GitHubUserProfile, resp.json(), "user")
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
"""Persistence for GitHub user identities.
2+
3+
Every query against ``github_users`` lives here; services above never build
4+
SQL and never see an ``AsyncSession`` method other than through this module.
5+
"""
6+
7+
import uuid
8+
9+
from sqlalchemy import select
10+
from sqlalchemy.ext.asyncio import AsyncSession
11+
12+
from helprs.modules.identity.models import GitHubUser
13+
14+
15+
async def get_by_github_id(session: AsyncSession, github_id: int) -> GitHubUser | None:
16+
result = await session.execute(select(GitHubUser).where(GitHubUser.github_id == github_id))
17+
return result.scalar_one_or_none()
18+
19+
20+
async def get_by_id(session: AsyncSession, user_id: uuid.UUID) -> GitHubUser | None:
21+
result = await session.execute(select(GitHubUser).where(GitHubUser.id == user_id))
22+
return result.scalar_one_or_none()
23+
24+
25+
async def add(session: AsyncSession, user: GitHubUser) -> GitHubUser:
26+
"""Stage a new user and flush so its generated id is available."""
27+
session.add(user)
28+
await session.flush()
29+
return user

0 commit comments

Comments
 (0)