Bläddra i källkod

Make sure that the `occupant_id` is also the `id` for occupants

insofar we have an `occupant_id`.

We do this by subclassing `create` on the `ChatRoomOccupants` collection
and `save` on the `ChatRoomOccupant` model, to make sure that whenever
an occupant is created or saved, that the `id` matches the `occupant_id`
value if it's available.

This lets us look up the occupant via `occupant_id` via dictionary lookup,
instead of array traversal.

Another change is to save `from_real_jid` when adding an occupant to a message
JC Brand 3 år sedan
förälder
incheckning
d22c063ae5

+ 7 - 5
src/headless/plugins/muc/message.js

@@ -81,13 +81,14 @@ const ChatRoomMessageMixin = {
         } else if (occupant.get('nick') !== Strophe.getResourceFromJid(this.get('from'))) {
             return;
         }
-        this.occupant = occupant;
-        this.trigger('occupantAdded');
-        this.listenTo(this.occupant, 'destroy', this.onOccupantRemoved);
         const chatbox = this?.collection?.chatbox;
         if (!chatbox) {
             return log.error(`Could not get collection.chatbox for message: ${JSON.stringify(this.toJSON())}`);
         }
+
+        this.occupant = occupant;
+        this.trigger('occupantAdded');
+        this.listenTo(this.occupant, 'destroy', this.onOccupantRemoved);
         this.stopListening(chatbox.occupants, 'add', this.onOccupantAdded);
     },
 
@@ -100,10 +101,11 @@ const ChatRoomMessageMixin = {
             return log.error(`Could not get collection.chatbox for message: ${JSON.stringify(this.toJSON())}`);
         }
         const nick = Strophe.getResourceFromJid(this.get('from'));
-        this.occupant = chatbox.occupants.findOccupant({ nick, 'occupant_id': this.get('occupant_id') });
+        const occupant_id = this.get('occupant_id');
+        this.occupant = chatbox.occupants.findOccupant({ nick, occupant_id });
 
         if (!this.occupant && api.settings.get('muc_send_probes')) {
-            this.occupant = chatbox.occupants.create({ nick, 'type': 'unavailable' });
+            this.occupant = chatbox.occupants.create({ nick, occupant_id, 'type': 'unavailable' });
             const jid = `${chatbox.get('jid')}/${nick}`;
             api.user.presence.send('probe', jid);
         }

+ 6 - 2
src/headless/plugins/muc/muc.js

@@ -1704,6 +1704,7 @@ const ChatRoomMixin = {
         if (data.type === 'error' || (!data.jid && !data.nick && !data.occupant_id)) {
             return true;
         }
+
         const occupant = this.occupants.findOccupant(data);
         // Destroy an unavailable occupant if this isn't a nick change operation and if they're not affiliated
         if (
@@ -1717,11 +1718,14 @@ const ChatRoomMixin = {
             occupant.destroy();
             return;
         }
+
         const jid = data.jid || '';
-        const attributes = Object.assign(data, {
+        const attributes = {
+            ...data,
             'jid': Strophe.getBareJidFromJid(jid) || occupant?.attributes?.jid,
             'resource': Strophe.getResourceFromJid(jid) || occupant?.attributes?.resource
-        });
+        }
+
         if (occupant) {
             occupant.save(attributes);
         } else {

+ 21 - 4
src/headless/plugins/muc/occupants.js

@@ -2,10 +2,12 @@ import ChatRoomOccupant from './occupant.js';
 import u from '../../utils/form';
 import { Collection } from '@converse/skeletor/src/collection.js';
 import { MUC_ROLE_WEIGHTS } from './constants.js';
+import { Model } from '@converse/skeletor/src/model.js';
 import { Strophe } from 'strophe.js/src/strophe.js';
 import { _converse, api } from '../../core.js';
 import { getAffiliationList } from './affiliations/utils.js';
 import { getAutoFetchedAffiliationLists } from './utils.js';
+import { getUniqueId } from '@converse/headless/utils/core.js';
 
 
 /**
@@ -29,6 +31,14 @@ class ChatRoomOccupants extends Collection {
         }
     }
 
+    create (attrs, options) {
+        if (attrs.id || attrs instanceof Model) {
+            return super.create(attrs, options);
+        }
+        attrs.id = attrs.occupant_id || getUniqueId();
+        return super.create(attrs, options);
+    }
+
     /**
      * Get the {@link _converse.ChatRoomOccupant} instance which
      * represents the current user.
@@ -94,16 +104,23 @@ class ChatRoomOccupants extends Collection {
      * Try to find an existing occupant based on the passed in
      * data object.
      *
-     * If we have a JID, we use that as lookup variable,
-     * otherwise we use the nick. We don't always have both,
-     * but should have at least one or the other.
+     * Fetching the user by occupant_id is the quickest, O(1),
+     * since it's a dictionary lookup.
+     *
+     * Fetching by jid or nick is O(n), since it requires traversing an array.
+     *
+     * Lookup by occupant_id is done first, then jid, and then nick.
+     *
      * @method _converse.ChatRoomOccupants#findOccupant
      * @param { OccupantData } data
      */
     findOccupant (data) {
+        if (data.occupant_id && this.get(data.occupant_id)) {
+            return this.get(data.occupant_id);
+        }
+
         const jid = data.jid && Strophe.getBareJidFromJid(data.jid);
         return jid && this.findWhere({ jid }) ||
-            data.occupant_id && this.findWhere({ 'occupant_id': data.occupant_id }) ||
             data.nick && this.findWhere({ 'nick': data.nick });
     }
 }