[FIX] mail: fix uncessary render in message (potential initie loop)

Overwriting array with new array triggers reactive. This leads to
uncessary renders, and potential infitine loop if both read and insert
methods are used inside rendering cycle.

Part-of: odoo/odoo#121418

addons/mail/static/src/attachments/attachment_service.js

@@ -17,13 +17,12 @@ export class AttachmentService {
         if (!("id" in data)) {
             throw new Error("Cannot insert attachment: id is missing in data");
         }
-        if (data.id in this.store.attachments) {
-            const attachment = this.store.attachments[data.id];
-            this.update(attachment, data);
-            return attachment;
+        let attachment = this.store.attachments[data.id];
+        if (!attachment) {
+            this.store.attachments[data.id] = new Attachment();
+            attachment = this.store.attachments[data.id];
+            Object.assign(attachment, { _store: this.store, id: data.id });
         }
-        const attachment = (this.store.attachments[data.id] = new Attachment());
-        Object.assign(attachment, { _store: this.store, id: data.id });
         this.update(attachment, data);
         return attachment;
     }
@@ -55,9 +54,8 @@ export class AttachmentService {
                 id: threadData.id,
             });
             attachment.originThreadLocalId = createLocalId(threadData.model, threadData.id);
-            const originThread = this.store.threads[attachment.originThreadLocalId];
-            if (!originThread.attachments.some((a) => a.id === attachment.id)) {
-                originThread.attachments.push(attachment);
+            if (!attachment.originThread.attachments.includes(attachment)) {
+                attachment.originThread.attachments.push(attachment);
             }
         }
     }

addons/mail/static/src/core/message_model.js

@@ -41,7 +41,7 @@ export class Message {
     parentMessage;
     /** @type {MessageReactions[]} */
     reactions = [];
-    /** @type {Notification[]} */
+    /** @type {import("@mail/core/notification_model").Notification[]} */
     notifications = [];
     /** @type {import("@mail/core/persona_model").Persona[]} */
     recipients = [];

addons/mail/static/src/core/message_service.js

@@ -1,7 +1,7 @@
 /** @odoo-module */
 
 import { Message } from "./message_model";
-import { removeFromArrayWithPredicate } from "../utils/arrays";
+import { removeFromArrayWithPredicate, replaceArrayWithCompare } from "../utils/arrays";
 import { convertBrToLineBreak, prettifyMessageContent } from "../utils/format";
 import { _t } from "@web/core/l10n/translation";
 import { registry } from "@web/core/registry";
@@ -150,7 +150,9 @@ export class MessageService {
         const thread = message.originThread;
         await this.env.services["mail.thread"].removeFollower(thread.followerOfSelf);
         this.env.services.notification.add(
-            sprintf(_t('You are no longer following "%(thread_name)s".'), { thread_name: thread.name }),
+            sprintf(_t('You are no longer following "%(thread_name)s".'), {
+                thread_name: thread.name,
+            }),
             { type: "success" }
         );
     }
@@ -221,7 +223,6 @@ export class MessageService {
             message = this.store.messages[data.id] = message;
         }
         this.update(message, data);
-        this.updateNotifications(message);
         // return reactive version
         return message;
     }
@@ -246,15 +247,14 @@ export class MessageService {
             linkPreviews = message.linkPreviews,
             message_type: type = message.type,
             model: resModel = message.resModel,
+            notifications = message.notifications,
+            recipients = message.recipients,
             res_id: resId = message.resId,
             subtype_description: subtypeDescription = message.subtypeDescription,
             ...remainingData
         } = data;
         assignDefined(message, remainingData);
         assignDefined(message, {
-            attachments: attachments.map((attachment) =>
-                this.attachmentService.insert({ message, ...attachment })
-            ),
             defaultSubject,
             isDiscussion,
             isNote,
@@ -262,13 +262,25 @@ export class MessageService {
                 ? message.starred_partner_ids.includes(this.store.user.id)
                 : false,
             isTransient,
-            linkPreviews: linkPreviews.map((data) => new LinkPreview(data)),
             parentMessage: message.parentMessage ? this.insert(message.parentMessage) : undefined,
             resId,
             resModel,
             subtypeDescription,
             type,
         });
+        // origin thread before other information (in particular notification insert uses it)
+        if (data.record_name) {
+            message.originThread.name = data.record_name;
+        }
+        if (data.res_model_name) {
+            message.originThread.modelName = data.res_model_name;
+        }
+        replaceArrayWithCompare(
+            message.attachments,
+            attachments.map((attachment) =>
+                this.attachmentService.insert({ message, ...attachment })
+            )
+        );
         if (
             Array.isArray(message.author) &&
             message.author.some((command) => command.includes("clear"))
@@ -288,17 +300,22 @@ export class MessageService {
                 channelId: message.originThread.id,
             });
         }
