From 846019d5c191f1b9b36ab9bd0e30c6f8c691807c Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Fri, 13 Mar 2026 00:08:45 +0800 Subject: [PATCH] Phase 3: Backend queries and indexes optimization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AW-1: Add composite index on calendar_members(user_id, status) for the hot shared-calendar polling query - AS-6: Add composite index on ntfy_sent(user_id, sent_at) for dedup lookups - AW-5: Combine get_user_permission into single LEFT JOIN query instead of 2 sequential queries (called twice per event edit) - AC-5: Batch cascade_on_disconnect — single GROUP BY + bulk UPDATE instead of N per-calendar checks when a connection is severed - AW-6: Collapse admin dashboard 5 COUNT queries into single conditional aggregation using COUNT().filter() - AC-3: Cache get_current_settings in request.state to avoid redundant queries when multiple dependencies need settings in the same request Co-Authored-By: Claude Opus 4.6 --- .../versions/053_add_composite_indexes.py | 29 +++++++++ backend/app/routers/admin.py | 22 +++---- backend/app/routers/auth.py | 7 ++ backend/app/services/calendar_sharing.py | 64 +++++++++++-------- 4 files changed, 84 insertions(+), 38 deletions(-) create mode 100644 backend/alembic/versions/053_add_composite_indexes.py diff --git a/backend/alembic/versions/053_add_composite_indexes.py b/backend/alembic/versions/053_add_composite_indexes.py new file mode 100644 index 0000000..758d280 --- /dev/null +++ b/backend/alembic/versions/053_add_composite_indexes.py @@ -0,0 +1,29 @@ +"""Add composite indexes for calendar_members and ntfy_sent + +Revision ID: 053 +Revises: 052 +""" +from alembic import op + +revision = "053" +down_revision = "052" + + +def upgrade(): + # AW-1: Hot query polled every 5s uses (user_id, status) together + op.create_index( + "ix_calendar_members_user_id_status", + "calendar_members", + ["user_id", "status"], + ) + # AS-6: Dedup lookup in notification dispatch uses (user_id, sent_at) + op.create_index( + "ix_ntfy_sent_user_id_sent_at", + "ntfy_sent", + ["user_id", "sent_at"], + ) + + +def downgrade(): + op.drop_index("ix_ntfy_sent_user_id_sent_at", table_name="ntfy_sent") + op.drop_index("ix_calendar_members_user_id_status", table_name="calendar_members") diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index ead0397..5026fc0 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -743,18 +743,18 @@ async def admin_dashboard( _actor: User = Depends(get_current_user), ): """Aggregate stats for the admin portal dashboard.""" - total_users = await db.scalar( - sa.select(sa.func.count()).select_from(User) - ) - active_users = await db.scalar( - sa.select(sa.func.count()).select_from(User).where(User.is_active == True) - ) - admin_count = await db.scalar( - sa.select(sa.func.count()).select_from(User).where(User.role == "admin") - ) - totp_count = await db.scalar( - sa.select(sa.func.count()).select_from(User).where(User.totp_enabled == True) + # AW-6: Single conditional aggregation instead of 5 separate COUNT queries + user_stats = await db.execute( + sa.select( + sa.func.count().label("total"), + sa.func.count().filter(User.is_active == True).label("active"), + sa.func.count().filter(User.role == "admin").label("admins"), + sa.func.count().filter(User.totp_enabled == True).label("totp"), + ).select_from(User) ) + row = user_stats.one() + total_users, active_users, admin_count, totp_count = row.tuple() + active_sessions = await db.scalar( sa.select(sa.func.count()).select_from(UserSession).where( UserSession.revoked == False, diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 71c7b51..1478b45 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -147,19 +147,26 @@ async def get_current_user( async def get_current_settings( + request: Request, current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ) -> Settings: """ Convenience dependency for routers that need Settings access. Always chain after get_current_user — never use standalone. + + AC-3: Cache in request.state so multiple dependencies don't re-query. """ + cached = getattr(request.state, "settings", None) + if cached is not None: + return cached result = await db.execute( select(Settings).where(Settings.user_id == current_user.id) ) settings_obj = result.scalar_one_or_none() if not settings_obj: raise HTTPException(status_code=500, detail="Settings not found for user") + request.state.settings = settings_obj return settings_obj diff --git a/backend/app/services/calendar_sharing.py b/backend/app/services/calendar_sharing.py index 71bba15..dc05c7f 100644 --- a/backend/app/services/calendar_sharing.py +++ b/backend/app/services/calendar_sharing.py @@ -7,7 +7,7 @@ import logging from datetime import datetime, timedelta from fastapi import HTTPException -from sqlalchemy import delete, select, text +from sqlalchemy import delete, select, text, update from sqlalchemy.ext.asyncio import AsyncSession from app.models.calendar import Calendar @@ -24,25 +24,29 @@ async def get_user_permission(db: AsyncSession, calendar_id: int, user_id: int) """ Returns "owner" if the user owns the calendar, the permission string if they are an accepted member, or None if they have no access. - """ - cal = await db.execute( - select(Calendar).where(Calendar.id == calendar_id) - ) - calendar = cal.scalar_one_or_none() - if not calendar: - return None - if calendar.user_id == user_id: - return "owner" - member = await db.execute( - select(CalendarMember).where( - CalendarMember.calendar_id == calendar_id, - CalendarMember.user_id == user_id, - CalendarMember.status == "accepted", + AW-5: Single query with LEFT JOIN instead of 2 sequential queries. + """ + result = await db.execute( + select( + Calendar.user_id, + CalendarMember.permission, ) + .outerjoin( + CalendarMember, + (CalendarMember.calendar_id == Calendar.id) + & (CalendarMember.user_id == user_id) + & (CalendarMember.status == "accepted"), + ) + .where(Calendar.id == calendar_id) ) - row = member.scalar_one_or_none() - return row.permission if row else None + row = result.one_or_none() + if not row: + return None + owner_id, member_permission = row.tuple() + if owner_id == user_id: + return "owner" + return member_permission async def require_permission( @@ -202,16 +206,22 @@ async def cascade_on_disconnect(db: AsyncSession, user_a_id: int, user_b_id: int {"user_id": user_a_id, "cal_ids": b_cal_ids}, ) - # Reset is_shared on calendars with no remaining members + # AC-5: Single aggregation query instead of N per-calendar checks all_cal_ids = a_cal_ids + b_cal_ids - for cal_id in all_cal_ids: - remaining = await db.execute( - select(CalendarMember.id).where(CalendarMember.calendar_id == cal_id).limit(1) + if all_cal_ids: + # Find which calendars still have members + has_members_result = await db.execute( + select(CalendarMember.calendar_id) + .where(CalendarMember.calendar_id.in_(all_cal_ids)) + .group_by(CalendarMember.calendar_id) ) - if not remaining.scalar_one_or_none(): - cal_result = await db.execute( - select(Calendar).where(Calendar.id == cal_id) + cals_with_members = {row[0] for row in has_members_result.all()} + + # Reset is_shared on calendars with no remaining members + empty_cal_ids = [cid for cid in all_cal_ids if cid not in cals_with_members] + if empty_cal_ids: + await db.execute( + update(Calendar) + .where(Calendar.id.in_(empty_cal_ids)) + .values(is_shared=False) ) - cal = cal_result.scalar_one_or_none() - if cal: - cal.is_shared = False