From 8c3e23caa0c024b375e1b7121a29ce39f43711a3 Mon Sep 17 00:00:00 2001 From: Karolis Strazdas Date: Fri, 29 Aug 2025 23:31:50 +0300 Subject: [PATCH] refactor: add storage backend factory and flags --- api/v1/routes/books.py | 19 +++++----- services/book_service.py | 30 +++------------- services/storage/__init__.py | 49 ++++++++++++++++++++++++++ services/storage/filesystem_storage.py | 4 +++ services/storage/minio_storage.py | 7 +++- services/storage/storage_backend.py | 6 ++++ 6 files changed, 79 insertions(+), 36 deletions(-) diff --git a/api/v1/routes/books.py b/api/v1/routes/books.py index b70cbc6..37cf61b 100644 --- a/api/v1/routes/books.py +++ b/api/v1/routes/books.py @@ -25,7 +25,6 @@ from core.config import settings from models.user import User from services.book_service import BookService, get_book_service -from services.storage.filesystem_storage import FileSystemStorage from services.storage.storage_backend import StorageFileType router = APIRouter() @@ -120,7 +119,13 @@ async def list_books( Lists books with pagination, optional search, filtering and sorting. """ books, total = await book_service.get_books( - user.id, skip, limit, search, tags, sort_by, sort_order, + user.id, + skip, + limit, + search, + tags, + sort_by, + sort_order, ) items = [construct_book_display(book, request) for book in books] return PaginatedBookResponse(items=items, total=total) @@ -229,7 +234,7 @@ async def get_book_cover( ) if cover_path and cover_path.exists(): - if not isinstance(storage_backend, FileSystemStorage): + if not storage_backend.is_local: background_tasks.add_task(cover_path.unlink) return FileResponse(cover_path, background=background_tasks) @@ -257,11 +262,7 @@ async def download_book_file( """ book = await book_service.get_book_by_id(book_id, user.id) - if ( - not book.file_path - or not book.file_hash - or not book.stored_filename - ): + if not book.file_path or not book.file_hash or not book.stored_filename: raise HTTPException(status_code=404, detail="File not found.") storage_backend = await book_service.get_storage_backend(user) @@ -276,7 +277,7 @@ async def download_book_file( if not book_path or not book_path.exists(): raise HTTPException(status_code=404, detail="File not found.") - if not isinstance(storage_backend, FileSystemStorage): + if not storage_backend.is_local: background_tasks.add_task(book_path.unlink) return FileResponse( diff --git a/services/book_service.py b/services/book_service.py index baafad0..3071620 100644 --- a/services/book_service.py +++ b/services/book_service.py @@ -17,9 +17,8 @@ from parsers.base_parser import BookParser from parsers.epub_parser import EpubParser from parsers.pdf_parser import PdfParser +from services.storage import create_storage_backend from services.storage.exceptions import StorageBackendError -from services.storage.filesystem_storage import FileSystemStorage -from services.storage.minio_storage import MinIOStorage from services.storage.storage_backend import StorageBackend, StorageFileType PARSER_MAPPING = { @@ -27,11 +26,6 @@ "PDF": PdfParser, } -STORAGE_BACKENDS = { - "FILE_SYSTEM": FileSystemStorage, - "MINIO": MinIOStorage, -} - class BookService: def __init__(self, db: AsyncSession): @@ -252,7 +246,7 @@ async def _store_book_impl( try: storage_backend = await self.get_storage_backend(user) - except (StorageBackendError, KeyError) as error: + except StorageBackendError as error: logger.exception("Error getting storage backend.") await self.update_book_status( @@ -396,7 +390,7 @@ async def delete_book_by_id(self, user: User, book_id: str) -> int: try: storage_backend = await self.get_storage_backend(user) - except (StorageBackendError, KeyError): + except StorageBackendError: logger.exception("Error getting storage backend.") return 0 @@ -439,23 +433,7 @@ async def get_storage_backend(self, user: User | str | None) -> StorageBackend: user_id = user.id storage = await get_default_storage(self.db, user_id) - if not storage: - return STORAGE_BACKENDS["FILE_SYSTEM"]() - - match storage.storage_type: - case "FILE_SYSTEM": - return STORAGE_BACKENDS["FILE_SYSTEM"]() - case "MINIO": - config = storage.config - return STORAGE_BACKENDS["MINIO"]( - access_key=config["access_key"], - secret_key=config["secret_key"], - endpoint=config["endpoint"], - bucket_name=config["bucket_name"], - secure=config.get("secure", False), - ) - case _: - raise StorageBackendError(StorageBackendError.NOT_FOUND) + return create_storage_backend(storage) def get_book_service( diff --git a/services/storage/__init__.py b/services/storage/__init__.py index e69de29..a537ee7 100644 --- a/services/storage/__init__.py +++ b/services/storage/__init__.py @@ -0,0 +1,49 @@ +"""Utilities for resolving storage backends.""" + +from models.storage import Storage +from services.storage.exceptions import StorageBackendError +from services.storage.filesystem_storage import FileSystemStorage +from services.storage.minio_storage import MinIOStorage +from services.storage.storage_backend import StorageBackend + +STORAGE_BACKENDS: dict[str, type[StorageBackend]] = { + "FILE_SYSTEM": FileSystemStorage, + "MINIO": MinIOStorage, +} + + +def create_storage_backend(storage: Storage | None) -> StorageBackend: + """Create a :class:`StorageBackend` from a storage model instance. + + Args: + storage: The storage configuration from the database. ``None`` + results in the default local file system storage. + + Raises: + StorageBackendError: If the storage type is unknown or misconfigured. + """ + + if storage is None: + return FileSystemStorage() + + backend_cls = STORAGE_BACKENDS.get(storage.storage_type) + + if backend_cls is None: + raise StorageBackendError(StorageBackendError.NOT_FOUND) + + if backend_cls is FileSystemStorage: + return backend_cls() + + # MINIO or other backends requiring config + config = storage.config or {} + + try: + return MinIOStorage( + bucket_name=config["bucket_name"], + endpoint=config["endpoint"], + access_key=config["access_key"], + secret_key=config["secret_key"], + secure=config.get("secure", False), + ) + except KeyError as exc: # pragma: no cover - configuration errors + raise StorageBackendError(StorageBackendError.NOT_CONFIGURED) from exc diff --git a/services/storage/filesystem_storage.py b/services/storage/filesystem_storage.py index 7f6a71d..4de2f15 100644 --- a/services/storage/filesystem_storage.py +++ b/services/storage/filesystem_storage.py @@ -7,6 +7,10 @@ class FileSystemStorage(StorageBackend): + @property + def is_local(self) -> bool: + return True + def get_prepared_book_dir(self, user: User, book_dir: str) -> Path: book_path = settings.BOOK_FILES_DIR / str(user.id) / book_dir book_path.mkdir(parents=True, exist_ok=True) diff --git a/services/storage/minio_storage.py b/services/storage/minio_storage.py index 1ac625f..517b240 100644 --- a/services/storage/minio_storage.py +++ b/services/storage/minio_storage.py @@ -28,6 +28,10 @@ def __init__( secure=secure, ) + @property + def is_local(self) -> bool: + return False + def get_prepared_book_dir(self, user: User, book_dir: str) -> Path: return Path(f"books/{user.id}/{book_dir}") @@ -39,7 +43,8 @@ def get_file( filetype: StorageFileType, ) -> Path | None: object_name = self._get_object_name(user, book_dir, filename, filetype) - local_path = settings.TEMP_FILES_DIR / filename + local_path = settings.TEMP_FILES_DIR / str(user.id) / book_dir / filename + local_path.parent.mkdir(parents=True, exist_ok=True) try: self.client.fget_object(self.bucket_name, object_name, str(local_path)) diff --git a/services/storage/storage_backend.py b/services/storage/storage_backend.py index 207c8f6..fce9941 100644 --- a/services/storage/storage_backend.py +++ b/services/storage/storage_backend.py @@ -11,6 +11,12 @@ class StorageFileType(Enum): class StorageBackend(ABC): + @property + @abstractmethod + def is_local(self) -> bool: + """Indicates whether files are stored locally on the server.""" + pass + @abstractmethod def get_file( self,