diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f857ceadf..90ac6b6a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,9 @@ jobs: - name: Lint run: yarn lint + - name: Unit tests + run: yarn run test-unit + test-types: runs-on: ubuntu-latest strategy: diff --git a/package.json b/package.json index c0ec989a2..bb0012b79 100644 --- a/package.json +++ b/package.json @@ -121,6 +121,7 @@ "test-types": "node test/typescript/index.js && tsc --esModuleInterop true --noEmit true --strictNullChecks true --noImplicitAny true --strict true test/typescript/*.ts", "eslint": "eslint '**/*.{js,md,ts}' --max-warnings 0 --ignore-path ./.eslintignore", "eslint-fix": "npx eslint --fix '**/*.{js,md,ts}' --max-warnings 0 --ignore-path ./.eslintignore", + "test-unit": "NODE_ENV=test mocha --exit --bail --timeout 3000 --require ./babel-register test/unit/*.js", "test": "NODE_ENV=test mocha --exit --bail --timeout 15000 --require ./babel-register test/integration/*.js --async-stack-traces", "test-local": "STREAM_LOCAL_TEST_RUN=true yarn test", "testall": "NODE_ENV=test mocha --exit --timeout 3000 --require ./babel-register test/integration/*.js --async-stack-traces", diff --git a/src/channel.ts b/src/channel.ts index 7645f528f..7566d80bd 100644 --- a/src/channel.ts +++ b/src/channel.ts @@ -1658,7 +1658,7 @@ export class Channel< if (!this.state.messages) { this.state.messages = Immutable([]); } - this.state.addMessagesSorted(messages, true); + this.state.addMessagesSorted(messages, false, true); this.state.watcher_count = state.watcher_count ? state.watcher_count : 0; // convert the arrays into objects for easier syncing... if (state.watchers) { diff --git a/src/channel_state.ts b/src/channel_state.ts index 40e0989a2..2f6c883d7 100644 --- a/src/channel_state.ts +++ b/src/channel_state.ts @@ -11,17 +11,6 @@ import { UserResponse, } from './types'; -const byDate = ( - a: { created_at: Date | Immutable.ImmutableDate }, - b: { created_at: Date | Immutable.ImmutableDate }, -) => { - if (!a.created_at) return -1; - - if (!b.created_at) return 1; - - return a.created_at.getTime() - b.created_at.getTime(); -}; - /** * ChannelState - A container class for the channel state. */ @@ -167,6 +156,7 @@ export class ChannelState< * addMessageSorted - Add a message to the state * * @param {MessageResponse} newMessage A new message + * @param {boolean} timestampChanged Whether updating a message with changed created_at value. * */ addMessageSorted( @@ -178,8 +168,9 @@ export class ChannelState< ReactionType, UserType >, + timestampChanged = false, ) { - return this.addMessagesSorted([newMessage]); + return this.addMessagesSorted([newMessage], timestampChanged); } /** @@ -213,7 +204,8 @@ export class ChannelState< * addMessagesSorted - Add the list of messages to state and resorts the messages * * @param {Array>} newMessages A list of messages - * @param {boolean} initializing Weather channel is being initialized. + * @param {boolean} timestampChanged Whether updating messages with changed created_at value. + * @param {boolean} initializing Whether channel is being initialized. * */ addMessagesSorted( @@ -225,21 +217,12 @@ export class ChannelState< ReactionType, UserType >[], + timestampChanged = false, initializing = false, ) { - // parse all the new message dates and add __html for react - const parsedMessages: ReturnType< - ChannelState< - AttachmentType, - ChannelType, - CommandType, - EventType, - MessageType, - ReactionType, - UserType - >['messageToImmutable'] - >[] = []; - for (const message of newMessages) { + for (let i = 0; i < newMessages.length; i += 1) { + const message = this.messageToImmutable(newMessages[i]); + if (initializing && message.id && this.threads[message.id]) { // If we are initializing the state of channel (e.g., in case of connection recovery), // then in that case we remove thread related to this message from threads object. @@ -247,51 +230,30 @@ export class ChannelState< // and consumer can refetch the replies. this.threads = this.threads.without(message.id); } - const parsedMsg = this.messageToImmutable(message); - - parsedMessages.push(parsedMsg); if (!this.last_message_at) { - this.last_message_at = new Date(parsedMsg.created_at.getTime()); + this.last_message_at = new Date(message.created_at.getTime()); } - if ( - this.last_message_at && - parsedMsg.created_at.getTime() > this.last_message_at.getTime() - ) { - this.last_message_at = new Date(parsedMsg.created_at.getTime()); + if (message.created_at.getTime() > this.last_message_at.getTime()) { + this.last_message_at = new Date(message.created_at.getTime()); } - } - // update or append the messages... - const updatedThreads: string[] = []; - for (const message of parsedMessages) { - const isThreadReply = !!(message.parent_id && !message.show_in_channel); + // update or append the messages... + const parentID = message.parent_id; + // add to the main message list - if (!isThreadReply) { - this.messages = this._addToMessageList(this.messages, message); + if (!parentID || message.show_in_channel) { + this.messages = this._addToMessageList(this.messages, message, timestampChanged); } + // add to the thread if applicable.. - const parentID: string | undefined = message.parent_id; if (parentID) { const thread = this.threads[parentID] || Immutable([]); - const threadMessages = this._addToMessageList(thread, message); + const threadMessages = this._addToMessageList(thread, message, timestampChanged); this.threads = this.threads.set(parentID, threadMessages); - updatedThreads.push(parentID); } } - - // Resort the main messages and the threads that changed... - const messages = Immutable.asMutable(this.messages); - messages.sort(byDate); - this.messages = Immutable(messages); - for (const parentID of updatedThreads) { - const threadMessages = this.threads[parentID] - ? Immutable.asMutable(this.threads[parentID]) - : []; - threadMessages.sort(byDate); - this.threads = this.threads.set(parentID, threadMessages); - } } addReaction( @@ -467,6 +429,7 @@ export class ChannelState< * * @param {Immutable.ImmutableArray['messageToImmutable']>>} messages A list of messages * @param {ReturnType['messageToImmutable']>} newMessage The new message + * @param {boolean} timestampChanged Whether updating a message with changed created_at value. * */ _addToMessageList( @@ -483,7 +446,7 @@ export class ChannelState< >['messageToImmutable'] > >, - newMessage: ReturnType< + message: ReturnType< ChannelState< AttachmentType, ChannelType, @@ -494,36 +457,61 @@ export class ChannelState< UserType >['messageToImmutable'] >, + timestampChanged = false, ) { - let updated = false; - let newMessages: Immutable.ImmutableArray['messageToImmutable'] - >> = Immutable([]); + let messageArr = messages; - for (let i = 0; i < messages.length; i++) { - const message = messages[i]; - const idMatch = !!message.id && !!newMessage.id && message.id === newMessage.id; + // if created_at has changed, message should be filtered and re-inserted in correct order + // slow op but usually this only happens for a message inserted to state before actual response with correct timestamp + if (timestampChanged) { + messageArr = messageArr.filter((msg) => !(msg.id && message.id === msg.id)); + } - if (idMatch) { - // @ts-expect-error - ImmutableArray.set exists in the documentation but not in the DefinitelyTyped types - newMessages = messages.set(i, newMessage); - updated = true; - } + // for empty list just concat and return + if (messageArr.length === 0) return messageArr.concat(message); + + const messageTime = message.created_at.getTime(); + + // if message is newer than last item in the list concat and return + if (messageArr[messageArr.length - 1].created_at.getTime() < messageTime) + return messageArr.concat(message); + + // find the closest index to push the new message + let left = 0; + let middle = 0; + let right = messageArr.length - 1; + while (left <= right) { + middle = Math.floor((right + left) / 2); + if (messageArr[middle].created_at.getTime() <= messageTime) left = middle + 1; + else right = middle - 1; } - if (!updated) { - newMessages = messages.concat([newMessage]); + // message already exists and not filtered due to timestampChanged, update and return + if (!timestampChanged && message.id) { + if (messageArr[left] && message.id === messageArr[left].id) + // @ts-expect-error - ImmutableArray.set exists in the documentation but not in the DefinitelyTyped types + return messageArr.set(left, message); + + if (messageArr[left - 1] && message.id === messageArr[left - 1].id) + // @ts-expect-error - ImmutableArray.set exists in the documentation but not in the DefinitelyTyped types + return messageArr.set(left - 1, message); } - return newMessages; + const mutable = messageArr.asMutable() as Array< + ReturnType< + ChannelState< + AttachmentType, + ChannelType, + CommandType, + EventType, + MessageType, + ReactionType, + UserType + >['messageToImmutable'] + > + >; + mutable.splice(left, 0, message); + return Immutable(mutable); } /** diff --git a/test/unit/channel_state.js b/test/unit/channel_state.js new file mode 100644 index 000000000..92f2ac8ae --- /dev/null +++ b/test/unit/channel_state.js @@ -0,0 +1,279 @@ +import Immutable from 'seamless-immutable'; +import chai from 'chai'; +import { v4 as uuidv4 } from 'uuid'; +import { ChannelState } from '../../src/channel_state'; + +const expect = chai.expect; + +const generateMsg = (msg = {}) => { + const date = msg.date || new Date().toISOString(); + return { + id: uuidv4(), + text: 'x', + html: '

