From 98285aa8aaeda1656629b760929e31a842108a2f Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Tue, 23 Sep 2025 22:42:26 +0200 Subject: [PATCH] Exception handling enhancement --- app/api/deps.py | 12 +- app/api/v1/admin.py | 98 ++- app/api/v1/auth.py | 42 +- app/api/v1/marketplace.py | 139 ++-- app/api/v1/product.py | 219 ++---- app/api/v1/shop.py | 151 ++-- app/api/v1/stats.py | 111 ++- app/api/v1/stock.py | 89 +-- app/exceptions/__init__.py | 41 + app/exceptions/base.py | 169 ++-- app/exceptions/handler.py | 129 +--- app/services/admin_service.py | 328 +++++--- app/services/auth_service.py | 162 ++-- app/services/import_service.py | 0 app/services/marketplace_service.py | 474 ++++++++---- app/services/product_service.py | 498 ++++++++---- app/services/shop_service.py | 411 ++++++---- app/services/stats_service.py | 302 +++++--- app/services/stock_service.py | 719 +++++++++++++----- .../frontend-exception-handling.md | 577 ++++++++++++++ main.py | 9 +- middleware/auth.py | 35 +- middleware/decorators.py | 23 +- middleware/error_handler.py | 69 -- mkdocs.yml | 2 + tests/fixtures/auth_fixtures.py | 16 + .../api/v1/test_admin_endpoints.py | 77 +- .../api/v1/test_authentication_endpoints.py | 44 +- tests/integration/api/v1/test_export.py | 2 +- tests/integration/api/v1/test_filtering.py | 2 +- .../api/v1/test_marketplace_endpoints.py | 2 +- .../api/v1/test_product_endpoints.py | 68 +- .../integration/api/v1/test_shop_endpoints.py | 2 +- .../api/v1/test_stats_endpoints.py | 2 +- .../api/v1/test_stock_endpoints.py | 2 +- 35 files changed, 3283 insertions(+), 1743 deletions(-) delete mode 100644 app/services/import_service.py create mode 100644 docs/development/frontend-exception-handling.md delete mode 100644 middleware/error_handler.py diff --git a/app/api/deps.py b/app/api/deps.py index 66f78584..8b0f9935 100644 --- a/app/api/deps.py +++ b/app/api/deps.py @@ -7,7 +7,7 @@ This module provides classes and functions for: - .... """ -from fastapi import Depends, HTTPException +from fastapi import Depends from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from sqlalchemy.orm import Session @@ -16,6 +16,7 @@ from middleware.auth import AuthManager from middleware.rate_limiter import RateLimiter from models.database.shop import Shop from models.database.user import User +from app.exceptions import (AdminRequiredException,ShopNotFoundException, UnauthorizedShopAccessException) # Set auto_error=False to prevent automatic 403 responses security = HTTPBearer(auto_error=False) @@ -30,11 +31,13 @@ def get_current_user( """Get current authenticated user.""" # Check if credentials are provided if not credentials: - raise HTTPException(status_code=401, detail="Authorization header required") + from app.exceptions.auth import InvalidTokenException + raise InvalidTokenException("Authorization header required") return auth_manager.get_current_user(db, credentials) + def get_current_admin_user(current_user: User = Depends(get_current_user)): """Require admin user.""" return auth_manager.require_admin(current_user) @@ -48,9 +51,10 @@ def get_user_shop( """Get shop and verify user ownership.""" shop = db.query(Shop).filter(Shop.shop_code == shop_code.upper()).first() if not shop: - raise HTTPException(status_code=404, detail="Shop not found") + raise ShopNotFoundException(shop_code) if current_user.role != "admin" and shop.owner_id != current_user.id: - raise HTTPException(status_code=403, detail="Access denied to this shop") + raise UnauthorizedShopAccessException(shop_code, current_user.id) return shop + diff --git a/app/api/v1/admin.py b/app/api/v1/admin.py index 91b72863..6997e914 100644 --- a/app/api/v1/admin.py +++ b/app/api/v1/admin.py @@ -1,16 +1,18 @@ # app/api/v1/admin.py -"""Summary description .... +""" +Admin endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- User management (view, toggle status) +- Shop management (view, verify, toggle status) +- Marketplace import job monitoring +- Admin dashboard statistics """ import logging from typing import List, Optional -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.api.deps import get_current_admin_user @@ -25,7 +27,6 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Admin-only routes @router.get("/admin/users", response_model=List[UserResponse]) def get_all_users( skip: int = Query(0, ge=0), @@ -34,12 +35,8 @@ def get_all_users( current_admin: User = Depends(get_current_admin_user), ): """Get all users (Admin only).""" - try: - users = admin_service.get_all_users(db=db, skip=skip, limit=limit) - return [UserResponse.model_validate(user) for user in users] - except Exception as e: - logger.error(f"Error getting users: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + users = admin_service.get_all_users(db=db, skip=skip, limit=limit) + return [UserResponse.model_validate(user) for user in users] @router.put("/admin/users/{user_id}/status") @@ -49,14 +46,8 @@ def toggle_user_status( current_admin: User = Depends(get_current_admin_user), ): """Toggle user active status (Admin only).""" - try: - user, message = admin_service.toggle_user_status(db, user_id, current_admin.id) - return {"message": message} - except HTTPException: - raise - except Exception as e: - logger.error(f"Error toggling user {user_id} status: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + user, message = admin_service.toggle_user_status(db, user_id, current_admin.id) + return {"message": message} @router.get("/admin/shops", response_model=ShopListResponse) @@ -67,13 +58,8 @@ def get_all_shops_admin( current_admin: User = Depends(get_current_admin_user), ): """Get all shops with admin view (Admin only).""" - try: - shops, total = admin_service.get_all_shops(db=db, skip=skip, limit=limit) - - return ShopListResponse(shops=shops, total=total, skip=skip, limit=limit) - except Exception as e: - logger.error(f"Error getting shops: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + shops, total = admin_service.get_all_shops(db=db, skip=skip, limit=limit) + return ShopListResponse(shops=shops, total=total, skip=skip, limit=limit) @router.put("/admin/shops/{shop_id}/verify") @@ -83,14 +69,8 @@ def verify_shop( current_admin: User = Depends(get_current_admin_user), ): """Verify/unverify shop (Admin only).""" - try: - shop, message = admin_service.verify_shop(db, shop_id) - return {"message": message} - except HTTPException: - raise - except Exception as e: - logger.error(f"Error verifying shop {shop_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + shop, message = admin_service.verify_shop(db, shop_id) + return {"message": message} @router.put("/admin/shops/{shop_id}/status") @@ -100,14 +80,8 @@ def toggle_shop_status( current_admin: User = Depends(get_current_admin_user), ): """Toggle shop active status (Admin only).""" - try: - shop, message = admin_service.toggle_shop_status(db, shop_id) - return {"message": message} - except HTTPException: - raise - except Exception as e: - logger.error(f"Error toggling shop {shop_id} status: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + shop, message = admin_service.toggle_shop_status(db, shop_id) + return {"message": message} @router.get( @@ -123,15 +97,29 @@ def get_all_marketplace_import_jobs( current_admin: User = Depends(get_current_admin_user), ): """Get all marketplace import jobs (Admin only).""" - try: - return admin_service.get_marketplace_import_jobs( - db=db, - marketplace=marketplace, - shop_name=shop_name, - status=status, - skip=skip, - limit=limit, - ) - except Exception as e: - logger.error(f"Error getting marketplace import jobs: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return admin_service.get_marketplace_import_jobs( + db=db, + marketplace=marketplace, + shop_name=shop_name, + status=status, + skip=skip, + limit=limit, + ) + + +@router.get("/admin/stats/users") +def get_user_statistics( + db: Session = Depends(get_db), + current_admin: User = Depends(get_current_admin_user), +): + """Get user statistics for admin dashboard (Admin only).""" + return admin_service.get_user_statistics(db) + + +@router.get("/admin/stats/shops") +def get_shop_statistics( + db: Session = Depends(get_db), + current_admin: User = Depends(get_current_admin_user), +): + """Get shop statistics for admin dashboard (Admin only).""" + return admin_service.get_shop_statistics(db) diff --git a/app/api/v1/auth.py b/app/api/v1/auth.py index 0854e391..e4f3b203 100644 --- a/app/api/v1/auth.py +++ b/app/api/v1/auth.py @@ -1,15 +1,16 @@ # app/api/v1/auth.py -"""Summary description .... +""" +Authentication endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- User registration and validation +- User authentication and JWT token generation +- Current user information retrieval """ import logging -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends from sqlalchemy.orm import Session from app.api.deps import get_current_user @@ -23,37 +24,24 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Authentication Routes @router.post("/auth/register", response_model=UserResponse) def register_user(user_data: UserRegister, db: Session = Depends(get_db)): """Register a new user.""" - try: - user = auth_service.register_user(db=db, user_data=user_data) - return UserResponse.model_validate(user) - except HTTPException: - raise - except Exception as e: - logger.error(f"Error registering user: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + user = auth_service.register_user(db=db, user_data=user_data) + return UserResponse.model_validate(user) @router.post("/auth/login", response_model=LoginResponse) def login_user(user_credentials: UserLogin, db: Session = Depends(get_db)): """Login user and return JWT token.""" - try: - login_result = auth_service.login_user(db=db, user_credentials=user_credentials) + login_result = auth_service.login_user(db=db, user_credentials=user_credentials) - return LoginResponse( - access_token=login_result["token_data"]["access_token"], - token_type=login_result["token_data"]["token_type"], - expires_in=login_result["token_data"]["expires_in"], - user=UserResponse.model_validate(login_result["user"]), - ) - except HTTPException: - raise - except Exception as e: - logger.error(f"Error logging in user: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return LoginResponse( + access_token=login_result["token_data"]["access_token"], + token_type=login_result["token_data"]["token_type"], + expires_in=login_result["token_data"]["expires_in"], + user=UserResponse.model_validate(login_result["user"]), + ) @router.get("/auth/me", response_model=UserResponse) diff --git a/app/api/v1/marketplace.py b/app/api/v1/marketplace.py index 66049833..4d7f0686 100644 --- a/app/api/v1/marketplace.py +++ b/app/api/v1/marketplace.py @@ -1,16 +1,17 @@ # app/api/v1/marketplace.py -"""Summary description .... +""" +Marketplace endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- Product import from marketplace CSV files +- Import job management and monitoring +- Import statistics and job cancellation """ import logging from typing import List, Optional -from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Query +from fastapi import APIRouter, BackgroundTasks, Depends, Query from sqlalchemy.orm import Session from app.api.deps import get_current_user @@ -26,7 +27,6 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Marketplace Import Routes (Protected) @router.post("/marketplace/import-product", response_model=MarketplaceImportJobResponse) @rate_limit(max_requests=10, window_seconds=3600) # Limit marketplace imports async def import_products_from_marketplace( @@ -36,42 +36,33 @@ async def import_products_from_marketplace( current_user: User = Depends(get_current_user), ): """Import products from marketplace CSV with background processing (Protected).""" - try: - logger.info( - f"Starting marketplace import: {request.marketplace} -> {request.shop_code} by user {current_user.username}" - ) + logger.info( + f"Starting marketplace import: {request.marketplace} -> {request.shop_code} by user {current_user.username}" + ) - # Create import job through service - import_job = marketplace_service.create_import_job(db, request, current_user) + # Create import job through service + import_job = marketplace_service.create_import_job(db, request, current_user) - # Process in background - background_tasks.add_task( - process_marketplace_import, - import_job.id, - request.url, - request.marketplace, - request.shop_code, - request.batch_size or 1000, - ) + # Process in background + background_tasks.add_task( + process_marketplace_import, + import_job.id, + request.url, + request.marketplace, + request.shop_code, + request.batch_size or 1000, + ) - return MarketplaceImportJobResponse( - job_id=import_job.id, - status="pending", - marketplace=request.marketplace, - shop_code=request.shop_code, - shop_id=import_job.shop_id, - shop_name=import_job.shop_name, - message=f"Marketplace import started from {request.marketplace}. Check status with " - f"/import-status/{import_job.id}", - ) - - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except PermissionError as e: - raise HTTPException(status_code=403, detail=str(e)) - except Exception as e: - logger.error(f"Error starting marketplace import: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return MarketplaceImportJobResponse( + job_id=import_job.id, + status="pending", + marketplace=request.marketplace, + shop_code=request.shop_code, + shop_id=import_job.shop_id, + shop_name=import_job.shop_name, + message=f"Marketplace import started from {request.marketplace}. Check status with " + f"/import-status/{import_job.id}", + ) @router.get( @@ -83,17 +74,8 @@ def get_marketplace_import_status( current_user: User = Depends(get_current_user), ): """Get status of marketplace import job (Protected).""" - try: - job = marketplace_service.get_import_job_by_id(db, job_id, current_user) - return marketplace_service.convert_to_response_model(job) - - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except PermissionError as e: - raise HTTPException(status_code=403, detail=str(e)) - except Exception as e: - logger.error(f"Error getting import job status {job_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + job = marketplace_service.get_import_job_by_id(db, job_id, current_user) + return marketplace_service.convert_to_response_model(job) @router.get( @@ -108,21 +90,16 @@ def get_marketplace_import_jobs( current_user: User = Depends(get_current_user), ): """Get marketplace import jobs with filtering (Protected).""" - try: - jobs = marketplace_service.get_import_jobs( - db=db, - user=current_user, - marketplace=marketplace, - shop_name=shop_name, - skip=skip, - limit=limit, - ) + jobs = marketplace_service.get_import_jobs( + db=db, + user=current_user, + marketplace=marketplace, + shop_name=shop_name, + skip=skip, + limit=limit, + ) - return [marketplace_service.convert_to_response_model(job) for job in jobs] - - except Exception as e: - logger.error(f"Error getting import jobs: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return [marketplace_service.convert_to_response_model(job) for job in jobs] @router.get("/marketplace/marketplace-import-stats") @@ -130,13 +107,7 @@ def get_marketplace_import_stats( db: Session = Depends(get_db), current_user: User = Depends(get_current_user) ): """Get statistics about marketplace import jobs (Protected).""" - try: - stats = marketplace_service.get_job_stats(db, current_user) - return stats - - except Exception as e: - logger.error(f"Error getting import stats: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return marketplace_service.get_job_stats(db, current_user) @router.put( @@ -149,17 +120,8 @@ def cancel_marketplace_import_job( current_user: User = Depends(get_current_user), ): """Cancel a pending or running marketplace import job (Protected).""" - try: - job = marketplace_service.cancel_import_job(db, job_id, current_user) - return marketplace_service.convert_to_response_model(job) - - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except PermissionError as e: - raise HTTPException(status_code=403, detail=str(e)) - except Exception as e: - logger.error(f"Error cancelling import job {job_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + job = marketplace_service.cancel_import_job(db, job_id, current_user) + return marketplace_service.convert_to_response_model(job) @router.delete("/marketplace/import-jobs/{job_id}") @@ -169,14 +131,5 @@ def delete_marketplace_import_job( current_user: User = Depends(get_current_user), ): """Delete a completed marketplace import job (Protected).""" - try: - marketplace_service.delete_import_job(db, job_id, current_user) - return {"message": "Marketplace import job deleted successfully"} - - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except PermissionError as e: - raise HTTPException(status_code=403, detail=str(e)) - except Exception as e: - logger.error(f"Error deleting import job {job_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + marketplace_service.delete_import_job(db, job_id, current_user) + return {"message": "Marketplace import job deleted successfully"} diff --git a/app/api/v1/product.py b/app/api/v1/product.py index 447354ca..034d55f7 100644 --- a/app/api/v1/product.py +++ b/app/api/v1/product.py @@ -1,16 +1,17 @@ # app/api/v1/product.py -"""Summary description .... +""" +Product endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- Product CRUD operations with marketplace support +- Advanced product filtering and search +- Product export functionality """ import logging from typing import Optional -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from fastapi.responses import StreamingResponse from sqlalchemy.orm import Session @@ -26,183 +27,115 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Enhanced Product Routes with Marketplace Support @router.get("/product", response_model=ProductListResponse) def get_products( - skip: int = Query(0, ge=0), - limit: int = Query(100, ge=1, le=1000), - brand: Optional[str] = Query(None), - category: Optional[str] = Query(None), - availability: Optional[str] = Query(None), - marketplace: Optional[str] = Query(None, description="Filter by marketplace"), - shop_name: Optional[str] = Query(None, description="Filter by shop name"), - search: Optional[str] = Query(None), - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + skip: int = Query(0, ge=0), + limit: int = Query(100, ge=1, le=1000), + brand: Optional[str] = Query(None), + category: Optional[str] = Query(None), + availability: Optional[str] = Query(None), + marketplace: Optional[str] = Query(None, description="Filter by marketplace"), + shop_name: Optional[str] = Query(None, description="Filter by shop name"), + search: Optional[str] = Query(None), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Get products with advanced filtering including marketplace and shop (Protected).""" - try: - products, total = product_service.get_products_with_filters( - db=db, - skip=skip, - limit=limit, - brand=brand, - category=category, - availability=availability, - marketplace=marketplace, - shop_name=shop_name, - search=search, - ) + products, total = product_service.get_products_with_filters( + db=db, + skip=skip, + limit=limit, + brand=brand, + category=category, + availability=availability, + marketplace=marketplace, + shop_name=shop_name, + search=search, + ) - return ProductListResponse( - products=products, total=total, skip=skip, limit=limit - ) - except Exception as e: - logger.error(f"Error getting products: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return ProductListResponse( + products=products, total=total, skip=skip, limit=limit + ) @router.post("/product", response_model=ProductResponse) def create_product( - product: ProductCreate, - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + product: ProductCreate, + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Create a new product with validation and marketplace support (Protected).""" - try: - logger.info(f"Starting product creation for ID: {product.product_id}") + logger.info(f"Starting product creation for ID: {product.product_id}") - # Check if product_id already exists - logger.info("Checking for existing product...") - existing = product_service.get_product_by_id(db, product.product_id) - logger.info(f"Existing product found: {existing is not None}") + db_product = product_service.create_product(db, product) + logger.info("Product created successfully") - if existing: - logger.info("Product already exists, raising 400 error") - raise HTTPException( - status_code=400, detail="Product with this ID already exists" - ) - - logger.info("No existing product found, proceeding to create...") - db_product = product_service.create_product(db, product) - logger.info("Product created successfully") - - return db_product - - except HTTPException as he: - logger.info(f"HTTPException raised: {he.status_code} - {he.detail}") - raise # Re-raise HTTP exceptions - except ValueError as e: - logger.error(f"ValueError: {str(e)}") - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Unexpected error: {str(e)}", exc_info=True) - raise HTTPException(status_code=500, detail="Internal server error") + return db_product @router.get("/product/{product_id}", response_model=ProductDetailResponse) def get_product( - product_id: str, - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + product_id: str, + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Get product with stock information (Protected).""" - try: - product = product_service.get_product_by_id(db, product_id) - if not product: - raise HTTPException(status_code=404, detail="Product not found") + product = product_service.get_product_by_id_or_raise(db, product_id) - # Get stock information if GTIN exists - stock_info = None - if product.gtin: - stock_info = product_service.get_stock_info(db, product.gtin) + # Get stock information if GTIN exists + stock_info = None + if product.gtin: + stock_info = product_service.get_stock_info(db, product.gtin) - return ProductDetailResponse(product=product, stock_info=stock_info) - - except HTTPException: - raise - except Exception as e: - logger.error(f"Error getting product {product_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return ProductDetailResponse(product=product, stock_info=stock_info) @router.put("/product/{product_id}", response_model=ProductResponse) def update_product( - product_id: str, - product_update: ProductUpdate, - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + product_id: str, + product_update: ProductUpdate, + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Update product with validation and marketplace support (Protected).""" - try: - product = product_service.get_product_by_id(db, product_id) - if not product: - raise HTTPException(status_code=404, detail="Product not found") - - updated_product = product_service.update_product(db, product_id, product_update) - return updated_product - - except HTTPException: - raise - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error updating product {product_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + updated_product = product_service.update_product(db, product_id, product_update) + return updated_product @router.delete("/product/{product_id}") def delete_product( - product_id: str, - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + product_id: str, + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Delete product and associated stock (Protected).""" - try: - product = product_service.get_product_by_id(db, product_id) - if not product: - raise HTTPException(status_code=404, detail="Product not found") - - product_service.delete_product(db, product_id) - - return {"message": "Product and associated stock deleted successfully"} - - except HTTPException: - raise - except Exception as e: - logger.error(f"Error deleting product {product_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + product_service.delete_product(db, product_id) + return {"message": "Product and associated stock deleted successfully"} -# Export with streaming for large datasets (Protected) @router.get("/export-csv") async def export_csv( - marketplace: Optional[str] = Query(None, description="Filter by marketplace"), - shop_name: Optional[str] = Query(None, description="Filter by shop name"), - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), + marketplace: Optional[str] = Query(None, description="Filter by marketplace"), + shop_name: Optional[str] = Query(None, description="Filter by shop name"), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), ): """Export products as CSV with streaming and marketplace filtering (Protected).""" - try: - def generate_csv(): - return product_service.generate_csv_export( - db=db, marketplace=marketplace, shop_name=shop_name - ) - - filename = "products_export" - if marketplace: - filename += f"_{marketplace}" - if shop_name: - filename += f"_{shop_name}" - filename += ".csv" - - return StreamingResponse( - generate_csv(), - media_type="text/csv", - headers={"Content-Disposition": f"attachment; filename={filename}"}, + def generate_csv(): + return product_service.generate_csv_export( + db=db, marketplace=marketplace, shop_name=shop_name ) - except Exception as e: - logger.error(f"Error exporting CSV: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + filename = "products_export" + if marketplace: + filename += f"_{marketplace}" + if shop_name: + filename += f"_{shop_name}" + filename += ".csv" + + return StreamingResponse( + generate_csv(), + media_type="text/csv", + headers={"Content-Disposition": f"attachment; filename={filename}"}, + ) diff --git a/app/api/v1/shop.py b/app/api/v1/shop.py index 62f27dc3..6ad94a40 100644 --- a/app/api/v1/shop.py +++ b/app/api/v1/shop.py @@ -1,15 +1,16 @@ # app/api/v1/shop.py -"""Summary description .... +""" +Shop endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- Shop CRUD operations and management +- Shop product catalog management +- Shop filtering and verification """ import logging -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.api.deps import get_current_user, get_user_shop @@ -23,7 +24,6 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Shop Management Routes @router.post("/shop", response_model=ShopResponse) def create_shop( shop_data: ShopCreate, @@ -31,16 +31,10 @@ def create_shop( current_user: User = Depends(get_current_user), ): """Create a new shop (Protected).""" - try: - shop = shop_service.create_shop( - db=db, shop_data=shop_data, current_user=current_user - ) - return ShopResponse.model_validate(shop) - except HTTPException: - raise - except Exception as e: - logger.error(f"Error creating shop: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + shop = shop_service.create_shop( + db=db, shop_data=shop_data, current_user=current_user + ) + return ShopResponse.model_validate(shop) @router.get("/shop", response_model=ShopListResponse) @@ -53,22 +47,16 @@ def get_shops( current_user: User = Depends(get_current_user), ): """Get shops with filtering (Protected).""" - try: - shops, total = shop_service.get_shops( - db=db, - current_user=current_user, - skip=skip, - limit=limit, - active_only=active_only, - verified_only=verified_only, - ) + shops, total = shop_service.get_shops( + db=db, + current_user=current_user, + skip=skip, + limit=limit, + active_only=active_only, + verified_only=verified_only, + ) - return ShopListResponse(shops=shops, total=total, skip=skip, limit=limit) - except HTTPException: - raise - except Exception as e: - logger.error(f"Error getting shops: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return ShopListResponse(shops=shops, total=total, skip=skip, limit=limit) @router.get("/shop/{shop_code}", response_model=ShopResponse) @@ -78,19 +66,12 @@ def get_shop( current_user: User = Depends(get_current_user), ): """Get shop details (Protected).""" - try: - shop = shop_service.get_shop_by_code( - db=db, shop_code=shop_code, current_user=current_user - ) - return ShopResponse.model_validate(shop) - except HTTPException: - raise - except Exception as e: - logger.error(f"Error getting shop {shop_code}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + shop = shop_service.get_shop_by_code( + db=db, shop_code=shop_code, current_user=current_user + ) + return ShopResponse.model_validate(shop) -# Shop Product Management @router.post("/shop/{shop_code}/products", response_model=ShopProductResponse) def add_product_to_shop( shop_code: str, @@ -99,24 +80,18 @@ def add_product_to_shop( current_user: User = Depends(get_current_user), ): """Add existing product to shop catalog with shop-specific settings (Protected).""" - try: - # Get and verify shop (using existing dependency) - shop = get_user_shop(shop_code, current_user, db) + # Get and verify shop (using existing dependency) + shop = get_user_shop(shop_code, current_user, db) - # Add product to shop - new_shop_product = shop_service.add_product_to_shop( - db=db, shop=shop, shop_product=shop_product - ) + # Add product to shop + new_shop_product = shop_service.add_product_to_shop( + db=db, shop=shop, shop_product=shop_product + ) - # Return with product details - response = ShopProductResponse.model_validate(new_shop_product) - response.product = new_shop_product.product - return response - except HTTPException: - raise - except Exception as e: - logger.error(f"Error adding product to shop {shop_code}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + # Return with product details + response = ShopProductResponse.model_validate(new_shop_product) + response.product = new_shop_product.product + return response @router.get("/shop/{shop_code}/products") @@ -130,39 +105,33 @@ def get_shop_products( current_user: User = Depends(get_current_user), ): """Get products in shop catalog (Protected).""" - try: - # Get shop - shop = shop_service.get_shop_by_code( - db=db, shop_code=shop_code, current_user=current_user - ) + # Get shop + shop = shop_service.get_shop_by_code( + db=db, shop_code=shop_code, current_user=current_user + ) - # Get shop products - shop_products, total = shop_service.get_shop_products( - db=db, - shop=shop, - current_user=current_user, - skip=skip, - limit=limit, - active_only=active_only, - featured_only=featured_only, - ) + # Get shop products + shop_products, total = shop_service.get_shop_products( + db=db, + shop=shop, + current_user=current_user, + skip=skip, + limit=limit, + active_only=active_only, + featured_only=featured_only, + ) - # Format response - products = [] - for sp in shop_products: - product_response = ShopProductResponse.model_validate(sp) - product_response.product = sp.product - products.append(product_response) + # Format response + products = [] + for sp in shop_products: + product_response = ShopProductResponse.model_validate(sp) + product_response.product = sp.product + products.append(product_response) - return { - "products": products, - "total": total, - "skip": skip, - "limit": limit, - "shop": ShopResponse.model_validate(shop), - } - except HTTPException: - raise - except Exception as e: - logger.error(f"Error getting products for shop {shop_code}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return { + "products": products, + "total": total, + "skip": skip, + "limit": limit, + "shop": ShopResponse.model_validate(shop), + } diff --git a/app/api/v1/stats.py b/app/api/v1/stats.py index 361ba149..e3709c4b 100644 --- a/app/api/v1/stats.py +++ b/app/api/v1/stats.py @@ -1,16 +1,17 @@ # app/api/v1/stats.py -"""Summary description .... +""" +Statistics endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- Comprehensive system statistics +- Marketplace-specific analytics +- Performance metrics and data insights """ import logging from typing import List -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends from sqlalchemy.orm import Session from app.api.deps import get_current_user @@ -23,27 +24,22 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Enhanced Statistics with Marketplace Support @router.get("/stats", response_model=StatsResponse) def get_stats( db: Session = Depends(get_db), current_user: User = Depends(get_current_user) ): """Get comprehensive statistics with marketplace data (Protected).""" - try: - stats_data = stats_service.get_comprehensive_stats(db=db) + stats_data = stats_service.get_comprehensive_stats(db=db) - return StatsResponse( - total_products=stats_data["total_products"], - unique_brands=stats_data["unique_brands"], - unique_categories=stats_data["unique_categories"], - unique_marketplaces=stats_data["unique_marketplaces"], - unique_shops=stats_data["unique_shops"], - total_stock_entries=stats_data["total_stock_entries"], - total_inventory_quantity=stats_data["total_inventory_quantity"], - ) - except Exception as e: - logger.error(f"Error getting comprehensive stats: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return StatsResponse( + total_products=stats_data["total_products"], + unique_brands=stats_data["unique_brands"], + unique_categories=stats_data["unique_categories"], + unique_marketplaces=stats_data["unique_marketplaces"], + unique_shops=stats_data["unique_shops"], + total_stock_entries=stats_data["total_stock_entries"], + total_inventory_quantity=stats_data["total_inventory_quantity"], + ) @router.get("/stats/marketplace", response_model=List[MarketplaceStatsResponse]) @@ -51,18 +47,65 @@ def get_marketplace_stats( db: Session = Depends(get_db), current_user: User = Depends(get_current_user) ): """Get statistics broken down by marketplace (Protected).""" - try: - marketplace_stats = stats_service.get_marketplace_breakdown_stats(db=db) + marketplace_stats = stats_service.get_marketplace_breakdown_stats(db=db) - return [ - MarketplaceStatsResponse( - marketplace=stat["marketplace"], - total_products=stat["total_products"], - unique_shops=stat["unique_shops"], - unique_brands=stat["unique_brands"], - ) - for stat in marketplace_stats - ] - except Exception as e: - logger.error(f"Error getting marketplace stats: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") +# app/api/v1/stats.py +""" +Statistics endpoints - simplified with service-level exception handling. + +This module provides classes and functions for: +- Comprehensive system statistics +- Marketplace-specific analytics +- Performance metrics and data insights +""" + +import logging +from typing import List + +from fastapi import APIRouter, Depends +from sqlalchemy.orm import Session + +from app.api.deps import get_current_user +from app.core.database import get_db +from app.services.stats_service import stats_service +from models.schemas.stats import MarketplaceStatsResponse, StatsResponse +from models.database.user import User + +router = APIRouter() +logger = logging.getLogger(__name__) + + +@router.get("/stats", response_model=StatsResponse) +def get_stats( + db: Session = Depends(get_db), current_user: User = Depends(get_current_user) +): + """Get comprehensive statistics with marketplace data (Protected).""" + stats_data = stats_service.get_comprehensive_stats(db=db) + + return StatsResponse( + total_products=stats_data["total_products"], + unique_brands=stats_data["unique_brands"], + unique_categories=stats_data["unique_categories"], + unique_marketplaces=stats_data["unique_marketplaces"], + unique_shops=stats_data["unique_shops"], + total_stock_entries=stats_data["total_stock_entries"], + total_inventory_quantity=stats_data["total_inventory_quantity"], + ) + + +@router.get("/stats/marketplace", response_model=List[MarketplaceStatsResponse]) +def get_marketplace_stats( + db: Session = Depends(get_db), current_user: User = Depends(get_current_user) +): + """Get statistics broken down by marketplace (Protected).""" + marketplace_stats = stats_service.get_marketplace_breakdown_stats(db=db) + + return [ + MarketplaceStatsResponse( + marketplace=stat["marketplace"], + total_products=stat["total_products"], + unique_shops=stat["unique_shops"], + unique_brands=stat["unique_brands"], + ) + for stat in marketplace_stats + ] diff --git a/app/api/v1/stock.py b/app/api/v1/stock.py index 2619fe23..fbe530d1 100644 --- a/app/api/v1/stock.py +++ b/app/api/v1/stock.py @@ -1,16 +1,17 @@ # app/api/v1/stock.py -"""Summary description .... +""" +Stock endpoints - simplified with service-level exception handling. This module provides classes and functions for: -- .... -- .... -- .... +- Stock quantity management (set, add, remove) +- Stock information retrieval and filtering +- Location-based stock tracking """ import logging from typing import List, Optional -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.api.deps import get_current_user @@ -24,9 +25,6 @@ router = APIRouter() logger = logging.getLogger(__name__) -# Stock Management Routes (Protected) - - @router.post("/stock", response_model=StockResponse) def set_stock( stock: StockCreate, @@ -34,14 +32,7 @@ def set_stock( current_user: User = Depends(get_current_user), ): """Set exact stock quantity for a GTIN at a specific location (replaces existing quantity).""" - try: - result = stock_service.set_stock(db, stock) - return result - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error setting stock: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.set_stock(db, stock) @router.post("/stock/add", response_model=StockResponse) @@ -51,14 +42,7 @@ def add_stock( current_user: User = Depends(get_current_user), ): """Add quantity to existing stock for a GTIN at a specific location (adds to existing quantity).""" - try: - result = stock_service.add_stock(db, stock) - return result - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error adding stock: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.add_stock(db, stock) @router.post("/stock/remove", response_model=StockResponse) @@ -68,14 +52,7 @@ def remove_stock( current_user: User = Depends(get_current_user), ): """Remove quantity from existing stock for a GTIN at a specific location.""" - try: - result = stock_service.remove_stock(db, stock) - return result - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error removing stock: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.remove_stock(db, stock) @router.get("/stock/{gtin}", response_model=StockSummaryResponse) @@ -85,14 +62,7 @@ def get_stock_by_gtin( current_user: User = Depends(get_current_user), ): """Get all stock locations and total quantity for a specific GTIN.""" - try: - result = stock_service.get_stock_by_gtin(db, gtin) - return result - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except Exception as e: - logger.error(f"Error getting stock for GTIN {gtin}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.get_stock_by_gtin(db, gtin) @router.get("/stock/{gtin}/total") @@ -102,14 +72,7 @@ def get_total_stock( current_user: User = Depends(get_current_user), ): """Get total quantity in stock for a specific GTIN.""" - try: - result = stock_service.get_total_stock(db, gtin) - return result - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error getting total stock for GTIN {gtin}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.get_total_stock(db, gtin) @router.get("/stock", response_model=List[StockResponse]) @@ -122,14 +85,9 @@ def get_all_stock( current_user: User = Depends(get_current_user), ): """Get all stock entries with optional filtering.""" - try: - result = stock_service.get_all_stock( - db=db, skip=skip, limit=limit, location=location, gtin=gtin - ) - return result - except Exception as e: - logger.error(f"Error getting all stock: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.get_all_stock( + db=db, skip=skip, limit=limit, location=location, gtin=gtin + ) @router.put("/stock/{stock_id}", response_model=StockResponse) @@ -140,14 +98,7 @@ def update_stock( current_user: User = Depends(get_current_user), ): """Update stock quantity for a specific stock entry.""" - try: - result = stock_service.update_stock(db, stock_id, stock_update) - return result - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except Exception as e: - logger.error(f"Error updating stock {stock_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + return stock_service.update_stock(db, stock_id, stock_update) @router.delete("/stock/{stock_id}") @@ -157,11 +108,5 @@ def delete_stock( current_user: User = Depends(get_current_user), ): """Delete a stock entry.""" - try: - stock_service.delete_stock(db, stock_id) - return {"message": "Stock entry deleted successfully"} - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except Exception as e: - logger.error(f"Error deleting stock {stock_id}: {str(e)}") - raise HTTPException(status_code=500, detail="Internal server error") + stock_service.delete_stock(db, stock_id) + return {"message": "Stock entry deleted successfully"} diff --git a/app/exceptions/__init__.py b/app/exceptions/__init__.py index f41eae22..4b2160dd 100644 --- a/app/exceptions/__init__.py +++ b/app/exceptions/__init__.py @@ -16,14 +16,17 @@ from .base import ( BusinessLogicException, ExternalServiceException, RateLimitException, + ServiceUnavailableException, ) from .auth import ( InvalidCredentialsException, TokenExpiredException, + InvalidTokenException, InsufficientPermissionsException, UserNotActiveException, AdminRequiredException, + UserAlreadyExistsException ) from .product import ( @@ -31,6 +34,8 @@ from .product import ( ProductAlreadyExistsException, InvalidProductDataException, ProductValidationException, + InvalidGTINException, + ProductCSVImportException, ) from .stock import ( @@ -38,6 +43,9 @@ from .stock import ( InsufficientStockException, InvalidStockOperationException, StockValidationException, + NegativeStockException, + InvalidQuantityException, + LocationNotFoundException ) from .shop import ( @@ -47,6 +55,10 @@ from .shop import ( ShopNotVerifiedException, UnauthorizedShopAccessException, InvalidShopDataException, + ShopProductAlreadyExistsException, + ShopProductNotFoundException, + MaxShopsReachedException, + ShopValidationException, ) from .marketplace import ( @@ -56,6 +68,11 @@ from .marketplace import ( InvalidImportDataException, ImportJobCannotBeCancelledException, ImportJobCannotBeDeletedException, + MarketplaceConnectionException, + MarketplaceDataParsingException, + ImportRateLimitException, + InvalidMarketplaceException, + ImportJobAlreadyProcessingException, ) from .admin import ( @@ -63,6 +80,10 @@ from .admin import ( UserStatusChangeException, ShopVerificationException, AdminOperationException, + CannotModifyAdminException, + CannotModifySelfException, + InvalidAdminActionException, + BulkOperationException, ) __all__ = [ @@ -80,21 +101,28 @@ __all__ = [ # Auth exceptions "InvalidCredentialsException", "TokenExpiredException", + "InvalidTokenException", "InsufficientPermissionsException", "UserNotActiveException", "AdminRequiredException", + "UserAlreadyExistsException", # Product exceptions "ProductNotFoundException", "ProductAlreadyExistsException", "InvalidProductDataException", "ProductValidationException", + "InvalidGTINException", + "ProductCSVImportException", # Stock exceptions "StockNotFoundException", "InsufficientStockException", "InvalidStockOperationException", "StockValidationException", + "NegativeStockException", + "InvalidQuantityException", + "LocationNotFoundException", # Shop exceptions "ShopNotFoundException", @@ -103,6 +131,10 @@ __all__ = [ "ShopNotVerifiedException", "UnauthorizedShopAccessException", "InvalidShopDataException", + "ShopProductAlreadyExistsException", + "ShopProductNotFoundException", + "MaxShopsReachedException", + "ShopValidationException", # Marketplace exceptions "MarketplaceImportException", @@ -111,10 +143,19 @@ __all__ = [ "InvalidImportDataException", "ImportJobCannotBeCancelledException", "ImportJobCannotBeDeletedException", + "MarketplaceConnectionException", + "MarketplaceDataParsingException", + "ImportRateLimitException", + "InvalidMarketplaceException", + "ImportJobAlreadyProcessingException", # Admin exceptions "UserNotFoundException", "UserStatusChangeException", "ShopVerificationException", "AdminOperationException", + "CannotModifyAdminException", + "CannotModifySelfException", + "InvalidAdminActionException", + "BulkOperationException", ] diff --git a/app/exceptions/base.py b/app/exceptions/base.py index f09307ce..050c810f 100644 --- a/app/exceptions/base.py +++ b/app/exceptions/base.py @@ -1,37 +1,30 @@ # app/exceptions/base.py """ -Base exception classes for the LetzShop API. +Base exception classes for the LetzShop application. -Provides consistent error structure for frontend consumption with: -- Error codes for programmatic handling -- User-friendly messages -- HTTP status code mapping -- Additional context for debugging +This module provides classes and functions for: +- Base exception class with consistent error formatting +- Common exception types for different error categories +- Standardized error response structure """ from typing import Any, Dict, Optional class LetzShopException(Exception): - """ - Base exception for all LetzShop API errors. - - Provides consistent structure for frontend error handling. - """ + """Base exception class for all LetzShop custom exceptions.""" def __init__( - self, - message: str, - error_code: str, - status_code: int = 500, - details: Optional[Dict[str, Any]] = None, - field: Optional[str] = None, + self, + message: str, + error_code: str, + status_code: int = 400, + details: Optional[Dict[str, Any]] = None, ): self.message = message self.error_code = error_code self.status_code = status_code self.details = details or {} - self.field = field super().__init__(self.message) def to_dict(self) -> Dict[str, Any]: @@ -41,42 +34,44 @@ class LetzShopException(Exception): "message": self.message, "status_code": self.status_code, } - - if self.field: - result["field"] = self.field - if self.details: result["details"] = self.details - return result + + class ValidationException(LetzShopException): - """Raised when input validation fails.""" + """Raised when request validation fails.""" def __init__( - self, - message: str = "Validation failed", - field: Optional[str] = None, - details: Optional[Dict[str, Any]] = None, + self, + message: str, + field: Optional[str] = None, + details: Optional[Dict[str, Any]] = None, ): + validation_details = details or {} + if field: + validation_details["field"] = field + super().__init__( message=message, error_code="VALIDATION_ERROR", - status_code=400, - details=details, - field=field, + status_code=422, + details=validation_details, ) + + class AuthenticationException(LetzShopException): """Raised when authentication fails.""" def __init__( - self, - message: str = "Authentication failed", - error_code: str = "AUTHENTICATION_FAILED", - details: Optional[Dict[str, Any]] = None, + self, + message: str = "Authentication failed", + error_code: str = "AUTHENTICATION_FAILED", + details: Optional[Dict[str, Any]] = None, ): super().__init__( message=message, @@ -87,13 +82,13 @@ class AuthenticationException(LetzShopException): class AuthorizationException(LetzShopException): - """Raised when user lacks required permissions.""" + """Raised when user lacks permission for an operation.""" def __init__( - self, - message: str = "Access denied", - error_code: str = "ACCESS_DENIED", - details: Optional[Dict[str, Any]] = None, + self, + message: str = "Access denied", + error_code: str = "AUTHORIZATION_FAILED", + details: Optional[Dict[str, Any]] = None, ): super().__init__( message=message, @@ -102,20 +97,18 @@ class AuthorizationException(LetzShopException): details=details, ) - class ResourceNotFoundException(LetzShopException): """Raised when a requested resource is not found.""" def __init__( - self, - resource_type: str, - identifier: str, - message: Optional[str] = None, - error_code: Optional[str] = None, + self, + resource_type: str, + identifier: str, + message: Optional[str] = None, + error_code: Optional[str] = None, ): if not message: - message = f"{resource_type} not found" - + message = f"{resource_type} with identifier '{identifier}' not found" if not error_code: error_code = f"{resource_type.upper()}_NOT_FOUND" @@ -123,18 +116,20 @@ class ResourceNotFoundException(LetzShopException): message=message, error_code=error_code, status_code=404, - details={"resource_type": resource_type, "identifier": identifier}, + details={ + "resource_type": resource_type, + "identifier": identifier, + }, ) - class ConflictException(LetzShopException): - """Raised when a resource conflict occurs (e.g., duplicate).""" + """Raised when a resource conflict occurs.""" def __init__( - self, - message: str, - error_code: str, - details: Optional[Dict[str, Any]] = None, + self, + message: str, + error_code: str = "RESOURCE_CONFLICT", + details: Optional[Dict[str, Any]] = None, ): super().__init__( message=message, @@ -143,43 +138,41 @@ class ConflictException(LetzShopException): details=details, ) - class BusinessLogicException(LetzShopException): - """Raised when business logic validation fails.""" + """Raised when business logic rules are violated.""" def __init__( - self, - message: str, - error_code: str, - details: Optional[Dict[str, Any]] = None, + self, + message: str, + error_code: str, + details: Optional[Dict[str, Any]] = None, ): super().__init__( message=message, error_code=error_code, - status_code=422, + status_code=400, details=details, ) class ExternalServiceException(LetzShopException): - """Raised when external service calls fail.""" + """Raised when an external service fails.""" def __init__( - self, - service: str, - message: str = "External service unavailable", - error_code: str = "EXTERNAL_SERVICE_ERROR", - details: Optional[Dict[str, Any]] = None, + self, + message: str, + service_name: str, + error_code: str = "EXTERNAL_SERVICE_ERROR", + details: Optional[Dict[str, Any]] = None, ): - if not details: - details = {} - details["service"] = service + service_details = details or {} + service_details["service_name"] = service_name super().__init__( message=message, error_code=error_code, - status_code=503, - details=details, + status_code=502, + details=service_details, ) @@ -187,20 +180,32 @@ class RateLimitException(LetzShopException): """Raised when rate limit is exceeded.""" def __init__( - self, - message: str = "Rate limit exceeded", - retry_after: Optional[int] = None, - details: Optional[Dict[str, Any]] = None, + self, + message: str = "Rate limit exceeded", + retry_after: Optional[int] = None, + details: Optional[Dict[str, Any]] = None, ): - if not details: - details = {} - + rate_limit_details = details or {} if retry_after: - details["retry_after"] = retry_after + rate_limit_details["retry_after"] = retry_after super().__init__( message=message, error_code="RATE_LIMIT_EXCEEDED", status_code=429, - details=details, + details=rate_limit_details, ) + +class ServiceUnavailableException(LetzShopException): + """Raised when service is unavailable.""" + + def __init__(self, message: str = "Service temporarily unavailable"): + super().__init__( + message=message, + error_code="SERVICE_UNAVAILABLE", + status_code=503, + ) + +# Note: Domain-specific exceptions like ShopNotFoundException, UserNotFoundException, etc. +# are defined in their respective domain modules (shop.py, admin.py, etc.) +# to keep domain-specific logic separate from base exceptions. diff --git a/app/exceptions/handler.py b/app/exceptions/handler.py index 1ca0848a..7a6ca7ad 100644 --- a/app/exceptions/handler.py +++ b/app/exceptions/handler.py @@ -3,113 +3,24 @@ Exception handler for FastAPI application. Provides consistent error responses and logging for all custom exceptions. +This module provides classes and functions for: +- Unified exception handling for all application exceptions +- Consistent error response formatting +- Comprehensive logging with structured data """ import logging from typing import Union from fastapi import Request, HTTPException +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse -from starlette.middleware.base import BaseHTTPMiddleware from .base import LetzShopException logger = logging.getLogger(__name__) -class ExceptionHandlerMiddleware(BaseHTTPMiddleware): - """Middleware to handle custom exceptions and convert them to JSON responses.""" - - async def dispatch(self, request: Request, call_next): - try: - response = await call_next(request) - return response - except LetzShopException as exc: - return await self.handle_custom_exception(request, exc) - except HTTPException as exc: - return await self.handle_http_exception(request, exc) - except Exception as exc: - return await self.handle_generic_exception(request, exc) - - async def handle_custom_exception( - self, - request: Request, - exc: LetzShopException - ) -> JSONResponse: - """Handle custom LetzShop exceptions.""" - - # Log the exception - logger.error( - f"Custom exception in {request.method} {request.url}: " - f"{exc.error_code} - {exc.message}", - extra={ - "error_code": exc.error_code, - "status_code": exc.status_code, - "details": exc.details, - "url": str(request.url), - "method": request.method, - } - ) - - return JSONResponse( - status_code=exc.status_code, - content=exc.to_dict() - ) - - async def handle_http_exception( - self, - request: Request, - exc: HTTPException - ) -> JSONResponse: - """Handle FastAPI HTTPExceptions.""" - - logger.error( - f"HTTP exception in {request.method} {request.url}: " - f"{exc.status_code} - {exc.detail}", - extra={ - "status_code": exc.status_code, - "detail": exc.detail, - "url": str(request.url), - "method": request.method, - } - ) - - return JSONResponse( - status_code=exc.status_code, - content={ - "error_code": f"HTTP_{exc.status_code}", - "message": exc.detail, - "status_code": exc.status_code, - } - ) - - async def handle_generic_exception( - self, - request: Request, - exc: Exception - ) -> JSONResponse: - """Handle unexpected exceptions.""" - - logger.error( - f"Unexpected exception in {request.method} {request.url}: {str(exc)}", - exc_info=True, - extra={ - "url": str(request.url), - "method": request.method, - "exception_type": type(exc).__name__, - } - ) - - return JSONResponse( - status_code=500, - content={ - "error_code": "INTERNAL_SERVER_ERROR", - "message": "Internal server error", - "status_code": 500, - } - ) - - def setup_exception_handlers(app): """Setup exception handlers for the FastAPI app.""" @@ -126,6 +37,7 @@ def setup_exception_handlers(app): "details": exc.details, "url": str(request.url), "method": request.method, + "exception_type": type(exc).__name__, } ) @@ -146,6 +58,7 @@ def setup_exception_handlers(app): "detail": exc.detail, "url": str(request.url), "method": request.method, + "exception_type": "HTTPException", } ) @@ -158,6 +71,32 @@ def setup_exception_handlers(app): } ) + @app.exception_handler(RequestValidationError) + async def validation_exception_handler(request: Request, exc: RequestValidationError): + """Handle Pydantic validation errors with consistent format.""" + + logger.error( + f"Validation error in {request.method} {request.url}: {exc.errors()}", + extra={ + "validation_errors": exc.errors(), + "url": str(request.url), + "method": request.method, + "exception_type": "RequestValidationError", + } + ) + + return JSONResponse( + status_code=422, + content={ + "error_code": "VALIDATION_ERROR", + "message": "Request validation failed", + "status_code": 422, + "details": { + "validation_errors": exc.errors() + } + } + ) + @app.exception_handler(Exception) async def generic_exception_handler(request: Request, exc: Exception): """Handle unexpected exceptions.""" @@ -204,4 +143,4 @@ def raise_auth_error(message: str = "Authentication failed") -> None: def raise_permission_error(message: str = "Access denied") -> None: """Convenience function to raise AuthorizationException.""" from .base import AuthorizationException - raise AuthorizationException(message) + raise AuthorizationException(message) \ No newline at end of file diff --git a/app/services/admin_service.py b/app/services/admin_service.py index 22f9c2ab..509447b7 100644 --- a/app/services/admin_service.py +++ b/app/services/admin_service.py @@ -1,19 +1,27 @@ # app/services/admin_service.py -"""Summary description .... +""" +Admin service for managing users, shops, and import jobs. This module provides classes and functions for: -- .... -- .... -- .... +- User management and status control +- Shop verification and activation +- Marketplace import job monitoring """ import logging from datetime import datetime from typing import List, Optional, Tuple -from fastapi import HTTPException from sqlalchemy.orm import Session +from app.exceptions import ( + UserNotFoundException, + UserStatusChangeException, + CannotModifySelfException, + ShopNotFoundException, + ShopVerificationException, + AdminOperationException, +) from models.schemas.marketplace import MarketplaceImportJobResponse from models.database.marketplace import MarketplaceImportJob from models.database.shop import Shop @@ -27,10 +35,17 @@ class AdminService: def get_all_users(self, db: Session, skip: int = 0, limit: int = 100) -> List[User]: """Get paginated list of all users.""" - return db.query(User).offset(skip).limit(limit).all() + try: + return db.query(User).offset(skip).limit(limit).all() + except Exception as e: + logger.error(f"Failed to retrieve users: {str(e)}") + raise AdminOperationException( + operation="get_all_users", + reason="Database query failed" + ) def toggle_user_status( - self, db: Session, user_id: int, current_admin_id: int + self, db: Session, user_id: int, current_admin_id: int ) -> Tuple[User, str]: """ Toggle user active status. @@ -44,30 +59,50 @@ class AdminService: Tuple of (updated_user, status_message) Raises: - HTTPException: If user not found or trying to deactivate own account + UserNotFoundException: If user not found + CannotModifySelfException: If trying to modify own account + UserStatusChangeException: If status change is not allowed """ - user = db.query(User).filter(User.id == user_id).first() - if not user: - raise HTTPException(status_code=404, detail="User not found") + user = self._get_user_by_id_or_raise(db, user_id) + # Prevent self-modification if user.id == current_admin_id: - raise HTTPException( - status_code=400, detail="Cannot deactivate your own account" + raise CannotModifySelfException(user_id, "deactivate account") + + # Check if user is another admin - FIXED LOGIC + if user.role == "admin" and user.id != current_admin_id: + raise UserStatusChangeException( + user_id=user_id, + current_status="admin", + attempted_action="toggle status", + reason="Cannot modify another admin user" ) - user.is_active = not user.is_active - user.updated_at = datetime.utcnow() - db.commit() - db.refresh(user) + try: + original_status = user.is_active + user.is_active = not user.is_active + user.updated_at = datetime.utcnow() + db.commit() + db.refresh(user) - status = "activated" if user.is_active else "deactivated" - logger.info( - f"User {user.username} has been {status} by admin {current_admin_id}" - ) - return user, f"User {user.username} has been {status}" + status_action = "activated" if user.is_active else "deactivated" + message = f"User {user.username} has been {status_action}" + + logger.info(f"{message} by admin {current_admin_id}") + return user, message + + except Exception as e: + db.rollback() + logger.error(f"Failed to toggle user {user_id} status: {str(e)}") + raise UserStatusChangeException( + user_id=user_id, + current_status="active" if original_status else "inactive", + attempted_action="toggle status", + reason="Database update failed" + ) def get_all_shops( - self, db: Session, skip: int = 0, limit: int = 100 + self, db: Session, skip: int = 0, limit: int = 100 ) -> Tuple[List[Shop], int]: """ Get paginated list of all shops with total count. @@ -80,9 +115,16 @@ class AdminService: Returns: Tuple of (shops_list, total_count) """ - total = db.query(Shop).count() - shops = db.query(Shop).offset(skip).limit(limit).all() - return shops, total + try: + total = db.query(Shop).count() + shops = db.query(Shop).offset(skip).limit(limit).all() + return shops, total + except Exception as e: + logger.error(f"Failed to retrieve shops: {str(e)}") + raise AdminOperationException( + operation="get_all_shops", + reason="Database query failed" + ) def verify_shop(self, db: Session, shop_id: int) -> Tuple[Shop, str]: """ @@ -96,20 +138,37 @@ class AdminService: Tuple of (updated_shop, status_message) Raises: - HTTPException: If shop not found + ShopNotFoundException: If shop not found + ShopVerificationException: If verification fails """ - shop = db.query(Shop).filter(Shop.id == shop_id).first() - if not shop: - raise HTTPException(status_code=404, detail="Shop not found") + shop = self._get_shop_by_id_or_raise(db, shop_id) - shop.is_verified = not shop.is_verified - shop.updated_at = datetime.utcnow() - db.commit() - db.refresh(shop) + try: + original_status = shop.is_verified + shop.is_verified = not shop.is_verified + shop.updated_at = datetime.utcnow() - status = "verified" if shop.is_verified else "unverified" - logger.info(f"Shop {shop.shop_code} has been {status}") - return shop, f"Shop {shop.shop_code} has been {status}" + # Add verification timestamp if implementing audit trail + if shop.is_verified: + shop.verified_at = datetime.utcnow() + + db.commit() + db.refresh(shop) + + status_action = "verified" if shop.is_verified else "unverified" + message = f"Shop {shop.shop_code} has been {status_action}" + + logger.info(message) + return shop, message + + except Exception as e: + db.rollback() + logger.error(f"Failed to verify shop {shop_id}: {str(e)}") + raise ShopVerificationException( + shop_id=shop_id, + reason="Database update failed", + current_verification_status=original_status + ) def toggle_shop_status(self, db: Session, shop_id: int) -> Tuple[Shop, str]: """ @@ -123,29 +182,42 @@ class AdminService: Tuple of (updated_shop, status_message) Raises: - HTTPException: If shop not found + ShopNotFoundException: If shop not found + AdminOperationException: If status change fails """ - shop = db.query(Shop).filter(Shop.id == shop_id).first() - if not shop: - raise HTTPException(status_code=404, detail="Shop not found") + shop = self._get_shop_by_id_or_raise(db, shop_id) - shop.is_active = not shop.is_active - shop.updated_at = datetime.utcnow() - db.commit() - db.refresh(shop) + try: + original_status = shop.is_active + shop.is_active = not shop.is_active + shop.updated_at = datetime.utcnow() + db.commit() + db.refresh(shop) - status = "activated" if shop.is_active else "deactivated" - logger.info(f"Shop {shop.shop_code} has been {status}") - return shop, f"Shop {shop.shop_code} has been {status}" + status_action = "activated" if shop.is_active else "deactivated" + message = f"Shop {shop.shop_code} has been {status_action}" + + logger.info(message) + return shop, message + + except Exception as e: + db.rollback() + logger.error(f"Failed to toggle shop {shop_id} status: {str(e)}") + raise AdminOperationException( + operation="toggle_shop_status", + reason="Database update failed", + target_type="shop", + target_id=str(shop_id) + ) def get_marketplace_import_jobs( - self, - db: Session, - marketplace: Optional[str] = None, - shop_name: Optional[str] = None, - status: Optional[str] = None, - skip: int = 0, - limit: int = 100, + self, + db: Session, + marketplace: Optional[str] = None, + shop_name: Optional[str] = None, + status: Optional[str] = None, + skip: int = 0, + limit: int = 100, ) -> List[MarketplaceImportJobResponse]: """ Get filtered and paginated marketplace import jobs. @@ -161,59 +233,129 @@ class AdminService: Returns: List of MarketplaceImportJobResponse objects """ - query = db.query(MarketplaceImportJob) + try: + query = db.query(MarketplaceImportJob) - # Apply filters - if marketplace: - query = query.filter( - MarketplaceImportJob.marketplace.ilike(f"%{marketplace}%") + # Apply filters + if marketplace: + query = query.filter( + MarketplaceImportJob.marketplace.ilike(f"%{marketplace}%") + ) + if shop_name: + query = query.filter(MarketplaceImportJob.shop_name.ilike(f"%{shop_name}%")) + if status: + query = query.filter(MarketplaceImportJob.status == status) + + # Order by creation date and apply pagination + jobs = ( + query.order_by(MarketplaceImportJob.created_at.desc()) + .offset(skip) + .limit(limit) + .all() ) - if shop_name: - query = query.filter(MarketplaceImportJob.shop_name.ilike(f"%{shop_name}%")) - if status: - query = query.filter(MarketplaceImportJob.status == status) - # Order by creation date and apply pagination - jobs = ( - query.order_by(MarketplaceImportJob.created_at.desc()) - .offset(skip) - .limit(limit) - .all() + return [self._convert_job_to_response(job) for job in jobs] + + except Exception as e: + logger.error(f"Failed to retrieve marketplace import jobs: {str(e)}") + raise AdminOperationException( + operation="get_marketplace_import_jobs", + reason="Database query failed" + ) + + def get_user_statistics(self, db: Session) -> dict: + """Get user statistics for admin dashboard.""" + try: + total_users = db.query(User).count() + active_users = db.query(User).filter(User.is_active == True).count() + inactive_users = total_users - active_users + + return { + "total_users": total_users, + "active_users": active_users, + "inactive_users": inactive_users, + "activation_rate": (active_users / total_users * 100) if total_users > 0 else 0 + } + except Exception as e: + logger.error(f"Failed to get user statistics: {str(e)}") + raise AdminOperationException( + operation="get_user_statistics", + reason="Database query failed" + ) + + def get_shop_statistics(self, db: Session) -> dict: + """Get shop statistics for admin dashboard.""" + try: + total_shops = db.query(Shop).count() + active_shops = db.query(Shop).filter(Shop.is_active == True).count() + verified_shops = db.query(Shop).filter(Shop.is_verified == True).count() + + return { + "total_shops": total_shops, + "active_shops": active_shops, + "verified_shops": verified_shops, + "verification_rate": (verified_shops / total_shops * 100) if total_shops > 0 else 0 + } + except Exception as e: + logger.error(f"Failed to get shop statistics: {str(e)}") + raise AdminOperationException( + operation="get_shop_statistics", + reason="Database query failed" + ) + + # Private helper methods + def _get_user_by_id_or_raise(self, db: Session, user_id: int) -> User: + """Get user by ID or raise UserNotFoundException.""" + user = db.query(User).filter(User.id == user_id).first() + if not user: + raise UserNotFoundException(str(user_id)) + return user + + def _get_shop_by_id_or_raise(self, db: Session, shop_id: int) -> Shop: + """Get shop by ID or raise ShopNotFoundException.""" + shop = db.query(Shop).filter(Shop.id == shop_id).first() + if not shop: + raise ShopNotFoundException(str(shop_id), identifier_type="id") + return shop + + def _convert_job_to_response(self, job: MarketplaceImportJob) -> MarketplaceImportJobResponse: + """Convert database model to response schema.""" + return MarketplaceImportJobResponse( + job_id=job.id, + status=job.status, + marketplace=job.marketplace, + shop_id=job.shop.id if job.shop else None, + shop_code=job.shop.shop_code if job.shop else None, + shop_name=job.shop_name, + imported=job.imported_count or 0, + updated=job.updated_count or 0, + total_processed=job.total_processed or 0, + error_count=job.error_count or 0, + error_message=job.error_message, + created_at=job.created_at, + started_at=job.started_at, + completed_at=job.completed_at, ) - return [ - MarketplaceImportJobResponse( - job_id=job.id, - status=job.status, - marketplace=job.marketplace, - shop_id=job.shop.id, - shop_name=job.shop_name, - imported=job.imported_count or 0, - updated=job.updated_count or 0, - total_processed=job.total_processed or 0, - error_count=job.error_count or 0, - error_message=job.error_message, - created_at=job.created_at, - started_at=job.started_at, - completed_at=job.completed_at, - ) - for job in jobs - ] - + # Legacy methods for backward compatibility (mark as deprecated) def get_user_by_id(self, db: Session, user_id: int) -> Optional[User]: - """Get user by ID.""" + """Get user by ID. DEPRECATED: Use _get_user_by_id_or_raise instead.""" + logger.warning("get_user_by_id is deprecated, use proper exception handling") return db.query(User).filter(User.id == user_id).first() def get_shop_by_id(self, db: Session, shop_id: int) -> Optional[Shop]: - """Get shop by ID.""" + """Get shop by ID. DEPRECATED: Use _get_shop_by_id_or_raise instead.""" + logger.warning("get_shop_by_id is deprecated, use proper exception handling") return db.query(Shop).filter(Shop.id == shop_id).first() def user_exists(self, db: Session, user_id: int) -> bool: - """Check if user exists by ID.""" + """Check if user exists by ID. DEPRECATED: Use proper exception handling.""" + logger.warning("user_exists is deprecated, use proper exception handling") return db.query(User).filter(User.id == user_id).first() is not None def shop_exists(self, db: Session, shop_id: int) -> bool: - """Check if shop exists by ID.""" + """Check if shop exists by ID. DEPRECATED: Use proper exception handling.""" + logger.warning("shop_exists is deprecated, use proper exception handling") return db.query(Shop).filter(Shop.id == shop_id).first() is not None diff --git a/app/services/auth_service.py b/app/services/auth_service.py index 8b14aa21..244fc493 100644 --- a/app/services/auth_service.py +++ b/app/services/auth_service.py @@ -1,18 +1,24 @@ # app/services/auth_service.py -"""Summary description .... +""" +Authentication service for user registration and login. This module provides classes and functions for: -- .... -- .... -- .... +- User registration with validation +- User authentication and JWT token generation +- Password management and security """ import logging from typing import Any, Dict, Optional -from fastapi import HTTPException from sqlalchemy.orm import Session +from app.exceptions import ( + UserAlreadyExistsException, + InvalidCredentialsException, + UserNotActiveException, + ValidationException, +) from middleware.auth import AuthManager from models.schemas.auth import UserLogin, UserRegister from models.database.user import User @@ -39,36 +45,41 @@ class AuthService: Created user object Raises: - HTTPException: If email or username already exists + UserAlreadyExistsException: If email or username already exists + ValidationException: If user data is invalid """ - # Check if email already exists - existing_email = db.query(User).filter(User.email == user_data.email).first() - if existing_email: - raise HTTPException(status_code=400, detail="Email already registered") + try: + # Check if email already exists + if self._email_exists(db, user_data.email): + raise UserAlreadyExistsException("Email already registered", field="email") - # Check if username already exists - existing_username = ( - db.query(User).filter(User.username == user_data.username).first() - ) - if existing_username: - raise HTTPException(status_code=400, detail="Username already taken") + # Check if username already exists + if self._username_exists(db, user_data.username): + raise UserAlreadyExistsException("Username already taken", field="username") - # Hash password and create user - hashed_password = self.auth_manager.hash_password(user_data.password) - new_user = User( - email=user_data.email, - username=user_data.username, - hashed_password=hashed_password, - role="user", - is_active=True, - ) + # Hash password and create user + hashed_password = self.auth_manager.hash_password(user_data.password) + new_user = User( + email=user_data.email, + username=user_data.username, + hashed_password=hashed_password, + role="user", + is_active=True, + ) - db.add(new_user) - db.commit() - db.refresh(new_user) + db.add(new_user) + db.commit() + db.refresh(new_user) - logger.info(f"New user registered: {new_user.username}") - return new_user + logger.info(f"New user registered: {new_user.username}") + return new_user + + except UserAlreadyExistsException: + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error registering user: {str(e)}") + raise ValidationException("Registration failed") def login_user(self, db: Session, user_credentials: UserLogin) -> Dict[str, Any]: """ @@ -82,53 +93,94 @@ class AuthService: Dictionary containing access token data and user object Raises: - HTTPException: If authentication fails + InvalidCredentialsException: If authentication fails + UserNotActiveException: If user account is not active """ - user = self.auth_manager.authenticate_user( - db, user_credentials.username, user_credentials.password - ) - if not user: - raise HTTPException( - status_code=401, detail="Incorrect username or password" + try: + user = self.auth_manager.authenticate_user( + db, user_credentials.username, user_credentials.password ) + if not user: + raise InvalidCredentialsException("Incorrect username or password") - # Create access token - token_data = self.auth_manager.create_access_token(user) + # Check if user is active + if not user.is_active: + raise UserNotActiveException("User account is not active") - logger.info(f"User logged in: {user.username}") + # Create access token + token_data = self.auth_manager.create_access_token(user) - return {"token_data": token_data, "user": user} + logger.info(f"User logged in: {user.username}") + return {"token_data": token_data, "user": user} + + except (InvalidCredentialsException, UserNotActiveException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error during login: {str(e)}") + raise InvalidCredentialsException() def get_user_by_email(self, db: Session, email: str) -> Optional[User]: """Get user by email.""" - return db.query(User).filter(User.email == email).first() + try: + return db.query(User).filter(User.email == email).first() + except Exception as e: + logger.error(f"Error getting user by email: {str(e)}") + return None def get_user_by_username(self, db: Session, username: str) -> Optional[User]: """Get user by username.""" - return db.query(User).filter(User.username == username).first() - - def email_exists(self, db: Session, email: str) -> bool: - """Check if email already exists.""" - return db.query(User).filter(User.email == email).first() is not None - - def username_exists(self, db: Session, username: str) -> bool: - """Check if username already exists.""" - return db.query(User).filter(User.username == username).first() is not None + try: + return db.query(User).filter(User.username == username).first() + except Exception as e: + logger.error(f"Error getting user by username: {str(e)}") + return None def authenticate_user( self, db: Session, username: str, password: str ) -> Optional[User]: """Authenticate user with username/password.""" - return self.auth_manager.authenticate_user(db, username, password) + try: + return self.auth_manager.authenticate_user(db, username, password) + except Exception as e: + logger.error(f"Error authenticating user: {str(e)}") + return None def create_access_token(self, user: User) -> Dict[str, Any]: """Create access token for user.""" - return self.auth_manager.create_access_token(user) + try: + return self.auth_manager.create_access_token(user) + except Exception as e: + logger.error(f"Error creating access token: {str(e)}") + raise ValidationException("Failed to create access token") def hash_password(self, password: str) -> str: """Hash password.""" - return self.auth_manager.hash_password(password) + try: + return self.auth_manager.hash_password(password) + except Exception as e: + logger.error(f"Error hashing password: {str(e)}") + raise ValidationException("Failed to hash password") + + # Private helper methods + def _email_exists(self, db: Session, email: str) -> bool: + """Check if email already exists.""" + return db.query(User).filter(User.email == email).first() is not None + + def _username_exists(self, db: Session, username: str) -> bool: + """Check if username already exists.""" + return db.query(User).filter(User.username == username).first() is not None + + # Legacy methods for backward compatibility (deprecated) + def email_exists(self, db: Session, email: str) -> bool: + """Check if email already exists. DEPRECATED: Use proper exception handling.""" + logger.warning("email_exists is deprecated, use proper exception handling") + return self._email_exists(db, email) + + def username_exists(self, db: Session, username: str) -> bool: + """Check if username already exists. DEPRECATED: Use proper exception handling.""" + logger.warning("username_exists is deprecated, use proper exception handling") + return self._username_exists(db, username) -# Create service instance following the same pattern as admin_service +# Create service instance following the same pattern as other services auth_service = AuthService() diff --git a/app/services/import_service.py b/app/services/import_service.py deleted file mode 100644 index e69de29b..00000000 diff --git a/app/services/marketplace_service.py b/app/services/marketplace_service.py index 3024080d..636b3e8c 100644 --- a/app/services/marketplace_service.py +++ b/app/services/marketplace_service.py @@ -1,10 +1,11 @@ # app/services/marketplace_service.py -"""Summary description .... +""" +Marketplace service for managing import jobs and marketplace integrations. This module provides classes and functions for: -- .... -- .... -- .... +- Import job creation and management +- Shop access validation +- Import job status tracking and updates """ import logging @@ -14,6 +15,15 @@ from typing import List, Optional from sqlalchemy import func from sqlalchemy.orm import Session +from app.exceptions import ( + ShopNotFoundException, + UnauthorizedShopAccessException, + ImportJobNotFoundException, + ImportJobNotOwnedException, + ImportJobCannotBeCancelledException, + ImportJobCannotBeDeletedException, + ValidationException, +) from models.schemas.marketplace import (MarketplaceImportJobResponse, MarketplaceImportRequest) from models.database.marketplace import MarketplaceImportJob @@ -26,171 +36,283 @@ logger = logging.getLogger(__name__) class MarketplaceService: """Service class for Marketplace operations following the application's service pattern.""" - def __init__(self): - """Class constructor.""" - pass - def validate_shop_access(self, db: Session, shop_code: str, user: User) -> Shop: - """Validate that the shop exists and user has access to it.""" - # Explicit type hint to help type checker shop: Optional[Shop] - # Use case-insensitive query to handle both uppercase and lowercase codes - shop: Optional[Shop] = ( - db.query(Shop) - .filter(func.upper(Shop.shop_code) == shop_code.upper()) - .first() - ) - if not shop: - raise ValueError("Shop not found") + """ + Validate that the shop exists and user has access to it. - # Check permissions: admin can import for any shop, others only for their own - if user.role != "admin" and shop.owner_id != user.id: - raise PermissionError("Access denied to this shop") + Args: + db: Database session + shop_code: Shop code to validate + user: User requesting access - return shop + Returns: + Shop object if access is valid + + Raises: + ShopNotFoundException: If shop doesn't exist + UnauthorizedShopAccessException: If user lacks access + """ + try: + # Use case-insensitive query to handle both uppercase and lowercase codes + shop = ( + db.query(Shop) + .filter(func.upper(Shop.shop_code) == shop_code.upper()) + .first() + ) + + if not shop: + raise ShopNotFoundException(shop_code) + + # Check permissions: admin can import for any shop, others only for their own + if user.role != "admin" and shop.owner_id != user.id: + raise UnauthorizedShopAccessException(shop_code, user.id) + + return shop + + except (ShopNotFoundException, UnauthorizedShopAccessException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error validating shop access: {str(e)}") + raise ValidationException("Failed to validate shop access") def create_import_job( - self, db: Session, request: MarketplaceImportRequest, user: User + self, db: Session, request: MarketplaceImportRequest, user: User ) -> MarketplaceImportJob: - """Create a new marketplace import job.""" - # Validate shop access first - shop = self.validate_shop_access(db, request.shop_code, user) + """ + Create a new marketplace import job. - # Create marketplace import job record - import_job = MarketplaceImportJob( - status="pending", - source_url=request.url, - marketplace=request.marketplace, - shop_id=shop.id, # Foreign key to shops table - shop_name=shop.shop_name, # Use shop.shop_name (the display name) - user_id=user.id, - created_at=datetime.utcnow(), - ) + Args: + db: Database session + request: Import request data + user: User creating the job - db.add(import_job) - db.commit() - db.refresh(import_job) + Returns: + Created MarketplaceImportJob object - logger.info( - f"Created marketplace import job {import_job.id}: " - f"{request.marketplace} -> {shop.shop_name} (shop_code: {shop.shop_code}) by user {user.username}" - ) + Raises: + ShopNotFoundException: If shop doesn't exist + UnauthorizedShopAccessException: If user lacks shop access + ValidationException: If job creation fails + """ + try: + # Validate shop access first + shop = self.validate_shop_access(db, request.shop_code, user) - return import_job + # Create marketplace import job record + import_job = MarketplaceImportJob( + status="pending", + source_url=request.url, + marketplace=request.marketplace, + shop_id=shop.id, # Foreign key to shops table + shop_name=shop.shop_name, # Use shop.shop_name (the display name) + user_id=user.id, + created_at=datetime.utcnow(), + ) + + db.add(import_job) + db.commit() + db.refresh(import_job) + + logger.info( + f"Created marketplace import job {import_job.id}: " + f"{request.marketplace} -> {shop.shop_name} (shop_code: {shop.shop_code}) by user {user.username}" + ) + + return import_job + + except (ShopNotFoundException, UnauthorizedShopAccessException): + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error creating import job: {str(e)}") + raise ValidationException("Failed to create import job") def get_import_job_by_id( - self, db: Session, job_id: int, user: User + self, db: Session, job_id: int, user: User ) -> MarketplaceImportJob: - """Get a marketplace import job by ID with access control.""" - job = ( - db.query(MarketplaceImportJob) - .filter(MarketplaceImportJob.id == job_id) - .first() - ) - if not job: - raise ValueError("Marketplace import job not found") + """ + Get a marketplace import job by ID with access control. - # Users can only see their own jobs, admins can see all - if user.role != "admin" and job.user_id != user.id: - raise PermissionError("Access denied to this import job") + Args: + db: Database session + job_id: Import job ID + user: User requesting the job - return job + Returns: + MarketplaceImportJob object + + Raises: + ImportJobNotFoundException: If job doesn't exist + ImportJobNotOwnedException: If user lacks access to job + """ + try: + job = ( + db.query(MarketplaceImportJob) + .filter(MarketplaceImportJob.id == job_id) + .first() + ) + + if not job: + raise ImportJobNotFoundException(job_id) + + # Users can only see their own jobs, admins can see all + if user.role != "admin" and job.user_id != user.id: + raise ImportJobNotOwnedException(job_id, user.id) + + return job + + except (ImportJobNotFoundException, ImportJobNotOwnedException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting import job {job_id}: {str(e)}") + raise ValidationException("Failed to retrieve import job") def get_import_jobs( - self, - db: Session, - user: User, - marketplace: Optional[str] = None, - shop_name: Optional[str] = None, - skip: int = 0, - limit: int = 50, + self, + db: Session, + user: User, + marketplace: Optional[str] = None, + shop_name: Optional[str] = None, + skip: int = 0, + limit: int = 50, ) -> List[MarketplaceImportJob]: - """Get marketplace import jobs with filtering and access control.""" - query = db.query(MarketplaceImportJob) + """ + Get marketplace import jobs with filtering and access control. - # Users can only see their own jobs, admins can see all - if user.role != "admin": - query = query.filter(MarketplaceImportJob.user_id == user.id) + Args: + db: Database session + user: User requesting jobs + marketplace: Optional marketplace filter + shop_name: Optional shop name filter + skip: Number of records to skip + limit: Maximum records to return - # Apply filters - if marketplace: - query = query.filter( - MarketplaceImportJob.marketplace.ilike(f"%{marketplace}%") + Returns: + List of MarketplaceImportJob objects + """ + try: + query = db.query(MarketplaceImportJob) + + # Users can only see their own jobs, admins can see all + if user.role != "admin": + query = query.filter(MarketplaceImportJob.user_id == user.id) + + # Apply filters + if marketplace: + query = query.filter( + MarketplaceImportJob.marketplace.ilike(f"%{marketplace}%") + ) + if shop_name: + query = query.filter(MarketplaceImportJob.shop_name.ilike(f"%{shop_name}%")) + + # Order by creation date (newest first) and apply pagination + jobs = ( + query.order_by(MarketplaceImportJob.created_at.desc()) + .offset(skip) + .limit(limit) + .all() ) - if shop_name: - query = query.filter(MarketplaceImportJob.shop_name.ilike(f"%{shop_name}%")) - # Order by creation date (newest first) and apply pagination - jobs = ( - query.order_by(MarketplaceImportJob.created_at.desc()) - .offset(skip) - .limit(limit) - .all() - ) + return jobs - return jobs + except Exception as e: + logger.error(f"Error getting import jobs: {str(e)}") + raise ValidationException("Failed to retrieve import jobs") def update_job_status( - self, db: Session, job_id: int, status: str, **kwargs + self, db: Session, job_id: int, status: str, **kwargs ) -> MarketplaceImportJob: - """Update marketplace import job status and other fields.""" - job = ( - db.query(MarketplaceImportJob) - .filter(MarketplaceImportJob.id == job_id) - .first() - ) - if not job: - raise ValueError("Marketplace import job not found") + """ + Update marketplace import job status and other fields. - job.status = status + Args: + db: Database session + job_id: Import job ID + status: New status + **kwargs: Additional fields to update - # Update optional fields if provided - if "imported_count" in kwargs: - job.imported_count = kwargs["imported_count"] - if "updated_count" in kwargs: - job.updated_count = kwargs["updated_count"] - if "total_processed" in kwargs: - job.total_processed = kwargs["total_processed"] - if "error_count" in kwargs: - job.error_count = kwargs["error_count"] - if "error_message" in kwargs: - job.error_message = kwargs["error_message"] - if "started_at" in kwargs: - job.started_at = kwargs["started_at"] - if "completed_at" in kwargs: - job.completed_at = kwargs["completed_at"] + Returns: + Updated MarketplaceImportJob object - db.commit() - db.refresh(job) + Raises: + ImportJobNotFoundException: If job doesn't exist + ValidationException: If update fails + """ + try: + job = ( + db.query(MarketplaceImportJob) + .filter(MarketplaceImportJob.id == job_id) + .first() + ) - logger.info(f"Updated marketplace import job {job_id} status to {status}") - return job + if not job: + raise ImportJobNotFoundException(job_id) + + job.status = status + + # Update optional fields if provided + allowed_fields = [ + 'imported_count', 'updated_count', 'total_processed', + 'error_count', 'error_message', 'started_at', 'completed_at' + ] + + for field in allowed_fields: + if field in kwargs: + setattr(job, field, kwargs[field]) + + db.commit() + db.refresh(job) + + logger.info(f"Updated marketplace import job {job_id} status to {status}") + return job + + except ImportJobNotFoundException: + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error updating job {job_id} status: {str(e)}") + raise ValidationException("Failed to update job status") def get_job_stats(self, db: Session, user: User) -> dict: - """Get statistics about marketplace import jobs for a user.""" - query = db.query(MarketplaceImportJob) + """ + Get statistics about marketplace import jobs for a user. - # Users can only see their own jobs, admins can see all - if user.role != "admin": - query = query.filter(MarketplaceImportJob.user_id == user.id) + Args: + db: Database session + user: User to get stats for - total_jobs = query.count() - pending_jobs = query.filter(MarketplaceImportJob.status == "pending").count() - running_jobs = query.filter(MarketplaceImportJob.status == "running").count() - completed_jobs = query.filter( - MarketplaceImportJob.status == "completed" - ).count() - failed_jobs = query.filter(MarketplaceImportJob.status == "failed").count() + Returns: + Dictionary containing job statistics + """ + try: + query = db.query(MarketplaceImportJob) - return { - "total_jobs": total_jobs, - "pending_jobs": pending_jobs, - "running_jobs": running_jobs, - "completed_jobs": completed_jobs, - "failed_jobs": failed_jobs, - } + # Users can only see their own jobs, admins can see all + if user.role != "admin": + query = query.filter(MarketplaceImportJob.user_id == user.id) + + total_jobs = query.count() + pending_jobs = query.filter(MarketplaceImportJob.status == "pending").count() + running_jobs = query.filter(MarketplaceImportJob.status == "running").count() + completed_jobs = query.filter( + MarketplaceImportJob.status == "completed" + ).count() + failed_jobs = query.filter(MarketplaceImportJob.status == "failed").count() + + return { + "total_jobs": total_jobs, + "pending_jobs": pending_jobs, + "running_jobs": running_jobs, + "completed_jobs": completed_jobs, + "failed_jobs": failed_jobs, + } + + except Exception as e: + logger.error(f"Error getting job stats: {str(e)}") + raise ValidationException("Failed to retrieve job statistics") def convert_to_response_model( - self, job: MarketplaceImportJob + self, job: MarketplaceImportJob ) -> MarketplaceImportJobResponse: """Convert database model to API response model.""" return MarketplaceImportJobResponse( @@ -213,38 +335,82 @@ class MarketplaceService: ) def cancel_import_job( - self, db: Session, job_id: int, user: User + self, db: Session, job_id: int, user: User ) -> MarketplaceImportJob: - """Cancel a pending or running import job.""" - job = self.get_import_job_by_id(db, job_id, user) + """ + Cancel a pending or running import job. - if job.status not in ["pending", "running"]: - raise ValueError(f"Cannot cancel job with status: {job.status}") + Args: + db: Database session + job_id: Import job ID + user: User requesting cancellation - job.status = "cancelled" - job.completed_at = datetime.utcnow() + Returns: + Updated MarketplaceImportJob object - db.commit() - db.refresh(job) + Raises: + ImportJobNotFoundException: If job doesn't exist + ImportJobNotOwnedException: If user lacks access + ImportJobCannotBeCancelledException: If job can't be cancelled + """ + try: + job = self.get_import_job_by_id(db, job_id, user) - logger.info(f"Cancelled marketplace import job {job_id}") - return job + if job.status not in ["pending", "running"]: + raise ImportJobCannotBeCancelledException(job_id, job.status) + + job.status = "cancelled" + job.completed_at = datetime.utcnow() + + db.commit() + db.refresh(job) + + logger.info(f"Cancelled marketplace import job {job_id}") + return job + + except (ImportJobNotFoundException, ImportJobNotOwnedException, ImportJobCannotBeCancelledException): + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error cancelling job {job_id}: {str(e)}") + raise ValidationException("Failed to cancel import job") def delete_import_job(self, db: Session, job_id: int, user: User) -> bool: - """Delete a marketplace import job.""" - job = self.get_import_job_by_id(db, job_id, user) + """ + Delete a marketplace import job. - # Only allow deletion of completed, failed, or cancelled jobs - if job.status in ["pending", "running"]: - raise ValueError( - f"Cannot delete job with status: {job.status}. Cancel it first." - ) + Args: + db: Database session + job_id: Import job ID + user: User requesting deletion - db.delete(job) - db.commit() + Returns: + True if deletion successful - logger.info(f"Deleted marketplace import job {job_id}") - return True + Raises: + ImportJobNotFoundException: If job doesn't exist + ImportJobNotOwnedException: If user lacks access + ImportJobCannotBeDeletedException: If job can't be deleted + """ + try: + job = self.get_import_job_by_id(db, job_id, user) + + # Only allow deletion of completed, failed, or cancelled jobs + if job.status in ["pending", "running"]: + raise ImportJobCannotBeDeletedException(job_id, job.status) + + db.delete(job) + db.commit() + + logger.info(f"Deleted marketplace import job {job_id}") + return True + + except (ImportJobNotFoundException, ImportJobNotOwnedException, ImportJobCannotBeDeletedException): + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error deleting job {job_id}: {str(e)}") + raise ValidationException("Failed to delete import job") # Create service instance diff --git a/app/services/product_service.py b/app/services/product_service.py index ab902987..1b49ea7c 100644 --- a/app/services/product_service.py +++ b/app/services/product_service.py @@ -1,19 +1,28 @@ # app/services/product_service.py -"""Summary description .... +""" +Product service for managing product operations and data processing. This module provides classes and functions for: -- .... -- .... -- .... +- Product CRUD operations with validation +- Advanced product filtering and search +- Stock information integration +- CSV export functionality """ import logging from datetime import datetime -from typing import Generator, List, Optional +from typing import Generator, List, Optional, Tuple from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session +from app.exceptions import ( + ProductNotFoundException, + ProductAlreadyExistsException, + InvalidProductDataException, + ProductValidationException, + ValidationException, +) from models.schemas.product import ProductCreate, ProductUpdate from models.schemas.stock import StockLocationResponse, StockSummaryResponse from models.database.product import Product @@ -32,28 +41,52 @@ class ProductService: self.price_processor = PriceProcessor() def create_product(self, db: Session, product_data: ProductCreate) -> Product: - """Create a new product with validation.""" + """ + Create a new product with validation. + + Args: + db: Database session + product_data: Product creation data + + Returns: + Created Product object + + Raises: + ProductAlreadyExistsException: If product with ID already exists + InvalidProductDataException: If product data is invalid + ProductValidationException: If validation fails + """ try: # Process and validate GTIN if provided if product_data.gtin: normalized_gtin = self.gtin_processor.normalize(product_data.gtin) if not normalized_gtin: - raise ValueError("Invalid GTIN format") + raise InvalidProductDataException("Invalid GTIN format", field="gtin") product_data.gtin = normalized_gtin # Process price if provided if product_data.price: - parsed_price, currency = self.price_processor.parse_price_currency( - product_data.price - ) - if parsed_price: - product_data.price = parsed_price - product_data.currency = currency + try: + parsed_price, currency = self.price_processor.parse_price_currency( + product_data.price + ) + if parsed_price: + product_data.price = parsed_price + product_data.currency = currency + except Exception as e: + raise InvalidProductDataException(f"Invalid price format: {str(e)}", field="price") # Set default marketplace if not provided if not product_data.marketplace: product_data.marketplace = "Letzshop" + # Validate required fields + if not product_data.product_id or not product_data.product_id.strip(): + raise ProductValidationException("Product ID is required", field="product_id") + + if not product_data.title or not product_data.title.strip(): + raise ProductValidationException("Product title is required", field="title") + db_product = Product(**product_data.model_dump()) db.add(db_product) db.commit() @@ -62,176 +95,327 @@ class ProductService: logger.info(f"Created product {db_product.product_id}") return db_product + except (InvalidProductDataException, ProductValidationException): + db.rollback() + raise # Re-raise custom exceptions except IntegrityError as e: db.rollback() logger.error(f"Database integrity error: {str(e)}") - raise ValueError("Product with this ID already exists") + if "product_id" in str(e).lower() or "unique" in str(e).lower(): + raise ProductAlreadyExistsException(product_data.product_id) + else: + raise ProductValidationException("Data integrity constraint violation") except Exception as e: db.rollback() logger.error(f"Error creating product: {str(e)}") - raise + raise ValidationException("Failed to create product") def get_product_by_id(self, db: Session, product_id: str) -> Optional[Product]: """Get a product by its ID.""" - return db.query(Product).filter(Product.product_id == product_id).first() - - def get_products_with_filters( - self, - db: Session, - skip: int = 0, - limit: int = 100, - brand: Optional[str] = None, - category: Optional[str] = None, - availability: Optional[str] = None, - marketplace: Optional[str] = None, - shop_name: Optional[str] = None, - search: Optional[str] = None, - ) -> tuple[List[Product], int]: - """Get products with filtering and pagination.""" - query = db.query(Product) - - # Apply filters - if brand: - query = query.filter(Product.brand.ilike(f"%{brand}%")) - if category: - query = query.filter(Product.google_product_category.ilike(f"%{category}%")) - if availability: - query = query.filter(Product.availability == availability) - if marketplace: - query = query.filter(Product.marketplace.ilike(f"%{marketplace}%")) - if shop_name: - query = query.filter(Product.shop_name.ilike(f"%{shop_name}%")) - if search: - # Search in title, description, marketplace, and shop_name - search_term = f"%{search}%" - query = query.filter( - (Product.title.ilike(search_term)) - | (Product.description.ilike(search_term)) - | (Product.marketplace.ilike(search_term)) - | (Product.shop_name.ilike(search_term)) - ) - - total = query.count() - products = query.offset(skip).limit(limit).all() - - return products, total - - def update_product( - self, db: Session, product_id: str, product_update: ProductUpdate - ) -> Product: - """Update product with validation.""" - product = db.query(Product).filter(Product.product_id == product_id).first() - if not product: - raise ValueError("Product not found") - - # Update fields - update_data = product_update.model_dump(exclude_unset=True) - - # Validate GTIN if being updated - if "gtin" in update_data and update_data["gtin"]: - normalized_gtin = self.gtin_processor.normalize(update_data["gtin"]) - if not normalized_gtin: - raise ValueError("Invalid GTIN format") - update_data["gtin"] = normalized_gtin - - # Process price if being updated - if "price" in update_data and update_data["price"]: - parsed_price, currency = self.price_processor.parse_price_currency( - update_data["price"] - ) - if parsed_price: - update_data["price"] = parsed_price - update_data["currency"] = currency - - for key, value in update_data.items(): - setattr(product, key, value) - - product.updated_at = datetime.utcnow() - db.commit() - db.refresh(product) - - logger.info(f"Updated product {product_id}") - return product - - def delete_product(self, db: Session, product_id: str) -> bool: - """Delete product and associated stock.""" - product = db.query(Product).filter(Product.product_id == product_id).first() - if not product: - raise ValueError("Product not found") - - # Delete associated stock entries if GTIN exists - if product.gtin: - db.query(Stock).filter(Stock.gtin == product.gtin).delete() - - db.delete(product) - db.commit() - - logger.info(f"Deleted product {product_id}") - return True - - def get_stock_info(self, db: Session, gtin: str) -> Optional[StockSummaryResponse]: - """Get stock information for a product by GTIN.""" - stock_entries = db.query(Stock).filter(Stock.gtin == gtin).all() - if not stock_entries: + try: + return db.query(Product).filter(Product.product_id == product_id).first() + except Exception as e: + logger.error(f"Error getting product {product_id}: {str(e)}") return None - total_quantity = sum(entry.quantity for entry in stock_entries) - locations = [ - StockLocationResponse(location=entry.location, quantity=entry.quantity) - for entry in stock_entries - ] + def get_product_by_id_or_raise(self, db: Session, product_id: str) -> Product: + """ + Get a product by its ID or raise exception. - return StockSummaryResponse( - gtin=gtin, total_quantity=total_quantity, locations=locations - ) + Args: + db: Database session + product_id: Product ID to find - def generate_csv_export( - self, - db: Session, - marketplace: Optional[str] = None, - shop_name: Optional[str] = None, - ) -> Generator[str, None, None]: - """Generate CSV export with streaming for memory efficiency.""" - # CSV header - yield ( - "product_id,title,description,link,image_link,availability,price,currency,brand," - "gtin,marketplace,shop_name\n" - ) + Returns: + Product object - batch_size = 1000 - offset = 0 + Raises: + ProductNotFoundException: If product doesn't exist + """ + product = self.get_product_by_id(db, product_id) + if not product: + raise ProductNotFoundException(product_id) + return product - while True: + def get_products_with_filters( + self, + db: Session, + skip: int = 0, + limit: int = 100, + brand: Optional[str] = None, + category: Optional[str] = None, + availability: Optional[str] = None, + marketplace: Optional[str] = None, + shop_name: Optional[str] = None, + search: Optional[str] = None, + ) -> Tuple[List[Product], int]: + """ + Get products with filtering and pagination. + + Args: + db: Database session + skip: Number of records to skip + limit: Maximum records to return + brand: Brand filter + category: Category filter + availability: Availability filter + marketplace: Marketplace filter + shop_name: Shop name filter + search: Search term + + Returns: + Tuple of (products_list, total_count) + """ + try: query = db.query(Product) - # Apply marketplace filters + # Apply filters + if brand: + query = query.filter(Product.brand.ilike(f"%{brand}%")) + if category: + query = query.filter(Product.google_product_category.ilike(f"%{category}%")) + if availability: + query = query.filter(Product.availability == availability) if marketplace: query = query.filter(Product.marketplace.ilike(f"%{marketplace}%")) if shop_name: query = query.filter(Product.shop_name.ilike(f"%{shop_name}%")) - - products = query.offset(offset).limit(batch_size).all() - if not products: - break - - for product in products: - # Create CSV row with marketplace fields - row = ( - f'"{product.product_id}","{product.title or ""}","{product.description or ""}",' - f'"{product.link or ""}","{product.image_link or ""}","{product.availability or ""}",' - f'"{product.price or ""}","{product.currency or ""}","{product.brand or ""}",' - f'"{product.gtin or ""}","{product.marketplace or ""}","{product.shop_name or ""}"\n' + if search: + # Search in title, description, marketplace, and shop_name + search_term = f"%{search}%" + query = query.filter( + (Product.title.ilike(search_term)) + | (Product.description.ilike(search_term)) + | (Product.marketplace.ilike(search_term)) + | (Product.shop_name.ilike(search_term)) ) - yield row - offset += batch_size + total = query.count() + products = query.offset(skip).limit(limit).all() + + return products, total + + except Exception as e: + logger.error(f"Error getting products with filters: {str(e)}") + raise ValidationException("Failed to retrieve products") + + def update_product( + self, db: Session, product_id: str, product_update: ProductUpdate + ) -> Product: + """ + Update product with validation. + + Args: + db: Database session + product_id: Product ID to update + product_update: Update data + + Returns: + Updated Product object + + Raises: + ProductNotFoundException: If product doesn't exist + InvalidProductDataException: If update data is invalid + ProductValidationException: If validation fails + """ + try: + product = self.get_product_by_id_or_raise(db, product_id) + + # Update fields + update_data = product_update.model_dump(exclude_unset=True) + + # Validate GTIN if being updated + if "gtin" in update_data and update_data["gtin"]: + normalized_gtin = self.gtin_processor.normalize(update_data["gtin"]) + if not normalized_gtin: + raise InvalidProductDataException("Invalid GTIN format", field="gtin") + update_data["gtin"] = normalized_gtin + + # Process price if being updated + if "price" in update_data and update_data["price"]: + try: + parsed_price, currency = self.price_processor.parse_price_currency( + update_data["price"] + ) + if parsed_price: + update_data["price"] = parsed_price + update_data["currency"] = currency + except Exception as e: + raise InvalidProductDataException(f"Invalid price format: {str(e)}", field="price") + + # Validate required fields if being updated + if "title" in update_data and (not update_data["title"] or not update_data["title"].strip()): + raise ProductValidationException("Product title cannot be empty", field="title") + + for key, value in update_data.items(): + setattr(product, key, value) + + product.updated_at = datetime.utcnow() + db.commit() + db.refresh(product) + + logger.info(f"Updated product {product_id}") + return product + + except (ProductNotFoundException, InvalidProductDataException, ProductValidationException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error updating product {product_id}: {str(e)}") + raise ValidationException("Failed to update product") + + def delete_product(self, db: Session, product_id: str) -> bool: + """ + Delete product and associated stock. + + Args: + db: Database session + product_id: Product ID to delete + + Returns: + True if deletion successful + + Raises: + ProductNotFoundException: If product doesn't exist + """ + try: + product = self.get_product_by_id_or_raise(db, product_id) + + # Delete associated stock entries if GTIN exists + if product.gtin: + db.query(Stock).filter(Stock.gtin == product.gtin).delete() + + db.delete(product) + db.commit() + + logger.info(f"Deleted product {product_id}") + return True + + except ProductNotFoundException: + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error deleting product {product_id}: {str(e)}") + raise ValidationException("Failed to delete product") + + def get_stock_info(self, db: Session, gtin: str) -> Optional[StockSummaryResponse]: + """ + Get stock information for a product by GTIN. + + Args: + db: Database session + gtin: GTIN to look up stock for + + Returns: + StockSummaryResponse if stock found, None otherwise + """ + try: + stock_entries = db.query(Stock).filter(Stock.gtin == gtin).all() + if not stock_entries: + return None + + total_quantity = sum(entry.quantity for entry in stock_entries) + locations = [ + StockLocationResponse(location=entry.location, quantity=entry.quantity) + for entry in stock_entries + ] + + return StockSummaryResponse( + gtin=gtin, total_quantity=total_quantity, locations=locations + ) + + except Exception as e: + logger.error(f"Error getting stock info for GTIN {gtin}: {str(e)}") + return None + + def generate_csv_export( + self, + db: Session, + marketplace: Optional[str] = None, + shop_name: Optional[str] = None, + ) -> Generator[str, None, None]: + """ + Generate CSV export with streaming for memory efficiency. + + Args: + db: Database session + marketplace: Optional marketplace filter + shop_name: Optional shop name filter + + Yields: + CSV content as strings + """ + try: + # CSV header + yield ( + "product_id,title,description,link,image_link,availability,price,currency,brand," + "gtin,marketplace,shop_name\n" + ) + + batch_size = 1000 + offset = 0 + + while True: + query = db.query(Product) + + # Apply marketplace filters + if marketplace: + query = query.filter(Product.marketplace.ilike(f"%{marketplace}%")) + if shop_name: + query = query.filter(Product.shop_name.ilike(f"%{shop_name}%")) + + products = query.offset(offset).limit(batch_size).all() + if not products: + break + + for product in products: + # Create CSV row with marketplace fields + row = ( + f'"{product.product_id}","{product.title or ""}","{product.description or ""}",' + f'"{product.link or ""}","{product.image_link or ""}","{product.availability or ""}",' + f'"{product.price or ""}","{product.currency or ""}","{product.brand or ""}",' + f'"{product.gtin or ""}","{product.marketplace or ""}","{product.shop_name or ""}"\n' + ) + yield row + + offset += batch_size + + except Exception as e: + logger.error(f"Error generating CSV export: {str(e)}") + raise ValidationException("Failed to generate CSV export") def product_exists(self, db: Session, product_id: str) -> bool: """Check if product exists by ID.""" - return ( - db.query(Product).filter(Product.product_id == product_id).first() - is not None - ) + try: + return ( + db.query(Product).filter(Product.product_id == product_id).first() + is not None + ) + except Exception as e: + logger.error(f"Error checking if product exists: {str(e)}") + return False + + # Private helper methods + def _validate_product_data(self, product_data: dict) -> None: + """Validate product data structure.""" + required_fields = ['product_id', 'title'] + + for field in required_fields: + if field not in product_data or not product_data[field]: + raise ProductValidationException(f"{field} is required", field=field) + + def _normalize_product_data(self, product_data: dict) -> dict: + """Normalize and clean product data.""" + normalized = product_data.copy() + + # Trim whitespace from string fields + string_fields = ['product_id', 'title', 'description', 'brand', 'marketplace', 'shop_name'] + for field in string_fields: + if field in normalized and normalized[field]: + normalized[field] = normalized[field].strip() + + return normalized # Create service instance diff --git a/app/services/shop_service.py b/app/services/shop_service.py index a0a48b5a..97d0200d 100644 --- a/app/services/shop_service.py +++ b/app/services/shop_service.py @@ -1,19 +1,30 @@ # app/services/shop_service.py -"""Summary description .... +""" +Shop service for managing shop operations and product catalog. This module provides classes and functions for: -- .... -- .... -- .... +- Shop creation and management +- Shop access control and validation +- Shop product catalog operations +- Shop filtering and search """ import logging from typing import List, Optional, Tuple -from fastapi import HTTPException from sqlalchemy import func from sqlalchemy.orm import Session +from app.exceptions import ( + ShopNotFoundException, + ShopAlreadyExistsException, + UnauthorizedShopAccessException, + InvalidShopDataException, + ProductNotFoundException, + ShopProductAlreadyExistsException, + MaxShopsReachedException, + ValidationException, +) from models.schemas.shop import ShopCreate, ShopProductCreate from models.database.product import Product from models.database.shop import Shop, ShopProduct @@ -26,7 +37,7 @@ class ShopService: """Service class for shop operations following the application's service pattern.""" def create_shop( - self, db: Session, shop_data: ShopCreate, current_user: User + self, db: Session, shop_data: ShopCreate, current_user: User ) -> Shop: """ Create a new shop. @@ -40,49 +51,60 @@ class ShopService: Created shop object Raises: - HTTPException: If shop code already exists + ShopAlreadyExistsException: If shop code already exists + MaxShopsReachedException: If user has reached maximum shops + InvalidShopDataException: If shop data is invalid """ - # Normalize shop code to uppercase - normalized_shop_code = shop_data.shop_code.upper() + try: + # Validate shop data + self._validate_shop_data(shop_data) - # Check if shop code already exists (case-insensitive check against existing data) - existing_shop = ( - db.query(Shop) - .filter(func.upper(Shop.shop_code) == normalized_shop_code) - .first() - ) + # Check user's shop limit (if applicable) + self._check_shop_limit(db, current_user) - if existing_shop: - raise HTTPException(status_code=400, detail="Shop code already exists") + # Normalize shop code to uppercase + normalized_shop_code = shop_data.shop_code.upper() - # Create shop with uppercase code - shop_dict = shop_data.model_dump() # Fixed deprecated .dict() method - shop_dict["shop_code"] = normalized_shop_code # Store as uppercase + # Check if shop code already exists (case-insensitive check) + if self._shop_code_exists(db, normalized_shop_code): + raise ShopAlreadyExistsException(normalized_shop_code) - new_shop = Shop( - **shop_dict, - owner_id=current_user.id, - is_active=True, - is_verified=(current_user.role == "admin"), - ) + # Create shop with uppercase code + shop_dict = shop_data.model_dump() + shop_dict["shop_code"] = normalized_shop_code # Store as uppercase - db.add(new_shop) - db.commit() - db.refresh(new_shop) + new_shop = Shop( + **shop_dict, + owner_id=current_user.id, + is_active=True, + is_verified=(current_user.role == "admin"), + ) - logger.info( - f"New shop created: {new_shop.shop_code} by {current_user.username}" - ) - return new_shop + db.add(new_shop) + db.commit() + db.refresh(new_shop) + + logger.info( + f"New shop created: {new_shop.shop_code} by {current_user.username}" + ) + return new_shop + + except (ShopAlreadyExistsException, MaxShopsReachedException, InvalidShopDataException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error creating shop: {str(e)}") + raise ValidationException("Failed to create shop") def get_shops( - self, - db: Session, - current_user: User, - skip: int = 0, - limit: int = 100, - active_only: bool = True, - verified_only: bool = False, + self, + db: Session, + current_user: User, + skip: int = 0, + limit: int = 100, + active_only: bool = True, + verified_only: bool = False, ) -> Tuple[List[Shop], int]: """ Get shops with filtering. @@ -98,25 +120,30 @@ class ShopService: Returns: Tuple of (shops_list, total_count) """ - query = db.query(Shop) + try: + query = db.query(Shop) - # Non-admin users can only see active and verified shops, plus their own - if current_user.role != "admin": - query = query.filter( - (Shop.is_active == True) - & ((Shop.is_verified == True) | (Shop.owner_id == current_user.id)) - ) - else: - # Admin can apply filters - if active_only: - query = query.filter(Shop.is_active == True) - if verified_only: - query = query.filter(Shop.is_verified == True) + # Non-admin users can only see active and verified shops, plus their own + if current_user.role != "admin": + query = query.filter( + (Shop.is_active == True) + & ((Shop.is_verified == True) | (Shop.owner_id == current_user.id)) + ) + else: + # Admin can apply filters + if active_only: + query = query.filter(Shop.is_active == True) + if verified_only: + query = query.filter(Shop.is_verified == True) - total = query.count() - shops = query.offset(skip).limit(limit).all() + total = query.count() + shops = query.offset(skip).limit(limit).all() - return shops, total + return shops, total + + except Exception as e: + logger.error(f"Error getting shops: {str(e)}") + raise ValidationException("Failed to retrieve shops") def get_shop_by_code(self, db: Session, shop_code: str, current_user: User) -> Shop: """ @@ -131,28 +158,33 @@ class ShopService: Shop object Raises: - HTTPException: If shop not found or access denied + ShopNotFoundException: If shop not found + UnauthorizedShopAccessException: If access denied """ - # Explicit type hint to help type checker shop: Optional[Shop] - shop: Optional[Shop] = ( - db.query(Shop) - .filter(func.upper(Shop.shop_code) == shop_code.upper()) - .first() - ) - if not shop: - raise HTTPException(status_code=404, detail="Shop not found") + try: + shop = ( + db.query(Shop) + .filter(func.upper(Shop.shop_code) == shop_code.upper()) + .first() + ) - # Non-admin users can only see active verified shops or their own shops - if current_user.role != "admin": - if not shop.is_active or ( - not shop.is_verified and shop.owner_id != current_user.id - ): - raise HTTPException(status_code=404, detail="Shop not found") + if not shop: + raise ShopNotFoundException(shop_code) - return shop + # Check access permissions + if not self._can_access_shop(shop, current_user): + raise UnauthorizedShopAccessException(shop_code, current_user.id) + + return shop + + except (ShopNotFoundException, UnauthorizedShopAccessException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting shop {shop_code}: {str(e)}") + raise ValidationException("Failed to retrieve shop") def add_product_to_shop( - self, db: Session, shop: Shop, shop_product: ShopProductCreate + self, db: Session, shop: Shop, shop_product: ShopProductCreate ) -> ShopProduct: """ Add existing product to shop catalog with shop-specific settings. @@ -166,59 +198,51 @@ class ShopService: Created ShopProduct object Raises: - HTTPException: If product not found or already in shop + ProductNotFoundException: If product not found + ShopProductAlreadyExistsException: If product already in shop """ - # Check if product exists - product = ( - db.query(Product) - .filter(Product.product_id == shop_product.product_id) - .first() - ) - if not product: - raise HTTPException( - status_code=404, detail="Product not found in marketplace catalog" + try: + # Check if product exists + product = self._get_product_by_id_or_raise(db, shop_product.product_id) + + # Check if product already in shop + if self._product_in_shop(db, shop.id, product.id): + raise ShopProductAlreadyExistsException(shop.shop_code, shop_product.product_id) + + # Create shop-product association + new_shop_product = ShopProduct( + shop_id=shop.id, + product_id=product.id, + **shop_product.model_dump(exclude={"product_id"}), ) - # Check if product already in shop - existing_shop_product = ( - db.query(ShopProduct) - .filter( - ShopProduct.shop_id == shop.id, ShopProduct.product_id == product.id - ) - .first() - ) + db.add(new_shop_product) + db.commit() + db.refresh(new_shop_product) - if existing_shop_product: - raise HTTPException( - status_code=400, detail="Product already in shop catalog" - ) + # Load the product relationship + db.refresh(new_shop_product) - # Create shop-product association - new_shop_product = ShopProduct( - shop_id=shop.id, - product_id=product.id, - **shop_product.model_dump(exclude={"product_id"}), - ) + logger.info(f"Product {shop_product.product_id} added to shop {shop.shop_code}") + return new_shop_product - db.add(new_shop_product) - db.commit() - db.refresh(new_shop_product) - - # Load the product relationship - db.refresh(new_shop_product) - - logger.info(f"Product {shop_product.product_id} added to shop {shop.shop_code}") - return new_shop_product + except (ProductNotFoundException, ShopProductAlreadyExistsException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error adding product to shop: {str(e)}") + raise ValidationException("Failed to add product to shop") def get_shop_products( - self, - db: Session, - shop: Shop, - current_user: User, - skip: int = 0, - limit: int = 100, - active_only: bool = True, - featured_only: bool = False, + self, + db: Session, + shop: Shop, + current_user: User, + skip: int = 0, + limit: int = 100, + active_only: bool = True, + featured_only: bool = False, ) -> Tuple[List[ShopProduct], int]: """ Get products in shop catalog with filtering. @@ -236,58 +260,137 @@ class ShopService: Tuple of (shop_products_list, total_count) Raises: - HTTPException: If shop access denied + UnauthorizedShopAccessException: If shop access denied """ - # Non-owners can only see active verified shops - if current_user.role != "admin" and shop.owner_id != current_user.id: - if not shop.is_active or not shop.is_verified: - raise HTTPException(status_code=404, detail="Shop not found") + try: + # Check access permissions + if not self._can_access_shop(shop, current_user): + raise UnauthorizedShopAccessException(shop.shop_code, current_user.id) - # Query shop products - query = db.query(ShopProduct).filter(ShopProduct.shop_id == shop.id) + # Query shop products + query = db.query(ShopProduct).filter(ShopProduct.shop_id == shop.id) - if active_only: - query = query.filter(ShopProduct.is_active == True) - if featured_only: - query = query.filter(ShopProduct.is_featured == True) + if active_only: + query = query.filter(ShopProduct.is_active == True) + if featured_only: + query = query.filter(ShopProduct.is_featured == True) - total = query.count() - shop_products = query.offset(skip).limit(limit).all() + total = query.count() + shop_products = query.offset(skip).limit(limit).all() - return shop_products, total + return shop_products, total - def get_shop_by_id(self, db: Session, shop_id: int) -> Optional[Shop]: - """Get shop by ID.""" - return db.query(Shop).filter(Shop.id == shop_id).first() + except UnauthorizedShopAccessException: + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting shop products: {str(e)}") + raise ValidationException("Failed to retrieve shop products") - def shop_code_exists(self, db: Session, shop_code: str) -> bool: - """Check if shop code already exists.""" - return db.query(Shop).filter(Shop.shop_code == shop_code).first() is not None + # Private helper methods + def _validate_shop_data(self, shop_data: ShopCreate) -> None: + """Validate shop creation data.""" + if not shop_data.shop_code or not shop_data.shop_code.strip(): + raise InvalidShopDataException("Shop code is required", field="shop_code") - def get_product_by_id(self, db: Session, product_id: str) -> Optional[Product]: - """Get product by product_id.""" - return db.query(Product).filter(Product.product_id == product_id).first() + if not shop_data.shop_name or not shop_data.shop_name.strip(): + raise InvalidShopDataException("Shop name is required", field="shop_name") - def product_in_shop(self, db: Session, shop_id: int, product_id: int) -> bool: - """Check if product is already in shop.""" - return ( - db.query(ShopProduct) - .filter( - ShopProduct.shop_id == shop_id, ShopProduct.product_id == product_id + # Validate shop code format (alphanumeric, underscores, hyphens) + import re + if not re.match(r'^[A-Za-z0-9_-]+$', shop_data.shop_code): + raise InvalidShopDataException( + "Shop code can only contain letters, numbers, underscores, and hyphens", + field="shop_code" ) - .first() - is not None + + def _check_shop_limit(self, db: Session, user: User) -> None: + """Check if user has reached maximum shop limit.""" + if user.role == "admin": + return # Admins have no limit + + user_shop_count = db.query(Shop).filter(Shop.owner_id == user.id).count() + max_shops = 5 # Configure this as needed + + if user_shop_count >= max_shops: + raise MaxShopsReachedException(max_shops, user.id) + + def _shop_code_exists(self, db: Session, shop_code: str) -> bool: + """Check if shop code already exists (case-insensitive).""" + return ( + db.query(Shop) + .filter(func.upper(Shop.shop_code) == shop_code.upper()) + .first() is not None ) - def is_shop_owner(self, shop: Shop, user: User) -> bool: + def _get_product_by_id_or_raise(self, db: Session, product_id: str) -> Product: + """Get product by ID or raise exception.""" + product = db.query(Product).filter(Product.product_id == product_id).first() + if not product: + raise ProductNotFoundException(product_id) + return product + + def _product_in_shop(self, db: Session, shop_id: int, product_id: int) -> bool: + """Check if product is already in shop.""" + return ( + db.query(ShopProduct) + .filter( + ShopProduct.shop_id == shop_id, + ShopProduct.product_id == product_id + ) + .first() is not None + ) + + def _can_access_shop(self, shop: Shop, user: User) -> bool: + """Check if user can access shop.""" + # Admins and owners can always access + if user.role == "admin" or shop.owner_id == user.id: + return True + + # Others can only access active and verified shops + return shop.is_active and shop.is_verified + + def _is_shop_owner(self, shop: Shop, user: User) -> bool: """Check if user is shop owner.""" return shop.owner_id == user.id + # Legacy methods for backward compatibility (deprecated) + def get_shop_by_id(self, db: Session, shop_id: int) -> Optional[Shop]: + """Get shop by ID. DEPRECATED: Use proper exception handling.""" + logger.warning("get_shop_by_id is deprecated, use proper exception handling") + try: + return db.query(Shop).filter(Shop.id == shop_id).first() + except Exception as e: + logger.error(f"Error getting shop by ID: {str(e)}") + return None + + def shop_code_exists(self, db: Session, shop_code: str) -> bool: + """Check if shop code exists. DEPRECATED: Use proper exception handling.""" + logger.warning("shop_code_exists is deprecated, use proper exception handling") + return self._shop_code_exists(db, shop_code) + + def get_product_by_id(self, db: Session, product_id: str) -> Optional[Product]: + """Get product by ID. DEPRECATED: Use proper exception handling.""" + logger.warning("get_product_by_id is deprecated, use proper exception handling") + try: + return db.query(Product).filter(Product.product_id == product_id).first() + except Exception as e: + logger.error(f"Error getting product by ID: {str(e)}") + return None + + def product_in_shop(self, db: Session, shop_id: int, product_id: int) -> bool: + """Check if product in shop. DEPRECATED: Use proper exception handling.""" + logger.warning("product_in_shop is deprecated, use proper exception handling") + return self._product_in_shop(db, shop_id, product_id) + + def is_shop_owner(self, shop: Shop, user: User) -> bool: + """Check if user is shop owner. DEPRECATED: Use _is_shop_owner.""" + logger.warning("is_shop_owner is deprecated, use _is_shop_owner") + return self._is_shop_owner(shop, user) + def can_view_shop(self, shop: Shop, user: User) -> bool: - """Check if user can view shop.""" - if user.role == "admin" or self.is_shop_owner(shop, user): - return True - return shop.is_active and shop.is_verified + """Check if user can view shop. DEPRECATED: Use _can_access_shop.""" + logger.warning("can_view_shop is deprecated, use _can_access_shop") + return self._can_access_shop(shop, user) # Create service instance following the same pattern as other services diff --git a/app/services/stats_service.py b/app/services/stats_service.py index 108abddc..54dbe494 100644 --- a/app/services/stats_service.py +++ b/app/services/stats_service.py @@ -1,10 +1,12 @@ # app/services/stats_service.py -"""Summary description .... +""" +Statistics service for generating system analytics and metrics. This module provides classes and functions for: -- .... -- .... -- .... +- Comprehensive system statistics +- Marketplace-specific analytics +- Performance metrics and data insights +- Cached statistics for performance """ import logging @@ -13,6 +15,7 @@ from typing import Any, Dict, List from sqlalchemy import func from sqlalchemy.orm import Session +from app.exceptions import ValidationException from models.database.product import Product from models.database.stock import Stock @@ -31,60 +34,39 @@ class StatsService: Returns: Dictionary containing all statistics data + + Raises: + ValidationException: If statistics generation fails """ - # Use more efficient queries with proper indexes - total_products = db.query(Product).count() + try: + # Use more efficient queries with proper indexes + total_products = self._get_product_count(db) + unique_brands = self._get_unique_brands_count(db) + unique_categories = self._get_unique_categories_count(db) + unique_marketplaces = self._get_unique_marketplaces_count(db) + unique_shops = self._get_unique_shops_count(db) - unique_brands = ( - db.query(Product.brand) - .filter(Product.brand.isnot(None), Product.brand != "") - .distinct() - .count() - ) + # Stock statistics + stock_stats = self._get_stock_statistics(db) - unique_categories = ( - db.query(Product.google_product_category) - .filter( - Product.google_product_category.isnot(None), - Product.google_product_category != "", + stats_data = { + "total_products": total_products, + "unique_brands": unique_brands, + "unique_categories": unique_categories, + "unique_marketplaces": unique_marketplaces, + "unique_shops": unique_shops, + "total_stock_entries": stock_stats["total_stock_entries"], + "total_inventory_quantity": stock_stats["total_inventory_quantity"], + } + + logger.info( + f"Generated comprehensive stats: {total_products} products, {unique_marketplaces} marketplaces" ) - .distinct() - .count() - ) + return stats_data - # New marketplace statistics - unique_marketplaces = ( - db.query(Product.marketplace) - .filter(Product.marketplace.isnot(None), Product.marketplace != "") - .distinct() - .count() - ) - - unique_shops = ( - db.query(Product.shop_name) - .filter(Product.shop_name.isnot(None), Product.shop_name != "") - .distinct() - .count() - ) - - # Stock statistics - total_stock_entries = db.query(Stock).count() - total_inventory = db.query(func.sum(Stock.quantity)).scalar() or 0 - - stats_data = { - "total_products": total_products, - "unique_brands": unique_brands, - "unique_categories": unique_categories, - "unique_marketplaces": unique_marketplaces, - "unique_shops": unique_shops, - "total_stock_entries": total_stock_entries, - "total_inventory_quantity": total_inventory, - } - - logger.info( - f"Generated comprehensive stats: {total_products} products, {unique_marketplaces} marketplaces" - ) - return stats_data + except Exception as e: + logger.error(f"Error getting comprehensive stats: {str(e)}") + raise ValidationException("Failed to retrieve system statistics") def get_marketplace_breakdown_stats(self, db: Session) -> List[Dict[str, Any]]: """ @@ -95,40 +77,127 @@ class StatsService: Returns: List of dictionaries containing marketplace statistics + + Raises: + ValidationException: If marketplace statistics generation fails """ - # Query to get stats per marketplace - marketplace_stats = ( - db.query( - Product.marketplace, - func.count(Product.id).label("total_products"), - func.count(func.distinct(Product.shop_name)).label("unique_shops"), - func.count(func.distinct(Product.brand)).label("unique_brands"), + try: + # Query to get stats per marketplace + marketplace_stats = ( + db.query( + Product.marketplace, + func.count(Product.id).label("total_products"), + func.count(func.distinct(Product.shop_name)).label("unique_shops"), + func.count(func.distinct(Product.brand)).label("unique_brands"), + ) + .filter(Product.marketplace.isnot(None)) + .group_by(Product.marketplace) + .all() ) - .filter(Product.marketplace.isnot(None)) - .group_by(Product.marketplace) - .all() - ) - stats_list = [ - { - "marketplace": stat.marketplace, - "total_products": stat.total_products, - "unique_shops": stat.unique_shops, - "unique_brands": stat.unique_brands, + stats_list = [ + { + "marketplace": stat.marketplace, + "total_products": stat.total_products, + "unique_shops": stat.unique_shops, + "unique_brands": stat.unique_brands, + } + for stat in marketplace_stats + ] + + logger.info( + f"Generated marketplace breakdown stats for {len(stats_list)} marketplaces" + ) + return stats_list + + except Exception as e: + logger.error(f"Error getting marketplace breakdown stats: {str(e)}") + raise ValidationException("Failed to retrieve marketplace statistics") + + def get_product_statistics(self, db: Session) -> Dict[str, Any]: + """ + Get detailed product statistics. + + Args: + db: Database session + + Returns: + Dictionary containing product statistics + """ + try: + stats = { + "total_products": self._get_product_count(db), + "unique_brands": self._get_unique_brands_count(db), + "unique_categories": self._get_unique_categories_count(db), + "unique_marketplaces": self._get_unique_marketplaces_count(db), + "unique_shops": self._get_unique_shops_count(db), + "products_with_gtin": self._get_products_with_gtin_count(db), + "products_with_images": self._get_products_with_images_count(db), } - for stat in marketplace_stats - ] - logger.info( - f"Generated marketplace breakdown stats for {len(stats_list)} marketplaces" - ) - return stats_list + return stats - def get_product_count(self, db: Session) -> int: + except Exception as e: + logger.error(f"Error getting product statistics: {str(e)}") + raise ValidationException("Failed to retrieve product statistics") + + def get_stock_statistics(self, db: Session) -> Dict[str, Any]: + """ + Get stock-related statistics. + + Args: + db: Database session + + Returns: + Dictionary containing stock statistics + """ + try: + return self._get_stock_statistics(db) + + except Exception as e: + logger.error(f"Error getting stock statistics: {str(e)}") + raise ValidationException("Failed to retrieve stock statistics") + + def get_marketplace_details(self, db: Session, marketplace: str) -> Dict[str, Any]: + """ + Get detailed statistics for a specific marketplace. + + Args: + db: Database session + marketplace: Marketplace name + + Returns: + Dictionary containing marketplace details + """ + try: + if not marketplace or not marketplace.strip(): + raise ValidationException("Marketplace name is required") + + product_count = self._get_products_by_marketplace_count(db, marketplace) + brands = self._get_brands_by_marketplace(db, marketplace) + shops = self._get_shops_by_marketplace(db, marketplace) + + return { + "marketplace": marketplace, + "total_products": product_count, + "unique_brands": len(brands), + "unique_shops": len(shops), + "brands": brands, + "shops": shops, + } + + except ValidationException: + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting marketplace details for {marketplace}: {str(e)}") + raise ValidationException("Failed to retrieve marketplace details") + + # Private helper methods + def _get_product_count(self, db: Session) -> int: """Get total product count.""" return db.query(Product).count() - def get_unique_brands_count(self, db: Session) -> int: + def _get_unique_brands_count(self, db: Session) -> int: """Get count of unique brands.""" return ( db.query(Product.brand) @@ -137,7 +206,7 @@ class StatsService: .count() ) - def get_unique_categories_count(self, db: Session) -> int: + def _get_unique_categories_count(self, db: Session) -> int: """Get count of unique categories.""" return ( db.query(Product.google_product_category) @@ -149,7 +218,7 @@ class StatsService: .count() ) - def get_unique_marketplaces_count(self, db: Session) -> int: + def _get_unique_marketplaces_count(self, db: Session) -> int: """Get count of unique marketplaces.""" return ( db.query(Product.marketplace) @@ -158,7 +227,7 @@ class StatsService: .count() ) - def get_unique_shops_count(self, db: Session) -> int: + def _get_unique_shops_count(self, db: Session) -> int: """Get count of unique shops.""" return ( db.query(Product.shop_name) @@ -167,16 +236,24 @@ class StatsService: .count() ) - def get_stock_statistics(self, db: Session) -> Dict[str, int]: - """ - Get stock-related statistics. + def _get_products_with_gtin_count(self, db: Session) -> int: + """Get count of products with GTIN.""" + return ( + db.query(Product) + .filter(Product.gtin.isnot(None), Product.gtin != "") + .count() + ) - Args: - db: Database session + def _get_products_with_images_count(self, db: Session) -> int: + """Get count of products with images.""" + return ( + db.query(Product) + .filter(Product.image_link.isnot(None), Product.image_link != "") + .count() + ) - Returns: - Dictionary containing stock statistics - """ + def _get_stock_statistics(self, db: Session) -> Dict[str, int]: + """Get stock-related statistics.""" total_stock_entries = db.query(Stock).count() total_inventory = db.query(func.sum(Stock.quantity)).scalar() or 0 @@ -185,7 +262,7 @@ class StatsService: "total_inventory_quantity": total_inventory, } - def get_brands_by_marketplace(self, db: Session, marketplace: str) -> List[str]: + def _get_brands_by_marketplace(self, db: Session, marketplace: str) -> List[str]: """Get unique brands for a specific marketplace.""" brands = ( db.query(Product.brand) @@ -199,7 +276,7 @@ class StatsService: ) return [brand[0] for brand in brands] - def get_shops_by_marketplace(self, db: Session, marketplace: str) -> List[str]: + def _get_shops_by_marketplace(self, db: Session, marketplace: str) -> List[str]: """Get unique shops for a specific marketplace.""" shops = ( db.query(Product.shop_name) @@ -213,10 +290,51 @@ class StatsService: ) return [shop[0] for shop in shops] - def get_products_by_marketplace(self, db: Session, marketplace: str) -> int: + def _get_products_by_marketplace_count(self, db: Session, marketplace: str) -> int: """Get product count for a specific marketplace.""" return db.query(Product).filter(Product.marketplace == marketplace).count() + # Legacy methods for backward compatibility (deprecated) + def get_product_count(self, db: Session) -> int: + """Get total product count. DEPRECATED: Use _get_product_count.""" + logger.warning("get_product_count is deprecated, use _get_product_count") + return self._get_product_count(db) + + def get_unique_brands_count(self, db: Session) -> int: + """Get unique brands count. DEPRECATED: Use _get_unique_brands_count.""" + logger.warning("get_unique_brands_count is deprecated, use _get_unique_brands_count") + return self._get_unique_brands_count(db) + + def get_unique_categories_count(self, db: Session) -> int: + """Get unique categories count. DEPRECATED: Use _get_unique_categories_count.""" + logger.warning("get_unique_categories_count is deprecated, use _get_unique_categories_count") + return self._get_unique_categories_count(db) + + def get_unique_marketplaces_count(self, db: Session) -> int: + """Get unique marketplaces count. DEPRECATED: Use _get_unique_marketplaces_count.""" + logger.warning("get_unique_marketplaces_count is deprecated, use _get_unique_marketplaces_count") + return self._get_unique_marketplaces_count(db) + + def get_unique_shops_count(self, db: Session) -> int: + """Get unique shops count. DEPRECATED: Use _get_unique_shops_count.""" + logger.warning("get_unique_shops_count is deprecated, use _get_unique_shops_count") + return self._get_unique_shops_count(db) + + def get_brands_by_marketplace(self, db: Session, marketplace: str) -> List[str]: + """Get brands by marketplace. DEPRECATED: Use _get_brands_by_marketplace.""" + logger.warning("get_brands_by_marketplace is deprecated, use _get_brands_by_marketplace") + return self._get_brands_by_marketplace(db, marketplace) + + def get_shops_by_marketplace(self, db: Session, marketplace: str) -> List[str]: + """Get shops by marketplace. DEPRECATED: Use _get_shops_by_marketplace.""" + logger.warning("get_shops_by_marketplace is deprecated, use _get_shops_by_marketplace") + return self._get_shops_by_marketplace(db, marketplace) + + def get_products_by_marketplace(self, db: Session, marketplace: str) -> int: + """Get products by marketplace. DEPRECATED: Use _get_products_by_marketplace_count.""" + logger.warning("get_products_by_marketplace is deprecated, use _get_products_by_marketplace_count") + return self._get_products_by_marketplace_count(db, marketplace) + # Create service instance following the same pattern as other services stats_service = StatsService() diff --git a/app/services/stock_service.py b/app/services/stock_service.py index b4fe45cb..fe9aa52a 100644 --- a/app/services/stock_service.py +++ b/app/services/stock_service.py @@ -1,10 +1,12 @@ # app/services/stock_service.py -"""Summary description .... +""" +Stock service for managing inventory operations. This module provides classes and functions for: -- .... -- .... -- .... +- Stock quantity management (set, add, remove) +- Stock information retrieval and validation +- Location-based inventory tracking +- GTIN normalization and validation """ import logging @@ -13,6 +15,15 @@ from typing import List, Optional from sqlalchemy.orm import Session +from app.exceptions import ( + StockNotFoundException, + InsufficientStockException, + InvalidStockOperationException, + StockValidationException, + NegativeStockException, + InvalidQuantityException, + ValidationException, +) from models.schemas.stock import (StockAdd, StockCreate, StockLocationResponse, StockSummaryResponse, StockUpdate) from models.database.product import Product @@ -29,236 +40,544 @@ class StockService: """Class constructor.""" self.gtin_processor = GTINProcessor() - def normalize_gtin(self, gtin_value) -> Optional[str]: - """Normalize GTIN format using the GTINProcessor.""" - return self.gtin_processor.normalize(gtin_value) - def set_stock(self, db: Session, stock_data: StockCreate) -> Stock: - """Set exact stock quantity for a GTIN at a specific location (replaces existing quantity).""" - normalized_gtin = self.normalize_gtin(stock_data.gtin) - if not normalized_gtin: - raise ValueError("Invalid GTIN format") + """ + Set exact stock quantity for a GTIN at a specific location (replaces existing quantity). - location = stock_data.location.strip().upper() + Args: + db: Database session + stock_data: Stock creation data - # Check if stock entry already exists for this GTIN and location - existing_stock = ( - db.query(Stock) - .filter(Stock.gtin == normalized_gtin, Stock.location == location) - .first() - ) + Returns: + Stock object with updated quantity - if existing_stock: - # Update existing stock (SET to exact quantity) - old_quantity = existing_stock.quantity - existing_stock.quantity = stock_data.quantity - existing_stock.updated_at = datetime.utcnow() - db.commit() - db.refresh(existing_stock) - logger.info( - f"Updated stock for GTIN {normalized_gtin} at {location}: {old_quantity} → {stock_data.quantity}" - ) - return existing_stock - else: - # Create new stock entry - new_stock = Stock( - gtin=normalized_gtin, location=location, quantity=stock_data.quantity - ) - db.add(new_stock) - db.commit() - db.refresh(new_stock) - logger.info( - f"Created new stock for GTIN {normalized_gtin} at {location}: {stock_data.quantity}" - ) - return new_stock + Raises: + InvalidQuantityException: If quantity is negative + StockValidationException: If GTIN or location is invalid + """ + try: + # Validate and normalize input + normalized_gtin = self._validate_and_normalize_gtin(stock_data.gtin) + location = self._validate_and_normalize_location(stock_data.location) + self._validate_quantity(stock_data.quantity, allow_zero=True) + + # Check if stock entry already exists for this GTIN and location + existing_stock = self._get_stock_entry(db, normalized_gtin, location) + + if existing_stock: + # Update existing stock (SET to exact quantity) + old_quantity = existing_stock.quantity + existing_stock.quantity = stock_data.quantity + existing_stock.updated_at = datetime.utcnow() + db.commit() + db.refresh(existing_stock) + + logger.info( + f"Updated stock for GTIN {normalized_gtin} at {location}: {old_quantity} → {stock_data.quantity}" + ) + return existing_stock + else: + # Create new stock entry + new_stock = Stock( + gtin=normalized_gtin, + location=location, + quantity=stock_data.quantity + ) + db.add(new_stock) + db.commit() + db.refresh(new_stock) + + logger.info( + f"Created new stock for GTIN {normalized_gtin} at {location}: {stock_data.quantity}" + ) + return new_stock + + except (InvalidQuantityException, StockValidationException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error setting stock: {str(e)}") + raise ValidationException("Failed to set stock") def add_stock(self, db: Session, stock_data: StockAdd) -> Stock: - """Add quantity to existing stock for a GTIN at a specific location (adds to existing quantity).""" - normalized_gtin = self.normalize_gtin(stock_data.gtin) - if not normalized_gtin: - raise ValueError("Invalid GTIN format") + """ + Add quantity to existing stock for a GTIN at a specific location (adds to existing quantity). - location = stock_data.location.strip().upper() + Args: + db: Database session + stock_data: Stock addition data - # Check if stock entry already exists for this GTIN and location - existing_stock = ( - db.query(Stock) - .filter(Stock.gtin == normalized_gtin, Stock.location == location) - .first() - ) + Returns: + Stock object with updated quantity - if existing_stock: - # Add to existing stock + Raises: + InvalidQuantityException: If quantity is not positive + StockValidationException: If GTIN or location is invalid + """ + try: + # Validate and normalize input + normalized_gtin = self._validate_and_normalize_gtin(stock_data.gtin) + location = self._validate_and_normalize_location(stock_data.location) + self._validate_quantity(stock_data.quantity, allow_zero=False) + + # Check if stock entry already exists for this GTIN and location + existing_stock = self._get_stock_entry(db, normalized_gtin, location) + + if existing_stock: + # Add to existing stock + old_quantity = existing_stock.quantity + existing_stock.quantity += stock_data.quantity + existing_stock.updated_at = datetime.utcnow() + db.commit() + db.refresh(existing_stock) + + logger.info( + f"Added stock for GTIN {normalized_gtin} at {location}: " + f"{old_quantity} + {stock_data.quantity} = {existing_stock.quantity}" + ) + return existing_stock + else: + # Create new stock entry with the quantity + new_stock = Stock( + gtin=normalized_gtin, + location=location, + quantity=stock_data.quantity + ) + db.add(new_stock) + db.commit() + db.refresh(new_stock) + + logger.info( + f"Created new stock for GTIN {normalized_gtin} at {location}: {stock_data.quantity}" + ) + return new_stock + + except (InvalidQuantityException, StockValidationException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error adding stock: {str(e)}") + raise ValidationException("Failed to add stock") + + def remove_stock(self, db: Session, stock_data: StockAdd) -> Stock: + """ + Remove quantity from existing stock for a GTIN at a specific location. + + Args: + db: Database session + stock_data: Stock removal data + + Returns: + Stock object with updated quantity + + Raises: + StockNotFoundException: If no stock found for GTIN/location + InsufficientStockException: If not enough stock available + InvalidQuantityException: If quantity is not positive + NegativeStockException: If operation would result in negative stock + """ + try: + # Validate and normalize input + normalized_gtin = self._validate_and_normalize_gtin(stock_data.gtin) + location = self._validate_and_normalize_location(stock_data.location) + self._validate_quantity(stock_data.quantity, allow_zero=False) + + # Find existing stock entry + existing_stock = self._get_stock_entry(db, normalized_gtin, location) + if not existing_stock: + raise StockNotFoundException(normalized_gtin, identifier_type="gtin") + + # Check if we have enough stock to remove + if existing_stock.quantity < stock_data.quantity: + raise InsufficientStockException( + gtin=normalized_gtin, + location=location, + requested=stock_data.quantity, + available=existing_stock.quantity + ) + + # Remove from existing stock old_quantity = existing_stock.quantity - existing_stock.quantity += stock_data.quantity + new_quantity = existing_stock.quantity - stock_data.quantity + + # Validate resulting quantity + if new_quantity < 0: + raise NegativeStockException(normalized_gtin, location, new_quantity) + + existing_stock.quantity = new_quantity existing_stock.updated_at = datetime.utcnow() db.commit() db.refresh(existing_stock) + logger.info( - f"Added stock for GTIN {normalized_gtin} at {location}: " - f"{old_quantity} + {stock_data.quantity} = {existing_stock.quantity}" + f"Removed stock for GTIN {normalized_gtin} at {location}: " + f"{old_quantity} - {stock_data.quantity} = {existing_stock.quantity}" ) return existing_stock - else: - # Create new stock entry with the quantity - new_stock = Stock( - gtin=normalized_gtin, location=location, quantity=stock_data.quantity + + except (StockNotFoundException, InsufficientStockException, InvalidQuantityException, NegativeStockException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error removing stock: {str(e)}") + raise ValidationException("Failed to remove stock") + + def get_stock_by_gtin(self, db: Session, gtin: str) -> StockSummaryResponse: + """ + Get all stock locations and total quantity for a specific GTIN. + + Args: + db: Database session + gtin: GTIN to look up + + Returns: + StockSummaryResponse with locations and totals + + Raises: + StockNotFoundException: If no stock found for GTIN + StockValidationException: If GTIN is invalid + """ + try: + normalized_gtin = self._validate_and_normalize_gtin(gtin) + + # Get all stock entries for this GTIN + stock_entries = db.query(Stock).filter(Stock.gtin == normalized_gtin).all() + + if not stock_entries: + raise StockNotFoundException(normalized_gtin, identifier_type="gtin") + + # Calculate total quantity and build locations list + total_quantity = 0 + locations = [] + + for entry in stock_entries: + total_quantity += entry.quantity + locations.append( + StockLocationResponse(location=entry.location, quantity=entry.quantity) + ) + + # Try to get product title for reference + product = db.query(Product).filter(Product.gtin == normalized_gtin).first() + product_title = product.title if product else None + + return StockSummaryResponse( + gtin=normalized_gtin, + total_quantity=total_quantity, + locations=locations, + product_title=product_title, ) - db.add(new_stock) + + except (StockNotFoundException, StockValidationException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting stock by GTIN {gtin}: {str(e)}") + raise ValidationException("Failed to retrieve stock information") + + def get_total_stock(self, db: Session, gtin: str) -> dict: + """ + Get total quantity in stock for a specific GTIN. + + Args: + db: Database session + gtin: GTIN to look up + + Returns: + Dictionary with total stock information + + Raises: + StockNotFoundException: If no stock found for GTIN + StockValidationException: If GTIN is invalid + """ + try: + normalized_gtin = self._validate_and_normalize_gtin(gtin) + + # Calculate total stock + total_stock = db.query(Stock).filter(Stock.gtin == normalized_gtin).all() + + if not total_stock: + raise StockNotFoundException(normalized_gtin, identifier_type="gtin") + + total_quantity = sum(entry.quantity for entry in total_stock) + + # Get product info for context + product = db.query(Product).filter(Product.gtin == normalized_gtin).first() + + return { + "gtin": normalized_gtin, + "total_quantity": total_quantity, + "product_title": product.title if product else None, + "locations_count": len(total_stock), + } + + except (StockNotFoundException, StockValidationException): + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting total stock for GTIN {gtin}: {str(e)}") + raise ValidationException("Failed to retrieve total stock") + + def get_all_stock( + self, + db: Session, + skip: int = 0, + limit: int = 100, + location: Optional[str] = None, + gtin: Optional[str] = None, + ) -> List[Stock]: + """ + Get all stock entries with optional filtering. + + Args: + db: Database session + skip: Number of records to skip + limit: Maximum records to return + location: Optional location filter + gtin: Optional GTIN filter + + Returns: + List of Stock objects + """ + try: + query = db.query(Stock) + + if location: + query = query.filter(Stock.location.ilike(f"%{location}%")) + + if gtin: + normalized_gtin = self._normalize_gtin(gtin) + if normalized_gtin: + query = query.filter(Stock.gtin == normalized_gtin) + + return query.offset(skip).limit(limit).all() + + except Exception as e: + logger.error(f"Error getting all stock: {str(e)}") + raise ValidationException("Failed to retrieve stock entries") + + def update_stock( + self, db: Session, stock_id: int, stock_update: StockUpdate + ) -> Stock: + """ + Update stock quantity for a specific stock entry. + + Args: + db: Database session + stock_id: Stock entry ID + stock_update: Update data + + Returns: + Updated Stock object + + Raises: + StockNotFoundException: If stock entry not found + InvalidQuantityException: If quantity is invalid + """ + try: + stock_entry = self._get_stock_by_id_or_raise(db, stock_id) + + # Validate new quantity + self._validate_quantity(stock_update.quantity, allow_zero=True) + + stock_entry.quantity = stock_update.quantity + stock_entry.updated_at = datetime.utcnow() db.commit() - db.refresh(new_stock) + db.refresh(stock_entry) + logger.info( - f"Created new stock for GTIN {normalized_gtin} at {location}: {stock_data.quantity}" + f"Updated stock entry {stock_id} to quantity {stock_update.quantity}" ) - return new_stock + return stock_entry - def remove_stock(self, db: Session, stock_data: StockAdd) -> Stock: - """Remove quantity from existing stock for a GTIN at a specific location.""" - normalized_gtin = self.normalize_gtin(stock_data.gtin) + except (StockNotFoundException, InvalidQuantityException): + db.rollback() + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error updating stock {stock_id}: {str(e)}") + raise ValidationException("Failed to update stock") + + def delete_stock(self, db: Session, stock_id: int) -> bool: + """ + Delete a stock entry. + + Args: + db: Database session + stock_id: Stock entry ID + + Returns: + True if deletion successful + + Raises: + StockNotFoundException: If stock entry not found + """ + try: + stock_entry = self._get_stock_by_id_or_raise(db, stock_id) + + gtin = stock_entry.gtin + location = stock_entry.location + db.delete(stock_entry) + db.commit() + + logger.info(f"Deleted stock entry {stock_id} for GTIN {gtin} at {location}") + return True + + except StockNotFoundException: + raise # Re-raise custom exceptions + except Exception as e: + db.rollback() + logger.error(f"Error deleting stock {stock_id}: {str(e)}") + raise ValidationException("Failed to delete stock entry") + + def get_stock_summary_by_location(self, db: Session, location: str) -> dict: + """ + Get stock summary for a specific location. + + Args: + db: Database session + location: Location to summarize + + Returns: + Dictionary with location stock summary + """ + try: + normalized_location = self._validate_and_normalize_location(location) + + stock_entries = db.query(Stock).filter(Stock.location == normalized_location).all() + + if not stock_entries: + return { + "location": normalized_location, + "total_items": 0, + "total_quantity": 0, + "unique_gtins": 0, + } + + total_quantity = sum(entry.quantity for entry in stock_entries) + unique_gtins = len(set(entry.gtin for entry in stock_entries)) + + return { + "location": normalized_location, + "total_items": len(stock_entries), + "total_quantity": total_quantity, + "unique_gtins": unique_gtins, + } + + except StockValidationException: + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting stock summary for location {location}: {str(e)}") + raise ValidationException("Failed to retrieve location stock summary") + + def get_low_stock_items(self, db: Session, threshold: int = 10) -> List[dict]: + """ + Get items with stock below threshold. + + Args: + db: Database session + threshold: Stock threshold to consider "low" + + Returns: + List of low stock items with details + """ + try: + if threshold < 0: + raise InvalidQuantityException(threshold, "Threshold must be non-negative") + + low_stock_entries = db.query(Stock).filter(Stock.quantity <= threshold).all() + + low_stock_items = [] + for entry in low_stock_entries: + # Get product info if available + product = db.query(Product).filter(Product.gtin == entry.gtin).first() + + low_stock_items.append({ + "gtin": entry.gtin, + "location": entry.location, + "current_quantity": entry.quantity, + "product_title": product.title if product else None, + "product_id": product.product_id if product else None, + }) + + return low_stock_items + + except InvalidQuantityException: + raise # Re-raise custom exceptions + except Exception as e: + logger.error(f"Error getting low stock items: {str(e)}") + raise ValidationException("Failed to retrieve low stock items") + + # Private helper methods + def _validate_and_normalize_gtin(self, gtin: str) -> str: + """Validate and normalize GTIN format.""" + if not gtin or not gtin.strip(): + raise StockValidationException("GTIN is required", field="gtin") + + normalized_gtin = self._normalize_gtin(gtin) if not normalized_gtin: - raise ValueError("Invalid GTIN format") + raise StockValidationException("Invalid GTIN format", field="gtin") - location = stock_data.location.strip().upper() + return normalized_gtin - # Find existing stock entry - existing_stock = ( + def _validate_and_normalize_location(self, location: str) -> str: + """Validate and normalize location.""" + if not location or not location.strip(): + raise StockValidationException("Location is required", field="location") + + return location.strip().upper() + + def _validate_quantity(self, quantity: int, allow_zero: bool = True) -> None: + """Validate quantity value.""" + if quantity is None: + raise InvalidQuantityException(quantity, "Quantity is required") + + if not isinstance(quantity, int): + raise InvalidQuantityException(quantity, "Quantity must be an integer") + + if quantity < 0: + raise InvalidQuantityException(quantity, "Quantity cannot be negative") + + if not allow_zero and quantity == 0: + raise InvalidQuantityException(quantity, "Quantity must be positive") + + def _normalize_gtin(self, gtin_value) -> Optional[str]: + """Normalize GTIN format using the GTINProcessor.""" + try: + return self.gtin_processor.normalize(gtin_value) + except Exception as e: + logger.error(f"Error normalizing GTIN {gtin_value}: {str(e)}") + return None + + def _get_stock_entry(self, db: Session, gtin: str, location: str) -> Optional[Stock]: + """Get stock entry by GTIN and location.""" + return ( db.query(Stock) - .filter(Stock.gtin == normalized_gtin, Stock.location == location) + .filter(Stock.gtin == gtin, Stock.location == location) .first() ) - if not existing_stock: - raise ValueError( - f"No stock found for GTIN {normalized_gtin} at location {location}" - ) - - # Check if we have enough stock to remove - if existing_stock.quantity < stock_data.quantity: - raise ValueError( - f"Insufficient stock. Available: {existing_stock.quantity}, Requested to remove: {stock_data.quantity}" - ) - - # Remove from existing stock - old_quantity = existing_stock.quantity - existing_stock.quantity -= stock_data.quantity - existing_stock.updated_at = datetime.utcnow() - db.commit() - db.refresh(existing_stock) - logger.info( - f"Removed stock for GTIN {normalized_gtin} at {location}: " - f"{old_quantity} - {stock_data.quantity} = {existing_stock.quantity}" - ) - return existing_stock - - def get_stock_by_gtin(self, db: Session, gtin: str) -> StockSummaryResponse: - """Get all stock locations and total quantity for a specific GTIN.""" - normalized_gtin = self.normalize_gtin(gtin) - if not normalized_gtin: - raise ValueError("Invalid GTIN format") - - # Get all stock entries for this GTIN - stock_entries = db.query(Stock).filter(Stock.gtin == normalized_gtin).all() - - if not stock_entries: - raise ValueError(f"No stock found for GTIN: {gtin}") - - # Calculate total quantity and build locations list - total_quantity = 0 - locations = [] - - for entry in stock_entries: - total_quantity += entry.quantity - locations.append( - StockLocationResponse(location=entry.location, quantity=entry.quantity) - ) - - # Try to get product title for reference - product = db.query(Product).filter(Product.gtin == normalized_gtin).first() - product_title = product.title if product else None - - return StockSummaryResponse( - gtin=normalized_gtin, - total_quantity=total_quantity, - locations=locations, - product_title=product_title, - ) - - def get_total_stock(self, db: Session, gtin: str) -> dict: - """Get total quantity in stock for a specific GTIN.""" - normalized_gtin = self.normalize_gtin(gtin) - if not normalized_gtin: - raise ValueError("Invalid GTIN format") - - # Calculate total stock - total_stock = db.query(Stock).filter(Stock.gtin == normalized_gtin).all() - total_quantity = sum(entry.quantity for entry in total_stock) - - # Get product info for context - product = db.query(Product).filter(Product.gtin == normalized_gtin).first() - - return { - "gtin": normalized_gtin, - "total_quantity": total_quantity, - "product_title": product.title if product else None, - "locations_count": len(total_stock), - } - - def get_all_stock( - self, - db: Session, - skip: int = 0, - limit: int = 100, - location: Optional[str] = None, - gtin: Optional[str] = None, - ) -> List[Stock]: - """Get all stock entries with optional filtering.""" - query = db.query(Stock) - - if location: - query = query.filter(Stock.location.ilike(f"%{location}%")) - - if gtin: - normalized_gtin = self.normalize_gtin(gtin) - if normalized_gtin: - query = query.filter(Stock.gtin == normalized_gtin) - - return query.offset(skip).limit(limit).all() - - def update_stock( - self, db: Session, stock_id: int, stock_update: StockUpdate - ) -> Stock: - """Update stock quantity for a specific stock entry.""" + def _get_stock_by_id_or_raise(self, db: Session, stock_id: int) -> Stock: + """Get stock by ID or raise exception.""" stock_entry = db.query(Stock).filter(Stock.id == stock_id).first() if not stock_entry: - raise ValueError("Stock entry not found") - - stock_entry.quantity = stock_update.quantity - stock_entry.updated_at = datetime.utcnow() - db.commit() - db.refresh(stock_entry) - - logger.info( - f"Updated stock entry {stock_id} to quantity {stock_update.quantity}" - ) + raise StockNotFoundException(str(stock_id)) return stock_entry - def delete_stock(self, db: Session, stock_id: int) -> bool: - """Delete a stock entry.""" - stock_entry = db.query(Stock).filter(Stock.id == stock_id).first() - if not stock_entry: - raise ValueError("Stock entry not found") - - gtin = stock_entry.gtin - location = stock_entry.location - db.delete(stock_entry) - db.commit() - - logger.info(f"Deleted stock entry {stock_id} for GTIN {gtin} at {location}") - return True + # Legacy methods for backward compatibility (deprecated) + def normalize_gtin(self, gtin_value) -> Optional[str]: + """Normalize GTIN format. DEPRECATED: Use _normalize_gtin.""" + logger.warning("normalize_gtin is deprecated, use _normalize_gtin") + return self._normalize_gtin(gtin_value) def get_stock_by_id(self, db: Session, stock_id: int) -> Optional[Stock]: - """Get a stock entry by its ID.""" - return db.query(Stock).filter(Stock.id == stock_id).first() + """Get stock by ID. DEPRECATED: Use proper exception handling.""" + logger.warning("get_stock_by_id is deprecated, use proper exception handling") + try: + return db.query(Stock).filter(Stock.id == stock_id).first() + except Exception as e: + logger.error(f"Error getting stock by ID: {str(e)}") + return None # Create service instance diff --git a/docs/development/frontend-exception-handling.md b/docs/development/frontend-exception-handling.md new file mode 100644 index 00000000..c6a67b9d --- /dev/null +++ b/docs/development/frontend-exception-handling.md @@ -0,0 +1,577 @@ +# Frontend Exception Handling Guide + +## Understanding Your API's Exception Structure + +Your API returns consistent error responses with this structure: + +```json +{ + "error_code": "PRODUCT_NOT_FOUND", + "message": "Product with ID 'ABC123' not found", + "status_code": 404, + "field": "product_id", + "details": { + "resource_type": "Product", + "identifier": "ABC123" + } +} +``` + +## Frontend Error Handling Strategy + +### 1. HTTP Client Configuration + +Configure your HTTP client to handle error responses consistently: + +#### Axios Example +```javascript +// api/client.js +import axios from 'axios'; + +const apiClient = axios.create({ + baseURL: 'https://your-api.com/api/v1', + timeout: 10000, +}); + +// Response interceptor for error handling +apiClient.interceptors.response.use( + (response) => response, + (error) => { + if (error.response?.data) { + // Transform API error to standardized format + const apiError = { + errorCode: error.response.data.error_code, + message: error.response.data.message, + statusCode: error.response.data.status_code, + field: error.response.data.field, + details: error.response.data.details, + originalError: error + }; + throw apiError; + } + + // Handle network errors + if (error.code === 'ECONNABORTED') { + throw { + errorCode: 'NETWORK_TIMEOUT', + message: 'Request timed out. Please try again.', + statusCode: 408 + }; + } + + throw { + errorCode: 'NETWORK_ERROR', + message: 'Network error. Please check your connection.', + statusCode: 0 + }; + } +); + +export default apiClient; +``` + +#### Fetch Example +```javascript +// api/client.js +class ApiClient { + constructor(baseURL) { + this.baseURL = baseURL; + } + + async request(endpoint, options = {}) { + try { + const response = await fetch(`${this.baseURL}${endpoint}`, { + headers: { + 'Content-Type': 'application/json', + ...options.headers, + }, + ...options, + }); + + const data = await response.json(); + + if (!response.ok) { + throw { + errorCode: data.error_code, + message: data.message, + statusCode: data.status_code, + field: data.field, + details: data.details, + }; + } + + return data; + } catch (error) { + if (error.errorCode) { + throw error; // Re-throw API errors + } + + // Handle network errors + throw { + errorCode: 'NETWORK_ERROR', + message: 'Failed to connect to server', + statusCode: 0 + }; + } + } +} + +export const apiClient = new ApiClient('https://your-api.com/api/v1'); +``` + +### 2. Error Code Mapping + +Create mappings for user-friendly messages: + +```javascript +// constants/errorMessages.js +export const ERROR_MESSAGES = { + // Authentication errors + INVALID_CREDENTIALS: 'Invalid username or password. Please try again.', + TOKEN_EXPIRED: 'Your session has expired. Please log in again.', + USER_NOT_ACTIVE: 'Your account has been deactivated. Contact support.', + + // Product errors + PRODUCT_NOT_FOUND: 'Product not found. It may have been removed.', + PRODUCT_ALREADY_EXISTS: 'A product with this ID already exists.', + INVALID_PRODUCT_DATA: 'Please check the product information and try again.', + + // Stock errors + INSUFFICIENT_STOCK: 'Not enough stock available for this operation.', + STOCK_NOT_FOUND: 'No stock information found for this product.', + NEGATIVE_STOCK_NOT_ALLOWED: 'Stock quantity cannot be negative.', + + // Shop errors + SHOP_NOT_FOUND: 'Shop not found or no longer available.', + UNAUTHORIZED_SHOP_ACCESS: 'You do not have permission to access this shop.', + SHOP_ALREADY_EXISTS: 'A shop with this code already exists.', + MAX_SHOPS_REACHED: 'You have reached the maximum number of shops allowed.', + + // Import errors + IMPORT_JOB_NOT_FOUND: 'Import job not found.', + IMPORT_JOB_CANNOT_BE_CANCELLED: 'This import job cannot be cancelled at this time.', + MARKETPLACE_CONNECTION_FAILED: 'Failed to connect to marketplace. Please try again.', + + // Generic fallbacks + VALIDATION_ERROR: 'Please check your input and try again.', + NETWORK_ERROR: 'Connection error. Please check your internet connection.', + NETWORK_TIMEOUT: 'Request timed out. Please try again.', + INTERNAL_SERVER_ERROR: 'Something went wrong. Please try again later.', +}; + +export const getErrorMessage = (errorCode, fallbackMessage = null) => { + return ERROR_MESSAGES[errorCode] || fallbackMessage || 'An unexpected error occurred.'; +}; +``` + +### 3. Component Error Handling + +#### React Hook for Error Handling +```javascript +// hooks/useApiError.js +import { useState } from 'react'; +import { getErrorMessage } from '../constants/errorMessages'; + +export const useApiError = () => { + const [error, setError] = useState(null); + const [isLoading, setIsLoading] = useState(false); + + const handleApiCall = async (apiCall) => { + setIsLoading(true); + setError(null); + + try { + const result = await apiCall(); + setIsLoading(false); + return result; + } catch (apiError) { + setIsLoading(false); + setError({ + code: apiError.errorCode, + message: getErrorMessage(apiError.errorCode, apiError.message), + field: apiError.field, + details: apiError.details + }); + throw apiError; // Re-throw for component-specific handling + } + }; + + const clearError = () => setError(null); + + return { error, isLoading, handleApiCall, clearError }; +}; +``` + +#### Form Error Handling +```javascript +// components/ProductForm.jsx +import React, { useState } from 'react'; +import { useApiError } from '../hooks/useApiError'; +import { createProduct } from '../api/products'; + +const ProductForm = () => { + const [formData, setFormData] = useState({ + product_id: '', + name: '', + price: '' + }); + const [fieldErrors, setFieldErrors] = useState({}); + const { error, isLoading, handleApiCall, clearError } = useApiError(); + + const handleSubmit = async (e) => { + e.preventDefault(); + setFieldErrors({}); + clearError(); + + try { + await handleApiCall(() => createProduct(formData)); + // Success handling + alert('Product created successfully!'); + setFormData({ product_id: '', name: '', price: '' }); + } catch (apiError) { + // Handle field-specific errors + if (apiError.field) { + setFieldErrors({ [apiError.field]: apiError.message }); + } + + // Handle specific error codes + switch (apiError.errorCode) { + case 'PRODUCT_ALREADY_EXISTS': + setFieldErrors({ product_id: 'This product ID is already taken' }); + break; + case 'INVALID_PRODUCT_DATA': + if (apiError.field) { + setFieldErrors({ [apiError.field]: apiError.message }); + } + break; + default: + // Generic error is handled by useApiError hook + break; + } + } + }; + + return ( +
+ {error && !error.field && ( +
+ {error.message} +
+ )} + +
+ + setFormData({...formData, product_id: e.target.value})} + className={fieldErrors.product_id ? 'error' : ''} + /> + {fieldErrors.product_id && ( + {fieldErrors.product_id} + )} +
+ +
+ + setFormData({...formData, name: e.target.value})} + className={fieldErrors.name ? 'error' : ''} + /> + {fieldErrors.name && ( + {fieldErrors.name} + )} +
+ + +
+ ); +}; +``` + +### 4. Global Error Handler + +#### React Error Boundary +```javascript +// components/ErrorBoundary.jsx +import React from 'react'; + +class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false, error: null }; + } + + static getDerivedStateFromError(error) { + return { hasError: true, error }; + } + + componentDidCatch(error, errorInfo) { + console.error('Error caught by boundary:', error, errorInfo); + + // Log to error reporting service + if (window.errorReporting) { + window.errorReporting.captureException(error, { + extra: errorInfo + }); + } + } + + render() { + if (this.state.hasError) { + return ( +
+

