Avoid race-condition that destroys vcards

VCards were being created before `fetch` was completed, so once fetch
was done those VCards were unset from their collection.

Add a new event and promise `VCardsInitialized` that triggers after
successful fetching and wait for it before creating VCards.

spec/chatbox.js

@@ -722,10 +722,8 @@
                         var view = _converse.chatboxviews.get(sender_jid);
                         expect(view).toBeDefined();
 
-                        await u.waitUntil(() => view.model.vcard.get('fullname') === mock.cur_names[1])
-                        // Check that the notification appears inside the chatbox in the DOM
-                        let events = view.el.querySelectorAll('.chat-state-notification');
-                        expect(events[0].textContent).toEqual(mock.cur_names[1] + ' is typing');
+                        const event = await u.waitUntil(() => view.el.querySelector('.chat-state-notification'));
+                        expect(event.textContent).toEqual(mock.cur_names[1] + ' is typing');
 
                         // Check that it doesn't appear twice
                         msg = $msg({
@@ -735,7 +733,7 @@
                                 id: (new Date()).getTime()
                             }).c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree();
                         await _converse.chatboxes.onMessage(msg);
-                        events = view.el.querySelectorAll('.chat-state-notification');
+                        const events = view.el.querySelectorAll('.chat-state-notification');
                         expect(events.length).toBe(1);
                         expect(events[0].textContent).toEqual(mock.cur_names[1] + ' is typing');
                         done();
@@ -778,7 +776,8 @@
                         expect(msg_obj.get('sender')).toEqual('me');
                         expect(msg_obj.get('is_delayed')).toEqual(false);
                         const chat_content = chatboxview.el.querySelector('.chat-content');
-                        const status_text = chat_content.querySelector('.chat-info.chat-state-notification').textContent;
+                        const el = await u.waitUntil(() => chat_content.querySelector('.chat-info.chat-state-notification'));
+                        const status_text = el.textContent;
                         expect(status_text).toBe('Typing from another device');
                         done();
                     }));
@@ -863,7 +862,7 @@
                         await _converse.chatboxes.onMessage(msg);
                         expect(_converse.api.trigger).toHaveBeenCalledWith('message', jasmine.any(Object));
                         await u.waitUntil(() => view.model.vcard.get('fullname') === mock.cur_names[1])
-                        var event = view.el.querySelector('.chat-info.chat-state-notification');
+                        const event = await u.waitUntil(() => view.el.querySelector('.chat-state-notification'));
                         expect(event.textContent).toEqual(mock.cur_names[1] + ' has stopped typing');
                         done();
                     }));
@@ -904,9 +903,8 @@
                         const msg_obj = chatbox.messages.models[0];
                         expect(msg_obj.get('sender')).toEqual('me');
                         expect(msg_obj.get('is_delayed')).toEqual(false);
-                        const chat_content = chatboxview.el.querySelector('.chat-content');
-                        const status_text = chat_content.querySelector('.chat-info.chat-state-notification').textContent;
-                        expect(status_text).toBe('Stopped typing on the other device');
+                        const el = await u.waitUntil(() => chatboxview.el.querySelector('.chat-info.chat-state-notification'));
+                        expect(el.textContent).toBe('Stopped typing on the other device');
                         done();
                     }));
                 });
@@ -1045,7 +1043,7 @@
                             .c('composing', {'xmlns': Strophe.NS.CHATSTATES}).up()
                             .tree();
                         await _converse.chatboxes.onMessage(msg);
-                        await u.waitUntil(() => view.model.messages.length);
+                        await u.waitUntil(() => view.el.querySelector('.chat-state-notification'));
                         expect(view.el.querySelectorAll('.chat-state-notification').length).toBe(1);
                         msg = $msg({
                                 from: sender_jid,
@@ -1056,7 +1054,7 @@
                         await _converse.chatboxes.onMessage(msg);
                         await u.waitUntil(() => (view.model.messages.length > 1));
                         expect(_converse.api.trigger).toHaveBeenCalledWith('message', jasmine.any(Object));
-                        expect(view.el.querySelectorAll('.chat-state-notification').length).toBe(0);
+                        await u.waitUntil(() => view.el.querySelectorAll('.chat-state-notification').length === 0);
                         done();
                     }));
                 });
