From 17a6e7df0cdcc9a00923cd29c4c55fde6dec6d68 Mon Sep 17 00:00:00 2001 From: Mason Fox Date: Thu, 19 Mar 2026 09:41:03 -0400 Subject: [PATCH 1/4] fix: invalidate shelf cache when book status changes When a book's status was changed on /books/:id, the shelf pages at /shelves/:id would show stale status for up to 30 seconds until the React Query cache expired. This was because invalidateBookQueries() invalidated book, session, progress, dashboard, and library queries, but did not invalidate shelf queries. Changes: - Modified invalidateBookQueries() to check React Query cache for the book's shelves and invalidate those shelf caches - Uses cache-based approach: if shelves are cached, invalidates only affected shelves (surgical); otherwise invalidates all shelves (nuclear) to ensure correctness - Removed misleading comment in status route about SessionService handling cache invalidation All 4007 tests pass. --- app/api/books/[id]/status/route.ts | 2 -- hooks/useBookStatus.ts | 28 +++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/app/api/books/[id]/status/route.ts b/app/api/books/[id]/status/route.ts index 23b19ddf..095fcc5f 100644 --- a/app/api/books/[id]/status/route.ts +++ b/app/api/books/[id]/status/route.ts @@ -66,8 +66,6 @@ export async function POST(request: NextRequest, props: { params: Promise<{ id: const result = await sessionService.updateStatus(bookId, statusData); - // Note: Cache invalidation handled by SessionService.invalidateCache() - // Return full result if session was archived, otherwise just the session if (result.sessionArchived) { return NextResponse.json({ diff --git a/hooks/useBookStatus.ts b/hooks/useBookStatus.ts index 0bcea6e2..0bb23062 100644 --- a/hooks/useBookStatus.ts +++ b/hooks/useBookStatus.ts @@ -38,15 +38,37 @@ function requiresArchiveConfirmation( /** * Invalidates all queries related to a book * Exported to allow reuse in components that make direct API calls + * + * Also invalidates shelf caches for shelves containing this book to ensure + * shelf pages reflect updated book status immediately. */ export function invalidateBookQueries(queryClient: any, bookId: string): void { - queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(parseInt(bookId)) }); - queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(parseInt(bookId)) }); - queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(parseInt(bookId)) }); + const numericBookId = parseInt(bookId); + + // Invalidate book-related queries + queryClient.invalidateQueries({ queryKey: queryKeys.book.detail(numericBookId) }); + queryClient.invalidateQueries({ queryKey: queryKeys.sessions.byBook(numericBookId) }); + queryClient.invalidateQueries({ queryKey: queryKeys.progress.byBook(numericBookId) }); queryClient.invalidateQueries({ queryKey: queryKeys.dashboard.all() }); queryClient.invalidateQueries({ queryKey: queryKeys.library.books() }); queryClient.invalidateQueries({ queryKey: queryKeys.readNext.base() }); + // Invalidate shelves containing this book + // Try to get shelves from cache; if available, invalidate only those shelves (surgical) + // If not cached, invalidate all shelves (nuclear) to ensure correctness + const bookShelvesKey = queryKeys.book.shelves(numericBookId); + const cachedShelves = queryClient.getQueryData(bookShelvesKey) as { success: boolean; data: Array<{ id: number }> } | undefined; + + if (cachedShelves?.data && cachedShelves.data.length > 0) { + // Surgical: Invalidate only affected shelves + cachedShelves.data.forEach((shelf: { id: number }) => { + queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) }); + }); + } else { + // Nuclear: Invalidate all shelves to be safe + queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() }); + } + // Clear entire LibraryService cache to ensure status changes reflect across all filters libraryService.clearCache(); } From 3b87ae9b1c19c2063924e4486855109c7d70b986 Mon Sep 17 00:00:00 2001 From: Mason Fox Date: Thu, 19 Mar 2026 09:47:50 -0400 Subject: [PATCH 2/4] fix: add type safety, logging, and test coverage for shelf cache invalidation - Replace type assertion with runtime type guard for safer cache validation - Add debug logging for surgical vs nuclear invalidation paths - Add 6 comprehensive tests covering all cache invalidation scenarios - All 4013 tests passing Addresses @review feedback: test coverage, type safety, and observability --- __tests__/hooks/useBookStatus.test.ts | 113 +++++++++++++++++++++++++- hooks/useBookStatus.ts | 26 +++++- 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/__tests__/hooks/useBookStatus.test.ts b/__tests__/hooks/useBookStatus.test.ts index 19a533f2..0e283936 100644 --- a/__tests__/hooks/useBookStatus.test.ts +++ b/__tests__/hooks/useBookStatus.test.ts @@ -1,7 +1,9 @@ import { test, expect, describe, beforeEach, afterEach, vi } from 'vitest'; import { renderHook, waitFor, act } from "../test-utils"; -import { useBookStatus } from "@/hooks/useBookStatus"; +import { useBookStatus, invalidateBookQueries } from "@/hooks/useBookStatus"; import type { Book } from "@/hooks/useBookDetail"; +import { QueryClient } from '@tanstack/react-query'; +import { queryKeys } from '@/lib/query-keys'; const originalFetch = global.fetch; @@ -387,4 +389,113 @@ describe("useBookStatus", () => { expect(result.current.selectedStatus).toBe("reading"); }); }); + + describe("invalidateBookQueries", () => { + let queryClient: QueryClient; + let invalidateSpy: any; + + beforeEach(() => { + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + }, + }); + invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries'); + }); + + afterEach(() => { + invalidateSpy.mockRestore(); + }); + + test("should invalidate all book-related queries", () => { + const bookId = '123'; + + invalidateBookQueries(queryClient, bookId); + + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.book.detail(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.sessions.byBook(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.progress.byBook(123) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.dashboard.all() }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.library.books() }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.readNext.base() }); + }); + + test("should invalidate shelf caches surgically when cached shelves available", () => { + const bookId = '123'; + + // Mock cached shelves + queryClient.setQueryData(queryKeys.book.shelves(123), { + success: true, + data: [{ id: 1 }, { id: 2 }, { id: 3 }] + }); + + invalidateBookQueries(queryClient, bookId); + + // Verify surgical invalidation for each shelf + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(1) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(2) }); + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.byId(3) }); + + // Verify nuclear invalidation was NOT called + expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + + test("should invalidate all shelves when cache unavailable", () => { + const bookId = '123'; + + // No cached shelves - cache unavailable + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + + // Verify surgical invalidation was NOT called + expect(invalidateSpy).not.toHaveBeenCalledWith( + expect.objectContaining({ + queryKey: expect.arrayContaining([expect.stringContaining('shelf'), 'byId']) + }) + ); + }); + + test("should invalidate all shelves when cache has no shelves", () => { + const bookId = '123'; + + // Mock cached shelves with empty array + queryClient.setQueryData(queryKeys.book.shelves(123), { + success: true, + data: [] + }); + + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation (fallback for empty shelves) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + + test("should handle invalid cache data gracefully", () => { + const bookId = '123'; + + // Mock invalid cache structure + queryClient.setQueryData(queryKeys.book.shelves(123), { + invalid: 'structure' + }); + + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation (fallback for invalid data) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + + test("should handle null cache data gracefully", () => { + const bookId = '123'; + + // Mock null cache + queryClient.setQueryData(queryKeys.book.shelves(123), null); + + invalidateBookQueries(queryClient, bookId); + + // Verify nuclear invalidation + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + }); + }); }); diff --git a/hooks/useBookStatus.ts b/hooks/useBookStatus.ts index 0bb23062..7c1d1500 100644 --- a/hooks/useBookStatus.ts +++ b/hooks/useBookStatus.ts @@ -57,15 +57,37 @@ export function invalidateBookQueries(queryClient: any, bookId: string): void { // Try to get shelves from cache; if available, invalidate only those shelves (surgical) // If not cached, invalidate all shelves (nuclear) to ensure correctness const bookShelvesKey = queryKeys.book.shelves(numericBookId); - const cachedShelves = queryClient.getQueryData(bookShelvesKey) as { success: boolean; data: Array<{ id: number }> } | undefined; + const cachedShelves = queryClient.getQueryData(bookShelvesKey); - if (cachedShelves?.data && cachedShelves.data.length > 0) { + // Type guard to validate cached shelves structure + const isValidShelvesData = (data: unknown): data is { success: boolean; data: Array<{ id: number }> } => { + return ( + data !== null && + typeof data === 'object' && + 'data' in data && + Array.isArray((data as any).data) + ); + }; + + if (isValidShelvesData(cachedShelves) && cachedShelves.data.length > 0) { // Surgical: Invalidate only affected shelves + if (typeof console !== 'undefined' && console.debug) { + console.debug( + `[useBookStatus] Surgical shelf invalidation for book ${numericBookId}:`, + cachedShelves.data.map(s => s.id).join(', ') + ); + } cachedShelves.data.forEach((shelf: { id: number }) => { queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) }); }); } else { // Nuclear: Invalidate all shelves to be safe + if (typeof console !== 'undefined' && console.debug) { + console.debug( + `[useBookStatus] Nuclear shelf invalidation for book ${numericBookId}`, + cachedShelves ? '(no shelves in cache)' : '(cache unavailable)' + ); + } queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() }); } From 0e525ddca8164840bbb9fa6082ed5ef30330511e Mon Sep 17 00:00:00 2001 From: Mason Fox Date: Thu, 19 Mar 2026 10:03:53 -0400 Subject: [PATCH 3/4] fix: implement @review feedback (iteration 1) - Add logger for structured logging (replace console.debug) - Fix empty shelves logic to treat [] as no-op - Update test assertions for better verification - Update empty shelves test expectation Fixes suggested by GitHub Copilot code review. --- __tests__/hooks/useBookStatus.test.ts | 30 ++++++++++++++++------- hooks/useBookStatus.ts | 34 ++++++++++++++------------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/__tests__/hooks/useBookStatus.test.ts b/__tests__/hooks/useBookStatus.test.ts index 0e283936..55fd40cf 100644 --- a/__tests__/hooks/useBookStatus.test.ts +++ b/__tests__/hooks/useBookStatus.test.ts @@ -449,18 +449,21 @@ describe("useBookStatus", () => { // Verify nuclear invalidation expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); - // Verify surgical invalidation was NOT called - expect(invalidateSpy).not.toHaveBeenCalledWith( - expect.objectContaining({ - queryKey: expect.arrayContaining([expect.stringContaining('shelf'), 'byId']) - }) + // Verify no surgical (specific shelf ID) invalidations occurred + const surgicalCalls = invalidateSpy.mock.calls.filter( + (call: any) => + Array.isArray(call[0]?.queryKey) && + call[0].queryKey[0] === 'shelf' && + call[0].queryKey.length === 2 && + typeof call[0].queryKey[1] === 'number' ); + expect(surgicalCalls).toHaveLength(0); }); - test("should invalidate all shelves when cache has no shelves", () => { + test("should NOT invalidate shelves when book is on no shelves", () => { const bookId = '123'; - // Mock cached shelves with empty array + // Mock cached shelves with empty array (book on zero shelves) queryClient.setQueryData(queryKeys.book.shelves(123), { success: true, data: [] @@ -468,8 +471,17 @@ describe("useBookStatus", () => { invalidateBookQueries(queryClient, bookId); - // Verify nuclear invalidation (fallback for empty shelves) - expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + // Should NOT trigger nuclear invalidation (performance optimization) + expect(invalidateSpy).not.toHaveBeenCalledWith({ queryKey: queryKeys.shelf.base() }); + + // Should NOT invalidate any specific shelves (none exist) + const surgicalCalls = invalidateSpy.mock.calls.filter( + (call: any) => + Array.isArray(call[0]?.queryKey) && + call[0].queryKey[0] === 'shelf' && + call[0].queryKey.length === 2 + ); + expect(surgicalCalls).toHaveLength(0); }); test("should handle invalid cache data gracefully", () => { diff --git a/hooks/useBookStatus.ts b/hooks/useBookStatus.ts index 7c1d1500..bb1ac1a9 100644 --- a/hooks/useBookStatus.ts +++ b/hooks/useBookStatus.ts @@ -7,6 +7,8 @@ import { getLogger } from "@/lib/logger"; import { bookApi, ApiError } from "@/lib/api"; import { libraryService } from "@/lib/library-service"; +const logger = getLogger().child({ hook: 'useBookStatus' }); + interface ProgressEntry { id: number; currentPage: number; @@ -69,25 +71,25 @@ export function invalidateBookQueries(queryClient: any, bookId: string): void { ); }; - if (isValidShelvesData(cachedShelves) && cachedShelves.data.length > 0) { - // Surgical: Invalidate only affected shelves - if (typeof console !== 'undefined' && console.debug) { - console.debug( - `[useBookStatus] Surgical shelf invalidation for book ${numericBookId}:`, - cachedShelves.data.map(s => s.id).join(', ') + if (isValidShelvesData(cachedShelves)) { + // Cache is valid - use surgical approach + if (cachedShelves.data.length > 0) { + // Book is on some shelves - invalidate only those + logger.debug( + { bookId: numericBookId, shelfIds: cachedShelves.data.map(s => s.id) }, + 'Surgical shelf invalidation' ); + cachedShelves.data.forEach((shelf: { id: number }) => { + queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) }); + }); } - cachedShelves.data.forEach((shelf: { id: number }) => { - queryClient.invalidateQueries({ queryKey: queryKeys.shelf.byId(shelf.id) }); - }); + // else: Book is on zero shelves (data.length === 0) - no-op, nothing to invalidate } else { - // Nuclear: Invalidate all shelves to be safe - if (typeof console !== 'undefined' && console.debug) { - console.debug( - `[useBookStatus] Nuclear shelf invalidation for book ${numericBookId}`, - cachedShelves ? '(no shelves in cache)' : '(cache unavailable)' - ); - } + // Cache unavailable or invalid - invalidate all shelves to be safe + logger.debug( + { bookId: numericBookId, cacheState: cachedShelves === null ? 'null' : 'invalid' }, + 'Nuclear shelf invalidation (cache unavailable)' + ); queryClient.invalidateQueries({ queryKey: queryKeys.shelf.base() }); } From 9e221f58677ea2f37a8d55e9ab3ad281cd08a797 Mon Sep 17 00:00:00 2001 From: Mason Fox Date: Thu, 19 Mar 2026 10:19:39 -0400 Subject: [PATCH 4/4] fix: implement @review feedback (iteration 2) - Use createTestQueryClient() for consistent test configuration - Type invalidateBookQueries queryClient parameter as QueryClient Addresses Copilot suggestions: 1. Test configuration consistency - use helper that disables caching 2. Type safety - replace 'any' with proper QueryClient type Co-authored-by: GitHub Copilot --- __tests__/hooks/useBookStatus.test.ts | 8 ++------ hooks/useBookStatus.ts | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/__tests__/hooks/useBookStatus.test.ts b/__tests__/hooks/useBookStatus.test.ts index 55fd40cf..52a3cb51 100644 --- a/__tests__/hooks/useBookStatus.test.ts +++ b/__tests__/hooks/useBookStatus.test.ts @@ -1,5 +1,5 @@ import { test, expect, describe, beforeEach, afterEach, vi } from 'vitest'; -import { renderHook, waitFor, act } from "../test-utils"; +import { renderHook, waitFor, act, createTestQueryClient } from "../test-utils"; import { useBookStatus, invalidateBookQueries } from "@/hooks/useBookStatus"; import type { Book } from "@/hooks/useBookDetail"; import { QueryClient } from '@tanstack/react-query'; @@ -395,11 +395,7 @@ describe("useBookStatus", () => { let invalidateSpy: any; beforeEach(() => { - queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false }, - }, - }); + queryClient = createTestQueryClient(); invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries'); }); diff --git a/hooks/useBookStatus.ts b/hooks/useBookStatus.ts index bb1ac1a9..66cfc13f 100644 --- a/hooks/useBookStatus.ts +++ b/hooks/useBookStatus.ts @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback, useMemo } from "react"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useMutation, useQueryClient, QueryClient } from "@tanstack/react-query"; import { queryKeys } from "@/lib/query-keys"; import type { Book } from "./useBookDetail"; import { toast } from "@/utils/toast"; @@ -44,7 +44,7 @@ function requiresArchiveConfirmation( * Also invalidates shelf caches for shelves containing this book to ensure * shelf pages reflect updated book status immediately. */ -export function invalidateBookQueries(queryClient: any, bookId: string): void { +export function invalidateBookQueries(queryClient: QueryClient, bookId: string): void { const numericBookId = parseInt(bookId); // Invalidate book-related queries