From 134ff0b29d3571e30d2651e91b8e804a6fb74766 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Tue, 16 Jul 2019 08:51:15 +0200 Subject: [PATCH] Removed concurrent aspell works within the same request (#32) --- .../spelling/app/js/SpellingAPIManager.js | 18 +---- .../test/unit/js/SpellingAPIManagerTests.js | 73 ------------------- 2 files changed, 1 insertion(+), 90 deletions(-) diff --git a/services/spelling/app/js/SpellingAPIManager.js b/services/spelling/app/js/SpellingAPIManager.js index 9a16a717c2..7c330eb312 100644 --- a/services/spelling/app/js/SpellingAPIManager.js +++ b/services/spelling/app/js/SpellingAPIManager.js @@ -9,14 +9,10 @@ const ASpell = require('./ASpell') const LearnedWordsManager = require('./LearnedWordsManager') const { callbackify } = require('util') -const _ = require('underscore') // The max number of words checked in a single request const REQUEST_LIMIT = 10000 -// Size of each chunk of words sent to Aspell checker -const CHUNK_SIZE = 1000 - const SpellingAPIManager = { whitelist: ['ShareLaTeX', 'sharelatex', 'LaTeX', 'http', 'https', 'www'], @@ -53,19 +49,7 @@ const promises = { // only the first 10K words are checked const wordSlice = request.words.slice(0, REQUEST_LIMIT) - // word list is splitted into chunks and sent to Aspell concurrently - const chunks = _.chunk(wordSlice, CHUNK_SIZE) - const chunkResults = await Promise.all( - chunks.map(chunk => ASpell.promises.checkWords(lang, chunk)) - ) - - // indexes have to be reconstructed for each chunk - chunkResults.forEach((result, chunkIndex) => - result.forEach(msp => (msp.index += chunkIndex * CHUNK_SIZE)) - ) - - // the final misspellings object is created merging all the chunks - const misspellings = _.flatten(chunkResults) + const misspellings = await ASpell.promises.checkWords(lang, wordSlice) if (token) { const learnedWords = await LearnedWordsManager.promises.getLearnedWords( diff --git a/services/spelling/test/unit/js/SpellingAPIManagerTests.js b/services/spelling/test/unit/js/SpellingAPIManagerTests.js index 641fa02aa6..5baecbc8b0 100644 --- a/services/spelling/test/unit/js/SpellingAPIManagerTests.js +++ b/services/spelling/test/unit/js/SpellingAPIManagerTests.js @@ -156,69 +156,6 @@ describe('SpellingAPIManager', function() { }) }) - describe('with a large collection of words', function() { - beforeEach(function(done) { - this.ASpell.promises.checkWords = sinon.spy(() => - promiseStub(this.misspellings) - ) - this.manyWords = __range__(1, 4500, true).map(i => 'word') - this.SpellingAPIManager.runRequest( - this.token, - { words: this.manyWords }, - (error, result) => { - this.result = result - done() - } - ) - }) - - it('should make concurrent requests of 1000 words', function() { - this.ASpell.promises.checkWords.callCount.should.equal(5) - }) - }) - - describe('with a collection of words with the max size of a chunk', function() { - beforeEach(function(done) { - this.ASpell.promises.checkWords = sinon.spy(() => - promiseStub(this.misspellings) - ) - this.manyWords = __range__(1, 1000, true).map(i => 'word') - this.SpellingAPIManager.runRequest( - this.token, - { words: this.manyWords }, - (error, result) => { - this.result = result - done() - } - ) - }) - - it('should make a single request', function() { - this.ASpell.promises.checkWords.callCount.should.equal(1) - }) - }) - - describe('with a very large collection of words', function() { - beforeEach(function(done) { - this.ASpell.promises.checkWords = sinon.spy(() => - promiseStub(this.misspellings) - ) - this.manyWords = __range__(1, 50000, true).map(i => 'word') - this.SpellingAPIManager.runRequest( - this.token, - { words: this.manyWords }, - (error, result) => { - this.result = result - done() - } - ) - }) - - it('should make a maximum of 10 concurrent requests', function() { - this.ASpell.promises.checkWords.callCount.should.equal(10) - }) - }) - describe('with words from the whitelist', function() { beforeEach(function(done) { this.whitelistWord = this.SpellingAPIManager.whitelist[0] @@ -293,13 +230,3 @@ describe('SpellingAPIManager', function() { }) }) }) - -function __range__(left, right, inclusive) { - let range = [] - let ascending = left < right - let end = !inclusive ? right : ascending ? right + 1 : right - 1 - for (let i = left; ascending ? i < end : i > end; ascending ? i++ : i--) { - range.push(i) - } - return range -}