-        if (data.recipients) {
-            message.recipients = data.recipients.map((recipient) =>
+        replaceArrayWithCompare(
+            message.linkPreviews,
+            linkPreviews.map((data) => this.insertLinkPreview({ ...data, message }))
+        );
+        replaceArrayWithCompare(
+            message.notifications,
+            notifications.map((notification) =>
+                this.insertNotification({ ...notification, messageId: message.id })
+            )
+        );
+        replaceArrayWithCompare(
+            message.recipients,
+            recipients.map((recipient) =>
                 this.personaService.insert({ ...recipient, type: "partner" })
-            );
-        }
-        if (data.record_name) {
-            message.originThread.name = data.record_name;
-        }
-        if (data.res_model_name) {
-            message.originThread.modelName = data.res_model_name;
-        }
+            )
+        );
         if ("user_follower_id" in data && data.user_follower_id && this.store.self) {
             this.env.services["mail.thread"].insertFollower({
                 followedThread: message.originThread,
@@ -307,7 +324,9 @@ export class MessageService {
                 partner: this.store.self,
             });
         }
-        this._updateReactions(message, data.messageReactionGroups);
+        if (data.messageReactionGroups) {
+            this._updateReactions(message, data.messageReactionGroups);
+        }
         if (message.isNotification && !message.notificationType) {
             const parser = new DOMParser();
             const htmlBody = parser.parseFromString(message.body, "text/html");
@@ -316,13 +335,7 @@ export class MessageService {
         }
     }
 
-    updateNotifications(message) {
-        message.notifications = message.notifications.map((notification) =>
-            this.insertNotification({ ...notification, messageId: message.id })
-        );
-    }
-
-    _updateReactions(message, reactionGroups = []) {
+    _updateReactions(message, reactionGroups) {
         const reactionContentToUnlink = new Set();
         const reactionsToInsert = [];
         for (const rawReaction of reactionGroups) {
@@ -349,6 +362,20 @@ export class MessageService {
         });
     }
 
+    /**
+     * @param {Object} data
+     * @returns {LinkPreview}
+     */
+    insertLinkPreview(data) {
+        const linkPreview = data.message.linkPreviews.find(
+            (linkPreview) => linkPreview.id === data.id
+        );
+        if (linkPreview) {
+            return Object.assign(linkPreview, data);
+        }
+        return new LinkPreview(data);
+    }
+
     /**
      * @param {Object} data
      * @returns {MessageReactions}
@@ -399,16 +426,13 @@ export class MessageService {
      * @returns {Notification}
      */
     insertNotification(data) {
-        let notification;
-        if (data.id in this.store.notifications) {
+        let notification = this.store.notifications[data.id];
+        if (!notification) {
+            this.store.notifications[data.id] = new Notification(this.store, data);
             notification = this.store.notifications[data.id];
-            this.updateNotification(notification, data);
-            return notification;
         }
-        notification = new Notification(this.store, data);
         this.updateNotification(notification, data);
-        // return reactive version
-        return this.store.notifications[data.id];
+        return notification;
     }
 
     updateNotification(notification, data) {

addons/mail/static/src/core/notification_model.js

@@ -23,7 +23,6 @@ export class Notification {
             id: data.id,
             _store: store,
         });
-        store.notifications[this.id] = this;
     }
 
     get message() {

addons/mail/static/src/core/thread_service.js

@@ -672,14 +672,14 @@ export class ThreadService {
      * @param {Object} data
      */
     update(thread, data) {
-        const { id, name, attachments, description, ...serverData } = data;
+        const { id, name, attachments: attachmentsData, description, ...serverData } = data;
         assignDefined(thread, { id, name, description });
-        if (attachments) {
-            // smart process to avoid triggering reactives when there is no change between the 2 arrays
+        if (attachmentsData) {
             replaceArrayWithCompare(
                 thread.attachments,
-                attachments.map((attachment) => this.attachmentsService.insert(attachment)),
-                (a1, a2) => a1.id === a2.id
+                attachmentsData.map((attachmentData) =>
+                    this.attachmentsService.insert(attachmentData)
+                )
             );
         }
         if (serverData) {

addons/mail/static/src/utils/arrays.js

@@ -1,5 +1,7 @@
 /* @odoo-module */
 
+import { toRaw } from "@odoo/owl";
+
 export function removeFromArray(array, elem) {
     const index = array.indexOf(elem);
     if (index >= 0) {
@@ -14,14 +16,27 @@ export function removeFromArrayWithPredicate(array, predicate) {
     }
 }
 
-export function replaceArrayWithCompare(array1, array2, compareFn) {
+/**
+ * Replaces the content of array1 with the content of array2. Order of elements
+ * is not guaranteed: new elements are inserted last.
+ *
+ * Smart process to avoid triggering reactives when there is no change between
+ * the 2 arrays.
+ */
+export function replaceArrayWithCompare(array1, array2) {
+    array1 = toRaw(array1);
+    array2 = toRaw(array2);
+    const elementsToRemove = new Set();
     for (const el1 of array1) {
-        if (!array2.some((el2) => compareFn(el1, el2))) {
-            removeFromArrayWithPredicate(array1, (el) => compareFn(el, el1));
+        if (!array2.includes(el1)) {
+            elementsToRemove.add(el1);
         }
     }
+    for (const el of elementsToRemove) {
+        removeFromArray(array1, el);
+    }
     for (const el2 of array2) {
-        if (!array1.some((el1) => compareFn(el1, el2))) {
+        if (!array1.includes(el2)) {
             array1.push(el2);
         }
     }