chore: Use transaction for saveUser function (#35203)

Author: gabriellsh

apps/meteor/app/api/server/api.ts

@@ -15,7 +15,6 @@ import { Meteor } from 'meteor/meteor';
 import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
 import { RateLimiter } from 'meteor/rate-limit';
 import { WebApp } from 'meteor/webapp';
-import semver from 'semver';
 import _ from 'underscore';
 
 import type { PermissionsPayload } from './api.helpers';
@@ -45,20 +44,20 @@ import type { Route } from './router';
 import { Router } from './router';
 import { isObject } from '../../../lib/utils/isObject';
 import { getNestedProp } from '../../../server/lib/getNestedProp';
+import { shouldBreakInVersion } from '../../../server/lib/shouldBreakInVersion';
 import { checkCodeForUser } from '../../2fa/server/code';
 import { hasPermissionAsync } from '../../authorization/server/functions/hasPermission';
 import { notifyOnUserChangeAsync } from '../../lib/server/lib/notifyListener';
 import { metrics } from '../../metrics/server';
 import { settings } from '../../settings/server';
-import { Info } from '../../utils/rocketchat.info';
 import { getDefaultUserFields } from '../../utils/server/functions/getDefaultUserFields';
 
 const logger = new Logger('API');
 
 // We have some breaking changes planned to the API.
 // To avoid conflicts or missing something during the period we are adopting a 'feature flag approach'
 // TODO: MAJOR check if this is still needed
-const applyBreakingChanges = semver.gte(Info.version, '8.0.0');
+const applyBreakingChanges = shouldBreakInVersion('8.0.0');
 
 interface IAPIProperties {
 	useDefaultAuth: boolean;

apps/meteor/app/api/server/v1/users.ts

@@ -117,10 +117,6 @@ API.v1.addRoute(
 
 			await saveUser(this.userId, userData);
 
-			if (this.bodyParams.data.customFields) {
-				await saveCustomFields(this.bodyParams.userId, this.bodyParams.data.customFields);
-			}
-
 			if (typeof this.bodyParams.data.active !== 'undefined') {
 				const {
 					userId,

apps/meteor/app/file-upload/server/lib/FileUpload.ts

@@ -17,7 +17,7 @@ import filesize from 'filesize';
 import { Match } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
 import { Cookies } from 'meteor/ostrio:cookies';
-import type { OptionalId } from 'mongodb';
+import type { ClientSession, OptionalId } from 'mongodb';
 import sharp from 'sharp';
 import type { WritableStreamBuffer } from 'stream-buffers';
 import streamBuffers from 'stream-buffers';
@@ -228,7 +228,7 @@ export const FileUpload = {
 
 	defaults,
 
-	async avatarsOnValidate(this: Store, file: IUpload) {
+	async avatarsOnValidate(this: Store, file: IUpload, options?: { session?: ClientSession }) {
 		if (settings.get('Accounts_AvatarResize') !== true) {
 			return;
 		}
@@ -277,6 +277,7 @@ export const FileUpload = {
 					...(['gif', 'svg'].includes(metadata.format || '') ? { type: 'image/png' } : {}),
 				},
 			},
+			options,
 		);
 	},
 
@@ -359,7 +360,7 @@ export const FileUpload = {
 		return store.insert(details, buffer);
 	},
 
-	async uploadsOnValidate(this: Store, file: IUpload) {
+	async uploadsOnValidate(this: Store, file: IUpload, options?: { session?: ClientSession }) {
 		if (!file.type || !/^image\/((x-windows-)?bmp|p?jpeg|png|gif|webp)$/.test(file.type)) {
 			return;
 		}
@@ -409,6 +410,7 @@ export const FileUpload = {
 			{
 				$set: { size, identify },
 			},
+			options,
 		);
 	},
 
@@ -721,13 +723,13 @@ export class FileUploadClass {
 		return modelsAvailable[modelName];
 	}
 
-	async delete(fileId: string) {
+	async delete(fileId: string, options?: { session?: ClientSession }) {
 		// TODO: Remove this method
 		if (this.store?.delete) {
-			await this.store.delete(fileId);
+			await this.store.delete(fileId, { session: options?.session });
 		}
 
-		return this.model.deleteFile(fileId);
+		return this.model.deleteFile(fileId, { session: options?.session });
 	}
 
 	async deleteById(fileId: string) {
@@ -742,8 +744,8 @@ export class FileUploadClass {
 		return store.delete(file._id);
 	}
 
-	async deleteByName(fileName: string) {
-		const file = await this.model.findOneByName(fileName);
+	async deleteByName(fileName: string, options?: { session?: ClientSession }) {
+		const file = await this.model.findOneByName(fileName, { session: options?.session });
 
 		if (!file) {
 			return;
@@ -766,8 +768,12 @@ export class FileUploadClass {
 		return store.delete(file._id);
 	}
 
-	async _doInsert(fileData: OptionalId<IUpload>, streamOrBuffer: ReadableStream | stream | Buffer): Promise<IUpload> {
-		const fileId = await this.store.create(fileData);
+	async _doInsert(
+		fileData: OptionalId<IUpload>,
+		streamOrBuffer: ReadableStream | stream | Buffer,
+		options?: { session?: ClientSession },
+	): Promise<IUpload> {
+		const fileId = await this.store.create(fileData, { session: options?.session });
 		const tmpFile = UploadFS.getTempFilePath(fileId);
 
 		try {
@@ -779,15 +785,19 @@ export class FileUploadClass {
 				throw new Error('Invalid file type');
 			}
 
-			const file = await ufsComplete(fileId, this.name);
+			const file = await ufsComplete(fileId, this.name, { session: options?.session });
 
 			return file;
 		} catch (e: any) {
 			throw e;
 		}
 	}
 
-	async insert(fileData: OptionalId<IUpload>, streamOrBuffer: ReadableStream | stream.Readable | Buffer) {
+	async insert(
+		fileData: OptionalId<IUpload>,
+		streamOrBuffer: ReadableStream | stream.Readable | Buffer,
+		options?: { session?: ClientSession },
+	) {
 		if (streamOrBuffer instanceof stream) {
 			streamOrBuffer = await streamToBuffer(streamOrBuffer);
 		}
@@ -803,6 +813,6 @@ export class FileUploadClass {
 			await filter.check(fileData, streamOrBuffer);
 		}
 
-		return this._doInsert(fileData, streamOrBuffer);
+		return this._doInsert(fileData, streamOrBuffer, { session: options?.session });
 	}
 }

apps/meteor/app/lib/server/functions/saveCustomFields.ts

@@ -1,13 +1,21 @@
+import type { IUser } from '@rocket.chat/core-typings';
+import type { Updater } from '@rocket.chat/models';
+import type { ClientSession } from 'mongodb';
+
 import { saveCustomFieldsWithoutValidation } from './saveCustomFieldsWithoutValidation';
 import { validateCustomFields } from './validateCustomFields';
 import { trim } from '../../../../lib/utils/stringUtils';
 import { settings } from '../../../settings/server';
 
-export const saveCustomFields = async function (userId: string, formData: Record<string, any>): Promise<void> {
+export const saveCustomFields = async function (
+	userId: string,
+	formData: Record<string, any>,
+	options?: { _updater?: Updater<IUser>; session?: ClientSession },
+): Promise<void> {
 	if (trim(settings.get('Accounts_CustomFields')).length === 0) {
 		return;
 	}
 
 	validateCustomFields(formData);
-	return saveCustomFieldsWithoutValidation(userId, formData);
+	return saveCustomFieldsWithoutValidation(userId, formData, options);
 };

apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts

@@ -1,7 +1,11 @@
+import type { IUser } from '@rocket.chat/core-typings';
+import type { Updater } from '@rocket.chat/models';
 import { Subscriptions, Users } from '@rocket.chat/models';
 import { Meteor } from 'meteor/meteor';
+import type { ClientSession } from 'mongodb';
 
 import { trim } from '../../../../lib/utils/stringUtils';
+import { onceTransactionCommitedSuccessfully } from '../../../../server/database/utils';
 import { settings } from '../../../settings/server';
 import { notifyOnSubscriptionChangedByUserIdAndRoomType } from '../lib/notifyListener';
 
@@ -12,7 +16,14 @@ const getCustomFieldsMeta = function (customFieldsMeta: string) {
 		throw new Meteor.Error('error-invalid-customfield-json', 'Invalid JSON for Custom Fields');
 	}
 };
-export const saveCustomFieldsWithoutValidation = async function (userId: string, formData: Record<string, any>): Promise<void> {
+export const saveCustomFieldsWithoutValidation = async function (
+	userId: string,
+	formData: Record<string, any>,
+	options?: {
+		_updater?: Updater<IUser>;
+		session?: ClientSession;
+	},
+): Promise<void> {
 	const customFieldsSetting = settings.get<string>('Accounts_CustomFields');
 	if (!customFieldsSetting || trim(customFieldsSetting).length === 0) {
 		return;
@@ -29,7 +40,9 @@ export const saveCustomFieldsWithoutValidation = async function (userId: string,
 		{},
 	);
 
-	const updater = Users.getUpdater();
+	const { _updater, session } = options || {};
+
+	const updater = _updater || Users.getUpdater();
 
 	updater.set('customFields', customFields);
 
@@ -48,11 +61,14 @@ export const saveCustomFieldsWithoutValidation = async function (userId: string,
 		}
 	});
 
-	await Users.updateFromUpdater({ _id: userId }, updater);
-
-	// Update customFields of all Direct Messages' Rooms for userId
-	const setCustomFieldsResponse = await Subscriptions.setCustomFieldsDirectMessagesByUserId(userId, customFields);
-	if (setCustomFieldsResponse.modifiedCount) {
-		void notifyOnSubscriptionChangedByUserIdAndRoomType(userId, 'd');
+	if (!_updater) {
+		await Users.updateFromUpdater({ _id: userId }, updater, { session });
 	}
+
+	await onceTransactionCommitedSuccessfully(async () => {
+		const setCustomFieldsResponse = await Subscriptions.setCustomFieldsDirectMessagesByUserId(userId, customFields);
+		if (setCustomFieldsResponse.modifiedCount) {
+			void notifyOnSubscriptionChangedByUserIdAndRoomType(userId, 'd');
+		}
+	}, session);
 };