[IMPROVE] User avatar cache invalidation (#17925)

Co-authored-by: Diego Sampaio <chinello@gmail.com>

app/api/server/lib/users.js

@@ -14,6 +14,7 @@ export async function findUsersToAutocomplete({ uid, selector }) {
 			name: 1,
 			username: 1,
 			status: 1,
+			avatarETag: 1,
 		},
 		sort: {
 			username: 1,

app/api/server/v1/users.js

@@ -716,6 +716,7 @@ API.v1.addRoute('users.presence', { authRequired: true }, {
 				status: 1,
 				utcOffset: 1,
 				statusText: 1,
+				avatarETag: 1,
 			},
 		};
 

app/lazy-load/client/index.js

@@ -9,14 +9,16 @@ const loadImage = (el) => {
 	map.delete(el);
 
 	if (!instance) {
-		return instance.loaded.set(true);
+		return;
 	}
+
 	const img = new Image();
 	const src = el.getAttribute('data-src');
 	img.onload = () => {
 		el.className = el.className.replace('lazy-img', '');
 		el.src = src;
 		el.removeAttribute('data-src');
+		instance.loaded.set(true);
 	};
 	img.src = src;
 };

app/lazy-load/client/lazyloadImage.js

@@ -4,7 +4,7 @@ import { Template } from 'meteor/templating';
 import './lazyloadImage.html';
 import { addImage } from '.';
 
-const emptyImageEncoded =	'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8+/u3PQAJJAM0dIyWdgAAAABJRU5ErkJggg==';
+const emptyImageEncoded = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8+/u3PQAJJAM0dIyWdgAAAABJRU5ErkJggg==';
 
 const imgsrcs = new Set();
 
@@ -15,12 +15,17 @@ Template.lazyloadImage.helpers({
 	},
 
 	srcUrl() {
+		if (Template.instance().loaded.get()) {
+			return;
+		}
 		return this.src;
 	},
 
 	lazySrcUrl() {
 		const { preview, placeholder, src } = this;
-		if (Template.instance().loaded.get() || (!preview && !placeholder) || imgsrcs.has(src)) {
+		const { loaded } = Template.instance();
+
+		if (loaded.get() || (!preview && !placeholder) || imgsrcs.has(src)) {
 			return src;
 		}
 

app/ldap/server/sync.js

@@ -409,10 +409,10 @@ export function syncUserData(user, ldapUser, ldap) {
 			};
 
 			Meteor.runAsUser(user._id, () => {
-				fileStore.insert(file, rs, () => {
+				fileStore.insert(file, rs, (err, result) => {
 					Meteor.setTimeout(function() {
-						Users.setAvatarOrigin(user._id, 'ldap');
-						Notifications.notifyLogged('updateAvatar', { username: user.username });
+						Users.setAvatarData(user._id, 'ldap', result.etag);
+						Notifications.notifyLogged('updateAvatar', { username: user.username, etag: result.etag });
 					}, 500);
 				});
 			});

app/lib/lib/roomTypes/direct.js

@@ -190,6 +190,11 @@ export class DirectMessageRoomType extends RoomTypeConfig {
 			return '';
 		}
 
+		// if coming from sidenav search
+		if (roomData.name && roomData.avatarETag) {
+			return getUserAvatarURL(roomData.name, roomData.avatarETag);
+		}
+
 		if (this.isGroupChat(roomData)) {
 			return getAvatarURL({ username: roomData.uids.length + roomData.usernames.join() });
 		}
@@ -197,7 +202,8 @@ export class DirectMessageRoomType extends RoomTypeConfig {
 		const sub = subData || Subscriptions.findOne({ rid: roomData._id }, { fields: { name: 1 } });
 
 		if (sub && sub.name) {
-			return getUserAvatarURL(sub.name);
+			const user = Meteor.users.findOne({ username: sub.name }, { fields: { username: 1, avatarETag: 1 } });
+			return getUserAvatarURL(user?.username || sub.name, user?.avatarETag);
 		}
 
 		if (roomData) {

app/lib/lib/roomTypes/public.js

@@ -4,7 +4,7 @@ import { openRoom } from '../../../ui-utils';
 import { ChatRoom, ChatSubscription } from '../../../models';
 import { settings } from '../../../settings';
 import { hasAtLeastOnePermission } from '../../../authorization';
-import { getUserPreference, RoomTypeConfig, RoomTypeRouteConfig, RoomSettingsEnum, UiTextContext, RoomMemberActions } from '../../../utils';
+import { getUserPreference, RoomTypeConfig, RoomTypeRouteConfig, RoomSettingsEnum, UiTextContext, RoomMemberActions, roomTypes } from '../../../utils';
 import { getAvatarURL } from '../../../utils/lib/getAvatarURL';
 
 export class PublicRoomRoute extends RoomTypeRouteConfig {
@@ -136,6 +136,17 @@ export class PublicRoomType extends RoomTypeConfig {
 	getAvatarPath(roomData) {
 		// TODO: change to always get avatar from _id when rooms have avatars
 
+		// if room is not a discussion, returns the avatar for its name
+		if (!roomData.prid) {
+			return getAvatarURL({ username: `@${ this.roomName(roomData) }` });
+		}
+
+		// if discussion's parent room is known, get his avatar
+		const proom = ChatRoom.findOne({ _id: roomData.prid }, { reactive: false });
+		if (proom) {
+			return roomTypes.getConfig(proom.t).getAvatarPath(proom);
+		}
+
 		return getAvatarURL({ username: `@${ this.roomName(roomData) }` });
 	}
 

app/lib/server/functions/getFullUserData.js

@@ -16,6 +16,7 @@ const defaultFields = {
 	active: 1,
 	reason: 1,
 	statusText: 1,
+	avatarETag: 1,
 };
 
 const fullFields = {

app/lib/server/functions/setUserAvatar.js

@@ -11,7 +11,7 @@ export const setUserAvatar = function(user, dataURI, contentType, service) {
 	let image;
 
 	if (service === 'initials') {
-		return Users.setAvatarOrigin(user._id, service);
+		return Users.setAvatarData(user._id, service, null);
 	} if (service === 'url') {
 		let result = null;
 
@@ -61,10 +61,10 @@ export const setUserAvatar = function(user, dataURI, contentType, service) {
 		size: buffer.length,
 	};
 
-	fileStore.insert(file, buffer, () => {
+	fileStore.insert(file, buffer, (err, result) => {
 		Meteor.setTimeout(function() {
-			Users.setAvatarOrigin(user._id, service);
-			Notifications.notifyLogged('updateAvatar', { username: user.username });
+			Users.setAvatarData(user._id, service, result.etag);
+			Notifications.notifyLogged('updateAvatar', { username: user.username, etag: result.etag });
 		}, 500);
 	});
 };

app/models/server/models/Users.js

@@ -1002,20 +1002,22 @@ export class Users extends Base {
 		return this.update(_id, update);
 	}
 
-	setAvatarOrigin(_id, origin) {
+	setAvatarData(_id, origin, etag) {
 		const update = {
 			$set: {
 				avatarOrigin: origin,
+				avatarETag: etag,
 			},
 		};
 
 		return this.update(_id, update);
 	}
 
-	unsetAvatarOrigin(_id) {
+	unsetAvatarData(_id) {
 		const update = {
 			$unset: {
 				avatarOrigin: 1,
+				avatarETag: 1,
 			},
 		};
 

app/ui-account/client/avatar/avatar.js

@@ -1,18 +1,24 @@
 import { Meteor } from 'meteor/meteor';
-import { Session } from 'meteor/session';
 import { Template } from 'meteor/templating';
 
 import { getUserAvatarURL } from '../../../utils/lib/getUserAvatarURL';
 
 const getUsername = ({ userId, username }) => {
+	const query = {};
 	if (username) {
-		return username;
+		query.username = username;
 	}
 
 	if (userId) {
-		const user = Meteor.users.findOne(userId, { fields: { username: 1 } });
-		return user && user.username;
+		query._id = userId;
+	}
+
+	const user = Meteor.users.findOne(query, { fields: { username: 1, avatarETag: 1 } });
+	if (!user) {
+		return {};
 	}
+
+	return user;
 };
 
 Template.avatar.helpers({
@@ -22,21 +28,23 @@ Template.avatar.helpers({
 			return url;
 		}
 
-		let username = getUsername(this);
-		if (!username) {
-			return;
+		if (this.roomIcon && this.username) {
+			return getUserAvatarURL(`@${ this.username }`);
 		}
 
-		Session.get(`avatar_random_${ username }`);
-
-		if (this.roomIcon) {
-			username = `@${ username }`;
+		const { username, avatarETag } = getUsername(this);
+		if (!username) {
+			if (this.username) {
+				return getUserAvatarURL(this.username);
+			}
+			return;
 		}
 
-		return getUserAvatarURL(username);
+		return getUserAvatarURL(username, avatarETag);
 	},
 
 	alt() {
-		return getUsername(this);
+		const { username } = getUsername(this);
+		return username;
 	},
 });

app/ui-flextab/client/tabs/membersList.html

@@ -34,7 +34,7 @@
 				<ul class='list clearfix lines'>
 					{{#each users}}
 						<li class='rc-member-list__user'>
-							{{> avatar username=user.username}}
+							{{> avatar url=avatarUrl}}
 							<div class="rc-member-list__username">
 								{{# userPresence uid=user._id}}<div class="rc-member-list__status rc-member-list__status--{{status}}"></div>{{/userPresence}}
 								{{ignored}} {{displayName}} {{utcOffset}}

app/ui-flextab/client/tabs/membersList.js

@@ -4,12 +4,12 @@ import { Session } from 'meteor/session';
 import { Template } from 'meteor/templating';
 
 import { getActions } from './userActions';
-import { RoomManager, popover } from '../../../ui-utils';
-import { ChatRoom, Subscriptions } from '../../../models';
-import { settings } from '../../../settings';
-import { t, isRtl, handleError, roomTypes } from '../../../utils';
+import { RoomManager, popover } from '../../../ui-utils/client';
+import { ChatRoom, Subscriptions } from '../../../models/client';
+import { settings } from '../../../settings/client';
+import { t, isRtl, handleError, roomTypes, getUserAvatarURL } from '../../../utils/client';
 import { WebRTC } from '../../../webrtc/client';
-import { hasPermission } from '../../../authorization';
+import { hasPermission } from '../../../authorization/client';
 
 Template.membersList.helpers({
 	ignored() {
@@ -129,6 +129,11 @@ Template.membersList.helpers({
 	loadingMore() {
 		return Template.instance().loadingMore.get();
 	},
+
+	avatarUrl() {
+		const { user: { username, avatarETag } } = this;
+		return getUserAvatarURL(username, avatarETag);
+	},
 });
 
 Template.membersList.events({

app/ui-message/client/popup/messagePopupConfig.js

@@ -67,13 +67,14 @@ const fetchUsersFromServer = _.throttle(async (filterText, records, rid, cb) =>
 
 	users
 		.slice(0, 5)
-		.forEach(({ username, name, status }) => {
+		.forEach(({ username, name, status, avatarETag }) => {
 			if (records.length < 5) {
 				records.push({
 					_id: username,
 					username,
 					name,
 					status,
+					avatarETag,
 					sort: 3,
 				});
 			}

app/ui-message/client/popup/messagePopupUser.html

@@ -1,7 +1,7 @@
 <template name="messagePopupUser">
 	{{#unless system}}
 		<div class="popup-user-status border-transparent-dark popup-user-status-{{status}}"></div>
-		<div class="popup-user-avatar" style="background-image:url({{avatarUrlFromUsername username}});"></div>
+		<div class="popup-user-avatar" style="background-image:url({{avatarUrlFromUsername username avatarETag}});"></div>
 	{{/unless}}
 	<strong>{{username}}</strong> {{name}}
 </template>

app/ui-sidenav/client/toolbar.js

@@ -52,6 +52,7 @@ const getFromServer = (cb, type) => {
 			t: 'd',
 			name: user.username,
 			fname: user.name,
+			avatarETag: user.avatarETag,
 		});
 
 		resultsFromServer.push(...results.users.map(userMap));

app/utils/lib/getAvatarURL.js

@@ -2,9 +2,9 @@ import { getURL } from './getURL';
 
 export const getAvatarURL = ({ username, roomId, cache }) => {
 	if (username) {
-		return getURL(`/avatar/${ encodeURIComponent(username) }${ cache ? `?_dc=${ cache }` : '' }`);
+		return getURL(`/avatar/${ encodeURIComponent(username) }${ cache ? `?etag=${ cache }` : '' }`);
 	}
 	if (roomId) {
-		return getURL(`/avatar/room/${ encodeURIComponent(roomId) }${ cache ? `?_dc=${ cache }` : '' }`);
+		return getURL(`/avatar/room/${ encodeURIComponent(roomId) }${ cache ? `?etag=${ cache }` : '' }`);
 	}
 };

app/utils/lib/getUserAvatarURL.js

@@ -1,10 +1,7 @@
-import { Session } from 'meteor/session';
-import { Tracker } from 'meteor/tracker';
-
 import { getAvatarURL } from './getAvatarURL';
 import { settings } from '../../settings';
 
-export const getUserAvatarURL = function(username) {
+export const getUserAvatarURL = function(username, cache = '') {
 	const externalSource = (settings.get('Accounts_AvatarExternalProviderUrl') || '').trim().replace(/\/$/, '');
 	if (externalSource !== '') {
 		return externalSource.replace('{username}', username);
@@ -12,8 +9,6 @@ export const getUserAvatarURL = function(username) {
 	if (username == null) {
 		return;
 	}
-	const key = `avatar_random_${ username }`;
-	const cache = Tracker.nonreactive(() => Session && Session.get(key)); // there is no Session on server
 
 	return getAvatarURL({ username, cache });
 };

client/admin/users/UsersTable.js

@@ -4,7 +4,7 @@ import React, { useMemo, useCallback, useState, useEffect } from 'react';
 
 import { GenericTable, Th } from '../../components/GenericTable';
 import { useTranslation } from '../../contexts/TranslationContext';
-import { roomTypes } from '../../../app/utils/client';
+import { getUserAvatarURL } from '../../../app/utils/client';
 import { useRoute } from '../../contexts/RouterContext';
 import { useEndpointData } from '../../hooks/useEndpointData';
 
@@ -26,7 +26,7 @@ const FilterByText = ({ setFilter, ...props }) => {
 const sortDir = (sortDir) => (sortDir === 'asc' ? 1 : -1);
 
 const useQuery = ({ text, itemsPerPage, current }, [column, direction]) => useMemo(() => ({
-	fields: JSON.stringify({ name: 1, username: 1, emails: 1, roles: 1, status: 1 }),
+	fields: JSON.stringify({ name: 1, username: 1, emails: 1, roles: 1, status: 1, avatarETag: 1 }),
 	query: JSON.stringify({
 		$or: [
 			{ 'emails.address': { $regex: text || '', $options: 'i' } },
@@ -78,8 +78,8 @@ export function UsersTable() {
 		<Th key={'status'} direction={sort[1]} active={sort[0] === 'status'} onClick={onHeaderClick} sort='status' w='x100'>{t('Status')}</Th>,
 	].filter(Boolean), [sort, onHeaderClick, t, mediaQuery]);
 
-	const renderRow = useCallback(({ emails, _id, username, name, roles, status, ...args }) => {
-		const avatarUrl = roomTypes.getConfig('d').getAvatarPath({ name: username || name, type: 'd', _id, ...args });
+	const renderRow = useCallback(({ emails, _id, username, name, roles, status, avatarETag }) => {
+		const avatarUrl = getUserAvatarURL(username, avatarETag);
 
 		return <Table.Row key={_id} onKeyDown={onClick(_id)} onClick={onClick(_id)} tabIndex={0} role='link' action qa-user-id={_id}>
 			<Table.Cell style={style}>

client/components/RoomForeword.js

@@ -1,11 +1,11 @@
 import React from 'react';
 import { Avatar, Margins, Flex, Box, Tag } from '@rocket.chat/fuselage';
 
-import { Rooms } from '../../app/models';
+import { Rooms, Users } from '../../app/models/client';
 import { useTranslation } from '../contexts/TranslationContext';
 import { useReactiveValue } from '../hooks/useReactiveValue';
 import { useUser } from '../contexts/UserContext';
-import { roomTypes } from '../../app/utils/client';
+import { getUserAvatarURL } from '../../app/utils/client';
 
 const RoomForeword = ({ _id: rid }) => {
 	const t = useTranslation();
@@ -17,8 +17,8 @@ const RoomForeword = ({ _id: rid }) => {
 		return t('Start_of_conversation');
 	}
 
-	const users = room.usernames.filter((username) => username !== user.username);
-	if (users.length < 1) {
+	const usernames = room.usernames.filter((username) => username !== user.username);
+	if (usernames.length < 1) {
 		return null;
 	}
 
@@ -26,9 +26,12 @@ const RoomForeword = ({ _id: rid }) => {
 		<Flex.Item grow={1}>
 			<Margins block='x24'>
 				<Avatar.Stack>
-					{users.map(
+					{usernames.map(
 						(username, index) => {
-							const avatarUrl = roomTypes.getConfig('d').getAvatarPath({ name: username, type: 'd' });
+							const user = Users.findOne({ username }, { fields: { avatarETag: 1 } });
+
+							const avatarUrl = getUserAvatarURL(username, user?.avatarETag);
+
 							return <Avatar size='x48' title={username} url={avatarUrl} key={index} data-username={username} />;
 						})}
 				</Avatar.Stack>
@@ -36,7 +39,7 @@ const RoomForeword = ({ _id: rid }) => {
 		</Flex.Item>
 		<Box color='default' fontScale='h1' flexGrow={1}>{t('Direct_message_you_have_joined')}</Box>
 		<Box is='div' mb='x8' flexGrow={1}>
-			{users.map((username, index) => <Margins inline='x4' key={index}>
+			{usernames.map((username, index) => <Margins inline='x4' key={index}>
 				<Tag
 					is='a'
 					fontScale='p2'

client/notifications/updateAvatar.js

@@ -1,10 +1,10 @@
 import { Meteor } from 'meteor/meteor';
 
-import { updateAvatarOfUsername } from '../../app/ui-utils';
 import { Notifications } from '../../app/notifications';
 
 Meteor.startup(function() {
 	Notifications.onLogged('updateAvatar', function(data) {
-		updateAvatarOfUsername(data.username);
+		const { username, etag } = data;
+		Meteor.users.update({ username }, { $set: { avatarETag: etag } });
 	});
 });

client/views/directory/ChannelsTab.js

@@ -76,8 +76,9 @@ function ChannelsTable() {
 	}, [channelRoute]);
 
 	const formatDate = useFormatDate();
-	const renderRow = useCallback(({ _id, ts, name, fname, description, usersCount, lastMessage, topic, ...room }) => {
-		const avatarUrl = roomTypes.getConfig('d').getAvatarPath({ name: name || fname, type: 'd', _id });
+	const renderRow = useCallback((room) => {
+		const { _id, ts, t, name, fname, usersCount, lastMessage, topic } = room;
+		const avatarUrl = roomTypes.getConfig(t).getAvatarPath(room);
 
 		return <Table.Row key={_id} onKeyDown={onClick(name)} onClick={onClick(name)} tabIndex={0} role='link' action>
 			<Table.Cell>
@@ -87,7 +88,7 @@ function ChannelsTable() {
 						<Box display='flex' alignItems='center'>
 							<Icon name={roomTypes.getIcon(room)} color='hint' /> <Box fontScale='p2' mi='x4'>{fname || name}</Box><RoomTags room={room} style={style} />
 						</Box>
-						{topic && <MarkdownText fontScale='p1' color='hint' style={style} content={topic} />}
+						{topic && <MarkdownText fontScale='p1' color='hint' style={style} withRichContent={false} content={topic} />}
 					</Box>
 				</Box>
 			</Table.Cell>

client/views/directory/UserTab.js

@@ -7,7 +7,7 @@ import { useTranslation } from '../../contexts/TranslationContext';
 import { useRoute } from '../../contexts/RouterContext';
 import { usePermission } from '../../contexts/AuthorizationContext';
 import { useQuery } from './hooks';
-import { roomTypes } from '../../../app/utils/client';
+import { getUserAvatarURL } from '../../../app/utils/client';
 import { useEndpointData } from '../../hooks/useEndpointData';
 import { useFormatDate } from '../../hooks/useFormatDate';
 import NotAuthorizedPage from '../../admin/NotAuthorizedPage';
@@ -72,8 +72,8 @@ function UserTable({
 
 	const formatDate = useFormatDate();
 
-	const renderRow = useCallback(({ createdAt, emails, _id, username, name, domain, bio }) => {
-		const avatarUrl = roomTypes.getConfig('d').getAvatarPath({ name: username || name, type: 'd', _id });
+	const renderRow = useCallback(({ createdAt, emails, _id, username, name, domain, bio, avatarETag }) => {
+		const avatarUrl = getUserAvatarURL(username, avatarETag);
 
 		return <Table.Row key={_id} onKeyDown={onClick(username)} onClick={onClick(username)} tabIndex={0} role='link' action>
 			<Table.Cell>

imports/startup/client/listenActiveUsers.js

@@ -25,6 +25,7 @@ export const saveUser = (user, force = false) => {
 				// utcOffset: user.utcOffset,
 				status: user.status,
 				statusText: user.statusText,
+				...user.avatarETag && { avatarETag: user.avatarETag },
 			},
 		});
 	}

server/methods/browseChannels.js

@@ -129,6 +129,7 @@ Meteor.methods({
 				createdAt: 1,
 				emails: 1,
 				federation: 1,
+				avatarETag: 1,
 			},
 		};
 

server/methods/getUsersOfRoom.js

@@ -26,6 +26,7 @@ function findUsers({ rid, status, skip, limit, filter = '' }) {
 				'u.name': 1,
 				'u.username': 1,
 				'u.status': 1,
+				'u.avatarETag': 1,
 			},
 		},
 		...status ? [{ $match: { 'u.status': status } }] : [],
@@ -42,6 +43,7 @@ function findUsers({ rid, status, skip, limit, filter = '' }) {
 				_id: { $arrayElemAt: ['$u._id', 0] },
 				name: { $arrayElemAt: ['$u.name', 0] },
 				username: { $arrayElemAt: ['$u.username', 0] },
+				avatarETag: { $arrayElemAt: ['$u.avatarETag', 0] },
 			},
 		},
 	]).toArray();

server/methods/resetAvatar.js

@@ -42,9 +42,10 @@ Meteor.methods({
 		}
 
 		FileUpload.getStore('Avatars').deleteByName(user.username);
-		Users.unsetAvatarOrigin(user._id);
+		Users.unsetAvatarData(user._id);
 		Notifications.notifyLogged('updateAvatar', {
 			username: user.username,
+			etag: null,
 		});
 	},
 });

server/publications/spotlight.js

@@ -61,6 +61,7 @@ Meteor.methods({
 				name: 1,
 				status: 1,
 				statusText: 1,
+				avatarETag: 1,
 			},
 			sort: {},
 		};

server/startup/initialData.js

@@ -41,7 +41,7 @@ Meteor.startup(function() {
 			};
 
 			Meteor.runAsUser('rocket.cat', () => {
-				fileStore.insert(file, rs, () => Users.setAvatarOrigin('rocket.cat', 'local'));
+				fileStore.insert(file, rs, () => Users.setAvatarData('rocket.cat', 'local', null));
 			});
 		}
 

server/startup/migrations/v002.js

@@ -38,7 +38,7 @@ Migrations.add({
 				type: contentType,
 			};
 
-			fileStore.insert(file, rs, () => Users.setAvatarOrigin(user._id, service));
+			fileStore.insert(file, rs, (err, result) => Users.setAvatarData(user._id, service, result.etag));
 		});
 	},
 });

tests/end-to-end/api/01-users.js

@@ -375,6 +375,7 @@ describe('[Users]', function() {
 						expect(res.body).to.have.property('full', true);
 						expect(res.body).to.have.property('users').to.have.property('0').to.deep.have.all.keys(
 							'_id',
+							'avatarETag',
 							'username',
 							'name',
 							'status',
@@ -410,6 +411,7 @@ describe('[Users]', function() {
 						expect(res.body).to.have.property('full', true);
 						expect(res.body).to.have.property('users').to.have.property('0').to.deep.have.all.keys(
 							'_id',
+							'avatarETag',
 							'username',
 							'name',
 							'status',