MUC: Ensure that MAM query is from latest cached messages

In some cases a race condition can occur where a new message is received
before the MAM query starts.

Previously, the newly received message would be considered the latest
message to query from, thereby causing a gap in the history.
This commit is contained in:
JC Brand 2021-02-03 10:20:35 +01:00
parent 5feaab9a95
commit 0a1cbf87b8
4 changed files with 67 additions and 8 deletions

View File

@ -272,10 +272,62 @@ describe("Message Archive Management", function () {
.map(e => e.textContent).join(' ') === "2nd Message 3rd Message 4th Message 5th Message 6th Message", 1000);
done();
}));
it("queries for messages since the most recent cached message in a newly entered MUC",
mock.initConverse(['discoInitialized'],
{
'archived_messages_page_size': 2,
'muc_nickname_from_jid': false,
'muc_clear_messages_on_leave': false,
}, async function (done, _converse) {
const { api } = _converse;
const sent_IQs = _converse.connection.IQ_stanzas;
const muc_jid = 'orchard@chat.shakespeare.lit';
const nick = 'romeo';
const room_creation_promise = api.rooms.open(muc_jid);
await mock.getRoomFeatures(_converse, muc_jid);
await mock.waitForReservedNick(_converse, muc_jid, nick);
await mock.receiveOwnMUCPresence(_converse, muc_jid, nick);
await room_creation_promise;
const view = _converse.chatboxviews.get(muc_jid);
await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED));
// Create "cached" message to test that only messages newer than the
// last cached message with body text will be fetched
view.model.messages.create({
'type': 'groupchat',
'to': muc_jid,
'from': `${_converse.bare_jid}/orchard`,
'body': 'Hello world',
'message': 'Hello world',
'time': '2021-02-02T12:00:00Z'
});
// Hack: Manually set attributes that would otherwise happen in fetchMessages
view.model.messages.fetched_flag = true;
view.model.afterMessagesFetched(view.model.messages);
view.model.messages.fetched.resolve();
const affs = _converse.muc_fetch_members;
const all_affiliations = Array.isArray(affs) ? affs : (affs ? ['member', 'admin', 'owner'] : []);
await mock.returnMemberLists(_converse, muc_jid, [], all_affiliations);
const iq_get = await u.waitUntil(() => sent_IQs.filter(iq => iq.querySelector(`iq query[xmlns="${Strophe.NS.MAM}"]`)).pop());
expect(Strophe.serialize(iq_get)).toBe(
`<iq id="${iq_get.getAttribute('id')}" to="${muc_jid}" type="set" xmlns="jabber:client">`+
`<query queryid="${iq_get.querySelector('query').getAttribute('queryid')}" xmlns="${Strophe.NS.MAM}">`+
`<x type="submit" xmlns="jabber:x:data">`+
`<field type="hidden" var="FORM_TYPE"><value>urn:xmpp:mam:2</value></field>`+
`<field var="start"><value>2021-02-02T12:00:00.000Z</value></field>`+
`</x>`+
`<set xmlns="http://jabber.org/protocol/rsm"><max>2</max></set>`+
`</query>`+
`</iq>`);
return done();
}));
});
describe("An archived message", function () {
describe("when received", function () {
it("is discarded if it doesn't come from the right sender",

View File

@ -385,7 +385,11 @@ describe("Groupchats", function () {
'time': '2021-02-02T12:00:00Z'
});
expect(view.model.session.get('connection_status')).toBe(converse.ROOMSTATUS.NICKNAME_REQUIRED);
expect(view.el.querySelectorAll('converse-chat-message').length).toBe(1);
await u.waitUntil(() => view.el.querySelectorAll('converse-chat-message').length === 1);
const sel = 'converse-message-history converse-chat-message .chat-msg__text';
await u.waitUntil(() => view.el.querySelector(sel)?.textContent.trim());
expect(view.el.querySelector(sel).textContent.trim()).toBe('Hello world')
view.el.querySelector('[name="nick"]').value = nick;
view.el.querySelector('.muc-nickname-form input[type="submit"]').click();

View File

@ -103,7 +103,8 @@ const ChatBox = ModelWithContact.extend({
this.notifications = new Model();
},
afterMessagesFetched () {
afterMessagesFetched (messages) {
this.most_recent_cached_message = messages ? this.getMostRecentMessage(messages) : null;
/**
* Triggered whenever a `_converse.ChatBox` instance has fetched its messages from
* `sessionStorage` but **NOT** from the server.
@ -119,11 +120,12 @@ const ChatBox = ModelWithContact.extend({
log.info(`Not re-fetching messages for ${this.get('jid')}`);
return;
}
this.most_recent_cached_message = null;
this.messages.fetched_flag = true;
const resolve = this.messages.fetched.resolve;
this.messages.fetch({
'add': true,
'success': () => { this.afterMessagesFetched(); resolve() },
'success': msgs => { this.afterMessagesFetched(msgs); resolve() },
'error': () => { this.afterMessagesFetched(); resolve() }
});
return this.messages.fetched;
@ -301,9 +303,10 @@ const ChatBox = ModelWithContact.extend({
}
},
getMostRecentMessage () {
for (let i=this.messages.length-1; i>=0; i--) {
const message = this.messages.at(i);
getMostRecentMessage (messages) {
messages = messages || this.messages;
for (let i=messages.length-1; i>=0; i--) {
const message = messages.at(i);
if (message.get('type') === this.get('message_type')) {
return message;
}

View File

@ -161,8 +161,8 @@ export function fetchNewestMessages (model) {
if (model.disable_mam) {
return;
}
const most_recent_msg = model.most_recent_cached_message;
const most_recent_msg = model.getMostRecentMessage();
// if clear_messages_on_reconnection is true, than any recent messages
// must have been received *after* connection and we instead must query
// for earlier messages