Sfoglia il codice sorgente

Merge pull request #2561 from overleaf/pr-reduce-spellcheck-initial-req

Reduce spellcheck initial (and subsequent) requests

GitOrigin-RevId: 81eee359ea1bc740991ba84b0fe121c37cb4c6a2
Simon Detheridge 6 anni fa
parent
commit
73defe82d9

+ 89 - 92
services/web/frontend/js/ide/editor/directives/aceEditor.js

@@ -154,12 +154,9 @@ define([
 
         if (scope.spellCheck) {
           // only enable spellcheck when explicitly required
-          const spellCheckCache =
-            $cacheFactory.get(`spellCheck-${scope.name}`) ||
-            $cacheFactory(`spellCheck-${scope.name}`, { capacity: 1000 })
           spellCheckManager = new SpellCheckManager(
             scope,
-            spellCheckCache,
+            $cacheFactory,
             $http,
             $q,
             new SpellCheckAdapter(editor)
@@ -893,63 +890,63 @@ define([
 
       template: `\
 <div class="ace-editor-wrapper">
-	<div
-		class="undo-conflict-warning alert alert-danger small"
-		ng-show="undo.show_remote_warning"
-	>
-		<strong>Watch out!</strong>
-		We had to undo some of your collaborators changes before we could undo yours.
-		<a
-			href="#"
-			class="pull-right"
-			ng-click="undo.show_remote_warning = false"
-		>Dismiss</a>
-	</div>
-	<div class="ace-editor-body"></div>
-	<spell-menu
-		open="spellMenu.open"
-		top="spellMenu.top"
-		left="spellMenu.left"
-		layout-from-bottom="spellMenu.layoutFromBottom"
-		highlight="spellMenu.highlight"
-		replace-word="replaceWord(highlight, suggestion)"
-		learn-word="learnWord(highlight)"
-	></spell-menu>
-	<div
-		class="annotation-label"
-		ng-show="annotationLabel.show"
-		ng-style="{
-			position: 'absolute',
-			left:     annotationLabel.left,
-			right:    annotationLabel.right,
-			bottom:   annotationLabel.bottom,
-			top:      annotationLabel.top,
-			'background-color': annotationLabel.backgroundColor
-		}"
-	>
-		{{ annotationLabel.text }}
-	</div>
-
-	<a
-		href
-		class="highlights-before-label btn btn-info btn-xs"
-		ng-show="updateLabels.highlightsBefore > 0"
-		ng-click="gotoHighlightAbove()"
-	>
-		<i class="fa fa-fw fa-arrow-up"></i>
-		{{ updateLabels.highlightsBefore }} more update{{ updateLabels.highlightsBefore > 1 && "" || "s" }} above
-	</a>
-
-	<a
-		href
-		class="highlights-after-label btn btn-info btn-xs"
-		ng-show="updateLabels.highlightsAfter > 0"
-		ng-click="gotoHighlightBelow()"
-	>
-		<i class="fa fa-fw fa-arrow-down"></i>
-		{{ updateLabels.highlightsAfter }} more update{{ updateLabels.highlightsAfter > 1 && "" || "s" }} below
-
-	</a>
+  <div
+    class="undo-conflict-warning alert alert-danger small"
+    ng-show="undo.show_remote_warning"
+  >
+    <strong>Watch out!</strong>
+    We had to undo some of your collaborators changes before we could undo yours.
+    <a
+      href="#"
+      class="pull-right"
+      ng-click="undo.show_remote_warning = false"
+    >Dismiss</a>
+  </div>
+  <div class="ace-editor-body"></div>
+  <spell-menu
+    open="spellMenu.open"
+    top="spellMenu.top"
+    left="spellMenu.left"
+    layout-from-bottom="spellMenu.layoutFromBottom"
+    highlight="spellMenu.highlight"
+    replace-word="replaceWord(highlight, suggestion)"
+    learn-word="learnWord(highlight)"
+  ></spell-menu>
+  <div
+    class="annotation-label"
+    ng-show="annotationLabel.show"
+    ng-style="{
+      position: 'absolute',
+      left:     annotationLabel.left,
+      right:    annotationLabel.right,
+      bottom:   annotationLabel.bottom,
+      top:      annotationLabel.top,
+      'background-color': annotationLabel.backgroundColor
+    }"
+  >
+    {{ annotationLabel.text }}
+  </div>
+
+  <a
+    href
+    class="highlights-before-label btn btn-info btn-xs"
+    ng-show="updateLabels.highlightsBefore > 0"
+    ng-click="gotoHighlightAbove()"
+  >
+    <i class="fa fa-fw fa-arrow-up"></i>
+    {{ updateLabels.highlightsBefore }} more update{{ updateLabels.highlightsBefore > 1 && "" || "s" }} above
+  </a>
+
+  <a
+    href
+    class="highlights-after-label btn btn-info btn-xs"
+    ng-show="updateLabels.highlightsAfter > 0"
+    ng-click="gotoHighlightBelow()"
+  >
+    <i class="fa fa-fw fa-arrow-down"></i>
+    {{ updateLabels.highlightsAfter }} more update{{ updateLabels.highlightsAfter > 1 && "" || "s" }} below
+
+  </a>
 </div>\
 `
     }
@@ -958,37 +955,37 @@ define([
   function monkeyPatchSearch($rootScope, $compile) {
     const searchHtml = `\
 <div class="ace_search right">
-	<a href type="button" action="hide" class="ace_searchbtn_close">
-		<i class="fa fa-fw fa-times"></i>
-	</a>
-	<div class="ace_search_form">
-		<input class="ace_search_field form-control input-sm" placeholder="Search for" spellcheck="false"></input>
-		<div class="btn-group">
-			<button type="button" action="findNext" class="ace_searchbtn next btn btn-default btn-sm">
-				<i class="fa fa-chevron-down fa-fw"></i>
-			</button>
-			<button type="button" action="findPrev" class="ace_searchbtn prev btn btn-default btn-sm">
-				<i class="fa fa-chevron-up fa-fw"></i>
-			</button>
-		</div>
-	</div>
-	<div class="ace_replace_form">
-		<input class="ace_search_field form-control input-sm" placeholder="Replace with" spellcheck="false"></input>
-		<div class="btn-group">
-			<button type="button" action="replaceAndFindNext" class="ace_replacebtn btn btn-default btn-sm">Replace</button>
-			<button type="button" action="replaceAll" class="ace_replacebtn btn btn-default btn-sm">All</button>
-		</div>
-	</div>
-	<div class="ace_search_options">
-		<div class="btn-group">
-			<button action="toggleRegexpMode" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="RegExp Search">.*</button>
-			<button action="toggleCaseSensitive" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="CaseSensitive Search">Aa</button>
-			<button action="toggleWholeWords" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="Whole Word Search">"..."</button>
-			<button action="searchInSelection" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="Search Within Selection"><i class="fa fa-align-left"></i></button>
-		</div>
-		<span class="ace_search_counter"></span>
-	</div>
-	<div action="toggleReplace" class="hidden"></div>
+  <a href type="button" action="hide" class="ace_searchbtn_close">
+    <i class="fa fa-fw fa-times"></i>
+  </a>
+  <div class="ace_search_form">
+    <input class="ace_search_field form-control input-sm" placeholder="Search for" spellcheck="false"></input>
+    <div class="btn-group">
+      <button type="button" action="findNext" class="ace_searchbtn next btn btn-default btn-sm">
+        <i class="fa fa-chevron-down fa-fw"></i>
+      </button>
+      <button type="button" action="findPrev" class="ace_searchbtn prev btn btn-default btn-sm">
+        <i class="fa fa-chevron-up fa-fw"></i>
+      </button>
+    </div>
+  </div>
+  <div class="ace_replace_form">
+    <input class="ace_search_field form-control input-sm" placeholder="Replace with" spellcheck="false"></input>
+    <div class="btn-group">
+      <button type="button" action="replaceAndFindNext" class="ace_replacebtn btn btn-default btn-sm">Replace</button>
+      <button type="button" action="replaceAll" class="ace_replacebtn btn btn-default btn-sm">All</button>
+    </div>
+  </div>
+  <div class="ace_search_options">
+    <div class="btn-group">
+      <button action="toggleRegexpMode" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="RegExp Search">.*</button>
+      <button action="toggleCaseSensitive" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="CaseSensitive Search">Aa</button>
+      <button action="toggleWholeWords" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="Whole Word Search">"..."</button>
+      <button action="searchInSelection" class="btn btn-default btn-sm" tooltip-placement="bottom" tooltip-append-to-body="true" tooltip="Search Within Selection"><i class="fa fa-align-left"></i></button>
+    </div>
+    <span class="ace_search_counter"></span>
+  </div>
+  <div action="toggleReplace" class="hidden"></div>
 </div>\
 `
 

+ 23 - 18
services/web/frontend/js/ide/editor/directives/aceEditor/spell-check/SpellCheckAdapter.js

@@ -1,23 +1,10 @@
-/* eslint-disable
-    max-len,
-    no-undef,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 define([
   'ace/ace',
   'ide/editor/directives/aceEditor/spell-check/HighlightedWordManager'
 ], function(Ace, HighlightedWordManager) {
-  let SpellCheckAdapter
   const { Range } = ace.require('ace/range')
 
-  return (SpellCheckAdapter = class SpellCheckAdapter {
+  class SpellCheckAdapter {
     constructor(editor) {
       this.replaceWord = this.replaceWord.bind(this)
       this.editor = editor
@@ -28,6 +15,22 @@ define([
       return this.editor.getValue().split('\n')
     }
 
+    getLineCount() {
+      return this.editor.session.getLength()
+    }
+
+    getFirstVisibleRowNum() {
+      return this.editor.renderer.layerConfig.firstRow
+    }
+
+    getLastVisibleRowNum() {
+      return this.editor.renderer.layerConfig.lastRow
+    }
+
+    getLinesByRows(rows) {
+      return rows.map(rowIdx => this.editor.session.doc.getLine(rowIdx))
+    }
+
     normalizeChangeEvent(e) {
       return e
     }
@@ -41,7 +44,7 @@ define([
     }
 
     preventContextMenuEventDefault(e) {
-      return e.domEvent.preventDefault()
+      e.domEvent.preventDefault()
     }
 
     getHighlightFromCoords(coords) {
@@ -68,7 +71,7 @@ define([
       const startColumn = highlight.range.start.column
       const endColumn = highlight.range.end.column
 
-      return this.editor
+      this.editor
         .getSession()
         .getSelection()
         .setSelectionRange(new Range(row, startColumn, row, endColumn))
@@ -84,7 +87,9 @@ define([
         .replace(new Range(row, startColumn, row, endColumn), newWord)
 
       // Bring editor back into focus after clicking on suggestion
-      return this.editor.focus()
+      this.editor.focus()
     }
-  })
+  }
+
+  return SpellCheckAdapter
 })

File diff suppressed because it is too large
+ 6 - 27
services/web/frontend/js/ide/editor/directives/aceEditor/spell-check/SpellCheckManager.js


+ 223 - 8
services/web/test/frontend/ide/editor/aceEditor/spell-check/SpellCheckManagerTests.js

@@ -15,7 +15,6 @@ define([
   describe('SpellCheckManager', function() {
     beforeEach(function(done) {
       this.timelord = sinon.useFakeTimers()
-
       window.user = { id: 1 }
       window.csrfToken = 'token'
       this.scope = {
@@ -29,17 +28,19 @@ define([
         addHighlight: sinon.stub()
       }
       this.adapter = {
-        getLines: sinon.stub(),
+        getLineCount: sinon.stub(),
+        getFirstVisibleRowNum: sinon.stub(),
+        getLastVisibleRowNum: sinon.stub(),
+        getLinesByRows: sinon.stub(),
         highlightedWordManager: this.highlightedWordManager
       }
       return inject(($q, $http, $httpBackend, $cacheFactory) => {
         this.$http = $http
         this.$q = $q
         this.$httpBackend = $httpBackend
-        const cache = $cacheFactory('spellCheckTest', { capacity: 1000 })
         this.spellCheckManager = new SpellCheckManager(
           this.scope,
-          cache,
+          $cacheFactory,
           $http,
           $q,
           this.adapter
@@ -52,7 +53,7 @@ define([
       return this.timelord.restore()
     })
 
-    it('runs a full check soon after init', function() {
+    it('adds an highlight when a misspelling is found', function() {
       this.$httpBackend.when('POST', '/spelling/check').respond({
         misspellings: [
           {
@@ -61,11 +62,225 @@ define([
           }
         ]
       })
-      this.adapter.getLines.returns(['oppozition'])
+      this.adapter.getLinesByRows.returns(['oppozition'])
       this.spellCheckManager.init()
       this.timelord.tick(200)
       this.$httpBackend.flush()
-      return expect(this.highlightedWordManager.addHighlight).to.have.been
-        .called
+      expect(this.highlightedWordManager.addHighlight).to.have.been.called
+    })
+
+    describe('runSpellCheck', function() {
+      beforeEach(function() {
+        this.adapter.getLineCount.returns(10)
+        this.adapter.getFirstVisibleRowNum.returns(3)
+        this.adapter.getLastVisibleRowNum.returns(5)
+        this.adapter.getLinesByRows.returns([
+          'Lorem ipsum dolor sit amet',
+          'consectetur adipisicing elit',
+          'sed do eiusmod'
+        ])
+        this.$httpBackend.when('POST', '/spelling/check').respond({
+          misspellings: [
+            {
+              index: 0,
+              suggestions: ['opposition']
+            }
+          ]
+        })
+      })
+      it('only checks visible lines', function() {
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(this.adapter.getLinesByRows).to.have.been.calledWith([3, 4, 5])
+      })
+
+      it('ignores updated lines', function() {
+        this.spellCheckManager.init()
+        this.spellCheckManager.changedLines[4] = false
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(this.adapter.getLinesByRows).to.have.been.calledWith([3, 5])
+      })
+
+      it('clears highlights for changed lines', function() {
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(
+          this.highlightedWordManager.clearRow.getCall(0).args[0]
+        ).to.equal(3)
+        expect(
+          this.highlightedWordManager.clearRow.getCall(1).args[0]
+        ).to.equal(4)
+        expect(
+          this.highlightedWordManager.clearRow.getCall(2).args[0]
+        ).to.equal(5)
+      })
+
+      it('initially flags all lines as dirty', function() {
+        this.spellCheckManager.init()
+        expect(this.spellCheckManager.changedLines)
+          .to.have.lengthOf(10)
+          .and.to.not.include(false)
+      })
+
+      it('initially flags checked lines as non-dirty', function() {
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(this.spellCheckManager.changedLines[2]).to.equal(true)
+        expect(this.spellCheckManager.changedLines[3]).to.equal(false)
+        expect(this.spellCheckManager.changedLines[4]).to.equal(false)
+        expect(this.spellCheckManager.changedLines[5]).to.equal(false)
+        expect(this.spellCheckManager.changedLines[6]).to.equal(true)
+      })
+    })
+
+    describe('cache', function() {
+      beforeEach(function() {
+        this.adapter.getLineCount.returns(1)
+        this.adapter.getFirstVisibleRowNum.returns(1)
+        this.adapter.getLastVisibleRowNum.returns(1)
+        this.adapter.getLinesByRows.returns(['Lorem ipsum dolor'])
+        this.$httpBackend.when('POST', '/spelling/check').respond({
+          misspellings: [
+            {
+              index: 0,
+              suggestions: ['foobarbaz']
+            }
+          ]
+        })
+      })
+
+      it('adds already checked words to the spellchecker cache', function() {
+        expect(this.spellCheckManager.cache.info().size).to.equal(0)
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(this.spellCheckManager.cache.info().size).to.equal(3)
+      })
+
+      it('adds misspeled word suggestions to the cache', function() {
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+
+        expect(
+          this.spellCheckManager.cache.get(
+            `${this.scope.spellCheckLanguage}:Lorem`
+          )
+        ).to.eql(['foobarbaz'])
+      })
+
+      it('adds non-misspeled words to the cache as a boolean', function() {
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        expect(
+          this.spellCheckManager.cache.get(
+            `${this.scope.spellCheckLanguage}:ipsum`
+          )
+        ).to.equal(true)
+      })
+    })
+
+    describe('backend', function() {
+      beforeEach(function() {
+        this.adapter.getLineCount.returns(1)
+        this.adapter.getFirstVisibleRowNum.returns(1)
+        this.adapter.getLastVisibleRowNum.returns(1)
+        this.adapter.getLinesByRows.returns(['Lorem ipsum dolor'])
+      })
+
+      it('hits the backend with all words at startup', function() {
+        this.$httpBackend
+          .expect('POST', '/spelling/check', {
+            language: this.scope.spellCheckLanguage,
+            words: ['Lorem', 'ipsum', 'dolor'],
+            token: window.user.id,
+            _csrf: window.csrfToken
+          })
+          .respond({
+            misspellings: [
+              {
+                index: 0,
+                suggestions: ['foobarbaz']
+              }
+            ]
+          })
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+      })
+
+      it('does not hit the backend when all words are already in the cache', function() {
+        this.$httpBackend
+          .expect('POST', '/spelling/check', {
+            language: this.scope.spellCheckLanguage,
+            words: ['Lorem', 'ipsum', 'dolor'],
+            token: window.user.id,
+            _csrf: window.csrfToken
+          })
+          .respond({
+            misspellings: [
+              {
+                index: 0,
+                suggestions: ['foobarbaz']
+              }
+            ]
+          })
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+      })
+
+      it('hits the backend only with non-cached words', function() {
+        this.$httpBackend
+          .expect('POST', '/spelling/check', {
+            language: this.scope.spellCheckLanguage,
+            words: ['Lorem', 'ipsum', 'dolor'],
+            token: window.user.id,
+            _csrf: window.csrfToken
+          })
+          .respond({
+            misspellings: [
+              {
+                index: 0,
+                suggestions: ['foobarbaz']
+              }
+            ]
+          })
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+
+        this.adapter.getLinesByRows.returns(['Lorem ipsum dolor sit amet'])
+        this.$httpBackend
+          .expect('POST', '/spelling/check', {
+            language: this.scope.spellCheckLanguage,
+            words: ['sit', 'amet'],
+            token: window.user.id,
+            _csrf: window.csrfToken
+          })
+          .respond({
+            misspellings: [
+              {
+                index: 0,
+                suggestions: ['bazbarfoo']
+              }
+            ]
+          })
+        this.spellCheckManager.init()
+        this.timelord.tick(200)
+        this.$httpBackend.flush()
+      })
+
+      afterEach(function() {
+        this.$httpBackend.verifyNoOutstandingRequest()
+        this.$httpBackend.verifyNoOutstandingExpectation()
+      })
     })
   }))

Some files were not shown because too many files changed in this diff