Skip to content

Commit 3ab005e

Browse files
d3r3kkd3r3kk
authored
Allow for the use of a proxy server during Python Language Server download (microsoft#2418)
- Updates to get downloader to become testable - Refactor the downloader to accept an interface for request handling - Make use of chmod async - update mock vscode file to match latest vscode interface for WorkspaceEdit
1 parent e3a6bc2 commit 3ab005e

12 files changed

Lines changed: 158 additions & 44 deletions

File tree

‎.vscode/launch.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,4 +133,4 @@
133133
]
134134
}
135135
]
136-
}
136+
}

‎news/1 Enhancements/2385.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make use of the `http.proxy` field in settings.json when downloading the Python Language Server.

‎src/client/activation/downloader.ts‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,14 @@
33

44
'use strict';
55

6-
import * as fileSystem from 'fs';
76
import * as path from 'path';
8-
import * as request from 'request';
97
import * as requestProgress from 'request-progress';
10-
import { OutputChannel, ProgressLocation, window } from 'vscode';
11-
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
8+
import { ProgressLocation, window } from 'vscode';
129
import { createDeferred } from '../common/helpers';
13-
import { IFileSystem, IPlatformService } from '../common/platform/types';
10+
import { IFileSystem } from '../common/platform/types';
1411
import { IExtensionContext, IOutputChannel } from '../common/types';
15-
import { IServiceContainer } from '../ioc/types';
1612
import { PlatformData, PlatformName } from './platformData';
13+
import { IDownloadFileService } from './types';
1714

1815
// tslint:disable-next-line:no-require-imports no-var-requires
1916
const StreamZip = require('node-stream-zip');
@@ -31,25 +28,21 @@ export const DownloadLinks = {
3128
};
3229

