From a6f2877ed9d8a2263596868cbd83d2bfd38470c0 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 10 Nov 2017 15:53:59 +0100 Subject: [PATCH] Disco API refactoring This came out of the desire to let `converse-muc` use the API to determine whether MUC is supported. However, we don't know the entity JID before hand and I couldn't think of a good way to query all current and future entities for a feature. So `converse-muc` still does it's own thing without the API, but some refactoring came as a result of attempting. --- docs/source/developer_api.rst | 9 ++- src/converse-controlbox.js | 2 +- src/converse-disco.js | 84 +++++++++++++++++++--------- src/converse-mam.js | 6 +- src/converse-muc.js | 101 +++++++++++++++++----------------- 5 files changed, 121 insertions(+), 81 deletions(-) diff --git a/docs/source/developer_api.rst b/docs/source/developer_api.rst index f15bc4784..d9c9be7e3 100644 --- a/docs/source/developer_api.rst +++ b/docs/source/developer_api.rst @@ -442,14 +442,19 @@ supports Used to determine whether an entity supports a given feature. +Returns a `Promise` which, when resolved, returns a map/object with keys +`supported` (a boolean) and `feature` which is a `Backbone.Model `_. + .. code-block:: javascript converse.plugins.add('myplugin', { initialize: function () { _converse.api.disco.supports(_converse.bare_jid, Strophe.NS.MAM).then( - function (supported) { - if (supported) { + function (value) { + // `value` is a map with two keys, `supported` and `feature`. + + if (value.supported) { // The feature is supported } else { // The feature is not supported diff --git a/src/converse-controlbox.js b/src/converse-controlbox.js index ce588bf29..639a983e0 100644 --- a/src/converse-controlbox.js +++ b/src/converse-controlbox.js @@ -268,7 +268,7 @@ .then(this.insertRoster.bind(this)) .catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL)); } - _converse.emit('controlboxInitialized'); + _converse.emit('controlboxInitialized', this); }, render () { diff --git a/src/converse-disco.js b/src/converse-disco.js index 73295ecc9..435322b02 100644 --- a/src/converse-disco.js +++ b/src/converse-disco.js @@ -12,7 +12,7 @@ define(["converse-core", "sizzle", "strophe.disco"], factory); }(this, function (converse, sizzle) { - const { Backbone, Promise, Strophe, b64_sha1, _ } = converse.env; + const { Backbone, Promise, Strophe, b64_sha1, utils, _ } = converse.env; converse.plugins.add('converse-disco', { @@ -49,6 +49,8 @@ idAttribute: 'jid', initialize () { + this.waitUntilFeaturesDiscovered = utils.getResolveablePromise(); + this.features = new Backbone.Collection(); this.features.browserStorage = new Backbone.BrowserStorage[_converse.storage]( b64_sha1(`converse.features-${this.get('jid')}`) @@ -60,6 +62,36 @@ b64_sha1(`converse.identities-${this.get('jid')}`) ); this.fetchFeatures(); + + }, + + hasFeature (feature) { + /* Returns a Promise which resolves with a map indicating + * whether a given feature is supported. + * + * Parameters: + * (String) feature - The feature that might be supported. + */ + const entity = this; + return new Promise((resolve, reject) => { + function fulfillPromise () { + const model = entity.features.findWhere({'var': feature }); + if (model) { + resolve({ + 'supported': true, + 'feature': model + }); + } else { + resolve({ + 'supported': false, + 'feature': null + }); + } + } + entity.waitUntilFeaturesDiscovered + .then(fulfillPromise) + .catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL)); + }); }, onFeatureAdded (feature) { @@ -70,7 +102,13 @@ if (this.features.browserStorage.records.length === 0) { this.queryInfo(); } else { - this.features.fetch({add: true}); + this.features.fetch({ + add: true, + success: () => { + this.waitUntilFeaturesDiscovered.resolve(); + this.trigger('featuresDiscovered'); + } + }); this.identities.fetch({add: true}); } }, @@ -105,6 +143,7 @@ 'from': stanza.getAttribute('from') }); }); + this.waitUntilFeaturesDiscovered.resolve(); this.trigger('featuresDiscovered'); } }); @@ -182,36 +221,31 @@ /* We extend the default converse.js API to add methods specific to service discovery */ _.extend(_converse.api, { 'disco': { - 'supports' (entity_jid, feature) { - /* Returns a Promise which returns a boolean indicating - * whether the feature is supported or by the given - * entity or not. + 'entities': { + 'get' (entity_jid, create=false) { + const entity = _converse.disco_entities.get(entity_jid); + if (entity || !create) { + return entity; + } + return _converse.disco_entities.create({'jid': entity_jid}); + } + }, + + 'supports' (feature, entity_jid) { + /* Returns a Promise which resolves with a map indicating + * whether a given feature is supported. * * Parameters: - * (String) entity_jid - The JID of the entity which might support the feature. * (String) feature - The feature that might be * supported. In the XML stanza, this is the `var` * attribute of the `` element. For * example: 'http://jabber.org/protocol/muc' + * (String) entity_jid - The JID of the entity which might support the feature. */ - return _converse.api.waitUntil('discoInitialized').then(() => - new Promise((resolve, reject) => { - function fulfillPromise (entity) { - if (entity.features.findWhere({'var': feature})) { - resolve(true); - } else { - resolve(false); - } - } - let entity = _converse.disco_entities.get(entity_jid); - if (_.isUndefined(entity)) { - entity = _converse.disco_entities.create({'jid': entity_jid}); - entity.on('featuresDiscovered', _.partial(fulfillPromise, entity)); - } else { - fulfillPromise(entity); - } - }) - ); + return _converse.api.waitUntil('discoInitialized').then(() => { + const entity = _converse.api.disco.entities.get(entity_jid, true); + return entity.hasFeature(feature); + }); } } }); diff --git a/src/converse-mam.js b/src/converse-mam.js index 41bb9d913..e2c09ddc1 100644 --- a/src/converse-mam.js +++ b/src/converse-mam.js @@ -57,9 +57,9 @@ const { _converse } = this.__super__; this.addSpinner(); - _converse.api.disco.supports(_converse.bare_jid, Strophe.NS.MAM).then( - (supported) => { // Success - if (supported) { + _converse.api.disco.supports(Strophe.NS.MAM, _converse.bare_jid).then( + (result) => { // Success + if (result.supported) { this.fetchArchivedMessages(); } else { this.clearSpinner(); diff --git a/src/converse-muc.js b/src/converse-muc.js index 594a5762f..24467aef6 100755 --- a/src/converse-muc.js +++ b/src/converse-muc.js @@ -175,56 +175,6 @@ } }, - featureAdded (feature) { - const { _converse } = this.__super__; - if ((feature.get('var') === Strophe.NS.MUC) && (_converse.allow_muc)) { - this.setMUCDomain(feature.get('from')); - } - }, - - getMUCDomainFromDisco () { - /* Check whether service discovery for the user's domain - * returned MUC information and use that to automatically - * set the MUC domain for the "Rooms" panel of the - * controlbox. - */ - const { _converse } = this.__super__; - _converse.api.waitUntil('discoInitialized').then(() => { - _converse.api.listen.on('serviceDiscovered', this.featureAdded, this); - // Features could have been added before the controlbox was - // initialized. We're only interested in MUC - const entity = _converse.disco_entities[_converse.domain]; - if (!_.isUndefined(entity)) { - const feature = entity.features.findWhere({'var': Strophe.NS.MUC }); - if (feature) { - this.featureAdded(feature); - } - } - }); - }, - - onConnected () { - const { _converse } = this.__super__; - this.__super__.onConnected.apply(this, arguments); - if (!this.model.get('connected')) { - return; - } - if (_.isUndefined(_converse.muc_domain)) { - this.getMUCDomainFromDisco(); - } else { - this.setMUCDomain(_converse.muc_domain); - } - }, - - setMUCDomain (domain) { - const { _converse } = this.__super__; - _converse.muc_domain = domain; - this.roomspanel.model.save({'muc_domain': domain}); - const $server= this.$el.find('input.new-chatroom-server'); - if (!$server.is(':focus')) { - $server.val(this.roomspanel.model.get('muc_domain')); - } - } }, ChatBoxViews: { @@ -2488,6 +2438,7 @@ }, onDomainChange (model) { + // TODO: Could instead use the vdom in render const $server = this.$el.find('input.new-chatroom-server'); $server.val(model.get('muc_domain')); if (_converse.auto_list_rooms) { @@ -2858,6 +2809,56 @@ }); }); + + function setMUCDomainFromDisco (controlboxview) { + /* Check whether service discovery for the user's domain + * returned MUC information and use that to automatically + * set the MUC domain for the "Rooms" panel of the controlbox. + */ + function featureAdded (feature) { + if ((feature.get('var') === Strophe.NS.MUC)) { + setMUCDomain(feature.get('from'), controlboxview); + } + } + + _converse.api.waitUntil('discoInitialized').then(() => { + _converse.api.listen.on('serviceDiscovered', featureAdded); + // Features could have been added before the controlbox was + // initialized. We're only interested in MUC + _converse.disco_entities.each((entity) => { + const feature = entity.features.findWhere({'var': Strophe.NS.MUC }); + if (feature) { + featureAdded(feature) + } + }); + }).catch(_.partial(_converse.log, _, Strophe.LogLevel.ERROR)); + } + + function setMUCDomain (domain, controlboxview) { + _converse.muc_domain = domain; + controlboxview.roomspanel.model.save({'muc_domain': domain}); + } + + function fetchAndSetMUCDomain (controlboxview) { + if (controlboxview.model.get('connected')) { + if (!controlboxview.roomspanel.model.get('muc_domain')) { + if (_.isUndefined(_converse.muc_domain)) { + setMUCDomainFromDisco(controlboxview); + } else { + setMUCDomain(_converse.muc_domain, controlboxview); + } + } + } + } + + _converse.on('controlboxInitialized', function (view) { + if (!_converse.allow_muc) { + return; + } + fetchAndSetMUCDomain(view); + view.model.on('change:connected', _.partial(fetchAndSetMUCDomain, view)); + }); + function disconnectChatRooms () { /* When disconnecting, or reconnecting, mark all chat rooms as * disconnected, so that they will be properly entered again