Skip to content

Commit

Permalink
feat!: rm status from provider, run stale in SDK
Browse files Browse the repository at this point in the history
Signed-off-by: Todd Baert <[email protected]>
  • Loading branch information
toddbaert committed Feb 5, 2024
1 parent 1666597 commit f83aaaa
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 294 deletions.
17 changes: 7 additions & 10 deletions packages/client/e2e/step-definitions/evaluation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { defineFeature, loadFeature } from 'jest-cucumber';
import {
JsonValue,
JsonObject,
EvaluationDetails,
EvaluationContext,
EvaluationDetails,
JsonObject,
JsonValue,
ResolutionDetails,
StandardResolutionReasons,
} from '@openfeature/core';
import { OpenFeature, ProviderEvents, InMemoryProvider } from '../../src';
import { defineFeature, loadFeature } from 'jest-cucumber';
import { InMemoryProvider, OpenFeature } from '../../src';
import flagConfiguration from './flags-config';
// load the feature file.
const feature = loadFeature('packages/client/e2e/features/evaluation.feature');
Expand All @@ -18,15 +18,12 @@ const client = OpenFeature.getClient();
const givenAnOpenfeatureClientIsRegisteredWithCacheDisabled = (
given: (stepMatcher: string, stepDefinitionCallback: () => void) => void
) => {
OpenFeature.setProvider(new InMemoryProvider(flagConfiguration));
given('a provider is registered with cache disabled', () => undefined);
};

