Skip to content

Commit

Permalink
[Code] fix a problem we start more than one lang-server for the same …
Browse files Browse the repository at this point in the history
…repo. (#33382)
  • Loading branch information
spacedragon authored and zfy0701 committed Mar 19, 2019
1 parent 1d2b35a commit 14c282a
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 18 deletions.
148 changes: 148 additions & 0 deletions x-pack/plugins/code/server/lsp/controller.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import fs from 'fs';
import mkdirp from 'mkdirp';
import * as os from 'os';
import path from 'path';
import rimraf from 'rimraf';
import sinon from 'sinon';
import { LanguageServerStatus } from '../../common/language_server';
import { LspRequest } from '../../model';
import { RepositoryConfigController } from '../repository_config_controller';
import { ServerOptions } from '../server_options';
import { ConsoleLoggerFactory } from '../utils/console_logger_factory';
import { LanguageServerController } from './controller';
import { InstallManager } from './install_manager';
import { ILanguageServerLauncher } from './language_server_launcher';
import { JAVA, LanguageServerDefinition, TYPESCRIPT } from './language_servers';
import { ILanguageServerHandler } from './proxy';

const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'code_test'));
const workspaceDir = path.join(baseDir, 'workspace');

// @ts-ignore
const options: ServerOptions = sinon.createStubInstance(ServerOptions);
// @ts-ignore
options.lsp = { detach: false };
// @ts-ignore
options.maxWorkspace = 2;

const installManager = sinon.createStubInstance(InstallManager);
// @ts-ignore
installManager.status = (def: LanguageServerDefinition) => {
return LanguageServerStatus.READY;
};

const repoConfigController = sinon.createStubInstance(RepositoryConfigController);
// @ts-ignore
repoConfigController.isLanguageDisabled = (uri: string, lang: string) => {
return Promise.resolve(false);
};

const launcherSpy = sinon.stub();

class LauncherStub implements ILanguageServerLauncher {
public get running(): boolean {
return launcherSpy.called;
}

public launch(
builtinWorkspace: boolean,
maxWorkspace: number,
installationPath?: string
): Promise<ILanguageServerHandler> {
return Promise.resolve(launcherSpy(builtinWorkspace, maxWorkspace, installationPath));
}
}

TYPESCRIPT.launcher = LauncherStub;
JAVA.launcher = LauncherStub;

let controller: typeof LanguageServerController;

beforeAll(() => {
mkdirp.sync(workspaceDir);
});
beforeEach(async () => {
sinon.reset();
const handler: ILanguageServerHandler = {
handleRequest(request: LspRequest): any {
return {};
},
exit(): any {
return {};
},
unloadWorkspace(_: string): any {
return {};
},
};
launcherSpy.returns(handler);
controller = new LanguageServerController(
options,
'127.0.0.1',
// @ts-ignore
installManager,
new ConsoleLoggerFactory(),
repoConfigController
);
});
afterAll(() => {
rimraf.sync(baseDir);
});

function mockRequest(repo: string, file: string) {
const repoPath = path.join(workspaceDir, repo);
mkdirp.sync(repoPath);
return {
method: 'request',
params: [],
workspacePath: repoPath,
timeoutForInitializeMs: 100,
resolvedFilePath: path.join(repoPath, file),
};
}

test('controller should launch a lang server', async () => {
const request = mockRequest('repo1', 'test.ts');
// @ts-ignore
await controller.handleRequest(request);
expect(launcherSpy.calledOnce).toBeTruthy();
});

test('java-lang-server support should only be launched exactly once', async () => {
const request1 = mockRequest('repo1', 'Test.java');
const request2 = mockRequest('repo2', 'Test.java');
// @ts-ignore
const p1 = controller.handleRequest(request1);
// @ts-ignore
const p2 = controller.handleRequest(request2);
await Promise.all([p1, p2]);
expect(launcherSpy.calledOnce).toBeTruthy();
});

test('should launch 2 ts-lang-server for different repo', async () => {
const request1 = mockRequest('repo1', 'test.ts');
const request2 = mockRequest('repo2', 'test.ts');
// @ts-ignore
const p1 = controller.handleRequest(request1);
// @ts-ignore
const p2 = controller.handleRequest(request2);
await Promise.all([p1, p2]);
expect(launcherSpy.calledTwice).toBeTruthy();
});

test('should only exactly 1 ts-lang-server for the same repo', async () => {
const request1 = mockRequest('repo1', 'test.ts');
const request2 = mockRequest('repo1', 'test.ts');
// @ts-ignore
const p1 = controller.handleRequest(request1);
// @ts-ignore
const p2 = controller.handleRequest(request2);
await Promise.all([p1, p2]);
expect(launcherSpy.calledOnce).toBeTruthy();
expect(launcherSpy.calledTwice).toBe(false);
});
40 changes: 23 additions & 17 deletions x-pack/plugins/code/server/lsp/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { LanguageServerDefinition, LanguageServers } from './language_servers';
import { ILanguageServerHandler } from './proxy';

