Merge pull request #6234 from overleaf/jpa-web-owns-spelling-preferences

[misc] move ownership of spellingPreferences collection to web

GitOrigin-RevId: f2584a1119a578c3df15371c6798923a4f2d15ae

services/spelling/app.js

@@ -16,7 +16,6 @@ if ((Settings.sentry != null ? Settings.sentry.dsn : undefined) != null) {
 }
 metrics.memory.monitor(logger)
 
-const mongodb = require('./app/js/mongodb')
 const SpellingAPIController = require('./app/js/SpellingAPIController')
 const express = require('express')
 const app = express()
@@ -27,11 +26,7 @@ const HealthCheckController = require('./app/js/HealthCheckController')
 app.use(bodyParser.json({ limit: '2mb' }))
 app.use(metrics.http.monitor(logger))
 
-app.delete('/user/:user_id', SpellingAPIController.deleteDic)
-app.get('/user/:user_id', SpellingAPIController.getDic)
 app.post('/user/:user_id/check', SpellingAPIController.check)
-app.post('/user/:user_id/learn', SpellingAPIController.learn)
-app.post('/user/:user_id/unlearn', SpellingAPIController.unlearn)
 app.get('/status', (req, res) => res.send({ status: 'spelling api is up' }))
 
 app.get('/health_check', HealthCheckController.healthCheck)
@@ -45,20 +40,12 @@ const port = settings && settings.port ? settings.port : 3005
 
 if (!module.parent) {
   // application entry point, called directly
-  mongodb
-    .waitForDb()
-    .then(() => {
-      app.listen(port, host, function (error) {
-        if (error != null) {
-          throw error
-        }
-        return logger.info(`spelling starting up, listening on ${host}:${port}`)
-      })
-    })
-    .catch(err => {
-      logger.fatal({ err }, 'Cannot connect to mongo. Exiting.')
-      process.exit(1)
-    })
+  app.listen(port, host, function (error) {
+    if (error != null) {
+      throw error
+    }
+    return logger.info(`spelling starting up, listening on ${host}:${port}`)
+  })
 }
 
 module.exports = app

services/spelling/app/js/MongoCache.js

@@ -1,11 +0,0 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-const LRU = require('lru-cache')
-const cacheOpts = {
-  max: 15000,
-  maxAge: 1000 * 60 * 60 * 10,
-}
-
-const cache = new LRU(cacheOpts)
-
-module.exports = cache

services/spelling/app/js/SpellingAPIController.js

@@ -10,12 +10,6 @@ function extractCheckRequestData(req) {
   return { token, wordCount }
 }
 
-function extractLearnRequestData(req) {
-  const token = req.params ? req.params.user_id : undefined
-  const word = req.body ? req.body.word : undefined
-  return { token, word }
-}
-
 module.exports = {
   check(req, res) {
     metrics.inc('spelling-check', 0.1)
@@ -34,55 +28,4 @@ module.exports = {
       res.send(result)
     })
   },
-
-  learn(req, res, next) {
-    metrics.inc('spelling-learn', 0.1)
-    const { token, word } = extractLearnRequestData(req)
-    logger.info({ token, word }, 'learning word')
-    SpellingAPIManager.learnWord(token, req.body, function (error) {
-      if (error != null) {
-        return next(OError.tag(error))
-      }
-      res.sendStatus(204)
-    })
-  },
-
-  unlearn(req, res, next) {
-    metrics.inc('spelling-unlearn', 0.1)
-    const { token, word } = extractLearnRequestData(req)
-    logger.info({ token, word }, 'unlearning word')
-    SpellingAPIManager.unlearnWord(token, req.body, function (error) {
-      if (error != null) {
-        return next(OError.tag(error))
-      }
-      res.sendStatus(204)
-    })
-  },
-
-  deleteDic(req, res, next) {
-    const { token, word } = extractLearnRequestData(req)
-    logger.log({ token, word }, 'deleting user dictionary')
-    SpellingAPIManager.deleteDic(token, function (error) {
-      if (error != null) {
-        return next(OError.tag(error))
-      }
-      res.sendStatus(204)
-    })
-  },
-
-  getDic(req, res, next) {
-    const token = req.params ? req.params.user_id : undefined
-    logger.info(
-      {
-        token,
-      },
-      'getting user dictionary'
-    )
-    SpellingAPIManager.getDic(token, function (error, words) {
-      if (error != null) {
-        return next(OError.tag(error))
-      }
-      res.send(words)
-    })
-  },
 }

