feat: merge books into one record
Files move on disk before any row moves, then everything repoints in one transaction and the folded records are deleted. Progress keeps whichever is furthest, shelves and tags union, and identifiers move only under a name the survivor lacks. Metadata is left alone unless the caller resolves it, since choosing between two titles is a judgement this endpoint cannot make. Nothing is removed from disk.
This commit is contained in:
@@ -706,6 +706,55 @@ class TestDuplicateBooks:
|
||||
response = await authenticated_client.get("/books/duplicate-books?library_id=1")
|
||||
assert len(response.json()) == 1
|
||||
|
||||
async def test_two_books_merge_into_one(
|
||||
self, authenticated_client: AsyncClient
|
||||
) -> None:
|
||||
"""The survivor keeps its id and gains the other's file; the other is gone."""
|
||||
first = await authenticated_client.post(
|
||||
"/books/fromFiles?library_id=1", files=self.upload("first.epub")
|
||||
)
|
||||
second = await authenticated_client.post(
|
||||
"/books/fromFiles?library_id=1", files=self.upload("second.epub", pad=True)
|
||||
)
|
||||
keep = first.json()["created"][0]
|
||||
fold = second.json()["created"][0]
|
||||
|
||||
response = await authenticated_client.post(
|
||||
"/books/merge?library_id=1",
|
||||
json={
|
||||
"survivor_id": keep["id"],
|
||||
"merged_ids": [fold["id"]],
|
||||
"metadata": {"title": "Metamorphosis", "edition": 2},
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
merged = response.json()
|
||||
|
||||
assert merged["id"] == keep["id"]
|
||||
assert merged["edition"] == 2
|
||||
assert len(merged["files"]) == 2
|
||||
|
||||
# The folded record is gone, and the group it formed with it.
|
||||
assert (await authenticated_client.get(f"/books/{fold['id']}")).status_code == 404
|
||||
groups = await authenticated_client.get("/books/duplicate-books?library_id=1")
|
||||
assert groups.json() == []
|
||||
|
||||
async def test_merging_an_unknown_book_is_refused(
|
||||
self, authenticated_client: AsyncClient
|
||||
) -> None:
|
||||
created = await authenticated_client.post(
|
||||
"/books/fromFiles?library_id=1", files=self.upload("first.epub")
|
||||
)
|
||||
keep = created.json()["created"][0]["id"]
|
||||
|
||||
response = await authenticated_client.post(
|
||||
"/books/merge?library_id=1",
|
||||
json={"survivor_id": keep, "merged_ids": [9999]},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
|
||||
async def test_dismissing_an_unknown_book_is_refused(
|
||||
self, authenticated_client: AsyncClient
|
||||
) -> None:
|
||||
|
||||
@@ -7,6 +7,7 @@ from pathlib import Path
|
||||
import pytest
|
||||
import aiofiles.os as aios
|
||||
from litestar.datastructures import UploadFile
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from chitai.config import DuplicateScope, settings
|
||||
@@ -961,6 +962,244 @@ class TestAuthorNames:
|
||||
assert not author.name.endswith((".epub", ".pdf", ".mobi"))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestMergeBooks:
|
||||
"""Folding several books into one, and what has to come with them."""
|
||||
|
||||
async def test_files_move_onto_the_survivor(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""The rows move and so do the bytes — `file.path` is relative to `book.path`."""
|
||||
first = await books_service.create_many_from_files(
|
||||
BooksCreateFromFiles(files=[upload(EPUB, "one.epub")]), test_library
|
||||
)
|
||||
second = await books_service.create_many_from_files(
|
||||
BooksCreateFromFiles(files=[upload(PDF, "two.pdf")]), test_library
|
||||
)
|
||||
keep, fold = first.books[0], second.books[0]
|
||||
gone = Path(fold.path)
|
||||
|
||||
merged = await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
|
||||
assert len(merged.files) == 2
|
||||
for file in merged.files:
|
||||
assert (Path(merged.path) / file.path).is_file()
|
||||
|
||||
assert not gone.exists()
|
||||
with pytest.raises(Exception):
|
||||
await books_service.get(fold.id)
|
||||
|
||||
async def test_a_filename_collision_is_given_its_own_name(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""Two books' files can share a name; one must not overwrite the other."""
|
||||
first = await books_service.create_many_from_files(
|
||||
BooksCreateFromFiles(files=[upload(EPUB, "book.epub")]), test_library
|
||||
)
|
||||
second = await books_service.create_many_from_files(
|
||||
BooksCreateFromFiles(files=[edition(EPUB, "book.epub")]), test_library
|
||||
)
|
||||
|
||||
merged = await books_service.merge_books(
|
||||
first.books[0].id, [second.books[0].id], test_library
|
||||
)
|
||||
|
||||
paths = sorted(file.path for file in merged.files)
|
||||
assert len(paths) == 2 and paths[0] != paths[1]
|
||||
for file in merged.files:
|
||||
assert (Path(merged.path) / file.path).is_file()
|
||||
|
||||
async def test_metadata_is_only_what_the_caller_resolved(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""The survivor keeps its own fields unless the caller says otherwise."""
|
||||
keep = await store_book(
|
||||
books_service, test_library, title="Building Microservices", authors=["Sam Newman"]
|
||||
)
|
||||
fold = await store_book(
|
||||
books_service,
|
||||
test_library,
|
||||
title="Building Microservices",
|
||||
authors=["Sam Newman"],
|
||||
publisher="O'Reilly",
|
||||
edition=2,
|
||||
)
|
||||
|
||||
merged = await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
assert merged.edition is None
|
||||
assert merged.publisher is None
|
||||
|
||||
other = await store_book(
|
||||
books_service, test_library, title="Monolith", authors=["Sam Newman"], edition=3
|
||||
)
|
||||
merged = await books_service.merge_books(
|
||||
keep.id, [other.id], test_library, metadata={"edition": 3}
|
||||
)
|
||||
assert merged.edition == 3
|
||||
|
||||
async def test_resolved_collections_do_not_collide_with_what_moved(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""
|
||||
Tags and identifiers move onto the survivor *and* appear in the resolution.
|
||||
|
||||
The survivor's loaded collections do not learn about a row repointed by a bulk
|
||||
statement, so reconciling them against stale state builds a second link and
|
||||
breaches the unique constraint. Scalar-only metadata never sees it.
|
||||
"""
|
||||
keep = await store_book(
|
||||
books_service, test_library, title="A", authors=["X"], tags=["Kept"]
|
||||
)
|
||||
fold = await store_book(
|
||||
books_service,
|
||||
test_library,
|
||||
title="B",
|
||||
authors=["X"],
|
||||
tags=["Moved"],
|
||||
identifiers={"asin": "B000FC0PDA"},
|
||||
)
|
||||
|
||||
merged = await books_service.merge_books(
|
||||
keep.id,
|
||||
[fold.id],
|
||||
test_library,
|
||||
metadata={
|
||||
"tags": ["Kept", "Moved"],
|
||||
"identifiers": {"asin": "B000FC0PDA"},
|
||||
"publisher": "O'Reilly",
|
||||
},
|
||||
)
|
||||
|
||||
assert sorted(tag.name for tag in merged.tags) == ["Kept", "Moved"]
|
||||
assert {i.name: i.value for i in merged.identifiers} == {"asin": "B000FC0PDA"}
|
||||
assert merged.publisher is not None and merged.publisher.name == "O'Reilly"
|
||||
|
||||
async def test_the_furthest_progress_survives(
|
||||
self,
|
||||
books_service: BookService,
|
||||
test_library: m.Library,
|
||||
test_user: m.User,
|
||||
session: AsyncSession,
|
||||
) -> None:
|
||||
"""A reader who got 60% through the PDF has not gone back to page one."""
|
||||
keep = await store_book(books_service, test_library, title="A", authors=["X"])
|
||||
fold = await store_book(books_service, test_library, title="B", authors=["X"])
|
||||
|
||||
session.add_all(
|
||||
[
|
||||
m.BookProgress(user_id=test_user.id, book_id=keep.id, percentage=0.1),
|
||||
m.BookProgress(user_id=test_user.id, book_id=fold.id, percentage=0.6),
|
||||
]
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
|
||||
rows = (
|
||||
await books_service.repository.session.execute(
|
||||
select(m.BookProgress).where(m.BookProgress.book_id == keep.id)
|
||||
)
|
||||
).scalars().all()
|
||||
|
||||
assert [row.percentage for row in rows] == [0.6]
|
||||
|
||||
async def test_shelves_and_tags_come_across_without_duplicating(
|
||||
self,
|
||||
books_service: BookService,
|
||||
test_library: m.Library,
|
||||
test_user: m.User,
|
||||
session: AsyncSession,
|
||||
) -> None:
|
||||
"""Both books on one shelf must not leave the survivor linked to it twice."""
|
||||
keep = await store_book(
|
||||
books_service, test_library, title="A", authors=["X"], tags=["Shared", "Only Keep"]
|
||||
)
|
||||
fold = await store_book(
|
||||
books_service, test_library, title="B", authors=["X"], tags=["Shared", "Only Fold"]
|
||||
)
|
||||
|
||||
shelf = m.BookList(title="Later", user_id=test_user.id, library_id=test_library.id)
|
||||
session.add(shelf)
|
||||
await session.commit()
|
||||
session.add_all(
|
||||
[
|
||||
m.BookListLink(book_id=keep.id, list_id=shelf.id, position=0),
|
||||
m.BookListLink(book_id=fold.id, list_id=shelf.id, position=0),
|
||||
]
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
merged = await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
|
||||
assert sorted(tag.name for tag in merged.tags) == ["Only Fold", "Only Keep", "Shared"]
|
||||
|
||||
links = (
|
||||
await books_service.repository.session.execute(
|
||||
select(m.BookListLink).where(m.BookListLink.book_id == keep.id)
|
||||
)
|
||||
).scalars().all()
|
||||
assert len(links) == 1
|
||||
|
||||
async def test_an_identifier_moves_only_under_a_name_the_survivor_lacks(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""`(name, book_id)` is unique, so a competing isbn-13 cannot come across."""
|
||||
keep = await store_book(
|
||||
books_service,
|
||||
test_library,
|
||||
title="A",
|
||||
authors=["X"],
|
||||
identifiers={"isbn-13": "9780486282114"},
|
||||
)
|
||||
fold = await store_book(
|
||||
books_service,
|
||||
test_library,
|
||||
title="B",
|
||||
authors=["X"],
|
||||
identifiers={"isbn-13": "9781492034025", "asin": "B000FC0PDA"},
|
||||
)
|
||||
|
||||
merged = await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
|
||||
stored = {i.name: i.value for i in merged.identifiers}
|
||||
assert stored == {"isbn-13": "9780486282114", "asin": "B000FC0PDA"}
|
||||
|
||||
async def test_dismissals_follow_the_survivor(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
"""A verdict about some third book still holds after its partner is merged."""
|
||||
keep = await store_book(books_service, test_library, title="A", authors=["X"])
|
||||
fold = await store_book(books_service, test_library, title="B", authors=["X"])
|
||||
third = await store_book(books_service, test_library, title="C", authors=["X"])
|
||||
|
||||
# One pair between the two being merged, one pointing outside the merge.
|
||||
await books_service.dismiss_duplicates(keep.id, fold.id)
|
||||
await books_service.dismiss_duplicates(fold.id, third.id)
|
||||
|
||||
await books_service.merge_books(keep.id, [fold.id], test_library)
|
||||
|
||||
pairs = await books_service._dismissed_pairs()
|
||||
assert pairs == {m.DuplicateDismissal.pair(keep.id, third.id)}
|
||||
|
||||
@pytest.mark.parametrize("merged", [[], [1]])
|
||||
async def test_a_merge_needs_two_different_books(
|
||||
self, books_service: BookService, test_library: m.Library, merged: list[int]
|
||||
) -> None:
|
||||
keep = await store_book(books_service, test_library, title="A", authors=["X"])
|
||||
ids = [keep.id] if merged else []
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
await books_service.merge_books(keep.id, ids, test_library)
|
||||
|
||||
async def test_an_unknown_book_is_refused(
|
||||
self, books_service: BookService, test_library: m.Library
|
||||
) -> None:
|
||||
keep = await store_book(books_service, test_library, title="A", authors=["X"])
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
await books_service.merge_books(keep.id, [keep.id + 10_000], test_library)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestDuplicateBookGroups:
|
||||
"""The pass over a library someone already has."""
|
||||
|
||||
Reference in New Issue
Block a user