From abd3e6e325e7778c69fa7424f99efa653dd02346 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Tue, 7 Oct 2025 09:48:44 +0100 Subject: [PATCH] Merge pull request #28811 from overleaf/dp-promisify-learned-words-manager Promisify LearnedWordsManager and LearnedWordsManagerTests GitOrigin-RevId: f4e30eca0292409bcefe82b17facd1129fdc85ae --- .../Features/Spelling/LearnedWordsManager.js | 142 +++++++++--------- .../src/Spelling/LearnedWordsManagerTests.js | 73 ++++----- 2 files changed, 104 insertions(+), 111 deletions(-) diff --git a/services/web/app/src/Features/Spelling/LearnedWordsManager.js b/services/web/app/src/Features/Spelling/LearnedWordsManager.js index 66d80fcaa8..4d3162c0a1 100644 --- a/services/web/app/src/Features/Spelling/LearnedWordsManager.js +++ b/services/web/app/src/Features/Spelling/LearnedWordsManager.js @@ -1,94 +1,98 @@ +// @ts-check + const { db } = require('../../infrastructure/mongodb') -const { promisify } = require('util') -const OError = require('@overleaf/o-error') +const { callbackify } = require('util') const Settings = require('@overleaf/settings') const { InvalidError } = require('../Errors/Errors') const LearnedWordsManager = { - learnWord(userToken, word, callback) { - LearnedWordsManager.getLearnedWordsSize(userToken, (error, wordsSize) => { - if (error != null) { - return callback(OError.tag(error)) + /** + * @param {string} userToken + * @param {string} word + */ + async learnWord(userToken, word) { + const wordsSize = await LearnedWordsManager.getLearnedWordsSize(userToken) + + const wordSize = Buffer.from(word).length + if (wordsSize + wordSize > Settings.maxDictionarySize) { + throw new InvalidError('Max dictionary size reached') + } + + return await db.spellingPreferences.updateOne( + { + token: userToken, + }, + { + $addToSet: { learnedWords: word }, + }, + { + upsert: true, } - const wordSize = Buffer.from(word).length - if (wordsSize + wordSize > Settings.maxDictionarySize) { - return callback(new InvalidError('Max dictionary size reached')) - } - db.spellingPreferences.updateOne( - { - token: userToken, - }, - { - $addToSet: { learnedWords: word }, - }, - { - upsert: true, - }, - callback - ) - }) + ) }, - unlearnWord(userToken, word, callback) { - return db.spellingPreferences.updateOne( + /** + * @param {string} userToken + * @param {string} word + */ + async unlearnWord(userToken, word) { + return await db.spellingPreferences.updateOne( { token: userToken, }, { $pull: { learnedWords: word }, - }, - callback - ) - }, - - getLearnedWords(userToken, callback) { - db.spellingPreferences.findOne( - { token: userToken }, - function (error, preferences) { - if (error != null) { - return callback(OError.tag(error)) - } - let words = - (preferences != null ? preferences.learnedWords : undefined) || [] - if (words) { - // remove duplicates - words = words.filter( - (value, index, self) => self.indexOf(value) === index - ) - } - callback(null, words) } ) }, - getLearnedWordsSize(userToken, callback) { - db.spellingPreferences.findOne( - { token: userToken }, - function (error, preferences) { - if (error != null) { - return callback(OError.tag(error)) - } - const words = (preferences && preferences.learnedWords) || [] - const wordsSize = Buffer.from(JSON.stringify(words)).length - callback(null, wordsSize) - } - ) + /** + * @param {string} userToken + */ + async getLearnedWords(userToken) { + const preferences = await db.spellingPreferences.findOne({ + token: userToken, + }) + + let words = + (preferences != null ? preferences.learnedWords : undefined) || [] + + if (words) { + // remove duplicates + words = words.filter( + (value, index, self) => self.indexOf(value) === index + ) + } + return words }, - deleteUsersLearnedWords(userToken, callback) { - db.spellingPreferences.deleteOne({ token: userToken }, callback) + /** + * @param {string} userToken + */ + async getLearnedWordsSize(userToken) { + const preferences = await db.spellingPreferences.findOne({ + token: userToken, + }) + + const words = (preferences && preferences.learnedWords) || [] + return Buffer.from(JSON.stringify(words)).length + }, + + /** + * @param {string} userToken + */ + async deleteUsersLearnedWords(userToken) { + return await db.spellingPreferences.deleteOne({ token: userToken }) }, } -const promises = { - learnWord: promisify(LearnedWordsManager.learnWord), - unlearnWord: promisify(LearnedWordsManager.unlearnWord), - getLearnedWords: promisify(LearnedWordsManager.getLearnedWords), - deleteUsersLearnedWords: promisify( +module.exports = { + learnWord: callbackify(LearnedWordsManager.learnWord), + unlearnWord: callbackify(LearnedWordsManager.unlearnWord), + getLearnedWords: callbackify(LearnedWordsManager.getLearnedWords), + getLearnedWordsSize: callbackify(LearnedWordsManager.getLearnedWordsSize), + deleteUsersLearnedWords: callbackify( LearnedWordsManager.deleteUsersLearnedWords ), + promises: LearnedWordsManager, } - -LearnedWordsManager.promises = promises - -module.exports = LearnedWordsManager diff --git a/services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js b/services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js index 111b8959b2..53e1ae3a8b 100644 --- a/services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js +++ b/services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js @@ -10,11 +10,10 @@ const { InvalidError } = require('../../../../app/src/Features/Errors/Errors') describe('LearnedWordsManager', function () { beforeEach(function () { this.token = 'a6b3cd919ge' - this.callback = sinon.stub() this.db = { spellingPreferences: { - updateOne: sinon.stub().yields(), - findOne: sinon.stub().yields(null, ['pear']), + updateOne: sinon.stub().resolves(), + findOne: sinon.stub().resolves(['pear']), }, } this.LearnedWordsManager = SandboxedModule.require(modulePath, { @@ -32,9 +31,9 @@ describe('LearnedWordsManager', function () { describe('learnWord', function () { describe('under size limit', function () { - beforeEach(function () { + beforeEach(async function () { this.word = 'instanton' - this.LearnedWordsManager.learnWord(this.token, this.word, this.callback) + await this.LearnedWordsManager.promises.learnWord(this.token, this.word) }) it('should insert the word in the word list in the database', function () { @@ -52,34 +51,26 @@ describe('LearnedWordsManager', function () { ) ).to.equal(true) }) - - it('should call the callback without error', function () { - sinon.assert.called(this.callback) - expect(this.callback.lastCall.args.length).to.equal(0) - }) }) describe('over size limit', function () { beforeEach(function () { this.word = 'superlongwordthatwillgobeyondthelimit' - this.LearnedWordsManager.learnWord(this.token, this.word, this.callback) }) - it('should not insert the word in the word list in the database', function () { + it('should throw an error and not insert the word in the word list in the database', async function () { + await expect( + this.LearnedWordsManager.promises.learnWord(this.token, this.word) + ).to.be.rejectedWith(InvalidError) expect(this.db.spellingPreferences.updateOne.notCalled).to.equal(true) }) - - it('should call the callback with error', function () { - sinon.assert.called(this.callback) - expect(this.callback.lastCall.args[0]).to.be.instanceof(InvalidError) - }) }) }) describe('unlearnWord', function () { - beforeEach(function () { + beforeEach(async function () { this.word = 'instanton' - this.LearnedWordsManager.unlearnWord(this.token, this.word, this.callback) + await this.LearnedWordsManager.promises.unlearnWord(this.token, this.word) }) it('should remove the word from the word list in the database', function () { @@ -94,22 +85,19 @@ describe('LearnedWordsManager', function () { ) ).to.equal(true) }) - - it('should call the callback', function () { - expect(this.callback.called).to.equal(true) - }) }) describe('getLearnedWords', function () { - beforeEach(function () { + beforeEach(async function () { this.wordList = ['apples', 'bananas', 'pears'] this.wordListWithDuplicates = this.wordList.slice() this.wordListWithDuplicates.push('bananas') - this.db.spellingPreferences.findOne = (conditions, callback) => { - callback(null, { learnedWords: this.wordListWithDuplicates }) + this.db.spellingPreferences.findOne = conditions => { + return Promise.resolve({ learnedWords: this.wordListWithDuplicates }) } sinon.spy(this.db.spellingPreferences, 'findOne') - this.LearnedWordsManager.getLearnedWords(this.token, this.callback) + this.learnedWords = + await this.LearnedWordsManager.promises.getLearnedWords(this.token) }) it('should get the word list for the given user', function () { @@ -118,35 +106,36 @@ describe('LearnedWordsManager', function () { ).to.equal(true) }) - it('should return the word list in the callback without duplicates', function () { - expect(this.callback.calledWith(null, this.wordList)).to.equal(true) + it('should return the word list without duplicates', function () { + expect(this.learnedWords).to.deep.equal(this.wordList) }) }) describe('getLearnedWordsSize', function () { - it('should return the word list size in the callback', function () { - this.db.spellingPreferences.findOne = (conditions, callback) => { - callback(null, { + it('should return the word list size in the callback', async function () { + this.db.spellingPreferences.findOne = conditions => { + return Promise.resolve({ learnedWords: ['apples', 'bananas', 'pears', 'bananas'], }) } - this.LearnedWordsManager.getLearnedWordsSize(this.token, this.callback) - sinon.assert.calledWith(this.callback, null, 38) + const learnedWordsSize = + await this.LearnedWordsManager.promises.getLearnedWordsSize(this.token) + expect(learnedWordsSize).to.equal(38) }) }) describe('deleteUsersLearnedWords', function () { beforeEach(function () { - this.db.spellingPreferences.deleteOne = sinon.stub().callsArgWith(1) + this.db.spellingPreferences.deleteOne = sinon.stub().resolves() }) - it('should get the word list for the given user', function (done) { - this.LearnedWordsManager.deleteUsersLearnedWords(this.token, () => { - this.db.spellingPreferences.deleteOne - .calledWith({ token: this.token }) - .should.equal(true) - done() - }) + it('should get the word list for the given user', async function () { + await this.LearnedWordsManager.promises.deleteUsersLearnedWords( + this.token + ) + expect( + this.db.spellingPreferences.deleteOne.calledWith({ token: this.token }) + ).to.equal(true) }) }) })