diff --git a/.gitignore b/.gitignore index e3870959..f03fda14 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,8 @@ wheels/ .venv .idea **/.DS_Store -backend/storage/* \ No newline at end of file +backend/storage/* + +# Other +.cp-images/ +data/ diff --git a/README.md b/README.md index 00da8f4c..925be2bf 100644 --- a/README.md +++ b/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`, чтобы исключить опечатки и гарантировать консистентность значений во всём коде. Не реализовано, так как требует миграции схемы БД, выходит за рамки рефакторинга diff --git a/backend/migrations/env.py b/backend/migrations/env.py index e9e9f01b..d086802c 100644 --- a/backend/migrations/env.py +++ b/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 diff --git a/backend/src/app.py b/backend/src/app.py index bec89a5f..aa52d23e 100644 --- a/backend/src/app.py +++ b/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, diff --git a/backend/src/config.py b/backend/src/config.py new file mode 100644 index 00000000..16c49237 --- /dev/null +++ b/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) diff --git a/backend/src/models.py b/backend/src/models.py index ad5e515b..7f6a5beb 100644 --- a/backend/src/models.py +++ b/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( diff --git a/backend/src/service.py b/backend/src/service.py index e707fdc7..dcc39572 100644 --- a/backend/src/service.py +++ b/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 diff --git a/backend/src/tasks.py b/backend/src/tasks.py index 4583aded..1ebe1d9c 100644 --- a/backend/src/tasks.py +++ b/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,27 +17,34 @@ 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" @@ -47,13 +52,12 @@ async def _scan_file_for_threats(file_id: str) -> None: 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() diff --git a/frontend/.env.production b/frontend/.env.production new file mode 100644 index 00000000..e69de29b