Browse Source

Updates #1548. MAM paging improvements.

* Explicit forwards and backwards paging
* Include upper or lower bound when calling `RSM.prototype.next` or `RSM.prototype.previous`
* Bugfix: Don't override new RSM parameters (caused infinite recursion)
JC Brand 6 years ago
parent
commit
7bfb172f6e
3 changed files with 53 additions and 31 deletions
  1. 8 6
      spec/mam.js
  2. 41 21
      src/headless/converse-mam.js
  3. 4 4
      src/headless/converse-rsm.js

+ 8 - 6
spec/mam.js

@@ -46,7 +46,7 @@
                             from="${muc_jid}">
                         <result xmlns="urn:xmpp:mam:2" queryid="${iq_get.querySelector('query').getAttribute('queryid')}" id="${first_msg_id}">
                             <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:15:23Z"/>
                                 <message from="${muc_jid}/some1" type="groupchat">
                                     <body>2nd Message</body>
                                 </message>
@@ -61,7 +61,7 @@
                             from="${muc_jid}">
                         <result xmlns="urn:xmpp:mam:2" queryid="${iq_get.querySelector('query').getAttribute('queryid')}" id="${last_msg_id}">
                             <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:16:23Z"/>
                                 <message from="${muc_jid}/some1" type="groupchat">
                                     <body>3rd Message</body>
                                 </message>
@@ -106,7 +106,7 @@
                             `<x type="submit" xmlns="jabber:x:data">`+
                                 `<field type="hidden" var="FORM_TYPE"><value>urn:xmpp:mam:2</value></field>`+
                             `</x>`+
-                            `<set xmlns="http://jabber.org/protocol/rsm"><max>2</max><after>${message.querySelector('result').getAttribute('id')}</after><before></before></set>`+
+                            `<set xmlns="http://jabber.org/protocol/rsm"><max>2</max><after>${message.querySelector('result').getAttribute('id')}</after></set>`+
                         `</query>`+
                     `</iq>`);
 
@@ -133,7 +133,7 @@
                             from="${muc_jid}">
                         <result xmlns="urn:xmpp:mam:2" queryid="${iq_get.querySelector('query').getAttribute('queryid')}" id="${last_msg_id}">
                             <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:18:23Z"/>
                                 <message from="${muc_jid}/some1" type="groupchat">
                                     <body>5th Message</body>
                                 </message>
@@ -166,7 +166,7 @@
                                 `<field type="hidden" var="FORM_TYPE"><value>urn:xmpp:mam:2</value></field>`+
                             `</x>`+
                             `<set xmlns="http://jabber.org/protocol/rsm">`+
-                                `<max>2</max><before>${first_msg_id}</before>`+
+                                `<max>2</max><after>${last_msg_id}</after>`+
                             `</set>`+
                         `</query>`+
                     `</iq>`);
@@ -178,7 +178,7 @@
                             from="${muc_jid}">
                         <result xmlns="urn:xmpp:mam:2" queryid="${iq_get.querySelector('query').getAttribute('queryid')}" id="${msg_id}">
                             <forwarded xmlns="urn:xmpp:forward:0">
-                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
+                                <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:19:23Z"/>
                                 <message from="${muc_jid}/some1" type="groupchat">
                                     <body>6th Message</body>
                                 </message>
@@ -200,6 +200,8 @@
                 _converse.connection._dataRecv(test_utils.createRequest(result));
                 await u.waitUntil(() => view.model.messages.length === 5);
                 expect(view.model.fetchArchivedMessages.calls.count()).toBe(3);
+                const msg_els = view.content.querySelectorAll('.chat-msg__text');
+                expect(Array.from(msg_els).map(e => e.textContent).join(' ')).toBe("2nd Message 3rd Message 4th Message 5th Message 6th Message");
                 done();
             }));
         });

+ 41 - 21
src/headless/converse-mam.js

