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

[Code] correctly handle url when workspace is a symbolic link. #31782

Merged
merged 1 commit into from
Feb 23, 2019
Merged
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
123 changes: 123 additions & 0 deletions x-pack/plugins/code/server/lsp/workspace_handler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* 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 path from 'path';

import mkdirp from 'mkdirp';
import * as os from 'os';
import rimraf from 'rimraf';
import { ResponseMessage } from 'vscode-jsonrpc/lib/messages';
import { LspRequest } from '../../model';
import { ConsoleLoggerFactory } from '../utils/console_logger_factory';
import { WorkspaceHandler } from './workspace_handler';

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

function handleResponseUri(wh: WorkspaceHandler, uri: string) {
const dummyRequest: LspRequest = {
method: 'textDocument/edefinition',
params: [],
};
const dummyResponse: ResponseMessage = {
id: null,
jsonrpc: '',
result: [
{
location: {
uri,
},
},
],
};
wh.handleResponse(dummyRequest, dummyResponse);
return dummyResponse.result[0].location.uri;
}

function makeAFile(
workspacePath: string = workspaceDir,
repo = 'github.com/Microsoft/TypeScript-Node-Starter',
revision = 'master',
file = 'src/controllers/user.ts'
) {
const fullPath = path.join(workspacePath, repo, '__randomString', revision, file);
mkdirp.sync(path.dirname(fullPath));
fs.writeFileSync(fullPath, '');
const strInUrl = fullPath
.split(path.sep)
.map(value => encodeURIComponent(value))
.join('/');
const uri = `file:///${strInUrl}`;
return { repo, revision, file, uri };
}

test('file system url should be converted', async () => {
const workspaceHandler = new WorkspaceHandler(
repoDir,
workspaceDir,
// @ts-ignore
null,
new ConsoleLoggerFactory()
);
const { repo, revision, file, uri } = makeAFile(workspaceDir);
const converted = handleResponseUri(workspaceHandler, uri);
expect(converted).toBe(`git://${repo}/blob/${revision}/${file}`);
});

test('should support symbol link', async () => {
const symlinkToWorkspace = path.join(baseDir, 'linkWorkspace');
fs.symlinkSync(workspaceDir, symlinkToWorkspace, 'dir');
// @ts-ignore
const workspaceHandler = new WorkspaceHandler(
repoDir,
symlinkToWorkspace,
// @ts-ignore
null,
new ConsoleLoggerFactory()
);

const { repo, revision, file, uri } = makeAFile(workspaceDir);
const converted = handleResponseUri(workspaceHandler, uri);
expect(converted).toBe(`git://${repo}/blob/${revision}/${file}`);
});

test('should support spaces in workspace dir', async () => {
const workspaceHasSpaces = path.join(baseDir, 'work space');
const workspaceHandler = new WorkspaceHandler(
repoDir,
workspaceHasSpaces,
// @ts-ignore
null,
new ConsoleLoggerFactory()
);
const { repo, revision, file, uri } = makeAFile(workspaceHasSpaces);
const converted = handleResponseUri(workspaceHandler, uri);
expect(converted).toBe(`git://${repo}/blob/${revision}/${file}`);
});

test('should throw a error if url is invalid', async () => {
const workspaceHandler = new WorkspaceHandler(
repoDir,
workspaceDir,
// @ts-ignore
null,
new ConsoleLoggerFactory()
);
const invalidDir = path.join(baseDir, 'invalid_dir');
const { uri } = makeAFile(invalidDir);
expect(() => handleResponseUri(workspaceHandler, uri)).toThrow();
});

beforeAll(() => {
mkdirp.sync(workspaceDir);
mkdirp.sync(repoDir);
});

afterAll(() => {
rimraf.sync(baseDir);
});
17 changes: 11 additions & 6 deletions x-pack/plugins/code/server/lsp/workspace_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import del from 'del';
import fs from 'fs';
import { delay } from 'lodash';
import mkdirp from 'mkdirp';
import { Clone, Commit, Error, Repository, Reset } from 'nodegit';
import { Clone, Commit, Error as GitError, Repository, Reset } from 'nodegit';
import path from 'path';
import { ResponseMessage } from 'vscode-jsonrpc/lib/messages';
import { Hover, Location, TextDocumentPositionParams } from 'vscode-languageserver';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class WorkspaceHandler {
this.log.info(`checkout ${workspaceRepo.workdir()} to commit ${targetCommit.sha()}`);
// @ts-ignore
const result = await Reset.reset(workspaceRepo, commit, Reset.TYPE.HARD, {});
if (result !== undefined && result !== Error.CODE.OK) {
if (result !== undefined && result !== GitError.CODE.OK) {
throw Boom.internal(`checkout workspace to commit ${targetCommit.sha()} failed.`);
}
}
Expand Down Expand Up @@ -244,13 +244,16 @@ export class WorkspaceHandler {
}
}

// todo add an unit test
private parseLocation(location: Location) {
const uri = location.uri;
if (uri && uri.startsWith('file://')) {
const workspaceUrl = new URL(`file://${this.workspacePath}`).toString();
if (uri.startsWith(workspaceUrl)) {
const relativePath = uri.substring(workspaceUrl.length + 1);
const locationPath = fs.realpathSync(decodeURIComponent(uri.substring('file://'.length)));
const workspacePath = fs.realpathSync(decodeURIComponent(this.workspacePath));
if (locationPath.startsWith(workspacePath)) {
const relativePath = path.relative(workspacePath, locationPath);
if (path.sep !== '/') {
Copy link
Contributor

Choose a reason for hiding this comment

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

so this is for the sake of path on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah

relativePath.replace(path.sep, '/');
}
const regex = /^(.*?\/.*?\/.*?)\/(__.*?\/)?([^_]+?)\/(.*)$/;
const m = relativePath.match(regex);
if (m) {
Expand All @@ -261,6 +264,8 @@ export class WorkspaceHandler {
return { repoUri, revision: gitRevision, file };
}
}
// @ts-ignore
throw new Error("path in response doesn't not starts with workspace path");
}
return null;
}
Expand Down