services/spelling/app/js/SpellingAPIManager.js

@@ -7,53 +7,13 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 const ASpell = require('./ASpell')
-const LearnedWordsManager = require('./LearnedWordsManager')
 const { callbackify } = require('util')
 const OError = require('@overleaf/o-error')
-const Settings = require('@overleaf/settings')
 
 // The max number of words checked in a single request
 const REQUEST_LIMIT = 10000
 
-const SpellingAPIManager = {
-  whitelist: Settings.ignoredMisspellings,
-
-  learnWord(token, request, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
-    if (request.word == null) {
-      return callback(new OError('malformed JSON'))
-    }
-    if (token == null) {
-      return callback(new OError('no token provided'))
-    }
-
-    return LearnedWordsManager.learnWord(token, request.word, callback)
-  },
-
-  unlearnWord(token, request, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
-    if (request.word == null) {
-      return callback(new OError('malformed JSON'))
-    }
-    if (token == null) {
-      return callback(new OError('no token provided'))
-    }
-
-    return LearnedWordsManager.unlearnWord(token, request.word, callback)
-  },
-
-  deleteDic(token, callback) {
-    return LearnedWordsManager.deleteUsersLearnedWords(token, callback)
-  },
-
-  getDic(token, callback) {
-    return LearnedWordsManager.getLearnedWordsNoCache(token, callback)
-  },
-}
+const SpellingAPIManager = {}
 
 const promises = {
   async runRequest(token, request) {
@@ -66,22 +26,7 @@ const promises = {
     const wordSlice = request.words.slice(0, REQUEST_LIMIT)
 
     const misspellings = await ASpell.promises.checkWords(lang, wordSlice)
-
-    if (token && !request.skipLearnedWords) {
-      const learnedWords = await LearnedWordsManager.promises.getLearnedWords(
-        token
-      )
-      const notLearntMisspellings = misspellings.filter(m => {
-        const word = wordSlice[m.index]
-        return (
-          learnedWords.indexOf(word) === -1 &&
-          SpellingAPIManager.whitelist.indexOf(word) === -1
-        )
-      })
-      return { misspellings: notLearntMisspellings }
-    } else {
-      return { misspellings }
-    }
+    return { misspellings }
   },
 }
 

services/spelling/config/settings.defaults.js

@@ -8,16 +8,6 @@ module.exports = {
     },
   },
 
-  mongo: {
-    options: {
-      useUnifiedTopology:
-        (process.env.MONGO_USE_UNIFIED_TOPOLOGY || 'true') === 'true',
-    },
-    url:
-      process.env.MONGO_CONNECTION_STRING ||
-      `mongodb://${process.env.MONGO_HOST || 'localhost'}/sharelatex`,
-  },
-
   cacheDir: Path.resolve('cache'),
 
   healthCheckUserId: '53c64d2fd68c8d000010bb5f',

services/spelling/test/acceptance/js/Init.js

@@ -1,8 +1,5 @@
-const { waitForDb } = require('../../../app/js/mongodb')
 const App = require('../../../app.js')
 const { PORT } = require('./helpers/request')
-
-before(waitForDb)
 before(function (done) {
   return App.listen(PORT, 'localhost', done)
 })

services/spelling/test/acceptance/js/LearnTest.js

