fix(web): prevent negative unread counts
This commit is contained in:
@@ -71,6 +71,24 @@ export function getP2PPeerUserId(topic: Topic, myUserId: string | null | undefin
|
|||||||
}
|
}
|
||||||
|
|
||||||
const confirmedReadSeq = new Map<string, number>();
|
const confirmedReadSeq = new Map<string, number>();
|
||||||
|
// Tinode reserves this sequence and higher values for optimistic local messages.
|
||||||
|
const LOCAL_SEQID = 0xfffffff;
|
||||||
|
|
||||||
|
export function getConfirmedRead(topicName: string): number {
|
||||||
|
const seq = confirmedReadSeq.get(topicName) || 0;
|
||||||
|
return seq > 0 && seq < LOCAL_SEQID ? seq : 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Repair an SDK contact polluted by a local sequence before it reaches the list or sync snapshot. */
|
||||||
|
export function repairContactReadState(contact: Pick<Contact, 'seq' | 'read' | 'recv' | 'unread'>): void {
|
||||||
|
const seq = contact.seq || 0;
|
||||||
|
const read = contact.read || 0;
|
||||||
|
const recv = contact.recv || 0;
|
||||||
|
if (read < LOCAL_SEQID && recv < LOCAL_SEQID && read <= seq && recv <= seq && (contact.unread || 0) >= 0) return;
|
||||||
|
contact.read = Math.min(Math.max(0, read), seq);
|
||||||
|
contact.recv = Math.min(Math.max(contact.read, recv), seq);
|
||||||
|
contact.unread = Math.max(0, seq - contact.read);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This app's OWN independent record of "the user has genuinely seen up to `seq` in this topic" --
|
* This app's OWN independent record of "the user has genuinely seen up to `seq` in this topic" --
|
||||||
@@ -83,7 +101,8 @@ const confirmedReadSeq = new Map<string, number>();
|
|||||||
* actually visible.
|
* actually visible.
|
||||||
*/
|
*/
|
||||||
export function markConfirmedRead(topicName: string, seq: number) {
|
export function markConfirmedRead(topicName: string, seq: number) {
|
||||||
const prev = confirmedReadSeq.get(topicName) || 0;
|
if (!Number.isInteger(seq) || seq <= 0 || seq >= LOCAL_SEQID) return;
|
||||||
|
const prev = getConfirmedRead(topicName);
|
||||||
if (seq > prev) confirmedReadSeq.set(topicName, seq);
|
if (seq > prev) confirmedReadSeq.set(topicName, seq);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -438,7 +457,8 @@ export function subscribeMeTopic(): Promise<unknown> {
|
|||||||
// Cast for the same reason getChatListSnapshot does -- read/unread/seq are real fields the SDK
|
// Cast for the same reason getChatListSnapshot does -- read/unread/seq are real fields the SDK
|
||||||
// sets directly on the Topic instance at runtime, just not declared on this hand-maintained .d.ts.
|
// sets directly on the Topic instance at runtime, just not declared on this hand-maintained .d.ts.
|
||||||
const topic = getTinode().getTopic(topicName) as unknown as Contact;
|
const topic = getTinode().getTopic(topicName) as unknown as Contact;
|
||||||
const correctRead = confirmedReadSeq.get(topicName) || 0;
|
repairContactReadState(topic);
|
||||||
|
const correctRead = getConfirmedRead(topicName);
|
||||||
if ((topic.read || 0) > correctRead) {
|
if ((topic.read || 0) > correctRead) {
|
||||||
topic.read = correctRead;
|
topic.read = correctRead;
|
||||||
topic.unread = Math.max(0, (topic.seq || 0) - correctRead);
|
topic.unread = Math.max(0, (topic.seq || 0) - correctRead);
|
||||||
@@ -620,7 +640,10 @@ function isChatListTopicName(name: string): boolean {
|
|||||||
* subscribe (`subscribeMeTopic`'s `withLaterSub()`) and any direct `openConversation()` both populate.
|
* subscribe (`subscribeMeTopic`'s `withLaterSub()`) and any direct `openConversation()` both populate.
|
||||||
*/
|
*/
|
||||||
export function getChatListSnapshot(): Contact[] {
|
export function getChatListSnapshot(): Contact[] {
|
||||||
if (chatListSnapshot) return chatListSnapshot;
|
if (chatListSnapshot) {
|
||||||
|
chatListSnapshot.forEach(repairContactReadState);
|
||||||
|
return chatListSnapshot;
|
||||||
|
}
|
||||||
const byTopic = new Map<string, Contact>();
|
const byTopic = new Map<string, Contact>();
|
||||||
getTinode().mapTopics((topic) => {
|
getTinode().mapTopics((topic) => {
|
||||||
if (!topic.name || !isChatListTopicName(topic.name) || goneTopicNames.has(topic.name)) return;
|
if (!topic.name || !isChatListTopicName(topic.name) || goneTopicNames.has(topic.name)) return;
|
||||||
@@ -633,6 +656,7 @@ export function getChatListSnapshot(): Contact[] {
|
|||||||
// (not a clone) so prototype methods like `latestMessage()` survive.
|
// (not a clone) so prototype methods like `latestMessage()` survive.
|
||||||
const contact = topic as unknown as Contact;
|
const contact = topic as unknown as Contact;
|
||||||
contact.topic = topic.name;
|
contact.topic = topic.name;
|
||||||
|
repairContactReadState(contact);
|
||||||
byTopic.set(topic.name, contact);
|
byTopic.set(topic.name, contact);
|
||||||
});
|
});
|
||||||
const rows = [...byTopic.values()];
|
const rows = [...byTopic.values()];
|
||||||
@@ -780,7 +804,7 @@ function reconcileMeSubs() {
|
|||||||
// `read` to what THIS client already knows it has confirmed seeing, regardless of what the
|
// `read` to what THIS client already knows it has confirmed seeing, regardless of what the
|
||||||
// fetch says -- a floor, never pushes `read` past the server's own value when the server is
|
// fetch says -- a floor, never pushes `read` past the server's own value when the server is
|
||||||
// genuinely ahead of this record (e.g. read from a different, already-synced device).
|
// genuinely ahead of this record (e.g. read from a different, already-synced device).
|
||||||
const confirmed = confirmedReadSeq.get(c.topic) || 0;
|
const confirmed = getConfirmedRead(c.topic);
|
||||||
if (confirmed > (c.read || 0)) {
|
if (confirmed > (c.read || 0)) {
|
||||||
c.read = confirmed;
|
c.read = confirmed;
|
||||||
if ((c.recv || 0) < confirmed) c.recv = confirmed;
|
if ((c.recv || 0) < confirmed) c.recv = confirmed;
|
||||||
|
|||||||
@@ -0,0 +1,35 @@
|
|||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import test from 'node:test';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
|
||||||
|
import { createJiti } from 'jiti';
|
||||||
|
|
||||||
|
globalThis.__APP_VERSION__ = 'test';
|
||||||
|
globalThis.window = { addEventListener() {}, removeEventListener() {} };
|
||||||
|
|
||||||
|
const jiti = createJiti(import.meta.url, { alias: { '@': fileURLToPath(new URL('../src', import.meta.url)) } });
|
||||||
|
const { getConfirmedRead, markConfirmedRead, repairContactReadState } = await jiti.import('../src/lib/tinode/topics.ts');
|
||||||
|
const LOCAL_SEQID = 0xfffffff;
|
||||||
|
|
||||||
|
test('optimistic local sequence never advances the confirmed read cursor', () => {
|
||||||
|
markConfirmedRead('unread-cursor-test', 260);
|
||||||
|
markConfirmedRead('unread-cursor-test', LOCAL_SEQID);
|
||||||
|
assert.equal(getConfirmedRead('unread-cursor-test'), 260);
|
||||||
|
markConfirmedRead('unread-cursor-test', 267);
|
||||||
|
assert.equal(getConfirmedRead('unread-cursor-test'), 267);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('repairs existing negative unread counts caused by local sequence IDs', () => {
|
||||||
|
for (const seq of [260, 267]) {
|
||||||
|
const contact = { seq, read: LOCAL_SEQID, recv: LOCAL_SEQID, unread: seq - LOCAL_SEQID };
|
||||||
|
assert.equal(contact.unread, seq === 260 ? -268435195 : -268435188);
|
||||||
|
repairContactReadState(contact);
|
||||||
|
assert.deepEqual(contact, { seq, read: seq, recv: seq, unread: 0 });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('keeps valid read and unread state unchanged', () => {
|
||||||
|
const contact = { seq: 267, read: 260, recv: 265, unread: 7 };
|
||||||
|
repairContactReadState(contact);
|
||||||
|
assert.deepEqual(contact, { seq: 267, read: 260, recv: 265, unread: 7 });
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user