Repository navigation
Bug Fixes - Cache Isolation, Async DB Pool, Timezone Dates, Currency Precision - #224
HachemOuanes wants to merge 1 commit into
Conversation
HachemOuanes
commented
Oct 13, 2026
- Fixed cross-tenant data leak where tenants sharing a property ID could receive each other's cached revenue data
- Fixed runtime errors caused by using a synchronous connection pool with an async database engine
- Fixed fragile database URL construction that bypassed the canonical application config
- Fixed incorrect async declaration on a method that returned synchronously, breaking async context manager usage
- Fixed incorrect monthly revenue boundaries for properties in non-UTC timezones by converting all date ranges to UTC before querying
- Fixed floating-point precision errors in currency values by representing amounts as integer minor units (e.g. cents) with an explicit exponent, supporting currencies with non-standard decimal places (JPY, KWD)
| currency = revenue_data['currency'] | ||
| currency_exponent = CURRENCY_EXPONENTS.get(currency, 2) | ||
| total_decimal = Decimal(str(revenue_data['total'])) | ||
| amount_minor_units = int(total_decimal * (10 ** currency_exponent)) |
There was a problem hiding this comment.
Bug: Converting total_decimal to minor units using int() truncates decimal values instead of rounding, causing precision loss for amounts with three decimal places.
Severity: MEDIUM
Suggested Fix
Use round() before converting to int, or use decimal quantization: int(round(total_decimal * (10 ** currency_exponent))).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/app/api/v1/dashboard.py#L21-L24
Potential issue: When converting `total_decimal` to minor units using `int(total_decimal
* (10 ** currency_exponent))`, the value is truncated toward zero instead of being
rounded. Because amounts are stored with NUMERIC(10, 3) precision, fractional cents
(such as 999.999) truncate down to 99999 instead of rounding to 100000, displaying as
999.99 instead of 1000.00 and losing up to one minor currency unit.
Did we get this right? 👍 / 👎 to inform future reviews.
| database_url = settings.database_url.replace("postgresql://", "postgresql+asyncpg://", 1) | ||
|
|
||
| self.engine = create_async_engine( | ||
| database_url, | ||
| poolclass=QueuePool, | ||
| poolclass=AsyncAdaptedQueuePool, |
There was a problem hiding this comment.
Bug: A new DatabasePool is instantiated on every cache miss without disposing of existing engines, leading to database connection accumulation.
Severity: HIGH
Suggested Fix
Use a shared singleton instance of DatabasePool managed during application lifespan events, or ensure db_pool.close() / engine.dispose() is called after operations complete.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/app/core/database_pool.py#L18-L22
Potential issue: Each cache miss in `calculate_total_revenue()` instantiates and
initializes a new `DatabasePool()` instance. With the corrected database URL
configuration, pool initialization succeeds but the newly created async engines are
never closed or disposed. Over time, unclosed engines and pooled connections accumulate
on each cache miss, risking database connection limit exhaustion.
Did we get this right? 👍 / 👎 to inform future reviews.