Skip to content

Commit

Permalink
[tree] fixed refresh to await when all expanded children are refreshed
Browse files Browse the repository at this point in the history 
Signed-off-by: Anton Kosyakov <[email protected]>
  • Loading branch information
@akosyakov
akosyakov committed Jul 25, 2019
1 parent de848de commit 7c78b38
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 146 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/browser/tree/tree-expansion.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class TreeExpansionServiceImpl implements TreeExpansionService {
this.tree.onNodeRefreshed(node => {
for (const child of node.children) {
if (ExpandableTreeNode.isExpanded(child)) {
this.tree.refresh(child);
node.waitUntil(this.tree.refresh(child));
}
}
});
Expand Down
265 changes: 133 additions & 132 deletions packages/core/src/browser/tree/tree.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import { ILogger } from '../../common';
// tslint:disable:no-unused-expression
describe('Tree', () => {

it('addChildren', () => {
assertTreeNode(`{
it('addChildren', () => {
assertTreeNode(`{
"id": "parent",
"name": "parent",
"children": [
Expand All @@ -58,12 +58,12 @@ describe('Tree', () => {
}
]
}`, getNode());
});
});

it('removeChild - first', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[0]);
assertTreeNode(`{
it('removeChild - first', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[0]);
assertTreeNode(`{
"id": "parent",
"name": "parent",
"children": [
Expand All @@ -81,12 +81,12 @@ describe('Tree', () => {
}
]
}`, node);
});
});

it('removeChild - second', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[1]);
assertTreeNode(`{
it('removeChild - second', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[1]);
assertTreeNode(`{
"id": "parent",
"name": "parent",
"children": [
Expand All @@ -104,12 +104,12 @@ describe('Tree', () => {
}
]
}`, node);
});
});

it('removeChild - third', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[2]);
assertTreeNode(`{
it('removeChild - third', () => {
const node = getNode();
CompositeTreeNode.removeChild(node, node.children[2]);
assertTreeNode(`{
"id": "parent",
"name": "parent",
"children": [
Expand All @@ -127,136 +127,137 @@ describe('Tree', () => {
}
]
}`, node);
});

let model: TreeModel;
beforeEach(() => {
model = createTreeModel();
model.root = MockTreeModel.HIERARCHICAL_MOCK_ROOT();
});
describe('getNode', () => {
it('returns undefined for undefined nodes', done => {
expect(model.getNode(undefined)).to.be.undefined;
done();
});

it('returns undefined for a non-existing id', done => {
expect(model.getNode('10')).to.be.undefined;
done();
let model: TreeModel;
beforeEach(() => {
model = createTreeModel();
model.root = MockTreeModel.HIERARCHICAL_MOCK_ROOT();
});
describe('getNode', () => {
it('returns undefined for undefined nodes', done => {
expect(model.getNode(undefined)).to.be.undefined;
done();
});

it('returns a valid node for existing an id', done => {
expect(model.getNode('1.1')).not.to.be.undefined;
done();
});
});
it('returns undefined for a non-existing id', done => {
expect(model.getNode('10')).to.be.undefined;
done();
});

describe('validateNode', () => {
it('returns undefined for undefined nodes', done => {
expect(model.validateNode(undefined)).to.be.undefined;
done();
it('returns a valid node for existing an id', done => {
expect(model.getNode('1.1')).not.to.be.undefined;
done();
});
});

it('returns undefined for non-existing nodes', done => {
expect(model.validateNode(MockTreeModel.Node.toTreeNode({ 'id': '10' }))).to.be.undefined;
done();
});
describe('validateNode', () => {
it('returns undefined for undefined nodes', done => {
expect(model.validateNode(undefined)).to.be.undefined;
done();
});

it('returns a valid node for an existing node', done => {
expect(model.validateNode(retrieveNode<TreeNode>('1.1'))).not.to.be.undefined;
done();
it('returns undefined for non-existing nodes', done => {
expect(model.validateNode(MockTreeModel.Node.toTreeNode({ 'id': '10' }))).to.be.undefined;
done();
});

it('returns a valid node for an existing node', done => {
expect(model.validateNode(retrieveNode<TreeNode>('1.1'))).not.to.be.undefined;
done();
});
});
});

describe('refresh', () => {
it('refreshes all composite nodes starting with the root', done => {
let result: Boolean = true;
const expectedRefreshedNodes = new Set([
retrieveNode<CompositeTreeNode>('1'),
retrieveNode<CompositeTreeNode>('1.1'),
retrieveNode<CompositeTreeNode>('1.2')]);
model.onNodeRefreshed((e: Readonly<CompositeTreeNode>) => {
result = result && expectedRefreshedNodes.has(e);
expectedRefreshedNodes.delete(e);
});
model.refresh().then(() => {
expect(result).to.be.true;
expect(expectedRefreshedNodes.size).to.be.equal(0);
done();
});
describe('refresh', () => {
it('refreshes all composite nodes starting with the root', done => {
let result: Boolean = true;
const expectedRefreshedNodes = new Set([
retrieveNode<CompositeTreeNode>('1'),
retrieveNode<CompositeTreeNode>('1.1'),
retrieveNode<CompositeTreeNode>('1.2'),
retrieveNode<CompositeTreeNode>('1.2.1')]);
model.onNodeRefreshed((e: Readonly<CompositeTreeNode>) => {
result = result && expectedRefreshedNodes.has(e);
expectedRefreshedNodes.delete(e);
});
model.refresh().then(() => {
expect(result).to.be.true;
expect(expectedRefreshedNodes.size).to.be.equal(0);
done();
});
});
});
});

describe('refresh(parent: Readonly<CompositeTreeNode>)', () => {
it('refreshes all composite nodes starting with the provided node', done => {
let result: Boolean = true;
const expectedRefreshedNodes = new Set([
retrieveNode<CompositeTreeNode>('1.2'),
retrieveNode<CompositeTreeNode>('1.2.1')
]);
model.onNodeRefreshed((e: Readonly<CompositeTreeNode>) => {
result = result && expectedRefreshedNodes.has(e);
expectedRefreshedNodes.delete(e);
});
model.refresh(retrieveNode<CompositeTreeNode>('1.2')).then(() => {
expect(result).to.be.true;
expect(expectedRefreshedNodes.size).to.be.equal(0);
done();
});
describe('refresh(parent: Readonly<CompositeTreeNode>)', () => {
it('refreshes all composite nodes starting with the provided node', done => {
let result: Boolean = true;
const expectedRefreshedNodes = new Set([
retrieveNode<CompositeTreeNode>('1.2'),
retrieveNode<CompositeTreeNode>('1.2.1')
]);
model.onNodeRefreshed((e: Readonly<CompositeTreeNode>) => {
result = result && expectedRefreshedNodes.has(e);
expectedRefreshedNodes.delete(e);
});
model.refresh(retrieveNode<CompositeTreeNode>('1.2')).then(() => {
expect(result).to.be.true;
expect(expectedRefreshedNodes.size).to.be.equal(0);
done();
});
});
});
});

function getNode(): CompositeTreeNode {
return CompositeTreeNode.addChildren({
id: 'parent',
name: 'parent',
children: [],
parent: undefined
}, [{
id: 'foo',
name: 'foo',
parent: undefined
}, {
id: 'bar',
name: 'bar',
parent: undefined
}, {
id: 'baz',
name: 'baz',
parent: undefined
}]);
}
function getNode(): CompositeTreeNode {
return CompositeTreeNode.addChildren({
id: 'parent',
name: 'parent',
children: [],
parent: undefined
}, [{
id: 'foo',
name: 'foo',
parent: undefined
}, {
id: 'bar',
name: 'bar',
parent: undefined
}, {
id: 'baz',
name: 'baz',
parent: undefined
}]);
}

function assertTreeNode(expectation: string, node: TreeNode): void {
// tslint:disable-next-line:no-any
assert.deepEqual(expectation, JSON.stringify(node, (key: keyof CompositeTreeNode, value: any) => {
if (key === 'parent' || key === 'previousSibling' || key === 'nextSibling') {
return value && value.id;
}
return value;
}, 2));
}
function assertTreeNode(expectation: string, node: TreeNode): void {
// tslint:disable-next-line:no-any
assert.deepEqual(expectation, JSON.stringify(node, (key: keyof CompositeTreeNode, value: any) => {
if (key === 'parent' || key === 'previousSibling' || key === 'nextSibling') {
return value && value.id;
}
return value;
}, 2));
}

function createTreeModel(): TreeModel {
const container = new Container({ defaultScope: 'Singleton' });
container.bind(TreeImpl).toSelf();
container.bind(Tree).toService(TreeImpl);
container.bind(TreeSelectionServiceImpl).toSelf();
container.bind(TreeSelectionService).toService(TreeSelectionServiceImpl);
container.bind(TreeExpansionServiceImpl).toSelf();
container.bind(TreeExpansionService).toService(TreeExpansionServiceImpl);
container.bind(TreeNavigationService).toSelf();
container.bind(TreeModelImpl).toSelf();
container.bind(TreeModel).toService(TreeModelImpl);
container.bind(TreeSearch).toSelf();
container.bind(FuzzySearch).toSelf();
container.bind(MockLogger).toSelf();
container.bind(ILogger).to(MockLogger).inSingletonScope();
return container.get(TreeModel);
}
function retrieveNode<T extends TreeNode>(id: string): Readonly<T> {
const readonlyNode: Readonly<T> = model.getNode(id) as T;
return readonlyNode;
}
function createTreeModel(): TreeModel {
const container = new Container({ defaultScope: 'Singleton' });
container.bind(TreeImpl).toSelf();
container.bind(Tree).toService(TreeImpl);
container.bind(TreeSelectionServiceImpl).toSelf();
container.bind(TreeSelectionService).toService(TreeSelectionServiceImpl);
container.bind(TreeExpansionServiceImpl).toSelf();
container.bind(TreeExpansionService).toService(TreeExpansionServiceImpl);
container.bind(TreeNavigationService).toSelf();
container.bind(TreeModelImpl).toSelf();
container.bind(TreeModel).toService(TreeModelImpl);
container.bind(TreeSearch).toSelf();
container.bind(FuzzySearch).toSelf();
container.bind(MockLogger).toSelf();
container.bind(ILogger).to(MockLogger).inSingletonScope();
return container.get(TreeModel);
}
function retrieveNode<T extends TreeNode>(id: string): Readonly<T> {
const readonlyNode: Readonly<T> = model.getNode(id) as T;
return readonlyNode;
}

});
18 changes: 9 additions & 9 deletions packages/core/src/browser/tree/tree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import { injectable } from 'inversify';
import { Event, Emitter, Disposable, DisposableCollection } from '../../common';
import { Event, Emitter, Disposable, DisposableCollection, WaitUntilEvent } from '../../common';

export const Tree = Symbol('Tree');

Expand Down Expand Up @@ -52,7 +52,7 @@ export interface Tree extends Disposable {
/**
* Emit when the children of the given node are refreshed.
*/
readonly onNodeRefreshed: Event<Readonly<CompositeTreeNode>>;
readonly onNodeRefreshed: Event<Readonly<CompositeTreeNode> & WaitUntilEvent>;
}

/**
Expand Down Expand Up @@ -202,7 +202,7 @@ export class TreeImpl implements Tree {

protected _root: TreeNode | undefined;
protected readonly onChangedEmitter = new Emitter<void>();
protected readonly onNodeRefreshedEmitter = new Emitter<CompositeTreeNode>();
protected readonly onNodeRefreshedEmitter = new Emitter<CompositeTreeNode & WaitUntilEvent>();
protected readonly toDispose = new DisposableCollection();

protected nodes: {
Expand Down Expand Up @@ -238,12 +238,12 @@ export class TreeImpl implements Tree {
this.onChangedEmitter.fire(undefined);
}

get onNodeRefreshed(): Event<CompositeTreeNode> {
get onNodeRefreshed(): Event<CompositeTreeNode & WaitUntilEvent> {
return this.onNodeRefreshedEmitter.event;
}

protected fireNodeRefreshed(parent: CompositeTreeNode): void {
this.onNodeRefreshedEmitter.fire(parent);
protected async fireNodeRefreshed(parent: CompositeTreeNode): Promise<void> {
await WaitUntilEvent.fire(this.onNodeRefreshedEmitter, parent);
this.fireChanged();
}

Expand All @@ -260,7 +260,7 @@ export class TreeImpl implements Tree {
const parent = !raw ? this._root : this.validateNode(raw);
if (CompositeTreeNode.is(parent)) {
const children = await this.resolveChildren(parent);
this.setChildren(parent, children);
await this.setChildren(parent, children);
}
// FIXME: it should not be here
// if the idea was to support refreshing of all kind of nodes, then API should be adapted
Expand All @@ -271,11 +271,11 @@ export class TreeImpl implements Tree {
return Promise.resolve(Array.from(parent.children));
}

protected setChildren(parent: CompositeTreeNode, children: TreeNode[]): void {
protected async setChildren(parent: CompositeTreeNode, children: TreeNode[]): Promise<void> {
this.removeNode(parent);
parent.children = children;
this.addNode(parent);
this.fireNodeRefreshed(parent);
await this.fireNodeRefreshed(parent);
}

protected removeNode(node: TreeNode | undefined): void {
Expand Down
Loading

0 comments on commit 7c78b38

Please sign in to comment.