mirror of
https://github.com/readest/readest.git
synced 2026-10-19 10:13:29 +00:00
feat(sync): sync the import-time page count across devices (#6775)
#6662 records a book's page count at import, but only on the importing device. Other devices got a count only once someone opened the book, and a cloud-only book could not be counted locally at all. - Add a nullable books.page_count column (migration 028) and map it both ways. A pulled row without a count leaves the key off, so the pull merge keeps the local value. - The server merge keeps the row winner's count and falls back to the other side's, so a push without one (old clients, the KOReader and Calibre plugins, devices that never counted the book) never erases it, and a server row missing a count gets it filled in without a date-read reorder. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
94ebb29d6c
commit
385889d57d
7 changed files with 100 additions and 1 deletions
|
|
@ -0,0 +1,24 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { resolvePageCountMerge } from '@/pages/api/sync';
|
||||
|
||||
describe('resolvePageCountMerge', () => {
|
||||
it('takes the row winner’s page count', () => {
|
||||
expect(resolvePageCountMerge({ page_count: 120 }, { page_count: 118 }, true)).toBe(120);
|
||||
expect(resolvePageCountMerge({ page_count: 120 }, { page_count: 118 }, false)).toBe(118);
|
||||
});
|
||||
|
||||
it('never lets a row without a count erase one', () => {
|
||||
// An old client, the KOReader or Calibre plugin, or a device that never
|
||||
// counted the book pushes no page_count.
|
||||
expect(resolvePageCountMerge({ page_count: null }, { page_count: 118 }, true)).toBe(118);
|
||||
expect(resolvePageCountMerge({}, { page_count: 118 }, true)).toBe(118);
|
||||
});
|
||||
|
||||
it('fills in a count the server does not have yet, even when the server wins the row', () => {
|
||||
expect(resolvePageCountMerge({ page_count: 120 }, { page_count: null }, false)).toBe(120);
|
||||
});
|
||||
|
||||
it('is null when neither side has one', () => {
|
||||
expect(resolvePageCountMerge({}, {}, true)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
@ -496,3 +496,27 @@ describe('transformBook metadataUpdatedAt (issue #5438)', () => {
|
|||
expect(back.metadataUpdatedAt).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('book page count', () => {
|
||||
const book: Book = {
|
||||
hash: 'h',
|
||||
format: 'EPUB',
|
||||
title: 'T',
|
||||
author: 'A',
|
||||
createdAt: 1,
|
||||
updatedAt: 2,
|
||||
};
|
||||
|
||||
it('pushes the import-time page count so peers get it without the file', () => {
|
||||
expect(transformBookToDB({ ...book, pageCount: 212 }, 'u').page_count).toBe(212);
|
||||
expect(transformBookToDB(book, 'u').page_count).toBeNull();
|
||||
});
|
||||
|
||||
it('pulls the page count, and leaves the key off a row without one', () => {
|
||||
const row = transformBookToDB(book, 'u');
|
||||
expect(transformBookFromDB({ ...row, page_count: 212 }).pageCount).toBe(212);
|
||||
// A key holding undefined would wipe the local count in the pull merge's
|
||||
// `{ ...local, ...remote }` spread.
|
||||
expect('pageCount' in transformBookFromDB({ ...row, page_count: null })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -102,6 +102,21 @@ export function resolveCoverMerge(
|
|||
: { cover_hash: server.cover_hash, cover_updated_at: server.cover_updated_at };
|
||||
}
|
||||
|
||||
/**
|
||||
* A books row's page count, counted from the file at import (#6662): the row
|
||||
* winner's value, else the other side's. Old clients, the KOReader and Calibre
|
||||
* plugins, and devices that never counted the book push no page_count, so a
|
||||
* row without one must never erase it.
|
||||
*/
|
||||
export function resolvePageCountMerge(
|
||||
client: Pick<DBBook, 'page_count'>,
|
||||
server: Pick<DBBook, 'page_count'>,
|
||||
clientIsNewer: boolean,
|
||||
): number | null {
|
||||
const [winner, loser] = clientIsNewer ? [client, server] : [server, client];
|
||||
return winner.page_count ?? loser.page_count ?? null;
|
||||
}
|
||||
|
||||
type BookMetadataFields = Pick<
|
||||
DBBook,
|
||||
'title' | 'author' | 'tags' | 'metadata' | 'metadata_updated_at'
|
||||
|
|
@ -755,6 +770,7 @@ export async function POST(req: NextRequest) {
|
|||
| 'group_id'
|
||||
| 'group_name'
|
||||
| 'group_updated_at'
|
||||
| 'page_count'
|
||||
>
|
||||
> &
|
||||
Pick<DBBook, 'title' | 'author' | 'tags'>;
|
||||
|
|
@ -766,6 +782,7 @@ export async function POST(req: NextRequest) {
|
|||
const meta = resolveMetadataMerge(clientBook, serverBook, clientIsNewer);
|
||||
// Group membership likewise merges on its own clock (issue #5911).
|
||||
const group = resolveGroupMerge(clientBook, serverBook, clientIsNewer);
|
||||
const pageCount = resolvePageCountMerge(clientBook, serverBook, clientIsNewer);
|
||||
if (clientIsNewer) {
|
||||
// Client wins the row; graft the fresher status + cover +
|
||||
// metadata onto it (server's may be the newer one even though
|
||||
|
|
@ -782,6 +799,7 @@ export async function POST(req: NextRequest) {
|
|||
clientBook.group_id = group.group_id;
|
||||
clientBook.group_name = group.group_name;
|
||||
clientBook.group_updated_at = group.group_updated_at;
|
||||
clientBook.page_count = pageCount;
|
||||
toUpdate.push(clientBook);
|
||||
} else {
|
||||
// Only rewrite when a resolved field VALUE differs from the
|
||||
|
|
@ -794,7 +812,14 @@ export async function POST(req: NextRequest) {
|
|||
const coverChanged = (cover.cover_hash ?? null) !== (serverBook.cover_hash ?? null);
|
||||
const metadataChanged = bookMetadataChanged(meta, serverBook);
|
||||
const groupChanged = bookGroupChanged(group, serverBook);
|
||||
if (statusChanged || coverChanged || metadataChanged || groupChanged) {
|
||||
const pageCountChanged = pageCount !== (serverBook.page_count ?? null);
|
||||
if (
|
||||
statusChanged ||
|
||||
coverChanged ||
|
||||
metadataChanged ||
|
||||
groupChanged ||
|
||||
pageCountChanged
|
||||
) {
|
||||
// Server wins the row, but the client's status, cover and/or
|
||||
// metadata is the fresher one. Graft the fresher fields onto
|
||||
// the server row and leave updated_at untouched; the
|
||||
|
|
@ -818,6 +843,7 @@ export async function POST(req: NextRequest) {
|
|||
propagated.group_id = group.group_id;
|
||||
propagated.group_name = group.group_name;
|
||||
propagated.group_updated_at = group.group_updated_at;
|
||||
propagated.page_count = pageCount;
|
||||
toUpdate.push(propagated);
|
||||
} else {
|
||||
batchAuthoritativeRecords.push(serverData);
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ export interface DBBook {
|
|||
reading_status_updated_at?: string | null;
|
||||
cover_hash?: string | null;
|
||||
cover_updated_at?: string | null;
|
||||
page_count?: number | null;
|
||||
|
||||
metadata?: string | null;
|
||||
metadata_updated_at?: string | null;
|
||||
|
|
|
|||
|
|
@ -82,6 +82,7 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => {
|
|||
readingStatusUpdatedAt,
|
||||
coverHash,
|
||||
coverUpdatedAt,
|
||||
pageCount,
|
||||
metadata,
|
||||
metadataUpdatedAt,
|
||||
createdAt,
|
||||
|
|
@ -108,6 +109,7 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => {
|
|||
: null,
|
||||
cover_hash: coverHash ?? null,
|
||||
cover_updated_at: coverUpdatedAt ? new Date(coverUpdatedAt).toISOString() : null,
|
||||
page_count: pageCount ?? null,
|
||||
source_title: sanitizeString(sourceTitle),
|
||||
metadata: metadata ? sanitizeString(JSON.stringify(metadata)) : null,
|
||||
metadata_updated_at: metadataUpdatedAt ? new Date(metadataUpdatedAt).toISOString() : null,
|
||||
|
|
@ -134,6 +136,7 @@ export const transformBookFromDB = (dbBook: DBBook): Book => {
|
|||
reading_status_updated_at,
|
||||
cover_hash,
|
||||
cover_updated_at,
|
||||
page_count,
|
||||
source_title,
|
||||
metadata,
|
||||
metadata_updated_at,
|
||||
|
|
@ -168,6 +171,9 @@ export const transformBookFromDB = (dbBook: DBBook): Book => {
|
|||
deletedAt: deleted_at ? new Date(deleted_at).getTime() : null,
|
||||
uploadedAt: uploaded_at ? new Date(uploaded_at).getTime() : null,
|
||||
};
|
||||
// Only when set: an undefined key would wipe the local count in the pull
|
||||
// merge's `{ ...local, ...remote }` spread.
|
||||
if (page_count != null) book.pageCount = page_count;
|
||||
// Native cloud DBBook has no `url` column; a feed book carries its feed URL in
|
||||
// metadata so the reader can rebuild the feed:// descriptor here.
|
||||
if (!book.url && book.metadata?.feedUrl) {
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ CREATE TABLE public.books (
|
|||
group_id text NULL,
|
||||
group_name text NULL,
|
||||
group_updated_at timestamp with time zone NULL,
|
||||
page_count integer NULL,
|
||||
metadata json NULL,
|
||||
CONSTRAINT books_pkey PRIMARY KEY (user_id, book_hash),
|
||||
CONSTRAINT books_user_id_fkey FOREIGN KEY (user_id) REFERENCES auth.users (id) ON DELETE CASCADE
|
||||
|
|
|
|||
17
docker/volumes/db/migrations/028_add_book_page_count.sql
Normal file
17
docker/volumes/db/migrations/028_add_book_page_count.sql
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
-- Migration 028: Add `page_count` to books
|
||||
--
|
||||
-- The page total the reader lays out for a book, counted from the file at
|
||||
-- import (#6662) so an unopened book can show, sort and filter by length. The
|
||||
-- importing device already did the work; syncing it lets every other device
|
||||
-- have the count too, including ones that only hold the book in the cloud and
|
||||
-- so cannot count it themselves. Once a book is opened, progress[1] carries the
|
||||
-- same total, so this only matters for books nobody has opened yet.
|
||||
--
|
||||
-- Additive + nullable. The merge keeps the row winner's value and falls back
|
||||
-- to the other side's when the winner has none, so a push without a count —
|
||||
-- old clients, the KOReader and Calibre plugins, a device that never counted
|
||||
-- the book — never erases it. Old clients ignore the column, so they never
|
||||
-- break. Not backfilled: only a client holding the file can count it.
|
||||
|
||||
ALTER TABLE public.books
|
||||
ADD COLUMN IF NOT EXISTS page_count integer NULL;
|
||||
Loading…
Add table
Add a link
Reference in a new issue