From b5783c0668aca1d6f7129d477844d181fa4d3427 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Wed, 30 Sep 2020 12:46:07 +0200 Subject: [PATCH] Refactor `converse-api.query` and the RSM class - The `converse.api.query` method now no longer accepts an RSM instance. - The RSM class now separates `query` parameters from `result` attributes - Improve JSDoc docs and remove need to make `converse-rsm` a plugin - Add typedefs for the options expected by RSM and `api.archive.query` --- CHANGES.md | 1 + spec/mam.js | 66 ++++-------------- spec/muc.js | 2 +- src/headless/converse-mam.js | 118 +++++++++++++++++--------------- src/headless/converse-rsm.js | 128 +++++++++++++++++++++++------------ src/headless/headless.js | 1 - 6 files changed, 162 insertions(+), 154 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bf9418ff4..54d066a0a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -33,6 +33,7 @@ Soon we'll deprecate the latter, so prepare now. - #2201: added html to converse.env - #2213: added CustomElement to converse.env - #2220: fix rendering of emojis in case `use_system_emojis == false` (again). +- The `api.archive.query` method no longer accepts an RSM instance as argument. - The plugin `converse-uniview` has been removed and its functionality merged into `converse-chatboxviews` - Removed the mockups from the project. Recommended to use tests instead. - The API method `api.settings.update` has been deprecated in favor of `api.settings.extend`. diff --git a/spec/mam.js b/spec/mam.js index 00b304ad4..a76e194e4 100644 --- a/spec/mam.js +++ b/spec/mam.js @@ -32,7 +32,7 @@ describe("Message Archive Management", function () { ``+ `urn:xmpp:mam:2`+ ``+ - `2`+ + `2`+ ``+ ``); @@ -105,7 +105,7 @@ describe("Message Archive Management", function () { ``+ `urn:xmpp:mam:2`+ ``+ - `2${message.querySelector('result').getAttribute('id')}`+ + `${message.querySelector('result').getAttribute('id')}2`+ ``+ ``); @@ -165,7 +165,8 @@ describe("Message Archive Management", function () { `urn:xmpp:mam:2`+ ``+ ``+ - `2${last_msg_id}`+ + `${last_msg_id}`+ + `2`+ ``+ ``+ ``); @@ -695,8 +696,8 @@ describe("Message Archive Management", function () { ``+ ``+ ``+ - `10`+ `09af3-cc343-b409f`+ + `10`+ ``+ ``+ ``); @@ -725,49 +726,7 @@ describe("Message Archive Management", function () { ``+ ``+ ``+ - `10`+ ``+ - ``+ - ``+ - ``); - done(); - })); - - it("accepts a _converse.RSM object for the query options", - mock.initConverse([], {}, async function (done, _converse) { - - await mock.waitUntilDiscoConfirmed(_converse, _converse.bare_jid, null, [Strophe.NS.MAM]); - let sent_stanza, IQ_id; - const sendIQ = _converse.connection.sendIQ; - spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { - sent_stanza = iq; - IQ_id = sendIQ.bind(this)(iq, callback, errback); - }); - // Normally the user wouldn't manually make a _converse.RSM object - // and pass it in. However, in the callback method an RSM object is - // returned which can be reused for easy paging. This test is - // more for that usecase. - const rsm = new _converse.RSM({'max': '10'}); - rsm['with'] = 'romeo@montague.lit'; // eslint-disable-line dot-notation - rsm.start = '2010-06-07T00:00:00Z'; - _converse.api.archive.query(rsm); - await u.waitUntil(() => sent_stanza); - const queryid = sent_stanza.querySelector('query').getAttribute('queryid'); - expect(Strophe.serialize(sent_stanza)).toBe( - ``+ - ``+ - ``+ - ``+ - `urn:xmpp:mam:2`+ - ``+ - ``+ - `romeo@montague.lit`+ - ``+ - ``+ - `${dayjs(rsm.start).toISOString()}`+ - ``+ - ``+ - ``+ `10`+ ``+ ``+ @@ -850,11 +809,10 @@ describe("Message Archive Management", function () { expect(result.messages.length).toBe(2); expect(result.messages[0].outerHTML).toBe(msg1.nodeTree.outerHTML); expect(result.messages[1].outerHTML).toBe(msg2.nodeTree.outerHTML); - expect(result.rsm['with']).toBe('romeo@capulet.lit'); // eslint-disable-line dot-notation - expect(result.rsm.max).toBe('10'); - expect(result.rsm.count).toBe('16'); - expect(result.rsm.first).toBe('23452-4534-1'); - expect(result.rsm.last).toBe('09af3-cc343-b409f'); + expect(result.rsm.query.max).toBe('10'); + expect(result.rsm.result.count).toBe(16); + expect(result.rsm.result.first).toBe('23452-4534-1'); + expect(result.rsm.result.last).toBe('09af3-cc343-b409f'); done() })); }); @@ -962,7 +920,7 @@ describe("Chatboxes", function () { `urn:xmpp:mam:2`+ `mercutio@montague.lit`+ ``+ - `50`+ + `50`+ ``+ `` ); @@ -1033,7 +991,7 @@ describe("Chatboxes", function () { `urn:xmpp:mam:2`+ `mercutio@montague.lit`+ ``+ - `50`+ + `50`+ ``+ ``); @@ -1058,7 +1016,7 @@ describe("Chatboxes", function () { `urn:xmpp:mam:2`+ `mercutio@montague.lit`+ ``+ - `50`+ + `50`+ ``+ ``); diff --git a/spec/muc.js b/spec/muc.js index 71bff5f86..ce050c2a8 100644 --- a/spec/muc.js +++ b/spec/muc.js @@ -458,7 +458,7 @@ describe("Groupchats", function () { ``+ `urn:xmpp:mam:2`+ ``+ - `50`+ + `50`+ ``+ ``); diff --git a/src/headless/converse-mam.js b/src/headless/converse-mam.js index 78a973fb0..7fdc5994c 100644 --- a/src/headless/converse-mam.js +++ b/src/headless/converse-mam.js @@ -7,19 +7,15 @@ import "./converse-disco"; import "./converse-rsm"; import { _converse, api, converse } from "@converse/headless/converse-core"; -import { intersection, pick } from 'lodash-es' import log from "./log"; import sizzle from "sizzle"; import st from "./utils/stanza"; +import { RSM } from '@converse/headless/converse-rsm'; const { Strophe, $iq, dayjs } = converse.env; const { NS } = Strophe; const u = converse.env.utils; -// XEP-0313 Message Archive Management -const MAM_ATTRIBUTES = ['with', 'start', 'end']; - - /** * The MUC utils object. Contains utility functions related to multi-user chat. * @mixin MAMEnabledChat @@ -77,7 +73,7 @@ const MAMEnabledChat = { * 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. + * @param { integer } [options.max] - The maximum 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. @@ -102,20 +98,21 @@ const MAMEnabledChat = { if (!(await api.disco.supports(NS.MAM, mam_jid))) { return; } + const max = api.settings.get('archived_messages_page_size') const query = Object.assign({ 'groupchat': is_muc, - 'max': api.settings.get('archived_messages_page_size'), + 'max': max, 'with': this.get('jid'), }, options); const result = await api.archive.query(query); await this.handleMAMResult(result, query, options, page_direction); - if (page_direction && result.rsm) { + if (page_direction && result.rsm && !result.complete) { if (page_direction === 'forwards') { - options = result.rsm.next(api.settings.get('archived_messages_page_size'), options.before); + options = result.rsm.next(max, options.before).query; } else if (page_direction === 'backwards') { - options = result.rsm.previous(api.settings.get('archived_messages_page_size'), options.after); + options = result.rsm.previous(max, options.after).query; } return this.fetchArchivedMessages(options, page_direction); } else { @@ -253,36 +250,38 @@ converse.plugins.add('converse-mam', { * option in the configuration settings section, which you'll * usually want to use in conjunction with this API. * - * @namespace api.archive - * @memberOf api + * @namespace _converse.api.archive + * @memberOf _converse.api */ - 'archive': { + archive: { + /** + * @typedef { module:converse-rsm~RSMQueryParameters } MAMFilterParameters + * Filter parameters which can be used to filter a MAM XEP-0313 archive + * @property { String } [end] - A date string in ISO-8601 format, before which messages should be returned. Implies backward paging. + * @property { String } [start] - A date string in ISO-8601 format, after which messages should be returned. Implies forward paging. + * @property { String } [with] - A JID against which to match messages, according to either their `to` or `from` attributes. + * An item in a MUC archive matches if the publisher of the item matches the JID. + * If `with` is omitted, all messages that match the rest of the query will be returned, regardless of to/from + * addresses of each message. + */ + + /** + * The options that can be passed in to the { @link _converse.api.archive.query } method + * @typedef { module:converse-mam~MAMFilterParameters } ArchiveQueryOptions + * @property { Boolean } [groupchat=false] - Whether the MAM archive is for a groupchat. + */ + /** * Query for archived messages. * * The options parameter can also be an instance of - * _converse.RSM to enable easy querying between results pages. + * RSM to enable easy querying between results pages. * - * @method api.archive.query - * @param {(Object|_converse.RSM)} options Query parameters, either - * MAM-specific or also for Result Set Management. - * Can be either an object or an instance of _converse.RSM. - * Valid query parameters are: - * * `with` - * * `start` - * * `end` - * * `first` - * * `last` - * * `after` - * * `before` - * * `index` - * * `count` - * * `groupchat` + * @method _converse.api.archive.query + * @param { module:converse-mam~ArchiveQueryOptions } options - An object containing query parameters * @throws {Error} An error is thrown if the XMPP server responds with an error. - * @returns { (Promise | _converse.TimeoutError) } A promise which resolves - * to an object which will have keys `messages` and `rsm` which contains a _converse.RSM - * object on which "next" or "previous" can be called before passing it in again - * to this method, to get the next or previous page in the result set. + * @returns { Promise } A promise which resolves + * to a { @link module:converse-mam~MAMQueryResult } object. * * @example * // Requesting all archived messages @@ -369,7 +368,7 @@ converse.plugins.add('converse-mam', { * // repeatedly make a further query to fetch the next batch of messages. * // * // To simplify this usecase for you, the callback method receives not only an array - * // with the returned archived messages, but also a special _converse.RSM (*Result Set Management*) + * // with the returned archived messages, but also a special RSM (*Result Set Management*) * // object which contains the query parameters you passed in, as well * // as two utility methods `next`, and `previous`. * // @@ -378,18 +377,19 @@ converse.plugins.add('converse-mam', { * // archived messages. Please note, when calling these methods, pass in an integer * // to limit your results. * + * const options = {'with': 'john@doe.net', 'max':10}; * let result; * try { - * result = await api.archive.query({'with': 'john@doe.net', 'max':10}); + * result = await api.archive.query(options); * } catch (e) { * // The query was not successful * } * // Do something with the messages, like showing them in your webpage. * result.messages.forEach(m => this.showMessage(m)); * - * while (result.rsm) { + * while (!result.complete) { * try { - * result = await api.archive.query(rsm.next(10)); + * result = await api.archive.query(Object.assign(options, rsm.next(10).query)); * } catch (e) { * // The query was not successful * } @@ -407,8 +407,9 @@ converse.plugins.add('converse-mam', { * // message, pass in the `before` parameter with an empty string value `''`. * * let result; + * const options = {'before': '', 'max':5}; * try { - * result = await api.archive.query({'before': '', 'max':5}); + * result = await api.archive.query(options); * } catch (e) { * // The query was not successful * } @@ -417,7 +418,7 @@ converse.plugins.add('converse-mam', { * * // Now we query again, to get the previous batch. * try { - * result = await api.archive.query(rsm.previous(5);); + * result = await api.archive.query(Object.assign(options, rsm.previous(5).query)); * } catch (e) { * // The query was not successful * } @@ -468,10 +469,9 @@ converse.plugins.add('converse-mam', { } }); stanza.up(); - if (options instanceof _converse.RSM) { - stanza.cnode(options.toXML()); - } else if (intersection(_converse.RSM_ATTRIBUTES, Object.keys(options)).length) { - stanza.cnode(new _converse.RSM(options).toXML()); + const rsm = new RSM(options); + if (Object.keys(rsm.query).length) { + stanza.cnode(rsm.toXML()); } } @@ -498,28 +498,40 @@ converse.plugins.add('converse-mam', { let error; const iq_result = await api.sendIQ(stanza, api.settings.get('message_archiving_timeout'), false) if (iq_result === null) { - const err_msg = "Timeout while trying to fetch archived messages."; + const { __ } = _converse; + const err_msg = __("Timeout while trying to fetch archived messages."); log.error(err_msg); error = new _converse.TimeoutError(err_msg); return { messages, error }; } else if (u.isErrorStanza(iq_result)) { - log.error("Error stanza received while trying to fetch archived messages"); + const { __ } = _converse; + const err_msg = __('An error occurred while querying for archived messages.'); + log.error(err_msg); log.error(iq_result); - return { messages }; + error = new Error(err_msg); + return { messages, error }; } _converse.connection.deleteHandler(message_handler); let rsm; const fin = iq_result && sizzle(`fin[xmlns="${NS.MAM}"]`, iq_result).pop(); - if (fin && [null, 'false'].includes(fin.getAttribute('complete'))) { - const set = sizzle(`set[xmlns="${NS.RSM}"]`, fin).pop(); - if (set) { - rsm = new _converse.RSM({'xml': set}); - Object.assign(rsm, Object.assign(pick(options, [...MAM_ATTRIBUTES, ..._converse.RSM_ATTRIBUTES]), rsm)); - } + const complete = fin?.getAttribute('complete') === 'true' + const set = sizzle(`set[xmlns="${NS.RSM}"]`, fin).pop(); + if (set) { + rsm = new RSM({...options, 'xml': set}); } - return { messages, rsm, error }; + /** + * @typedef { Object } MAMQueryResult + * @property { Array } messages + * @property { RSM } [rsm] - An instance of { @link RSM }. + * You can call `next()` or `previous()` on this instance, + * to get the RSM query parameters for the next or previous + * page in the result set. + * @property { Boolean } complete + * @property { Error } [error] + */ + return { messages, rsm, complete }; } } }); diff --git a/src/headless/converse-rsm.js b/src/headless/converse-rsm.js index c5c38a1c3..f65d1ab19 100644 --- a/src/headless/converse-rsm.js +++ b/src/headless/converse-rsm.js @@ -6,64 +6,102 @@ * Some code taken from the Strophe RSM plugin, licensed under the MIT License * Copyright 2006-2017 Strophe (https://github.com/strophe/strophejs) */ -import { converse } from "./converse-core"; +import { _converse, converse } from "./converse-core"; +import { pick } from 'lodash-es' const { Strophe, $build } = converse.env; Strophe.addNamespace('RSM', 'http://jabber.org/protocol/rsm'); -converse.plugins.add('converse-rsm', { - initialize () { - const { _converse } = this; - const RSM_ATTRIBUTES = ['max', 'first', 'last', 'after', 'before', 'index', 'count']; - _converse.RSM_ATTRIBUTES = RSM_ATTRIBUTES; +/** + * @typedef { Object } RSMQueryParameters + * [XEP-0059 RSM](https://xmpp.org/extensions/xep-0059.html) Attributes that can be used to filter query results + * @property { String } [after] - The XEP-0359 stanza ID of a message after which messages should be returned. Implies forward paging. + * @property { String } [before] - The XEP-0359 stanza ID of a message before which messages should be returned. Implies backward paging. + * @property { Integer } [index=0] - The index of the results page to return. + * @property { Integer } [max] - The maximum number of items to return. + */ + +const RSM_QUERY_PARAMETERS = ['after', 'before', 'index', 'max']; + +const toNumber = v => Number(v); +const toString = v => v.toString(); + +export const RSM_TYPES = { + 'after': toString, + 'before': toString, + 'count': toNumber, + 'first': toString, + 'index': toNumber, + 'last': toString, + 'max': toNumber +}; + +const isUndefined = (x) => typeof x === 'undefined'; - class RSM { - constructor (options) { - if (typeof options.xml != 'undefined') { - this.fromXMLElement(options.xml); - } else { - for (let ii = 0; ii < RSM_ATTRIBUTES.length; ii++) { - const attrib = RSM_ATTRIBUTES[ii]; - this[attrib] = options[attrib]; - } - } - } +// This array contains both query attributes and response attributes +export const RSM_ATTRIBUTES = Object.keys(RSM_TYPES); - toXML () { - let xml = $build('set', {xmlns: Strophe.NS.RSM}); - for (let ii = 0; ii < RSM_ATTRIBUTES.length; ii++) { - const attrib = RSM_ATTRIBUTES[ii]; - if (typeof this[attrib] != 'undefined') { - xml = xml.c(attrib).t(this[attrib].toString()).up(); - } - } - return xml.tree(); - } - next (max, before) { - return new RSM({max: max, after: this.last, before}); - } +/** + * Instances of this class are used to page through query results according to XEP-0059 Result Set Management + * @class RSM + */ +export class RSM { - previous (max, after) { - return new RSM({max: max, before: this.first, after}); - } + static getQueryParameters (options={}) { + return pick(options, RSM_QUERY_PARAMETERS); + } - fromXMLElement (xmlElement) { - for (var ii = 0; ii < RSM_ATTRIBUTES.length; ii++) { - const attrib = RSM_ATTRIBUTES[ii]; - const elem = xmlElement.getElementsByTagName(attrib)[0]; - if (typeof elem != 'undefined' && elem !== null) { - this[attrib] = Strophe.getText(elem); - if (attrib == 'first') { - this.index = elem.getAttribute('index'); - } - } + static parseXMLResult (set) { + const result = {}; + for (var i = 0; i < RSM_ATTRIBUTES.length; i++) { + const attr = RSM_ATTRIBUTES[i]; + const elem = set.getElementsByTagName(attr)[0]; + if (!isUndefined(elem) && elem !== null) { + result[attr] = RSM_TYPES[attr](Strophe.getText(elem)); + if (attr == 'first') { + result.index = RSM_TYPES['index'](elem.getAttribute('index')); } } } - _converse.RSM = RSM; + return result; } -}); + + /** + * Create a new RSM instance + * @param { Object } options - Configuration options + * @constructor + */ + constructor (options={}) { + this.query = RSM.getQueryParameters(options); + this.result = options.xml ? RSM.parseXMLResult(options.xml) : {}; + } + + /** + * Returns a `` XML element that confirms to XEP-0059 Result Set Management. + * The element is constructed based on the { @link module:converse-rsm~RSMQueryParameters } + * that are set on this RSM instance. + * @returns { XMLElement } + */ + toXML () { + const xml = $build('set', {xmlns: Strophe.NS.RSM}); + const reducer = (xml, a) => !isUndefined(this.query[a]) ? xml.c(a).t((this.query[a] || '').toString()).up() : xml; + return RSM_QUERY_PARAMETERS.reduce(reducer, xml).tree(); + } + + next (max, before) { + const options = Object.assign({}, this.query, { after: this.result.last, before, max }); + return new RSM(options); + } + + previous (max, after) { + const options = Object.assign({}, this.query, { after, before: this.result.first, max }); + return new RSM(options); + } +} + +_converse.RSM_ATTRIBUTES = RSM_ATTRIBUTES; +_converse.RSM = RSM; diff --git a/src/headless/headless.js b/src/headless/headless.js index 8bbc814d5..4a301458e 100644 --- a/src/headless/headless.js +++ b/src/headless/headless.js @@ -16,7 +16,6 @@ import "./converse-muc"; // XEP-0045 Multi-user chat import "./converse-ping"; // XEP-0199 XMPP Ping import "./converse-pubsub"; // XEP-0060 Pubsub import "./converse-roster"; // RFC-6121 Contacts Roster -import "./converse-rsm"; // XEP-0059 Result Set management import "./converse-smacks"; // XEP-0198 Stream Management import "./converse-status"; // XEP-0199 XMPP Ping import "./converse-vcard"; // XEP-0054 VCard-temp