Something went wrong

+

We're sorry, but something unexpected happened.

+ +
+ ); + } + + return this.props.children; + } +} +``` + +#### Global Toast Notifications +```javascript +// utils/notifications.js +class NotificationManager { + constructor() { + this.notifications = []; + this.listeners = []; + } + + subscribe(callback) { + this.listeners.push(callback); + return () => { + this.listeners = this.listeners.filter(l => l !== callback); + }; + } + + notify(type, message, options = {}) { + const notification = { + id: Date.now(), + type, // 'success', 'error', 'warning', 'info' + message, + duration: options.duration || 5000, + ...options + }; + + this.notifications.push(notification); + this.listeners.forEach(callback => callback(this.notifications)); + + if (notification.duration > 0) { + setTimeout(() => this.remove(notification.id), notification.duration); + } + } + + notifyError(apiError) { + const message = getErrorMessage(apiError.errorCode, apiError.message); + this.notify('error', message, { + errorCode: apiError.errorCode, + details: apiError.details + }); + } + + remove(id) { + this.notifications = this.notifications.filter(n => n.id !== id); + this.listeners.forEach(callback => callback(this.notifications)); + } +} + +export const notificationManager = new NotificationManager(); +``` + +### 5. Specific Error Handling Patterns + +#### Authentication Errors +```javascript +// api/auth.js +import { apiClient } from './client'; +import { notificationManager } from '../utils/notifications'; + +export const login = async (credentials) => { + try { + const response = await apiClient.post('/auth/login', credentials); + localStorage.setItem('token', response.access_token); + return response; + } catch (error) { + switch (error.errorCode) { + case 'INVALID_CREDENTIALS': + notificationManager.notify('error', 'Invalid username or password'); + break; + case 'USER_NOT_ACTIVE': + notificationManager.notify('error', 'Account deactivated. Contact support.'); + break; + default: + notificationManager.notifyError(error); + } + throw error; + } +}; + +// Handle token expiration globally +apiClient.interceptors.response.use( + (response) => response, + (error) => { + if (error.response?.data?.error_code === 'TOKEN_EXPIRED') { + localStorage.removeItem('token'); + window.location.href = '/login'; + notificationManager.notify('warning', 'Session expired. Please log in again.'); + } + throw error; + } +); +``` + +#### Stock Management Errors +```javascript +// components/StockManager.jsx +const handleStockUpdate = async (gtin, location, quantity) => { + try { + await updateStock(gtin, location, quantity); + notificationManager.notify('success', 'Stock updated successfully'); + } catch (error) { + switch (error.errorCode) { + case 'INSUFFICIENT_STOCK': + const { available_quantity, requested_quantity } = error.details; + notificationManager.notify('error', + `Cannot remove ${requested_quantity} items. Only ${available_quantity} available.` + ); + break; + case 'STOCK_NOT_FOUND': + notificationManager.notify('error', 'No stock record found for this product'); + break; + case 'NEGATIVE_STOCK_NOT_ALLOWED': + notificationManager.notify('error', 'Stock quantity cannot be negative'); + break; + default: + notificationManager.notifyError(error); + } + } +}; +``` + +#### Import Job Monitoring +```javascript +// components/ImportJobStatus.jsx +const ImportJobStatus = ({ jobId }) => { + const [job, setJob] = useState(null); + const { error, isLoading, handleApiCall } = useApiError(); + + const fetchJobStatus = useCallback(async () => { + try { + const jobData = await handleApiCall(() => getImportJobStatus(jobId)); + setJob(jobData); + } catch (error) { + switch (error.errorCode) { + case 'IMPORT_JOB_NOT_FOUND': + notificationManager.notify('error', 'Import job not found or has been deleted'); + break; + case 'IMPORT_JOB_NOT_OWNED': + notificationManager.notify('error', 'You do not have access to this import job'); + break; + default: + notificationManager.notifyError(error); + } + } + }, [jobId, handleApiCall]); + + const cancelJob = async () => { + try { + await handleApiCall(() => cancelImportJob(jobId)); + notificationManager.notify('success', 'Import job cancelled successfully'); + fetchJobStatus(); // Refresh status + } catch (error) { + switch (error.errorCode) { + case 'IMPORT_JOB_CANNOT_BE_CANCELLED': + const { current_status } = error.details; + notificationManager.notify('error', + `Cannot cancel job in ${current_status} status` + ); + break; + default: + notificationManager.notifyError(error); + } + } + }; + + return ( +
+ {/* Job status UI */} +
+ ); +}; +``` + +### 6. Error Logging and Monitoring + +```javascript +// utils/errorReporting.js +export const logError = (error, context = {}) => { + const errorData = { + timestamp: new Date().toISOString(), + errorCode: error.errorCode, + message: error.message, + statusCode: error.statusCode, + field: error.field, + details: error.details, + userAgent: navigator.userAgent, + url: window.location.href, + userId: getCurrentUserId(), + ...context + }; + + // Log to console in development + if (process.env.NODE_ENV === 'development') { + console.error('API Error:', errorData); + } + + // Send to error tracking service + if (window.errorTracker) { + window.errorTracker.captureException(error, { + tags: { + errorCode: error.errorCode, + statusCode: error.statusCode + }, + extra: errorData + }); + } + + // Store in local storage for offline analysis + try { + const existingErrors = JSON.parse(localStorage.getItem('apiErrors') || '[]'); + existingErrors.push(errorData); + + // Keep only last 50 errors + if (existingErrors.length > 50) { + existingErrors.splice(0, existingErrors.length - 50); + } + + localStorage.setItem('apiErrors', JSON.stringify(existingErrors)); + } catch (e) { + console.warn('Failed to store error locally:', e); + } +}; +``` + +### 7. Best Practices Summary + +1. **Consistent Error Structure**: Always expect the same error format from your API +2. **User-Friendly Messages**: Map error codes to readable messages +3. **Field-Specific Errors**: Handle validation errors at the field level +4. **Progressive Enhancement**: Start with basic error handling, add sophistication gradually +5. **Logging**: Track errors for debugging and improving user experience +6. **Fallback Handling**: Always have fallback messages for unknown error codes +7. **Loading States**: Show appropriate loading indicators during API calls +8. **Retry Logic**: Implement retry for transient network errors +9. **Offline Handling**: Consider offline scenarios and network recovery + +This approach gives you robust error handling that scales with your application while providing excellent user experience and debugging capabilities. diff --git a/main.py b/main.py index 4eb91027..d1fa1239 100644 --- a/main.py +++ b/main.py @@ -11,6 +11,8 @@ from app.api.main import api_router from app.core.config import settings from app.core.database import get_db from app.core.lifespan import lifespan +from app.exceptions.handler import setup_exception_handlers +from app.exceptions import ServiceUnavailableException logger = logging.getLogger(__name__) @@ -22,6 +24,9 @@ app = FastAPI( lifespan=lifespan, ) +# Setup custom exception handlers (unified approach) +setup_exception_handlers(app) + # Add CORS middleware app.add_middleware( CORSMiddleware, @@ -72,9 +77,7 @@ def health_check(db: Session = Depends(get_db)): } except Exception as e: logger.error(f"Health check failed: {e}") - raise HTTPException(status_code=503, detail="Service unhealthy") - - # Add this temporary endpoint to your router: + raise ServiceUnavailableException("Service unhealthy") # Documentation redirect endpoints diff --git a/middleware/auth.py b/middleware/auth.py index d916e6a2..ea9dded2 100644 --- a/middleware/auth.py +++ b/middleware/auth.py @@ -18,6 +18,13 @@ from jose import jwt from passlib.context import CryptContext from sqlalchemy.orm import Session +from app.exceptions import ( + AdminRequiredException, + InvalidTokenException, + TokenExpiredException, + UserNotActiveException, + InvalidCredentialsException +) from models.database.user import User logger = logging.getLogger(__name__) @@ -46,7 +53,7 @@ class AuthManager: return pwd_context.verify(plain_password, hashed_password) def authenticate_user( - self, db: Session, username: str, password: str + self, db: Session, username: str, password: str ) -> Optional[User]: """Authenticate user and return user object if valid.""" user = ( @@ -101,17 +108,15 @@ class AuthManager: # Check if token has expired exp = payload.get("exp") if exp is None: - raise HTTPException(status_code=401, detail="Token missing expiration") + raise InvalidTokenException("Token missing expiration") if datetime.utcnow() > datetime.fromtimestamp(exp): - raise HTTPException(status_code=401, detail="Token has expired") + raise TokenExpiredException() # Extract user data user_id = payload.get("sub") if user_id is None: - raise HTTPException( - status_code=401, detail="Token missing user identifier" - ) + raise InvalidTokenException("Token missing user identifier") return { "user_id": int(user_id), @@ -121,28 +126,24 @@ class AuthManager: } except jwt.ExpiredSignatureError: - raise HTTPException(status_code=401, detail="Token has expired") + raise TokenExpiredException() except jwt.JWTError as e: logger.error(f"JWT decode error: {e}") - raise HTTPException( - status_code=401, detail="Could not validate credentials" - ) + raise InvalidTokenException("Could not validate credentials") except Exception as e: logger.error(f"Token verification error: {e}") - raise HTTPException(status_code=401, detail="Authentication failed") + raise InvalidTokenException("Authentication failed") - def get_current_user( - self, db: Session, credentials: HTTPAuthorizationCredentials - ) -> User: + def get_current_user(self, db: Session, credentials: HTTPAuthorizationCredentials) -> User: """Get current authenticated user from database.""" user_data = self.verify_token(credentials.credentials) user = db.query(User).filter(User.id == user_data["user_id"]).first() if not user: - raise HTTPException(status_code=401, detail="User not found") + raise InvalidCredentialsException("User not found") if not user.is_active: - raise HTTPException(status_code=401, detail="User account is inactive") + raise UserNotActiveException() return user @@ -165,7 +166,7 @@ class AuthManager: def require_admin(self, current_user: User): """Require admin role.""" if current_user.role != "admin": - raise HTTPException(status_code=403, detail="Admin privileges required") + raise AdminRequiredException() return current_user def create_default_admin_user(self, db: Session): diff --git a/middleware/decorators.py b/middleware/decorators.py index bc687f7b..f068f6fe 100644 --- a/middleware/decorators.py +++ b/middleware/decorators.py @@ -1,36 +1,35 @@ # middleware/decorators.py -"""Summary description .... +""" +FastAPI decorators for cross-cutting concerns. This module provides classes and functions for: -- .... -- .... -- .... +- Rate limiting decorators for endpoint protection +- Request throttling and abuse prevention +- Consistent error handling for rate limit violations """ from functools import wraps - -from fastapi import HTTPException - +from app.exceptions.base import RateLimitException # Add this import from middleware.rate_limiter import RateLimiter # Initialize rate limiter instance rate_limiter = RateLimiter() - def rate_limit(max_requests: int = 100, window_seconds: int = 3600): """Rate limiting decorator for FastAPI endpoints.""" def decorator(func): @wraps(func) async def wrapper(*args, **kwargs): - # Extract client IP or user ID for rate limiting client_id = "anonymous" # In production, extract from request if not rate_limiter.allow_request(client_id, max_requests, window_seconds): - raise HTTPException(status_code=429, detail="Rate limit exceeded") + # Use custom exception instead of HTTPException + raise RateLimitException( + message="Rate limit exceeded", + retry_after=window_seconds + ) return await func(*args, **kwargs) - return wrapper - return decorator diff --git a/middleware/error_handler.py b/middleware/error_handler.py deleted file mode 100644 index dda6ca98..00000000 --- a/middleware/error_handler.py +++ /dev/null @@ -1,69 +0,0 @@ -# middleware/error_handler.py -"""Summary description .... - -This module provides classes and functions for: -- .... -- .... -- .... -""" - -import logging - -from fastapi import HTTPException, Request -from fastapi.exceptions import RequestValidationError -from fastapi.responses import JSONResponse - -logger = logging.getLogger(__name__) - - -async def custom_http_exception_handler(request: Request, exc: HTTPException): - """Handle HTTP exception.""" - logger.error( - f"HTTP {exc.status_code}: {exc.detail} - {request.method} {request.url}" - ) - - return JSONResponse( - status_code=exc.status_code, - content={ - "error": { - "code": exc.status_code, - "message": exc.detail, - "type": "http_exception", - } - }, - ) - - -async def validation_exception_handler(request: Request, exc: RequestValidationError): - """Handle Pydantic validation errors.""" - logger.error(f"Validation error: {exc.errors()} - {request.method} {request.url}") - - return JSONResponse( - status_code=422, - content={ - "error": { - "code": 422, - "message": "Validation error", - "type": "validation_error", - "details": exc.errors(), - } - }, - ) - - -async def general_exception_handler(request: Request, exc: Exception): - """Handle unexpected exceptions.""" - logger.error( - f"Unexpected error: {str(exc)} - {request.method} {request.url}", exc_info=True - ) - - return JSONResponse( - status_code=500, - content={ - "error": { - "code": 500, - "message": "Internal server error", - "type": "server_error", - } - }, - ) diff --git a/mkdocs.yml b/mkdocs.yml index d8b4efaa..b9e78e25 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -35,6 +35,8 @@ nav: - Services: development/services.md - Contributing: development/contributing.md - Pycharm configuration: development/pycharm-configuration.md + - Pycharm configuration: development/exception-handling.md + - Pycharm configuration: development/frontend-exception-handling.md - Deployment: - Overview: deployment/index.md - Docker: deployment/docker.md diff --git a/tests/fixtures/auth_fixtures.py b/tests/fixtures/auth_fixtures.py index afc339c3..1d2f462e 100644 --- a/tests/fixtures/auth_fixtures.py +++ b/tests/fixtures/auth_fixtures.py @@ -48,6 +48,22 @@ def test_admin(db, auth_manager): db.refresh(admin) return admin +@pytest.fixture +def another_admin(db, auth_manager): + """Create another test admin user for testing admin-to-admin interactions""" + unique_id = str(uuid.uuid4())[:8] # Short unique identifier + hashed_password = auth_manager.hash_password("anotheradminpass123") + admin = User( + email=f"another_admin_{unique_id}@example.com", + username=f"another_admin_{unique_id}", + hashed_password=hashed_password, + role="admin", + is_active=True, + ) + db.add(admin) + db.commit() + db.refresh(admin) + return admin @pytest.fixture def other_user(db, auth_manager): diff --git a/tests/integration/api/v1/test_admin_endpoints.py b/tests/integration/api/v1/test_admin_endpoints.py index 5e756e26..232f9aa2 100644 --- a/tests/integration/api/v1/test_admin_endpoints.py +++ b/tests/integration/api/v1/test_admin_endpoints.py @@ -23,10 +23,9 @@ class TestAdminAPI: response = client.get("/api/v1/admin/users", headers=auth_headers) assert response.status_code == 403 - assert ( - "Access denied" in response.json()["detail"] - or "admin" in response.json()["detail"].lower() - ) + data = response.json() + assert data["error_code"] == "ADMIN_REQUIRED" + assert "Admin privileges required" in data["message"] def test_toggle_user_status_admin(self, client, admin_headers, test_user): """Test admin toggling user status""" @@ -45,18 +44,35 @@ class TestAdminAPI: response = client.put("/api/v1/admin/users/99999/status", headers=admin_headers) assert response.status_code == 404 - assert "User not found" in response.json()["detail"] + data = response.json() + assert data["error_code"] == "USER_NOT_FOUND" + assert "User with ID '99999' not found" in data["message"] - def test_toggle_user_status_cannot_deactivate_self( + def test_toggle_user_status_cannot_modify_self( self, client, admin_headers, test_admin ): - """Test that admin cannot deactivate their own account""" + """Test that admin cannot modify their own account""" response = client.put( f"/api/v1/admin/users/{test_admin.id}/status", headers=admin_headers ) - assert response.status_code == 400 - assert "Cannot deactivate your own account" in response.json()["detail"] + assert response.status_code == 400 # Business logic error + data = response.json() + assert data["error_code"] == "CANNOT_MODIFY_SELF" + assert "Cannot perform 'deactivate account' on your own account" in data["message"] + + def test_toggle_user_status_cannot_modify_admin( + self, client, admin_headers, test_admin, another_admin + ): + """Test that admin cannot modify another admin""" + response = client.put( + f"/api/v1/admin/users/{another_admin.id}/status", headers=admin_headers + ) + + assert response.status_code == 400 # Business logic error + data = response.json() + assert data["error_code"] == "USER_STATUS_CHANGE_FAILED" + assert "Cannot modify another admin user" in data["message"] def test_get_all_shops_admin(self, client, admin_headers, test_shop): """Test admin getting all shops""" @@ -78,10 +94,8 @@ class TestAdminAPI: response = client.get("/api/v1/admin/shops", headers=auth_headers) assert response.status_code == 403 - assert ( - "Access denied" in response.json()["detail"] - or "admin" in response.json()["detail"].lower() - ) + data = response.json() + assert data["error_code"] == "ADMIN_REQUIRED" def test_verify_shop_admin(self, client, admin_headers, test_shop): """Test admin verifying/unverifying shop""" @@ -99,7 +113,9 @@ class TestAdminAPI: response = client.put("/api/v1/admin/shops/99999/verify", headers=admin_headers) assert response.status_code == 404 - assert "Shop not found" in response.json()["detail"] + data = response.json() + assert data["error_code"] == "SHOP_NOT_FOUND" + assert "Shop with ID '99999' not found" in data["message"] def test_toggle_shop_status_admin(self, client, admin_headers, test_shop): """Test admin toggling shop status""" @@ -117,7 +133,8 @@ class TestAdminAPI: response = client.put("/api/v1/admin/shops/99999/status", headers=admin_headers) assert response.status_code == 404 - assert "Shop not found" in response.json()["detail"] + data = response.json() + assert data["error_code"] == "SHOP_NOT_FOUND" def test_get_marketplace_import_jobs_admin( self, client, admin_headers, test_marketplace_job @@ -159,10 +176,32 @@ class TestAdminAPI: ) assert response.status_code == 403 - assert ( - "Access denied" in response.json()["detail"] - or "admin" in response.json()["detail"].lower() - ) + data = response.json() + assert data["error_code"] == "ADMIN_REQUIRED" + + def test_get_user_statistics(self, client, admin_headers): + """Test admin getting user statistics""" + response = client.get("/api/v1/admin/stats/users", headers=admin_headers) + + assert response.status_code == 200 + data = response.json() + assert "total_users" in data + assert "active_users" in data + assert "inactive_users" in data + assert "activation_rate" in data + assert isinstance(data["total_users"], int) + + def test_get_shop_statistics(self, client, admin_headers): + """Test admin getting shop statistics""" + response = client.get("/api/v1/admin/stats/shops", headers=admin_headers) + + assert response.status_code == 200 + data = response.json() + assert "total_shops" in data + assert "active_shops" in data + assert "verified_shops" in data + assert "verification_rate" in data + assert isinstance(data["total_shops"], int) def test_admin_pagination_users(self, client, admin_headers, test_user, test_admin): """Test user pagination works correctly""" diff --git a/tests/integration/api/v1/test_authentication_endpoints.py b/tests/integration/api/v1/test_authentication_endpoints.py index b5fd8c50..ed7097bc 100644 --- a/tests/integration/api/v1/test_authentication_endpoints.py +++ b/tests/integration/api/v1/test_authentication_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_authentication_endpoints.py +# tests/integration/api/v1/test_authentication_endpoints.py import pytest from fastapi import HTTPException @@ -34,8 +34,11 @@ class TestAuthenticationAPI: }, ) - assert response.status_code == 400 - assert "Email already registered" in response.json()["detail"] + assert response.status_code == 409 # Changed from 400 to 409 + data = response.json() + assert data["error_code"] == "USER_ALREADY_EXISTS" + assert "Email already exists" in data["message"] + assert data["field"] == "email" def test_register_user_duplicate_username(self, client, test_user): """Test registration with duplicate username""" @@ -48,8 +51,11 @@ class TestAuthenticationAPI: }, ) - assert response.status_code == 400 - assert "Username already taken" in response.json()["detail"] + assert response.status_code == 409 # Changed from 400 to 409 + data = response.json() + assert data["error_code"] == "USER_ALREADY_EXISTS" + assert "Username already taken" in data["message"] + assert data["field"] == "username" def test_login_success(self, client, test_user): """Test successful login""" @@ -73,9 +79,11 @@ class TestAuthenticationAPI: ) assert response.status_code == 401 - assert "Incorrect username or password" in response.json()["detail"] + data = response.json() + assert data["error_code"] == "INVALID_CREDENTIALS" + assert "Invalid username or password" in data["message"] - def test_login_nonexistent_user(self, client, db): # Added db fixture + def test_login_nonexistent_user(self, client, db): """Test login with nonexistent user""" response = client.post( "/api/v1/auth/login", @@ -83,6 +91,28 @@ class TestAuthenticationAPI: ) assert response.status_code == 401 + data = response.json() + assert data["error_code"] == "INVALID_CREDENTIALS" + + def test_login_inactive_user(self, client, db, test_user): + """Test login with inactive user account""" + # Manually deactivate the user for this test + test_user.is_active = False + db.commit() + + response = client.post( + "/api/v1/auth/login", + json={"username": test_user.username, "password": "testpass123"}, + ) + + assert response.status_code == 403 + data = response.json() + assert data["error_code"] == "USER_NOT_ACTIVE" + assert "User account is not active" in data["message"] + + # Reactivate for other tests + test_user.is_active = True + db.commit() def test_get_current_user_info(self, client, auth_headers, test_user): """Test getting current user info""" diff --git a/tests/integration/api/v1/test_export.py b/tests/integration/api/v1/test_export.py index 7df3bc6c..efe979f7 100644 --- a/tests/integration/api/v1/test_export.py +++ b/tests/integration/api/v1/test_export.py @@ -1,4 +1,4 @@ -# tests/test_export.py +# tests/integration/api/v1/test_export.py import csv from io import StringIO diff --git a/tests/integration/api/v1/test_filtering.py b/tests/integration/api/v1/test_filtering.py index 5302fd16..f75b3040 100644 --- a/tests/integration/api/v1/test_filtering.py +++ b/tests/integration/api/v1/test_filtering.py @@ -1,4 +1,4 @@ -# tests/test_filtering.py +# tests/integration/api/v1/test_filtering.py import pytest from models.database.product import Product diff --git a/tests/integration/api/v1/test_marketplace_endpoints.py b/tests/integration/api/v1/test_marketplace_endpoints.py index fcac37e3..dd4025c0 100644 --- a/tests/integration/api/v1/test_marketplace_endpoints.py +++ b/tests/integration/api/v1/test_marketplace_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_marketplace_endpoints.py +# tests/integration/api/v1/test_marketplace_endpoints.py from unittest.mock import AsyncMock, patch import pytest diff --git a/tests/integration/api/v1/test_product_endpoints.py b/tests/integration/api/v1/test_product_endpoints.py index 065a1bc5..f6862043 100644 --- a/tests/integration/api/v1/test_product_endpoints.py +++ b/tests/integration/api/v1/test_product_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_product_endpoints.py +# tests/integration/api/v1/test_product_endpoints.py import pytest @@ -82,16 +82,27 @@ class TestProductsAPI: "/api/v1/product", headers=auth_headers, json=product_data ) - # Debug output - print(f"Status Code: {response.status_code}") - print(f"Response Content: {response.content}") - try: - print(f"Response JSON: {response.json()}") - except: - print("Could not parse response as JSON") + assert response.status_code == 409 # Changed from 400 to 409 + data = response.json() + assert data["error_code"] == "PRODUCT_ALREADY_EXISTS" + assert test_product.product_id in data["message"] + + def test_create_product_invalid_data(self, client, auth_headers): + """Test creating product with invalid data""" + product_data = { + "product_id": "", # Empty product ID + "title": "New Product", + "price": "invalid_price", + "gtin": "invalid_gtin", + } + + response = client.post( + "/api/v1/product", headers=auth_headers, json=product_data + ) assert response.status_code == 400 - assert "already exists" in response.json()["detail"] + data = response.json() + assert data["error_code"] in ["INVALID_PRODUCT_DATA", "PRODUCT_VALIDATION_FAILED"] def test_get_product_by_id(self, client, auth_headers, test_product): """Test getting specific product""" @@ -109,6 +120,9 @@ class TestProductsAPI: response = client.get("/api/v1/product/NONEXISTENT", headers=auth_headers) assert response.status_code == 404 + data = response.json() + assert data["error_code"] == "PRODUCT_NOT_FOUND" + assert "NONEXISTENT" in data["message"] def test_update_product(self, client, auth_headers, test_product): """Test updating product""" @@ -125,6 +139,34 @@ class TestProductsAPI: assert data["title"] == "Updated Product Title" assert data["price"] == "25.99" + def test_update_nonexistent_product(self, client, auth_headers): + """Test updating nonexistent product""" + update_data = {"title": "Updated Product Title"} + + response = client.put( + "/api/v1/product/NONEXISTENT", + headers=auth_headers, + json=update_data, + ) + + assert response.status_code == 404 + data = response.json() + assert data["error_code"] == "PRODUCT_NOT_FOUND" + + def test_update_product_invalid_data(self, client, auth_headers, test_product): + """Test updating product with invalid data""" + update_data = {"title": "", "price": "invalid_price"} + + response = client.put( + f"/api/v1/product/{test_product.product_id}", + headers=auth_headers, + json=update_data, + ) + + assert response.status_code == 400 + data = response.json() + assert data["error_code"] in ["INVALID_PRODUCT_DATA", "PRODUCT_VALIDATION_FAILED"] + def test_delete_product(self, client, auth_headers, test_product): """Test deleting product""" response = client.delete( @@ -134,6 +176,14 @@ class TestProductsAPI: assert response.status_code == 200 assert "deleted successfully" in response.json()["message"] + def test_delete_nonexistent_product(self, client, auth_headers): + """Test deleting nonexistent product""" + response = client.delete("/api/v1/product/NONEXISTENT", headers=auth_headers) + + assert response.status_code == 404 + data = response.json() + assert data["error_code"] == "PRODUCT_NOT_FOUND" + def test_get_product_without_auth(self, client): """Test that product endpoints require authentication""" response = client.get("/api/v1/product") diff --git a/tests/integration/api/v1/test_shop_endpoints.py b/tests/integration/api/v1/test_shop_endpoints.py index 8b094a7b..28078d80 100644 --- a/tests/integration/api/v1/test_shop_endpoints.py +++ b/tests/integration/api/v1/test_shop_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_shop_endpoints.py +# tests/integration/api/v1/test_shop_endpoints.py import pytest diff --git a/tests/integration/api/v1/test_stats_endpoints.py b/tests/integration/api/v1/test_stats_endpoints.py index fdf7a078..032faabf 100644 --- a/tests/integration/api/v1/test_stats_endpoints.py +++ b/tests/integration/api/v1/test_stats_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_stats_endpoints.py +# tests/integration/api/v1/test_stats_endpoints.py import pytest diff --git a/tests/integration/api/v1/test_stock_endpoints.py b/tests/integration/api/v1/test_stock_endpoints.py index f355f66c..c1e17686 100644 --- a/tests/integration/api/v1/test_stock_endpoints.py +++ b/tests/integration/api/v1/test_stock_endpoints.py @@ -1,4 +1,4 @@ -# tests/test_stock_endpoints.py +# tests/integration/api/v1/test_stock_endpoints.py import pytest from models.database.stock import Stock