Repository: avantifellows/quiz-backend
Issue category: Performance Issue

At some places we're updating just one key of an object or one element of an array but we send the whole object to MongoDB to update which is inefficient.



Functions to edit:
app/routers/session_answers.py:update_session_answer
app/routers/session_answers.py:get_session_answer
app/routers/sessions.py:create_session
app/routers/sessions.py:update_session
Functions added: app/routers/session_answers.py:update_session_answer_in_a_session
app/routers/session_answers.py:get_session_answer_from_a_session

Patch:
diff --git a/app/models.py b/app/models.py
index cfb9644b..80f94b94 100644
--- a/app/models.py
+++ b/app/models.py
@@ -365,6 +365,11 @@ class Config:
         schema_extra = {"example": {"answer": [0, 1, 2], "visited": True}}
 
 
+"""
+Note : The below model is not being used currently anywhere
+"""
+
+
 class SessionAnswerResponse(SessionAnswer):
     """Model for the response of any request that returns a session answer"""
 
diff --git a/app/routers/session_answers.py b/app/routers/session_answers.py
index 9d11e4a6..cd3f4937 100644
--- a/app/routers/session_answers.py
+++ b/app/routers/session_answers.py
@@ -2,68 +2,82 @@
 from fastapi.responses import JSONResponse
 from fastapi.encoders import jsonable_encoder
 from database import client
-from models import SessionAnswerResponse, UpdateSessionAnswer
+from models import UpdateSessionAnswer
 from utils import remove_optional_unset_args
 
 router = APIRouter(prefix="/session_answers", tags=["Session Answers"])
 
 
-@router.patch("/{session_answer_id}", response_model=SessionAnswerResponse)
-async def update_session_answer(
-    session_answer_id: str, session_answer: UpdateSessionAnswer
+@router.patch("/{session_id}/{position_index}", response_model=None)
+async def update_session_answer_in_a_session(
+    session_id: str, position_index: int, session_answer: UpdateSessionAnswer
 ):
+    """
+    Update a session answer in a session by its position index in the session answers array
+    Path Params:
+    session_id - the id of the session
+    position_index - the position index of the session answer in the session answers array. This corresponds to the position of the question in the quiz
+    """
     session_answer = remove_optional_unset_args(session_answer)
     session_answer = jsonable_encoder(session_answer)
 
-    if (client.quiz.session_answers.find_one({"_id": session_answer_id})) is None:
+    # check if the session exists
+    session = client.quiz.sessions.find_one({"_id": session_id})
+    if session is None:
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
-            detail=f"session_answer {session_answer_id} not found",
+            detail=f"Provided session with id {session_id} not found",
         )
 
-    # update the document in the session_answers collection
-    client.quiz.session_answers.update_one(
-        {"_id": session_answer_id}, {"$set": session_answer}
-    )
-
-    updated_session_answer = client.quiz.session_answers.find_one(
-        {"_id": session_answer_id}
-    )
+    # check if the session has session answers key
+    if "session_answers" not in session or session["session_answers"] is None:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"No session answers found in the session with id {session_id}",
+        )
 
-    # update the document in the sessions collection if this answer
-    # is present in the subset of session answers we store in the document
-    # corresponding to the session
-    session_to_update = client.quiz.sessions.find_one(
-        {"_id": updated_session_answer["session_id"]}
-    )
+    # check if the session answer index that we're trying to access is out of bounds or not
+    if position_index > len(session["session_answers"]):
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail=f"Provided position index {position_index} is out of bounds of length of the session answers array",
+        )
 
-    session_answers = list(session_to_update["session_answers"])
-    update_session = False
-    for index, _ in enumerate(session_answers):
-        if session_answers[index]["_id"] == session_answer_id:
-            session_answers[index].update(session_answer)
-            update_session = True
-            break
+    # constructing the $set query for mongodb
+    setQuery = {}
+    for key, value in session_answer.items():
+        setQuery[f"session_answers.{position_index}.{key}"] = value
 
-    if update_session:
-        client.quiz.sessions.update_one(
-            {"_id": session_to_update["_id"]},
-            {"$set": {"session_answers": session_answers}},
-        )
+    # update the document in the session_answers collection
+    client.quiz.sessions.update_one({"_id": session_id}, {"$set": setQuery})
 
