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.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(

View File

@ -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)

View File

@ -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,20 @@ 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 == "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):
"""Accept None, dict, RecurrenceRule, or JSON/legacy strings gracefully."""
@ -47,10 +61,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 +81,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

View File

@ -20,6 +20,20 @@ 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)."""
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:
"""
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
# 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(

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
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,15 @@ server {
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)
location /api {
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
refetchInterval: 30_000,
staleTime: 30_000,
});
const selectedEvent = useMemo(