Skip to content

Commit 3d14162

Browse files
Expire poll terminate chat events
Co-authored-by: trevor-signal <[email protected]>
1 parent 68bbf36 commit 3d14162

File tree

12 files changed

+195
-32
lines changed

12 files changed

+195
-32
lines changed

ts/background.preload.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2530,7 +2530,7 @@ export async function startApp(): Promise<void> {
25302530
confirm();
25312531
return;
25322532
}
2533-
const { pollTerminate, timestamp } = data.message;
2533+
const { pollTerminate, timestamp, expireTimer } = data.message;
25342534

25352535
const parsedTerm = safeParsePartial(PollTerminateSchema, pollTerminate);
25362536
if (!parsedTerm.success) {
@@ -2562,6 +2562,8 @@ export async function startApp(): Promise<void> {
25622562
targetTimestamp: parsedTerm.data.targetTimestamp,
25632563
receivedAtDate: data.receivedAtDate,
25642564
timestamp,
2565+
expireTimer,
2566+
expirationStartTimestamp: undefined,
25652567
};
25662568

25672569
drop(Polls.onPollTerminate(attributes));
@@ -3026,7 +3028,7 @@ export async function startApp(): Promise<void> {
30263028
confirm();
30273029
return;
30283030
}
3029-
const { pollTerminate, timestamp } = data.message;
3031+
const { pollTerminate, timestamp, expireTimer } = data.message;
30303032

30313033
const parsedTerm = safeParsePartial(PollTerminateSchema, pollTerminate);
30323034
if (!parsedTerm.success) {
@@ -3052,6 +3054,8 @@ export async function startApp(): Promise<void> {
30523054
source: Polls.PollSource.FromSync,
30533055
targetTimestamp: parsedTerm.data.targetTimestamp,
30543056
receivedAtDate: data.receivedAtDate,
3057+
expireTimer,
3058+
expirationStartTimestamp: data.expirationStartTimestamp,
30553059
timestamp,
30563060
};
30573061

ts/messageModifiers/Polls.preload.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { strictAssert } from '../util/assert.std.js';
2323
import { getMessageIdForLogging } from '../util/idForLogging.preload.js';
2424
import { drop } from '../util/drop.std.js';
2525
import { maybeNotify } from '../messages/maybeNotify.preload.js';
26+
import type { DurationInSeconds } from '../util/durations/duration-in-seconds.std.js';
2627

2728
const log = createLogger('Polls');
2829

@@ -53,6 +54,8 @@ export type PollTerminateAttributesType = {
5354
targetTimestamp: number;
5455
timestamp: number;
5556
receivedAtDate: number;
57+
expireTimer: DurationInSeconds | undefined;
58+
expirationStartTimestamp: number | undefined;
5659
};
5760

5861
const pollVoteCache = new Map<string, PollVoteAttributesType>();
@@ -578,6 +581,8 @@ export async function handlePollTerminate(
578581
terminatorId: terminate.fromConversationId,
579582
timestamp: terminate.timestamp,
580583
isMeTerminating: isMe(author.attributes),
584+
expireTimer: terminate.expireTimer,
585+
expirationStartTimestamp: terminate.expirationStartTimestamp,
581586
});
582587

583588
window.reduxActions.conversations.markOpenConversationRead(conversation.id);

ts/messageModifiers/ReadSyncs.preload.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import { StartupQueue } from '../util/StartupQueue.std.js';
1010
import { drop } from '../util/drop.std.js';
1111
import { getMessageIdForLogging } from '../util/idForLogging.preload.js';
1212
import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp.std.js';
13-
import { isIncoming } from '../state/selectors/message.preload.js';
13+
import {
14+
isIncoming,
15+
isPollTerminate,
16+
} from '../state/selectors/message.preload.js';
1417
import { isMessageUnread } from '../util/isMessageUnread.std.js';
1518
import { notificationService } from '../services/notifications.preload.js';
1619
import { queueUpdateMessage } from '../util/messageBatcher.preload.js';
@@ -175,8 +178,10 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
175178
serviceId: item.sourceServiceId,
176179
reason: logId,
177180
});
178-
179-
return isIncoming(item) && sender?.id === readSync.senderId;
181+
return (
182+
(isIncoming(item) || isPollTerminate(item)) &&
183+
sender?.id === readSync.senderId
184+
);
180185
});
181186

