Merge pull request #153 from SELab-2/submission-delete

delete submission fix

backend/src/submission/models.py

@@ -37,8 +37,11 @@ class Submission(Base):
     stdout: Mapped[str] = mapped_column(nullable=True)
     stderr: Mapped[str] = mapped_column(nullable=True)
 
+    # Without passive_deletes="all", sqlalchemy will for some reason try to set submission_id of TestResult to NULL,
+    # causing a violation error of not_null-constraint.
+    # see https://docs.sqlalchemy.org/en/20/orm/relationship_api.html#sqlalchemy.orm.relationship.params.passive_deletes
     testresults: Mapped[List["TestResult"]] = relationship(
-        back_populates="submission", lazy="joined"
+        back_populates="submission", lazy="joined", passive_deletes="all"
     )
 
 

backend/src/submission/router.py

@@ -15,7 +15,7 @@
 )
 from src.submission.exceptions import FileNotFound
 from src.submission.exceptions import FilesNotFound
-from src.submission.utils import upload_files
+from src.submission.utils import upload_files, remove_files
 from src.user.dependencies import admin_user_validation, get_authenticated_user
 from src.user.schemas import User
 from . import service
@@ -71,8 +71,9 @@ async def create_submission(background_tasks: BackgroundTasks,
 @router.delete("/{submission_id}",
                dependencies=[Depends(admin_user_validation)],
                status_code=200)
-async def delete_submision(submission_id: int, db: AsyncSession = Depends(get_async_db)):
-    await service.delete_submission(db, submission_id)
+async def delete_submision(submission: Submission = Depends(retrieve_submission), db: AsyncSession = Depends(get_async_db)):
+    remove_files(submission.files_uuid)
+    await service.delete_submission(db, submission.id)
 
 
 @router.get("/{submission_id}/files", response_model=list[File])
@@ -82,7 +83,7 @@ async def get_files(submission: Submission = Depends(retrieve_submission)):
 
 
 @router.get("/{submission_id}/files/{path:path}", response_class=FileResponse)
-async def get_file(path: str, submission: Submission = Depends(get_submission)):
+async def get_file(path: str, submission: Submission = Depends(retrieve_submission)):
     path = submission_path(submission.files_uuid, path)
 
     if not os.path.isfile(path):
@@ -100,7 +101,7 @@ async def get_artifacts(submission: Submission = Depends(retrieve_submission)):
 
 
 @router.get("/{submission_id}/artifacts/{path:path}", response_class=FileResponse)
-async def get_artifact(path: str, submission: Submission = Depends(get_submission)):
+async def get_artifact(path: str, submission: Submission = Depends(retrieve_submission)):
     if submission.status == Status.InProgress:
         raise FileNotFound
 

backend/src/submission/utils.py

@@ -6,7 +6,7 @@
 
 from fastapi import UploadFile
 
-from src.docker_tests.utils import submission_path
+from src.docker_tests.utils import submission_path, submissions_path
 from src.project.schemas import Project
 from src.submission.exceptions import UnMetRequirements
 
@@ -43,3 +43,7 @@ def upload_files(files: list[UploadFile], project: Project) -> str:
         raise UnMetRequirements(errors)
 
     return uuid
+
+
+def remove_files(uuid: str):
+    shutil.rmtree(submissions_path(uuid))

backend/tests/test_docker.py

@@ -1,4 +1,4 @@
-import shutil
+import os
 from datetime import datetime, timedelta, timezone
 from pathlib import Path
 
@@ -81,6 +81,15 @@ async def group_id_with_default_tests(client: AsyncClient, db: AsyncSession, pro
     return await join_group(client, db, project_with_default_tests_id)
 
 
+@pytest_asyncio.fixture
+async def cleanup_files(client: AsyncClient, db: AsyncSession):
+    async def cleaner(submission_id):
+        await set_admin(db, "test", True)
+        await client.delete(f"/api/submissions/{submission_id}")
+        await set_admin(db, "test", False)
+    return cleaner
+
+
 async def join_group(client: AsyncClient, db: AsyncSession, project_id: int):
     group_data["project_id"] = project_id
     await set_admin(db, "test", True)
@@ -92,7 +101,7 @@ async def join_group(client: AsyncClient, db: AsyncSession, project_id: int):
 
 
 @pytest.mark.asyncio
-async def test_no_docker_tests(client: AsyncClient, group_id: int, project_id: int):
+async def test_no_docker_tests(client: AsyncClient, group_id: int, project_id: int, cleanup_files):
     with open(test_files_path / "submission_files/correct.py", "rb") as f:
         response = await client.post("/api/submissions/",
                                      files={"files": ("correct.py", f)},
@@ -110,11 +119,16 @@ async def test_no_docker_tests(client: AsyncClient, group_id: int, project_id: i
 
     assert artifact_response.json() == []  # no artifacts generated because no tests were run
     # cleanup files
-    shutil.rmtree(docker_utils.submissions_path(response.json()["files_uuid"]))
+    await cleanup_files(submission_id)
+
+    assert not os.path.exists(
+        docker_utils.submissions_path(response.json()["files_uuid"]))
+    response = await client.get(f"/api/submissions/{submission_id}")
+    assert response.status_code == 404
 
 
 @pytest.mark.asyncio
-async def test_default_tests_success(client: AsyncClient, group_id_with_default_tests: int):
+async def test_default_tests_success(client: AsyncClient, group_id_with_default_tests: int, cleanup_files):
     # make submission
     files = [
         ('files', ('submission.py', open(test_files_path / 'submission_files/correct.py', 'rb'))),
@@ -145,11 +159,11 @@ async def test_default_tests_success(client: AsyncClient, group_id_with_default_
         {'filename': 'artifact.txt', 'media_type': 'text/plain'}]  # generated artifacts
 
     # cleanup files
-    shutil.rmtree(docker_utils.submissions_path(response.json()["files_uuid"]))
+    await cleanup_files(submission_id)
 
 
 @pytest.mark.asyncio
-async def test_default_tests_failure(client: AsyncClient, group_id_with_default_tests: int):
+async def test_default_tests_failure(client: AsyncClient, group_id_with_default_tests: int, cleanup_files):
     # make submission
     files = [
         ('files', ('submission.py', open(test_files_path / 'submission_files/incorrect.py', 'rb'))),
@@ -179,11 +193,11 @@ async def test_default_tests_failure(client: AsyncClient, group_id_with_default_
     assert artifact_response.json() == []  # no generated artifacts
 
     # cleanup files
-    shutil.rmtree(docker_utils.submissions_path(response.json()["files_uuid"]))
+    await cleanup_files(submission_id)
 
 
 @pytest.mark.asyncio
-async def test_default_tests_crash(client: AsyncClient, group_id_with_default_tests: int):
+async def test_default_tests_crash(client: AsyncClient, group_id_with_default_tests: int, cleanup_files):
     # make submission
     files = [
         ('files', ('submission.py', open(test_files_path / 'submission_files/crashed.py', 'rb'))),
@@ -214,4 +228,4 @@ async def test_default_tests_crash(client: AsyncClient, group_id_with_default_te
     assert artifact_response.json() == []  # no generated artifacts
 
     # cleanup files
-    shutil.rmtree(docker_utils.submissions_path(response.json()["files_uuid"]))
+    await cleanup_files(submission_id)