Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: removes exports of OpenFeatureClient class and makes event props readonly #918

Merged
merged 2 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading