Use uuids for logos (#488)

Otherwise uploads can replace current files with the same name.
Also fixes and adds more tests for logo uploads.

Author: evroon

.gitignore

@@ -15,3 +15,4 @@ coverage.xml
 coverage.json
 
 backend/yarn.lock
+backend/static

backend/bracket/routes/tournaments.py

@@ -1,3 +1,6 @@
+import os
+from uuid import uuid4
+
 import aiofiles.os
 import asyncpg  # type: ignore[import-untyped]
 from fastapi import APIRouter, Depends, HTTPException, UploadFile
@@ -9,7 +12,6 @@
 from bracket.logic.subscriptions import check_requirement
 from bracket.logic.tournaments import get_tournament_logo_path
 from bracket.models.db.tournament import (
-    Tournament,
     TournamentBody,
     TournamentToInsert,
     TournamentUpdateBody,
@@ -25,11 +27,11 @@
 from bracket.schema import tournaments
 from bracket.sql.tournaments import (
     sql_delete_tournament,
+    sql_get_tournament,
     sql_get_tournament_by_endpoint_name,
     sql_get_tournaments,
 )
 from bracket.sql.users import get_user_access_to_club, get_which_clubs_has_user_access_to
-from bracket.utils.db import fetch_one_parsed_certain
 from bracket.utils.errors import (
     ForeignKey,
     UniqueIndex,
@@ -51,9 +53,7 @@
 async def get_tournament(
     tournament_id: int, user: UserPublic | None = Depends(user_authenticated_or_public_dashboard)
 ) -> TournamentResponse:
-    tournament = await fetch_one_parsed_certain(
-        database, Tournament, tournaments.select().where(tournaments.c.id == tournament_id)
-    )
+    tournament = await sql_get_tournament(tournament_id)
     if user is None and not tournament.dashboard_public:
         raise unauthorized_exception
 
@@ -154,13 +154,22 @@ async def upload_logo(
     tournament_id: int,
     file: UploadFile | None = None,
     _: UserPublic = Depends(user_authenticated_for_tournament),
-) -> SuccessResponse:
+) -> TournamentResponse:
     old_logo_path = await get_tournament_logo_path(tournament_id)
-    new_logo_path = f"static/{file.filename}" if file is not None else None
+    filename: str | None = None
+    new_logo_path: str | None = None
 
-    if file and new_logo_path:
-        async with aiofiles.open(new_logo_path, "wb") as f:
-            await f.write(await file.read())
+    if file:
+        assert file.filename is not None
+        extension = os.path.splitext(file.filename)[1]
+        assert extension in (".png", ".jpg", ".jpeg")
+
+        filename = f"{uuid4()}{extension}"
+        new_logo_path = f"static/{filename}" if file is not None else None
+
+        if new_logo_path:
+            async with aiofiles.open(new_logo_path, "wb") as f:
+                await f.write(await file.read())
 
     if old_logo_path is not None and old_logo_path != new_logo_path:
         try:
@@ -170,6 +179,6 @@ async def upload_logo(
 
     await database.execute(
         tournaments.update().where(tournaments.c.id == tournament_id),
-        values={"logo_path": file.filename if file else None},
+        values={"logo_path": filename},
     )
-    return SuccessResponse()
+    return TournamentResponse(data=await sql_get_tournament(tournament_id))

backend/tests/integration_tests/api/shared.py

@@ -70,7 +70,7 @@ async def down(self) -> None:
 async def send_request(
     method: HTTPMethod,
     endpoint: str,
-    body: JsonDict | AsyncIterator[bytes] | None = None,
+    body: JsonDict | AsyncIterator[bytes] | aiohttp.FormData | None = None,
     json: JsonDict | None = None,
     headers: JsonDict = {},
 ) -> JsonDict:
@@ -111,7 +111,7 @@ async def send_tournament_request(
     method: HTTPMethod,
     endpoint: str,
     auth_context: AuthContext,
-    body: JsonDict | AsyncIterator[bytes] | None = None,
+    body: JsonDict | AsyncIterator[bytes] | aiohttp.FormData | None = None,
     json: JsonDict | None = None,
 ) -> JsonDict:
     return await send_request(

backend/tests/integration_tests/api/tournaments_test.py

@@ -1,6 +1,5 @@
-from collections.abc import AsyncIterator
-
 import aiofiles
+import aiohttp
 
 from bracket.database import database
 from bracket.models.db.tournament import Tournament
@@ -130,22 +129,31 @@ async def test_delete_tournament(
     await sql_delete_tournament(assert_some(tournament_inserted.id))
 
 
-async def file_sender(file_name: str) -> AsyncIterator[bytes]:
-    async with aiofiles.open(file_name, "rb") as f:
-        chunk = await f.read(64 * 1024)
-        while chunk:
-            yield chunk
-            chunk = await f.read(64 * 1024)
-
-
-async def test_tournament_upload_logo(
+async def test_tournament_upload_and_remove_logo(
     startup_and_shutdown_uvicorn_server: None, auth_context: AuthContext
 ) -> None:
+    test_file_path = "tests/integration_tests/assets/test_logo.png"
+    data = aiohttp.FormData()
+    data.add_field(
+        "file",
+        open(test_file_path, "rb"),  # pylint: disable=consider-using-with
+        filename="test_logo.png",
+        content_type="image/png",
+    )
+
     response = await send_tournament_request(
         method=HTTPMethod.POST,
         endpoint="logo",
         auth_context=auth_context,
-        body=file_sender(file_name="tests/integration_tests/assets/test_logo.png"),
+        body=data,
+    )
+
+    assert response["data"]["logo_path"], f"Response: {response}"
+    assert await aiofiles.os.path.exists(f"static/{response['data']['logo_path']}")
+
+    response = await send_tournament_request(
+        method=HTTPMethod.POST, endpoint="logo", auth_context=auth_context, body=aiohttp.FormData()
     )
 
-    assert response.get("success"), f"Response: {response}"
+    assert response["data"]["logo_path"] is None, f"Response: {response}"
+    assert not await aiofiles.os.path.exists(f"static/{response['data']['logo_path']}")

frontend/public/locales/en/common.json

@@ -75,7 +75,7 @@
     "drop_match_alert_title": "Drop a match here",
     "dropzone_accept_text": "Drop files here",
     "dropzone_idle_text": "Upload Logo",
-    "dropzone_reject_text": "Image must be less than 10MB",
+    "dropzone_reject_text": "Image must be less than 5MB.",
     "duration_minutes_choose_title": "Please choose a duration of the matches",
     "edit_club_button": "Edit Club",
     "edit_details_tab_title": "Edit details",

frontend/public/locales/nl/common.json

@@ -75,7 +75,7 @@
   "drop_match_alert_title": "Plaats hier een wedstrijd",
   "dropzone_accept_text": "Upload hier bestanden",
   "dropzone_idle_text": "Logo uploaden",
-  "dropzone_reject_text": "De afbeelding moet kleiner zijn dan 10 MB",
+  "dropzone_reject_text": "De afbeelding moet kleiner zijn dan 5 MB.",
   "duration_minutes_choose_title": "Vul de duur van de wedstrijden in",
   "edit_club_button": "Club bewerken",
   "edit_details_tab_title": "Details bewerken",

frontend/public/locales/zh-CN/common.json

@@ -71,7 +71,7 @@
     "drop_match_alert_title": "在这里放下一场比赛",
     "dropzone_accept_text": "在此处放置文件",
     "dropzone_idle_text": "上传标志",
-    "dropzone_reject_text": "图片必须小于 10MB",
+    "dropzone_reject_text": "图片必须小于 5MB",
     "duration_minutes_choose_title": "请选择比赛的持续时间",
     "edit_club_button": "编辑俱乐部",
     "edit_details_tab_title": "编辑详情",

frontend/src/components/utils/file_upload.tsx

@@ -32,7 +32,7 @@ export function DropzoneButton({
         // className={classes.dropzone}
         radius="md"
         accept={[MIME_TYPES.png, MIME_TYPES.jpeg]}
-        maxSize={30 * 1024 ** 2}
+        maxSize={5 * 1024 ** 2}
       >
         <div style={{ pointerEvents: 'none' }}>
           <Group justify="center">
@@ -54,6 +54,8 @@ export function DropzoneButton({
           </Text>
           <Text ta="center" size="sm" mt="xs" c="dimmed">
             {t('upload_placeholder')}
+            <br />
+            {t('dropzone_reject_text')}
           </Text>
         </div>
       </Dropzone>