diff --git a/services/spelling/app/js/ASpellWorker.js b/services/spelling/app/js/ASpellWorker.js index 5d5e510390..c4ead15083 100644 --- a/services/spelling/app/js/ASpellWorker.js +++ b/services/spelling/app/js/ASpellWorker.js @@ -128,12 +128,18 @@ class ASpellWorker { } }) + this.pipe.stdout.setEncoding('utf8') // ensure utf8 output is handled correctly var output = '' - const endMarker = new RegExp('^[a-z][a-z]', 'm') - this.pipe.stdout.on('data', chunk => { - output = output + chunk - // We receive the language code from Aspell as the end of data marker - if (chunk.toString().match(endMarker)) { + const endMarkerRegex = new RegExp('^[a-z][a-z]', 'gm') + this.pipe.stdout.on('data', data => { + // We receive the language code from Aspell as the end of data marker in + // the data. The input is a utf8 encoded string. + let oldPos = output.length + output = output + data + // The end marker may cross the end of a chunk, so we optimise the search + // using the regex lastIndex property. + endMarkerRegex.lastIndex = oldPos > 2 ? oldPos - 2 : 0 + if (endMarkerRegex.test(output)) { if (this.callback != null) { this.callback(null, output.slice()) this.callback = null // only allow one callback in use diff --git a/services/spelling/test/acceptance/js/CheckTest.js b/services/spelling/test/acceptance/js/CheckTest.js index 981d64b563..2f55d992d8 100644 --- a/services/spelling/test/acceptance/js/CheckTest.js +++ b/services/spelling/test/acceptance/js/CheckTest.js @@ -3,11 +3,12 @@ const request = require('./helpers/request') const USER_ID = 101 -const checkWord = words => +const checkWord = (words, language) => request.post({ url: `/user/${USER_ID}/check`, body: JSON.stringify({ - words + words, + language }) }) @@ -78,4 +79,53 @@ describe('checking words', () => { } }) }) + + describe('when a very long list of words with utf8 responses', () => { + beforeEach(async () => { + let words = [] + for (let i = 0; i <= 20000; i++) { + words.push('anéther') + } + response = await checkWord(words, 'bg') // use Bulgarian to generate utf8 response + }) + + it('should return misspellings for the first 10K results only', async () => { + const body = JSON.parse(response.body) + expect(body.misspellings.length).to.equal(10000) + }) + + it('should have misspelling suggestions with consecutive indexes', async () => { + const body = JSON.parse(response.body) + const indexList = body.misspellings.map(mspl => mspl.index) + expect(indexList.length).to.equal(10000) // avoid testing over an incorrect array + for (let i = 0; i < indexList.length - 1; i++) { + expect(indexList[i] + 1).to.equal(indexList[i + 1]) + } + }) + }) + + + describe('when multiple words with utf8 are submitted', () => { + beforeEach(async () => { + response = await checkWord(['mneá', 'meniésn', 'meônoi', 'mneá'], 'pt_BR') + }) + + it('should return the misspellings for all the words', async () => { + const body = JSON.parse(response.body) + expect(body.misspellings.length).to.equal(4) + }) + + it('should have misspelling suggestions with consecutive indexes', async () => { + const body = JSON.parse(response.body) + const indexes = body.misspellings.map(mspl => mspl.index) + expect(indexes).to.deep.equal([0, 1, 2, 3]) + }) + + it('should return identical suggestions for the same entry', async () => { + const body = JSON.parse(response.body) + expect(body.misspellings[0].suggestions).to.deep.equal( + body.misspellings[3].suggestions + ) + }) + }) }) diff --git a/services/spelling/test/unit/js/ASpellWorkerTests.js b/services/spelling/test/unit/js/ASpellWorkerTests.js new file mode 100644 index 0000000000..9cdb7a8841 --- /dev/null +++ b/services/spelling/test/unit/js/ASpellWorkerTests.js @@ -0,0 +1,105 @@ +/* eslint-disable + handle-callback-err, + no-undef +*/ +const sinon = require('sinon') +const chai = require('chai') +const { expect } = chai +const SandboxedModule = require('sandboxed-module') +const EventEmitter = require('events') + +describe('ASpellWorker', function () { + beforeEach(function () { + this.child_process = {} + return (this.ASpellWorker = SandboxedModule.require('../../../app/js/ASpellWorker', { + requires: { + 'logger-sharelatex': { + log() { }, + info() { }, + err() { } + }, + 'metrics-sharelatex': { + gauge() { }, + inc() { } + }, + 'child_process': this.child_process + } + })) + }) + + describe("creating a worker", function () { + beforeEach(function () { + this.pipe = { + 'stdout': new EventEmitter(), + 'stderr': { on: sinon.stub() }, + 'stdin': {on: sinon.stub() }, + 'on': sinon.stub(), + 'pid': 12345 + } + this.child_process.spawn = sinon.stub().returns(this.pipe) + this.pipe.stdout.setEncoding = sinon.stub() + worker = new this.ASpellWorker('en') + + }) + + describe("with normal aspell output", function () { + beforeEach(function () { + this.callback = worker.callback = sinon.stub() + this.pipe.stdout.emit('data', '& hello\n') + this.pipe.stdout.emit('data', '& world\n') + this.pipe.stdout.emit('data', 'en\n') + this.pipe.stdout.emit('data', '& goodbye') + }) + + it('should call the callback', function() { + expect(this.callback.called).to.equal(true) + expect(this.callback.calledWith(null, "& hello\n& world\nen\n")).to.equal(true) + }) + }) + + describe("with the aspell end marker split across chunks", function () { + beforeEach(function () { + this.callback = worker.callback = sinon.stub() + this.pipe.stdout.emit('data', '& hello\n') + this.pipe.stdout.emit('data', '& world\ne') + this.pipe.stdout.emit('data', 'n\n') + this.pipe.stdout.emit('data', '& goodbye') + }) + + it('should call the callback', function() { + expect(this.callback.called).to.equal(true) + expect(this.callback.calledWith(null, "& hello\n& world\nen\n")).to.equal(true) + }) + }) + + describe("with the aspell end marker newline split across chunks", function () { + beforeEach(function () { + this.callback = worker.callback = sinon.stub() + this.pipe.stdout.emit('data', '& hello\n') + this.pipe.stdout.emit('data', '& world\n') + this.pipe.stdout.emit('data', 'en') + this.pipe.stdout.emit('data', '\n& goodbye') + }) + + it('should call the callback', function() { + expect(this.callback.called).to.equal(true) + expect(this.callback.calledWith(null, "& hello\n& world\nen")).to.equal(true) + }) + }) + + describe("with everything split across chunks", function () { + beforeEach(function () { + this.callback = worker.callback = sinon.stub() + '& hello\n& world\nen\n& goodbye'.split('').forEach(x => { + this.pipe.stdout.emit('data', x) + }) + }) + + it('should call the callback', function() { + expect(this.callback.called).to.equal(true) + expect(this.callback.calledWith(null, "& hello\n& world\nen")).to.equal(true) + }) + }) + + }) +})