Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions .changeset/perf-quick-wins.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@rocket.chat/random': patch
'@rocket.chat/i18n': patch
'@rocket.chat/license': patch
'@rocket.chat/ui-client': patch
'@rocket.chat/meteor': patch
---

Improves performance of several hot code paths without changing behavior: generates random IDs with a single `crypto.randomBytes` call instead of one per character, caches constant regular expressions used by the markdown/mention/autotranslate parsers instead of recompiling them for every message, skips the channel-mention database query for messages without channel mentions, deduplicates the room member count query when a message contains both `@all` and `@here`, and replaces linear array scans and spread-accumulators with Map/Set lookups in API response shaping (files, DM members, directory search and team listing).
14 changes: 8 additions & 6 deletions apps/meteor/app/markdown/lib/parser/filtered/filtered.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { getFilteredLinkRegexes } from '../linkRegexes';

const inlineCodeRegex = /`([^`\r\n]+)\`/gm;

/**
* Filter markdown tags in message
* Use case: notifications
Expand All @@ -10,21 +14,19 @@ export const filtered = (
},
) => {
const schemes = (options.supportSchemesForLink || 'http,https').split(',').join('|');
const linkRegexes = getFilteredLinkRegexes(schemes);

// Remove block code backticks
message = message.replace(/```/g, '');

// Remove inline code backticks
message = message.replace(new RegExp(/`([^`\r\n]+)\`/gm), (match) => match.substr(1, match.length - 2));
message = message.replace(inlineCodeRegex, (match) => match.substr(1, match.length - 2));

// Filter [text](url), ![alt_text](image_url)
message = message.replace(new RegExp(`!?\\[([^\\]]+)\\]\\((?:${schemes}):\\/\\/[^\\)]+\\)`, 'gm'), (match, title) => title);
message = message.replace(linkRegexes.link, (match, title) => title);

// Filter <http://link|Text>
message = message.replace(
new RegExp(`(?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
(match, title) => title,
);
message = message.replace(linkRegexes.pipedLink, (match, title) => title);

