diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index f14aa660b..50455d54f 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -123,18 +123,6 @@ def get_current_active_superuser_org(current_user: CurrentUserOrg) -> User: return current_user -async def http_exception_handler(request: Request, exc: HTTPException): - """ - Global handler for HTTPException to return standardized response format. - """ - return JSONResponse( - status_code=exc.status_code, - content=APIResponse.failure_response(exc.detail).model_dump() - # TEMPORARY: Keep "detail" for backward compatibility - | {"detail": exc.detail}, - ) - - def verify_user_project_organization( db: SessionDep, current_user: CurrentUserOrg, diff --git a/backend/app/api/routes/api_keys.py b/backend/app/api/routes/api_keys.py index 977dd02a5..1afecfc72 100644 --- a/backend/app/api/routes/api_keys.py +++ b/backend/app/api/routes/api_keys.py @@ -12,11 +12,11 @@ from app.crud.project import validate_project from app.models import APIKeyPublic, User from app.utils import APIResponse +from app.core.exception_handlers import HTTPException router = APIRouter(prefix="/apikeys", tags=["API Keys"]) -# Create API Key @router.post("/", response_model=APIResponse[APIKeyPublic]) def create_key( project_id: int, @@ -27,31 +27,26 @@ def create_key( """ Generate a new API key for the user's organization. """ - try: - # Validate organization - project = validate_project(session, project_id) - - existing_api_key = get_api_key_by_project_user(session, project_id, user_id) - if existing_api_key: - raise HTTPException( - status_code=400, - detail="API Key already exists for this user and project.", - ) - - # Create and return API key - api_key = create_api_key( - session, - organization_id=project.organization_id, - user_id=user_id, - project_id=project_id, + # Validate organization + project = validate_project(session, project_id) + + existing_api_key = get_api_key_by_project_user(session, project_id, user_id) + if existing_api_key: + raise HTTPException( + status_code=400, + detail="API Key already exists for this user and project.", ) - return APIResponse.success_response(api_key) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) + # Create and return API key + api_key = create_api_key( + session, + organization_id=project.organization_id, + user_id=user_id, + project_id=project_id, + ) + return APIResponse.success_response(api_key) -# List API Keys @router.get("/", response_model=APIResponse[list[APIKeyPublic]]) def list_keys( project_id: int, @@ -62,27 +57,29 @@ def list_keys( Retrieve all API keys for the given project. Superusers get all keys; regular users get only their own. """ - try: - # Validate project - project = validate_project(session=session, project_id=project_id) - - if current_user.is_superuser: - # Superuser: fetch all API keys for the project - api_keys = get_api_keys_by_project(session=session, project_id=project_id) - else: - # Regular user: fetch only their own API key - user_api_key = get_api_key_by_project_user( - session=session, project_id=project_id, user_id=current_user.id - ) - api_keys = [user_api_key] if user_api_key else [] + # Validate project + project = validate_project(session=session, project_id=project_id) + + if current_user.is_superuser: + # Superuser: fetch all API keys for the project + api_keys = get_api_keys_by_project(session=session, project_id=project_id) + else: + # Regular user: fetch only their own API key + user_api_key = get_api_key_by_project_user( + session=session, project_id=project_id, user_id=current_user.id + ) + api_keys = [user_api_key] if user_api_key else [] - return APIResponse.success_response(api_keys) + # Raise an exception if no API keys are found for the project + if not api_keys: + raise HTTPException( + status_code=404, + detail="No API keys found for this project.", + ) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) + return APIResponse.success_response(api_keys) -# Get API Key by ID @router.get("/{api_key_id}", response_model=APIResponse[APIKeyPublic]) def get_key( api_key_id: int, @@ -94,12 +91,11 @@ def get_key( """ api_key = get_api_key(session, api_key_id) if not api_key: - raise HTTPException(status_code=404, detail="API Key does not exist") + raise HTTPException(404, "API Key does not exist") return APIResponse.success_response(api_key) -# Revoke API Key (Soft Delete) @router.delete("/{api_key_id}", response_model=APIResponse[dict]) def revoke_key( api_key_id: int, @@ -109,8 +105,11 @@ def revoke_key( """ Soft delete an API key (revoke access). """ - try: - delete_api_key(session, api_key_id) - return APIResponse.success_response({"message": "API key revoked successfully"}) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) + api_key = get_api_key(session, api_key_id) + + if not api_key: + raise HTTPException(404, "API key not found or already deleted") + + delete_api_key(session, api_key_id) + + return APIResponse.success_response({"message": "API key revoked successfully"}) diff --git a/backend/app/api/routes/collections.py b/backend/app/api/routes/collections.py index 86f85ffec..532bab721 100644 --- a/backend/app/api/routes/collections.py +++ b/backend/app/api/routes/collections.py @@ -317,15 +317,7 @@ def collection_info( collection_id: UUID = FastPath(description="Collection to retrieve"), ): collection_crud = CollectionCrud(session, current_user.id) - try: - data = collection_crud.read_one(collection_id) - except NoResultFound as err: - raise HTTPException(status_code=404, detail=str(err)) - except MultipleResultsFound as err: - raise HTTPException(status_code=503, detail=str(err)) - except Exception as err: - raise_from_unknown(err) - + data = collection_crud.read_one(collection_id) return APIResponse.success_response(data) @@ -339,13 +331,7 @@ def list_collections( current_user: CurrentUser, ): collection_crud = CollectionCrud(session, current_user.id) - try: - data = collection_crud.read_all() - except (ValueError, SQLAlchemyError) as err: - raise HTTPException(status_code=403, detail=str(err)) - except Exception as err: - raise_from_unknown(err) - + data = collection_crud.read_all() return APIResponse.success_response(data) @@ -363,10 +349,6 @@ def collection_documents( ): collection_crud = CollectionCrud(session, current_user.id) document_collection_crud = DocumentCollectionCrud(session) - try: - collection = collection_crud.read_one(collection_id) - data = document_collection_crud.read(collection, skip, limit) - except (SQLAlchemyError, ValueError) as err: - raise HTTPException(status_code=400, detail=str(err)) - + collection = collection_crud.read_one(collection_id) + data = document_collection_crud.read(collection, skip, limit) return APIResponse.success_response(data) diff --git a/backend/app/api/routes/credentials.py b/backend/app/api/routes/credentials.py index ca0237a9c..0091e8e07 100644 --- a/backend/app/api/routes/credentials.py +++ b/backend/app/api/routes/credentials.py @@ -1,7 +1,6 @@ from typing import List -from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy.exc import IntegrityError +from fastapi import APIRouter, Depends from app.api.deps import SessionDep, get_current_active_superuser from app.crud.credentials import ( @@ -12,11 +11,13 @@ update_creds_for_org, remove_provider_credential, ) +from app.crud import validate_organization, validate_project from app.models import CredsCreate, CredsPublic, CredsUpdate from app.models.organization import Organization from app.models.project import Project from app.utils import APIResponse from app.core.providers import validate_provider +from app.core.exception_handlers import HTTPException router = APIRouter(prefix="/credentials", tags=["credentials"]) @@ -29,48 +30,41 @@ description="Creates new credentials for a specific organization and project combination. This endpoint requires superuser privileges. Each organization can have different credentials for different providers and projects. Only one credential per provider is allowed per organization-project combination.", ) def create_new_credential(*, session: SessionDep, creds_in: CredsCreate): - try: - # Check if organization exists - organization = session.get(Organization, creds_in.organization_id) - if not organization: - raise HTTPException(status_code=404, detail="Organization not found") - - # Check if project exists if project_id is provided - if creds_in.project_id: - project = session.get(Project, creds_in.project_id) - if not project: - raise HTTPException(status_code=404, detail="Project not found") - if project.organization_id != creds_in.organization_id: - raise HTTPException( - status_code=400, - detail="Project does not belong to the specified organization", - ) - - # Check for existing credentials for each provider - for provider in creds_in.credential.keys(): - existing_cred = get_provider_credential( - session=session, - org_id=creds_in.organization_id, - provider=provider, - project_id=creds_in.project_id, + # Validate organization + validate_organization(session, creds_in.organization_id) + + # Validate project if provided + if creds_in.project_id: + project = validate_project(session, creds_in.project_id) + if project.organization_id != creds_in.organization_id: + raise HTTPException( + status_code=400, + detail="Project does not belong to the specified organization", ) - if existing_cred: - raise HTTPException( - status_code=400, - detail=f"Credentials for provider '{provider}' already exist for this organization and project combination", - ) - - # Create new credentials - new_creds = set_creds_for_org(session=session, creds_add=creds_in) - if not new_creds: - raise HTTPException(status_code=500, detail="Failed to create credentials") - return APIResponse.success_response([cred.to_public() for cred in new_creds]) - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except Exception as e: - raise HTTPException( - status_code=500, detail=f"An unexpected error occurred: {str(e)}" + + # Prevent duplicate credentials + for provider in creds_in.credential.keys(): + existing_cred = get_provider_credential( + session=session, + org_id=creds_in.organization_id, + provider=provider, + project_id=creds_in.project_id, ) + if existing_cred: + raise HTTPException( + status_code=400, + detail=( + f"Credentials for provider '{provider}' already exist " + f"for this organization and project combination" + ), + ) + + # Create credentials + new_creds = set_creds_for_org(session=session, creds_add=creds_in) + if not new_creds: + raise Exception(status_code=500, detail="Failed to create credentials") + + return APIResponse.success_response([cred.to_public() for cred in new_creds]) @router.get( @@ -84,6 +78,7 @@ def read_credential(*, session: SessionDep, org_id: int, project_id: int | None creds = get_creds_by_org(session=session, org_id=org_id, project_id=project_id) if not creds: raise HTTPException(status_code=404, detail="Credentials not found") + return APIResponse.success_response([cred.to_public() for cred in creds]) @@ -106,6 +101,7 @@ def read_provider_credential( ) if provider_creds is None: raise HTTPException(status_code=404, detail="Provider credentials not found") + return APIResponse.success_response(provider_creds) @@ -117,70 +113,47 @@ def read_provider_credential( description="Updates credentials for a specific organization and project combination. Can update specific provider credentials or add new providers. If project_id is provided in the update, credentials will be moved to that project. This endpoint requires superuser privileges.", ) def update_credential(*, session: SessionDep, org_id: int, creds_in: CredsUpdate): - try: - if not creds_in or not creds_in.provider or not creds_in.credential: - raise HTTPException( - status_code=400, detail="Provider and credential must be provided" - ) - organization = session.get(Organization, org_id) - if not organization: - raise HTTPException(status_code=404, detail="Organization not found") - updated_creds = update_creds_for_org( - session=session, org_id=org_id, creds_in=creds_in - ) - if not updated_creds: - raise HTTPException(status_code=404, detail="Failed to update credentials") - return APIResponse.success_response( - [cred.to_public() for cred in updated_creds] - ) - except IntegrityError as e: - if "ForeignKeyViolation" in str(e): - raise HTTPException( - status_code=400, - detail="Invalid organization ID. Ensure the organization exists before updating credentials.", - ) - raise HTTPException( - status_code=500, detail=f"An unexpected database error occurred: {str(e)}" - ) - except ValueError as e: - raise HTTPException(status_code=404, detail=str(e)) - except Exception as e: + validate_organization(session, org_id) + if not creds_in or not creds_in.provider or not creds_in.credential: raise HTTPException( - status_code=500, detail=f"An unexpected error occurred: {str(e)}" + status_code=400, detail="Provider and credential must be provided" ) + updated_creds = update_creds_for_org( + session=session, org_id=org_id, creds_in=creds_in + ) + + return APIResponse.success_response([cred.to_public() for cred in updated_creds]) + @router.delete( "/{org_id}/{provider}", dependencies=[Depends(get_current_active_superuser)], response_model=APIResponse[dict], summary="Delete specific provider credentials for an organization and project", - description="Removes credentials for a specific provider while keeping other provider credentials intact. If project_id is provided, only removes credentials for that project. This endpoint requires superuser privileges.", ) def delete_provider_credential( *, session: SessionDep, org_id: int, provider: str, project_id: int | None = None ): - try: - provider_enum = validate_provider(provider) - updated_creds = remove_provider_credential( - session=session, - org_id=org_id, - provider=provider_enum, - project_id=project_id, - ) - if not updated_creds: - raise HTTPException( - status_code=404, detail="Provider credentials not found" - ) - return APIResponse.success_response( - {"message": "Provider credentials removed successfully"} - ) - except ValueError: + provider_enum = validate_provider(provider) + if not provider_enum: + raise HTTPException(status_code=400, detail="Invalid provider") + provider_creds = get_provider_credential( + session=session, + org_id=org_id, + provider=provider_enum, + project_id=project_id, + ) + if provider_creds is None: raise HTTPException(status_code=404, detail="Provider credentials not found") - except Exception as e: - raise HTTPException( - status_code=500, detail=f"An unexpected error occurred: {str(e)}" - ) + + updated_creds = remove_provider_credential( + session=session, org_id=org_id, provider=provider_enum, project_id=project_id + ) + + return APIResponse.success_response( + {"message": "Provider credentials removed successfully"} + ) @router.delete( @@ -193,18 +166,10 @@ def delete_provider_credential( def delete_all_credentials( *, session: SessionDep, org_id: int, project_id: int | None = None ): - try: - creds = remove_creds_for_org( - session=session, org_id=org_id, project_id=project_id - ) - if not creds: - raise HTTPException( - status_code=404, detail="Credentials for organization not found" - ) - return APIResponse.success_response( - {"message": "Credentials deleted successfully"} - ) - except Exception as e: + creds = remove_creds_for_org(session=session, org_id=org_id, project_id=project_id) + if not creds: raise HTTPException( - status_code=500, detail=f"An unexpected error occurred: {str(e)}" + status_code=404, detail="Credentials for organization not found" ) + + return APIResponse.success_response({"message": "Credentials deleted successfully"}) diff --git a/backend/app/api/routes/documents.py b/backend/app/api/routes/documents.py index 7ad967610..f0f981583 100644 --- a/backend/app/api/routes/documents.py +++ b/backend/app/api/routes/documents.py @@ -2,17 +2,14 @@ from typing import List from pathlib import Path -from fastapi import APIRouter, File, UploadFile, HTTPException, Query +from fastapi import APIRouter, File, UploadFile, Query from fastapi import Path as FastPath -from sqlalchemy.exc import NoResultFound, MultipleResultsFound, SQLAlchemyError - from app.crud import DocumentCrud, CollectionCrud from app.models import Document from app.utils import APIResponse, load_description from app.api.deps import CurrentUser, SessionDep -from app.core.util import raise_from_unknown -from app.core.cloud import AmazonCloudStorage, CloudStorageError +from app.core.cloud import AmazonCloudStorage from app.crud.rag import OpenAIAssistantCrud router = APIRouter(prefix="/documents", tags=["documents"]) @@ -30,13 +27,7 @@ def list_docs( limit: int = Query(100, gt=0, le=100), ): crud = DocumentCrud(session, current_user.id) - try: - data = crud.read_many(skip, limit) - except (ValueError, SQLAlchemyError) as err: - raise HTTPException(status_code=403, detail=str(err)) - except Exception as err: - raise_from_unknown(err) - + data = crud.read_many(skip, limit) return APIResponse.success_response(data) @@ -52,12 +43,7 @@ def upload_doc( ): storage = AmazonCloudStorage(current_user) document_id = uuid4() - try: - object_store_url = storage.put(src, Path(str(document_id))) - except CloudStorageError as err: - raise HTTPException(status_code=503, detail=str(err)) - except Exception as err: - raise_from_unknown(err) + object_store_url = storage.put(src, Path(str(document_id))) crud = DocumentCrud(session, current_user.id) document = Document( @@ -65,14 +51,7 @@ def upload_doc( fname=src.filename, object_store_url=str(object_store_url), ) - - try: - data = crud.update(document) - except SQLAlchemyError as err: - raise HTTPException(status_code=403, detail=str(err)) - except Exception as err: - raise_from_unknown(err) - + data = crud.update(document) return APIResponse.success_response(data) @@ -84,20 +63,14 @@ def upload_doc( def remove_doc( session: SessionDep, current_user: CurrentUser, - doc_id: UUID = Path(description="Document to delete"), + doc_id: UUID = FastPath(description="Document to delete"), ): a_crud = OpenAIAssistantCrud() - (d_crud, c_crud) = ( - x(session, current_user.id) for x in (DocumentCrud, CollectionCrud) - ) - try: - document = d_crud.delete(doc_id) - data = c_crud.delete(document, a_crud) - except NoResultFound as err: - raise HTTPException(status_code=400, detail=str(err)) - except Exception as err: - raise_from_unknown(err) + d_crud = DocumentCrud(session, current_user.id) + c_crud = CollectionCrud(session, current_user.id) + document = d_crud.delete(doc_id) + data = c_crud.delete(document, a_crud) return APIResponse.success_response(data) @@ -112,13 +85,5 @@ def doc_info( doc_id: UUID = FastPath(description="Document to retrieve"), ): crud = DocumentCrud(session, current_user.id) - try: - data = crud.read_one(doc_id) - except NoResultFound as err: - raise HTTPException(status_code=404, detail=str(err)) - except MultipleResultsFound as err: - raise HTTPException(status_code=503, detail=str(err)) - except Exception as err: - raise_from_unknown(err) - + data = crud.read_one(doc_id) return APIResponse.success_response(data) diff --git a/backend/app/api/routes/onboarding.py b/backend/app/api/routes/onboarding.py index f267e37f6..12f43dd3d 100644 --- a/backend/app/api/routes/onboarding.py +++ b/backend/app/api/routes/onboarding.py @@ -51,69 +51,69 @@ class OnboardingResponse(BaseModel): ) def onboard_user(request: OnboardingRequest, session: SessionDep): """ - Handles quick onboarding of a new user : Accepts Organization name, project name, email, password and user name, then gives back an API key which + Handles quick onboarding of a new user: Accepts Organization name, project name, email, password, and user name, then gives back an API key which will be further used for authentication. """ - try: - existing_organization = get_organization_by_name( - session=session, name=request.organization_name + # Validate organization + existing_organization = get_organization_by_name( + session=session, name=request.organization_name + ) + if existing_organization: + organization = existing_organization + else: + org_create = OrganizationCreate(name=request.organization_name) + organization = create_organization(session=session, org_create=org_create) + + # Validate project + existing_project = ( + session.query(Project).filter(Project.name == request.project_name).first() + ) + if existing_project: + project = existing_project # Use the existing project + else: + project_create = ProjectCreate( + name=request.project_name, organization_id=organization.id ) - if existing_organization: - organization = existing_organization - else: - org_create = OrganizationCreate(name=request.organization_name) - organization = create_organization(session=session, org_create=org_create) - - existing_project = ( - session.query(Project).filter(Project.name == request.project_name).first() + project = create_project(session=session, project_create=project_create) + + # Validate user + existing_user = session.query(User).filter(User.email == request.email).first() + if existing_user: + user = existing_user + else: + user_create = UserCreate( + name=request.user_name, + email=request.email, + password=request.password, ) - if existing_project: - project = existing_project # Use the existing project - else: - project_create = ProjectCreate( - name=request.project_name, organization_id=organization.id - ) - project = create_project(session=session, project_create=project_create) - - existing_user = session.query(User).filter(User.email == request.email).first() - if existing_user: - user = existing_user - else: - user_create = UserCreate( - name=request.user_name, - email=request.email, - password=request.password, - ) - user = create_user(session=session, user_create=user_create) - - existing_key = get_api_key_by_project_user( - session=session, user_id=user.id, project_id=project.id - ) - - if existing_key: - raise HTTPException( - status_code=400, - detail="API key already exists for this user and project.", - ) - - api_key_public = create_api_key( - session=session, - organization_id=organization.id, - user_id=user.id, - project_id=project.id, - ) - - user.is_superuser = False - session.add(user) - session.commit() - - return OnboardingResponse( - organization_id=organization.id, - project_id=project.id, - user_id=user.id, - api_key=api_key_public.key, + user = create_user(session=session, user_create=user_create) + + # Check if API key exists for the user and project + existing_key = get_api_key_by_project_user( + session=session, user_id=user.id, project_id=project.id + ) + if existing_key: + raise HTTPException( + status_code=400, + detail="API key already exists for this user and project.", ) - except Exception as e: - session.rollback() - raise HTTPException(status_code=400, detail=str(e)) + # Create API key + api_key_public = create_api_key( + session=session, + organization_id=organization.id, + user_id=user.id, + project_id=project.id, + ) + + # Set user as non-superuser and save to session + user.is_superuser = False + session.add(user) + session.commit() + + return OnboardingResponse( + organization_id=organization.id, + project_id=project.id, + user_id=user.id, + api_key=api_key_public.key, + ) diff --git a/backend/app/api/routes/responses.py b/backend/app/api/routes/responses.py index d1abb0d32..b6151c23a 100644 --- a/backend/app/api/routes/responses.py +++ b/backend/app/api/routes/responses.py @@ -129,4 +129,4 @@ async def responses_sync( ), ) except openai.OpenAIError as e: - return ResponsesAPIResponse.failure_response(error=handle_openai_error(e)) + return Exception(error=handle_openai_error(e)) diff --git a/backend/app/api/routes/threads.py b/backend/app/api/routes/threads.py index 6a28678ef..4b621a458 100644 --- a/backend/app/api/routes/threads.py +++ b/backend/app/api/routes/threads.py @@ -2,7 +2,7 @@ import openai import requests -from fastapi import APIRouter, BackgroundTasks, Depends +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException from openai import OpenAI from sqlmodel import Session from langfuse.decorators import observe, langfuse_context @@ -229,9 +229,8 @@ async def threads( project_id=request.get("project_id"), ) if not credentials or "api_key" not in credentials: - return APIResponse.failure_response( - error="OpenAI API key not configured for this organization." - ) + raise HTTPException(404, "OpenAI API key not configured for this organization.") + client = OpenAI(api_key=credentials["api_key"]) langfuse_credentials = get_provider_credential( @@ -241,9 +240,7 @@ async def threads( project_id=request.get("project_id"), ) if not langfuse_credentials: - return APIResponse.failure_response( - error="LANGFUSE keys not configured for this organization." - ) + raise HTTPException(404, "LANGFUSE keys not configured for this organization.") langfuse_context.configure( secret_key=langfuse_credentials["secret_key"], @@ -253,12 +250,11 @@ async def threads( # Validate thread is_valid, error_message = validate_thread(client, request.get("thread_id")) if not is_valid: - return APIResponse.failure_response(error=error_message) - + raise Exception(error_message) # Setup thread is_success, error_message = setup_thread(client, request) if not is_success: - return APIResponse.failure_response(error=error_message) + raise Exception(error_message) # Send immediate response initial_response = APIResponse.success_response( @@ -291,8 +287,8 @@ async def threads_sync( project_id=request.get("project_id"), ) if not credentials or "api_key" not in credentials: - return APIResponse.failure_response( - error="OpenAI API key not configured for this organization." + raise HTTPException( + 404, error="OpenAI API key not configured for this organization." ) client = OpenAI(api_key=credentials["api_key"]) @@ -300,12 +296,11 @@ async def threads_sync( # Validate thread is_valid, error_message = validate_thread(client, request.get("thread_id")) if not is_valid: - return APIResponse.failure_response(error=error_message) - + raise Exception(error_message) # Setup thread is_success, error_message = setup_thread(client, request) if not is_success: - return APIResponse.failure_response(error=error_message) + raise Exception(error_message) try: # Process run @@ -337,7 +332,7 @@ async def threads_sync( ) except openai.OpenAIError as e: - return APIResponse.failure_response(error=handle_openai_error(e)) + raise Exception(error=handle_openai_error(e)) @router.post("/threads/start") @@ -355,7 +350,7 @@ async def start_thread( is_success, error = setup_thread(client, request) if not is_success: - return APIResponse.failure_response(error=error) + raise Exception(error) thread_id = request["thread_id"] @@ -394,7 +389,7 @@ async def get_thread( result = get_thread_result(db, thread_id) if not result: - return APIResponse.failure_response(error="Thread not found.") + raise HTTPException(404, "thread not found") status = result.status or ("success" if result.response else "processing") diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 956461010..94eccd067 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -1,8 +1,8 @@ import uuid from typing import Any -from fastapi import APIRouter, Depends, HTTPException -from sqlmodel import col, delete, func, select +from fastapi import APIRouter, Depends +from sqlmodel import func, select from app.api.deps import ( CurrentUser, @@ -24,6 +24,7 @@ UserUpdateMe, ) from app.utils import generate_new_account_email, send_email +from app.core.exception_handlers import HTTPException router = APIRouter(prefix="/users", tags=["users"]) @@ -35,16 +36,8 @@ include_in_schema=False, ) def read_users(session: SessionDep, skip: int = 0, limit: int = 100) -> Any: - """ - Retrieve users. - """ - - count_statement = select(func.count()).select_from(User) - count = session.exec(count_statement).one() - - statement = select(User).offset(skip).limit(limit) - users = session.exec(statement).all() - + count = session.exec(select(func.count()).select_from(User)).one() + users = session.exec(select(User).offset(skip).limit(limit)).all() return UsersPublic(data=users, count=count) @@ -55,17 +48,14 @@ def read_users(session: SessionDep, skip: int = 0, limit: int = 100) -> Any: include_in_schema=False, ) def create_user_endpoint(*, session: SessionDep, user_in: UserCreate) -> Any: - """ - Create new user. - """ - user = get_user_by_email(session=session, email=user_in.email) - if user: + if get_user_by_email(session=session, email=user_in.email): raise HTTPException( status_code=400, detail="The user with this email already exists in the system.", ) user = create_user(session=session, user_create=user_in) + if settings.emails_enabled and user_in.email: email_data = generate_new_account_email( email_to=user_in.email, username=user_in.email, password=user_in.password @@ -82,18 +72,14 @@ def create_user_endpoint(*, session: SessionDep, user_in: UserCreate) -> Any: def update_user_me( *, session: SessionDep, user_in: UserUpdateMe, current_user: CurrentUser ) -> Any: - """ - Update own user. - """ - if user_in.email: existing_user = get_user_by_email(session=session, email=user_in.email) if existing_user and existing_user.id != current_user.id: raise HTTPException( status_code=409, detail="User with this email already exists" ) - user_data = user_in.model_dump(exclude_unset=True) - current_user.sqlmodel_update(user_data) + + current_user.sqlmodel_update(user_in.model_dump(exclude_unset=True)) session.add(current_user) session.commit() session.refresh(current_user) @@ -104,17 +90,16 @@ def update_user_me( def update_password_me( *, session: SessionDep, body: UpdatePassword, current_user: CurrentUser ) -> Any: - """ - Update own password. - """ if not verify_password(body.current_password, current_user.hashed_password): raise HTTPException(status_code=400, detail="Incorrect password") + if body.current_password == body.new_password: raise HTTPException( - status_code=400, detail="New password cannot be the same as the current one" + status_code=400, + detail="New password cannot be the same as the current one", ) - hashed_password = get_password_hash(body.new_password) - current_user.hashed_password = hashed_password + + current_user.hashed_password = get_password_hash(body.new_password) session.add(current_user) session.commit() return Message(message="Password updated successfully") @@ -122,17 +107,11 @@ def update_password_me( @router.get("/me", response_model=UserPublic) def read_user_me(current_user: CurrentUser) -> Any: - """ - Get current user. - """ return current_user @router.delete("/me", response_model=Message) def delete_user_me(session: SessionDep, current_user: CurrentUser) -> Any: - """ - Delete own user. - """ if current_user.is_superuser: raise HTTPException( status_code=403, detail="Super users are not allowed to delete themselves" @@ -144,35 +123,30 @@ def delete_user_me(session: SessionDep, current_user: CurrentUser) -> Any: @router.post("/signup", response_model=UserPublic) def register_user(session: SessionDep, user_in: UserRegister) -> Any: - """ - Create new user without the need to be logged in. - """ - user = get_user_by_email(session=session, email=user_in.email) - if user: + if get_user_by_email(session=session, email=user_in.email): raise HTTPException( status_code=400, detail="The user with this email already exists in the system", ) + user_create = UserCreate.model_validate(user_in) - user = create_user(session=session, user_create=user_create) - return user + return create_user(session=session, user_create=user_create) @router.get("/{user_id}", response_model=UserPublic, include_in_schema=False) def read_user_by_id( user_id: int, session: SessionDep, current_user: CurrentUser ) -> Any: - """ - Get a specific user by id. - """ user = session.get(User, user_id) if user == current_user: return user + if not current_user.is_superuser: raise HTTPException( status_code=403, detail="The user doesn't have enough privileges", ) + return user @@ -188,16 +162,13 @@ def update_user_endpoint( user_id: int, user_in: UserUpdate, ) -> Any: - """ - Update a user. - """ - db_user = session.get(User, user_id) if not db_user: raise HTTPException( status_code=404, detail="The user with this id does not exist in the system", ) + if user_in.email: existing_user = get_user_by_email(session=session, email=user_in.email) if existing_user and existing_user.id != user_id: @@ -205,8 +176,7 @@ def update_user_endpoint( status_code=409, detail="User with this email already exists" ) - db_user = update_user(session=session, db_user=db_user, user_in=user_in) - return db_user + return update_user(session=session, db_user=db_user, user_in=user_in) @router.delete( @@ -217,16 +187,15 @@ def update_user_endpoint( def delete_user( session: SessionDep, current_user: CurrentUser, user_id: int ) -> Message: - """ - Delete a user. - """ user = session.get(User, user_id) if not user: raise HTTPException(status_code=404, detail="User not found") + if user == current_user: raise HTTPException( status_code=403, detail="Super users are not allowed to delete themselves" ) + session.delete(user) session.commit() return Message(message="User deleted successfully") diff --git a/backend/app/core/exception_handlers.py b/backend/app/core/exception_handlers.py new file mode 100644 index 000000000..f6e614f5d --- /dev/null +++ b/backend/app/core/exception_handlers.py @@ -0,0 +1,33 @@ +from fastapi import FastAPI, Request, HTTPException +from fastapi.responses import JSONResponse +from fastapi.exceptions import RequestValidationError +from app.utils import APIResponse +from starlette.status import ( + HTTP_422_UNPROCESSABLE_ENTITY, + HTTP_500_INTERNAL_SERVER_ERROR, +) + + +def register_exception_handlers(app: FastAPI): + @app.exception_handler(RequestValidationError) + async def validation_error_handler(request: Request, exc: RequestValidationError): + return JSONResponse( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + content=APIResponse.failure_response(exc.errors()).model_dump(), + ) + + @app.exception_handler(HTTPException) + async def http_exception_handler(request: Request, exc: HTTPException): + return JSONResponse( + status_code=exc.status_code, + content=APIResponse.failure_response(exc.detail).model_dump(), + ) + + @app.exception_handler(Exception) + async def generic_error_handler(request: Request, exc: Exception): + return JSONResponse( + status_code=HTTP_500_INTERNAL_SERVER_ERROR, + content=APIResponse.failure_response( + str(exc) or "An unexpected error occurred." + ).model_dump(), + ) diff --git a/backend/app/crud/__init__.py b/backend/app/crud/__init__.py index a20e8f4aa..671cf267b 100644 --- a/backend/app/crud/__init__.py +++ b/backend/app/crud/__init__.py @@ -19,6 +19,7 @@ create_project, get_project_by_id, get_projects_by_organization, + validate_project, ) from .api_key import ( diff --git a/backend/app/crud/api_key.py b/backend/app/crud/api_key.py index 905d0a1b1..e787514f3 100644 --- a/backend/app/crud/api_key.py +++ b/backend/app/crud/api_key.py @@ -8,6 +8,7 @@ ) from app.core import settings from app.core.util import now +from app.core.exception_handlers import HTTPException from app.models.api_key import APIKey, APIKeyPublic @@ -76,9 +77,6 @@ def delete_api_key(session: Session, api_key_id: int) -> None: """ api_key = session.get(APIKey, api_key_id) - if not api_key or api_key.is_deleted: - raise ValueError("API key not found or already deleted") - api_key.is_deleted = True api_key.deleted_at = now() api_key.updated_at = now() diff --git a/backend/app/crud/credentials.py b/backend/app/crud/credentials.py index 32dcf6465..ba3503e14 100644 --- a/backend/app/crud/credentials.py +++ b/backend/app/crud/credentials.py @@ -11,6 +11,7 @@ ) from app.core.security import encrypt_credentials, decrypt_credentials from app.core.util import now +from app.core.exception_handlers import HTTPException def set_creds_for_org(*, session: Session, creds_add: CredsCreate) -> List[Credential]: @@ -18,7 +19,7 @@ def set_creds_for_org(*, session: Session, creds_add: CredsCreate) -> List[Crede created_credentials = [] if not creds_add.credential: - raise ValueError("No credentials provided") + raise HTTPException(400, "No credentials provided") for provider, credentials in creds_add.credential.items(): # Validate provider and credentials @@ -136,9 +137,10 @@ def update_creds_for_org( else True, ) creds = session.exec(statement).first() - - if not creds: - raise ValueError(f"No credentials found for provider {creds_in.provider}") + if creds is None: + raise HTTPException( + status_code=404, detail="Credentials not found for this provider" + ) creds.credential = encrypted_credentials creds.updated_at = now() @@ -162,21 +164,15 @@ def remove_provider_credential( ) creds = session.exec(statement).first() - if not creds: - raise ValueError(f"Credentials not found for provider '{provider}'") - - # Soft delete by setting is_active to False + # Soft delete creds.is_active = False creds.updated_at = now() - try: - session.add(creds) - session.commit() - session.refresh(creds) - return creds - except IntegrityError as e: - session.rollback() - raise ValueError(f"Error while removing provider credentials: {str(e)}") + session.add(creds) + session.commit() + session.refresh(creds) + + return creds def remove_creds_for_org( diff --git a/backend/app/crud/document.py b/backend/app/crud/document.py index 32b59f484..ecb818905 100644 --- a/backend/app/crud/document.py +++ b/backend/app/crud/document.py @@ -5,6 +5,7 @@ from app.models import Document from app.core.util import now +from app.core.exception_handlers import HTTPException class DocumentCrud: @@ -20,7 +21,11 @@ def read_one(self, doc_id: UUID): ) ) - return self.session.exec(statement).one() + result = self.session.exec(statement).one_or_none() + if result is None: + raise HTTPException(status_code=404, detail="Document not found") + + return result def read_many( self, diff --git a/backend/app/crud/organization.py b/backend/app/crud/organization.py index a1da8eccc..598311c34 100644 --- a/backend/app/crud/organization.py +++ b/backend/app/crud/organization.py @@ -1,6 +1,7 @@ from typing import Any, Optional from datetime import datetime, timezone from sqlmodel import Session, select +from fastapi import HTTPException from app.models import Organization, OrganizationCreate from app.core.util import now @@ -36,9 +37,9 @@ def validate_organization(session: Session, org_id: int) -> Organization: """ organization = get_organization_by_id(session, org_id) if not organization: - raise ValueError("Organization not found") + raise HTTPException(404, "Organization not found") if not organization.is_active: - raise ValueError("Organization is not active") + raise HTTPException("Organization is not active") return organization diff --git a/backend/app/crud/project.py b/backend/app/crud/project.py index 983af5655..f64ee7816 100644 --- a/backend/app/crud/project.py +++ b/backend/app/crud/project.py @@ -1,6 +1,7 @@ from typing import List, Optional from datetime import datetime, timezone from sqlmodel import Session, select +from fastapi import HTTPException from app.models import Project, ProjectCreate, Organization from app.core.util import now @@ -32,9 +33,9 @@ def validate_project(session: Session, project_id: int) -> Project: """ project = get_project_by_id(session=session, project_id=project_id) if not project: - raise ValueError("Project not found") + raise HTTPException(404, "Project not found") if not project.is_active: - raise ValueError("Project is not active") + raise HTTPException(404, "Project is not active") return project diff --git a/backend/app/main.py b/backend/app/main.py index 49cc0e9e7..3655b4dfb 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -5,8 +5,8 @@ from starlette.middleware.cors import CORSMiddleware from app.api.main import api_router -from app.api.deps import http_exception_handler from app.core.config import settings +from app.core.exception_handlers import register_exception_handlers def custom_generate_unique_id(route: APIRoute) -> str: @@ -34,4 +34,4 @@ def custom_generate_unique_id(route: APIRoute) -> str: app.include_router(api_router, prefix=settings.API_V1_STR) -app.add_exception_handler(HTTPException, http_exception_handler) +register_exception_handlers(app) diff --git a/backend/app/tests/api/routes/test_api_key.py b/backend/app/tests/api/routes/test_api_key.py index 0191a594b..552530ccc 100644 --- a/backend/app/tests/api/routes/test_api_key.py +++ b/backend/app/tests/api/routes/test_api_key.py @@ -77,7 +77,7 @@ def test_create_duplicate_api_key(db: Session, superuser_token_headers: dict[str headers=superuser_token_headers, ) assert response.status_code == 400 - assert "API Key already exists" in response.json()["detail"] + assert "API Key already exists" in response.json()["error"] def test_list_api_keys(db: Session, superuser_token_headers: dict[str, str]): @@ -130,7 +130,7 @@ def test_get_nonexistent_api_key(db: Session, superuser_token_headers: dict[str, headers=superuser_token_headers, ) assert response.status_code == 404 - assert "API Key does not exist" in response.json()["detail"] + assert "API Key does not exist" in response.json()["error"] def test_revoke_api_key(db: Session, superuser_token_headers: dict[str, str]): @@ -161,5 +161,5 @@ def test_revoke_nonexistent_api_key( f"{settings.API_V1_STR}/apikeys/999999", headers=superuser_token_headers, ) - assert response.status_code == 400 - assert "API key not found or already deleted" in response.json()["detail"] + assert response.status_code == 404 + assert "API key not found or already deleted" in response.json()["error"] diff --git a/backend/app/tests/api/routes/test_creds.py b/backend/app/tests/api/routes/test_creds.py index c931c20cb..428d4c1c7 100644 --- a/backend/app/tests/api/routes/test_creds.py +++ b/backend/app/tests/api/routes/test_creds.py @@ -7,11 +7,13 @@ from app.main import app from app.api.deps import get_db from app.crud.credentials import set_creds_for_org -from app.models import CredsCreate, Organization, OrganizationCreate +from app.models import CredsCreate, Organization, OrganizationCreate, Project from app.core.config import settings from app.core.security import encrypt_api_key from app.core.providers import Provider from app.models.credentials import Credential +from app.core.security import decrypt_credentials + client = TestClient(app) @@ -77,6 +79,68 @@ def test_set_creds_for_org(db: Session, superuser_token_headers: dict[str, str]) assert data[0]["credential"]["model"] == "gpt-4" +def test_set_creds_for_invalid_project_org_relationship( + db: Session, superuser_token_headers: dict[str, str] +): + # Setup: Create two organizations and one project for each + org1 = Organization(name="Org 1", is_active=True) + org2 = Organization(name="Org 2", is_active=True) + db.add_all([org1, org2]) + db.commit() + db.refresh(org1) + db.refresh(org2) + + project2 = Project(name="Project Org2", organization_id=org2.id) + db.add(project2) + db.commit() + + # Invalid case: Organization mismatch (org1's creds for project2 of org2) + creds_data_invalid = { + "organization_id": org1.id, + "is_active": True, + "project_id": project2.id, # Invalid project for org1 + "credential": {Provider.OPENAI.value: {"api_key": "sk-123", "model": "gpt-4"}}, + } + + response_invalid = client.post( + f"{settings.API_V1_STR}/credentials/", + json=creds_data_invalid, + headers=superuser_token_headers, + ) + assert response_invalid.status_code == 400 + print(response_invalid.json()) + assert ( + response_invalid.json()["error"] + == "Project does not belong to the specified organization" + ) + + +def test_set_creds_for_project_not_found( + db: Session, superuser_token_headers: dict[str, str] +): + # Setup: Create an organization but no project + org = Organization(name="Org for Project Not Found", is_active=True) + db.add(org) + db.commit() + db.refresh(org) + + creds_data_invalid_project = { + "organization_id": org.id, + "is_active": True, + "project_id": 99999, + "credential": {Provider.OPENAI.value: {"api_key": "sk-123", "model": "gpt-4"}}, + } + + response_invalid_project = client.post( + f"{settings.API_V1_STR}/credentials/", + json=creds_data_invalid_project, + headers=superuser_token_headers, + ) + + assert response_invalid_project.status_code == 404 + assert response_invalid_project.json()["error"] == "Project not found" + + def test_read_credentials_with_creds( db: Session, superuser_token_headers: dict[str, str], create_organization_and_creds ): @@ -105,7 +169,7 @@ def test_read_credentials_not_found( headers=superuser_token_headers, ) assert response.status_code == 404 - assert "Credentials not found" in response.json()["detail"] + assert "Credentials not found" in response.json()["error"] def test_read_provider_credential( @@ -136,7 +200,7 @@ def test_read_provider_credential_not_found( ) assert response.status_code == 404 - assert response.json()["detail"] == "Provider credentials not found" + assert response.json()["error"] == "Provider credentials not found" def test_update_credentials( @@ -169,6 +233,46 @@ def test_update_credentials( assert data[0]["updated_at"] is not None +def test_update_credentials_failed_update( + db: Session, superuser_token_headers: dict[str, str], create_organization_and_creds +): + org, creds_data = create_organization_and_creds + + set_creds_for_org(session=db, creds_add=creds_data) + + org_without_creds = Organization(name="Org Without Creds", is_active=True) + db.add(org_without_creds) + db.commit() + db.refresh(org_without_creds) + + existing_creds = ( + db.query(Credential) + .filter(Credential.organization_id == org_without_creds.id) + .all() + ) + assert len(existing_creds) == 0 + + update_data = { + "provider": Provider.OPENAI.value, + "credential": { + "api_key": "sk-" + generate_random_string(), + "model": "gpt-4-turbo", + "temperature": 0.8, + }, + } + + response_invalid_org = client.patch( + f"{settings.API_V1_STR}/credentials/{org_without_creds.id}", # Valid org id but no creds + json=update_data, + headers=superuser_token_headers, + ) + assert response_invalid_org.status_code == 404 + assert ( + response_invalid_org.json()["error"] + == "Credentials not found for this provider" + ) + + def test_update_credentials_not_found( db: Session, superuser_token_headers: dict[str, str] ): @@ -190,8 +294,8 @@ def test_update_credentials_not_found( headers=superuser_token_headers, ) - assert response.status_code == 500 # Expect 404 for non-existent organization - assert "Organization not found" in response.json()["detail"] + assert response.status_code == 404 # Expect 404 for non-existent organization + assert "Organization not found" in response.json()["error"] def test_delete_provider_credential( @@ -220,8 +324,9 @@ def test_delete_provider_credential_not_found( headers=superuser_token_headers, ) - assert response.status_code == 404 # Expect 404 for not found - assert response.json()["detail"] == "Provider credentials not found" + assert response.status_code == 404 + print(response.json()) + assert response.json()["error"] == f"Provider credentials not found" def test_delete_all_credentials( @@ -245,7 +350,7 @@ def test_delete_all_credentials( headers=superuser_token_headers, ) assert response.status_code == 404 # Expect 404 as credentials are soft deleted - assert response.json()["detail"] == "Credentials not found" + assert response.json()["error"] == "Credentials not found" def test_delete_all_credentials_not_found( @@ -256,8 +361,8 @@ def test_delete_all_credentials_not_found( headers=superuser_token_headers, ) - assert response.status_code == 500 # Expect 404 for not found - assert "Credentials for organization not found" in response.json()["detail"] + assert response.status_code == 404 + assert "Credentials for organization not found" in response.json()["error"] def test_duplicate_credential_creation( @@ -278,8 +383,8 @@ def test_duplicate_credential_creation( json=creds_data.dict(), headers=superuser_token_headers, ) - assert response.status_code == 500 - assert "already exist" in response.json()["detail"] + assert response.status_code == 400 + assert "already exist" in response.json()["error"] def test_multiple_provider_credentials( @@ -355,9 +460,6 @@ def test_credential_encryption( ) assert response.status_code == 200 - # Get the raw credential from database to verify encryption - from app.core.security import decrypt_credentials - db_cred = ( db.query(Credential) .filter( diff --git a/backend/app/tests/api/routes/test_login.py b/backend/app/tests/api/routes/test_login.py index 80fa78797..a74723704 100644 --- a/backend/app/tests/api/routes/test_login.py +++ b/backend/app/tests/api/routes/test_login.py @@ -113,6 +113,6 @@ def test_reset_password_invalid_token( ) response = r.json() - assert "detail" in response + assert "error" in response assert r.status_code == 400 - assert response["detail"] == "Invalid token" + assert response["error"] == "Invalid token" diff --git a/backend/app/tests/api/routes/test_onboarding.py b/backend/app/tests/api/routes/test_onboarding.py index bf332a583..6f0327003 100644 --- a/backend/app/tests/api/routes/test_onboarding.py +++ b/backend/app/tests/api/routes/test_onboarding.py @@ -75,11 +75,9 @@ def test_create_user_existing_email( response = client.post( f"{settings.API_V1_STR}/onboard", json=data, headers=superuser_token_headers ) - assert response.status_code == 400 assert ( - response.json()["detail"] - == "400: API key already exists for this user and project." + response.json()["error"] == "API key already exists for this user and project." ) diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index ac518075e..d7e8db246 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -112,7 +112,7 @@ def test_get_existing_user_permissions_error( headers=normal_user_token_headers, ) assert r.status_code == 403 - assert r.json()["detail"] == "The user doesn't have enough privileges" + assert r.json()["error"] == "The user doesn't have enough privileges" def test_create_user_existing_username( @@ -244,7 +244,7 @@ def test_update_password_me_incorrect_password( ) assert r.status_code == 400 updated_user = r.json() - assert updated_user["detail"] == "Incorrect password" + assert updated_user["error"] == "Incorrect password" def test_update_user_me_email_exists( @@ -262,7 +262,7 @@ def test_update_user_me_email_exists( json=data, ) assert r.status_code == 409 - assert r.json()["detail"] == "User with this email already exists" + assert r.json()["error"] == "User with this email already exists" def test_update_password_me_same_password_error( @@ -279,9 +279,7 @@ def test_update_password_me_same_password_error( ) assert r.status_code == 400 updated_user = r.json() - assert ( - updated_user["detail"] == "New password cannot be the same as the current one" - ) + assert updated_user["error"] == "New password cannot be the same as the current one" def test_register_user(client: TestClient, db: Session) -> None: @@ -319,7 +317,7 @@ def test_register_user_already_exists_error(client: TestClient) -> None: json=data, ) assert r.status_code == 400 - assert r.json()["detail"] == "The user with this email already exists in the system" + assert r.json()["error"] == "The user with this email already exists in the system" def test_update_user( @@ -360,7 +358,7 @@ def test_update_user_not_exists( json=data, ) assert r.status_code == 404 - assert r.json()["detail"] == "The user with this id does not exist in the system" + assert r.json()["error"] == "The user with this id does not exist in the system" def test_update_user_email_exists( @@ -383,7 +381,7 @@ def test_update_user_email_exists( json=data, ) assert r.status_code == 409 - assert r.json()["detail"] == "User with this email already exists" + assert r.json()["error"] == "User with this email already exists" def test_delete_user_me(client: TestClient, db: Session) -> None: @@ -426,7 +424,7 @@ def test_delete_user_me_as_superuser( ) assert r.status_code == 403 response = r.json() - assert response["detail"] == "Super users are not allowed to delete themselves" + assert response["error"] == "Super users are not allowed to delete themselves" def test_delete_user_super_user( @@ -458,7 +456,7 @@ def test_delete_user_not_found( headers=superuser_token_headers, ) assert r.status_code == 404 - assert r.json()["detail"] == "User not found" + assert r.json()["error"] == "User not found" def test_delete_user_current_super_user_error( @@ -473,7 +471,7 @@ def test_delete_user_current_super_user_error( headers=superuser_token_headers, ) assert r.status_code == 403 - assert r.json()["detail"] == "Super users are not allowed to delete themselves" + assert r.json()["error"] == "Super users are not allowed to delete themselves" def test_delete_user_without_privileges( @@ -489,4 +487,4 @@ def test_delete_user_without_privileges( headers=normal_user_token_headers, ) assert r.status_code == 403 - assert r.json()["detail"] == "The user doesn't have enough privileges" + assert r.json()["error"] == "The user doesn't have enough privileges" diff --git a/backend/app/tests/crud/documents/test_crud_document_delete.py b/backend/app/tests/crud/documents/test_crud_document_delete.py index 8b42451a5..093294108 100644 --- a/backend/app/tests/crud/documents/test_crud_document_delete.py +++ b/backend/app/tests/crud/documents/test_crud_document_delete.py @@ -6,6 +6,7 @@ from app.models import Document from app.tests.utils.document import DocumentStore +from app.core.exception_handlers import HTTPException @pytest.fixture @@ -36,5 +37,8 @@ def test_cannot_delete_others_documents(self, db: Session): other_owner_id = store.documents.owner_id + 1 crud = DocumentCrud(db, other_owner_id) - with pytest.raises(NoResultFound): + with pytest.raises(HTTPException) as exc_info: crud.delete(document.id) + + assert exc_info.value.status_code == 404 + assert "Document not found" in str(exc_info.value.detail) diff --git a/backend/app/tests/crud/documents/test_crud_document_read_one.py b/backend/app/tests/crud/documents/test_crud_document_read_one.py index 01529bdaa..463ea02ea 100644 --- a/backend/app/tests/crud/documents/test_crud_document_read_one.py +++ b/backend/app/tests/crud/documents/test_crud_document_read_one.py @@ -5,6 +5,7 @@ from app.crud import DocumentCrud from app.tests.utils.document import DocumentStore +from app.core.exception_handlers import HTTPException @pytest.fixture @@ -25,9 +26,13 @@ def test_cannot_select_invalid_id(self, db: Session, store: DocumentStore): document = next(store.documents) crud = DocumentCrud(db, store.owner) - with pytest.raises(NoResultFound): + + with pytest.raises(HTTPException) as exc_info: crud.read_one(document.id) + assert exc_info.value.status_code == 404 + assert "Document not found" in str(exc_info.value.detail) + def test_cannot_read_others_documents( self, db: Session, @@ -37,5 +42,8 @@ def test_cannot_read_others_documents( other = DocumentStore(db) crud = DocumentCrud(db, other.owner) - with pytest.raises(NoResultFound): + with pytest.raises(HTTPException) as exc_info: crud.read_one(document.id) + + assert exc_info.value.status_code == 404 + assert "Document not found" in str(exc_info.value.detail) diff --git a/backend/app/tests/crud/test_api_key.py b/backend/app/tests/crud/test_api_key.py index 07d2819c7..197756bd3 100644 --- a/backend/app/tests/crud/test_api_key.py +++ b/backend/app/tests/crud/test_api_key.py @@ -1,10 +1,12 @@ import uuid import pytest +from datetime import datetime from sqlmodel import Session, select from app.crud import api_key as api_key_crud from app.models import APIKey, User, Organization, Project from app.tests.utils.utils import random_email from app.core.security import get_password_hash, verify_password, decrypt_api_key +from app.core.exception_handlers import HTTPException # Helper function to create a user @@ -88,18 +90,6 @@ def test_delete_api_key(db: Session) -> None: assert deleted_key.deleted_at is not None -def test_delete_api_key_already_deleted(db: Session) -> None: - user = create_test_user(db) - org = create_test_organization(db) - project = create_test_project(db, org.id) - - api_key = api_key_crud.create_api_key(db, org.id, user.id, project.id) - api_key_crud.delete_api_key(db, api_key.id) - - with pytest.raises(ValueError, match="API key not found or already deleted"): - api_key_crud.delete_api_key(db, api_key.id) - - def test_get_api_key_by_value(db: Session) -> None: user = create_test_user(db) org = create_test_organization(db) diff --git a/backend/app/tests/crud/test_project.py b/backend/app/tests/crud/test_project.py index 87d019e39..c9b135ce4 100644 --- a/backend/app/tests/crud/test_project.py +++ b/backend/app/tests/crud/test_project.py @@ -1,5 +1,6 @@ import pytest from sqlmodel import Session +from fastapi import HTTPException from app.models import Project, ProjectCreate, Organization from app.crud.project import ( @@ -110,7 +111,7 @@ def test_validate_project_success(db: Session) -> None: def test_validate_project_not_found(db: Session) -> None: """Test that validation fails when project does not exist.""" - with pytest.raises(ValueError, match="Project not found"): + with pytest.raises(HTTPException, match="Project not found"): validate_project(session=db, project_id=9999) @@ -131,5 +132,5 @@ def test_validate_project_inactive(db: Session) -> None: ), ) - with pytest.raises(ValueError, match="Project is not active"): + with pytest.raises(HTTPException, match="Project is not active"): validate_project(session=db, project_id=inactive_project.id) diff --git a/backend/app/utils.py b/backend/app/utils.py index f9d91503f..63712ebd3 100644 --- a/backend/app/utils.py +++ b/backend/app/utils.py @@ -35,8 +35,13 @@ def success_response( return cls(success=True, data=data, error=None, metadata=metadata) @classmethod - def failure_response(cls, error: str) -> "APIResponse[None]": - return cls(success=False, data=None, error=error) + def failure_response(cls, error: str | list) -> "APIResponse[None]": + if isinstance(error, list): # to handle cases when error is a list of errors + error_message = "\n".join([f"{err['loc']}: {err['msg']}" for err in error]) + else: + error_message = error + + return cls(success=False, data=None, error=error_message) @dataclass