Skip to content

Commit 8e91332

Browse files
karthiknadigkarthiknadig
authored
Simplify buffer decoder. (#19836)
1 parent fc45b1c commit 8e91332

17 files changed

+45
-106
lines changed

‎src/client/common/process/decoder.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@
22
// Licensed under the MIT License.
33

44
import * as iconv from 'iconv-lite';
5-
import { injectable } from 'inversify';
65
import { DEFAULT_ENCODING } from './constants';
7-
import { IBufferDecoder } from './types';
86

9-
@injectable()
10-
export class BufferDecoder implements IBufferDecoder {
11-
public decode(buffers: Buffer[], encoding: string = DEFAULT_ENCODING): string {
12-
encoding = iconv.encodingExists(encoding) ? encoding : DEFAULT_ENCODING;
13-
return iconv.decode(Buffer.concat(buffers), encoding);
14-
}
7+
export function decodeBuffer(buffers: Buffer[], encoding: string = DEFAULT_ENCODING): string {
8+
encoding = iconv.encodingExists(encoding) ? encoding : DEFAULT_ENCODING;
9+
return iconv.decode(Buffer.concat(buffers), encoding);
1510
}

‎src/client/common/process/proc.ts‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,12 @@ import { traceError } from '../../logging';
66
import { IDisposable } from '../types';
77
import { EnvironmentVariables } from '../variables/types';
88
import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis';
9-
import {
10-
ExecutionResult,
11-
IBufferDecoder,
12-
IProcessService,
13-
ObservableExecutionResult,
14-
ShellOptions,
15-
SpawnOptions,
16-
} from './types';
9+
import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types';
1710

1811
export class ProcessService extends EventEmitter implements IProcessService {
1912
private processesToKill = new Set<IDisposable>();
2013

21-
constructor(private readonly decoder: IBufferDecoder, private readonly env?: EnvironmentVariables) {
14+
constructor(private readonly env?: EnvironmentVariables) {
2215
super();
2316
}
2417

@@ -47,13 +40,13 @@ export class ProcessService extends EventEmitter implements IProcessService {
4740
}
4841

4942
public execObservable(file: string, args: string[], options: SpawnOptions = {}): ObservableExecutionResult<string> {
50-
const result = execObservable(file, args, options, this.decoder, this.env, this.processesToKill);
43+
const result = execObservable(file, args, options, this.env, this.processesToKill);
5144
this.emit('exec', file, args, options);
5245
return result;
5346
}
5447

5548
public exec(file: string, args: string[], options: SpawnOptions = {}): Promise<ExecutionResult<string>> {
56-
const promise = plainExec(file, args, options, this.decoder, this.env, this.processesToKill);
49+
const promise = plainExec(file, args, options, this.env, this.processesToKill);
5750
this.emit('exec', file, args, options);
5851
return promise;
5952
}

‎src/client/common/process/processFactory.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,18 @@ import { Uri } from 'vscode';
88
import { IDisposableRegistry } from '../types';
99
import { IEnvironmentVariablesProvider } from '../variables/types';
1010
import { ProcessService } from './proc';
11-
import { IBufferDecoder, IProcessLogger, IProcessService, IProcessServiceFactory } from './types';
11+
import { IProcessLogger, IProcessService, IProcessServiceFactory } from './types';
1212

1313
@injectable()
1414
export class ProcessServiceFactory implements IProcessServiceFactory {
1515
constructor(
1616
@inject(IEnvironmentVariablesProvider) private readonly envVarsService: IEnvironmentVariablesProvider,
1717
@inject(IProcessLogger) private readonly processLogger: IProcessLogger,
18-
@inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
1918
@inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry,
2019
) {}
2120
public async create(resource?: Uri): Promise<IProcessService> {
2221
const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource);
23-
const proc: IProcessService = new ProcessService(this.decoder, customEnvVars);
22+
const proc: IProcessService = new ProcessService(customEnvVars);
2423
this.disposableRegistry.push(proc);
2524
return proc.on('exec', this.processLogger.logProcess.bind(this.processLogger));
2625
}

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import { createPythonProcessService } from './pythonProcess';
1515
import {
1616
ExecutionFactoryCreateWithEnvironmentOptions,
1717
ExecutionFactoryCreationOptions,
18-
IBufferDecoder,
1918
IProcessLogger,
2019
IProcessService,
2120
IProcessServiceFactory,
@@ -40,7 +39,6 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
4039
@inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
4140
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
4241
@inject(IConfigurationService) private readonly configService: IConfigurationService,
43-
@inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
4442
@inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
4543
@inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
4644
@inject(IInterpreterPathService) private readonly interpreterPathExpHelper: IInterpreterPathService,
@@ -110,7 +108,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
110108
const pythonPath = options.interpreter
111109
? options.interpreter.path
112110
: this.configService.getSettings(options.resource).pythonPath;
113-
const processService: IProcessService = new ProcessService(this.decoder, { ...envVars });
111+
const processService: IProcessService = new ProcessService({ ...envVars });
114112
processService.on('exec', this.logger.logProcess.bind(this.logger));
115113
this.disposables.push(processService);
116114

‎src/client/common/process/rawProcessApis.ts‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,9 @@ import { IDisposable } from '../types';
88
import { createDeferred } from '../utils/async';
99
import { EnvironmentVariables } from '../variables/types';
1010
import { DEFAULT_ENCODING } from './constants';
11-
import {
12-
ExecutionResult,
13-
IBufferDecoder,
14-
ObservableExecutionResult,
15-
Output,
16-
ShellOptions,
17-
SpawnOptions,
18-
StdErrError,
19-
} from './types';
11+
import { ExecutionResult, ObservableExecutionResult, Output, ShellOptions, SpawnOptions, StdErrError } from './types';
2012
import { noop } from '../utils/misc';
13+
import { decodeBuffer } from './decoder';
2114

2215
function getDefaultOptions<T extends ShellOptions | SpawnOptions>(options: T, defaultEnv?: EnvironmentVariables): T {
2316
const defaultOptions = { ...options };
@@ -90,7 +83,6 @@ export function plainExec(
9083
file: string,
9184
args: string[],
9285
options: SpawnOptions = {},
93-
decoder?: IBufferDecoder,
9486
defaultEnv?: EnvironmentVariables,
9587
disposables?: Set<IDisposable>,
9688
): Promise<ExecutionResult<string>> {
@@ -141,11 +133,11 @@ export function plainExec(
141133
return;
142134
}
143135
const stderr: string | undefined =
144-
stderrBuffers.length === 0 ? undefined : decoder?.decode(stderrBuffers, encoding);
136+
stderrBuffers.length === 0 ? undefined : decodeBuffer(stderrBuffers, encoding);
145137
if (stderr && stderr.length > 0 && options.throwOnStdErr) {
146138
deferred.reject(new StdErrError(stderr));
147139
} else {
148-
let stdout = decoder ? decoder.decode(stdoutBuffers, encoding) : '';
140+
let stdout = decodeBuffer(stdoutBuffers, encoding);
149141
stdout = filterOutputUsingCondaRunMarkers(stdout);
150142
deferred.resolve({ stdout, stderr });
151143
}
@@ -177,7 +169,6 @@ export function execObservable(
177169
file: string,
178170
args: string[],
179171
options: SpawnOptions = {},
180-
decoder?: IBufferDecoder,
181172
defaultEnv?: EnvironmentVariables,
182173
disposables?: Set<IDisposable>,
183174
): ObservableExecutionResult<string> {
@@ -220,7 +211,7 @@ export function execObservable(
220211
}
221212

222213
const sendOutput = (source: 'stdout' | 'stderr', data: Buffer) => {
223-
let out = decoder ? decoder.decode([data], encoding) : '';
214+
let out = decodeBuffer([data], encoding);
224215
if (source === 'stderr' && options.throwOnStdErr) {
225216
subscriber.error(new StdErrError(out));
226217
} else {

‎src/client/common/process/serviceRegistry.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,12 @@
22
// Licensed under the MIT License.
33

44
import { IServiceManager } from '../../ioc/types';
5-
import { BufferDecoder } from './decoder';
65
import { ProcessServiceFactory } from './processFactory';
76
import { PythonExecutionFactory } from './pythonExecutionFactory';
87
import { PythonToolExecutionService } from './pythonToolService';
9-
import { IBufferDecoder, IProcessServiceFactory, IPythonExecutionFactory, IPythonToolExecutionService } from './types';
8+
import { IProcessServiceFactory, IPythonExecutionFactory, IPythonToolExecutionService } from './types';
109

1110
export function registerTypes(serviceManager: IServiceManager) {
12-
serviceManager.addSingleton<IBufferDecoder>(IBufferDecoder, BufferDecoder);
1311
serviceManager.addSingleton<IProcessServiceFactory>(IProcessServiceFactory, ProcessServiceFactory);
1412
serviceManager.addSingleton<IPythonExecutionFactory>(IPythonExecutionFactory, PythonExecutionFactory);
1513
serviceManager.addSingleton<IPythonToolExecutionService>(IPythonToolExecutionService, PythonToolExecutionService);

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ import { PythonExecInfo } from '../../pythonEnvironments/exec';
88
import { InterpreterInformation, PythonEnvironment } from '../../pythonEnvironments/info';
99
import { ExecutionInfo, IDisposable } from '../types';
1010

11-
export const IBufferDecoder = Symbol('IBufferDecoder');
12-
export interface IBufferDecoder {
13-
decode(buffers: Buffer[], encoding: string): string;
14-
}
15-
1611
export type Output<T extends string | Buffer> = {
1712
source: 'stdout' | 'stderr';
1813
out: T;

‎src/test/common.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,9 @@ export function correctPathForOsType(pathToCorrect: string, os?: OSType): string
303303
* @return `SemVer` version of the Python interpreter, or `undefined` if an error occurs.
304304
*/
305305
export async function getPythonSemVer(procService?: IProcessService): Promise<SemVer | undefined> {
306-
const decoder = await import('../client/common/process/decoder');
307306
const proc = await import('../client/common/process/proc');
308307

309-
const pythonProcRunner = procService ? procService : new proc.ProcessService(new decoder.BufferDecoder());
308+
const pythonProcRunner = procService ? procService : new proc.ProcessService();
310309
const pyVerArgs = ['-c', 'import sys;print("{0}.{1}.{2}".format(*sys.version_info[:3]))'];
311310

312311
return pythonProcRunner

‎src/test/common/process/decoder.test.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { expect } from 'chai';
55
import { encode, encodingExists } from 'iconv-lite';
6-
import { BufferDecoder } from '../../../client/common/process/decoder';
6+
import { decodeBuffer } from '../../../client/common/process/decoder';
77
import { initialize } from './../../initialize';
88

99
suite('Decoder', () => {
@@ -13,8 +13,7 @@ suite('Decoder', () => {
1313
test('Test decoding utf8 strings', () => {
1414
const value = 'Sample input string Сделать это';
1515
const buffer = encode(value, 'utf8');
16-
const decoder = new BufferDecoder();
17-
const decodedValue = decoder.decode([buffer]);
16+
const decodedValue = decodeBuffer([buffer]);
1817
expect(decodedValue).equal(value, 'Decoded string is incorrect');
1918
});
2019

@@ -24,11 +23,10 @@ suite('Decoder', () => {
2423
}
2524
const value = 'Sample input string Сделать это';
2625
const buffer = encode(value, 'cp866');
27-
const decoder = new BufferDecoder();
28-
let decodedValue = decoder.decode([buffer]);
26+
let decodedValue = decodeBuffer([buffer]);
2927
expect(decodedValue).not.equal(value, 'Decoded string is the same');
3028

31-
decodedValue = decoder.decode([buffer], 'cp866');
29+
decodedValue = decodeBuffer([buffer], 'cp866');
3230
expect(decodedValue).equal(value, 'Decoded string is incorrect');
3331
});
3432
});

‎src/test/common/process/proc.exec.test.ts‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import { expect, use } from 'chai';
77
import * as chaiAsPromised from 'chai-as-promised';
88
import { CancellationTokenSource } from 'vscode';
9-
import { BufferDecoder } from '../../../client/common/process/decoder';
109
import { ProcessService } from '../../../client/common/process/proc';
1110
import { StdErrError } from '../../../client/common/process/types';
1211
import { OSType } from '../../../client/common/utils/platform';
@@ -26,7 +25,7 @@ suite('ProcessService Observable', () => {
2625
teardown(initialize);
2726

2827
test('exec should output print statements', async () => {
29-
const procService = new ProcessService(new BufferDecoder());
28+
const procService = new ProcessService();
3029
const printOutput = '1234';
3130
const result = await procService.exec(pythonPath, ['-c', `print("${printOutput}")`]);
3231

@@ -42,7 +41,7 @@ suite('ProcessService Observable', () => {
4241
return this.skip();
4342
}
4443

45-
const procService = new ProcessService(new BufferDecoder());
44+
const procService = new ProcessService();
4645
const printOutput = 'öä';
4746
const result = await procService.exec(pythonPath, ['-c', `print("${printOutput}")`]);
4847

@@ -53,7 +52,7 @@ suite('ProcessService Observable', () => {
5352

5453
test('exec should wait for completion of program with new lines', async function () {
5554
this.timeout(5000);
56-
const procService = new ProcessService(new BufferDecoder());
55+
const procService = new ProcessService();
5756
const pythonCode = [
5857
'import sys',
5958
'import time',
@@ -79,7 +78,7 @@ suite('ProcessService Observable', () => {
7978

8079
test('exec should wait for completion of program without new lines', async function () {
8180
this.timeout(5000);
82-
const procService = new ProcessService(new BufferDecoder());
81+
const procService = new ProcessService();
8382
const pythonCode = [
8483
'import sys',
8584
'import time',
@@ -105,7 +104,7 @@ suite('ProcessService Observable', () => {
105104

106105
test('exec should end when cancellationToken is cancelled', async function () {
107106
this.timeout(15000);
108-
const procService = new ProcessService(new BufferDecoder());
107+
const procService = new ProcessService();
109108
const pythonCode = [
110109
'import sys',
111110
'import time',
@@ -133,7 +132,7 @@ suite('ProcessService Observable', () => {
133132

134133
test('exec should stream stdout and stderr separately and filter output using conda related markers', async function () {
135134
this.timeout(7000);
136-
const procService = new ProcessService(new BufferDecoder());
135+
const procService = new ProcessService();
137136
const pythonCode = [
138137
'print(">>>PYTHON-EXEC-OUTPUT")',
139138
'import sys',
@@ -176,7 +175,7 @@ suite('ProcessService Observable', () => {
176175

177176
test('exec should merge stdout and stderr streams', async function () {
178177
this.timeout(7000);
179-
const procService = new ProcessService(new BufferDecoder());
178+
const procService = new ProcessService();
180179
const pythonCode = [
181180
'import sys',
182181
'import time',
@@ -210,29 +209,29 @@ suite('ProcessService Observable', () => {
210209
});
211210

212211
test('exec should throw an error with stderr output', async () => {
213-
const procService = new ProcessService(new BufferDecoder());
212+
const procService = new ProcessService();
214213
const pythonCode = ['import sys', 'sys.stderr.write("a")', 'sys.stderr.flush()'];
215214
const result = procService.exec(pythonPath, ['-c', pythonCode.join(';')], { throwOnStdErr: true });
216215

217216
await expect(result).to.eventually.be.rejectedWith(StdErrError, 'a', 'Expected error to be thrown');
218217
});
219218

220219
test('exec should throw an error when spawn file not found', async () => {
221-
const procService = new ProcessService(new BufferDecoder());
220+
const procService = new ProcessService();
222221
const result = procService.exec(Date.now().toString(), []);
223222

224223
await expect(result).to.eventually.be.rejected.and.to.have.property('code', 'ENOENT', 'Invalid error code');
225224
});
226225

227226
test('exec should exit without no output', async () => {
228-
const procService = new ProcessService(new BufferDecoder());
227+
const procService = new ProcessService();
229228
const result = await procService.exec(pythonPath, ['-c', 'import sys', 'sys.exit()']);
230229

231230
expect(result.stdout).equals('', 'stdout is invalid');
232231
expect(result.stderr).equals(undefined, 'stderr is invalid');
233232
});
234233
test('shellExec should be able to run python and filter output using conda related markers', async () => {
235-
const procService = new ProcessService(new BufferDecoder());
234+
const procService = new ProcessService();
236235
const printOutput = '1234';
237236
const result = await procService.shellExec(
238237
`"${pythonPath}" -c "print('>>>PYTHON-EXEC-OUTPUT');print('${printOutput}');print('<<<PYTHON-EXEC-OUTPUT')"`,
@@ -243,12 +242,12 @@ suite('ProcessService Observable', () => {
243242
expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output');
244243
});
245244
test('shellExec should fail on invalid command', async () => {
246-
const procService = new ProcessService(new BufferDecoder());
245+
const procService = new ProcessService();
247246
const result = procService.shellExec('invalid command');
248247
await expect(result).to.eventually.be.rejectedWith(Error, 'a', 'Expected error to be thrown');
249248
});
250249
test('variables can be changed after the fact', async () => {
251-
const procService = new ProcessService(new BufferDecoder(), process.env);
250+
const procService = new ProcessService(process.env);
252251
let result = await procService.exec(pythonPath, ['-c', `import os;print(os.environ.get("MY_TEST_VARIABLE"))`], {
253252
extraVariables: { MY_TEST_VARIABLE: 'foo' },
254253
});

0 commit comments

Comments
 (0)