Skip to content

feat: add library management with Calibre support#61

Draft
OGKevin wants to merge 6 commits intomainfrom
feat/lib-management
Draft

feat: add library management with Calibre support#61
OGKevin wants to merge 6 commits intomainfrom
feat/lib-management

Conversation

@OGKevin
Copy link
Copy Markdown
Owner

@OGKevin OGKevin commented Nov 5, 2025

Implement a trait-based library abstraction layer that enables reading
book metadata from Calibre databases. Add unified error handling and
Tauri commands for library operations.

  • Create BookLibrary trait for extensible library backend support
  • Implement CalibreLibrary with thread-safe database access
  • Add AppError enum for consistent error handling across modules
  • Implement LibraryState for managing open library instances
  • Add Tauri commands: library_open, library_get_all_books,
    library_get_book, library_get_book_count
  • Add comprehensive tests for library operations and error cases
  • Organize imports alphabetically across modules (style)

Change-Id: 4050ec25b8f88b0eb211f6691a725e6d
Change-Id-Short: vzuzlnxuorkr

@OGKevin OGKevin force-pushed the feat/lib-management branch from 89e6460 to 35b1dbe Compare November 5, 2025 18:02
Implement a trait-based library abstraction layer that enables reading
book metadata from Calibre databases. Add unified error handling and
Tauri commands for library operations.

- Create BookLibrary trait for extensible library backend support
- Implement CalibreLibrary with thread-safe database access
- Add AppError enum for consistent error handling across modules
- Implement LibraryState for managing open library instances
- Add Tauri commands: library_open, library_get_all_books,
  library_get_book, library_get_book_count
- Add comprehensive tests for library operations and error cases
- Organize imports alphabetically across modules (style)

Change-Id: 4050ec25b8f88b0eb211f6691a725e6d
Change-Id-Short: vzuzlnxuorkr
Add comprehensive book cover functionality with library-agnostic
architecture supporting streaming, caching, and proper test coverage.
Backend uses trait-based design for easy library implementation while
frontend provides React context and hooks for efficient cover display.

- Add library-agnostic Book model with only frontend-used fields
- Implement BookLibrary trait with get_book_cover method
- Add library_stream_book_covers Tauri command for async streaming
- Create BookCoverContext for global cover caching
- Add useStreamBookCovers and useBookCover hooks
- Convert BookCoverSkeleton to BookCover component with loading states
- Integrate BookCoverProvider into PageWrapperFactory
- Add comprehensive Rust and TypeScript tests (64 + 275 passing)
- Update BooksTable to display actual book covers
- Export Identifier type from calibre_db crate
- Fix test utilities to include required BookCoverProvider wrapper

Change-Id: 8321e2867e1dc3ef8aaff22bb585728c
Change-Id-Short: rwxylxrtslym
@OGKevin OGKevin force-pushed the feat/lib-management branch from 91872c8 to 07155ee Compare November 6, 2025 08:13
@OGKevin
Copy link
Copy Markdown
Owner Author

OGKevin commented Nov 6, 2025

@copilot Can you have a look at the failing CI.

  • The code must be linted and the violations fixed.
  • I believe the frontend test might have a race condition
