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.

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

don't send plugin metadata over JSON-RPC #6233

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { JsonRpcProxyFactory, ConnectionHandler, JsonRpcConnectionHandler, JsonR

export type BindFrontendService = <T extends object>(path: string, serviceIdentifier: interfaces.ServiceIdentifier<T>) => interfaces.BindingWhenOnSyntax<T>;
export type BindBackendService = <T extends object, C extends object = object>(
path: string, serviceIdentifier: interfaces.ServiceIdentifier<T>, onActivation?: (service: T, proxy: JsonRpcProxy<C>) => T
path: string, serviceIdentifier: interfaces.ServiceIdentifier<T>, onActivation?: (service: T, proxy: JsonRpcProxy<C>, context: interfaces.Context) => T
) => void;
export type ConnectionContainerModuleCallBack = (registry: {
bind: interfaces.Bind
Expand Down Expand Up @@ -86,7 +86,7 @@ export const ConnectionContainerModule: symbol & { create(callback: ConnectionCo
bind(ConnectionHandler).toDynamicValue(context =>
new JsonRpcConnectionHandler<any>(path, proxy => {
const service = context.container.get(serviceIdentifier);
return onActivation ? onActivation(service, proxy) : service;
return onActivation ? onActivation(service, proxy, context) : service;
})
).inSingletonScope();
};
Expand Down
2 changes: 2 additions & 0 deletions packages/plugin-ext/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
"@theia/task": "^0.10.0",
"@theia/terminal": "^0.10.0",
"@theia/workspace": "^0.10.0",
"@types/compression": "^1.0.1",
"compression": "^1.7.4",
"decompress": "^4.2.0",
"escape-html": "^1.0.3",
"getmac": "^1.4.6",
Expand Down
45 changes: 27 additions & 18 deletions packages/plugin-ext/src/common/plugin-api-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import {
import { ExtPluginApi } from './plugin-ext-api-contribution';
import { KeysToAnyValues, KeysToKeysToAnyValue } from './types';
import { CancellationToken, Progress, ProgressOptions } from '@theia/plugin';
import { IJSONSchema, IJSONSchemaSnippet } from '@theia/core/lib/common/json-schema';
import { DebuggerDescription } from '@theia/debug/lib/common/debug-service';
import { DebugProtocol } from 'vscode-debugprotocol';
import { SymbolInformation } from 'vscode-languageserver-types';
Expand All @@ -76,16 +75,6 @@ import { MaybePromise } from '@theia/core/lib/common/types';
import { QuickOpenItem, QuickOpenItemOptions } from '@theia/core/lib/common/quick-open-model';
import { QuickTitleButton } from '@theia/core/lib/common/quick-open-model';

export interface PluginInitData {
plugins: PluginMetadata[];
preferences: PreferenceData;
globalState: KeysToKeysToAnyValue;
workspaceState: KeysToKeysToAnyValue;
env: EnvInit;
extApi?: ExtPluginApi[];
activationEvents: string[]
}

export interface PreferenceData {
[scope: number]: any;
}
Expand All @@ -100,7 +89,7 @@ export interface Plugin {

export interface ConfigStorage {
hostLogPath: string;
hostStoragePath: string,
hostStoragePath?: string,
}

export interface EnvInit {
Expand Down Expand Up @@ -162,12 +151,35 @@ export const emptyPlugin: Plugin = {
}
};

export interface PluginManagerInitializeParams {
preferences: PreferenceData
globalState: KeysToKeysToAnyValue
workspaceState: KeysToKeysToAnyValue
env: EnvInit
extApi?: ExtPluginApi[]
}

export interface PluginManagerStartParams {
plugins: PluginMetadata[]
configStorage: ConfigStorage
activationEvents: string[]
}

export interface PluginManagerExt {
$stop(pluginId?: string): PromiseLike<void>;

$init(pluginInit: PluginInitData, configStorage: ConfigStorage): PromiseLike<void>;
/** initialize the manager, should be called only once */
$initiliaze(params: PluginManagerInitializeParams): Promise<void>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: initialize


/** load and activate plugins */
$start(params: PluginManagerStartParams): Promise<void>;

/** deactivate the plugin */
$stop(pluginId: string): Promise<void>;

/** deactivate all plugins */
$stop(): Promise<void>;

$updateStoragePath(path: string | undefined): PromiseLike<void>;
$updateStoragePath(path: string | undefined): Promise<void>;

$activateByEvent(event: string): Promise<void>;
}
Expand Down Expand Up @@ -1236,9 +1248,6 @@ export interface DebugExt {
$sessionDidChange(sessionId: string | undefined): void;
$provideDebugConfigurations(debugType: string, workspaceFolder: string | undefined): Promise<theia.DebugConfiguration[]>;
$resolveDebugConfigurations(debugConfiguration: theia.DebugConfiguration, workspaceFolder: string | undefined): Promise<theia.DebugConfiguration | undefined>;
$getSupportedLanguages(debugType: string): Promise<string[]>;
$getSchemaAttributes(debugType: string): Promise<IJSONSchema[]>;
$getConfigurationSnippets(debugType: string): Promise<IJSONSchemaSnippet[]>;
$createDebugSession(debugConfiguration: theia.DebugConfiguration): Promise<string>;
$terminateDebugSession(sessionId: string): Promise<void>;
$getTerminalCreationOptions(debugType: string): Promise<TerminalOptionsExt | undefined>;
Expand Down
30 changes: 27 additions & 3 deletions packages/plugin-ext/src/common/plugin-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,20 @@ export interface ExtensionContext {
subscriptions: Disposable[];
}

/**
* PluginMetadataHandle should be sent instead of PluginMetadata from the frontend to the backend.
* The plugin server will expand it to PluginMetadata if a plugin for such handle is deployed.
*/
export interface PluginMetadataHandle {
pluginHandle: string
}
export namespace PluginMetadataHandle {
// tslint:disable-next-line:no-any
export function is(arg: any): arg is PluginMetadataHandle {
return !!arg && typeof arg === 'object' && 'pluginHandle' in arg;
}
}

export interface PluginMetadata {
host: string;
source: PluginPackage;
Expand All @@ -601,13 +615,21 @@ export function buildFrontendModuleName(plugin: PluginPackage | PluginModel): st
return `${plugin.publisher}_${plugin.name}`.replace(/\W/g, '_');
}

export interface InitializePluginClientResult {
clientId: string
}

export const HostedPluginClient = Symbol('HostedPluginClient');
export interface HostedPluginClient {

initialize(): Promise<InitializePluginClientResult>;

postMessage(message: string): Promise<void>;

log(logPart: LogPart): void;

onDidDeploy(): void;

}

export const PluginDeployerHandler = Symbol('PluginDeployerHandler');
Expand All @@ -619,11 +641,13 @@ export interface PluginDeployerHandler {
}

export const HostedPluginServer = Symbol('HostedPluginServer');
/**
* Don't expose plugin metadata via JSON-RPC. It is expensive to load and blocks the underlying connection.
* Instead use `/plugins` http endpoint, it serves compressed metadata for the give set of plugins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: .. the given set of

*/
export interface HostedPluginServer extends JsonRpcServer<HostedPluginClient> {

getDeployedMetadata(): Promise<PluginMetadata[]>;
getDeployedFrontendMetadata(): Promise<PluginMetadata[]>;
getDeployedBackendMetadata(): Promise<PluginMetadata[]>;
getDeployedPlugins(): Promise<string[]>;

getExtPluginAPI(): Promise<ExtPluginApi[]>;

Expand Down
17 changes: 17 additions & 0 deletions packages/plugin-ext/src/hosted/browser/hosted-plugin-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { UUID } from '@phosphor/coreutils';
import { injectable } from 'inversify';
import { Emitter, Event } from '@theia/core/lib/common/event';
import { HostedPluginClient } from '../../common/plugin-protocol';
import { LogPart } from '../../common/types';
import { Deferred } from '@theia/core/lib/common/promise-util';

@injectable()
export class HostedPluginWatcher {
Expand All @@ -27,10 +29,25 @@ export class HostedPluginWatcher {
private readonly onDidDeployEmitter = new Emitter<void>();
readonly onDidDeploy = this.onDidDeployEmitter.event;

readonly clientId = UUID.uuid4();

protected _initialized = new Deferred<void>();
get initialized(): Promise<void> {
return this._initialized.promise;
}

reset(): void {
this._initialized = new Deferred<void>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also generate a new id here, just to avoid confusion with previous connections?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be confusing if the same window will get different id on reconnect, e.g. you see that some plugin was disconnected for some client id, but then it gets immediately started for another client id without first loading it. I think it would be more consistent to see that a plugin was disconnected for such client and then started again for the same client.

}

getHostedPluginClient(): HostedPluginClient {
const messageEmitter = this.onPostMessage;
const logEmitter = this.onLogMessage;
return {
initialize: async () => {
this._initialized.resolve();
return { clientId: this.clientId };
},
postMessage(message: string): Promise<void> {
messageEmitter.fire(JSON.parse(message));
return Promise.resolve();
Expand Down
Loading