Question Service: Add exception classes and refactor routers and service logic

question-service/app/crud/questions.py

@@ -1,5 +1,7 @@
-from app.models.questions import CreateQuestionModel, UpdateQuestionModel, QuestionCollection, QuestionModel
+from app.models.questions import CreateQuestionModel, UpdateQuestionModel, QuestionCollection, QuestionModel, MessageModel
+from app.exceptions.questions_exceptions import DuplicateQuestionError, QuestionNotFoundError, BatchUploadFailedError, InvalidQuestionIdError
 from bson import ObjectId
+from bson.errors import InvalidId
 from dotenv import load_dotenv
 import motor.motor_asyncio
 from typing import List
@@ -15,10 +17,10 @@
 db = client.get_database("question_service")
 question_collection = db.get_collection("questions")
 
-async def create_question(question: CreateQuestionModel):
+async def create_question(question: CreateQuestionModel) -> QuestionModel:
     existing_question = await question_collection.find_one({"title": question.title})
     if existing_question:
-        return None
+        raise DuplicateQuestionError(question.title)
     new_question = await question_collection.insert_one(question.model_dump())
     return await question_collection.find_one({"_id": new_question.inserted_id})
 
@@ -27,47 +29,62 @@ async def get_all_questions() -> QuestionCollection:
     return QuestionCollection(questions=questions)
 
 async def get_question_by_id(question_id: str) -> QuestionModel:
-    existing_question = await question_collection.find_one({"_id": ObjectId(question_id)})
+    try:
+        object_id = ObjectId(question_id)
+    except InvalidId:
+        raise InvalidQuestionIdError(question_id)
+
+    existing_question = await question_collection.find_one({"_id": object_id})
+    if existing_question is None:
+        raise QuestionNotFoundError(question_id)
     return existing_question
 
-async def delete_question(question_id: str):
-    existing_question = await question_collection.find_one({"_id": ObjectId(question_id)})
+async def delete_question(question_id: str) -> MessageModel:
+    try:
+        object_id = ObjectId(question_id)
+    except InvalidId:
+        raise InvalidQuestionIdError(question_id)
+
+    existing_question = await question_collection.find_one({"_id": object_id})
     if existing_question is None:
-        return None
+        raise QuestionNotFoundError(question_id)
     await question_collection.delete_one({"_id": ObjectId(question_id)})
     return {"message": f"Question with id {existing_question['_id']} and title '{existing_question['title']}' deleted."}
 
-async def update_question_by_id(question_id: str, question_data: UpdateQuestionModel):
-    existing_question = await question_collection.find_one({"_id": ObjectId(question_id)})
+async def update_question_by_id(question_id: str, question_data: UpdateQuestionModel) -> QuestionModel:
+    try:
+        object_id = ObjectId(question_id)
+    except InvalidId:
+        raise InvalidQuestionIdError(question_id)
+
+    existing_question = await question_collection.find_one({"_id": object_id})
 
     if existing_question is None:
-        return None
+        raise QuestionNotFoundError(question_id)
 
     update_data = question_data.model_dump(exclude_unset=True)
 
     # Check if the new title already exists and belongs to another question
     if "title" in update_data and update_data["title"] != existing_question["title"]:
         existing_title = await question_collection.find_one({"title": update_data["title"]})
         if existing_title and str(existing_title["_id"]) != question_id:
-            return "duplicate_title"
+            raise DuplicateQuestionError(existing_title["title"])
 
     if not update_data:
         return existing_question
 
     await question_collection.update_one({"_id": ObjectId(question_id)}, {"$set": update_data})
     return await question_collection.find_one({"_id": ObjectId(question_id)})
 
-async def batch_create_questions(questions: List[CreateQuestionModel]):
+async def batch_create_questions(questions: List[CreateQuestionModel]) -> MessageModel:
     new_questions = []
     for question in questions:
         existing_question = await question_collection.find_one({"title": question.title})
         if not existing_question:
-            # Convert Pydantic model to dictionary
             new_questions.append(question.model_dump())
 
     if not new_questions:
-        return {"error": "No new questions to add."}
-
-    result = await question_collection.insert_many(new_questions)
+        raise BatchUploadFailedError()
 
+    result = await question_collection.insert_many(new_questions)
     return {"message": f"{len(result.inserted_ids)} questions added successfully."}

question-service/app/exceptions/questions_exceptions.py

