Updates to how dupes are detected.

- Collapse 3 different loops into one.
- Check all saved stanza ids in the parsed attrs, not just the archive id
- Remove check for archive id in converse-mam since it just duplicates what's now being done in converse-chat
- Don't use disco to check for support, XEP-359 mandates that services SHOULD advertise support,
  which is not a strong enough guarantee that they do.

updates #1856

spec/mam.js

@@ -294,12 +294,12 @@
                                 </forwarded>
                             </result>
                         </message>`);
-                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    spyOn(view.model, 'getDuplicateMessage').and.callThrough();
                     spyOn(view.model, 'updateMessage').and.callThrough();
                     view.model.onMessage(stanza);
-                    await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
-                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
-                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    await u.waitUntil(() => view.model.getDuplicateMessage.calls.count());
+                    expect(view.model.getDuplicateMessage.calls.count()).toBe(1);
+                    const result = view.model.getDuplicateMessage.calls.all()[0].returnValue
                     expect(result instanceof _converse.Message).toBe(true);
                     expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
 
@@ -339,11 +339,11 @@
                                 </forwarded>
                             </result>
                         </message>`);
-                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    spyOn(view.model, 'getDuplicateMessage').and.callThrough();
                     view.model.onMessage(stanza);
-                    await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
-                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
-                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    await u.waitUntil(() => view.model.getDuplicateMessage.calls.count());
+                    expect(view.model.getDuplicateMessage.calls.count()).toBe(1);
+                    const result = await view.model.getDuplicateMessage.calls.all()[0].returnValue
                     expect(result instanceof _converse.Message).toBe(true);
                     expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                     done();
@@ -389,11 +389,11 @@
                             </result>
                         </message>`);
 
-                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    spyOn(view.model, 'getDuplicateMessage').and.callThrough();
                     view.model.onMessage(stanza);
-                    await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
-                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
-                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    await u.waitUntil(() => view.model.getDuplicateMessage.calls.count());
+                    expect(view.model.getDuplicateMessage.calls.count()).toBe(1);
+                    const result = await view.model.getDuplicateMessage.calls.all()[0].returnValue
                     expect(result instanceof _converse.Message).toBe(true);
                     expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                     done();

spec/muc_messages.js

@@ -156,7 +156,7 @@
             done();
         }));
 
-        it("is ignored if it has the same stanza-id of an already received on",
+        it("is ignored if it has the same archive-id of an already received one",
             mock.initConverse(
                 ['rosterGroupsFetched'], {},
                 async function (done, _converse) {
@@ -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, 'findDuplicateFromArchiveID').and.callThrough();
+            spyOn(view.model, 'getDuplicateMessage').and.callThrough();
             let stanza = u.toStanza(`
                 <message xmlns="jabber:client"
                          from="room@muc.example.com/some1"
@@ -177,9 +177,9 @@
                 </message>`);
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
             await u.waitUntil(() => view.model.messages.length === 1);
-            await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count() === 1);
-            let result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue;
-            expect(result).toBe(null);
+            await u.waitUntil(() => view.model.getDuplicateMessage.calls.count() === 1);
+            let result = await view.model.getDuplicateMessage.calls.all()[0].returnValue;
+            expect(result).toBe(undefined);
 
             stanza = u.toStanza(`
                 <message xmlns="jabber:client"
@@ -197,8 +197,56 @@
 
             spyOn(view.model, 'updateMessage');
             await view.model.onMessage(stanza);
-            await u.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count() === 2);
-            result = await view.model.findDuplicateFromArchiveID.calls.all()[1].returnValue;
+            await u.waitUntil(() => view.model.getDuplicateMessage.calls.count() === 2);
+            result = await view.model.getDuplicateMessage.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());
+            done();
+        }));
+
+        it("is ignored if it has the same stanza-id of an already received one",
+            mock.initConverse(
+                ['rosterGroupsFetched'], {},
+                async function (done, _converse) {
+
+            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();
+            let 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"/>
+                </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 instanceof Array).toBe(true);
+            expect(result[0] instanceof Object).toBe(true);
+            expect(result[0]['stanza_id room@muc.example.com']).toBe("5f3dbc5e-e1d3-4077-a492-693f3769c7ad");
+
+            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"/>
+                </message>`);
+            spyOn(view.model, 'updateMessage');
+            spyOn(view.model, 'getDuplicateMessage').and.callThrough();
+            _converse.connection._dataRecv(test_utils.createRequest(stanza));
+            await u.waitUntil(() => view.model.getDuplicateMessage.calls.count());
+            result = await view.model.getDuplicateMessage.calls.all()[0].returnValue;
             expect(result instanceof _converse.Message).toBe(true);
             expect(view.model.messages.length).toBe(1);
             await u.waitUntil(() => view.model.updateMessage.calls.count());

src/headless/converse-chat.js

@@ -3,7 +3,7 @@
  * @copyright 2020, the Converse.js contributors
  * @license Mozilla Public License (MPLv2)
  */
-import { get, isObject, isString, pick } from "lodash";
+import { find, get, isMatch, isObject, isString, pick } from "lodash";
 import { Collection } from "skeletor.js/src/collection";
 import { Model } from 'skeletor.js/src/model.js';
 import converse from "./converse-core";
@@ -371,7 +371,7 @@ converse.plugins.add('converse-chat', {
 
             async onMessage (stanza, original_stanza, from_jid) {
                 const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
-                const message = await this.getDuplicateMessage(attrs);
+                const message = this.getDuplicateMessage(attrs);
                 if (message) {
                     this.updateMessage(message, original_stanza);
                 } else if (
@@ -661,29 +661,47 @@ converse.plugins.add('converse-chat', {
                 return message;
             },
 
+            /**
+             * Returns an already cached message (if it exists) based on the
+             * passed in attributes map.
+             * @private
+             * @method _converse.ChatBox#getDuplicateMessage
+             * @param { object } attrs - Attributes representing a received
+             *  message, as returned by {@link stanza_utils.getMessageAttributesFromStanza}
+             * @returns {Promise<_converse.Message>}
+             */
             getDuplicateMessage (attrs) {
-                return this.findDuplicateFromOriginID(attrs) || this.findDuplicateFromMessage(attrs);
+                const predicates = [
+                        ...this.findDuplicateFromStanzaID(attrs),
+                        this.findDuplicateFromOriginID(attrs),
+                        this.findDuplicateFromMessage(attrs)
+                    ].filter(s => s);
+                const msgs = this.messages.models;
+                return find(msgs, m => predicates.reduce((out, p) => (out || isMatch(m.attributes, p)), false));
             },
 
             findDuplicateFromOriginID (attrs) {
-                if (!attrs.origin_id) {
-                    return null;
-                }
-                return this.messages.findWhere({
-                    'origin_id': attrs.origin_id,
-                    'from': attrs.from
+                return attrs.origin_id && {'origin_id': attrs.origin_id, 'from': attrs.from};
+            },
+
+            findDuplicateFromStanzaID (attrs) {
+                const keys = Object.keys(attrs).filter(k => k.startsWith('stanza_id '));
+                return keys.map(key => {
+                    const by_jid = key.replace(/^stanza_id /, '');
+                    const query = {};
+                    query[`stanza_id ${by_jid}`] = attrs[key];
+                    return query;
                 });
             },
 
             findDuplicateFromMessage (attrs) {
-                if (!attrs.message || !attrs.msgid) {
-                    return null;
+                if (attrs.message && attrs.msgid) {
+                    return {
+                        'message': attrs.message,
+                        'from': attrs.from,
+                        'msgid': attrs.msgid
+                    }
                 }
-                return this.messages.findWhere({
-                    'message': attrs.message,
-                    'from': attrs.from,
-                    'msgid': attrs.msgid
-                });
             },
 
             /**

src/headless/converse-mam.js

@@ -22,20 +22,6 @@ converse.plugins.add('converse-mam', {
 
     dependencies: ['converse-rsm', 'converse-disco', 'converse-muc'],
 
-    overrides: {
-        // Overrides mentioned here will be picked up by converse.js's
-        // plugin architecture they will replace existing methods on the
-        // relevant objects or classes.
-        ChatBox: {
-            getDuplicateMessage (attrs) {
-                const message = this.__super__.getDuplicateMessage.apply(this, arguments);
-                if (!message) {
-                    return this.findDuplicateFromArchiveID(attrs);
-                }
-                return message;
-            }
-        }
-    },
 
     initialize () {
         /* The initialize function gets called as soon as the plugin is
@@ -139,14 +125,6 @@ converse.plugins.add('converse-mam', {
                     // render as a link to fetch further messages, either
                     // to fetch older messages or to fill in a gap.
                 }
-            },
-
-            findDuplicateFromArchiveID (attrs) {
-                if (!attrs.is_archived) { return null; }
-                const key = `stanza_id ${this.get('jid')}`;
-                const query = {};
-                query[key] = attrs[`stanza_id ${this.get('jid')}`];
-                return this.messages.findWhere(query);
             }
         }
         Object.assign(_converse.ChatBox.prototype, MAMEnabledChat);

src/headless/converse-muc.js

@@ -1748,7 +1748,7 @@ converse.plugins.add('converse-muc', {
                 this.fetchFeaturesIfConfigurationChanged(stanza);
 
                 const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
-                const message = await this.getDuplicateMessage(attrs);
+                const message = this.getDuplicateMessage(attrs);
                 if (message) {
                     this.updateMessage(message, original_stanza);
                 }