Refactor identification of duplicates...

to rely on the parsed stanza attributes

This was to fix a bug whereby a full stanza was used to query for 1:1
messages with a full JID when the bare JID is stored.

We now are sure that the attributes we're using to query for duplicates
are the same attributes that get saved for a messages.
This commit is contained in:
JC Brand 2020-02-02 17:20:05 +01:00
parent 6430691c22
commit f78837cbc5
5 changed files with 43 additions and 65 deletions

View File

@ -1228,7 +1228,7 @@
// check that msg_counter is incremented from zero again
await _converse.handleMessageStanza(msgFactory());
view = _converse.chatboxviews.get(sender_jid);
expect(u.isVisible(view.el)).toBeTruthy();
await u.waitUntil(() => u.isVisible(view.el));
expect(document.title).toBe('Messages (1) Converse Tests');
done();
}));

View File

@ -164,7 +164,7 @@
const muc_jid = 'room@muc.example.com';
await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
const view = _converse.api.chatviews.get(muc_jid);
spyOn(view.model, 'findDuplicateFromStanzaID').and.callThrough();
spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
let stanza = u.toStanza(`
<message xmlns="jabber:client"
from="room@muc.example.com/some1"
@ -177,24 +177,28 @@
</message>`);
_converse.connection._dataRecv(test_utils.createRequest(stanza));
await u.waitUntil(() => view.model.messages.length === 1);
await u.waitUntil(() => view.model.findDuplicateFromStanzaID.calls.count() === 1);
let result = await view.model.findDuplicateFromStanzaID.calls.all()[0].returnValue;
expect(result).toBe(undefined);
await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count() === 1);
let result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue;
expect(result).toBe(null);
stanza = u.toStanza(`
<message xmlns="jabber:client"
from="room@muc.example.com/some1"
to="${_converse.connection.jid}"
type="groupchat">
<body>Typical body text</body>
<stanza-id xmlns="urn:xmpp:sid:0"
id="5f3dbc5e-e1d3-4077-a492-693f3769c7ad"
by="room@muc.example.com"/>
to="${_converse.connection.jid}"
from="room@muc.example.com">
<result xmlns="urn:xmpp:mam:2" queryid="82d9db27-6cf8-4787-8c2c-5a560263d823" id="5f3dbc5e-e1d3-4077-a492-693f3769c7ad">
<forwarded xmlns="urn:xmpp:forward:0">
<delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
<message from="room@muc.example.com/some1" type="groupchat">
<body>Typical body text</body>
</message>
</forwarded>
</result>
</message>`);
spyOn(view.model, 'updateMessage');
_converse.connection._dataRecv(test_utils.createRequest(stanza));
await u.waitUntil(() => view.model.findDuplicateFromStanzaID.calls.count() === 2);
result = await view.model.findDuplicateFromStanzaID.calls.all()[1].returnValue;
await view.model.onMessage(stanza);
await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count() === 2);
result = await view.model.findDuplicateFromArchiveID.calls.all()[1].returnValue;
expect(result instanceof _converse.Message).toBe(true);
expect(view.model.messages.length).toBe(1);
await u.waitUntil(() => view.model.updateMessage.calls.count());

View File

@ -370,14 +370,14 @@ converse.plugins.add('converse-chat', {
},
async onMessage (stanza, original_stanza, from_jid) {
const message = await this.getDuplicateMessage(stanza);
const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
const message = await this.getDuplicateMessage(attrs);
if (message) {
this.updateMessage(message, original_stanza);
} else if (
!this.handleReceipt (stanza, from_jid) &&
!this.handleChatMarker(stanza, from_jid)
) {
const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
if (this.handleRetraction(attrs)) {
return;
}
@ -661,46 +661,28 @@ converse.plugins.add('converse-chat', {
return message;
},
async getDuplicateMessage (stanza) {
return this.findDuplicateFromOriginID(stanza) ||
await this.findDuplicateFromStanzaID(stanza) ||
this.findDuplicateFromMessage(stanza);
getDuplicateMessage (attrs) {
return this.findDuplicateFromOriginID(attrs) || this.findDuplicateFromMessage(attrs);
},
findDuplicateFromOriginID (stanza) {
const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
if (!origin_id) {
findDuplicateFromOriginID (attrs) {
if (!attrs.origin_id) {
return null;
}
return this.messages.findWhere({
'origin_id': origin_id.getAttribute('id'),
'from': stanza.getAttribute('from')
'origin_id': attrs.origin_id,
'from': attrs.from
});
},
async findDuplicateFromStanzaID (stanza) {
const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
if (!stanza_id) {
return false;
findDuplicateFromMessage (attrs) {
if (!attrs.message || !attrs.msgid) {
return null;
}
const by_jid = stanza_id.getAttribute('by');
if (!(await _converse.api.disco.supports(Strophe.NS.SID, by_jid))) {
return false;
}
const query = {};
query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
return this.messages.findWhere(query);
},
findDuplicateFromMessage (stanza) {
const text = stanza_utils.getMessageBody(stanza) || undefined;
if (!text) { return false; }
const id = stanza.getAttribute('id');
if (!id) { return false; }
return this.messages.findWhere({
'message': text,
'from': stanza.getAttribute('from'),
'msgid': id
'message': attrs.message,
'from': attrs.from,
'msgid': attrs.msgid
});
},

View File

@ -27,10 +27,10 @@ converse.plugins.add('converse-mam', {
// plugin architecture they will replace existing methods on the
// relevant objects or classes.
ChatBox: {
async getDuplicateMessage (stanza) {
const message = await this.__super__.getDuplicateMessage.apply(this, arguments);
getDuplicateMessage (attrs) {
const message = this.__super__.getDuplicateMessage.apply(this, arguments);
if (!message) {
return this.findDuplicateFromArchiveID(stanza);
return this.findDuplicateFromArchiveID(attrs);
}
return message;
}
@ -141,21 +141,13 @@ converse.plugins.add('converse-mam', {
}
},
async findDuplicateFromArchiveID (stanza) {
const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
if (!result) {
return null;
}
const by_jid = stanza.getAttribute('from') || this.get('jid');
const supported = await _converse.api.disco.supports(Strophe.NS.MAM, by_jid);
if (!supported) {
return null;
}
findDuplicateFromArchiveID (attrs) {
if (!attrs.is_archived) { return null; }
const key = `stanza_id ${this.get('jid')}`;
const query = {};
query[`stanza_id ${by_jid}`] = result.getAttribute('id');
query[key] = attrs[`stanza_id ${this.get('jid')}`];
return this.messages.findWhere(query);
},
}
}
Object.assign(_converse.ChatBox.prototype, MAMEnabledChat);

View File

@ -1747,7 +1747,8 @@ converse.plugins.add('converse-muc', {
this.createInfoMessages(stanza);
this.fetchFeaturesIfConfigurationChanged(stanza);
const message = await this.getDuplicateMessage(original_stanza);
const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
const message = await this.getDuplicateMessage(attrs);
if (message) {
this.updateMessage(message, original_stanza);
}
@ -1755,7 +1756,6 @@ converse.plugins.add('converse-muc', {
return _converse.api.trigger('message', {'stanza': original_stanza});
}
const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
if (this.handleRetraction(attrs) ||
this.handleModeration(attrs) ||
this.subjectChangeHandled(attrs) ||