From bba1fca5d9d659b1e8c043f763d5f377d11f9600 Mon Sep 17 00:00:00 2001 From: Davit Date: Tue, 25 Aug 2026 17:52:26 +0400 Subject: [PATCH 1/3] fix: scope revenue cache key to tenant to prevent cross-tenant leakage Revenue cache keys and the DB-unavailable fallback data were keyed by property_id alone. Since property IDs are only unique per tenant (see the composite primary key on properties), two different tenants requesting the same property_id could receive each other's cached revenue data. Both the cache key and the fallback lookup are now scoped by (property_id, tenant_id). --- backend/app/services/cache.py | 12 +++---- backend/app/services/reservations.py | 47 ++++++++++++++-------------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index b81474957..e278c508f 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -10,20 +10,20 @@ async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any """ Fetches revenue summary, utilizing caching to improve performance. """ - cache_key = f"revenue:{property_id}" - + cache_key = f"revenue:{property_id}:tenant:{tenant_id}" + # Try to get from cache cached = await redis_client.get(cache_key) if cached: return json.loads(cached) - + # 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)) - + return result diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 384bd00ab..2eb613ea5 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -12,7 +12,7 @@ async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_ 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) @@ -24,11 +24,11 @@ async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_ AND check_in_date >= $3 AND check_in_date < $4 """ - + # 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 async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: @@ -38,39 +38,39 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, 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 + SELECT property_id, SUM(total_amount) as total_revenue, COUNT(*) as reservation_count - FROM reservations + 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, + "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", + "currency": "USD", "count": row.reservation_count } else: @@ -84,25 +84,26 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, } 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} + ('prop-001', 'tenant-a'): {'total': '1000.00', 'count': 3}, + ('prop-002', 'tenant-a'): {'total': '4975.50', 'count': 4}, + ('prop-003', 'tenant-a'): {'total': '6100.50', 'count': 2}, + ('prop-001', 'tenant-b'): {'total': '2340.75', 'count': 2}, + ('prop-004', 'tenant-b'): {'total': '1776.50', 'count': 4}, + ('prop-005', 'tenant-b'): {'total': '3256.00', 'count': 3} } - - mock_property_data = mock_data.get(property_id, {'total': '0.00', 'count': 0}) - + + mock_property_data = mock_data.get((property_id, tenant_id), {'total': '0.00', 'count': 0}) + return { "property_id": property_id, - "tenant_id": tenant_id, + "tenant_id": tenant_id, "total": mock_property_data['total'], "currency": "USD", "count": mock_property_data['count'] From 599759ce53e35d49f410a5efbc2678ef6ddc46c1 Mon Sep 17 00:00:00 2001 From: Davit Date: Tue, 25 Aug 2026 17:52:54 +0400 Subject: [PATCH 2/3] fix: compute monthly revenue using each property's local timezone calculate_monthly_revenue was a stub that always returned zero, and the revenue path actually in use summed reservations over all time with no date filtering at all, so a monthly report could never match totals a client computed from the raw booking data for a given month. Implemented the monthly aggregation: month boundaries are built in the property's own timezone (read from properties.timezone) and converted to UTC before comparing against check_in_date, so a reservation is bucketed by the date it falls on for that property, not the server's UTC date. Added month/year as optional query params on /dashboard/summary, and extended the cache key to include them so a monthly total can't collide with the all-time total in cache. --- backend/app/api/v1/dashboard.py | 17 +++--- backend/app/services/cache.py | 26 +++++++--- backend/app/services/reservations.py | 78 ++++++++++++++++++++-------- 3 files changed, 84 insertions(+), 37 deletions(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index 1ec352d7e..af4c528b7 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,5 +1,5 @@ from fastapi import APIRouter, Depends, HTTPException -from typing import Dict, Any +from typing import Dict, Any, Optional from app.services.cache import get_revenue_summary from app.core.auth import authenticate_request as get_current_user @@ -8,15 +8,20 @@ @router.get("/dashboard/summary") async def get_dashboard_summary( property_id: str, + month: Optional[int] = None, + year: Optional[int] = None, current_user: dict = Depends(get_current_user) ) -> Dict[str, Any]: - + + if (month is None) != (year is None): + raise HTTPException(status_code=400, detail="month and year must be provided together") + tenant_id = getattr(current_user, "tenant_id", "default_tenant") or "default_tenant" - - revenue_data = await get_revenue_summary(property_id, tenant_id) - + + revenue_data = await get_revenue_summary(property_id, tenant_id, month, year) + total_revenue_float = float(revenue_data['total']) - + return { "property_id": revenue_data['property_id'], "total_revenue": total_revenue_float, diff --git a/backend/app/services/cache.py b/backend/app/services/cache.py index e278c508f..e4b7f2cd5 100644 --- a/backend/app/services/cache.py +++ b/backend/app/services/cache.py @@ -1,27 +1,37 @@ import json import redis.asyncio as redis -from typing import Dict, Any +from typing import Dict, Any, Optional import os # 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]: +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}:tenant:{tenant_id}" + if month is not None and year is not None: + cache_key = f"revenue:{property_id}:tenant:{tenant_id}:month:{month}:year:{year}" + else: + cache_key = f"revenue:{property_id}:tenant:{tenant_id}" # Try to get from cache cached = await redis_client.get(cache_key) if cached: return json.loads(cached) - # 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) + if month is not None and year is not None: + # Revenue calculation is delegated to the reservation service. + from app.services.reservations import calculate_monthly_revenue + result = await calculate_monthly_revenue(property_id, tenant_id, month, year) + else: + from app.services.reservations import calculate_total_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)) diff --git a/backend/app/services/reservations.py b/backend/app/services/reservations.py index 2eb613ea5..ed5d3c75f 100644 --- a/backend/app/services/reservations.py +++ b/backend/app/services/reservations.py @@ -1,35 +1,67 @@ -from datetime import datetime +from datetime import datetime, timezone from decimal import Decimal from typing import Dict, Any, List +from zoneinfo import ZoneInfo -async def calculate_monthly_revenue(property_id: str, month: int, year: int, db_session=None) -> Decimal: +async def calculate_monthly_revenue(property_id: str, tenant_id: str, month: int, year: int) -> Dict[str, Any]: """ Calculates revenue for a specific month. """ + from app.core.database_pool import DatabasePool + from sqlalchemy import text - 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 - """ + db_pool = DatabasePool() + await db_pool.initialize() - # 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') + if not db_pool.session_factory: + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": "0.00", + "currency": "USD", + "count": 0 + } - return Decimal('0') # Placeholder for now until DB connection is finalized + async with db_pool.get_session() as session: + tz_result = 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} + ) + tz_row = tz_result.fetchone() + property_tz = ZoneInfo(tz_row.timezone if tz_row else "UTC") + + local_start = datetime(year, month, 1, tzinfo=property_tz) + if month < 12: + local_end = datetime(year, month + 1, 1, tzinfo=property_tz) + else: + local_end = datetime(year + 1, 1, 1, tzinfo=property_tz) + + start_utc = local_start.astimezone(timezone.utc) + end_utc = local_end.astimezone(timezone.utc) + + query = text(""" + SELECT SUM(total_amount) as total, COUNT(*) as cnt + FROM reservations + WHERE property_id = :property_id AND tenant_id = :tenant_id + AND check_in_date >= :start_date AND check_in_date < :end_date + """) + result = await session.execute(query, { + "property_id": property_id, + "tenant_id": tenant_id, + "start_date": start_utc, + "end_date": end_utc + }) + row = result.fetchone() + total = Decimal(str(row.total)) if row and row.total is not None else Decimal("0") + count = row.cnt if row and row.cnt is not None else 0 + + return { + "property_id": property_id, + "tenant_id": tenant_id, + "total": str(total), + "currency": "USD", + "count": count + } async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str, Any]: """ From ec68e7df14e25846deea7673a604980e4c7b796f Mon Sep 17 00:00:00 2001 From: Davit Date: Tue, 25 Aug 2026 17:53:05 +0400 Subject: [PATCH 3/3] fix: round revenue to cents explicitly before float conversion Revenue totals are tracked with sub-cent precision (NUMERIC(10,3)) and computed as Decimal throughout, but were cast straight to float with no rounding step, so the cent value that reached the client depended on incidental binary floating-point representation rather than a deliberate rounding rule. Quantize to 2 decimal places with standard half-up rounding before the float conversion. --- backend/app/api/v1/dashboard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/api/v1/dashboard.py b/backend/app/api/v1/dashboard.py index af4c528b7..675447c75 100644 --- a/backend/app/api/v1/dashboard.py +++ b/backend/app/api/v1/dashboard.py @@ -1,5 +1,6 @@ from fastapi import APIRouter, Depends, HTTPException from typing import Dict, Any, Optional +from decimal import Decimal, ROUND_HALF_UP from app.services.cache import get_revenue_summary from app.core.auth import authenticate_request as get_current_user @@ -20,7 +21,8 @@ async def get_dashboard_summary( revenue_data = await get_revenue_summary(property_id, tenant_id, month, year) - total_revenue_float = float(revenue_data['total']) + total_revenue_decimal = Decimal(revenue_data['total']).quantize(Decimal('0.01'), rounding=ROUND_HALF_UP) + total_revenue_float = float(total_revenue_decimal) return { "property_id": revenue_data['property_id'],