disco: Refactor service discovery and add tests.

* `disco#items` are now only fetched when advertised by the entity.
* `identity` information is now also stored on the `DiscoEntity` model.

.eslintrc.json

@@ -18,7 +18,7 @@
         "lodash/prefer-lodash-method": [2, {
             "ignoreMethods": [
                 "find", "endsWith", "startsWith", "filter", "reduce",
-                "map", "replace", "toLower", "split", "trim", "forEach"
+                "map", "replace", "toLower", "split", "trim", "forEach", "toUpperCase"
             ]
         }],
         "lodash/prefer-startswith": "off",

spec/disco.js

@@ -8,8 +8,161 @@
 } (this, function (jasmine, $, converse, mock, test_utils) {
     "use strict";
     var Strophe = converse.env.Strophe;
+    var $iq = converse.env.$iq;
+    var _ = converse.env._;
 
     describe("Service Discovery", function () {
+
+        describe("Whenever converse.js queries a server for its features", function () {
+            it("stores the features it receives", mock.initConverseWithAsync(function (done, _converse) {
+                var IQ_stanzas = _converse.connection.IQ_stanzas;
+                var IQ_ids =  _converse.connection.IQ_ids;
+                test_utils.waitUntil(function () {
+                    return _.filter(IQ_stanzas, function (iq) {
+                        return iq.nodeTree.querySelector('query[xmlns="http://jabber.org/protocol/disco#info"]');
+                    }).length > 0;
+                }, 300).then(function () {
+                    /* <iq type='result'
+                     *      from='plays.shakespeare.lit'
+                     *      to='romeo@montague.net/orchard'
+                     *      id='info1'>
+                     *  <query xmlns='http://jabber.org/protocol/disco#info'>
+                     *      <identity
+                     *          category='conference'
+                     *          type='text'
+                     *          name='Play-Specific Chatrooms'/>
+                     *      <identity
+                     *          category='directory'
+                     *          type='chatroom'
+                     *          name='Play-Specific Chatrooms'/>
+                     *      <feature var='http://jabber.org/protocol/disco#info'/>
+                     *      <feature var='http://jabber.org/protocol/disco#items'/>
+                     *      <feature var='http://jabber.org/protocol/muc'/>
+                     *      <feature var='jabber:iq:register'/>
+                     *      <feature var='jabber:iq:search'/>
+                     *      <feature var='jabber:iq:time'/>
+                     *      <feature var='jabber:iq:version'/>
+                     *  </query>
+                     *  </iq>
+                     */
+                    var info_IQ_id = IQ_ids[0];
+                    var stanza = $iq({
+                        'type': 'result',
+                        'from': 'localhost',
+                        'to': 'dummy@localhost/resource',
+                        'id': info_IQ_id
+                    }).c('query', {'xmlns': 'http://jabber.org/protocol/disco#info'})
+                        .c('identity', {
+                            'category': 'conference',
+                            'type': 'text',
+                            'name': 'Play-Specific Chatrooms'}).up()
+                        .c('identity', {
+                            'category': 'directory',
+                            'type': 'chatroom',
+                            'name': 'Play-Specific Chatrooms'}).up()
+                        .c('feature', {
+                            'var': 'http://jabber.org/protocol/disco#info'}).up()
+                        .c('feature', {
+                            'var': 'http://jabber.org/protocol/disco#items'}).up()
+                        .c('feature', {
+                            'var': 'jabber:iq:register'}).up()
+                        .c('feature', {
+                            'var': 'jabber:iq:time'}).up()
+                        .c('feature', {
+                            'var': 'jabber:iq:version'});
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+
+                    var entities = _converse.disco_entities;
+                    expect(entities.length).toBe(1);
+                    expect(entities.get('localhost').features.length).toBe(5);
+                    expect(entities.get('localhost').features.where({'var': 'jabber:iq:version'}).length).toBe(1);
+                    expect(entities.get('localhost').features.where({'var': 'jabber:iq:time'}).length).toBe(1);
+                    expect(entities.get('localhost').features.where({'var': 'jabber:iq:register'}).length).toBe(1);
+                    expect(entities.get('localhost').features.where(
+                        {'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
+                    expect(entities.get('localhost').features.where(
+                        {'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
+
+
+                test_utils.waitUntil(function () {
+                    // Converse.js sees that the entity has a disco#items feature,
+                    // so it will make a query for it.
+                    return _.filter(IQ_stanzas, function (iq) {
+                        return iq.nodeTree.querySelector('query[xmlns="http://jabber.org/protocol/disco#items"]');
+                    }).length > 0;
+                }, 300).then(function () {
+                    /* <iq type='result'
+                     *     from='catalog.shakespeare.lit'
+                     *     to='romeo@montague.net/orchard'
+                     *     id='items2'>
+                     * <query xmlns='http://jabber.org/protocol/disco#items'>
+                     *     <item jid='people.shakespeare.lit'
+                     *         name='Directory of Characters'/>
+                     *     <item jid='plays.shakespeare.lit'
+                     *         name='Play-Specific Chatrooms'/>
+                     *     <item jid='mim.shakespeare.lit'
+                     *         name='Gateway to Marlowe IM'/>
+                     *     <item jid='words.shakespeare.lit'
+                     *         name='Shakespearean Lexicon'/>
+                     *
+                     *     <item jid='catalog.shakespeare.lit'
+                     *         node='books'
+                     *         name='Books by and about Shakespeare'/>
+                     *     <item jid='catalog.shakespeare.lit'
+                     *         node='clothing'
+                     *         name='Wear your literary taste with pride'/>
+                     *     <item jid='catalog.shakespeare.lit'
+                     *         node='music'
+                     *         name='Music from the time of Shakespeare'/>
+                     * </query>
+                     * </iq>
+                     */
+                   var items_IQ_id = IQ_ids.pop();
+                   stanza = $iq({
+                       'type': 'result',
+                       'from': 'localhost',
+                       'to': 'dummy@localhost/resource',
+                       'id': items_IQ_id
+                   }).c('query', {'xmlns': 'http://jabber.org/protocol/disco#items'})
+                       .c('item', {
+                           'jid': 'people.shakespeare.lit',
+                           'name': 'Directory of Characters'}).up()
+                       .c('item', {
+                           'jid': 'plays.shakespeare.lit',
+                           'name': 'Play-Specific Chatrooms'}).up()
+                       .c('item', {
+                           'jid': 'words.shakespeare.lit',
+                           'name': 'Gateway to Marlowe IM'}).up()
+                       .c('item', {
+                           'jid': 'localhost',
+                           'name': 'Shakespearean Lexicon'}).up()
+
+                       .c('item', {
+                           'jid': 'localhost',
+                           'node': 'books',
+                           'name': 'Books by and about Shakespeare'}).up()
+                       .c('item', {
+                           'node': 'localhost',
+                           'name': 'Wear your literary taste with pride'}).up()
+                       .c('item', {
+                           'jid': 'localhost',
+                           'node': 'music',
+                           'name': 'Music from the time of Shakespeare'
+                       });
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+
+                    entities = _converse.disco_entities;
+                    expect(entities.length).toBe(4);
+                    expect(entities.get(_converse.domain).features.length).toBe(5);
+                    expect(entities.get(_converse.domain).identities.length).toBe(2);
+                    expect(entities.get(_converse.domain).identities.where({'category': 'conference'}).length).toBe(1);
+                    expect(entities.get(_converse.domain).identities.where({'category': 'directory'}).length).toBe(1);
+                    done();
+                });
+                });
+            }));
+        });
+
         describe("Whenever converse.js discovers a new server feature", function () {
            it("emits the serviceDiscovered event",
                 mock.initConverseWithPromises(
@@ -19,6 +172,7 @@
                 sinon.spy(_converse, 'emit');
                 _converse.disco_entities.get(_converse.domain).features.create({'var': Strophe.NS.MAM});
                 expect(_converse.emit.called).toBe(true);
+                expect(_converse.emit.args[0][0]).toBe('serviceDiscovered');
                 expect(_converse.emit.args[0][1].get('var')).toBe(Strophe.NS.MAM);
                 done();
             }));

spec/mam.js

@@ -381,7 +381,7 @@
                     'var': Strophe.NS.MAM
                 });
                 spyOn(feature, 'save').and.callFake(feature.set); // Save will complain about a url not being set
-                _converse.disco_entities.get(_converse.domain).features.onFeatureAdded(feature);
+                _converse.disco_entities.get(_converse.domain).onFeatureAdded(feature);
 
                 expect(_converse.connection.sendIQ).toHaveBeenCalled();
                 expect(sent_stanza.toLocaleString()).toBe(

src/converse-disco.js

@@ -22,19 +22,90 @@
              */
             const { _converse } = this;
 
+            function onDiscoItems (stanza) {
+                _.each(stanza.querySelectorAll('query item'), (item) => {
+                    if (item.getAttribute("node")) {
+                        // XXX: ignore nodes for now.
+                        // See: https://xmpp.org/extensions/xep-0030.html#items-nodes
+                        return;
+                    }
+                    const jid = item.getAttribute('jid');
+                    const entities = _converse.disco_entities;
+                    if (_.isUndefined(entities.get(jid))) {
+                        entities.create({'jid': jid});
+                    }
+                });
+            }
+
             // Promises exposed by this plugin
             _converse.api.promises.add('discoInitialized');
 
             _converse.DiscoEntity = Backbone.Model.extend({
                 /* A Disco Entity is a JID addressable entity that can be queried
                 * for features.
+                *
                 * See XEP-0030: https://xmpp.org/extensions/xep-0030.html
                 */
-                initialize (settings) {
-                    if (_.isNil(settings.jid)) {
-                        throw new Error('DiscoEntity must be instantiated with a JID');
+                idAttribute: 'jid',
+
+                initialize () {
+                    this.features = new Backbone.Collection();
+                    this.features.browserStorage = new Backbone.BrowserStorage[_converse.storage](
+                        b64_sha1(`converse.features-${this.get('jid')}`)
+                    );
+                    this.features.on('add', this.onFeatureAdded);
+
+                    this.identities = new Backbone.Collection();
+                    this.identities.browserStorage = new Backbone.BrowserStorage[_converse.storage](
+                        b64_sha1(`converse.identities-${this.get('jid')}`)
+                    );
+                    this.fetchFeatures();
+                },
+
+                onFeatureAdded (feature) {
+                    _converse.emit('serviceDiscovered', feature);
+                },
+
+                fetchFeatures () {
+                    if (this.features.browserStorage.records.length === 0) {
+                        this.queryInfo();
+                    } else {
+                        this.features.fetch({add: true});
+                        this.identities.fetch({add: true});
                     }
-                    this.features = new _converse.Features({'jid': settings.jid});
+                },
+
+                queryInfo () {
+                    _converse.connection.disco.info(this.get('jid'), null, this.onInfo.bind(this));
+                },
+
+                queryForItems () {
+                    if (_.isEmpty(this.identities.where({'category': 'server'})) &&
+                        _.isEmpty(this.identities.where({'category': 'conference'}))) {
+                        // Don't fetch features and items if this is not a
+                        // server or a conference component.
+                        return;
+                    }
+                    _converse.connection.disco.items(this.get('jid'), null, onDiscoItems);
+                },
+
+                onInfo (stanza) {
+                    _.forEach(stanza.querySelectorAll('identity'), (identity) => {
+                        this.identities.create({
+                            'category': identity.getAttribute('category'),
+                            'type': stanza.getAttribute('type'),
+                            'name': stanza.getAttribute('name')
+                        });
+                    });
+                    if (stanza.querySelector('feature[var="'+Strophe.NS.DISCO_ITEMS+'"]')) {
+                        this.queryForItems();
+                    }
+                    _.forEach(stanza.querySelectorAll('feature'), (feature) => {
+                        this.features.create({
+                            'var': feature.getAttribute('var'),
+                            'from': stanza.getAttribute('from')
+                        });
+                    });
                 }
             });
 
@@ -56,14 +127,8 @@
                         this.fetch({
                             add: true,
                             success: function (collection) {
-                                if (collection.length === 0) {
-                                    /* The sessionStorage is empty */
-                                    // TODO: check for domain in collection even if
-                                    // not empty
-                                    this.create({
-                                        'id': _converse.domain,
-                                        'jid': _converse.domain
-                                    });
+                                if (collection.length === 0 || !collection.get(_converse.domain)) {
+                                    this.create({'jid': _converse.domain});
                                 }
                                 resolve();
                             }.bind(this),
@@ -75,95 +140,29 @@
                 }
             });
 
-            _converse.Features = Backbone.Collection.extend({
-                /* Service Discovery
-                * -----------------
-                * This collection stores Feature Models, representing features
-                * provided by available XMPP entities (e.g. servers)
-                * See XEP-0030 for more details: http://xmpp.org/extensions/xep-0030.html
-                * All features are shown here: http://xmpp.org/registrar/disco-features.html
-                */
-                model: Backbone.Model,
-
-                initialize (settings) {
-                    const jid = settings.jid;
-                    if (_.isNil(jid)) {
-                        throw new Error('DiscoEntity must be instantiated with a JID');
-                    }
-                    this.addClientIdentities().addClientFeatures();
-                    this.browserStorage = new Backbone.BrowserStorage[_converse.storage](
-                        b64_sha1(`converse.features-${jid}`)
-                    );
-                    this.on('add', this.onFeatureAdded, this);
-                    this.fetchFeatures(jid);
-                },
-
-                fetchFeatures (jid) {
-                    if (this.browserStorage.records.length === 0) {
-                        // browserStorage is empty, so we've likely never queried this
-                        // domain for features yet
-                        _converse.connection.disco.info(jid, null, this.onInfo.bind(this));
-                        _converse.connection.disco.items(jid, null, this.onItems.bind(this));
-                    } else {
-                        this.fetch({add:true});
-                    }
-                },
-
-                onFeatureAdded (feature) {
-                    _converse.emit('serviceDiscovered', feature);
-                },
-
-                addClientIdentities () {
-                    /* See http://xmpp.org/registrar/disco-categories.html
-                    */
-                    _converse.connection.disco.addIdentity('client', 'web', 'Converse.js');
-                    return this;
-                },
-
-                addClientFeatures () {
-                    /* The strophe.disco.js plugin keeps a list of features which
-                    * it will advertise to any #info queries made to it.
-                    *
-                    * See: http://xmpp.org/extensions/xep-0030.html#info
-                    */
-                    _converse.connection.disco.addFeature(Strophe.NS.BOSH);
-                    _converse.connection.disco.addFeature(Strophe.NS.CHATSTATES);
-                    _converse.connection.disco.addFeature(Strophe.NS.DISCO_INFO);
-                    _converse.connection.disco.addFeature(Strophe.NS.ROSTERX); // Limited support
-                    if (_converse.message_carbons) {
-                        _converse.connection.disco.addFeature(Strophe.NS.CARBONS);
-                    }
-                    _converse.emit('addClientFeatures');
-                    return this;
-                },
-
-                onItems (stanza) {
-                    _.each(stanza.querySelectorAll('query item'), (item) => {
-                        _converse.connection.disco.info(
-                            item.getAttribute('jid'),
-                            null,
-                            this.onInfo.bind(this));
-                    });
-                },
-
-                onInfo (stanza) {
-                    if ((sizzle('identity[category=server][type=im]', stanza).length === 0) &&
-                        (sizzle('identity[category=conference][type=text]', stanza).length === 0)) {
-                        // This isn't an IM server component
-                        return;
-                    }
-                    _.forEach(stanza.querySelectorAll('feature'), (feature) => {
-                        const namespace = feature.getAttribute('var');
-                        this[namespace] = true;
-                        this.create({
-                            'var': namespace,
-                            'from': stanza.getAttribute('from')
-                        });
-                    });
+            function addClientFeatures () {
+                /* The strophe.disco.js plugin keeps a list of features which
+                 * it will advertise to any #info queries made to it.
+                 *
+                 * See: http://xmpp.org/extensions/xep-0030.html#info
+                 */
+
+                // See http://xmpp.org/registrar/disco-categories.html
+                _converse.connection.disco.addIdentity('client', 'web', 'Converse.js');
+
+                _converse.connection.disco.addFeature(Strophe.NS.BOSH);
+                _converse.connection.disco.addFeature(Strophe.NS.CHATSTATES);
+                _converse.connection.disco.addFeature(Strophe.NS.DISCO_INFO);
+                _converse.connection.disco.addFeature(Strophe.NS.ROSTERX); // Limited support
+                if (_converse.message_carbons) {
+                    _converse.connection.disco.addFeature(Strophe.NS.CARBONS);
                 }
-            });
+                _converse.emit('addClientFeatures');
+                return this;
+            }
 
             function initializeDisco () {
+                addClientFeatures();
                 _converse.disco_entities = new _converse.DiscoEntities();
             }
             _converse.api.listen.on('reconnected', initializeDisco);

tests/mock.js

@@ -49,6 +49,17 @@
         return function () {
             Strophe.Bosh.prototype._processRequest = function () {}; // Don't attempt to send out stanzas
             var c = new Strophe.Connection('jasmine tests');
+            var sendIQ = c.sendIQ;
+
+            c.IQ_stanzas = [];
+            c.IQ_ids = [];
+            c.sendIQ = function (iq, callback, errback) {
+                this.IQ_stanzas.push(iq);
+                var id = sendIQ.bind(this)(iq, callback, errback);
+                this.IQ_ids.push(id);
+                return id;
+            }
+
             c.vcard = {
                 'get': function (callback, jid) {
                     var fullname;
@@ -111,18 +122,15 @@
         return function (done) {
             var _converse = initConverse(settings, spies);
             var promises = _.map(promise_names, _converse.api.waitUntil);
-            Promise.all(promises).then(_.partial(func, done, _converse));
+            Promise.all(promises)
+                .then(_.partial(func, done, _converse))
+                .catch(_.partial(_converse.log, _, Strophe.LogLevel.FATAL));
         }
     };
 
     mock.initConverseWithConnectionSpies = function (spies, settings, func) {
-        if (_.isFunction(settings)) {
-            var _func = settings;
-            settings = func;
-            func = _func;
-        }
-        return function () {
-            return func(initConverse(settings, spies));
+        return function (done) {
+            return func(done, initConverse(settings, spies));
         };
     };