Merge pull request #6237 from overleaf/em-revert-jumprope

Revert use of the jumprope library in document-updater

GitOrigin-RevId: bf83f04cf5a53bf710af74a366f4dd86c06a1fd5

services/document-updater/app/js/sharejs/types/text.js

@@ -32,8 +32,6 @@
 // NOTE: The global scope here is shared with other sharejs files when built with closure.
 // Be careful what ends up in your namespace.
 
-const Rope = require('jumprope')
-
 let append, transformComponent
 const text = {}
 
@@ -70,23 +68,28 @@ const checkValidOp = function (op) {
 }
 
 text.apply = function (snapshot, op) {
-  const rope = new Rope(snapshot)
   checkValidOp(op)
   for (const component of Array.from(op)) {
     if (component.i != null) {
-      rope.insert(component.p, component.i)
+      snapshot = strInject(snapshot, component.p, component.i)
     } else if (component.d != null) {
-      const deleted = ropeSubstring(rope, component.p, component.d.length)
+      const deleted = snapshot.slice(
+        component.p,
+        component.p + component.d.length
+      )
       if (component.d !== deleted) {
         throw new Error(
           `Delete component '${component.d}' does not match deleted text '${deleted}'`
         )
       }
-      rope.del(component.p, component.d.length)
+      snapshot =
+        snapshot.slice(0, component.p) +
+        snapshot.slice(component.p + component.d.length)
     } else if (component.c != null) {
-      // The rope has strict bounds checks, so we need to make sure we don't
+      const comment = snapshot.slice(
-      // extract text beyond the end of the rope
+        component.p,
-      const comment = ropeSubstring(rope, component.p, component.c.length)
+        component.p + component.c.length
+      )
       if (component.c !== comment) {
         throw new Error(
           `Comment component '${component.c}' does not match commented text '${comment}'`
@@ -96,7 +99,7 @@ text.apply = function (snapshot, op) {
       throw new Error('Unknown op type')
     }
   }
-  return rope.toString()
+  return snapshot
 }
 
 // Exported for use by the random op generator.
@@ -385,11 +388,3 @@ if (typeof WEB !== 'undefined' && WEB !== null) {
     append
   )
 }
-
-function ropeSubstring(rope, start, length) {
-  // The rope has strict bounds checks, so we need to make sure we don't
-  // extract text beyond the end of the rope.
-  start = Math.min(rope.length, start)
-  length = Math.min(rope.length - start, length)
-  return rope.substring(start, length)
-}

services/document-updater/benchmarks/apply_string_rope.js

@@ -1,43 +0,0 @@
-const { Suite } = require('benchmark')
-const Rope = require('jumprope')
-
-const suite = new Suite()
-
-for (const size of [5000, 100000, 2000000]) {
-  const positions = []
-  for (let i = 0; i < 100; i++) {
-    positions.push(Math.floor(Math.random() * size))
-  }
-  for (const inserts of [0, 1, 2, 5, 10, 20, 100, 1000, 10000]) {
-    const initialString = 'a'.repeat(size)
-    suite.add(`| string | ${inserts} | ${size} |`, () => {
-      let str = initialString
-      for (let i = 0; i < inserts; i++) {
-        const pos = positions[i % 100]
-        str = str.slice(0, pos) + '1234567890' + str.slice(pos)
-      }
-      return str.length
-    })
-
-    suite.add(`| rope | ${inserts} | ${size} |`, () => {
-      const rope = new Rope(initialString)
-      for (let i = 0; i < inserts; i++) {
-        const pos = positions[i % 100]
-        rope.insert(pos, '1234567890')
-      }
-      return rope.toString().length
-    })
-  }
-}
-
-suite.on('cycle', function (event) {
-  const meanMs = event.target.stats.mean * 1000
-  const meanToString = Math.ceil(meanMs * 100000) / 100000
-  console.log(`${event.target.name} ${meanToString}ms | `)
-})
-
-// markdown table headers
-console.log('| method | inserts | string size | mean |')
-console.log('|-|-|-|-|')
-
-suite.run({ async: true })

services/document-updater/package-lock.json

@@ -575,16 +575,6 @@
         "tweetnacl": "^0.14.3"
       }
     },
-    "benchmark": {
-      "version": "2.1.4",
-      "resolved": "https://registry.npmjs.org/benchmark/-/benchmark-2.1.4.tgz",
-      "integrity": "sha1-CfPeMckWQl1JjMLuVloOvzwqVik=",
-      "dev": true,
-      "requires": {
-        "lodash": "^4.17.4",
-        "platform": "^1.3.3"
-      }
-    },
     "bignumber.js": {
       "version": "9.0.1",
       "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.1.tgz",
@@ -1732,10 +1722,6 @@
         "verror": "1.10.0"
       }
     },
-    "jumprope": {
-      "version": "https://github.com/overleaf/jumprope/archive/b3ce482e83925a6c23278f3230a47a82bd440592.tar.gz",
-      "integrity": "sha512-VSZr8UrKvY551MPwPR8lramBcCcnMDBh03qliCb1q2RaeQQR1GEsbJ5rAY1PCITtZXCOqeYgsEP0cSj2A0eY8w=="
-    },
     "just-extend": {
       "version": "4.2.1",
       "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.2.1.tgz",
@@ -2284,12 +2270,6 @@
       "resolved": "https://registry.npmjs.org/pify/-/pify-5.0.0.tgz",
       "integrity": "sha512-eW/gHNMlxdSP6dmG6uJip6FXN0EQBwm2clYYd8Wul42Cwu/DK8HEftzsapcNdYe2MfLiIwZqsDk2RDEsTE79hA=="
     },
