fix(compass-preference-model): account for all the preferences for the implementation of ReadOnlyPreferenceService (#5356)

* fix: fixed the simple preference service implementation

* fix: use ref for simple service

* chore: moved preference schema definitions to a different file to avoid compass specific imports in provider

* chore: review fixup

* chore: further fixes to imports

* chore: fix lost type assertion

* chore: fix the import for our service

* chore: fix testcases by incorporating the derived values in preference defaults

* chore: further adaptation to read only preference service

* chore: provide an implementation for CreateSandbox

* chore: further refactor, remove getDefaultPreferences duplication

* chore: decouple preferences and storage and use them separately

* chore: import z from zod

* chore: split preferences and schema definitions

Author: himanshusinghs

packages/compass-preferences-model/src/index.ts

@@ -8,7 +8,8 @@ import type {
   PreferenceStateInformation,
   AllPreferences,
 } from './preferences-schema';
-import type { Preferences } from './preferences';
+import type { Preferences, PreferencesAccess } from './preferences';
+
 export type {
   UserPreferences,
   UserConfigurablePreferences,
@@ -33,22 +34,7 @@ export {
   isAIFeatureEnabled,
 } from './utils';
 export type { User, UserStorage } from './storage';
-
-export interface PreferencesAccess {
-  savePreferences(
-    attributes: Partial<UserPreferences>
-  ): Promise<AllPreferences>;
-  refreshPreferences(): Promise<AllPreferences>;
-  getPreferences(): AllPreferences;
-  ensureDefaultConfigurableUserPreferences(): Promise<void>;
-  getConfigurableUserPreferences(): Promise<UserConfigurablePreferences>;
-  getPreferenceStates(): Promise<PreferenceStateInformation>;
-  onPreferenceValueChanged<K extends keyof AllPreferences>(
-    preferenceName: K,
-    callback: (value: AllPreferences[K]) => void
-  ): () => void;
-  createSandbox(): Promise<PreferencesAccess>;
-}
+export type { PreferencesAccess };
 export { setupPreferences };
 export const defaultPreferencesInstance: PreferencesAccess =
   preferencesIpc ?? preferencesMain;

packages/compass-preferences-model/src/preferences-schema.ts

@@ -172,6 +172,12 @@ export type PreferenceStateInformation = Partial<
   Record<keyof AllPreferences, PreferenceState>
 >;
 
+export type StoredPreferencesValidator = ReturnType<
+  typeof getPreferencesValidator
+>;
+
+export type StoredPreferences = z.output<StoredPreferencesValidator>;
+
 // Preference definitions
 const featureFlagsProps: Required<{
   [K in keyof FeatureFlags]: PreferenceDefinition<K>;
@@ -862,6 +868,76 @@ function featureFlagToPreferenceDefinition(
   };
 }
 
+export function makeComputePreferencesValuesAndStates(
+  values: AllPreferences,
+  states: Partial<Record<string, PreferenceState>> = {}
+) {
+  return function _computePreferenceValuesAndStates() {
+    const originalValues = { ...values };
+    const originalStates = { ...states };
+
+    function deriveValue<K extends keyof AllPreferences>(
+      key: K
+    ): {
+      value: AllPreferences[K];
+      state: PreferenceState;
+    } {
+      const descriptor = allPreferencesProps[key];
+      if (!descriptor.deriveValue) {
+        return { value: originalValues[key], state: originalStates[key] };
+      }
+      return (descriptor.deriveValue as DeriveValueFunction<AllPreferences[K]>)(
+        // `as unknown` to work around TS bug(?) https://twitter.com/addaleax/status/1572191664252551169
+        (k) =>
+          (k as unknown) === key ? originalValues[k] : deriveValue(k).value,
+        (k) =>
+          (k as unknown) === key ? originalStates[k] : deriveValue(k).state
+      );
+    }
+
+    for (const key of Object.keys(allPreferencesProps)) {
+      // awkward IIFE to make typescript understand that `key` is the *same* key
+      // in each loop iteration
+      (<K extends keyof AllPreferences>(key: K) => {
+        const result = deriveValue(key);
+        values[key] = result.value;
+        if (result.state !== undefined) states[key] = result.state;
+      })(key as keyof AllPreferences);
+    }
+
+    return { values, states };
+  };
+}
+
+export function getPreferencesValidator() {
+  const preferencesPropsValidator = Object.fromEntries(
+    Object.entries(storedUserPreferencesProps).map(([key, { validator }]) => [
+      key,
+      validator,
+    ])
+  ) as {
+    [K in keyof typeof storedUserPreferencesProps]: typeof storedUserPreferencesProps[K]['validator'];
+  };
+
+  return z.object(preferencesPropsValidator);
+}
+
+export function getDefaultsForStoredPreferences(): StoredPreferences {
+  return Object.fromEntries(
+    Object.entries(storedUserPreferencesProps)
+      .map(([key, value]) => [key, value.validator.parse(undefined)])
+      .filter(([, value]) => value !== undefined)
+  );
+}
+
+export function getInitialValuesForStoredPreferences() {
+  const computeValuesAndStates = makeComputePreferencesValuesAndStates(
+    getDefaultsForStoredPreferences()
+  );
+
+  return computeValuesAndStates().values;
+}
+
 export function getSettingDescription<
   Name extends Exclude<keyof AllPreferences, keyof InternalUserPreferences>
 >(

packages/compass-preferences-model/src/preferences.spec.ts

@@ -4,6 +4,8 @@ import os from 'os';
 import { Preferences } from './preferences';
 import { expect } from 'chai';
 import { featureFlags } from './feature-flags';
+import { PersistentStorage } from './storage';
+import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging';
 
 const releasedFeatureFlags = Object.entries(featureFlags)
   .filter(([, v]) => v.stage === 'released')
@@ -13,10 +15,20 @@ const expectedReleasedFeatureFlagsStates = Object.fromEntries(
   releasedFeatureFlags.map((ff) => [ff, 'hardcoded'])
 );
 
+const logger = createLoggerAndTelemetry('COMPASS-PREFERENCES');
+
 const setupPreferences = async (
-  ...args: ConstructorParameters<typeof Preferences>
+  basePath: string,
+  globalPreferences?: ConstructorParameters<
+    typeof Preferences
+  >[0]['globalPreferences']
 ) => {
-  const preferences = new Preferences(...args);
+  const preferencesStorage = new PersistentStorage(basePath);
+  const preferences = new Preferences({
+    preferencesStorage,
+    globalPreferences,
+    logger,
+  });
   await preferences.setupStorage();
   return preferences;
 };
@@ -270,7 +282,8 @@ describe('Preferences class', function () {
     });
     await mainPreferences.savePreferences({ trackUsageStatistics: true });
     const sandbox = await Preferences.CreateSandbox(
-      await mainPreferences.getPreferenceSandboxProperties()
+      await mainPreferences.getPreferenceSandboxProperties(),
+      logger
     );
 
     await sandbox.savePreferences({ readOnly: true });

packages/compass-preferences-model/src/preferences.ts

@@ -1,22 +1,35 @@
-import type { ParsedGlobalPreferencesResult } from './global-config';
-import {
-  SandboxPreferences,
-  StoragePreferences,
-  type BasePreferencesStorage,
-} from './storage';
-import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging';
 import { z } from 'zod';
+import { type LoggerAndTelemetry } from '@mongodb-js/compass-logging';
+
+import type { ParsedGlobalPreferencesResult } from './global-config';
+import type {
+  AllPreferences,
+  PreferenceState,
+  PreferenceStateInformation,
+  UserConfigurablePreferences,
+  UserPreferences,
+} from './preferences-schema';
 import {
-  type AllPreferences,
-  type PreferenceState,
-  type UserPreferences,
-  type DeriveValueFunction,
-  type UserConfigurablePreferences,
-  type PreferenceStateInformation,
   allPreferencesProps,
+  makeComputePreferencesValuesAndStates,
 } from './preferences-schema';
+import { InMemoryStorage, type BasePreferencesStorage } from './storage';
 
-const { log, mongoLogId } = createLoggerAndTelemetry('COMPASS-PREFERENCES');
+export interface PreferencesAccess {
+  savePreferences(
+    attributes: Partial<UserPreferences>
+  ): Promise<AllPreferences>;
+  refreshPreferences(): Promise<AllPreferences>;
+  getPreferences(): AllPreferences;
+  ensureDefaultConfigurableUserPreferences(): Promise<void>;
+  getConfigurableUserPreferences(): Promise<UserConfigurablePreferences>;
+  getPreferenceStates(): Promise<PreferenceStateInformation>;
+  onPreferenceValueChanged<K extends keyof AllPreferences>(
+    preferenceName: K,
+    callback: (value: AllPreferences[K]) => void
+  ): () => void;
+  createSandbox(): Promise<PreferencesAccess>;
+}
 
 type OnPreferencesChangedCallback = (
   changedPreferencesValues: Partial<AllPreferences>
@@ -30,6 +43,7 @@ type PreferenceSandboxPropertiesImpl = {
 };
 
 export class Preferences {
+  private _logger: LoggerAndTelemetry;
   private _onPreferencesChangedCallbacks: OnPreferencesChangedCallback[];
   private _preferencesStorage: BasePreferencesStorage;
   private _globalPreferences: {
@@ -38,14 +52,17 @@ export class Preferences {
     hardcoded: Partial<AllPreferences>;
   };
 
-  constructor(
-    basepath?: string,
-    globalPreferences?: Partial<ParsedGlobalPreferencesResult>,
-    isSandbox?: boolean
-  ) {
-    this._preferencesStorage = isSandbox
-      ? new SandboxPreferences()
-      : new StoragePreferences(basepath);
+  constructor({
+    logger,
+    globalPreferences,
+    preferencesStorage = new InMemoryStorage(),
+  }: {
+    logger: LoggerAndTelemetry;
+    preferencesStorage: BasePreferencesStorage;
+    globalPreferences?: Partial<ParsedGlobalPreferencesResult>;
+  }) {
+    this._logger = logger;
+    this._preferencesStorage = preferencesStorage;
 
     this._onPreferencesChangedCallbacks = [];
     this._globalPreferences = {
@@ -56,8 +73,8 @@ export class Preferences {
     };
 
     if (Object.keys(this._globalPreferences.hardcoded).length > 0) {
-      log.info(
-        mongoLogId(1_001_000_159),
+      this._logger.log.info(
+        this._logger.mongoLogId(1_001_000_159),
         'preferences',
         'Created Preferences object with hardcoded options',
         { options: this._globalPreferences.hardcoded }
@@ -80,12 +97,17 @@ export class Preferences {
 
   // Create a
   static async CreateSandbox(
-    props: PreferenceSandboxProperties | undefined
+    props: PreferenceSandboxProperties | undefined,
+    logger: LoggerAndTelemetry
   ): Promise<Preferences> {
     const { user, global } = props
       ? (JSON.parse(props) as PreferenceSandboxPropertiesImpl)
       : { user: {}, global: {} };
-    const instance = new Preferences(undefined, global, true);
+    const instance = new Preferences({
+      logger,
+      globalPreferences: global,
+      preferencesStorage: new InMemoryStorage(),
+    });
     await instance.savePreferences(user);
     return instance;
   }
@@ -114,8 +136,8 @@ export class Preferences {
     try {
       await this._preferencesStorage.updatePreferences(attributes);
     } catch (err) {
-      log.error(
-        mongoLogId(1_001_000_157),
+      this._logger.log.error(
+        this._logger.mongoLogId(1_001_000_157),
         'preferences',
         'Failed to save preferences, error while saving models',
         {
@@ -185,39 +207,12 @@ export class Preferences {
     for (const key of Object.keys(this._globalPreferences.hardcoded))
       states[key] = 'hardcoded';
 
-    const originalValues = { ...values };
-    const originalStates = { ...states };
-
-    function deriveValue<K extends keyof AllPreferences>(
-      key: K
-    ): {
-      value: AllPreferences[K];
-      state: PreferenceState;
-    } {
-      const descriptor = allPreferencesProps[key];
-      if (!descriptor.deriveValue) {
-        return { value: originalValues[key], state: originalStates[key] };
-      }
-      return (descriptor.deriveValue as DeriveValueFunction<AllPreferences[K]>)(
-        // `as unknown` to work around TS bug(?) https://twitter.com/addaleax/status/1572191664252551169
-        (k) =>
-          (k as unknown) === key ? originalValues[k] : deriveValue(k).value,
-        (k) =>
-          (k as unknown) === key ? originalStates[k] : deriveValue(k).state
-      );
-    }
-
-    for (const key of Object.keys(allPreferencesProps)) {
-      // awkward IIFE to make typescript understand that `key` is the *same* key
-      // in each loop iteration
-      (<K extends keyof AllPreferences>(key: K) => {
-        const result = deriveValue(key);
-        values[key] = result.value;
-        if (result.state !== undefined) states[key] = result.state;
-      })(key as keyof AllPreferences);
-    }
+    const computeValuesAndStates = makeComputePreferencesValuesAndStates(
+      values,
+      states
+    );
 
-    return { values, states };
+    return computeValuesAndStates();
   }
 
   /**