Ensure keys whose default value type is an array are not duplicated (#17627) (#17634)

Author: Kartik Raj

src/client/common/persistentState.ts

@@ -53,13 +53,13 @@ export class PersistentState<T> implements IPersistentState<T> {
     }
 }
 
-const GLOBAL_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_GLOBAL_STORAGE_KEYS';
-const WORKSPACE_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_WORKSPACE_STORAGE_KEYS';
+export const GLOBAL_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_GLOBAL_STORAGE_KEYS';
+export const WORKSPACE_PERSISTENT_KEYS_DEPRECATED = 'PYTHON_EXTENSION_WORKSPACE_STORAGE_KEYS';
 
 const GLOBAL_PERSISTENT_KEYS = 'PYTHON_GLOBAL_STORAGE_KEYS';
 const WORKSPACE_PERSISTENT_KEYS = 'PYTHON_WORKSPACE_STORAGE_KEYS';
 type KeysStorageType = 'global' | 'workspace';
-type KeysStorage = { key: string; defaultValue: unknown };
+export type KeysStorage = { key: string; defaultValue: unknown };
 
 @injectable()
 export class PersistentStateFactory implements IPersistentStateFactory, IExtensionSingleActivationService {
@@ -122,7 +122,7 @@ export class PersistentStateFactory implements IPersistentStateFactory, IExtensi
     @cache(-1, true)
     private async addKeyToStorage<T>(keyStorageType: KeysStorageType, key: string, defaultValue?: T) {
         const storage = keyStorageType === 'global' ? this._globalKeysStorage : this._workspaceKeysStorage;
-        const found = storage.value.find((value) => value.key === key && value.defaultValue === defaultValue);
+        const found = storage.value.find((value) => value.key === key);
         if (!found) {
             await storage.updateValue([{ key, defaultValue }, ...storage.value]);
         }

src/test/common/persistentState.unit.test.ts

@@ -8,7 +8,12 @@ import * as TypeMoq from 'typemoq';
 import { Memento } from 'vscode';
 import { ICommandManager } from '../../client/common/application/types';
 import { Commands } from '../../client/common/constants';
-import { PersistentStateFactory } from '../../client/common/persistentState';
+import {
+    GLOBAL_PERSISTENT_KEYS_DEPRECATED,
+    KeysStorage,
+    PersistentStateFactory,
+    WORKSPACE_PERSISTENT_KEYS_DEPRECATED,
+} from '../../client/common/persistentState';
 import { IDisposable } from '../../client/common/types';
 import { sleep } from '../core';
 import { MockMemento } from '../mocks/mementos';
@@ -91,9 +96,9 @@ suite('Persistent State', () => {
     test('Ensure internal global storage extension uses to track other storages does not contain duplicate entries', async () => {
         persistentStateFactory.createGlobalPersistentState('key1');
         await sleep(1);
-        persistentStateFactory.createGlobalPersistentState('key2', 'defaultValue1');
+        persistentStateFactory.createGlobalPersistentState('key2', ['defaultValue1']); // Default value type is an array
         await sleep(1);
-        persistentStateFactory.createGlobalPersistentState('key2', 'defaultValue1');
+        persistentStateFactory.createGlobalPersistentState('key2', ['defaultValue1']);
         await sleep(1);
         persistentStateFactory.createGlobalPersistentState('key1');
         await sleep(1);
@@ -102,13 +107,13 @@ suite('Persistent State', () => {
             value.sort((k1, k2) => k1.key.localeCompare(k2.key)),
             [
                 { key: 'key1', defaultValue: undefined },
-                { key: 'key2', defaultValue: 'defaultValue1' },
+                { key: 'key2', defaultValue: ['defaultValue1'] },
             ].sort((k1, k2) => k1.key.localeCompare(k2.key)),
         );
     });
 
     test('Ensure internal workspace storage extension uses to track other storages does not contain duplicate entries', async () => {
-        persistentStateFactory.createWorkspacePersistentState('key2', 'defaultValue1');
+        persistentStateFactory.createWorkspacePersistentState('key2', 'defaultValue1'); // Default value type is a string
         await sleep(1);
         persistentStateFactory.createWorkspacePersistentState('key1');
         await sleep(1);
@@ -125,4 +130,38 @@ suite('Persistent State', () => {
             ].sort((k1, k2) => k1.key.localeCompare(k2.key)),
         );
     });
+
+    test('Ensure deprecated global storage extension used to track other storages with is reset', async () => {
+        const global = persistentStateFactory.createGlobalPersistentState<KeysStorage[]>(
+            GLOBAL_PERSISTENT_KEYS_DEPRECATED,
+        );
+        await global.updateValue([
+            { key: 'oldKey', defaultValue: [] },
+            { key: 'oldKey2', defaultValue: [{}] },
+            { key: 'oldKey3', defaultValue: ['1', '2', '3'] },
+        ]);
+        expect(global.value.length).to.equal(3);
+
+        await persistentStateFactory.activate();
+        await sleep(1);
+
+        expect(global.value.length).to.equal(0);
+    });
+
+    test('Ensure deprecated global storage extension used to track other storages with is reset', async () => {
+        const workspace = persistentStateFactory.createWorkspacePersistentState<KeysStorage[]>(
+            WORKSPACE_PERSISTENT_KEYS_DEPRECATED,
+        );
+        await workspace.updateValue([
+            { key: 'oldKey', defaultValue: [] },
+            { key: 'oldKey2', defaultValue: [{}] },
+            { key: 'oldKey3', defaultValue: ['1', '2', '3'] },
+        ]);
+        expect(workspace.value.length).to.equal(3);
+
+        await persistentStateFactory.activate();
+        await sleep(1);
+
+        expect(workspace.value.length).to.equal(0);
+    });
 });