Compare commits

..

3 Commits

Author SHA1 Message Date
348fe8988b Merge feature/calendar-backend-optimisations into main 2026-03-15 01:46:33 +08:00
a2c1058f9c 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 <noreply@anthropic.com>
2026-03-15 01:46:11 +08:00
be1fdc4551 Calendar backend optimisations: safety caps, shared calendar fix, query consolidation
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 <noreply@anthropic.com>
2026-03-15 01:31:48 +08:00
7 changed files with 81 additions and 69 deletions

View File

@ -8,11 +8,11 @@ from app.database import get_db
from app.models.settings import Settings from app.models.settings import Settings
from app.models.todo import Todo from app.models.todo import Todo
from app.models.calendar_event import CalendarEvent from app.models.calendar_event import CalendarEvent
from app.models.calendar import Calendar
from app.models.reminder import Reminder from app.models.reminder import Reminder
from app.models.project import Project from app.models.project import Project
from app.models.user import User from app.models.user import User
from app.routers.auth import get_current_user, get_current_settings from app.routers.auth import get_current_user, get_current_settings
from app.services.calendar_sharing import get_accessible_calendar_ids
router = APIRouter() router = APIRouter()
@ -35,12 +35,8 @@ async def get_dashboard(
today = client_date or date.today() today = client_date or date.today()
upcoming_cutoff = today + timedelta(days=current_settings.upcoming_days) upcoming_cutoff = today + timedelta(days=current_settings.upcoming_days)
# Fetch calendar IDs once as a plain list — avoids repeating the subquery # Fetch all accessible calendar IDs (owned + accepted shared memberships)
# expression in each of the 3+ queries below and makes intent clearer. user_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
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()]
# Today's events (exclude parent templates — they are hidden, children are shown) # Today's events (exclude parent templates — they are hidden, children are shown)
today_start = datetime.combine(today, datetime.min.time()) today_start = datetime.combine(today, datetime.min.time())
@ -75,18 +71,15 @@ async def get_dashboard(
reminders_result = await db.execute(reminders_query) reminders_result = await db.execute(reminders_query)
active_reminders = reminders_result.scalars().all() active_reminders = reminders_result.scalars().all()
# Project stats (scoped to user) # Project stats — single GROUP BY query, derive total in Python
total_projects_result = await db.execute( projects_by_status_result = await db.execute(
select(func.count(Project.id)).where(Project.user_id == current_user.id) 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} 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: total and incomplete in a single query
todo_counts_result = await db.execute( todo_counts_result = await db.execute(
@ -177,11 +170,8 @@ async def get_upcoming(
overdue_floor = today - timedelta(days=30) overdue_floor = today - timedelta(days=30)
overdue_floor_dt = datetime.combine(overdue_floor, datetime.min.time()) overdue_floor_dt = datetime.combine(overdue_floor, datetime.min.time())
# Fetch calendar IDs once as a plain list (same rationale as /dashboard handler) # Fetch all accessible calendar IDs (owned + accepted shared memberships)
calendar_id_result = await db.execute( user_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
select(Calendar.id).where(Calendar.user_id == current_user.id)
)
user_calendar_ids = [row[0] for row in calendar_id_result.all()]
# Build queries — include overdue todos (up to 30 days back) and snoozed reminders # Build queries — include overdue todos (up to 30 days back) and snoozed reminders
todos_query = select(Todo).where( todos_query = select(Todo).where(

View File

@ -18,9 +18,8 @@ from app.schemas.calendar_event import (
) )
from app.routers.auth import get_current_user from app.routers.auth import get_current_user
from app.models.user import User from app.models.user import User
from app.models.calendar_member import CalendarMember
from app.services.recurrence import generate_occurrences 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() router = APIRouter()
@ -145,12 +144,7 @@ async def get_events(
are what get displayed on the calendar. are what get displayed on the calendar.
""" """
# Scope events through calendar ownership + shared memberships # Scope events through calendar ownership + shared memberships
user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
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)
query = ( query = (
select(CalendarEvent) select(CalendarEvent)
@ -172,7 +166,7 @@ async def get_events(
if end: if end:
query = query.where(CalendarEvent.start_datetime <= 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) result = await db.execute(query)
events = result.scalars().all() events = result.scalars().all()
@ -246,8 +240,7 @@ async def create_event(
await db.flush() # assign parent.id before generating children await db.flush() # assign parent.id before generating children
children = generate_occurrences(parent) children = generate_occurrences(parent)
for child in children: db.add_all(children)
db.add(child)
await db.commit() await db.commit()
@ -288,12 +281,7 @@ async def get_event(
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
current_user: User = Depends(get_current_user), current_user: User = Depends(get_current_user),
): ):
user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
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)
result = await db.execute( result = await db.execute(
select(CalendarEvent) select(CalendarEvent)
@ -318,12 +306,7 @@ async def update_event(
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
current_user: User = Depends(get_current_user), current_user: User = Depends(get_current_user),
): ):
user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
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)
result = await db.execute( result = await db.execute(
select(CalendarEvent) select(CalendarEvent)
@ -422,8 +405,7 @@ async def update_event(
if event.recurrence_rule: if event.recurrence_rule:
await db.flush() await db.flush()
children = generate_occurrences(event) children = generate_occurrences(event)
for child in children: db.add_all(children)
db.add(child)
else: else:
# This IS a parent — update it and regenerate all children # This IS a parent — update it and regenerate all children
for key, value in update_data.items(): for key, value in update_data.items():
@ -439,8 +421,7 @@ async def update_event(
) )
await db.flush() await db.flush()
children = generate_occurrences(event) children = generate_occurrences(event)
for child in children: db.add_all(children)
db.add(child)
await db.commit() await db.commit()
@ -470,12 +451,7 @@ async def delete_event(
db: AsyncSession = Depends(get_db), db: AsyncSession = Depends(get_db),
current_user: User = Depends(get_current_user), current_user: User = Depends(get_current_user),
): ):
user_calendar_ids = select(Calendar.id).where(Calendar.user_id == current_user.id) all_calendar_ids = await get_accessible_calendar_ids(current_user.id, db)
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)
result = await db.execute( result = await db.execute(
select(CalendarEvent).where( select(CalendarEvent).where(
@ -501,20 +477,13 @@ async def delete_event(
this_original_start = event.original_start or event.start_datetime this_original_start = event.original_start or event.start_datetime
if parent_id is not None: 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( await db.execute(
delete(CalendarEvent).where( delete(CalendarEvent).where(
CalendarEvent.parent_event_id == parent_id, CalendarEvent.parent_event_id == parent_id,
CalendarEvent.original_start >= this_original_start, 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: else:
# This event IS the parent — delete it and all children (CASCADE handles children) # This event IS the parent — delete it and all children (CASCADE handles children)
await db.delete(event) await db.delete(event)

View File

@ -1,6 +1,6 @@
import json as _json 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 datetime import datetime
from typing import Literal, Optional from typing import Literal, Optional
@ -17,6 +17,20 @@ class RecurrenceRule(BaseModel):
# monthly_date # monthly_date
day: Optional[int] = Field(None, ge=1, le=31) 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 == "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'")
if self.type == "monthly_date" and self.day is None:
raise ValueError("monthly_date rule requires 'day'")
return self
def _coerce_recurrence_rule(v): def _coerce_recurrence_rule(v):
"""Accept None, dict, RecurrenceRule, or JSON/legacy strings gracefully.""" """Accept None, dict, RecurrenceRule, or JSON/legacy strings gracefully."""
@ -47,10 +61,10 @@ class CalendarEventCreate(BaseModel):
end_datetime: datetime end_datetime: datetime
all_day: bool = False all_day: bool = False
color: Optional[str] = Field(None, max_length=20) 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 recurrence_rule: Optional[RecurrenceRule] = None
is_starred: bool = False 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") @field_validator("recurrence_rule", mode="before")
@classmethod @classmethod
@ -67,10 +81,10 @@ class CalendarEventUpdate(BaseModel):
end_datetime: Optional[datetime] = None end_datetime: Optional[datetime] = None
all_day: Optional[bool] = None all_day: Optional[bool] = None
color: Optional[str] = Field(None, max_length=20) 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 recurrence_rule: Optional[RecurrenceRule] = None
is_starred: Optional[bool] = 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 # Controls which occurrences an edit applies to; absent = non-recurring or whole-series
edit_scope: Optional[Literal["this", "this_and_future"]] = None edit_scope: Optional[Literal["this", "this_and_future"]] = None

View File

@ -20,6 +20,20 @@ PERMISSION_RANK = {"read_only": 1, "create_modify": 2, "full_access": 3}
LOCK_DURATION_MINUTES = 5 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)."""
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 result.all()]
async def get_user_permission(db: AsyncSession, calendar_id: int, user_id: int) -> str | None: 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 Returns "owner" if the user owns the calendar, the permission string

View File

@ -10,6 +10,9 @@ from typing import Optional
from app.models.calendar_event import CalendarEvent 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]: 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) interval: int = _rule_int(rule, "interval", 1)
if interval < 1: if interval < 1:
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) current = parent_start + timedelta(days=interval)
while current < horizon: while current < effective_horizon:
if len(occurrences) >= MAX_OCCURRENCES:
break
occurrences.append(_make_child(current)) occurrences.append(_make_child(current))
current += timedelta(days=interval) current += timedelta(days=interval)
@ -112,6 +119,8 @@ def generate_occurrences(
days_ahead = 7 days_ahead = 7
current = parent_start + timedelta(days=days_ahead) current = parent_start + timedelta(days=days_ahead)
while current < horizon: while current < horizon:
if len(occurrences) >= MAX_OCCURRENCES:
break
occurrences.append(_make_child(current)) occurrences.append(_make_child(current))
current += timedelta(weeks=1) current += timedelta(weeks=1)
@ -126,6 +135,8 @@ def generate_occurrences(
month = 1 month = 1
year += 1 year += 1
while True: while True:
if len(occurrences) >= MAX_OCCURRENCES:
break
target = _nth_weekday_of_month(year, month, weekday, week) target = _nth_weekday_of_month(year, month, weekday, week)
if target is None: if target is None:
# Skip months where Nth weekday doesn't exist # Skip months where Nth weekday doesn't exist
@ -157,6 +168,8 @@ def generate_occurrences(
month = 1 month = 1
year += 1 year += 1
while True: while True:
if len(occurrences) >= MAX_OCCURRENCES:
break
# Some months don't have day 29-31 # Some months don't have day 29-31
try: try:
occ_start = datetime( occ_start = datetime(

View File

@ -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 # 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_search_limit:10m rate=10r/m;
limit_req_zone $binary_remote_addr zone=conn_request_limit:10m rate=3r/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 # Use X-Forwarded-Proto from upstream proxy when present, fall back to $scheme for direct access
map $http_x_forwarded_proto $forwarded_proto { map $http_x_forwarded_proto $forwarded_proto {
@ -122,6 +124,15 @@ server {
include /etc/nginx/proxy-params.conf; include /etc/nginx/proxy-params.conf;
} }
# 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;
include /etc/nginx/proxy-params.conf;
}
# API proxy (catch-all for non-rate-limited endpoints) # API proxy (catch-all for non-rate-limited endpoints)
location /api { location /api {
proxy_set_header Upgrade $http_upgrade; proxy_set_header Upgrade $http_upgrade;

View File

@ -231,6 +231,7 @@ export default function CalendarPage() {
}, },
// AW-3: Reduce from 5s to 30s — personal organiser doesn't need 12 calls/min // AW-3: Reduce from 5s to 30s — personal organiser doesn't need 12 calls/min
refetchInterval: 30_000, refetchInterval: 30_000,
staleTime: 30_000,
}); });
const selectedEvent = useMemo( const selectedEvent = useMemo(