From 006a148ec315e00be703f8a0a18dddb403f7cd48 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Wed, 22 Mar 2023 17:59:32 -0600 Subject: [PATCH] refactor: Don't use meteor collections on migrations (#28563) --- apps/meteor/app/api/server/v1/push.ts | 29 +++++------ .../server/NotificationQueue.ts | 8 +-- .../server/lib/PushNotification.ts | 4 +- apps/meteor/app/push/server/index.ts | 2 +- apps/meteor/app/push/server/methods.ts | 47 ++++++++++-------- apps/meteor/app/push/server/push.js | 49 ++++++++++--------- apps/meteor/server/lib/pushConfig.ts | 7 +-- apps/meteor/server/models/AppsTokens.ts | 6 +++ apps/meteor/server/models/raw/AppsTokens.ts | 11 +++++ apps/meteor/server/models/startup.ts | 1 + apps/meteor/server/startup/migrations/v269.ts | 13 +++-- apps/meteor/server/startup/migrations/v273.ts | 5 +- apps/meteor/server/startup/migrations/v276.ts | 6 +-- packages/core-typings/src/AppsTokens.ts | 13 +++++ packages/core-typings/src/index.ts | 1 + packages/model-typings/src/index.ts | 1 + .../src/models/IAppsTokensModel.ts | 5 ++ packages/models/src/index.ts | 2 + 18 files changed, 132 insertions(+), 78 deletions(-) create mode 100644 apps/meteor/server/models/AppsTokens.ts create mode 100644 apps/meteor/server/models/raw/AppsTokens.ts create mode 100644 packages/core-typings/src/AppsTokens.ts create mode 100644 packages/model-typings/src/models/IAppsTokensModel.ts diff --git a/apps/meteor/app/api/server/v1/push.ts b/apps/meteor/app/api/server/v1/push.ts index aafb3e4d49a..96e060746ce 100644 --- a/apps/meteor/app/api/server/v1/push.ts +++ b/apps/meteor/app/api/server/v1/push.ts @@ -1,9 +1,8 @@ import { Meteor } from 'meteor/meteor'; import { Random } from '@rocket.chat/random'; import { Match, check } from 'meteor/check'; -import { Messages } from '@rocket.chat/models'; +import { Messages, AppsTokens } from '@rocket.chat/models'; -import { appTokensCollection } from '../../../push/server'; import { API } from '../api'; import PushNotification from '../../../push-notifications/server/lib/PushNotification'; import { canAccessRoomAsync } from '../../../authorization/server/functions/canAccessRoom'; @@ -46,24 +45,26 @@ API.v1.addRoute( return API.v1.success({ result }); }, - delete() { + async delete() { const { token } = this.bodyParams; if (!token || typeof token !== 'string') { throw new Meteor.Error('error-token-param-not-valid', 'The required "token" body param is missing or invalid.'); } - const affectedRecords = appTokensCollection.remove({ - $or: [ - { - 'token.apn': token, - }, - { - 'token.gcm': token, - }, - ], - userId: this.userId, - }); + const affectedRecords = ( + await AppsTokens.deleteMany({ + $or: [ + { + 'token.apn': token, + }, + { + 'token.gcm': token, + }, + ], + userId: this.userId, + }) + ).deletedCount; if (affectedRecords === 0) { return API.v1.notFound(); diff --git a/apps/meteor/app/notification-queue/server/NotificationQueue.ts b/apps/meteor/app/notification-queue/server/NotificationQueue.ts index c750751bb7b..3660923a5b4 100644 --- a/apps/meteor/app/notification-queue/server/NotificationQueue.ts +++ b/apps/meteor/app/notification-queue/server/NotificationQueue.ts @@ -68,10 +68,10 @@ class NotificationClass { } try { - for (const item of notification.items) { + for await (const item of notification.items) { switch (item.type) { case 'push': - this.push(notification, item); + await this.push(notification, item); break; case 'email': this.email(item); @@ -98,8 +98,8 @@ class NotificationClass { return NotificationQueue.findNextInQueueOrExpired(expired); } - push({ uid, rid, mid }: INotification, item: INotificationItemPush): void { - PushNotification.send({ + async push({ uid, rid, mid }: INotification, item: INotificationItemPush): Promise { + await PushNotification.send({ rid, uid, mid, diff --git a/apps/meteor/app/push-notifications/server/lib/PushNotification.ts b/apps/meteor/app/push-notifications/server/lib/PushNotification.ts index 5cf098dd136..83540e12467 100644 --- a/apps/meteor/app/push-notifications/server/lib/PushNotification.ts +++ b/apps/meteor/app/push-notifications/server/lib/PushNotification.ts @@ -95,7 +95,7 @@ class PushNotification { return config; } - send({ rid, uid, mid, roomName, username, message, payload, badge = 1, category }: PushNotificationData): void { + async send({ rid, uid, mid, roomName, username, message, payload, badge = 1, category }: PushNotificationData): Promise { const idOnly = settings.get('Push_request_content_from_server'); const config = this.getNotificationConfig({ rid, @@ -111,7 +111,7 @@ class PushNotification { }); metrics.notificationsSent.inc({ notification_type: 'mobile' }); - Push.send(config); + await Push.send(config); } async getNotificationForMessageId({ diff --git a/apps/meteor/app/push/server/index.ts b/apps/meteor/app/push/server/index.ts index 32ac42bd317..4cfcaa2da26 100644 --- a/apps/meteor/app/push/server/index.ts +++ b/apps/meteor/app/push/server/index.ts @@ -1,3 +1,3 @@ import './methods'; -export { Push, appTokensCollection } from './push'; +export { Push } from './push'; diff --git a/apps/meteor/app/push/server/methods.ts b/apps/meteor/app/push/server/methods.ts index 7463a3a0aa3..324656306c5 100644 --- a/apps/meteor/app/push/server/methods.ts +++ b/apps/meteor/app/push/server/methods.ts @@ -3,9 +3,10 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; import { Random } from '@rocket.chat/random'; import type { ServerMethods } from '@rocket.chat/ui-contexts'; -import type { Mongo } from 'meteor/mongo'; +import { AppsTokens } from '@rocket.chat/models'; +import type { IAppsTokens } from '@rocket.chat/core-typings'; -import { _matchToken, appTokensCollection } from './push'; +import { _matchToken } from './push'; import { logger } from './logger'; declare module '@rocket.chat/ui-contexts' { @@ -18,13 +19,13 @@ declare module '@rocket.chat/ui-contexts' { appName: string; userId?: string; metadata?: Record; - }): void; - 'raix:push-setuser'(options: { id: string; userId: string }): boolean; + }): Promise>; + 'raix:push-setuser'(options: { id: string; userId: string }): Promise; } } Meteor.methods({ - 'raix:push-update'(options) { + async 'raix:push-update'(options) { logger.debug('Got push token from app:', options); check(options, { @@ -48,15 +49,15 @@ Meteor.methods({ // lookup app by id if one was included if (options.id) { - doc = appTokensCollection.findOne({ _id: options.id }); + doc = await AppsTokens.findOne({ _id: options.id }); } else if (options.userId) { - doc = appTokensCollection.findOne({ userId: options.userId }); + doc = await AppsTokens.findOne({ userId: options.userId }); } // No doc was found - we check the database to see if // we can find a match for the app via token and appName if (!doc) { - doc = appTokensCollection.findOne({ + doc = await AppsTokens.findOne({ $and: [ { token: options.token }, // Match token { appName: options.appName }, // Match appName @@ -76,6 +77,7 @@ Meteor.methods({ enabled: true, createdAt: new Date(), updatedAt: new Date(), + metadata: options.metadata || {}, // XXX: We might want to check the id - Why isnt there a match for id // in the Meteor check... Normal length 17 (could be larger), and @@ -86,10 +88,10 @@ Meteor.methods({ // we respect this and try to create a document with the selected id; }; - (appTokensCollection as Mongo.Collection & { _collection: Mongo.Collection })._collection.insert(doc); + await AppsTokens.insertOne(doc); } else { // We found the app so update the updatedAt and set the token - appTokensCollection.update( + await AppsTokens.updateOne( { _id: doc._id }, { $set: { @@ -102,14 +104,16 @@ Meteor.methods({ } if (doc.token) { - const removed = appTokensCollection.remove({ - $and: [ - { _id: { $ne: doc._id } }, - { token: doc.token }, // Match token - { appName: doc.appName }, // Match appName - { token: { $exists: true } }, // Make sure token exists - ], - }); + const removed = ( + await AppsTokens.deleteMany({ + $and: [ + { _id: { $ne: doc._id } }, + { token: doc.token }, // Match token + { appName: doc.appName }, // Match appName + { token: { $exists: true } }, // Make sure token exists + ], + }) + ).deletedCount; if (removed) { logger.debug(`Removed ${removed} existing app items`); @@ -122,11 +126,14 @@ Meteor.methods({ return doc; }, // Deprecated - 'raix:push-setuser'(id) { + async 'raix:push-setuser'(id) { check(id, String); + if (!this.userId) { + throw new Meteor.Error(403, 'Forbidden access'); + } logger.debug(`Settings userId "${this.userId}" for app:`, id); - const found = appTokensCollection.update({ _id: id }, { $set: { userId: this.userId } }); + const found = await AppsTokens.updateOne({ _id: id }, { $set: { userId: this.userId } }); return !!found; }, diff --git a/apps/meteor/app/push/server/push.js b/apps/meteor/app/push/server/push.js index c32858d343f..de4be5207b0 100644 --- a/apps/meteor/app/push/server/push.js +++ b/apps/meteor/app/push/server/push.js @@ -1,8 +1,8 @@ import { Meteor } from 'meteor/meteor'; import { Match, check } from 'meteor/check'; -import { Mongo } from 'meteor/mongo'; import { HTTP } from 'meteor/http'; import _ from 'underscore'; +import { AppsTokens } from '@rocket.chat/models'; import { initAPN, sendAPN } from './apn'; import { sendGCM } from './gcm'; @@ -10,7 +10,6 @@ import { logger } from './logger'; import { settings } from '../../settings/server'; export const _matchToken = Match.OneOf({ apn: String }, { gcm: String }); -export const appTokensCollection = new Mongo.Collection('_raix_push_app_tokens'); class PushClass { options = {}; @@ -64,11 +63,11 @@ class PushClass { } _replaceToken(currentToken, newToken) { - appTokensCollection.rawCollection().updateMany({ token: currentToken }, { $set: { token: newToken } }); + void AppsTokens.updateMany({ token: currentToken }, { $set: { token: newToken } }); } _removeToken(token) { - appTokensCollection.rawCollection().deleteOne({ token }); + void AppsTokens.deleteOne({ token }); } _shouldUseGateway() { @@ -123,16 +122,18 @@ class PushClass { return HTTP.post(`${gateway}/push/${service}/send`, data, (error, response) => { if (response?.statusCode === 406) { logger.info('removing push token', token); - appTokensCollection.remove({ - $or: [ - { - 'token.apn': token, - }, - { - 'token.gcm': token, - }, - ], - }); + Promise.await( + AppsTokens.deleteMany({ + $or: [ + { + 'token.apn': token, + }, + { + 'token.gcm': token, + }, + ], + }), + ); return; } @@ -180,7 +181,7 @@ class PushClass { } } - sendNotification(notification = { badge: 0 }) { + async sendNotification(notification = { badge: 0 }) { logger.debug('Sending notification', notification); const countApn = []; @@ -203,7 +204,7 @@ class PushClass { $or: [{ 'token.apn': { $exists: true } }, { 'token.gcm': { $exists: true } }], }; - appTokensCollection.find(query).forEach((app) => { + await AppsTokens.find(query).forEach((app) => { logger.debug('send to token', app.token); if (this._shouldUseGateway()) { @@ -219,16 +220,16 @@ class PushClass { // Add some verbosity about the send result, making sure the developer // understands what just happened. if (!countApn.length && !countGcm.length) { - if (appTokensCollection.find().count() === 0) { - logger.debug('GUIDE: The "appTokensCollection" is empty - No clients have registered on the server yet...'); + if ((await AppsTokens.col.estimatedDocumentCount()) === 0) { + logger.debug('GUIDE: The "AppsTokens" is empty - No clients have registered on the server yet...'); } } else if (!countApn.length) { - if (appTokensCollection.find({ 'token.apn': { $exists: true } }).count() === 0) { - logger.debug('GUIDE: The "appTokensCollection" - No APN clients have registered on the server yet...'); + if ((await AppsTokens.col.countDocuments({ 'token.apn': { $exists: true } })) === 0) { + logger.debug('GUIDE: The "AppsTokens" - No APN clients have registered on the server yet...'); } } else if (!countGcm.length) { - if (appTokensCollection.find({ 'token.gcm': { $exists: true } }).count() === 0) { - logger.debug('GUIDE: The "appTokensCollection" - No GCM clients have registered on the server yet...'); + if ((await AppsTokens.col.countDocuments({ 'token.gcm': { $exists: true } })) === 0) { + logger.debug('GUIDE: The "AppsTokens" - No GCM clients have registered on the server yet...'); } } } @@ -289,7 +290,7 @@ class PushClass { } } - send(options) { + async send(options) { // If on the client we set the user id - on the server we need an option // set or we default to "" as the creator of the notification // If current user not set see if we can set it to the logged in user @@ -344,7 +345,7 @@ class PushClass { this._validateDocument(notification); try { - this.sendNotification(notification); + await this.sendNotification(notification); } catch (error) { logger.debug(`Could not send notification id: "${notification._id}", Error: ${error.message}`); logger.debug(error.stack); diff --git a/apps/meteor/server/lib/pushConfig.ts b/apps/meteor/server/lib/pushConfig.ts index 4bfd3f2432c..d7b9d06cc75 100644 --- a/apps/meteor/server/lib/pushConfig.ts +++ b/apps/meteor/server/lib/pushConfig.ts @@ -1,11 +1,12 @@ import { Meteor } from 'meteor/meteor'; import { TAPi18n } from 'meteor/rocketchat:tap-i18n'; import type { ServerMethods } from '@rocket.chat/ui-contexts'; +import { AppsTokens } from '@rocket.chat/models'; import { getWorkspaceAccessToken } from '../../app/cloud/server'; import { hasPermissionAsync } from '../../app/authorization/server/functions/hasPermission'; import { settings } from '../../app/settings/server'; -import { appTokensCollection, Push } from '../../app/push/server'; +import { Push } from '../../app/push/server'; declare module '@rocket.chat/ui-contexts' { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -58,7 +59,7 @@ Meteor.methods({ ], }; - const tokens = appTokensCollection.find(query).count(); + const tokens = await AppsTokens.col.countDocuments(query); if (tokens === 0) { throw new Meteor.Error('error-no-tokens-for-this-user', 'There are no tokens for this user', { @@ -66,7 +67,7 @@ Meteor.methods({ }); } - Push.send({ + await Push.send({ from: 'push', title: `@${user.username}`, text: TAPi18n.__('This_is_a_push_test_messsage'), diff --git a/apps/meteor/server/models/AppsTokens.ts b/apps/meteor/server/models/AppsTokens.ts new file mode 100644 index 00000000000..559545b445b --- /dev/null +++ b/apps/meteor/server/models/AppsTokens.ts @@ -0,0 +1,6 @@ +import { registerModel } from '@rocket.chat/models'; + +import { db } from '../database/utils'; +import { AppsTokens } from './raw/AppsTokens'; + +registerModel('IAppsTokensModel', new AppsTokens(db)); diff --git a/apps/meteor/server/models/raw/AppsTokens.ts b/apps/meteor/server/models/raw/AppsTokens.ts new file mode 100644 index 00000000000..c1c80322b76 --- /dev/null +++ b/apps/meteor/server/models/raw/AppsTokens.ts @@ -0,0 +1,11 @@ +import type { Db } from 'mongodb'; +import type { IAppsTokens } from '@rocket.chat/core-typings'; +import type { IAppsTokensModel } from '@rocket.chat/model-typings'; + +import { BaseRaw } from './BaseRaw'; + +export class AppsTokens extends BaseRaw implements IAppsTokensModel { + constructor(db: Db) { + super(db, '_raix_push_app_tokens', undefined, { collectionNameResolver: (name) => name }); + } +} diff --git a/apps/meteor/server/models/startup.ts b/apps/meteor/server/models/startup.ts index 3915bcbf212..165128bea98 100644 --- a/apps/meteor/server/models/startup.ts +++ b/apps/meteor/server/models/startup.ts @@ -59,3 +59,4 @@ import './WebdavAccounts'; import './FederationRoomEvents'; import './Imports'; import './RawImports'; +import './AppsTokens'; diff --git a/apps/meteor/server/startup/migrations/v269.ts b/apps/meteor/server/startup/migrations/v269.ts index 220e5c2cac8..6cacbbf10ac 100644 --- a/apps/meteor/server/startup/migrations/v269.ts +++ b/apps/meteor/server/startup/migrations/v269.ts @@ -1,10 +1,15 @@ +import { Rooms } from '@rocket.chat/models'; + import { addMigration } from '../../lib/migrations'; -import { Rooms } from '../../../app/models/server'; addMigration({ version: 269, - up() { - Rooms.tryDropIndex({ 'tokenpass.tokens.token': 1 }); - Rooms.tryDropIndex({ tokenpass: 1 }); + async up() { + try { + await Rooms.col.dropIndex('tokenpass.tokens.token_1'); + await Rooms.col.dropIndex('tokenpass_1'); + } catch (e) { + console.log(e); + } }, }); diff --git a/apps/meteor/server/startup/migrations/v273.ts b/apps/meteor/server/startup/migrations/v273.ts index 52eb1580a59..f731f71cb7d 100644 --- a/apps/meteor/server/startup/migrations/v273.ts +++ b/apps/meteor/server/startup/migrations/v273.ts @@ -1,11 +1,12 @@ -import { appTokensCollection } from '../../../app/push/server/push'; +import { AppsTokens } from '@rocket.chat/models'; + import { addMigration } from '../../lib/migrations'; addMigration({ version: 273, async up() { try { - await appTokensCollection.rawCollection().dropIndex('userId_1'); + await AppsTokens.col.dropIndex('userId_1'); return; } catch (error: unknown) { console.warn('Error dropping index for _raix_push_app_tokens, continuing...'); diff --git a/apps/meteor/server/startup/migrations/v276.ts b/apps/meteor/server/startup/migrations/v276.ts index 7363daa5981..f7860b18c8b 100644 --- a/apps/meteor/server/startup/migrations/v276.ts +++ b/apps/meteor/server/startup/migrations/v276.ts @@ -1,17 +1,15 @@ -import { Settings } from '@rocket.chat/models'; +import { Settings, Users } from '@rocket.chat/models'; import { addMigration } from '../../lib/migrations'; -import { Users } from '../../../app/models/server'; addMigration({ version: 276, async up() { - Users.update( + await Users.updateMany( { 'settings.preferences.enableNewMessageTemplate': { $exists: 1 } }, { $unset: { 'settings.preferences.enableNewMessageTemplate': 1 }, }, - { multi: true }, ); await Settings.removeById('Accounts_Default_User_Preferences_enableNewMessageTemplate'); }, diff --git a/packages/core-typings/src/AppsTokens.ts b/packages/core-typings/src/AppsTokens.ts new file mode 100644 index 00000000000..e7efebe0f44 --- /dev/null +++ b/packages/core-typings/src/AppsTokens.ts @@ -0,0 +1,13 @@ +import type { IRocketChatRecord } from './IRocketChatRecord'; + +// The above to a ts interface +export interface IAppsTokens extends IRocketChatRecord { + token: string; + authToken: string; + appName: string; + userId: string | null; + metadata: Record; + enabled: boolean; + createdAt: Date; + updatedAt: Date; +} diff --git a/packages/core-typings/src/index.ts b/packages/core-typings/src/index.ts index b77b5924538..33003e6ad96 100644 --- a/packages/core-typings/src/index.ts +++ b/packages/core-typings/src/index.ts @@ -122,3 +122,4 @@ export * from './SpotlightUser'; export * from './search'; export * from './omnichannel'; +export * from './AppsTokens'; diff --git a/packages/model-typings/src/index.ts b/packages/model-typings/src/index.ts index a093d9503ae..e8a053e6fb5 100644 --- a/packages/model-typings/src/index.ts +++ b/packages/model-typings/src/index.ts @@ -70,3 +70,4 @@ export * from './models/IAppsPersistenceModel'; export * from './models/IImportsModel'; export * from './models/IRawImportsModel'; export * from './models/IFederationRoomEventsModel'; +export * from './models/IAppsTokensModel'; diff --git a/packages/model-typings/src/models/IAppsTokensModel.ts b/packages/model-typings/src/models/IAppsTokensModel.ts new file mode 100644 index 00000000000..13ef9b0cb24 --- /dev/null +++ b/packages/model-typings/src/models/IAppsTokensModel.ts @@ -0,0 +1,5 @@ +import type { IAppsTokens } from '@rocket.chat/core-typings'; + +import type { IBaseModel } from './IBaseModel'; + +export type IAppsTokensModel = IBaseModel; diff --git a/packages/models/src/index.ts b/packages/models/src/index.ts index 894556b3972..a8b5989619e 100644 --- a/packages/models/src/index.ts +++ b/packages/models/src/index.ts @@ -70,6 +70,7 @@ import type { IImportsModel, IRawImportsModel, IFederationRoomEventsModel, + IAppsTokensModel, } from '@rocket.chat/model-typings'; import { proxify } from './proxify'; @@ -82,6 +83,7 @@ export function getCollectionName(name: string): string { export { registerModel } from './proxify'; export const Apps = proxify('IAppsModel'); +export const AppsTokens = proxify('IAppsTokensModel'); export const AppsPersistence = proxify('IAppsPersistenceModel'); export const AppLogs = proxify('IAppLogsModel'); export const Analytics = proxify('IAnalyticsModel');