Set `jid` as the id attribute of the VCards collection

This reduces lookup time for vcards greatly, since we don't can access a
map of ids instead of using `findWhere` through an array of models.

CHANGES.md

@@ -5,6 +5,7 @@
 - GIFs don't render inside unfurls and cause a TypeError
 - Improve how the `muc_domain` setting is populated via service discovery
 - Remove local (non-requesting) contacts not returned from a full roster response
+- Improve performance by looking up VCards via map instead of traversing an array
 - #2746: Always reply to all iqs, even those not understood
 - #2794: Some display problems with mobile view mode
 - #2868: Selected emoji is inserted into all open chat boxes

src/headless/plugins/muc/message.js

@@ -23,7 +23,7 @@ const ChatRoomMessageMixin = {
         this.setTimerForEphemeralMessage();
         this.setOccupant();
         /**
-         * Triggered once a {@link _converse.ChatRoomMessageInitialized} has been created and initialized.
+         * Triggered once a { @link _converse.ChatRoomMessage } has been created and initialized.
          * @event _converse#chatRoomMessageInitialized
          * @type { _converse.ChatRoomMessages}
          * @example _converse.api.listen.on('chatRoomMessageInitialized', model => { ... });

src/headless/plugins/muc/occupant.js

@@ -15,28 +15,6 @@ const ChatRoomOccupant = Model.extend({
         'states': []
     },
 
-    initialize (attributes) {
-        this.set(Object.assign({ 'id': u.getUniqueId() }, attributes));
-        this.on('change:image_hash', this.onAvatarChanged, this);
-    },
-
-    onAvatarChanged () {
-        const hash = this.get('image_hash');
-        const vcards = [];
-        if (this.get('jid')) {
-            vcards.push(_converse.vcards.findWhere({ 'jid': this.get('jid') }));
-        }
-        vcards.push(_converse.vcards.findWhere({ 'jid': this.get('from') }));
-
-        vcards
-            .filter(v => v)
-            .forEach(vcard => {
-                if (hash && vcard.get('image_hash') !== hash) {
-                    api.vcard.update(vcard, true);
-                }
-            });
-    },
-
     getDisplayName () {
         return this.get('nick') || this.get('jid');
     },

src/headless/plugins/vcard/api.js

@@ -68,7 +68,7 @@ export default {
          *     If a `Model` instance is passed in, then it must have either a `jid`
          *     attribute or a `muc_jid` attribute.
          * @param {boolean} [force] A boolean indicating whether the vcard should be
-         *     fetched even if it's been fetched before.
+         *     fetched from the server even if it's been fetched before.
          * @returns {promise} A Promise which resolves with the VCard data for a particular JID or for
          *     a `Model` instance which represents an entity with a JID (such as a roster contact,
          *     chat or chatroom occupant).
@@ -119,13 +119,15 @@ export default {
          */
         async update (model, force) {
             const data = await this.get(model, force);
-            model = typeof model === 'string' ? _converse.vcards.findWhere({'jid': model}) : model;
+            model = typeof model === 'string' ? _converse.vcards.get(model) : model;
             if (!model) {
                 log.error(`Could not find a VCard model for ${model}`);
                 return;
             }
-            delete data['stanza']
-            model.save(data);
+            if (Object.keys(data).length) {
+                delete data['stanza']
+                model.save(data);
+            }
         }
     }
 }

src/headless/plugins/vcard/index.js