// Filter headings
message = message.replace(
Expand Down
20 changes: 20 additions & 0 deletions apps/meteor/app/markdown/lib/parser/linkRegexes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import mem from 'mem';

// The link regexes only vary with the configured URL schemes, which come from
// a setting that rarely changes; cache the compiled regexes per schemes value
// instead of recompiling them for every message.
//
// The two parsers intentionally use different patterns: `filtered` only strips
// markup (no URL capture, URLs may contain `)` escapes) while `original`
// renders it (captures the URL, splits image from link).

Comment thread
KevLehman marked this conversation as resolved.
export const getMarkdownLinkRegexes = mem((schemes) => ({
image: new RegExp(`!\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'),
link: new RegExp(`\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'),
pipedLink: new RegExp(`(?:<|&lt;)((?:${schemes}):\\/\\/[^\\|]+)\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
}));

export const getFilteredLinkRegexes = mem((schemes) => ({
link: new RegExp(`!?\\[([^\\]]+)\\]\\((?:${schemes}):\\/\\/[^\\)]+\\)`, 'gm'),
pipedLink: new RegExp(`(?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'),
}));
14 changes: 10 additions & 4 deletions apps/meteor/app/markdown/lib/parser/original/markdown.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { addAsToken, isToken, validateAllowedTokens } from './token';
import { getMarkdownLinkRegexes } from '../linkRegexes';

const validateUrl = (url, message) => {
// Don't render markdown inside links
Expand Down Expand Up @@ -31,7 +32,11 @@ const getTextWrapper = (marker, tagName) => (textPrepend, wrappedText, textAppen

const getRegexReplacer = (replaceFunction, getRegex) => (marker, tagName) => {
const wrapper = getTextWrapper(marker, tagName);
return (msg) => msg.replace(getRegex(marker), (...args) => replaceFunction(wrapper, ...args));
// The regex only depends on the marker, so build it once when the parser is
// created instead of on every message. Safe with /g because
// String.prototype.replace resets lastIndex on each call.
const regex = getRegex(marker);
return (msg) => msg.replace(regex, (...args) => replaceFunction(wrapper, ...args));
};

const getParserWithCustomMarker = getRegexReplacer(
Expand Down Expand Up @@ -68,6 +73,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
}

const schemes = (supportSchemesForLink || '').split(',').join('|');
const linkRegexes = getMarkdownLinkRegexes(schemes);

if (headers) {
// Support # Text for h1
Expand Down Expand Up @@ -130,7 +136,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
msg = msg.replace(/<\/blockquote>\n<blockquote/gm, '</blockquote><blockquote');

// Support ![alt text](http://image url)
msg = msg.replace(new RegExp(`!\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'), (match, title, url) => {
msg = msg.replace(linkRegexes.image, (match, title, url) => {
if (!validateUrl(url, message)) {
return match;
}
Expand All @@ -148,7 +154,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
});

// Support [Text](http://link)
msg = msg.replace(new RegExp(`\\[([^\\]]+)\\]\\(((?:${schemes}):\\/\\/[^\\s]+)\\)`, 'gm'), (match, title, url) => {
msg = msg.replace(linkRegexes.link, (match, title, url) => {
if (!validateUrl(url, message)) {
return match;
}
Expand All @@ -168,7 +174,7 @@ const parseNotEscaped = (message, { supportSchemesForLink, headers, rootUrl }) =
});

// Support <http://link|Text>
msg = msg.replace(new RegExp(`(?:<|&lt;)((?:${schemes}):\\\/\\\/[^\\|]+)\\|(.+?)(?=>|&gt;)(?:>|&gt;)`, 'gm'), (match, url, title) => {
msg = msg.replace(linkRegexes.pipedLink, (match, url, title) => {
if (!validateUrl(url, message)) {
return match;
}
Expand Down
44 changes: 31 additions & 13 deletions apps/meteor/app/mentions/lib/MentionsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,34 @@ export class MentionsParser {
this.roomTemplate = roomTemplate;
}

get userMentionRegex() {
return new RegExp(`(^|\\s|>)@(${this.pattern()}(@(${this.pattern()}))?(:([0-9a-zA-Z-_.]+))?)`, 'gm');
// The mention regexes only depend on the configured pattern (a setting that
// rarely changes), but these getters are hit multiple times per message;
// cache the compiled regexes and rebuild only when the pattern changes.
// Sharing the instances is safe because they're only used with
// String.prototype.replace/match, which reset lastIndex on each call.
private cachedPattern: string | undefined;

private cachedUserMentionRegex: RegExp | undefined;

private cachedChannelMentionRegex: RegExp | undefined;

private updateRegexCache() {
const pattern = this.pattern();
if (pattern !== this.cachedPattern || !this.cachedUserMentionRegex || !this.cachedChannelMentionRegex) {
this.cachedPattern = pattern;
this.cachedUserMentionRegex = new RegExp(`(^|\\s|>)@(${pattern}(@(${pattern}))?(:([0-9a-zA-Z-_.]+))?)`, 'gm');
this.cachedChannelMentionRegex = new RegExp(`(^|\\s|>)#(${pattern}(@(${pattern}))?)`, 'gm');
}
}

get userMentionRegex(): RegExp {
this.updateRegexCache();
return this.cachedUserMentionRegex as RegExp;
}

get channelMentionRegex() {
return new RegExp(`(^|\\s|>)#(${this.pattern()}(@(${this.pattern()}))?)`, 'gm');
get channelMentionRegex(): RegExp {
this.updateRegexCache();
return this.cachedChannelMentionRegex as RegExp;
}

replaceUsers = (msg: string, { mentions, temp }: IMessage, me: string) =>
Expand Down Expand Up @@ -103,18 +125,14 @@ export class MentionsParser {

replaceChannels = (msg: string, { temp, channels }: IMessage) =>
msg.replace(/&#39;/g, "'").replace(this.channelMentionRegex, (match, prefix, mention) => {
if (
!temp &&
!channels?.find((c) => {
return c.name === mention;
})
) {
return match;
}

const channel = channels?.find(({ name }) => {
return name === mention;
});

if (!temp && !channel) {
return match;
}

const reference = channel ? channel._id : mention;
return this.roomTemplate({ prefix, reference, channel, mention });
});
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/server/api/lib/addUserToFileObj.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ export async function addUserToFileObj(files: IUpload[]): Promise<(IUpload & { u
const uids = files.map(({ userId }) => userId).filter(isString);

const users = await Users.findByIds(uids, { projection: { name: 1, username: 1 } }).toArray();
const usersById = new Map(users.map((user) => [user._id, user]));

return files.map((file) => {
const user = users.find(({ _id: userId }) => file.userId && userId === file.userId);
const user = file.userId ? usersById.get(file.userId) : undefined;
if (!user) {
return file;
}
Expand Down
4 changes: 3 additions & 1 deletion apps/meteor/server/api/v1/im.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,10 @@ const dmMembersAction = <Path extends string>(_path: Path): TypedAction<typeof d
{ projection: { u: 1, status: 1, ts: 1, roles: 1 } },
).toArray();

const subsByUserId = new Map(subs.map((sub) => [sub.u._id, sub]));

const membersWithSubscriptionInfo = members.map((member) => {
const sub = subs.find((sub) => sub.u._id === member._id);
const sub = subsByUserId.get(member._id);

const { u: _u, ...subscription } = sub || {};

Expand Down
71 changes: 35 additions & 36 deletions apps/meteor/server/lib/autotranslate/autotranslate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ import { notifyOnMessageChange } from '../notifyListener';

const translationLogger = new Logger('AutoTranslate');

// Constant tokenizer patterns, compiled once instead of on every message.
// Safe to share with /g flags because String.prototype.replace resets
// lastIndex on each call.
const urlSchemes = 'http,https';
const markdownLinkRegex = new RegExp(`(!?\\[)([^\\]]+)(\\]\\((?:${urlSchemes}):\\/\\/[^\\)]+\\))`, 'gm');
const pipedLinkRegex = new RegExp(`((?:<|&lt;)(?:${urlSchemes}):\\/\\/[^\\|]+\\|)(.+?)(?=>|&gt;)((?:>|&gt;))`, 'gm');
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const wrappedParagraphRegex = new RegExp('^\\s*<p>|</p>\\s*$', 'gm');

const Providers = Symbol('Providers');
const Provider = Symbol('Provider');

Expand Down Expand Up @@ -179,47 +187,39 @@ export abstract class AutoTranslate {
tokenizeURLs(message: IMessage): IMessage {
let count = message.tokens?.length || 0;

const schemes = 'http,https';

// Support ![alt text](http://image url) and [text](http://link)
message.msg = message.msg.replace(
new RegExp(`(!?\\[)([^\\]]+)(\\]\\((?:${schemes}):\\/\\/[^\\)]+\\))`, 'gm'),
(_match, pre, text, post) => {
const pretoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: pretoken,
text: pre,
});
message.msg = message.msg.replace(markdownLinkRegex, (_match, pre, text, post) => {
const pretoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: pretoken,
text: pre,
});

const posttoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: posttoken,
text: post,
});
const posttoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: posttoken,
text: post,
});

return pretoken + text + posttoken;
},
);
return pretoken + text + posttoken;
});

// Support <http://link|Text>
message.msg = message.msg.replace(
new RegExp(`((?:<|&lt;)(?:${schemes}):\\/\\/[^\\|]+\\|)(.+?)(?=>|&gt;)((?:>|&gt;))`, 'gm'),
(_match, pre, text, post) => {
const pretoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: pretoken,
text: pre,
});
message.msg = message.msg.replace(pipedLinkRegex, (_match, pre, text, post) => {
const pretoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: pretoken,
text: pre,
});

const posttoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: posttoken,
text: post,
});
const posttoken = `<i class=notranslate>{${count++}}</i>`;
message.tokens?.push({
token: posttoken,
text: post,
});

return pretoken + text + posttoken;
},
);
return pretoken + text + posttoken;
});

