diff --git a/backend/src/chitai/services/dependencies.py b/backend/src/chitai/services/dependencies.py index 310fb9e..63e5b6f 100644 --- a/backend/src/chitai/services/dependencies.py +++ b/backend/src/chitai/services/dependencies.py @@ -14,7 +14,7 @@ from advanced_alchemy.extensions.litestar.providers import ( from advanced_alchemy.exceptions import NotFoundError from advanced_alchemy.filters import CollectionFilter, StatementFilter from advanced_alchemy.service import FilterTypeT -from sqlalchemy.orm import selectinload, with_loader_criteria +from sqlalchemy.orm import selectinload from sqlalchemy.ext.asyncio import AsyncSession from litestar import Request from litestar.params import Dependency, Parameter @@ -51,8 +51,19 @@ from chitai.services.filters.book import ( async def provide_book_service( - db_session: AsyncSession, current_user: m.User | None = None + db_session: AsyncSession, current_user: m.User = Dependency(skip_validation=True) ) -> AsyncGenerator[BookService, None]: + """ + Provide a BookService with per-user data scoped to the caller. + + `current_user` is a required dependency, not an optional argument. It used to + default to None with the scoping below wrapped in `if current_user:` — and + when it was not injected, that block was silently skipped, so + `Book.progress_records` and `Book.list_links` loaded *every* user's rows. + `Book.progress` returns `progress_records[0]`, so one user could see another + user's reading position; the shelf checkboxes leaked the same way. Failing + loudly on a missing user is the point of the change. + """ load = [ selectinload(m.Book.author_links).selectinload(m.BookAuthorLink.author), selectinload(m.Book.tag_links).selectinload(m.BookTagLink.tag), @@ -60,31 +71,26 @@ async def provide_book_service( m.Book.files, m.Book.identifiers, m.Book.series, + # Reading progress, restricted to the caller. + # + # The restriction lives on the relationship via .and_() rather than in a + # separate with_loader_criteria(). advanced_alchemy's + # get_abstract_loader_options() keeps only _AbstractLoad, + # InstrumentedAttribute, RelationshipProperty and "*" entries and drops + # everything else — and with_loader_criteria() is none of those, so the + # previous criteria were discarded before reaching a query. A + # selectinload() carrying its own .and_() survives that filter. + selectinload( + m.Book.progress_records.and_(m.BookProgress.user_id == current_user.id) + ), + # Bookshelf membership, restricted to the caller + selectinload( + m.Book.list_links.and_( + m.BookListLink.book_list.has(m.BookList.user_id == current_user.id) + ) + ).selectinload(m.BookListLink.book_list), ] - # Load in specific user-book data - if current_user: - # Load progress data - load.extend( - [ - selectinload(m.Book.progress_records), - with_loader_criteria( - m.BookProgress, m.BookProgress.user_id == current_user.id - ), - ] - ) - - # Load shelf data - load.extend( - [ - selectinload(m.Book.list_links).selectinload(m.BookListLink.book_list), - with_loader_criteria( - m.BookListLink, - m.Book.lists.any(m.BookList.user_id == current_user.id), - ), - ] - ) - provider_func = create_service_provider( BookService, load=load,