fix: Fixed media duplication (#207)

* chore: Added python-magic to deps

* feat: Updated bucket name resolution and delete media

Author: frgfm

poetry.lock

@@ -22,6 +22,7 @@ aiofiles = "==0.6.0"
 requests = "^2.22.0"
 sentry-sdk = "^1.5.12"
 qarnot = "^2.2.1"
+python-magic = "^0.4.17"
 
 flake8 = { version = ">=3.9.0,<5.0.0", optional = true }
 isort = { version = "^5.7.0", optional = true }

pyproject.toml

@@ -3,8 +3,11 @@
 # This program is licensed under the Apache License version 2.
 # See LICENSE or go to <https://www.apache.org/licenses/LICENSE-2.0.txt> for full license details.
 
+from datetime import datetime
+from mimetypes import guess_extension
 from typing import Any, Dict, List, Optional
 
+import magic
 from fastapi import APIRouter, BackgroundTasks, Depends, File, HTTPException, Path, Security, UploadFile, status
 
 from app.api import crud
@@ -116,7 +119,11 @@ async def delete_media(media_id: int = Path(..., gt=0), _=Security(get_current_a
     """
     Based on a media_id, deletes the specified media
     """
-    return await crud.delete_entry(media, media_id)
+    # Delete entry
+    entry = await crud.delete_entry(media, media_id)
+    # Delete media file
+    await bucket_service.delete_file(entry["bucket_key"])
+    return entry
 
 
 @router.post("/{media_id}/upload", response_model=MediaOut, status_code=200)
@@ -133,9 +140,13 @@ async def upload_media_from_device(
     # Check in DB
     entry = await check_media_registration(media_id, current_device.id)
 
-    # Concatenate the first 32 chars (to avoid system interactions issues) of SHA256 hash with file extension
+    # Concatenate the first 8 chars (to avoid system interactions issues) of SHA256 hash with file extension
     file_hash = hash_content_file(file.file.read())
-    file_name = f"{file_hash[:32]}.{file.filename.rpartition('.')[-1]}"
+    await file.seek(0)
+    # guess_extension will return none if this fails
+    extension = guess_extension(magic.from_buffer(file.file.read(), mime=True)) or ""
+    # Concatenate timestamp & hash
+    file_name = f"{datetime.utcnow().strftime('%Y%m%d%H%M%S')}-{file_hash[:8]}{extension}"
     # Reset byte position of the file (cf. https://fastapi.tiangolo.com/tutorial/request-files/#uploadfile)
     await file.seek(0)
     # If files are in a subfolder of the bucket, prepend the folder path
@@ -167,6 +178,9 @@ async def upload_media_from_device(
             raise HTTPException(
                 status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Data was corrupted during upload"
             )
+        # If a file was previously uploaded, delete it
+        if isinstance(entry["bucket_key"], str):
+            await bucket_service.delete_file(entry["bucket_key"])
 
         entry_dict = dict(**entry)
         entry_dict["bucket_key"] = bucket_key

src/app/api/routes/media.py

@@ -2,8 +2,8 @@ aiofiles==0.6.0
 anyio==3.6.1; python_version >= "3.6" and python_full_version >= "3.6.2"
 asyncpg==0.25.0; python_full_version >= "3.6.0"
 bcrypt==3.2.2; python_version >= "3.6"
-boto3==1.24.19; python_version >= "3.7"
-botocore==1.27.19; python_version >= "3.7"
+boto3==1.24.20; python_version >= "3.7"
+botocore==1.27.20; python_version >= "3.7"
 certifi==2022.6.15; python_version >= "3.7" and python_version < "4"
 cffi==1.15.0; python_version >= "3.6"
 charset-normalizer==2.1.0; python_version >= "3.7" and python_version < "4" and python_full_version >= "3.6.0"
@@ -26,6 +26,7 @@ pydantic==1.9.1; python_full_version >= "3.6.1"
 pyparsing==3.0.9; python_full_version >= "3.6.8" and python_version >= "3.6"
 python-dateutil==2.8.2; python_version >= "3.7" and python_full_version < "3.0.0" or python_full_version >= "3.3.0" and python_version >= "3.7"
 python-jose==3.3.0
+python-magic==0.4.27; (python_version >= "2.7" and python_full_version < "3.0.0") or (python_full_version >= "3.5.0")
 python-multipart==0.0.5
 qarnot==2.10.0; python_version >= "3.6"
 requests==2.28.1; python_version >= "3.7" and python_version < "4"

src/app/requirements.txt

@@ -6,8 +6,8 @@ asyncpg==0.25.0; python_full_version >= "3.6.0"
 atomicwrites==1.4.0; python_version >= "3.7" and python_full_version < "3.0.0" and sys_platform == "win32" or sys_platform == "win32" and python_version >= "3.7" and python_full_version >= "3.4.0"
 attrs==21.4.0; python_version >= "3.7" and python_full_version < "3.0.0" or python_full_version >= "3.5.0" and python_version >= "3.7"
 bcrypt==3.2.2; python_version >= "3.6"
-boto3==1.24.19; python_version >= "3.7"
-botocore==1.27.19; python_version >= "3.7"
+boto3==1.24.20; python_version >= "3.7"
+botocore==1.27.20; python_version >= "3.7"
 certifi==2022.6.15; python_version >= "3.7" and python_version < "4"
 cffi==1.15.0; python_version >= "3.6"
 charset-normalizer==2.1.0; python_version >= "3.7" and python_version < "4" and python_full_version >= "3.6.0"
@@ -41,6 +41,7 @@ pytest==7.1.2; python_version >= "3.7"
 python-dateutil==2.8.2; python_version >= "3.7" and python_full_version < "3.0.0" or python_full_version >= "3.6.0" and python_version >= "3.7"
 python-editor==1.0.4; python_version >= "2.7" and python_full_version < "3.0.0" or python_full_version >= "3.6.0"
 python-jose==3.3.0
+python-magic==0.4.27; (python_version >= "2.7" and python_full_version < "3.0.0") or (python_full_version >= "3.5.0")
 python-multipart==0.0.5
 qarnot==2.10.0; python_version >= "3.6"
 requests==2.28.1; python_version >= "3.7" and python_version < "4"

src/requirements-dev.txt

@@ -274,13 +274,20 @@ async def test_update_media(
     ],
 )
 @pytest.mark.asyncio
-async def test_delete_media(test_app_asyncio, init_test_db, access_idx, media_id, status_code, status_details):
+async def test_delete_media(
+    test_app_asyncio, init_test_db, monkeypatch, access_idx, media_id, status_code, status_details
+):
 
     # Create a custom access token
     auth = None
     if isinstance(access_idx, int):
         auth = await pytest.get_token(ACCESS_TABLE[access_idx]["id"], ACCESS_TABLE[access_idx]["scope"].split())
 
+    async def mock_delete_file(filename):
+        return True
+
+    monkeypatch.setattr(bucket_service, "delete_file", mock_delete_file)
+
     response = await test_app_asyncio.delete(f"/media/{media_id}/", headers=auth)
     assert response.status_code == status_code
     if isinstance(status_details, str):