3330
export class LanguageServerDownloader {
34-
private readonly output: OutputChannel;
35-
private readonly platform: IPlatformService;
36-
private readonly platformData: PlatformData;
37-
private readonly fs: IFileSystem;
38-
39-
constructor(private readonly services: IServiceContainer, private engineFolder: string) {
40-
this.output = this.services.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
41-
this.fs = this.services.get<IFileSystem>(IFileSystem);
42-
this.platform = this.services.get<IPlatformService>(IPlatformService);
43-
this.platformData = new PlatformData(this.platform, this.fs);
44-
}
45-
46-
public async getDownloadUri() {
47-
const platformString = await this.platformData.getPlatformName();
31+
constructor(
32+
private readonly output: IOutputChannel,
33+
private readonly fs: IFileSystem,
34+
private readonly platformData: PlatformData,
35+
private requestHandler: IDownloadFileService,
36+
private engineFolder: string
37+
) { }
38+
39+
public getDownloadUri() {
40+
const platformString = this.platformData.getPlatformName();
4841
return DownloadLinks[platformString];
4942
}
5043

5144
public async downloadLanguageServer(context: IExtensionContext): Promise<void> {
52-
const downloadUri = await this.getDownloadUri();
45+
const downloadUri = this.getDownloadUri();
5346

5447
let localTempFilePath = '';
5548
try {
@@ -71,7 +64,7 @@ export class LanguageServerDownloader {
7164
const tempFile = await this.fs.createTemporaryFile(downloadFileExtension);
7265

7366
const deferred = createDeferred();
74-
const fileStream = fileSystem.createWriteStream(tempFile.filePath);
67+
const fileStream = this.fs.createWriteStream(tempFile.filePath);
7568
fileStream.on('finish', () => {
7669
fileStream.close();
7770
}).on('error', (err) => {
@@ -83,7 +76,8 @@ export class LanguageServerDownloader {
8376
location: ProgressLocation.Window
8477
}, (progress) => {
8578

86-
requestProgress(request(uri))
79+
requestProgress(
80+
this.requestHandler!.downloadFile(uri))
8781
.on('progress', (state) => {
8882
// https://www.npmjs.com/package/request-progress
8983
const received = Math.round(state.size.transferred / 1024);
@@ -147,11 +141,10 @@ export class LanguageServerDownloader {
147141
return deferred.promise;
148142
});
149143

150-
// Set file to executable
151-
if (!this.platform.isWindows) {
152-
const executablePath = path.join(installFolder, this.platformData.getEngineExecutableName());
153-
fileSystem.chmodSync(executablePath, '0764'); // -rwxrw-r--
154-
}
144+
// Set file to executable (nothing happens in Windows, as chmod has no definition there)
145+
const executablePath = path.join(installFolder, this.platformData.getEngineExecutableName());
146+
await this.fs.chmod(executablePath, '0764'); // -rwxrw-r--
147+
155148
this.output.appendLine('done.');
156149
}
157150
}

‎src/client/activation/languageServer.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { LanguageServerDownloader } from './downloader';
3333
import { InterpreterData, InterpreterDataService } from './interpreterDataService';
3434
import { PlatformData } from './platformData';
3535
import { ProgressReporting } from './progress';
36+
import { RequestWithProxy } from './requestWithProxy';
3637
import { IExtensionActivator } from './types';
3738

3839
const PYTHON = 'python';
@@ -137,7 +138,12 @@ export class LanguageServerExtensionActivator implements IExtensionActivator {
137138

138139
const mscorlib = path.join(this.context.extensionPath, languageServerFolder, 'mscorlib.dll');
139140
if (!await this.fs.fileExists(mscorlib)) {
140-
const downloader = new LanguageServerDownloader(this.services, languageServerFolder);
141+
const downloader = new LanguageServerDownloader(
142+
this.output,
143+
this.fs,
144+
this.platformData,
145+
new RequestWithProxy(this.workspace.getConfiguration('http').get('proxy', '')),
146+
languageServerFolder);
141147
await downloader.downloadLanguageServer(this.context);
142148
reporter.sendTelemetryEvent(PYTHON_LANGUAGE_SERVER_DOWNLOADED);
143149
}

‎src/client/activation/platformData.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export enum PlatformLSExecutables {
2424

2525
export class PlatformData {
2626
constructor(private platform: IPlatformService, fs: IFileSystem) { }
27-
public async getPlatformName(): Promise<PlatformName> {
27+
public getPlatformName(): PlatformName {
2828
if (this.platform.isWindows) {
2929
return this.platform.is64bit ? PlatformName.Windows64Bit : PlatformName.Windows32Bit;
3030
}

‎src/client/activation/requestWithProxy.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import * as request from 'request';
7+
import { IDownloadFileService } from './types';
8+
9+
// Simple wrapper for request to allow for the use of a proxy server being
10+
// specified in the request options.
11+
export class RequestWithProxy implements IDownloadFileService {
12+
constructor(private proxyUri: string) { }
13+
14+
public get requestOptions(): request.CoreOptions | undefined {
15+
if (this.proxyUri && this.proxyUri.length > 0) {
16+
return {
17+
proxy: this.proxyUri
18+
};
19+
} else {
20+
return;
21+
}
22+
}
23+
24+
public downloadFile(uri: string): request.Request {
25+
const requestOptions: request.CoreOptions | undefined = this.requestOptions;
26+
return request(uri, requestOptions);
27+
}
28+
}

‎src/client/activation/types.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
'use strict';
5+
6+
import { Request as RequestResult } from 'request';
7+
48
export const IExtensionActivationService = Symbol('IExtensionActivationService');
59
export interface IExtensionActivationService {
610
activate(): Promise<void>;
@@ -16,3 +20,7 @@ export interface IExtensionActivator {
1620
activate(): Promise<boolean>;
1721
deactivate(): Promise<void>;
1822
}
23+
24+
export interface IDownloadFileService {
25+
downloadFile(uri: string): RequestResult;
26+
}

‎src/client/common/platform/fileSystem.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
'use strict';
44

55
import { createHash } from 'crypto';
6+
import * as fileSystem from 'fs';
67
import * as fs from 'fs-extra';
78
import * as glob from 'glob';
89
import { inject, injectable } from 'inversify';
@@ -151,6 +152,20 @@ export class FileSystem implements IFileSystem {
151152
resolve({ filePath: tmpFile, dispose: cleanupCallback });
152153
});
153154
});
155+
}
156+
157+
public createWriteStream(filePath: string): fileSystem.WriteStream {
158+
return fileSystem.createWriteStream(filePath);
159+
}
154160

161+
public chmod(filePath: string, mode: string): Promise<void> {
162+
return new Promise<void>((resolve, reject) => {
163+
fileSystem.chmod(filePath, mode, (err: NodeJS.ErrnoException) => {
164+
if (err) {
165+
return reject(err);
166+
}
167+
resolve();
168+
});
169+
});
155170
}
156171
}

‎src/client/common/platform/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,6 @@ export interface IFileSystem {
8686
getFileHash(filePath: string): Promise<string | undefined>;
8787
search(globPattern: string): Promise<string[]>;
8888
createTemporaryFile(extension: string): Promise<TemporaryFile>;
89+
createWriteStream(path: string): fs.WriteStream;
90+
chmod(path: string, mode: string): Promise<void>;
8991
}

‎src/test/activation/downloader.unit.test.ts‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,38 @@
66
// tslint:disable:no-unused-variable
77

88
import * as assert from 'assert';
9+
import * as request from 'request';
910
import * as TypeMoq from 'typemoq';
11+
import { WorkspaceConfiguration } from 'vscode';
1012
import { DownloadLinks, LanguageServerDownloader } from '../../client/activation/downloader';
11-
import { PlatformName } from '../../client/activation/platformData';
13+
import { PlatformData, PlatformName } from '../../client/activation/platformData';
14+
import { RequestWithProxy } from '../../client/activation/requestWithProxy';
15+
import { IDownloadFileService } from '../../client/activation/types';
16+
import { IWorkspaceService } from '../../client/common/application/types';
1217
import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
1318
import { IOutputChannel } from '../../client/common/types';
14-
import { IServiceContainer } from '../../client/ioc/types';
1519

1620
suite('Activation - Downloader', () => {
1721
let languageServerDownloader: LanguageServerDownloader;
18-
let serviceContainer: TypeMoq.IMock<IServiceContainer>;
1922
let platformService: TypeMoq.IMock<IPlatformService>;
23+
2024
setup(() => {
21-
serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
2225
platformService = TypeMoq.Mock.ofType<IPlatformService>();
2326
const fs = TypeMoq.Mock.ofType<IFileSystem>();
2427
const output = TypeMoq.Mock.ofType<IOutputChannel>();
28+
const workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
29+
const platformData: PlatformData = new PlatformData(platformService.object, fs.object);
30+
const wsConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
31+
workspace.setup(a => a.getConfiguration(TypeMoq.It.isValue('http'))).returns(() => wsConfig.object);
32+
wsConfig.setup(a => a.get(TypeMoq.It.isValue('proxy'), TypeMoq.It.isAnyString())).returns(() => '');
2533

26-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())).returns(() => output.object);
27-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformService.object);
28-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fs.object);
34+
languageServerDownloader = new LanguageServerDownloader(
35+
output.object,
36+
fs.object,
37+
platformData,
38+
new RequestWithProxy(''),
39+
'');
2940

30-
languageServerDownloader = new LanguageServerDownloader(serviceContainer.object, '');
3141
});
3242
type PlatformIdentifier = {
3343
windows?: boolean;
@@ -43,22 +53,22 @@ suite('Activation - Downloader', () => {
4353
}
4454
test('Windows 32Bit', async () => {
4555
setupPlatform({ windows: true });
46-
const link = await languageServerDownloader.getDownloadUri();
56+
const link = languageServerDownloader.getDownloadUri();
4757
assert.equal(link, DownloadLinks[PlatformName.Windows32Bit]);
4858
});
4959
test('Windows 64Bit', async () => {
5060
setupPlatform({ windows: true, is64Bit: true });
51-
const link = await languageServerDownloader.getDownloadUri();
61+
const link = languageServerDownloader.getDownloadUri();
5262
assert.equal(link, DownloadLinks[PlatformName.Windows64Bit]);
5363
});
5464
test('Mac 64Bit', async () => {
5565
setupPlatform({ mac: true, is64Bit: true });
56-
const link = await languageServerDownloader.getDownloadUri();
66+
const link = languageServerDownloader.getDownloadUri();
5767
assert.equal(link, DownloadLinks[PlatformName.Mac64Bit]);
5868
});
5969
test('Linux 64Bit', async () => {
6070
setupPlatform({ linux: true, is64Bit: true });
61-
const link = await languageServerDownloader.getDownloadUri();
71+
const link = languageServerDownloader.getDownloadUri();
6272
assert.equal(link, DownloadLinks[PlatformName.Linux64Bit]);
6373
});
6474
});

0 commit comments

Comments
 (0)