diff --git a/libraries/promise-utils/.dockerignore b/libraries/promise-utils/.dockerignore new file mode 100644 index 0000000000..c2658d7d1b --- /dev/null +++ b/libraries/promise-utils/.dockerignore @@ -0,0 +1 @@ +node_modules/ diff --git a/libraries/promise-utils/.gitignore b/libraries/promise-utils/.gitignore new file mode 100644 index 0000000000..edb0f85350 --- /dev/null +++ b/libraries/promise-utils/.gitignore @@ -0,0 +1,3 @@ + +# managed by monorepo$ bin/update_build_scripts +.npmrc diff --git a/libraries/promise-utils/.mocharc.json b/libraries/promise-utils/.mocharc.json new file mode 100644 index 0000000000..3be9ee53ae --- /dev/null +++ b/libraries/promise-utils/.mocharc.json @@ -0,0 +1,5 @@ +{ + "ui": "bdd", + "recursive": "true", + "reporter": "spec" +} diff --git a/libraries/promise-utils/.nvmrc b/libraries/promise-utils/.nvmrc new file mode 100644 index 0000000000..02c8b485ed --- /dev/null +++ b/libraries/promise-utils/.nvmrc @@ -0,0 +1 @@ +18.18.0 diff --git a/libraries/promise-utils/buildscript.txt b/libraries/promise-utils/buildscript.txt new file mode 100644 index 0000000000..2425d7ee05 --- /dev/null +++ b/libraries/promise-utils/buildscript.txt @@ -0,0 +1,10 @@ +promise-utils +--dependencies=None +--docker-repos=gcr.io/overleaf-ops +--env-add= +--env-pass-through= +--esmock-loader=False +--is-library=True +--node-version=18.18.0 +--public-repo=False +--script-version=4.4.0 diff --git a/libraries/promise-utils/index.js b/libraries/promise-utils/index.js new file mode 100644 index 0000000000..58bdc81639 --- /dev/null +++ b/libraries/promise-utils/index.js @@ -0,0 +1,214 @@ +const { promisify, callbackify } = require('util') +const pLimit = require('p-limit') + +module.exports = { + promisify, + promisifyAll, + promisifyClass, + promisifyMultiResult, + callbackify, + callbackifyAll, + callbackifyMultiResult, + expressify, + promiseMapWithLimit, +} + +/** + * Promisify all functions in a module. + * + * This is meant to be used only when all functions in the module are async + * callback-style functions. + * + * It's very much tailored to our current module structure. In particular, it + * binds `this` to the module when calling the function in order not to break + * modules that call sibling functions using `this`. + * + * This will not magically fix all modules. Special cases should be promisified + * manually. + * + * The second argument is a bag of options: + * + * - without: an array of function names that shouldn't be promisified + * + * - multiResult: an object whose keys are function names and values are lists + * of parameter names. This is meant for functions that invoke their callbacks + * with more than one result in separate parameters. The promisifed function + * will return these results as a single object, with each result keyed under + * the corresponding parameter name. + */ +function promisifyAll(module, opts = {}) { + const { without = [], multiResult = {} } = opts + const promises = {} + for (const propName of Object.getOwnPropertyNames(module)) { + if (without.includes(propName)) { + continue + } + const propValue = module[propName] + if (typeof propValue !== 'function') { + continue + } + if (multiResult[propName] != null) { + promises[propName] = promisifyMultiResult( + propValue, + multiResult[propName] + ).bind(module) + } else { + promises[propName] = promisify(propValue).bind(module) + } + } + return promises +} + +/** + * Promisify all methods in a class. + * + * Options are the same as for promisifyAll + */ +function promisifyClass(cls, opts = {}) { + const promisified = class extends cls {} + const { without = [], multiResult = {} } = opts + for (const propName of Object.getOwnPropertyNames(cls.prototype)) { + if (propName === 'constructor' || without.includes(propName)) { + continue + } + const propValue = cls.prototype[propName] + if (typeof propValue !== 'function') { + continue + } + if (multiResult[propName] != null) { + promisified.prototype[propName] = promisifyMultiResult( + propValue, + multiResult[propName] + ) + } else { + promisified.prototype[propName] = promisify(propValue) + } + } + return promisified +} + +/** + * Promisify a function that returns multiple results via additional callback + * parameters. + * + * The promisified function returns the results in a single object whose keys + * are the names given in the array `resultNames`. + * + * Example: + * + * function f(callback) { + * return callback(null, 1, 2, 3) + * } + * + * const g = promisifyMultiResult(f, ['a', 'b', 'c']) + * + * const result = await g() // returns {a: 1, b: 2, c: 3} + */ +function promisifyMultiResult(fn, resultNames) { + function promisified(...args) { + return new Promise((resolve, reject) => { + try { + fn.bind(this)(...args, (err, ...results) => { + if (err != null) { + return reject(err) + } + const promiseResult = {} + for (let i = 0; i < resultNames.length; i++) { + promiseResult[resultNames[i]] = results[i] + } + resolve(promiseResult) + }) + } catch (err) { + reject(err) + } + }) + } + return promisified +} + +/** + * Reverse of `promisifyAll`. + * + * Callbackify all async functions in a module and return them in an object. In + * contrast with `promisifyAll`, all other exports from the module are added to + * the result. + * + * This is meant to be used like this: + * + * const MyPromisifiedModule = {...} + * module.exports = { + * ...callbackifyAll(MyPromisifiedModule), + * promises: MyPromisifiedModule + * } + * + * @param {Object} module - The module to callbackify + * @param {Object} opts - Options + * @param {Object} opts.multiResult - Spec of methods to be callbackified with + * callbackifyMultiResult() + */ +function callbackifyAll(module, opts = {}) { + const { multiResult = {} } = opts + const callbacks = {} + for (const propName of Object.getOwnPropertyNames(module)) { + const propValue = module[propName] + if (typeof propValue === 'function') { + if (propValue.constructor.name === 'AsyncFunction') { + if (multiResult[propName] != null) { + callbacks[propName] = callbackifyMultiResult( + propValue, + multiResult[propName] + ).bind(module) + } else { + callbacks[propName] = callbackify(propValue).bind(module) + } + } else { + callbacks[propName] = propValue.bind(module) + } + } else { + callbacks[propName] = propValue + } + } + return callbacks +} + +/** + * Reverse the effect of `promisifyMultiResult`. + * + * This is meant for providing a temporary backward compatible callback + * interface while we migrate to promises. + */ +function callbackifyMultiResult(fn, resultNames) { + function callbackified(...args) { + const [callback] = args.splice(-1) + fn(...args) + .then(result => { + const cbResults = resultNames.map(resultName => result[resultName]) + callback(null, ...cbResults) + }) + .catch(err => { + callback(err) + }) + } + return callbackified +} + +/** + * Transform an async function into an Express middleware + * + * Any error will be passed to the error middlewares via `next()` + */ +function expressify(fn) { + return (req, res, next) => { + fn(req, res, next).catch(next) + } +} + +/** + * Map values in `array` with the async function `fn` + * + * Limit the number of unresolved promises to `concurrency`. + */ +function promiseMapWithLimit(concurrency, array, fn) { + const limit = pLimit(concurrency) + return Promise.all(array.map(x => limit(() => fn(x)))) +} diff --git a/libraries/promise-utils/package.json b/libraries/promise-utils/package.json new file mode 100644 index 0000000000..92a2da1cb5 --- /dev/null +++ b/libraries/promise-utils/package.json @@ -0,0 +1,25 @@ +{ + "name": "@overleaf/promise-utils", + "version": "0.1.0", + "description": "utilities to help working with promises", + "main": "index.js", + "scripts": { + "test": "npm run lint && npm run format && npm run test:unit", + "test:unit": "mocha", + "lint": "eslint --max-warnings 0 --format unix .", + "lint:fix": "eslint --fix .", + "format": "prettier --list-different $PWD/'**/*.js'", + "format:fix": "prettier --write $PWD/'**/*.js'", + "test:ci": "npm run test:unit" + }, + "author": "Overleaf (https://www.overleaf.com)", + "license": "AGPL-3.0-only", + "devDependencies": { + "chai": "^4.3.10", + "chai-as-promised": "^7.1.1", + "mocha": "^10.2.0" + }, + "dependencies": { + "p-limit": "^2.3.0" + } +} diff --git a/libraries/promise-utils/test/setup.js b/libraries/promise-utils/test/setup.js new file mode 100644 index 0000000000..09068185a8 --- /dev/null +++ b/libraries/promise-utils/test/setup.js @@ -0,0 +1,4 @@ +const chai = require('chai') +const chaiAsPromised = require('chai-as-promised') + +chai.use(chaiAsPromised) diff --git a/libraries/promise-utils/test/unit/PromiseUtilsTests.js b/libraries/promise-utils/test/unit/PromiseUtilsTests.js new file mode 100644 index 0000000000..bfd7beb78d --- /dev/null +++ b/libraries/promise-utils/test/unit/PromiseUtilsTests.js @@ -0,0 +1,298 @@ +const { expect } = require('chai') +const { + promisifyAll, + promisifyClass, + callbackifyMultiResult, + callbackifyAll, +} = require('../..') + +describe('promisifyAll', function () { + describe('basic functionality', function () { + before(function () { + this.module = { + SOME_CONSTANT: 1, + asyncAdd(a, b, callback) { + callback(null, a + b) + }, + asyncDouble(x, callback) { + this.asyncAdd(x, x, callback) + }, + } + this.promisified = promisifyAll(this.module) + }) + + it('promisifies functions in the module', async function () { + const sum = await this.promisified.asyncAdd(29, 33) + expect(sum).to.equal(62) + }) + + it('binds this to the original module', async function () { + const sum = await this.promisified.asyncDouble(38) + expect(sum).to.equal(76) + }) + + it('does not copy over non-functions', async function () { + expect(this.promisified).not.to.have.property('SOME_CONSTANT') + }) + + it('does not modify the prototype of the module', async function () { + expect(this.promisified.toString()).to.equal('[object Object]') + }) + }) + + describe('without option', function () { + before(function () { + this.module = { + asyncAdd(a, b, callback) { + callback(null, a + b) + }, + syncAdd(a, b) { + return a + b + }, + } + this.promisified = promisifyAll(this.module, { without: ['syncAdd'] }) + }) + + it('does not promisify excluded functions', function () { + expect(this.promisified.syncAdd).not.to.exist + }) + + it('promisifies other functions', async function () { + const sum = await this.promisified.asyncAdd(12, 89) + expect(sum).to.equal(101) + }) + }) + + describe('multiResult option', function () { + before(function () { + this.module = { + asyncAdd(a, b, callback) { + callback(null, a + b) + }, + asyncArithmetic(a, b, callback) { + callback(null, a + b, a * b) + }, + } + this.promisified = promisifyAll(this.module, { + multiResult: { asyncArithmetic: ['sum', 'product'] }, + }) + }) + + it('promisifies multi-result functions', async function () { + const result = await this.promisified.asyncArithmetic(3, 6) + expect(result).to.deep.equal({ sum: 9, product: 18 }) + }) + + it('promisifies other functions normally', async function () { + const sum = await this.promisified.asyncAdd(6, 1) + expect(sum).to.equal(7) + }) + }) +}) + +describe('promisifyClass', function () { + describe('basic functionality', function () { + before(function () { + this.Class = class { + constructor(a) { + this.a = a + } + + asyncAdd(b, callback) { + callback(null, this.a + b) + } + } + this.Promisified = promisifyClass(this.Class) + }) + + it('promisifies the class methods', async function () { + const adder = new this.Promisified(1) + const sum = await adder.asyncAdd(2) + expect(sum).to.equal(3) + }) + }) + + describe('without option', function () { + before(function () { + this.Class = class { + constructor(a) { + this.a = a + } + + asyncAdd(b, callback) { + callback(null, this.a + b) + } + + syncAdd(b) { + return this.a + b + } + } + this.Promisified = promisifyClass(this.Class, { without: ['syncAdd'] }) + }) + + it('does not promisify excluded functions', function () { + const adder = new this.Promisified(10) + const sum = adder.syncAdd(12) + expect(sum).to.equal(22) + }) + + it('promisifies other functions', async function () { + const adder = new this.Promisified(23) + const sum = await adder.asyncAdd(3) + expect(sum).to.equal(26) + }) + }) + + describe('multiResult option', function () { + before(function () { + this.Class = class { + constructor(a) { + this.a = a + } + + asyncAdd(b, callback) { + callback(null, this.a + b) + } + + asyncArithmetic(b, callback) { + callback(null, this.a + b, this.a * b) + } + } + this.Promisified = promisifyClass(this.Class, { + multiResult: { asyncArithmetic: ['sum', 'product'] }, + }) + }) + + it('promisifies multi-result functions', async function () { + const adder = new this.Promisified(3) + const result = await adder.asyncArithmetic(6) + expect(result).to.deep.equal({ sum: 9, product: 18 }) + }) + + it('promisifies other functions normally', async function () { + const adder = new this.Promisified(6) + const sum = await adder.asyncAdd(1) + expect(sum).to.equal(7) + }) + }) +}) + +describe('callbackifyMultiResult', function () { + it('callbackifies a multi-result function', function (done) { + async function asyncArithmetic(a, b) { + return { sum: a + b, product: a * b } + } + const callbackified = callbackifyMultiResult(asyncArithmetic, [ + 'sum', + 'product', + ]) + callbackified(3, 11, (err, sum, product) => { + if (err != null) { + return done(err) + } + expect(sum).to.equal(14) + expect(product).to.equal(33) + done() + }) + }) + + it('propagates errors', function (done) { + async function asyncBomb() { + throw new Error('BOOM!') + } + const callbackified = callbackifyMultiResult(asyncBomb, [ + 'explosives', + 'dynamite', + ]) + callbackified(err => { + expect(err).to.exist + done() + }) + }) +}) + +describe('callbackifyAll', function () { + describe('basic functionality', function () { + before(function () { + this.module = { + SOME_CONSTANT: 1, + async asyncAdd(a, b) { + return a + b + }, + async asyncDouble(x, callback) { + return await this.asyncAdd(x, x) + }, + dashConcat(a, b) { + return `${a}-${b}` + }, + } + this.callbackified = callbackifyAll(this.module) + }) + + it('callbackifies async functions in the module', function (done) { + this.callbackified.asyncAdd(77, 18, (err, sum) => { + if (err) { + return done(err) + } + expect(sum).to.equal(95) + done() + }) + }) + + it('binds this to the original module', function (done) { + this.callbackified.asyncDouble(20, (err, double) => { + if (err) { + return done(err) + } + expect(double).to.equal(40) + done() + }) + }) + + it('copies over regular functions', function () { + const s = this.callbackified.dashConcat('ping', 'pong') + expect(s).to.equal('ping-pong') + }) + + it('copies over non-functions', function () { + expect(this.callbackified.SOME_CONSTANT).to.equal(1) + }) + }) + + describe('multiResult option', function () { + before(function () { + this.module = { + async asyncAdd(a, b) { + return a + b + }, + async asyncArithmetic(a, b) { + return { sum: a + b, product: a * b } + }, + } + this.callbackified = callbackifyAll(this.module, { + multiResult: { asyncArithmetic: ['sum', 'product'] }, + }) + }) + + it('callbackifies multi-result functions', function (done) { + this.callbackified.asyncArithmetic(4, 5, (err, sum, product) => { + if (err) { + return done(err) + } + expect(sum).to.equal(9) + expect(product).to.equal(20) + done() + }) + }) + + it('callbackifies other functions normally', function (done) { + this.callbackified.asyncAdd(77, 18, (err, sum) => { + if (err) { + return done(err) + } + expect(sum).to.equal(95) + done() + }) + }) + }) +}) diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/cell.tsx b/services/web/frontend/js/features/source-editor/components/table-generator/cell.tsx index cde64a37d1..a8639739a9 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/cell.tsx +++ b/services/web/frontend/js/features/source-editor/components/table-generator/cell.tsx @@ -202,8 +202,10 @@ export const Cell: FC<{ ) const onDoubleClick = useCallback(() => { - startEditing(rowIndex, columnIndex, cellData.content) - }, [columnIndex, rowIndex, startEditing, cellData.content]) + if (!view.state.readOnly) { + startEditing(rowIndex, columnIndex, cellData.content) + } + }, [columnIndex, rowIndex, startEditing, cellData.content, view]) return ( // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/table.tsx b/services/web/frontend/js/features/source-editor/components/table-generator/table.tsx index 56bc84ca9a..e2060a3df4 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/table.tsx +++ b/services/web/frontend/js/features/source-editor/components/table-generator/table.tsx @@ -148,6 +148,9 @@ export const Table: FC = () => { if (startCompileKeypress(event)) { return } + if (view.state.readOnly) { + return + } const commandKey = isMac ? event.metaKey : event.ctrlKey if (event.code === 'Enter' && !event.shiftKey) { event.preventDefault() @@ -270,6 +273,9 @@ export const Table: FC = () => { useEffect(() => { const onPaste = (event: ClipboardEvent) => { + if (view.state.readOnly) { + return false + } if (cellData || !selection) { // We're editing a cell, so allow browser to insert there return false diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx b/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx index 23d9fd8059..fac8be3ce3 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx +++ b/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx @@ -291,7 +291,7 @@ const TabularWrapper: FC = () => { }, [cellData, commitCellData, selection, setSelection, ref, view]) return (
- + {!view.state.readOnly && } ) diff --git a/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts b/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts index 65c511e137..98dd6dcd24 100644 --- a/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts +++ b/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts @@ -64,6 +64,8 @@ import { TabularWidget } from './visual-widgets/tabular' import { nextSnippetField, pickedCompletion } from '@codemirror/autocomplete' import { skipPreambleWithCursor } from './skip-preamble-cursor' import { TableRenderingErrorWidget } from './visual-widgets/table-rendering-error' +import { GraphicsWidget } from './visual-widgets/graphics' +import { InlineGraphicsWidget } from './visual-widgets/inline-graphics' type Options = { fileTreeManager: { @@ -886,9 +888,12 @@ export const atomicDecorations = (options: Options) => { line.text.trim().length === nodeRef.to - nodeRef.from if (lineContainsOnlyNode) { + const Widget = state.readOnly + ? GraphicsWidget + : EditableGraphicsWidget decorations.push( Decoration.replace({ - widget: new EditableGraphicsWidget( + widget: new Widget( filePath, getPreviewByPath, centered, @@ -898,9 +903,12 @@ export const atomicDecorations = (options: Options) => { }).range(line.from, line.to) ) } else { + const Widget = state.readOnly + ? InlineGraphicsWidget + : EditableInlineGraphicsWidget decorations.push( Decoration.replace({ - widget: new EditableInlineGraphicsWidget( + widget: new Widget( filePath, getPreviewByPath, centered, diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor-visual-readonly.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor-visual-readonly.spec.tsx index b8e3875542..1f81663abd 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor-visual-readonly.spec.tsx +++ b/services/web/test/frontend/features/source-editor/components/codemirror-editor-visual-readonly.spec.tsx @@ -7,6 +7,23 @@ const Container: FC = ({ children }) => (
{children}
) +const mountEditor = (content: string, ...args: any[]) => { + const scope = mockScope(content) + scope.permissionsLevel = 'readOnly' + scope.editor.showVisual = true + + cy.mount( + + + + + + ) + + // wait for the content to be parsed and revealed + cy.get('.cm-content').should('have.css', 'opacity', '1') +} + describe(' in Visual mode with read-only permission', function () { beforeEach(function () { window.metaAttributesCache.set('ol-preventCompileOnLoad', true) @@ -19,20 +36,7 @@ describe(' in Visual mode with read-only permission', functio }) it('decorates footnote content', function () { - const scope = mockScope('Foo \\footnote{Bar.} ') - scope.permissionsLevel = 'readOnly' - scope.editor.showVisual = true - - cy.mount( - - - - - - ) - - // wait for the content to be parsed and revealed - cy.get('.cm-content').should('have.css', 'opacity', '1') + mountEditor('Foo \\footnote{Bar.} ') // select the footnote, so it expands cy.get('.ol-cm-footnote').click() @@ -41,4 +45,68 @@ describe(' in Visual mode with read-only permission', functio cy.get('@first-line').should('contain', 'Foo') cy.get('@first-line').should('contain', 'Bar') }) + + it('does not display the table toolbar', function () { + mountEditor('\\begin{tabular}{c}\n cell\n\\end{tabular}') + + cy.get('.table-generator-floating-toolbar').should('not.exist') + cy.get('.table-generator-cell').click() + cy.get('.table-generator-floating-toolbar').should('not.exist') + }) + + it('does not enter a table cell on double-click', function () { + mountEditor('\\begin{tabular}{c}\n cell\n\\end{tabular}') + + cy.get('.table-generator-cell').dblclick() + cy.get('.table-generator-cell').get('textarea').should('not.exist') + }) + + it('does not enter a table cell on Enter', function () { + mountEditor('\\begin{tabular}{c}\n cell\n\\end{tabular}') + + cy.get('.table-generator-cell').trigger('keydown', { key: 'Enter' }) + cy.get('.table-generator-cell').get('textarea').should('not.exist') + }) + + it('does not paste into a table cell', function () { + mountEditor('\\begin{tabular}{c}\n cell\n\\end{tabular}\n\n') + + cy.get('.cm-line').last().click() + cy.get('.table-generator-cell-render').eq(0).click() + + const clipboardData = new DataTransfer() + clipboardData.setData('text/plain', 'bar') + cy.get('.table-generator-cell-render') + .eq(0) + .trigger('paste', { clipboardData }) + + cy.get('.cm-content').should('have.text', 'cell') + }) + + it('does not display the figure edit button', function () { + const fileTreeManager = { + findEntityById: cy.stub(), + findEntityByPath: cy.stub(), + getEntityPath: cy.stub(), + getRootDocDirname: cy.stub(), + getPreviewByPath: cy + .stub() + .returns({ url: '/images/frog.jpg', extension: 'jpg' }), + } + + cy.intercept('/images/frog.jpg', { fixture: 'images/gradient.png' }) + + mountEditor( + `\\begin{figure} +\\centering +\\includegraphics[width=0.5\\linewidth]{frog.jpg} +\\caption{My caption} +\\label{fig:my-label} +\\end{figure}`, + { fileTreeManager } + ) + + cy.get('img.ol-cm-graphics').should('have.length', 1) + cy.findByRole('button', { name: 'Edit figure' }).should('not.exist') + }) })