From 1e446b0a81d6e87db3df46301ca6fc29350b68ff Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 8 Sep 2020 09:37:51 +1000 Subject: [PATCH] fix order for open groups message which are out of sync --- js/background.js | 9 +++++++-- js/models/conversations.js | 6 +++++- js/models/messages.js | 13 +++++++++++++ js/modules/loki_app_dot_net_api.d.ts | 2 +- js/modules/loki_app_dot_net_api.js | 10 ++++++---- js/modules/loki_message_api.js | 3 ++- ts/session/sending/MessageQueue.ts | 5 +++-- ts/session/sending/MessageSender.ts | 4 ++-- ts/test/session/unit/sending/MessageQueue_test.ts | 8 ++++---- 9 files changed, 43 insertions(+), 17 deletions(-) diff --git a/js/background.js b/js/background.js index 9010d6f3d..8707c3a8b 100644 --- a/js/background.js +++ b/js/background.js @@ -1121,10 +1121,15 @@ Whisper.events.on( 'publicMessageSent', - ({ pubKey, timestamp, serverId }) => { + ({ pubKey, timestamp, serverId, serverTimestamp }) => { try { const conversation = ConversationController.get(pubKey); - conversation.onPublicMessageSent(pubKey, timestamp, serverId); + conversation.onPublicMessageSent( + pubKey, + timestamp, + serverId, + serverTimestamp + ); } catch (e) { window.log.error('Error setting public on message'); } diff --git a/js/models/conversations.js b/js/models/conversations.js index 6bf7dac12..111ed1a2f 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -540,12 +540,13 @@ await Promise.all(messages.map(m => m.setCalculatingPoW())); }, - async onPublicMessageSent(pubKey, timestamp, serverId) { + async onPublicMessageSent(pubKey, timestamp, serverId, serverTimestamp) { const messages = this._getMessagesWithTimestamp(pubKey, timestamp); await Promise.all( messages.map(message => [ message.setIsPublic(true), message.setServerId(serverId), + message.setServerTimestamp(serverTimestamp), ]) ); }, @@ -1295,6 +1296,9 @@ if (this.isPrivate()) { message.set({ destination }); } + if (this.isPublic()) { + message.setServerTimestamp(new Date().getTime()); + } const id = await window.Signal.Data.saveMessage(message.attributes, { Message: Whisper.Message, diff --git a/js/models/messages.js b/js/models/messages.js index 4e826b410..67f2fe7fb 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1441,6 +1441,19 @@ Message: Whisper.Message, }); }, + async setServerTimestamp(serverTimestamp) { + if (_.isEqual(this.get('serverTimestamp'), serverTimestamp)) { + return; + } + + this.set({ + serverTimestamp, + }); + + await window.Signal.Data.saveMessage(this.attributes, { + Message: Whisper.Message, + }); + }, async setIsPublic(isPublic) { if (_.isEqual(this.get('isPublic'), isPublic)) { return; diff --git a/js/modules/loki_app_dot_net_api.d.ts b/js/modules/loki_app_dot_net_api.d.ts index 75cb0f02d..68d705850 100644 --- a/js/modules/loki_app_dot_net_api.d.ts +++ b/js/modules/loki_app_dot_net_api.d.ts @@ -30,7 +30,7 @@ export interface LokiPublicChannelAPI { body?: string; }, timestamp: number - ): Promise; + ): Promise<{ serverId; serverTimestamp }>; } declare class LokiAppDotNetServerAPI implements LokiAppDotNetServerInterface { diff --git a/js/modules/loki_app_dot_net_api.js b/js/modules/loki_app_dot_net_api.js index b2a7a6b6e..921692309 100644 --- a/js/modules/loki_app_dot_net_api.js +++ b/js/modules/loki_app_dot_net_api.js @@ -2345,7 +2345,10 @@ class LokiPublicChannelAPI { objBody: payload, }); if (!res.err && res.response) { - return res.response.data.id; + return { + serverId: res.response.data.id, + serverTimestamp: new Date(`${res.response.data.created_at}`).getTime(), + }; } if (res.err) { log.error(`POST ${this.baseChannelUrl}/messages failed`); @@ -2357,9 +2360,8 @@ class LokiPublicChannelAPI { } else { log.warn(res.response); } - // there's no retry on desktop - // this is supposed to be after retries - return -1; + + return { serverId: -1, serverTimestamp: -1 }; } } diff --git a/js/modules/loki_message_api.js b/js/modules/loki_message_api.js index 586e57df3..fa67288cb 100644 --- a/js/modules/loki_message_api.js +++ b/js/modules/loki_message_api.js @@ -60,7 +60,8 @@ class LokiMessageAPI { 'Failed to send public chat message' ); } - messageEventData.serverId = res; + messageEventData.serverId = res.serverId; + messageEventData.serverTimestamp = res.serverTimestamp; window.Whisper.events.trigger('publicMessageSent', messageEventData); return; } diff --git a/ts/session/sending/MessageQueue.ts b/ts/session/sending/MessageQueue.ts index a92248378..d880e91f0 100644 --- a/ts/session/sending/MessageQueue.ts +++ b/ts/session/sending/MessageQueue.ts @@ -62,13 +62,14 @@ export class MessageQueue implements MessageQueueInterface { try { const result = await MessageSender.sendToOpenGroup(message); // sendToOpenGroup returns -1 if failed or an id if succeeded - if (result < 0) { + if (result.serverId < 0) { this.events.emit('fail', message, error); } else { const messageEventData = { pubKey: message.group.groupId, timestamp: message.timestamp, - serverId: result, + serverId: result.serverId, + serverTimestamp: result.serverTimestamp, }; this.events.emit('success', message); diff --git a/ts/session/sending/MessageSender.ts b/ts/session/sending/MessageSender.ts index 835f9468f..1477e25ce 100644 --- a/ts/session/sending/MessageSender.ts +++ b/ts/session/sending/MessageSender.ts @@ -98,7 +98,7 @@ function wrapEnvelope(envelope: SignalService.Envelope): Uint8Array { */ export async function sendToOpenGroup( message: OpenGroupMessage -): Promise { +): Promise<{ serverId: number; serverTimestamp: number }> { /* Note: Retrying wasn't added to this but it can be added in the future if needed. The only problem is that `channelAPI.sendMessage` returns true/false and doesn't throw any error so we can never be sure why sending failed. @@ -112,7 +112,7 @@ export async function sendToOpenGroup( ); if (!channelAPI) { - return -1; + return { serverId: -1, serverTimestamp: -1 }; } // Returns -1 on fail or an id > 0 on success diff --git a/ts/test/session/unit/sending/MessageQueue_test.ts b/ts/test/session/unit/sending/MessageQueue_test.ts index c300eb62b..6e07eac2f 100644 --- a/ts/test/session/unit/sending/MessageQueue_test.ts +++ b/ts/test/session/unit/sending/MessageQueue_test.ts @@ -321,12 +321,12 @@ describe('MessageQueue', () => { describe('open groups', async () => { let sendToOpenGroupStub: sinon.SinonStub< [OpenGroupMessage], - Promise + Promise<{ serverId: number; serverTimestamp: number }> >; beforeEach(() => { sendToOpenGroupStub = sandbox .stub(MessageSender, 'sendToOpenGroup') - .resolves(-1); + .resolves({ serverId: -1, serverTimestamp: -1 }); }); it('can send to open group', async () => { @@ -336,7 +336,7 @@ describe('MessageQueue', () => { }); it('should emit a success event when send was successful', async () => { - sendToOpenGroupStub.resolves(123456); + sendToOpenGroupStub.resolves({ serverId: -1, serverTimestamp: -1 }); const message = TestUtils.generateOpenGroupMessage(); const eventPromise = PromiseUtils.waitForTask(complete => { @@ -348,7 +348,7 @@ describe('MessageQueue', () => { }); it('should emit a fail event if something went wrong', async () => { - sendToOpenGroupStub.resolves(-1); + sendToOpenGroupStub.resolves({ serverId: -1, serverTimestamp: -1 }); const message = TestUtils.generateOpenGroupMessage(); const eventPromise = PromiseUtils.waitForTask(complete => { messageQueueStub.events.once('fail', complete);