From 4eaad69547a3fe7beac365edc32d21c10fb54465 Mon Sep 17 00:00:00 2001 From: Luca Stocchi <49404737+lstocchi@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:30:00 +0100 Subject: [PATCH] fix: retain autostart setting (#4647) (#4879) Signed-off-by: lstocchi --- .../main/src/plugin/autostart-engine.spec.ts | 39 +++++-------------- packages/main/src/plugin/autostart-engine.ts | 14 +++---- .../main/src/plugin/configuration-impl.ts | 6 +-- .../main/src/plugin/configuration-registry.ts | 12 +++++- 4 files changed, 27 insertions(+), 44 deletions(-) diff --git a/packages/main/src/plugin/autostart-engine.spec.ts b/packages/main/src/plugin/autostart-engine.spec.ts index 601e81c4eee2..138a3a2a21f6 100644 --- a/packages/main/src/plugin/autostart-engine.spec.ts +++ b/packages/main/src/plugin/autostart-engine.spec.ts @@ -50,17 +50,11 @@ beforeAll(() => { providerRegistry.runAutostart = mockRunAutostart; }); -test('Check that default value is false if provider autostart setting is undefined and old global setting is false', async () => { - vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation((section?: string) => { - if (section === `preferences.${extensionId}`) { - return { - get: (_section: string) => undefined, - } as Configuration; - } else { - return { - get: (_section: string) => false, - } as Configuration; - } +test('Check that default value is false if provider autostart setting is false', async () => { + vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { + return { + get: (_section: string, _defaultValue: boolean) => false, + } as Configuration; }); const autoStartConfigurationNode: IConfigurationNode = { @@ -85,23 +79,10 @@ test('Check that default value is false if provider autostart setting is undefin expect(mockRegisterConfiguration).toBeCalledWith([autoStartConfigurationNode]); }); -test('Check that old global setting is not checked if provider autostart setting is already set', async () => { - const getConfigurationMock = vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { - return { - get: (_section: string) => false, - } as Configuration; - }); - - const disposable = autostartEngine.registerProvider(extensionId, extensionDisplayName, 'internalId'); - disposable.dispose(); - expect(getConfigurationMock).toBeCalledTimes(1); - expect(getConfigurationMock).toBeCalledWith(`preferences.${extensionId}`); -}); - -test('Check that default value is true if neither provider autostart setting nor old autostart setting are set', async () => { +test('Check that default value is true if provider autostart setting is not set', async () => { vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { return { - get: (_section: string) => undefined, + get: (_section: string, defaultValue: boolean) => defaultValue, } as Configuration; }); @@ -131,7 +112,7 @@ test('Check that default value is true if neither provider autostart setting nor test('Check that runAutostart is called once if only one provider has registered autostart process', async () => { vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { return { - get: (_section: string) => true, + get: (_section: string, _defaultValue: boolean) => true, } as Configuration; }); @@ -146,7 +127,7 @@ test('Check that runAutostart is called once if only one provider has registered test('Check that runAutostart is never called if only one provider has registered autostart process but its setting is false', async () => { vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { return { - get: (_section: string) => false, + get: (_section: string, _defaultValue: boolean) => false, } as Configuration; }); @@ -161,7 +142,7 @@ test('Check that runAutostart is never called if only one provider has registere test('Check that runAutostart is called twice if only two providers has registered autostart process', async () => { vi.spyOn(configurationRegistry, 'getConfiguration').mockImplementation(() => { return { - get: (_section: string) => true, + get: (_section: string, _defaultValue: boolean) => true, } as Configuration; }); diff --git a/packages/main/src/plugin/autostart-engine.ts b/packages/main/src/plugin/autostart-engine.ts index 4888fc08b361..2646ce7a45e6 100644 --- a/packages/main/src/plugin/autostart-engine.ts +++ b/packages/main/src/plugin/autostart-engine.ts @@ -40,12 +40,7 @@ export class AutostartEngine { private registerProviderConfiguration(extensionId: string, extensionDisplayName: string): IConfigurationNode { const extensionConfiguration = this.configurationRegistry.getConfiguration(`preferences.${extensionId}`); - let autostart = extensionConfiguration.get('engine.autostart'); - // if there is no configuration set, we try to retrieve the value of the old deprecated preferences.engine.autostart setting - if (autostart === undefined) { - const autoStartConfigurationGlobal = this.configurationRegistry.getConfiguration('preferences.engine'); - autostart = autoStartConfigurationGlobal.get('autostart'); - } + const autostart = extensionConfiguration.get('engine.autostart', true); const autoStartConfigurationNode: IConfigurationNode = { id: `preferences.${extensionId}.engine.autostart`, @@ -58,7 +53,7 @@ export class AutostartEngine { [`preferences.${extensionId}.engine.autostart`]: { description: `Autostart ${extensionDisplayName} engine when launching Podman Desktop`, type: 'boolean', - default: autostart !== undefined ? autostart : true, + default: autostart, scope: [CONFIGURATION_DEFAULT_SCOPE, CONFIGURATION_ONBOARDING_SCOPE], }, }, @@ -72,7 +67,8 @@ export class AutostartEngine { this.providerExtension.forEach((extensionId, providerInternalId) => { // grab value const autoStartConfiguration = this.configurationRegistry.getConfiguration(`preferences.${extensionId}`); - const autostart = autoStartConfiguration.get('engine.autostart'); + // if there is no value in the config, we use the default true value + const autostart = autoStartConfiguration.get('engine.autostart', true); if (autostart) { console.log(`Autostarting ${extensionId} container engine`); // send autostart @@ -83,7 +79,7 @@ export class AutostartEngine { // start the engine if we toggle the property this.configurationRegistry.onDidChangeConfiguration(async e => { - if (e.key === `${extensionId}.engine.autostart` && e.value === true) { + if (e.key === `preferences.${extensionId}.engine.autostart` && e.value === true) { await this.providerRegistry.runAutostart(providerInternalId); } }); diff --git a/packages/main/src/plugin/configuration-impl.ts b/packages/main/src/plugin/configuration-impl.ts index a8d8bd917755..e02285ce383b 100644 --- a/packages/main/src/plugin/configuration-impl.ts +++ b/packages/main/src/plugin/configuration-impl.ts @@ -55,12 +55,8 @@ export class ConfigurationImpl implements containerDesktopAPI.Configuration { const localView = this.getLocalView(); if (localView[localKey] !== undefined) { return localView[localKey]; - } else if (defaultValue) { - // not there but we have a default value, then return it - return defaultValue; - } else { - return undefined; } + return defaultValue; } has(section: string): boolean { diff --git a/packages/main/src/plugin/configuration-registry.ts b/packages/main/src/plugin/configuration-registry.ts index 8b8e96d674ae..8323dddf6110 100644 --- a/packages/main/src/plugin/configuration-registry.ts +++ b/packages/main/src/plugin/configuration-registry.ts @@ -173,7 +173,7 @@ export class ConfigurationRegistry implements IConfigurationRegistry { // register default if not yet set if ( configProperty.default && - !configProperty.scope && + this.isDefaultScope(configProperty.scope) && this.configurationValues.get(CONFIGURATION_DEFAULT_SCOPE)[key] === undefined ) { this.configurationValues.get(CONFIGURATION_DEFAULT_SCOPE)[key] = configProperty.default; @@ -191,6 +191,16 @@ export class ConfigurationRegistry implements IConfigurationRegistry { return properties; } + private isDefaultScope(scope?: ConfigurationScope | ConfigurationScope[]): boolean { + if (!scope) { + return true; + } + if (Array.isArray(scope) && scope.find(s => s === CONFIGURATION_DEFAULT_SCOPE)) { + return true; + } + return scope === CONFIGURATION_DEFAULT_SCOPE; + } + public deregisterConfigurations(configurations: IConfigurationNode[]): void { this.doDeregisterConfigurations(configurations, true); }