chore: remove persists from updater

apps/meteor/app/federation/server/endpoints/dispatch.js

@@ -296,7 +296,7 @@ const eventHandlers = {
 					const roomUpdater = Rooms.getUpdater();
 					await notifyUsersOnMessage(denormalizedMessage, room, roomUpdater);
 					if (roomUpdater.hasChanges()) {
-						await roomUpdater.persist({ _id: room._id });
+						await Rooms.updateFromUpdater({ _id: room._id }, roomUpdater);
 					}
 
 					sendAllNotifications(denormalizedMessage, room);

apps/meteor/app/lib/server/lib/notifyUsersOnMessage.ts

@@ -188,7 +188,7 @@ callbacks.add(
 		await notifyUsersOnMessage(message, room, roomUpdater);
 
 		if (roomUpdater.hasChanges()) {
-			await roomUpdater.persist({ _id: room._id });
+			await Rooms.updateFromUpdater({ _id: room._id }, roomUpdater);
 		}
 
 		return message;

apps/meteor/app/livechat/server/hooks/afterSaveOmnichannelMessage.ts

@@ -14,7 +14,7 @@ callbacks.add(
 		const result = await callbacks.run('afterOmnichannelSaveMessage', message, { room, roomUpdater: updater });
 
 		if (updater.hasChanges()) {
-			await updater.persist({ _id: room._id });
+			await LivechatRooms.updateFromUpdater({ _id: room._id }, updater);
 		}
 
 		return result;

apps/meteor/server/models/dummy/BaseDummy.ts

@@ -42,7 +42,11 @@ export class BaseDummy<
 	}
 
 	public getUpdater(): Updater<T> {
-		return new UpdaterImpl<T>(this.col as unknown as IBaseModel<T>);
+		return new UpdaterImpl<T>();
+	}
+
+	public updateFromUpdater(query: Filter<T>, updater: Updater<T>): Promise<UpdateResult> {
+		return this.updateOne(query, updater);
 	}
 
 	getCollectionName(): string {

apps/meteor/server/models/raw/BaseRaw.ts

@@ -111,7 +111,15 @@ export abstract class BaseRaw<
 	}
 
 	public getUpdater(): Updater<T> {
-		return new UpdaterImpl<T>(this.col as unknown as IBaseModel<T>);
+		return new UpdaterImpl<T>();
+	}
+
+	public updateFromUpdater(query: Filter<T>, updater: Updater<T>): Promise<UpdateResult> {
+		const updateFilter = updater.getUpdateFilter();
+		return this.updateOne(query, updateFilter).catch((e) => {
+			console.warn(e, updateFilter);
+			return Promise.reject(e);
+		});
 	}
 
 	private doNotMixInclusionAndExclusionFields(options: FindOptions<T> = {}): FindOptions<T> {

packages/model-typings/src/models/IBaseModel.ts

@@ -51,6 +51,7 @@ export interface IBaseModel<
 
 	getCollectionName(): string;
 	getUpdater(): Updater<T>;
+	updateFromUpdater(query: Filter<T>, updater: Updater<T>): Promise<UpdateResult>;
 
 	findOneAndUpdate(query: Filter<T>, update: UpdateFilter<T> | T, options?: FindOneAndUpdateOptions): Promise<ModifyResult<T>>;
 

packages/model-typings/src/updater.ts

@@ -1,12 +1,11 @@
 /* eslint-disable @typescript-eslint/naming-convention */
-import type { Join, NestedPaths, PropertyType, ArrayElement, NestedPathsOfType, Filter, UpdateFilter } from 'mongodb';
+import type { Join, NestedPaths, PropertyType, ArrayElement, NestedPathsOfType, UpdateFilter } from 'mongodb';
 
 export interface Updater<T extends { _id: string }> {
 	set<P extends SetProps<T>, K extends keyof P>(key: K, value: P[K]): Updater<T>;
 	unset<K extends keyof UnsetProps<T>>(key: K): Updater<T>;
 	inc<K extends keyof IncProps<T>>(key: K, value: number): Updater<T>;
 	addToSet<K extends keyof AddToSetProps<T>>(key: K, value: ArrayElementType<AddToSetProps<T>[K]>): Updater<T>;
-	persist(query: Filter<T>): Promise<void>;
 	hasChanges(): boolean;
 	getUpdateFilter(): UpdateFilter<T>;
 }

packages/models/src/updater.spec.ts

@@ -15,194 +15,138 @@ test('updater typings', () => {
 			e: string;
 		};
 		e: string[];
-	}>({} as any);
+	}>();
 
-	const omnichannel = new UpdaterImpl<IOmnichannelRoom>({} as any);
-	omnichannel.addToSet('v.activity', 'asd');
-	// @ts-expect-error
-	omnichannel.addToSet('v.activity', 1);
-	// @ts-expect-error
-	omnichannel.addToSet('v.activity', {
-		asdas: 1,
-	});
-
-	// @ts-expect-error
-	omnichannel.addToSet('v.activity.asd', {
-		asdas: 1,
-	});
-
-	updater.addToSet('e', 'a');
-
-	// @ts-expect-error
-	updater.addToSet('e', 1);
-	// @ts-expect-error
-	updater.addToSet('a', 'b');
-
-	// @ts-expect-error
-	updater.set('njame', 1);
-	// @ts-expect-error
-	updater.set('ttes', 1);
-	// @ts-expect-error
+	// @ts-expect-error: it should not allow any string to `t` only `l` is allowed
 	updater.set('t', 'a');
+	// `l` is allowed
 	updater.set('t', 'l');
-	// @ts-expect-error
-	updater.set('a', 'b');
-	// @ts-expect-error
-	updater.set('c', 'b');
-	updater.set('c', 1);
 
-	updater.set('a', {
-		b: 'set',
-	});
+	// `a` is { b: string }
+	updater.set('a', { b: 'test' });
 	updater.set('a.b', 'test');
-
-	// @ts-expect-error
+	// @ts-expect-error: it should not allow strings to `a`, a is an object containing `b: string`
+	updater.set('a', 'b');
+	// @ts-expect-error: `a` is not optional so unset is not allowed
 	updater.unset('a');
+	// @ts-expect-error: strings cannot be incremented
+	updater.inc('a', 1);
 
+	// `c` is number but it should be optional, so unset is allowed
 	updater.unset('c');
+	updater.set('c', 1);
+	// @ts-expect-error: `c` is a number
+	updater.set('c', 'b');
+	// inc is allowed for numbers
+	updater.inc('c', 1);
 
+	// `d` is { e: string } but it should be optional, so unset is allowed
 	updater.unset('d');
+	updater.set('d', { e: 'a' });
+	// @ts-expect-error: `d` is an object
+	updater.set('d', 'a');
+
+	// @ts-expect-error: it should not allow numbers, since e is a string
+	updater.addToSet('e', 1);
+	// @ts-expect-error: it should not allow strings, since a is an object
+	updater.addToSet('a', 'b');
 
+	updater.addToSet('e', 'a');
+	// @ts-expect-error: it should not allow `njame` its not specified in the model
+	updater.set('njame', 1);
+
+	// `d` is { e: string } and also it should be optional, so unset is allowed
 	updater.unset('d.e');
-	// @ts-expect-error
+	// @ts-expect-error: `d` is an object cannot be incremented
 	updater.inc('d', 1);
-	updater.inc('c', 1);
+
+	// `activity` is a string
+	const omnichannel = new UpdaterImpl<IOmnichannelRoom>();
+	omnichannel.addToSet('v.activity', 'asd');
+	// @ts-expect-error: it should not allow numbers, since activity is a string
+	omnichannel.addToSet('v.activity', 1);
+	// @ts-expect-error: it should not allow objects, since activity is a string
+	omnichannel.addToSet('v.activity', {
+		asdas: 1,
+	});
+	// @ts-expect-error: it should not allow sub properties, since activity is a string
+	omnichannel.addToSet('v.activity.asd', {
+		asdas: 1,
+	});
 });
 
 test('updater $set operations', async () => {
-	const updateOne = jest.fn();
-
 	const updater = new UpdaterImpl<{
 		_id: string;
 		t: 'l';
 		a: {
 			b: string;
 		};
 		c?: number;
-	}>({
-		updateOne,
-	} as any);
+	}>();
 
 	updater.set('a', {
 		b: 'set',
 	});
 
-	await updater.persist({
-		_id: 'test',
-	});
-
-	expect(updateOne).toBeCalledWith(
-		{
-			_id: 'test',
-		},
-		{ $set: { a: { b: 'set' } } },
-	);
+	expect(updater.getUpdateFilter()).toEqual({ $set: { a: { b: 'set' } } });
 });
 
 test('updater $unset operations', async () => {
-	const updateOne = jest.fn();
-
 	const updater = new UpdaterImpl<{
 		_id: string;
 		t: 'l';
 		a: {
 			b: string;
 		};
 		c?: number;
-	}>({
-		updateOne,
-	} as any);
-
+	}>();
 	updater.unset('c');
-
-	await updater.persist({
-		_id: 'test',
-	});
-
-	expect(updateOne).toBeCalledWith(
-		{
-			_id: 'test',
-		},
-		{ $unset: { c: 1 } },
-	);
+	expect(updater.getUpdateFilter()).toEqual({ $unset: { c: 1 } });
 });
 
 test('updater inc multiple operations', async () => {
-	const updateOne = jest.fn();
-
 	const updater = new UpdaterImpl<{
 		_id: string;
 		t: 'l';
 		a: {
 			b: string;
 		};
 		c?: number;
-	}>({
-		updateOne,
-	} as any);
+	}>();
 
 	updater.inc('c', 1);
 	updater.inc('c', 1);
 
-	await updater.persist({
-		_id: 'test',
-	});
-
-	expect(updateOne).toBeCalledWith(
-		{
-			_id: 'test',
-		},
-		{ $inc: { c: 2 } },
-	);
+	expect(updater.getUpdateFilter()).toEqual({ $inc: { c: 2 } });
 });
 
 test('it should add items to array', async () => {
-	const updateOne = jest.fn();
 	const updater = new UpdaterImpl<{
 		_id: string;
 		a: string[];
-	}>({
-		updateOne,
-	} as any);
+	}>();
 
 	updater.addToSet('a', 'b');
 	updater.addToSet('a', 'c');
 
-	await updater.persist({
-		_id: 'test',
-	});
-
-	expect(updateOne).toBeCalledWith(
-		{
-			_id: 'test',
-		},
-		{ $addToSet: { a: { $each: ['b', 'c'] } } },
-	);
+	expect(updater.getUpdateFilter()).toEqual({ $addToSet: { a: { $each: ['b', 'c'] } } });
 });
 
-test('it should persist only once', async () => {
-	const updateOne = jest.fn();
-
+test('it should getUpdateFilter only once', async () => {
 	const updater = new UpdaterImpl<{
 		_id: string;
 		t: 'l';
 		a: {
 			b: string;
 		};
 		c?: number;
-	}>({
-		updateOne,
-	} as any);
+	}>();
 
 	updater.set('a', {
 		b: 'set',
 	});
 
-	await updater.persist({
-		_id: 'test',
-	});
-
-	expect(updateOne).toBeCalledTimes(1);
-
-	expect(() => updater.persist({ _id: 'test' })).rejects.toThrow();
+	expect(updater.getUpdateFilter()).toEqual({ $set: { a: { b: 'set' } } });
+	expect(() => updater.getUpdateFilter()).toThrow();
 });