Skip to content

Commit 57abd67

Browse files
authored
Merge pull request #14393 from nextcloud/fix/noid/fetch-message-dir
2 parents 7ef7816 + b8dc3c7 commit 57abd67

File tree

7 files changed

+153
-148
lines changed

7 files changed

+153
-148
lines changed

src/FilesSidebarTabApp.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export default {
7373
Talk: OCA.Talk,
7474
sidebarState: OCA.Files.Sidebar.state,
7575
/**
76-
* Stores the cancel function returned by `cancelableLookForNewMessages`,
76+
* Stores the cancel function returned by `cancelablePollNewMessages`,
7777
*/
7878
cancelGetFileConversation: () => {},
7979
isTalkSidebarSupportedForFile: undefined,

src/components/MessagesList/MessagesList.vue

+15-31
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ export default {
273273
immediate: true,
274274
handler(newValue, oldValue) {
275275
if (oldValue) {
276-
this.$store.dispatch('cancelLookForNewMessages', { requestId: oldValue })
276+
this.$store.dispatch('cancelPollNewMessages', { requestId: oldValue })
277277
}
278278
this.handleStartGettingMessagesPreconditions(this.token)
279279

@@ -362,7 +362,7 @@ export default {
362362
EventBus.off('focus-message', this.focusMessage)
363363
EventBus.off('route-change', this.onRouteChange)
364364
EventBus.off('message-height-changed', this.onMessageHeightChanged)
365-
this.$store.dispatch('cancelLookForNewMessages', { requestId: this.chatIdentifier })
365+
this.$store.dispatch('cancelPollNewMessages', { requestId: this.chatIdentifier })
366366
this.destroying = true
367367

368368
unsubscribe('networkOffline', this.handleNetworkOffline)
@@ -695,29 +695,14 @@ export default {
695695

696696
this.isInitialisingMessages = false
697697

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

701701
} else {
702-
this.$store.dispatch('cancelLookForNewMessages', { requestId: this.chatIdentifier })
702+
this.$store.dispatch('cancelPollNewMessages', { requestId: this.chatIdentifier })
703703
}
704704
},
705705

706-
/**
707-
* Fetches the messages of a conversation given the conversation token. Triggers
708-
* a long-polling request for new messages.
709-
* @param token token of conversation where a method was called
710-
*/
711-
async lookForNewMessages(token) {
712-
// Once the history is received, starts looking for new messages.
713-
if (this._isBeingDestroyed || this._isDestroyed) {
714-
console.debug('Prevent getting new messages on a destroyed MessagesList')
715-
return
716-
}
717-
718-
await this.getNewMessages(token)
719-
},
720-
721706
async getMessageContext(token, messageId) {
722707
// Make the request
723708
this.loadingOldMessages = true
@@ -785,12 +770,13 @@ export default {
785770
},
786771

787772
/**
788-
* Creates a long polling request for a new message.
789-
*
773+
* Fetches the messages of a conversation given the conversation token.
774+
* Creates a long polling request for new messages.
790775
* @param token token of conversation where a method was called
791776
*/
792-
async getNewMessages(token) {
777+
async pollNewMessages(token) {
793778
if (this.destroying) {
779+
console.debug('Prevent polling new messages on MessagesList being destroyed')
794780
return
795781
}
796782
// Check that the token has not changed
@@ -803,7 +789,7 @@ export default {
803789
debugTimer.start(`${token} | long polling`)
804790
// TODO: move polling logic to the store and also cancel timers on cancel
805791
this.pollingErrorTimeout = 1
806-
await this.$store.dispatch('lookForNewMessages', {
792+
await this.$store.dispatch('pollNewMessages', {
807793
token,
808794
lastKnownMessageId: this.$store.getters.getLastKnownMessageId(token),
809795
requestId: this.chatIdentifier,
@@ -822,7 +808,7 @@ export default {
822808
// This is not an error, so reset error timeout and poll again
823809
this.pollingErrorTimeout = 1
824810
setTimeout(() => {
825-
this.getNewMessages(token)
811+
this.pollNewMessages(token)
826812
}, 500)
827813
return
828814
}
@@ -836,13 +822,13 @@ export default {
836822
console.debug('Error happened while getting chat messages. Trying again in ', this.pollingErrorTimeout, exception)
837823

838824
setTimeout(() => {
839-
this.getNewMessages(token)
825+
this.pollNewMessages(token)
840826
}, this.pollingErrorTimeout * 1000)
841827
return
842828
}
843829

844830
setTimeout(() => {
845-
this.getNewMessages(token)
831+
this.pollNewMessages(token)
846832
}, 500)
847833
},
848834

@@ -1217,14 +1203,12 @@ export default {
12171203

12181204
handleNetworkOffline() {
12191205
console.debug('Canceling message request as we are offline')
1220-
if (this.cancelLookForNewMessages) {
1221-
this.$store.dispatch('cancelLookForNewMessages', { requestId: this.chatIdentifier })
1222-
}
1206+
this.$store.dispatch('cancelPollNewMessages', { requestId: this.chatIdentifier })
12231207
},
12241208

12251209
handleNetworkOnline() {
12261210
console.debug('Restarting polling of new chat messages')
1227-
this.getNewMessages(this.token)
1211+
this.pollNewMessages(this.token)
12281212
},
12291213

12301214
async onRouteChange({ from, to }) {

src/constants.ts

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export const SESSION = {
2929
export const CHAT = {
3030
FETCH_LIMIT: 100,
3131
MINIMUM_VISIBLE: 5,
32+
FETCH_OLD: 0,
33+
FETCH_NEW: 1,
3234
} as const
3335

3436
export const CALL = {

src/services/__tests__/messagesService.spec.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { CHAT } from '../../constants.ts'
99
import {
1010
fetchMessages,
1111
getMessageContext,
12-
lookForNewMessages,
12+
pollNewMessages,
1313
postNewMessage,
1414
deleteMessage,
1515
editMessage,
@@ -106,8 +106,8 @@ describe('messagesService', () => {
106106
)
107107
})
108108

109-
test('lookForNewMessages calls the chat API endpoint excluding last known', () => {
110-
lookForNewMessages({
109+
test('pollNewMessages calls the chat API endpoint excluding last known', () => {
110+
pollNewMessages({
111111
token: 'XXTOKENXX',
112112
lastKnownMessageId: 1234,
113113
}, {

src/services/messagesService.ts

+17-5
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import SHA256 from 'crypto-js/sha256.js'
99
import axios from '@nextcloud/axios'
1010
import { generateOcsUrl } from '@nextcloud/router'
1111

12+
import { CHAT } from '../constants.ts'
1213
import type {
1314
ChatMessage,
1415
clearHistoryResponse,
@@ -45,15 +46,22 @@ type EditMessagePayload = { token: string, messageId: number, updatedMessage: ed
4546
* @param data.token the conversation token;
4647
* @param data.lastKnownMessageId last known message id;
4748
* @param data.includeLastKnown whether to include the last known message in the response;
49+
* @param [data.lookIntoFuture=0] direction of message fetch
4850
* @param [data.limit=100] Number of messages to load
4951
* @param options options;
5052
*/
51-
const fetchMessages = async function({ token, lastKnownMessageId, includeLastKnown, limit = 100 }: ReceiveMessagesPayload, options?: object): receiveMessagesResponse {
53+
const fetchMessages = async function({
54+
token,
55+
lastKnownMessageId,
56+
includeLastKnown,
57+
lookIntoFuture = CHAT.FETCH_OLD,
58+
limit = 100,
59+
}: ReceiveMessagesPayload, options?: object): receiveMessagesResponse {
5260
return axios.get(generateOcsUrl('apps/spreed/api/v1/chat/{token}', { token }, options), {
5361
...options,
5462
params: {
5563
setReadMarker: 0,
56-
lookIntoFuture: 0,
64+
lookIntoFuture,
5765
lastKnownMessageId,
5866
limit,
5967
includeLastKnown: includeLastKnown ? 1 : 0,
@@ -71,12 +79,16 @@ const fetchMessages = async function({ token, lastKnownMessageId, includeLastKno
7179
* @param [data.limit=100] Number of messages to load
7280
* @param options options
7381
*/
74-
const lookForNewMessages = async ({ token, lastKnownMessageId, limit = 100 }: ReceiveMessagesPayload, options?: object): receiveMessagesResponse => {
82+
const pollNewMessages = async ({
83+
token,
84+
lastKnownMessageId,
85+
limit = 100,
86+
}: ReceiveMessagesPayload, options?: object): receiveMessagesResponse => {
7587
return axios.get(generateOcsUrl('apps/spreed/api/v1/chat/{token}', { token }, options), {
7688
...options,
7789
params: {
7890
setReadMarker: 0,
79-
lookIntoFuture: 1,
91+
lookIntoFuture: CHAT.FETCH_NEW,
8092
lastKnownMessageId,
8193
limit,
8294
includeLastKnown: 0,
@@ -226,7 +238,7 @@ const summarizeChat = async function(token: string, fromMessageId: summarizeChat
226238

227239
export {
228240
fetchMessages,
229-
lookForNewMessages,
241+
pollNewMessages,
230242
getMessageContext,
231243
postNewMessage,
232244
clearConversationHistory,

src/store/messagesStore.js

+39-23
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
editMessage,
2323
updateLastReadMessage,
2424
fetchMessages,
25-
lookForNewMessages,
25+
pollNewMessages,
2626
getMessageContext,
2727
postNewMessage,
2828
postRichObjectToConversation,
@@ -125,11 +125,11 @@ const state = {
125125
*/
126126
cancelGetMessageContext: null,
127127
/**
128-
* Stores the cancel function returned by `cancelableLookForNewMessages`,
128+
* Stores the cancel function returned by `cancelablePollNewMessages`,
129129
* which allows to cancel the previous long polling request for new
130130
* messages before making another one.
131131
*/
132-
cancelLookForNewMessages: {},
132+
cancelPollNewMessages: {},
133133
/**
134134
* Array of temporary message id to cancel function for the "postNewMessage" action
135135
*/
@@ -140,7 +140,7 @@ const getters = {
140140
/**
141141
* Returns whether more messages can be loaded, which means that the current
142142
* message list doesn't yet contain all future messages.
143-
* If false, the next call to "lookForNewMessages" will be blocking/long-polling.
143+
* If false, the next call to "pollNewMessages" will be blocking/long-polling.
144144
*
145145
* @param {object} state the state object.
146146
* @param {object} getters the getters object.
@@ -264,11 +264,11 @@ const mutations = {
264264
state.cancelGetMessageContext = cancelFunction
265265
},
266266

267-
setCancelLookForNewMessages(state, { requestId, cancelFunction }) {
267+
setCancelPollNewMessages(state, { requestId, cancelFunction }) {
268268
if (cancelFunction) {
269-
Vue.set(state.cancelLookForNewMessages, requestId, cancelFunction)
269+
Vue.set(state.cancelPollNewMessages, requestId, cancelFunction)
270270
} else {
271-
Vue.delete(state.cancelLookForNewMessages, requestId)
271+
Vue.delete(state.cancelPollNewMessages, requestId)
272272
}
273273
},
274274

@@ -856,8 +856,16 @@ const actions = {
856856
* @param {string} data.lastKnownMessageId last known message id;
857857
* @param {number} data.minimumVisible Minimum number of chat messages we want to load
858858
* @param {boolean} data.includeLastKnown whether to include the last known message in the response;
859+
* @param {number} [data.lookIntoFuture=0] direction of message fetch
859860
*/
860-
async fetchMessages(context, { token, lastKnownMessageId, includeLastKnown, requestOptions, minimumVisible }) {
861+
async fetchMessages(context, {
862+
token,
863+
lastKnownMessageId,
864+
includeLastKnown,
865+
requestOptions,
866+
minimumVisible,
867+
lookIntoFuture = CHAT.FETCH_OLD,
868+
}) {
861869
minimumVisible = (typeof minimumVisible === 'undefined') ? CHAT.MINIMUM_VISIBLE : minimumVisible
862870

863871
context.dispatch('cancelFetchMessages')
@@ -871,6 +879,7 @@ const actions = {
871879
token,
872880
lastKnownMessageId,
873881
includeLastKnown,
882+
lookIntoFuture,
874883
limit: CHAT.FETCH_LIMIT,
875884
}, requestOptions)
876885

@@ -904,10 +913,12 @@ const actions = {
904913
})
905914

906915
if (response.headers['x-chat-last-given']) {
907-
context.dispatch('setFirstKnownMessageId', {
908-
token,
909-
id: parseInt(response.headers['x-chat-last-given'], 10),
910-
})
916+
const id = parseInt(response.headers['x-chat-last-given'], 10)
917+
if (lookIntoFuture === CHAT.FETCH_NEW) {
918+
context.dispatch('setLastKnownMessageId', { token, id })
919+
} else {
920+
context.dispatch('setFirstKnownMessageId', { token, id })
921+
}
911922
}
912923

913924
// For guests we also need to set the last known message id
@@ -925,12 +936,16 @@ const actions = {
925936

926937
if (minimumVisible > 0) {
927938
debugTimer.tick(`${token} | fetch history`, 'first chunk')
939+
const lastKnownMessageId = lookIntoFuture === CHAT.FETCH_NEW
940+
? context.getters.getLastKnownMessageId(token)
941+
: context.getters.getFirstKnownMessageId(token)
928942
// There are not yet enough visible messages loaded, so fetch another chunk.
929943
// This can happen when a lot of reactions or poll votings happen
930944
return await context.dispatch('fetchMessages', {
931945
token,
932-
lastKnownMessageId: context.getters.getFirstKnownMessageId(token),
946+
lastKnownMessageId,
933947
includeLastKnown,
948+
lookIntoFuture,
934949
minimumVisible,
935950
})
936951
}
@@ -1022,6 +1037,7 @@ const actions = {
10221037
token,
10231038
lastKnownMessageId: context.getters.getFirstKnownMessageId(token),
10241039
includeLastKnown: false,
1040+
lookIntoFuture: CHAT.FETCH_OLD,
10251041
minimumVisible: minimumVisible * 2,
10261042
})
10271043
}
@@ -1072,8 +1088,8 @@ const actions = {
10721088
* @param {object} data.requestOptions request options;
10731089
* @param {number} data.lastKnownMessageId The id of the last message in the store.
10741090
*/
1075-
async lookForNewMessages(context, { token, lastKnownMessageId, requestId, requestOptions }) {
1076-
context.dispatch('cancelLookForNewMessages', { requestId })
1091+
async pollNewMessages(context, { token, lastKnownMessageId, requestId, requestOptions }) {
1092+
context.dispatch('cancelPollNewMessages', { requestId })
10771093

10781094
if (!lastKnownMessageId) {
10791095
// if param is null | undefined, it won't be included in the request query
@@ -1082,17 +1098,17 @@ const actions = {
10821098
}
10831099

10841100
// Get a new cancelable request function and cancel function pair
1085-
const { request, cancel } = CancelableRequest(lookForNewMessages)
1101+
const { request, cancel } = CancelableRequest(pollNewMessages)
10861102

10871103
// Assign the new cancel function to our data value
1088-
context.commit('setCancelLookForNewMessages', { cancelFunction: cancel, requestId })
1104+
context.commit('setCancelPollNewMessages', { cancelFunction: cancel, requestId })
10891105

10901106
const response = await request({
10911107
token,
10921108
lastKnownMessageId,
10931109
limit: CHAT.FETCH_LIMIT,
10941110
}, requestOptions)
1095-
context.commit('setCancelLookForNewMessages', { requestId })
1111+
context.commit('setCancelPollNewMessages', { requestId })
10961112

10971113
if ('x-chat-last-common-read' in response.headers) {
10981114
const lastCommonReadMessage = parseInt(response.headers['x-chat-last-common-read'], 10)
@@ -1195,16 +1211,16 @@ const actions = {
11951211
},
11961212

11971213
/**
1198-
* Cancels a previously running "lookForNewMessages" action if applicable.
1214+
* Cancels a previously running "pollNewMessages" action if applicable.
11991215
*
12001216
* @param {object} context default store context;
12011217
* @param {string} requestId request id
12021218
* @return {boolean} true if a request got cancelled, false otherwise
12031219
*/
1204-
cancelLookForNewMessages(context, { requestId }) {
1205-
if (context.state.cancelLookForNewMessages[requestId]) {
1206-
context.state.cancelLookForNewMessages[requestId]('canceled')
1207-
context.commit('setCancelLookForNewMessages', { requestId })
1220+
cancelPollNewMessages(context, { requestId }) {
1221+
if (context.state.cancelPollNewMessages[requestId]) {
1222+
context.state.cancelPollNewMessages[requestId]('canceled')
1223+
context.commit('setCancelPollNewMessages', { requestId })
12081224
return true
12091225
}
12101226
return false

0 commit comments

Comments
 (0)