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.

Sign up for GitHub

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

GLSP-1438: Improve GLSPClient default implementations #402

Merged
merged 1 commit into from
Dec 6, 2024
Merged

GLSP-1438: Improve GLSPClient default implementations #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 11 additions & 5 deletions packages/client/src/base/action-dispatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
import {
Action,
ActionDispatcher,
ActionHandlerRegistry,
EMPTY_ROOT,
GModelRoot,
IActionDispatcher,
RequestAction,
ResponseAction,
SetModelAction
SetModelAction,
TYPES
} from '@eclipse-glsp/sprotty';
import { inject, injectable } from 'inversify';
import { GLSPActionHandlerRegistry } from './action-handler-registry';
Expand All @@ -37,6 +39,12 @@ export class GLSPActionDispatcher extends ActionDispatcher implements IGModelRoo
@inject(ModelInitializationConstraint)
protected initializationConstraint: ModelInitializationConstraint;

@inject(ActionHandlerRegistry)
protected override actionHandlerRegistry: ActionHandlerRegistry;

/** @deprecated No longer in used. The {@link ActionHandlerRegistry} is now directly injected */
// eslint-disable-next-line deprecation/deprecation
@inject(TYPES.ActionHandlerRegistryProvider) protected override actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>;
protected postUpdateQueue: Action[] = [];

override initialize(): Promise<void> {
Expand All @@ -47,10 +55,8 @@ export class GLSPActionDispatcher extends ActionDispatcher implements IGModelRoo
}

