From de6800ea549833898e37169fd1a0f0982a26a829 Mon Sep 17 00:00:00 2001
From: Michael Plunkett <5885605+michplunkett@users.noreply.github.com>
Date: Wed, 2 Apr 2025 13:14:09 -0500
Subject: [PATCH] Standardize router names (#70)

---
 app/shared/constants.py                       |  4 ++++
 .../{endpoints => routers}/test_default.py    |  0
 .../test_interoperability.py                  |  4 ++--
 .../web/{endpoints => routers}/test_sheet.py  |  5 +++--
 .../web/{endpoints => routers}/test_task.py   | 20 ++++++++++---------
 .../web/{endpoints => routers}/test_url.py    | 15 +++++++-------
 app/tests/web/test_main.py                    |  2 +-
 app/web/main.py                               | 10 +++++-----
 app/web/{endpoints => routers}/__init__.py    |  0
 app/web/{endpoints => routers}/default.py     | 14 ++++++-------
 .../interoperability.py                       |  6 ++----
 app/web/{endpoints => routers}/sheet.py       | 12 +++++------
 app/web/{endpoints => routers}/task.py        | 13 ++++++++----
 app/web/{endpoints => routers}/url.py         | 10 ++++------
 14 files changed, 61 insertions(+), 54 deletions(-)
 create mode 100644 app/shared/constants.py
 rename app/tests/web/{endpoints => routers}/test_default.py (100%)
 rename app/tests/web/{endpoints => routers}/test_interoperability.py (96%)
 rename app/tests/web/{endpoints => routers}/test_sheet.py (98%)
 rename app/tests/web/{endpoints => routers}/test_task.py (71%)
 rename app/tests/web/{endpoints => routers}/test_url.py (96%)
 rename app/web/{endpoints => routers}/__init__.py (100%)
 rename app/web/{endpoints => routers}/default.py (88%)
 rename app/web/{endpoints => routers}/interoperability.py (94%)
 rename app/web/{endpoints => routers}/sheet.py (94%)
 rename app/web/{endpoints => routers}/task.py (81%)
 rename app/web/{endpoints => routers}/url.py (94%)

diff --git a/app/shared/constants.py b/app/shared/constants.py
new file mode 100644
index 0000000..90a5067
--- /dev/null
+++ b/app/shared/constants.py
@@ -0,0 +1,4 @@
+# Statuses
+STATUS_FAILURE = "FAILURE"
+STATUS_PENDING = "PENDING"
+STATUS_SUCCESS = "SUCCESS"
diff --git a/app/tests/web/endpoints/test_default.py b/app/tests/web/routers/test_default.py
similarity index 100%
rename from app/tests/web/endpoints/test_default.py
rename to app/tests/web/routers/test_default.py
diff --git a/app/tests/web/endpoints/test_interoperability.py b/app/tests/web/routers/test_interoperability.py
similarity index 96%
rename from app/tests/web/endpoints/test_interoperability.py
rename to app/tests/web/routers/test_interoperability.py
index d102116..d53c289 100644
--- a/app/tests/web/endpoints/test_interoperability.py
+++ b/app/tests/web/routers/test_interoperability.py
@@ -15,7 +15,7 @@ def test_submit_manual_archive_not_user_auth(client_with_auth, test_no_auth):
 
 
 @patch(
-    "app.web.endpoints.interoperability.business_logic",
+    "app.web.routers.interoperability.business_logic",
     return_value=MagicMock(
         get_store_archive_until=MagicMock(return_value=datetime)
     ),
@@ -103,7 +103,7 @@ def test_submit_manual_archive_invalid_json(client_with_token):
 
 
 @patch(
-    "app.web.endpoints.interoperability.business_logic.get_store_archive_until",
+    "app.web.routers.interoperability.business_logic.get_store_archive_until",
     side_effect=AssertionError("AssertionError"),
 )
 def test_submit_manual_archive_no_store_until(
diff --git a/app/tests/web/endpoints/test_sheet.py b/app/tests/web/routers/test_sheet.py
similarity index 98%
rename from app/tests/web/endpoints/test_sheet.py
rename to app/tests/web/routers/test_sheet.py
index c318496..41c6fd6 100644
--- a/app/tests/web/endpoints/test_sheet.py
+++ b/app/tests/web/routers/test_sheet.py
@@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch
 
 from fastapi.testclient import TestClient
 
+from app.shared.constants import STATUS_PENDING
 from app.shared.db import models
 from app.shared.schemas import TaskResult
 from app.web.db.user_state import UserState
@@ -184,7 +185,7 @@ def test_delete_sheet_endpoint(client_with_auth, db_session):
 
 
 class TestArchiveUserSheetEndpoint:
-    @patch("app.web.endpoints.sheet.celery", return_value=MagicMock())
+    @patch("app.web.routers.sheet.celery", return_value=MagicMock())
     def test_normal_flow(self, m_celery, client_with_auth, db_session):
         db_session.add(
             models.Sheet(
@@ -199,7 +200,7 @@ class TestArchiveUserSheetEndpoint:
 
         m_signature = MagicMock()
         m_signature.apply_async.return_value = TaskResult(
-            id="123-taskid", status="PENDING", result=""
+            id="123-taskid", status=STATUS_PENDING, result=""
         )
         m_celery.signature.return_value = m_signature
 
diff --git a/app/tests/web/endpoints/test_task.py b/app/tests/web/routers/test_task.py
similarity index 71%
rename from app/tests/web/endpoints/test_task.py
rename to app/tests/web/routers/test_task.py
index 038babf..8165d58 100644
--- a/app/tests/web/endpoints/test_task.py
+++ b/app/tests/web/routers/test_task.py
@@ -1,14 +1,16 @@
 from http import HTTPStatus
 from unittest.mock import patch
 
+from app.shared.constants import STATUS_FAILURE, STATUS_PENDING, STATUS_SUCCESS
+
 
 def test_endpoint_task_status_no_auth(client, test_no_auth):
     test_no_auth(client.get, "/task/test-task-id")
 
 
-@patch("app.web.endpoints.task.AsyncResult")
+@patch("app.web.routers.task.AsyncResult")
 def test_get_status_success(mock_async_result, client_with_auth):
-    mock_async_result.return_value.status = "SUCCESS"
+    mock_async_result.return_value.status = STATUS_SUCCESS
     mock_async_result.return_value.result = {"data": "some result"}
 
     response = client_with_auth.get("/task/test-task-id")
@@ -16,14 +18,14 @@ def test_get_status_success(mock_async_result, client_with_auth):
     assert response.status_code == HTTPStatus.OK
     assert response.json() == {
         "id": "test-task-id",
-        "status": "SUCCESS",
+        "status": STATUS_SUCCESS,
         "result": {"data": "some result"},
     }
 
 
-@patch("app.web.endpoints.task.AsyncResult")
+@patch("app.web.routers.task.AsyncResult")
 def test_get_status_failure(mock_async_result, client_with_auth):
-    mock_async_result.return_value.status = "FAILURE"
+    mock_async_result.return_value.status = STATUS_FAILURE
     mock_async_result.return_value.result = Exception("Some error")
 
     response = client_with_auth.get("/task/test-task-id")
@@ -31,14 +33,14 @@ def test_get_status_failure(mock_async_result, client_with_auth):
     assert response.status_code == HTTPStatus.OK
     assert response.json() == {
         "id": "test-task-id",
-        "status": "FAILURE",
+        "status": STATUS_FAILURE,
         "result": {"error": "Some error"},
     }
 
 
-@patch("app.web.endpoints.task.AsyncResult")
+@patch("app.web.routers.task.AsyncResult")
 def test_get_status_pending(mock_async_result, client_with_auth):
-    mock_async_result.return_value.status = "PENDING"
+    mock_async_result.return_value.status = STATUS_PENDING
     mock_async_result.return_value.result = None
 
     response = client_with_auth.get("/task/test-task-id")
@@ -46,6 +48,6 @@ def test_get_status_pending(mock_async_result, client_with_auth):
     assert response.status_code == HTTPStatus.OK
     assert response.json() == {
         "id": "test-task-id",
-        "status": "PENDING",
+        "status": STATUS_PENDING,
         "result": None,
     }
diff --git a/app/tests/web/endpoints/test_url.py b/app/tests/web/routers/test_url.py
similarity index 96%
rename from app/tests/web/endpoints/test_url.py
rename to app/tests/web/routers/test_url.py
index cd64262..0d1d452 100644
--- a/app/tests/web/endpoints/test_url.py
+++ b/app/tests/web/routers/test_url.py
@@ -3,6 +3,7 @@ from http import HTTPStatus
 from unittest.mock import MagicMock, patch
 
 from app.shared import schemas
+from app.shared.constants import STATUS_PENDING
 from app.shared.db import worker_crud
 from app.shared.schemas import ArchiveCreate, TaskResult
 from app.web.config import ALLOW_ANY_EMAIL
@@ -12,12 +13,12 @@ def test_archive_url_unauthenticated(client, test_no_auth):
     test_no_auth(client.post, "/url/archive")
 
 
-@patch("app.web.endpoints.url.UserState")
-@patch("app.web.endpoints.url.celery", return_value=MagicMock())
+@patch("app.web.routers.url.UserState")
+@patch("app.web.routers.url.celery", return_value=MagicMock())
 def test_archive_url(m_celery, m2, client_with_auth):
     m_signature = MagicMock()
     m_signature.apply_async.return_value = TaskResult(
-        id="123-456-789", status="PENDING", result=""
+        id="123-456-789", status=STATUS_PENDING, result=""
     )
     m_celery.signature.return_value = m_signature
 
@@ -123,7 +124,7 @@ def test_archive_url(m_celery, m2, client_with_auth):
     assert m_signature.apply_async.call_count == 2
 
 
-@patch("app.web.endpoints.url.UserState")
+@patch("app.web.routers.url.UserState")
 def test_archive_url_quotas(m1, client_with_auth):
     m_user_state = MagicMock()
     m1.return_value = m_user_state
@@ -152,11 +153,11 @@ def test_archive_url_quotas(m1, client_with_auth):
     m_user_state.has_quota_max_monthly_mbs.assert_called_once()
 
 
-@patch("app.web.endpoints.url.celery", return_value=MagicMock())
+@patch("app.web.routers.url.celery", return_value=MagicMock())
 def test_archive_url_with_api_token(m_celery, client_with_token):
     m_signature = MagicMock()
     m_signature.apply_async.return_value = TaskResult(
-        id="123-456-789", status="PENDING", result=""
+        id="123-456-789", status=STATUS_PENDING, result=""
     )
     m_celery.signature.return_value = m_signature
     response = client_with_token.post(
@@ -274,7 +275,7 @@ def test_search_by_url(client_with_auth, client_with_token, db_session):
     assert len(response.json()) == 10
 
 
-@patch("app.web.endpoints.url.UserState")
+@patch("app.web.routers.url.UserState")
 def test_search_no_read_access(mock_user_state, client_with_auth):
     mock_user_state.return_value.read = False
     mock_user_state.return_value.read_public = False
diff --git a/app/tests/web/test_main.py b/app/tests/web/test_main.py
index 1b6e86b..a8a2c4f 100644
--- a/app/tests/web/test_main.py
+++ b/app/tests/web/test_main.py
@@ -40,7 +40,7 @@ def test_alembic(db_session):
 
 
 @patch(
-    "app.web.endpoints.url.crud.soft_delete_archive",
+    "app.web.routers.url.crud.soft_delete_archive",
     side_effect=Exception("mocked error"),
 )
 def test_logging_middleware(m1, client_with_auth):
diff --git a/app/web/main.py b/app/web/main.py
index 525eff4..df5ca73 100644
--- a/app/web/main.py
+++ b/app/web/main.py
@@ -9,13 +9,13 @@ from prometheus_fastapi_instrumentator import Instrumentator
 from app.shared.settings import Settings, get_settings
 from app.shared.task_messaging import get_celery
 from app.web.config import API_DESCRIPTION, VERSION
-from app.web.endpoints.default import default_router
-from app.web.endpoints.interoperability import interoperability_router
-from app.web.endpoints.sheet import sheet_router
-from app.web.endpoints.task import task_router
-from app.web.endpoints.url import url_router
 from app.web.events import lifespan
 from app.web.middleware import logging_middleware
+from app.web.routers.default import router as default_router
+from app.web.routers.interoperability import router as interoperability_router
+from app.web.routers.sheet import router as sheet_router
+from app.web.routers.task import router as task_router
+from app.web.routers.url import router as url_router
 from app.web.security import token_api_key_auth
 
 
diff --git a/app/web/endpoints/__init__.py b/app/web/routers/__init__.py
similarity index 100%
rename from app/web/endpoints/__init__.py
rename to app/web/routers/__init__.py
diff --git a/app/web/endpoints/default.py b/app/web/routers/default.py
similarity index 88%
rename from app/web/endpoints/default.py
rename to app/web/routers/default.py
index ce3edd2..0588ea8 100644
--- a/app/web/endpoints/default.py
+++ b/app/web/routers/default.py
@@ -11,22 +11,22 @@ from app.web.db.user_state import UserState
 from app.web.security import get_user_state
 
 
-default_router = APIRouter()
+router = APIRouter()
 
 
-@default_router.get("/")
+@router.get("/")
 async def home() -> JSONResponse:
     return JSONResponse(
         {"version": VERSION, "breakingChanges": BREAKING_CHANGES}
     )
 
 
-@default_router.get("/health")
+@router.get("/health")
 async def health() -> JSONResponse:
     return JSONResponse({"status": "ok"})
 
 
-@default_router.get(
+@router.get(
     "/user/active", summary="Check if the user is active and can use the tool."
 )
 async def active(
@@ -35,7 +35,7 @@ async def active(
     return ActiveUser(active=user.active)
 
 
-@default_router.get(
+@router.get(
     "/user/permissions",
     summary="Get the user's global 'all' permissions and the permissions for each group they belong to.",
 )
@@ -45,7 +45,7 @@ def get_user_permissions(
     return user.permissions
 
 
-@default_router.get(
+@router.get(
     "/user/usage",
     summary="Get the user's monthly URLs/MBs usage along with the total active sheets, breakdown by group.",
 )
@@ -59,6 +59,6 @@ def get_user_usage(
     return user.usage()
 
 
-@default_router.get("/favicon.ico", include_in_schema=False)
+@router.get("/favicon.ico", include_in_schema=False)
 async def favicon() -> FileResponse:
     return FileResponse("app/web/static/favicon.ico")
diff --git a/app/web/endpoints/interoperability.py b/app/web/routers/interoperability.py
similarity index 94%
rename from app/web/endpoints/interoperability.py
rename to app/web/routers/interoperability.py
index d08bf50..1292698 100644
--- a/app/web/endpoints/interoperability.py
+++ b/app/web/routers/interoperability.py
@@ -17,13 +17,11 @@ from app.web.security import token_api_key_auth
 from app.web.utils.misc import get_all_urls
 
 
-interoperability_router = APIRouter(
-    prefix="/interop", tags=["Interoperability endpoints."]
-)
+router = APIRouter(prefix="/interop", tags=["Interoperability endpoints."])
 
 
 # ----- endpoint to submit data archived elsewhere
-@interoperability_router.post(
+@router.post(
     "/submit-archive",
     status_code=HTTPStatus.CREATED,
     summary="Submit a manual archive entry, for data that was archived elsewhere.",
diff --git a/app/web/endpoints/sheet.py b/app/web/routers/sheet.py
similarity index 94%
rename from app/web/endpoints/sheet.py
rename to app/web/routers/sheet.py
index 5247107..2a06bf3 100644
--- a/app/web/endpoints/sheet.py
+++ b/app/web/routers/sheet.py
@@ -18,14 +18,12 @@ from app.web.db.user_state import UserState
 from app.web.security import get_user_state
 
 
-sheet_router = APIRouter(
-    prefix="/sheet", tags=["Google Spreadsheet operations"]
-)
+router = APIRouter(prefix="/sheet", tags=["Google Spreadsheet operations"])
 
 celery = get_celery()
 
 
-@sheet_router.post(
+@router.post(
     "/create",
     status_code=HTTPStatus.CREATED,
     summary="Store a new Google Sheet for regular archiving.",
@@ -69,7 +67,7 @@ def create_sheet(
         ) from e
 
 
-@sheet_router.get(
+@router.get(
     "/mine",
     status_code=HTTPStatus.OK,
     summary="Get the authenticated user's Google Sheets.",
@@ -81,7 +79,7 @@ def get_user_sheets(
     return crud.get_user_sheets(db, user.email)
 
 
-@sheet_router.delete("/{sheet_id}", summary="Delete a Google Sheet by ID.")
+@router.delete("/{sheet_id}", summary="Delete a Google Sheet by ID.")
 def delete_sheet(
     sheet_id: str,
     user: UserState = Depends(get_user_state),
@@ -92,7 +90,7 @@ def delete_sheet(
     )
 
 
-@sheet_router.post(
+@router.post(
     "/{sheet_id}/archive",
     status_code=HTTPStatus.CREATED,
     summary="Trigger an archiving task for a GSheet you own.",
diff --git a/app/web/endpoints/task.py b/app/web/routers/task.py
similarity index 81%
rename from app/web/endpoints/task.py
rename to app/web/routers/task.py
index 3581ab2..9b81429 100644
--- a/app/web/endpoints/task.py
+++ b/app/web/routers/task.py
@@ -4,18 +4,19 @@ from fastapi.encoders import jsonable_encoder
 from fastapi.responses import JSONResponse
 
 from app.shared import schemas
+from app.shared.constants import STATUS_FAILURE
 from app.shared.log import log_error
 from app.shared.task_messaging import get_celery
 from app.web.security import get_token_or_user_auth
 from app.web.utils.misc import custom_jsonable_encoder
 
 
-task_router = APIRouter(prefix="/task", tags=["Async task operations"])
+router = APIRouter(prefix="/task", tags=["Async task operations"])
 
 celery = get_celery()
 
 
-@task_router.get(
+@router.get(
     "/{task_id}",
     summary="Check the status of an async task by its id, works for URLs and Sheet tasks.",
 )
@@ -24,7 +25,7 @@ def get_status(
 ) -> schemas.TaskResult:
     task = AsyncResult(task_id, app=celery)
     try:
-        if task.status == "FAILURE":
+        if task.status == STATUS_FAILURE:
             # *FAILURE* The task raised an exception, or has exceeded the retry limit.
             # The :attr:`result` attribute then contains the exception raised by
             # the task.
@@ -43,5 +44,9 @@ def get_status(
     except Exception as e:
         log_error(e)
         return JSONResponse(
-            {"id": task_id, "status": "FAILURE", "result": {"error": str(e)}}
+            {
+                "id": task_id,
+                "status": STATUS_FAILURE,
+                "result": {"error": str(e)},
+            }
         )
diff --git a/app/web/endpoints/url.py b/app/web/routers/url.py
similarity index 94%
rename from app/web/endpoints/url.py
rename to app/web/routers/url.py
index 10b0adc..dd3e7c5 100644
--- a/app/web/endpoints/url.py
+++ b/app/web/routers/url.py
@@ -18,12 +18,12 @@ from app.web.security import get_token_or_user_auth, get_user_state
 from app.web.utils.misc import convert_priority_to_queue_dict
 
 
-url_router = APIRouter(prefix="/url", tags=["Single URL operations"])
+router = APIRouter(prefix="/url", tags=["Single URL operations"])
 
 celery = get_celery()
 
 
-@url_router.post(
+@router.post(
     "/archive",
     status_code=HTTPStatus.CREATED,
     summary="Submit a single URL archive request, starts an archiving task.",
@@ -77,7 +77,7 @@ def archive_url(
     )
 
 
-@url_router.get("/search", summary="Search for archive entries by URL.")
+@router.get("/search", summary="Search for archive entries by URL.")
 def search_by_url(
     url: str,
     skip: int = 0,
@@ -110,9 +110,7 @@ def search_by_url(
     )
 
 
-@url_router.delete(
-    "/{archive_id}", summary="Delete a single URL archive by id."
-)
+@router.delete("/{archive_id}", summary="Delete a single URL archive by id.")
 def delete_archive(
     archive_id: str,
     user: UserState = Depends(get_user_state),