From 44349079abe652a484aab62e8b35e1ced8f55ac4 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 31 Jul 2020 09:49:36 +1000 Subject: [PATCH 1/3] fix delete of open group messages --- js/modules/loki_app_dot_net_api.d.ts | 2 +- ts/session/sending/MessageQueue.ts | 14 +++++++++++--- ts/session/sending/MessageSender.ts | 16 +--------------- ts/test/session/sending/MessageQueue_test.ts | 2 +- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/js/modules/loki_app_dot_net_api.d.ts b/js/modules/loki_app_dot_net_api.d.ts index f578c40ab..34947e0af 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; } declare class LokiAppDotNetServerAPI implements LokiAppDotNetServerInterface { diff --git a/ts/session/sending/MessageQueue.ts b/ts/session/sending/MessageQueue.ts index 228d9d8be..68ed00efc 100644 --- a/ts/session/sending/MessageQueue.ts +++ b/ts/session/sending/MessageQueue.ts @@ -61,10 +61,18 @@ export class MessageQueue implements MessageQueueInterface { // This is absolutely yucky ... we need to make it not use Promise try { const result = await MessageSender.sendToOpenGroup(message); - if (result) { - this.events.emit('success', message); - } else { + // sendToOpenGroup returns false if failed or a number if succeeded + if (typeof result === 'boolean') { this.events.emit('fail', message, error); + } else { + const messageEventData = { + pubKey: message.group.groupId, + timestamp: message.timestamp, + serverId: result, + }; + this.events.emit('success', message); + + window.Whisper.events.trigger('publicMessageSent', messageEventData); } } catch (e) { console.warn( diff --git a/ts/session/sending/MessageSender.ts b/ts/session/sending/MessageSender.ts index d138cba81..035e4e6d8 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 { /* 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. @@ -125,18 +125,4 @@ export async function sendToOpenGroup( }, timestamp ); - - // TODO: The below should be handled in whichever class calls this - /* - const res = await sendToOpenGroup(message); - if (!res) { - throw new textsecure.PublicChatError('Failed to send public chat message'); - } - const messageEventData = { - pubKey, - timestamp: messageTimeStamp, - }; - messageEventData.serverId = res; - window.Whisper.events.trigger('publicMessageSent', messageEventData); - */ } diff --git a/ts/test/session/sending/MessageQueue_test.ts b/ts/test/session/sending/MessageQueue_test.ts index 4e1644d27..c92bb03e2 100644 --- a/ts/test/session/sending/MessageQueue_test.ts +++ b/ts/test/session/sending/MessageQueue_test.ts @@ -324,7 +324,7 @@ describe('MessageQueue', () => { describe('open groups', async () => { let sendToOpenGroupStub: sinon.SinonStub< [OpenGroupMessage], - Promise + Promise >; beforeEach(() => { sendToOpenGroupStub = sandbox From 2637981fdb3c4c9c7eb9b8a0df8f4b2fb0b2b722 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 31 Jul 2020 11:03:52 +1000 Subject: [PATCH 2/3] fix tests --- ts/test/session/sending/MessageQueue_test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ts/test/session/sending/MessageQueue_test.ts b/ts/test/session/sending/MessageQueue_test.ts index c92bb03e2..c2501b795 100644 --- a/ts/test/session/sending/MessageQueue_test.ts +++ b/ts/test/session/sending/MessageQueue_test.ts @@ -339,6 +339,8 @@ describe('MessageQueue', () => { }); it('should emit a success event when send was successful', async () => { + sendToOpenGroupStub.resolves(123456); + const message = TestUtils.generateOpenGroupMessage(); const eventPromise = PromiseUtils.waitForTask(complete => { messageQueueStub.events.once('success', complete); From 6d65c9cc0a6d02133b0f3597b158fdaf995afe01 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 31 Jul 2020 11:15:37 +1000 Subject: [PATCH 3/3] make sendMessage return -1 on fail rather than false --- js/modules/loki_app_dot_net_api.d.ts | 2 +- js/modules/loki_app_dot_net_api.js | 2 +- ts/session/sending/MessageQueue.ts | 4 ++-- ts/session/sending/MessageSender.ts | 6 +++--- ts/test/session/sending/MessageQueue_test.ts | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/js/modules/loki_app_dot_net_api.d.ts b/js/modules/loki_app_dot_net_api.d.ts index 34947e0af..318facf52 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; } 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 cc05254cc..526f1b44f 100644 --- a/js/modules/loki_app_dot_net_api.js +++ b/js/modules/loki_app_dot_net_api.js @@ -2355,7 +2355,7 @@ class LokiPublicChannelAPI { } // there's no retry on desktop // this is supposed to be after retries - return false; + return -1; } } diff --git a/ts/session/sending/MessageQueue.ts b/ts/session/sending/MessageQueue.ts index 68ed00efc..a92248378 100644 --- a/ts/session/sending/MessageQueue.ts +++ b/ts/session/sending/MessageQueue.ts @@ -61,8 +61,8 @@ export class MessageQueue implements MessageQueueInterface { // This is absolutely yucky ... we need to make it not use Promise try { const result = await MessageSender.sendToOpenGroup(message); - // sendToOpenGroup returns false if failed or a number if succeeded - if (typeof result === 'boolean') { + // sendToOpenGroup returns -1 if failed or an id if succeeded + if (result < 0) { this.events.emit('fail', message, error); } else { const messageEventData = { diff --git a/ts/session/sending/MessageSender.ts b/ts/session/sending/MessageSender.ts index 035e4e6d8..835f9468f 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 { /* 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,10 +112,10 @@ export async function sendToOpenGroup( ); if (!channelAPI) { - return false; + return -1; } - // Don't think returning true/false on `sendMessage` is a good way + // Returns -1 on fail or an id > 0 on success return channelAPI.sendMessage( { quote, diff --git a/ts/test/session/sending/MessageQueue_test.ts b/ts/test/session/sending/MessageQueue_test.ts index c2501b795..f0d94ac4a 100644 --- a/ts/test/session/sending/MessageQueue_test.ts +++ b/ts/test/session/sending/MessageQueue_test.ts @@ -324,12 +324,12 @@ describe('MessageQueue', () => { describe('open groups', async () => { let sendToOpenGroupStub: sinon.SinonStub< [OpenGroupMessage], - Promise + Promise >; beforeEach(() => { sendToOpenGroupStub = sandbox .stub(MessageSender, 'sendToOpenGroup') - .resolves(true); + .resolves(-1); }); it('can send to open group', async () => { @@ -351,7 +351,7 @@ describe('MessageQueue', () => { }); it('should emit a fail event if something went wrong', async () => { - sendToOpenGroupStub.resolves(false); + sendToOpenGroupStub.resolves(-1); const message = TestUtils.generateOpenGroupMessage(); const eventPromise = PromiseUtils.waitForTask(complete => { messageQueueStub.events.once('fail', complete);