@@ -76,18 +76,38 @@ converse.plugins.add('converse-mam', {
                 const most_recent_msg = u.getMostRecentMessage(this);
 
                 if (!most_recent_msg) {
-                    this.fetchArchivedMessages();
+                    this.fetchArchivedMessages({'before': ''});
                 } else {
                     const stanza_id = most_recent_msg.get(`stanza_id ${this.get('jid')}`);
                     if (stanza_id) {
-                        this.fetchArchivedMessages({'after': stanza_id});
+                        this.fetchArchivedMessages({'after': stanza_id}, 'forwards');
                     } else {
-                        this.fetchArchivedMessages({'start': most_recent_msg.get('time')});
+                        this.fetchArchivedMessages({'start': most_recent_msg.get('time')}, 'forwards');
                     }
                 }
             },
 
-            async fetchArchivedMessages (options) {
+            /**
+             * Fetch XEP-0313 archived messages based on the passed in criteria.
+             * @private
+             * @param { Object } options
+             * @param { integer } [options.max] - The maxinum number of items to return.
+             *  Defaults to "archived_messages_page_size"
+             * @param { string } [options.after] - The XEP-0359 stanza ID of a message
+             *  after which messages should be returned. Implies forward paging.
+             * @param { string } [options.before] - The XEP-0359 stanza ID of a message
+             *  before which messages should be returned. Implies backward paging.
+             * @param { string } [options.end] - A date string in ISO-8601 format,
+             *  before which messages should be returned. Implies backward paging.
+             * @param { string } [options.start] - A date string in ISO-8601 format,
+             *  after which messages should be returned. Implies forward paging.
+             * @param { string } [options.with] - The JID of the entity with
+             *  which messages were exchanged.
+             * @param { boolean } [page] - Whether this function should recursively
+             *  page through the entire result set if a limited number of results
+             *  were returned.
+             */
+            async fetchArchivedMessages (options={}, page) {
                 if (this.disable_mam) {
                     return;
                 }
@@ -96,30 +116,30 @@ converse.plugins.add('converse-mam', {
                 if (!(await _converse.api.disco.supports(Strophe.NS.MAM, mam_jid))) {
                     return;
                 }
-                let message_handler;
-                if (is_groupchat) {
-                    message_handler = this.onMessage.bind(this);
-                } else {
-                    message_handler = _converse.chatboxes.onMessage.bind(_converse.chatboxes)
-                }
+                const message_handler = is_groupchat ?
+                    this.onMessage.bind(this) :
+                    _converse.chatboxes.onMessage.bind(_converse.chatboxes);
+
                 const query = Object.assign({
                         'groupchat': is_groupchat,
-                        'before': '', // Page backwards from the most recent message
                         'max': _converse.archived_messages_page_size,
                         'with': this.get('jid'),
                     }, options);
+
                 const result = await _converse.api.archive.query(query);
                 result.messages.forEach(message_handler);
 
-                const catching_up = query.before || query.after;
-                if (result.rsm) {
-                    if (catching_up) {
-                        return this.fetchArchivedMessages(result.rsm.previous(_converse.archived_messages_page_size));
-                    } else {
-                        // TODO: Add a special kind of message which will
-                        // render as a link to fetch further messages, either
-                        // to fetch older messages or to fill in a gap.
+                if (page && result.rsm) {
+                    if (page === 'forwards') {
+                        options = result.rsm.next(_converse.archived_messages_page_size, options.before);
+                    } else if (page === 'backwards') {
+                        options = result.rsm.previous(_converse.archived_messages_page_size, options.after);
                     }
+                    return this.fetchArchivedMessages(options, page);
+                } else {
+                    // TODO: Add a special kind of message which will
+                    // render as a link to fetch further messages, either
+                    // to fetch older messages or to fill in a gap.
                 }
             },
 
@@ -149,7 +169,7 @@ converse.plugins.add('converse-mam', {
                         this.get('mam_initialized')) {
                     return;
                 }
-                this.fetchArchivedMessages();
+                this.fetchNewestMessages();
                 this.save({'mam_initialized': true});
             }
         });
@@ -499,7 +519,7 @@ converse.plugins.add('converse-mam', {
                         const set = sizzle(`set[xmlns="${Strophe.NS.RSM}"]`, fin).pop();
                         if (set) {
                             rsm = new _converse.RSM({'xml': set});
-                            Object.assign(rsm, pick(options, [...MAM_ATTRIBUTES, ..._converse.RSM_ATTRIBUTES]));
+                            Object.assign(rsm, Object.assign(pick(options, [...MAM_ATTRIBUTES, ..._converse.RSM_ATTRIBUTES]), rsm));
                         }
                     }
                     return { messages, rsm }

+ 4 - 4
src/headless/converse-rsm.js

@@ -49,12 +49,12 @@ converse.plugins.add('converse-rsm', {
                 return xml.tree();
             }
 
-            next (max) {
-                return new RSM({max: max, after: this.last});
+            next (max, before) {
+                return new RSM({max: max, after: this.last, before});
             }
 
-            previous (max) {
-                return new RSM({max: max, before: this.first});
+            previous (max, after) {
+                return new RSM({max: max, before: this.first, after});
             }
 
             fromXMLElement (xmlElement) {