x

\n', + type: 'regular', + user: { id: 'id' }, + attachments: [], + latest_reactions: [], + own_reactions: [], + reaction_counts: null, + reaction_scores: {}, + reply_count: 0, + created_at: date, + updated_at: date, + mentioned_users: [], + silent: false, + status: 'received', + __html: '

x

\n', + ...msg, + }; +}; + +describe('ChannelState addMessagesSorted', function () { + it('empty state add single messages', async function () { + const state = new ChannelState(); + expect(state.messages).to.have.length(0); + state.addMessagesSorted([ + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + expect(state.messages).to.have.length(1); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:01.000Z' }), + ]); + + expect(state.messages).to.have.length(2); + expect(state.messages[0].id).to.be.equal('0'); + expect(state.messages[1].id).to.be.equal('1'); + }); + + it('empty state add multiple messages', async function () { + const state = new ChannelState(); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:00.001Z' }), + generateMsg({ id: '2', date: '2020-01-01T00:00:00.002Z' }), + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + + expect(state.messages).to.have.length(3); + expect(state.messages[0].id).to.be.equal('0'); + expect(state.messages[1].id).to.be.equal('1'); + expect(state.messages[2].id).to.be.equal('2'); + }); + + it('update a message in place 1', async function () { + const state = new ChannelState(); + state.addMessagesSorted([generateMsg({ id: '0' })]); + state.addMessagesSorted([{ ...state.messages[0].asMutable(), text: 'update' }]); + + expect(state.messages).to.have.length(1); + expect(state.messages[0].text).to.be.equal('update'); + }); + + it('update a message in place 2', async function () { + const state = new ChannelState(); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:00.001Z' }), + generateMsg({ id: '2', date: '2020-01-01T00:00:00.002Z' }), + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + + state.addMessagesSorted([{ ...state.messages[1].asMutable(), text: 'update' }]); + + expect(state.messages).to.have.length(3); + expect(state.messages[1].text).to.be.equal('update'); + expect(state.messages[0].id).to.be.equal('0'); + expect(state.messages[1].id).to.be.equal('1'); + expect(state.messages[2].id).to.be.equal('2'); + }); + + it('update a message in place 3', async function () { + const state = new ChannelState(); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:00.001Z' }), + generateMsg({ id: '2', date: '2020-01-01T00:00:00.002Z' }), + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + generateMsg({ id: '3', date: '2020-01-01T00:00:00.003Z' }), + ]); + + state.addMessagesSorted([{ ...state.messages[0].asMutable(), text: 'update 0' }]); + expect(state.messages).to.have.length(4); + expect(state.messages[0].text).to.be.equal('update 0'); + + state.addMessagesSorted([{ ...state.messages[2].asMutable(), text: 'update 2' }]); + expect(state.messages).to.have.length(4); + expect(state.messages[2].text).to.be.equal('update 2'); + + state.addMessagesSorted([{ ...state.messages[3].asMutable(), text: 'update 3' }]); + expect(state.messages).to.have.length(4); + expect(state.messages[3].text).to.be.equal('update 3'); + }); + + it('add a message with same created_at', async function () { + const state = new ChannelState(); + + for (let i = 0; i < 10; i++) { + state.addMessagesSorted([ + generateMsg({ id: `${i}`, date: `2020-01-01T00:00:00.00${i}Z` }), + ]); + } + + for (let i = 10; i < state.messages.length - 1; i++) { + for (let j = i + 1; i < state.messages.length - 1; j++) + expect(state.messages[i].created_at.getTime()).to.be.lessThan( + state.messages[j].created_at.getTime(), + ); + } + + expect(state.messages).to.have.length(10); + state.addMessagesSorted([ + generateMsg({ id: 'id', date: `2020-01-01T00:00:00.007Z` }), + ]); + expect(state.messages).to.have.length(11); + expect(state.messages[7].id).to.be.equal('7'); + expect(state.messages[8].id).to.be.equal('id'); + }); + + it('add lots of messages in order', async function () { + const state = new ChannelState(); + + for (let i = 100; i < 300; i++) { + state.addMessagesSorted([ + generateMsg({ id: `${i}`, date: `2020-01-01T00:00:00.${i}Z` }), + ]); + } + + expect(state.messages).to.have.length(200); + for (let i = 100; i < state.messages.length - 1; i++) { + for (let j = i + 1; j < state.messages.length - 1; j++) + expect(state.messages[i].created_at.getTime()).to.be.lessThan( + state.messages[j].created_at.getTime(), + ); + } + }); + + it('add lots of messages out of order', async function () { + const state = new ChannelState(); + + const messages = []; + for (let i = 100; i < 300; i++) { + messages.push(generateMsg({ id: `${i}`, date: `2020-01-01T00:00:00.${i}Z` })); + } + // shuffle + for (let i = messages.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + [messages[i], messages[j]] = [messages[j], messages[i]]; + } + + state.addMessagesSorted(messages); + + expect(state.messages).to.have.length(200); + for (let i = 0; i < 200; i++) { + expect(state.messages[i].id).to.be.equal(`${i + 100}`); + } + }); + + it('should avoid duplicates if message.created_at changes', async function () { + const state = new ChannelState(); + state.addMessagesSorted([ + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + expect(state.messages).to.have.length(1); + + state.addMessageSorted( + { + ...state.messages[0].asMutable(), + created_at: '2020-01-01T00:00:00.044Z', + text: 'update 0', + }, + true, + ); + expect(state.messages).to.have.length(1); + expect(state.messages[0].text).to.be.equal('update 0'); + expect(state.messages[0].created_at.getTime()).to.be.equal( + new Date('2020-01-01T00:00:00.044Z').getTime(), + ); + }); + + it('should respect order and avoid duplicates if message.created_at changes', async function () { + const state = new ChannelState(); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:00.001Z' }), + generateMsg({ id: '2', date: '2020-01-01T00:00:00.002Z' }), + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + generateMsg({ id: '3', date: '2020-01-01T00:00:00.003Z' }), + ]); + expect(state.messages).to.have.length(4); + + state.addMessagesSorted( + [ + { + ...state.messages[3].asMutable(), + created_at: '2020-01-01T00:00:00.033Z', + text: 'update 3', + }, + ], + true, + ); + expect(state.messages).to.have.length(4); + expect(state.messages[3].text).to.be.equal('update 3'); + + state.addMessageSorted( + { + ...state.messages[0].asMutable(), + created_at: '2020-01-01T00:00:00.044Z', + text: 'update 0', + }, + true, + ); + expect(state.messages).to.have.length(4); + expect(state.messages[3].text).to.be.equal('update 0'); + expect(state.messages[0].id).to.be.equal('1'); + expect(state.messages[1].id).to.be.equal('2'); + expect(state.messages[2].id).to.be.equal('3'); + expect(state.messages[3].id).to.be.equal('0'); + }); + + it('should return Immutable', async function () { + const state = new ChannelState(); + expect(Immutable.isImmutable(state.messages)).to.be.true; + state.addMessagesSorted([ + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + expect(Immutable.isImmutable(state.messages)).to.be.true; + expect(Immutable.isImmutable(state.messages[0])).to.be.true; + + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2020-01-01T00:00:00.001Z', parent_id: '0' }), + ]); + expect(Immutable.isImmutable(state.messages)).to.be.true; + expect(state.messages).to.have.length(1); + expect(state.threads['0']).to.have.length(1); + expect(Immutable.isImmutable(state.threads)).to.be.true; + expect(Immutable.isImmutable(state.threads['0'])).to.be.true; + expect(Immutable.isImmutable(state.threads['0'][0])).to.be.true; + }); + + it('updates last_message_at correctly', async function () { + const state = new ChannelState(); + expect(state.last_message_at).to.be.null; + state.addMessagesSorted([ + generateMsg({ id: '0', date: '2020-01-01T00:00:00.000Z' }), + ]); + expect(state.last_message_at.getTime()).to.be.equal( + new Date('2020-01-01T00:00:00.000Z').getTime(), + ); + state.addMessagesSorted([ + generateMsg({ id: '1', date: '2019-01-01T00:00:00.000Z' }), + ]); + expect(state.last_message_at.getTime()).to.be.equal( + new Date('2020-01-01T00:00:00.000Z').getTime(), + ); + + state.addMessagesSorted([ + generateMsg({ id: '2', date: '2020-01-01T00:00:00.001Z' }), + ]); + expect(state.last_message_at.getTime()).to.be.equal( + new Date('2020-01-01T00:00:00.001Z').getTime(), + ); + }); +});