Skip to content

Commit

Permalink
[plugin] Cache command arguments to safely pass the command over JSON…
Browse files Browse the repository at this point in the history
…-RPC

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
  • Loading branch information
vinokurig committed Aug 19, 2019
1 parent f718cb3 commit f703994
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/common/plugin-api-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export interface CommandRegistryMain {
}

export interface CommandRegistryExt {
$executeCommand<T>(id: string, ...ars: any[]): PromiseLike<T>;
$executeCommand<T>(id: string, ...ars: any[]): PromiseLike<T | undefined>;
registerArgumentProcessor(processor: ArgumentProcessor): void;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class PluginTree extends TreeImpl {
}, update);
}
if (TreeViewNode.is(node)) {
return Object.assign(node, update);
return Object.assign(node, update, { command: item.command });
}
return Object.assign({
id: item.id,
Expand Down
67 changes: 64 additions & 3 deletions packages/plugin-ext/src/plugin/command-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import { CommandRegistryExt, PLUGIN_RPC_CONTEXT as Ext, CommandRegistryMain } fr
import { RPCProtocol } from '../common/rpc-protocol';
import { Disposable } from './types-impl';
import { KnownCommands } from './type-converters';
import { DisposableCollection } from '@theia/core';

// tslint:disable-next-line:no-any
export type Handler = <T>(...args: any[]) => T | PromiseLike<T>;
export type Handler = <T>(...args: any[]) => T | PromiseLike<T | undefined>;

export interface ArgumentProcessor {
// tslint:disable-next-line:no-any
Expand All @@ -34,10 +35,16 @@ export class CommandRegistryImpl implements CommandRegistryExt {
private readonly commands = new Set<string>();
private readonly handlers = new Map<string, Handler>();
private readonly argumentProcessors: ArgumentProcessor[];
private readonly commandsConverter: CommandsConverter;

constructor(rpc: RPCProtocol) {
this.proxy = rpc.getProxy(Ext.COMMAND_REGISTRY_MAIN);
this.argumentProcessors = [];
this.commandsConverter = new CommandsConverter(this);
}

get converter(): CommandsConverter {
return this.commandsConverter;
}

// tslint:disable-next-line:no-any
Expand Down Expand Up @@ -78,7 +85,7 @@ export class CommandRegistryImpl implements CommandRegistryExt {
}

// tslint:disable-next-line:no-any
$executeCommand<T>(id: string, ...args: any[]): PromiseLike<T> {
$executeCommand<T>(id: string, ...args: any[]): PromiseLike<T | undefined> {
if (this.handlers.has(id)) {
return this.executeLocalCommand(id, ...args);
} else {
Expand All @@ -102,7 +109,7 @@ export class CommandRegistryImpl implements CommandRegistryExt {
}

// tslint:disable-next-line:no-any
private async executeLocalCommand<T>(id: string, ...args: any[]): Promise<T> {
private async executeLocalCommand<T>(id: string, ...args: any[]): Promise<T | undefined> {
const handler = this.handlers.get(id);
if (handler) {
return handler<T>(...args.map(arg => this.argumentProcessors.reduce((r, p) => p.processArgument(r), arg)));
Expand All @@ -123,3 +130,57 @@ export class CommandRegistryImpl implements CommandRegistryExt {
this.argumentProcessors.push(processor);
}
}

/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

// copied and modified from https://github.com/microsoft/vscode/blob/1.37.1/src/vs/workbench/api/common/extHostCommands.ts#L217-L259
export class CommandsConverter {

private readonly safeCommandId: string;
private readonly commands: CommandRegistryImpl;
private readonly commandsMap = new Map<number, theia.Command>();
private handle = 0;
private isCommandRegistered: boolean;

constructor(commands: CommandRegistryImpl) {
this.safeCommandId = `theia_safe_cmd_${Date.now().toString()}`;
this.commands = commands;
this.isCommandRegistered = false;
}

/**
* Convert to a command that can be safely passed over JSON-RPC.
*/
toSafeCommand(command: theia.Command, disposables: DisposableCollection): theia.Command {
if (!this.isCommandRegistered) {
this.commands.registerCommand({ id: this.safeCommandId }, this.executeSafeCommand, this);
this.isCommandRegistered = true;
}

const result: theia.Command = {};
Object.assign(result, command);

if (command.command && command.arguments && command.arguments.length > 0) {
const id = this.handle++;
this.commandsMap.set(id, command);
disposables.push(new Disposable(() => this.commandsMap.delete(id)));
result.command = this.safeCommandId;
result.arguments = [id];
}

return result;
}

// tslint:disable-next-line:no-any
private executeSafeCommand<R>(...args: any[]): PromiseLike<R | undefined> {
const command = this.commandsMap.get(args[0]);
if (!command || !command.command) {
return Promise.reject('command NOT FOUND');
}
return this.commands.executeCommand(command.command, ...(command.arguments || []));
}

}
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/plugin/plugin-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export function createAPIFactory(
return function (plugin: InternalPlugin): typeof theia {
const commands: typeof theia.commands = {
// tslint:disable-next-line:no-any
registerCommand(command: theia.CommandDescription, handler?: <T>(...args: any[]) => T | Thenable<T>, thisArg?: any): Disposable {
registerCommand(command: theia.CommandDescription, handler?: <T>(...args: any[]) => T | Thenable<T | undefined>, thisArg?: any): Disposable {
return commandRegistry.registerCommand(command, handler, thisArg);
},
// tslint:disable-next-line:no-any
Expand Down
19 changes: 11 additions & 8 deletions packages/plugin-ext/src/plugin/tree/tree-views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ import { Emitter } from '@theia/core/lib/common/event';
import { Disposable, ThemeIcon } from '../types-impl';
import { Plugin, PLUGIN_RPC_CONTEXT, TreeViewsExt, TreeViewsMain, TreeViewItem } from '../../common/plugin-api-rpc';
import { RPCProtocol } from '../../common/rpc-protocol';
import { CommandRegistryImpl } from '../command-registry';
import { CommandRegistryImpl, CommandsConverter } from '../command-registry';
import { TreeViewSelection } from '../../common';
import { PluginPackage } from '../../common/plugin-protocol';
import { DisposableCollection } from '@theia/core/lib/common/disposable';
import { toInternalCommand } from '../type-converters';

export class TreeViewsExtImpl implements TreeViewsExt {

private proxy: TreeViewsMain;

private treeViews: Map<string, TreeViewExtImpl<any>> = new Map<string, TreeViewExtImpl<any>>();

constructor(rpc: RPCProtocol, commandRegistry: CommandRegistryImpl) {
constructor(rpc: RPCProtocol, readonly commandRegistry: CommandRegistryImpl) {
this.proxy = rpc.getProxy(PLUGIN_RPC_CONTEXT.TREE_VIEWS_MAIN);
commandRegistry.registerArgumentProcessor({
processArgument: arg => {
Expand Down Expand Up @@ -61,7 +63,7 @@ export class TreeViewsExtImpl implements TreeViewsExt {
throw new Error('Options with treeDataProvider is mandatory');
}

const treeView = new TreeViewExtImpl(plugin, treeViewId, options.treeDataProvider, this.proxy);
const treeView = new TreeViewExtImpl(plugin, treeViewId, options.treeDataProvider, this.proxy, this.commandRegistry.converter);
this.treeViews.set(treeViewId, treeView);

return {
Expand Down Expand Up @@ -120,6 +122,8 @@ class TreeViewExtImpl<T> extends Disposable {
private onDidCollapseElementEmitter: Emitter<TreeViewExpansionEvent<T>> = new Emitter<TreeViewExpansionEvent<T>>();
public readonly onDidCollapseElement = this.onDidCollapseElementEmitter.event;

private disposables = new DisposableCollection();

private selection: T[] = [];
get selectedElements(): T[] { return this.selection; }

Expand All @@ -129,7 +133,8 @@ class TreeViewExtImpl<T> extends Disposable {
private plugin: Plugin,
private treeViewId: string,
private treeDataProvider: TreeDataProvider<T>,
private proxy: TreeViewsMain) {
private proxy: TreeViewsMain,
readonly commandsConverter: CommandsConverter) {

super(() => {
proxy.$unregisterTreeDataProvider(treeViewId);
Expand Down Expand Up @@ -169,6 +174,7 @@ class TreeViewExtImpl<T> extends Disposable {
console.error(`No tree item with id '${parentId}' found.`);
return [];
}
this.disposables.dispose();

// ask data provider for children for cached element
const result = await this.treeDataProvider.getChildren(parent);
Expand Down Expand Up @@ -244,9 +250,6 @@ class TreeViewExtImpl<T> extends Disposable {
}
}

if (treeItem.command) {
treeItem.command.arguments = [id];
}
const treeViewItem = {
id,
label,
Expand All @@ -257,7 +260,7 @@ class TreeViewExtImpl<T> extends Disposable {
tooltip: treeItem.tooltip,
collapsibleState: treeItem.collapsibleState,
contextValue: treeItem.contextValue,
command: treeItem.command
command: treeItem.command ? toInternalCommand(this.commandsConverter.toSafeCommand(treeItem.command, this.disposables)) : undefined
} as TreeViewItem;

treeItems.push(treeViewItem);
Expand Down

0 comments on commit f703994

Please sign in to comment.