From 4064508af56dc3eb602cb8a00a63680f3325331f Mon Sep 17 00:00:00 2001 From: "Didier (did)" Date: Wed, 18 Oct 2023 14:11:39 +0000 Subject: [PATCH] [IMP] mail: send cache control header on avatars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR cache control header for avatar was set to no-cache which is not the right value. This PR mark the avatar routes response as immutable when a unique string is present. We also use the partner write_date as unique string when possible. task-3551636 closes odoo/odoo#141326 X-original-commit: 3de4f1f1225229cbdb0512df5790ce491f04ea7e Signed-off-by: Sébastien Theys (seb) --- addons/im_livechat/controllers/cors/binary.py | 8 ++-- .../tests/test_get_discuss_channel.py | 1 + addons/mail/controllers/discuss/binary.py | 16 +++---- addons/mail/models/discuss/mail_guest.py | 5 ++- addons/mail/models/res_partner.py | 5 ++- .../static/src/core/common/persona_model.js | 2 + .../static/src/core/common/thread_service.js | 10 ++++- .../discuss/core/common/thread_model_patch.js | 5 ++- .../static/tests/discuss_app/sidebar_tests.js | 2 +- .../tests/discuss/test_message_controller.py | 43 +++++++++++++++++++ .../tests/test_performance.py | 20 ++++++++- 11 files changed, 98 insertions(+), 19 deletions(-) diff --git a/addons/im_livechat/controllers/cors/binary.py b/addons/im_livechat/controllers/cors/binary.py index 66069acd68b..900434ea436 100644 --- a/addons/im_livechat/controllers/cors/binary.py +++ b/addons/im_livechat/controllers/cors/binary.py @@ -39,9 +39,9 @@ class LivechatBinaryController(BinaryController): auth="public", cors="*", ) - def livechat_channel_partner_avatar_128(self, guest_token, channel_id, partner_id, **kwargs): + def livechat_channel_partner_avatar_128(self, guest_token, channel_id, partner_id, unique=False): force_guest_env(guest_token) - return self.discuss_channel_partner_avatar_128(channel_id, partner_id, **kwargs) + return self.discuss_channel_partner_avatar_128(channel_id, partner_id, unique=unique) @route( "/im_livechat/cors/channel//guest//avatar_128", @@ -50,6 +50,6 @@ class LivechatBinaryController(BinaryController): auth="public", cors="*", ) - def livechat_channel_guest_avatar_128(self, guest_token, channel_id, guest_id, **kwargs): + def livechat_channel_guest_avatar_128(self, guest_token, channel_id, guest_id, unique=False): force_guest_env(guest_token) - return self.discuss_channel_guest_avatar_128(channel_id, guest_id, **kwargs) + return self.discuss_channel_guest_avatar_128(channel_id, guest_id, unique=unique) diff --git a/addons/im_livechat/tests/test_get_discuss_channel.py b/addons/im_livechat/tests/test_get_discuss_channel.py index 903adf5afc6..d5e168a1494 100644 --- a/addons/im_livechat/tests/test_get_discuss_channel.py +++ b/addons/im_livechat/tests/test_get_discuss_channel.py @@ -48,6 +48,7 @@ class TestGetDiscussChannel(TestImLivechatCommon): 'name': 'Visitor', 'im_status': 'offline', 'type': "guest", + 'write_date': fields.Datetime.to_string(self.env['discuss.channel'].browse(channel_info['id']).channel_member_ids.filtered(lambda m: m.guest_id)[0].guest_id.write_date), }, { 'active': True, 'country': False, diff --git a/addons/mail/controllers/discuss/binary.py b/addons/mail/controllers/discuss/binary.py index 43b2e341a94..3763d12f1a9 100644 --- a/addons/mail/controllers/discuss/binary.py +++ b/addons/mail/controllers/discuss/binary.py @@ -16,29 +16,29 @@ class BinaryController(Binary): auth="public", ) @add_guest_to_context - def discuss_channel_partner_avatar_128(self, channel_id, partner_id, **kwargs): + def discuss_channel_partner_avatar_128(self, channel_id, partner_id, unique=False): channel = request.env["discuss.channel"].search([("id", "=", channel_id)]) partner = request.env["res.partner"].browse(partner_id).exists() domain = [("channel_id", "=", channel_id), ("partner_id", "=", partner_id)] if channel and partner and request.env["discuss.channel.member"].search(domain): # sudo: res.partner - the partner is in the same channel as the current user, so they can see their avatar return ( - request.env["ir.binary"]._get_image_stream_from(partner.sudo(), field_name="avatar_128").get_response() + request.env["ir.binary"]._get_image_stream_from(partner.sudo(), field_name="avatar_128").get_response(immutable=True if unique else False) ) - return self.content_image(model="res.partner", id=partner_id, field="avatar_128") + return self.content_image(model="res.partner", id=partner_id, field="avatar_128", unique=unique) @http.route( "/discuss/channel//guest//avatar_128", methods=["GET"], type="http", auth="public" ) @add_guest_to_context - def discuss_channel_guest_avatar_128(self, channel_id, guest_id, **kwargs): + def discuss_channel_guest_avatar_128(self, channel_id, guest_id, unique=False): channel = request.env["discuss.channel"].search([("id", "=", channel_id)]) guest = request.env["mail.guest"].browse(guest_id).exists() domain = [("channel_id", "=", channel_id), ("guest_id", "=", guest_id)] if channel and guest and request.env["discuss.channel.member"].search(domain): # sudo: mail.guest - the guest is in the same channel as the current user, so they can see their avatar - return request.env["ir.binary"]._get_image_stream_from(guest.sudo(), field_name="avatar_128").get_response() - return self.content_image(model="mail.guest", id=guest_id, field="avatar_128") + return request.env["ir.binary"]._get_image_stream_from(guest.sudo(), field_name="avatar_128").get_response(immutable=True if unique else False) + return self.content_image(model="mail.guest", id=guest_id, field="avatar_128", unique=unique) @http.route( "/discuss/channel//attachment/", methods=["GET"], type="http", auth="public" @@ -61,8 +61,8 @@ class BinaryController(Binary): @http.route("/discuss/channel//avatar_128", methods=["GET"], type="http", auth="public") @add_guest_to_context - def discuss_channel_avatar_128(self, channel_id, **kwargs): - return self.content_image(model="discuss.channel", id=channel_id, field="avatar_128") + def discuss_channel_avatar_128(self, channel_id, unique=False): + return self.content_image(model="discuss.channel", id=channel_id, field="avatar_128", unique=unique) @http.route( [ diff --git a/addons/mail/models/discuss/mail_guest.py b/addons/mail/models/discuss/mail_guest.py index 562a232ff44..10771a5a883 100644 --- a/addons/mail/models/discuss/mail_guest.py +++ b/addons/mail/models/discuss/mail_guest.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta from functools import wraps from inspect import Parameter, signature +import odoo from odoo.tools import consteq, get_lang from odoo import _, api, fields, models from odoo.http import request @@ -163,7 +164,7 @@ class MailGuest(models.Model): def _guest_format(self, fields=None): if not fields: - fields = {'id': True, 'name': True, 'im_status': True} + fields = {'id': True, 'name': True, 'im_status': True, "write_date": True} guests_formatted_data = {} for guest in self: data = {} @@ -173,6 +174,8 @@ class MailGuest(models.Model): data['name'] = guest.name if 'im_status' in fields: data['im_status'] = guest.im_status + if "write_date" in fields: + data["write_date"] = odoo.fields.Datetime.to_string(guest.write_date) data['type'] = "guest" guests_formatted_data[guest] = data return guests_formatted_data diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index fbbceb99d1d..5776d536c9e 100644 --- a/addons/mail/models/res_partner.py +++ b/addons/mail/models/res_partner.py @@ -3,6 +3,7 @@ import re +import odoo from odoo import _, api, fields, models, tools from odoo.osv import expression @@ -213,7 +214,7 @@ class Partner(models.Model): def mail_partner_format(self, fields=None): partners_format = dict() if not fields: - fields = {'id': True, 'name': True, 'email': True, 'active': True, 'im_status': True, 'is_company': True, 'user': {}} + fields = {'id': True, 'name': True, 'email': True, 'active': True, 'im_status': True, 'is_company': True, 'user': {}, "write_date": True} for partner in self: data = {} if 'id' in fields: @@ -228,6 +229,8 @@ class Partner(models.Model): data['im_status'] = partner.im_status if 'is_company' in fields: data['is_company'] = partner.is_company + if "write_date" in fields: + data["write_date"] = odoo.fields.Datetime.to_string(partner.write_date) if 'user' in fields: internal_users = partner.user_ids - partner.user_ids.filtered('share') main_user = internal_users[0] if len(internal_users) > 0 else partner.user_ids[0] if len(partner.user_ids) > 0 else self.env['res.users'] diff --git a/addons/mail/static/src/core/common/persona_model.js b/addons/mail/static/src/core/common/persona_model.js index e7a4d09b716..960c3dc3555 100644 --- a/addons/mail/static/src/core/common/persona_model.js +++ b/addons/mail/static/src/core/common/persona_model.js @@ -61,6 +61,8 @@ export class Persona extends Record { /** @type {ImStatus} */ im_status; isAdmin = false; + /** @type {string} */ + write_date; /** * @returns {boolean} diff --git a/addons/mail/static/src/core/common/thread_service.js b/addons/mail/static/src/core/common/thread_service.js index 591aa5352fc..fa5fdcd26ac 100644 --- a/addons/mail/static/src/core/common/thread_service.js +++ b/addons/mail/static/src/core/common/thread_service.js @@ -854,15 +854,19 @@ export class ThreadService { if (!persona) { return DEFAULT_AVATAR; } + const urlParams = {}; + if (persona.write_date) { + urlParams.unique = persona.write_date; + } if (persona.is_company === undefined && this.store.self?.user?.isInternalUser) { this.personaService.fetchIsCompany(persona); } if (thread?.model === "discuss.channel") { if (persona.type === "partner") { - return url(`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`); + return url(`/discuss/channel/${thread.id}/partner/${persona.id}/avatar_128`, urlParams); } if (persona.type === "guest") { - return url(`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`); + return url(`/discuss/channel/${thread.id}/guest/${persona.id}/avatar_128`, urlParams); } } if (persona.type === "partner" && persona?.id) { @@ -870,6 +874,7 @@ export class ThreadService { field: "avatar_128", id: persona.id, model: "res.partner", + ...urlParams, }); return avatar; } @@ -878,6 +883,7 @@ export class ThreadService { field: "avatar_128", id: persona.user.id, model: "res.users", + ...urlParams, }); return avatar; } diff --git a/addons/mail/static/src/discuss/core/common/thread_model_patch.js b/addons/mail/static/src/discuss/core/common/thread_model_patch.js index 024949633bf..d90ae9c3c07 100644 --- a/addons/mail/static/src/discuss/core/common/thread_model_patch.js +++ b/addons/mail/static/src/discuss/core/common/thread_model_patch.js @@ -66,7 +66,10 @@ patch(Thread.prototype, { ); } if (this.type === "chat") { - return `/web/image/res.partner/${this.chatPartner.id}/avatar_128`; + return url( + `/web/image/res.partner/${this.chatPartner.id}/avatar_128`, + assignDefined({}, { unique: this.chatPartner.write_date }) + ); } return super.imgUrl; }, diff --git a/addons/mail/static/tests/discuss_app/sidebar_tests.js b/addons/mail/static/tests/discuss_app/sidebar_tests.js index 1fce3fbec09..5d94af1fad0 100644 --- a/addons/mail/static/tests/discuss_app/sidebar_tests.js +++ b/addons/mail/static/tests/discuss_app/sidebar_tests.js @@ -1000,7 +1000,7 @@ QUnit.test("chat - avatar: should have correct avatar", async () => { openDiscuss(); await contains(".o-mail-DiscussSidebarChannel img"); - await contains(`img[data-src='/web/image/res.partner/${partnerId}/avatar_128']`); + await contains(`img[data-src$='/web/image/res.partner/${partnerId}/avatar_128']`); }); QUnit.test("chat should be sorted by last activity time [REQUIRE FOCUS]", async () => { diff --git a/addons/mail/tests/discuss/test_message_controller.py b/addons/mail/tests/discuss/test_message_controller.py index 3e400a2045c..216174db6eb 100644 --- a/addons/mail/tests/discuss/test_message_controller.py +++ b/addons/mail/tests/discuss/test_message_controller.py @@ -5,6 +5,8 @@ import json import odoo from odoo.tools import mute_logger, date_utils from odoo.tests import HttpCase +from odoo.http import STATIC_CACHE_LONG +from odoo import Command @odoo.tests.tagged("-at_install", "post_install") @@ -342,3 +344,44 @@ class TestMessageController(HttpCase): self.env["res.partner"].search_count([('email', '=', "john2@test.be")]), "'mail/message/post' does not create another user if there's already a user with matching email", ) + + def test_mail_cache_control_header(self): + testuser = self.env['res.users'].create({ + 'email': 'testuser@testuser.com', + 'groups_id': [Command.set([self.ref('base.group_portal')])], + 'name': 'Test User', + 'login': 'testuser', + 'password': 'testuser', + }) + test_user = self.authenticate("testuser", "testuser") + partner = self.env["res.users"].browse(test_user.uid).partner_id + self.channel.add_members(testuser.partner_id.ids) + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/avatar_128?unique={self.channel._get_avatar_cache_key()}" + ) + self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}") + + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/avatar_128" + ) + self.assertEqual(res.headers["Cache-Control"], "no-cache") + + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/partner/{partner.id}/avatar_128?unique={partner.write_date.isoformat()}" + ) + self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}") + + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/partner/{partner.id}/avatar_128" + ) + self.assertEqual(res.headers["Cache-Control"], "no-cache") + + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/guest/{self.guest.id}/avatar_128?unique={self.guest.write_date.isoformat()}" + ) + self.assertEqual(res.headers["Cache-Control"], f"public, max-age={STATIC_CACHE_LONG}") + + res = self.url_open( + url=f"/discuss/channel/{self.channel.id}/guest/{self.guest.id}/avatar_128" + ) + self.assertEqual(res.headers["Cache-Control"], "no-cache") diff --git a/addons/test_discuss_full/tests/test_performance.py b/addons/test_discuss_full/tests/test_performance.py index d72cd5e3231..5deb72b746d 100644 --- a/addons/test_discuss_full/tests/test_performance.py +++ b/addons/test_discuss_full/tests/test_performance.py @@ -5,7 +5,7 @@ from datetime import date from dateutil.relativedelta import relativedelta from unittest.mock import patch, PropertyMock -from odoo import Command +from odoo import Command, fields from odoo.tests.common import users, tagged, HttpCase, warmup from odoo.tools.misc import DEFAULT_SERVER_DATETIME_FORMAT @@ -157,6 +157,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }], key=lambda member_data: member_data['id']))], 'custom_channel_name': False, @@ -208,6 +209,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }], key=lambda member_data: member_data['id']))], 'custom_channel_name': False, @@ -259,6 +261,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }], key=lambda member_data: member_data['id']))], 'custom_channel_name': False, @@ -310,6 +313,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }], key=lambda member_data: member_data['id']))], 'custom_channel_name': False, @@ -361,6 +365,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }], key=lambda member_data: member_data['id']))], 'custom_channel_name': False, @@ -413,6 +418,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, { @@ -434,6 +440,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[12].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, ], key=lambda member_data: member_data['id']))], @@ -501,6 +508,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, { @@ -522,6 +530,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[14].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, ], key=lambda member_data: member_data['id']))], @@ -589,6 +598,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, { @@ -610,6 +620,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[15].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, ], key=lambda member_data: member_data['id']))], @@ -677,6 +688,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, { @@ -698,6 +710,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[2].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, ], key=lambda member_data: member_data['id']))], @@ -765,6 +778,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, }, { @@ -786,6 +800,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[3].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[3].partner_id.write_date), }, }, ], key=lambda member_data: member_data['id']))], @@ -952,6 +967,7 @@ class TestDiscussFullPerformance(HttpCase): 'im_status': self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).guest_id.im_status, 'name': self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).guest_id.name, 'type': "guest", + 'write_date': fields.Datetime.to_string(self.channel_livechat_2.channel_member_ids.filtered(lambda m: m.guest_id).guest_id.write_date), }, }, ])], @@ -1014,6 +1030,7 @@ class TestDiscussFullPerformance(HttpCase): 'out_of_office_date_end': False, 'type': "partner", 'user': False, + 'write_date': fields.Datetime.to_string(self.user_root.partner_id.write_date), }, 'currentGuest': False, 'current_partner': { @@ -1029,6 +1046,7 @@ class TestDiscussFullPerformance(HttpCase): 'id': self.users[0].id, 'isInternalUser': True, }, + 'write_date': fields.Datetime.to_string(self.users[0].partner_id.write_date), }, 'current_user_id': self.users[0].id, 'current_user_settings': {