From 5505d2e9a957ba12462909a9ddf740da224fbb59 Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Sat, 12 Aug 2023 17:43:02 -0400
Subject: [PATCH 1/6] feat(backend): file deletion flow

---
 backend/rotini/api/files.py       | 20 ++++++++++++++++++++
 backend/rotini/use_cases/files.py | 27 +++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/backend/rotini/api/files.py b/backend/rotini/api/files.py
index a78a6f7..c947970 100644
--- a/backend/rotini/api/files.py
+++ b/backend/rotini/api/files.py
@@ -40,3 +40,23 @@ def get_file_details(file_id: str):
         raise HTTPException(status_code=404)
 
     return file
+
+
+@router.delete("/{file_id}/")
+def delete_file(file_id: str):
+    """
+    Deletes a file given its ID.
+
+    This will delete the file in the database records as well
+    as on disk. The operation is not reversible.
+
+    DELETE /files/{file_id}/
+
+    200 { <FileData> }
+    """
+    try:
+        file = files_use_cases.delete_file_record_by_id(file_id)
+    except files_use_cases.DoesNotExist as exc:
+        raise HTTPException(status_code=404) from exc
+
+    return file
diff --git a/backend/rotini/use_cases/files.py b/backend/rotini/use_cases/files.py
index c92452f..ed32412 100644
--- a/backend/rotini/use_cases/files.py
+++ b/backend/rotini/use_cases/files.py
@@ -11,6 +11,12 @@ import pathlib
 from db import get_connection
 
 
+class DoesNotExist(Exception):
+    """
+    General purpose exception signalling a failure to find a database record.
+    """
+
+
 class FileRecord(typing.TypedDict):
     """
     Database record associated with a file tracked
@@ -84,3 +90,24 @@ def get_file_record_by_id(file_id: str) -> typing.Optional[FileRecord]:
     return FileRecord(
         id=row[0], path=row[1], size=row[2], filename=pathlib.Path(row[1]).name
     )
+
+
+def delete_file_record_by_id(file_id: str) -> typing.Union[typing.NoReturn, FileRecord]:
+    """
+    Deletes a single file by ID, including its presence in storage.
+
+    If the ID doesn't correspond to a record, DoesNotExist is raised.
+    """
+
+    row = None
+    with get_connection() as connection, connection.cursor() as cursor:
+        cursor.execute("DELETE FROM files WHERE id=%s RETURNING *;", (file_id,))
+        row = cursor.fetchone()
+        pathlib.Path(row[1]).unlink()
+
+    if row is None:
+        raise DoesNotExist()
+
+    return FileRecord(
+        id=row[0], path=row[1], size=row[2], filename=pathlib.Path(row[1]).name
+    )
-- 
2.45.2


From 0b11877f81c92d0a3d32f873e46b6679d932a6a4 Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Sat, 12 Aug 2023 17:46:43 -0400
Subject: [PATCH 2/6] ci: set change-check flags when no changes detected

---
 .github/workflows/ci.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 6d16908..fc58900 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -41,10 +41,14 @@ jobs:
           if [ -n "$(git diff --name-only origin/main origin/${GITHUB_HEAD_REF} -- ./frontend)" ]
           then
             echo "fe_changed=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "fe_changed=false" >> "$GITHUB_OUTPUT"
           fi
           if [ -n "$(git diff --name-only origin/main origin/${GITHUB_HEAD_REF} -- ./backend)" ]
           then
             echo "be_changed=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "be_changed=false" >> "$GITHUB_OUTPUT"
           fi
   backend:
     uses: ./.github/workflows/backend-pipeline.yml
-- 
2.45.2


From faa09f67fcc9328a69564c8d6b56b49cd446b3c8 Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Sun, 13 Aug 2023 15:57:22 -0400
Subject: [PATCH 3/6] feat(backend): refactor settings to be a singleton object

---
 Taskfile.backend.yml                 |  2 ++
 backend/rotini/api/files.py          |  9 ++++--
 backend/rotini/db.py                 |  2 +-
 backend/rotini/envs/test.py          |  4 +++
 backend/rotini/migrations/migrate.py |  3 +-
 backend/rotini/settings.py           | 46 +++++++++++++++++++++++++---
 backend/rotini/use_cases/files.py    |  3 +-
 backend/tests/conftest.py            | 19 ++++++++++++
 8 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/Taskfile.backend.yml b/Taskfile.backend.yml
index 03e6f7b..12e2198 100644
--- a/Taskfile.backend.yml
+++ b/Taskfile.backend.yml
@@ -47,6 +47,8 @@ tasks:
   migrate:
     desc: "Applies migrations. Usage: be:migrate -- <up|down>"
     deps: [bootstrap]
+    env:
+      PYTHONPATH: '..'
     dotenv:
       - "{{ .DOTENV }}"
     cmd: "{{ .VENV_BIN }}/python migrate.py {{ .CLI_ARGS }}"
diff --git a/backend/rotini/api/files.py b/backend/rotini/api/files.py
index c947970..dd0aa08 100644
--- a/backend/rotini/api/files.py
+++ b/backend/rotini/api/files.py
@@ -5,9 +5,12 @@ This API allows users to create and query for existing data about
 files that live in the system.
 """
 
