From b5dc2e1145cc028c007cc01910c245cd86cae9a4 Mon Sep 17 00:00:00 2001 From: "tembo-io[bot]" <208362400+tembo-io[bot]@users.noreply.github.com> Date: Fri, 5 Sep 2025 22:01:36 +0000 Subject: [PATCH] perf: optimize system performance and database operations --- .../20250905_add_performance_indexes.py | 87 +++++++++++ .../app/api/api_v1/endpoints/categories.py | 35 ++++- .../app/app/api/api_v1/endpoints/expenses.py | 78 ++++++---- backend/app/app/crud/crud_expense.py | 143 +++++++++++++++++- backend/app/app/crud/crud_income.py | 137 ++++++++++++++++- backend/app/app/utilities/redis.py | 65 ++++++++ 6 files changed, 507 insertions(+), 38 deletions(-) create mode 100644 backend/app/alembic/versions/20250905_add_performance_indexes.py diff --git a/backend/app/alembic/versions/20250905_add_performance_indexes.py b/backend/app/alembic/versions/20250905_add_performance_indexes.py new file mode 100644 index 0000000..0ea7d7d --- /dev/null +++ b/backend/app/alembic/versions/20250905_add_performance_indexes.py @@ -0,0 +1,87 @@ +"""add performance indexes + +Revision ID: perf_idx_001 +Revises: fa7a11b93453 +Create Date: 2025-09-05 20:53:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'perf_idx_001' +down_revision = 'fa7a11b93453' +branch_labels = None +depends_on = None + + +def upgrade(): + # Add indexes for frequently queried fields + + # Expense table indexes + op.create_index('idx_expense_owner_id', 'expense', ['owner_id']) + op.create_index('idx_expense_date', 'expense', ['date']) + op.create_index('idx_expense_category_id', 'expense', ['category_id']) + op.create_index('idx_expense_subcategory_id', 'expense', ['subcategory_id']) + op.create_index('idx_expense_account_id', 'expense', ['account_id']) + op.create_index('idx_expense_place_id', 'expense', ['place_id']) + op.create_index('idx_expense_owner_date', 'expense', ['owner_id', 'date']) + + # Income table indexes + op.create_index('idx_income_owner_id', 'income', ['owner_id']) + op.create_index('idx_income_date', 'income', ['date']) + op.create_index('idx_income_subcategory_id', 'income', ['subcategory_id']) + op.create_index('idx_income_account_id', 'income', ['account_id']) + op.create_index('idx_income_place_id', 'income', ['place_id']) + op.create_index('idx_income_owner_date', 'income', ['owner_id', 'date']) + + # Transfer table indexes + op.create_index('idx_transfer_owner_id', 'transfer', ['owner_id']) + op.create_index('idx_transfer_date', 'transfer', ['date']) + op.create_index('idx_transfer_from_acc', 'transfer', ['from_acc']) + op.create_index('idx_transfer_to_acc', 'transfer', ['to_acc']) + op.create_index('idx_transfer_owner_date', 'transfer', ['owner_id', 'date']) + + # Account table indexes + op.create_index('idx_account_owner_id', 'account', ['owner_id']) + + # Category table indexes + op.create_index('idx_category_owner_id', 'category', ['owner_id']) + + # Subcategory table indexes + op.create_index('idx_subcategory_owner_id', 'subcategory', ['owner_id']) + op.create_index('idx_subcategory_category_id', 'subcategory', ['category_id']) + + # Place table indexes + op.create_index('idx_place_name', 'place', ['name']) + + +def downgrade(): + # Remove indexes + op.drop_index('idx_expense_owner_id') + op.drop_index('idx_expense_date') + op.drop_index('idx_expense_category_id') + op.drop_index('idx_expense_subcategory_id') + op.drop_index('idx_expense_account_id') + op.drop_index('idx_expense_place_id') + op.drop_index('idx_expense_owner_date') + + op.drop_index('idx_income_owner_id') + op.drop_index('idx_income_date') + op.drop_index('idx_income_subcategory_id') + op.drop_index('idx_income_account_id') + op.drop_index('idx_income_place_id') + op.drop_index('idx_income_owner_date') + + op.drop_index('idx_transfer_owner_id') + op.drop_index('idx_transfer_date') + op.drop_index('idx_transfer_from_acc') + op.drop_index('idx_transfer_to_acc') + op.drop_index('idx_transfer_owner_date') + + op.drop_index('idx_account_owner_id') + op.drop_index('idx_category_owner_id') + op.drop_index('idx_subcategory_owner_id') + op.drop_index('idx_subcategory_category_id') + op.drop_index('idx_place_name') \ No newline at end of file diff --git a/backend/app/app/api/api_v1/endpoints/categories.py b/backend/app/app/api/api_v1/endpoints/categories.py index 2513a43..444cea2 100644 --- a/backend/app/app/api/api_v1/endpoints/categories.py +++ b/backend/app/app/api/api_v1/endpoints/categories.py @@ -8,6 +8,7 @@ from app import crud, models, schemas from app.api import deps +from app.utilities import redis router = APIRouter() @@ -20,8 +21,14 @@ async def read_categories( current_user: models.User = Depends(deps.get_current_active_user), ) -> Any: """ - Retrieve categories. + Retrieve categories with caching. """ + # Check cache first for non-superuser requests with default pagination + if not crud.user.is_superuser(current_user) and skip == 0 and limit == 100: + cached_categories = await redis.get_cached_user_categories(current_user.id) + if cached_categories: + return cached_categories + if crud.user.is_superuser(current_user): categories = await crud.category.get_multi(db, skip=skip, limit=limit) else: @@ -29,6 +36,24 @@ async def read_categories( db=db, owner_id=current_user.id, skip=skip, limit=limit ) + # Cache the result for regular users with default pagination + if not crud.user.is_superuser(current_user) and skip == 0 and limit == 100: + # Convert SQLAlchemy objects to dict for JSON serialization + categories_data = [ + { + "id": cat.id, + "name": cat.name, + "color": cat.color, + "is_income": cat.is_income, + "is_default": cat.is_default, + "total": cat.total, + "owner_id": cat.owner_id, + "created_at": cat.created_at, + "updated_at": cat.updated_at + } for cat in categories + ] + await redis.cache_user_categories(current_user.id, categories_data) + return categories @@ -45,6 +70,8 @@ async def create_category( category = await crud.category.create_with_owner( db=db, obj_in=category_in, owner_id=current_user.id ) + # Invalidate cache after creating new category + await redis.invalidate_user_categories_cache(current_user.id) return category @@ -95,6 +122,8 @@ async def update_category( category_in.updated_at = datetime.now(timezone.utc) category = await crud.category.update(db=db, db_obj=category, obj_in=category_in) + # Invalidate cache after updating category + await redis.invalidate_user_categories_cache(current_user.id) return category @@ -114,5 +143,7 @@ async def delete_category( raise HTTPException(status_code=400, detail="This item cannot be deleted") await crud.category.remove(db=db, id=id) - + + # Invalidate cache after deleting category + await redis.invalidate_user_categories_cache(current_user.id) return schemas.DeletionResponse(message=f"Item {id} deleted") diff --git a/backend/app/app/api/api_v1/endpoints/expenses.py b/backend/app/app/api/api_v1/endpoints/expenses.py index e5db6cb..057bbc0 100644 --- a/backend/app/app/api/api_v1/endpoints/expenses.py +++ b/backend/app/app/api/api_v1/endpoints/expenses.py @@ -5,7 +5,7 @@ from typing import Any from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import update as updateDb +from sqlalchemy import update as updateDb, select from sqlalchemy.ext.asyncio import AsyncSession from app import crud, models, schemas @@ -431,50 +431,74 @@ async def delete_expenses_bulk( if not valid_ids: raise HTTPException(status_code=404, detail="No valid expenses found") - # Update category and subcategory totals before deleting + # Batch update category and subcategory totals before deleting + category_updates = {} + subcategory_updates = {} + for expense in expenses_to_delete: - # Update category total if it exists if expense.category_id: - category = await crud.category.get(db=db, id=expense.category_id) - - if category: + category_updates[expense.category_id] = category_updates.get(expense.category_id, 0) + expense.amount + if expense.subcategory_id: + subcategory_updates[expense.subcategory_id] = subcategory_updates.get(expense.subcategory_id, 0) + expense.amount + + # Batch fetch categories and subcategories + if category_updates: + category_results = await db.execute( + select(models.Category).filter(models.Category.id.in_(category_updates.keys())) + ) + categories_dict = {cat.id: cat for cat in category_results.scalars().all()} + + for category_id, amount_to_subtract in category_updates.items(): + if category_id in categories_dict: + category = categories_dict[category_id] await db.execute( updateDb(category.__class__) .where(category.__class__.id == category.id) - .values(total=category.total - expense.amount) + .values(total=category.total - amount_to_subtract) .execution_options(synchronize_session="fetch") ) - await db.commit() - - # Update subcategory total if it exists - if expense.subcategory_id: - subcategory = await crud.subcategory.get(db=db, id=expense.subcategory_id) - - if subcategory: + + if subcategory_updates: + subcategory_results = await db.execute( + select(models.Subcategory).filter(models.Subcategory.id.in_(subcategory_updates.keys())) + ) + subcategories_dict = {sub.id: sub for sub in subcategory_results.scalars().all()} + + for subcategory_id, amount_to_subtract in subcategory_updates.items(): + if subcategory_id in subcategories_dict: + subcategory = subcategories_dict[subcategory_id] await db.execute( updateDb(subcategory.__class__) .where(subcategory.__class__.id == subcategory.id) - .values(total=subcategory.total - expense.amount) + .values(total=subcategory.total - amount_to_subtract) .execution_options(synchronize_session="fetch") ) - await db.commit() # Now delete the expenses removed_expenses = await crud.expense.remove_multi(db=db, ids=valid_ids) - # Update balances + # Batch update balances + total_user_expense = sum(expense.amount for expense in removed_expenses) + account_updates = {} + for expense in removed_expenses: - await crud.user.update_balance( - db=db, user_id=current_user.id, is_Expense=True, amount=-expense.amount - ) if expense.account_id: - await crud.account.update_by_id_and_field( - db=db, - owner_id=current_user.id, - id=expense.account_id, - column="total_expenses", - amount=-expense.amount, - ) + account_updates[expense.account_id] = account_updates.get(expense.account_id, 0) + expense.amount + + # Update user balance once + await crud.user.update_balance( + db=db, user_id=current_user.id, is_Expense=True, amount=-total_user_expense + ) + + # Batch update account balances + for account_id, amount in account_updates.items(): + await crud.account.update_by_id_and_field( + db=db, + owner_id=current_user.id, + id=account_id, + column="total_expenses", + amount=-amount, + ) return schemas.BulkDeletionResponse( message=f"Deleted {len(removed_expenses)} expenses", diff --git a/backend/app/app/crud/crud_expense.py b/backend/app/app/crud/crud_expense.py index e6474e2..65088d4 100644 --- a/backend/app/app/crud/crud_expense.py +++ b/backend/app/app/crud/crud_expense.py @@ -52,7 +52,6 @@ async def create_with_owner( .values(total=category.total + obj_in_data["amount"]) .execution_options(synchronize_session="fetch") ) - await db.commit() if obj_in_data["subcategory_id"]: subcategory = await crud.subcategory.get( @@ -69,7 +68,6 @@ async def create_with_owner( .values(total=subcategory.total + obj_in_data["amount"]) .execution_options(synchronize_session="fetch") ) - await db.commit() if obj_in_data["place_id"]: place = await crud.place.get(db=db, id=obj_in_data["place_id"]) @@ -91,10 +89,147 @@ async def create_with_owner( async def create_multi_with_owner( self, db: AsyncSession, *, obj_list: list[ExpenseCreate], owner_id: int ) -> list[Expense]: + if not obj_list: + return [] + + # Pre-fetch and validate all referenced entities + account_ids = [obj.account_id for obj in obj_list if obj.account_id] + category_ids = [obj.category_id for obj in obj_list if obj.category_id] + subcategory_ids = [obj.subcategory_id for obj in obj_list if obj.subcategory_id] + place_ids = [obj.place_id for obj in obj_list if obj.place_id] + + # Batch fetch entities + accounts = {} + categories = {} + subcategories = {} + places = {} + + if account_ids: + account_results = await db.execute( + select(crud.account.model).filter(crud.account.model.id.in_(account_ids)) + ) + accounts = {acc.id: acc for acc in account_results.scalars().all()} + + if category_ids: + category_results = await db.execute( + select(crud.category.model).filter(crud.category.model.id.in_(category_ids)) + ) + categories = {cat.id: cat for cat in category_results.scalars().all()} + + if subcategory_ids: + subcategory_results = await db.execute( + select(crud.subcategory.model).filter(crud.subcategory.model.id.in_(subcategory_ids)) + ) + subcategories = {sub.id: sub for sub in subcategory_results.scalars().all()} + + if place_ids: + place_results = await db.execute( + select(crud.place.model).filter(crud.place.model.id.in_(place_ids)) + ) + places = {place.id: place for place in place_results.scalars().all()} + + # Process expenses and calculate totals created_expenses = [] + account_updates = {} + category_updates = {} + subcategory_updates = {} + total_user_expense = 0 + for obj_in in obj_list: - expense = await self.create_with_owner(db, obj_in=obj_in, owner_id=owner_id) - created_expenses.append(expense) + obj_in_data = jsonable_encoder(obj_in) + + # Convert date string to datetime object + date_str = obj_in_data["date"] + if date_str: + try: + obj_in_data["date"] = datetime.strptime(date_str, "%Y-%m-%d").date() + except: + obj_in_data["date"] = None + + # Validate and accumulate account updates + if obj_in_data["account_id"] and obj_in_data["account_id"] in accounts: + account = accounts[obj_in_data["account_id"]] + if account.owner_id == owner_id: + account_updates[obj_in_data["account_id"]] = account_updates.get(obj_in_data["account_id"], 0) + obj_in_data["amount"] + else: + obj_in_data["account_id"] = None + else: + obj_in_data["account_id"] = None + + # Validate and accumulate category updates + if obj_in_data["category_id"] and obj_in_data["category_id"] in categories: + category = categories[obj_in_data["category_id"]] + if category.owner_id == owner_id: + category_updates[obj_in_data["category_id"]] = category_updates.get(obj_in_data["category_id"], 0) + obj_in_data["amount"] + else: + obj_in_data["category_id"] = None + else: + obj_in_data["category_id"] = None + + # Validate and accumulate subcategory updates + if obj_in_data["subcategory_id"] and obj_in_data["subcategory_id"] in subcategories: + subcategory = subcategories[obj_in_data["subcategory_id"]] + if (subcategory.category_id == obj_in_data["category_id"] and + subcategory.owner_id == owner_id): + subcategory_updates[obj_in_data["subcategory_id"]] = subcategory_updates.get(obj_in_data["subcategory_id"], 0) + obj_in_data["amount"] + else: + obj_in_data["subcategory_id"] = None + else: + obj_in_data["subcategory_id"] = None + + # Validate place + if obj_in_data["place_id"] and obj_in_data["place_id"] not in places: + obj_in_data["place_id"] = None + + total_user_expense += obj_in_data["amount"] + + # Create expense object + db_obj = self.model(**obj_in_data, owner_id=owner_id) + db.add(db_obj) + created_expenses.append(db_obj) + + # Batch update accounts + for account_id, amount in account_updates.items(): + await crud.account.update_by_id_and_field( + db=db, + owner_id=owner_id, + id=account_id, + column="total_expenses", + amount=amount, + ) + + # Batch update categories + for category_id, amount in category_updates.items(): + category = categories[category_id] + await db.execute( + updateDb(category.__class__) + .where(category.__class__.id == category_id) + .values(total=category.total + amount) + .execution_options(synchronize_session="fetch") + ) + + # Batch update subcategories + for subcategory_id, amount in subcategory_updates.items(): + subcategory = subcategories[subcategory_id] + await db.execute( + updateDb(subcategory.__class__) + .where(subcategory.__class__.id == subcategory_id) + .values(total=subcategory.total + amount) + .execution_options(synchronize_session="fetch") + ) + + # Update user balance once + await crud.user.update_balance( + db=db, user_id=owner_id, is_Expense=True, amount=total_user_expense + ) + + # Single commit for all operations + await db.commit() + + # Refresh all created expenses + for expense in created_expenses: + await db.refresh(expense) + return created_expenses async def remove_multi(self, db: AsyncSession, *, ids: list[int]) -> list[Expense]: diff --git a/backend/app/app/crud/crud_income.py b/backend/app/app/crud/crud_income.py index 2be5814..565b32c 100644 --- a/backend/app/app/crud/crud_income.py +++ b/backend/app/app/crud/crud_income.py @@ -54,7 +54,6 @@ async def create_with_owner( .values(total=subcategory.total + obj_in_data["amount"]) .execution_options(synchronize_session="fetch") ) - await db.commit() # Update category total if there's a category associated with the subcategory if subcategory.category_id: @@ -67,7 +66,6 @@ async def create_with_owner( .values(total=category.total + obj_in_data["amount"]) .execution_options(synchronize_session="fetch") ) - await db.commit() if obj_in_data["place_id"]: place = await crud.place.get(db=db, id=obj_in_data["place_id"]) @@ -86,14 +84,143 @@ async def create_with_owner( await db.refresh(db_obj) return db_obj - # refactor this to only update balance once and not for each income async def create_multi_with_owner( self, db: AsyncSession, *, obj_list: list[IncomeCreate], owner_id: int ) -> list[Income]: + if not obj_list: + return [] + + # Pre-fetch and validate all referenced entities + account_ids = [obj.account_id for obj in obj_list if obj.account_id] + subcategory_ids = [obj.subcategory_id for obj in obj_list if obj.subcategory_id] + place_ids = [obj.place_id for obj in obj_list if obj.place_id] + + # Batch fetch entities + accounts = {} + subcategories = {} + categories = {} + places = {} + + if account_ids: + account_results = await db.execute( + select(crud.account.model).filter(crud.account.model.id.in_(account_ids)) + ) + accounts = {acc.id: acc for acc in account_results.scalars().all()} + + if subcategory_ids: + subcategory_results = await db.execute( + select(crud.subcategory.model).filter(crud.subcategory.model.id.in_(subcategory_ids)) + ) + subcategories = {sub.id: sub for sub in subcategory_results.scalars().all()} + + # Fetch categories for subcategories + category_ids = [sub.category_id for sub in subcategories.values() if sub.category_id] + if category_ids: + category_results = await db.execute( + select(crud.category.model).filter(crud.category.model.id.in_(category_ids)) + ) + categories = {cat.id: cat for cat in category_results.scalars().all()} + + if place_ids: + place_results = await db.execute( + select(crud.place.model).filter(crud.place.model.id.in_(place_ids)) + ) + places = {place.id: place for place in place_results.scalars().all()} + + # Process incomes and calculate totals created_incomes = [] + account_updates = {} + subcategory_updates = {} + category_updates = {} + total_user_income = 0 + for obj_in in obj_list: - income = await self.create_with_owner(db, obj_in=obj_in, owner_id=owner_id) - created_incomes.append(income) + obj_in_data = jsonable_encoder(obj_in) + + # Convert date string to datetime object + date_str = obj_in_data["date"] + if date_str: + try: + obj_in_data["date"] = datetime.strptime(date_str, "%Y-%m-%d").date() + except: + obj_in_data["date"] = None + + # Validate and accumulate account updates + if obj_in_data["account_id"] and obj_in_data["account_id"] in accounts: + account = accounts[obj_in_data["account_id"]] + if account.owner_id == owner_id: + account_updates[obj_in_data["account_id"]] = account_updates.get(obj_in_data["account_id"], 0) + obj_in_data["amount"] + else: + obj_in_data["account_id"] = None + else: + obj_in_data["account_id"] = None + + # Validate and accumulate subcategory updates + if obj_in_data["subcategory_id"] and obj_in_data["subcategory_id"] in subcategories: + subcategory = subcategories[obj_in_data["subcategory_id"]] + if subcategory.owner_id == owner_id: + subcategory_updates[obj_in_data["subcategory_id"]] = subcategory_updates.get(obj_in_data["subcategory_id"], 0) + obj_in_data["amount"] + # Also update category if exists + if subcategory.category_id and subcategory.category_id in categories: + category_updates[subcategory.category_id] = category_updates.get(subcategory.category_id, 0) + obj_in_data["amount"] + else: + obj_in_data["subcategory_id"] = None + else: + obj_in_data["subcategory_id"] = None + + # Validate place + if obj_in_data["place_id"] and obj_in_data["place_id"] not in places: + obj_in_data["place_id"] = None + + total_user_income += obj_in_data["amount"] + + # Create income object + db_obj = self.model(**obj_in_data, owner_id=owner_id) + db.add(db_obj) + created_incomes.append(db_obj) + + # Batch update accounts + for account_id, amount in account_updates.items(): + await crud.account.update_by_id_and_field( + db=db, + owner_id=owner_id, + id=account_id, + column="total_incomes", + amount=amount, + ) + + # Batch update subcategories + for subcategory_id, amount in subcategory_updates.items(): + subcategory = subcategories[subcategory_id] + await db.execute( + updateDb(subcategory.__class__) + .where(subcategory.__class__.id == subcategory_id) + .values(total=subcategory.total + amount) + .execution_options(synchronize_session="fetch") + ) + + # Batch update categories + for category_id, amount in category_updates.items(): + category = categories[category_id] + await db.execute( + updateDb(category.__class__) + .where(category.__class__.id == category_id) + .values(total=category.total + amount) + .execution_options(synchronize_session="fetch") + ) + + # Update user balance once + await crud.user.update_balance( + db=db, user_id=owner_id, is_Expense=False, amount=total_user_income + ) + + # Single commit for all operations + await db.commit() + + # Refresh all created incomes + for income in created_incomes: + await db.refresh(income) + return created_incomes async def remove_multi(self, db: AsyncSession, *, ids: list[int]) -> list[Income]: diff --git a/backend/app/app/utilities/redis.py b/backend/app/app/utilities/redis.py index b9ac594..fa912c3 100644 --- a/backend/app/app/utilities/redis.py +++ b/backend/app/app/utilities/redis.py @@ -57,3 +57,68 @@ async def delete_transaction(transaction_id: str): except Exception as e: logging.error(f"Error deleting transaction {transaction_id}: {str(e)}") return False + +# Cache categories and subcategories for 1 hour by default +async def cache_user_categories(user_id: int, categories_data, expire_time=3600): + """Cache user categories and subcategories""" + try: + cache_key = f"user:{user_id}:categories" + await r.setex( + cache_key, + expire_time, + json.dumps(categories_data, cls=DateEncoder) + ) + logging.info(f"Cached categories for user {user_id}") + return True + except Exception as e: + logging.error(f"Error caching categories for user {user_id}: {str(e)}") + return False + +async def get_cached_user_categories(user_id: int): + """Retrieve cached user categories and subcategories""" + try: + cache_key = f"user:{user_id}:categories" + cached_data = await r.get(cache_key) + if cached_data: + return json.loads(cached_data) + return None + except (Exception, json.JSONDecodeError) as e: + logging.error(f"Error retrieving cached categories for user {user_id}: {str(e)}") + return None + +async def invalidate_user_categories_cache(user_id: int): + """Invalidate user categories cache when data changes""" + try: + cache_key = f"user:{user_id}:categories" + await r.delete(cache_key) + logging.info(f"Invalidated categories cache for user {user_id}") + return True + except Exception as e: + logging.error(f"Error invalidating categories cache for user {user_id}: {str(e)}") + return False + +async def cache_category_lookup(category_ids: list[int], categories_data, expire_time=1800): + """Cache category lookup data for batch operations""" + try: + cache_key = f"categories:batch:{':'.join(map(str, sorted(category_ids)))}" + await r.setex( + cache_key, + expire_time, + json.dumps(categories_data, cls=DateEncoder) + ) + return True + except Exception as e: + logging.error(f"Error caching category batch lookup: {str(e)}") + return False + +async def get_cached_category_lookup(category_ids: list[int]): + """Get cached category lookup data""" + try: + cache_key = f"categories:batch:{':'.join(map(str, sorted(category_ids)))}" + cached_data = await r.get(cache_key) + if cached_data: + return json.loads(cached_data) + return None + except (Exception, json.JSONDecodeError) as e: + logging.error(f"Error retrieving cached category batch lookup: {str(e)}") + return None