diff --git a/CHANGES.md b/CHANGES.md index 37ba58fc1..2abb089ad 100644 --- a/CHANGES.md +++ b/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 diff --git a/src/headless/plugins/muc/message.js b/src/headless/plugins/muc/message.js index 48ab13371..de9fee9a8 100644 --- a/src/headless/plugins/muc/message.js +++ b/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 => { ... }); diff --git a/src/headless/plugins/muc/occupant.js b/src/headless/plugins/muc/occupant.js index f24529aa6..d6ca958b3 100644 --- a/src/headless/plugins/muc/occupant.js +++ b/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'); }, diff --git a/src/headless/plugins/vcard/api.js b/src/headless/plugins/vcard/api.js index 77716220c..b4a9c7c77 100644 --- a/src/headless/plugins/vcard/api.js +++ b/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); + } } } } diff --git a/src/headless/plugins/vcard/index.js b/src/headless/plugins/vcard/index.js index 19b62f75f..0d7890211 100644 --- a/src/headless/plugins/vcard/index.js +++ b/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)); diff --git a/src/headless/plugins/vcard/utils.js b/src/headless/plugins/vcard/utils.js index 5ce8b2f95..a62de0645 100644 --- a/src/headless/plugins/vcard/utils.js +++ b/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'); } diff --git a/src/headless/plugins/vcard/vcard.js b/src/headless/plugins/vcard/vcard.js index 72d19d27f..3bde1d1d6 100644 --- a/src/headless/plugins/vcard/vcard.js +++ b/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; diff --git a/src/plugins/muc-views/modals/occupant.js b/src/plugins/muc-views/modals/occupant.js index db5827e81..5d5aefa61 100644 --- a/src/plugins/muc-views/modals/occupant.js +++ b/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');