@@ -1084,7 +1082,7 @@
                         expect(_converse.api.trigger).toHaveBeenCalledWith('message', jasmine.any(Object));
                         const view = _converse.chatboxviews.get(sender_jid);
                         await u.waitUntil(() => view.model.vcard.get('fullname') === mock.cur_names[1]);
-                        const event = view.el.querySelector('.chat-state-notification');
+                        const event = await u.waitUntil(() => view.el.querySelector('.chat-state-notification'));
                         expect(event.textContent).toEqual(mock.cur_names[1] + ' has gone away');
                         done();
                     }));

spec/protocol.js

@@ -165,7 +165,7 @@
                 // A contact should now have been created
                 expect(_converse.roster.get('contact@example.org') instanceof _converse.RosterContact).toBeTruthy();
                 expect(contact.get('jid')).toBe('contact@example.org');
-                expect(_converse.api.vcard.get).toHaveBeenCalled();
+                await u.waitUntil(() => contact.initialized);
 
                 /* To subscribe to the contact's presence information,
                  * the user's client MUST send a presence stanza of

spec/roster.js

@@ -601,7 +601,7 @@
 
             it("do not have a header if there aren't any",
                 mock.initConverse(
-                    ['rosterGroupsFetched'], {},
+                    ['rosterGroupsFetched', 'VCardsInitialized'], {},
                     async function (done, _converse) {
 
                 await test_utils.openControlBox(_converse);
@@ -613,16 +613,15 @@
                     ask: 'subscribe',
                     fullname: name
                 });
-                spyOn(window, 'confirm').and.returnValue(true);
                 await u.waitUntil(() => {
                     const el = _converse.rosterview.get('Pending contacts').el;
                     return u.isVisible(el) && _.filter(el.querySelectorAll('li'), li => u.isVisible(li)).length;
                 }, 700)
 
-                spyOn(_converse.connection, 'sendIQ').and.callThrough();
-                sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, _converse.rosterview.el).pop().click();
+                const remove_el = await u.waitUntil(() => sizzle(`.remove-xmpp-contact[title="Click to remove ${name} as a contact"]`, _converse.rosterview.el).pop());
+                spyOn(window, 'confirm').and.returnValue(true);
+                remove_el.click();
                 expect(window.confirm).toHaveBeenCalled();
-                expect(_converse.connection.sendIQ).toHaveBeenCalled();
 
                 const iq = _converse.connection.IQ_stanzas.pop();
                 expect(Strophe.serialize(iq)).toBe(
@@ -739,7 +738,7 @@
                         ask: null,
                         fullname: name
                     });
-                    return u.waitUntil(() => contact.vcard.get('fullname'));
+                    return u.waitUntil(() => contact.initialized);
                 }));
                 await u.waitUntil(() => sizzle('li', _converse.rosterview.el).length, 600);
                 // Check that they are sorted alphabetically
@@ -1041,7 +1040,7 @@
                         requesting: true,
                         nickname: name
                     });
-                    return u.waitUntil(() => contact.vcard.get('fullname'));
+                    return u.waitUntil(() => contact.initialized);
                 }));
                 await u.waitUntil(() => _converse.rosterview.get('Contact requests').el.querySelectorAll('li').length, 700);
                 expect(_converse.rosterview.update).toHaveBeenCalled();

src/converse-chatview.js

@@ -701,6 +701,7 @@ converse.plugins.add('converse-chatview', {
              * @param { _converse.Message } message - The message object
              */
             async showMessage (message) {
+                await message.initialized;
                 const view = this.add(message.get('id'), new _converse.MessageView({'model': message}));
                 await view.render();
                 // Clear chat state notifications

src/converse-profile.js

@@ -310,7 +310,7 @@ converse.plugins.add('converse-profile', {
         /******************** Event Handlers ********************/
 
         _converse.api.listen.on('controlBoxPaneInitialized', async view => {
-            await _converse.api.waitUntil('statusInitialized');
+            await _converse.api.waitUntil('VCardsInitialized');
             _converse.xmppstatusview = new _converse.XMPPStatusView({'model': _converse.xmppstatus});
             view.el.insertAdjacentElement('afterBegin', _converse.xmppstatusview.render().el);
         });

src/converse-rosterview.js

@@ -336,7 +336,8 @@ converse.plugins.add('converse-rosterview', {
                 "click .remove-xmpp-contact": "removeContact"
             },
 
-            initialize () {
+            async initialize () {
+                await this.model.initialized;
                 this.listenTo(this.model, "change", this.render);
                 this.listenTo(this.model, "highlight", this.highlight);
                 this.listenTo(this.model, "destroy", this.remove);
@@ -345,6 +346,7 @@ converse.plugins.add('converse-rosterview', {
 
                 this.listenTo(this.model.presence, "change:show", this.render);
                 this.listenTo(this.model.vcard, 'change:fullname', this.render);
+                this.render();
             },
 
             render () {
@@ -978,7 +980,7 @@ converse.plugins.add('converse-rosterview', {
         });
 
 
-        function initRoster () {
+        function initRosterView () {
             /* Create an instance of RosterView once the RosterGroups
              * collection has been created (in @converse/headless/converse-core.js)
              */
@@ -996,8 +998,8 @@ converse.plugins.add('converse-rosterview', {
              */
             _converse.api.trigger('rosterViewInitialized');
         }
-        _converse.api.listen.on('rosterInitialized', initRoster);
-        _converse.api.listen.on('rosterReadyAfterReconnection', initRoster);
+        _converse.api.listen.on('rosterInitialized', initRosterView);
+        _converse.api.listen.on('rosterReadyAfterReconnection', initRosterView);
 
         _converse.api.listen.on('afterTearDown', () => {
             if (converse.rosterview) {

src/headless/converse-chatboxes.js

@@ -123,11 +123,11 @@ converse.plugins.add('converse-chatboxes', {
                 };
             },
 
-            initialize () {
+            async initialize () {
+                this.initialized = u.getResolveablePromise();
                 ModelWithContact.prototype.initialize.apply(this, arguments);
 
                 if (this.get('type') === 'chat') {
-                    this.setVCard();
                     this.setRosterContact(Strophe.getBareJidFromJid(this.get('from')));
                 }
 
@@ -137,6 +137,8 @@ converse.plugins.add('converse-chatboxes', {
                 if (this.isEphemeral()) {
                     window.setTimeout(this.safeDestroy.bind(this), 10000);
                 }
+                await _converse.api.trigger('messageInitialized', this, {'Synchronous': true});
+                this.initialized.resolve();
             },
 
             safeDestroy () {
@@ -147,19 +149,6 @@ converse.plugins.add('converse-chatboxes', {
                 }
             },
 
-            setVCard () {
-                if (!_converse.vcards) {
-                    // VCards aren't supported
-                    return;
-                }
-                if (this.get('type') === 'error') {
-                    return;
-                } else {
-                    const jid = Strophe.getBareJidFromJid(this.get('from'));
-                    this.vcard = _converse.vcards.findWhere({'jid': jid}) || _converse.vcards.create({'jid': jid});
-                }
-            },
-
             isOnlyChatStateNotification () {
                 return u.isOnlyChatStateNotification(this);
             },

src/headless/converse-muc.js

@@ -340,10 +340,10 @@ converse.plugins.add('converse-muc', {
                 }
             },
 
-            setVCard () {
+            async setVCard () {
+                await _converse.api.waitUntil('VCardsInitialized');
                 if (!_converse.vcards) {
-                    // VCards aren't supported
-                    return;
+                    return; // VCards aren't supported
                 }
                 if (['error', 'info'].includes(this.get('type'))) {
                     return;
@@ -403,10 +403,7 @@ converse.plugins.add('converse-muc', {
             },
 
             async initialize() {
-                if (_converse.vcards) {
-                    this.vcard = _converse.vcards.findWhere({'jid': this.get('jid')}) ||
-                        _converse.vcards.create({'jid': this.get('jid')});
-                }
+                this.setVCard();
                 this.set('box_id', `box-${btoa(this.get('jid'))}`);
 
                 this.initFeatures(); // sendChatState depends on this.features
@@ -421,6 +418,14 @@ converse.plugins.add('converse-muc', {
                 this.enterRoom();
             },
 
+            async setVCard () {
+                await _converse.api.waitUntil('VCardsInitialized');
+                if (_converse.vcards) {
+                    this.vcard = _converse.vcards.findWhere({'jid': this.get('jid')}) ||
+                        _converse.vcards.create({'jid': this.get('jid')});
+                }
+            },
+
             async enterRoom () {
                 const conn_status = this.get('connection_status');
                 _converse.log(

src/headless/converse-roster.js

@@ -74,39 +74,6 @@ converse.plugins.add('converse-roster', {
         };
 
 
-        /**
-         * Initialize the Bakcbone collections that represent the contats
-         * roster and the roster groups.
-         * @private
-         * @method _converse.initRoster
-         */
-        _converse.initRoster = function () {
-            const storage = _converse.config.get('storage');
-            _converse.roster = new _converse.RosterContacts();
-            let id = `converse.contacts-${_converse.bare_jid}`;
-            _converse.roster.browserStorage = _converse.createStore(id, storage);
-
-            _converse.roster.data = new Backbone.Model();
-            id = `converse-roster-model-${_converse.bare_jid}`;
-            _converse.roster.data.id = id;
-            _converse.roster.data.browserStorage = _converse.createStore(id, storage);
-            _converse.roster.data.fetch();
-
-            id = `converse.roster.groups${_converse.bare_jid}`;
-            _converse.rostergroups = new _converse.RosterGroups();
-            _converse.rostergroups.browserStorage = _converse.createStore(id, storage);
-            /**
-             * Triggered once the `_converse.RosterContacts` and `_converse.RosterGroups` have
-             * been created, but not yet populated with data.
-             * This event is useful when you want to create views for these collections.
-             * @event _converse#chatBoxMaximized
-             * @example _converse.api.listen.on('rosterInitialized', () => { ... });
-             * @example _converse.api.waitUntil('rosterInitialized').then(() => { ... });
-             */
-            _converse.api.trigger('rosterInitialized');
-        };
-
-
         _converse.sendInitialPresence = function () {
             if (_converse.send_initial_presence) {
                 _converse.xmppstatus.sendPresence();
@@ -243,6 +210,7 @@ converse.plugins.add('converse-roster', {
             },
 
             async initialize (attributes) {
+                this.initialized = u.getResolveablePromise();
                 this.setPresence();
                 const { jid } = attributes;
                 const bare_jid = Strophe.getBareJidFromJid(jid).toLowerCase();
@@ -268,6 +236,7 @@ converse.plugins.add('converse-roster', {
                  * @param { _converse.RosterContact } contact
                  */
                 await _converse.api.trigger('rosterContactInitialized', this, {'Synchronous': true});
+                this.initialized.resolve();
             },
 
             setPresence () {
@@ -995,7 +964,36 @@ converse.plugins.add('converse-roster', {
         });
 
 
-        _converse.api.listen.on('presencesInitialized', (reconnecting) => {
+        async function initRoster () {
+            // Initialize the Bakcbone collections that represent the contats
+            // roster and the roster groups.
+            await _converse.api.waitUntil('VCardsInitialized');
+            const storage = _converse.config.get('storage');
+            _converse.roster = new _converse.RosterContacts();
+            let id = `converse.contacts-${_converse.bare_jid}`;
+            _converse.roster.browserStorage = _converse.createStore(id, storage);
+
+            _converse.roster.data = new Backbone.Model();
+            id = `converse-roster-model-${_converse.bare_jid}`;
+            _converse.roster.data.id = id;
+            _converse.roster.data.browserStorage = _converse.createStore(id, storage);
+            _converse.roster.data.fetch();
+
+            id = `converse.roster.groups${_converse.bare_jid}`;
+            _converse.rostergroups = new _converse.RosterGroups();
+            _converse.rostergroups.browserStorage = _converse.createStore(id, storage);
+            /**
+             * Triggered once the `_converse.RosterContacts` and `_converse.RosterGroups` have
+             * been created, but not yet populated with data.
+             * This event is useful when you want to create views for these collections.
+             * @event _converse#chatBoxMaximized
+             * @example _converse.api.listen.on('rosterInitialized', () => { ... });
+             * @example _converse.api.waitUntil('rosterInitialized').then(() => { ... });
+             */
+            _converse.api.trigger('rosterInitialized');
+        }
+
+        _converse.api.listen.on('presencesInitialized', async (reconnecting) => {
             if (reconnecting) {
                 /**
                  * Similar to `rosterInitialized`, but instead pertaining to reconnection.
@@ -1006,7 +1004,7 @@ converse.plugins.add('converse-roster', {
                  */
                 _converse.api.trigger('rosterReadyAfterReconnection');
             } else {
-                _converse.initRoster();
+                await initRoster();
             }
             _converse.roster.onConnected();
             _converse.registerPresenceHandler();

src/headless/converse-vcard.js

@@ -65,6 +65,8 @@ converse.plugins.add('converse-vcard', {
          */
         const { _converse } = this;
 
+        _converse.api.promises.add('VCardsInitialized');
+
 
         _converse.VCard = Backbone.Model.extend({
             defaults: {
@@ -101,7 +103,9 @@ converse.plugins.add('converse-vcard', {
             model: _converse.VCard,
 
             initialize () {
-                this.on('add', vcard => _converse.api.vcard.update(vcard));
+                this.on('add', vcard => {
+                    _converse.api.vcard.update(vcard);
+                });
             }
         });
 
@@ -157,26 +161,36 @@ converse.plugins.add('converse-vcard', {
         }
 
         /************************ BEGIN Event Handlers ************************/
-        _converse.initVCardCollection = function () {
+        _converse.initVCardCollection = async function () {
             _converse.vcards = new _converse.VCards();
-            const id = `${_converse.bare_jid}-converse.vcards`;
-            _converse.vcards.browserStorage = _converse.createStore(id, _converse.config.get('storage'));
-            _converse.vcards.fetch();
-        }
-
-        _converse.api.listen.on('statusInitialized', () => {
-            _converse.initVCardCollection();
+            _converse.vcards.browserStorage = _converse.createStore(`${_converse.bare_jid}-converse.vcards`);
+            await new Promise(resolve => {
+                _converse.vcards.fetch({
+                    'success': resolve,
+                    'error': resolve
+                }, {'silent': true});
+            });
             const vcards = _converse.vcards;
             if (_converse.session) {
                 const jid = _converse.session.get('bare_jid');
                 _converse.xmppstatus.vcard = vcards.findWhere({'jid': jid}) || vcards.create({'jid': jid});
             }
-        });
+            /**
+             * Triggered as soon as the `_converse.vcards` collection has been initialized and populated from cache.
+             * @event _converse#VCardsInitialized
+             */
+            _converse.api.trigger('VCardsInitialized');
+        }
+
+        _converse.api.listen.on('statusInitialized', _converse.initVCardCollection);
 
         _converse.api.listen.on('clearSession', () => {
-            if (_converse.shouldClearCache() && _converse.vcards) {
-                _converse.vcards.clearSession();
-                delete _converse.vcards;
+            if (_converse.shouldClearCache()) {
+                _converse.api.promises.add('VCardsInitialized');
+                if (_converse.vcards) {
+                    _converse.vcards.clearSession();
+                    delete _converse.vcards;
+                }
             }
         });
 
@@ -184,16 +198,24 @@ converse.plugins.add('converse-vcard', {
         _converse.api.listen.on('addClientFeatures', () => _converse.api.disco.own.features.add(Strophe.NS.VCARD));
 
 
-        function setVCardOnModel (model) {
-            // TODO: if we can make this method async and wait for the VCard to
-            // be updated, then we'll avoid unnecessary re-rendering of roster contacts.
-            const jid = model.get('jid');
+        async function setVCardOnModel (model) {
+            let jid;
+            if (model instanceof _converse.Message) {
+                if (model.get('type') === 'error') {
+                    return;
+                }
+                jid = model.get('from');
+            } else {
+                jid = model.get('jid');
+            }
+            await _converse.api.waitUntil('VCardsInitialized');
             model.vcard = _converse.vcards.findWhere({'jid': jid});
             if (!model.vcard) {
                 model.vcard = _converse.vcards.create({'jid': jid});
             }
         }
-        _converse.api.listen.on('rosterContactInitialized', contact => setVCardOnModel(contact));
+        _converse.api.listen.on('rosterContactInitialized', m => setVCardOnModel(m));
+        _converse.api.listen.on('messageInitialized', m => setVCardOnModel(m));
 
 
         /************************ BEGIN API ************************/
@@ -286,9 +308,9 @@ converse.plugins.add('converse-vcard', {
                  * });
                  */
                 async update (model, force) {
-                    const vcard = await this.get(model, force);
-                    delete vcard['stanza']
-                    model.save(vcard);
+                    const data = await this.get(model, force);
+                    delete data['stanza']
+                    model.save(data);
                 }
             }
         });