protected async doInitialize(): Promise<void> {
const registry = await this.actionHandlerRegistryProvider();
this.actionHandlerRegistry = registry;
if (registry instanceof GLSPActionHandlerRegistry) {
registry.initialize();
if (this.actionHandlerRegistry instanceof GLSPActionHandlerRegistry) {
this.actionHandlerRegistry.initialize();
}
this.handleAction(SetModelAction.create(EMPTY_ROOT)).catch(() => {
/* Logged in handleAction method */
Expand Down
54 changes: 43 additions & 11 deletions packages/protocol/src/client-server-protocol/base-glsp-client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,48 @@ describe('Node GLSP Client', () => {
describe('start', () => {
it('should fail if no server is configured', async () => {
resetClient(false);
const stateChangeHandler = sinon.spy();
client.onCurrentStateChanged(stateChangeHandler);
client.setStartupTimeout(5);
await expectToThrowAsync(() => client.start());
expect(client.currentState).to.be.equal(ClientState.StartFailed);
expect(stateChangeHandler.calledWith(ClientState.StartFailed)).to.be.true;
});
it('Should resolve when server is configured', async () => {
resetClient(false);
const stateChangeHandler = sinon.spy();
client.onCurrentStateChanged(stateChangeHandler);
const started = client.start();
expect(client.currentState).to.be.equal(ClientState.Starting);
expect(stateChangeHandler.calledWith(ClientState.Starting)).to.be.true;
client.configureServer(server);
const result = await client.start();
expect(result).to.be.undefined;
await started;
expect(client.currentState).to.be.equal(ClientState.Running);
expect(stateChangeHandler.calledWith(ClientState.Running)).to.be.true;
});
});

describe('stop & onStop', () => {
beforeEach(() => resetClient());
it('onStop should not resolve if stop has not been called', () => {
resetClient();
expect(util.inspect(client.onStop())).to.include('pending');
});
it('should be in stopped state and onStop should resolve', async () => {
resetClient();
expect(client.currentState).to.be.not.equal(ClientState.Stopped);
const stopResult = await client.stop();
expect(stopResult).to.be.undefined;
const stateChangeHandler = sinon.spy();
client.onCurrentStateChanged(stateChangeHandler);
await client.stop();
expect(client.currentState).to.be.equal(ClientState.Stopped);
expect(stateChangeHandler.calledWith(ClientState.Stopping)).to.be.true;
expect(stateChangeHandler.calledWith(ClientState.Stopped)).to.be.true;
expect(server.shutdown.calledOnce).to.be.true;
});
it('should only stop a running client once, if stop is called multiple times ', async () => {
resetClient();
client.stop();
await client.stop();
expect(client.currentState).to.be.equal(ClientState.Stopped);
const onStopResult = await client.onStop();
expect(onStopResult).to.be.undefined;
expect(server.shutdown.calledOnce).to.be.true;
});
});
Expand All @@ -104,12 +121,14 @@ describe('Node GLSP Client', () => {
resetClient(false);
await expectToThrowAsync(() => client.initializeServer({ applicationId: '', protocolVersion: '' }));
expect(server.initialize.called).to.be.false;
expect(client.initializeResult).to.be.undefined;
});
it('should fail if client is not running', async () => {
resetClient(false);
client.configureServer(server);
await expectToThrowAsync(() => client.initializeServer({ applicationId: '', protocolVersion: '' }));
expect(server.initialize.called).to.be.false;
expect(client.initializeResult).to.be.undefined;
});
it('should invoke the corresponding server method', async () => {
resetClient();
Expand All @@ -122,17 +141,17 @@ describe('Node GLSP Client', () => {
expect(client.initializeResult).to.be.equal(result);
});
it('should return cached result on consecutive invocation', async () => {
await resetClient();
resetClient();
const expectedResult = { protocolVersion: '1.0.0', serverActions: {} };
const params = { applicationId: 'id', protocolVersion: '1.0.0' };
server.initialize.returns(Promise.resolve(expectedResult));
client['_initializeResult'] = expectedResult;
client.initializeServer(params);
const result = await client.initializeServer(params);
expect(result).to.be.deep.equal(client.initializeResult);
expect(server.initialize.called).to.be.false;
expect(server.initialize.calledOnce).to.be.true;
});
it('should fire event on first invocation', async () => {
await resetClient();
resetClient();
const expectedResult = { protocolVersion: '1.0.0', serverActions: {} };
const params = { applicationId: 'id', protocolVersion: '1.0.0' };
server.initialize.returns(Promise.resolve(expectedResult));
Expand All @@ -144,6 +163,19 @@ describe('Node GLSP Client', () => {
await client.initializeServer(params);
expect(eventHandlerSpy.calledOnceWith(expectedResult)).to.be.true;
});
it('should not use cached result on consecutive invocation if previous invocation errored', async () => {
resetClient();
const expectedResult = { protocolVersion: '1.0.0', serverActions: {} };
const params = { applicationId: 'id', protocolVersion: '1.0.0' };
server.initialize.throws(new Error('error'));
expectToThrowAsync(() => client.initializeServer(params));
expect(client.initializeResult).to.be.undefined;
server.initialize.returns(Promise.resolve(expectedResult));
const result = await client.initializeServer(params);
expect(result).to.be.deep.equal(expectedResult);
expect(server.initialize.calledTwice).to.be.true;
expect(client.initializeResult).to.be.equal(result);
});
});

describe('initializeClientSession', () => {
Expand Down
66 changes: 48 additions & 18 deletions packages/protocol/src/client-server-protocol/base-glsp-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,49 @@ export const GLOBAL_HANDLER_ID = '*';
* directly communicates with a given {@link GLSPServer} instance.
*/
export class BaseGLSPClient implements GLSPClient {
protected state: ClientState;
protected _server?: GLSPServer;
protected serverDeferred = new Deferred<GLSPServer>();
protected onStartDeferred = new Deferred<void>();
protected onStopDeferred = new Deferred<void>();
readonly proxy: GLSPClientProxy;
protected startupTimeout = 1500;
protected actionMessageHandlers: Map<string, ActionMessageHandler[]> = new Map([[GLOBAL_HANDLER_ID, []]]);
protected _initializeResult: InitializeResult;
protected pendingServerInitialize?: Promise<InitializeResult>;

protected onServerInitializedEmitter = new Emitter<InitializeResult>();
get onServerInitialized(): Event<InitializeResult> {
return this.onServerInitializedEmitter.event;
}

protected onCurrentStateChangedEmitter = new Emitter<ClientState>();
get onCurrentStateChanged(): Event<ClientState> {
return this.onCurrentStateChangedEmitter.event;
}

protected _state: ClientState;
protected set state(state: ClientState) {
if (this._state !== state) {
this._state = state;
this.onCurrentStateChangedEmitter.fire(state);
}
}
protected get state(): ClientState {
return this._state;
}

protected _server?: GLSPServer;
protected get checkedServer(): GLSPServer {
this.checkState();
if (!this._server) {
throw new Error(`No server is configured for GLSPClient with id '${this.id}'`);
}
return this._server;
}

protected _initializeResult?: InitializeResult;
get initializeResult(): InitializeResult | undefined {
return this._initializeResult;
}

constructor(protected options: GLSPClient.Options) {
this.state = ClientState.Initial;
this.proxy = this.createProxy();
Expand All @@ -71,7 +99,7 @@ export class BaseGLSPClient implements GLSPClient {
}

start(): Promise<void> {
if (this.state === ClientState.Running) {
if (this.state === ClientState.Running || this.state === ClientState.Starting) {
return this.onStartDeferred.promise;
}

Expand All @@ -96,15 +124,25 @@ export class BaseGLSPClient implements GLSPClient {
}

async initializeServer(params: InitializeParameters): Promise<InitializeResult> {
if (!this._initializeResult) {
if (this.initializeResult) {
return this.initializeResult;
} else if (this.pendingServerInitialize) {
return this.pendingServerInitialize;
}

const initializeDeferred = new Deferred<InitializeResult>();
try {
this.pendingServerInitialize = initializeDeferred.promise;
this._initializeResult = await this.checkedServer.initialize(params);
this.onServerInitializedEmitter.fire(this._initializeResult);
initializeDeferred.resolve(this._initializeResult);
this.pendingServerInitialize = undefined;
} catch (error) {
initializeDeferred.reject(error);
this._initializeResult = undefined;
this.pendingServerInitialize = undefined;
}
return this._initializeResult;
}

get initializeResult(): InitializeResult | undefined {
return this._initializeResult;
return initializeDeferred.promise;
}

initializeClientSession(params: InitializeClientSessionParameters): Promise<void> {
Expand Down Expand Up @@ -174,14 +212,6 @@ export class BaseGLSPClient implements GLSPClient {
}
}

protected get checkedServer(): GLSPServer {
this.checkState();
if (!this._server) {
throw new Error(`No server is configured for GLSPClient with id '${this.id}'`);
}
return this._server;
}

setStartupTimeout(ms: number): void {
this.startupTimeout = ms;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/protocol/src/client-server-protocol/glsp-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export interface GLSPClient {
* Current client state.
*/
readonly currentState: ClientState;
/**
* Event that is fired whenever the client state changes.
*/
readonly onCurrentStateChanged: Event<ClientState>;

/**
* Initializes the client and the server connection. During the start procedure the client is in the
Expand Down Expand Up @@ -134,6 +138,7 @@ export interface GLSPClient {
/**
* Stops the client and disposes unknown resources. During the stop procedure the client is in the `Stopping` state and will
* transition to either `Stopped` or `ServerError`.
* Calling the method if client is already stopped has no effect.
*
* @returns A promise that resolves after the server was stopped and disposed.
*/
Expand Down
Loading
Loading