@@ -1,103 +0,0 @@
-const { expect } = require('chai')
-const request = require('./helpers/request')
-
-const USER_ID = 101
-
-const checkWord = words =>
-  request.post({
-    url: `/user/${USER_ID}/check`,
-    body: JSON.stringify({
-      words,
-    }),
-  })
-
-const learnWord = word =>
-  request.post({
-    url: `/user/${USER_ID}/learn`,
-    body: JSON.stringify({
-      word,
-    }),
-  })
-
-const unlearnWord = word =>
-  request.post({
-    url: `/user/${USER_ID}/unlearn`,
-    body: JSON.stringify({
-      word,
-    }),
-  })
-
-const getDict = () =>
-  request.get({
-    url: `/user/${USER_ID}`,
-  })
-
-const deleteDict = () =>
-  request.del({
-    url: `/user/${USER_ID}`,
-  })
-
-describe('learning words', function () {
-  afterEach(async function () {
-    await deleteDict()
-  })
-
-  it('should return status 204 when posting a word successfully', async function () {
-    const response = await learnWord('abcd')
-    expect(response.statusCode).to.equal(204)
-  })
-
-  it('should not learn the same word twice', async function () {
-    await learnWord('foobar')
-    const learnResponse = await learnWord('foobar')
-    expect(learnResponse.statusCode).to.equal(204)
-
-    const dictResponse = await getDict()
-    const responseBody = JSON.parse(dictResponse.body)
-    // the response from getlearnedwords filters out duplicates, so this test
-    // can succeed even if the word is stored twice in the database
-    expect(responseBody.length).to.equals(1)
-  })
-
-  it('should return no misspellings after a word is learnt', async function () {
-    const response = await checkWord(['abv'])
-    const responseBody = JSON.parse(response.body)
-    expect(responseBody.misspellings.length).to.equals(1)
-
-    await learnWord('abv')
-
-    const response2 = await checkWord(['abv'])
-    const responseBody2 = JSON.parse(response2.body)
-    expect(responseBody2.misspellings.length).to.equals(0)
-  })
-
-  it('should return misspellings again after a personal dictionary is deleted', async function () {
-    await learnWord('bvc')
-    await deleteDict()
-
-    const response = await checkWord(['bvc'])
-    const responseBody = JSON.parse(response.body)
-    expect(responseBody.misspellings.length).to.equals(1)
-  })
-})
-
-describe('unlearning words', function () {
-  it('should return status 204 when posting a word successfully', async function () {
-    const response = await unlearnWord('anything')
-    expect(response.statusCode).to.equal(204)
-  })
-
-  it('should return misspellings after a word is unlearnt', async function () {
-    await learnWord('abv')
-
-    const response = await checkWord(['abv'])
-    const responseBody = JSON.parse(response.body)
-    expect(responseBody.misspellings.length).to.equals(0)
-
-    await unlearnWord('abv')
-
-    const response2 = await checkWord(['abv'])
-    const responseBody2 = JSON.parse(response2.body)
-    expect(responseBody2.misspellings.length).to.equals(1)
-  })
-})

services/spelling/test/unit/js/SpellingAPIManagerTests.js

