Skip to content

Commit

Permalink
Support async command execution (#39)
Browse files Browse the repository at this point in the history 
* Support async command execution

Change command API to support MaybePromise<void> instead of just plain voids. This enables async command execution e.g. for communicating with a modelserver
Contributed on behalf of STMicroelectronics

* Fix  command test cases

* Address review feedback
  • Loading branch information
@tortmayr
tortmayr authored Feb 28, 2023
1 parent 44bd1e0 commit 44aae0c
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 93 deletions.
64 changes: 32 additions & 32 deletions packages/server/src/common/command/command-stack.spec.ts
View file
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
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
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

0 comments on commit 44aae0c

Please sign in to comment.