- useLibrary " can manually load books after opening library

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 2

      258 |     });
      259 |
    > 260 |     expect(libraryApi.libraryGetAllBooks).toHaveBeenCalledTimes(1);
          |                                           ^
      261 |   });
      262 |
      263 |   describe("Bug fix: Backend calls should be made when opening library with known path", () => {

      at Object.toHaveBeenCalledTimes (src/__tests__/hooks/useLibrary.test.tsx:260:43)

As this test passes locally. Investigate if this is possible and attempt to address it.

  • flake.nix needs to be updated with new hashes for cargo and pnpm, as the dependency changed.

Make a separate commit for each bullet point.

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 6, 2025

@OGKevin I've opened a new pull request, #62, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits November 6, 2025 21:01
…62)

* Initial plan

* fix: address Rust linting violations

- Add Send/Sync implementations for CalibreDatabase
- Remove unused Schema validation methods
- Fix clippy::useless_vec warning in tests
- Apply cargo fmt formatting

* fix: address frontend linting violations

- Remove unused useRef import from BooksTable
- Add required blank lines before return statements
- Add blank lines after if statements per eslint rules

* fix: resolve race condition in useLibrary test

Prevent auto-load effect from triggering when manually opening a library.
The auto-load effect should only trigger when loading a saved path from
localStorage on mount, not when calling openLibrary() manually.

This fixes the flaky test 'can manually load books after opening library'
which was sometimes seeing 2 calls to libraryGetAllBooks instead of 1.

* chore: final update

Co-authored-by: OGKevin <17928966+OGKevin@users.noreply.github.com>

* fix: clarify safety comment and fix lifetime elision warning

- Update safety comment to clarify current read-only operations
- Add note about reassessing if write operations are introduced
- Fix mismatched_lifetime_syntaxes warning by using explicit named lifetime
  in DatabaseConnection::prepare method

* refactor: improve BookBuilder with database-aware pattern

Refactored the BookBuilder pattern to eliminate mid-construction
state exposure and improve encapsulation. The builder now holds an
optional database reference and uses boolean flags to control which
relations to fetch during build().

- Made BookBuilder database-aware with optional &DatabaseConnection
- Added 9 boolean flags to control relation fetching (authors,
  publishers, tags, series, comments, rating, formats, identifiers,
  languages)
- Moved relation-fetching logic into build() method
- Exposed fetch_book_*() functions as public query API
- Added with_db() and fetch_all() builder methods
- Updated get_book() and all_books() to use new pattern
- Removed get_id() method that broke encapsulation
- Added comprehensive doc tests with lifetime design explanation
- All tests pass: 11 unit tests + 17 integration tests

* refactor: implement ReadOnlyDatabase trait with compile-time safeguards

- Add ReadOnlyDatabase trait to mark database types as read-only
- Remove execute() method from DatabaseConnection to prevent write operations
- Add compile-time query validation in debug builds to ensure only SELECT queries
- Add comprehensive tests for read-only constraint validation
- Update safety documentation with ReadOnlyDatabase marker trait
- Update CI workflow to use --workspace flag for consistency

Changes ensure type-safe read-only guarantee at compile time while maintaining
runtime validation in debug builds. Write operations now require explicit removal
of ReadOnlyDatabase implementation.

* refactor: enhance CalibreDbError handling and implement ReadOnlyDatabase trait for improved database operations

* refactor: enhance build method documentation with performance trade-offs and query strategy rationale

* refactor: enhance BookBuilder to require a database reference for improved relation fetching

* refactor: remove redundant read-only query validation

The read-only validation in DatabaseConnection was unnecessary
defensive programming. Since only the ReadOnlyDatabase trait is
exposed to users (with SELECT-only methods), and DatabaseConnection
is an internal implementation detail, the type system already
enforces read-only semantics at the public API level.

- Removed is_read_only_query() function and helpers
- Removed comment/string literal stripping logic
- Simplified prepare() method
- Removed validation-related tests
- Kept essential connection and query tests

* refactor: improve code readability and address review feedback

- Extract long tuple type into BookRow struct with named fields
- Add named constants for SQL column indices (IDX_ID, IDX_TITLE, etc.)
- Remove redundant list_books() method from ReadOnlyDatabase trait
- Update safety comment to suggest ReadWriteDatabase trait for future writes
- Replace Into::into with explicit CalibreDbError::from for clarity
- Simplify all_books() by using BookRow::from_row helper

These changes improve code maintainability and make the codebase easier to understand.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: OGKevin <17928966+OGKevin@users.noreply.github.com>
Apply formatting and style improvements across the codebase,
including fixing lifetime syntax warnings in Rust code and
improving code consistency in TypeScript and Markdown files.

- Fix `BookBuilder` return type to explicitly show lifetime `<'_>`
- Reformat Rust function signatures for better readability
- Apply consistent line breaks in TypeScript test files
- Simplify destructured imports in frontend components
- Normalize Markdown formatting with consistent spacing
- Simplify cargo commands in devenv by using workspace root
- Fix various line length and indentation issues

Change-Id: 85e497a7abb33f4c592cd26899437264
Change-Id-Short: rulvqspspoow
Update Cargo.lock files and flake dependencies to latest versions.
Remove cargoRoot configuration from flake.nix as it's no longer
needed with the workspace structure.

- Delete src-tauri/Cargo.lock and calibre_db/Cargo.lock from VCS
- Update quote crate to 1.0.42 in Cargo.lock
- Update nixpkgs to latest version (1762233356)
- Update cargoHash in flake.nix due to dependency changes
- Remove unused cargoRoot and buildAndTestSubdir from flake.nix

Change-Id: 680869581f6438bb27258817c61b7bf1
Change-Id-Short: trzrtquryktv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants