2
0
Эх сурвалжийг харах

No need for a separate `archive_id` value.

With MAM2 we can just use stanza-id
JC Brand 6 жил өмнө
parent
commit
33600eeece

+ 108 - 106
dist/converse.js

@@ -61911,6 +61911,9 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         return this.vcard.get('fullname') || this.get('jid');
       },
 
+      updateMessage(message, stanza) {// Overridden in converse-muc and converse-mam
+      },
+
       handleMessageCorrection(stanza) {
         const replace = sizzle(`replace[xmlns="${Strophe.NS.MESSAGE_CORRECT}"]`, stanza).pop();
 
@@ -61942,11 +61945,15 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         return false;
       },
 
+      getDuplicateMessage(stanza) {
+        return this.findDuplicateFromOriginID(stanza) || this.findDuplicateFromStanzaID(stanza);
+      },
+
       findDuplicateFromOriginID(stanza) {
         const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
 
         if (!origin_id) {
-          return false;
+          return null;
         }
 
         return this.messages.findWhere({
@@ -61955,27 +61962,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         });
       },
 
-      async hasDuplicateArchiveID(stanza) {
-        const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
-
-        if (!result) {
-          return false;
-        }
-
-        const by_jid = stanza.getAttribute('from') || this.get('jid');
-        const supported = await _converse.api.disco.supports(Strophe.NS.MAM, by_jid);
-
-        if (!supported.length) {
-          return false;
-        }
-
-        const query = {};
-        query[`stanza_id ${by_jid}`] = result.getAttribute('id');
-        const msg = this.messages.findWhere(query);
-        return !_.isNil(msg);
-      },
-
-      async hasDuplicateStanzaID(stanza) {
+      async findDuplicateFromStanzaID(stanza) {
         const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
 
         if (!stanza_id) {
@@ -61991,8 +61978,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
 
         const query = {};
         query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
-        const msg = this.messages.findWhere(query);
-        return !_.isNil(msg);
+        return this.messages.findWhere(query);
       },
 
       sendMarker(to_jid, id, type) {
@@ -62349,6 +62335,10 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         return attrs;
       },
 
+      isArchived(original_stanza) {
+        return !_.isNil(sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, original_stanza).pop());
+      },
+
       getMessageAttributesFromStanza(stanza, original_stanza) {
         /* Parses a passed in message stanza and returns an object
          * of attributes.
@@ -62361,15 +62351,14 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
          *      that contains the message stanza, if it was
          *      contained, otherwise it's the message stanza itself.
          */
-        const archive = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, original_stanza).pop(),
-              spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(),
+        const spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(),
               delay = sizzle(`delay[xmlns="${Strophe.NS.DELAY}"]`, original_stanza).pop(),
               text = _converse.chatboxes.getMessageBody(stanza) || undefined,
               chat_state = stanza.getElementsByTagName(_converse.COMPOSING).length && _converse.COMPOSING || stanza.getElementsByTagName(_converse.PAUSED).length && _converse.PAUSED || stanza.getElementsByTagName(_converse.INACTIVE).length && _converse.INACTIVE || stanza.getElementsByTagName(_converse.ACTIVE).length && _converse.ACTIVE || stanza.getElementsByTagName(_converse.GONE).length && _converse.GONE;
 
         const attrs = _.extend({
           'chat_state': chat_state,
-          'is_archived': !_.isNil(archive),
+          'is_archived': this.isArchived(original_stanza),
           'is_delayed': !_.isNil(delay),
           'is_spoiler': !_.isNil(spoiler),
           'is_single_emoji': text ? u.isSingleEmoji(text) : false,
@@ -62637,12 +62626,20 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
           'nickname': roster_nick
         }, has_body);
 
-        if (chatbox && !chatbox.findDuplicateFromOriginID(stanza) && !(await chatbox.hasDuplicateArchiveID(original_stanza)) && !(await chatbox.hasDuplicateStanzaID(stanza)) && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
-          const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
+        if (chatbox) {
+          const message = await chatbox.getDuplicateMessage(stanza);
 
-          if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
-            const msg = chatbox.messages.create(attrs);
-            chatbox.incrementUnreadMsgCounter(msg);
+          if (message) {
+            chatbox.updateMessage(message, original_stanza);
+          }
+
+          if (!message && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
+            const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
+
+            if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
+              const msg = chatbox.messages.create(attrs);
+              chatbox.incrementUnreadMsgCounter(msg);
+            }
           }
         }
 
@@ -65603,18 +65600,6 @@ const RSM_ATTRIBUTES = ['max', 'first', 'last', 'after', 'before', 'index', 'cou
 
 const MAM_ATTRIBUTES = ['with', 'start', 'end'];
 
-function getMessageArchiveID(stanza) {
-  // See https://xmpp.org/extensions/xep-0313.html#results
-  //
-  // The result messages MUST contain a <result/> element with an 'id'
-  // attribute that gives the current message's archive UID
-  const result = sizzle__WEBPACK_IMPORTED_MODULE_3___default()(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
-
-  if (!_.isUndefined(result)) {
-    return result.getAttribute('id');
-  }
-}
-
 function queryForArchivedMessages(_converse, options, callback, errback) {
   /* Internal function, called by the "archive.query" API method.
    */
@@ -65739,10 +65724,44 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-mam
     //
     // New functions which don't exist yet can also be added.
     ChatBox: {
-      async getMessageAttributesFromStanza(message, original_stanza) {
-        const attrs = await this.__super__.getMessageAttributesFromStanza.apply(this, arguments);
-        attrs.archive_id = getMessageArchiveID(original_stanza);
-        return attrs;
+      async findDuplicateFromArchiveID(stanza) {
+        const _converse = this.__super__._converse;
+        const result = sizzle__WEBPACK_IMPORTED_MODULE_3___default()(`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.length) {
+          return null;
+        }
+
+        const query = {};
+        query[`stanza_id ${by_jid}`] = result.getAttribute('id');
+        return this.messages.findWhere(query);
+      },
+
+      async getDuplicateMessage(stanza) {
+        const message = await this.__super__.getDuplicateMessage.apply(this, arguments);
+
+        if (!message) {
+          return this.findDuplicateFromArchiveID(stanza);
+        }
+
+        return message;
+      },
+
+      updateMessage(message, stanza) {
+        this.__super__.updateMessage.apply(this, arguments);
+
+        if (message && !message.get('is_archived')) {
+          message.save(_.extend({
+            'is_archived': this.isArchived(stanza)
+          }, this.getStanzaIDs(stanza)));
+        }
       }
 
     },
@@ -65771,11 +65790,11 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-mam
         if (_.isNil(most_recent_msg)) {
           this.fetchArchivedMessages();
         } else {
-          const archive_id = most_recent_msg.get('archive_id');
+          const stanza_id = most_recent_msg.get(`stanza_id ${this.model.get('jid')}`);
 
-          if (archive_id) {
+          if (stanza_id) {
             this.fetchArchivedMessages({
-              'after': most_recent_msg.get('archive_id')
+              'after': stanza_id
             });
           } else {
             this.fetchArchivedMessages({
@@ -65874,11 +65893,12 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-mam
 
         if (this.content.scrollTop === 0 && this.model.messages.length) {
           const oldest_message = this.model.messages.at(0);
-          const archive_id = oldest_message.get('archive_id');
+          const by_jid = this.model.get('jid');
+          const stanza_id = oldest_message.get(`stanza_id ${by_jid}`);
 
-          if (archive_id) {
+          if (stanza_id) {
             this.fetchArchivedMessages({
-              'before': archive_id
+              'before': stanza_id
             });
           } else {
             this.fetchArchivedMessages({
@@ -65888,24 +65908,6 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-mam
         }
       }
 
-    },
-    ChatRoom: {
-      isDuplicate(message, original_stanza) {
-        const result = this.__super__.isDuplicate.apply(this, arguments);
-
-        if (result) {
-          return result;
-        }
-
-        const archive_id = getMessageArchiveID(original_stanza);
-
-        if (archive_id) {
-          return this.messages.filter({
-            'archive_id': archive_id
-          }).length > 0;
-        }
-      }
-
     },
     ChatRoomView: {
       initialize() {
@@ -67350,39 +67352,6 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
                      acknowledged[xmlns="${Strophe.NS.MARKERS}"]`, stanza).length > 0;
       },
 
-      handleReflection(stanza) {
-        /* Handle a MUC reflected message and return true if so.
-         *
-         * Parameters:
-         *  (XMLElement) stanza: The message stanza
-         */
-        const from = stanza.getAttribute('from');
-        const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
-
-        if (own_message) {
-          const msg = this.findDuplicateFromOriginID(stanza);
-
-          if (msg) {
-            const attrs = {};
-            const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
-            const by_jid = stanza_id ? stanza_id.getAttribute('by') : undefined;
-
-            if (by_jid) {
-              const key = `stanza_id ${by_jid}`;
-              attrs[key] = stanza_id.getAttribute('id');
-            }
-
-            if (!msg.get('received')) {
-              attrs.received = moment().format();
-            }
-
-            msg.save(attrs);
-          }
-
-          return msg ? true : false;
-        }
-      },
-
       subjectChangeHandled(attrs) {
         /* Handle a subject change and return `true` if so.
          *
@@ -67419,6 +67388,33 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
         return is_csn && (attrs.is_delayed || own_message);
       },
 
+      updateMessage(message, stanza) {
+        /* Make sure that the already cached message is updated with
+         * the stanza ID.
+         */
+        _converse.ChatBox.prototype.updateMessage.call(this, message, stanza);
+
+        const from = stanza.getAttribute('from');
+        const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
+
+        if (own_message) {
+          const attrs = {};
+          const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+          const by_jid = stanza_id ? stanza_id.getAttribute('by') : undefined;
+
+          if (by_jid) {
+            const key = `stanza_id ${by_jid}`;
+            attrs[key] = stanza_id.getAttribute('id');
+          }
+
+          if (!message.get('received')) {
+            attrs.received = moment().format();
+          }
+
+          message.save(attrs);
+        }
+      },
+
       async onMessage(stanza) {
         /* Handler for all MUC messages sent to this groupchat.
          *
@@ -67433,7 +67429,13 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
           stanza = forwarded.querySelector('message');
         }
 
-        if (this.handleReflection(stanza) || (await this.hasDuplicateArchiveID(original_stanza)) || (await this.hasDuplicateStanzaID(stanza)) || this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) {
+        const message = await this.getDuplicateMessage(original_stanza);
+
+        if (message) {
+          this.updateMessage(message, original_stanza);
+        }
+
+        if (message || this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) {
           return _converse.emit('message', {
             'stanza': original_stanza
           });

+ 141 - 90
spec/mam.js

@@ -14,96 +14,147 @@
     describe("Message Archive Management", function () {
         // Implement the protocol defined in https://xmpp.org/extensions/xep-0313.html#config
 
-        describe("Archived Messages", function () {
-
-           it("aren't shown as duplicates by comparing their stanza id and archive id",
-                mock.initConverse(
-                    null, ['discoInitialized'], {},
-                    async function (done, _converse) {
-
-                await test_utils.openAndEnterChatRoom(_converse, 'trek-radio', 'conference.lightwitch.org', 'jcbrand');
-                const view = _converse.chatboxviews.get('trek-radio@conference.lightwitch.org');
-                let stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="jcbrand@lightwitch.org/converse.js-73057452" type="groupchat" from="trek-radio@conference.lightwitch.org/comndrdukath#0805 (STO)">
-                        <body>negan</body>
-                        <stanza-id xmlns="urn:xmpp:sid:0" id="45fbbf2a-1059-479d-9283-c8effaf05621" by="trek-radio@conference.lightwitch.org"/>
-                    </message>`);
-                _converse.connection._dataRecv(test_utils.createRequest(stanza));
-                await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
-                // Not sure whether such a race-condition might pose a problem
-                // in "real-world" situations.
-                stanza = u.toStanza(
-                    `<message xmlns="jabber:client"
-                              to="jcbrand@lightwitch.org/converse.js-73057452"
-                              from="trek-radio@conference.lightwitch.org">
-                        <result xmlns="urn:xmpp:mam:2" queryid="82d9db27-6cf8-4787-8c2c-5a560263d823" id="45fbbf2a-1059-479d-9283-c8effaf05621">
-                            <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
-                                <message from="trek-radio@conference.lightwitch.org/comndrdukath#0805 (STO)" type="groupchat">
-                                    <body>negan</body>
-                                </message>
-                            </forwarded>
-                        </result>
-                    </message>`);
-                spyOn(view.model, 'hasDuplicateArchiveID').and.callThrough();
-                view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.hasDuplicateArchiveID.calls.count());
-                expect(view.model.hasDuplicateArchiveID.calls.count()).toBe(1);
-                const result = await view.model.hasDuplicateArchiveID.calls.all()[0].returnValue
-                expect(result).toBe(true);
-                expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
-                done();
-            }));
-
-           it("aren't shown as duplicates by comparing only their archive id",
-                mock.initConverse(
-                    null, ['discoInitialized'], {},
-                    async function (done, _converse) {
-
-                await test_utils.openAndEnterChatRoom(_converse, 'discuss', 'conference.conversejs.org', 'dummy');
-                const view = _converse.chatboxviews.get('discuss@conference.conversejs.org');
-                let stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="dummy@localhost/resource" from="discuss@conference.conversejs.org">
-                        <result xmlns="urn:xmpp:mam:2" queryid="06fea9ca-97c9-48c4-8583-009ff54ea2e8" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3">
-                            <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-12-05T04:53:12Z"/>
-                                <message xmlns="jabber:client" to="discuss@conference.conversejs.org" type="groupchat" xml:lang="en" from="discuss@conference.conversejs.org/prezel">
-                                    <body>looks like omemo fails completely with "bundle is undefined" when there is a device in the devicelist that has no keys published</body>
-                                    <x xmlns="http://jabber.org/protocol/muc#user">
-                                        <item affiliation="none" jid="prezel@blubber.im" role="participant"/>
-                                    </x>
-                                </message>
-                            </forwarded>
-                        </result>
-                    </message>`);
-                view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
-                expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
-
-                stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="dummy@localhost/resource" from="discuss@conference.conversejs.org">
-                        <result xmlns="urn:xmpp:mam:2" queryid="06fea9ca-97c9-48c4-8583-009ff54ea2e8" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3">
-                            <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-12-05T04:53:12Z"/>
-                                <message xmlns="jabber:client" to="discuss@conference.conversejs.org" type="groupchat" xml:lang="en" from="discuss@conference.conversejs.org/prezel">
-                                    <body>looks like omemo fails completely with "bundle is undefined" when there is a device in the devicelist that has no keys published</body>
-                                    <x xmlns="http://jabber.org/protocol/muc#user">
-                                        <item affiliation="none" jid="prezel@blubber.im" role="participant"/>
-                                    </x>
-                                </message>
-                            </forwarded>
-                        </result>
-                    </message>`);
-
-                spyOn(view.model, 'hasDuplicateArchiveID').and.callThrough();
-                view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.hasDuplicateArchiveID.calls.count());
-                expect(view.model.hasDuplicateArchiveID.calls.count()).toBe(1);
-                const result = await view.model.hasDuplicateArchiveID.calls.all()[0].returnValue
-                expect(result).toBe(true);
-                expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
-                done();
-            }))
+        describe("An archived message", function () {
+
+            describe("when recieved", function () {
+
+                it("updates the is_archived value of an already cached version",
+                    mock.initConverse(
+                        null, ['discoInitialized'], {},
+                        async function (done, _converse) {
+
+                    await test_utils.openAndEnterChatRoom(_converse, 'trek-radio', 'conference.lightwitch.org', 'dummy');
+
+                    const view = _converse.chatboxviews.get('trek-radio@conference.lightwitch.org');
+                    let stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="dummy@localhost/resource" type="groupchat" from="trek-radio@conference.lightwitch.org/some1">
+                            <body>Hello</body>
+                            <stanza-id xmlns="urn:xmpp:sid:0" id="45fbbf2a-1059-479d-9283-c8effaf05621" by="trek-radio@conference.lightwitch.org"/>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
+                    expect(view.model.messages.length).toBe(1);
+                    expect(view.model.messages.at(0).get('is_archived')).toBe(false);
+                    expect(view.model.messages.at(0).get('stanza_id trek-radio@conference.lightwitch.org')).toBe('45fbbf2a-1059-479d-9283-c8effaf05621');
+
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client"
+                                to="dummy@localhost/resource"
+                                from="trek-radio@conference.lightwitch.org">
+                            <result xmlns="urn:xmpp:mam:2" queryid="82d9db27-6cf8-4787-8c2c-5a560263d823" id="45fbbf2a-1059-479d-9283-c8effaf05621">
+                                <forwarded xmlns="urn:xmpp:forward:0">
+                                    <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                    <message from="trek-radio@conference.lightwitch.org/some1" type="groupchat">
+                                        <body>Hello</body>
+                                    </message>
+                                </forwarded>
+                            </result>
+                        </message>`);
+                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    spyOn(view.model, 'updateMessage').and.callThrough();
+                    view.model.onMessage(stanza);
+                    await test_utils.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
+                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
+                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    expect(result instanceof _converse.Message).toBe(true);
+                    expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
+
+                    await test_utils.waitUntil(() => view.model.updateMessage.calls.count());
+                    expect(view.model.messages.length).toBe(1);
+                    expect(view.model.messages.at(0).get('is_archived')).toBe(true);
+                    expect(view.model.messages.at(0).get('stanza_id trek-radio@conference.lightwitch.org')).toBe('45fbbf2a-1059-479d-9283-c8effaf05621');
+                    done();
+                }));
+
+                it("isn't shown as duplicate by comparing its stanza id or archive id",
+                    mock.initConverse(
+                        null, ['discoInitialized'], {},
+                        async function (done, _converse) {
+
+                    await test_utils.openAndEnterChatRoom(_converse, 'trek-radio', 'conference.lightwitch.org', 'jcbrand');
+                    const view = _converse.chatboxviews.get('trek-radio@conference.lightwitch.org');
+                    let stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="jcbrand@lightwitch.org/converse.js-73057452" type="groupchat" from="trek-radio@conference.lightwitch.org/comndrdukath#0805 (STO)">
+                            <body>negan</body>
+                            <stanza-id xmlns="urn:xmpp:sid:0" id="45fbbf2a-1059-479d-9283-c8effaf05621" by="trek-radio@conference.lightwitch.org"/>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
+                    // Not sure whether such a race-condition might pose a problem
+                    // in "real-world" situations.
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client"
+                                to="jcbrand@lightwitch.org/converse.js-73057452"
+                                from="trek-radio@conference.lightwitch.org">
+                            <result xmlns="urn:xmpp:mam:2" queryid="82d9db27-6cf8-4787-8c2c-5a560263d823" id="45fbbf2a-1059-479d-9283-c8effaf05621">
+                                <forwarded xmlns="urn:xmpp:forward:0">
+                                    <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                    <message from="trek-radio@conference.lightwitch.org/comndrdukath#0805 (STO)" type="groupchat">
+                                        <body>negan</body>
+                                    </message>
+                                </forwarded>
+                            </result>
+                        </message>`);
+                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    view.model.onMessage(stanza);
+                    await test_utils.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
+                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
+                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    expect(result instanceof _converse.Message).toBe(true);
+                    expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
+                    done();
+                }));
+
+                it("isn't shown as duplicate by comparing only the archive id",
+                    mock.initConverse(
+                        null, ['discoInitialized'], {},
+                        async function (done, _converse) {
+
+                    await test_utils.openAndEnterChatRoom(_converse, 'discuss', 'conference.conversejs.org', 'dummy');
+                    const view = _converse.chatboxviews.get('discuss@conference.conversejs.org');
+                    let stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="dummy@localhost/resource" from="discuss@conference.conversejs.org">
+                            <result xmlns="urn:xmpp:mam:2" queryid="06fea9ca-97c9-48c4-8583-009ff54ea2e8" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3">
+                                <forwarded xmlns="urn:xmpp:forward:0">
+                                    <delay xmlns="urn:xmpp:delay" stamp="2018-12-05T04:53:12Z"/>
+                                    <message xmlns="jabber:client" to="discuss@conference.conversejs.org" type="groupchat" xml:lang="en" from="discuss@conference.conversejs.org/prezel">
+                                        <body>looks like omemo fails completely with "bundle is undefined" when there is a device in the devicelist that has no keys published</body>
+                                        <x xmlns="http://jabber.org/protocol/muc#user">
+                                            <item affiliation="none" jid="prezel@blubber.im" role="participant"/>
+                                        </x>
+                                    </message>
+                                </forwarded>
+                            </result>
+                        </message>`);
+                    view.model.onMessage(stanza);
+                    await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
+                    expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
+
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="dummy@localhost/resource" from="discuss@conference.conversejs.org">
+                            <result xmlns="urn:xmpp:mam:2" queryid="06fea9ca-97c9-48c4-8583-009ff54ea2e8" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3">
+                                <forwarded xmlns="urn:xmpp:forward:0">
+                                    <delay xmlns="urn:xmpp:delay" stamp="2018-12-05T04:53:12Z"/>
+                                    <message xmlns="jabber:client" to="discuss@conference.conversejs.org" type="groupchat" xml:lang="en" from="discuss@conference.conversejs.org/prezel">
+                                        <body>looks like omemo fails completely with "bundle is undefined" when there is a device in the devicelist that has no keys published</body>
+                                        <x xmlns="http://jabber.org/protocol/muc#user">
+                                            <item affiliation="none" jid="prezel@blubber.im" role="participant"/>
+                                        </x>
+                                    </message>
+                                </forwarded>
+                            </result>
+                        </message>`);
+
+                    spyOn(view.model, 'findDuplicateFromArchiveID').and.callThrough();
+                    view.model.onMessage(stanza);
+                    await test_utils.waitUntil(() => view.model.findDuplicateFromArchiveID.calls.count());
+                    expect(view.model.findDuplicateFromArchiveID.calls.count()).toBe(1);
+                    const result = await view.model.findDuplicateFromArchiveID.calls.all()[0].returnValue
+                    expect(result instanceof _converse.Message).toBe(true);
+                    expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
+                    done();
+                }))
+            });
         });
 
         describe("The archive.query API", function () {

+ 15 - 9
spec/messages.js

@@ -2223,7 +2223,7 @@
 
             await test_utils.openAndEnterChatRoom(_converse, 'room', 'muc.example.com', 'dummy');
             const view = _converse.chatboxviews.get('room@muc.example.com');
-            spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
+            spyOn(view.model, 'findDuplicateFromStanzaID').and.callThrough();
             let stanza = u.toStanza(`
                 <message xmlns="jabber:client"
                          from="room@muc.example.com/some1"
@@ -2238,9 +2238,9 @@
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
             await test_utils.waitUntil(() => _converse.api.chats.get().length);
             await test_utils.waitUntil(() => view.model.messages.length === 1);
-            await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count() === 1);
-            let result = await view.model.hasDuplicateStanzaID.calls.all()[0].returnValue;
-            expect(result).toBe(false);
+            await test_utils.waitUntil(() => view.model.findDuplicateFromStanzaID.calls.count() === 1);
+            let result = await view.model.findDuplicateFromStanzaID.calls.all()[0].returnValue;
+            expect(result).toBe(undefined);
 
             stanza = u.toStanza(`
                 <message xmlns="jabber:client"
@@ -2254,9 +2254,9 @@
                     <origin-id xmlns="urn:xmpp:sid:0" id="de305d54-75b4-431b-adb2-eb6b9e546013"/>
                 </message>`);
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
-            await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count() === 2);
-            result = await view.model.hasDuplicateStanzaID.calls.all()[1].returnValue;
-            expect(result).toBe(true);
+            await test_utils.waitUntil(() => view.model.findDuplicateFromStanzaID.calls.count() === 2);
+            result = await view.model.findDuplicateFromStanzaID.calls.all()[1].returnValue;
+            expect(result instanceof _converse.Message).toBe(true);
             expect(view.model.messages.length).toBe(1);
             done();
         }));
@@ -2477,7 +2477,13 @@
                     <origin-id xmlns="urn:xmpp:sid:0" id="${msg_obj.get('origin_id')}"/>
                 </message>`);
             await view.model.onMessage(stanza);
+            await test_utils.waitUntil(() => view.el.querySelectorAll('.chat-msg__receipt').length);
             expect(view.el.querySelectorAll('.chat-msg__receipt').length).toBe(1);
+            expect(view.model.messages.length).toBe(1);
+
+            const message = view.model.messages.at(0);
+            expect(message.get('stanza_id lounge@localhost')).toBe('5f3dbc5e-e1d3-4077-a492-693f3769c7ad');
+            expect(message.get('origin_id')).toBe(msg_obj.get('origin_id'));
             done();
         }));
 
@@ -2518,9 +2524,9 @@
                                by="room@muc.example.com"/>
                     <origin-id xmlns="urn:xmpp:sid:0" id="${attrs.origin_id}"/>
                 </message>`);
-            spyOn(view.model, 'handleReflection').and.callThrough();
+            spyOn(view.model, 'updateMessage').and.callThrough();
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
-            await test_utils.waitUntil(() => view.model.handleReflection.calls.count() === 1);
+            await test_utils.waitUntil(() => view.model.updateMessage.calls.count() === 1);
             expect(view.model.messages.length).toBe(1);
             expect(view.model.messages.at(0).get('stanza_id room@muc.example.com')).toBe("5f3dbc5e-e1d3-4077-a492-693f3769c7ad");
             expect(view.model.messages.at(0).get('origin_id')).toBe(attrs.origin_id);

+ 32 - 35
src/headless/converse-chatboxes.js

@@ -290,6 +290,10 @@ converse.plugins.add('converse-chatboxes', {
                 return this.vcard.get('fullname') || this.get('jid');
             },
 
+            updateMessage (message, stanza) {
+                // Overridden in converse-muc and converse-mam
+            },
+
             handleMessageCorrection (stanza) {
                 const replace = sizzle(`replace[xmlns="${Strophe.NS.MESSAGE_CORRECT}"]`, stanza).pop();
                 if (replace) {
@@ -316,10 +320,14 @@ converse.plugins.add('converse-chatboxes', {
                 return false;
             },
 
+            getDuplicateMessage (stanza) {
+                return  this.findDuplicateFromOriginID(stanza) || this.findDuplicateFromStanzaID(stanza);
+            },
+
             findDuplicateFromOriginID  (stanza) {
                 const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
                 if (!origin_id) {
-                    return false;
+                    return null;
                 }
                 return this.messages.findWhere({
                     'origin_id': origin_id.getAttribute('id'),
@@ -327,23 +335,7 @@ converse.plugins.add('converse-chatboxes', {
                 });
             },
 
-            async hasDuplicateArchiveID (stanza) {
-                const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
-                if (!result) {
-                    return false;
-                }
-                const by_jid = stanza.getAttribute('from') || this.get('jid');
-                const supported = await _converse.api.disco.supports(Strophe.NS.MAM, by_jid);
-                if (!supported.length) {
-                    return false;
-                }
-                const query = {};
-                query[`stanza_id ${by_jid}`] = result.getAttribute('id');
-                const msg = this.messages.findWhere(query);
-                return !_.isNil(msg);
-            },
-
-            async hasDuplicateStanzaID (stanza) {
+            async findDuplicateFromStanzaID(stanza) {
                 const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
                 if (!stanza_id) {
                     return false;
@@ -355,8 +347,7 @@ converse.plugins.add('converse-chatboxes', {
                 }
                 const query = {};
                 query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
-                const msg = this.messages.findWhere(query);
-                return !_.isNil(msg);
+                return this.messages.findWhere(query);
             },
 
             
@@ -654,6 +645,10 @@ converse.plugins.add('converse-chatboxes', {
                 return attrs;
             },
 
+            isArchived (original_stanza) {
+                return !_.isNil(sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, original_stanza).pop());
+            },
+
             getMessageAttributesFromStanza (stanza, original_stanza) {
                 /* Parses a passed in message stanza and returns an object
                  * of attributes.
@@ -666,8 +661,7 @@ converse.plugins.add('converse-chatboxes', {
                  *      that contains the message stanza, if it was
                  *      contained, otherwise it's the message stanza itself.
                  */
-                const archive = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, original_stanza).pop(),
-                      spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(),
+                const spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(),
                       delay = sizzle(`delay[xmlns="${Strophe.NS.DELAY}"]`, original_stanza).pop(),
                       text = _converse.chatboxes.getMessageBody(stanza) || undefined,
                       chat_state = stanza.getElementsByTagName(_converse.COMPOSING).length && _converse.COMPOSING ||
@@ -678,7 +672,7 @@ converse.plugins.add('converse-chatboxes', {
 
                 const attrs = _.extend({
                     'chat_state': chat_state,
-                    'is_archived': !_.isNil(archive),
+                    'is_archived': this.isArchived(original_stanza),
                     'is_delayed': !_.isNil(delay),
                     'is_spoiler': !_.isNil(spoiler),
                     'is_single_emoji': text ? u.isSingleEmoji(text) : false,
@@ -926,18 +920,21 @@ converse.plugins.add('converse-chatboxes', {
                       roster_nick = _.get(_converse.api.contacts.get(contact_jid), 'attributes.nickname'),
                       chatbox = this.getChatBox(contact_jid, {'nickname': roster_nick}, has_body);
 
-                if (chatbox &&
-                        !chatbox.findDuplicateFromOriginID(stanza) &&
-                        !await chatbox.hasDuplicateArchiveID(original_stanza) &&
-                        !await chatbox.hasDuplicateStanzaID(stanza) &&
-                        !chatbox.handleMessageCorrection(stanza) &&
-                        !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me) &&
-                        !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
-
-                    const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
-                    if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
-                        const msg = chatbox.messages.create(attrs);
-                        chatbox.incrementUnreadMsgCounter(msg);
+                if (chatbox) {
+                    const message = await chatbox.getDuplicateMessage(stanza);
+                    if (message) {
+                        chatbox.updateMessage(message, original_stanza);
+                    }
+                    if (!message &&
+                            !chatbox.handleMessageCorrection(stanza) &&
+                            !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me) &&
+                            !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
+
+                        const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
+                        if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
+                            const msg = chatbox.messages.create(attrs);
+                            chatbox.incrementUnreadMsgCounter(msg);
+                        }
                     }
                 }
                 _converse.emit('message', {'stanza': original_stanza, 'chatbox': chatbox});

+ 40 - 42
src/headless/converse-mam.js

@@ -23,17 +23,6 @@ const RSM_ATTRIBUTES = ['max', 'first', 'last', 'after', 'before', 'index', 'cou
 const MAM_ATTRIBUTES = ['with', 'start', 'end'];
 
 
-function getMessageArchiveID (stanza) {
-    // See https://xmpp.org/extensions/xep-0313.html#results
-    //
-    // The result messages MUST contain a <result/> element with an 'id'
-    // attribute that gives the current message's archive UID
-    const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
-    if (!_.isUndefined(result)) {
-        return result.getAttribute('id');
-    }
-}
-
 function queryForArchivedMessages (_converse, options, callback, errback) {
     /* Internal function, called by the "archive.query" API method.
      */
@@ -128,10 +117,38 @@ converse.plugins.add('converse-mam', {
         // New functions which don't exist yet can also be added.
         ChatBox: {
 
-            async getMessageAttributesFromStanza (message, original_stanza) {
-                const attrs = await this.__super__.getMessageAttributesFromStanza.apply(this, arguments);
-                attrs.archive_id = getMessageArchiveID(original_stanza);
-                return attrs;
+            async findDuplicateFromArchiveID (stanza) {
+                const { _converse } = this.__super__;
+                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.length) {
+                    return null;
+                }
+                const query = {};
+                query[`stanza_id ${by_jid}`] = result.getAttribute('id');
+                return this.messages.findWhere(query);
+            },
+
+            async getDuplicateMessage (stanza) {
+                const message = await this.__super__.getDuplicateMessage.apply(this, arguments);
+                if (!message) {
+                    return this.findDuplicateFromArchiveID(stanza);
+                }
+                return message;
+            },
+
+
+            updateMessage (message, stanza) {
+                this.__super__.updateMessage.apply(this, arguments);
+                if (message && !message.get('is_archived')) {
+                    message.save(_.extend({
+                        'is_archived': this.isArchived(stanza)
+                    }, this.getStanzaIDs(stanza)));
+                }
             }
         },
 
@@ -155,15 +172,11 @@ converse.plugins.add('converse-mam', {
                 if (_.isNil(most_recent_msg)) {
                     this.fetchArchivedMessages();
                 } else {
-                    const archive_id = most_recent_msg.get('archive_id');
-                    if (archive_id) {
-                        this.fetchArchivedMessages({
-                            'after': most_recent_msg.get('archive_id')
-                        });
+                    const stanza_id = most_recent_msg.get(`stanza_id ${this.model.get('jid')}`);
+                    if (stanza_id) {
+                        this.fetchArchivedMessages({'after': stanza_id});
                     } else {
-                        this.fetchArchivedMessages({
-                            'start': most_recent_msg.get('time')
-                        });
+                        this.fetchArchivedMessages({'start': most_recent_msg.get('time')});
                     }
                 }
             },
@@ -250,11 +263,10 @@ converse.plugins.add('converse-mam', {
                 const { _converse } = this.__super__;
                 if (this.content.scrollTop === 0 && this.model.messages.length) {
                     const oldest_message = this.model.messages.at(0);
-                    const archive_id = oldest_message.get('archive_id');
-                    if (archive_id) {
-                        this.fetchArchivedMessages({
-                            'before': archive_id
-                        });
+                    const by_jid = this.model.get('jid');
+                    const stanza_id = oldest_message.get(`stanza_id ${by_jid}`);
+                    if (stanza_id) {
+                        this.fetchArchivedMessages({'before': stanza_id});
                     } else {
                         this.fetchArchivedMessages({
                             'end': oldest_message.get('time')
@@ -264,20 +276,6 @@ converse.plugins.add('converse-mam', {
             },
         },
 
-        ChatRoom: {
-
-            isDuplicate (message, original_stanza) {
-                const result = this.__super__.isDuplicate.apply(this, arguments);
-                if (result) {
-                    return result;
-                }
-                const archive_id = getMessageArchiveID(original_stanza);
-                if (archive_id) {
-                    return this.messages.filter({'archive_id': archive_id}).length > 0;
-                }
-            }
-        },
-
         ChatRoomView: {
 
             initialize () {

+ 27 - 30
src/headless/converse-muc.js

@@ -972,33 +972,6 @@ converse.plugins.add('converse-muc', {
                      acknowledged[xmlns="${Strophe.NS.MARKERS}"]`, stanza).length > 0;
             },
 
-            handleReflection (stanza) {
-                /* Handle a MUC reflected message and return true if so.
-                 *
-                 * Parameters:
-                 *  (XMLElement) stanza: The message stanza
-                 */
-                const from = stanza.getAttribute('from');
-                const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
-                if (own_message) {
-                    const msg = this.findDuplicateFromOriginID(stanza);
-                    if (msg) {
-                        const attrs = {};
-                        const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
-                        const by_jid = stanza_id ? stanza_id.getAttribute('by') : undefined;
-                        if (by_jid) {
-                            const key = `stanza_id ${by_jid}`;
-                            attrs[key] = stanza_id.getAttribute('id');
-                        }
-                        if (!msg.get('received')) {
-                            attrs.received = moment().format();
-                        }
-                        msg.save(attrs);
-                    }
-                    return msg ? true : false;
-                }
-            },
-
             subjectChangeHandled (attrs) {
                 /* Handle a subject change and return `true` if so.
                  *
@@ -1029,6 +1002,28 @@ converse.plugins.add('converse-muc', {
                 return is_csn && (attrs.is_delayed || own_message);
             },
 
+            updateMessage (message, stanza) {
+                /* Make sure that the already cached message is updated with
+                 * the stanza ID.
+                 */
+                _converse.ChatBox.prototype.updateMessage.call(this, message, stanza);
+                const from = stanza.getAttribute('from');
+                const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
+                if (own_message) {
+                    const attrs = {};
+                    const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+                    const by_jid = stanza_id ? stanza_id.getAttribute('by') : undefined;
+                    if (by_jid) {
+                        const key = `stanza_id ${by_jid}`;
+                        attrs[key] = stanza_id.getAttribute('id');
+                    }
+                    if (!message.get('received')) {
+                        attrs.received = moment().format();
+                    }
+                    message.save(attrs);
+                }
+            },
+
             async onMessage (stanza) {
                 /* Handler for all MUC messages sent to this groupchat.
                  *
@@ -1042,9 +1037,11 @@ converse.plugins.add('converse-muc', {
                 if (forwarded) {
                     stanza = forwarded.querySelector('message');
                 }
-                if (this.handleReflection(stanza) ||
-                        await this.hasDuplicateArchiveID(original_stanza) ||
-                        await this.hasDuplicateStanzaID(stanza) ||
+                const message = await this.getDuplicateMessage(original_stanza);
+                if (message) {
+                    this.updateMessage(message, original_stanza);
+                }
+                if (message ||
                         this.handleMessageCorrection(stanza) ||
                         this.isReceipt(stanza) ||
                         this.isChatMarker(stanza)) {