Skip to content

Commit

Permalink
Sort message-initiated timer updates before the initiating message
Browse files Browse the repository at this point in the history
Co-authored-by: Scott Nonnenberg <scott@signal.org>
  • Loading branch information
automated-signal and scottnonnenberg-signal committed Mar 29, 2022
1 parent a613ce0 commit 6ae03fa
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 22 deletions.
10 changes: 3 additions & 7 deletions ts/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2600,13 +2600,10 @@ export async function startApp(): Promise<void> {
const { expireTimer } = details;
const isValidExpireTimer = typeof expireTimer === 'number';
if (isValidExpireTimer) {
const ourId = window.ConversationController.getOurConversationId();
const receivedAt = Date.now();

await conversation.updateExpirationTimer(
expireTimer,
ourId,
receivedAt,
window.ConversationController.getOurConversationId(),
undefined,
{
fromSync: true,
}
Expand Down Expand Up @@ -2699,11 +2696,10 @@ export async function startApp(): Promise<void> {
return;
}

const receivedAt = Date.now();
await conversation.updateExpirationTimer(
expireTimer,
window.ConversationController.getOurConversationId(),
receivedAt,
undefined,
{
fromSync: true,
}
Expand Down
20 changes: 13 additions & 7 deletions ts/models/conversations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4396,11 +4396,13 @@ export class ConversationModel extends window.Backbone
async updateExpirationTimer(
providedExpireTimer: number | undefined,
providedSource?: unknown,
receivedAt?: number,
initiatingMessage?: MessageModel,
options: { fromSync?: unknown; fromGroupUpdate?: unknown } = {}
): Promise<boolean | null | MessageModel | void> {
const isSetByOther = providedSource || initiatingMessage;

if (isGroupV2(this.attributes)) {
if (providedSource || receivedAt) {
if (isSetByOther) {
throw new Error(
'updateExpirationTimer: GroupV2 timers are not updated this way'
);
Expand Down Expand Up @@ -4444,7 +4446,7 @@ export class ConversationModel extends window.Backbone
});

// if change wasn't made remotely, send it to the number/group
if (!receivedAt) {
if (!isSetByOther) {
try {
await conversationJobQueue.add({
type: conversationQueueJobEnum.enum.DirectExpirationTimerUpdate,
Expand All @@ -4464,7 +4466,11 @@ export class ConversationModel extends window.Backbone

// When we add a disappearing messages notification to the conversation, we want it
// to be above the message that initiated that change, hence the subtraction.
const timestamp = (receivedAt || Date.now()) - 1;
const receivedAt =
initiatingMessage?.get('received_at') ||
window.Signal.Util.incrementMessageCounter();
const receivedAtMS = initiatingMessage?.get('received_at_ms') || Date.now();
const sentAt = (initiatingMessage?.get('sent_at') || receivedAtMS) - 1;

this.set({ expireTimer });

Expand All @@ -4480,9 +4486,9 @@ export class ConversationModel extends window.Backbone
readStatus: ReadStatus.Unread,
conversationId: this.id,
// No type; 'incoming' messages are specially treated by conversation.markRead()
sent_at: timestamp,
received_at: window.Signal.Util.incrementMessageCounter(),
received_at_ms: timestamp,
sent_at: sentAt,
received_at: receivedAt,
received_at_ms: receivedAtMS,
flags: Proto.DataMessage.Flags.EXPIRATION_TIMER_UPDATE,
expirationTimerUpdate: {
expireTimer,
Expand Down
10 changes: 2 additions & 8 deletions ts/models/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2480,8 +2480,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
conversation.updateExpirationTimer(
dataMessage.expireTimer,
source,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
message.getReceivedAt()!,
message,
{
fromGroupUpdate: isGroupUpdate(message.attributes),
}
Expand All @@ -2492,12 +2491,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
// We only turn off timers if it's not a group update
!isGroupUpdate(message.attributes)
) {
conversation.updateExpirationTimer(
undefined,
source,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
message.getReceivedAt()!
);
conversation.updateExpirationTimer(undefined, source, message);
}
}
}
Expand Down

0 comments on commit 6ae03fa

Please sign in to comment.