Skip to content

Commit

Permalink
fix(browser-repl): ensure that input can't be modified or evaluated w…
Browse files Browse the repository at this point in the history
…hile operation is in progress (#2162)
  • Loading branch information
gribnoysup authored Sep 11, 2024
1 parent 0600bf3 commit ae460a5
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 6 deletions.
36 changes: 35 additions & 1 deletion packages/browser-repl/src/components/editor.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import sinon from 'sinon';
import { expect } from '../../testing/chai';
import { createCommands } from './editor';
import { Editor, createCommands } from './editor';
import type { Command } from '@mongodb-js/compass-editor';
import { shallow } from '../../testing/enzyme';
import React from 'react';

describe('<Editor />', function () {
let commandSpies: Parameters<typeof createCommands>[number];
Expand Down Expand Up @@ -104,4 +106,36 @@ describe('<Editor />', function () {
expect(commandSpies.onArrowDownOnLastLine).to.not.have.been.called;
});
});

describe('command callback props', function () {
it('does not call callback props that can modify shell input', function () {
const callbackPropSpies = {
onEnter: sinon.spy(),
onArrowUpOnFirstLine: sinon.stub().resolves(false),
onArrowDownOnLastLine: sinon.stub().resolves(false),
onClearCommand: sinon.spy(),
onSigInt: sinon.spy(),
};

const wrapper = shallow(
<Editor {...callbackPropSpies} operationInProgress />
);

wrapper.instance().onEnter();
expect(callbackPropSpies.onEnter).to.not.have.been.called;

wrapper.instance().onArrowUpOnFirstLine();
expect(callbackPropSpies.onArrowUpOnFirstLine).to.not.have.been.called;

wrapper.instance().onArrowDownOnLastLine();
expect(callbackPropSpies.onArrowDownOnLastLine).to.not.have.been.called;

wrapper.instance().onClearCommand();
expect(callbackPropSpies.onClearCommand).to.not.have.been.called;

// Only onSigInt is allowed when operation is in progress
wrapper.instance().onSigInt();
expect(callbackPropSpies.onSigInt).to.have.been.called;
});
});
});
42 changes: 41 additions & 1 deletion packages/browser-repl/src/components/editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,47 @@ export class Editor extends Component<EditorProps> {
() => null
);
};
this.commands = createCommands(this.props);
this.commands = createCommands(this);
}

onEnter() {
// Do not allow commands that can modify / evaluate the input while
// operation is in progress
if (this.props.operationInProgress) {
return;
}
return this.props.onEnter();
}

onArrowDownOnLastLine() {
// Do not allow commands that can modify / evaluate the input while
// operation is in progress
if (this.props.operationInProgress) {
return Promise.resolve(false);
}
return this.props.onArrowDownOnLastLine();
}

onArrowUpOnFirstLine() {
// Do not allow commands that can modify / evaluate the input while
// operation is in progress
if (this.props.operationInProgress) {
return Promise.resolve(false);
}
return this.props.onArrowUpOnFirstLine();
}

onClearCommand() {
// Do not allow commands that can modify / evaluate the input while
// operation is in progress
if (this.props.operationInProgress) {
return;
}
return this.props.onClearCommand();
}

onSigInt() {
return this.props.onSigInt();
}

render(): JSX.Element {
Expand Down
44 changes: 44 additions & 0 deletions packages/browser-repl/src/components/shell.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ const wait: (ms?: number) => Promise<void> = (ms = 10) => {
return new Promise((resolve) => setTimeout(resolve, ms));
};

const waitFor = async (fn: () => void, timeout = 10_000) => {
const start = Date.now();
// eslint-disable-next-line no-constant-condition
while (true) {
await wait();
try {
fn();
return;
} catch (err) {
if (Date.now() - start >= timeout) {
throw err;
}
}
}
};

describe('<Shell />', function () {
let onOutputChangedSpy;
let onHistoryChangedSpy;
Expand Down Expand Up @@ -523,4 +539,32 @@ describe('<Shell />', function () {
);
expect(wrapper.find('Editor').prop('value')).to.eq('db.coll.find({})');
});

it('evaluates initial lines', async function () {
wrapper = mount(
<Shell
runtime={fakeRuntime}
initialEvaluate={['use test', 'db.coll.find({})']}
/>
);

// The `operationInProgress` state should be set to true right away
expect(wrapper.state('operationInProgress')).to.eq(true);
expect(wrapper.state('output')).to.deep.eq([]);

await waitFor(() => {
// Eventually we can see through output state that initial lines were
// evaluated
expect(
wrapper
.state('output')
.filter((line) => {
return line.format === 'input';
})
.map((line) => {
return line.value;
})
).to.deep.eq(['use test', 'db.coll.find({})']);
});
});
});
23 changes: 19 additions & 4 deletions packages/browser-repl/src/components/shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,23 @@ const noop = (): void => {
/* */
};

const normalizeInitialEvaluate = (initialEvaluate: string | string[]) => {
return (
Array.isArray(initialEvaluate) ? initialEvaluate : [initialEvaluate]
).filter((line) => {
// Filter out empty lines if passed by accident
return !!line;
});
};

const isInitialEvaluateEmpty = (
initialEvaluate?: string | string[] | undefined
) => {
return (
!initialEvaluate || normalizeInitialEvaluate(initialEvaluate).length === 0
);
};

/**
* The browser-repl Shell component
*/
Expand All @@ -166,7 +183,7 @@ export class _Shell extends Component<ShellProps, ShellState> {
private onCancelPasswordPrompt: () => void = noop;

readonly state: ShellState = {
operationInProgress: false,
operationInProgress: !isInitialEvaluateEmpty(this.props.initialEvaluate),
output: this.props.initialOutput.slice(-this.props.maxOutputLength),
history: this.props.initialHistory.slice(0, this.props.maxHistoryLength),
passwordPrompt: '',
Expand All @@ -178,9 +195,7 @@ export class _Shell extends Component<ShellProps, ShellState> {
// updated one when actually running the lines
let evalLines: string[] = [];
if (this.props.initialEvaluate) {
evalLines = Array.isArray(this.props.initialEvaluate)
? this.props.initialEvaluate
: [this.props.initialEvaluate];
evalLines = normalizeInitialEvaluate(this.props.initialEvaluate);
}
this.scrollToBottom();
void this.updateShellPrompt().then(async () => {
Expand Down

0 comments on commit ae460a5

Please sign in to comment.