Convert config service to be strict compliant

libs/common/spec/fake-state.ts

@@ -91,7 +91,7 @@ export class FakeSingleUserState<T> implements SingleUserState<T> {
   // eslint-disable-next-line rxjs/no-exposed-subjects -- exposed for testing setup
   stateSubject = new ReplaySubject<CombinedState<T>>(1);
 
-  state$: Observable<T>;
+  state$: Observable<T | undefined>;
   combinedState$: Observable<CombinedState<T>>;
 
   constructor(
@@ -104,7 +104,7 @@ export class FakeSingleUserState<T> implements SingleUserState<T> {
     this.state$ = this.combinedState$.pipe(map(([_userId, state]) => state));
   }
 
-  nextState(state: T) {
+  nextState(state: T | undefined) {
     this.stateSubject.next([this.userId, state]);
   }
 

libs/common/src/platform/abstractions/config/config.service.ts

@@ -8,37 +8,37 @@ import { ServerConfig } from "./server-config";
 
 export abstract class ConfigService {
   /** The server config of the currently active user */
-  serverConfig$: Observable<ServerConfig | null>;
+  abstract serverConfig$: Observable<ServerConfig | undefined>;
   /** The cloud region of the currently active user */
-  cloudRegion$: Observable<Region>;
+  abstract cloudRegion$: Observable<Region>;
   /**
    * Retrieves the value of a feature flag for the currently active user
    * @param key The feature flag to retrieve
    * @param defaultValue The default value to return if the feature flag is not set or the server's config is irretrievable
    * @returns An observable that emits the value of the feature flag, updates as the server config changes
    */
-  getFeatureFlag$: <T extends boolean | number | string>(
+  abstract getFeatureFlag$<T extends boolean | number | string>(
     key: FeatureFlag,
     defaultValue?: T,
-  ) => Observable<T>;
+  ): Observable<T | undefined>;
   /**
    * Retrieves the value of a feature flag for the currently active user
    * @param key The feature flag to retrieve
    * @param defaultValue The default value to return if the feature flag is not set or the server's config is irretrievable
    * @returns The value of the feature flag
    */
-  getFeatureFlag: <T extends boolean | number | string>(
+  abstract getFeatureFlag<T extends boolean | number | string>(
     key: FeatureFlag,
     defaultValue?: T,
-  ) => Promise<T>;
+  ): Promise<T | undefined>;
   /**
    * Verifies whether the server version meets the minimum required version
    * @param minimumRequiredServerVersion The minimum version required
    * @returns True if the server version is greater than or equal to the minimum required version
    */
-  checkServerMeetsVersionRequirement$: (
+  abstract checkServerMeetsVersionRequirement$(
     minimumRequiredServerVersion: SemVer,
-  ) => Observable<boolean>;
+  ): Observable<boolean>;
 
   /**
    * Triggers a check that the config for the currently active user is up-to-date. If it is not, it will be fetched from the server and stored.

libs/common/src/platform/abstractions/config/server-config.ts

@@ -25,7 +25,7 @@ export class ServerConfig {
     this.featureStates = serverConfigData.featureStates;
 
     if (this.server?.name == null && this.server?.url == null) {
-      this.server = null;
+      this.server = undefined;
     }
   }
 
@@ -37,9 +37,9 @@ export class ServerConfig {
     return this.getAgeInMilliseconds() <= dayInMilliseconds;
   }
 
-  static fromJSON(obj: Jsonify<ServerConfig>): ServerConfig {
+  static fromJSON(obj: Jsonify<ServerConfig>): ServerConfig | undefined {
     if (obj == null) {
-      return null;
+      return undefined;
     }
 
     return new ServerConfig(obj);

libs/common/src/platform/abstractions/environment.service.ts

@@ -123,7 +123,7 @@ export abstract class EnvironmentService {
    * @param userId - The user id to set the cloud web vault app URL for. If null or undefined the global environment is set.
    * @param region - The region of the cloud web vault app.
    */
-  abstract setCloudRegion(userId: UserId, region: Region): Promise<void>;
+  abstract setCloudRegion(userId: UserId | undefined, region: Region): Promise<void>;
 
   /**
    * Get the environment from state. Useful if you need to get the environment for another user.

libs/common/src/platform/services/config/config.service.spec.ts → libs/common/src/platform/services/config/default-config.service.spec.ts

@@ -61,7 +61,7 @@ describe("ConfigService", () => {
     let sut: DefaultConfigService;
 
     beforeAll(async () => {
-      await accountService.switchAccount(activeUserId);
+      await accountService.switchAccount(activeUserId as any);
     });
 
     beforeEach(() => {
@@ -75,9 +75,9 @@ describe("ConfigService", () => {
     });
 
     describe("serverConfig$", () => {
-      it.each([{}, null])("handles null stored state", async (globalTestState) => {
+      it.each([{} as any, null])("handles null stored state", async (globalTestState) => {
         globalState.stateSubject.next(globalTestState);
-        userState.nextState(null);
+        userState.nextState(undefined);
         await expect(firstValueFrom(sut.serverConfig$)).resolves.not.toThrow();
       });
 
@@ -95,7 +95,7 @@ describe("ConfigService", () => {
 
         beforeEach(() => {
           globalState.stateSubject.next(globalStored);
-          userState.nextState(userStored);
+          userState.nextState(userStored as any);
         });
 
         // sanity check
@@ -147,12 +147,11 @@ describe("ConfigService", () => {
 
             const actual = await firstValueFrom(sut.serverConfig$);
 
-            // This is the time the response is converted to a config
-            expect(actual.utcDate).toAlmostEqual(newConfig.utcDate, 1000);
-            delete actual.utcDate;
-            delete newConfig.utcDate;
-
-            expect(actual).toEqual(newConfig);
+            expect(actual).toEqual({
+              ...newConfig,
+              // This is the time the response is converted to a config
+              utcDate: expect.toAlmostEqual(newConfig.utcDate, 1000),
+            });
           });
         });
       });
@@ -194,7 +193,7 @@ describe("ConfigService", () => {
 
     beforeAll(async () => {
       // updating environment with an active account is undefined behavior
-      await accountService.switchAccount(null);
+      await accountService.switchAccount(null as any);
     });
 
     beforeEach(() => {
@@ -226,15 +225,13 @@ describe("ConfigService", () => {
         const actual = await spy.pauseUntilReceived(2);
         expect(actual.length).toBe(2);
 
-        // validate dates this is done separately because the dates are created when ServerConfig is initialized
-        expect(actual[0].utcDate).toAlmostEqual(expected[0].utcDate, 1000);
-        expect(actual[1].utcDate).toAlmostEqual(expected[1].utcDate, 1000);
-        delete actual[0].utcDate;
-        delete actual[1].utcDate;
-        delete expected[0].utcDate;
-        delete expected[1].utcDate;
-
-        expect(actual).toEqual(expected);
+        expect(actual).toEqual(
+          expected.map((e) => ({
+            ...e,
+            // validate dates this is done separately because the dates are created when ServerConfig is initialized
+            utcDate: expect.toAlmostEqual(e.utcDate, 1000),
+          })),
+        );
         spy.unsubscribe();
       });
     });

libs/common/src/platform/services/config/default-config.service.ts

@@ -28,7 +28,7 @@ export const RETRIEVAL_INTERVAL = 3_600_000; // 1 hour
 export type ApiUrl = string;
 
 export const USER_SERVER_CONFIG = new UserKeyDefinition<ServerConfig>(CONFIG_DISK, "serverConfig", {
-  deserializer: (data) => (data == null ? null : ServerConfig.fromJSON(data)),
+  deserializer: (data) => (data == null ? undefined : ServerConfig.fromJSON(data)),
   clearOn: ["logout"],
 });
 
@@ -37,15 +37,15 @@ export const GLOBAL_SERVER_CONFIGURATIONS = KeyDefinition.record<ServerConfig, A
   CONFIG_DISK,
   "byServer",
   {
-    deserializer: (data) => (data == null ? null : ServerConfig.fromJSON(data)),
+    deserializer: (data) => (data == null ? undefined : ServerConfig.fromJSON(data)),
   },
 );
 
 // FIXME: currently we are limited to api requests for active users. Update to accept a UserId and APIUrl once ApiService supports it.
 export class DefaultConfigService implements ConfigService {
-  private failedFetchFallbackSubject = new Subject<ServerConfig>();
+  private failedFetchFallbackSubject = new Subject<ServerConfig | undefined>();
 
-  serverConfig$: Observable<ServerConfig>;
+  serverConfig$: Observable<ServerConfig | undefined>;
 
   cloudRegion$: Observable<Region>;
 
@@ -69,7 +69,7 @@ export class DefaultConfigService implements ConfigService {
         const [existingConfig, userId, apiUrl] = rec;
         // Grab new config if older retrieval interval
         if (!existingConfig || this.olderThanRetrievalInterval(existingConfig.utcDate)) {
-          await this.renewConfig(existingConfig, userId, apiUrl);
+          await this.renewConfig(apiUrl, existingConfig, userId);
         }
       }),
       switchMap(([existingConfig]) => {
@@ -90,6 +90,7 @@ export class DefaultConfigService implements ConfigService {
       map((config) => config?.environment?.cloudRegion ?? Region.US),
     );
   }
+
   getFeatureFlag$<T extends FeatureFlagValue>(key: FeatureFlag, defaultValue?: T) {
     return this.serverConfig$.pipe(
       map((serverConfig) => {
@@ -129,9 +130,9 @@ export class DefaultConfigService implements ConfigService {
 
   // Updates the on-disk configuration with a newly retrieved configuration
   private async renewConfig(
-    existingConfig: ServerConfig,
-    userId: UserId,
     apiUrl: string,
+    existingConfig?: ServerConfig,
+    userId?: UserId,
   ): Promise<void> {
     try {
       const response = await this.configApiService.get(userId);
@@ -165,13 +166,13 @@ export class DefaultConfigService implements ConfigService {
     }
   }
 
-  private globalConfigFor$(apiUrl: string): Observable<ServerConfig> {
+  private globalConfigFor$(apiUrl: string): Observable<ServerConfig | undefined> {
     return this.stateProvider
       .getGlobal(GLOBAL_SERVER_CONFIGURATIONS)
       .state$.pipe(map((configs) => configs?.[apiUrl]));
   }
 
-  private userConfigFor$(userId: UserId): Observable<ServerConfig> {
+  private userConfigFor$(userId: UserId): Observable<ServerConfig | undefined> {
     return this.stateProvider.getUser(userId, USER_SERVER_CONFIG).state$;
   }
 }

libs/common/src/platform/state/global-state.ts

@@ -26,5 +26,5 @@ export interface GlobalState<T> {
    * An observable stream of this state, the first emission of this will be the current state on disk
    * and subsequent updates will be from an update to that state.
    */
-  state$: Observable<T>;
+  state$: Observable<T | undefined>;
 }

libs/common/src/platform/state/key-definition.ts

@@ -18,7 +18,7 @@ export type KeyDefinitionOptions<T> = {
    * @param jsonValue The JSON object representation of your state.
    * @returns The fully typed version of your state.
    */
-  readonly deserializer: (jsonValue: Jsonify<T>) => T;
+  readonly deserializer: (jsonValue?: Jsonify<T>) => T | undefined;
   /**
    * The number of milliseconds to wait before cleaning up the state after the last subscriber has unsubscribed.
    * Defaults to 1000ms.

libs/common/src/platform/state/user-state.ts

@@ -4,12 +4,12 @@ import { UserId } from "../../types/guid";
 
 import { StateUpdateOptions } from "./state-update-options";
 
-export type CombinedState<T> = readonly [userId: UserId, state: T];
+export type CombinedState<T> = readonly [userId: UserId, state: T | undefined];
 
 /** A helper object for interacting with state that is scoped to a specific user. */
 export interface UserState<T> {
   /** Emits a stream of data. Emits null if the user does not have specified state. */
-  readonly state$: Observable<T | null>;
+  readonly state$: Observable<T | undefined>;
 
   /** Emits a stream of tuples, with the first element being a user id and the second element being the data for that user. */
   readonly combinedState$: Observable<CombinedState<T>>;
@@ -23,7 +23,7 @@ export interface ActiveUserState<T> extends UserState<T> {
    * Emits a stream of data. Emits null if the user does not have specified state.
    * Note: Will not emit if there is no active user.
    */
-  readonly state$: Observable<T | null>;
+  readonly state$: Observable<T | undefined>;
 
   /**
    * Updates backing stores for the active user.