Skip to content

Commit

Permalink
fix: removes exports of OpenFeatureClient class and makes event props…
Browse files Browse the repository at this point in the history
… readonly (#918)

<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

Removes OpenFeatureClient class from exports and makes event details
readonly as described here:
#799

Signed-off-by: Lukas Reining <[email protected]>
  • Loading branch information
lukas-reining authored May 2, 2024
1 parent 6dd558e commit e9a25c2
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 50 deletions.
1 change: 0 additions & 1 deletion packages/client/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './client';
export * from './open-feature-client';
20 changes: 13 additions & 7 deletions packages/client/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import {
objectOrUndefined,
stringOrUndefined,
} from '@openfeature/core';
import { Client, OpenFeatureClient } from './client';
import { Client } from './client';
import { OpenFeatureEventEmitter, ProviderEvents } from './events';
import { Hook } from './hooks';
import { NOOP_PROVIDER, Provider, ProviderStatus } from './provider';
import { OpenFeatureClient } from './client/open-feature-client';

// use a symbol as a key for the global singleton
const GLOBAL_OPENFEATURE_API_KEY = Symbol.for('@openfeature/web-sdk/api');
Expand All @@ -26,10 +27,17 @@ type DomainRecord = {

const _globalThis = globalThis as OpenFeatureGlobal;

export class OpenFeatureAPI extends OpenFeatureCommonAPI<ClientProviderStatus, Provider, Hook> implements ManageContext<Promise<void>> {
export class OpenFeatureAPI
extends OpenFeatureCommonAPI<ClientProviderStatus, Provider, Hook>
implements ManageContext<Promise<void>>
{
protected _statusEnumType: typeof ProviderStatus = ProviderStatus;
protected _apiEmitter: GenericEventEmitter<ProviderEvents> = new OpenFeatureEventEmitter();
protected _defaultProvider: ProviderWrapper<Provider, ClientProviderStatus> = new ProviderWrapper(NOOP_PROVIDER, ProviderStatus.NOT_READY, this._statusEnumType);
protected _defaultProvider: ProviderWrapper<Provider, ClientProviderStatus> = new ProviderWrapper(
NOOP_PROVIDER,
ProviderStatus.NOT_READY,
this._statusEnumType,
);
protected _domainScopedProviders: Map<string, ProviderWrapper<Provider, ClientProviderStatus>> = new Map();
protected _createEventEmitter = () => new OpenFeatureEventEmitter();

Expand Down Expand Up @@ -111,9 +119,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<ClientProviderStatus, P
...unboundProviders,
];
await Promise.all(
allDomainRecords.map((dm) =>
this.runProviderContextChangeHandler(dm.domain, dm.wrapper, oldContext, context),
),
allDomainRecords.map((dm) => this.runProviderContextChangeHandler(dm.domain, dm.wrapper, oldContext, context)),
);
}
}
Expand Down Expand Up @@ -222,7 +228,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<ClientProviderStatus, P
): Promise<void> {
// this should always be set according to the typings, but let's be defensive considering JS
const providerName = wrapper.provider?.metadata?.name || 'unnamed-provider';

try {
if (typeof wrapper.provider.onContextChange === 'function') {
wrapper.incrementPendingContextChanges();
Expand Down
44 changes: 22 additions & 22 deletions packages/client/test/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import {
JsonObject,
JsonValue,
OpenFeature,
OpenFeatureClient,
Provider,
ProviderFatalError,
ProviderStatus,
ResolutionDetails,
StandardResolutionReasons,
} from '../src';
import { OpenFeatureClient } from '../src/client/open-feature-client';

const BOOLEAN_VALUE = true;
const STRING_VALUE = 'val';
Expand Down Expand Up @@ -382,7 +382,7 @@ describe('OpenFeatureClient', () => {
// No generic information exists at runtime, but this test has some value in ensuring the generic args still exist in the typings.
const client = OpenFeature.getClient();
const details: ResolutionDetails<JsonValue> = client.getObjectDetails('flag', { key: 'value' });

expect(details).toBeDefined();
});
});
Expand Down Expand Up @@ -464,45 +464,45 @@ describe('OpenFeatureClient', () => {
expect(details.errorCode).toEqual(ErrorCode.PROVIDER_NOT_READY);
});
});

