fix: retain autostart setting (#4647) (#4879)

Signed-off-by: lstocchi <lstocchi@redhat.com>

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;
   });
 

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<boolean>('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<boolean>('autostart');
-    }
+    const autostart = extensionConfiguration.get<boolean>('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<boolean>('engine.autostart');
+      // if there is no value in the config, we use the default true value
+      const autostart = autoStartConfiguration.get<boolean>('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);
         }
       });

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 {

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);
   }