defineFeature(feature, (test) => {
beforeAll((done) => {
client.addHandler(ProviderEvents.Ready, async () => {
done();
});
beforeAll(async () => {
await OpenFeature.setProvider(new InMemoryProvider(flagConfiguration));
});

afterAll(async () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/client/src/client/open-feature-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class OpenFeatureClient implements Client {
// functions are passed here to make sure that these values are always up to date,
// and so we don't have to make these public properties on the API class.
private readonly providerAccessor: () => Provider,
private readonly providerStatusAccessor: () => ProviderStatus,
private readonly emitterAccessor: () => InternalEventEmitter,
private readonly globalLogger: () => Logger,
private readonly options: OpenFeatureClientOptions,
Expand All @@ -51,12 +52,12 @@ export class OpenFeatureClient implements Client {
}

get providerStatus(): ProviderStatus {
return this.providerAccessor()?.status || ProviderStatus.READY;
return this.providerStatusAccessor();
}

addHandler(eventType: ProviderEvents, handler: EventHandler): void {
this.emitterAccessor().addHandler(eventType, handler);
const shouldRunNow = statusMatchesEvent(eventType, this._provider.status);
const shouldRunNow = statusMatchesEvent(eventType, this.providerStatus);

if (shouldRunNow) {
// run immediately, we're in the matching state
Expand Down
29 changes: 19 additions & 10 deletions packages/client/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
GenericEventEmitter,
ManageContext,
OpenFeatureCommonAPI,
ProviderStatus,
StatusProviderRecord,
objectOrUndefined,
stringOrUndefined,
} from '@openfeature/core';
Expand All @@ -26,7 +28,7 @@ const _globalThis = globalThis as OpenFeatureGlobal;

export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> implements ManageContext<Promise<void>> {
protected _events: GenericEventEmitter<ProviderEvents> = new OpenFeatureEventEmitter();
protected _defaultProvider: Provider = NOOP_PROVIDER;
protected _defaultProvider: StatusProviderRecord<Provider> = { provider: NOOP_PROVIDER, status: ProviderStatus.NOT_READY };
protected _createEventEmitter = () => new OpenFeatureEventEmitter();
protected _namedProviderContext: Map<string, EvaluationContext> = new Map();

Expand Down Expand Up @@ -74,11 +76,11 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
const context = objectOrUndefined<T>(nameOrContext) ?? objectOrUndefined(contextOrUndefined) ?? {};

if (clientName) {
const provider = this._clientProviders.get(clientName);
if (provider) {
const proxy = this._clientProviders.get(clientName);
if (proxy) {
const oldContext = this.getContext(clientName);
this._namedProviderContext.set(clientName, context);
await this.runProviderContextChangeHandler(clientName, provider, oldContext, context);
await this.runProviderContextChangeHandler(clientName, proxy.provider, oldContext, context);
} else {
this._namedProviderContext.set(clientName, context);
}
Expand All @@ -89,14 +91,14 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
// collect all providers that are using the default context (not mapped to a name)
const defaultContextNameProviders: NameProviderRecord[] = Array.from(this._clientProviders.entries())
.filter(([name]) => !this._namedProviderContext.has(name))
.reduce<NameProviderRecord[]>((acc, [name, provider]) => {
acc.push({ name, provider });
.reduce<NameProviderRecord[]>((acc, [name, proxy]) => {
acc.push({ name, provider: proxy.provider });
return acc;
}, []);

const allProviders: NameProviderRecord[] = [
// add in the default (no name)
{ name: undefined, provider: this._defaultProvider },
{ name: undefined, provider: this._defaultProvider.provider },
...defaultContextNameProviders,
];
await Promise.all(
Expand Down Expand Up @@ -144,7 +146,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
async clearContext(nameOrUndefined?: string): Promise<void> {
const clientName = stringOrUndefined(nameOrUndefined);
if (clientName) {
const provider = this._clientProviders.get(clientName);
const provider = this._clientProviders.get(clientName)?.provider;
if (provider) {
const oldContext = this.getContext(clientName);
this._namedProviderContext.delete(clientName);
Expand Down Expand Up @@ -187,6 +189,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
// functions are passed here to make sure that these values are always up to date,
// and so we don't have to make these public properties on the API class.
() => this.getProviderForClient(name),
() => this.getProviderStatus(name),
() => this.buildAndCacheEventEmitterForClient(name),
() => this._logger,
{ name, version },
Expand All @@ -209,9 +212,15 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
newContext: EvaluationContext,
): Promise<void> {
const providerName = provider.metadata.name;

try {
await provider.onContextChange?.(oldContext, newContext);

if (typeof provider.onContextChange === 'function') {
this.getAssociatedEventEmitters(clientName).forEach((emitter) => {
emitter?.emit(ProviderEvents.Stale, { clientName, providerName });
});
this._events?.emit(ProviderEvents.Stale, { clientName, providerName });
await provider.onContextChange(oldContext, newContext);
}
// only run the event handlers if the onContextChange method succeeded
this.getAssociatedEventEmitters(clientName).forEach((emitter) => {
emitter?.emit(ProviderEvents.ContextChanged, { clientName, providerName });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
ResolutionDetails,
StandardResolutionReasons,
TypeMismatchError,
ProviderStatus,
} from '@openfeature/core';
import { Provider } from '../provider';
import { OpenFeatureEventEmitter, ProviderEvents } from '../../events';
Expand All @@ -22,7 +21,6 @@ import { VariantNotFoundError } from './variant-not-found-error';
export class InMemoryProvider implements Provider {
public readonly events = new OpenFeatureEventEmitter();
public readonly runsOn = 'client';
status: ProviderStatus = ProviderStatus.NOT_READY;
readonly metadata = {
name: 'in-memory',
} as const;
Expand All @@ -35,18 +33,12 @@ export class InMemoryProvider implements Provider {

async initialize(context?: EvaluationContext | undefined): Promise<void> {
try {

for (const key in this._flagConfiguration) {
this.resolveFlagWithReason(key, context);
}

this._context = context;
// set the provider's state, but don't emit events manually;
// the SDK does this based on the resolution/rejection of the init promise
this.status = ProviderStatus.READY;
} catch (error) {
this.status = ProviderStatus.ERROR;
throw error;
} catch (err) {
throw new Error('initialization failure', { cause: err });
}
}

Expand All @@ -59,16 +51,10 @@ export class InMemoryProvider implements Provider {
.filter(([key, value]) => this._flagConfiguration[key] !== value)
.map(([key]) => key);

this.status = ProviderStatus.STALE;
this.events.emit(ProviderEvents.Stale);

this._flagConfiguration = { ...flagConfiguration };
this.events.emit(ProviderEvents.ConfigurationChanged, { flagsChanged });

try {
await this.initialize(this._context);
// we need to emit our own events in this case, since it's not part of the init flow.
this.events.emit(ProviderEvents.Ready);
this.events.emit(ProviderEvents.ConfigurationChanged, { flagsChanged });
} catch (err) {
this.events.emit(ProviderEvents.Error);
throw err;
Expand Down Expand Up @@ -165,9 +151,7 @@ export class InMemoryProvider implements Provider {

const value = variant && flagSpec?.variants[variant];

const evalReason = isContextEval ? StandardResolutionReasons.TARGETING_MATCH : StandardResolutionReasons.STATIC;

const reason = this.status === ProviderStatus.STALE ? StandardResolutionReasons.CACHED : evalReason;
const reason = isContextEval ? StandardResolutionReasons.TARGETING_MATCH : StandardResolutionReasons.STATIC;

return {
value: value as T,
Expand Down
12 changes: 1 addition & 11 deletions packages/client/src/provider/no-op-provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JsonValue, ProviderStatus, ResolutionDetails } from '@openfeature/core';
import { JsonValue, ResolutionDetails } from '@openfeature/core';
import { Provider } from './provider';

const REASON_NO_OP = 'No-op';
Expand All @@ -11,16 +11,6 @@ class NoopFeatureProvider implements Provider {
name: 'No-op Provider',
} as const;

get status(): ProviderStatus {
/**
* This is due to the NoopProvider not being a real provider.
* We do not want it to trigger the Ready event handlers, so we never set this to ready.
* With the NoopProvider assigned, the client can be assumed to be uninitialized.
* https://github.com/open-feature/js-sdk/pull/429#discussion_r1202642654
*/
return ProviderStatus.NOT_READY;
}

resolveBooleanEvaluation(_: string, defaultValue: boolean): ResolutionDetails<boolean> {
return this.noOp(defaultValue);
}
Expand Down
19 changes: 14 additions & 5 deletions packages/client/test/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,13 +530,22 @@ describe('OpenFeatureClient', () => {
});

describe('providerStatus', () => {
it('should return current provider status', ()=> {
OpenFeature.setProvider({ ...MOCK_PROVIDER, status: ProviderStatus.STALE});
expect(OpenFeature.getClient().providerStatus).toEqual(ProviderStatus.STALE);
it('should return current provider status', (done) => {
OpenFeature.setProviderAndWait({
...MOCK_PROVIDER,
initialize: () => {
return new Promise<void>((resolve) => setTimeout(resolve, 1000));
},
}).then(() => {
expect(OpenFeature.getClient().providerStatus).toEqual(ProviderStatus.READY);
done();
});

expect(OpenFeature.getClient().providerStatus).toEqual(ProviderStatus.NOT_READY);
});

it('should return READY if not defined', ()=> {
OpenFeature.setProvider(MOCK_PROVIDER);
it('should return READY if initialize not defined', async () => {
await OpenFeature.setProviderAndWait({ ...MOCK_PROVIDER, initialize: undefined });
expect(OpenFeature.getClient().providerStatus).toEqual(ProviderStatus.READY);
});
});
Expand Down
Loading

0 comments on commit f83aaaa

Please sign in to comment.