diff --git a/.github/workflows/run-python-tests.yml b/.github/workflows/run-python-tests.yml index ecbd341..4a24fe6 100644 --- a/.github/workflows/run-python-tests.yml +++ b/.github/workflows/run-python-tests.yml @@ -38,9 +38,58 @@ jobs: - name: Download sample data run: curl https://atlas-education.s3.amazonaws.com/sampledata.archive -o sampledata.archive - - name: Add sample data to database - run: mongorestore --archive=sampledata.archive --port=27017 + - name: Setup Database (Data & Indexes) + run: | + # 1. Restore the data + mongorestore --archive=sampledata.archive --port=27017 + + # 2. Prepare the Search Index Definition + echo '{ + "name": "movieSearchIndex", + "database": "sample_mflix", + "collectionName": "movies", + "mappings": { + "dynamic": false, + "fields": { + "plot": {"type": "string", "analyzer": "lucene.standard"}, + "fullplot": {"type": "string", "analyzer": "lucene.standard"}, + "directors": {"type": "string", "analyzer": "lucene.standard"}, + "writers": {"type": "string", "analyzer": "lucene.standard"}, + "cast": {"type": "string", "analyzer": "lucene.standard"} + } + } + }' > search_index.json + + # 3. Create the Search Index + atlas deployments search indexes create \ + --deploymentName myLocalRs1 \ + --file search_index.json + + # 4. Prepare the Vector Index Definition + echo '{ + "name": "vector_index", + "database": "sample_mflix", + "collectionName": "embedded_movies", + "type": "vectorSearch", + "fields": [ + { + "type": "vector", + "path": "plot_embedding_voyage_3_large", + "numDimensions": 2048, + "similarity": "cosine" + } + ] + }' > vector_index.json + # 5. Create the Vector Index + atlas deployments search indexes create \ + --deploymentName myLocalRs1 \ + --file vector_index.json + + # 6. Wait for indexes to build + echo "Waiting for indexes to build..." + sleep 20 + - name: Set up Python uses: actions/setup-python@v5 with: @@ -63,10 +112,10 @@ jobs: - name: Run integration tests working-directory: mflix/server/python-fastapi - run: pytest -m integration --verbose --tb=short --junit-xml=test-results-integration.xml || true + run: pytest -m integration --verbose --tb=short --junit-xml=test-results-integration.xml env: - MONGO_URI: mongodb://localhost:27017 - MONGO_DB: sample_mflix + MONGO_URI: mongodb://localhost:27017/?directConnection=true + MONGO_DB: sample_mflix - name: Upload test results uses: actions/upload-artifact@v4 diff --git a/mflix/server/python-fastapi/main.py b/mflix/server/python-fastapi/main.py index 9d2290e..6a770cb 100644 --- a/mflix/server/python-fastapi/main.py +++ b/mflix/server/python-fastapi/main.py @@ -2,7 +2,6 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware from src.routers import movies -from src.utils.errorHandler import register_error_handlers from src.database.mongo_client import db, get_collection import os @@ -15,8 +14,9 @@ @asynccontextmanager async def lifespan(app: FastAPI): # Startup: Create search indexes - await ensure_search_index() - await vector_search_index() + await ensure_mongodb_search_index() + await ensure_vector_search_index() + await ensure_standard_index() # Print server information print(f"\n{'='*60}") @@ -30,10 +30,9 @@ async def lifespan(app: FastAPI): # Add any cleanup code here -async def ensure_search_index(): +async def ensure_mongodb_search_index(): try: movies_collection = db.get_collection("movies") - comments_collection = db.get_collection("comments") # Check and create search index for movies collection result = await movies_collection.list_search_indexes() @@ -71,7 +70,7 @@ async def ensure_search_index(): ) -async def vector_search_index(): +async def ensure_vector_search_index(): """ Creates vector search index on application startup if it doesn't already exist. This ensures the index is ready before any vector search requests are made. @@ -114,6 +113,26 @@ async def vector_search_index(): f"and verify the 'embedded_movies' collection exists with the required embedding field." ) +async def ensure_standard_index(): + """ + Creates a standard MongoDB index on the comments collection on application startup. + This improves performance for queries filtering by movie_id such as ReportingByComments(). + """ + + try: + comments_collection = db.get_collection("comments") + + existing_indexes_cursor = await comments_collection.list_indexes() + existing_indexes = [index async for index in existing_indexes_cursor] + index_names = [index.get("name") for index in existing_indexes] + standard_index_name = "movie_id_index" + if standard_index_name not in index_names: + await comments_collection.create_index([("movie_id", 1)], name=standard_index_name) + + except Exception as e: + print(f"Failed to create standard index on 'comments' collection: {str(e)}. ") + print(f"Performance may be degraded. Please check your MongoDB configuration.") + app = FastAPI(lifespan=lifespan) @@ -127,6 +146,5 @@ async def vector_search_index(): allow_headers=["*"], ) -register_error_handlers(app) app.include_router(movies.router, prefix="/api/movies", tags=["movies"]) diff --git a/mflix/server/python-fastapi/src/database/mongo_client.py b/mflix/server/python-fastapi/src/database/mongo_client.py index 0ff350e..024fbdd 100644 --- a/mflix/server/python-fastapi/src/database/mongo_client.py +++ b/mflix/server/python-fastapi/src/database/mongo_client.py @@ -18,4 +18,6 @@ def get_collection(name:str): def voyage_ai_available(): """Check if Voyage API Key is available and valid.""" api_key = os.getenv("VOYAGE_API_KEY") + if api_key is None or api_key =="your_voyage_api_key": + return None return api_key is not None and api_key.strip() != "" \ No newline at end of file diff --git a/mflix/server/python-fastapi/src/models/models.py b/mflix/server/python-fastapi/src/models/models.py index f9494c7..01c1489 100644 --- a/mflix/server/python-fastapi/src/models/models.py +++ b/mflix/server/python-fastapi/src/models/models.py @@ -127,22 +127,9 @@ class SuccessResponse(BaseModel, Generic[T]): timestamp: str pagination: Optional[Pagination] = None - -class ErrorDetails(BaseModel): - message: str - code: Optional[str] - details: Optional[Any] = None - class BatchUpdateRequest(BaseModel): filter: MovieFilter update: UpdateMovieRequest class BatchDeleteRequest(BaseModel): filter: MovieFilter - -class ErrorResponse(BaseModel): - success: bool = False - message: str - error: ErrorDetails - timestamp: str - \ No newline at end of file diff --git a/mflix/server/python-fastapi/src/routers/movies.py b/mflix/server/python-fastapi/src/routers/movies.py index 4cc2154..d8fa176 100644 --- a/mflix/server/python-fastapi/src/routers/movies.py +++ b/mflix/server/python-fastapi/src/routers/movies.py @@ -1,10 +1,9 @@ -from fastapi import APIRouter, Query, Path, Body +from fastapi import APIRouter, Query, Path, Body, HTTPException from src.database.mongo_client import get_collection, voyage_ai_available from src.models.models import VectorSearchResult, CreateMovieRequest, Movie, SuccessResponse, UpdateMovieRequest, SearchMoviesResponse - -from typing import Any, List -from src.utils.errorHandler import create_success_response, create_error_response -from bson import ObjectId +from typing import Any, List, Optional +from src.utils.successResponse import create_success_response +from bson import ObjectId, errors import re from bson.errors import InvalidId import voyageai @@ -105,15 +104,15 @@ @router.get( "/search", response_model=SuccessResponse[SearchMoviesResponse], - status_code=200, + status_code = 200, summary="Search movies using MongoDB Search." ) async def search_movies( - plot: str = Query(default=None), - fullplot: str = Query(default=None), - directors: str = Query(default=None), - writers: str = Query(default=None), - cast: str = Query(default=None), + plot: Optional[str] = None, + fullplot: Optional[str] = None, + directors: Optional[str] = None, + writers: Optional[str] = None, + cast: Optional[str] = None, limit:int = Query(default=20, ge=1, le=100), skip:int = Query(default=0, ge=0), search_operator: str = Query(default="must", alias="searchOperator") @@ -123,17 +122,17 @@ async def search_movies( # Validate the search_operator parameter to ensure it's a valid compound operator valid_operators = {"must", "should", "mustNot", "filter"} + if search_operator not in valid_operators: - return create_error_response( - message=f"Invalid search_operator '{search_operator}'. The search_operator must be one of {valid_operators}.", - code="INVALID_SEARCH_OPERATOR", - details=None - ) + raise HTTPException( + status_code = 400, + detail=f"Invalid search operator '{search_operator}'. The search operator must be one of {valid_operators}." + ) # Build the search_phrases list based on which fields were provided by the user. # Each phrase becomes a separate clause in the MongoDB Search compound query. - if plot: + if plot is not None: search_phrases.append({ # The phrase operator performs an exact phrase match on the specified field. This is useful for searching for specific phrases within text fields. # The text operator is more flexible and allows for fuzzy matching, making it suitable for fields like names where typos may occur. @@ -142,14 +141,14 @@ async def search_movies( "path": "plot", } }) - if fullplot: + if fullplot is not None: search_phrases.append({ "phrase": { "query": fullplot, "path": "fullplot", } }) - if directors: + if directors is not None: # The "fuzzy" option enables typo-tolerant (fuzzy) search within MongoDB Search. # - maxEdits: The maximum number of single-character edits (insertions, deletions, or substitutions) # allowed when matching the search term to indexed terms. (Range: 1-2; higher = more tolerant) @@ -165,7 +164,7 @@ async def search_movies( } }) - if writers: + if writers is not None: # See comments above regarding fuzzy search options. search_phrases.append({ "text": { @@ -174,7 +173,7 @@ async def search_movies( "fuzzy":{"maxEdits":1, "prefixLength":5} } }) - if cast: + if cast is not None: # See comments above regarding fuzzy search options. search_phrases.append({ "text": { @@ -185,10 +184,9 @@ async def search_movies( }) if not search_phrases: - return create_error_response( - message="At least one search parameter must be provided.", - code="NO_SEARCH_PARAMETERS", - details=None + raise HTTPException( + status_code = 400, + detail="At least one search parameter must be provided." ) # Build the aggregation pipeline for MongoDB Search. @@ -241,11 +239,11 @@ async def search_movies( try: results = await execute_aggregation(aggregation_pipeline) except Exception as e: - return create_error_response( - message="An error occurred while performing the search.", - code="DATABASE_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred while performing the search: {str(e)}" ) + # Extract total count and movies from facet results with proper bounds checking if not results or len(results) == 0: @@ -319,12 +317,10 @@ async def vector_search_movies( SuccessResponse containing a list of movies with similarity scores """ if not voyage_ai_available(): - return create_error_response( - message="Vector search unavailable", - code="SERVICE_UNAVAILABLE", - details="VOYAGE_API_KEY not configured. Please add your API key to your .env file." + raise HTTPException( + status_code = 503, + detail="Vector search unavailable: VOYAGE_API_KEY not configured. Please add your API key to your .env file." ) - try: # Initialize the client here to avoid import-time errors vo = voyageai.Client() @@ -395,10 +391,9 @@ async def vector_search_movies( ) except Exception as e: - return create_error_response( - message="Vector search failed", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred during vector search: {str(e)}" ) """ @@ -412,34 +407,32 @@ async def vector_search_movies( @router.get("/{id}", response_model=SuccessResponse[Movie], - status_code=200, + status_code = 200, summary="Retrieve a single movie by its ID.") async def get_movie_by_id(id: str): # Validate ObjectId format try: object_id = ObjectId(id) - except InvalidId: - return create_error_response( - message="Invalid movie ID format", - code="INTERNAL_SERVER_ERROR", - details=f"The provided ID '{id}' is not a valid ObjectId" + except errors.InvalidId: + raise HTTPException( + status_code = 400, + detail=f"The provided ID '{id}' is not a valid ObjectId" ) movies_collection = get_collection("movies") try: movie = await movies_collection.find_one({"_id": object_id}) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) + if movie is None: - return create_error_response( - message="Movie not found", - code="INTERNAL_SERVER_ERROR", - details=f"No movie found with ID: {id}" + raise HTTPException( + status_code = 404, + detail=f"No movie found with ID: {id}" ) movie["_id"] = str(movie["_id"]) # Convert ObjectId to string @@ -468,7 +461,7 @@ async def get_movie_by_id(id: str): @router.get("/", response_model=SuccessResponse[List[Movie]], - status_code=200, + status_code = 200, summary="Retrieve a list of movies with optional filtering, sorting, and pagination.") # Validate the query parameters using FastAPI's Query functionality. async def get_all_movies( @@ -511,25 +504,25 @@ async def get_all_movies( try: result = movies_collection.find(filter_dict).sort(sort).skip(skip).limit(limit) except Exception as e: - return create_error_response( - message="An error occurred while fetching movies.", - code="DATABASE_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred while fetching movies. {str(e)}" ) movies = [] async for movie in result: - movie["_id"] = str(movie["_id"]) # Convert ObjectId to string - # Ensure that the year field contains int value. - if "year" in movie and not isinstance(movie["year"], int): - cleaned_year = re.sub(r"\D", "", str(movie["year"])) - try: - movie["year"] = int(cleaned_year) if cleaned_year else None - except ValueError: - movie["year"] = None - - movies.append(movie) + if "title" in movie: + movie["_id"] = str(movie["_id"]) # Convert ObjectId to string + # Ensure that the year field contains int value. + if "year" in movie and not isinstance(movie["year"], int): + cleaned_year = re.sub(r"\D", "", str(movie["year"])) + try: + movie["year"] = int(cleaned_year) if cleaned_year else None + except ValueError: + movie["year"] = None + + movies.append(movie) # Return the results wrapped in a SuccessResponse return create_success_response(movies, f"Found {len(movies)} movies.") @@ -545,7 +538,7 @@ async def get_all_movies( @router.post("/", response_model=SuccessResponse[Movie], - status_code=201, + status_code = 201, summary="Creates a new movie in the database.") async def create_movie(movie: CreateMovieRequest): # Pydantic automatically validates the structure @@ -555,35 +548,31 @@ async def create_movie(movie: CreateMovieRequest): try: result = await movies_collection.insert_one(movie_data) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) # Verify that the document was created before querying it if not result.acknowledged: - return create_error_response( - message="Failed to create movie", - code="INTERNAL_SERVER_ERROR", - details="The database did not acknowledge the insert operation" + raise HTTPException( + status_code = 500, + detail="Failed to create movie: The database did not acknowledge the insert operation" ) try: # Retrieve the created document to return complete data created_movie = await movies_collection.find_one({"_id": result.inserted_id}) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) if created_movie is None: - return create_error_response( - message="Movie creation verification failed", - code="INTERNAL_SERVER_ERROR", - details="Movie was created but could not be retrieved for verification" + raise HTTPException( + status_code = 500, + detail="Movie was created but could not be retrieved for verification" ) created_movie["_id"] = str(created_movie["_id"]) # Convert ObjectId to string @@ -627,10 +616,9 @@ async def create_movies_batch(movies: List[CreateMovieRequest]) ->SuccessRespons #Verify that the movies list is not empty if not movies: - return create_error_response( - message="Request body must be a non-empty list of movies.", - code="INVALID_INPUT", - details=None + raise HTTPException( + status_code = 400, + detail="Request body must be a non-empty list of movies." ) movies_dicts = [] @@ -650,10 +638,9 @@ async def create_movies_batch(movies: List[CreateMovieRequest]) ->SuccessRespons f"Successfully created {len(result.inserted_ids)} movies." ) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) """ @@ -673,7 +660,7 @@ async def create_movies_batch(movies: List[CreateMovieRequest]) ->SuccessRespons @router.patch( "/{id}", response_model=SuccessResponse[Movie], - status_code=200, + status_code = 200, summary="Update a single movie by its ID.") async def update_movie( movie_data: UpdateMovieRequest, @@ -686,20 +673,18 @@ async def update_movie( try: movie_id = ObjectId(movie_id) except Exception : - return create_error_response( - message="Invalid movie_id format.", - code="INVALID_OBJECT_ID", - details=str(movie_id) + raise HTTPException( + status_code = 400, + detail=f"Invalid movie_id format: {movie_id}" ) update_dict = movie_data.model_dump(exclude_unset=True, exclude_none=True) # Validate that the dict is not empty if not update_dict: - return create_error_response( - message="No valid fields provided for update.", - code="NO_UPDATE_DATA", - details=None + raise HTTPException( + status_code = 400, + detail="No valid fields provided for update." ) try: @@ -708,17 +693,15 @@ async def update_movie( {"$set":update_dict} ) except Exception as e: - return create_error_response( - message="An error occurred while updating the movie.", - code="DATABASE_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred while updating the movie: {str(e)}" ) if result.matched_count == 0: - return create_error_response( - message="No movie with that _id was found.", - code="MOVIE_NOT_FOUND", - details=str(movie_id) + raise HTTPException( + status_code = 404, + detail=f"No movie with that _id was found: {movie_id}" ) updatedMovie = await movies_collection.find_one({"_id": movie_id}) @@ -740,7 +723,7 @@ async def update_movie( @router.patch("/", response_model=SuccessResponse[dict], - status_code=200, + status_code = 200, summary="Batch update movies matching the given filter." ) async def update_movies_batch( @@ -753,10 +736,9 @@ async def update_movies_batch( update_data = request_body.get("update", {}) if not filter_data or not update_data: - return create_error_response( - message="Both filter and update objects are required", - code="MISSING_REQUIRED_FIELDS", - details=None + raise HTTPException( + status_code = 400, + detail="Both filter and update objects are required" ) # Convert string IDs to ObjectIds if _id filter is present @@ -766,19 +748,17 @@ async def update_movies_batch( try: filter_data["_id"]["$in"] = [ObjectId(id_str) for id_str in filter_data["_id"]["$in"]] except Exception: - return create_error_response( - message="Invalid ObjectId format in filter", - code="INVALID_OBJECT_ID", - details=None + raise HTTPException( + status_code = 400, + detail="Invalid ObjectId format in filter", ) try: result = await movies_collection.update_many(filter_data, {"$set": update_data}) except Exception as e: - return create_error_response( - message="An error occurred while updating movies.", - code="DATABASE_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred while updating movies: {str(e)}" ) return create_success_response({ @@ -799,16 +779,15 @@ async def update_movies_batch( @router.delete("/{id}", response_model=SuccessResponse[dict], - status_code=200, + status_code = 200, summary="Delete a single movie by its ID.") async def delete_movie_by_id(id: str): try: object_id = ObjectId(id) - except InvalidId: - return create_error_response( - message="Invalid movie ID format", - code="INTERNAL_SERVER_ERROR", - details=f"The provided ID '{id}' is not a valid ObjectId" + except errors.InvalidId: + raise HTTPException( + status_code = 400, + detail=f"Invalid movie ID format: The provided ID '{id}' is not a valid ObjectId" ) movies_collection = get_collection("movies") @@ -816,17 +795,15 @@ async def delete_movie_by_id(id: str): # Use deleteOne() to remove a single document result = await movies_collection.delete_one({"_id": object_id}) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) if result.deleted_count == 0: - return create_error_response( - message="Movie not found", - code="INTERNAL_SERVER_ERROR", - details=f"No movie found with ID: {id}" + raise HTTPException( + status_code = 404, + detail=f"No movie found with ID: {id}" ) return create_success_response( @@ -849,7 +826,7 @@ async def delete_movie_by_id(id: str): @router.delete( "/", response_model=SuccessResponse[dict], - status_code=200, + status_code = 200, summary="Delete multiple movies matching the given filter." ) async def delete_movies_batch(request_body: dict = Body(...)) -> SuccessResponse[dict]: @@ -860,10 +837,9 @@ async def delete_movies_batch(request_body: dict = Body(...)) -> SuccessResponse filter_data = request_body.get("filter", {}) if not filter_data: - return create_error_response( - message="Filter object is required and cannot be empty.", - code="MISSING_FILTER", - details=None + raise HTTPException( + status_code = 400, + detail="Filter object is required and cannot be empty." ) # Convert string IDs to ObjectIds if _id filter is present @@ -873,19 +849,17 @@ async def delete_movies_batch(request_body: dict = Body(...)) -> SuccessResponse try: filter_data["_id"]["$in"] = [ObjectId(id_str) for id_str in filter_data["_id"]["$in"]] except Exception: - return create_error_response( - message="Invalid ObjectId format in filter", - code="INVALID_OBJECT_ID", - details=None + raise HTTPException( + status_code = 400, + detail="Invalid ObjectId format in filter." ) try: result = await movies_collection.delete_many(filter_data) except Exception as e: - return create_error_response( - message="An error occurred while deleting movies.", - code="DATABASE_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"An error occurred while deleting movies: {str(e)}" ) return create_success_response( @@ -905,16 +879,15 @@ async def delete_movies_batch(request_body: dict = Body(...)) -> SuccessResponse @router.delete("/{id}/find-and-delete", response_model=SuccessResponse[Movie], - status_code=200, + status_code = 200, summary="Find and delete a movie in a single operation.") async def find_and_delete_movie(id: str): try: object_id = ObjectId(id) - except InvalidId: - return create_error_response( - message="Invalid movie ID format", - code="INTERNAL_SERVER_ERROR", - details=f"The provided ID '{id}' is not a valid ObjectId" + except errors.InvalidId: + raise HTTPException( + status_code = 400, + detail=f"Invalid movie ID format: The provided ID '{id}' is not a valid ObjectId" ) movies_collection = get_collection("movies") @@ -924,17 +897,15 @@ async def find_and_delete_movie(id: str): try: deleted_movie = await movies_collection.find_one_and_delete({"_id": object_id}) except Exception as e: - return create_error_response( - message="Database error occurred", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred: {str(e)}" ) if deleted_movie is None: - return create_error_response( - message="Movie not found", - code="INTERNAL_SERVER_ERROR", - details=f"No movie found with ID: {id}" + raise HTTPException( + status_code = 404, + detail=f"No movie found with ID: {id}" ) deleted_movie["_id"] = str(deleted_movie["_id"]) # Convert ObjectId to string @@ -953,7 +924,7 @@ async def find_and_delete_movie(id: str): @router.get("/aggregations/reportingByComments", response_model=SuccessResponse[List[dict]], - status_code=200, + status_code = 200, summary="Aggregate movies with their most recent comments.") async def aggregate_movies_recent_commented( limit: int = Query(default=10, ge=1, le=50), @@ -984,10 +955,9 @@ async def aggregate_movies_recent_commented( object_id = ObjectId(movie_id) pipeline[0]["$match"]["_id"] = object_id except Exception: - return create_error_response( - message="Invalid movie ID format", - code="INTERNAL_SERVER_ERROR", - details="The provided movie_id is not a valid ObjectId" + raise HTTPException( + status_code = 400, + detail="The provided movie_id is not a valid ObjectId" ) # Add remaining pipeline stages @@ -1074,10 +1044,9 @@ async def aggregate_movies_recent_commented( try: results = await execute_aggregation(pipeline) except Exception as e: - return create_error_response( - message="Database error occurred during aggregation", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred during aggregation: {str(e)}" ) # Convert ObjectId to string for response @@ -1103,7 +1072,7 @@ async def aggregate_movies_recent_commented( @router.get("/aggregations/reportingByYear", response_model=SuccessResponse[List[dict]], - status_code=200, + status_code = 200, summary="Aggregate movies by year with average rating and movie count.") async def aggregate_movies_by_year(): # Define aggregation pipeline to group movies by year with statistics @@ -1205,10 +1174,9 @@ async def aggregate_movies_by_year(): try: results = await execute_aggregation(pipeline) except Exception as e: - return create_error_response( - message="Database error occurred during aggregation", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred during aggregation: {str(e)}" ) return create_success_response( @@ -1228,7 +1196,7 @@ async def aggregate_movies_by_year(): @router.get("/aggregations/reportingByDirectors", response_model=SuccessResponse[List[dict]], - status_code=200, + status_code = 200, summary="Aggregate directors with the most movies and their statistics.") async def aggregate_directors_most_movies( limit: int = Query(default=20, ge=1, le=100) @@ -1304,10 +1272,9 @@ async def aggregate_directors_most_movies( try: results = await execute_aggregation(pipeline) except Exception as e: - return create_error_response( - message="Database error occurred during aggregation", - code="INTERNAL_SERVER_ERROR", - details=str(e) + raise HTTPException( + status_code = 500, + detail=f"Database error occurred during aggregation: {str(e)}" ) return create_success_response( @@ -1389,3 +1356,4 @@ def get_embedding(data, input_type = "document", client=None): data, model = model, output_dimension = outputDimension, input_type = input_type ).embeddings return embeddings[0] + diff --git a/mflix/server/python-fastapi/src/utils/errorHandler.py b/mflix/server/python-fastapi/src/utils/errorHandler.py deleted file mode 100644 index 5e235ec..0000000 --- a/mflix/server/python-fastapi/src/utils/errorHandler.py +++ /dev/null @@ -1,109 +0,0 @@ -from fastapi import Request -from fastapi.responses import JSONResponse -from pymongo.errors import PyMongoError, DuplicateKeyError, WriteError -from datetime import datetime, timezone -from typing import Any, Optional -from src.models.models import ErrorDetails, ErrorResponse, SuccessResponse, T - -''' -Creates a standardized success response. - - -Args: - data (T): The data to include in the response. - message (Optional[str]): An optional message to include. - -Returns: - SuccessResponse[T]: A standardized success response object. - ''' - -def create_success_response(data:T, message: Optional[str] = None) -> SuccessResponse[T]: - return SuccessResponse( - message=message or "Operation completed successfully.", - data=data, - timestamp=datetime.now(timezone.utc).isoformat() + "Z", - - ) - -''' -Creates a standardized error response. - -Args: - message (str): The error message. - code (Optional[str]): An optional error code. - details (Optional[Any]): Additional error details. - -Returns: - ErrorResponse: A standardized error response object. - -''' - -def create_error_response(message: str, code: Optional[str]=None, details: Optional[Any]=None) -> ErrorResponse: - return ErrorResponse( - message=message, - error=ErrorDetails( - message=message, - code=code, - details=details - ), - timestamp=datetime.now(timezone.utc).isoformat() + "Z", - ) - - -def parse_mongo_exception(exc: Exception) -> dict: - if isinstance(exc, DuplicateKeyError): - return{ - "message": "Duplicate key error occurred.", - "code": "DUPLICATE_KEY_ERROR", - "details": "A document with the same key already exists.", - "statusCode":409 - } - - # This is stating that the data that you are trying to implement is the wrong shape - # for the schema implemented in MongoDB. - elif isinstance(exc, WriteError): - return{ - "message": "Document validation failed.", - "code": "WRITE_ERROR", - "details": str(exc), - "statusCode":400 - } - - elif isinstance(exc, PyMongoError): - return { - "message" : "A database error occurred.", - "code": "DATABASE_ERROR", - "details": str(exc), - "statusCode":500 - } - return { - "message": "An unknown error occurred.", - "code": "UNKNOWN_ERROR", - "details": str(exc), - "statusCode": 500 - } - -def register_error_handlers(app): - - @app.exception_handler(PyMongoError) - async def mongo_exception_handler(request: Request, exc: PyMongoError): - error_details = parse_mongo_exception(exc) - return JSONResponse( - status_code = error_details["statusCode"], - content=create_error_response( - message=error_details["message"], - code=error_details["code"], - details=error_details["details"] - ).model_dump() - ) - - @app.exception_handler(Exception) - async def generic_exception_handler(request: Request, exc: Exception): - return JSONResponse( - status_code=500, - content=create_error_response( - message=str(exc), - code="INTERNAL_SERVER_ERROR", - details=getattr(exc, 'detail', None) or getattr(exc, 'args', None) - ).model_dump() - ) diff --git a/mflix/server/python-fastapi/src/utils/successResponse.py b/mflix/server/python-fastapi/src/utils/successResponse.py new file mode 100644 index 0000000..1353dff --- /dev/null +++ b/mflix/server/python-fastapi/src/utils/successResponse.py @@ -0,0 +1,24 @@ +from datetime import datetime, timezone +from typing import Optional +from src.models.models import SuccessResponse, T + +''' +Creates a standardized success response. + + +Args: + data (T): The data to include in the response. + message (Optional[str]): An optional message to include. + +Returns: + SuccessResponse[T]: A standardized success response object. + ''' + +def create_success_response(data:T, message: Optional[str] = None) -> SuccessResponse[T]: + return SuccessResponse( + message=message or "Operation completed successfully.", + data=data, + timestamp=datetime.now(timezone.utc).isoformat() + "Z", + + ) + diff --git a/mflix/server/python-fastapi/tests/integration/conftest.py b/mflix/server/python-fastapi/tests/integration/conftest.py index 6fb4e2d..80ed68d 100644 --- a/mflix/server/python-fastapi/tests/integration/conftest.py +++ b/mflix/server/python-fastapi/tests/integration/conftest.py @@ -56,13 +56,13 @@ def server(): # Start the server process process = subprocess.Popen( [sys.executable, "-m", "uvicorn", "main:app", "--host", "127.0.0.1", "--port", str(test_port)], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, cwd=server_python_dir ) - # Wait for server to be ready (max 10 seconds) - max_wait = 10 + # Wait for server to be ready (max 30 seconds) + max_wait = 30 start_time = time.time() while time.time() - start_time < max_wait: if is_port_in_use(test_port): @@ -209,5 +209,5 @@ async def test_batch_operation(multiple_test_movies): if response_data.get("success") is False and "not found" in response_data.get("error", {}).get("message", "").lower(): # Movie was already deleted, which is fine continue - assert cleanup_response.status_code == 200, f"Failed to clean up movie {movie_id}" + assert cleanup_response.status_code in [200,404], f"Failed to clean up movie {movie_id}" diff --git a/mflix/server/python-fastapi/tests/integration/test_movie_routes_integration.py b/mflix/server/python-fastapi/tests/integration/test_movie_routes_integration.py index f018bbd..c9c9730 100644 --- a/mflix/server/python-fastapi/tests/integration/test_movie_routes_integration.py +++ b/mflix/server/python-fastapi/tests/integration/test_movie_routes_integration.py @@ -64,7 +64,7 @@ async def test_create_and_retrieve_movie(self, client, test_movie_data): finally: # Cleanup: Delete the test movie delete_response = await client.delete(f"/api/movies/{movie_id}") - assert delete_response.status_code == 200 + assert delete_response.status_code in [200, 404] @pytest.mark.asyncio async def test_update_movie(self, client, created_movie): @@ -123,12 +123,11 @@ async def test_delete_movie(self, client, test_movie_data): assert delete_data["success"] is True # Verify movie no longer exists - # Note: The API returns 200 with INTERNAL_SERVER_ERROR code, not 404 get_response = await client.get(f"/api/movies/{movie_id}") + assert get_response.status_code == 404 error_data = get_response.json() - assert error_data["success"] is False - assert error_data["error"]["code"] == "INTERNAL_SERVER_ERROR" - assert "not found" in error_data["error"]["message"].lower() + assert "detail" in error_data + assert "no movie found" in error_data["detail"].lower() # No cleanup needed - movie already deleted @@ -240,7 +239,8 @@ async def test_batch_create_movies(self, client): finally: # Cleanup: Delete all created movies for movie_id in created_ids: - await client.delete(f"/api/movies/{movie_id}") + delete_response = await client.delete(f"/api/movies/{movie_id}") + assert delete_response.status_code in [200, 404] @pytest.mark.asyncio async def test_batch_delete_movies(self, client, multiple_test_movies): @@ -279,10 +279,10 @@ async def test_batch_delete_movies(self, client, multiple_test_movies): # Note: The API returns 200 with INTERNAL_SERVER_ERROR code, not 404 for movie_id in multiple_test_movies: get_response = await client.get(f"/api/movies/{movie_id}") - response_data = get_response.json() - assert response_data["success"] is False - assert response_data["error"]["code"] == "INTERNAL_SERVER_ERROR" - assert "not found" in response_data["error"]["message"].lower() + assert get_response.status_code == 404 + error_data = get_response.json() + assert "detail" in error_data + assert "no movie found" in error_data["detail"].lower() # Note: Fixture cleanup will try to delete but movies are already gone # The fixture should handle this gracefully diff --git a/mflix/server/python-fastapi/tests/test_movie_routes.py b/mflix/server/python-fastapi/tests/test_movie_routes.py index b98937f..ce4b3b6 100644 --- a/mflix/server/python-fastapi/tests/test_movie_routes.py +++ b/mflix/server/python-fastapi/tests/test_movie_routes.py @@ -9,6 +9,7 @@ import pytest from unittest.mock import AsyncMock, MagicMock, patch from bson import ObjectId +from fastapi import HTTPException from src.models.models import CreateMovieRequest, UpdateMovieRequest @@ -57,21 +58,23 @@ async def test_get_movie_by_id_not_found(self, mock_get_collection): # Import and call the route handler from src.routers.movies import get_movie_by_id - result = await get_movie_by_id(TEST_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await get_movie_by_id(TEST_MOVIE_ID) # Assertions - assert result.success is False - assert "not found" in result.message.lower() + assert e.value.status_code == 404 + assert "no movie found" in str(e.value.detail).lower() async def test_get_movie_by_id_invalid_id(self): """Should return error when invalid ObjectId format is provided.""" # Import and call the route handler from src.routers.movies import get_movie_by_id - result = await get_movie_by_id(INVALID_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await get_movie_by_id(INVALID_MOVIE_ID) # Assertions - assert result.success is False - assert "invalid" in result.message.lower() + assert e.value.status_code == 400 + assert " not a valid" in str(e.value.detail).lower() @patch('src.routers.movies.get_collection') async def test_get_movie_by_id_database_error(self, mock_get_collection): @@ -83,11 +86,12 @@ async def test_get_movie_by_id_database_error(self, mock_get_collection): # Import and call the route handler from src.routers.movies import get_movie_by_id - result = await get_movie_by_id(TEST_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await get_movie_by_id(TEST_MOVIE_ID) # Assertions - assert result.success is False - assert "error" in result.message.lower() + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @pytest.mark.unit @@ -140,11 +144,12 @@ async def test_create_movie_database_error(self, mock_get_collection): # Create request from src.routers.movies import create_movie movie_request = CreateMovieRequest(title="New Movie") - result = await create_movie(movie_request) + with pytest.raises(HTTPException) as e: + await create_movie(movie_request) # Assertions - assert result.success is False - assert "error" in result.message.lower() + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @pytest.mark.unit @@ -194,22 +199,26 @@ async def test_update_movie_not_found(self, mock_get_collection): # Create request from src.routers.movies import update_movie update_request = UpdateMovieRequest(title="Updated Movie") - result = await update_movie(update_request, TEST_MOVIE_ID) - - # Assertions - assert result.success is False - assert "was found" in result.message.lower() or "not found" in result.message.lower() + + with pytest.raises(HTTPException) as e: + await update_movie(update_request, TEST_MOVIE_ID) + + #Assertions + assert e.value.status_code == 404 + assert "no movie" in str(e.value.detail.lower()) async def test_update_movie_invalid_id(self): """Should return error when invalid ObjectId format is provided.""" # Create request from src.routers.movies import update_movie update_request = UpdateMovieRequest(title="Updated Movie") - result = await update_movie(update_request, INVALID_MOVIE_ID) + + with pytest.raises(HTTPException) as e: + await update_movie(update_request, INVALID_MOVIE_ID) - # Assertions - assert result.success is False - assert "invalid" in result.message.lower() + # Assertions + assert e.value.status_code == 400 + assert "invalid" in str(e.value.detail.lower()) @pytest.mark.unit @@ -248,21 +257,23 @@ async def test_delete_movie_not_found(self, mock_get_collection): # Call the route handler from src.routers.movies import delete_movie_by_id - result = await delete_movie_by_id(TEST_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await delete_movie_by_id(TEST_MOVIE_ID) - # Assertions - assert result.success is False - assert "not found" in result.message.lower() + # Assertions + assert e.value.status_code == 404 + assert "no movie" in str(e.value.detail.lower()) async def test_delete_movie_invalid_id(self): """Should return error when invalid ObjectId format is provided.""" # Call the route handler from src.routers.movies import delete_movie_by_id - result = await delete_movie_by_id(INVALID_MOVIE_ID) - + with pytest.raises(HTTPException) as e: + await delete_movie_by_id(INVALID_MOVIE_ID) + # Assertions - assert result.success is False - assert "invalid" in result.message.lower() + assert e.value.status_code == 400 + assert "invalid movie id" in str(e.value.detail.lower()) @patch('src.routers.movies.get_collection') async def test_delete_movie_database_error(self, mock_get_collection): @@ -274,14 +285,12 @@ async def test_delete_movie_database_error(self, mock_get_collection): # Call the route handler from src.routers.movies import delete_movie_by_id - result = await delete_movie_by_id(TEST_MOVIE_ID) - - # Assertions - assert result.success is False - assert "error" in result.message.lower() - - + with pytest.raises(HTTPException) as e: + await delete_movie_by_id(TEST_MOVIE_ID) + # Assertions + assert e.value.status_code == 500 + assert "error" in str(e.value.detail.lower()) @pytest.mark.unit @pytest.mark.asyncio @@ -384,11 +393,12 @@ async def test_get_all_movies_database_error(self, mock_get_collection): # Call the route handler from src.routers.movies import get_all_movies - result = await get_all_movies() + with pytest.raises(HTTPException) as e: + await get_all_movies() # Assertions - assert result.success is False - assert "error" in result.message.lower() + assert e.value.status_code == 500 + assert "error" in str(e.value.detail.lower()) @pytest.mark.unit @@ -430,11 +440,12 @@ async def test_create_movies_batch_empty_list(self, mock_get_collection): # Create request with empty list from src.routers.movies import create_movies_batch - result = await create_movies_batch([]) + with pytest.raises(HTTPException) as e: + await create_movies_batch([]) # Assertions - assert result.success is False - assert "empty" in result.message.lower() + assert e.value.status_code == 400 + assert "empty" in str(e.value.detail.lower()) @patch('src.routers.movies.get_collection') async def test_delete_movies_batch_success(self, mock_get_collection): @@ -464,11 +475,12 @@ async def test_delete_movies_batch_missing_filter(self, mock_get_collection): # Create request without filter from src.routers.movies import delete_movies_batch request_body = {} - result = await delete_movies_batch(request_body) + with pytest.raises(HTTPException) as e: + await delete_movies_batch(request_body) # Assertions - assert result.success is False - assert "filter" in result.message.lower() + assert e.value.status_code == 400 + assert "filter" in e.value.detail.lower() @@ -510,21 +522,23 @@ async def test_find_and_delete_not_found(self, mock_get_collection): # Call the route handler from src.routers.movies import find_and_delete_movie - result = await find_and_delete_movie(TEST_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await find_and_delete_movie(TEST_MOVIE_ID) # Assertions - assert result.success is False - assert "not found" in result.message.lower() + assert e.value.status_code == 404 + assert "no movie" in str(e.value.detail.lower()) async def test_find_and_delete_invalid_id(self): """Should return error when invalid ObjectId format is provided.""" # Call the route handler from src.routers.movies import find_and_delete_movie - result = await find_and_delete_movie(INVALID_MOVIE_ID) + with pytest.raises(HTTPException) as e: + await find_and_delete_movie(INVALID_MOVIE_ID) # Assertions - assert result.success is False - assert "invalid" in result.message.lower() + assert e.value.status_code == 400 + assert "invalid" in str(e.value.detail.lower()) @pytest.mark.unit @@ -565,11 +579,12 @@ async def test_update_movies_batch_missing_filter(self, mock_get_collection): # Create request without filter from src.routers.movies import update_movies_batch request_body = {"update": {"$set": {"rated": "PG-13"}}} - result = await update_movies_batch(request_body) + with pytest.raises(HTTPException) as e: + await update_movies_batch(request_body) # Assertions - assert result.success is False - assert "filter" in result.message.lower() or "required" in result.message.lower() + assert e.value.status_code == 400 + assert "filter" in str(e.value.detail).lower() @patch('src.routers.movies.get_collection') async def test_update_movies_batch_missing_update(self, mock_get_collection): @@ -579,11 +594,12 @@ async def test_update_movies_batch_missing_update(self, mock_get_collection): # Create request without update from src.routers.movies import update_movies_batch request_body = {"filter": {"year": 2020}} - result = await update_movies_batch(request_body) + with pytest.raises(HTTPException) as e: + await update_movies_batch(request_body) # Assertions - assert result.success is False - assert "update" in result.message.lower() or "required" in result.message.lower() + assert e.value.status_code == 400 + assert "update" in str(e.value.detail).lower() @patch('src.routers.movies.get_collection') async def test_update_movies_batch_no_matches(self, mock_get_collection): @@ -683,20 +699,22 @@ async def test_search_movies_with_pagination(self, mock_execute_aggregation): async def test_search_movies_no_parameters(self): """Should return error when no search parameters provided.""" from src.routers.movies import search_movies - result = await search_movies(search_operator="must") + with pytest.raises(HTTPException) as e: + await search_movies(search_operator="must") # Assertions - assert result.success is False - assert result.error.code == "DATABASE_ERROR" + assert e.value.status_code == 400 + assert "one search parameter" in str(e.value.detail).lower() async def test_search_movies_invalid_operator(self): """Should return error for invalid search operator.""" from src.routers.movies import search_movies - result = await search_movies(plot="test", search_operator="invalid") + with pytest.raises(HTTPException) as e: + await search_movies(plot="test", search_operator="invalid") # Assertions - assert result.success is False - assert result.error.code == "INVALID_SEARCH_OPERATOR" + assert e.value.status_code == 400 + assert "invalid search operator" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_search_movies_database_error(self, mock_execute_aggregation): @@ -706,11 +724,12 @@ async def test_search_movies_database_error(self, mock_execute_aggregation): # Call the route handler from src.routers.movies import search_movies - result = await search_movies(plot="test", search_operator="must") + with pytest.raises(HTTPException) as e: + await search_movies(plot="test", search_operator="must") # Assertions - assert result.success is False - assert result.error.code == "DATABASE_ERROR" + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_search_movies_empty_results(self, mock_execute_aggregation): @@ -744,12 +763,13 @@ async def test_vector_search_unavailable(self, mock_voyage_available): # Call the route handler from src.routers.movies import vector_search_movies - result = await vector_search_movies(q="action movie") + with pytest.raises(HTTPException) as e: + await vector_search_movies(q="action movie") # Assertions - assert result.success is False - assert result.error.code == "SERVICE_UNAVAILABLE" - assert "VOYAGE_API_KEY" in result.error.details + assert e.value.status_code == 503 + assert str("VOYAGE_API_KEY not configured").lower() in str(e.value.detail).lower() + @patch('src.routers.movies.voyage_ai_available') @patch('src.routers.movies.voyageai.Client') @@ -798,11 +818,12 @@ async def test_vector_search_embedding_error(self, mock_get_embedding, mock_voya # Call the route handler from src.routers.movies import vector_search_movies - result = await vector_search_movies(q="action movie") + with pytest.raises(HTTPException) as e: + await vector_search_movies(q="action movie") # Assertions - assert result.success is False - assert result.error.code == "INTERNAL_SERVER_ERROR" + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @patch('src.routers.movies.voyage_ai_available') @patch('src.routers.movies.voyageai.Client') @@ -896,12 +917,12 @@ async def test_aggregate_movies_by_movie_id(self, mock_execute_aggregation): async def test_aggregate_movies_invalid_movie_id(self): """Should return error for invalid movie ID format.""" from src.routers.movies import aggregate_movies_recent_commented - result = await aggregate_movies_recent_commented(movie_id="invalid_id") + with pytest.raises(HTTPException) as e: + await aggregate_movies_recent_commented(movie_id="invalid_id") # Assertions - assert result.success is False - assert result.error.code == "INTERNAL_SERVER_ERROR" - assert "ObjectId" in result.error.details + assert e.value.status_code == 400 + assert "movie_id is not" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_aggregate_movies_database_error(self, mock_execute_aggregation): @@ -911,11 +932,12 @@ async def test_aggregate_movies_database_error(self, mock_execute_aggregation): # Call the route handler from src.routers.movies import aggregate_movies_recent_commented - result = await aggregate_movies_recent_commented(limit=10, movie_id=None) + with pytest.raises(HTTPException) as e: + await aggregate_movies_recent_commented(limit=10, movie_id=None) # Assertions - assert result.success is False - assert result.error.code == "INTERNAL_SERVER_ERROR" + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_aggregate_movies_empty_results(self, mock_execute_aggregation): @@ -966,11 +988,12 @@ async def test_aggregate_movies_by_year_database_error(self, mock_execute_aggreg # Call the route handler from src.routers.movies import aggregate_movies_by_year - result = await aggregate_movies_by_year() + with pytest.raises(HTTPException) as e: + await aggregate_movies_by_year() # Assertions - assert result.success is False - assert result.error.code == "INTERNAL_SERVER_ERROR" + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_aggregate_movies_by_year_empty_results(self, mock_execute_aggregation): @@ -1039,11 +1062,12 @@ async def test_aggregate_directors_database_error(self, mock_execute_aggregation # Call the route handler from src.routers.movies import aggregate_directors_most_movies - result = await aggregate_directors_most_movies() + with pytest.raises(HTTPException) as e: + await aggregate_directors_most_movies() # Assertions - assert result.success is False - assert result.error.code == "INTERNAL_SERVER_ERROR" + assert e.value.status_code == 500 + assert "error" in str(e.value.detail).lower() @patch('src.routers.movies.execute_aggregation') async def test_aggregate_directors_empty_results(self, mock_execute_aggregation):