+import pathlib
+
 from fastapi import APIRouter, HTTPException, UploadFile
 
 import use_cases.files as files_use_cases
+from settings import settings
 
 router = APIRouter(prefix="/files")
 
@@ -23,11 +26,13 @@ async def upload_file(file: UploadFile):
     size = len(content)
     await file.seek(0)
 
-    with open(file.filename, "wb") as f:
+    print("ROOT:", settings.STORAGE_ROOT)
+    dest_path = pathlib.Path(settings.STORAGE_ROOT, file.filename)
+    with open(dest_path, "wb") as f:
         content = await file.read()
         f.write(content)
 
-    created_record = files_use_cases.create_file_record(file.filename, size)
+    created_record = files_use_cases.create_file_record(str(dest_path), size)
 
     return created_record
 
diff --git a/backend/rotini/db.py b/backend/rotini/db.py
index 6d2242f..58be6b7 100644
--- a/backend/rotini/db.py
+++ b/backend/rotini/db.py
@@ -1,6 +1,6 @@
 import psycopg2
 
-import settings
+from settings import settings
 
 
 def get_connection():
diff --git a/backend/rotini/envs/test.py b/backend/rotini/envs/test.py
index f5e97a3..d348b5c 100644
--- a/backend/rotini/envs/test.py
+++ b/backend/rotini/envs/test.py
@@ -1,5 +1,9 @@
+import os
+
 DATABASE_USERNAME = "postgres"
 DATABASE_PASSWORD = "test"
 DATABASE_HOST = "localhost"
 DATABASE_PORT = 5431
 DATABASE_NAME = "postgres"
