diff --git a/package-lock.json b/package-lock.json index de97b70e6c..2af8b95e13 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44163,6 +44163,7 @@ "name": "@overleaf/notifications", "license": "ISC", "dependencies": { + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/mongo-utils": "*", @@ -44173,11 +44174,9 @@ "bunyan": "^1.8.15", "express": "^4.21.2", "method-override": "^3.0.0", - "mongodb-legacy": "6.1.3", - "request": "^2.88.2" + "mongodb-legacy": "6.1.3" }, "devDependencies": { - "@types/request": "^2.48.12", "chai-as-promised": "^7.1.1", "typescript": "^5.0.4", "vitest": "^3.2.4" diff --git a/services/notifications/app.js b/services/notifications/app.js index a55d61199f..60f6b199fa 100644 --- a/services/notifications/app.js +++ b/services/notifications/app.js @@ -38,16 +38,7 @@ app.delete( app.get('/status', (req, res) => res.send('notifications is up')) -app.get('/health_check', (req, res) => - HealthCheckController.check(function (err) { - if (err) { - logger.err({ err }, 'error performing health check') - res.sendStatus(500) - } else { - res.sendStatus(200) - } - }) -) +app.get('/health_check', HealthCheckController.check) app.get('*', (req, res) => res.sendStatus(404)) diff --git a/services/notifications/app/js/HealthCheckController.js b/services/notifications/app/js/HealthCheckController.js index 96b189bf73..c46edf3084 100644 --- a/services/notifications/app/js/HealthCheckController.js +++ b/services/notifications/app/js/HealthCheckController.js @@ -1,110 +1,130 @@ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import { db, ObjectId } from './mongodb.js' -import request from 'request' -import async from 'async' import settings from '@overleaf/settings' import logger from '@overleaf/logger' +import { + fetchJson, + fetchNothing, + RequestFailedError, +} from '@overleaf/fetch-utils' +import { expressify } from '@overleaf/promise-utils' const { port } = settings.internal.notifications -export default { - check(callback) { - const userId = new ObjectId() - const cleanupNotifications = callback => - db.notifications.deleteOne({ user_id: userId }, callback) - - let notificationKey = `smoke-test-notification-${new ObjectId()}` - const getOpts = endPath => ({ - url: `http://127.0.0.1:${port}/user/${userId}${endPath}`, - timeout: 5000, - }) - logger.debug( - { opts: getOpts(), key: notificationKey, userId }, - 'Health Check: running' - ) - const jobs = [ - function (cb) { - const opts = getOpts('/') - opts.json = { - key: notificationKey, - messageOpts: '', - templateKey: 'f4g5', - user_id: userId, - } - return request.post(opts, cb) - }, - function (cb) { - const opts = getOpts('/') - opts.json = true - return request.get(opts, function (err, res, body) { - if (err != null) { - logger.err({ err }, 'Health Check: error getting notification') - return callback(err) - } else if (res.statusCode !== 200) { - const e = `status code not 200 ${res.statusCode}` - logger.err({ err }, e) - return cb(e) - } - const hasNotification = body.some( - notification => - notification.key === notificationKey && - notification.user_id === userId.toString() - ) - if (hasNotification) { - return cb(null, body) - } else { - logger.err( - { body, notificationKey }, - 'Health Check: notification not in response' - ) - return cb(new Error('notification not found in response')) - } - }) - }, - ] - return async.series(jobs, function (err, body) { - if (err != null) { - logger.err({ err }, 'Health Check: error running health check') - return cleanupNotifications(() => callback(err)) - } else { - const notificationId = body[1][0]._id - notificationKey = body[1][0].key - let opts = getOpts(`/notification/${notificationId}`) - logger.debug( - { notificationId, notificationKey }, - 'Health Check: doing cleanup' - ) - return request.del(opts, function (err, res, body) { - if (err != null) { - logger.err( - err, - opts, - 'Health Check: error cleaning up notification' - ) - return callback(err) - } - opts = getOpts('') - opts.json = { key: notificationKey } - return request.del(opts, function (err, res, body) { - if (err != null) { - logger.err( - err, - opts, - 'Health Check: error cleaning up notification' - ) - return callback(err) - } - return cleanupNotifications(callback) - }) - }) - } - }) - }, +function makeUrl(userId, endPath = '') { + return new URL(`/user/${userId}/${endPath}`, `http://127.0.0.1:${port}`) +} + +async function makeNotification(notificationKey, userId) { + const postOpts = { + method: 'POST', + json: { + key: notificationKey, + messageOpts: '', + templateKey: 'f4g5', + user_id: userId, + }, + signal: AbortSignal.timeout(5000), + } + const url = makeUrl(userId) + await fetchNothing(url, postOpts) +} + +async function getUsersNotifications(userId) { + const url = makeUrl(userId) + try { + return await fetchJson(url, { + signal: AbortSignal.timeout(5000), + }) + } catch (err) { + if (err instanceof RequestFailedError) { + logger.err({ err }, 'Non-2xx status code received') + throw err + } + logger.err({ err }, 'Health Check: error getting notification') + throw err + } +} + +async function userHasNotification(userId, notificationKey) { + const body = await getUsersNotifications(userId) + const hasNotification = body.some( + notification => + notification.key === notificationKey && notification.user_id === userId + ) + if (hasNotification) { + return body + } else { + logger.err( + { body, notificationKey }, + 'Health Check: notification not in response' + ) + throw new Error('notification not found in response') + } +} + +async function cleanupNotifications(userId) { + await db.notifications.deleteOne({ user_id: userId }) +} + +async function deleteNotification(userId, notificationId, notificationKey) { + const deleteByIdUrl = makeUrl(userId, `notification/${notificationId}`) + try { + await fetchNothing(deleteByIdUrl, { + signal: AbortSignal.timeout(5000), + method: 'DELETE', + }) + } catch (err) { + logger.err( + { err, url: deleteByIdUrl }, + 'Health Check: error cleaning up notification' + ) + throw err + } + + const deleteByKeyUrl = makeUrl(userId) + + try { + await fetchNothing(deleteByKeyUrl, { + signal: AbortSignal.timeout(5000), + method: 'DELETE', + body: { + key: notificationKey, + }, + }) + } catch (err) { + logger.err( + { err, url: deleteByKeyUrl }, + 'Health Check: error cleaning up notification' + ) + throw err + } +} + +async function check(req, res) { + const userId = new ObjectId().toString() + let notificationKey = `smoke-test-notification-${new ObjectId()}` + + logger.debug({ userId, key: notificationKey }, 'Health Check: running') + + await makeNotification(notificationKey, userId) + try { + const body = await userHasNotification(userId, notificationKey) + const notificationId = body[0]._id + notificationKey = body[0].key + logger.debug( + { notificationId, notificationKey }, + 'Health Check: doing cleanup' + ) + await deleteNotification(userId, notificationId, notificationKey) + res.sendStatus(200) + } catch (err) { + logger.err({ err }, 'Health Check: error running health check') + res.sendStatus(500) + } finally { + await cleanupNotifications(userId) + } +} + +export default { + check: expressify(check), } diff --git a/services/notifications/app/js/Notifications.js b/services/notifications/app/js/Notifications.js index 7c7fb8f237..1431ed7a4f 100644 --- a/services/notifications/app/js/Notifications.js +++ b/services/notifications/app/js/Notifications.js @@ -1,121 +1,109 @@ import logger from '@overleaf/logger' import { db, ObjectId } from './mongodb.js' -export default { - getUserNotifications(userId, callback) { - if (callback == null) { - callback = function () {} - } - const query = { - user_id: new ObjectId(userId), - templateKey: { $exists: true }, - } - db.notifications.find(query).toArray(callback) - }, - - _countExistingNotifications(userId, notification, callback) { - if (callback == null) { - callback = function () {} - } - const query = { - user_id: new ObjectId(userId), - key: notification.key, - } - return db.notifications.count(query, function (err, count) { - if (err != null) { - return callback(err) - } - return callback(null, count) - }) - }, - - addNotification(userId, notification, callback) { - return this._countExistingNotifications( - userId, - notification, - function (err, count) { - if (err != null) { - return callback(err) - } - if (count !== 0 && !notification.forceCreate) { - return callback() - } - const doc = { - user_id: new ObjectId(userId), - key: notification.key, - messageOpts: notification.messageOpts, - templateKey: notification.templateKey, - } - // TTL index on the optional `expires` field, which should arrive as an iso date-string, corresponding to - // a datetime in the future when the document should be automatically removed. - // in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing - // see `README.md` for instruction on creating TTL index - if (notification.expires != null) { - try { - doc.expires = new Date(notification.expires) - const _testValue = doc.expires.toISOString() - } catch (error) { - err = error - logger.error( - { userId, expires: notification.expires }, - 'error converting `expires` field to Date' - ) - return callback(err) - } - } - db.notifications.updateOne( - { user_id: doc.user_id, key: notification.key }, - { $set: doc }, - { upsert: true }, - callback - ) - } - ) - }, - - removeNotificationId(userId, notificationId, callback) { - const searchOps = { - user_id: new ObjectId(userId), - _id: new ObjectId(notificationId), - } - const updateOperation = { $unset: { templateKey: true, messageOpts: true } } - db.notifications.updateOne(searchOps, updateOperation, callback) - }, - - removeNotificationKey(userId, notificationKey, callback) { - const searchOps = { - user_id: new ObjectId(userId), - key: notificationKey, - } - const updateOperation = { $unset: { templateKey: true } } - db.notifications.updateOne(searchOps, updateOperation, callback) - }, - - removeNotificationByKeyOnly(notificationKey, callback) { - const searchOps = { key: notificationKey } - const updateOperation = { $unset: { templateKey: true } } - db.notifications.updateOne(searchOps, updateOperation, callback) - }, - - countNotificationsByKeyOnly(notificationKey, callback) { - const searchOps = { key: notificationKey, templateKey: { $exists: true } } - db.notifications.count(searchOps, callback) - }, - - deleteUnreadNotificationsByKeyOnlyBulk(notificationKey, callback) { - if (typeof notificationKey !== 'string') { - throw new Error('refusing to bulk delete arbitrary notifications') - } - const searchOps = { key: notificationKey, templateKey: { $exists: true } } - db.notifications.deleteMany(searchOps, (err, result) => { - if (err) return callback(err) - callback(null, result.deletedCount) - }) - }, - - // hard delete of doc, rather than removing the templateKey - deleteNotificationByKeyOnly(notificationKey, callback) { - const searchOps = { key: notificationKey } - db.notifications.deleteOne(searchOps, callback) - }, +async function getUserNotifications(userId) { + const query = { + user_id: new ObjectId(userId), + templateKey: { $exists: true }, + } + return await db.notifications.find(query).toArray() +} + +async function _countExistingNotifications(userId, notification) { + const query = { + user_id: new ObjectId(userId), + key: notification.key, + } + return await db.notifications.count(query) +} + +async function addNotification(userId, notification, callback) { + const count = await _countExistingNotifications(userId, notification) + if (count !== 0 && !notification.forceCreate) { + return + } + const doc = { + user_id: new ObjectId(userId), + key: notification.key, + messageOpts: notification.messageOpts, + templateKey: notification.templateKey, + } + // TTL index on the optional `expires` field, which should arrive as an iso date-string, corresponding to + // a datetime in the future when the document should be automatically removed. + // in Mongo, TTL indexes only work on date fields, and ignore the document when that field is missing + // see `README.md` for instruction on creating TTL index + if (notification.expires != null) { + try { + doc.expires = new Date(notification.expires) + // _testValue assignment will throw if `expires` is not a valid date + // eslint-disable-next-line no-unused-vars + const _testValue = doc.expires.toISOString() + } catch (error) { + logger.error( + { userId, expires: notification.expires }, + 'error converting `expires` field to Date' + ) + throw error + } + } + return await db.notifications.updateOne( + { user_id: doc.user_id, key: notification.key }, + { $set: doc }, + { upsert: true } + ) +} + +async function removeNotificationId(userId, notificationId) { + const searchOps = { + user_id: new ObjectId(userId), + _id: new ObjectId(notificationId), + } + const updateOperation = { $unset: { templateKey: true, messageOpts: true } } + return await db.notifications.updateOne(searchOps, updateOperation) +} + +async function removeNotificationKey(userId, notificationKey) { + const searchOps = { + user_id: new ObjectId(userId), + key: notificationKey, + } + const updateOperation = { $unset: { templateKey: true } } + return await db.notifications.updateOne(searchOps, updateOperation) +} + +async function removeNotificationByKeyOnly(notificationKey) { + const searchOps = { key: notificationKey } + const updateOperation = { $unset: { templateKey: true } } + return await db.notifications.updateOne(searchOps, updateOperation) +} + +async function countNotificationsByKeyOnly(notificationKey) { + const searchOps = { key: notificationKey, templateKey: { $exists: true } } + return await db.notifications.countDocuments(searchOps) +} + +async function deleteUnreadNotificationsByKeyOnlyBulk(notificationKey) { + if (typeof notificationKey !== 'string') { + throw new Error('refusing to bulk delete arbitrary notifications') + } + const searchOps = { key: notificationKey, templateKey: { $exists: true } } + const result = await db.notifications.deleteMany(searchOps) + return result.deletedCount +} + +// hard delete of doc, rather than removing the templateKey +async function deleteNotificationByKeyOnly(notificationKey) { + const searchOps = { key: notificationKey } + return await db.notifications.deleteOne(searchOps) +} + +export default { + addNotification, + getUserNotifications, + removeNotificationId, + removeNotificationKey, + removeNotificationByKeyOnly, + countNotificationsByKeyOnly, + deleteUnreadNotificationsByKeyOnlyBulk, + deleteNotificationByKeyOnly, } diff --git a/services/notifications/app/js/NotificationsController.js b/services/notifications/app/js/NotificationsController.js index 31d15ba581..d3a11d862c 100644 --- a/services/notifications/app/js/NotificationsController.js +++ b/services/notifications/app/js/NotificationsController.js @@ -1,117 +1,103 @@ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ import logger from '@overleaf/logger' import metrics from '@overleaf/metrics' import Notifications from './Notifications.js' +import { expressify } from '@overleaf/promise-utils' + +async function getUserNotifications(req, res, next) { + logger.debug( + { userId: req.params.user_id }, + 'getting user unread notifications' + ) + metrics.inc('getUserNotifications') + const notifications = await Notifications.getUserNotifications( + req.params.user_id + ) + res.json(notifications) +} + +async function addNotification(req, res) { + logger.debug( + { userId: req.params.user_id, notification: req.body }, + 'adding notification' + ) + metrics.inc('addNotification') + try { + await Notifications.addNotification(req.params.user_id, req.body) + res.sendStatus(200) + } catch (err) { + res.sendStatus(500) + } +} + +async function removeNotificationId(req, res) { + logger.debug( + { + userId: req.params.user_id, + notificationId: req.params.notification_id, + }, + 'mark id notification as read' + ) + metrics.inc('removeNotificationId') + await Notifications.removeNotificationId( + req.params.user_id, + req.params.notification_id + ) + res.sendStatus(200) +} + +async function removeNotificationKey(req, res) { + logger.debug( + { userId: req.params.user_id, notificationKey: req.body.key }, + 'mark key notification as read' + ) + metrics.inc('removeNotificationKey') + await Notifications.removeNotificationKey(req.params.user_id, req.body.key) + + res.sendStatus(200) +} + +async function removeNotificationByKeyOnly(req, res) { + const notificationKey = req.params.key + logger.debug({ notificationKey }, 'mark notification as read by key only') + metrics.inc('removeNotificationKey') + await Notifications.removeNotificationByKeyOnly(notificationKey) + res.sendStatus(200) +} + +async function countNotificationsByKeyOnly(req, res) { + const notificationKey = req.params.key + try { + const count = + await Notifications.countNotificationsByKeyOnly(notificationKey) + res.json({ count }) + } catch (err) { + logger.err({ err, notificationKey }, 'cannot count by key') + res.sendStatus(500) + } +} + +async function deleteUnreadNotificationsByKeyOnlyBulk(req, res) { + const notificationKey = req.params.key + try { + const count = + await Notifications.deleteUnreadNotificationsByKeyOnlyBulk( + notificationKey + ) + res.json({ count }) + } catch (err) { + logger.err({ err, notificationKey }, 'cannot bulk remove by key') + res.sendStatus(500) + } +} export default { - getUserNotifications(req, res, next) { - logger.debug( - { userId: req.params.user_id }, - 'getting user unread notifications' - ) - metrics.inc('getUserNotifications') - return Notifications.getUserNotifications( - req.params.user_id, - (err, notifications) => { - if (err) return next(err) - res.json(notifications) - } - ) - }, - - addNotification(req, res) { - logger.debug( - { userId: req.params.user_id, notification: req.body }, - 'adding notification' - ) - metrics.inc('addNotification') - return Notifications.addNotification( - req.params.user_id, - req.body, - function (err, notifications) { - if (err != null) { - return res.sendStatus(500) - } else { - return res.sendStatus(200) - } - } - ) - }, - - removeNotificationId(req, res, next) { - logger.debug( - { - userId: req.params.user_id, - notificationId: req.params.notification_id, - }, - 'mark id notification as read' - ) - metrics.inc('removeNotificationId') - return Notifications.removeNotificationId( - req.params.user_id, - req.params.notification_id, - err => { - if (err) return next(err) - res.sendStatus(200) - } - ) - }, - - removeNotificationKey(req, res, next) { - logger.debug( - { userId: req.params.user_id, notificationKey: req.body.key }, - 'mark key notification as read' - ) - metrics.inc('removeNotificationKey') - return Notifications.removeNotificationKey( - req.params.user_id, - req.body.key, - (err, notifications) => { - if (err) return next(err) - res.sendStatus(200) - } - ) - }, - - removeNotificationByKeyOnly(req, res, next) { - const notificationKey = req.params.key - logger.debug({ notificationKey }, 'mark notification as read by key only') - metrics.inc('removeNotificationKey') - return Notifications.removeNotificationByKeyOnly(notificationKey, err => { - if (err) return next(err) - res.sendStatus(200) - }) - }, - - countNotificationsByKeyOnly(req, res) { - const notificationKey = req.params.key - Notifications.countNotificationsByKeyOnly(notificationKey, (err, count) => { - if (err) { - logger.err({ err, notificationKey }, 'cannot count by key') - return res.sendStatus(500) - } - res.json({ count }) - }) - }, - - deleteUnreadNotificationsByKeyOnlyBulk(req, res) { - const notificationKey = req.params.key - Notifications.deleteUnreadNotificationsByKeyOnlyBulk( - notificationKey, - (err, count) => { - if (err) { - logger.err({ err, notificationKey }, 'cannot bulk remove by key') - return res.sendStatus(500) - } - res.json({ count }) - } - ) - }, + getUserNotifications: expressify(getUserNotifications), + addNotification: expressify(addNotification), + deleteUnreadNotificationsByKeyOnlyBulk: expressify( + deleteUnreadNotificationsByKeyOnlyBulk + ), + removeNotificationByKeyOnly: expressify(removeNotificationByKeyOnly), + removeNotificationId: expressify(removeNotificationId), + removeNotificationKey: expressify(removeNotificationKey), + countNotificationsByKeyOnly: expressify(countNotificationsByKeyOnly), } diff --git a/services/notifications/package.json b/services/notifications/package.json index 83d4a8f32a..ee8631d20d 100644 --- a/services/notifications/package.json +++ b/services/notifications/package.json @@ -19,6 +19,7 @@ "author": "", "license": "ISC", "dependencies": { + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/mongo-utils": "*", @@ -29,11 +30,9 @@ "bunyan": "^1.8.15", "express": "^4.21.2", "method-override": "^3.0.0", - "mongodb-legacy": "6.1.3", - "request": "^2.88.2" + "mongodb-legacy": "6.1.3" }, "devDependencies": { - "@types/request": "^2.48.12", "chai-as-promised": "^7.1.1", "typescript": "^5.0.4", "vitest": "^3.2.4" diff --git a/services/notifications/test/unit/js/Notifications.test.js b/services/notifications/test/unit/js/Notifications.test.js index fe21037086..6554ed4411 100644 --- a/services/notifications/test/unit/js/Notifications.test.js +++ b/services/notifications/test/unit/js/Notifications.test.js @@ -38,7 +38,7 @@ describe('Notifications Tests', () => { ObjectId, })) - notifications = await import(modulePath) + notifications = (await import(modulePath)).default stubbedNotification = { user_id: new ObjectId(userId), diff --git a/services/notifications/test/unit/js/NotificationsController.test.js b/services/notifications/test/unit/js/NotificationsController.test.js index b1d39bcb49..f4bb68febd 100644 --- a/services/notifications/test/unit/js/NotificationsController.test.js +++ b/services/notifications/test/unit/js/NotificationsController.test.js @@ -23,7 +23,9 @@ describe('Notifications Controller', () => { removeNotificationKey: vi.fn(), } - vi.doMock('../../../app/js/Notifications', () => notifications) + vi.doMock('../../../app/js/Notifications', () => ({ + default: notifications, + })) vi.doMock('@overleaf/metrics', () => ({ default: {