-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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'); | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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[]>; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -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>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo:
initialize