+
+STORAGE_ROOT = os.getenv("ROTINI_STORAGE_ROOT")
diff --git a/backend/rotini/migrations/migrate.py b/backend/rotini/migrations/migrate.py
index 1d6da62..79cf84e 100644
--- a/backend/rotini/migrations/migrate.py
+++ b/backend/rotini/migrations/migrate.py
@@ -27,7 +27,6 @@ Not including a migration name executes everything from the last executed
 migration.
 """
 
-import os
 import collections
 import pathlib
 import datetime
@@ -38,7 +37,7 @@ import sys
 
 import psycopg2
 
-import settings
+from settings import settings
 
 VALID_COMMANDS = ["up", "down", "new"]
 
diff --git a/backend/rotini/settings.py b/backend/rotini/settings.py
index 093eff2..158888b 100644
--- a/backend/rotini/settings.py
+++ b/backend/rotini/settings.py
@@ -1,12 +1,50 @@
-# pylint: disable=unused-import, wildcard-import, unused-wildcard-import
+# pylint: disable=unused-import, wildcard-import, unused-wildcard-import, too-few-public-methods
 import os
+import typing
 
 IS_CI = os.getenv("ROTINI_CI")
 IS_TEST = os.getenv("ROTINI_TEST")
 
+
+class Settings:
+    """
+    Representation of the configuration settings available to the
+    application.
+    """
+
+    ENV: str
+    DATABASE_USERNAME: str
+    DATABASE_PASSWORD: str
+    DATABASE_HOST: str
+    DATABASE_PORT: int
+    DATABASE_NAME: str
+    STORAGE_ROOT: typing.Optional[str] = "."
+
+    def __init__(self, *_, **kwargs):
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+
+def extract_settings(env: str, imported_module) -> Settings:
+    """
+    Extracts all the exposed values from the given module and
+    creates a corresponding Settings object.
+    """
+    imported_values = {
+        k: v for k, v in imported_module.__dict__.items() if not k.startswith("__")
+    }
+    return Settings(ENV=env, **imported_values)
+
+
 if IS_CI is not None:
-    from envs.ci import *
+    import envs.ci as ci_config
+
+    settings = extract_settings("ci", ci_config)
 elif IS_TEST is not None:
-    from envs.test import *
+    import envs.test as test_config
+
+    settings = extract_settings("ci", test_config)
 else:
-    from envs.local import *
+    import envs.local as local_config
+
+    settings = extract_settings("ci", local_config)
diff --git a/backend/rotini/use_cases/files.py b/backend/rotini/use_cases/files.py
index ed32412..cdf888f 100644
--- a/backend/rotini/use_cases/files.py
+++ b/backend/rotini/use_cases/files.py
@@ -9,6 +9,7 @@ import typing
 import pathlib
 
 from db import get_connection
+from settings import settings
 
 
 class DoesNotExist(Exception):
@@ -103,7 +104,7 @@ def delete_file_record_by_id(file_id: str) -> typing.Union[typing.NoReturn, File
     with get_connection() as connection, connection.cursor() as cursor:
         cursor.execute("DELETE FROM files WHERE id=%s RETURNING *;", (file_id,))
         row = cursor.fetchone()
-        pathlib.Path(row[1]).unlink()
+        pathlib.Path(pathlib.Path(settings.STORAGE_ROOT, row[1])).unlink()
 
     if row is None:
         raise DoesNotExist()
diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py
index ec18053..649e779 100644
--- a/backend/tests/conftest.py
+++ b/backend/tests/conftest.py
@@ -1,9 +1,12 @@
 from fastapi.testclient import TestClient
 import pytest
+import unittest.mock
 
 from rotini.main import app
 from rotini.db import get_connection
 
+from settings import settings
+
 
 @pytest.fixture(name="client")
 def fixture_client():
@@ -12,5 +15,21 @@ def fixture_client():
 
 @pytest.fixture(autouse=True)
 def reset_database():
+    """
+    Empties all user tables between tests.
+    """
     with get_connection() as conn, conn.cursor() as cursor:
         cursor.execute("DELETE FROM files;")
+
+
+@pytest.fixture(autouse=True)
+def set_storage_path(tmp_path, monkeypatch):
+    """
+    Ensures that files stored by tests are stored
+    in temporary directories.
+    """
+
+    files_dir = tmp_path / "files"
+    files_dir.mkdir()
+
+    monkeypatch.setattr(settings, "STORAGE_ROOT", str(files_dir))
-- 
2.45.2


From 94d6db64a40f80cfc1a50bc97e82baee67aa944c Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Sun, 13 Aug 2023 17:05:38 -0400
Subject: [PATCH 4/6] refactor(backend): cleaning delete flow cruft

---
 backend/rotini/api/files.py       | 1 -
 backend/rotini/use_cases/files.py | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/backend/rotini/api/files.py b/backend/rotini/api/files.py
index dd0aa08..c8f9f12 100644
--- a/backend/rotini/api/files.py
+++ b/backend/rotini/api/files.py
@@ -26,7 +26,6 @@ async def upload_file(file: UploadFile):
     size = len(content)
     await file.seek(0)
 
-    print("ROOT:", settings.STORAGE_ROOT)
     dest_path = pathlib.Path(settings.STORAGE_ROOT, file.filename)
     with open(dest_path, "wb") as f:
         content = await file.read()
diff --git a/backend/rotini/use_cases/files.py b/backend/rotini/use_cases/files.py
index cdf888f..7dd27a9 100644
--- a/backend/rotini/use_cases/files.py
+++ b/backend/rotini/use_cases/files.py
@@ -104,11 +104,12 @@ def delete_file_record_by_id(file_id: str) -> typing.Union[typing.NoReturn, File
     with get_connection() as connection, connection.cursor() as cursor:
         cursor.execute("DELETE FROM files WHERE id=%s RETURNING *;", (file_id,))
         row = cursor.fetchone()
-        pathlib.Path(pathlib.Path(settings.STORAGE_ROOT, row[1])).unlink()
 
     if row is None:
         raise DoesNotExist()
 
+    pathlib.Path(pathlib.Path(settings.STORAGE_ROOT, row[1])).unlink()
+
     return FileRecord(
         id=row[0], path=row[1], size=row[2], filename=pathlib.Path(row[1]).name
     )
-- 
2.45.2


From 2a6b4a08345ddbe17536ae464f1930d92d02e320 Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Sun, 13 Aug 2023 17:05:54 -0400
Subject: [PATCH 5/6] test(backend): add coverage for deletion + adapt coverage
 to only rely on endpoints

---
 backend/tests/test_api_files.py | 85 +++++++++++++++++++++++++++++----
 1 file changed, 76 insertions(+), 9 deletions(-)

diff --git a/backend/tests/test_api_files.py b/backend/tests/test_api_files.py
index eeffdf1..fd462ee 100644
--- a/backend/tests/test_api_files.py
+++ b/backend/tests/test_api_files.py
@@ -1,23 +1,43 @@
-from rotini.use_cases import files as files_use_cases
+import pathlib
 
 
-def test_list_files_returns_registered_files_and_200(client):
-    file_1 = files_use_cases.create_file_record("/test1.txt", 123)
-    file_2 = files_use_cases.create_file_record("/test2.txt", 456)
+def test_list_files_returns_registered_files_and_200(client, tmp_path):
+    mock_file_1 = tmp_path / "test1.txt"
+    mock_file_1.write_text("testtest")
+
+    with open(str(mock_file_1), "rb") as mock_file_stream:
+        response = client.post("/files/", files={"file": mock_file_stream})
+
+    mock_file_1_data = response.json()
+
+    mock_file_2 = tmp_path / "test2.txt"
+    mock_file_2.write_text("testtest")
+
+    with open(str(mock_file_2), "rb") as mock_file_stream:
+        response = client.post("/files/", files={"file": mock_file_stream})
+
+    mock_file_2_data = response.json()
 
     response = client.get("/files/")
 
     assert response.status_code == 200
-    assert response.json() == [file_1, file_2]
+    assert response.json() == [mock_file_1_data, mock_file_2_data]
 
 
-def test_file_details_returns_specified_file_and_200(client):
-    file = files_use_cases.create_file_record("/test1.txt", 123)
+def test_file_details_returns_specified_file_and_200(client, tmp_path):
+    mock_file = tmp_path / "test.txt"
+    mock_file.write_text("testtest")
 
-    response = client.get(f"/files/{file['id']}/")
+    with open(str(mock_file), "rb") as mock_file_stream:
+        response = client.post("/files/", files={"file": mock_file_stream})
+
+    response_data = response.json()
+    created_file_id = response_data["id"]
+
+    response = client.get(f"/files/{created_file_id}/")
 
     assert response.status_code == 200
-    assert response.json() == file
+    assert response.json() == response_data
 
 
 def test_file_details_returns_404_if_does_not_exist(client):
@@ -25,3 +45,50 @@ def test_file_details_returns_404_if_does_not_exist(client):
     response = client.get(f"/files/{non_existent_id}/")
 
     assert response.status_code == 404
+
+
+def test_file_deletion_returns_404_if_does_not_exist(client):
+    non_existent_id = "06f02980-864d-4832-a894-2e9d2543a79a"
+    response = client.delete(f"/files/{non_existent_id}/")
+
+    assert response.status_code == 404
+
+
+def test_file_deletion_deletes_record_and_file(client, tmp_path):
+    mock_file = tmp_path / "test.txt"
+    mock_file.write_text("testtest")
+
+    with open(str(mock_file), "rb") as mock_file_stream:
+        response = client.post("/files/", files={"file": mock_file_stream})
+
+    response_data = response.json()
+    file_id = response_data["id"]
+    file_path = response_data["path"]
+
+    assert pathlib.Path(file_path).exists()
+    response = client.get(f"/files/{file_id}/")
+
+    assert response.status_code == 200
+
+    client.delete(f"/files/{file_id}/")
+    assert not pathlib.Path(file_path).exists()
+
+    response = client.get(f"/files/{file_id}/")
+
+    assert response.status_code == 404
+
+
+def test_file_deletion_200_and_return_deleted_resource(client, tmp_path):
+    mock_file = tmp_path / "test.txt"
+    mock_file.write_text("testtest")
+
+    with open(str(mock_file), "rb") as mock_file_stream:
+        response = client.post("/files/", files={"file": mock_file_stream})
+
+    response_data = response.json()
+    file_id = response_data["id"]
+
+    response = client.delete(f"/files/{file_id}/")
+
+    assert response.status_code == 200
+    assert response.json() == response_data
-- 
2.45.2


From 68b9a8748866e79d2ef3831b4e7023000e5295f8 Mon Sep 17 00:00:00 2001
From: Marc Cataford <mcat@riseup.net>
Date: Mon, 14 Aug 2023 08:23:31 -0400
Subject: [PATCH 6/6] docs: type responses for deletions

---
 backend/requirements.in           |  1 +
 backend/requirements.txt          |  1 +
 backend/rotini/api/files.py       | 12 ++++++++++--
 backend/rotini/use_cases/files.py |  3 ++-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/backend/requirements.in b/backend/requirements.in
index 0809eaa..c6503c3 100644
--- a/backend/requirements.in
+++ b/backend/requirements.in
@@ -2,3 +2,4 @@ fastapi~=0.101
 uvicorn[standard]
 python-multipart
 psycopg2
+typing_extensions
diff --git a/backend/requirements.txt b/backend/requirements.txt
index 304c772..2a0d9cc 100644
--- a/backend/requirements.txt
+++ b/backend/requirements.txt
@@ -34,6 +34,7 @@ starlette==0.27.0
     # via fastapi
 typing-extensions==4.7.1
     # via
+    #   -r requirements.in
     #   fastapi
     #   pydantic
     #   pydantic-core
diff --git a/backend/rotini/api/files.py b/backend/rotini/api/files.py
index c8f9f12..6c02064 100644
--- a/backend/rotini/api/files.py
+++ b/backend/rotini/api/files.py
@@ -47,7 +47,7 @@ def get_file_details(file_id: str):
 
 
 @router.delete("/{file_id}/")
-def delete_file(file_id: str):
+def delete_file(file_id: str) -> files_use_cases.FileRecord:
     """
     Deletes a file given its ID.
 
@@ -56,7 +56,15 @@ def delete_file(file_id: str):
 
     DELETE /files/{file_id}/
 
-    200 { <FileData> }
+    200 { <FileRecord> }
+
+        The file exists and has been deleted from storage and
+        from the database.
+
+    404 {}
+
+        The file ID did not map to anything.
+
     """
     try:
         file = files_use_cases.delete_file_record_by_id(file_id)
diff --git a/backend/rotini/use_cases/files.py b/backend/rotini/use_cases/files.py
index 7dd27a9..e413d24 100644
--- a/backend/rotini/use_cases/files.py
+++ b/backend/rotini/use_cases/files.py
@@ -5,9 +5,10 @@ Use cases and data structures defined in this file
 manipulate file records in the database or represent them
 after they have been read.
 """
-import typing
 import pathlib
 
+import typing_extensions as typing
+
 from db import get_connection
 from settings import settings
 
-- 
2.45.2