describe('Evaluation details structure', () => {
const flagKey = 'number-details';
const defaultValue = 1970;
let details: EvaluationDetails<number>;

describe('Normal execution', () => {
beforeEach(() => {
const client = OpenFeature.getClient();
details = client.getNumberDetails(flagKey, defaultValue);

expect(details).toBeDefined();
});

describe('Requirement 1.4.2, 1.4.3', () => {
it('should contain flag value', () => {
expect(details.value).toEqual(NUMBER_VALUE);
});
});

describe('Requirement 1.4.4', () => {
it('should contain flag key', () => {
expect(details.flagKey).toEqual(flagKey);
});
});

describe('Requirement 1.4.5', () => {
it('should contain flag variant', () => {
expect(details.variant).toEqual(NUMBER_VARIANT);
});
});

describe('Requirement 1.4.6', () => {
it('should contain reason', () => {
expect(details.reason).toEqual(REASON);
});
});
});

describe('Abnormal execution', () => {
const NON_OPEN_FEATURE_ERROR_MESSAGE = 'A null dereference or something, I dunno.';
const OPEN_FEATURE_ERROR_MESSAGE = "This ain't the flag you're looking for.";
Expand All @@ -522,68 +522,68 @@ describe('OpenFeatureClient', () => {
} as unknown as Provider;
const defaultNumberValue = 123;
const defaultStringValue = 'hey!';

beforeEach(() => {
OpenFeature.setProvider(errorProvider);
client = OpenFeature.getClient();
nonOpenFeatureErrorDetails = client.getNumberDetails('some-flag', defaultNumberValue);
openFeatureErrorDetails = client.getStringDetails('some-flag', defaultStringValue);
});

describe('Requirement 1.4.7', () => {
describe('OpenFeatureError', () => {
it('should contain error code', () => {
expect(openFeatureErrorDetails.errorCode).toBeTruthy();
expect(openFeatureErrorDetails.errorCode).toEqual(ErrorCode.FLAG_NOT_FOUND); // should get code from thrown OpenFeatureError
});
});

describe('Non-OpenFeatureError', () => {
it('should contain error code', () => {
expect(nonOpenFeatureErrorDetails.errorCode).toBeTruthy();
expect(nonOpenFeatureErrorDetails.errorCode).toEqual(ErrorCode.GENERAL); // should fall back to GENERAL
});
});
});

describe('Requirement 1.4.8', () => {
it('should contain error reason', () => {
expect(nonOpenFeatureErrorDetails.reason).toEqual(StandardResolutionReasons.ERROR);
expect(openFeatureErrorDetails.reason).toEqual(StandardResolutionReasons.ERROR);
});
});

describe('Requirement 1.4.9', () => {
it('must not throw, must return default', () => {
nonOpenFeatureErrorDetails = client.getNumberDetails('some-flag', defaultNumberValue);

expect(nonOpenFeatureErrorDetails).toBeTruthy();
expect(nonOpenFeatureErrorDetails.value).toEqual(defaultNumberValue);
});
});

describe('Requirement 1.4.12', () => {
describe('OpenFeatureError', () => {
it('should contain "error" message', () => {
expect(openFeatureErrorDetails.errorMessage).toEqual(OPEN_FEATURE_ERROR_MESSAGE);
});
});

describe('Non-OpenFeatureError', () => {
it('should contain "error" message', () => {
expect(nonOpenFeatureErrorDetails.errorMessage).toEqual(NON_OPEN_FEATURE_ERROR_MESSAGE);
});
});
});
});

describe('Requirement 1.4.13, Requirement 1.4.14', () => {
it('should return immutable `flag metadata` as defined by the provider', () => {
const flagMetadata = {
url: 'https://test.dev',
version: '1',
};

const flagMetadataProvider = {
metadata: {
name: 'flag-metadata',
Expand All @@ -595,14 +595,14 @@ describe('OpenFeatureClient', () => {
};
}),
} as unknown as Provider;

OpenFeature.setProvider(flagMetadataProvider);
const client = OpenFeature.getClient();
const response = client.getBooleanDetails('some-flag', false);
expect(response.flagMetadata).toBe(flagMetadata);
expect(Object.isFrozen(response.flagMetadata)).toBeTruthy();
});

it('should return empty `flag metadata` because it was not set by the provider', () => {
// The mock provider doesn't contain flag metadata
OpenFeature.setProvider(MOCK_PROVIDER);
Expand Down
3 changes: 2 additions & 1 deletion packages/client/test/open-feature.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Paradigm } from '@openfeature/core';
import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src';
import { OpenFeature, OpenFeatureAPI, Provider, ProviderStatus } from '../src';
import { OpenFeatureClient } from '../src/client/open-feature-client';

const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => {
return {
Expand Down
8 changes: 4 additions & 4 deletions packages/nest/test/open-feature.module.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Test, TestingModule } from '@nestjs/testing';
import { getOpenFeatureClientToken, OpenFeatureModule, ServerProviderEvents } from '../src';
import { OpenFeature, OpenFeatureClient } from '@openfeature/server-sdk';
import { Client, OpenFeature } from '@openfeature/server-sdk';
import { getOpenFeatureDefaultTestModule } from './fixtures';

describe('OpenFeatureModule', () => {
Expand Down Expand Up @@ -31,19 +31,19 @@ describe('OpenFeatureModule', () => {

it('should return the SDKs default provider and not throw', async () => {
expect(() => {
moduleWithoutProvidersRef.get<OpenFeatureClient>(getOpenFeatureClientToken());
moduleWithoutProvidersRef.get<Client>(getOpenFeatureClientToken());
}).not.toThrow();
});
});

it('should return the default provider', async () => {
const client = moduleRef.get<OpenFeatureClient>(getOpenFeatureClientToken());
const client = moduleRef.get<Client>(getOpenFeatureClientToken());
expect(client).toBeDefined();
expect(await client.getStringValue('testStringFlag', '')).toEqual('expected-string-value-default');
});

it('should inject the client with the given scope', async () => {
const client = moduleRef.get<OpenFeatureClient>(getOpenFeatureClientToken('domainScopedClient'));
const client = moduleRef.get<Client>(getOpenFeatureClientToken('domainScopedClient'));
expect(client).toBeDefined();
expect(await client.getStringValue('testStringFlag', '')).toEqual('expected-string-value-scoped');
});
Expand Down
6 changes: 3 additions & 3 deletions packages/nest/test/test-app.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { Controller, Get, Injectable, UseInterceptors } from '@nestjs/common';
import { Observable, map } from 'rxjs';
import { BooleanFeatureFlag, ObjectFeatureFlag, NumberFeatureFlag, FeatureClient, StringFeatureFlag } from '../src';
import { OpenFeatureClient, EvaluationDetails, FlagValue } from '@openfeature/server-sdk';
import { Client, EvaluationDetails, FlagValue } from '@openfeature/server-sdk';
import { EvaluationContextInterceptor } from '../src';

@Injectable()
export class OpenFeatureTestService {
constructor(
@FeatureClient() public defaultClient: OpenFeatureClient,
@FeatureClient({ domain: 'domainScopedClient' }) public domainScopedClient: OpenFeatureClient,
@FeatureClient() public defaultClient: Client,
@FeatureClient({ domain: 'domainScopedClient' }) public domainScopedClient: Client,
) {}

public async serviceMethod(flag: EvaluationDetails<FlagValue>) {
Expand Down
1 change: 0 additions & 1 deletion packages/server/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './client';
export * from './open-feature-client';
13 changes: 10 additions & 3 deletions packages/server/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
objectOrUndefined,
stringOrUndefined,
} from '@openfeature/core';
import { Client, OpenFeatureClient } from './client';
import { Client } from './client';
import { OpenFeatureEventEmitter } from './events';
import { Hook } from './hooks';
import { NOOP_PROVIDER, Provider, ProviderStatus } from './provider';
Expand All @@ -17,6 +17,7 @@ import {
TransactionContextPropagator,
} from './transaction-context';
import { ServerProviderStatus } from '@openfeature/core';
import { OpenFeatureClient } from './client/open-feature-client';

// use a symbol as a key for the global singleton
const GLOBAL_OPENFEATURE_API_KEY = Symbol.for('@openfeature/js-sdk/api');
Expand All @@ -28,11 +29,17 @@ const _globalThis = globalThis as OpenFeatureGlobal;

export class OpenFeatureAPI
extends OpenFeatureCommonAPI<ServerProviderStatus, Provider, Hook>
implements ManageContext<OpenFeatureAPI>, ManageTransactionContextPropagator<OpenFeatureCommonAPI<ServerProviderStatus, Provider>>
implements
ManageContext<OpenFeatureAPI>,
ManageTransactionContextPropagator<OpenFeatureCommonAPI<ServerProviderStatus, Provider>>
{
protected _statusEnumType: typeof ProviderStatus = ProviderStatus;
protected _apiEmitter = new OpenFeatureEventEmitter();
protected _defaultProvider: ProviderWrapper<Provider, ServerProviderStatus> = new ProviderWrapper(NOOP_PROVIDER, ProviderStatus.NOT_READY, this._statusEnumType);
protected _defaultProvider: ProviderWrapper<Provider, ServerProviderStatus> = new ProviderWrapper(
NOOP_PROVIDER,
ProviderStatus.NOT_READY,
this._statusEnumType,
);
protected _domainScopedProviders: Map<string, ProviderWrapper<Provider, ServerProviderStatus>> = new Map();
protected _createEventEmitter = () => new OpenFeatureEventEmitter();

Expand Down
2 changes: 1 addition & 1 deletion packages/server/test/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
JsonObject,
JsonValue,
OpenFeature,
OpenFeatureClient,
Provider,
ProviderFatalError,
ProviderStatus,
Expand All @@ -18,6 +17,7 @@ import {
TransactionContext,
TransactionContextPropagator,
} from '../src';
import { OpenFeatureClient } from '../src/client/open-feature-client';

const BOOLEAN_VALUE = true;
const STRING_VALUE = 'val';
Expand Down
3 changes: 2 additions & 1 deletion packages/server/test/open-feature.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Paradigm } from '@openfeature/core';
import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src';
import { OpenFeature, OpenFeatureAPI, Provider, ProviderStatus } from '../src';
import { OpenFeatureClient } from '../src/client/open-feature-client';

const mockProvider = (config?: { initialStatus?: ProviderStatus; runsOn?: Paradigm }) => {
return {
Expand Down
12 changes: 6 additions & 6 deletions packages/shared/src/events/eventing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@ export type EventMetadata = {
};

export type CommonEventDetails = {
providerName: string;
readonly providerName: string;
/**
* @deprecated alias of "domain", use domain instead
*/
clientName?: string;
readonly clientName?: string;
readonly domain?: string;
};

type CommonEventProps = {
message?: string;
metadata?: EventMetadata;
readonly message?: string;
readonly metadata?: EventMetadata;
};

export type ReadyEvent = CommonEventProps;
export type ErrorEvent = CommonEventProps;
export type StaleEvent = CommonEventProps;
export type ReconcilingEvent = CommonEventProps & { errorCode: ErrorCode };
export type ConfigChangeEvent = CommonEventProps & { flagsChanged?: string[] };
export type ReconcilingEvent = CommonEventProps & { readonly errorCode: ErrorCode };
export type ConfigChangeEvent = CommonEventProps & { readonly flagsChanged?: string[] };

type ServerEventMap = {
[ServerProviderEvents.Ready]: ReadyEvent;
Expand Down

0 comments on commit e9a25c2

Please sign in to comment.