-    return JSONResponse(status_code=status.HTTP_200_OK, content=updated_session_answer)
+    return JSONResponse(status_code=status.HTTP_200_OK)
 
 
-@router.get("/{session_answer_id}", response_model=SessionAnswerResponse)
-async def get_session_answer(session_answer_id: str):
-    if (
-        session_answer := client.quiz.session_answers.find_one(
-            {"_id": session_answer_id}
+@router.get("/{session_id}/{position_index}", response_model=None)
+async def get_session_answer_from_a_session(session_id: str, position_index: int):
+    pipeline = [
+        {
+            "$match": {  # match the session with the provided session_id
+                "_id": session_id
+            }
+        },
+        {
+            "$project": {  # project the required element from session_answers array
+                "_id": 0,
+                "session_answer": {
+                    "$arrayElemAt": ["$session_answers", position_index]
+                },
+            }
+        },
+    ]
+    aggregation_result = list(client.quiz.sessions.aggregate(pipeline))
+    if len(aggregation_result) == 0:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Either session_id is wrong or position_index is out of bounds",
         )
-    ) is not None:
-        return session_answer
 
-    raise HTTPException(
-        status_code=status.HTTP_404_NOT_FOUND,
-        detail=f"session_answer {session_answer_id} not found",
+    return JSONResponse(
+        status_code=status.HTTP_200_OK, content=aggregation_result[0]["session_answer"]
     )
diff --git a/app/routers/sessions.py b/app/routers/sessions.py
index 34f07615..6b14efca 100644
--- a/app/routers/sessions.py
+++ b/app/routers/sessions.py
@@ -100,17 +100,13 @@ async def create_session(session: Session):
         current_session["has_quiz_ended"] = last_session.get("has_quiz_ended", False)
 
         # restore the answers from the last (previous) sessions
-        last_session_answers = list(
-            client.quiz.session_answers.find(
-                {"session_id": last_session["_id"]},
-                sort=[("_id", pymongo.ASCENDING)],
-            )
-        )
+        session_answers_of_the_last_session = last_session["session_answers"]
 
-        for index, session_answer in enumerate(last_session_answers):
+        for _, session_answer in enumerate(session_answers_of_the_last_session):
             # note: we retain created_at key in session_answer
             for key in ["_id", "session_id"]:
-                session_answer.pop(key)
+                if key in session_answer:
+                    session_answer.pop(key)
 
             # append with new session_answer "_id" keys
             session_answers.append(
@@ -120,17 +116,10 @@ async def create_session(session: Session):
     current_session["session_answers"] = session_answers
 
     # insert current session into db
-    new_session = client.quiz.sessions.insert_one(current_session)
-    created_session = client.quiz.sessions.find_one({"_id": new_session.inserted_id})
-
-    # update with new session_id and insert to db
-    for index, _ in enumerate(session_answers):
-        session_answers[index]["session_id"] = new_session.inserted_id
-
-    client.quiz.session_answers.insert_many(session_answers)
+    client.quiz.sessions.insert_one(current_session)
 
     # return the created session
-    return JSONResponse(status_code=status.HTTP_201_CREATED, content=created_session)
+    return JSONResponse(status_code=status.HTTP_201_CREATED, content=current_session)
 
 
 @router.patch("/{session_id}", response_model=UpdateSessionResponse)
@@ -143,6 +132,7 @@ async def update_session(session_id: str, session_updates: UpdateSession):
     * dummy event logic added for JNV -- will be removed!
     """
     new_event = jsonable_encoder(session_updates)["event"]
+    session_update_query = {}
 
     # if new_event == EventType.dummy_event:
     #     return JSONResponse(
@@ -159,8 +149,16 @@ async def update_session(session_id: str, session_updates: UpdateSession):
     event_obj = jsonable_encoder(Event.parse_obj({"event_type": new_event}))
     if session["events"] is None:
         session["events"] = [event_obj]
+        if "$set" not in session_update_query:
+            session_update_query["$set"] = {"events": [event_obj]}
+        else:
+            session_update_query["$set"].update({"events": [event_obj]})
     else:
         session["events"].append(event_obj)
+        if "$push" not in session_update_query:
+            session_update_query["$push"] = {"events": event_obj}
+        else:
+            session_update_query["$push"].update({"events": event_obj})
 
     # diff between times of last two events
     time_elapsed = 0
@@ -212,15 +210,21 @@ async def update_session(session_id: str, session_updates: UpdateSession):
     ):
         # if `time_remaining` key is not present =>
         # no time limit is set, no need to respond with time_remaining
-        session["time_remaining"] = max(0, session["time_remaining"] - time_elapsed)
-        response_content = {"time_remaining": session["time_remaining"]}
+        time_remaining = max(0, session["time_remaining"] - time_elapsed)
+        if "$set" not in session_update_query:
+            session_update_query["$set"] = {"time_remaining": time_remaining}
+        else:
+            session_update_query["$set"].update({"time_remaining": time_remaining})
+        response_content = {"time_remaining": time_remaining}
 
     # update the document in the sessions collection
     if new_event == EventType.end_quiz:
-        session["has_quiz_ended"] = True
-    client.quiz.sessions.update_one(
-        {"_id": session_id}, {"$set": jsonable_encoder(session)}
-    )
+        if "$set" not in session_update_query:
+            session_update_query["$set"] = {"has_quiz_ended": True}
+        else:
+            session_update_query["$set"].update({"has_quiz_ended": True})
+
+    client.quiz.sessions.update_one({"_id": session_id}, session_update_query)
 
     return JSONResponse(status_code=status.HTTP_200_OK, content=response_content)
 

Test patch: diff --git a/app/tests/test_session_answers.py b/app/tests/test_session_answers.py
index a2c04a1e..2d05a9be 100644
--- a/app/tests/test_session_answers.py
+++ b/app/tests/test_session_answers.py
@@ -7,12 +7,13 @@ class SessionAnswerTestCase(SessionsBaseTestCase):
     def setUp(self):
         super().setUp()
         self.session_answers = self.homework_session["session_answers"]
+        self.session_id = self.homework_session["_id"]
+        self.session_answer_position_index = 0
         self.session_answer = self.session_answers[0]
-        self.session_answer_id = self.session_answer["_id"]
 
-    def test_gets_session_answer_with_valid_id(self):
+    def test_gets_session_answer_from_a_session(self):
         response = self.client.get(
-            f"{session_answers.router.prefix}/{self.session_answer_id}"
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}"
         )
         assert response.status_code == 200
         session_answer = json.loads(response.content)
@@ -22,12 +23,12 @@ def test_gets_session_answer_with_valid_id(self):
     def test_update_session_answer_with_only_answer(self):
         new_answer = [0, 1, 2]
         response = self.client.patch(
-            f"{session_answers.router.prefix}/{self.session_answer_id}",
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}",
             json={"answer": new_answer},
         )
         assert response.status_code == 200
         response = self.client.get(
-            f"{session_answers.router.prefix}/{self.session_answer_id}"
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}"
         )
         session_answer = json.loads(response.content)
 
@@ -40,12 +41,12 @@ def test_update_session_answer_with_only_answer(self):
     def test_update_session_answer_with_only_visited(self):
         new_visited = True
         response = self.client.patch(
-            f"{session_answers.router.prefix}/{self.session_answer_id}",
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}",
             json={"visited": new_visited},
         )
         assert response.status_code == 200
         response = self.client.get(
-            f"{session_answers.router.prefix}/{self.session_answer_id}"
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}"
         )
         session_answer = json.loads(response.content)
 
diff --git a/app/tests/test_sessions.py b/app/tests/test_sessions.py
index 66105d14..531db98b 100644
--- a/app/tests/test_sessions.py
+++ b/app/tests/test_sessions.py
@@ -96,12 +96,14 @@ def test_create_session_with_previous_session_and_start_event(self):
         assert response["is_first"] is False
 
     def test_create_session_with_valid_quiz_id_and_previous_session(self):
+        self.session_id = self.homework_session["_id"]
         self.session_answers = self.homework_session["session_answers"]
+        self.session_answer_position_index = 0
         self.session_answer = self.session_answers[0]
         self.session_answer_id = self.session_answer["_id"]
         new_answer = [0, 1, 2]
         response = self.client.patch(
-            f"{session_answers.router.prefix}/{self.session_answer_id}",
+            f"{session_answers.router.prefix}/{self.session_id}/{self.session_answer_position_index}",
             json={"answer": new_answer},
         )
         response = self.client.post(
@@ -214,8 +216,5 @@ def test_time_remaining_in_new_session_with_quiz_resume(self):
             f"{sessions.router.prefix}/{resumed_session_id}"
         ).json()
 
-        # because time has passed between both quizzes
-        assert (
-            json.loads(response.content)["time_remaining"] < quiz["time_limit"]["max"]
-        )
+        # because time has passed between both sessions
         assert updated_resumed_session["time_remaining"] < quiz["time_limit"]["max"]