feat: Emit ClientReady only after ConfigCat emitted it

Signed-off-by: Lukas Reining <[email protected]>
open-feature · Jun 21, 2023 · 1cabbb1 · 1cabbb1
1 parent 3fec8cc
commit 1cabbb1
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 70 deletions.
diff --git a/libs/providers/config-cat/README.md b/libs/providers/config-cat/README.md
@@ -11,18 +11,21 @@ $ npm install @openfeature/config-cat-provider
 #### Required peer dependencies
 
 The OpenFeature SDK is required as peer dependency.
+
 The minimum required version of `@openfeature/js-sdk` currently is `1.3.0`.
 
+The minimum required version of `configcat-js` currently is `7.0.0`.
+
 ```
-$ npm install @openfeature/js-sdk
+$ npm install @openfeature/js-sdk configcat-js
 ```
 
 ## Usage
 
 The ConfigCat provider uses the [ConfigCat Javascript SDK](https://configcat.com/docs/sdk-reference/js/).
 
-It can either be created by passing the ConfigCat SDK options to ```ConfigCatProvider.create``` or injecting a ConfigCat
-SDK client into ```ConfigCatProvider.createFromClient```.
+It can either be created by passing the ConfigCat SDK options to ```ConfigCatProvider.create``` or
+the ```ConfigCatProvider``` constructor.
 
 The available options can be found in the [ConfigCat Javascript SDK docs](https://configcat.com/docs/sdk-reference/js/).
 
@@ -47,18 +50,6 @@ const provider = ConfigCatProvider.create('<sdk_key>', PollingMode.LazyLoad, {
 OpenFeature.setProvider(provider);
 ```
 
-### Example injecting a client
-
-```javascript
-import { ConfigCatProvider } from '@openfeature/config-cat-provider';
-import * as configcat from 'configcat-js';
-
-const configCatClient = configcat.getClient("<sdk_key>")
-const provider = ConfigCatProvider.createFromClient(configCatClient);
-
-OpenFeature.setProvider(provider);
-```
-
 ## Evaluation Context
 
 ConfigCat only supports string values in its "evaluation

diff --git a/libs/providers/config-cat/src/lib/config-cat-provider.spec.ts b/libs/providers/config-cat/src/lib/config-cat-provider.spec.ts
@@ -1,28 +1,21 @@
 import { ConfigCatProvider } from './config-cat-provider';
-import { ParseError, ProviderEvents, TypeMismatchError } from '@openfeature/js-sdk';
+import { OpenFeature, ParseError, ProviderEvents, TypeMismatchError } from '@openfeature/js-sdk';
 import {
   createConsoleLogger,
   createFlagOverridesFromMap,
+  HookEvents,
   IConfigCatClient,
   LogLevel,
   OverrideBehaviour,
   PollingMode,
-  HookEvents,
   ProjectConfig,
-  getClient,
 } from 'configcat-js';
 
-import * as configcatcommon from 'configcat-common';
-import { HttpConfigFetcher } from 'configcat-js/lib/ConfigFetcher';
-import CONFIGCAT_SDK_VERSION from 'configcat-js/lib/Version';
 import { IEventEmitter } from 'configcat-common/lib/EventEmitter';
 
-import { EventEmitter } from 'events';
-
 describe('ConfigCatProvider', () => {
   const targetingKey = 'abc';
 
-  let client: IConfigCatClient;
   let provider: ConfigCatProvider;
   let configCatEmitter: IEventEmitter<HookEvents>;
 
@@ -38,46 +31,40 @@ describe('ConfigCatProvider', () => {
   };
 
   beforeAll(async () => {
-    client = configcatcommon.getClient(
-      '__key__',
-      PollingMode.AutoPoll,
-      {
-        logger: createConsoleLogger(LogLevel.Off),
-        offline: true,
-        flagOverrides: createFlagOverridesFromMap(values, OverrideBehaviour.LocalOnly),
-      },
-      {
-        configFetcher: new HttpConfigFetcher(),
-        sdkType: 'ConfigCat-JS',
-        sdkVersion: CONFIGCAT_SDK_VERSION,
-        eventEmitterFactory() {
-          configCatEmitter = new EventEmitter() as IEventEmitter<HookEvents>;
-          return configCatEmitter;
-        },
-      }
-    );
-
-    provider = ConfigCatProvider.createFromClient(client);
-    await provider.initialize()
+    provider = ConfigCatProvider.create('__key__', PollingMode.ManualPoll, {
+      logger: createConsoleLogger(LogLevel.Off),
+      offline: true,
+      flagOverrides: createFlagOverridesFromMap(values, OverrideBehaviour.LocalOnly),
+    });
+
+    await provider.initialize();
+
+    // Currently there is no option to get access to the event emitter
+    configCatEmitter = (provider.configCatClient as any).options.hooks;
   });
 
   afterAll(() => {
-    client.dispose();
+    provider.onClose();
   });
 
   it('should be an instance of ConfigCatProvider', () => {
     expect(provider).toBeInstanceOf(ConfigCatProvider);
   });
 
   it('should dispose the configcat client on provider closing', async () => {
-    const newClient = getClient('__another_key__', PollingMode.AutoPoll, {
+    const newProvider = ConfigCatProvider.create('__another_key__', PollingMode.AutoPoll, {
       logger: createConsoleLogger(LogLevel.Off),
       offline: true,
       flagOverrides: createFlagOverridesFromMap(values, OverrideBehaviour.LocalOnly),
     });
 
-    const clientDisposeSpy = jest.spyOn(newClient, 'dispose');
-    const newProvider = ConfigCatProvider.createFromClient(newClient);
+    await newProvider.initialize();
+
+    if (!newProvider.configCatClient) {
+      throw Error('No ConfigCat client');
+    }
+
+    const clientDisposeSpy = jest.spyOn(newProvider.configCatClient, 'dispose');
     await newProvider.onClose();
 
     expect(clientDisposeSpy).toHaveBeenCalled();
@@ -86,9 +73,21 @@ describe('ConfigCatProvider', () => {
   describe('events', () => {
     it('should emit PROVIDER_READY event', () => {
       const handler = jest.fn();
-
       provider.events.addHandler(ProviderEvents.Ready, handler);
       configCatEmitter.emit('clientReady');
+      expect(handler).toHaveBeenCalled();
+    });
+
+    it('should emit PROVIDER_READY event on initialization', async () => {
+      const newProvider = ConfigCatProvider.create('__another_key__', PollingMode.ManualPoll, {
+        logger: createConsoleLogger(LogLevel.Off),
+        offline: true,
+        flagOverrides: createFlagOverridesFromMap(values, OverrideBehaviour.LocalOnly),
+      });
+
+      const handler = jest.fn();
+      newProvider.events.addHandler(ProviderEvents.Ready, handler);
+      await newProvider.initialize();
 
       expect(handler).toHaveBeenCalled();
     });

diff --git a/libs/providers/config-cat/src/lib/config-cat-provider.ts b/libs/providers/config-cat/src/lib/config-cat-provider.ts
@@ -1,5 +1,6 @@
 import {
   EvaluationContext,
+  GeneralError,
   JsonValue,
   OpenFeatureEventEmitter,
   ParseError,
@@ -14,47 +15,74 @@ import { getClient, IConfigCatClient, IEvaluationDetails, SettingValue } from 'c
 import { transformContext } from './context-transformer';
 
 export class ConfigCatProvider implements Provider {
-  private client: IConfigCatClient;
-  public events = new OpenFeatureEventEmitter();
+  private readonly clientParameters: Parameters<typeof getClient>;
+  public readonly events = new OpenFeatureEventEmitter();
+  private client?: IConfigCatClient;
 
   public metadata = {
     name: ConfigCatProvider.name,
   };
 
-  private constructor(client: IConfigCatClient) {
-    this.client = client;
+  constructor(...params: Parameters<typeof getClient>) {
+    this.clientParameters = params;
   }
 
-  public async initialize(context?: EvaluationContext): Promise<void> {
-    this.client.on('clientReady', () => this.events.emit(ProviderEvents.Ready));
-    this.client.on('configChanged', (projectConfig) =>
-      this.events.emit(ProviderEvents.ConfigurationChanged, { metadata: { ...projectConfig } })
-    );
-    this.client.on('clientError', (message: string, error) =>
-      this.events.emit(ProviderEvents.Error, {
-        message: message,
-        metadata: error,
-      })
-    );
+  public static create(...params: Parameters<typeof getClient>) {
+    return new ConfigCatProvider(...params);
   }
 
-  public static create(...params: Parameters<typeof getClient>) {
-    return new ConfigCatProvider(getClient(...params));
+  public async initialize(): Promise<void> {
+    return new Promise((resolve) => {
+      const originalParameters = this.clientParameters;
+      const options = originalParameters[2];
+
+      if (options) {
+        const oldSetupHooks = options.setupHooks;
+
+        options.setupHooks = (hooks) => {
+          oldSetupHooks?.(hooks);
+
+          // After resolving, once, we can simply emit events the next time
+          hooks.once('clientReady', () => {
+            hooks.on('clientReady', () => this.events.emit(ProviderEvents.Ready));
+            this.events.emit(ProviderEvents.Ready);
+            resolve();
+          });
+
+          hooks.on('configChanged', (projectConfig) =>
+            this.events.emit(ProviderEvents.ConfigurationChanged, { metadata: { ...projectConfig } })
+          );
+
+          hooks.on('clientError', (message: string, error) =>
+            this.events.emit(ProviderEvents.Error, {
+              message: message,
+              metadata: error,
+            })
+          );
+        };
+      }
+
+      this.client = getClient(...originalParameters);
+    });
   }
 
-  public static createFromClient(client: IConfigCatClient) {
-    return new ConfigCatProvider(client);
+  public get configCatClient() {
+    return this.client;
   }
 
   public async onClose(): Promise<void> {
-    await this.client.dispose();
+    await this.client?.dispose();
   }
 
   async resolveBooleanEvaluation(
     flagKey: string,
     defaultValue: boolean,
     context: EvaluationContext
   ): Promise<ResolutionDetails<boolean>> {
+    if (!this.client) {
+      throw new GeneralError('Provider is not initialized');
+    }
+
     const { value, ...evaluationData } = await this.client.getValueDetailsAsync<SettingValue>(
       flagKey,
       undefined,
@@ -73,6 +101,10 @@ export class ConfigCatProvider implements Provider {
     defaultValue: string,
     context: EvaluationContext
   ): Promise<ResolutionDetails<string>> {
+    if (!this.client) {
+      throw new GeneralError('Provider is not initialized');
+    }
+
     const { value, ...evaluationData } = await this.client.getValueDetailsAsync<SettingValue>(
       flagKey,
       undefined,
@@ -91,6 +123,10 @@ export class ConfigCatProvider implements Provider {
     defaultValue: number,
     context: EvaluationContext
   ): Promise<ResolutionDetails<number>> {
+    if (!this.client) {
+      throw new GeneralError('Provider is not initialized');
+    }
+
     const { value, ...evaluationData } = await this.client.getValueDetailsAsync<SettingValue>(
       flagKey,
       undefined,
@@ -109,6 +145,10 @@ export class ConfigCatProvider implements Provider {
     defaultValue: U,
     context: EvaluationContext
   ): Promise<ResolutionDetails<U>> {
+    if (!this.client) {
+      throw new GeneralError('Provider is not initialized');
+    }
+
     const { value, ...evaluationData } = await this.client.getValueDetailsAsync(
       flagKey,
       undefined,