From f273a27b9534419c50f8323208779d34ceb52534 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 27 Sep 2019 11:28:03 +0100 Subject: [PATCH 1/6] use utf8 encoding for stdout from aspell --- services/spelling/app/js/ASpellWorker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/services/spelling/app/js/ASpellWorker.js b/services/spelling/app/js/ASpellWorker.js index 5d5e510390..66edfe8009 100644 --- a/services/spelling/app/js/ASpellWorker.js +++ b/services/spelling/app/js/ASpellWorker.js @@ -128,6 +128,7 @@ 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 => { From b0e0eeff3cc1eb05825c8f9157c2a7941cf1b503 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 27 Sep 2019 11:57:45 +0100 Subject: [PATCH 2/6] look for end marker across chunk boundaries --- services/spelling/app/js/ASpellWorker.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/services/spelling/app/js/ASpellWorker.js b/services/spelling/app/js/ASpellWorker.js index 66edfe8009..6d9921ece4 100644 --- a/services/spelling/app/js/ASpellWorker.js +++ b/services/spelling/app/js/ASpellWorker.js @@ -130,11 +130,16 @@ 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]', 'm') + 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 From d96ec9d7fd994e00fdd157037c95c73ad5c83d35 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 27 Sep 2019 11:58:35 +0100 Subject: [PATCH 3/6] add acceptance tests for utf8 responses --- .../spelling/test/acceptance/js/CheckTest.js | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) 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 + ) + }) + }) }) From 1f861a67720a18a6246f123eeaffafba77f0055a Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 2 Oct 2019 16:29:18 +0100 Subject: [PATCH 4/6] fix regex and add AspellWorker unit tests --- services/spelling/app/js/ASpellWorker.js | 2 +- .../test/unit/js/ASpellWorkerTests.js | 112 ++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 services/spelling/test/unit/js/ASpellWorkerTests.js diff --git a/services/spelling/app/js/ASpellWorker.js b/services/spelling/app/js/ASpellWorker.js index 6d9921ece4..c4ead15083 100644 --- a/services/spelling/app/js/ASpellWorker.js +++ b/services/spelling/app/js/ASpellWorker.js @@ -130,7 +130,7 @@ class ASpellWorker { this.pipe.stdout.setEncoding('utf8') // ensure utf8 output is handled correctly var output = '' - const endMarkerRegex = new RegExp('^[a-z][a-z]', 'm') + 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. diff --git a/services/spelling/test/unit/js/ASpellWorkerTests.js b/services/spelling/test/unit/js/ASpellWorkerTests.js new file mode 100644 index 0000000000..fa66f2ac05 --- /dev/null +++ b/services/spelling/test/unit/js/ASpellWorkerTests.js @@ -0,0 +1,112 @@ +/* eslint-disable + handle-callback-err, + no-undef +*/ +// TODO: This file was created by bulk-decaffeinate. +// Sanity-check the conversion and remove this comment. +/* + * decaffeinate suggestions: + * DS102: Remove unnecessary code created because of implicit returns + * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md + */ +const sinon = require('sinon') +const chai = require('chai') +const { should, expect, assert } = 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) + }) + }) + + }) +}) From 0e14ad50dbbedc42d2adf2fbfd58976187807477 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 2 Oct 2019 16:34:15 +0100 Subject: [PATCH 5/6] fix eslint check --- services/spelling/test/unit/js/ASpellWorkerTests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spelling/test/unit/js/ASpellWorkerTests.js b/services/spelling/test/unit/js/ASpellWorkerTests.js index fa66f2ac05..381d5ad468 100644 --- a/services/spelling/test/unit/js/ASpellWorkerTests.js +++ b/services/spelling/test/unit/js/ASpellWorkerTests.js @@ -11,7 +11,7 @@ */ const sinon = require('sinon') const chai = require('chai') -const { should, expect, assert } = chai +const { expect } = chai const SandboxedModule = require('sandboxed-module') const EventEmitter = require('events') From c7a7887d6718990a29e764d28cb44a23104b8a3f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 2 Oct 2019 16:47:36 +0100 Subject: [PATCH 6/6] remove decaff comments --- services/spelling/test/unit/js/ASpellWorkerTests.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/spelling/test/unit/js/ASpellWorkerTests.js b/services/spelling/test/unit/js/ASpellWorkerTests.js index 381d5ad468..9cdb7a8841 100644 --- a/services/spelling/test/unit/js/ASpellWorkerTests.js +++ b/services/spelling/test/unit/js/ASpellWorkerTests.js @@ -2,13 +2,6 @@ handle-callback-err, no-undef */ -// TODO: This file was created by bulk-decaffeinate. -// Sanity-check the conversion and remove this comment. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const chai = require('chai') const { expect } = chai