Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(MessagesList): rename method to pollNewMessages #14393

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 10 additions & 24 deletions src/components/MessagesList/MessagesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -707,29 +707,14 @@ export default {

this.isInitialisingMessages = false

// get new messages
await this.lookForNewMessages(token)
// Once the history is received, starts looking for new messages.
await this.pollNewMessages(token)

} else {
this.$store.dispatch('cancelLookForNewMessages', { requestId: this.chatIdentifier })
}
},

/**
* Fetches the messages of a conversation given the conversation token. Triggers
* a long-polling request for new messages.
* @param token token of conversation where a method was called
*/
async lookForNewMessages(token) {
// Once the history is received, starts looking for new messages.
if (this._isBeingDestroyed || this._isDestroyed) {
console.debug('Prevent getting new messages on a destroyed MessagesList')
return
}

await this.getNewMessages(token)
},

async getMessageContext(token, messageId) {
// Make the request
this.loadingOldMessages = true
Expand Down Expand Up @@ -797,12 +782,13 @@ export default {
},

/**
* Creates a long polling request for a new message.
*
* Fetches the messages of a conversation given the conversation token.
* Creates a long polling request for new messages.
* @param token token of conversation where a method was called
*/
async getNewMessages(token) {
async pollNewMessages(token) {
if (this.destroying) {
console.debug('Prevent polling new messages on MessagesList being destroyed')
return
}
// Check that the token has not changed
Expand Down Expand Up @@ -834,7 +820,7 @@ export default {
// This is not an error, so reset error timeout and poll again
this.pollingErrorTimeout = 1
setTimeout(() => {
this.getNewMessages(token)
this.pollNewMessages(token)
}, 500)
return
}
Expand All @@ -848,13 +834,13 @@ export default {
console.debug('Error happened while getting chat messages. Trying again in ', this.pollingErrorTimeout, exception)

setTimeout(() => {
this.getNewMessages(token)
this.pollNewMessages(token)
}, this.pollingErrorTimeout * 1000)
return
}

setTimeout(() => {
this.getNewMessages(token)
this.pollNewMessages(token)
}, 500)
},

Expand Down Expand Up @@ -1236,7 +1222,7 @@ export default {

handleNetworkOnline() {
console.debug('Restarting polling of new chat messages')
this.getNewMessages(this.token)
this.pollNewMessages(this.token)
},

async onRouteChange({ from, to }) {
Expand Down
2 changes: 2 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export const SESSION = {
export const CHAT = {
FETCH_LIMIT: 100,
MINIMUM_VISIBLE: 5,
FETCH_OLD: 0,
FETCH_NEW: 1,
} as const

export const CALL = {
Expand Down
20 changes: 16 additions & 4 deletions src/services/messagesService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import SHA256 from 'crypto-js/sha256.js'
import axios from '@nextcloud/axios'
import { generateOcsUrl } from '@nextcloud/router'

import { CHAT } from '../constants.ts'
import type {
ChatMessage,
clearHistoryResponse,
Expand Down Expand Up @@ -45,15 +46,22 @@ type EditMessagePayload = { token: string, messageId: number, updatedMessage: ed
* @param data.token the conversation token;
* @param data.lastKnownMessageId last known message id;
* @param data.includeLastKnown whether to include the last known message in the response;
* @param [data.lookIntoFuture=0] direction of message fetch
* @param [data.limit=100] Number of messages to load
* @param options options;
*/
const fetchMessages = async function({ token, lastKnownMessageId, includeLastKnown, limit = 100 }: ReceiveMessagesPayload, options?: object): receiveMessagesResponse {
const fetchMessages = async function({
token,
lastKnownMessageId,
includeLastKnown,
lookIntoFuture = CHAT.FETCH_OLD,
limit = 100,
}: ReceiveMessagesPayload, options?: object): receiveMessagesResponse {
return axios.get(generateOcsUrl('apps/spreed/api/v1/chat/{token}', { token }, options), {
...options,
params: {
setReadMarker: 0,
lookIntoFuture: 0,
lookIntoFuture,
lastKnownMessageId,
limit,
includeLastKnown: includeLastKnown ? 1 : 0,
Expand All @@ -71,12 +79,16 @@ const fetchMessages = async function({ token, lastKnownMessageId, includeLastKno
* @param [data.limit=100] Number of messages to load
* @param options options
*/
const lookForNewMessages = async ({ token, lastKnownMessageId, limit = 100 }: ReceiveMessagesPayload, options?: object): receiveMessagesResponse => {
const lookForNewMessages = async ({
token,
lastKnownMessageId,
limit = 100,
}: ReceiveMessagesPayload, options?: object): receiveMessagesResponse => {
return axios.get(generateOcsUrl('apps/spreed/api/v1/chat/{token}', { token }, options), {
...options,
params: {
setReadMarker: 0,
lookIntoFuture: 1,
lookIntoFuture: CHAT.FETCH_NEW,
lastKnownMessageId,
limit,
includeLastKnown: 0,
Expand Down
28 changes: 22 additions & 6 deletions src/store/messagesStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -855,8 +855,16 @@ const actions = {
* @param {string} data.lastKnownMessageId last known message id;
* @param {number} data.minimumVisible Minimum number of chat messages we want to load
* @param {boolean} data.includeLastKnown whether to include the last known message in the response;
* @param {number} [data.lookIntoFuture=0] direction of message fetch
*/
async fetchMessages(context, { token, lastKnownMessageId, includeLastKnown, requestOptions, minimumVisible }) {
async fetchMessages(context, {
token,
lastKnownMessageId,
includeLastKnown,
requestOptions,
minimumVisible,
lookIntoFuture = CHAT.FETCH_OLD,
}) {
minimumVisible = (typeof minimumVisible === 'undefined') ? CHAT.MINIMUM_VISIBLE : minimumVisible

context.dispatch('cancelFetchMessages')
Expand All @@ -870,6 +878,7 @@ const actions = {
token,
lastKnownMessageId,
includeLastKnown,
lookIntoFuture,
limit: CHAT.FETCH_LIMIT,
}, requestOptions)

Expand Down Expand Up @@ -903,10 +912,12 @@ const actions = {
})

if (response.headers['x-chat-last-given']) {
context.dispatch('setFirstKnownMessageId', {
token,
id: parseInt(response.headers['x-chat-last-given'], 10),
})
const id = parseInt(response.headers['x-chat-last-given'], 10)
if (lookIntoFuture === CHAT.FETCH_NEW) {
context.dispatch('setLastKnownMessageId', { token, id })
} else {
context.dispatch('setFirstKnownMessageId', { token, id })
}
}

// For guests we also need to set the last known message id
Expand All @@ -924,12 +935,16 @@ const actions = {

if (minimumVisible > 0) {
debugTimer.tick(`${token} | fetch history`, 'first chunk')
const lastKnownMessageId = lookIntoFuture === CHAT.FETCH_NEW
? context.getters.getLastKnownMessageId(token)
: context.getters.getFirstKnownMessageId(token)
// There are not yet enough visible messages loaded, so fetch another chunk.
// This can happen when a lot of reactions or poll votings happen
return await context.dispatch('fetchMessages', {
token,
lastKnownMessageId: context.getters.getFirstKnownMessageId(token),
lastKnownMessageId,
includeLastKnown,
lookIntoFuture,
minimumVisible,
})
}
Expand Down Expand Up @@ -1021,6 +1036,7 @@ const actions = {
token,
lastKnownMessageId: context.getters.getFirstKnownMessageId(token),
includeLastKnown: false,
lookIntoFuture: CHAT.FETCH_OLD,
minimumVisible: minimumVisible * 2,
})
}
Expand Down
121 changes: 56 additions & 65 deletions src/store/messagesStore.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,30 @@ describe('messagesStore', () => {
let addGuestNameAction
let cancelFunctionMock

const oldMessagesList = [{
id: 98,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.USERS,
}, {
id: 99,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.GUESTS,
}]
const originalMessagesList = [{
id: 100,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.USERS,
}]
const newMessagesList = [{
id: 101,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.USERS,
}, {
id: 102,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.GUESTS,
}]

beforeEach(() => {
testStoreConfig = cloneDeep(messagesStore)
const guestNameStore = useGuestNameStore()
Expand All @@ -851,79 +875,44 @@ describe('messagesStore', () => {
})

store = new Vuex.Store(testStoreConfig)
})

test('fetches messages from server including last known', async () => {
const messages = [{
id: 1,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.USERS,
}, {
id: 2,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.GUESTS,
}]
const response = generateOCSResponse({
headers: {
'x-chat-last-common-read': '123',
'x-chat-last-given': '100',
},
payload: messages,
})
fetchMessages.mockResolvedValueOnce(response)

await store.dispatch('fetchMessages', {
token: TOKEN,
lastKnownMessageId: 100,
includeLastKnown: true,
requestOptions: {
dummyOption: true,
},
minimumVisible: 0,
})

expect(fetchMessages).toHaveBeenCalledWith({
token: TOKEN,
lastKnownMessageId: 100,
includeLastKnown: true,
limit: CHAT.FETCH_LIMIT,
}, {
dummyOption: true,
})

expect(updateLastCommonReadMessageAction)
.toHaveBeenCalledWith(expect.anything(), { token: TOKEN, lastCommonReadMessage: 123 })

expect(addGuestNameAction).toHaveBeenCalledWith(messages[1], { noUpdate: true })

expect(store.getters.messagesList(TOKEN)).toStrictEqual(messages)
expect(store.getters.getFirstKnownMessageId(TOKEN)).toBe(100)
expect(store.getters.getLastKnownMessageId(TOKEN)).toBe(2)
for (const index in originalMessagesList) {
store.commit('addMessage', { token: TOKEN, message: originalMessagesList[index] })
if (index === '0') {
store.dispatch('setFirstKnownMessageId', { token: TOKEN, id: originalMessagesList[index].id })
}
if (index === `${originalMessagesList.length - 1}`) {
store.dispatch('setLastKnownMessageId', { token: TOKEN, id: originalMessagesList[index].id })
}
}
})

test('fetches messages from server excluding last known', async () => {
const messages = [{
id: 1,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.USERS,
}, {
id: 2,
token: TOKEN,
actorType: ATTENDEE.ACTOR_TYPE.GUESTS,
}]
const testCasesOld = [
[true, CHAT.FETCH_OLD, [...oldMessagesList, originalMessagesList.at(0)].reverse(), 98, 100],
[false, CHAT.FETCH_OLD, [...oldMessagesList].reverse(), 98, 100],
[true, CHAT.FETCH_NEW, [originalMessagesList.at(-1), ...newMessagesList], 100, 102],
[false, CHAT.FETCH_NEW, newMessagesList, 100, 102],
]
test.each(testCasesOld)('fetches messages from server: including last known - %s, look into future - %s', async (includeLastKnown, lookIntoFuture, payload, firstKnown, lastKnown) => {

const response = generateOCSResponse({
headers: {
'x-chat-last-common-read': '123',
'x-chat-last-given': '100',
'x-chat-last-given': payload.at(-1).id.toString(),
},
payload: messages,
payload,
})
fetchMessages.mockResolvedValueOnce(response)
const expectedMessages = lookIntoFuture
? [originalMessagesList[0], ...newMessagesList]
: [...oldMessagesList, originalMessagesList[0]]
const expectedMessageFromGuest = expectedMessages.find(message => message.actorType === ATTENDEE.ACTOR_TYPE.GUESTS)

await store.dispatch('fetchMessages', {
token: TOKEN,
lastKnownMessageId: 100,
includeLastKnown: false,
includeLastKnown,
lookIntoFuture,
requestOptions: {
dummyOption: true,
},
Expand All @@ -933,7 +922,8 @@ describe('messagesStore', () => {
expect(fetchMessages).toHaveBeenCalledWith({
token: TOKEN,
lastKnownMessageId: 100,
includeLastKnown: false,
includeLastKnown,
lookIntoFuture,
limit: CHAT.FETCH_LIMIT,
}, {
dummyOption: true,
Expand All @@ -942,11 +932,11 @@ describe('messagesStore', () => {
expect(updateLastCommonReadMessageAction)
.toHaveBeenCalledWith(expect.anything(), { token: TOKEN, lastCommonReadMessage: 123 })

expect(addGuestNameAction).toHaveBeenCalledWith(messages[1], { noUpdate: true })
expect(addGuestNameAction).toHaveBeenCalledWith(expectedMessageFromGuest, { noUpdate: true })

expect(store.getters.messagesList(TOKEN)).toStrictEqual(messages)
expect(store.getters.getFirstKnownMessageId(TOKEN)).toBe(100)
expect(store.getters.getLastKnownMessageId(TOKEN)).toBe(null)
expect(store.getters.messagesList(TOKEN)).toStrictEqual(expectedMessages)
expect(store.getters.getFirstKnownMessageId(TOKEN)).toBe(firstKnown)
expect(store.getters.getLastKnownMessageId(TOKEN)).toBe(lastKnown)
})

test('cancels fetching messages', () => {
Expand Down Expand Up @@ -1111,6 +1101,7 @@ describe('messagesStore', () => {
token: TOKEN,
lastKnownMessageId: 3,
includeLastKnown: false,
lookIntoFuture: CHAT.FETCH_OLD,
limit: CHAT.FETCH_LIMIT,
}, undefined)

Expand Down
Loading