mirror of
https://github.com/signalapp/Signal-Desktop.git
synced 2026-09-28 07:57:21 +01:00
Fix duplicate messages at backup page boundaries
Co-authored-by: Rolfe Schmidt <141083381+rolfe-signal@users.noreply.github.com>
This commit is contained in:
co-authored by
Rolfe Schmidt
parent
f1c4f8cbc7
commit
cb87fc33f2
@@ -589,7 +589,7 @@ export type PageMessagesResultType = Readonly<{
|
||||
export type PageBackupMessagesCursorType = Readonly<{
|
||||
__page_backup_messages_cursor: never;
|
||||
|
||||
nextRowid: number;
|
||||
lastRowId: number;
|
||||
done: boolean;
|
||||
}>;
|
||||
|
||||
|
||||
@@ -9186,7 +9186,8 @@ function pageBackupMessages(
|
||||
${MESSAGE_COLUMNS_FRAGMENT}
|
||||
FROM messages
|
||||
WHERE
|
||||
rowid >= ${cursor?.nextRowid ?? 0}
|
||||
rowid > ${cursor?.lastRowId ?? 0}
|
||||
ORDER BY rowid ASC
|
||||
LIMIT ${LIMIT}
|
||||
`;
|
||||
const rows: Array<MessageTypeUnhydrated & { rowid: number }> = db
|
||||
@@ -9194,7 +9195,7 @@ function pageBackupMessages(
|
||||
.all(params);
|
||||
return {
|
||||
cursor: {
|
||||
nextRowid: rows.at(-1)?.rowid ?? 0,
|
||||
lastRowId: rows.at(-1)?.rowid ?? 0,
|
||||
done: rows.length < LIMIT,
|
||||
} as PageBackupMessagesCursorType,
|
||||
messages: hydrateMessages(db, rows),
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
// Copyright 2026 Signal Messenger, LLC
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
|
||||
import { cwd } from 'node:process';
|
||||
import { assert } from 'chai';
|
||||
|
||||
import type {
|
||||
PageBackupMessagesCursorType,
|
||||
WritableDB,
|
||||
} from '../../sql/Interface.std.ts';
|
||||
import { DataReader, setupTests } from '../../sql/Server.node.ts';
|
||||
import { createDB } from './helpers.node.ts';
|
||||
|
||||
describe('SQL/pageBackupMessages', () => {
|
||||
// Must match the LIMIT inside pageBackupMessages.
|
||||
const PAGE_SIZE = 1000;
|
||||
|
||||
let db: WritableDB;
|
||||
|
||||
beforeEach(() => {
|
||||
db = createDB();
|
||||
setupTests(db, { userDataPath: cwd() });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
db.close();
|
||||
});
|
||||
|
||||
function insertMessages(count: number): void {
|
||||
const stmt = db.prepare(`
|
||||
INSERT INTO messages (id, conversationId, json)
|
||||
VALUES ($id, $conversationId, $json)
|
||||
`);
|
||||
db.transaction(() => {
|
||||
for (let i = 0; i < count; i += 1) {
|
||||
stmt.run({
|
||||
id: `m${i}`,
|
||||
conversationId: 'c1',
|
||||
json: JSON.stringify({}),
|
||||
});
|
||||
}
|
||||
})();
|
||||
}
|
||||
|
||||
function pageAll(): {
|
||||
ids: Array<string>;
|
||||
pages: number;
|
||||
} {
|
||||
const ids: Array<string> = [];
|
||||
let pages = 0;
|
||||
let cursor: PageBackupMessagesCursorType | undefined;
|
||||
|
||||
while (!cursor?.done) {
|
||||
const { messages, cursor: nextCursor } = DataReader.pageBackupMessages(
|
||||
db,
|
||||
cursor
|
||||
);
|
||||
ids.push(...messages.map(message => message.id));
|
||||
cursor = nextCursor;
|
||||
pages += 1;
|
||||
|
||||
assert.isBelow(pages, 100, 'pagination did not terminate');
|
||||
}
|
||||
|
||||
return { ids, pages };
|
||||
}
|
||||
|
||||
it('returns every message exactly once when paging', () => {
|
||||
const count = PAGE_SIZE * 2 + 500;
|
||||
insertMessages(count);
|
||||
|
||||
const { ids } = pageAll();
|
||||
|
||||
assert.strictEqual(ids.length, count, 'wrong number of messages');
|
||||
assert.strictEqual(
|
||||
new Set(ids).size,
|
||||
count,
|
||||
'a message was returned more than once'
|
||||
);
|
||||
});
|
||||
|
||||
it('does not repeat the row on a page boundary', () => {
|
||||
insertMessages(PAGE_SIZE + 1);
|
||||
|
||||
const first = DataReader.pageBackupMessages(db);
|
||||
assert.strictEqual(first.messages.length, PAGE_SIZE);
|
||||
assert.isFalse(first.cursor.done);
|
||||
|
||||
const second = DataReader.pageBackupMessages(db, first.cursor);
|
||||
assert.deepStrictEqual(
|
||||
second.messages.map(message => message.id),
|
||||
[`m${PAGE_SIZE}`],
|
||||
'second page must start after the last row of the first page'
|
||||
);
|
||||
assert.isTrue(second.cursor.done);
|
||||
});
|
||||
|
||||
it('terminates on an exact multiple of the page size', () => {
|
||||
const count = PAGE_SIZE * 2;
|
||||
insertMessages(count);
|
||||
|
||||
const { ids, pages } = pageAll();
|
||||
|
||||
assert.strictEqual(ids.length, count);
|
||||
assert.strictEqual(new Set(ids).size, count);
|
||||
// Two full pages, then one empty page to learn that we are done.
|
||||
assert.strictEqual(pages, 3);
|
||||
});
|
||||
|
||||
it('returns messages in ascending rowid order', () => {
|
||||
const count = PAGE_SIZE + 10;
|
||||
insertMessages(count);
|
||||
|
||||
const { ids } = pageAll();
|
||||
|
||||
const expected = Array.from({ length: count }, (_, i) => `m${i}`);
|
||||
assert.deepStrictEqual(ids, expected);
|
||||
});
|
||||
|
||||
it('handles an empty table', () => {
|
||||
const { ids, pages } = pageAll();
|
||||
|
||||
assert.deepStrictEqual(ids, []);
|
||||
assert.strictEqual(pages, 1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user