diff --git a/ts/components/conversation/message/message-item/GenericReadableMessage.tsx b/ts/components/conversation/message/message-item/GenericReadableMessage.tsx index 24ff43023..742050313 100644 --- a/ts/components/conversation/message/message-item/GenericReadableMessage.tsx +++ b/ts/components/conversation/message/message-item/GenericReadableMessage.tsx @@ -8,7 +8,7 @@ import _ from 'lodash'; import { Data } from '../../../../data/data'; import { MessageRenderingProps } from '../../../../models/messageType'; import { getConversationController } from '../../../../session/conversations'; -import { messageExpired } from '../../../../state/ducks/conversations'; +import { messagesExpired } from '../../../../state/ducks/conversations'; import { getGenericReadableMessageSelectorProps, getIsMessageSelected, @@ -68,10 +68,12 @@ function useIsExpired(props: ExpiringProps) { await Data.removeMessage(messageId); if (convoId) { dispatch( - messageExpired({ - conversationKey: convoId, - messageId, - }) + messagesExpired([ + { + conversationKey: convoId, + messageId, + }, + ]) ); const convo = getConversationController().get(convoId); convo?.updateLastMessage(); diff --git a/ts/data/data.ts b/ts/data/data.ts index 8374850c4..bc5b92a1d 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -139,7 +139,7 @@ export const Data = { saveMessage, saveMessages, removeMessage, - _removeMessages, + removeMessagesByIds, getMessageIdsFromServerIds, getMessageById, getMessageBySenderAndSentAt, @@ -396,9 +396,13 @@ async function removeMessage(id: string): Promise { } } -// Note: this method will not clean up external files, just delete from SQL -async function _removeMessages(ids: Array): Promise { - await channels.removeMessage(ids); +/** + * Note: this method will not clean up external files, just delete from SQL. + * Files are cleaned up on app start if they are not linked to any messages + * + */ +async function removeMessagesByIds(ids: Array): Promise { + await channels.removeMessagesByIds(ids); } async function getMessageIdsFromServerIds( @@ -644,7 +648,7 @@ async function removeAllMessagesInConversation(conversationId: string): Promise< await Promise.all(messages.map(message => message.cleanup())); // eslint-disable-next-line no-await-in-loop - await channels.removeMessage(ids); + await channels.removeMessagesByIds(ids); } while (messages.length > 0); } diff --git a/ts/data/dataInit.ts b/ts/data/dataInit.ts index 1bd0f40b7..f838adfcc 100644 --- a/ts/data/dataInit.ts +++ b/ts/data/dataInit.ts @@ -38,7 +38,7 @@ const channelsToMake = new Set([ 'saveSeenMessageHashes', 'saveMessages', 'removeMessage', - '_removeMessages', + 'removeMessagesByIds', 'getUnreadByConversation', 'markAllAsReadByConversationNoExpiration', 'getUnreadCountByConversation', diff --git a/ts/mains/main_renderer.tsx b/ts/mains/main_renderer.tsx index 4c3ca8df4..c5879d65c 100644 --- a/ts/mains/main_renderer.tsx +++ b/ts/mains/main_renderer.tsx @@ -217,8 +217,10 @@ async function start() { ); window.log.info(`Cleanup: Found ${messagesForCleanup.length} messages for cleanup`); + + const idsToCleanUp: Array = []; await Promise.all( - messagesForCleanup.map(async (message: MessageModel) => { + messagesForCleanup.map((message: MessageModel) => { const sentAt = message.get('sent_at'); if (message.hasErrors()) { @@ -226,9 +228,12 @@ async function start() { } window.log.info(`Cleanup: Deleting unsent message ${sentAt}`); - await Data.removeMessage(message.id); + idsToCleanUp.push(message.id); }) ); + if (idsToCleanUp.length) { + await Data.removeMessagesByIds(idsToCleanUp); + } window.log.info('Cleanup: complete'); window.log.info('listening for registration events'); diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index 266629ee6..820bab980 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -249,12 +249,6 @@ export class ConversationModel extends Backbone.Model { await deleteExternalFilesOfConversation(this.attributes); } - public async onExpired(_message: MessageModel) { - await this.updateLastMessage(); - - // removeMessage(); - } - public getGroupAdmins(): Array { const groupAdmins = this.get('groupAdmins'); @@ -478,26 +472,6 @@ export class ConversationModel extends Backbone.Model { return true; } - public async onReadMessage(message: MessageModel, readAt: number) { - // We mark as read everything older than this message - to clean up old stuff - // still marked unread in the database. If the user generally doesn't read in - // the desktop app, so the desktop app only gets read syncs, we can very - // easily end up with messages never marked as read (our previous early read - // sync handling, read syncs never sent because app was offline) - - // We queue it because we often get a whole lot of read syncs at once, and - // their markRead calls could very easily overlap given the async pull from DB. - - // Lastly, we don't send read syncs for any message marked read due to a read - // sync. That's a notification explosion we don't need. - return this.queueJob(() => - this.markReadBouncy(message.get('received_at') as any, { - sendReadReceipts: false, - readAt, - }) - ); - } - public async getUnreadCount() { const unreadCount = await Data.getUnreadCountByConversation(this.id); @@ -1693,15 +1667,17 @@ export class ConversationModel extends Backbone.Model { return this.get('type') === ConversationTypeEnum.GROUP; } - public async removeMessage(messageId: any) { + public async removeMessage(messageId: string) { await Data.removeMessage(messageId); this.updateLastMessage(); window.inboxStore?.dispatch( - conversationActions.messageDeleted({ - conversationKey: this.id, - messageId, - }) + conversationActions.messagesDeleted([ + { + conversationKey: this.id, + messageId, + }, + ]) ); } diff --git a/ts/models/message.ts b/ts/models/message.ts index 9671b95d6..2096133ee 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -125,7 +125,6 @@ export class MessageModel extends Backbone.Model { throw new Error('A message always needs to have an conversationId.'); } - // this.on('expired', this.onExpired); if (!attributes.skipTimerInit) { void this.setToExpire(); } diff --git a/ts/node/sql.ts b/ts/node/sql.ts index 33ae61980..0f6fd847d 100644 --- a/ts/node/sql.ts +++ b/ts/node/sql.ts @@ -945,21 +945,44 @@ function saveMessages(arrayOfMessages: Array) { } function removeMessage(id: string, instance?: BetterSqlite3.Database) { - if (!Array.isArray(id)) { - assertGlobalInstanceOrInstance(instance) - .prepare(`DELETE FROM ${MESSAGES_TABLE} WHERE id = $id;`) - .run({ id }); + if (!isString(id)) { + throw new Error('removeMessage: only takes single message to delete!'); + return; } - if (!id.length) { - throw new Error('removeMessages: No ids to delete!'); + assertGlobalInstanceOrInstance(instance) + .prepare(`DELETE FROM ${MESSAGES_TABLE} WHERE id = $id;`) + .run({ id }); +} + +function removeMessagesByIds(ids: Array, instance?: BetterSqlite3.Database) { + if (!Array.isArray(ids)) { + throw new Error('removeMessagesByIds only allowed an array of strings'); + } + + if (!ids.length) { + throw new Error('removeMessagesByIds: No ids to delete!'); } // Our node interface doesn't seem to allow you to replace one single ? with an array assertGlobalInstanceOrInstance(instance) - .prepare(`DELETE FROM ${MESSAGES_TABLE} WHERE id IN ( ${id.map(() => '?').join(', ')} );`) - .run(id); + .prepare(`DELETE FROM ${MESSAGES_TABLE} WHERE id IN ( ${ids.map(() => '?').join(', ')} );`) + .run(ids); +} + +function removeAllMessagesInConversation( + conversationId: string, + instance?: BetterSqlite3.Database +) { + if (!conversationId) { + return; + } + + // Our node interface doesn't seem to allow you to replace one single ? with an array + assertGlobalInstanceOrInstance(instance) + .prepare(`DELETE FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId`) + .run({ conversationId }); } function getMessageIdsFromServerIds(serverIds: Array, conversationId: string) { @@ -2440,6 +2463,8 @@ export const sqlNode = { updateLastHash, saveMessages, removeMessage, + removeMessagesByIds, + removeAllMessagesInConversation, getUnreadByConversation, markAllAsReadByConversationNoExpiration, getUnreadCountByConversation, diff --git a/ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts b/ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts index dbaa248de..731fb6f34 100644 --- a/ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts +++ b/ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts @@ -1,4 +1,4 @@ -import _, { compact, isArray, isNumber, isObject, pick } from 'lodash'; +import _, { compact, isArray, isEmpty, isNumber, isObject, pick } from 'lodash'; import { OpenGroupData } from '../../../../data/opengroups'; import { handleOpenGroupV4Message } from '../../../../receiver/opengroup'; import { OpenGroupRequestCommonType } from '../opengroupV2/ApiUtil'; @@ -35,6 +35,8 @@ import { ConversationTypeEnum } from '../../../../models/conversationAttributes' import { createSwarmMessageSentFromUs } from '../../../../models/messageFactory'; import { Data } from '../../../../data/data'; import { processMessagesUsingCache } from './sogsV3MutationCache'; +import { destroyMessagesAndUpdateRedux } from '../../../../util/expiringMessages'; +import { sogsRollingDeletions } from './sogsRollingDeletions'; /** * Get the convo matching those criteria and make sure it is an opengroup convo, or return null. @@ -159,34 +161,38 @@ const handleSogsV3DeletedMessages = async ( serverUrl: string, roomId: string ) => { - const deletions = messages.filter(m => Boolean(m.deleted)); - const exceptDeletion = messages.filter(m => !m.deleted); - if (!deletions.length) { - return messages; + const messagesDeleted = messages.filter(m => Boolean(m.deleted)); + const messagesWithoutDeleted = messages.filter(m => !m.deleted); + if (!messagesDeleted.length) { + return messagesWithoutDeleted; } - const allIdsRemoved = deletions.map(m => m.id); + + const allIdsRemoved = messagesDeleted.map(m => m.id); + try { const convoId = getOpenGroupV2ConversationId(serverUrl, roomId); const convo = getConversationController().get(convoId); const messageIds = await Data.getMessageIdsFromServerIds(allIdsRemoved, convo.id); - // we shouldn't get too many messages to delete at a time, so no need to add a function to remove multiple messages for now + allIdsRemoved.forEach(removedId => { + sogsRollingDeletions.addMessageDeletedId(convoId, removedId); + }); - await Promise.all( - (messageIds || []).map(async id => { - if (convo) { - await convo.removeMessage(id); - } - await Data.removeMessage(id); - }) - ); + if (messageIds && messageIds.length) { + await destroyMessagesAndUpdateRedux( + messageIds.map(messageId => ({ + conversationKey: convoId, + messageId, + })) + ); + } } catch (e) { window?.log?.warn('handleDeletions failed:', e); } - return exceptDeletion; + return messagesWithoutDeleted; }; -// tslint:disable-next-line: cyclomatic-complexity +// tslint:disable-next-line: max-func-body-length cyclomatic-complexity const handleMessagesResponseV4 = async ( messages: Array, serverUrl: string, @@ -284,6 +290,7 @@ const handleMessagesResponseV4 = async ( const incomingMessageSeqNo = compact(messages.map(n => n.seqno)); const maxNewMessageSeqNo = Math.max(...incomingMessageSeqNo); + for (let index = 0; index < messagesWithResolvedBlindedIdsIfFound.length; index++) { const msgToHandle = messagesWithResolvedBlindedIdsIfFound[index]; try { @@ -309,13 +316,24 @@ const handleMessagesResponseV4 = async ( await OpenGroupData.saveV2OpenGroupRoom(roomInfosRefreshed); const messagesWithReactions = messages.filter(m => m.reactions !== undefined); + if (messagesWithReactions.length > 0) { const conversationId = getOpenGroupV2ConversationId(serverUrl, roomId); const groupConvo = getConversationController().get(conversationId); if (groupConvo && groupConvo.isOpenGroupV2()) { - for (const message of messagesWithReactions) { + for (const messageWithReaction of messagesWithReactions) { + if (isEmpty(messageWithReaction.reactions)) { + /* + * When a message is deleted from the server, we get the deleted event as a data: null on the message itself + * and an update on its reactions. + * But, because we just deleted that message, we can skip trying to update its reactions: it's not in the DB anymore. + */ + if (sogsRollingDeletions.hasMessageDeletedId(conversationId, messageWithReaction.id)) { + continue; + } + } void groupConvo.queueJob(async () => { - await processMessagesUsingCache(serverUrl, roomId, message); + await processMessagesUsingCache(serverUrl, roomId, messageWithReaction); }); } } @@ -526,6 +544,7 @@ export const handleBatchPollResults = async ( break; case 'pollInfo': await handlePollInfoResponse(subResponse.code, subResponse.body, serverUrl); + break; case 'inbox': await handleInboxOutboxMessages(subResponse.body, serverUrl, false); diff --git a/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts b/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts new file mode 100644 index 000000000..9582c6e1b --- /dev/null +++ b/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts @@ -0,0 +1,48 @@ +import { RingBuffer } from '../../../utils/RingBuffer'; + +const rollingDeletedMessageIds: Map> = new Map(); + +const addMessageDeletedId = (conversationId: string, messageDeletedId: number) => { + if (!rollingDeletedMessageIds.has(conversationId)) { + rollingDeletedMessageIds.set( + conversationId, + new RingBuffer(sogsRollingDeletions.getPerRoomCount()) + ); + } + const ringBuffer = rollingDeletedMessageIds.get(conversationId); + if (!ringBuffer) { + return; + } + ringBuffer.insert(messageDeletedId); +}; + +const hasMessageDeletedId = (conversationId: string, messageDeletedId: number) => { + if (!rollingDeletedMessageIds.has(conversationId)) { + return false; + } + + const messageIdWasDeletedRecently = rollingDeletedMessageIds + ?.get(conversationId) + ?.has(messageDeletedId); + + return messageIdWasDeletedRecently; +}; + +/** + * emptyMessageDeleteIds should only be used for testing purposes. + */ +const emptyMessageDeleteIds = () => { + rollingDeletedMessageIds.clear(); +}; + +export const sogsRollingDeletions = { + addMessageDeletedId, + hasMessageDeletedId, + emptyMessageDeleteIds, + getPerRoomCount, +}; + +// keep 2000 deleted message ids in memory +function getPerRoomCount() { + return 2000; +} diff --git a/ts/session/utils/RingBuffer.ts b/ts/session/utils/RingBuffer.ts new file mode 100644 index 000000000..259a3962a --- /dev/null +++ b/ts/session/utils/RingBuffer.ts @@ -0,0 +1,76 @@ +/** + * This ringbuffer class can be used to keep a list of at most a size and removing old items first when the size is exceeded. + * Internally, it uses an array to keep track of the order, so two times the same item can exist in it. + * + */ +export class RingBuffer { + private newest = -1; + private oldest = 0; + private buffer: Array = []; + private readonly capacity: number; + + constructor(capacity: number) { + this.capacity = capacity; + } + + public getCapacity(): number { + return this.capacity; + } + + public getLength(): number { + if (this.isEmpty()) { + return 0; + } + + // When only one item was added, newest = 0 and oldest = 0. + // When more than one item was added, but less than capacity, newest = nbItemsAdded & oldest = 0. + // As soon as we overflow, oldest is incremented to oldest+1 and newest rolls back to 0, + // so this test fails here and we have to extract the length based on the two parts instead. + if (this.newest >= this.oldest) { + return this.newest + 1; + } + const firstPart = this.capacity - this.oldest; + const secondPart = this.newest + 1; + return firstPart + secondPart; + } + + public insert(item: T) { + // see comments in `getLength()` + this.newest = (this.newest + 1) % this.capacity; + if (this.buffer.length >= this.capacity) { + this.oldest = (this.oldest + 1) % this.capacity; + } + this.buffer[this.newest] = item; + } + + public has(item: T) { + // no items at all + if (this.isEmpty()) { + return false; + } + return this.toArray().includes(item); + } + + public isEmpty() { + return this.newest === -1; + } + + public clear() { + this.buffer = []; + this.newest = -1; + this.oldest = 0; + } + + public toArray(): Array { + if (this.isEmpty()) { + return []; + } + + if (this.newest >= this.oldest) { + return this.buffer.slice(0, this.newest + 1); + } + const firstPart = this.buffer.slice(this.oldest, this.capacity); + const secondPart = this.buffer.slice(0, this.newest + 1); + return [...firstPart, ...secondPart]; + } +} diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 6c2dbd28c..12c3fb9f2 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -504,12 +504,12 @@ function handleMessagesChangedOrAdded( function handleMessageExpiredOrDeleted( state: ConversationsStateType, - action: PayloadAction<{ + payload: { messageId: string; conversationKey: string; - }> -): ConversationsStateType { - const { conversationKey, messageId } = action.payload; + } +) { + const { conversationKey, messageId } = payload; if (conversationKey === state.selectedConversation) { // search if we find this message id. // we might have not loaded yet, so this case might not happen @@ -539,6 +539,23 @@ function handleMessageExpiredOrDeleted( return state; } +function handleMessagesExpiredOrDeleted( + state: ConversationsStateType, + action: PayloadAction< + Array<{ + messageId: string; + conversationKey: string; + }> + > +): ConversationsStateType { + action.payload.forEach(element => { + // tslint:disable-next-line: no-parameter-reassignment + state = handleMessageExpiredOrDeleted(state, element); + }); + + return state; +} + function handleConversationReset(state: ConversationsStateType, action: PayloadAction) { const conversationKey = action.payload; if (conversationKey === state.selectedConversation) { @@ -670,24 +687,28 @@ const conversationsSlice = createSlice({ return handleMessagesChangedOrAdded(state, action.payload); }, - messageExpired( + messagesExpired( state: ConversationsStateType, - action: PayloadAction<{ - messageId: string; - conversationKey: string; - }> + action: PayloadAction< + Array<{ + messageId: string; + conversationKey: string; + }> + > ) { - return handleMessageExpiredOrDeleted(state, action); + return handleMessagesExpiredOrDeleted(state, action); }, - messageDeleted( + messagesDeleted( state: ConversationsStateType, - action: PayloadAction<{ - messageId: string; - conversationKey: string; - }> + action: PayloadAction< + Array<{ + messageId: string; + conversationKey: string; + }> + > ) { - return handleMessageExpiredOrDeleted(state, action); + return handleMessagesExpiredOrDeleted(state, action); }, conversationReset(state: ConversationsStateType, action: PayloadAction) { @@ -985,8 +1006,8 @@ export const { conversationsChanged, conversationRemoved, removeAllConversations, - messageExpired, - messageDeleted, + messagesExpired, + messagesDeleted, conversationReset, messagesChanged, resetOldTopMessageId, diff --git a/ts/test/session/unit/sogsv3/sogsRollingDeletions_test.ts b/ts/test/session/unit/sogsv3/sogsRollingDeletions_test.ts new file mode 100644 index 000000000..922b85f41 --- /dev/null +++ b/ts/test/session/unit/sogsv3/sogsRollingDeletions_test.ts @@ -0,0 +1,71 @@ +import { expect } from 'chai'; +import Sinon from 'sinon'; +import { sogsRollingDeletions } from '../../../../session/apis/open_group_api/sogsv3/sogsRollingDeletions'; + +describe('sogsRollingDeletions', () => { + beforeEach(() => { + sogsRollingDeletions.emptyMessageDeleteIds(); + Sinon.stub(sogsRollingDeletions, 'getPerRoomCount').returns(5); + }); + + afterEach(() => { + Sinon.restore(); + }); + + it('no items at all returns false', () => { + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 1)).to.be.equal( + false, + '1 should not be there' + ); + }); + + it('no items in that convo returns false', () => { + sogsRollingDeletions.addMessageDeletedId('convo1', 1); + + expect(sogsRollingDeletions.hasMessageDeletedId('convo2', 1)).to.be.equal( + false, + '1 should not be there' + ); + }); + + it('can add 1 item', () => { + sogsRollingDeletions.addMessageDeletedId('convo1', 1); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 1)).to.be.equal( + true, + '1 should be there' + ); + }); + + it('can add more than capacity items', () => { + sogsRollingDeletions.addMessageDeletedId('convo1', 1); + sogsRollingDeletions.addMessageDeletedId('convo1', 2); + sogsRollingDeletions.addMessageDeletedId('convo1', 3); + sogsRollingDeletions.addMessageDeletedId('convo1', 4); + sogsRollingDeletions.addMessageDeletedId('convo1', 5); + sogsRollingDeletions.addMessageDeletedId('convo1', 6); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 1)).to.be.equal( + false, + '1 should not be there' + ); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 2)).to.be.equal( + true, + '2 should be there' + ); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 3)).to.be.equal( + true, + '3 should be there' + ); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 4)).to.be.equal( + true, + '4 should be there' + ); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 5)).to.be.equal( + true, + '5 should be there' + ); + expect(sogsRollingDeletions.hasMessageDeletedId('convo1', 6)).to.be.equal( + true, + '6 should be there' + ); + }); +}); diff --git a/ts/test/session/unit/utils/RingBuffer_test.ts b/ts/test/session/unit/utils/RingBuffer_test.ts new file mode 100644 index 000000000..add8aa712 --- /dev/null +++ b/ts/test/session/unit/utils/RingBuffer_test.ts @@ -0,0 +1,224 @@ +// tslint:disable: no-implicit-dependencies max-func-body-length no-unused-expression no-require-imports no-var-requires + +import chai from 'chai'; +import { RingBuffer } from '../../../../session/utils/RingBuffer'; + +const { expect } = chai; + +describe('RingBuffer Utils', () => { + it('gets created with right capacity', () => { + const ring = new RingBuffer(5000); + expect(ring.getCapacity()).to.equal(5000); + expect(ring.getLength()).to.equal(0); + expect(ring.has(0)).to.equal(false, '0 should not be there'); + }); + + describe('length & capacity are right', () => { + it('length is right 0', () => { + const ring = new RingBuffer(4); + expect(ring.getLength()).to.equal(0); + }); + + it('length is right 1', () => { + const ring = new RingBuffer(4); + ring.insert(0); + expect(ring.getLength()).to.equal(1); + }); + + it('length is right 4', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + expect(ring.getLength()).to.equal(4); + }); + + it('capacity does not get exceeded', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); + expect(ring.getLength()).to.equal(4); + }); + }); + + describe('isEmpty is correct', () => { + it('no items', () => { + const ring = new RingBuffer(4); + expect(ring.isEmpty()).to.equal(true, 'no items isEmpty should be true'); + }); + + it('length is right 1', () => { + const ring = new RingBuffer(4); + ring.insert(0); + expect(ring.isEmpty()).to.equal(false, '1 item isEmpty should be false'); + }); + + it('length is right 4', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + expect(ring.isEmpty()).to.equal(false, '4 items isEmpty should be false'); + }); + + it('more than capacity', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); + expect(ring.isEmpty()).to.equal(false, '5 item isEmpty should be false'); + }); + }); + + it('items are removed in order 1', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); + expect(ring.has(0)).to.equal(false, '0 should not be there anymore'); + expect(ring.has(1)).to.equal(true, '1 should still be there'); + expect(ring.has(2)).to.equal(true, '2 should still be there'); + expect(ring.has(3)).to.equal(true, '3 should still be there'); + expect(ring.has(4)).to.equal(true, '4 should still be there'); + }); + + it('two times the same items can exist', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(1); + ring.insert(4); + expect(ring.has(0)).to.equal(false, '0 should not be there anymore'); + expect(ring.has(1)).to.equal(true, '1 should still be there'); + expect(ring.has(2)).to.equal(true, '2 should still be there'); + expect(ring.has(3)).to.equal(false, '3 should not be there'); + expect(ring.has(4)).to.equal(true, '4 should still be there'); + }); + + it('items are removed in order completely', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(10); + ring.insert(20); + ring.insert(30); + ring.insert(40); + expect(ring.has(0)).to.equal(false, '0 should not be there anymore'); + expect(ring.has(1)).to.equal(false, '1 should not be there'); + expect(ring.has(2)).to.equal(false, '2 should not be there'); + expect(ring.has(3)).to.equal(false, '3 should not be there'); + expect(ring.has(4)).to.equal(false, '4 should not be there'); + + expect(ring.has(10)).to.equal(true, '10 should still be there'); + expect(ring.has(20)).to.equal(true, '20 should still be there'); + expect(ring.has(30)).to.equal(true, '30 should still be there'); + expect(ring.has(40)).to.equal(true, '40 should still be there'); + }); + + it('clear empties the list but keeps the capacity', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(1); + expect(ring.getLength()).to.equal(4); + expect(ring.getCapacity()).to.equal(4); + ring.clear(); + expect(ring.getCapacity()).to.equal(4); + + expect(ring.getLength()).to.equal(0); + }); + + describe('toArray', () => { + it('empty buffer', () => { + const ring = new RingBuffer(4); + expect(ring.toArray()).to.deep.eq([]); + }); + + it('with 1', () => { + const ring = new RingBuffer(4); + ring.insert(0); + + expect(ring.toArray()).to.deep.eq([0]); + }); + + it('with 4', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + + expect(ring.toArray()).to.deep.eq([0, 1, 2, 3]); + }); + + it('with 5', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); + + expect(ring.toArray()).to.deep.eq([1, 2, 3, 4]); + }); + + it('more than 2 full laps erasing data', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); // first lap first item + ring.insert(5); + ring.insert(6); // first item in toArray should be this one + ring.insert(7); + ring.insert(8); // second lap first item + ring.insert(9); + + expect(ring.toArray()).to.deep.eq([6, 7, 8, 9]); + }); + }); + + describe('clear', () => { + it('empty buffer', () => { + const ring = new RingBuffer(4); + ring.clear(); + expect(ring.getCapacity()).to.deep.eq(4); + expect(ring.getLength()).to.deep.eq(0); + }); + + it('with 1', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.clear(); + expect(ring.getCapacity()).to.deep.eq(4); + expect(ring.getLength()).to.deep.eq(0); + }); + + it('with 5', () => { + const ring = new RingBuffer(4); + ring.insert(0); + ring.insert(1); + ring.insert(2); + ring.insert(3); + ring.insert(4); + + ring.clear(); + expect(ring.getCapacity()).to.deep.eq(4); + expect(ring.getLength()).to.deep.eq(0); + }); + }); +}); diff --git a/ts/util/expiringMessages.ts b/ts/util/expiringMessages.ts index 147e0f51d..31e1381d5 100644 --- a/ts/util/expiringMessages.ts +++ b/ts/util/expiringMessages.ts @@ -1,42 +1,61 @@ -import _ from 'lodash'; +import { throttle, uniq } from 'lodash'; import moment from 'moment'; -import { MessageModel } from '../models/message'; -import { messageExpired } from '../state/ducks/conversations'; +import { messagesExpired } from '../state/ducks/conversations'; import { TimerOptionsArray } from '../state/ducks/timerOptions'; import { LocalizerKeys } from '../types/LocalizerKeys'; import { initWallClockListener } from './wallClockListener'; import { Data } from '../data/data'; +import { getConversationController } from '../session/conversations'; + +export async function destroyMessagesAndUpdateRedux( + messages: Array<{ + conversationKey: string; + messageId: string; + }> +) { + if (!messages.length) { + return; + } + const conversationWithChanges = uniq(messages.map(m => m.conversationKey)); + + try { + // Delete all thoses messages in a single sql call + await Data.removeMessagesByIds(messages.map(m => m.messageId)); + } catch (e) { + window.log.error('destroyMessages: removeMessagesByIds failed', e && e.message ? e.message : e); + } + // trigger a redux update if needed for all those messages + window.inboxStore?.dispatch(messagesExpired(messages)); + + // trigger a refresh the last message for all those uniq conversation + conversationWithChanges.map(convoIdToUpdate => { + getConversationController() + .get(convoIdToUpdate) + ?.updateLastMessage(); + }); +} async function destroyExpiredMessages() { try { window.log.info('destroyExpiredMessages: Loading messages...'); const messages = await Data.getExpiredMessages(); - await Promise.all( - messages.map(async (message: MessageModel) => { - window.log.info('Message expired', { - sentAt: message.get('sent_at'), - }); - - // We delete after the trigger to allow the conversation time to process - // the expiration before the message is removed from the database. - await Data.removeMessage(message.id); - - // trigger the expiration of the message on the redux itself. - window.inboxStore?.dispatch( - messageExpired({ - conversationKey: message.attributes.conversationId, - messageId: message.id, - }) - ); - - const conversation = message.getConversation(); - if (conversation) { - await conversation.onExpired(message); - } - }) - ); + const messagesExpiredDetails: Array<{ + conversationKey: string; + messageId: string; + }> = messages.map(m => ({ + conversationKey: m.attributes.conversationId, + messageId: m.id, + })); + + messages.map(expired => { + window.log.info('Message expired', { + sentAt: expired.get('sent_at'), + }); + }); + + await destroyMessagesAndUpdateRedux(messagesExpiredDetails); } catch (error) { window.log.error( 'destroyExpiredMessages: Error deleting expired messages', @@ -81,7 +100,7 @@ async function checkExpiringMessages() { } timeout = global.setTimeout(destroyExpiredMessages, wait); } -const throttledCheckExpiringMessages = _.throttle(checkExpiringMessages, 1000); +const throttledCheckExpiringMessages = throttle(checkExpiringMessages, 1000); let isInit = false;