return message;
}
Expand All @@ -230,8 +230,7 @@ export abstract class AutoTranslate {
message = Markdown.parseMessageNotEscaped(message);

// Some parsers (e. g. Marked) wrap the complete message in a <p> - this is unnecessary and should be ignored with respect to translations
const regexWrappedParagraph = new RegExp('^\\s*<p>|</p>\\s*$', 'gm');
message.msg = message.msg.replace(regexWrappedParagraph, '');
message.msg = message.msg.replace(wrappedParagraphRegex, '');

for (const [tokenIndex, value] of message.tokens?.entries() ?? []) {
const { token } = value;
Expand Down
9 changes: 6 additions & 3 deletions apps/meteor/server/lib/messages/parseUrlsInMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ import { getMessageUrlRegex } from '../../../lib/getMessageUrlRegex';
import { settings } from '../../settings';
import { Markdown } from '../messaging/markdown';

const prepareUrl = (url: string, previewUrls: string[] | undefined) => ({
const prepareUrl = (url: string, previewUrls: string[] | undefined, siteUrl: string) => ({
url,
meta: {},
...(previewUrls && !previewUrls.includes(url) && !url.includes(settings.get('Site_Url')) && { ignoreParse: true }),
...(previewUrls && !previewUrls.includes(url) && !url.includes(siteUrl) && { ignoreParse: true }),
});

const prepareUrls = (urls: string[], previewUrls?: string[]) => [...new Set(urls)].map((url) => prepareUrl(url, previewUrls));
const prepareUrls = (urls: string[], previewUrls?: string[]) => {
const siteUrl = settings.get<string>('Site_Url');
Comment thread
KevLehman marked this conversation as resolved.
return [...new Set(urls)].map((url) => prepareUrl(url, previewUrls, siteUrl));
};

export const parseUrlsInMessage = (
message: AtLeast<IMessage, 'msg' | 'md'> & {
Expand Down
19 changes: 16 additions & 3 deletions apps/meteor/server/lib/messaging/mentions/Mentions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export class MentionsServer extends MentionsParser {
const mentionsAll: { _id: string; username: string }[] = [];
const userMentions = new Set<string>();

// A message may mention both @all and @here; fetch the member count at
// most once per message instead of once per mention.
let totalChannelMembers: number | undefined;

for (const m of mentions) {
let mention: string;
if (m.includes(':')) {
Expand All @@ -72,9 +76,12 @@ export class MentionsServer extends MentionsParser {
userMentions.add(mention);
continue;
}
if (this.messageMaxAll() > 0 && (await this.getTotalChannelMembers(rid)) > this.messageMaxAll()) {
await this.onMaxRoomMembersExceeded({ sender, rid });
continue;
if (this.messageMaxAll() > 0) {
totalChannelMembers ??= await this.getTotalChannelMembers(rid);
if (totalChannelMembers > this.messageMaxAll()) {
await this.onMaxRoomMembersExceeded({ sender, rid });
continue;
}
}
mentionsAll.push({
_id: mention,
Expand All @@ -96,6 +103,12 @@ export class MentionsServer extends MentionsParser {
}

async convertMentionsToChannels(channels: string[]): Promise<Pick<IRoom, '_id' | 'name' | 'fname' | 'federated'>[]> {
// Most messages don't mention any channel; skip the (empty) database
// query entirely in that case. An empty $in matches nothing anyway.
if (channels.length === 0) {
return [];
}

return this.getChannels(channels.map((c) => (c.startsWith('#') ? c.substring(1) : c)));
}

Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/server/meteor-methods/rooms/browseChannels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ const getChannelsAndGroups = async (

const teamIds = result.map(({ teamId }) => teamId).filter(isTruthy);
const teamsMains = await Team.listByIds([...new Set(teamIds)], { projection: { _id: 1, name: 1 } });
const teamsById = new Map(teamsMains.map((team) => [team._id, team]));

const results = result.map((room) => {
if (room.teamId) {
const team = teamsMains.find((mainRoom) => mainRoom._id === room.teamId);
const team = teamsById.get(room.teamId);
if (team) {
return { ...room, belongsTo: team.name };
}
Expand Down
8 changes: 2 additions & 6 deletions apps/meteor/server/services/messages/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,8 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
}

private getMarkdownConfig() {
const customDomains = settings.get<string>('Message_CustomDomain_AutoLink')
? settings
.get<string>('Message_CustomDomain_AutoLink')
.split(',')
.map((domain) => domain.trim())
: [];
const customDomainAutoLink = settings.get<string>('Message_CustomDomain_AutoLink');
const customDomains = customDomainAutoLink ? customDomainAutoLink.split(',').map((domain) => domain.trim()) : [];

return {
colors: settings.get<boolean>('HexColorPreview_Enabled'),
Expand Down
Loading
Loading