export interface LanguageServerHandlerMap {
[workspaceUri: string]: ILanguageServerHandler;
[workspaceUri: string]: Promise<ILanguageServerHandler>;
}

interface LanguageServerData {
Expand All @@ -34,7 +34,7 @@ interface LanguageServerData {
maxWorkspace: number;
languages: string[];
launcher: ILanguageServerLauncher;
languageServerHandlers?: ILanguageServerHandler | LanguageServerHandlerMap;
languageServerHandlers?: Promise<ILanguageServerHandler> | LanguageServerHandlerMap;
}

/**
Expand Down Expand Up @@ -95,15 +95,15 @@ export class LanguageServerController implements ILanguageServerHandler {
if (lang) {
const ls = this.findLanguageServer(lang);
if (ls.builtinWorkspaceFolders) {
if (!ls.launcher.running) {
ls.languageServerHandlers = await ls.launcher.launch(
if (!ls.languageServerHandlers && !ls.launcher.running) {
ls.languageServerHandlers = ls.launcher.launch(
ls.builtinWorkspaceFolders,
ls.maxWorkspace,
this.installManager.installationPath(ls.definition)
);
}
const handler = ls.languageServerHandlers as ILanguageServerHandler;
return handler.handleRequest(request);
const handler = ls.languageServerHandlers as Promise<ILanguageServerHandler>;
return (await handler).handleRequest(request);
} else {
const handler = await this.findOrCreateHandler(ls, request);
handler.lastAccess = Date.now();
Expand All @@ -127,11 +127,13 @@ export class LanguageServerController implements ILanguageServerHandler {
if (ls.languageServerHandlers) {
if (ls.builtinWorkspaceFolders) {
if (ls.languageServerHandlers) {
await (ls.languageServerHandlers as ILanguageServerHandler).exit();
const h = await (ls.languageServerHandlers as Promise<ILanguageServerHandler>);
await h.exit();
}
} else {
const handlers = ls.languageServerHandlers as LanguageServerHandlerMap;
for (const handler of Object.values(handlers)) {
for (const handlerPromise of Object.values(handlers)) {
const handler = await handlerPromise;
await handler.exit();
}
}
Expand All @@ -145,7 +147,7 @@ export class LanguageServerController implements ILanguageServerHandler {
// for those language server has builtin workspace support, we can launch them during kibana startup
if (installed && ls.builtinWorkspaceFolders) {
try {
ls.languageServerHandlers = await ls.launcher.launch(
ls.languageServerHandlers = ls.launcher.launch(
true,
ls.maxWorkspace,
this.installManager.installationPath(ls.definition)!
Expand All @@ -161,14 +163,16 @@ export class LanguageServerController implements ILanguageServerHandler {
for (const languageServer of this.languageServers) {
if (languageServer.languageServerHandlers) {
if (languageServer.builtinWorkspaceFolders) {
const handler = languageServer.languageServerHandlers as ILanguageServerHandler;
const handler = await (languageServer.languageServerHandlers as Promise<
ILanguageServerHandler
>);
await handler.unloadWorkspace(workspaceDir);
} else {
const handlers = languageServer.languageServerHandlers as LanguageServerHandlerMap;
const realPath = fs.realpathSync(workspaceDir);
const handler = handlers[realPath];
if (handler) {
await handler.unloadWorkspace(realPath);
await (await handler).unloadWorkspace(realPath);
delete handlers[realPath];
}
}
Expand Down Expand Up @@ -213,7 +217,7 @@ export class LanguageServerController implements ILanguageServerHandler {
const maxWorkspace = languageServer.maxWorkspace;
const handlerArray = Object.entries(handlers);
if (handlerArray.length < maxWorkspace) {
handler = await languageServer.launcher.launch(
handler = languageServer.launcher.launch(
languageServer.builtinWorkspaceFolders,
maxWorkspace,
this.installManager.installationPath(languageServer.definition)
Expand All @@ -222,13 +226,15 @@ export class LanguageServerController implements ILanguageServerHandler {
return handler;
} else {
let [oldestWorkspace, oldestHandler] = handlerArray[0];
handlerArray.forEach(p => {
const [ws, h] = p;
if (h.lastAccess! < oldestHandler.lastAccess!) {
for (const e of handlerArray) {
const [ws, handlePromise] = e;
const h = await handlePromise;
const oldestAccess = (await oldestHandler).lastAccess!;
if (h.lastAccess! < oldestAccess!) {
oldestWorkspace = ws;
oldestHandler = h;
oldestHandler = handlePromise;
}
});
}
delete handlers[oldestWorkspace];
handlers[request.workspacePath] = oldestHandler;
return oldestHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import fs from 'fs';
import { ServerOptions } from '../server_options';
import { createTestServerOption } from '../test_utils';
import { ConsoleLoggerFactory } from '../utils/console_logger_factory';
import { TYPESCRIPT } from './language_servers';
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/code/server/lsp/workspace_handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import fs from 'fs';
import path from 'path';

Expand Down

0 comments on commit 14c282a

Please sign in to comment.