From b39621891e01c7d34cc077e188e662f974b2cce2 Mon Sep 17 00:00:00 2001 From: cuong Date: Fri, 16 Oct 2026 23:12:02 +0700 Subject: [PATCH] fix: tenant-scoped revenue cache, real DB queries, timezone-aware months and cent rounding --- .gitignore | 6 + backend/app/api/v1/dashboard.py | 43 +++-- backend/app/core/database_pool.py | 46 +++-- backend/app/services/cache.py | 55 ++++-- backend/app/services/reservations.py | 200 ++++++++++----------- backend/pyproject.toml | 1 + backend/requirements-dev.txt | 4 + backend/requirements.txt | 3 +- backend/tests/__init__.py | 0 backend/tests/conftest.py | 131 ++++++++++++++ backend/tests/test_dashboard_api.py | 104 +++++++++++ backend/tests/test_money_and_time.py | 61 +++++++ backend/tests/test_reservations_service.py | 93 ++++++++++ backend/tests/test_revenue_cache.py | 49 +++++ 14 files changed, 649 insertions(+), 147 deletions(-) create mode 100644 .gitignore create mode 100644 backend/requirements-dev.txt create mode 100644 backend/tests/__init__.py create mode 100644 backend/tests/conftest.py create mode 100644 backend/tests/test_dashboard_api.py create mode 100644 backend/tests/test_money_and_time.py create mode 100644 backend/tests/test_reservations_service.py create mode 100644 backend/tests/test_revenue_cache.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000000..d28148cdd1 --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +__pycache__/ +*.pyc +.pytest_cache/ +backend/.venv/ +backend/venv/ +.DS_Store diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e3..048d576204 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,25 +1,46 @@ -from fastapi import APIRouter, Depends, HTTPException -from typing import Dict, Any +from fastapi import APIRouter, Depends, HTTPException, Query +from typing import Dict, Any, Optional +import logging from app.services.cache import get_revenue_summary +from app.services.reservations import PropertyNotFoundError, to_cents from app.core.auth import authenticate_request as get_current_user +logger = logging.getLogger(__name__) + router = APIRouter() @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, + month: Optional[int] = Query(None, ge=1, le=12), + year: Optional[int] = Query(None, ge=2000, le=2100), current_user: dict = Depends(get_current_user) ) -> Dict[str, Any]: - - tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" - - revenue_data = await get_revenue_summary(property_id, tenant_id) - - total_revenue_float = float(revenue_data['total']) - - return { + + # Never fall back to a shared tenant: that would mix data between clients + tenant_id = getattr(current_user, "tenant_id", None) + if not tenant_id: + raise HTTPException(status_code=403, detail="No tenant associated with this user") + + if (month is None) != (year is None): + raise HTTPException(status_code=422, detail="month and year must be provided together") + + try: + revenue_data = await get_revenue_summary(property_id, tenant_id, month, year) + except PropertyNotFoundError: + raise HTTPException(status_code=404, detail="Property not found") + except Exception as e: + logger.error(f"Revenue lookup failed for {property_id} (tenant: {tenant_id}): {e}") + raise HTTPException(status_code=503, detail="Revenue data temporarily unavailable") + + response = { "property_id": revenue_data['property_id'], - "total_revenue": total_revenue_float, + # Rounded to cents as Decimal first, so the float sent over JSON is the exact cent value + "total_revenue": float(to_cents(revenue_data['total'])), "currency": revenue_data['currency'], "reservations_count": revenue_data['count'] } + if month and year: + response["month"] = month + response["year"] = year + return response diff --git a/backend/app/core/database_pool.py b/backend/app/core/database_pool.py index d638dfcfe3..5c1157b374 100644 --- a/backend/app/core/database_pool.py +++ b/backend/app/core/database_pool.py @@ -1,52 +1,61 @@ -import asyncio -from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker -from sqlalchemy.pool import QueuePool import logging +from typing import AsyncIterator +from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker from ..config import settings logger = logging.getLogger(__name__) + +def build_async_database_url(database_url: str) -> str: + """Convert a plain postgres URL into one that uses the asyncpg driver.""" + for prefix in ("postgresql+asyncpg://", "postgresql://", "postgres://"): + if database_url.startswith(prefix): + return "postgresql+asyncpg://" + database_url[len(prefix):] + raise ValueError(f"Unsupported database URL scheme: {database_url.split('://', 1)[0]}") + + class DatabasePool: def __init__(self): self.engine = None self.session_factory = None - + async def initialize(self): - """Initialize database connection pool""" + """Initialize database connection pool (idempotent).""" + if self.session_factory is not None: + return try: - # Create async engine with connection pooling - database_url = f"postgresql+asyncpg://{settings.supabase_db_user}:{settings.supabase_db_password}@{settings.supabase_db_host}:{settings.supabase_db_port}/{settings.supabase_db_name}" - + # Async engines use AsyncAdaptedQueuePool by default; passing QueuePool is rejected by SQLAlchemy. self.engine = create_async_engine( - database_url, - poolclass=QueuePool, + build_async_database_url(settings.database_url), pool_size=20, # Number of connections to maintain max_overflow=30, # Additional connections when needed pool_pre_ping=True, # Validate connections pool_recycle=3600, # Recycle connections every hour echo=False # Set to True for SQL debugging ) - + self.session_factory = async_sessionmaker( bind=self.engine, class_=AsyncSession, expire_on_commit=False ) - + logger.info("✅ Database connection pool initialized") - + except Exception as e: logger.error(f"❌ Database pool initialization failed: {e}") self.engine = None self.session_factory = None - + async def close(self): """Close database connections""" if self.engine: await self.engine.dispose() - - async def get_session(self) -> AsyncSession: - """Get database session from pool""" + self.engine = None + self.session_factory = None + + def get_session(self) -> AsyncSession: + """Get a database session from the pool, for use as `async with db_pool.get_session() as session`.""" if not self.session_factory: raise Exception("Database pool not initialized") return self.session_factory() @@ -54,7 +63,8 @@ async def get_session(self) -> AsyncSession: # Global database pool instance db_pool = DatabasePool() -async def get_db_session() -> AsyncSession: +async def get_db_session() -> AsyncIterator[AsyncSession]: """Dependency to get database session""" + await db_pool.initialize() async with db_pool.get_session() as session: yield session diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957f..ae5c32eb85 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -1,29 +1,50 @@ import json +import logging import redis.asyncio as redis -from typing import Dict, Any +from typing import Dict, Any, Optional import os +logger = logging.getLogger(__name__) + # Initialize Redis client (typically configured centrally). redis_client = redis.Redis.from_url(os.getenv("REDIS_URL", "redis://localhost:6379/0")) -async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any]: +CACHE_TTL_SECONDS = 300 + + +def revenue_cache_key(tenant_id: str, property_id: str, month: Optional[int] = None, year: Optional[int] = None) -> str: + """Cache key scoped by tenant: property IDs are only unique per tenant.""" + period = f"{year:04d}-{month:02d}" if month and year else "all" + return f"revenue:{tenant_id}:{property_id}:{period}" + + +async def get_revenue_summary( + property_id: str, tenant_id: str, month: Optional[int] = None, year: Optional[int] = None +) -> Dict[str, Any]: """ Fetches revenue summary, utilizing caching to improve performance. """ - cache_key = f"revenue:{property_id}" - - # Try to get from cache - cached = await redis_client.get(cache_key) - if cached: - return json.loads(cached) - + cache_key = revenue_cache_key(tenant_id, property_id, month, year) + + # Try to get from cache; a cache outage must not break the dashboard + try: + cached = await redis_client.get(cache_key) + if cached: + return json.loads(cached) + except Exception as e: + logger.warning(f"Revenue cache read failed for {cache_key}: {e}") + # Revenue calculation is delegated to the reservation service. - from app.services.reservations import calculate_total_revenue - - # Calculate revenue - result = await calculate_total_revenue(property_id, tenant_id) - - # Cache the result for 5 minutes - await redis_client.setex(cache_key, 300, json.dumps(result)) - + from app.services.reservations import calculate_monthly_revenue, calculate_total_revenue + + if month and year: + result = await calculate_monthly_revenue(property_id, tenant_id, month, year) + else: + result = await calculate_total_revenue(property_id, tenant_id) + + try: + await redis_client.setex(cache_key, CACHE_TTL_SECONDS, json.dumps(result)) + except Exception as e: + logger.warning(f"Revenue cache write failed for {cache_key}: {e}") + return result diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00abe..4a2172dc90 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,109 +1,109 @@ -from datetime import datetime -from decimal import Decimal -from typing import Dict, Any, List +from datetime import datetime, timezone +from decimal import Decimal, ROUND_HALF_UP +from typing import Dict, Any, Optional, Tuple +from zoneinfo import ZoneInfo +import logging -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: +from sqlalchemy import text + +logger = logging.getLogger(__name__) + +CENTS = Decimal("0.01") + + +class PropertyNotFoundError(Exception): + """Raised when a property does not exist for the given tenant.""" + + +def to_cents(value: Any) -> Decimal: + """Round a monetary amount to cents using half-up rounding (never via float).""" + if value is None: + return Decimal("0.00") + return Decimal(str(value)).quantize(CENTS, rounding=ROUND_HALF_UP) + + +def month_bounds_utc(year: int, month: int, tz_name: str) -> Tuple[datetime, datetime]: """ - Calculates revenue for a specific month. + Return the [start, end) UTC instants of a calendar month as experienced in the property's timezone. """ + if not 1 <= month <= 12: + raise ValueError(f"Invalid month: {month}") + tz = ZoneInfo(tz_name) + start_local = datetime(year, month, 1, tzinfo=tz) + end_local = datetime(year + 1, 1, 1, tzinfo=tz) if month == 12 else datetime(year, month + 1, 1, tzinfo=tz) + return start_local.astimezone(timezone.utc), end_local.astimezone(timezone.utc) + - start_date = datetime(year, month, 1) - if month < 12: - end_date = datetime(year, month + 1, 1) - else: - end_date = datetime(year + 1, 1, 1) - - print(f"DEBUG: Querying revenue for {property_id} from {start_date} to {end_date}") - - # SQL Simulation (This would be executed against the actual DB) - query = """ - SELECT SUM(total_amount) as total - FROM reservations - WHERE property_id = $1 - AND tenant_id = $2 - AND check_in_date >= $3 - AND check_in_date < $4 +async def _get_session_factory(): + from app.core.database_pool import db_pool + + await db_pool.initialize() + if not db_pool.session_factory: + raise RuntimeError("Database pool not available") + return db_pool + + +async def calculate_monthly_revenue(property_id: str, tenant_id: str, month: int, year: int) -> Dict[str, Any]: """ - - # In production this query executes against a database session. - # result = await db.fetch_val(query, property_id, tenant_id, start_date, end_date) - # return result or Decimal('0') - - return Decimal('0') # Placeholder for now until DB connection is finalized + Calculates revenue for a calendar month in the property's local timezone. + """ + pool = await _get_session_factory() + async with pool.get_session() as session: + prop = (await session.execute( + text("SELECT timezone FROM properties WHERE id = :property_id AND tenant_id = :tenant_id"), + {"property_id": property_id, "tenant_id": tenant_id}, + )).fetchone() + if not prop: + raise PropertyNotFoundError(property_id) + + start_utc, end_utc = month_bounds_utc(year, month, prop.timezone or "UTC") + + row = (await session.execute( + text(""" + SELECT + COALESCE(SUM(total_amount), 0) AS total_revenue, + COUNT(*) AS reservation_count + FROM reservations + WHERE property_id = :property_id + AND tenant_id = :tenant_id + AND check_in_date >= :start_date + AND check_in_date < :end_date + """), + {"property_id": property_id, "tenant_id": tenant_id, "start_date": start_utc, "end_date": end_utc}, + )).one() + + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(to_cents(row.total_revenue)), + "currency": "USD", + "count": row.reservation_count, + "month": month, + "year": year, + } + async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: """ - Aggregates revenue from database. + Aggregates all-time revenue for a property, scoped to the tenant. """ - try: - # Import database pool - from app.core.database_pool import DatabasePool - - # Initialize pool if needed - db_pool = DatabasePool() - await db_pool.initialize() - - if db_pool.session_factory: - async with db_pool.get_session() as session: - # Use SQLAlchemy text for raw SQL - from sqlalchemy import text - - query = text(""" - SELECT - property_id, - SUM(total_amount) as total_revenue, - COUNT(*) as reservation_count - FROM reservations - WHERE property_id = :property_id AND tenant_id = :tenant_id - GROUP BY property_id - """) - - result = await session.execute(query, { - "property_id": property_id, - "tenant_id": tenant_id - }) - row = result.fetchone() - - if row: - total_revenue = Decimal(str(row.total_revenue)) - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": str(total_revenue), - "currency": "USD", - "count": row.reservation_count - } - else: - # No reservations found for this property - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": "0.00", - "currency": "USD", - "count": 0 - } - else: - raise Exception("Database pool not available") - - except Exception as e: - print(f"Database error for {property_id} (tenant: {tenant_id}): {e}") - - # Create property-specific mock data for testing when DB is unavailable - # This ensures each property shows different figures - mock_data = { - 'prop-001': {'total': '1000.00', 'count': 3}, - 'prop-002': {'total': '4975.50', 'count': 4}, - 'prop-003': {'total': '6100.50', 'count': 2}, - 'prop-004': {'total': '1776.50', 'count': 4}, - 'prop-005': {'total': '3256.00', 'count': 3} - } - - mock_property_data = mock_data.get(property_id, {'total': '0.00', 'count': 0}) - - return { - "property_id": property_id, - "tenant_id": tenant_id, - "total": mock_property_data['total'], - "currency": "USD", - "count": mock_property_data['count'] - } + pool = await _get_session_factory() + async with pool.get_session() as session: + row = (await session.execute( + text(""" + SELECT + COALESCE(SUM(total_amount), 0) AS total_revenue, + COUNT(*) AS reservation_count + FROM reservations + WHERE property_id = :property_id AND tenant_id = :tenant_id + """), + {"property_id": property_id, "tenant_id": tenant_id}, + )).one() + + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(to_cents(row.total_revenue)), + "currency": "USD", + "count": row.reservation_count, + } diff --git a/backend/pyproject.toml b/backend/pyproject.toml index ebaa4bc92d..e90747a72f 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -42,3 +42,4 @@ dev = [ [tool.pytest.ini_options] testpaths = ["tests"] pythonpath = ["."] +asyncio_mode = "auto" diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt new file mode 100644 index 0000000000..e5c0080444 --- /dev/null +++ b/backend/requirements-dev.txt @@ -0,0 +1,4 @@ +-r requirements.txt +pytest +pytest-asyncio +fakeredis diff --git a/backend/requirements.txt b/backend/requirements.txt index 6b777d2aaa..888f487e90 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -18,7 +18,7 @@ pytz redis>=5.0.0 requests sendgrid -sqlalchemy>=2.0.0 +sqlalchemy[asyncio]>=2.0.0 supabase tenacity>=8.0.0 uvicorn[standard] @@ -27,3 +27,4 @@ loguru redis>=5.0.0 asyncpg>=0.27.0 requests +tzdata diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py new file mode 100644 index 0000000000..580c5b089d --- /dev/null +++ b/backend/tests/conftest.py @@ -0,0 +1,131 @@ +""" +Shared fixtures: an in-memory stand-in for the Postgres database, loaded with the same +rows as database/seed.sql, so the revenue services can be tested without Docker. +""" +from dataclasses import dataclass +from datetime import datetime +from decimal import Decimal +from types import SimpleNamespace + +import pytest + +TENANT_A = "tenant-a" +TENANT_B = "tenant-b" + +PROPERTIES = [ + ("prop-001", TENANT_A, "Europe/Paris"), + ("prop-001", TENANT_B, "America/New_York"), + ("prop-002", TENANT_A, "Europe/Paris"), + ("prop-003", TENANT_A, "Europe/Paris"), + ("prop-004", TENANT_B, "America/New_York"), + ("prop-005", TENANT_B, "America/New_York"), +] + +# (property_id, tenant_id, check_in UTC, total_amount) as in database/seed.sql +RESERVATIONS = [ + ("prop-001", TENANT_A, "2024-02-29T23:30:00+00:00", "1250.000"), + ("prop-001", TENANT_A, "2024-03-15T10:00:00+00:00", "333.333"), + ("prop-001", TENANT_A, "2024-03-16T10:00:00+00:00", "333.333"), + ("prop-001", TENANT_A, "2024-03-17T10:00:00+00:00", "333.334"), + ("prop-002", TENANT_A, "2024-03-05T14:00:00+00:00", "1250.00"), + ("prop-002", TENANT_A, "2024-03-12T16:00:00+00:00", "1475.50"), + ("prop-002", TENANT_A, "2024-03-20T15:00:00+00:00", "1199.25"), + ("prop-002", TENANT_A, "2024-03-25T18:00:00+00:00", "1050.75"), + ("prop-003", TENANT_A, "2024-03-02T15:00:00+00:00", "2850.00"), + ("prop-003", TENANT_A, "2024-03-18T16:00:00+00:00", "3250.50"), + ("prop-004", TENANT_B, "2024-03-08T18:00:00+00:00", "420.00"), + ("prop-004", TENANT_B, "2024-03-14T17:00:00+00:00", "560.75"), + ("prop-004", TENANT_B, "2024-03-22T16:00:00+00:00", "480.25"), + ("prop-004", TENANT_B, "2024-03-28T19:00:00+00:00", "315.50"), + ("prop-005", TENANT_B, "2024-03-06T19:00:00+00:00", "920.00"), + ("prop-005", TENANT_B, "2024-03-15T18:00:00+00:00", "1080.40"), + ("prop-005", TENANT_B, "2024-03-24T20:00:00+00:00", "1255.60"), +] + + +@dataclass +class FakeDatabase: + properties: list + reservations: list + + def __post_init__(self): + self.executed = [] # (sql, params) of every query, for assertions + + def run(self, sql: str, params: dict): + self.executed.append((sql, params)) + if "FROM properties" in sql: + for pid, tid, tz in self.properties: + if pid == params["property_id"] and tid == params["tenant_id"]: + return SimpleNamespace(timezone=tz) + return None + if "FROM reservations" in sql: + # Mirror the SQL filters; refuse queries that are not tenant-scoped. + assert "tenant_id = :tenant_id" in sql, "reservation query is not scoped by tenant" + rows = [ + Decimal(amount) + for pid, tid, check_in, amount in self.reservations + if pid == params["property_id"] + and tid == params["tenant_id"] + and ("start_date" not in params or params["start_date"] <= datetime.fromisoformat(check_in)) + and ("end_date" not in params or datetime.fromisoformat(check_in) < params["end_date"]) + ] + return SimpleNamespace(total_revenue=sum(rows, Decimal("0")), reservation_count=len(rows)) + raise AssertionError(f"Unexpected query: {sql}") + + +class FakeResult: + def __init__(self, row): + self._row = row + + def fetchone(self): + return self._row + + def one(self): + assert self._row is not None, "query returned no rows" + return self._row + + +class FakeSession: + def __init__(self, db: FakeDatabase): + self._db = db + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + async def execute(self, query, params): + return FakeResult(self._db.run(str(query), params)) + + +class FakePool: + def __init__(self, db: FakeDatabase): + self._db = db + self.session_factory = object() + + async def initialize(self): + pass + + def get_session(self): + return FakeSession(self._db) + + +@pytest.fixture +def fake_db(monkeypatch): + """Route the services' database pool to an in-memory copy of the seed data.""" + from app.core import database_pool + + db = FakeDatabase(list(PROPERTIES), list(RESERVATIONS)) + monkeypatch.setattr(database_pool, "db_pool", FakePool(db)) + return db + + +@pytest.fixture +def fake_redis(monkeypatch): + import fakeredis + from app.services import cache + + client = fakeredis.FakeAsyncRedis() + monkeypatch.setattr(cache, "redis_client", client) + return client diff --git a/backend/tests/test_dashboard_api.py b/backend/tests/test_dashboard_api.py new file mode 100644 index 0000000000..1f9995a93e --- /dev/null +++ b/backend/tests/test_dashboard_api.py @@ -0,0 +1,104 @@ +from types import SimpleNamespace + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.api.v1 import dashboard, login +from app.core.auth import authenticate_request + +from .conftest import TENANT_A, TENANT_B + + +def make_client(user=None): + app = FastAPI() + app.include_router(dashboard.router, prefix="/api/v1") + app.include_router(login.router, prefix="/api/v1") + if user is not None: + app.dependency_overrides[authenticate_request] = lambda: user + return TestClient(app) + + +@pytest.fixture +def tenant_a_client(fake_db, fake_redis): + return make_client(SimpleNamespace(tenant_id=TENANT_A)) + + +def test_returns_tenant_scoped_total(tenant_a_client): + body = tenant_a_client.get("/api/v1/dashboard/summary", params={"property_id": "prop-002"}).json() + assert body == { + "property_id": "prop-002", + "total_revenue": 4975.5, + "currency": "USD", + "reservations_count": 4, + } + + +def test_monthly_summary_uses_property_timezone(tenant_a_client): + body = tenant_a_client.get( + "/api/v1/dashboard/summary", params={"property_id": "prop-001", "month": 3, "year": 2024} + ).json() + assert body["total_revenue"] == 2250.0 + assert body["reservations_count"] == 4 + assert (body["month"], body["year"]) == (3, 2024) + + +def test_total_is_rounded_to_cents(fake_db, fake_redis): + fake_db.reservations.append(("prop-002", TENANT_A, "2024-03-29T10:00:00+00:00", "0.005")) + client = make_client(SimpleNamespace(tenant_id=TENANT_A)) + body = client.get("/api/v1/dashboard/summary", params={"property_id": "prop-002"}).json() + assert body["total_revenue"] == 4975.51 + + +def test_user_without_tenant_is_rejected(fake_db, fake_redis): + client = make_client(SimpleNamespace(tenant_id=None)) + response = client.get("/api/v1/dashboard/summary", params={"property_id": "prop-001"}) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + "params", + [ + {"property_id": "prop-001", "month": 3}, + {"property_id": "prop-001", "year": 2024}, + {"property_id": "prop-001", "month": 13, "year": 2024}, + ], +) +def test_invalid_period_is_rejected(tenant_a_client, params): + assert tenant_a_client.get("/api/v1/dashboard/summary", params=params).status_code == 422 + + +def test_unknown_property_for_month_is_404(fake_db, fake_redis): + client = make_client(SimpleNamespace(tenant_id=TENANT_B)) + response = client.get( + "/api/v1/dashboard/summary", params={"property_id": "prop-002", "month": 3, "year": 2024} + ) + assert response.status_code == 404 + + +def test_database_failure_is_503_not_fake_data(monkeypatch, fake_redis): + async def boom(*args, **kwargs): + raise RuntimeError("Database pool not available") + + monkeypatch.setattr("app.services.reservations.calculate_total_revenue", boom) + client = make_client(SimpleNamespace(tenant_id=TENANT_A)) + response = client.get("/api/v1/dashboard/summary", params={"property_id": "prop-001"}) + assert response.status_code == 503 + + +def test_real_logins_see_only_their_own_prop_001(fake_db, fake_redis): + """End to end through login + JWT auth: the two clients share property id prop-001.""" + client = make_client() + + def summary_for(email, password): + token = client.post("/api/v1/auth/login", json={"email": email, "password": password}).json() + headers = {"Authorization": f"Bearer {token['access_token']}"} + return client.get("/api/v1/dashboard/summary", params={"property_id": "prop-001"}, headers=headers).json() + + sunset = summary_for("sunset@propertyflow.com", "client_a_2024") + ocean = summary_for("ocean@propertyflow.com", "client_b_2024") + sunset_again = summary_for("sunset@propertyflow.com", "client_a_2024") + + assert sunset["total_revenue"] == 2250.0 + assert ocean["total_revenue"] == 0.0 + assert sunset_again == sunset diff --git a/backend/tests/test_money_and_time.py b/backend/tests/test_money_and_time.py new file mode 100644 index 0000000000..d0f5ed279e --- /dev/null +++ b/backend/tests/test_money_and_time.py @@ -0,0 +1,61 @@ +from datetime import datetime, timezone +from decimal import Decimal + +import pytest + +from app.services.reservations import month_bounds_utc, to_cents + + +class TestToCents: + @pytest.mark.parametrize( + "raw, expected", + [ + ("1000.000", "1000.00"), + ("2.345", "2.35"), # half-up, not banker's rounding + ("2.344", "2.34"), + ("0.005", "0.01"), + ("4975.5", "4975.50"), + (Decimal("333.333") * 3, "1000.00"), + (0, "0.00"), + (None, "0.00"), + ], + ) + def test_rounds_half_up_to_cents(self, raw, expected): + assert to_cents(raw) == Decimal(expected) + + def test_does_not_inherit_float_error(self): + # 0.1 + 0.2 == 0.30000000000000004 as a float; summing Decimals must stay exact + total = sum([Decimal("0.1"), Decimal("0.2")], Decimal("0")) + assert to_cents(total) == Decimal("0.30") + + def test_float_of_cents_is_exact_cent_value(self): + assert float(to_cents("1199.255")) == 1199.26 + + +class TestMonthBoundsUtc: + def test_paris_march_starts_before_utc_midnight(self): + start, end = month_bounds_utc(2024, 3, "Europe/Paris") + assert start == datetime(2024, 2, 29, 23, 0, tzinfo=timezone.utc) # CET = UTC+1 + assert end == datetime(2024, 3, 31, 22, 0, tzinfo=timezone.utc) # CEST = UTC+2 after DST + + def test_new_york_march_handles_dst(self): + start, end = month_bounds_utc(2024, 3, "America/New_York") + assert start == datetime(2024, 3, 1, 5, 0, tzinfo=timezone.utc) # EST = UTC-5 + assert end == datetime(2024, 4, 1, 4, 0, tzinfo=timezone.utc) # EDT = UTC-4 + + def test_december_rolls_over_to_next_year(self): + start, end = month_bounds_utc(2024, 12, "UTC") + assert start == datetime(2024, 12, 1, tzinfo=timezone.utc) + assert end == datetime(2025, 1, 1, tzinfo=timezone.utc) + + def test_late_feb_utc_booking_belongs_to_march_in_paris(self): + check_in = datetime(2024, 2, 29, 23, 30, tzinfo=timezone.utc) # res-tz-1 + start, end = month_bounds_utc(2024, 3, "Europe/Paris") + assert start <= check_in < end + utc_start, _ = month_bounds_utc(2024, 3, "UTC") + assert check_in < utc_start # the old naive-UTC logic dropped it from March + + @pytest.mark.parametrize("month", [0, 13]) + def test_rejects_invalid_month(self, month): + with pytest.raises(ValueError): + month_bounds_utc(2024, month, "UTC") diff --git a/backend/tests/test_reservations_service.py b/backend/tests/test_reservations_service.py new file mode 100644 index 0000000000..0872e36747 --- /dev/null +++ b/backend/tests/test_reservations_service.py @@ -0,0 +1,93 @@ +import pytest + +from app.core.database_pool import build_async_database_url +from app.services.reservations import ( + PropertyNotFoundError, + calculate_monthly_revenue, + calculate_total_revenue, +) + +from .conftest import TENANT_A, TENANT_B + + + +class TestTotalRevenue: + async def test_same_property_id_is_isolated_per_tenant(self, fake_db): + a = await calculate_total_revenue("prop-001", TENANT_A) + b = await calculate_total_revenue("prop-001", TENANT_B) + assert (a["total"], a["count"]) == ("2250.00", 4) + assert (b["total"], b["count"]) == ("0.00", 0) + assert a["tenant_id"] == TENANT_A and b["tenant_id"] == TENANT_B + + async def test_every_query_is_tenant_scoped(self, fake_db): + await calculate_total_revenue("prop-004", TENANT_B) + for sql, params in fake_db.executed: + assert "tenant_id = :tenant_id" in sql + assert params["tenant_id"] == TENANT_B + + @pytest.mark.parametrize( + "property_id, tenant_id, total, count", + [ + ("prop-002", TENANT_A, "4975.50", 4), + ("prop-003", TENANT_A, "6100.50", 2), + ("prop-004", TENANT_B, "1776.50", 4), + ("prop-005", TENANT_B, "3256.00", 3), + ], + ) + async def test_totals_match_seed_data(self, fake_db, property_id, tenant_id, total, count): + result = await calculate_total_revenue(property_id, tenant_id) + assert (result["total"], result["count"]) == (total, count) + + async def test_other_tenants_property_returns_nothing(self, fake_db): + result = await calculate_total_revenue("prop-002", TENANT_B) + assert (result["total"], result["count"]) == ("0.00", 0) + + async def test_database_failure_raises_instead_of_returning_fake_numbers(self, monkeypatch): + from app.core import database_pool + + class BrokenPool: + session_factory = None + + async def initialize(self): + pass + + monkeypatch.setattr(database_pool, "db_pool", BrokenPool()) + with pytest.raises(RuntimeError): + await calculate_total_revenue("prop-001", TENANT_A) + + +class TestMonthlyRevenue: + async def test_paris_march_includes_booking_made_late_feb_utc(self, fake_db): + result = await calculate_monthly_revenue("prop-001", TENANT_A, month=3, year=2024) + assert (result["total"], result["count"]) == ("2250.00", 4) + + async def test_paris_february_excludes_that_booking(self, fake_db): + result = await calculate_monthly_revenue("prop-001", TENANT_A, month=2, year=2024) + assert (result["total"], result["count"]) == ("0.00", 0) + + async def test_uses_property_timezone_for_bounds(self, fake_db): + await calculate_monthly_revenue("prop-004", TENANT_B, month=3, year=2024) + _, params = fake_db.executed[-1] + assert params["start_date"].isoformat() == "2024-03-01T05:00:00+00:00" + assert params["end_date"].isoformat() == "2024-04-01T04:00:00+00:00" + + async def test_property_of_another_tenant_is_not_found(self, fake_db): + with pytest.raises(PropertyNotFoundError): + await calculate_monthly_revenue("prop-002", TENANT_B, month=3, year=2024) + + +class TestDatabaseUrl: + @pytest.mark.parametrize( + "url", + [ + "postgresql://postgres:postgres@db:5432/propertyflow", + "postgres://postgres:postgres@db:5432/propertyflow", + "postgresql+asyncpg://postgres:postgres@db:5432/propertyflow", + ], + ) + def test_uses_asyncpg_driver(self, url): + assert build_async_database_url(url) == "postgresql+asyncpg://postgres:postgres@db:5432/propertyflow" + + def test_rejects_other_schemes(self): + with pytest.raises(ValueError): + build_async_database_url("mysql://x@y/z") diff --git a/backend/tests/test_revenue_cache.py b/backend/tests/test_revenue_cache.py new file mode 100644 index 0000000000..9723ecebde --- /dev/null +++ b/backend/tests/test_revenue_cache.py @@ -0,0 +1,49 @@ +import pytest + +from app.services import cache +from app.services.cache import get_revenue_summary, revenue_cache_key + +from .conftest import TENANT_A, TENANT_B + + + +def test_cache_key_contains_tenant_and_period(): + assert revenue_cache_key(TENANT_A, "prop-001") == "revenue:tenant-a:prop-001:all" + assert revenue_cache_key(TENANT_A, "prop-001", 3, 2024) == "revenue:tenant-a:prop-001:2024-03" + assert revenue_cache_key(TENANT_A, "prop-001") != revenue_cache_key(TENANT_B, "prop-001") + + +async def test_cached_value_of_one_tenant_is_never_served_to_another(fake_db, fake_redis): + first = await get_revenue_summary("prop-001", TENANT_A) + second = await get_revenue_summary("prop-001", TENANT_B) + assert first["total"] == "2250.00" + assert second["total"] == "0.00" + assert second["tenant_id"] == TENANT_B + + +async def test_second_request_is_served_from_cache(fake_db, fake_redis): + await get_revenue_summary("prop-002", TENANT_A) + queries_after_first = len(fake_db.executed) + again = await get_revenue_summary("prop-002", TENANT_A) + assert again["total"] == "4975.50" + assert len(fake_db.executed) == queries_after_first + assert await fake_redis.ttl("revenue:tenant-a:prop-002:all") > 0 + + +async def test_monthly_and_all_time_are_cached_separately(fake_db, fake_redis): + march = await get_revenue_summary("prop-001", TENANT_A, 3, 2024) + feb = await get_revenue_summary("prop-001", TENANT_A, 2, 2024) + assert (march["total"], feb["total"]) == ("2250.00", "0.00") + + +async def test_redis_outage_falls_back_to_database(fake_db, monkeypatch): + class DownRedis: + async def get(self, key): + raise ConnectionError("redis down") + + async def setex(self, key, ttl, value): + raise ConnectionError("redis down") + + monkeypatch.setattr(cache, "redis_client", DownRedis()) + result = await get_revenue_summary("prop-005", TENANT_B) + assert result["total"] == "3256.00"