Skip to content

Commit 9cf502e

Browse files
author
Akos Kitta
committed
fix: can unset network#proxy in the CLI config
An empty object (`{}`) must be used to correctly unset the CLI config value to its default. Closes #2184
1 parent f97a484 commit 9cf502e

File tree

2 files changed

+117
-1
lines changed

2 files changed

+117
-1
lines changed

Diff for: arduino-ide-extension/src/node/config-service-impl.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export class ConfigServiceImpl
9797
};
9898
copyDefaultCliConfig.locale = locale || 'en';
9999
const proxy = Network.stringify(network);
100-
copyDefaultCliConfig.network = { proxy };
100+
copyDefaultCliConfig.network = proxy ? { proxy } : {}; // must be an empty object to unset the default prop with the `WriteRequest`.
101101

102102
// always use the port of the daemon
103103
const port = await this.daemon.getPort();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { DisposableCollection } from '@theia/core/lib/common/disposable';
2+
import { deepClone } from '@theia/core/lib/common/objects';
3+
import type { MaybePromise, Mutable } from '@theia/core/lib/common/types';
4+
import { Container } from '@theia/core/shared/inversify';
5+
import { expect } from 'chai';
6+
import { load as parseYaml } from 'js-yaml';
7+
import { promises as fs } from 'node:fs';
8+
import { Config, Network } from '../../common/protocol/config-service';
9+
import { CLI_CONFIG, DefaultCliConfig } from '../../node/cli-config';
10+
import { ConfigServiceImpl } from '../../node/config-service-impl';
11+
import { ConfigDirUriProvider } from '../../node/theia/env-variables/env-variables-server';
12+
import { createBaseContainer, startDaemon } from './node-test-bindings';
13+
14+
describe('config-service-impl', () => {
15+
describe('setConfiguration', () => {
16+
const manualProxy: Network = {
17+
protocol: 'http',
18+
hostname: 'hostname',
19+
password: 'secret',
20+
username: 'username',
21+
port: '1234',
22+
};
23+
const noProxy: Network = 'none';
24+
25+
let configService: ConfigServiceImpl;
26+
let toDispose: DisposableCollection;
27+
let cliConfigPath: string;
28+
29+
beforeEach(async () => {
30+
const container = await createContainer();
31+
toDispose = new DisposableCollection();
32+
await startDaemon(container, toDispose);
33+
configService = container.get<ConfigServiceImpl>(ConfigServiceImpl);
34+
const configDirUriProvider =
35+
container.get<ConfigDirUriProvider>(ConfigDirUriProvider);
36+
cliConfigPath = configDirUriProvider
37+
.configDirUri()
38+
.resolve(CLI_CONFIG)
39+
.path.fsPath();
40+
});
41+
42+
afterEach(() => toDispose.dispose());
43+
44+
it("should detect 'none' proxy with th default config", async () => {
45+
const state = await configService.getConfiguration();
46+
expect(state.config).to.be.not.undefined;
47+
const config = <Config>state.config;
48+
expect(config.network).to.be.equal(noProxy);
49+
expect(Network.stringify(config.network)).is.undefined;
50+
await assertRawConfigModel((actualModel) => {
51+
expect(actualModel.network).to.be.undefined;
52+
});
53+
});
54+
55+
it('should ignore noop changes', async () => {
56+
const beforeState = await configService.getConfiguration();
57+
const config = <Mutable<Config>>deepClone(beforeState).config;
58+
let eventCounter = 0;
59+
toDispose.push(configService.onConfigChange(() => eventCounter++));
60+
await configService.setConfiguration(config);
61+
const afterState = await configService.getConfiguration();
62+
expect(beforeState.config).to.be.deep.equal(afterState.config);
63+
expect(eventCounter).to.be.equal(0);
64+
});
65+
66+
it('should set the manual proxy', async () => {
67+
const beforeState = await configService.getConfiguration();
68+
const config = <Mutable<Config>>deepClone(beforeState).config;
69+
config.network = manualProxy;
70+
let eventCounter = 0;
71+
toDispose.push(configService.onConfigChange(() => eventCounter++));
72+
await configService.setConfiguration(config);
73+
const afterState = await configService.getConfiguration();
74+
expect(beforeState.config).to.be.not.deep.equal(afterState.config);
75+
expect(afterState.config?.network).to.be.deep.equal(manualProxy);
76+
expect(eventCounter).to.be.equal(1);
77+
await assertRawConfigModel((actualModel) => {
78+
expect(actualModel.network?.proxy).to.be.equal(
79+
Network.stringify(manualProxy)
80+
);
81+
});
82+
});
83+
84+
it('should unset the manual proxy', async () => {
85+
const initialState = await configService.getConfiguration();
86+
const config = <Mutable<Config>>deepClone(initialState).config;
87+
config.network = manualProxy;
88+
let eventCounter = 0;
89+
toDispose.push(configService.onConfigChange(() => eventCounter++));
90+
await configService.setConfiguration(config);
91+
const beforeState = await configService.getConfiguration();
92+
const config2 = <Mutable<Config>>deepClone(config);
93+
config2.network = noProxy;
94+
await configService.setConfiguration(config2);
95+
const afterState = await configService.getConfiguration();
96+
expect(beforeState.config).to.be.not.deep.equal(afterState.config);
97+
expect(afterState.config?.network).to.be.deep.equal(noProxy);
98+
expect(eventCounter).to.be.equal(2);
99+
await assertRawConfigModel((actualModel) => {
100+
expect(actualModel.network?.proxy).to.be.undefined;
101+
});
102+
});
103+
104+
async function createContainer(): Promise<Container> {
105+
return createBaseContainer();
106+
}
107+
108+
async function assertRawConfigModel(
109+
assert: (actual: DefaultCliConfig) => MaybePromise<void>
110+
): Promise<void> {
111+
const raw = await fs.readFile(cliConfigPath, { encoding: 'utf8' });
112+
const model = parseYaml(raw);
113+
await assert(model);
114+
}
115+
});
116+
});

0 commit comments

Comments
 (0)