Backend refactoring

.gitignore

@@ -10,4 +10,8 @@ wheels/
 .venv
 .idea
 **/.DS_Store
-backend/storage/*
+backend/storage/*
+
+# Other
+.cp-images/
+data/

README.md

@@ -1,20 +1,30 @@
 ## Тестовое задание на позицию Fullstack разработчика (Python + React)
 
 **Вводные:**
+
 1. Здесь представлен MVP проект файлообменника. Он позволяет загружать файлы, проверяет их на подозрительный контент и отправляет алерты;
 2. Репозиторий содержит в себе бэкенд и фронтенд части;
 3. В обоих частях присутствуют баги, неоптимизированный код, неудачные архитектурные решения.
 
 **Задачи:**
+
 1. Проведите рефакторинг бэкенда, не ломая бизнес-логики: предложите свое видение архитектуры и реализуйте его;
 2. (Дополнительно) На бэкенде есть возможность неочевидной оптимизации - выполните ее;
 3. (Дополнительно) Разбейте логику фронтенда на слои;
 
 **Запуск:**
-1. ```docker compose -f docker-compose.dev.yml up```
-2. ```docker exec -it backend alembic upgrade head```
 
+1. `docker compose -f docker-compose.dev.yml up`
+2. `docker exec -it backend alembic upgrade head`
+
+**Открыть фронт:** `http://localhost:3000/test`
+
+**Открыть бэк:** `http://localhost:8000/docs`
+
+## Решения при рефакторинге
+
+**Celery оставлен намеренно.** Текущие проверки просты, и технически можно было бы обойтись FastAPI `BackgroundTasks`. Однако если предположить, что это эмуляция реального сканирования (ClamAV, VirusTotal), то Celery оправдан — задачи могут занимать секунды, и `BackgroundTasks` не даст retry-логики и независимого масштабирования воркеров
 
-**Открыть фронт:** ```http://localhost:3000/test``` 
+**Ограничение архитектуры: Celery + async.** Celery не поддерживает `async def` таски нативно, поэтому используется `run_in_worker_loop` с ручным управлением event loop. Это хрупкое решение — глобальный loop может закрыться неожиданно при перезапуске воркера. В продакшне стоит рассмотреть переход на `gevent`-воркеры: это позволит убрать бойлерплейт и объявлять таски напрямую как `async def`. Не реализовано, так как выходит за рамки рефакторинга
 
-**Открыть бэк:** ```http://localhost:8000/docs```
+**Статусные поля без Enum.** `processing_status`, `scan_status`, `level` хранятся как строки без ограничений на уровне БД или Python. В идеале — `Enum`, чтобы исключить опечатки и гарантировать консистентность значений во всём коде. Не реализовано, так как требует миграции схемы БД, выходит за рамки рефакторинга

backend/migrations/env.py

@@ -4,7 +4,7 @@
 from sqlalchemy.engine import Connection
 from sqlalchemy.ext.asyncio import async_engine_from_config
 from alembic import context
-from src.service import DB_URL
+from src.config import DB_URL
 from src.models import Base
 import src.models
 

backend/src/app.py

@@ -1,11 +1,18 @@
-from fastapi import FastAPI, HTTPException
-from fastapi import File, Form, UploadFile
+from celery import chain
+from fastapi import FastAPI, File, Form, UploadFile
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.responses import FileResponse
-from starlette import status
 from src.schemas import AlertItem, FileItem, FileUpdate
-from src.service import create_file, delete_file, get_file, list_alerts, list_files, update_file, STORAGE_DIR
-from src.tasks import scan_file_for_threats
+from src.service import (
+    create_file,
+    delete_file,
+    get_file,
+    get_file_for_download,
+    list_alerts,
+    list_files,
+    update_file,
+)
+from src.tasks import extract_file_metadata, scan_file_for_threats, send_file_alert
 
 app = FastAPI()
 app.add_middleware(
@@ -36,7 +43,11 @@ async def create_file_view(
     file: UploadFile = File(...),
 ):
     file_item = await create_file(title=title, upload_file=file)
-    scan_file_for_threats.delay(file_item.id)
+    chain(
+        scan_file_for_threats.si(file_item.id),
+        extract_file_metadata.si(file_item.id),
+        send_file_alert.si(file_item.id),
+    ).delay()
     return file_item
 
 
@@ -54,11 +65,8 @@ async def update_file_view(
 
 
 @app.get("/files/{file_id}/download")
-async def download_file(file_id: str):
-    file_item = await get_file(file_id)
-    stored_path = STORAGE_DIR / file_item.stored_name
-    if not stored_path.exists():
-        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Stored file not found")
+async def download_file_view(file_id: str):
+    file_item, stored_path = await get_file_for_download(file_id)
     return FileResponse(
         path=stored_path,
         media_type=file_item.mime_type,

backend/src/config.py

@@ -0,0 +1,19 @@
+import os
+from pathlib import Path
+
+from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+
+BASE_DIR = Path(__file__).resolve().parent.parent
+STORAGE_DIR = BASE_DIR / "storage" / "files"
+STORAGE_DIR.mkdir(parents=True, exist_ok=True)
+
+REDIS_URL = os.environ.get("REDIS_URL", "redis://backend-redis:6379/0")
+
+DB_URL = (
+    f"postgresql+asyncpg://{os.environ.get('POSTGRES_USER')}:"
+    f"{os.environ.get('POSTGRES_PASSWORD')}@{os.environ.get('POSTGRES_HOST')}:"
+    f"{os.environ.get('PGPORT')}/{os.environ.get('POSTGRES_DB')}"
+)
+
+engine = create_async_engine(DB_URL)
+async_session_maker = async_sessionmaker(engine, expire_on_commit=False)

backend/src/models.py

@@ -1,6 +1,6 @@
 from datetime import datetime
 
-from sqlalchemy import Boolean, DateTime, ForeignKey, Integer, JSON, String, func
+from sqlalchemy import JSON, Boolean, DateTime, ForeignKey, Integer, String, func
 from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column
 
 
@@ -17,11 +17,15 @@ class StoredFile(Base):
     stored_name: Mapped[str] = mapped_column(String(255), nullable=False, unique=True)
     mime_type: Mapped[str] = mapped_column(String(255), nullable=False)
     size: Mapped[int] = mapped_column(Integer, nullable=False)
-    processing_status: Mapped[str] = mapped_column(String(50), nullable=False, default="uploaded")
+    processing_status: Mapped[str] = mapped_column(
+        String(50), nullable=False, default="uploaded"
+    )
     scan_status: Mapped[str | None] = mapped_column(String(50), nullable=True)
     scan_details: Mapped[str | None] = mapped_column(String(500), nullable=True)
     metadata_json: Mapped[dict | None] = mapped_column(JSON, nullable=True)
-    requires_attention: Mapped[bool] = mapped_column(Boolean, nullable=False, default=False)
+    requires_attention: Mapped[bool] = mapped_column(
+        Boolean, nullable=False, default=False
+    )
     created_at: Mapped[datetime] = mapped_column(
         DateTime(timezone=True),
         server_default=func.now(),
@@ -39,7 +43,9 @@ class Alert(Base):
     __tablename__ = "alerts"
 
     id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True)
-    file_id: Mapped[str] = mapped_column(String(36), ForeignKey("files.id"), nullable=False)
+    file_id: Mapped[str] = mapped_column(
+        String(36), ForeignKey("files.id"), nullable=False
+    )
     level: Mapped[str] = mapped_column(String(50), nullable=False)
     message: Mapped[str] = mapped_column(String(500), nullable=False)
     created_at: Mapped[datetime] = mapped_column(

backend/src/service.py

@@ -1,30 +1,18 @@
 import mimetypes
-import os
 from pathlib import Path
 from uuid import uuid4
 
 from fastapi import HTTPException, UploadFile, status
 from sqlalchemy import select
-from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker
-
+from src.config import STORAGE_DIR, async_session_maker
 from src.models import Alert, StoredFile
 
 
-BASE_DIR = Path(__file__).resolve().parent.parent
-STORAGE_DIR = BASE_DIR / "storage" / "files"
-STORAGE_DIR.mkdir(parents=True, exist_ok=True)
-DB_URL = (
-    f"postgresql+asyncpg://{os.environ.get('POSTGRES_USER')}:"
-    f"{os.environ.get('POSTGRES_PASSWORD')}@{os.environ.get('POSTGRES_HOST')}:"
-    f"{os.environ.get('PGPORT')}/{os.environ.get('POSTGRES_DB')}"
-)
-engine = create_async_engine(DB_URL)
-async_session_maker = async_sessionmaker(engine, expire_on_commit=False)
-
-
 async def list_files() -> list[StoredFile]:
     async with async_session_maker() as session:
-        result = await session.execute(select(StoredFile).order_by(StoredFile.created_at.desc()))
+        result = await session.execute(
+            select(StoredFile).order_by(StoredFile.created_at.desc())
+        )
         return list(result.scalars().all())
 
 
@@ -38,14 +26,18 @@ async def get_file(file_id: str) -> StoredFile:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
-            raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found")
+            raise HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="File not found"
+            )
         return file_item
 
 
 async def create_file(title: str, upload_file: UploadFile) -> StoredFile:
     content = await upload_file.read()
     if not content:
-        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="File is empty")
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST, detail="File is empty"
+        )
 
     file_id = str(uuid4())
     suffix = Path(upload_file.filename or "").suffix
@@ -58,7 +50,9 @@ async def create_file(title: str, upload_file: UploadFile) -> StoredFile:
         title=title,
         original_name=upload_file.filename or stored_name,
         stored_name=stored_name,
-        mime_type=upload_file.content_type or mimetypes.guess_type(stored_name)[0] or "application/octet-stream",
+        mime_type=upload_file.content_type
+        or mimetypes.guess_type(stored_name)[0]
+        or "application/octet-stream",
         size=len(content),
         processing_status="uploaded",
     )
@@ -73,37 +67,34 @@ async def update_file(file_id: str, title: str) -> StoredFile:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
-            raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found")
+            raise HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="File not found"
+            )
         file_item.title = title
         await session.commit()
         await session.refresh(file_item)
         return file_item
 
 
+async def get_file_for_download(file_id: str) -> tuple[StoredFile, Path]:
+    file_item = await get_file(file_id)
+    stored_path = STORAGE_DIR / file_item.stored_name
+    if not stored_path.exists():
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND, detail="Stored file not found"
+        )
+    return file_item, stored_path
+
+
 async def delete_file(file_id: str) -> None:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
-            raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found")
+            raise HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="File not found"
+            )
         stored_path = STORAGE_DIR / file_item.stored_name
         if stored_path.exists():
             stored_path.unlink()
         await session.delete(file_item)
         await session.commit()
-
-
-async def get_file_path(file_id: str) -> tuple[StoredFile, Path]:
-    file_item = await get_file(file_id)
-    stored_path = STORAGE_DIR / file_item.stored_name
-    if not stored_path.exists():
-        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Stored file not found")
-    return file_item, stored_path
-
-
-async def create_alert(file_id: str, level: str, message: str) -> Alert:
-    alert = Alert(file_id=file_id, level=level, message=message)
-    async with async_session_maker() as session:
-        session.add(alert)
-        await session.commit()
-        await session.refresh(alert)
-        return alert

backend/src/tasks.py

@@ -1,12 +1,10 @@
 import asyncio
-import os
 from pathlib import Path
+
 from celery import Celery
-from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker
+from src.config import REDIS_URL, STORAGE_DIR, async_session_maker
 from src.models import Alert, StoredFile
-from src.service import STORAGE_DIR, DB_URL
 
-REDIS_URL = os.environ.get("REDIS_URL", "redis://backend-redis:6379/0")
 _worker_loop: asyncio.AbstractEventLoop | None = None
 
 
@@ -19,41 +17,47 @@ def run_in_worker_loop(coroutine):
 
 
 celery_app = Celery("file_tasks", broker=REDIS_URL, backend=REDIS_URL)
-engine = create_async_engine(DB_URL)
-async_session_maker = async_sessionmaker(engine, expire_on_commit=False)
+
+MAX_FILE_SIZE = 10 * 1024 * 1024
+SUSPICIOUS_EXTENSIONS = {".exe", ".bat", ".cmd", ".sh", ".js"}
 
 
 async def _scan_file_for_threats(file_id: str) -> None:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
+            # TODO: should log a warning or raise so Celery marks the task as FAILED
             return
 
         file_item.processing_status = "processing"
+        await session.commit()
+
         reasons: list[str] = []
         extension = Path(file_item.original_name).suffix.lower()
 
-        if extension in {".exe", ".bat", ".cmd", ".sh", ".js"}:
+        if extension in SUSPICIOUS_EXTENSIONS:
             reasons.append(f"suspicious extension {extension}")
 
-        if file_item.size > 10 * 1024 * 1024:
-            reasons.append("file is larger than 10 MB")
+        if file_item.size > MAX_FILE_SIZE:
+            reasons.append(f"file is larger than {MAX_FILE_SIZE // (1024 * 1024)} MB")
 
-        if extension == ".pdf" and file_item.mime_type not in {"application/pdf", "application/octet-stream"}:
+        if extension == ".pdf" and file_item.mime_type not in {
+            "application/pdf",
+            "application/octet-stream",
+        }:
             reasons.append("pdf extension does not match mime type")
 
         file_item.scan_status = "suspicious" if reasons else "clean"
         file_item.scan_details = ", ".join(reasons) if reasons else "no threats found"
         file_item.requires_attention = bool(reasons)
         await session.commit()
 
-    extract_file_metadata.delay(file_id)
-
 
 async def _extract_file_metadata(file_id: str) -> None:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
+            # TODO: should log a warning or raise so Celery marks the task as FAILED
             return
 
         stored_path = STORAGE_DIR / file_item.stored_name
@@ -62,7 +66,6 @@ async def _extract_file_metadata(file_id: str) -> None:
             file_item.scan_status = file_item.scan_status or "failed"
             file_item.scan_details = "stored file not found during metadata extraction"
             await session.commit()
-            send_file_alert.delay(file_id)
             return
 
         metadata = {
@@ -83,25 +86,25 @@ async def _extract_file_metadata(file_id: str) -> None:
         file_item.processing_status = "processed"
         await session.commit()
 
-    send_file_alert.delay(file_id)
-
 
 async def _send_file_alert(file_id: str) -> None:
     async with async_session_maker() as session:
         file_item = await session.get(StoredFile, file_id)
         if not file_item:
+            # TODO: should log a warning or raise so Celery marks the task as FAILED
             return
 
         if file_item.processing_status == "failed":
-            alert = Alert(file_id=file_id, level="critical", message="File processing failed")
+            level, message = "critical", "File processing failed"
         elif file_item.requires_attention:
-            alert = Alert(
-                file_id=file_id,
-                level="warning",
-                message=f"File requires attention: {file_item.scan_details}",
+            level, message = (
+                "warning",
+                f"File requires attention: {file_item.scan_details}",
             )
         else:
-            alert = Alert(file_id=file_id, level="info", message="File processed successfully")
+            level, message = "info", "File processed successfully"
+
+        alert = Alert(file_id=file_id, level=level, message=message)
 
         session.add(alert)
         await session.commit()