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

feat: maintain state in SDK, add RECONCILING #795

Merged
merged 10 commits into from
Mar 1, 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
2 changes: 1 addition & 1 deletion jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export default {
],

// Use this configuration option to add custom reporters to Jest
reporters: [['github-actions', { silent: false }], 'summary'],
reporters: ['default', ['github-actions', { silent: false }], 'summary'],
toddbaert marked this conversation as resolved.
Show resolved Hide resolved

// Automatically reset mock state before every test
// resetMocks: false,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"private": true,
"description": "OpenFeature SDK for JavaScript",
"scripts": {
"test": "jest --selectProjects=shared --selectProjects=server --selectProjects=client --verbose",
"test": "jest --selectProjects=shared --selectProjects=server --selectProjects=client --silent",
"e2e-server": "git submodule update --init --recursive && shx cp test-harness/features/evaluation.feature packages/server/e2e/features && jest --selectProjects=server-e2e --verbose",
"e2e-client": "git submodule update --init --recursive && shx cp test-harness/features/evaluation.feature packages/client/e2e/features && jest --selectProjects=client-e2e --verbose",
"e2e": "npm run e2e-server && npm run e2e-client",
Expand Down
3 changes: 0 additions & 3 deletions packages/client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ import {
Logger,
Provider,
ProviderEventEmitter,
ProviderStatus,
ResolutionDetails
} from '@openfeature/web-sdk';

Expand Down Expand Up @@ -309,8 +308,6 @@ class MyProvider implements Provider {
// reconcile the provider's cached flags, if applicable
}

status?: ProviderStatus | undefined;

// implement with "new OpenFeatureEventEmitter()", and use "emit()" to emit events
events?: ProviderEventEmitter<AnyProviderEvent> | undefined;

Expand Down
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/client.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { ClientMetadata, EvaluationLifeCycle, Eventing, ManageLogger, ProviderStatus } from '@openfeature/core';
import { ClientMetadata, EvaluationLifeCycle, Eventing, ManageLogger } from '@openfeature/core';
import { Features } from '../evaluation';
import { ProviderStatus } from '../provider';

export interface Client extends EvaluationLifeCycle<Client>, Features, ManageLogger<Client>, Eventing {
readonly metadata: ClientMetadata;
/**
* Returns the status of the associated provider.
*/
readonly providerStatus: ProviderStatus;
}
}
17 changes: 13 additions & 4 deletions packages/client/src/client/open-feature-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
JsonValue,
Logger,
OpenFeatureError,
ProviderStatus,
ProviderFatalError,
ResolutionDetails,
SafeLogger,
StandardResolutionReasons,
Expand All @@ -21,8 +21,9 @@ import { ProviderEvents } from '../events';
import { InternalEventEmitter } from '../events/internal/internal-event-emitter';
import { Hook } from '../hooks';
import { OpenFeature } from '../open-feature';
import { Provider } from '../provider';
import { Provider, ProviderStatus } from '../provider';
import { Client } from './client';
import { ProviderNotReadyError } from '@openfeature/core';

