MDL-66607 message: Resolve race conditions in message deletion process

This commit makes several changes:
1) Explicitly stop polling for messages when a conversation is deleted;
2) Check for deleted conversations when displaying new messages;
3) Do not add a new empty conversation; and
4) Introduce pendingJS checks to ensure that Behat waits for messags to finish rendering.
This commit is contained in:
Andrew Nicols
2020-03-11 16:23:16 +08:00
parent 9df2f66fb4
commit efeaa51edc
6 changed files with 54 additions and 15 deletions
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -107,6 +107,8 @@ function(
var isResetting = true;
// If the UI is currently sending a message.
var isSendingMessage = false;
// If the UI is currently deleting a conversation.
var isDeletingConversationContent = false;
// A buffer of messages to send.
var sendMessageBuffer = [];
// These functions which will be generated when this module is
@@ -558,7 +560,7 @@ function(
var mostRecentMessage = messages.length ? messages[messages.length - 1] : null;
var lastTimeCreated = mostRecentMessage ? mostRecentMessage.timeCreated : null;
if (lastTimeCreated && !isResetting && !isSendingMessage) {
if (lastTimeCreated && !isResetting && !isSendingMessage && !isDeletingConversationContent) {
// There may be multiple messages with the same time created value since
// the accuracy is only down to the second. The server will include these
// messages in the result (since it does a >= comparison on time from) so
@@ -893,6 +895,14 @@ function(
}
}
// Mark that we are deleting content from the conversation to prevent updates of it.
isDeletingConversationContent = true;
// Stop polling for new messages to the open conversation.
if (newMessagesPollTimer) {
newMessagesPollTimer.stop();
}
return deleteMessagesPromise.then(function() {
var newState = StateManager.removeMessagesById(viewState, messageIds);
newState = StateManager.removePendingDeleteMessagesById(newState, messageIds);
@@ -910,6 +920,7 @@ function(
PubSub.publish(MessageDrawerEvents.CONVERSATION_DELETED, newState.id);
}
isDeletingConversationContent = false;
return render(newState);
})
.catch(Notification.exception);
@@ -937,6 +948,14 @@ function(
var newState = StateManager.setLoadingConfirmAction(viewState, true);
render(newState);
// Mark that we are deleting the conversation to prevent updates of it.
isDeletingConversationContent = true;
// Stop polling for new messages to the open conversation.
if (newMessagesPollTimer) {
newMessagesPollTimer.stop();
}
return Repository.deleteConversation(viewState.loggedInUserId, viewState.id)
.then(function() {
var newState = StateManager.removeMessages(viewState, viewState.messages);
@@ -945,6 +964,8 @@ function(
newState = StateManager.setLoadingConfirmAction(newState, false);
PubSub.publish(MessageDrawerEvents.CONVERSATION_DELETED, newState.id);
isDeletingConversationContent = false;
return render(newState);
});
};
@@ -1132,10 +1153,11 @@ function(
return;
})
.catch(function(e) {
var errorMessage;
if (e.message) {
var errorMessage = $.Deferred().resolve(e.message).promise();
errorMessage = $.Deferred().resolve(e.message).promise();
} else {
var errorMessage = Str.get_string('unknownerror', 'core');
errorMessage = Str.get_string('unknownerror', 'core');
}
var handleFailedMessages = function(errorMessage) {
@@ -1211,8 +1233,6 @@ function(
/**
* Cancel edit mode (selecting the messages).
*
* @return {Promise} Renderer promise.
*/
var cancelEditMode = function() {
cancelRequest(getOtherUserId());
@@ -1249,6 +1269,8 @@ function(
renderable.deferred.resolve(true);
// Keep processing the buffer until it's empty.
processRenderBuffer(header, body, footer);
return;
})
.catch(function(error) {
isRendering = false;
@@ -1760,6 +1782,7 @@ function(
renderBuffer = [];
isResetting = true;
isSendingMessage = false;
isDeletingConversationContent = false;
sendMessageBuffer = [];
var loggedInUserId = loggedInUserProfile.id;
@@ -75,6 +75,7 @@ function(
var LOAD_LIMIT = 50;
var loadedConversationsById = {};
var deletedConversationsById = {};
var loadedTotalCounts = false;
var loadedUnreadCounts = false;
@@ -196,7 +197,6 @@ function(
/**
* Render the messages in the overview page.
*
* @param {Object} contentContainer Conversations content container.
* @param {Array} conversations List of conversations to render.
* @param {Number} userId Logged in user id.
* @return {Object} jQuery promise.
@@ -662,6 +662,7 @@ function(
return;
}
var pendingPromise = new Pending('core_message/message_drawer_view_overview_section:new');
var loggedInUserId = conversation.loggedInUserId;
var conversationId = conversation.id;
var element = getConversationElement(root, conversationId);
@@ -670,19 +671,34 @@ function(
var contentContainer = LazyLoadList.getContentContainer(root);
render([conversation], loggedInUserId)
.then(function(html) {
contentContainer.prepend(html);
element.remove();
return html;
})
if (deletedConversationsById[conversationId]) {
// This conversation was deleted at some point since the messaging drawer was created.
if (conversation.messages[0].timeadded < deletedConversationsById[conversationId]) {
// The 'new' message was added before the conversation was deleted.
// This is probably stale data.
return;
}
}
contentContainer.prepend(html);
element.remove();
return;
})
.then(pendingPromise.resolve)
.catch(Notification.exception);
} else if (conversation.messages.length) {
createNewConversationFromEvent(root, conversation, loggedInUserId)
.then(pendingPromise.resolve)
.catch();
} else {
createNewConversationFromEvent(root, conversation, loggedInUserId);
pendingPromise.resolve();
}
});
PubSub.subscribe(MessageDrawerEvents.CONVERSATION_DELETED, function(conversationId) {
var conversationElement = getConversationElement(root, conversationId);
delete loadedConversationsById[conversationId];
deletedConversationsById[conversationId] = new Date();
if (conversationElement.length) {
deleteConversation(root, conversationElement);
}