Merge pull request #84 from avantifellows/optimize_session_and_sessio…

…n_answers_logic

Removed dependency on redundant `session_answers` table + optimized some mongo queries

app/models.py

@@ -367,6 +367,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"""
 

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"]
     )

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)
 

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)
 

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"]