@@ -10,6 +10,7 @@ import { _converse, api, converse } from "../../core.js";
 import {
     clearVCardsSession,
     initVCardCollection,
+    onOccupantAvatarChanged,
     setVCardOnMUCMessage,
     setVCardOnModel,
     setVCardOnOccupant,
@@ -74,13 +75,14 @@ converse.plugins.add('converse-vcard', {
         _converse.VCards = Collection.extend({
             model: _converse.VCard,
             initialize () {
-                this.on('add', vcard => (vcard.get('jid') && api.vcard.update(vcard)));
+                this.on('add', v => v.get('jid') && api.vcard.update(v));
             }
         });
 
         api.listen.on('chatRoomInitialized', m => {
             setVCardOnModel(m)
             m.occupants.forEach(setVCardOnOccupant);
+            m.occupants.on('change:image_hash', o => onOccupantAvatarChanged(o));
             m.listenTo(m.occupants, 'add', setVCardOnOccupant);
         });
         api.listen.on('chatBoxInitialized', m => setVCardOnModel(m));

src/headless/plugins/vcard/utils.js

@@ -42,38 +42,52 @@ export function createStanza (type, jid, vcard_el) {
 }
 
 
+export function onOccupantAvatarChanged (occupant) {
+    const hash = occupant.get('image_hash');
+    const vcards = [];
+    if (occupant.get('jid')) {
+        vcards.push(_converse.vcards.get(occupant.get('jid')));
+    }
+    vcards.push(_converse.vcards.get(occupant.get('from')));
+    vcards.forEach(v => (hash && v?.get('image_hash') !== hash) && api.vcard.update(v, true));
+}
+
+
 export async function setVCardOnModel (model) {
     let jid;
     if (model instanceof _converse.Message) {
-        if (model.get('type') === 'error') {
+        if (['error', 'info'].includes(model.get('type'))) {
             return;
         }
         jid = model.get('from');
     } else {
         jid = model.get('jid');
     }
-    await api.waitUntil('VCardsInitialized');
-    model.vcard = _converse.vcards.findWhere({'jid': jid});
-    if (!model.vcard) {
-        model.vcard = _converse.vcards.create({'jid': jid});
+
+    if (!jid) {
+        log.warn(`Could not set VCard on model because no JID found!`);
+        return;
     }
+
+    await api.waitUntil('VCardsInitialized');
+    model.vcard = _converse.vcards.get(jid) || _converse.vcards.create({ jid });
     model.vcard.on('change', () => model.trigger('vcard:change'));
     model.trigger('vcard:add');
 }
 
 
-function getVCardForChatroomOccupant (message) {
-    const chatbox = message?.collection?.chatbox;
-    const nick = Strophe.getResourceFromJid(message.get('from'));
+function getVCardForOccupant (occupant) {
+    const muc = occupant?.collection?.chatroom;
+    const nick = occupant.get('nick');
 
-    if (chatbox && chatbox.get('nick') === nick) {
+    if (nick && muc?.get('nick') === nick) {
         return _converse.xmppstatus.vcard;
     } else {
-        const jid = message.occupant && message.occupant.get('jid') || message.get('from');
+        const jid = occupant.get('jid') || occupant.get('from');
         if (jid) {
-            return _converse.vcards.findWhere({jid}) || _converse.vcards.create({jid});
+            return _converse.vcards.get(jid) || _converse.vcards.create({ jid });
         } else {
-            log.error(`Could not assign VCard for message because no JID found! msgid: ${message.get('msgid')}`);
+            log.warn(`Could not get VCard for occupant because no JID found!`);
             return;
         }
     }
@@ -81,19 +95,37 @@ function getVCardForChatroomOccupant (message) {
 
 export async function setVCardOnOccupant (occupant) {
     await api.waitUntil('VCardsInitialized');
-    occupant.vcard = getVCardForChatroomOccupant(occupant);
+    occupant.vcard = getVCardForOccupant(occupant);
     if (occupant.vcard) {
         occupant.vcard.on('change', () => occupant.trigger('vcard:change'));
         occupant.trigger('vcard:add');
     }
 }
 
+
+function getVCardForMUCMessage (message) {
+    const muc = message?.collection?.chatbox;
+    const nick = Strophe.getResourceFromJid(message.get('from'));
+
+    if (nick && muc?.get('nick') === nick) {
+        return _converse.xmppstatus.vcard;
+    } else {
+        const jid = message.occupant?.get('jid') || message.get('from');
+        if (jid) {
+            return _converse.vcards.get(jid) || _converse.vcards.create({ jid });
+        } else {
+            log.warn(`Could not get VCard for message because no JID found! msgid: ${message.get('msgid')}`);
+            return;
+        }
+    }
+}
+
 export async function setVCardOnMUCMessage (message) {
     if (['error', 'info'].includes(message.get('type'))) {
         return;
     } else {
         await api.waitUntil('VCardsInitialized');
-        message.vcard = getVCardForChatroomOccupant(message);
+        message.vcard = getVCardForMUCMessage(message);
         if (message.vcard) {
             message.vcard.on('change', () => message.trigger('vcard:change'));
             message.trigger('vcard:add');
@@ -116,16 +148,16 @@ export async function initVCardCollection () {
     if (_converse.session) {
         const jid = _converse.session.get('bare_jid');
         const status = _converse.xmppstatus;
-        status.vcard = vcards.findWhere({'jid': jid}) || vcards.create({'jid': jid});
+        status.vcard = vcards.get(jid) || vcards.create({'jid': jid});
         if (status.vcard) {
             status.vcard.on('change', () => status.trigger('vcard:change'));
             status.trigger('vcard:add');
         }
     }
     /**
-        * Triggered as soon as the `_converse.vcards` collection has been initialized and populated from cache.
-        * @event _converse#VCardsInitialized
-        */
+     * Triggered as soon as the `_converse.vcards` collection has been initialized and populated from cache.
+     * @event _converse#VCardsInitialized
+     */
     api.trigger('VCardsInitialized');
 }
 

src/headless/plugins/vcard/vcard.js

@@ -1,40 +1,42 @@
 import { Model } from '@converse/skeletor/src/model.js';
 import { _converse } from "../../core.js";
 
- /**
-  * Represents a VCard
-  * @class
-  * @namespace _converse.VCard
-  * @memberOf _converse
-  */
- const VCard = Model.extend({
-     defaults: {
-         'image': _converse.DEFAULT_IMAGE,
-         'image_type': _converse.DEFAULT_IMAGE_TYPE
-     },
+/**
+ * Represents a VCard
+ * @class
+ * @namespace _converse.VCard
+ * @memberOf _converse
+ */
+const VCard = Model.extend({
+    idAttribute: 'jid',
 
-     set (key, val, options) {
-         // Override Model.prototype.set to make sure that the
-         // default `image` and `image_type` values are maintained.
-         let attrs;
-         if (typeof key === 'object') {
-             attrs = key;
-             options = val;
-         } else {
-             (attrs = {})[key] = val;
-         }
-         if ('image' in attrs && !attrs['image']) {
-             attrs['image'] = _converse.DEFAULT_IMAGE;
-             attrs['image_type'] = _converse.DEFAULT_IMAGE_TYPE;
-             return Model.prototype.set.call(this, attrs, options);
-         } else {
-             return Model.prototype.set.apply(this, arguments);
-         }
-     },
+    defaults: {
+        'image': _converse.DEFAULT_IMAGE,
+        'image_type': _converse.DEFAULT_IMAGE_TYPE
+    },
 
-     getDisplayName () {
-         return this.get('nickname') || this.get('fullname') || this.get('jid');
-     }
- });
+    set (key, val, options) {
+        // Override Model.prototype.set to make sure that the
+        // default `image` and `image_type` values are maintained.
+        let attrs;
+        if (typeof key === 'object') {
+            attrs = key;
+            options = val;
+        } else {
+            (attrs = {})[key] = val;
+        }
+        if ('image' in attrs && !attrs['image']) {
+            attrs['image'] = _converse.DEFAULT_IMAGE;
+            attrs['image_type'] = _converse.DEFAULT_IMAGE_TYPE;
+            return Model.prototype.set.call(this, attrs, options);
+        } else {
+            return Model.prototype.set.apply(this, arguments);
+        }
+    },
+
+    getDisplayName () {
+        return this.get('nickname') || this.get('fullname') || this.get('jid');
+    }
+});
 
 export default VCard;

src/plugins/muc-views/modals/occupant.js

@@ -23,7 +23,7 @@ const OccupantModal = BaseModal.extend({
     toHTML () {
         const model = this.model ?? this.message;
         const jid = model?.get('jid');
-        const vcard = _converse.vcards.findWhere({ jid });
+        const vcard = _converse.vcards.get(jid);
         const display_name = model?.getDisplayName();
         const nick = model.get('nick');
         const occupant_id = model.get('occupant_id');