type OpenFeatureClientOptions = {
/**
Expand All @@ -41,6 +42,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 @@ -57,12 +59,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 Expand Up @@ -206,6 +208,13 @@ export class OpenFeatureClient implements Client {

try {
this.beforeHooks(allHooks, hookContext, options);

// short circuit evaluation entirely if provider is in a bad state
if (this.providerStatus === ProviderStatus.NOT_READY) {
throw new ProviderNotReadyError('provider has not yet initialized');
} else if (this.providerStatus === ProviderStatus.FATAL) {
throw new ProviderFatalError('provider is in an irrecoverable error state');
}

// run the referenced resolver, binding the provider.
const resolution = resolver.call(this._provider, flagKey, defaultValue, context, this._logger);
Expand Down
96 changes: 63 additions & 33 deletions packages/client/src/open-feature.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import {
ClientProviderStatus,
EvaluationContext,
GenericEventEmitter,
ManageContext,
OpenFeatureCommonAPI,
ProviderWrapper,
objectOrUndefined,
stringOrUndefined,
} from '@openfeature/core';
import { Client, OpenFeatureClient } from './client';
import { OpenFeatureEventEmitter, ProviderEvents } from './events';
import { Hook } from './hooks';
import { NOOP_PROVIDER, Provider } from './provider';
import { NOOP_PROVIDER, Provider, ProviderStatus } from './provider';

// use a symbol as a key for the global singleton
const GLOBAL_OPENFEATURE_API_KEY = Symbol.for('@openfeature/web-sdk/api');
Expand All @@ -19,14 +21,16 @@ type OpenFeatureGlobal = {
};
type DomainRecord = {
domain?: string;
provider: Provider;
wrapper: ProviderWrapper<Provider, ClientProviderStatus>;
};

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;
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 _domainScopedProviders: Map<string, ProviderWrapper<Provider, ClientProviderStatus>> = new Map();
protected _createEventEmitter = () => new OpenFeatureEventEmitter();

private constructor() {
Expand All @@ -49,6 +53,14 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
return instance;
}

private getProviderStatus(domain?: string): ProviderStatus {
if (!domain) {
return this._defaultProvider.status;
}

return this._domainScopedProviders.get(domain)?.status ?? this._defaultProvider.status;
}

/**
* Sets the evaluation context globally.
* This will be used by all providers that have not bound to a domain.
Expand All @@ -73,11 +85,11 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
const context = objectOrUndefined<T>(domainOrContext) ?? objectOrUndefined(contextOrUndefined) ?? {};

if (domain) {
const provider = this._domainScopedProviders.get(domain);
if (provider) {
const wrapper = this._domainScopedProviders.get(domain);
if (wrapper) {
const oldContext = this.getContext(domain);
this._domainScopedContext.set(domain, context);
await this.runProviderContextChangeHandler(domain, provider, oldContext, context);
await this.runProviderContextChangeHandler(domain, wrapper, oldContext, context);
} else {
this._domainScopedContext.set(domain, context);
}
Expand All @@ -88,19 +100,19 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
// collect all providers that are using the default context (not bound to a domain)
const unboundProviders: DomainRecord[] = Array.from(this._domainScopedProviders.entries())
.filter(([domain]) => !this._domainScopedContext.has(domain))
.reduce<DomainRecord[]>((acc, [domain, provider]) => {
acc.push({ domain, provider });
.reduce<DomainRecord[]>((acc, [domain, wrapper]) => {
acc.push({ domain, wrapper });
return acc;
}, []);

const allProviders: DomainRecord[] = [
const allDomainRecords: DomainRecord[] = [
// add in the default (no domain)
{ domain: undefined, provider: this._defaultProvider },
{ domain: undefined, wrapper: this._defaultProvider },
...unboundProviders,
];
await Promise.all(
allProviders.map((tuple) =>
this.runProviderContextChangeHandler(tuple.domain, tuple.provider, oldContext, context),
allDomainRecords.map((dm) =>
this.runProviderContextChangeHandler(dm.domain, dm.wrapper, oldContext, context),
),
);
}
Expand Down Expand Up @@ -143,12 +155,12 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme
async clearContext(domainOrUndefined?: string): Promise<void> {
const domain = stringOrUndefined(domainOrUndefined);
if (domain) {
const provider = this._domainScopedProviders.get(domain);
if (provider) {
const wrapper = this._domainScopedProviders.get(domain);
if (wrapper) {
const oldContext = this.getContext(domain);
this._domainScopedContext.delete(domain);
const newContext = this.getContext();
await this.runProviderContextChangeHandler(domain, provider, oldContext, newContext);
await this.runProviderContextChangeHandler(domain, wrapper, oldContext, newContext);
} else {
this._domainScopedContext.delete(domain);
}
Expand Down Expand Up @@ -186,6 +198,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(domain),
() => this.getProviderStatus(domain),
() => this.buildAndCacheEventEmitterForClient(domain),
() => this._logger,
{ domain, version },
Expand All @@ -203,28 +216,45 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider, Hook> impleme

private async runProviderContextChangeHandler(
domain: string | undefined,
provider: Provider,
wrapper: ProviderWrapper<Provider, ClientProviderStatus>,
oldContext: EvaluationContext,
newContext: EvaluationContext,
): Promise<void> {
const providerName = provider.metadata.name;
// this should always be set according to the typings, but let's be defensive considering JS
const providerName = wrapper.provider?.metadata?.name || 'unnamed-provider';
beeme1mr marked this conversation as resolved.
Show resolved Hide resolved

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

// only run the event handlers if the onContextChange method succeeded
this.getAssociatedEventEmitters(domain).forEach((emitter) => {
emitter?.emit(ProviderEvents.ContextChanged, { clientName: domain, domain, providerName });
});
this._events?.emit(ProviderEvents.ContextChanged, { clientName: domain, domain, providerName });
if (typeof wrapper.provider.onContextChange === 'function') {
wrapper.incrementPendingContextChanges();
wrapper.status = this._statusEnumType.RECONCILING;
this.getAssociatedEventEmitters(domain).forEach((emitter) => {
emitter?.emit(ProviderEvents.Reconciling, { domain, providerName });
});
this._apiEmitter?.emit(ProviderEvents.Reconciling, { domain, providerName });
await wrapper.provider.onContextChange(oldContext, newContext);
wrapper.decrementPendingContextChanges();
}
// only run the event handlers, and update the state if the onContextChange method succeeded
wrapper.status = this._statusEnumType.READY;
if (wrapper.allContextChangesSettled) {
this.getAssociatedEventEmitters(domain).forEach((emitter) => {
emitter?.emit(ProviderEvents.ContextChanged, { clientName: domain, domain, providerName });
});
this._apiEmitter?.emit(ProviderEvents.ContextChanged, { clientName: domain, domain, providerName });
}
} catch (err) {
// run error handlers instead
const error = err as Error | undefined;
const message = `Error running ${provider?.metadata?.name}'s context change handler: ${error?.message}`;
this._logger?.error(`${message}`, err);
this.getAssociatedEventEmitters(domain).forEach((emitter) => {
emitter?.emit(ProviderEvents.Error, { clientName: domain, domain, providerName, message });
});
this._events?.emit(ProviderEvents.Error, { clientName: domain, domain, providerName, message });
wrapper.decrementPendingContextChanges();
wrapper.status = this._statusEnumType.ERROR;
if (wrapper.allContextChangesSettled) {
const error = err as Error | undefined;
const message = `Error running ${providerName}'s context change handler: ${error?.message}`;
this._logger?.error(`${message}`, err);
this.getAssociatedEventEmitters(domain).forEach((emitter) => {
emitter?.emit(ProviderEvents.Error, { clientName: domain, domain, providerName, message });
});
this._apiEmitter?.emit(ProviderEvents.Error, { clientName: domain, domain, providerName, message });
}
}
}
}
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
Loading
Loading