From 062db5caab4c01d7dd4a0100347a02737c5984c0 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 13 Apr 2022 14:55:22 +1000 Subject: [PATCH] move filterDuplicatesFromDbAndIncoming to its own file and test also add pending tests to do for in memory db and updater --- .../settings/section/CategoryAppearance.tsx | 6 +- ts/data/data.ts | 7 +- ts/node/sql.ts | 2 +- .../opengroupV2/OpenGroupServerPoller.ts | 44 +------ .../opengroupV2/SogsFilterDuplicate.ts | 36 ++++++ .../receiver/opengroup/deduplicate_test.ts | 117 ++++++++++++++++++ ts/test/session/unit/updater/updater_test.ts | 4 + ts/test/test-utils/utils/stubbing.ts | 1 + 8 files changed, 171 insertions(+), 46 deletions(-) create mode 100644 ts/session/apis/open_group_api/opengroupV2/SogsFilterDuplicate.ts create mode 100644 ts/test/session/unit/receiver/opengroup/deduplicate_test.ts create mode 100644 ts/test/session/unit/updater/updater_test.ts diff --git a/ts/components/settings/section/CategoryAppearance.tsx b/ts/components/settings/section/CategoryAppearance.tsx index 975f74265..39d47d4b3 100644 --- a/ts/components/settings/section/CategoryAppearance.tsx +++ b/ts/components/settings/section/CategoryAppearance.tsx @@ -3,11 +3,7 @@ import React from 'react'; import { useDispatch, useSelector } from 'react-redux'; // tslint:disable-next-line: no-submodule-imports import useUpdate from 'react-use/lib/useUpdate'; -import { - createOrUpdateItem, - fillWithTestData, - hasLinkPreviewPopupBeenDisplayed, -} from '../../../data/data'; +import { createOrUpdateItem, hasLinkPreviewPopupBeenDisplayed } from '../../../data/data'; import { ToastUtils } from '../../../session/utils'; import { updateConfirmModal } from '../../../state/ducks/modalDialog'; import { toggleAudioAutoplay } from '../../../state/ducks/userConfig'; diff --git a/ts/data/data.ts b/ts/data/data.ts index e0efcf99f..dddf735de 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -3,10 +3,15 @@ import _ from 'lodash'; import { MessageResultProps } from '../components/search/MessageSearchResults'; -import { ConversationCollection, ConversationModel } from '../models/conversation'; +import { + ConversationCollection, + ConversationModel, + ConversationTypeEnum, +} from '../models/conversation'; import { MessageCollection, MessageModel } from '../models/message'; import { MessageAttributes, MessageDirection } from '../models/messageType'; import { HexKeyPair } from '../receiver/keypairs'; +import { getConversationController } from '../session/conversations'; import { getSodiumRenderer } from '../session/crypto'; import { PubKey } from '../session/types'; import { ReduxConversationType } from '../state/ducks/conversations'; diff --git a/ts/node/sql.ts b/ts/node/sql.ts index d190dbd9d..2bd7f2e73 100644 --- a/ts/node/sql.ts +++ b/ts/node/sql.ts @@ -1423,7 +1423,7 @@ async function initializeSql({ messages: LocaleMessagesType; passwordAttempt: boolean; }) { - console.warn('initializeSql sqlnode'); + console.info('initializeSql sqlnode'); if (globalInstance) { throw new Error('Cannot initialize more than once!'); } diff --git a/ts/session/apis/open_group_api/opengroupV2/OpenGroupServerPoller.ts b/ts/session/apis/open_group_api/opengroupV2/OpenGroupServerPoller.ts index 2e3e7123f..696adef7a 100644 --- a/ts/session/apis/open_group_api/opengroupV2/OpenGroupServerPoller.ts +++ b/ts/session/apis/open_group_api/opengroupV2/OpenGroupServerPoller.ts @@ -13,11 +13,7 @@ import { } from './OpenGroupAPIV2CompactPoll'; import _ from 'lodash'; import { ConversationModel } from '../../../../models/conversation'; -import { - filterAlreadyFetchedOpengroupMessage, - getMessageIdsFromServerIds, - removeMessage, -} from '../../../../data/data'; +import { getMessageIdsFromServerIds, removeMessage } from '../../../../data/data'; import { getV2OpenGroupRoom, saveV2OpenGroupRoom } from '../../../../data/opengroups'; import { OpenGroupMessageV2 } from './OpenGroupMessageV2'; import autoBind from 'auto-bind'; @@ -27,6 +23,8 @@ import { processNewAttachment } from '../../../../types/MessageAttachment'; import { MIME } from '../../../../types'; import { handleOpenGroupV2Message } from '../../../../receiver/opengroup'; import { callUtilsWorker } from '../../../../webworker/workers/util_worker_interface'; +import { filterDuplicatesFromDbAndIncoming } from './SogsFilterDuplicate'; + const pollForEverythingInterval = DURATION.SECONDS * 10; const pollForRoomAvatarInterval = DURATION.DAYS * 1; const pollForMemberCountInterval = DURATION.MINUTES * 10; @@ -397,39 +395,6 @@ const handleDeletions = async ( } }; -const filterDuplicatesFromDbAndIncoming = async ( - newMessages: Array -): Promise> => { - const start = Date.now(); - // open group messages are deduplicated by sender and serverTimestamp only. - // first make sure that the incoming messages have no duplicates: - const filtered = _.uniqWith(newMessages, (a, b) => { - return ( - Boolean(a.sender) && - Boolean(a.sentTimestamp) && - a.sender === b.sender && - a.sentTimestamp === b.sentTimestamp - ); - // make sure a sender is set, as we cast it just below - }).filter(m => Boolean(m.sender)); - - // now, check database to make sure those messages are not already fetched - const filteredInDb = await filterAlreadyFetchedOpengroupMessage( - filtered.map(m => { - return { sender: m.sender as string, serverTimestamp: m.sentTimestamp }; - }) - ); - - window.log.debug( - `filterDuplicatesFromDbAndIncoming of ${newMessages.length} messages took ${Date.now() - - start}ms.` - ); - const opengroupMessagesFiltered = filteredInDb?.map(f => { - return newMessages.find(m => m.sender === f.sender && m.sentTimestamp === f.serverTimestamp); - }); - return _.compact(opengroupMessagesFiltered) || []; -}; - const handleNewMessages = async ( newMessages: Array, conversationId: string, @@ -452,9 +417,10 @@ const handleNewMessages = async ( } const incomingMessageIds = _.compact(newMessages.map(n => n.serverId)); const maxNewMessageId = Math.max(...incomingMessageIds); - // TODO filter out duplicates ? const roomDetails: OpenGroupRequestCommonType = _.pick(roomInfos, 'serverUrl', 'roomId'); + + // this call filters duplicates based on the sender & senttimestamp from the incoming messages array and the database const filteredDuplicates = await filterDuplicatesFromDbAndIncoming(newMessages); // tslint:disable-next-line: prefer-for-of diff --git a/ts/session/apis/open_group_api/opengroupV2/SogsFilterDuplicate.ts b/ts/session/apis/open_group_api/opengroupV2/SogsFilterDuplicate.ts new file mode 100644 index 000000000..bd812f0aa --- /dev/null +++ b/ts/session/apis/open_group_api/opengroupV2/SogsFilterDuplicate.ts @@ -0,0 +1,36 @@ +import _ from 'lodash'; +import { filterAlreadyFetchedOpengroupMessage } from '../../../../data/data'; +import { OpenGroupMessageV2 } from './OpenGroupMessageV2'; + +export const filterDuplicatesFromDbAndIncoming = async ( + newMessages: Array +): Promise> => { + const start = Date.now(); + // open group messages are deduplicated by sender and serverTimestamp only. + // first make sure that the incoming messages have no duplicates: + const filtered = _.uniqWith(newMessages, (a, b) => { + return ( + Boolean(a.sender) && + Boolean(a.sentTimestamp) && + a.sender === b.sender && + a.sentTimestamp === b.sentTimestamp + ); + // make sure a sender is set, as we cast it just below + }).filter(m => Boolean(m.sender)); + + // now, check database to make sure those messages are not already fetched + const filteredInDb = await filterAlreadyFetchedOpengroupMessage( + filtered.map(m => { + return { sender: m.sender as string, serverTimestamp: m.sentTimestamp }; + }) + ); + + window.log.debug( + `filterDuplicatesFromDbAndIncoming of ${newMessages.length} messages took ${Date.now() - + start}ms.` + ); + const opengroupMessagesFiltered = filteredInDb?.map(f => { + return newMessages.find(m => m.sender === f.sender && m.sentTimestamp === f.serverTimestamp); + }); + return _.compact(opengroupMessagesFiltered) || []; +}; diff --git a/ts/test/session/unit/receiver/opengroup/deduplicate_test.ts b/ts/test/session/unit/receiver/opengroup/deduplicate_test.ts new file mode 100644 index 000000000..c5023a316 --- /dev/null +++ b/ts/test/session/unit/receiver/opengroup/deduplicate_test.ts @@ -0,0 +1,117 @@ +// tslint:disable: no-implicit-dependencies max-func-body-length no-unused-expression + +import chai from 'chai'; +import { describe } from 'mocha'; +import { filterDuplicatesFromDbAndIncoming } from '../../../../../session/apis/open_group_api/opengroupV2/SogsFilterDuplicate'; +import { TestUtils } from '../../../../test-utils'; + +const { expect } = chai; + +// tslint:disable-next-line: max-func-body-length +describe('filterDuplicatesFromDbAndIncoming', () => { + describe('filters already duplicated message in the same incoming batch', () => { + beforeEach(() => { + TestUtils.stubData('filterAlreadyFetchedOpengroupMessage').returnsArg(0); + TestUtils.stubWindowLog(); + }); + + afterEach(() => { + TestUtils.restoreStubs(); + }); + + it('no duplicates', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + const msg3 = TestUtils.generateOpenGroupMessageV2(); + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(3); + expect(filtered[0]).to.be.deep.eq(msg1); + expect(filtered[1]).to.be.deep.eq(msg2); + expect(filtered[2]).to.be.deep.eq(msg3); + }); + + it('two duplicate sender but not the same timestamp', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + msg2.sender = msg1.sender; + msg2.sentTimestamp = Date.now() + 2; + const msg3 = TestUtils.generateOpenGroupMessageV2(); + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(3); + expect(filtered[0]).to.be.deep.eq(msg1); + expect(filtered[1]).to.be.deep.eq(msg2); + expect(filtered[2]).to.be.deep.eq(msg3); + }); + + it('two duplicate timestamp but not the same sender', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + msg2.sentTimestamp = msg1.sentTimestamp; + const msg3 = TestUtils.generateOpenGroupMessageV2(); + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(3); + expect(filtered[0]).to.be.deep.eq(msg1); + expect(filtered[1]).to.be.deep.eq(msg2); + expect(filtered[2]).to.be.deep.eq(msg3); + }); + + it('two duplicate timestamp but not the same sender', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + msg2.sentTimestamp = msg1.sentTimestamp; + const msg3 = TestUtils.generateOpenGroupMessageV2(); + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(3); + expect(filtered[0]).to.be.deep.eq(msg1); + expect(filtered[1]).to.be.deep.eq(msg2); + expect(filtered[2]).to.be.deep.eq(msg3); + }); + + it('two duplicates in the same poll ', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + msg2.sentTimestamp = msg1.sentTimestamp; + msg2.sender = msg1.sender; + const msg3 = TestUtils.generateOpenGroupMessageV2(); + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(2); + expect(filtered[0]).to.be.deep.eq(msg1); + expect(filtered[1]).to.be.deep.eq(msg3); + }); + + it('three duplicates in the same poll', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + + const msg3 = TestUtils.generateOpenGroupMessageV2(); + msg2.sentTimestamp = msg1.sentTimestamp; + msg2.sender = msg1.sender; + msg3.sentTimestamp = msg1.sentTimestamp; + msg3.sender = msg1.sender; + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(1); + expect(filtered[0]).to.be.deep.eq(msg1); + }); + + it('three duplicates in the same poll', async () => { + const msg1 = TestUtils.generateOpenGroupMessageV2(); + const msg2 = TestUtils.generateOpenGroupMessageV2(); + + const msg3 = TestUtils.generateOpenGroupMessageV2(); + msg2.sentTimestamp = msg1.sentTimestamp; + msg2.sender = msg1.sender; + msg3.sentTimestamp = msg1.sentTimestamp; + msg3.sender = msg1.sender; + const filtered = await filterDuplicatesFromDbAndIncoming([msg1, msg2, msg3]); + expect(filtered.length).to.be.eq(1); + expect(filtered[0]).to.be.deep.eq(msg1); + }); + }); + + describe('filters duplicated message from database', () => { + //sadly better-sqlite3 does not allow us to easily create an in memory db for now (issues with sqlite binary) + // so testing this part is not easy as all the logic is made in sqlite + // tslint:disable-next-line: no-empty + it.skip('in memory database', () => {}); + }); +}); diff --git a/ts/test/session/unit/updater/updater_test.ts b/ts/test/session/unit/updater/updater_test.ts new file mode 100644 index 000000000..bce28eb9e --- /dev/null +++ b/ts/test/session/unit/updater/updater_test.ts @@ -0,0 +1,4 @@ +describe('Updater', () => { + // tslint:disable-next-line: no-empty + it.skip('updater test', () => {}); +}); diff --git a/ts/test/test-utils/utils/stubbing.ts b/ts/test/test-utils/utils/stubbing.ts index 99e05c6eb..c467f122c 100644 --- a/ts/test/test-utils/utils/stubbing.ts +++ b/ts/test/test-utils/utils/stubbing.ts @@ -67,5 +67,6 @@ export const stubWindowLog = () => { info: (args: any) => console.info(args), warn: (args: any) => console.warn(args), error: (args: any) => console.error(args), + debug: (args: any) => console.debug(args), }); };