182187
if (!found) {

ts/messages/maybeNotify.preload.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { shouldNotify as shouldNotifyDuringNotificationProfile } from '../types/
1212
import { NotificationType } from '../types/notifications.std.js';
1313
import { isMessageUnread } from '../util/isMessageUnread.std.js';
1414
import { isDirectConversation } from '../util/whatTypeOfConversation.dom.js';
15-
import { hasExpiration } from '../types/Message2.preload.js';
15+
import { isExpiringMessage } from '../types/Message2.preload.js';
1616
import { notificationService } from '../services/notifications.preload.js';
1717
import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage.preload.js';
1818
import type { MessageAttributesType } from '../model-types.d.ts';
@@ -128,7 +128,6 @@ export async function maybeNotify(args: MaybeNotifyArgs): Promise<void> {
128128
const { url, absolutePath } = await conversation.getAvatarOrIdenticon();
129129

130130
const messageId = messageForNotification.id;
131-
const isExpiringMessage = hasExpiration(messageForNotification);
132131

133132
notificationService.add({
134133
senderTitle,
@@ -138,7 +137,7 @@ export async function maybeNotify(args: MaybeNotifyArgs): Promise<void> {
138137
: messageForNotification.storyId,
139138
notificationIconUrl: url,
140139
notificationIconAbsolutePath: absolutePath,
141-
isExpiringMessage,
140+
isExpiringMessage: isExpiringMessage(messageForNotification),
142141
message: getNotificationTextForMessage(messageForNotification),
143142
messageId,
144143
reaction: reaction

ts/models/conversations.preload.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,6 +3501,8 @@ export class ConversationModel {
35013501
terminatorId: string;
35023502
timestamp: number;
35033503
isMeTerminating: boolean;
3504+
expireTimer: DurationInSeconds | undefined;
3505+
expirationStartTimestamp: number | undefined;
35043506
}): Promise<void> {
35053507
const terminatorConversation = window.ConversationController.get(
35063508
params.terminatorId
@@ -3522,6 +3524,8 @@ export class ConversationModel {
35223524
readStatus: params.isMeTerminating ? ReadStatus.Read : ReadStatus.Unread,
35233525
seenStatus: params.isMeTerminating ? SeenStatus.Seen : SeenStatus.Unseen,
35243526
schemaVersion: Message.VERSION_NEEDED_FOR_DISPLAY,
3527+
expireTimer: params.expireTimer,
3528+
expirationStartTimestamp: params.expirationStartTimestamp,
35253529
});
35263530

35273531
await window.MessageCache.saveMessage(message, { forceSave: true });

ts/polls/enqueuePollTerminateForSend.preload.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ export async function enqueuePollTerminateForSend({
5050
targetTimestamp,
5151
receivedAtDate: timestamp,
5252
timestamp,
53+
expireTimer: conversation.get('expireTimer'),
54+
expirationStartTimestamp: Date.now(),
5355
};
5456

5557
await handlePollTerminate(message, terminate, { shouldPersist: true });

ts/sql/Server.node.ts

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3349,28 +3349,40 @@ function getUnreadByConversationAndMarkRead(
33493349
return db.transaction(() => {
33503350
const expirationStartTimestamp = Math.min(now, readAt ?? Infinity);
33513351

3352-
const expirationJsonPatch = JSON.stringify({ expirationStartTimestamp });
3353-
3354-
const [updateExpirationQuery, updateExpirationParams] = sql`
3352+
const updateExpirationFragment = sqlFragment`
33553353
UPDATE messages
3356-
INDEXED BY expiring_message_by_conversation_and_received_at
33573354
SET
3358-
expirationStartTimestamp = ${expirationStartTimestamp},
3359-
json = json_patch(json, ${expirationJsonPatch})
3355+
expirationStartTimestamp = ${expirationStartTimestamp}
33603356
WHERE
33613357
conversationId = ${conversationId} AND
33623358
(${_storyIdPredicate(storyId, includeStoryReplies)}) AND
3363-
isStory IS 0 AND
3364-
type IS 'incoming' AND
3365-
(
3366-
expirationStartTimestamp IS NULL OR
3367-
expirationStartTimestamp > ${expirationStartTimestamp}
3368-
) AND
3369-
expireTimer > 0 AND
3370-
received_at <= ${readMessageReceivedAt};
3359+
type IN ('incoming', 'poll-terminate') AND
3360+
hasExpireTimer IS 1 AND
3361+
received_at <= ${readMessageReceivedAt}
33713362
`;
33723363

3373-
db.prepare(updateExpirationQuery).run(updateExpirationParams);
3364+
// 1. Update expirationStartTimestamps for messages without an
3365+
// expirationStartTimestamp
3366+
const [updateNullEpirationStartQuery, updateNullExpirationStartParams] =
3367+
sql`
3368+
${updateExpirationFragment} AND
3369+
expirationStartTimestamp IS NULL;
3370+
`;
3371+
db.prepare(updateNullEpirationStartQuery).run(
3372+
updateNullExpirationStartParams
3373+
);
3374+
3375+
// 2. Update expirationStartTimestamps for messages with a later
3376+
// expirationStartTimestamp. These are run in two separate queries to allow
3377+
// each to use the index on expirationStartTimestamp
3378+
const [updateLateExpirationStartQuery, updateLateExpirationStartParams] =
3379+
sql`
3380+
${updateExpirationFragment} AND
3381+
expirationStartTimestamp > ${expirationStartTimestamp};
3382+
`;
3383+
db.prepare(updateLateExpirationStartQuery).run(
3384+
updateLateExpirationStartParams
3385+
);
33743386

33753387
const [selectQuery, selectParams] = sql`
33763388
SELECT
@@ -5497,7 +5509,8 @@ function getMessagesUnexpectedlyMissingExpirationStartTimestamp(
54975509
readStatus = ${ReadStatus.Read} OR
54985510
readStatus = ${ReadStatus.Viewed} OR
54995511
readStatus IS NULL
5500-
))
5512+
)) OR
5513+
(type IS 'poll-terminate')
55015514
);
55025515
`
55035516
)
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2021 Signal Messenger, LLC
2+
// SPDX-License-Identifier: AGPL-3.0-only
3+
4+
import type { Database } from '@signalapp/sqlcipher';
5+
6+
export default function updateToSchemaVersion1520(db: Database): void {
7+
db.exec(
8+
'DROP INDEX IF EXISTS expiring_message_by_conversation_and_received_at;'
9+
);
10+
11+
db.exec(`
12+
ALTER TABLE messages ADD COLUMN hasExpireTimer INTEGER NOT NULL
13+
GENERATED ALWAYS AS (COALESCE(expireTimer, 0) > 0) VIRTUAL;
14+
`);
15+
16+
db.exec(`
17+
CREATE INDEX messages_conversationId_hasExpireTimer_expirationStartTimestamp
18+
ON messages (conversationId, hasExpireTimer, expirationStartTimestamp);
19+
`);
20+
}

ts/sql/migrations/index.node.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ import updateToSchemaVersion1490 from './1490-lowercase-notification-profiles.st
128128
import updateToSchemaVersion1500 from './1500-search-polls.std.js';
129129
import updateToSchemaVersion1510 from './1510-chat-folders-normalize-all-chats.std.js';
130130
import updateToSchemaVersion1520 from './1520-poll-votes-unread.std.js';
131+
import updateToSchemaVersion1530 from './1530-update-expiring-index.std.js';
131132

132133
import { DataWriter } from '../Server.node.js';
133134

@@ -1614,6 +1615,7 @@ export const SCHEMA_VERSIONS: ReadonlyArray<SchemaUpdateType> = [
16141615
{ version: 1500, update: updateToSchemaVersion1500 },
16151616
{ version: 1510, update: updateToSchemaVersion1510 },
16161617
{ version: 1520, update: updateToSchemaVersion1520 },
1618+
{ version: 1530, update: updateToSchemaVersion1530 },
16171619
];
16181620

16191621
export class DBVersionFromFutureError extends Error {

ts/test-electron/sql/markRead_test.preload.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ describe('sql/markRead', () => {
330330
const now = Date.now();
331331
assert.lengthOf(await _getAllMessages(), 0);
332332

333-
const start = Date.now();
333+
const start = now;
334334
const readAt = start + 20;
335335
const conversationId = generateUuid();
336336
const expireTimer = DurationInSeconds.fromSeconds(15);
@@ -345,7 +345,7 @@ describe('sql/markRead', () => {
345345
received_at: start + 1,
346346
timestamp: start + 1,
347347
expireTimer,
348-
expirationStartTimestamp: start + 1,
348+
expirationStartTimestamp: start + 100,
349349
readStatus: ReadStatus.Read,
350350
};
351351
const message2: MessageAttributesType = {
@@ -436,16 +436,23 @@ describe('sql/markRead', () => {
436436
(left, right) => left.timestamp - right.timestamp
437437
);
438438

439+
assert.strictEqual(sorted[0].id, message1.id, 'checking message 1');
440+
assert.strictEqual(
441+
sorted[0].expirationStartTimestamp,
442+
now,
443+
"message1's expirationStartTimestamp was moved earlier"
444+
);
445+
439446
assert.strictEqual(sorted[1].id, message2.id, 'checking message 2');
440-
assert.isAtMost(
441-
sorted[1].expirationStartTimestamp ?? Infinity,
447+
assert.strictEqual(
448+
sorted[1].expirationStartTimestamp,
442449
now,
443450
'checking message 2 expirationStartTimestamp'
444451
);
445452

446453
assert.strictEqual(sorted[3].id, message4.id, 'checking message 4');
447-
assert.isAtMost(
448-
sorted[3].expirationStartTimestamp ?? Infinity,
454+
assert.strictEqual(
455+
sorted[3].expirationStartTimestamp,
449456
now,
450457
'checking message 4 expirationStartTimestamp'
451458
);

0 commit comments

Comments
 (0)