mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(client): send only the reader settings that changed
Every settings write from the reader PUT the whole object: the defaults write sent all twenty-one EPUB fields and the per-book write sent the entire accumulated delta. PUT replaces the stored row, so a write from this client threw away whatever another one had set since the page loaded. That is a real conflict now rather than a theoretical one. The iOS reader owns the look fields and keeps the layout fields on the device, so it writes a small subset and leaves the rest alone. A full snapshot from a web tab reverted its look edits silently, and there was nothing either side could do about it: last writer won the whole row. Move the three writes to the PATCH routes, which merge field by field on the server, so two clients editing different fields no longer race. An empty patch is dropped rather than sent, because the server rejects an empty body. Reset still deletes, on both routes: removing a customization is a whole-row operation and means the same thing to every client.
This commit is contained in:
@@ -341,16 +341,31 @@ describe('useReaderSettings - updateBookSettings', () => {
|
||||
expect(apiMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('calls api() with PUT when sync is enabled', () => {
|
||||
it('patches only the changed field when sync is enabled', () => {
|
||||
useAuthMock.mockReturnValue({
|
||||
user: ref({ settings: { syncReaderPreferences: true } }),
|
||||
})
|
||||
apiMock.mockResolvedValue({ ok: true })
|
||||
// A field another client pinned on this same book, already in the local delta.
|
||||
localStorage.setItem(`reader:book:${BOOK_FILE_ID}`, JSON.stringify({ themeName: 'sepia' }))
|
||||
|
||||
const s = useReaderSettings(BOOK_FILE_ID, 'epub')
|
||||
s.updateBookSettings({ fontSize: 22 } as never)
|
||||
|
||||
expect(apiMock).toHaveBeenCalledWith(`/api/v1/reader/preferences/${BOOK_FILE_ID}`, expect.objectContaining({ method: 'PATCH' }))
|
||||
expect(JSON.parse(apiMock.mock.calls[0][1].body)).toEqual({ set: { fontSize: 22 } })
|
||||
})
|
||||
|
||||
it('sends nothing when the patch is empty, because the server rejects an empty body', () => {
|
||||
useAuthMock.mockReturnValue({
|
||||
user: ref({ settings: { syncReaderPreferences: true } }),
|
||||
})
|
||||
apiMock.mockResolvedValue({ ok: true })
|
||||
|
||||
const s = useReaderSettings(BOOK_FILE_ID, 'epub')
|
||||
s.updateBookSettings({ fontSize: 22 } as never)
|
||||
s.updateBookSettings({} as never)
|
||||
|
||||
expect(apiMock).toHaveBeenCalledWith(`/api/v1/reader/preferences/${BOOK_FILE_ID}`, expect.objectContaining({ method: 'PUT' }))
|
||||
expect(apiMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -408,7 +423,7 @@ describe('useReaderSettings - updateDefaultSettings', () => {
|
||||
expect(stored).toMatchObject({ fontSize: 18, isDark: true })
|
||||
})
|
||||
|
||||
it('calls api() with PUT when sync is enabled', () => {
|
||||
it('patches only the changed field rather than sending a full snapshot', () => {
|
||||
useAuthMock.mockReturnValue({
|
||||
user: ref({ settings: { syncReaderPreferences: true } }),
|
||||
})
|
||||
@@ -417,7 +432,12 @@ describe('useReaderSettings - updateDefaultSettings', () => {
|
||||
const s = useReaderSettings(BOOK_FILE_ID, 'epub')
|
||||
s.updateDefaultSettings({ fontSize: 18 } as never)
|
||||
|
||||
expect(apiMock).toHaveBeenCalledWith('/api/v1/reader/defaults/epub', expect.objectContaining({ method: 'PUT' }))
|
||||
expect(apiMock).toHaveBeenCalledWith('/api/v1/reader/defaults/epub', expect.objectContaining({ method: 'PATCH' }))
|
||||
const body = JSON.parse(apiMock.mock.calls[0][1].body)
|
||||
expect(body).toEqual({ set: { fontSize: 18 } })
|
||||
// The whole point: nothing another client owns rides along and overwrites it.
|
||||
expect(body.set.themeName).toBeUndefined()
|
||||
expect(body.settings).toBeUndefined()
|
||||
})
|
||||
|
||||
it('does not call api() when sync is disabled', () => {
|
||||
@@ -632,7 +652,7 @@ describe('useReaderDefaultSettings - update', () => {
|
||||
expect(stored).toMatchObject({ fontSize: 22 })
|
||||
})
|
||||
|
||||
it('calls api() with PUT when sync is enabled', () => {
|
||||
it('patches only the changed field rather than sending a full snapshot', () => {
|
||||
useAuthMock.mockReturnValue({
|
||||
user: ref({ settings: { syncReaderPreferences: true } }),
|
||||
})
|
||||
@@ -641,7 +661,20 @@ describe('useReaderDefaultSettings - update', () => {
|
||||
const s = useReaderDefaultSettings('epub')
|
||||
s.update({ fontSize: 22 } as never)
|
||||
|
||||
expect(apiMock).toHaveBeenCalledWith('/api/v1/reader/defaults/epub', expect.objectContaining({ method: 'PUT' }))
|
||||
expect(apiMock).toHaveBeenCalledWith('/api/v1/reader/defaults/epub', expect.objectContaining({ method: 'PATCH' }))
|
||||
expect(JSON.parse(apiMock.mock.calls[0][1].body)).toEqual({ set: { fontSize: 22 } })
|
||||
})
|
||||
|
||||
it('sends nothing when the patch is empty', () => {
|
||||
useAuthMock.mockReturnValue({
|
||||
user: ref({ settings: { syncReaderPreferences: true } }),
|
||||
})
|
||||
apiMock.mockResolvedValue({ ok: true })
|
||||
|
||||
const s = useReaderDefaultSettings('epub')
|
||||
s.update({} as never)
|
||||
|
||||
expect(apiMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
type CbxReaderSettings,
|
||||
type EpubReaderSettings,
|
||||
type PdfReaderSettings,
|
||||
type ReaderDefaultsPatchBody,
|
||||
type ReaderFormatGroup,
|
||||
type ReaderSettings,
|
||||
READER_GROUP_DEFAULTS,
|
||||
@@ -328,6 +329,22 @@ export function useReaderSettings(bookFileId: number, format: string) {
|
||||
}
|
||||
}
|
||||
|
||||
// Sends only what changed.
|
||||
//
|
||||
// These used to PUT the whole object. PUT replaces the stored row, so a write from here threw
|
||||
// away every field another client had set since this page loaded: the iOS reader owns the look
|
||||
// fields and keeps the layout fields on the device, and a full snapshot from this tab silently
|
||||
// reverted whatever it had just changed. PATCH merges field by field on the server, so two
|
||||
// clients editing different fields no longer race over the whole row.
|
||||
function patchSettings(path: string, set: Partial<ReaderSettings>) {
|
||||
if (Object.keys(set).length === 0) return
|
||||
api(path, {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ set } satisfies ReaderDefaultsPatchBody),
|
||||
}).catch(() => {})
|
||||
}
|
||||
|
||||
// Merges only the changed field(s) into the existing delta — never saves a full snapshot.
|
||||
function updateBookSettings(patch: Partial<ReaderSettings>) {
|
||||
const next = {
|
||||
@@ -339,11 +356,7 @@ export function useReaderSettings(bookFileId: number, format: string) {
|
||||
writeLs(lsBookKey(bookFileId), next)
|
||||
|
||||
if (syncEnabled.value) {
|
||||
api(`/api/v1/reader/preferences/${bookFileId}`, {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ settings: next }),
|
||||
}).catch(() => {})
|
||||
patchSettings(`/api/v1/reader/preferences/${bookFileId}`, patch)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -366,11 +379,7 @@ export function useReaderSettings(bookFileId: number, format: string) {
|
||||
writeLs(lsDefaultKey(group), next)
|
||||
|
||||
if (syncEnabled.value) {
|
||||
api(`/api/v1/reader/defaults/${group}`, {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ settings: next }),
|
||||
}).catch(() => {})
|
||||
patchSettings(`/api/v1/reader/defaults/${group}`, patch)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -441,11 +450,11 @@ export function useReaderDefaultSettings<T extends ReaderSettings>(format: strin
|
||||
settings.value = next
|
||||
writeLs(lsDefaultKey(group), next)
|
||||
|
||||
if (syncEnabled.value) {
|
||||
if (syncEnabled.value && Object.keys(patch).length > 0) {
|
||||
api(`/api/v1/reader/defaults/${group}`, {
|
||||
method: 'PUT',
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ settings: next }),
|
||||
body: JSON.stringify({ set: patch } satisfies ReaderDefaultsPatchBody),
|
||||
}).catch(() => {})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user