diff --git a/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts b/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts index 7b32360d8..80bbb8de2 100644 --- a/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts +++ b/ts/session/apis/open_group_api/sogsv3/sogsRollingDeletions.ts @@ -2,12 +2,12 @@ import { RingBuffer } from '../../../utils/RingBuffer'; const rollingDeletedMessageIds: Map> = new Map(); -// keep 2000 deleted message ids in memory -const perRoomRollingRemovedIds = 2000; - const addMessageDeletedId = (conversationId: string, messageDeletedId: number) => { if (!rollingDeletedMessageIds.has(conversationId)) { - rollingDeletedMessageIds.set(conversationId, new RingBuffer(perRoomRollingRemovedIds)); + rollingDeletedMessageIds.set( + conversationId, + new RingBuffer(sogsRollingDeletions.getPerRoomCount()) + ); } const ringBuffer = rollingDeletedMessageIds.get(conversationId); if (!ringBuffer) { @@ -16,7 +16,6 @@ const addMessageDeletedId = (conversationId: string, messageDeletedId: number) = ringBuffer.add(messageDeletedId); }; - const hasMessageDeletedId = (conversationId: string, messageDeletedId: number) => { if (!rollingDeletedMessageIds.has(conversationId)) { return false; @@ -29,4 +28,21 @@ const hasMessageDeletedId = (conversationId: string, messageDeletedId: number) = return messageIdWasDeletedRecently; }; -export const sogsRollingDeletions = { addMessageDeletedId, hasMessageDeletedId }; +/** + * 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/apis/open_group_api/sogsv3/sogsV3BatchPoll.ts b/ts/session/apis/open_group_api/sogsv3/sogsV3BatchPoll.ts index cef51e209..7461b16f4 100644 --- a/ts/session/apis/open_group_api/sogsv3/sogsV3BatchPoll.ts +++ b/ts/session/apis/open_group_api/sogsv3/sogsV3BatchPoll.ts @@ -241,8 +241,7 @@ const makeBatchRequestPayload = ( method: 'GET', path: isNumber(options.messages.sinceSeqNo) ? `/room/${options.messages.roomId}/messages/since/${options.messages.sinceSeqNo}?t=r&reactors=${Reactions.SOGSReactorsFetchCount}` - : // : `/room/${options.messages.roomId}/messages/since/180000?t=r&reactors=${Reactions.SOGSReactorsFetchCount}`, - `/room/${options.messages.roomId}/messages/recent?reactors=${Reactions.SOGSReactorsFetchCount}`, + : `/room/${options.messages.roomId}/messages/recent?reactors=${Reactions.SOGSReactorsFetchCount}`, }; } break; diff --git a/ts/session/utils/RingBuffer.ts b/ts/session/utils/RingBuffer.ts index 7a192e858..6d9173d6f 100644 --- a/ts/session/utils/RingBuffer.ts +++ b/ts/session/utils/RingBuffer.ts @@ -1,3 +1,8 @@ +/** + * 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 buffer: Array = []; private readonly capacity: number; @@ -10,6 +15,10 @@ export class RingBuffer { return this.capacity; } + public getLength(): number { + return this.buffer.length; + } + public add(item: T) { this.buffer.push(item); this.crop(); 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..eee05f34c --- /dev/null +++ b/ts/test/session/unit/utils/RingBuffer_test.ts @@ -0,0 +1,106 @@ +// 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, '4 should not be there'); + }); + + describe('length & capacity are right', () => { + it('length is right 1', () => { + const ring = new RingBuffer(4); + ring.add(0); + expect(ring.getLength()).to.equal(1); + }); + + it('length is right 4', () => { + const ring = new RingBuffer(4); + ring.add(0); + ring.add(1); + ring.add(2); + ring.add(3); + expect(ring.getLength()).to.equal(4); + }); + + it('capacity does not get exceeded', () => { + const ring = new RingBuffer(4); + ring.add(0); + ring.add(1); + ring.add(2); + ring.add(3); + ring.add(4); + expect(ring.getLength()).to.equal(4); + }); + }); + + it('items are removed in order 1', () => { + const ring = new RingBuffer(4); + ring.add(0); + ring.add(1); + ring.add(2); + ring.add(3); + ring.add(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.add(0); + ring.add(1); + ring.add(2); + ring.add(1); + ring.add(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.add(0); + ring.add(1); + ring.add(2); + ring.add(3); + ring.add(10); + ring.add(20); + ring.add(30); + ring.add(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.add(0); + ring.add(1); + ring.add(2); + ring.add(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); + }); +});