@@ -15,42 +15,22 @@ describe('SpellingAPIManager', function () {
   beforeEach(function () {
     this.token = 'user-id-123'
     this.ASpell = {}
-    this.learnedWords = ['lerned']
-    this.LearnedWordsManager = {
-      getLearnedWords: sinon.stub().callsArgWith(1, null, this.learnedWords),
-      learnWord: sinon.stub().callsArg(2),
-      unlearnWord: sinon.stub().callsArg(2),
-      promises: {
-        getLearnedWords: sinon.stub().returns(promiseStub(this.learnedWords)),
-      },
-    }
-
     this.SpellingAPIManager = SandboxedModule.require(modulePath, {
       requires: {
         './ASpell': this.ASpell,
         '@overleaf/settings': { ignoredMisspellings: ['ShareLaTeX'] },
-        './LearnedWordsManager': this.LearnedWordsManager,
       },
     })
   })
 
   describe('runRequest', function () {
     beforeEach(function () {
-      this.nonLearnedWords = [
-        'some',
-        'words',
-        'htat',
-        'are',
-        'speled',
-        'rong',
-        'lerned',
-      ]
-      this.allWords = this.nonLearnedWords.concat(this.learnedWords)
+      this.nonLearnedWords = ['some', 'words', 'htat', 'are', 'speled', 'rong']
+      this.allWords = this.nonLearnedWords
       this.misspellings = [
         { index: 2, suggestions: ['that'] },
         { index: 4, suggestions: ['spelled'] },
         { index: 5, suggestions: ['wrong', 'ring'] },
-        { index: 6, suggestions: ['learned'] },
       ]
       this.misspellingsWithoutLearnedWords = this.misspellings.slice(0, 3)
 
@@ -99,24 +79,6 @@ describe('SpellingAPIManager', function () {
       })
     })
 
-    describe('with a missing token', function () {
-      beforeEach(function (done) {
-        this.SpellingAPIManager.runRequest(
-          null,
-          { words: this.allWords },
-          (error, result) => {
-            this.error = error
-            this.result = result
-            done()
-          }
-        )
-      })
-
-      it('should spell check without using any learned words', function () {
-        this.LearnedWordsManager.getLearnedWords.called.should.equal(false)
-      })
-    })
-
     describe('without a language', function () {
       beforeEach(function (done) {
         this.SpellingAPIManager.runRequest(
@@ -158,131 +120,5 @@ describe('SpellingAPIManager', function () {
           .should.equal(true)
       })
     })
-
-    describe('with words from the whitelist', function () {
-      beforeEach(function (done) {
-        this.whitelistWord = this.SpellingAPIManager.whitelist[0]
-        this.words = ['One', 'Two', this.whitelistWord]
-        this.SpellingAPIManager.runRequest(
-          this.token,
-          { words: this.words },
-          (error, result) => {
-            if (error) return done(error)
-            this.result = result
-            done()
-          }
-        )
-      })
-
-      it('should ignore the white-listed word', function () {
-        expect(this.result.misspellings.length).to.equal(
-          this.misspellings.length - 1
-        )
-      })
-    })
-  })
-
-  describe('learnWord', function () {
-    describe('without a token', function () {
-      beforeEach(function (done) {
-        this.SpellingAPIManager.learnWord(null, { word: 'banana' }, error => {
-          this.error = error
-          done()
-        })
-      })
-
-      it('should return an error', function () {
-        expect(this.error).to.exist
-        expect(this.error).to.be.instanceof(Error)
-        expect(this.error.message).to.equal('no token provided')
-      })
-    })
-
-    describe('without a word', function () {
-      beforeEach(function (done) {
-        this.SpellingAPIManager.learnWord(this.token, {}, error => {
-          this.error = error
-          done()
-        })
-      })
-
-      it('should return an error', function () {
-        expect(this.error).to.exist
-        expect(this.error).to.be.instanceof(Error)
-        expect(this.error.message).to.equal('malformed JSON')
-      })
-    })
-
-    describe('with a word and a token', function () {
-      beforeEach(function (done) {
-        this.word = 'banana'
-        this.SpellingAPIManager.learnWord(
-          this.token,
-          { word: this.word },
-          error => {
-            this.error = error
-            done()
-          }
-        )
-      })
-
-      it('should call LearnedWordsManager.learnWord', function () {
-        this.LearnedWordsManager.learnWord
-          .calledWith(this.token, this.word)
-          .should.equal(true)
-      })
-    })
-  })
-
-  describe('unlearnWord', function () {
-    describe('without a token', function () {
-      beforeEach(function (done) {
-        this.SpellingAPIManager.unlearnWord(null, { word: 'banana' }, error => {
-          this.error = error
-          done()
-        })
-      })
-
-      it('should return an error', function () {
-        expect(this.error).to.exist
-        expect(this.error).to.be.instanceof(Error)
-        expect(this.error.message).to.equal('no token provided')
-      })
-    })
-
-    describe('without a word', function () {
-      beforeEach(function (done) {
-        this.SpellingAPIManager.unlearnWord(this.token, {}, error => {
-          this.error = error
-          done()
-        })
-      })
-
-      it('should return an error', function () {
-        expect(this.error).to.exist
-        expect(this.error).to.be.instanceof(Error)
-        expect(this.error.message).to.equal('malformed JSON')
-      })
-    })
-
-    describe('with a word and a token', function () {
-      beforeEach(function (done) {
-        this.word = 'banana'
-        this.SpellingAPIManager.unlearnWord(
-          this.token,
-          { word: this.word },
-          error => {
-            this.error = error
-            done()
-          }
-        )
-      })
-
-      it('should call LearnedWordsManager.unlearnWord', function () {
-        this.LearnedWordsManager.unlearnWord
-          .calledWith(this.token, this.word)
-          .should.equal(true)
-      })
-    })
   })
 })

services/web/app/src/Features/Project/ProjectController.js

@@ -696,7 +696,7 @@ const ProjectController = {
           if (!userId) {
             return cb(null, [])
           }
-          SpellingHandler.getUserDictionaryWithRetries(userId, cb)
+          SpellingHandler.getUserDictionary(userId, cb)
         },
         subscription(cb) {
           if (userId == null) {

services/spelling/app/js/LearnedWordsManager.js → services/web/app/src/Features/Spelling/LearnedWordsManager.js

@@ -1,5 +1,4 @@
-const { db } = require('./mongodb')
-const mongoCache = require('./MongoCache')
+const { db } = require('../../infrastructure/mongodb')
 const logger = require('@overleaf/logger')
 const metrics = require('@overleaf/metrics')
 const { promisify } = require('util')
@@ -7,10 +6,6 @@ const OError = require('@overleaf/o-error')
 
 const LearnedWordsManager = {
   learnWord(userToken, word, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
-    mongoCache.del(userToken)
     return db.spellingPreferences.updateOne(
       {
         token: userToken,
@@ -26,10 +21,6 @@ const LearnedWordsManager = {
   },
 
   unlearnWord(userToken, word, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
-    mongoCache.del(userToken)
     return db.spellingPreferences.updateOne(
       {
         token: userToken,
@@ -42,26 +33,6 @@ const LearnedWordsManager = {
   },
 
   getLearnedWords(userToken, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
-    const mongoCachedWords = mongoCache.get(userToken)
-    if (mongoCachedWords != null) {
-      metrics.inc('mongoCache', 0.1, { status: 'hit' })
-      return callback(null, mongoCachedWords)
-    }
-
-    metrics.inc('mongoCache', 0.1, { status: 'miss' })
-    logger.info({ userToken }, 'mongoCache miss')
-
-    LearnedWordsManager.getLearnedWordsNoCache(userToken, (err, words) => {
-      if (err) return callback(err)
-      mongoCache.set(userToken, words)
-      callback(null, words)
-    })
-  },
-
-  getLearnedWordsNoCache(userToken, callback) {
     db.spellingPreferences.findOne(
       { token: userToken },
       function (error, preferences) {
@@ -82,9 +53,6 @@ const LearnedWordsManager = {
   },
 
   deleteUsersLearnedWords(userToken, callback) {
-    if (callback == null) {
-      callback = () => {}
-    }
     db.spellingPreferences.deleteOne({ token: userToken }, callback)
   },
 }

services/web/app/src/Features/Spelling/SpellingController.js

@@ -2,6 +2,7 @@ const request = require('request')
 const Settings = require('@overleaf/settings')
 const logger = require('@overleaf/logger')
 const SessionManager = require('../Authentication/SessionManager')
+const LearnedWordsManager = require('./LearnedWordsManager')
 
 const TEN_SECONDS = 1000 * 10
 
@@ -9,6 +10,15 @@ const languageCodeIsSupported = code =>
   Settings.languages.some(lang => lang.code === code)
 
 module.exports = {
+  learn(req, res, next) {
+    const { word } = req.body
+    const userId = SessionManager.getLoggedInUserId(req.session)
+    LearnedWordsManager.learnWord(userId, word, err => {
+      if (err) return next(err)
+      res.sendStatus(204)
+    })
+  },
+
   proxyRequestToSpellingApi(req, res) {
     const { language } = req.body
 

services/web/app/src/Features/Spelling/SpellingHandler.js

@@ -1,117 +1,26 @@
-const request = require('request')
-const requestRetry = require('requestretry')
-const Settings = require('@overleaf/settings')
 const OError = require('@overleaf/o-error')
 const Metrics = require('@overleaf/metrics')
-
-const TIMEOUT = 10 * 1000
+const LearnedWordsManager = require('./LearnedWordsManager')
 
 module.exports = {
-  getUserDictionaryWithRetries(userId, callback) {
+  getUserDictionary(userId, callback) {
     const timer = new Metrics.Timer('spelling_get_dict')
-    const options = {
-      url: `${Settings.apis.spelling.url}/user/${userId}`,
-      timeout: 3 * 1000,
-      json: true,
-      retryDelay: 1,
-      maxAttempts: 3,
-    }
-    requestRetry(options, (error, response, body) => {
+    LearnedWordsManager.getLearnedWords(userId, (error, words) => {
       if (error) {
         return callback(
           OError.tag(error, 'error getting user dictionary', { error, userId })
         )
       }
-
-      if (response.statusCode !== 200) {
-        return callback(
-          new OError(
-            'Non-success code from spelling API when getting user dictionary',
-            { userId, statusCode: response.statusCode }
-          )
-        )
-      }
-
       timer.done()
-      callback(null, body)
-    })
-  },
-
-  getUserDictionary(userId, callback) {
-    const url = `${Settings.apis.spelling.url}/user/${userId}`
-    request.get({ url: url, timeout: TIMEOUT }, (error, response) => {
-      if (error) {
-        return callback(
-          OError.tag(error, 'error getting user dictionary', { error, userId })
-        )
-      }
-
-      if (response.statusCode < 200 || response.statusCode >= 300) {
-        return callback(
-          new OError(
-            'Non-success code from spelling API when getting user dictionary',
-            { userId, statusCode: response.statusCode }
-          )
-        )
-      }
-
-      callback(null, JSON.parse(response.body))
+      callback(null, words)
     })
   },
 
   deleteWordFromUserDictionary(userId, word, callback) {
-    const url = `${Settings.apis.spelling.url}/user/${userId}/unlearn`
-    request.post(
-      {
-        url: url,
-        json: {
-          word,
-        },
-        timeout: TIMEOUT,
-      },
-      (error, response) => {
-        if (error) {
-          return callback(
-            OError.tag(error, 'error deleting word from user dictionary', {
-              userId,
-              word,
-            })
-          )
-        }
-
-        if (response.statusCode < 200 || response.statusCode >= 300) {
-          return callback(
-            new OError(
-              'Non-success code from spelling API when removing word from user dictionary',
-              { userId, word, statusCode: response.statusCode }
-            )
-          )
-        }
-
-        callback()
-      }
-    )
+    LearnedWordsManager.unlearnWord(userId, word, callback)
   },
 
   deleteUserDictionary(userId, callback) {
-    const url = `${Settings.apis.spelling.url}/user/${userId}`
-    request.delete({ url: url, timeout: TIMEOUT }, (error, response) => {
-      if (error) {
-        return callback(
-          OError.tag(error, 'error deleting user dictionary', { userId })
-        )
-      }
-
-      if (response.statusCode < 200 || response.statusCode >= 300) {
-        return callback(
-          new OError(
-            'Non-success code from spelling API when removing user dictionary',
-            { userId, statusCode: response.statusCode }
-          )
-        )
-      }
-
-      callback()
-    })
+    LearnedWordsManager.deleteUsersLearnedWords(userId, callback)
   },
 }

services/web/app/src/router.js

@@ -847,8 +847,13 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
   webRouter.post(
     '/spelling/learn',
+    validate({
+      body: Joi.object({
+        word: Joi.string().required(),
+      }),
+    }),
     AuthenticationController.requireLogin(),
-    SpellingController.proxyRequestToSpellingApi
+    SpellingController.learn
   )
 
   webRouter.get(

services/web/test/acceptance/src/LearnTest.js

@@ -0,0 +1,52 @@
+const { expect } = require('chai')
+const cheerio = require('cheerio')
+const User = require('./helpers/User').promises
+
+describe('Spelling', function () {
+  let user, projectId
+  async function learnWord(word) {
+    const { response } = await user.doRequest('POST', {
+      url: '/spelling/learn',
+      json: { word },
+    })
+    return response
+  }
+
+  async function getDict() {
+    const { body, response } = await user.doRequest(
+      'GET',
+      `/project/${projectId}`
+    )
+    expect(response.statusCode).to.equal(200)
+    const dom = cheerio.load(body)
+    const metaEl = dom('meta[name="ol-learnedWords"]')[0]
+    return JSON.parse(metaEl.attribs.content)
+  }
+
+  describe('learning words', function () {
+    beforeEach(async function () {
+      user = new User()
+      await user.login()
+      projectId = await user.createProject('foo')
+    })
+
+    it('should return status 400 when posting an empty word', async function () {
+      const response = await learnWord('')
+      expect(response.statusCode).to.equal(400)
+    })
+
+    it('should return status 204 when posting a word successfully', async function () {
+      const response = await learnWord('abcd')
+      expect(response.statusCode).to.equal(204)
+    })
+
+    it('should not learn the same word twice', async function () {
+      await learnWord('foobar')
+      const learnResponse = await learnWord('foobar')
+      expect(learnResponse.statusCode).to.equal(204)
+
+      const dict = await getDict()
+      expect(dict.length).to.equals(1)
+    })
+  })
+})

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -194,7 +194,7 @@ describe('ProjectController', function () {
         },
         '../Helpers/NewLogsUI': this.NewLogsUIHelper,
         '../Spelling/SpellingHandler': {
-          getUserDictionaryWithRetries: sinon.stub().yields(null, []),
+          getUserDictionary: sinon.stub().yields(null, []),
         },
       },
     })

services/spelling/test/unit/js/LearnedWordsManagerTests.js → services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js

@@ -1,9 +1,9 @@
 const sinon = require('sinon')
-const { assert, expect } = require('chai')
+const { expect } = require('chai')
 const SandboxedModule = require('sandboxed-module')
 const modulePath = require('path').join(
   __dirname,
-  '../../../app/js/LearnedWordsManager'
+  '/../../../../app/src/Features/Spelling/LearnedWordsManager'
 )
 
 describe('LearnedWordsManager', function () {
@@ -15,15 +15,9 @@ describe('LearnedWordsManager', function () {
         updateOne: sinon.stub().yields(),
       },
     }
-    this.cache = {
-      get: sinon.stub(),
-      set: sinon.stub(),
-      del: sinon.stub(),
-    }
     this.LearnedWordsManager = SandboxedModule.require(modulePath, {
       requires: {
-        './mongodb': { db: this.db },
-        './MongoCache': this.cache,
+        '../../infrastructure/mongodb': { db: this.db },
         '@overleaf/metrics': {
           timeAsyncMethod: sinon.stub(),
           inc: sinon.stub(),
@@ -106,39 +100,6 @@ describe('LearnedWordsManager', function () {
     })
   })
 
-  describe('caching the result', function () {
-    it('should use the cache first if it is primed', function (done) {
-      this.wordList = ['apples', 'bananas', 'pears']
-      this.cache.get.returns(this.wordList)
-      this.db.spellingPreferences.findOne = sinon.stub()
-      this.LearnedWordsManager.getLearnedWords(this.token, (err, spellings) => {
-        expect(err).not.to.exist
-        this.db.spellingPreferences.findOne.called.should.equal(false)
-        assert.deepEqual(this.wordList, spellings)
-        done()
-      })
-    })
-
-    it('should set the cache after hitting the db', function (done) {
-      this.wordList = ['apples', 'bananas', 'pears']
-      this.db.spellingPreferences.findOne = sinon
-        .stub()
-        .callsArgWith(1, null, { learnedWords: this.wordList })
-      this.LearnedWordsManager.getLearnedWords(this.token, () => {
-        this.cache.set.calledWith(this.token, this.wordList).should.equal(true)
-        done()
-      })
-    })
-
-    it('should break cache when update is called', function (done) {
-      this.word = 'instanton'
-      this.LearnedWordsManager.learnWord(this.token, this.word, () => {
-        this.cache.del.calledWith(this.token).should.equal(true)
-        done()
-      })
-    })
-  })
-
   describe('deleteUsersLearnedWords', function () {
     beforeEach(function () {
       this.db.spellingPreferences.deleteOne = sinon.stub().callsArgWith(1)

services/web/test/unit/src/Spelling/SpellingControllerTests.js

@@ -27,6 +27,7 @@ describe('SpellingController', function () {
     }
     this.controller = SandboxedModule.require(modulePath, {
       requires: {
+        './LearnedWordsManager': {},
         request: this.request,
         '@overleaf/settings': {
           languages: [

services/web/test/unit/src/Spelling/SpellingHandlerTests.js

@@ -1,124 +0,0 @@
-const SandboxedModule = require('sandboxed-module')
-const { expect } = require('chai')
-const sinon = require('sinon')
-const modulePath = require('path').join(
-  __dirname,
-  '../../../../app/src/Features/Spelling/SpellingHandler.js'
-)
-
-const TIMEOUT = 1000 * 10
-
-const SPELLING_HOST = 'http://spelling.service.test'
-const SPELLING_URL = 'http://spelling.service.test'
-
-describe('SpellingHandler', function () {
-  let userId, word, dictionary, dictionaryString, request, SpellingHandler
-
-  beforeEach(function () {
-    userId = 'wombat'
-    word = 'potato'
-    dictionary = ['wombaat', 'woombat']
-    dictionaryString = JSON.stringify(dictionary)
-    request = {
-      get: sinon
-        .stub()
-        .yields(null, { statusCode: 200, body: dictionaryString }),
-      post: sinon.stub().yields(null, { statusCode: 204 }),
-      delete: sinon.stub().yields(null, { statusCode: 204 }),
-    }
-
-    SpellingHandler = SandboxedModule.require(modulePath, {
-      requires: {
-        request: request,
-        '@overleaf/settings': {
-          apis: { spelling: { host: SPELLING_HOST, url: SPELLING_URL } },
-        },
-      },
-    })
-  })
-
-  describe('getUserDictionary', function () {
-    it('calls the spelling API', function (done) {
-      SpellingHandler.getUserDictionary(userId, () => {
-        expect(request.get).to.have.been.calledWith({
-          url: 'http://spelling.service.test/user/wombat',
-          timeout: TIMEOUT,
-        })
-        done()
-      })
-    })
-
-    it('returns the dictionary', function (done) {
-      SpellingHandler.getUserDictionary(userId, (err, dictionary) => {
-        expect(err).not.to.exist
-        expect(dictionary).to.deep.equal(dictionary)
-        done()
-      })
-    })
-
-    it('returns an error when the request fails', function (done) {
-      request.get = sinon.stub().yields(new Error('ugh'))
-      SpellingHandler.getUserDictionary(userId, err => {
-        expect(err).to.exist
-        done()
-      })
-    })
-  })
-
-  describe('deleteWordFromUserDictionary', function () {
-    it('calls the spelling API', function (done) {
-      SpellingHandler.deleteWordFromUserDictionary(userId, word, () => {
-        expect(request.post).to.have.been.calledWith({
-          url: 'http://spelling.service.test/user/wombat/unlearn',
-          json: {
-            word: word,
-          },
-          timeout: TIMEOUT,
-        })
-        done()
-      })
-    })
-
-    it('does not return an error', function (done) {
-      SpellingHandler.deleteWordFromUserDictionary(userId, word, err => {
-        expect(err).not.to.exist
-        done()
-      })
-    })
-
-    it('returns an error when the request fails', function (done) {
-      request.post = sinon.stub().yields(new Error('ugh'))
-      SpellingHandler.deleteWordFromUserDictionary(userId, word, err => {
-        expect(err).to.exist
-        done()
-      })
-    })
-  })
-
-  describe('deleteUserDictionary', function () {
-    it('calls the spelling API', function (done) {
-      SpellingHandler.deleteUserDictionary(userId, () => {
-        expect(request.delete).to.have.been.calledWith({
-          url: 'http://spelling.service.test/user/wombat',
-          timeout: TIMEOUT,
-        })
-        done()
-      })
-    })
-
-    it('does not return an error', function (done) {
-      SpellingHandler.deleteUserDictionary(userId, err => {
-        expect(err).not.to.exist
-        done()
-      })
-    })
-
-    it('returns an error when the request fails', function (done) {
-      request.delete = sinon.stub().yields(new Error('ugh'))
-      SpellingHandler.deleteUserDictionary(userId, err => {
-        expect(err).to.exist
-        done()
-      })
-    })
-  })
-})