-    "platform": {
-      "version": "1.3.6",
-      "resolved": "https://registry.npmjs.org/platform/-/platform-1.3.6.tgz",
-      "integrity": "sha512-fnWVljUchTro6RiCFvCXBbNhJc2NijN7oIQxbwsyL0buWJPG85v81ehlHI9fXrJsMNgTofEoWIQeClKpgxFLrg==",
-      "dev": true
-    },
     "pprof": {
       "version": "3.2.0",
       "resolved": "https://registry.npmjs.org/pprof/-/pprof-3.2.0.tgz",

services/document-updater/package.json

@@ -29,14 +29,12 @@
     "bunyan": "^1.8.15",
     "diff-match-patch": "https://github.com/overleaf/diff-match-patch/archive/89805f9c671a77a263fc53461acd62aa7498f688.tar.gz",
     "express": "4.17.1",
-    "jumprope": "https://github.com/overleaf/jumprope/archive/b3ce482e83925a6c23278f3230a47a82bd440592.tar.gz",
     "lodash": "^4.17.21",
     "mongodb": "^3.6.6",
     "request": "^2.88.2",
     "requestretry": "^4.1.2"
   },
   "devDependencies": {
-    "benchmark": "^2.1.4",
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
     "cluster-key-slot": "^1.0.5",

services/document-updater/test/unit/js/ShareJS/TextTransformTests.js

@@ -13,7 +13,6 @@
  * DS205: Consider reworking code to avoid use of IIFEs
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-const { expect } = require('chai')
 const text = require('../../../../app/js/sharejs/types/text')
 const RangesTracker = require('../../../../app/js/RangesTracker')
 
@@ -269,84 +268,15 @@ describe('ShareJS text type', function () {
         .should.equal('foo123bar')
     })
 
-    it('should apply delete operations that are not in order', function () {
-      const result = text.apply('0123456789', [
-        { d: '9', p: 9 },
-        { d: '4', p: 4 },
-        { d: '7', p: 6 },
-      ])
-      expect(result).to.equal('0123568')
-    })
-
-    it('should apply a mix of inserts and deletes in any order', function () {
-      const result = text.apply('we can insert and delete in any order!', [
-        { d: 'any ', p: 28 }, // we can insert and delete in order!
-        { i: 'not', p: 6 }, // we cannot insert and delete in order!
-        { d: 'insert and ', p: 10 }, // we cannot delete in order!
-        { i: 'the same ', p: 20 },
-      ])
-      expect(result).to.equal('we cannot delete in the same order!')
-    })
-
-    it('should apply a mix of inserts and deletes in order', function () {
-      const result = text.apply('we can insert and delete in any order!', [
-        { i: 'not', p: 6 }, // we cannot insert and delete in any order!
-        { d: 'insert and ', p: 10 }, // we cannot delete in any order!
-        { d: 'any ', p: 20 }, // we cannot delete in order!
-        { i: 'the same ', p: 20 },
-      ])
-      expect(result).to.equal('we cannot delete in the same order!')
-    })
-
-    it('should be able to insert a string, then delete it along with some more text that comes before', function () {
-      const result = text.apply('I love cake and cookies', [
-        { i: ', vegetables', p: 11 }, // I love cake, vegetables and cookies
-        { d: 'cake, vege', p: 7 },
-      ])
-      expect(result).to.equal('I love tables and cookies')
-    })
-
-    it('should be able to insert a string, then delete it along with some more text that comes after', function () {
-      const result = text.apply('I love cake and cookies', [
-        { i: 'chocolate, ', p: 7 }, // I love chocolate, cake and cookies
-        { d: ', cake and', p: 16 },
-      ])
-      expect(result).to.equal('I love chocolate cookies')
-    })
-
     it('should throw an error when deleted content does not match', function () {
-      ;(() => text.apply('foo123bar', [{ d: '456', p: 3 }])).should.throw(
+      return (() => text.apply('foo123bar', [{ d: '456', p: 3 }])).should.throw(
-        /^Delete component/
+        Error
       )
     })
 
-    it('should throw an error when deleted content extends beyond the text', function () {
+    return it('should throw an error when comment content does not match', function () {
-      ;(() => text.apply('foo123bar', [{ d: '123barbaz', p: 3 }])).should.throw(
+      return (() => text.apply('foo123bar', [{ c: '456', p: 3 }])).should.throw(
-        /^Delete component/
+        Error
-      )
-    })
-
-    it('should throw an error when deleted content is out of bounds', function () {
-      ;(() => text.apply('foo123bar', [{ d: '456', p: 20 }])).should.throw(
-        /^Delete component/
-      )
-    })
-
-    it('should throw an error when comment content does not match', function () {
-      ;(() => text.apply('foo123bar', [{ c: '456', p: 3 }])).should.throw(
-        /^Comment component/
-      )
-    })
-
-    it('should throw an error when commented content extends beyond the text', function () {
-      ;(() => text.apply('foo123bar', [{ c: '123barbaz', p: 3 }])).should.throw(
-        /^Comment component/
-      )
-    })
-
-    it('should throw an error when commented content is out of bounds', function () {
-      ;(() => text.apply('foo123bar', [{ c: '456', p: 20 }])).should.throw(
-        /^Comment component/
       )
     })
   })