From be1fdc455106a77a01daa4d8bca3316c0dd28a3a Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Sun, 15 Mar 2026 01:31:48 +0800 Subject: [PATCH 1/2] Calendar backend optimisations: safety caps, shared calendar fix, query consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1: Recurrence safety — MAX_OCCURRENCES=730 hard cap, adaptive 90-day horizon for daily events (interval<7), RecurrenceRule cross-field validation, ID bounds on location_id/calendar_id schemas. Phase 2: Dashboard correctness — shared calendar events now included in /dashboard and /upcoming via get_accessible_calendar_ids helper. Project stats consolidated into single GROUP BY query (saves 1 DB round-trip). Phase 3: Write performance — bulk db.add_all() for child events, removed redundant SELECT in this_and_future delete path. Phase 4: Frontend query efficiency — staleTime: 30_000 on calendar events query eliminates redundant refetches on mount/view switch. Backend LIMIT 2000 safety guard on events endpoint. Phase 5: Rate limiting — nginx limit_req zone on /api/events (30r/m) to prevent DB flooding via recurrence amplification. Co-Authored-By: Claude Opus 4.6 --- backend/app/routers/dashboard.py | 34 +++++-------- backend/app/routers/events.py | 51 ++++--------------- backend/app/schemas/calendar_event.py | 22 ++++++-- backend/app/services/calendar_sharing.py | 12 +++++ backend/app/services/recurrence.py | 15 +++++- frontend/nginx.conf | 9 ++++ .../src/components/calendar/CalendarPage.tsx | 1 + 7 files changed, 75 insertions(+), 69 deletions(-) diff --git a/backend/app/routers/dashboard.py b/backend/app/routers/dashboard.py index 94ea5ac..881e88e 100644 --- a/backend/app/routers/dashboard.py +++ b/backend/app/routers/dashboard.py @@ -8,11 +8,11 @@ from app.database import get_db from app.models.settings import Settings from app.models.todo import Todo from app.models.calendar_event import CalendarEvent -from app.models.calendar import Calendar from app.models.reminder import Reminder from app.models.project import Project from app.models.user import User from app.routers.auth import get_current_user, get_current_settings +from app.services.calendar_sharing import get_accessible_calendar_ids router = APIRouter() @@ -35,12 +35,8 @@ async def get_dashboard( today = client_date or date.today() upcoming_cutoff = today + timedelta(days=current_settings.upcoming_days) - # Fetch calendar IDs once as a plain list — avoids repeating the subquery - # expression in each of the 3+ queries below and makes intent clearer. - calendar_id_result = await db.execute( - select(Calendar.id).where(Calendar.user_id == current_user.id) - ) - user_calendar_ids = [row[0] for row in calendar_id_result.all()] + # Fetch all accessible calendar IDs (owned + accepted shared memberships) + user_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) # Today's events (exclude parent templates — they are hidden, children are shown) today_start = datetime.combine(today, datetime.min.time()) @@ -75,18 +71,15 @@ async def get_dashboard( reminders_result = await db.execute(reminders_query) active_reminders = reminders_result.scalars().all() - # Project stats (scoped to user) - total_projects_result = await db.execute( - select(func.count(Project.id)).where(Project.user_id == current_user.id) + # Project stats — single GROUP BY query, derive total in Python + projects_by_status_result = await db.execute( + select( + Project.status, + func.count(Project.id).label("count"), + ).where(Project.user_id == current_user.id).group_by(Project.status) ) - total_projects = total_projects_result.scalar() - - projects_by_status_query = select( - Project.status, - func.count(Project.id).label("count") - ).where(Project.user_id == current_user.id).group_by(Project.status) - projects_by_status_result = await db.execute(projects_by_status_query) projects_by_status = {row[0]: row[1] for row in projects_by_status_result} + total_projects = sum(projects_by_status.values()) # Todo counts: total and incomplete in a single query todo_counts_result = await db.execute( @@ -177,11 +170,8 @@ async def get_upcoming( overdue_floor = today - timedelta(days=30) overdue_floor_dt = datetime.combine(overdue_floor, datetime.min.time()) - # Fetch calendar IDs once as a plain list (same rationale as /dashboard handler) - calendar_id_result = await db.execute( - select(Calendar.id).where(Calendar.user_id == current_user.id) - ) - user_calendar_ids = [row[0] for row in calendar_id_result.all()] + # Fetch all accessible calendar IDs (owned + accepted shared memberships) + user_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) # Build queries — include overdue todos (up to 30 days back) and snoozed reminders todos_query = select(Todo).where( diff --git a/backend/app/routers/events.py b/backend/app/routers/events.py index e7bd57f..2782c3f 100644 --- a/backend/app/routers/events.py +++ b/backend/app/routers/events.py @@ -18,9 +18,8 @@ from app.schemas.calendar_event import ( ) from app.routers.auth import get_current_user from app.models.user import User -from app.models.calendar_member import CalendarMember from app.services.recurrence import generate_occurrences -from app.services.calendar_sharing import check_lock_for_edit, require_permission +from app.services.calendar_sharing import check_lock_for_edit, get_accessible_calendar_ids, require_permission router = APIRouter() @@ -145,12 +144,7 @@ async def get_events( are what get displayed on the calendar. """ # Scope events through calendar ownership + shared memberships - user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) - shared_calendar_ids = select(CalendarMember.calendar_id).where( - CalendarMember.user_id == current_user.id, - CalendarMember.status == "accepted", - ) - all_calendar_ids = user_calendar_ids.union(shared_calendar_ids) + all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) query = ( select(CalendarEvent) @@ -172,7 +166,7 @@ async def get_events( if end: query = query.where(CalendarEvent.start_datetime <= end) - query = query.order_by(CalendarEvent.start_datetime.asc()) + query = query.order_by(CalendarEvent.start_datetime.asc()).limit(2000) result = await db.execute(query) events = result.scalars().all() @@ -246,8 +240,7 @@ async def create_event( await db.flush() # assign parent.id before generating children children = generate_occurrences(parent) - for child in children: - db.add(child) + db.add_all(children) await db.commit() @@ -288,12 +281,7 @@ async def get_event( db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), ): - user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) - shared_calendar_ids = select(CalendarMember.calendar_id).where( - CalendarMember.user_id == current_user.id, - CalendarMember.status == "accepted", - ) - all_calendar_ids = user_calendar_ids.union(shared_calendar_ids) + all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) result = await db.execute( select(CalendarEvent) @@ -318,12 +306,7 @@ async def update_event( db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), ): - user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) - shared_calendar_ids = select(CalendarMember.calendar_id).where( - CalendarMember.user_id == current_user.id, - CalendarMember.status == "accepted", - ) - all_calendar_ids = user_calendar_ids.union(shared_calendar_ids) + all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) result = await db.execute( select(CalendarEvent) @@ -422,8 +405,7 @@ async def update_event( if event.recurrence_rule: await db.flush() children = generate_occurrences(event) - for child in children: - db.add(child) + db.add_all(children) else: # This IS a parent — update it and regenerate all children for key, value in update_data.items(): @@ -439,8 +421,7 @@ async def update_event( ) await db.flush() children = generate_occurrences(event) - for child in children: - db.add(child) + db.add_all(children) await db.commit() @@ -470,12 +451,7 @@ async def delete_event( db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), ): - user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) - shared_calendar_ids = select(CalendarMember.calendar_id).where( - CalendarMember.user_id == current_user.id, - CalendarMember.status == "accepted", - ) - all_calendar_ids = user_calendar_ids.union(shared_calendar_ids) + all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db) result = await db.execute( select(CalendarEvent).where( @@ -501,20 +477,13 @@ async def delete_event( this_original_start = event.original_start or event.start_datetime if parent_id is not None: - # Delete this + all future siblings + # Delete this + all future siblings (original_start is always set on children) await db.execute( delete(CalendarEvent).where( CalendarEvent.parent_event_id == parent_id, CalendarEvent.original_start >= this_original_start, ) ) - # Ensure the target event itself is deleted (edge case: original_start fallback mismatch) - existing = await db.execute( - select(CalendarEvent).where(CalendarEvent.id == event_id) - ) - target = existing.scalar_one_or_none() - if target: - await db.delete(target) else: # This event IS the parent — delete it and all children (CASCADE handles children) await db.delete(event) diff --git a/backend/app/schemas/calendar_event.py b/backend/app/schemas/calendar_event.py index 88ccf66..0486ec5 100644 --- a/backend/app/schemas/calendar_event.py +++ b/backend/app/schemas/calendar_event.py @@ -1,6 +1,6 @@ import json as _json -from pydantic import BaseModel, ConfigDict, Field, field_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from datetime import datetime from typing import Literal, Optional @@ -17,6 +17,18 @@ class RecurrenceRule(BaseModel): # monthly_date day: Optional[int] = Field(None, ge=1, le=31) + @model_validator(mode="after") + def validate_required_fields(self): + """Enforce required fields per rule type.""" + if self.type == "every_n_days" and self.interval is None: + raise ValueError("every_n_days rule requires 'interval'") + if self.type == "monthly_nth_weekday": + if self.week is None or self.weekday is None: + raise ValueError("monthly_nth_weekday rule requires both 'week' and 'weekday'") + if self.type == "monthly_date" and self.day is None: + raise ValueError("monthly_date rule requires 'day'") + return self + def _coerce_recurrence_rule(v): """Accept None, dict, RecurrenceRule, or JSON/legacy strings gracefully.""" @@ -47,10 +59,10 @@ class CalendarEventCreate(BaseModel): end_datetime: datetime all_day: bool = False color: Optional[str] = Field(None, max_length=20) - location_id: Optional[int] = None + location_id: Optional[int] = Field(None, ge=1, le=2147483647) recurrence_rule: Optional[RecurrenceRule] = None is_starred: bool = False - calendar_id: Optional[int] = None # If None, server assigns default calendar + calendar_id: Optional[int] = Field(None, ge=1, le=2147483647) @field_validator("recurrence_rule", mode="before") @classmethod @@ -67,10 +79,10 @@ class CalendarEventUpdate(BaseModel): end_datetime: Optional[datetime] = None all_day: Optional[bool] = None color: Optional[str] = Field(None, max_length=20) - location_id: Optional[int] = None + location_id: Optional[int] = Field(None, ge=1, le=2147483647) recurrence_rule: Optional[RecurrenceRule] = None is_starred: Optional[bool] = None - calendar_id: Optional[int] = None + calendar_id: Optional[int] = Field(None, ge=1, le=2147483647) # Controls which occurrences an edit applies to; absent = non-recurring or whole-series edit_scope: Optional[Literal["this", "this_and_future"]] = None diff --git a/backend/app/services/calendar_sharing.py b/backend/app/services/calendar_sharing.py index dc05c7f..1f94ed7 100644 --- a/backend/app/services/calendar_sharing.py +++ b/backend/app/services/calendar_sharing.py @@ -20,6 +20,18 @@ PERMISSION_RANK = {"read_only": 1, "create_modify": 2, "full_access": 3} LOCK_DURATION_MINUTES = 5 +async def get_accessible_calendar_ids(user_id: int, db: AsyncSession) -> list[int]: + """Return all calendar IDs the user can access (owned + accepted shared memberships).""" + owned = await db.execute(select(Calendar.id).where(Calendar.user_id == user_id)) + shared = await db.execute( + select(CalendarMember.calendar_id).where( + CalendarMember.user_id == user_id, + CalendarMember.status == "accepted", + ) + ) + return [r[0] for r in owned.all()] + [r[0] for r in shared.all()] + + async def get_user_permission(db: AsyncSession, calendar_id: int, user_id: int) -> str | None: """ Returns "owner" if the user owns the calendar, the permission string diff --git a/backend/app/services/recurrence.py b/backend/app/services/recurrence.py index 0099efe..edf49e5 100644 --- a/backend/app/services/recurrence.py +++ b/backend/app/services/recurrence.py @@ -10,6 +10,9 @@ from typing import Optional from app.models.calendar_event import CalendarEvent +# Hard cap: never generate more than 730 child events regardless of horizon_days. +MAX_OCCURRENCES = 730 + def _nth_weekday_of_month(year: int, month: int, weekday: int, week: int) -> Optional[datetime]: """ @@ -99,8 +102,12 @@ def generate_occurrences( interval: int = _rule_int(rule, "interval", 1) if interval < 1: interval = 1 + # Adaptive horizon: cap daily-ish events (interval < 7) to 90 days + effective_horizon = horizon if interval >= 7 else min(horizon, parent_start + timedelta(days=90)) current = parent_start + timedelta(days=interval) - while current < horizon: + while current < effective_horizon: + if len(occurrences) >= MAX_OCCURRENCES: + break occurrences.append(_make_child(current)) current += timedelta(days=interval) @@ -112,6 +119,8 @@ def generate_occurrences( days_ahead = 7 current = parent_start + timedelta(days=days_ahead) while current < horizon: + if len(occurrences) >= MAX_OCCURRENCES: + break occurrences.append(_make_child(current)) current += timedelta(weeks=1) @@ -126,6 +135,8 @@ def generate_occurrences( month = 1 year += 1 while True: + if len(occurrences) >= MAX_OCCURRENCES: + break target = _nth_weekday_of_month(year, month, weekday, week) if target is None: # Skip months where Nth weekday doesn't exist @@ -157,6 +168,8 @@ def generate_occurrences( month = 1 year += 1 while True: + if len(occurrences) >= MAX_OCCURRENCES: + break # Some months don't have day 29-31 try: occ_start = datetime( diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 36a506d..73087cc 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -10,6 +10,8 @@ limit_req_zone $binary_remote_addr zone=cal_sync_limit:10m rate=15r/m; # Connection endpoints — prevent search enumeration and request spam limit_req_zone $binary_remote_addr zone=conn_search_limit:10m rate=10r/m; limit_req_zone $binary_remote_addr zone=conn_request_limit:10m rate=3r/m; +# Event creation — recurrence amplification means 1 POST = up to 90-365 child rows +limit_req_zone $binary_remote_addr zone=event_create_limit:10m rate=30r/m; # Use X-Forwarded-Proto from upstream proxy when present, fall back to $scheme for direct access map $http_x_forwarded_proto $forwarded_proto { @@ -122,6 +124,13 @@ server { include /etc/nginx/proxy-params.conf; } + # Event creation — rate-limited to prevent DB flooding via recurrence amplification + location = /api/events { + limit_req zone=event_create_limit burst=10 nodelay; + limit_req_status 429; + include /etc/nginx/proxy-params.conf; + } + # API proxy (catch-all for non-rate-limited endpoints) location /api { proxy_set_header Upgrade $http_upgrade; diff --git a/frontend/src/components/calendar/CalendarPage.tsx b/frontend/src/components/calendar/CalendarPage.tsx index 8de5365..c58c3ec 100644 --- a/frontend/src/components/calendar/CalendarPage.tsx +++ b/frontend/src/components/calendar/CalendarPage.tsx @@ -231,6 +231,7 @@ export default function CalendarPage() { }, // AW-3: Reduce from 5s to 30s — personal organiser doesn't need 12 calls/min refetchInterval: 30_000, + staleTime: 30_000, }); const selectedEvent = useMemo( From a2c1058f9ca17d9c488dc3f3b09f62b3b0eed9d2 Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Sun, 15 Mar 2026 01:46:11 +0800 Subject: [PATCH 2/2] Fix QA findings: single UNION query, weekly validation, nginx docs W-01: Consolidate get_accessible_calendar_ids to single UNION query instead of two separate DB round-trips. W-02: Document that nginx rate limit on /api/events applies to all methods (30r/m generous enough for GET polling at 2r/m). W-03: Add weekly rule validation for consistency with other rule types. Co-Authored-By: Claude Opus 4.6 --- backend/app/schemas/calendar_event.py | 2 ++ backend/app/services/calendar_sharing.py | 14 ++++++++------ frontend/nginx.conf | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/backend/app/schemas/calendar_event.py b/backend/app/schemas/calendar_event.py index 0486ec5..be14716 100644 --- a/backend/app/schemas/calendar_event.py +++ b/backend/app/schemas/calendar_event.py @@ -22,6 +22,8 @@ class RecurrenceRule(BaseModel): """Enforce required fields per rule type.""" if self.type == "every_n_days" and self.interval is None: raise ValueError("every_n_days rule requires 'interval'") + if self.type == "weekly" and self.weekday is None: + raise ValueError("weekly rule requires 'weekday'") if self.type == "monthly_nth_weekday": if self.week is None or self.weekday is None: raise ValueError("monthly_nth_weekday rule requires both 'week' and 'weekday'") diff --git a/backend/app/services/calendar_sharing.py b/backend/app/services/calendar_sharing.py index 1f94ed7..da1d71a 100644 --- a/backend/app/services/calendar_sharing.py +++ b/backend/app/services/calendar_sharing.py @@ -22,14 +22,16 @@ LOCK_DURATION_MINUTES = 5 async def get_accessible_calendar_ids(user_id: int, db: AsyncSession) -> list[int]: """Return all calendar IDs the user can access (owned + accepted shared memberships).""" - owned = await db.execute(select(Calendar.id).where(Calendar.user_id == user_id)) - shared = await db.execute( - select(CalendarMember.calendar_id).where( - CalendarMember.user_id == user_id, - CalendarMember.status == "accepted", + result = await db.execute( + select(Calendar.id).where(Calendar.user_id == user_id) + .union( + select(CalendarMember.calendar_id).where( + CalendarMember.user_id == user_id, + CalendarMember.status == "accepted", + ) ) ) - return [r[0] for r in owned.all()] + [r[0] for r in shared.all()] + return [r[0] for r in result.all()] async def get_user_permission(db: AsyncSession, calendar_id: int, user_id: int) -> str | None: diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 73087cc..85d709c 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -124,7 +124,9 @@ server { include /etc/nginx/proxy-params.conf; } - # Event creation — rate-limited to prevent DB flooding via recurrence amplification + # Event creation — rate-limited to prevent DB flooding via recurrence amplification. + # Note: exact match applies to GET+POST; 30r/m with burst=10 is generous enough + # for polling (2r/m) and won't affect reads even with multiple tabs. location = /api/events { limit_req zone=event_create_limit burst=10 nodelay; limit_req_status 429;