Skip to content

[IMPROVE] User avatar cache invalidation #17925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 9 commits into from
Jun 19, 2020
Merged

[IMPROVE] User avatar cache invalidation #17925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1f8a780
Store avatar etag
pierre-lehnen-rc Jun 16, 2020
c2dfa59
Return etag on API methods
pierre-lehnen-rc Jun 16, 2020
e7db3de
Added etag to avar changed stream event
pierre-lehnen-rc Jun 16, 2020
05979a1
Merge branch 'develop' into avatar-cache
pierre-lehnen-rc Jun 16, 2020
2777d25
Added new field to api tests
pierre-lehnen-rc Jun 16, 2020
d55aee5
Merge branch 'avatar-cache' of github.com:RocketChat/Rocket.Chat into…
pierre-lehnen-rc Jun 16, 2020
828b564
Merge branch 'develop' into avatar-cache
sampaiodiego Jun 18, 2020
1bf981a
Merge branch 'develop' into avatar-cache
sampaiodiego Jun 18, 2020
69654a8
Use avatarETag
sampaiodiego Jun 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/api/server/lib/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export async function findUsersToAutocomplete({ uid, selector }) {
name: 1,
username: 1,
status: 1,
avatarETag: 1,
},
sort: {
username: 1,
Expand Down
1 change: 1 addition & 0 deletions app/api/server/v1/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ API.v1.addRoute('users.presence', { authRequired: true }, {
status: 1,
utcOffset: 1,
statusText: 1,
avatarETag: 1,
},
};

Expand Down
4 changes: 3 additions & 1 deletion app/lazy-load/client/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
9 changes: 7 additions & 2 deletions app/lazy-load/client/lazyloadImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions app/ldap/server/sync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down
8 changes: 7 additions & 1 deletion app/lib/lib/roomTypes/direct.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,20 @@ 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() });
}

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) {
Expand Down
13 changes: 12 additions & 1 deletion app/lib/lib/roomTypes/public.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) }` });
}

Expand Down
1 change: 1 addition & 0 deletions app/lib/server/functions/getFullUserData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const defaultFields = {
active: 1,
reason: 1,
statusText: 1,
avatarETag: 1,
};

const fullFields = {
Expand Down
8 changes: 4 additions & 4 deletions app/lib/server/functions/setUserAvatar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
});
};
6 changes: 4 additions & 2 deletions app/models/server/models/Users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down
34 changes: 21 additions & 13 deletions app/ui-account/client/avatar/avatar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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({
Expand All @@ -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;
},
});
2 changes: 1 addition & 1 deletion app/ui-flextab/client/tabs/membersList.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
15 changes: 10 additions & 5 deletions app/ui-flextab/client/tabs/membersList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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({
Expand Down
3 changes: 2 additions & 1 deletion app/ui-message/client/popup/messagePopupConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Expand Down
2 changes: 1 addition & 1 deletion app/ui-message/client/popup/messagePopupUser.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
1 change: 1 addition & 0 deletions app/ui-sidenav/client/toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions app/utils/lib/getAvatarURL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }` : '' }`);
}
};
7 changes: 1 addition & 6 deletions app/utils/lib/getUserAvatarURL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
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);
}
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 });
};
8 changes: 4 additions & 4 deletions client/admin/users/UsersTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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' } },
Expand Down Expand Up @@ -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}>
Expand Down
Loading