[plugin] Cache command arguments to safely pass the command over JSON-RPC

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>

Author: vinokurig

packages/plugin-ext/src/common/plugin-api-rpc.ts

@@ -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;
 }
 

packages/plugin-ext/src/main/browser/view/tree-view-widget.tsx

@@ -127,7 +127,7 @@ export class PluginTree extends TreeImpl {
             contextValue: item.contextValue
         };
         const node = this.getNode(item.id);
-        if (item.collapsibleState !== TreeViewItemCollapsibleState.None) {
+        if (item.collapsibleState !== undefined && item.collapsibleState !== TreeViewItemCollapsibleState.None) {
             if (CompositeTreeViewNode.is(node)) {
                 return Object.assign(node, update);
             }
@@ -141,13 +141,14 @@ 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,
             parent,
             visible: true,
-            selected: false
+            selected: false,
+            command: item.command
         }, update);
     }
 

packages/plugin-ext/src/plugin/command-registry.ts

@@ -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
@@ -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
@@ -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 {
@@ -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)));
@@ -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 isSafeCommandRegistered: boolean;
+
+    constructor(commands: CommandRegistryImpl) {
+        this.safeCommandId = `theia_safe_cmd_${Date.now().toString()}`;
+        this.commands = commands;
+        this.isSafeCommandRegistered = false;
+    }
+
+    /**
+     * Convert to a command that can be safely passed over JSON-RPC.
+     */
+    toSafeCommand(command: theia.Command, disposables: DisposableCollection): theia.Command {
+        if (!this.isSafeCommandRegistered) {
+            this.commands.registerCommand({ id: this.safeCommandId }, this.executeSafeCommand, this);
+            this.isSafeCommandRegistered = 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 || []));
+    }
+
+}

packages/plugin-ext/src/plugin/plugin-context.ts

@@ -174,7 +174,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

packages/plugin-ext/src/plugin/tree/tree-views.ts

@@ -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 => {
@@ -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 {
@@ -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; }
 
@@ -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);
@@ -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);
@@ -244,9 +250,6 @@ class TreeViewExtImpl<T> extends Disposable {
                     }
                 }
 
-                if (treeItem.command) {
-                    treeItem.command.arguments = [id];
-                }
                 const treeViewItem = {
                     id,
                     label,
@@ -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);