Wait until VCards restored from cache to avoid unnecessary fetches

Also fix a bug in the logic to determine whether a vcard was fetched recently.

src/headless/plugins/vcard/api.js

@@ -53,6 +53,8 @@ export default {
          * }).
          */
         async set (jid, data) {
+            api.waitUntil('VCardsInitialized');
+
             if (!jid) {
                 throw Error("No jid provided for the VCard data");
             }
@@ -101,35 +103,40 @@ export default {
          * });
          */
         async get(model, force) {
+            api.waitUntil("VCardsInitialized");
+
             if (typeof model === "string") return fetchVCard(model);
 
-            // For a VCard fetch that returned an error, we
-            // check how long ago it was fetched. If it was longer ago than
-            // the last 7 days plus some jitter (to prevent an IQ fetch flood),
-            // we try again.
-            const { random, round } = Math;
-            const subtract_flag = round(random());
             const error_date = model.get("vcard_error");
-            const tried_recently =
-                error_date &&
-                dayjs(error_date).isBefore(
-                    dayjs().subtract(7, "days")
-                    .subtract(round(random() * 24)*subtract_flag, "hours")
-                    .add(round(random() * 24)*(!subtract_flag ? 1 : 0), "hours")
-                );
-            if (tried_recently) return null;
+            if (error_date) {
+                // For a VCard fetch that returned an error, we check how long ago
+                // it was fetched. If it was longer ago than the last 21 days plus
+                // some jitter (to prevent an IQ fetch flood), we try again.
+                const { random, round } = Math;
+                const subtract_flag = round(random());
+                const recent_date = dayjs()
+                    .subtract(21, "days")
+                    .subtract(round(random() * 24) * subtract_flag, "hours")
+                    .add(round(random() * 24) * (!subtract_flag ? 1 : 0), "hours");
+
+                const tried_recently = dayjs(error_date).isAfter(recent_date);
+                if (!force && tried_recently) return null;
+            }
 
-            // For a successful VCard fetch, we check how long ago it was fetched.
-            // If it was longer ago than the last 7 days plus some jitter
-            // (to prevent an IQ fetch flood), we try again.
             const vcard_updated = model.get("vcard_updated");
-            const updated_recently =
-                dayjs(vcard_updated).isBefore(
-                    dayjs().subtract(7, "days")
-                    .subtract(round(random() * 24)*subtract_flag, "hours")
-                    .add(round(random() * 24)*(!subtract_flag ? 1 : 0), "hours")
-                );
-            if (!force && updated_recently) return null;
+            if (vcard_updated) {
+                // For a successful VCard fetch, we check how long ago it was fetched.
+                // If it was longer ago than the last 7 days plus some jitter
+                // (to prevent an IQ fetch flood), we try again.
+                const { random, round } = Math;
+                const subtract_flag = round(random());
+                const recent_date = dayjs()
+                    .subtract(7, "days")
+                    .subtract(round(random() * 24) * subtract_flag, "hours")
+                    .add(round(random() * 24) * (!subtract_flag ? 1 : 0), "hours");
+                const updated_recently = dayjs(vcard_updated).isAfter(recent_date);
+                if (!force && updated_recently) return null;
+            }
 
             const jid = model.get("jid");
             if (!jid) {
@@ -157,7 +164,12 @@ export default {
          * });
          */
         async update (model, force) {
+            api.waitUntil('VCardsInitialized');
             const data = await this.get(model, force);
+            if (data === null) {
+                log.debug('api.vcard.update: null data returned, not updating the vcard');
+                return;
+            }
             model = typeof model === 'string' ? _converse.state.vcards.get(model) : model;
             if (!model) {
                 log.error(`Could not find a VCard model for ${model}`);
@@ -165,7 +177,7 @@ export default {
             }
             if (Object.keys(data).length) {
                 delete data['stanza']
-                model.save(data);
+                u.safeSave(model, data);
             }
         }
     }

src/headless/plugins/vcard/plugin.js

@@ -3,51 +3,55 @@
  * @license Mozilla Public License (MPLv2)
  */
 import "../status/index.js";
-import VCard from './vcard.js';
-import _converse from '../../shared/_converse.js';
-import api from '../../shared/api/index.js';
-import converse from '../../shared/api/public.js';
-import vcard_api from './api.js';
+import VCard from "./vcard.js";
+import _converse from "../../shared/_converse.js";
+import api from "../../shared/api/index.js";
+import converse from "../../shared/api/public.js";
+import vcard_api from "./api.js";
 import VCards from "./vcards";
-import {
-    clearVCardsSession,
-    onOccupantAvatarChanged,
-} from './utils.js';
+import { clearVCardsSession, onOccupantAvatarChanged } from "./utils.js";
 
 const { Strophe } = converse.env;
 
-
-converse.plugins.add('converse-vcard', {
-
+converse.plugins.add("converse-vcard", {
     dependencies: ["converse-status", "converse-roster"],
 
-    enabled () {
-        return !api.settings.get('blacklisted_plugins')?.includes('converse-vcard');
+    enabled() {
+        return !api.settings.get("blacklisted_plugins")?.includes("converse-vcard");
     },
 
-    initialize () {
+    initialize() {
         api.settings.extend({
             lazy_load_vcards: true,
         });
 
-        api.promises.add('VCardsInitialized');
+        api.promises.add("VCardsInitialized");
+
+        Object.assign(_converse.api, vcard_api);
 
         const exports = { VCard, VCards };
         Object.assign(_converse, exports); // XXX DEPRECATED
         Object.assign(_converse.exports, exports);
 
-        api.listen.on('chatRoomInitialized', (m) => {
-            m.listenTo(m.occupants, 'change:image_hash', (o) => onOccupantAvatarChanged(o));
-        });
+        api.listen.on(
+            "chatRoomInitialized",
+            /** @param {import('../muc/muc').default} m */ (m) => {
+                m.listenTo(m.occupants, "change:image_hash", (o) => onOccupantAvatarChanged(o));
+            }
+        );
 
-        api.listen.on('addClientFeatures', () => api.disco.own.features.add(Strophe.NS.VCARD));
-        api.listen.on('clearSession', () => clearVCardsSession());
+        api.listen.on("addClientFeatures", () => api.disco.own.features.add(Strophe.NS.VCARD));
+        api.listen.on("clearSession", () => clearVCardsSession());
 
-        api.listen.on('visibilityChanged', ({ el }) => {
+        api.listen.on("visibilityChanged", ({ el }) => {
             const { model } = el;
-            if (model?.vcard) model.vcard.trigger('visibilityChanged');
+            if (model?.vcard) model.vcard.trigger("visibilityChanged");
         });
 
-        Object.assign(_converse.api, vcard_api);
-    }
+        api.listen.on("connected", () => {
+            const vcards = new _converse.exports.VCards();
+            _converse.state.vcards = vcards;
+            Object.assign(_converse, { vcards }); // XXX DEPRECATED
+        });
+    },
 });

src/headless/plugins/vcard/utils.js

@@ -11,7 +11,6 @@ import _converse from "../../shared/_converse.js";
 import api from "../../shared/api/index.js";
 import converse from "../../shared/api/public.js";
 import log from "../../log.js";
-import { initStorage } from "../../utils/storage.js";
 import { shouldClearCache } from "../../utils/session.js";
 import { isElement } from "../../utils/html.js";
 import { parseErrorStanza } from "../../shared/parsers.js";
@@ -76,7 +75,7 @@ export function onOccupantAvatarChanged(occupant) {
  * @returns {Promise<VCard|null>}
  */
 export async function getVCardForModel(model, lazy_load = false) {
-    await initVCardCollection();
+    await api.waitUntil('VCardsInitialized');
 
     let vcard;
     if (model instanceof _converse.exports.MUCOccupant) {
@@ -159,32 +158,6 @@ async function getVCardForMUCMessage(message, lazy_load = true) {
     }
 }
 
-async function initVCardCollection() {
-    if (_converse.state.vcards) return _converse.state.vcards;
-
-    const vcards = new _converse.exports.VCards();
-    _converse.state.vcards = vcards;
-    Object.assign(_converse, { vcards }); // XXX DEPRECATED
-
-    const bare_jid = _converse.session.get("bare_jid");
-    const id = `${bare_jid}-converse.vcards`;
-    initStorage(vcards, id);
-    await new Promise((resolve) => {
-        vcards.fetch(
-            {
-                success: resolve,
-                error: resolve,
-            },
-            { silent: true }
-        );
-    });
-    /**
-     * Triggered as soon as the `_converse.vcards` collection has been initialized and populated from cache.
-     * @event _converse#VCardsInitialized
-     */
-    api.trigger("VCardsInitialized");
-}
-
 export function clearVCardsSession() {
     if (shouldClearCache(_converse)) {
         api.promises.add("VCardsInitialized");

src/headless/plugins/vcard/vcards.js

@@ -1,12 +1,42 @@
-import VCard from "./vcard";
 import { Collection } from "@converse/skeletor";
+import { getOpenPromise } from "@converse/openpromise";
+import api from "../../shared/api/index.js";
+import _converse from "../../shared/_converse.js";
+import { initStorage } from "../../utils/storage.js";
+import VCard from "./vcard";
 
 class VCards extends Collection {
-
-    constructor () {
+    constructor() {
         super();
         this.model = VCard;
     }
+
+    async initialize() {
+        const { session } = _converse;
+        const bare_jid = session.get("bare_jid");
+        const cache_key = `${bare_jid}-converse.vcards`;
+        initStorage(this, cache_key);
+
+        await this.fetchVCards();
+
+        /**
+         * Triggered as soon as the `_converse.state.vcards` collection has
+         * been initialized and populated from cache.
+         * @event _converse#VCardsInitialized
+         */
+        api.trigger("VCardsInitialized");
+    }
+
+    fetchVCards() {
+        const deferred = getOpenPromise();
+        this.fetch(
+            {
+                success: () => deferred.resolve(),
+                error: () => deferred.resolve(),
+            },
+        );
+        return deferred;
+    }
 }
 
 export default VCards;

src/headless/types/plugins/vcard/vcards.d.ts

@@ -2,6 +2,8 @@ export default VCards;
 declare class VCards extends Collection {
     constructor();
     model: typeof VCard;
+    initialize(): Promise<void>;
+    fetchVCards(): any;
 }
 import { Collection } from "@converse/skeletor";
 import VCard from "./vcard";