Fix QA review #2: W-03/W-04, S-01 through S-04
W-03: Unify split transactions — _create_db_session() now uses flush()
instead of commit(), callers own the final commit.
W-04: Time-bound dedup key fetch to 7-day purge window.
S-01: Type admin dashboard response with RecentLoginItem/RecentAuditItem.
S-02: Convert starred events index to partial index WHERE is_starred = true.
S-03: EventTemplate.created_at default changed to func.now() for consistency.
S-04: Add single-worker scaling note to weather cache.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
72e00f3a69
commit
619e220622
@ -27,11 +27,13 @@ def upgrade() -> None:
|
|||||||
["calendar_id", "start_datetime", "end_datetime"],
|
["calendar_id", "start_datetime", "end_datetime"],
|
||||||
)
|
)
|
||||||
|
|
||||||
# Partial index for starred events dashboard query
|
# Partial index for starred events dashboard query — only rows where
|
||||||
|
# is_starred = true are ever queried, so a partial index is smaller and faster.
|
||||||
op.create_index(
|
op.create_index(
|
||||||
"ix_calendar_events_calendar_starred",
|
"ix_calendar_events_calendar_starred",
|
||||||
"calendar_events",
|
"calendar_events",
|
||||||
["calendar_id", "is_starred"],
|
["calendar_id", "start_datetime"],
|
||||||
|
postgresql_where="is_starred = true",
|
||||||
)
|
)
|
||||||
|
|
||||||
# FK lookup index for recurring children DELETE
|
# FK lookup index for recurring children DELETE
|
||||||
|
|||||||
@ -41,9 +41,13 @@ UMBRA_URL = "http://10.0.69.35"
|
|||||||
# ── Dedup helpers ─────────────────────────────────────────────────────────────
|
# ── Dedup helpers ─────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
async def _get_sent_keys(db: AsyncSession, user_id: int) -> set[str]:
|
async def _get_sent_keys(db: AsyncSession, user_id: int) -> set[str]:
|
||||||
"""Batch-fetch all notification keys for a user in one query."""
|
"""Batch-fetch recent notification keys for a user (within the 7-day purge window)."""
|
||||||
|
cutoff = datetime.now() - timedelta(days=7)
|
||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
select(NtfySent.notification_key).where(NtfySent.user_id == user_id)
|
select(NtfySent.notification_key).where(
|
||||||
|
NtfySent.user_id == user_id,
|
||||||
|
NtfySent.sent_at >= cutoff,
|
||||||
|
)
|
||||||
)
|
)
|
||||||
return set(result.scalars().all())
|
return set(result.scalars().all())
|
||||||
|
|
||||||
|
|||||||
@ -24,4 +24,4 @@ class EventTemplate(Base):
|
|||||||
Integer, ForeignKey("locations.id", ondelete="SET NULL"), nullable=True
|
Integer, ForeignKey("locations.id", ondelete="SET NULL"), nullable=True
|
||||||
)
|
)
|
||||||
is_starred: Mapped[bool] = mapped_column(Boolean, default=False)
|
is_starred: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||||
created_at: Mapped[datetime] = mapped_column(default=datetime.now, server_default=func.now())
|
created_at: Mapped[datetime] = mapped_column(default=func.now(), server_default=func.now())
|
||||||
|
|||||||
@ -195,7 +195,7 @@ async def _create_db_session(
|
|||||||
user_agent=(user_agent or "")[:255] if user_agent else None,
|
user_agent=(user_agent or "")[:255] if user_agent else None,
|
||||||
)
|
)
|
||||||
db.add(db_session)
|
db.add(db_session)
|
||||||
await db.commit()
|
await db.flush()
|
||||||
token = create_session_token(user.id, session_id)
|
token = create_session_token(user.id, session_id)
|
||||||
return session_id, token
|
return session_id, token
|
||||||
|
|
||||||
@ -335,6 +335,7 @@ async def login(
|
|||||||
user_agent = request.headers.get("user-agent")
|
user_agent = request.headers.get("user-agent")
|
||||||
_, token = await _create_db_session(db, user, client_ip, user_agent)
|
_, token = await _create_db_session(db, user, client_ip, user_agent)
|
||||||
_set_session_cookie(response, token)
|
_set_session_cookie(response, token)
|
||||||
|
await db.commit()
|
||||||
return {
|
return {
|
||||||
"authenticated": True,
|
"authenticated": True,
|
||||||
"must_change_password": True,
|
"must_change_password": True,
|
||||||
@ -415,6 +416,7 @@ async def register(
|
|||||||
|
|
||||||
_, token = await _create_db_session(db, new_user, ip, user_agent)
|
_, token = await _create_db_session(db, new_user, ip, user_agent)
|
||||||
_set_session_cookie(response, token)
|
_set_session_cookie(response, token)
|
||||||
|
await db.commit()
|
||||||
|
|
||||||
return {"message": "Registration successful", "authenticated": True}
|
return {"message": "Registration successful", "authenticated": True}
|
||||||
|
|
||||||
|
|||||||
@ -21,6 +21,8 @@ router = APIRouter()
|
|||||||
# SEC-15: Bounded LRU cache keyed by (user_id, location) — max 100 entries.
|
# SEC-15: Bounded LRU cache keyed by (user_id, location) — max 100 entries.
|
||||||
# OrderedDict preserves insertion order; move_to_end on hit, popitem(last=False)
|
# OrderedDict preserves insertion order; move_to_end on hit, popitem(last=False)
|
||||||
# to evict the oldest when capacity is exceeded.
|
# to evict the oldest when capacity is exceeded.
|
||||||
|
# NOTE: This cache is process-local. With multiple workers each process would
|
||||||
|
# maintain its own copy, wasting API quota. Currently safe — single Uvicorn worker.
|
||||||
_CACHE_MAX = 100
|
_CACHE_MAX = 100
|
||||||
_cache: OrderedDict = OrderedDict()
|
_cache: OrderedDict = OrderedDict()
|
||||||
|
|
||||||
|
|||||||
@ -99,14 +99,30 @@ class SystemConfigUpdate(BaseModel):
|
|||||||
# Admin dashboard
|
# Admin dashboard
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class RecentLoginItem(BaseModel):
|
||||||
|
username: str
|
||||||
|
last_login_at: Optional[datetime] = None
|
||||||
|
|
||||||
|
model_config = ConfigDict(from_attributes=True)
|
||||||
|
|
||||||
|
|
||||||
|
class RecentAuditItem(BaseModel):
|
||||||
|
action: str
|
||||||
|
actor_username: Optional[str] = None
|
||||||
|
target_username: Optional[str] = None
|
||||||
|
created_at: datetime
|
||||||
|
|
||||||
|
model_config = ConfigDict(from_attributes=True)
|
||||||
|
|
||||||
|
|
||||||
class AdminDashboardResponse(BaseModel):
|
class AdminDashboardResponse(BaseModel):
|
||||||
total_users: int
|
total_users: int
|
||||||
active_users: int
|
active_users: int
|
||||||
admin_count: int
|
admin_count: int
|
||||||
active_sessions: int
|
active_sessions: int
|
||||||
mfa_adoption_rate: float
|
mfa_adoption_rate: float
|
||||||
recent_logins: list[dict]
|
recent_logins: list[RecentLoginItem]
|
||||||
recent_audit_entries: list[dict]
|
recent_audit_entries: list[RecentAuditItem]
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user