Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support async command execution #39

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Support async command execution #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f4bcdf
Support async command execution
tortmayr Feb 28, 2023
c04b3c2
Fix command test cases
tortmayr Feb 28, 2023
a2c9ee3
Address review feedback
tortmayr Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions packages/server/src/common/command/command-stack.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { expect } from 'chai';
import { Container, ContainerModule } from 'inversify';
import * as sinon from 'sinon';
import { StubCommand, StubLogger } from '../test/mock-util';
import { expectToThrowAsync, StubCommand, StubLogger } from '../test/mock-util';
import { Logger } from '../utils/logger';
import { DefaultCommandStack } from './command-stack';

Expand All @@ -41,165 +41,165 @@ describe('test DefaultCommandStack', () => {
});

describe('execute', () => {
it('should execute the given command and become dirty', () => {
it('should execute the given command and become dirty', async () => {
expect(commandStack.isDirty).to.be.false;
commandStack.execute(command1);
await commandStack.execute(command1);
expect(command1.execute.calledOnce).to.be.true;
expect(commandStack.isDirty).to.be.true;
});

it('should execute the given commands in order and become dirty', () => {
it('should execute the given commands in order and become dirty', async () => {
expect(commandStack.isDirty).to.be.false;
commandStack.execute(command1);
commandStack.execute(command2);
await commandStack.execute(command1);
await commandStack.execute(command2);

expect(command1.execute.calledOnce).to.be.true;
expect(command2.execute.calledOnce).to.be.true;
expect(command2.execute.calledAfter(command1.execute)).to.be.true;
expect(commandStack.isDirty).to.be.true;
});

it('should be able to undo after execute', () => {
it('should be able to undo after execute', async () => {
expect(commandStack.canUndo()).to.be.false;
commandStack.execute(command1);
await commandStack.execute(command1);
expect(commandStack.canUndo()).to.be.true;
});

it('should clear the redo stack after execution', () => {
it('should clear the redo stack after execution', async () => {
commandStack['commands'].push(command2);
commandStack['top'] = -1;
expect(commandStack.canRedo()).to.be.true;

commandStack.execute(command1);
await commandStack.execute(command1);
expect(commandStack.canRedo()).to.be.false;
});

it('should flush the stack in case of an execution error', () => {
it('should flush the stack in case of an execution error', async () => {
command2.execute.throwsException();
const flushSpy = sandbox.spy(commandStack, 'flush');

expect(() => commandStack.execute(command2)).to.throw();
await expectToThrowAsync(() => commandStack.execute(command2));
expect(command2.execute.calledOnce).to.be.true;
expect(flushSpy.calledOnce).to.be.true;
});
});

describe('undo', () => {
it('should do nothing if the command stack is empty', () => {
it('should do nothing if the command stack is empty', async () => {
expect(commandStack.isDirty).to.be.false;

commandStack.undo();
await commandStack.undo();
expect(commandStack.canUndo()).to.be.false;
expect(commandStack.canRedo()).to.be.false;
expect(commandStack.isDirty).to.be.false;
});

it('should undo the command and become non-dirty again', () => {
it('should undo the command and become non-dirty again', async () => {
commandStack['commands'].push(command1);
commandStack['top'] = 0;
expect(commandStack.isDirty).to.be.true;
expect(commandStack.canUndo()).to.be.true;
expect(commandStack.canRedo()).to.be.false;

commandStack.undo();
await commandStack.undo();
expect(command1.undo.calledOnce).to.be.true;
expect(commandStack.isDirty).to.be.false;
expect(commandStack.canRedo()).to.be.true;
expect(commandStack.canUndo()).to.be.false;
});

it('should undo multiple command and become non-dirty again', () => {
it('should undo multiple command and become non-dirty again', async () => {
commandStack['commands'].push(command1, command2);
commandStack['top'] = 1;
expect(commandStack.isDirty).to.be.true;
expect(commandStack.canUndo()).to.be.true;
expect(commandStack.canRedo()).to.be.false;

commandStack.undo();
await commandStack.undo();
expect(command2.undo.calledOnce).to.be.true;
expect(commandStack.canRedo()).to.be.true;
expect(commandStack.canUndo()).to.be.true;
expect(commandStack.isDirty).to.be.true;

commandStack.undo();
await commandStack.undo();
expect(command1.undo.calledOnce).to.be.true;
expect(command1.undo.calledAfter(command2.undo)).to.be.true;
expect(commandStack.isDirty).to.be.false;
expect(commandStack.canRedo()).to.be.true;
expect(commandStack.canUndo()).to.be.false;
});
it('should flush the stack in case of an execution error', () => {
it('should flush the stack in case of an execution error', async () => {
command2.undo.throwsException();
const flushSpy = sandbox.spy(commandStack, 'flush');
commandStack['commands'].push(command2);
commandStack['top'] = 0;

expect(() => commandStack.undo()).to.throw();
await expectToThrowAsync(() => commandStack.undo());
expect(command2.undo.calledOnce).to.be.true;
expect(flushSpy.calledOnce).to.be.true;
});
});

describe('redo', () => {
it('should do nothing if the command stack is empty', () => {
it('should do nothing if the command stack is empty', async () => {
expect(commandStack.isDirty).to.be.false;

commandStack.redo();
await commandStack.redo();
expect(commandStack.canUndo()).to.be.false;
expect(commandStack.canRedo()).to.be.false;
expect(commandStack.isDirty).to.be.false;
});

it('should redo the command and become dirty again', () => {
it('should redo the command and become dirty again', async () => {
commandStack['commands'].push(command1);
commandStack['top'] = -1;
expect(commandStack.isDirty).to.be.false;
expect(commandStack.canUndo()).to.be.false;
expect(commandStack.canRedo()).to.be.true;

commandStack.redo();
await commandStack.redo();
expect(command1.redo.calledOnce).to.be.true;
expect(commandStack.isDirty).to.be.true;
expect(commandStack.canRedo()).to.be.false;
expect(commandStack.canUndo()).to.be.true;
});

it('should undo multiple command and become non-dirty again', () => {
it('should undo multiple command and become non-dirty again', async () => {
commandStack['commands'].push(command2, command1);
commandStack['top'] = -1;
commandStack['saveIndex'] = -1;
expect(commandStack.isDirty).to.be.false;
expect(commandStack.canUndo()).to.be.false;
expect(commandStack.canRedo()).to.be.true;

commandStack.redo();
await commandStack.redo();
expect(command2.redo.calledOnce).to.be.true;
expect(commandStack.canRedo()).to.be.true;
expect(commandStack.canUndo()).to.be.true;
expect(commandStack.isDirty).to.be.true;

commandStack.redo();
await commandStack.redo();
expect(command1.redo.calledOnce).to.be.true;
expect(command1.redo.calledAfter(command2.redo)).to.be.true;
expect(commandStack.isDirty).to.be.true;
expect(commandStack.canRedo()).to.be.false;
expect(commandStack.canUndo()).to.be.true;
});
it('should flush the stack in case of an execution error', () => {
it('should flush the stack in case of an execution error', async () => {
command2.redo.throwsException();
const flushSpy = sandbox.spy(commandStack, 'flush');
commandStack['commands'].push(command2);
commandStack['top'] = -1;

expect(() => commandStack.redo()).to.throw();
await expectToThrowAsync(() => commandStack.redo());
expect(command2.redo.calledOnce).to.be.true;
expect(flushSpy.calledOnce).to.be.true;
});
it('should be able to undo after redo', () => {
it('should be able to undo after redo', async () => {
commandStack['commands'].push(command1);
commandStack['top'] = -1;
expect(commandStack.canUndo()).to.be.false;
commandStack.redo();
await commandStack.redo();
expect(commandStack.canUndo()).to.be.true;
});
});
Expand Down
19 changes: 10 additions & 9 deletions packages/server/src/common/command/command-stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { inject, injectable } from 'inversify';
import { MaybePromise } from '..';
import { Logger } from '../utils/logger';
import { Command } from './command';

Expand All @@ -31,12 +32,12 @@ export interface CommandStack {
* Clears any redoable commands not yet redone, adds the command to the stack and then invokes {@link Command.execute}.
* @param command the command to execute.
*/
execute(command: Command): void;
execute(command: Command): MaybePromise<void>;

/**
* Removes the topmost (i.e. last executed) command from the stack and invokes {@link Command.undo}.
*/
undo(): void;
undo(): MaybePromise<void>;

/**
* Returns `true` if the top command on the stack can be undone.
Expand All @@ -46,7 +47,7 @@ export interface CommandStack {
/**
* Re-adds the last undo command on top of the stack and invokes {@link Command.redo}
*/
redo(): void;
redo(): MaybePromise<void>;

/**
* Returns `true` if there are redoable commands in the stack.
Expand Down Expand Up @@ -86,9 +87,9 @@ export class DefaultCommandStack implements CommandStack {
*/
protected saveIndex = -1;

execute(command: Command): void {
async execute(command: Command): Promise<void> {
try {
command.execute();
await command.execute();
} catch (error) {
this.handleError(error);
}
Expand All @@ -104,11 +105,11 @@ export class DefaultCommandStack implements CommandStack {
}
}

undo(): void {
async undo(): Promise<void> {
if (this.canUndo()) {
const command = this.commands[this.top--];
try {
command.undo();
await command.undo();
} catch (error) {
this.handleError(error);
}
Expand All @@ -121,11 +122,11 @@ export class DefaultCommandStack implements CommandStack {
: false;
}

redo(): void {
async redo(): Promise<void> {
if (this.canRedo()) {
const command = this.commands[++this.top];
try {
command.redo();
await command.redo();
} catch (error) {
this.handleError(error);
}
Expand Down
19 changes: 9 additions & 10 deletions packages/server/src/common/command/command.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/
import { expect } from 'chai';
import * as sinon from 'sinon';
import { StubCommand } from '../test/mock-util';
import { expectToThrowAsync, StubCommand } from '../test/mock-util';
import { CompoundCommand } from './command';

describe('CompoundCommand', () => {
Expand All @@ -32,18 +32,18 @@ describe('CompoundCommand', () => {
});

describe('execute', () => {
it('Should execute the subcommands in order', () => {
compoundCommand.execute();
it('Should execute the subcommands in order', async () => {
await compoundCommand.execute();
expect(command1.execute.calledOnce).to.be.true;
expect(command2.execute.calledOnce).to.be.true;
expect(command3.execute.calledOnce).to.be.true;
expect(command1.execute.calledBefore(command2.execute)).to.be.true;
expect(command2.execute.calledBefore(command3.execute)).to.be.true;
});
it('Should undo partially executed subcommands in case of an error', () => {
it('Should undo partially executed subcommands in case of an error', async () => {
command3.execute.throwsException();

expect(() => compoundCommand.execute()).to.throw();
await expectToThrowAsync(() => compoundCommand.execute());

expect(command1.execute.calledOnce).to.be.true;
expect(command2.execute.calledOnce).to.be.true;
Expand All @@ -54,19 +54,18 @@ describe('CompoundCommand', () => {
});

describe('undo', () => {
it('Should undo the subcommands in reverse order', () => {
compoundCommand.undo();
it('Should undo the subcommands in reverse order', async () => {
await compoundCommand.undo();
expect(command1.undo.calledOnce).to.be.true;
expect(command2.undo.calledOnce).to.be.true;
expect(command3.undo.calledOnce).to.be.true;
expect(command1.undo.calledAfter(command2.undo)).to.be.true;
expect(command2.undo.calledAfter(command3.undo)).to.be.true;
});

it('Should redo partially undone subcommands in case of an error', () => {
it('Should redo partially undone subcommands in case of an error', async () => {
command1.undo.throwsException();

expect(() => compoundCommand.undo()).to.throw();
await expectToThrowAsync(() => compoundCommand.undo());

expect(command1.undo.calledOnce).to.be.true;
expect(command2.undo.calledOnce).to.be.true;
Expand Down
Loading