feat: add provider compatibility check (#537)

## This PR

- add provider compatibility check

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

Fixes #535 

### Notes

This implementation throws if you set a provider that's incompatible
with its intended use. This could easily be changed to a warning if
that's the preferred approach. I think it's a good idea to fail early
but I would be interested in others thoughts on this.

This is a non-breaking change. Providers can optionally specify their
intended use and the SDK will perform a runtime check during provider
registration.

### Follow up

- Add hook compatibility checks.
- Update readme to include examples.
- Update existing providers.

---------

Signed-off-by: Michael Beemer <michael.beemer@dynatrace.com>
This commit is contained in:
Michael Beemer 2023-08-29 19:57:29 -04:00 committed by GitHub
parent b87338449d
commit 2bc5d63266
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 89 additions and 14 deletions

View File

@ -15,7 +15,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider> implements Ma
// eslint-disable-next-line @typescript-eslint/no-empty-function
private constructor() {
super();
super('client');
}
/**

View File

@ -367,7 +367,9 @@ describe('Evaluation details structure', () => {
let openFeatureErrorDetails: EvaluationDetails<string>;
let client: Client;
const errorProvider = {
name: 'error-mock',
metadata: {
name: 'error-mock',
},
resolveNumberEvaluation: jest.fn((): Promise<ResolutionDetails<number>> => {
throw new Error(NON_OPEN_FEATURE_ERROR_MESSAGE); // throw a non-open-feature error
@ -441,7 +443,9 @@ describe('Evaluation details structure', () => {
};
const flagMetadataProvider = {
name: 'flag-metadata',
metadata: {
name: 'flag-metadata',
},
resolveBooleanEvaluation: jest.fn((): ResolutionDetails<boolean> => {
return {
value: true,

View File

@ -1,11 +1,16 @@
import { Paradigm } from '@openfeature/shared';
import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src';
const mockProvider = (initialStatus?: ProviderStatus) => {
const mockProvider = (config?: {
initialStatus?: ProviderStatus,
runsOn?: Paradigm,
}) => {
return {
metadata: {
name: 'mock-events-success',
},
status: initialStatus || ProviderStatus.NOT_READY,
runsOn: config?.runsOn,
status: config?.initialStatus || ProviderStatus.NOT_READY,
initialize: jest.fn(() => {
return Promise.resolve('started');
}),
@ -33,6 +38,19 @@ describe('OpenFeature', () => {
expect(OpenFeature.providerMetadata === fakeProvider.metadata).toBeTruthy();
});
describe('Requirement 1.1.2.1', () => {
it('should throw because the provider is not intended for the client', () => {
const provider = mockProvider({ runsOn: 'server'});
expect(() => OpenFeature.setProvider(provider)).toThrowError(
"Provider 'mock-events-success' is intended for use on the server."
);
});
it('should succeed because the provider is intended for the client', () => {
const provider = mockProvider({ runsOn: 'client'});
expect(() => OpenFeature.setProvider(provider)).not.toThrowError();
});
});
describe('Requirement 1.1.2.2', () => {
it('MUST invoke the `initialize` function on the newly registered provider before using it to resolve flag values', () => {
const provider = mockProvider();
@ -42,7 +60,7 @@ describe('OpenFeature', () => {
});
it('should not invoke initialze function if the provider is not in state NOT_READY', () => {
const provider = mockProvider(ProviderStatus.READY);
const provider = mockProvider({ initialStatus: ProviderStatus.READY });
OpenFeature.setProvider(provider);
expect(OpenFeature.providerMetadata.name).toBe('mock-events-success');
expect(provider.initialize).not.toHaveBeenCalled();

View File

@ -21,7 +21,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider> implements Ma
// eslint-disable-next-line @typescript-eslint/no-empty-function
private constructor() {
super();
super('server');
}
/**

View File

@ -345,7 +345,9 @@ describe('OpenFeatureClient', () => {
let openFeatureErrorDetails: EvaluationDetails<string>;
let client: Client;
const errorProvider = {
name: 'error-mock',
metadata: {
name: 'error-mock',
},
resolveNumberEvaluation: jest.fn((): Promise<ResolutionDetails<number>> => {
throw new Error(NON_OPEN_FEATURE_ERROR_MESSAGE); // throw a non-open-feature error
@ -420,7 +422,9 @@ describe('OpenFeatureClient', () => {
};
const flagMetadataProvider = {
name: 'flag-metadata',
metadata: {
name: 'flag-metadata',
},
resolveBooleanEvaluation: jest.fn((): Promise<ResolutionDetails<boolean>> => {
return Promise.resolve({
value: true,
@ -448,7 +452,9 @@ describe('OpenFeatureClient', () => {
describe('Requirement 1.6.1', () => {
describe('Provider', () => {
const nonTransformingProvider = {
name: 'non-transforming',
metadata: {
name: 'non-transforming',
},
resolveBooleanEvaluation: jest.fn((): Promise<ResolutionDetails<boolean>> => {
return Promise.resolve({
value: true,
@ -476,7 +482,9 @@ describe('OpenFeatureClient', () => {
describe('Evaluation Context', () => {
const provider = {
name: 'evaluation-context',
metadata: {
name: 'evaluation-context',
},
resolveBooleanEvaluation: jest.fn((): Promise<ResolutionDetails<boolean>> => {
return Promise.resolve({
value: true,

View File

@ -1,11 +1,16 @@
import { Paradigm } from '@openfeature/shared';
import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src';
const mockProvider = (initialStatus?: ProviderStatus) => {
const mockProvider = (config?: {
initialStatus?: ProviderStatus,
runsOn?: Paradigm,
}) => {
return {
metadata: {
name: 'mock-events-success',
},
status: initialStatus || ProviderStatus.NOT_READY,
runsOn: config?.runsOn,
status: config?.initialStatus || ProviderStatus.NOT_READY,
initialize: jest.fn(() => {
return Promise.resolve('started');
}),
@ -33,6 +38,19 @@ describe('OpenFeature', () => {
expect(OpenFeature.providerMetadata === fakeProvider.metadata).toBeTruthy();
});
describe('Requirement 1.1.2.1', () => {
it('should throw because the provider is not intended for the server', () => {
const provider = mockProvider({ runsOn: 'client'});
expect(() => OpenFeature.setProvider(provider)).toThrowError(
"Provider 'mock-events-success' is intended for use on the client."
);
});
it('should succeed because the provider is intended for the server', () => {
const provider = mockProvider({ runsOn: 'server'});
expect(() => OpenFeature.setProvider(provider)).not.toThrowError();
});
});
describe('Requirement 1.1.2.2', () => {
it('MUST invoke the `initialize` function on the newly registered provider before using it to resolve flag values', () => {
const provider = mockProvider();
@ -42,7 +60,7 @@ describe('OpenFeature', () => {
});
it('should not invoke initialze function if the provider is not in state NOT_READY', () => {
const provider = mockProvider(ProviderStatus.READY);
const provider = mockProvider({ initialStatus: ProviderStatus.READY });
OpenFeature.setProvider(provider);
expect(OpenFeature.providerMetadata.name).toBe('mock-events-success');
expect(provider.initialize).not.toHaveBeenCalled();

View File

@ -1,3 +1,4 @@
import { GeneralError } from './errors';
import { EvaluationContext, FlagValue } from './evaluation';
import { EventDetails, EventHandler, Eventing, ProviderEvents } from './events';
import { InternalEventEmitter } from './events/open-feature-event-emitter';
@ -12,6 +13,7 @@ import {
TransactionContextPropagator,
} from './transaction-context';
import { objectOrUndefined, stringOrUndefined } from './type-guards';
import { Paradigm } from './types';
export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProvider>
implements
@ -32,6 +34,11 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
new Map();
protected _clientProviders: Map<string, P> = new Map();
protected _clientEvents: Map<string | undefined, InternalEventEmitter> = new Map();
protected _runsOn: Paradigm;
constructor(category: Paradigm) {
this._runsOn = category;
}
addHooks(...hooks: Hook<FlagValue>[]): this {
this._hooks = [...this._hooks, ...hooks];
@ -112,6 +119,7 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
const provider = objectOrUndefined<P>(clientOrProvider) ?? objectOrUndefined<P>(providerOrUndefined);
if (!provider) {
this._logger.debug('No provider defined, ignoring setProvider call');
return this;
}
@ -119,9 +127,16 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
// ignore no-ops
if (oldProvider === provider) {
this._logger.debug('Provider is already set, ignoring setProvider call');
return this;
}
if (!provider.runsOn) {
this._logger.debug(`Provider '${provider.metadata.name}' has not defined its intended use.`);
} else if (provider.runsOn !== this._runsOn){
throw new GeneralError(`Provider '${provider.metadata.name}' is intended for use on the ${provider.runsOn}.`);
}
const emitters = this.getAssociatedEventEmitters(clientName);
// warn of improper implementations

View File

@ -1,6 +1,7 @@
import { OpenFeatureEventEmitter } from '../events';
import { Metadata } from '../types';
import { EvaluationContext } from '../evaluation';
import { Paradigm } from '../types';
/**
* The state of the provider.
@ -32,6 +33,12 @@ export interface ProviderMetadata extends Metadata {
export interface CommonProvider {
readonly metadata: ProviderMetadata;
/**
* Represents where the provider is intended to be run. If defined,
* the SDK will enforce that the defined paradigm at runtime.
*/
readonly runsOn?: Paradigm;
/**
* Returns a representation of the current readiness of the provider.
* If the provider needs to be initialized, it should return {@link ProviderStatus.READY}.

View File

@ -1 +1,2 @@
export * from './metadata';
export * from './paradigm';

View File

@ -0,0 +1,4 @@
/**
* Defines where the library is intended to be run.
*/
export type Paradigm = 'server' | 'client';