feat: check for duplicate files when importing books

Incoming files are matched against what is already stored, keyed on the
KOReader hash and the file size. Bulk uploads skip and report them, deliberate
creates are refused with a 409, the consume directory parks them aside, and
allow_duplicates overrides all three.

Also: books whose metadata generates a path another book already owns are moved
aside, so a forced copy cannot overwrite the original's files.
This commit is contained in:
2026-08-13 17:23:48 -04:00
parent 968166c1fd
commit d78b21c27f
14 changed files with 1422 additions and 67 deletions
+7 -1
View File
@@ -40,7 +40,13 @@ pytest_plugins = [
@pytest.fixture(autouse=True)
def _patch_settings(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(settings, "book_cover_path", f"{tmp_path}/covers")
# PIL will not create the directory it is asked to save into, so anything that
# imports a file carrying a cover needs it to exist first.
covers = tmp_path / "covers"
covers.mkdir()
monkeypatch.setattr(settings, "book_cover_path", str(covers))
monkeypatch.setattr(settings, "duplicate_path", str(tmp_path / "duplicates"))
@pytest.fixture(name="engine")
+169 -3
View File
@@ -366,7 +366,8 @@ async def test_create_multiple_books_from_directory(
assert response.status_code == 201
data = response.json()
assert len(data.get("items") or data.get("data")) >= 1
assert len(data["created"]) == 2
assert data["skipped"] == []
async def test_create_books_from_parent_directory_keeps_embedded_title(
@@ -396,7 +397,7 @@ async def test_create_books_from_parent_directory_keeps_embedded_title(
assert response.status_code == 201
books = response.json()["items"]
books = response.json()["created"]
assert len(books) == 1
assert books[0]["title"] == "Metamorphosis"
@@ -419,11 +420,176 @@ async def test_create_books_groups_formats_within_one_folder(
assert response.status_code == 201
books = response.json()["items"]
books = response.json()["created"]
assert len(books) == 1
assert len(books[0]["files"]) == 2
class TestDuplicateHandling:
"""A file the library already holds must not be stored a second time."""
epub_path = Path("tests/data_files/Metamorphosis - Franz Kafka.epub")
def upload(self, name: str | None = None) -> list[tuple[str, tuple]]:
return [
(
"files",
(
name or self.epub_path.name,
self.epub_path.read_bytes(),
"application/epub+zip",
),
)
]
async def test_bulk_upload_reports_skipped_files(
self, authenticated_client: AsyncClient
) -> None:
"""Re-dropping a folder must import what is new and name what was not."""
first = await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
assert first.status_code == 201
created = first.json()["created"][0]
second = await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
assert second.status_code == 201
result = second.json()
assert result["created"] == []
assert len(result["skipped"]) == 1
skipped = result["skipped"][0]
assert skipped["filename"] == self.epub_path.name
assert skipped["book_id"] == created["id"]
assert skipped["book_title"] == created["title"]
async def test_bulk_upload_can_be_forced(
self, authenticated_client: AsyncClient
) -> None:
await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
response = await authenticated_client.post(
"/books/fromFiles?library_id=1&allow_duplicates=true", files=self.upload()
)
assert response.status_code == 201
assert len(response.json()["created"]) == 1
assert response.json()["skipped"] == []
async def test_single_book_create_conflicts(
self, authenticated_client: AsyncClient
) -> None:
"""Naming files deliberately earns a refusal rather than a silent drop."""
await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
response = await authenticated_client.post(
"/books?library_id=1", files=self.upload(), data={"library_id": 1}
)
assert response.status_code == 409
assert response.json()["extra"][0]["filename"] == self.epub_path.name
forced = await authenticated_client.post(
"/books?library_id=1&allow_duplicates=true",
files=self.upload(),
data={"library_id": 1},
)
assert forced.status_code == 201
async def test_adding_another_books_file_conflicts(
self, authenticated_client: AsyncClient
) -> None:
created = await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
book_id = created.json()["created"][0]["id"]
other = await authenticated_client.post(
"/books?library_id=1",
files=[
(
"files",
(
"war.epub",
Path("tests/data_files/The Art of War - Sun Tzu.epub").read_bytes(),
"application/epub+zip",
),
)
],
data={"library_id": 1},
)
other_id = other.json()["id"]
response = await authenticated_client.post(
f"/books/{book_id}/files",
files=[
(
"files",
(
"war.epub",
Path("tests/data_files/The Art of War - Sun Tzu.epub").read_bytes(),
"application/epub+zip",
),
)
],
)
assert response.status_code == 409
assert response.json()["extra"][0]["book_id"] == other_id
async def test_resending_a_books_own_file_changes_nothing(
self, authenticated_client: AsyncClient
) -> None:
created = await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
book_id = created.json()["created"][0]["id"]
response = await authenticated_client.post(
f"/books/{book_id}/files", files=self.upload()
)
assert response.status_code == 201
assert len(response.json()["files"]) == 1
async def test_duplicates_can_be_checked_before_uploading(
self, authenticated_client: AsyncClient
) -> None:
"""The pre-flight check answers from hashes alone, with no file sent."""
created = await authenticated_client.post(
"/books/fromFiles?library_id=1", files=self.upload()
)
book = created.json()["created"][0]
stored = book["files"][0]
response = await authenticated_client.post(
"/books/duplicates?library_id=1",
json=[
{
"hash": stored["hash"],
"size": stored["size"],
"filename": "local-copy.epub",
},
{"hash": stored["hash"], "size": stored["size"] + 1},
{"hash": "0" * 32, "size": 1234},
],
)
assert response.status_code == 200
matches = response.json()
assert len(matches) == 1
assert matches[0]["filename"] == "local-copy.epub"
assert matches[0]["book_id"] == book["id"]
# 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
@@ -6,11 +6,29 @@ from pathlib import Path
import pytest
import aiofiles.os as aios
from litestar.datastructures import UploadFile
from sqlalchemy.ext.asyncio import AsyncSession
from chitai.schemas import BookCreate
from chitai.config import DuplicateScope, settings
from chitai.schemas import BookCreate, BooksCreateFromFiles
from chitai.services import BookService
from chitai.services.book import DuplicateFilesError
from chitai.database import models as m
DATA_FILES = Path("tests/data_files")
EPUB = DATA_FILES / "Metamorphosis - Franz Kafka.epub"
OTHER_EPUB = DATA_FILES / "The Art of War - Sun Tzu.epub"
PDF = DATA_FILES / "Calculus Made Easy - Silvanus Thompson.pdf"
def upload(path: Path, name: str | None = None) -> UploadFile:
"""An uploaded file carrying the bytes of one of the test fixtures."""
return UploadFile(
content_type="application/pdf" if path.suffix == ".pdf" else "application/epub+zip",
filename=name or path.name,
file_data=path.read_bytes(),
)
@pytest.mark.asyncio
class TestBookServiceCRUD:
@@ -202,3 +220,378 @@ class TestBookServiceCRUD:
for name, payload in contents.items():
entry = next(n for n in names if Path(n).name == name)
assert archive.read(entry) == payload
@pytest.mark.asyncio
class TestBookServiceDuplicates:
"""Files that are already stored must not be stored again."""
async def test_duplicate_upload_is_skipped_and_reported(
self, books_service: BookService, test_library: m.Library
) -> None:
"""The second import of a file creates nothing and names where it already is."""
first = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
assert len(first.books) == 1
assert first.duplicates == []
second = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
assert second.books == []
assert len(second.duplicates) == 1
duplicate = second.duplicates[0]
assert duplicate.filename == EPUB.name
assert duplicate.book_id == first.books[0].id
assert duplicate.book_title == first.books[0].title
async def test_new_format_beside_a_duplicate_is_still_imported(
self, books_service: BookService, test_library: m.Library
) -> None:
"""A folder is not all-or-nothing: the file that is new must still land."""
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB, "Metamorphosis/book.epub")]),
test_library,
)
result = await books_service.create_many_from_files(
BooksCreateFromFiles(
files=[
upload(EPUB, "Metamorphosis/book.epub"),
upload(PDF, "Metamorphosis/book.pdf"),
]
),
test_library,
)
assert len(result.books) == 1
assert [d.filename for d in result.duplicates] == ["Metamorphosis/book.epub"]
book = await books_service.get(result.books[0].id)
assert [file.path for file in book.files] == ["book.pdf"]
async def test_duplicate_within_one_upload_is_caught(
self, books_service: BookService, test_library: m.Library
) -> None:
"""The same bytes twice in one request has no row to match against yet."""
result = await books_service.create_many_from_files(
BooksCreateFromFiles(
files=[upload(EPUB, "first.epub"), upload(EPUB, "second.epub")]
),
test_library,
)
assert len(result.books) == 1
assert len(result.duplicates) == 1
assert result.duplicates[0].filename == "second.epub"
# Nothing in the database holds it yet, so there is no book to point at.
assert result.duplicates[0].book_id is None
async def test_allow_duplicates_stores_the_file_anyway(
self, books_service: BookService, test_library: m.Library
) -> None:
"""The escape hatch has to work: the hash is not proof of identity."""
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]),
test_library,
allow_duplicates=True,
)
assert len(result.books) == 1
assert result.duplicates == []
async def test_a_different_file_is_not_a_duplicate(
self, books_service: BookService, test_library: m.Library
) -> None:
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB), upload(OTHER_EPUB)]), test_library
)
assert len(result.books) == 2
assert result.duplicates == []
async def test_matching_hash_with_a_different_size_is_not_a_duplicate(
self, books_service: BookService, test_library: m.Library
) -> None:
"""The hash samples 12 KiB, so the size is what makes a match trustworthy."""
created = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
stored = (await books_service.get(created.books[0].id)).files[0]
matches = await books_service.find_duplicate_files(
[(stored.hash, stored.size), (stored.hash, stored.size + 1)], test_library
)
assert (stored.hash, stored.size) in matches
assert (stored.hash, stored.size + 1) not in matches
async def test_create_book_refuses_and_writes_nothing(
self, books_service: BookService, test_library: m.Library
) -> None:
"""A single-book create names its files, so it is refused rather than trimmed."""
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
before = sorted(Path(test_library.root_path).rglob("*"))
with pytest.raises(DuplicateFilesError) as excinfo:
await books_service.create_book(
BookCreate(
library_id=test_library.id,
title="Metamorphosis",
authors=["Franz Kafka"],
files=[upload(EPUB)],
).model_dump(),
test_library,
)
assert len(excinfo.value.duplicates) == 1
assert sorted(Path(test_library.root_path).rglob("*")) == before
async def test_scope_decides_whether_libraries_share(
self,
books_service: BookService,
test_library: m.Library,
session: AsyncSession,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A second library is a separate collection by default, and not under `global`."""
other = m.Library(
name="Second Library",
slug="second-library",
root_path=str(tmp_path / "second"),
path_template=test_library.path_template,
read_only=False,
)
session.add(other)
await session.commit()
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), other
)
assert len(result.books) == 1
assert result.duplicates == []
monkeypatch.setattr(settings, "duplicate_scope", DuplicateScope.GLOBAL)
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), other
)
assert result.books == []
assert len(result.duplicates) == 1
async def test_scope_off_disables_detection(
self,
books_service: BookService,
test_library: m.Library,
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(settings, "duplicate_scope", DuplicateScope.OFF)
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
result = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB), upload(EPUB, "copy.epub")]),
test_library,
)
assert len(result.books) == 2
assert result.duplicates == []
async def test_re_adding_a_file_to_its_own_book_does_nothing(
self, books_service: BookService, test_library: m.Library
) -> None:
"""Asking for a state that already holds is not a conflict."""
created = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
book_id = created.books[0].id
await books_service.add_files(book_id, [upload(EPUB)], test_library)
book = await books_service.get(book_id)
assert len(book.files) == 1
async def test_adding_another_books_file_is_refused(
self, books_service: BookService, test_library: m.Library
) -> None:
created = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB), upload(OTHER_EPUB)]), test_library
)
first, second = created.books
with pytest.raises(DuplicateFilesError) as excinfo:
await books_service.add_files(first.id, [upload(OTHER_EPUB)], test_library)
assert excinfo.value.duplicates[0].book_id == second.id
assert len((await books_service.get(first.id)).files) == 1
async def test_consume_duplicate_is_moved_aside(
self,
books_service: BookService,
test_library: m.Library,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The watcher cannot ask, so a refused file is parked rather than dropped."""
quarantine = tmp_path / "duplicates"
monkeypatch.setattr(settings, "duplicate_path", str(quarantine))
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
consume = tmp_path / "consume"
await aios.makedirs(consume)
dropped = consume / EPUB.name
dropped.write_bytes(EPUB.read_bytes())
result = await books_service.create_many_from_existing_files(
[dropped], consume, test_library
)
assert result.books == []
assert len(result.duplicates) == 1
assert not dropped.exists()
assert (quarantine / test_library.slug / EPUB.name).is_file()
async def test_consume_imports_what_is_new(
self,
books_service: BookService,
test_library: m.Library,
tmp_path: Path,
) -> None:
"""The screening must not disturb the ordinary consume import."""
consume = tmp_path / "consume"
await aios.makedirs(consume / "Metamorphosis")
dropped = consume / "Metamorphosis" / EPUB.name
dropped.write_bytes(EPUB.read_bytes())
result = await books_service.create_many_from_existing_files(
[dropped], consume, test_library
)
assert len(result.books) == 1
assert result.duplicates == []
book = await books_service.get(result.books[0].id)
assert (Path(book.path) / book.files[0].path).is_file()
@pytest.mark.asyncio
class TestBookPathCollisions:
"""Two books must never share a directory, whatever their metadata says."""
async def test_forced_duplicate_gets_its_own_copy(
self, books_service: BookService, test_library: m.Library
) -> None:
"""`allow_duplicates` must add a book, not overwrite the one already there.
The path comes from the metadata alone, so a forced duplicate generates the
same directory and the same filename. Writing it lands on top of the original:
one file on disk, two books pointing at it, and deleting either takes both.
"""
first = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
second = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]),
test_library,
allow_duplicates=True,
)
original = await books_service.get(first.books[0].id)
forced = await books_service.get(second.books[0].id)
assert original.path != forced.path
paths = {
Path(book.path) / book.files[0].path for book in (original, forced)
}
assert len(paths) == 2
assert all(path.is_file() for path in paths)
async def test_deleting_a_forced_duplicate_keeps_the_original(
self, books_service: BookService, test_library: m.Library
) -> None:
first = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
second = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]),
test_library,
allow_duplicates=True,
)
original = await books_service.get(first.books[0].id)
kept = Path(original.path) / original.files[0].path
await books_service.delete_books(
[second.books[0].id], test_library, delete_files=True
)
assert kept.is_file()
async def test_editing_metadata_cannot_merge_into_another_book(
self, books_service: BookService, test_library: m.Library
) -> None:
"""A rename that collides must step aside rather than move in on top."""
first = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
second = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(OTHER_EPUB)]), test_library
)
original = await books_service.get(first.books[0].id)
# Renamed onto the first book's author and title.
await books_service.update_book(
second.books[0].id,
{"title": original.title, "authors": [author.name for author in original.authors]},
test_library,
)
moved = await books_service.get(second.books[0].id)
assert moved.path != original.path
assert (Path(original.path) / original.files[0].path).is_file()
assert (Path(moved.path) / moved.files[0].path).is_file()
async def test_adding_a_file_lands_in_the_books_own_directory(
self, books_service: BookService, test_library: m.Library
) -> None:
"""`add_files` must follow the book, not the template."""
await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]), test_library
)
forced = await books_service.create_many_from_files(
BooksCreateFromFiles(files=[upload(EPUB)]),
test_library,
allow_duplicates=True,
)
book_id = forced.books[0].id
await books_service.add_files(book_id, [upload(PDF)], test_library)
book = await books_service.get(book_id)
assert len(book.files) == 2
for file in book.files:
assert (Path(book.path) / file.path).is_file()