From 510306f24da666fcb14ab3ff9adf69c7eefd727e Mon Sep 17 00:00:00 2001 From: patrick Date: Wed, 12 Aug 2026 14:54:49 -0400 Subject: [PATCH] fix: keep embedded metadata and group formats on directory upload The filepath extractor was merged on the right, so a parent folder's name overrode the title inside the file. Grouping also split a book's formats when its own folder was the upload root. --- backend/src/chitai/services/book.py | 19 +++++-- .../src/chitai/services/metadata_extractor.py | 7 ++- backend/tests/integration/test_book.py | 55 +++++++++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/backend/src/chitai/services/book.py b/backend/src/chitai/services/book.py index c3af57c..e045bd8 100644 --- a/backend/src/chitai/services/book.py +++ b/backend/src/chitai/services/book.py @@ -152,15 +152,24 @@ class BookService(SQLAlchemyAsyncRepositoryService[Book]): if not data.files: raise ValueError("Must upload at least one file") - # Group book files if they are within the same nested directory - books = defaultdict(list) + # Group the files that make up a single book. + # + # `file.filename` carries the browser's webkitRelativePath, whose first + # component is whichever folder the user picked. Depth is therefore relative + # to the selection, not to the library: picking a book's own folder and + # picking the shelf above it submit the same files at different depths. + # + # Inside a subdirectory the parent directory is unambiguously the book. Sitting + # directly in the selected folder it is genuinely ambiguous — "Fluent Python.epub" + # alongside "Fluent Python.pdf" is one book in two formats, while "a.epub" + # alongside "b.epub" is two books — so the filename stem breaks the tie. + books: dict[tuple[Path, str], list[UploadFile]] = defaultdict(list) for file in data.files: filepath = Path(file.filename) - # Books within the root directory should be treated as separate books if len(filepath.parent.parts) > 1: - books[filepath.parent].append(file) + books[(filepath.parent, "")].append(file) else: - books[filepath].append(file) + books[(filepath.parent, filepath.stem)].append(file) return [ await self.create_book( diff --git a/backend/src/chitai/services/metadata_extractor.py b/backend/src/chitai/services/metadata_extractor.py index d456e31..eb6c77c 100644 --- a/backend/src/chitai/services/metadata_extractor.py +++ b/backend/src/chitai/services/metadata_extractor.py @@ -67,8 +67,11 @@ class Extractor: for file in files: metadata = FilenameExtractor.extract_metadata(file) | metadata - # Get metadata from filepath - metadata = metadata | FilepathExtractor.extract_metadata(files[0], root_path) + # Get metadata from filepath. Kept on the left so that anything the file + # itself declared outranks a guess made from its directory names — a folder + # called "Fluent Python - Luciano Ramalho" must not overwrite the title the + # EPUB already carries. + metadata = FilepathExtractor.extract_metadata(files[0], root_path) | metadata # format the title if metadata.get('title', None): diff --git a/backend/tests/integration/test_book.py b/backend/tests/integration/test_book.py index 769615a..186a9d2 100644 --- a/backend/tests/integration/test_book.py +++ b/backend/tests/integration/test_book.py @@ -369,6 +369,61 @@ async def test_create_multiple_books_from_directory( assert len(data.get("items") or data.get("data")) >= 1 +async def test_create_books_from_parent_directory_keeps_embedded_title( + authenticated_client: AsyncClient, +) -> None: + """A folder name in the upload path must not override the file's own metadata. + + The browser sends webkitRelativePath, so picking the shelf above a book's folder + submits one more path component than picking the folder itself. That extra level + used to make the directory name win over the title inside the EPUB. + """ + source = Path("tests/data_files/Metamorphosis - Franz Kafka.epub") + files = [ + ( + "files", + ( + "Shelf/Metamorphosis - Franz Kafka/Metamorphosis.epub", + source.read_bytes(), + "application/epub+zip", + ), + ) + ] + + response = await authenticated_client.post( + "/books/fromFiles?library_id=1", files=files, data={"library_id": 1} + ) + + assert response.status_code == 201 + + books = response.json()["items"] + assert len(books) == 1 + assert books[0]["title"] == "Metamorphosis" + + +async def test_create_books_groups_formats_within_one_folder( + authenticated_client: AsyncClient, +) -> None: + """Picking a book's own folder yields one book with both formats, not two books.""" + epub = Path("tests/data_files/Metamorphosis - Franz Kafka.epub").read_bytes() + pdf = Path("tests/data_files/Calculus Made Easy - Silvanus Thompson.pdf").read_bytes() + + files = [ + ("files", ("Metamorphosis/Metamorphosis.epub", epub, "application/epub+zip")), + ("files", ("Metamorphosis/Metamorphosis.pdf", pdf, "application/pdf")), + ] + + response = await authenticated_client.post( + "/books/fromFiles?library_id=1", files=files, data={"library_id": 1} + ) + + assert response.status_code == 201 + + books = response.json()["items"] + assert len(books) == 1 + assert len(books[0]["files"]) == 2 + + # NOTE: the multi-book ZIP download is covered at the service level, in # tests/unit/test_services/test_book_service.py. Driving `/books/download` through # AsyncTestClient hangs in fixture teardown: it is the only `Stream` endpoint in the