Skip to content

Commit

Permalink
fix: can unset network#proxy in the CLI config
Browse files Browse the repository at this point in the history
An empty object (`{}`) must be used to correctly unset the CLI config
value to its default.

Closes #2184
  • Loading branch information
Akos Kitta committed Aug 22, 2023
1 parent 5fd02b9 commit e1b314f
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 1 deletion.
2 changes: 1 addition & 1 deletion arduino-ide-extension/src/node/config-service-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class ConfigServiceImpl
};
copyDefaultCliConfig.locale = locale || 'en';
const proxy = Network.stringify(network);
copyDefaultCliConfig.network = { proxy };
copyDefaultCliConfig.network = proxy ? { proxy } : {}; // must be an empty object to unset the default prop with the `WriteRequest`.

// always use the port of the daemon
const port = await this.daemon.getPort();
Expand Down
116 changes: 116 additions & 0 deletions arduino-ide-extension/src/test/node/config-service-impl.slow-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { DisposableCollection } from '@theia/core/lib/common/disposable';
import { deepClone } from '@theia/core/lib/common/objects';
import type { MaybePromise, Mutable } from '@theia/core/lib/common/types';
import { Container } from '@theia/core/shared/inversify';
import { expect } from 'chai';
import { load as parseYaml } from 'js-yaml';
import { promises as fs } from 'node:fs';
import { Config, Network } from '../../common/protocol/config-service';
import { CLI_CONFIG, DefaultCliConfig } from '../../node/cli-config';
import { ConfigServiceImpl } from '../../node/config-service-impl';
import { ConfigDirUriProvider } from '../../node/theia/env-variables/env-variables-server';
import { createBaseContainer, startDaemon } from './node-test-bindings';

describe('config-service-impl', () => {
describe('setConfiguration', () => {
const manualProxy: Network = {
protocol: 'http',
hostname: 'hostname',
password: 'secret',
username: 'username',
port: '1234',
};
const noProxy: Network = 'none';

let configService: ConfigServiceImpl;
let toDispose: DisposableCollection;
let cliConfigPath: string;

beforeEach(async () => {
const container = await createContainer();
toDispose = new DisposableCollection();
await startDaemon(container, toDispose);
configService = container.get<ConfigServiceImpl>(ConfigServiceImpl);
const configDirUriProvider =
container.get<ConfigDirUriProvider>(ConfigDirUriProvider);
cliConfigPath = configDirUriProvider
.configDirUri()
.resolve(CLI_CONFIG)
.path.fsPath();
});

afterEach(() => toDispose.dispose());

it("should detect 'none' proxy with th default config", async () => {
const state = await configService.getConfiguration();
expect(state.config).to.be.not.undefined;
const config = <Config>state.config;
expect(config.network).to.be.equal(noProxy);
expect(Network.stringify(config.network)).is.undefined;
await assertRawConfigModel((actualModel) => {
expect(actualModel.network).to.be.undefined;
});
});

it('should ignore noop changes', async () => {
const beforeState = await configService.getConfiguration();
const config = <Mutable<Config>>deepClone(beforeState).config;
let eventCounter = 0;
toDispose.push(configService.onConfigChange(() => eventCounter++));
await configService.setConfiguration(config);
const afterState = await configService.getConfiguration();
expect(beforeState.config).to.be.deep.equal(afterState.config);
expect(eventCounter).to.be.equal(0);
});

it('should set the manual proxy', async () => {
const beforeState = await configService.getConfiguration();
const config = <Mutable<Config>>deepClone(beforeState).config;
config.network = manualProxy;
let eventCounter = 0;
toDispose.push(configService.onConfigChange(() => eventCounter++));
await configService.setConfiguration(config);
const afterState = await configService.getConfiguration();
expect(beforeState.config).to.be.not.deep.equal(afterState.config);
expect(afterState.config?.network).to.be.deep.equal(manualProxy);
expect(eventCounter).to.be.equal(1);
await assertRawConfigModel((actualModel) => {
expect(actualModel.network?.proxy).to.be.equal(
Network.stringify(manualProxy)
);
});
});

it('should unset the manual proxy', async () => {
const initialState = await configService.getConfiguration();
const config = <Mutable<Config>>deepClone(initialState).config;
config.network = manualProxy;
let eventCounter = 0;
toDispose.push(configService.onConfigChange(() => eventCounter++));
await configService.setConfiguration(config);
const beforeState = await configService.getConfiguration();
const config2 = <Mutable<Config>>deepClone(config);
config2.network = noProxy;
await configService.setConfiguration(config2);
const afterState = await configService.getConfiguration();
expect(beforeState.config).to.be.not.deep.equal(afterState.config);
expect(afterState.config?.network).to.be.deep.equal(noProxy);
expect(eventCounter).to.be.equal(2);
await assertRawConfigModel((actualModel) => {
expect(actualModel.network?.proxy).to.be.undefined;
});
});

async function createContainer(): Promise<Container> {
return createBaseContainer();
}

async function assertRawConfigModel(
assert: (actual: DefaultCliConfig) => MaybePromise<void>
): Promise<void> {
const raw = await fs.readFile(cliConfigPath, { encoding: 'utf8' });
const model = parseYaml(raw);
await assert(model);
}
});
});

0 comments on commit e1b314f

Please sign in to comment.