From ba311e1bed3adbe4c5ac9e48becdeb76c0982570 Mon Sep 17 00:00:00 2001 From: codebude Date: Tue, 25 Aug 2026 07:08:45 +0200 Subject: [PATCH 01/15] Added support for multiple authors per book --- ...a3_create_author_and_book_author_tables.py | 104 +++++ ...b2c3d4e5a6_drop_author_column_from_book.py | 42 ++ backend/app/models.py | 25 +- backend/app/routers/books.py | 52 ++- backend/app/routers/hygiene.py | 107 +++-- backend/app/routers/import_.py | 4 +- backend/app/routers/profile.py | 5 +- backend/app/routers/statistics.py | 39 +- backend/app/schemas.py | 9 +- backend/app/services/authors.py | 232 +++++++++++ backend/app/services/book_import.py | 15 +- backend/app/services/data_export.py | 22 +- backend/app/services/data_import.py | 29 +- backend/app/services/search.py | 24 +- backend/app/services/tags.py | 4 + backend/app/services/user_deletion.py | 13 +- backend/tests/test_admin.py | 2 +- backend/tests/test_authors.py | 193 +++++++++ backend/tests/test_books.py | 122 ++++++ backend/tests/test_data_import.py | 7 +- backend/tests/test_hygiene.py | 56 ++- docs/api/index.md | 12 +- docs/guide/database-layout.md | 371 +++++++++++++++++- docs/guide/using-librislog/import-export.md | 15 + docs/guide/using-librislog/library.md | 2 + docs/guide/using-librislog/search.md | 2 + .../e2e/fixtures/pages/add-book-modal.page.ts | 1 + .../e2e/fixtures/pages/book-drawer.page.ts | 1 + frontend/e2e/fixtures/seed-data.ts | 8 +- frontend/e2e/fixtures/seed.api.ts | 6 +- .../e2e/specs/03-library-browsing.spec.ts | 5 +- frontend/e2e/specs/05-edit-book.spec.ts | 37 ++ frontend/e2e/specs/08-statistics.spec.ts | 20 + frontend/e2e/specs/09-data-import.spec.ts | 95 +++++ .../src/lib/components/AddBookModal.svelte | 17 +- .../src/lib/components/AddBookModal.test.ts | 11 +- frontend/src/lib/components/BookCard.svelte | 3 +- frontend/src/lib/components/BookCard.test.ts | 1 + .../lib/components/BookDetailDialog.svelte | 3 +- .../lib/components/BookDetailDialog.test.ts | 1 + frontend/src/lib/components/BookDrawer.svelte | 19 +- .../src/lib/components/BookDrawer.test.ts | 1 + .../src/lib/components/BookListItem.svelte | 3 +- .../src/lib/components/ImportSearch.svelte | 21 +- .../lib/components/RatedBooksSection.svelte | 3 +- .../lib/components/RatedBooksSection.test.ts | 1 + frontend/src/lib/components/TagInput.svelte | 109 +++-- frontend/src/lib/components/TagInput.test.ts | 60 +++ frontend/src/lib/i18n/locales/de.json | 15 +- frontend/src/lib/i18n/locales/en.json | 15 +- frontend/src/lib/i18n/locales/es.json | 9 +- frontend/src/lib/i18n/locales/fr.json | 9 +- frontend/src/lib/i18n/locales/zh.json | 9 +- frontend/src/lib/types.ts | 5 + frontend/src/lib/utils/authors.test.ts | 22 ++ frontend/src/lib/utils/authors.ts | 7 + frontend/src/routes/dashboard/+page.svelte | 3 +- frontend/src/routes/data-hygiene/+page.svelte | 3 +- frontend/src/routes/data-hygiene/page.test.ts | 1 + .../src/routes/missing-covers/+page.svelte | 5 +- frontend/src/routes/search/page.test.ts | 3 +- frontend/src/routes/timeline/+page.svelte | 3 +- 62 files changed, 1850 insertions(+), 193 deletions(-) create mode 100644 backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py create mode 100644 backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py create mode 100644 backend/app/services/authors.py create mode 100644 backend/tests/test_authors.py create mode 100644 frontend/src/lib/utils/authors.test.ts create mode 100644 frontend/src/lib/utils/authors.ts diff --git a/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py b/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py new file mode 100644 index 00000000..b72e6c72 --- /dev/null +++ b/backend/alembic/versions/c7a8d9e1f2a3_create_author_and_book_author_tables.py @@ -0,0 +1,104 @@ +"""create author and book_author tables and backfill + +Revision ID: c7a8d9e1f2a3 +Revises: e2f3a4b5c6d7 +Create Date: 2026-08-24 21:00:00 +""" + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "c7a8d9e1f2a3" +down_revision = "e2f3a4b5c6d7" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + + if not inspector.has_table("author"): + op.create_table( + "author", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("user_id", sa.Integer(), nullable=False), + sa.Column("name", sa.String(), nullable=False), + sa.Column("created_at", sa.DateTime(), nullable=False, server_default=sa.text("CURRENT_TIMESTAMP")), + sa.ForeignKeyConstraint(["user_id"], ["user.id"]), + sa.UniqueConstraint("user_id", "name", name="uq_author_user_id_name"), + sa.PrimaryKeyConstraint("id"), + ) + + existing_author_indexes = {idx["name"] for idx in inspector.get_indexes("author")} + if "ix_author_user_id" not in existing_author_indexes: + op.create_index("ix_author_user_id", "author", ["user_id"], unique=False) + if "ix_author_name" not in existing_author_indexes: + op.create_index("ix_author_name", "author", ["name"], unique=False) + + if not inspector.has_table("book_author"): + op.create_table( + "book_author", + sa.Column("book_id", sa.Integer(), nullable=False), + sa.Column("author_id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["book_id"], ["book.id"]), + sa.ForeignKeyConstraint(["author_id"], ["author.id"]), + sa.PrimaryKeyConstraint("book_id", "author_id"), + ) + + existing_book_author_indexes = {idx["name"] for idx in inspector.get_indexes("book_author")} + if "ix_book_author_book_id" not in existing_book_author_indexes: + op.create_index("ix_book_author_book_id", "book_author", ["book_id"], unique=False) + if "ix_book_author_author_id" not in existing_book_author_indexes: + op.create_index("ix_book_author_author_id", "book_author", ["author_id"], unique=False) + + # Backfill authors from the legacy book.author column. Each legacy value is + # treated as a single author name so names like "Asimov, Isaac" are preserved. + rows = bind.execute( + sa.text("SELECT id, user_id, author FROM book WHERE author IS NOT NULL AND author <> ''") + ).fetchall() + + for book_id, user_id, raw in rows: + name = " ".join(raw.strip().split()) + if not name: + continue + + author_id = bind.execute( + sa.text("SELECT id FROM author WHERE user_id = :user_id AND name = :name"), + {"user_id": user_id, "name": name}, + ).scalar() + if author_id is None: + author_id = bind.execute( + sa.text("INSERT INTO author (user_id, name) VALUES (:user_id, :name) RETURNING id"), + {"user_id": user_id, "name": name}, + ).scalar_one() + + bind.execute( + sa.text( + "INSERT OR IGNORE INTO book_author (book_id, author_id) VALUES (:book_id, :author_id)" + ), + {"book_id": book_id, "author_id": author_id}, + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = sa.inspect(bind) + + if inspector.has_table("book_author"): + existing_book_author_indexes = {idx["name"] for idx in inspector.get_indexes("book_author")} + if "ix_book_author_author_id" in existing_book_author_indexes: + op.drop_index("ix_book_author_author_id", table_name="book_author") + if "ix_book_author_book_id" in existing_book_author_indexes: + op.drop_index("ix_book_author_book_id", table_name="book_author") + op.drop_table("book_author") + + if inspector.has_table("author"): + existing_author_indexes = {idx["name"] for idx in inspector.get_indexes("author")} + if "ix_author_name" in existing_author_indexes: + op.drop_index("ix_author_name", table_name="author") + if "ix_author_user_id" in existing_author_indexes: + op.drop_index("ix_author_user_id", table_name="author") + op.drop_table("author") \ No newline at end of file diff --git a/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py b/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py new file mode 100644 index 00000000..8b51979b --- /dev/null +++ b/backend/alembic/versions/f1b2c3d4e5a6_drop_author_column_from_book.py @@ -0,0 +1,42 @@ +"""drop author column from book + +Revision ID: f1b2c3d4e5a6 +Revises: c7a8d9e1f2a3 +Create Date: 2026-08-24 21:10:00 +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "f1b2c3d4e5a6" +down_revision = "c7a8d9e1f2a3" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + with op.batch_alter_table("book", schema=None) as batch_op: + batch_op.drop_column("author") + + +def downgrade() -> None: + with op.batch_alter_table("book", schema=None) as batch_op: + batch_op.add_column(sa.Column("author", sa.String(), nullable=True, server_default="")) + + # Re-populate book.author from the relation tables before the author tables + # are dropped by the previous revision's downgrade. + bind = op.get_bind() + bind.execute( + sa.text( + """ + UPDATE book SET author = ( + SELECT group_concat(a.name, ', ') + FROM book_author ba + JOIN author a ON ba.author_id = a.id + WHERE ba.book_id = book.id + ) + """ + ) + ) \ No newline at end of file diff --git a/backend/app/models.py b/backend/app/models.py index b3a48178..425b2cc2 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -70,7 +70,6 @@ class Book(SQLModel, table=True): id: Optional[int] = Field(default=None, primary_key=True) title: str = Field(index=True) subtitle: Optional[str] = None - author: str = Field(default="", index=True) isbn: Optional[str] = Field(default=None) cover_url: Optional[str] = None publisher: Optional[str] = None @@ -128,6 +127,30 @@ class BookTag(SQLModel, table=True): tag_id: int = Field(foreign_key="tag.id", primary_key=True, index=True) +class Author(SQLModel, table=True): + """A user-specific author name that can be associated with books.""" + + __tablename__: str = "author" + __table_args__ = (sa.UniqueConstraint("user_id", "name", name="uq_author_user_id_name"),) + + id: Optional[int] = Field(default=None, primary_key=True) + user_id: int = Field(foreign_key="user.id", index=True) + name: str = Field(index=True) + created_at: datetime = Field( + default_factory=utcnow, + sa_column=Column(UtcDateTime, default=utcnow) + ) + + +class BookAuthor(SQLModel, table=True): + """Many-to-many association between books and authors.""" + + __tablename__: str = "book_author" + + book_id: int = Field(foreign_key="book.id", primary_key=True) + author_id: int = Field(foreign_key="author.id", primary_key=True, index=True) + + class User(SQLModel, table=True): """A user account.""" diff --git a/backend/app/routers/books.py b/backend/app/routers/books.py index e33a3e72..c6c5e8de 100644 --- a/backend/app/routers/books.py +++ b/backend/app/routers/books.py @@ -12,7 +12,7 @@ from app.auth import require_user from app.config import settings from app.database import get_session -from app.models import AcquisitionStatus, Book, BookTag, ReadingProgress, ReadingStatus, Tag, User +from app.models import AcquisitionStatus, Author, Book, BookAuthor, BookTag, ReadingProgress, ReadingStatus, Tag, User from app.schemas import ( BookCreate, BookListResponse, @@ -26,6 +26,13 @@ SuggestionList, TagCloudEntry, ) +from app.services.authors import ( + cleanup_orphan_authors, + join_authors, + load_authors_batch, + resolve_authors_payload, + sync_book_authors, +) from app.services.cover_storage import ( delete_cover_file, local_cover_filename, @@ -130,11 +137,14 @@ def _raise_integrity_conflict(exc: IntegrityError) -> None: raise -def _build_book_read_with_tags(book: Book, tags_text: str | None) -> BookRead: - """Build a BookRead from a Book model with a pre-resolved tags string.""" +def _build_book_read_with_tags(book: Book, tags_text: str | None, authors: list[str] | None = None) -> BookRead: + """Build a BookRead from a Book model with pre-resolved tags and authors.""" payload = book.model_dump() payload.pop("user_id", None) payload["tags"] = tags_text + authors = authors or [] + payload["authors"] = authors + payload["author"] = join_authors(authors) return BookRead.model_validate(payload) @@ -227,9 +237,10 @@ def list_books( logger.debug("list_books — returning %d/%d book(s)", len(books), total) book_ids = [b.id for b in books if b.id is not None] book_tags_map = load_tags_batch(session, book_ids) if book_ids else {} + book_authors_map = load_authors_batch(session, book_ids) if book_ids else {} return BookListResponse( books=[ - _build_book_read_with_tags(book, book_tags_map.get(book.id)) + _build_book_read_with_tags(book, book_tags_map.get(book.id), book_authors_map.get(book.id)) for book in books ], total=total, @@ -348,10 +359,22 @@ def suggest_authors( current_user: User = Depends(require_user), session: Session = Depends(get_session), ) -> SuggestionList: - """Autocomplete author names from the user's existing books.""" + """Autocomplete author names from the user's existing authors.""" assert current_user.id is not None - suggestions = _suggest_field(session, current_user.id, "author", q, limit) - return SuggestionList(suggestions=suggestions) + if not q.strip(): + return SuggestionList(suggestions=[]) + pattern = f"%{_escape_like(q)}%" + rows = session.exec( + select(Author.name) + .where( + Author.user_id == current_user.id, + col(Author.name).ilike(pattern, escape="\\"), + ) + .distinct() + .order_by(Author.name) + .limit(limit) + ).all() + return SuggestionList(suggestions=list(rows)) @router.get("/suggestions/publishers", response_model=SuggestionList) @@ -420,6 +443,8 @@ async def create_book( book_data["language"] = _normalize_language(book_data.get("language")) book_data["cover_url"] = cover_url book_data.pop("tags", None) + book_data.pop("author", None) + book_data.pop("authors", None) book_data["user_id"] = current_user.id _validate_dates(book_data) book = Book.model_validate(book_data) @@ -430,6 +455,8 @@ async def create_book( session.rollback() _raise_integrity_conflict(exc) sync_book_tags(session, current_user.id, book.id or 0, book_in.tags) + names = resolve_authors_payload(book_in.author, book_in.authors) or [] + sync_book_authors(session, current_user.id, book.id or 0, names) try: session.commit() except IntegrityError as exc: @@ -475,6 +502,10 @@ async def update_book( update_data["language"] = _normalize_language(update_data.get("language")) tags_provided = "tags" in update_data tags_raw = update_data.pop("tags", None) if tags_provided else None + authors_payload = resolve_authors_payload( + update_data.pop("author", None), update_data.pop("authors", None) + ) + authors_provided = authors_payload is not None target_status = update_data.get("reading_status", book.reading_status) # Download external cover URL -> local file. @@ -520,6 +551,10 @@ async def update_book( assert book.id is not None sync_book_tags(session, current_user.id, book.id, tags_raw) cleanup_orphan_tags(session, current_user.id) + if authors_provided: + assert book.id is not None + sync_book_authors(session, current_user.id, book.id, authors_payload) + cleanup_orphan_authors(session, current_user.id) try: session.commit() except IntegrityError as exc: @@ -676,11 +711,14 @@ def delete_book( for link in session.exec(select(BookTag).where(BookTag.book_id == book.id)).all(): session.delete(link) + for link in session.exec(select(BookAuthor).where(BookAuthor.book_id == book.id)).all(): + session.delete(link) for entry in session.exec( select(ReadingProgress).where(ReadingProgress.book_id == book.id) ).all(): session.delete(entry) session.delete(book) cleanup_orphan_tags(session, current_user.id) + cleanup_orphan_authors(session, current_user.id) session.commit() logger.info("Deleted book id=%s", book_id) diff --git a/backend/app/routers/hygiene.py b/backend/app/routers/hygiene.py index 8ea294ba..46e509cd 100644 --- a/backend/app/routers/hygiene.py +++ b/backend/app/routers/hygiene.py @@ -10,7 +10,7 @@ from app.auth import require_user from app.config import settings from app.database import get_session -from app.models import Book, User +from app.models import Book, BookAuthor, User from app.schemas import ( HygieneAttribute, HygieneBatchUpdateRequest, @@ -18,8 +18,9 @@ HygieneMissingBook, HygieneMissingResponse, ) +from app.services.authors import cleanup_orphan_authors, join_authors, load_authors_batch, sync_book_authors from app.services.cover_import import import_cover_from_url, is_external_cover_url -from app.services.tags import build_book_read +from app.services.tags import load_tags_batch logger = logging.getLogger(__name__) @@ -30,18 +31,24 @@ def _missing_condition(attr: HygieneAttribute): """Return a SQLAlchemy filter condition for a given attribute being missing.""" - col = getattr(Book, attr.value) if attr == HygieneAttribute.author: - return or_(col == "", col.is_(None)) + return ~col(Book.id).in_( + select(BookAuthor.book_id).where(BookAuthor.book_id == col(Book.id)) + ) + col_expr = getattr(Book, attr.value) if attr == HygieneAttribute.page_count: - return or_(col == 0, col.is_(None)) - return col.is_(None) + return or_(col_expr == 0, col_expr.is_(None)) + return col_expr.is_(None) -def _compute_missing_attributes(book: Book) -> list[HygieneAttribute]: +def _compute_missing_attributes(book: Book, author_names: list[str]) -> list[HygieneAttribute]: """Return the list of hygiene attributes that are missing for a given book.""" missing: list[HygieneAttribute] = [] for attr in HygieneAttribute: + if attr == HygieneAttribute.author: + if not author_names: + missing.append(attr) + continue val = getattr(book, attr.value) is_missing = val is None or val == "" or val == 0 if is_missing: @@ -102,21 +109,26 @@ def list_missing( ).all() hygiene_books = [] + book_ids = [book.id for book in books if book.id is not None] + tags_map = load_tags_batch(session, book_ids) + authors_map = load_authors_batch(session, book_ids) for book in books: - br = build_book_read(session, book) - missing_attrs = _compute_missing_attributes(book) + assert book.id is not None + author_names = authors_map.get(book.id, []) + missing_attrs = _compute_missing_attributes(book, author_names) hygiene_books.append(HygieneMissingBook( - id=br.id, - title=br.title, - author=br.author, - isbn=br.isbn, - publisher=br.publisher, - published_year=br.published_year, - blurb=br.blurb, - language=br.language, - subtitle=br.subtitle, - page_count=br.page_count or 0, - cover_url=br.cover_url, + id=book.id, + title=book.title, + author=join_authors(author_names), + authors=author_names, + isbn=book.isbn, + publisher=book.publisher, + published_year=book.published_year, + blurb=book.blurb, + language=book.language, + subtitle=book.subtitle, + page_count=book.page_count or 0, + cover_url=book.cover_url, missing_attributes=[a for a in requested if a in missing_attrs], )) @@ -233,8 +245,18 @@ async def batch_update( skipped_ids: list[int] = [] to_update_ids: list[int] = [] + current_authors_map = ( + load_authors_batch(session, [b.id for b in books if b.id is not None]) + if req.field == HygieneAttribute.author + else {} + ) for book in books: - current_val = getattr(book, req.field.value) + if req.field == HygieneAttribute.author: + # Compare against the current author set so multi-author books are + # only skipped when they already contain exactly the target author. + current_val = ", ".join(current_authors_map.get(book.id, [])) or None + else: + current_val = getattr(book, req.field.value) if current_val == req.value: skipped_ids.append(book.id) # ty: ignore[invalid-argument-type] else: @@ -242,19 +264,36 @@ async def batch_update( updated = 0 if to_update_ids: - try: - stmt = ( - sqlmodel_update(Book) - .where(col(Book.id).in_(to_update_ids)) - .values({req.field.value: req.value}) - ) - updated = len(to_update_ids) - session.exec(stmt) - session.commit() - except Exception: - session.rollback() - logger.exception("Batch update failed for %d books", len(to_update_ids)) - raise HTTPException(status_code=500, detail="Batch update failed due to a database error") + if req.field == HygieneAttribute.author: + # Authors live in a relation table; a bulk SQL update does not work. + assert current_user.id is not None + for book in books: + if book.id not in set(to_update_ids): + continue + assert book.id is not None + sync_book_authors(session, current_user.id, book.id, [str(req.value)]) + updated += 1 + cleanup_orphan_authors(session, current_user.id) + try: + session.commit() + except Exception: + session.rollback() + logger.exception("Batch author update failed for %d books", len(to_update_ids)) + raise HTTPException(status_code=500, detail="Batch update failed due to a database error") + else: + try: + stmt = ( + sqlmodel_update(Book) + .where(col(Book.id).in_(to_update_ids)) + .values({req.field.value: req.value}) + ) + updated = len(to_update_ids) + session.exec(stmt) + session.commit() + except Exception: + session.rollback() + logger.exception("Batch update failed for %d books", len(to_update_ids)) + raise HTTPException(status_code=500, detail="Batch update failed due to a database error") return HygieneBatchUpdateResponse( updated=updated, diff --git a/backend/app/routers/import_.py b/backend/app/routers/import_.py index d094bdc9..acde920c 100644 --- a/backend/app/routers/import_.py +++ b/backend/app/routers/import_.py @@ -16,6 +16,7 @@ from app.models import Book, User from app.schemas import BookImportCandidate, BookImportRequest, BookRead from app.services import book_import +from app.services.authors import resolve_authors_payload, sync_book_authors from app.services.cover_storage import download_cover from app.services.tags import build_book_read, sync_book_tags @@ -145,7 +146,6 @@ async def import_book( book = Book( title=c.title, subtitle=c.subtitle, - author=c.author or "", isbn=c.isbn, cover_url=cover_url, publisher=c.publisher, @@ -164,6 +164,8 @@ async def import_book( session.rollback() _raise_integrity_conflict(exc) sync_book_tags(session, current_user.id, book.id or 0, c.tags) + names = resolve_authors_payload(c.author, c.authors) or [] + sync_book_authors(session, current_user.id, book.id or 0, names) try: session.commit() except IntegrityError as exc: diff --git a/backend/app/routers/profile.py b/backend/app/routers/profile.py index eddff7e4..b838b98e 100644 --- a/backend/app/routers/profile.py +++ b/backend/app/routers/profile.py @@ -158,8 +158,8 @@ def reset_data( raise logger.warning( - "User %s reset personal data: books=%s tags=%s progress_entries=%s", - current_user.id, deleted.books, deleted.tags, deleted.progress_entries, + "User %s reset personal data: books=%s tags=%s authors=%s progress_entries=%s", + current_user.id, deleted.books, deleted.tags, deleted.authors, deleted.progress_entries, ) return DataResetResponse( @@ -167,6 +167,7 @@ def reset_data( deleted=DataResetDeleted( books=deleted.books, tags=deleted.tags, + authors=deleted.authors, progress_entries=deleted.progress_entries, ), ) diff --git a/backend/app/routers/statistics.py b/backend/app/routers/statistics.py index c14d2db4..b74f86ba 100644 --- a/backend/app/routers/statistics.py +++ b/backend/app/routers/statistics.py @@ -14,7 +14,8 @@ from app.auth import require_user from app.database import get_session -from app.models import AcquisitionStatus, Book, ReadingProgress, ReadingStatus, User, UserSettings +from app.models import AcquisitionStatus, Author, Book, BookAuthor, ReadingProgress, ReadingStatus, User, UserSettings +from app.services.authors import join_authors, load_authors_batch from app.schemas import ( AcquisitionStatusDistribution, DailyPages, @@ -509,25 +510,37 @@ def get_statistics( else: books_finished_per_year = [] - author_counts: Counter[str] = Counter() - for book in books: - if book.author and book.author.strip(): - author_counts[book.author.strip()] += 1 + author_count_label = func.count(func.distinct(BookAuthor.book_id)).label("cnt") + author_count_rows = session.exec( + select(Author.name, author_count_label) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .join(Book, col(Book.id) == col(BookAuthor.book_id)) + .where(Book.user_id == current_user.id) + .group_by(col(Author.id)) + .order_by(author_count_label.desc(), col(Author.name).asc()) + .limit(3) + ).all() + author_counts = Counter({name: count for name, count in author_count_rows}) top_authors: list[TopAuthor] = [] if author_counts: - author_items = sorted(author_counts.items(), key=lambda item: item[0].lower()) - top_author_counts = sorted(author_items, key=lambda item: item[1], reverse=True)[:3] + top_author_counts = author_counts.most_common(3) top_author_names = [name for name, _ in top_author_counts] covers_by_author: dict[str, list[TopAuthorCover]] = {} for author_name in top_author_names: max_slots = min(5, author_counts[author_name]) + book_ids_with_author = select(BookAuthor.book_id).join( + Author, col(Author.id) == col(BookAuthor.author_id) + ).where( + Author.user_id == current_user.id, + Author.name == author_name, + ) cover_rows = session.exec( select(Book.id, Book.title, Book.reading_status, Book.cover_url) .where( Book.user_id == current_user.id, - Book.author == author_name, + col(Book.id).in_(book_ids_with_author), col(Book.cover_url).is_not(None), ) .order_by(col(Book.id)) @@ -544,7 +557,7 @@ def get_statistics( select(Book.id, Book.title, Book.reading_status, Book.cover_url) .where( Book.user_id == current_user.id, - Book.author == author_name, + col(Book.id).in_(book_ids_with_author), col(Book.cover_url).is_(None), ) .order_by(col(Book.id)) @@ -573,6 +586,8 @@ def get_statistics( average_rating = round(mean(rating_values), 2) if rating_values else None rated_books = [b for b in books if b.rating is not None] + rated_book_ids = [b.id for b in rated_books if b.id is not None] + rated_authors_map = load_authors_batch(session, rated_book_ids) def _rating_sort_key(book: Book) -> tuple[int, float]: assert book.rating is not None @@ -582,16 +597,18 @@ def _rating_sort_key(book: Book) -> tuple[int, float]: for b in sorted(rated_books, key=_rating_sort_key): assert b.id is not None assert b.rating is not None + author_names = rated_authors_map.get(b.id, []) top_rated_books.append( - TopRatedBook(book_id=b.id, title=b.title or "", author=b.author, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) + TopRatedBook(book_id=b.id, title=b.title or "", author=join_authors(author_names), authors=author_names, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) ) worst_rated_books = [] for b in sorted(rated_books, key=lambda x: (-_rating_sort_key(x)[0], -_rating_sort_key(x)[1])): assert b.id is not None assert b.rating is not None + author_names = rated_authors_map.get(b.id, []) worst_rated_books.append( - TopRatedBook(book_id=b.id, title=b.title or "", author=b.author, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) + TopRatedBook(book_id=b.id, title=b.title or "", author=join_authors(author_names), authors=author_names, rating=b.rating, reading_status=b.reading_status, cover_url=b.cover_url) ) return StatisticsResponse( diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 4d127e69..bd15585c 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -41,7 +41,8 @@ class BookCreate(SQLModel): """Request body to create a new book.""" title: str subtitle: Optional[str] = None - author: str + author: Optional[str] = None + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -63,6 +64,7 @@ class BookUpdate(SQLModel): title: Optional[str] = None subtitle: Optional[str] = None author: Optional[str] = None + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -84,6 +86,7 @@ class BookImportCandidate(SQLModel): title: str subtitle: Optional[str] = None author: Optional[str] = None + authors: Optional[list[str]] = None isbn: Optional[str] = None cover_url: Optional[str] = None publisher: Optional[str] = None @@ -130,6 +133,7 @@ class BookRead(SQLModel): title: str subtitle: Optional[str] author: Optional[str] + authors: list[str] = [] isbn: Optional[str] cover_url: Optional[str] publisher: Optional[str] @@ -246,6 +250,7 @@ class TopRatedBook(SQLModel): book_id: int title: str author: Optional[str] + authors: list[str] = [] rating: int reading_status: ReadingStatus cover_url: Optional[str] @@ -385,6 +390,7 @@ class DataResetDeleted(SQLModel): """Counts of deleted items after a data reset.""" books: int tags: int + authors: int progress_entries: int @@ -482,6 +488,7 @@ class HygieneMissingBook(SQLModel): id: int title: str author: str | None + authors: list[str] = [] isbn: str | None publisher: str | None published_year: int | None diff --git a/backend/app/services/authors.py b/backend/app/services/authors.py new file mode 100644 index 00000000..396e4795 --- /dev/null +++ b/backend/app/services/authors.py @@ -0,0 +1,232 @@ +"""Author parsing, synchronization, and query helpers. + +Authors are stored per-user in the ``author`` table and linked to books through +the ``book_author`` relation table, mirroring how tags work. The public API keeps +an ``author`` joined-string field for backward compatibility alongside the new +``authors`` list field. +""" + +import re + +from sqlmodel import Session, col, select + +from app.models import Author, Book, BookAuthor +from app.time_utils import utcnow + + +def normalize_author_name(name: str) -> str | None: + """Trim whitespace and collapse internal whitespace in an author name.""" + cleaned = " ".join(name.strip().split()) + return cleaned or None + + +def normalize_author_list(names: list[str] | None) -> list[str]: + """Normalize a list of author names from the API or UI. + + Trims whitespace, collapses internal whitespace, and deduplicates + case-insensitively while preserving the first-cased spelling seen. + """ + if not names: + return [] + seen: set[str] = set() + result: list[str] = [] + for name in names: + cleaned = normalize_author_name(name) + if cleaned is None: + continue + key = cleaned.lower() + if key in seen: + continue + seen.add(key) + result.append(cleaned) + return result + + +def parse_legacy_author(raw: str | None) -> list[str]: + """Parse a legacy comma-separated author string (for BookCreate/Update.author). + + Mirrors tag parsing: split on commas, trim, collapse whitespace, deduplicate. + """ + if not raw: + return [] + seen: set[str] = set() + parsed: list[str] = [] + for piece in raw.split(","): + cleaned = normalize_author_name(piece) + if cleaned is None: + continue + key = cleaned.lower() + if key in seen: + continue + seen.add(key) + parsed.append(cleaned) + return parsed + + +def parse_authors(raw: str | list[str] | None) -> list[str]: + """Normalize an author value from a file import. + + A list contributes one author per entry. A string is passed through + ``split_author_string``: it stays a single author unless it contains a + ``;``, `` & ``, or `` and `` separator (how the CSV export writes the + ``authors`` column), so it can round-trip. Commas inside a name like + ``"Asimov, Isaac"`` are always preserved. The result is deduplicated + case-insensitively while preserving the first spelling seen. + """ + if raw is None: + return [] + pieces = split_author_string(raw) if isinstance(raw, str) else raw + seen: set[str] = set() + result: list[str] = [] + for piece in pieces: + name = normalize_author_name(piece) + if name is None: + continue + key = name.lower() + if key in seen: + continue + seen.add(key) + result.append(name) + return result + + +def resolve_authors_payload( + author: str | None = None, + authors: list[str] | None = None, +) -> list[str] | None: + """Resolve the hybrid create/update payload. + + * If ``authors`` is provided (including ``[]``), use it. + * Otherwise fall back to parsing the legacy ``author`` string. + * If neither is provided, return None so the caller can treat it as + "not provided" (for updates) or "empty" (for creates). + """ + if authors is not None: + return normalize_author_list(authors) + if author is not None: + return parse_legacy_author(author) + return None + + +# Conservative separator regex used only when an external source returns a +# single delimited string. Never splits on commas. +_AUTHOR_SEPARATOR_RE = re.compile( + r"\s*;\s*|\s+&\s+|\s+and\s+", + flags=re.IGNORECASE, +) + + +def split_author_string(value: str | None) -> list[str]: + """Split a single author string from an external source. + + Only semicolons, ``" & "``, and ``" and "`` are treated as separators, so + names containing commas (e.g. ``"Asimov, Isaac"``) are preserved. + If no known separator is found, the whole string is returned as one author. + """ + if not value: + return [] + if not _AUTHOR_SEPARATOR_RE.search(value): + name = normalize_author_name(value) + return [name] if name else [] + parts = _AUTHOR_SEPARATOR_RE.split(value) + return [name for name in (normalize_author_name(p) for p in parts) if name] + + +def sync_book_authors( + session: Session, user_id: int, book_id: int, names: list[str] | None +) -> None: + """Set the authors for a book to *names*, creating Author rows as needed.""" + parsed = normalize_author_list(names) + + existing_links = list( + session.exec(select(BookAuthor).where(BookAuthor.book_id == book_id)).all() + ) + existing_author_ids = {link.author_id for link in existing_links} + + if not parsed: + for link in existing_links: + session.delete(link) + return + + existing_authors = list( + session.exec( + select(Author).where(Author.user_id == user_id, col(Author.name).in_(parsed)) + ).all() + ) + name_to_author = {author.name: author for author in existing_authors} + + for name in parsed: + if name in name_to_author: + continue + author = Author(user_id=user_id, name=name, created_at=utcnow()) + session.add(author) + session.flush() + name_to_author[name] = author + + target_ids: set[int] = set() + for name in parsed: + author_id = name_to_author[name].id + if author_id is not None: + target_ids.add(author_id) + + for author_id in target_ids - existing_author_ids: + session.add(BookAuthor(book_id=book_id, author_id=author_id)) + + for link in existing_links: + if link.author_id not in target_ids: + session.delete(link) + + +def cleanup_orphan_authors(session: Session, user_id: int) -> None: + """Delete authors of this user that are no longer linked to any book.""" + linked_ids = set( + session.exec( + select(BookAuthor.author_id).where( + col(BookAuthor.author_id).in_( + select(Author.id).where(Author.user_id == user_id) + ) + ) + ).all() + ) + authors = list(session.exec(select(Author).where(Author.user_id == user_id)).all()) + for author in authors: + if author.id not in linked_ids: + session.delete(author) + + +def authors_list_for_book(session: Session, book_id: int | None) -> list[str]: + """Return ordered author names for a book.""" + if book_id is None: + return [] + names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == book_id) + .order_by(col(Author.name).asc()) + ).all() + ) + return names + + +def load_authors_batch(session: Session, book_ids: list[int]) -> dict[int, list[str]]: + """Batch-load author lists for many book IDs.""" + if not book_ids: + return {} + rows = session.exec( + select(BookAuthor.book_id, Author.name) + .join(Author, col(Author.id) == col(BookAuthor.author_id)) + .where(col(BookAuthor.book_id).in_(book_ids)) + .order_by(col(BookAuthor.book_id), col(Author.name).asc()) + ).all() + result: dict[int, list[str]] = {} + for book_id, name in rows: + result.setdefault(book_id, []).append(name) + return {bid: names for bid, names in result.items()} + + +def join_authors(names: list[str] | None) -> str | None: + """Join author names for the legacy ``author`` response field.""" + if not names: + return None + return ", ".join(names) \ No newline at end of file diff --git a/backend/app/services/book_import.py b/backend/app/services/book_import.py index d1c34d16..13b3be53 100644 --- a/backend/app/services/book_import.py +++ b/backend/app/services/book_import.py @@ -364,7 +364,7 @@ async def _search_open_library( def map_open_library(doc: dict) -> BookImportCandidate: """Map a single Open Library search doc to BookImportCandidate.""" # Authors: list of strings - authors = doc.get("author_name") or [] + authors = [a for a in (doc.get("author_name") or []) if a] author = ", ".join(authors) if authors else None # ISBN: first entry of the list, prefer ISBN-13 (length 13) @@ -391,6 +391,7 @@ def map_open_library(doc: dict) -> BookImportCandidate: title=doc["title"], subtitle=doc.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=publisher, @@ -561,7 +562,7 @@ def map_google_books(item: dict) -> BookImportCandidate: vi = item.get("volumeInfo", {}) # Authors - authors: list[str] = vi.get("authors") or [] + authors: list[str] = [a for a in (vi.get("authors") or []) if a] author = ", ".join(authors) if authors else None # ISBN: prefer ISBN_13 @@ -596,6 +597,7 @@ def map_google_books(item: dict) -> BookImportCandidate: title=vi["title"], subtitle=vi.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=vi.get("publisher"), @@ -770,12 +772,12 @@ def map_hardcover(edition: dict) -> BookImportCandidate | None: return None contributions = edition.get("contributions") or [] - author = None + authors: list[str] = [] for c in contributions: - author_name = c.get("author", {}).get("name") + author_name = (c.get("author") or {}).get("name") if author_name: - author = author_name - break + authors.append(author_name) + author = ", ".join(authors) if authors else None isbn = edition.get("isbn_13") or None @@ -811,6 +813,7 @@ def map_hardcover(edition: dict) -> BookImportCandidate | None: title=title, subtitle=edition.get("subtitle") or None, author=author, + authors=authors, isbn=isbn, cover_url=cover_url, publisher=publisher, diff --git a/backend/app/services/data_export.py b/backend/app/services/data_export.py index c6f047f2..c3cc0d5c 100644 --- a/backend/app/services/data_export.py +++ b/backend/app/services/data_export.py @@ -12,6 +12,7 @@ from app._build_info import __git_sha__, __version__ from app.models import Book, BookTag, ReadingProgress, Tag, User +from app.services.authors import authors_list_for_book from app.time_utils import utcnow from app.services.cover_storage import local_cover_filename, resolve_cover_path from app.services.tags import tags_text_for_book @@ -20,6 +21,7 @@ "title", "subtitle", "author", + "authors", "isbn", "publisher", "published_year", @@ -48,12 +50,24 @@ def _serialize_datetime(value: datetime | None) -> str | None: return value.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") -def _book_to_dict(session: Session, book: Book) -> dict: - """Convert a Book model to a flat export dict.""" +def _book_to_dict(session: Session, book: Book, export_format: str) -> dict: + """Convert a Book model to a flat export dict. + + ``author`` carries the joined string in CSV and the list in JSON, so JSON + exports round-trip through the adaptive import (each array entry becomes an + author). The ``authors`` field always holds the list of names. + """ + authors = authors_list_for_book(session, book.id) + joined = ", ".join(authors) if authors else None + author_value = authors if export_format == "json" else joined + # CSV has no list type: the dedicated `authors` column uses `; ` so it can + # round-trip through the adaptive import (string values split on `;`). + authors_value = "; ".join(authors) if export_format == "csv" else authors return { "title": book.title, "subtitle": book.subtitle, - "author": book.author, + "author": author_value, + "authors": authors_value, "isbn": book.isbn, "publisher": book.publisher, "published_year": book.published_year, @@ -150,7 +164,7 @@ def build_export_zip( book_titles = {book.id: book.title for book in books if book.id is not None} - books_rows = [_book_to_dict(session, book) for book in books] + books_rows = [_book_to_dict(session, book, export_format) for book in books] progress_rows = [_progress_to_dict(entry, book_titles) for entry in progress_entries] tag_rows = [_tag_to_dict(tag, tag_counts) for tag in tags] diff --git a/backend/app/services/data_import.py b/backend/app/services/data_import.py index 1b353cd1..e282b80b 100644 --- a/backend/app/services/data_import.py +++ b/backend/app/services/data_import.py @@ -20,6 +20,7 @@ logger = logging.getLogger(__name__) from app.time_utils import utcnow +from app.services.authors import parse_authors, sync_book_authors from app.services.cover_storage import download_cover from app.services.tags import sync_book_tags from app.services.isbn_utils import normalize_isbn @@ -128,11 +129,15 @@ def delete_parsed_upload(file_id: str, user_id: int) -> None: _temp_file_path(user_id, file_id).unlink(missing_ok=True) -def _to_flat_row(row: dict) -> dict[str, str | int | float | bool | None]: - """Flatten a row dict, rejecting nested values.""" - flat: dict[str, str | int | float | bool | None] = {} +def _to_flat_row(row: dict) -> dict[str, object]: + """Flatten a row dict, rejecting nested dict values. + + List values (e.g. a JSON ``authors`` array) are kept as-is so the adaptive + author handling can turn each entry into a separate author. + """ + flat: dict[str, object] = {} for key, value in row.items(): - if isinstance(value, (dict, list)): + if isinstance(value, dict): raise ValueError("error.importNestedValuesNotSupported") flat[str(key)] = value return flat @@ -411,6 +416,10 @@ def _mapped_row( if not source: continue value = row.get(source, "") + if target == "author" and isinstance(value, list): + # Adaptive author handling: an array contributes one author per entry. + mapped[target] = value + continue value_str = "" if value is None else str(value) if target in transform_cache: from app.services.transform_engine import TransformExecutionError, execute_transform @@ -655,9 +664,9 @@ def preview_import( ) # Convert raw values to strings for display - source_display = {k: str(v) if v is not None else "" for k, v in row.items()} + source_display = {k: (", ".join(v) if isinstance(v, list) else str(v)) if v is not None else "" for k, v in row.items()} # Convert transformed values to strings for display - transformed_display = {k: str(v) if v is not None else "" for k, v in row_data.items()} + transformed_display = {k: (", ".join(v) if isinstance(v, list) else str(v)) if v is not None else "" for k, v in row_data.items()} preview_rows.append({ "row_number": idx, @@ -772,7 +781,6 @@ async def execute_import( book = Book( title=title, subtitle=None if row_data.get("subtitle") in (None, "") else str(row_data.get("subtitle")), - author=None if row_data.get("author") in (None, "") else str(row_data.get("author")), # ty: ignore[invalid-argument-type] isbn=None if row_data.get("isbn") in (None, "") else str(row_data.get("isbn")), cover_url=cover_url, publisher=None if row_data.get("publisher") in (None, "") else str(row_data.get("publisher")), @@ -792,6 +800,13 @@ async def execute_import( session.flush() assert book.id is not None + sync_book_authors( + session, + user.id, + book.id, + parse_authors(row_data.get("author")), + ) + if create_progress_for_read and reading_status == ReadingStatus.read and page_count is not None and date_finished is not None: log_date = date_finished if log_date.tzinfo is None: diff --git a/backend/app/services/search.py b/backend/app/services/search.py index 471007cd..65117959 100644 --- a/backend/app/services/search.py +++ b/backend/app/services/search.py @@ -16,12 +16,11 @@ import sqlalchemy as sa from sqlmodel import col, or_, select -from app.models import AcquisitionStatus, Book, BookTag, Tag +from app.models import AcquisitionStatus, Author, Book, BookAuthor, BookTag, Tag # Fields that can be targeted with a prefix. The keys are the canonical, # always-English prefix names; the values are the book model columns. FIELD_COLUMNS: dict[str, Any] = { - "author": Book.author, "title": Book.title, "publisher": Book.publisher, "language": Book.language, @@ -32,14 +31,15 @@ # Availability is a special case: it maps to an exact enum comparison. AVAILABILITY_PREFIX = "availability" TAG_PREFIX = "tag" +AUTHOR_PREFIX = "author" SUPPORTED_PREFIXES: frozenset[str] = frozenset( - [*FIELD_COLUMNS.keys(), AVAILABILITY_PREFIX, TAG_PREFIX] + [*FIELD_COLUMNS.keys(), AVAILABILITY_PREFIX, TAG_PREFIX, AUTHOR_PREFIX] ) # Default fields searched by an unprefixed term (unchanged from the previous -# single-pattern search). -DEFAULT_SEARCH_COLUMNS: tuple[str, ...] = ("title", "subtitle", "author", "blurb") +# single-pattern search, minus the removed Book.author column). +DEFAULT_SEARCH_COLUMNS: tuple[str, ...] = ("title", "subtitle", "blurb") # ``:value`` — value is either a quoted string or a non-space token. _FIELD_TERM_RE = re.compile(r"^([a-zA-Z_]+):(\"(?:\\.|[^\"\\])*\"|\S+)") @@ -148,11 +148,23 @@ def _tag_condition(value: str, user_id: int) -> Any: return col(Book.id).in_(matching_tag_book_ids) +def _author_condition(value: str, user_id: int) -> Any: + """Return a condition matching books that have an author containing *value*.""" + escaped = _escape_like(value) + matching_author_book_ids = ( + select(BookAuthor.book_id) + .join(Author, col(Author.id) == BookAuthor.author_id) + .where(Author.user_id == user_id, col(Author.name).ilike(f"%{escaped}%", escape="\\")) + ) + return col(Book.id).in_(matching_author_book_ids) + + def _unprefixed_condition(value: str, user_id: int) -> Any: """Build the cross-field substring condition for an unprefixed term.""" return or_( *[_ilike(getattr(Book, column), value) for column in DEFAULT_SEARCH_COLUMNS], _tag_condition(value, user_id), + _author_condition(value, user_id), ) @@ -172,6 +184,8 @@ def _field_condition(field: str, value: str, user_id: int) -> Any | None: return _availability_condition(value) if field == TAG_PREFIX: return _tag_condition(value, user_id) + if field == AUTHOR_PREFIX: + return _author_condition(value, user_id) column = FIELD_COLUMNS[field] return _ilike(column, value) diff --git a/backend/app/services/tags.py b/backend/app/services/tags.py index 68c60a9c..64ceba63 100644 --- a/backend/app/services/tags.py +++ b/backend/app/services/tags.py @@ -6,6 +6,7 @@ from app.models import Book, BookTag, Tag from app.schemas import BookRead +from app.services.authors import authors_list_for_book, join_authors from app.time_utils import utcnow @@ -131,4 +132,7 @@ def build_book_read(session: Session, book: Book) -> BookRead: payload = book.model_dump() payload.pop("user_id", None) payload["tags"] = tags_text_for_book(session, book.id) if book.id is not None else None + authors = authors_list_for_book(session, book.id) + payload["authors"] = authors + payload["author"] = join_authors(authors) return BookRead.model_validate(payload) diff --git a/backend/app/services/user_deletion.py b/backend/app/services/user_deletion.py index 0634ac48..9cd11067 100644 --- a/backend/app/services/user_deletion.py +++ b/backend/app/services/user_deletion.py @@ -6,7 +6,7 @@ from fastapi import HTTPException, status from sqlmodel import Session, col, func, select -from app.models import ApiKey, Book, BookTag, OidcLink, ReadingProgress, Tag, User, UserRole, UserSettings +from app.models import ApiKey, Author, Book, BookAuthor, BookTag, OidcLink, ReadingProgress, Tag, User, UserRole, UserSettings from app.time_utils import utcnow from app.services.cover_storage import delete_cover_file, local_cover_filename @@ -16,6 +16,7 @@ class ReadingDataDeletionCounts: """Counts of items deleted during a reading-data or account deletion.""" books: int tags: int + authors: int progress_entries: int @@ -47,6 +48,9 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> tags_count = session.exec( select(func.count()).select_from(Tag).where(Tag.user_id == user_id) ).one() + authors_count = session.exec( + select(func.count()).select_from(Author).where(Author.user_id == user_id) + ).one() if book_ids: for cover_url in {book.cover_url for book in user_books if book.cover_url}: @@ -62,18 +66,25 @@ def delete_user_reading_data(session: Session, user_id: int, covers_dir: str) -> for link in session.exec(select(BookTag).where(col(BookTag.book_id).in_(book_ids))).all(): session.delete(link) + for link in session.exec(select(BookAuthor).where(col(BookAuthor.book_id).in_(book_ids))).all(): + session.delete(link) + for entry in session.exec(select(ReadingProgress).where(ReadingProgress.user_id == user_id)).all(): session.delete(entry) for tag in session.exec(select(Tag).where(Tag.user_id == user_id)).all(): session.delete(tag) + for author in session.exec(select(Author).where(Author.user_id == user_id)).all(): + session.delete(author) + for book in user_books: session.delete(book) return ReadingDataDeletionCounts( books=len(user_books), tags=tags_count, + authors=authors_count, progress_entries=progress_count, ) diff --git a/backend/tests/test_admin.py b/backend/tests/test_admin.py index 6ef2d0d4..658550e1 100644 --- a/backend/tests/test_admin.py +++ b/backend/tests/test_admin.py @@ -208,7 +208,7 @@ def test_admin_restore_success(admin_client_with_file_db: tuple[TestClient, str] # 2. Modify the database (add a new book) conn = sqlite3.connect(db_path) - conn.execute("INSERT INTO book (title, author, page_count, user_id, reading_status, acquisition_status) VALUES ('New Book', '', 0, 1, 'read', 'owned')") + conn.execute("INSERT INTO book (title, page_count, user_id, reading_status, acquisition_status) VALUES ('New Book', 0, 1, 'read', 'owned')") conn.commit() row = conn.execute("SELECT COUNT(*) FROM book").fetchone() assert row[0] == 2 diff --git a/backend/tests/test_authors.py b/backend/tests/test_authors.py new file mode 100644 index 00000000..ae4b10c7 --- /dev/null +++ b/backend/tests/test_authors.py @@ -0,0 +1,193 @@ +"""Tests for the author parsing and relation helpers.""" + +from sqlmodel import Session, col, select + +from app.models import Author, BookAuthor +from app.services.authors import ( + authors_list_for_book, + join_authors, + normalize_author_list, + normalize_author_name, + parse_authors, + parse_legacy_author, + resolve_authors_payload, + split_author_string, + sync_book_authors, +) + + +# ── normalization ────────────────────────────────────────────────────────────── + +def test_normalize_author_name_collapses_whitespace() -> None: + assert normalize_author_name(" Isaac Asimov ") == "Isaac Asimov" + + +def test_normalize_author_name_empty_returns_none() -> None: + assert normalize_author_name(" ") is None + + +def test_normalize_author_list_dedupes_case_insensitively() -> None: + assert normalize_author_list(["Isaac Asimov", "isaac asimov", "Robert A. Heinlein"]) == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_normalize_author_list_drops_blank_entries() -> None: + assert normalize_author_list([" ", "Asimov, Isaac"]) == ["Asimov, Isaac"] + + +# ── parsing ─────────────────────────────────────────────────────────────────── + +def test_parse_legacy_author_splits_on_commas() -> None: + assert parse_legacy_author("Isaac Asimov, Robert A. Heinlein") == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_parse_legacy_author_blank_pieces_skipped() -> None: + assert parse_legacy_author("Isaac Asimov, , Robert A. Heinlein") == [ + "Isaac Asimov", + "Robert A. Heinlein", + ] + + +def test_parse_authors_string_is_single_author() -> None: + """A file-import string without a separator becomes one author.""" + assert parse_authors("Asimov, Isaac") == ["Asimov, Isaac"] + assert parse_authors("Frank Herbert") == ["Frank Herbert"] + + +def test_parse_authors_string_splits_on_semicolon() -> None: + """Strings may encode several authors with `;` (CSV export round-trip).""" + assert parse_authors("Frank Herbert; Brian Herbert") == ["Frank Herbert", "Brian Herbert"] + + +def test_parse_authors_list_is_one_per_entry() -> None: + assert parse_authors(["Frank Herbert", "Brian Herbert"]) == [ + "Frank Herbert", + "Brian Herbert", + ] + + +def test_parse_authors_none_returns_empty() -> None: + assert parse_authors(None) == [] + + +# ── hybrid payload resolution ───────────────────────────────────────────────── + +def test_resolve_authors_payload_authors_takes_precedence() -> None: + assert resolve_authors_payload(author="A, B", authors=["C", "D"]) == ["C", "D"] + + +def test_resolve_authors_payload_empty_list_clears() -> None: + assert resolve_authors_payload(author="A", authors=[]) == [] + + +def test_resolve_authors_payload_falls_back_to_legacy() -> None: + assert resolve_authors_payload(author="A, B") == ["A", "B"] + + +def test_resolve_authors_payload_none_means_not_provided() -> None: + assert resolve_authors_payload() is None + + +# ── external splitter ───────────────────────────────────────────────────────── + +def test_split_author_string_preserves_comma_names() -> None: + assert split_author_string("Asimov, Isaac") == ["Asimov, Isaac"] + + +def test_split_author_string_splits_on_semicolon() -> None: + assert split_author_string("Asimov, Isaac; Clarke, Arthur") == [ + "Asimov, Isaac", + "Clarke, Arthur", + ] + + +def test_split_author_string_splits_on_ampersand() -> None: + assert split_author_string("Frank Herbert & Brian Herbert") == [ + "Frank Herbert", + "Brian Herbert", + ] + + +def test_split_author_string_splits_on_and() -> None: + assert split_author_string("Terry Pratchett and Neil Gaiman") == [ + "Terry Pratchett", + "Neil Gaiman", + ] + + +def test_split_author_string_empty_returns_empty() -> None: + assert split_author_string(None) == [] + assert split_author_string(" ") == [] + + +# ── sync / query ────────────────────────────────────────────────────────────── + +def test_sync_book_authors_creates_rows_and_links(session: Session) -> None: + user_id = 1 + book_id = 1 + sync_book_authors(session, user_id, book_id, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + names = authors_list_for_book(session, book_id) + assert names == ["Frank Herbert", "Isaac Asimov"] # alphabetical + + authors = session.exec(select(Author).where(Author.user_id == user_id)).all() + assert {a.name for a in authors} == {"Isaac Asimov", "Frank Herbert"} + + +def test_sync_book_authors_reuses_existing_author(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov"]) + session.commit() + first_id = session.exec( + select(Author.id).where(Author.user_id == user_id, Author.name == "Isaac Asimov") + ).one() + + sync_book_authors(session, user_id, 2, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + second_id = session.exec( + select(Author.id).where(Author.user_id == user_id, Author.name == "Isaac Asimov") + ).one() + assert first_id == second_id + + link_count = session.exec( + select(col(BookAuthor.author_id)).where(BookAuthor.author_id == second_id) + ).all() + assert len(link_count) == 2 + + +def test_sync_book_authors_removes_stale_links(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov", "Frank Herbert"]) + session.commit() + + sync_book_authors(session, user_id, 1, ["Frank Herbert"]) + session.commit() + + names = authors_list_for_book(session, 1) + assert names == ["Frank Herbert"] + + +def test_sync_book_authors_empty_clears_links(session: Session) -> None: + user_id = 1 + sync_book_authors(session, user_id, 1, ["Isaac Asimov"]) + session.commit() + + sync_book_authors(session, user_id, 1, []) + session.commit() + + assert authors_list_for_book(session, 1) == [] + + +# ── join ────────────────────────────────────────────────────────────────────── + +def test_join_authors() -> None: + assert join_authors(["A", "B"]) == "A, B" + assert join_authors([]) is None + assert join_authors(None) is None \ No newline at end of file diff --git a/backend/tests/test_books.py b/backend/tests/test_books.py index 852ff2e6..23f71088 100644 --- a/backend/tests/test_books.py +++ b/backend/tests/test_books.py @@ -81,6 +81,102 @@ def test_create_book_invalid_rating_returns_422(client: TestClient) -> None: assert resp.status_code == 422 +# ── authors (multiple) ───────────────────────────────────────────────────────── + +def test_create_book_with_authors_list(client: TestClient) -> None: + resp = client.post("/api/books", json={ + "title": "Good Omens", + "authors": ["Terry Pratchett", "Neil Gaiman"], + "page_count": 288, + }) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert data["author"] == "Neil Gaiman, Terry Pratchett" + + +def test_create_book_legacy_author_comma_separated(client: TestClient) -> None: + resp = client.post("/api/books", json={"title": "Dune", "author": "Frank Herbert", "page_count": 412}) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + assert data["author"] == "Frank Herbert" + + +def test_create_book_authors_takes_precedence_over_author(client: TestClient) -> None: + resp = client.post("/api/books", json={ + "title": "Dune", + "author": "Wrong Author", + "authors": ["Frank Herbert"], + "page_count": 412, + }) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + + +def test_create_book_no_author_returns_empty_list(client: TestClient) -> None: + resp = client.post("/api/books", json={"title": "No Author", "page_count": 100}) + assert resp.status_code == 201 + data = resp.json() + assert data["authors"] == [] + assert data["author"] is None + + +def test_update_book_authors_replaces(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"authors": ["Frank Herbert", "Brian Herbert"]}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Brian Herbert", "Frank Herbert"] + assert data["author"] == "Brian Herbert, Frank Herbert" + + +def test_update_book_authors_empty_clears(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"authors": []}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == [] + assert data["author"] is None + + +def test_update_book_without_authors_keeps_them(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"title": "Dune 2"}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Frank Herbert"] + + +def test_update_book_legacy_author_parses(client: TestClient) -> None: + book = _create_book(client, title="Dune", author="Frank Herbert") + resp = client.patch(f"/api/books/{book['id']}", json={"author": "Isaac Asimov"}) + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Isaac Asimov"] + + +def test_list_books_joins_authors(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get("/api/books") + assert resp.status_code == 200 + body = resp.json() + assert body["total"] == 1 + book = body["books"][0] + assert book["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert book["author"] == "Neil Gaiman, Terry Pratchett" + + +def test_get_book_returns_authors_list(client: TestClient) -> None: + book = _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get(f"/api/books/{book['id']}") + assert resp.status_code == 200 + data = resp.json() + assert data["authors"] == ["Neil Gaiman", "Terry Pratchett"] + assert data["author"] == "Neil Gaiman, Terry Pratchett" + + # ── list ────────────────────────────────────────────────────────────────────── def test_list_books_empty(client: TestClient) -> None: @@ -1137,6 +1233,32 @@ def test_suggest_authors_deduplication(client: TestClient) -> None: assert resp.json()["suggestions"] == ["Frank Herbert"] +def test_search_author_prefix_matches_any_author(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + _create_book(client, title="Hogfather", author="Terry Pratchett") + resp = client.get("/api/books?q=author:Gaiman") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Good Omens"] + + +def test_search_unprefixed_matches_author(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + resp = client.get("/api/books?q=Pratchett") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Good Omens"] + + +def test_search_negated_author_excludes(client: TestClient) -> None: + _create_book(client, title="Good Omens", authors=["Terry Pratchett", "Neil Gaiman"]) + _create_book(client, title="Hogfather", author="Terry Pratchett") + resp = client.get("/api/books?q=-author:Gaiman") + assert resp.status_code == 200 + body = resp.json() + assert [b["title"] for b in body["books"]] == ["Hogfather"] + + def test_suggest_publishers_returns_matching(client: TestClient) -> None: _create_book(client, title="A", publisher="Ace Books") _create_book(client, title="B", publisher="Bantam Books") diff --git a/backend/tests/test_data_import.py b/backend/tests/test_data_import.py index c8529e1a..05ea8db4 100644 --- a/backend/tests/test_data_import.py +++ b/backend/tests/test_data_import.py @@ -50,9 +50,10 @@ def test_to_flat_row_nested_dict_raises() -> None: di._to_flat_row({"key": {"nested": 1}}) -def test_to_flat_row_nested_list_raises() -> None: - with pytest.raises(ValueError, match="error.importNestedValuesNotSupported"): - di._to_flat_row({"key": [1, 2]}) +def test_to_flat_row_list_is_preserved() -> None: + """List values are kept as-is so JSON author arrays survive flattening.""" + flat = di._to_flat_row({"author": ["Asimov, Isaac", "Robert Heinlein"]}) + assert flat["author"] == ["Asimov, Isaac", "Robert Heinlein"] # ── parse_upload ────────────────────────────────────────────────────────────── diff --git a/backend/tests/test_hygiene.py b/backend/tests/test_hygiene.py index ef065d63..8928b334 100644 --- a/backend/tests/test_hygiene.py +++ b/backend/tests/test_hygiene.py @@ -3,17 +3,22 @@ import pytest from fastapi.testclient import TestClient from pytest import MonkeyPatch -from sqlmodel import Session +from sqlmodel import Session, col, select -from app.models import Book, ReadingStatus, User +from app.models import Author, Book, BookAuthor, ReadingStatus, User from app.routers import hygiene as hygiene_router +from app.services.authors import normalize_author_list def _create_book(session: Session, user_id: int, **overrides: object) -> Book: - """Create a test book with sensible defaults.""" + """Create a test book with sensible defaults. + + The ``author`` override is applied through the author relation table, so an + empty string or None leaves the book without authors. + """ + raw_author: object = overrides.pop("author", "Test Author") defaults: dict = { "title": "Test Book", - "author": "Test Author", "isbn": None, "publisher": "Test Publisher", "published_year": 2020, @@ -30,6 +35,25 @@ def _create_book(session: Session, user_id: int, **overrides: object) -> Book: session.add(book) session.commit() session.refresh(book) + + names: list[str] = ( + [str(raw_author).strip()] + if isinstance(raw_author, str) and str(raw_author).strip() + else [] + ) + if names and book.id is not None: + for name in names: + author = session.exec( + select(Author).where(Author.user_id == user_id, Author.name == name) + ).first() + if author is None: + author = Author(user_id=user_id, name=name) + session.add(author) + session.flush() + assert author.id is not None + session.add(BookAuthor(book_id=book.id, author_id=author.id)) + session.commit() + session.refresh(book) return book @@ -154,8 +178,22 @@ def test_batch_update_single_field(self, client: TestClient, session: Session) - session.refresh(b1) session.refresh(b2) - assert b1.author == "New Author" - assert b2.author == "New Author" + author_names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == b1.id) + ).all() + ) + assert author_names == ["New Author"] + author_names = list( + session.exec( + select(Author.name) + .join(BookAuthor, col(BookAuthor.author_id) == col(Author.id)) + .where(BookAuthor.book_id == b2.id) + ).all() + ) + assert author_names == ["New Author"] def test_batch_update_too_many_ids(self, client: TestClient, session: Session) -> None: """Rejects more than 500 book IDs.""" @@ -393,7 +431,7 @@ def test_batch_update_database_error(self, client: TestClient, session: Session, """A database error during the update should return 500.""" from sqlalchemy.sql.dml import Update - b1 = _create_book(session, 1, title="B1", author="Old") + b1 = _create_book(session, 1, title="B1", published_year=2020) original_exec = session.exec def fake_exec(statement, *args, **kwargs): @@ -405,8 +443,8 @@ def fake_exec(statement, *args, **kwargs): resp = client.post("/api/hygiene/batch-update", json={ "book_ids": [b1.id], - "field": "author", - "value": "New", + "field": "published_year", + "value": 2021, }) assert resp.status_code == 500 assert resp.json()["detail"] == "Batch update failed due to a database error" diff --git a/docs/api/index.md b/docs/api/index.md index 53b8bee5..812ff4fe 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -46,8 +46,18 @@ curl -H "X-API-Key: YOUR_KEY_HERE" \ curl -X POST \ -H "X-API-Key: YOUR_KEY_HERE" \ -H "Content-Type: application/json" \ - -d '{"title": "The Great Gatsby", "author": "F. Scott Fitzgerald"}' \ + -d '{"title": "The Great Gatsby", "authors": ["F. Scott Fitzgerald"]}' \ http://localhost:8000/api/books +``` + +### Book author fields + +Book responses contain two author fields: + +- `author` — the **joined** string of all authors (e.g. `"Neil Gaiman, Terry Pratchett"`). Kept for backward compatibility with existing consumers. +- `authors` — the **list** of individual author names (e.g. `["Neil Gaiman", "Terry Pratchett"]`). + +When creating or updating a book you may send `authors` as a list, or the legacy `author` string (which is parsed on commas, tag-style). If both are sent, `authors` takes precedence. # Update reading status curl -X POST \ diff --git a/docs/guide/database-layout.md b/docs/guide/database-layout.md index db8e8e5b..8d96c83f 100644 --- a/docs/guide/database-layout.md +++ b/docs/guide/database-layout.md @@ -1,3 +1,372 @@ # Database Layout -> _Auto-generated from SQLModel metadata — run `npm run docs:gen-db` (or `uv run --directory backend python scripts/gen_db_docs.py`) to regenerate._ +> _Auto-generated from SQLModel metadata on 2026-08-24._ + +This page documents the LibrisLog database schema. It is intended for +developers who need to understand the data model, write queries, or extend +the application. + +```mermaid +erDiagram + + user ||--|{ apikey : "1:N" + user ||--|{ author : "1:N" + user ||--o{ book : "0..N" + user ||--|{ embed_token : "1:N" + user ||--|{ import_mapping : "1:N" + user ||--|| oidclink : "1:1" + user ||--|{ tag : "1:N" + user ||--|| usersettings : "1:1" + book ||--|{ book_author : "1:N" + author ||--|{ book_author : "1:N" + book ||--|{ book_tag : "1:N" + tag ||--|{ book_tag : "1:N" + book ||--|{ reading_progress : "1:N" + user ||--|{ reading_progress : "1:N" + + user { + integer id PK + varchar firstname + varchar lastname + varchar email UK + varchar role + varchar hashed_password + integer credentials_version + datetime created_at + datetime updated_at + } + + apikey { + integer id PK + integer user_id + varchar key_prefix + varchar key_hash UK + varchar key_encrypted + varchar description + datetime created_at + datetime last_used_at + datetime revoked_at + } + + author { + integer id PK + integer user_id + varchar name + datetime created_at + } + + book { + integer id PK + varchar title + varchar subtitle + varchar isbn + varchar cover_url + varchar publisher + integer published_year + integer page_count + varchar(2) language + varchar notes + varchar blurb + integer rating + varchar reading_status + varchar acquisition_status + integer user_id + datetime date_added + datetime date_started + datetime date_finished + } + + embed_token { + integer id PK + integer user_id + varchar(255) name + varchar token_prefix + varchar token_hash UK + varchar scopes + varchar allowed_origins + datetime expires_at + datetime last_used_at + datetime created_at + datetime revoked_at + } + + import_mapping { + integer id PK + integer user_id + varchar(255) name + varchar(64) schema_fingerprint + varchar source_fields_json + varchar mapping_json + datetime created_at + datetime updated_at + } + + oidclink { + integer id PK + integer user_id UK + varchar provider_id + varchar oidc_sub UK + varchar oidc_email + varchar oidc_name + datetime linked_at + } + + tag { + integer id PK + integer user_id + varchar name + datetime created_at + } + + usersettings { + integer id PK + integer user_id UK + varchar(10) language + varchar(64) timezone + varchar(20) theme + varchar(30) custom_theme + } + + book_author { + integer book_id PK + integer author_id PK + } + + book_tag { + integer book_id PK + integer tag_id PK + } + + reading_progress { + integer id PK + integer book_id + integer user_id + integer page + datetime created_at + datetime updated_at + } + +``` + +## Tables + +### `user` + +A user account. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `firstname` | `VARCHAR` | NOT NULL | | +| `lastname` | `VARCHAR` | NOT NULL | | +| `email` | `VARCHAR` | UNIQUE, NOT NULL | | +| `role` | `VARCHAR` | NOT NULL, INDEX | default `user` | +| `hashed_password` | `VARCHAR` | NOT NULL | | +| `credentials_version` | `INTEGER` | NOT NULL | default 0 | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + +### `apikey` + +API key for programmatic access. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `key_prefix` | `VARCHAR` | NOT NULL, INDEX | | +| `key_hash` | `VARCHAR` | UNIQUE, NOT NULL | | +| `key_encrypted` | `VARCHAR` | | | +| `description` | `VARCHAR` | | | +| `created_at` | `DATETIME` | | UTC | +| `last_used_at` | `DATETIME` | | UTC | +| `revoked_at` | `DATETIME` | | UTC | + +### `author` + +A user-specific author name that can be associated with books. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR` | NOT NULL, INDEX | | +| `created_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_author_user_id_name + +### `book` + +A book in the user's library. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `title` | `VARCHAR` | NOT NULL, INDEX | | +| `subtitle` | `VARCHAR` | | | +| `isbn` | `VARCHAR` | | | +| `cover_url` | `VARCHAR` | | | +| `publisher` | `VARCHAR` | | | +| `published_year` | `INTEGER` | | | +| `page_count` | `INTEGER` | NOT NULL | default 0 | +| `language` | `VARCHAR(2)` | | | +| `notes` | `VARCHAR` | | | +| `blurb` | `VARCHAR` | | | +| `rating` | `INTEGER` | | ≥ 1; ≤ 5 | +| `reading_status` | `VARCHAR` | NOT NULL, INDEX | default `want_to_read` | +| `acquisition_status` | `VARCHAR` | NOT NULL, INDEX | default `owned` | +| `user_id` | `INTEGER` | FK → user.id, INDEX | | +| `date_added` | `DATETIME` | INDEX | UTC | +| `date_started` | `DATETIME` | INDEX | UTC | +| `date_finished` | `DATETIME` | INDEX | UTC | + +**Unique constraint:** `(user_id, isbn)` — uq_book_user_id_isbn + +### `embed_token` + +A scoped embed token for iframe/dashboard integrations. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR(255)` | NOT NULL | | +| `token_prefix` | `VARCHAR` | NOT NULL, INDEX | | +| `token_hash` | `VARCHAR` | UNIQUE, NOT NULL | | +| `scopes` | `VARCHAR` | NOT NULL | default `embed:stats:read` | +| `allowed_origins` | `VARCHAR` | | | +| `expires_at` | `DATETIME` | | UTC | +| `last_used_at` | `DATETIME` | | UTC | +| `created_at` | `DATETIME` | | UTC | +| `revoked_at` | `DATETIME` | | UTC | + +### `import_mapping` + +A saved column-mapping configuration for data import. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR(255)` | NOT NULL | | +| `schema_fingerprint` | `VARCHAR(64)` | NOT NULL, INDEX | | +| `source_fields_json` | `VARCHAR` | NOT NULL | | +| `mapping_json` | `VARCHAR` | NOT NULL | | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_import_mapping_user_id_name + +### `oidclink` + +Links an OIDC identity to a local user account. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, UNIQUE, NOT NULL | | +| `provider_id` | `VARCHAR` | NOT NULL, INDEX | | +| `oidc_sub` | `VARCHAR` | UNIQUE, NOT NULL | | +| `oidc_email` | `VARCHAR` | | | +| `oidc_name` | `VARCHAR` | | | +| `linked_at` | `DATETIME` | | UTC | + +### `tag` + +A user-specific tag that can be applied to books. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `name` | `VARCHAR` | NOT NULL, INDEX | | +| `created_at` | `DATETIME` | | UTC | + +**Unique constraint:** `(user_id, name)` — uq_tag_user_id_name + +### `usersettings` + +Per-user settings such as language, timezone, and theme. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `user_id` | `INTEGER` | FK → user.id, UNIQUE, NOT NULL | | +| `language` | `VARCHAR(10)` | NOT NULL | default `en` | +| `timezone` | `VARCHAR(64)` | NOT NULL | default `UTC` | +| `theme` | `VARCHAR(20)` | NOT NULL | default `light` | +| `custom_theme` | `VARCHAR(30)` | | | + +### `book_author` + +Many-to-many association between books and authors. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `book_id` | `INTEGER` | PK, FK → book.id | | +| `author_id` | `INTEGER` | PK, FK → author.id | | + +### `book_tag` + +Many-to-many association between books and tags. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `book_id` | `INTEGER` | PK, FK → book.id | | +| `tag_id` | `INTEGER` | PK, FK → tag.id | | + +### `reading_progress` + +A page-number reading progress entry for a book. + +| Column | Type | Constraints | Notes | +|--------|------|-------------|-------| +| `id` | `INTEGER` | PK | Auto-increment | +| `book_id` | `INTEGER` | FK → book.id, NOT NULL, INDEX | | +| `user_id` | `INTEGER` | FK → user.id, NOT NULL, INDEX | | +| `page` | `INTEGER` | NOT NULL | ≥ 0 | +| `created_at` | `DATETIME` | | UTC | +| `updated_at` | `DATETIME` | | UTC | + + +## Enums + +### `AcquisitionStatus` + +| Value | Meaning | +|-------|---------| +| `owned` | Owned | +| `borrowed` | Borrowed | +| `digital_access` | Digital Access | +| `to_acquire` | To Acquire | + +### `ReadingStatus` + +| Value | Meaning | +|-------|---------| +| `want_to_read` | Want To Read | +| `currently_reading` | Currently Reading | +| `read` | Read | +| `did_not_finish` | Did Not Finish | + +### `UserRole` + +| Value | Meaning | +|-------|---------| +| `admin` | Admin | +| `user` | User | + + +## Conventions + +- **Timestamps** are stored as UTC via the `UtcDateTime` type decorator. + Values are stored as naive UTC in SQLite and returned as timezone-aware + `datetime` objects by the application. +- **Soft deletes** — `ApiKey` and `EmbedToken` use a `revoked_at` timestamp + instead of `DELETE`. Revoked entries are excluded from all queries. +- **Foreign keys** — all user-owned tables reference `user.id` via foreign + key constraints. Cascading behavior is handled in application code (not + at the database level). +- **Unique constraints** — compound constraints like `(user_id, isbn)` on + `book` and `(user_id, name)` on `tag` enforce per-user uniqueness without + restricting other users. diff --git a/docs/guide/using-librislog/import-export.md b/docs/guide/using-librislog/import-export.md index dd5fbe75..d5e01eb4 100644 --- a/docs/guide/using-librislog/import-export.md +++ b/docs/guide/using-librislog/import-export.md @@ -29,6 +29,12 @@ On mobile devices: If no search results are found, enter book details manually. Title, author, page count, and availability are required; all other fields are optional. +Authors can be added as multiple values: type a name and press **Enter** (or pick a suggestion) to add a chip. A book can have any number of authors. Commas inside an author name (e.g. `Asimov, Isaac`) are preserved — they are not treated as separators. + +### Search Import + +When a search source returns multiple authors for a book (e.g. Open Library, Google Books, or Hardcover), the app keeps them as a list and creates one author per entry. Sources that return a single combined string are split only on `;`, ` & `, or ` and ` — never on commas. + ## Data Export Export your entire library or subsets of data: @@ -76,6 +82,15 @@ When importing CSV, map source columns to LibrisLog fields: `acquisition_status` is required for imports. Map it to one of `owned`, `borrowed`, `digital_access`, or `to_acquire`; use a transform when the source file uses different names. +#### Authors are adaptive + +The `author` / `authors` field adapts to the source value: + +- **Array value** (e.g. a JSON `authors` list) → each array entry becomes a separate author. +- **String value** (e.g. a CSV cell) → normally becomes **one** author, and commas inside the name are preserved, so `"Asimov, Isaac"` stays a single author. To encode several authors in a single cell, separate them with `;`, ` & `, or ` and ` (e.g. `"Frank Herbert; Brian Herbert"`). This is how the CSV export writes the dedicated `authors` column, so exports round-trip losslessly. + +The import preview shows how each row's author value will be interpreted before you import. + ### Transform DSL Per-field Python expressions allow data transformation: diff --git a/docs/guide/using-librislog/library.md b/docs/guide/using-librislog/library.md index 76436e34..d8bdf321 100644 --- a/docs/guide/using-librislog/library.md +++ b/docs/guide/using-librislog/library.md @@ -54,6 +54,8 @@ Clicking a book opens the detail dialog/drawer showing: Use the "Add Book" button to manually enter book details. Fill in title, author, and optional fields like ISBN, publisher, page count, etc. +A book can have **multiple authors**: type a name and press **Enter** to add it as a chip. Authors are shown joined with "; " throughout the app, so names written last-name-first (e.g. `"Doe, Jane"`) stay unambiguous. + ### Import Search Search external sources for book metadata: diff --git a/docs/guide/using-librislog/search.md b/docs/guide/using-librislog/search.md index 6b457843..8e08d2e1 100644 --- a/docs/guide/using-librislog/search.md +++ b/docs/guide/using-librislog/search.md @@ -19,6 +19,8 @@ Use `:` to search in a single field. The field prefixes are always Use quotes for values that contain spaces: `title:"The Silmarillion"`. +The `author:` prefix matches **any** author assigned to a book — a book with multiple authors matches if any of them contains the search value. + ### Availability values The `availability` prefix matches the exact acquisition status. Accepted values include: diff --git a/frontend/e2e/fixtures/pages/add-book-modal.page.ts b/frontend/e2e/fixtures/pages/add-book-modal.page.ts index 006ec7cf..7ccbaf2a 100644 --- a/frontend/e2e/fixtures/pages/add-book-modal.page.ts +++ b/frontend/e2e/fixtures/pages/add-book-modal.page.ts @@ -16,6 +16,7 @@ export class AddBookModalPage { async fillAuthor(author: string) { const input = this.page.locator('[role="dialog"] input[name="author"], [role="dialog"] input[placeholder*="Author"]').first(); await input.fill(author); + await input.press('Enter'); } async clickSave() { diff --git a/frontend/e2e/fixtures/pages/book-drawer.page.ts b/frontend/e2e/fixtures/pages/book-drawer.page.ts index 4ef1ec92..f41f3d38 100644 --- a/frontend/e2e/fixtures/pages/book-drawer.page.ts +++ b/frontend/e2e/fixtures/pages/book-drawer.page.ts @@ -15,6 +15,7 @@ export class BookDrawerPage { async fillAuthor(author: string) { const input = this.page.locator('input[name="author"], input[placeholder*="Author"]').first(); await input.fill(author); + await input.press('Enter'); } async clickSave() { diff --git a/frontend/e2e/fixtures/seed-data.ts b/frontend/e2e/fixtures/seed-data.ts index a950a6dd..478f4e2d 100644 --- a/frontend/e2e/fixtures/seed-data.ts +++ b/frontend/e2e/fixtures/seed-data.ts @@ -8,6 +8,7 @@ export const SEED_USER = { export interface SeedBook { title: string; author: string; + authors?: string[]; isbn?: string; page_count?: number; reading_status: 'want_to_read' | 'currently_reading' | 'read' | 'did_not_finish'; @@ -27,7 +28,8 @@ export const SEED_BOOKS: SeedBook[] = [ { title: '1984', author: 'George Orwell', isbn: '9780451524935', reading_status: 'read', rating: 5, page_count: 328, date_started: '2024-10-01', date_finished: '2024-10-20' }, { title: 'Brave New World', author: 'Aldous Huxley', isbn: '9780060850524', reading_status: 'read', rating: 4, page_count: 311, date_started: '2024-09-01', date_finished: '2024-09-18' }, { title: 'Atlas Shrugged', author: 'Ayn Rand', reading_status: 'did_not_finish', page_count: 1168 }, - { title: 'Die Fragezeichen', author: 'Christoph Dittert', reading_status: 'want_to_read' }, - { title: 'Cars & Mercedes', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars,audi' }, - { title: 'Cars Only', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars' }, + { title: 'Die Fragezeichen', author: 'Christoph Dittert', reading_status: 'want_to_read', page_count: 96 }, + { title: 'Cars & Mercedes', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars,audi', page_count: 120 }, + { title: 'Cars Only', author: 'Jane Driver', reading_status: 'want_to_read', tags: 'cars', page_count: 100 }, + { title: 'Good Omens', author: 'Terry Pratchett, Neil Gaiman', authors: ['Terry Pratchett', 'Neil Gaiman'], isbn: '9780060853983', reading_status: 'want_to_read', page_count: 288 }, ]; diff --git a/frontend/e2e/fixtures/seed.api.ts b/frontend/e2e/fixtures/seed.api.ts index cfb81987..b1c30086 100644 --- a/frontend/e2e/fixtures/seed.api.ts +++ b/frontend/e2e/fixtures/seed.api.ts @@ -18,13 +18,17 @@ async function getCsrfToken(page: Page): Promise { export async function seedBooks(page: Page, books: SeedBook[]): Promise { for (const book of books) { const csrf = await getCsrfToken(page); - await page.request.post(bookApiPath(), { + const resp = await page.request.post(bookApiPath(), { data: book, headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrf, }, }); + // 409 = book already seeded by an earlier test in this suite; that's fine. + if (!resp.ok() && resp.status() !== 409) { + throw new Error(`Seeding book "${book.title}" failed: ${resp.status()} ${await resp.text()}`); + } } } diff --git a/frontend/e2e/specs/03-library-browsing.spec.ts b/frontend/e2e/specs/03-library-browsing.spec.ts index d4f4be17..0971e9c6 100644 --- a/frontend/e2e/specs/03-library-browsing.spec.ts +++ b/frontend/e2e/specs/03-library-browsing.spec.ts @@ -78,7 +78,10 @@ test.describe('Library Browsing', () => { await expect(availability).toHaveValue(''); await modal.getByLabel('Title *').fill('Digital E2E Book'); - await modal.getByRole('searchbox', { name: /Author/ }).fill('E2E Author'); + // The author field is a chip input: type and press Enter to add the author. + const authorInput = modal.getByRole('textbox', { name: /Author/ }); + await authorInput.fill('E2E Author'); + await authorInput.press('Enter'); await modal.getByLabel(/Pages/).fill('200'); await availability.selectOption('digital_access'); await modal.getByRole('button', { name: 'Add Book' }).click(); diff --git a/frontend/e2e/specs/05-edit-book.spec.ts b/frontend/e2e/specs/05-edit-book.spec.ts index e9a46280..89940acf 100644 --- a/frontend/e2e/specs/05-edit-book.spec.ts +++ b/frontend/e2e/specs/05-edit-book.spec.ts @@ -88,4 +88,41 @@ test.describe('Edit Book', () => { const checkedStar = page.locator('[role="dialog"] input[type="radio"]:checked'); await expect(checkedStar).toHaveAttribute('aria-label', /2 (star|Stern)/); }); + + test('5.4 add a second author in the edit drawer', async ({ page }) => { + const library = new LibraryPage(page); + await library.goto(); + await page.waitForTimeout(1000); + + await library.switchTab('want to read'); + await page.waitForTimeout(500); + + const cards = library.getBookCards(); + await expect(cards.first()).toBeVisible({ timeout: 5000 }); + + // Pick the Dune card (author: Frank Herbert). + const duneCard = cards.filter({ hasText: 'Dune' }).first(); + await expect(duneCard).toBeVisible({ timeout: 5000 }); + await duneCard.click(); + await page.waitForSelector('[role="dialog"]', { timeout: 5000 }); + + const editBtn = page.locator('[role="dialog"] button').filter({ hasText: 'Edit' }); + await editBtn.first().click(); + await page.waitForTimeout(500); + + const authorInput = page.locator('input[name="author"]'); + await expect(authorInput).toBeVisible({ timeout: 5000 }); + + // Add a second author chip by typing and pressing Enter. + await authorInput.fill('Brian Herbert'); + await authorInput.press('Enter'); + await expect(page.locator('text=Brian Herbert')).toBeVisible({ timeout: 5000 }); + + await page.locator('button[type="submit"]').click(); + await page.waitForTimeout(800); + + // Verify the joined author string appears in the library detail. + // Authors are returned sorted alphabetically: Brian Herbert, Frank Herbert. + await expect(page.getByText(/Brian Herbert; Frank Herbert/i)).toBeVisible({ timeout: 5000 }); + }); }); diff --git a/frontend/e2e/specs/08-statistics.spec.ts b/frontend/e2e/specs/08-statistics.spec.ts index 225aeb77..1fa43a5d 100644 --- a/frontend/e2e/specs/08-statistics.spec.ts +++ b/frontend/e2e/specs/08-statistics.spec.ts @@ -33,4 +33,24 @@ test.describe('Statistics', () => { await expect(page.getByText('The Great Gatsby').first()).toBeVisible(); await expect(page.getByText('Brave New World').first()).toBeVisible(); }); + + test('8.3 top authors reflect multi-author books', async ({ page }) => { + // Give Frank Herbert more books (Good Omens already counts Pratchett/Gaiman). + await seedBooks(page, [ + { title: 'Children of Dune', author: 'Frank Herbert', reading_status: 'read', rating: 4, page_count: 408, date_started: '2024-01-01', date_finished: '2024-01-20' }, + { title: 'God Emperor of Dune', author: 'Frank Herbert', reading_status: 'read', rating: 4, page_count: 496, date_started: '2024-02-01', date_finished: '2024-02-20' }, + ]); + + await page.goto('/statistics'); + await page.waitForTimeout(2000); + + // Frank Herbert now has the most books and must be the top author. + await expect(page.getByText(/Top Authors|Beliebteste Autoren/i)).toBeVisible(); + const topAuthorsCard = page + .locator('.card') + .filter({ has: page.getByRole('heading', { name: /Top Authors|Beliebteste Autoren/i }) }); + const frankCard = topAuthorsCard.locator('.rounded-xl').filter({ hasText: 'Frank Herbert' }).first(); + await expect(frankCard).toBeVisible({ timeout: 5000 }); + await expect(frankCard.getByText('#1', { exact: true })).toBeVisible(); + }); }); diff --git a/frontend/e2e/specs/09-data-import.spec.ts b/frontend/e2e/specs/09-data-import.spec.ts index 6364d8f5..01d82bc5 100644 --- a/frontend/e2e/specs/09-data-import.spec.ts +++ b/frontend/e2e/specs/09-data-import.spec.ts @@ -180,4 +180,99 @@ test.describe('Data Import', () => { expect(books).toHaveLength(1); expect(books[0].acquisition_status).toBe('to_acquire'); }); + + test('9.6 JSON author array maps to multiple authors', async ({ page }) => { + await deleteAllBooks(page); + await page.goto('/data?tab=import'); + await page.waitForTimeout(1000); + + const JSON_DATA = JSON.stringify([ + { + title: 'Good Omens', + authors: ['Terry Pratchett', 'Neil Gaiman'], + isbn: '9780060853983', + pages: 288, + status: 'want_to_read', + availability: 'owned', + }, + ]); + + await page.locator('input[type="file"]').setInputFiles({ + name: 'test-books.json', + mimeType: 'application/json', + buffer: Buffer.from(JSON_DATA), + }); + + await page.locator('button').filter({ hasText: 'Parse file' }).click(); + await page.waitForTimeout(2000); + + await page.locator('select[name="mapping-target-title"]').selectOption('title'); + await page.locator('select[name="mapping-target-author"]').selectOption('authors'); + await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); + await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); + await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); + await page.locator('select[name="mapping-target-acquisition_status"]').selectOption('availability'); + + await page.locator('button').filter({ hasText: 'Generate' }).click(); + await page.waitForTimeout(2000); + + await page.locator('button').filter({ hasText: 'Simulate' }).click(); + await page.waitForTimeout(2000); + + await expect(page.locator('body')).toContainText('Validation passed.', { timeout: 10000 }); + + await page.locator('button.btn-secondary.btn-sm').filter({ hasText: 'Import now' }).click(); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).waitFor({ state: 'visible', timeout: 5000 }); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).click(); + await page.waitForTimeout(2000); + + const response = await page.request.get('/api/books?q=Good%20Omens'); + const books = (await response.json()).books; + expect(books).toHaveLength(1); + expect(books[0].authors).toEqual(['Neil Gaiman', 'Terry Pratchett']); + }); + + test('9.7 CSV author cell with a comma stays a single author', async ({ page }) => { + await deleteAllBooks(page); + await page.goto('/data?tab=import'); + await page.waitForTimeout(1000); + + const CSV_COMMA = `title,author,isbn,pages,status,availability +"Fahrenheit 451","Bradbury, Ray","9781451673319",249,want_to_read,owned`; + + await page.locator('input[type="file"]').setInputFiles({ + name: 'test-books.csv', + mimeType: 'text/csv', + buffer: Buffer.from(CSV_COMMA), + }); + + await page.locator('button').filter({ hasText: 'Parse file' }).click(); + await page.waitForTimeout(2000); + + await page.locator('select[name="mapping-target-title"]').selectOption('title'); + await page.locator('select[name="mapping-target-author"]').selectOption('author'); + await page.locator('select[name="mapping-target-isbn"]').selectOption('isbn'); + await page.locator('select[name="mapping-target-page_count"]').selectOption('pages'); + await page.locator('select[name="mapping-target-reading_status"]').selectOption('status'); + await page.locator('select[name="mapping-target-acquisition_status"]').selectOption('availability'); + + await page.locator('button').filter({ hasText: 'Generate' }).click(); + await page.waitForTimeout(2000); + + await page.locator('button').filter({ hasText: 'Simulate' }).click(); + await page.waitForTimeout(2000); + + await expect(page.locator('body')).toContainText('Validation passed.', { timeout: 10000 }); + + await page.locator('button.btn-secondary.btn-sm').filter({ hasText: 'Import now' }).click(); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).waitFor({ state: 'visible', timeout: 5000 }); + await page.locator('dialog.modal-open .btn-secondary').filter({ hasText: 'Import now' }).click(); + await page.waitForTimeout(2000); + + const response = await page.request.get('/api/books?q=Fahrenheit'); + const books = (await response.json()).books; + expect(books).toHaveLength(1); + expect(books[0].authors).toEqual(['Bradbury, Ray']); + expect(books[0].author).toBe('Bradbury, Ray'); + }); }); diff --git a/frontend/src/lib/components/AddBookModal.svelte b/frontend/src/lib/components/AddBookModal.svelte index e2a67784..9bb46695 100644 --- a/frontend/src/lib/components/AddBookModal.svelte +++ b/frontend/src/lib/components/AddBookModal.svelte @@ -28,7 +28,7 @@ // Manual form state let title = $state(''); let subtitle = $state(''); - let author = $state(''); + let authors = $state([]); let isbn = $state(''); let publisher = $state(''); let published_year = $state(''); @@ -46,7 +46,7 @@ function reset() { title = ''; subtitle = ''; - author = ''; + authors = []; isbn = ''; publisher = ''; published_year = ''; @@ -64,7 +64,7 @@ async function submitManual() { if (!title.trim()) return; - if (!author.trim()) return; + if (authors.length === 0) return; if (!page_count) return; if (!acquisitionStatus) return; submitting = true; @@ -72,7 +72,7 @@ const book = await api.books.create({ title: title.trim(), subtitle: subtitle || null, - author: author.trim(), + authors, isbn: isbn || null, publisher: publisher || null, published_year: published_year ? parseInt(published_year) : null, @@ -149,11 +149,12 @@
- api.books.suggestions.authors(q)} />