@@ -0,0 +1,22 @@
+class InvalidQuestionIdError(Exception):
+    """Raised when a question id cannot be parsed properly"""
+    def __init__(self, question_id):
+        self.question_id = question_id
+        super().__init__(f"Question ID '{question_id}' is invalid and cannot be parsed properly.")
+
+class DuplicateQuestionError(Exception):
+    """Raised when a question with the same title already exists."""
+    def __init__(self, title):
+        self.title = title
+        super().__init__(f"A question with the title '{title}' already exists.")
+
+class QuestionNotFoundError(Exception):
+    """Raised when a question with the given ID is not found."""
+    def __init__(self, question_id):
+        self.question_id = question_id
+        super().__init__(f"Question with ID '{question_id}' not found.")
+
+class BatchUploadFailedError(Exception):
+    """Raised when batch upload fails to upload any questions successfully"""
+    def __init__(self):
+        super().__init__("No questions were added successfully")

question-service/app/routers/questions.py

@@ -1,8 +1,8 @@
 from fastapi import APIRouter, HTTPException
 from app.models.questions import CreateQuestionModel, UpdateQuestionModel, QuestionModel, QuestionCollection, MessageModel
-from app.crud.questions import create_question, get_all_questions, get_question_by_id, delete_question, update_question_by_id
+from app.crud.questions import create_question, get_all_questions, get_question_by_id, delete_question, update_question_by_id, batch_create_questions
+from app.exceptions.questions_exceptions import DuplicateQuestionError, QuestionNotFoundError, BatchUploadFailedError, InvalidQuestionIdError
 from typing import List
-from app.crud.questions import batch_create_questions
 router = APIRouter()
 
 @router.post("/",
@@ -21,42 +21,46 @@
                 ,
             })
 async def create(question: CreateQuestionModel):
-    existing_question = await create_question(question)
-    if existing_question is None:
-        raise HTTPException(status_code=409, detail="Question with this title already exists.")
-    return existing_question
+    try:
+        return await create_question(question)
+    except DuplicateQuestionError as e:
+        raise HTTPException(status_code=409, detail=str(e))
 
 @router.get("/", response_description="Get all questions", response_model=QuestionCollection)
 async def get_all():
     return await get_all_questions()
 
 @router.get("/{question_id}", response_description="Get question with specified id", response_model=QuestionModel)
 async def get_question(question_id: str):
-    existing_question: QuestionModel = await get_question_by_id(question_id)
-    if existing_question is None:
-        raise HTTPException(status_code=404, detail="Question with this id does not exist.")
-    return existing_question
+    try:
+        return await get_question_by_id(question_id)
+    except InvalidQuestionIdError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+    except QuestionNotFoundError as e:
+        raise HTTPException(status_code=404, detail=str(e))
 
 @router.delete("/{question_id}", response_description="Delete question with specified id", response_model=MessageModel)
 async def delete(question_id: str):
-    response = await delete_question(question_id)
-    if response is None:
-        raise HTTPException(status_code=404, detail="Question with this id does not exist.")
-    return response
+    try:
+        response = await delete_question(question_id)
+        return response
+    except InvalidQuestionIdError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+    except QuestionNotFoundError as e:
+        raise HTTPException(status_code=404, detail=str(e))
 
 @router.put("/{question_id}", response_description="Update question with specified id", response_model=QuestionModel)
 async def update_question(question_id: str, question_data: UpdateQuestionModel):
-    updated_question = await update_question_by_id(question_id, question_data)
+    try:
+        updated_question = await update_question_by_id(question_id, question_data)
+        return updated_question
+    except InvalidQuestionIdError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+    except QuestionNotFoundError as e:
+        raise HTTPException(status_code=404, detail=str(e))
+    except DuplicateQuestionError as e:
+        raise HTTPException(status_code=409, detail=str(e))
 
-    if updated_question is None:
-        raise HTTPException(status_code=404, detail="Question with this id does not exist.")
-
-    if updated_question == "duplicate_title":
-        raise HTTPException(status_code=409, detail="A question with this title already exists.")
-
-    return updated_question
-
-
 @router.post("/batch-upload",
              response_description="Batch upload questions",
              response_model=MessageModel,
@@ -72,7 +76,8 @@ async def update_question(question_id: str, question_data: UpdateQuestionModel):
                  },
              })
 async def batch_upload(questions: List[CreateQuestionModel]):
-    result = await batch_create_questions(questions)
-    if "error" in result:
-        raise HTTPException(status_code=400, detail=result["error"])
-    return result
+    try:
+        result = await batch_create_questions(questions)
+        return result
+    except BatchUploadFailedError as e:
+        raise HTTPException(status_code=400, detail=str(e))