From 5e320ae3811e270985e867c1c85a301eacd99a49 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 31 Jul 2023 09:42:30 -0400 Subject: [PATCH] fix: only initialize NOT_READY providers (#507) See here. We should only run initialize if `ProviderStatus` is `NOT_READY` to prevent duplicate calls to `initialize`, as we [Java](https://github.com/open-feature/java-sdk/blob/main/src/main/java/dev/openfeature/sdk/ProviderRepository.java#L107-L108) and [Go](https://github.com/open-feature/go-sdk/blob/main/pkg/openfeature/api.go#L173). Most of this is test and formatting changes (I had to update the tests not to use the same object literal). The import change is the single line here. Note: it's likely this will break _some_ providers that didn't implement `status`. I think the [flagd web provider](https://github.com/open-feature/js-sdk-contrib/blob/main/libs/providers/flagd-web/src/lib/flagd-web-provider.ts), [GoFF](https://github.com/open-feature/js-sdk-contrib/blob/main/libs/providers/go-feature-flag/src/lib/go-feature-flag-provider.ts) provider and the [config-cat](https://github.com/open-feature/js-sdk-contrib/blob/main/libs/providers/config-cat/src/lib/config-cat-provider.ts) could be impacted. I created issues for these: - https://github.com/open-feature/js-sdk-contrib/issues/488 - https://github.com/open-feature/js-sdk-contrib/issues/489 - https://github.com/open-feature/js-sdk-contrib/issues/490 Fixes: https://github.com/open-feature/js-sdk/issues/505 I've also removed all the `setTimeouts` in our test suite. Everything is working "properly" now with plain events, since all flagd providers are fully updated. --------- Signed-off-by: Todd Baert --- package-lock.json | 114 ++++++------- package.json | 4 +- .../e2e/step-definitions/evaluation.spec.ts | 5 +- packages/client/e2e/step-definitions/setup.ts | 20 ++- packages/client/test/open-feature.spec.ts | 104 +++++++----- .../e2e/step-definitions/evaluation.spec.ts | 4 +- packages/server/test/open-feature.spec.ts | 151 ++++++++++-------- packages/shared/src/open-feature.ts | 12 +- 8 files changed, 217 insertions(+), 197 deletions(-) diff --git a/package-lock.json b/package-lock.json index df8dea4ab..07e397d8c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,8 +14,8 @@ "packages/shared" ], "devDependencies": { - "@openfeature/flagd-provider": "^0.7.6", - "@openfeature/flagd-web-provider": "^0.3.4", + "@openfeature/flagd-provider": "^0.8.1", + "@openfeature/flagd-web-provider": "^0.3.5", "@rollup/plugin-alias": "^5.0.0", "@rollup/plugin-typescript": "^11.0.0", "@types/events": "^3.0.0", @@ -612,30 +612,30 @@ "license": "MIT" }, "node_modules/@bufbuild/connect": { - "version": "0.8.5", - "resolved": "https://registry.npmjs.org/@bufbuild/connect/-/connect-0.8.5.tgz", - "integrity": "sha512-MHsVL8DTkWz/O1HRVwl5/33sCkJWlf69A6gly9eHPHH6/xUb62Db4uUYIXxO6bsasqHyVIMqr0+F9Kv7YJt28g==", + "version": "0.11.0", + "resolved": "https://registry.npmjs.org/@bufbuild/connect/-/connect-0.11.0.tgz", + "integrity": "sha512-a6hrNtBzDzj4hpqylPqpJfMpIP4+O/SnszGgOuRzcuifpTvkwjSmVHLtcvkUYh0wpvjYB0CFmTYzrvAMOftbHw==", "dev": true, "peerDependencies": { - "@bufbuild/protobuf": "^1.2.0" + "@bufbuild/protobuf": "^1.2.1" } }, "node_modules/@bufbuild/connect-web": { - "version": "0.8.5", - "resolved": "https://registry.npmjs.org/@bufbuild/connect-web/-/connect-web-0.8.5.tgz", - "integrity": "sha512-UAsTa51v1PeYyLfJ0GHalfrFFbUdGlEG3fp11IdJO/Ro4ypNZQFoHbuen0bWDFp4bHuezevVdBwPkTJaSTrXEA==", + "version": "0.11.0", + "resolved": "https://registry.npmjs.org/@bufbuild/connect-web/-/connect-web-0.11.0.tgz", + "integrity": "sha512-H0tSsn7dMJY5EQNHoQyE/TXmmRtJ6GauRl9RWk4GncQCXulo5ab5yn8cEtu7UKnPCvF7nYbK1ESE0vHi5Y2xaw==", "dev": true, "dependencies": { - "@bufbuild/connect": "0.8.5" + "@bufbuild/connect": "0.11.0" }, "peerDependencies": { - "@bufbuild/protobuf": "^1.2.0" + "@bufbuild/protobuf": "^1.2.1" } }, "node_modules/@bufbuild/protobuf": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/@bufbuild/protobuf/-/protobuf-1.2.0.tgz", - "integrity": "sha512-MBVuQMOBHxgGnZ9XCUIi8WOy5O/T4ma3TduCRhRvndv3UDbG9cHgd8h6nOYSGyBYPEvXf1z9nTwhp8mVIDbq2g==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@bufbuild/protobuf/-/protobuf-1.2.1.tgz", + "integrity": "sha512-cwwGvLGqvoaOZmoP5+i4v/rbW+rHkguvTehuZyM2p/xpmaNSdT2h3B7kHw33aiffv35t1XrYHIkdJSEkSEMJuA==", "dev": true }, "node_modules/@cnakazawa/watch": { @@ -760,10 +760,11 @@ } }, "node_modules/@grpc/grpc-js": { - "version": "1.8.13", - "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.8.13.tgz", - "integrity": "sha512-iY3jsdfbc0ARoCLFvbvUB8optgyb0r1XLPb142u+QtgBcKJYkCIFt3Fd/881KqjLYWjsBJF57N3b8Eop9NDfUA==", + "version": "1.8.20", + "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.8.20.tgz", + "integrity": "sha512-SPse1wE4PcIFojOISsAnrWXCBsCBwDdcDqz2SS0T8nBSxg9jwmCK70Jy7ypRn2nIspwLy3Ls5TNaKNHo+6dF8A==", "dev": true, + "peer": true, "dependencies": { "@grpc/proto-loader": "^0.7.0", "@types/node": ">=12.12.47" @@ -773,15 +774,16 @@ } }, "node_modules/@grpc/proto-loader": { - "version": "0.7.7", - "resolved": "https://registry.npmjs.org/@grpc/proto-loader/-/proto-loader-0.7.7.tgz", - "integrity": "sha512-1TIeXOi8TuSCQprPItwoMymZXxWT0CPxUhkrkeCUH+D8U7QDwQ6b7SUz2MaLuWM2llT+J/TVFLmQI5KtML3BhQ==", + "version": "0.7.8", + "resolved": "https://registry.npmjs.org/@grpc/proto-loader/-/proto-loader-0.7.8.tgz", + "integrity": "sha512-GU12e2c8dmdXb7XUlOgYWZ2o2i+z9/VeACkxTA/zzAe2IjclC5PnVL0lpgjhrqfpDYHzM8B1TF6pqWegMYAzlA==", "dev": true, + "peer": true, "dependencies": { "@types/long": "^4.0.1", "lodash.camelcase": "^4.3.0", "long": "^4.0.0", - "protobufjs": "^7.0.0", + "protobufjs": "^7.2.4", "yargs": "^17.7.2" }, "bin": { @@ -1235,39 +1237,37 @@ } }, "node_modules/@openfeature/flagd-provider": { - "version": "0.7.6", - "resolved": "https://registry.npmjs.org/@openfeature/flagd-provider/-/flagd-provider-0.7.6.tgz", - "integrity": "sha512-WZrE9zLMuRW+OBOlN4TPcUTJaUDYkcURMB6fA7gg3E1DsZqDiRBqWIjH6B2BgqUBPbVu3u7pdXPqZxwPt68N7g==", + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/@openfeature/flagd-provider/-/flagd-provider-0.8.1.tgz", + "integrity": "sha512-6yO0JJmM4m4nVJM6emSLe+XSLmnTG8LFhAFzfTvwxPsKGjki1uruICJSKznPAdlmG9KmZD3cOEubEV/eq/TW8Q==", "dev": true, "dependencies": { - "@grpc/grpc-js": "1.8.13", - "@protobuf-ts/grpc-transport": "2.8.3", - "@protobuf-ts/runtime": "2.8.3", - "@protobuf-ts/runtime-rpc": "2.8.3", - "lru-cache": "8.0.5" + "@protobuf-ts/runtime-rpc": "2.9.0", + "lru-cache": "10.0.0" }, "peerDependencies": { - "@openfeature/js-sdk": "^1.1.0" + "@grpc/grpc-js": "^1.6.0", + "@openfeature/js-sdk": ">=1.3.0" } }, "node_modules/@openfeature/flagd-provider/node_modules/lru-cache": { - "version": "8.0.5", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-8.0.5.tgz", - "integrity": "sha512-MhWWlVnuab1RG5/zMRRcVGXZLCXrZTgfwMikgzCegsPnG62yDQo5JnqKkrK4jO5iKqDAZGItAqN5CtKBCBWRUA==", + "version": "10.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-10.0.0.tgz", + "integrity": "sha512-svTf/fzsKHffP42sujkO/Rjs37BCIsQVRCeNYIm9WN8rgT7ffoUnRtZCqU+6BqcSBdv8gwJeTz8knJpgACeQMw==", "dev": true, "engines": { - "node": ">=16.14" + "node": "14 || >=16.14" } }, "node_modules/@openfeature/flagd-web-provider": { - "version": "0.3.4", - "resolved": "https://registry.npmjs.org/@openfeature/flagd-web-provider/-/flagd-web-provider-0.3.4.tgz", - "integrity": "sha512-uroo3XvlnZaqlKDdW1Hc+zPQADSNL04PbxxYWaAikao+S7b6AhY7JHKRItzlIdecyxOmZzvfXrqGJ7uYSbqmfg==", + "version": "0.3.5", + "resolved": "https://registry.npmjs.org/@openfeature/flagd-web-provider/-/flagd-web-provider-0.3.5.tgz", + "integrity": "sha512-zGL/NQqB4pVlBlYjCJHT2C1yc5AnfOaA8dOaP2lMyijcePLcCZ6Y2E+3vblD4FIlPefaTaiZi6I09jfckvaA0A==", "dev": true, "dependencies": { - "@bufbuild/connect": "0.8.5", - "@bufbuild/connect-web": "0.8.5", - "@bufbuild/protobuf": "1.2.0" + "@bufbuild/connect": "0.11.0", + "@bufbuild/connect-web": "0.11.0", + "@bufbuild/protobuf": "1.2.1" }, "peerDependencies": { "@openfeature/web-sdk": "*" @@ -1285,32 +1285,19 @@ "resolved": "packages/client", "link": true }, - "node_modules/@protobuf-ts/grpc-transport": { - "version": "2.8.3", - "resolved": "https://registry.npmjs.org/@protobuf-ts/grpc-transport/-/grpc-transport-2.8.3.tgz", - "integrity": "sha512-rswUuVDEK92uJEGqMNLIK2u997wO8kkBruT2D1D9yWhx855l0UF6rJ6JEvPviyltkeb9aa1vMTZM3tQdzFINEw==", - "dev": true, - "dependencies": { - "@protobuf-ts/runtime": "^2.8.3", - "@protobuf-ts/runtime-rpc": "^2.8.3" - }, - "peerDependencies": { - "@grpc/grpc-js": "^1.6.0" - } - }, "node_modules/@protobuf-ts/runtime": { - "version": "2.8.3", - "resolved": "https://registry.npmjs.org/@protobuf-ts/runtime/-/runtime-2.8.3.tgz", - "integrity": "sha512-nVL1s5wWpF6U+wtWTEWfUPD9Ockckj+fHqhzgm41CKV4Oma3D/2M6tzqOQ+wrm4GZu1bt+s6f43feNnar6fhjA==", + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/@protobuf-ts/runtime/-/runtime-2.9.0.tgz", + "integrity": "sha512-DnJtLZFMglADv9jiawBmg0RaET4a6fNSAaAHuU6Ovw2ZhJ23ehIY0NrlYLS0Lc8HRH0S5rkLI1QF1A1h8uKUnA==", "dev": true }, "node_modules/@protobuf-ts/runtime-rpc": { - "version": "2.8.3", - "resolved": "https://registry.npmjs.org/@protobuf-ts/runtime-rpc/-/runtime-rpc-2.8.3.tgz", - "integrity": "sha512-Tb6nuevgezjGNnT8WF+aveGWeI5xeAbNpySd/nzknKx6ynepXdlNz5cN0xIADeOgDJHo/05Ka+vZ5ZI33tz2Og==", + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/@protobuf-ts/runtime-rpc/-/runtime-rpc-2.9.0.tgz", + "integrity": "sha512-h2S86+u2cNJACjzbBubbjKmNrnXkTmJ9yJHW4t7ZVS9xdV1C68blsVIh3Su4ghR8Nlj0459FuIUTsjWR8hA/7g==", "dev": true, "dependencies": { - "@protobuf-ts/runtime": "^2.8.3" + "@protobuf-ts/runtime": "^2.9.0" } }, "node_modules/@protobufjs/aspromise": { @@ -9468,7 +9455,8 @@ "version": "4.3.0", "resolved": "https://registry.npmjs.org/lodash.camelcase/-/lodash.camelcase-4.3.0.tgz", "integrity": "sha512-TwuEnCnxbc3rAvhf/LbG7tJUDzhqXyFnv3dtzLOPgCG/hODL7WFnsbwktkD7yUV0RrreP/l1PALq/YSg6VvjlA==", - "dev": true + "dev": true, + "peer": true }, "node_modules/lodash.memoize": { "version": "4.1.2", @@ -10235,6 +10223,7 @@ "integrity": "sha512-AT+RJgD2sH8phPmCf7OUZR8xGdcJRga4+1cOaXJ64hvcSkVhNcRHOwIxUatPH15+nj59WAGTDv3LSGZPEQbJaQ==", "dev": true, "hasInstallScript": true, + "peer": true, "dependencies": { "@protobufjs/aspromise": "^1.1.2", "@protobufjs/base64": "^1.1.2", @@ -10257,7 +10246,8 @@ "version": "5.2.3", "resolved": "https://registry.npmjs.org/long/-/long-5.2.3.tgz", "integrity": "sha512-lcHwpNoggQTObv5apGNCTdJrO69eHOZMi4BNC+rTLER8iHAqGrUVeLh/irVIM7zTw2bOXA8T6uNPeujwOLg/2Q==", - "dev": true + "dev": true, + "peer": true }, "node_modules/psl": { "version": "1.9.0", diff --git a/package.json b/package.json index 6a34095fc..2879ae152 100644 --- a/package.json +++ b/package.json @@ -34,8 +34,8 @@ "node": ">=16" }, "devDependencies": { - "@openfeature/flagd-provider": "^0.7.6", - "@openfeature/flagd-web-provider": "^0.3.4", + "@openfeature/flagd-provider": "^0.8.1", + "@openfeature/flagd-web-provider": "^0.3.5", "@rollup/plugin-alias": "^5.0.0", "@rollup/plugin-typescript": "^11.0.0", "@types/events": "^3.0.0", diff --git a/packages/client/e2e/step-definitions/evaluation.spec.ts b/packages/client/e2e/step-definitions/evaluation.spec.ts index 9cc1b6a6b..5bcc36bbf 100644 --- a/packages/client/e2e/step-definitions/evaluation.spec.ts +++ b/packages/client/e2e/step-definitions/evaluation.spec.ts @@ -17,16 +17,13 @@ const client = OpenFeature.getClient(); const givenAnOpenfeatureClientIsRegisteredWithCacheDisabled = ( given: (stepMatcher: string, stepDefinitionCallback: () => void) => void ) => { - // TODO: when the FlagdProvider is updated to support caching, we may need to disable it here for this test to work as expected. given('a provider is registered with cache disabled', () => undefined); }; defineFeature(feature, (test) => { beforeAll((done) => { client.addHandler(ProviderEvents.Ready, async () => { - setTimeout(() => { - done(); // TODO remove this once flagd provider properly implements readiness (for now, we add a 2s wait). - }, 2000); + done(); }); }); diff --git a/packages/client/e2e/step-definitions/setup.ts b/packages/client/e2e/step-definitions/setup.ts index abb3b5e5e..5f2256731 100644 --- a/packages/client/e2e/step-definitions/setup.ts +++ b/packages/client/e2e/step-definitions/setup.ts @@ -2,20 +2,18 @@ import assert from 'assert'; import { OpenFeature } from '../..'; import { FlagdWebProvider } from '@openfeature/flagd-web-provider'; -const FLAGD_NAME = 'flagd-web'; +const FLAGD_WEB_NAME = 'flagd-web'; // register the flagd provider before the tests. console.log('Setting flagd web provider...'); -OpenFeature.setProvider( - new FlagdWebProvider({ - host: 'localhost', - port: 8013, - tls: false, - maxRetries: -1, - }) -); +OpenFeature.setProvider(new FlagdWebProvider({ + host: 'localhost', + port: 8013, + tls: false, + maxRetries: -1, +})); assert( - OpenFeature.providerMetadata.name === FLAGD_NAME, - new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`) + OpenFeature.providerMetadata.name === FLAGD_WEB_NAME, + new Error(`Expected ${FLAGD_WEB_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`) ); console.log('flagd web provider configured!'); diff --git a/packages/client/test/open-feature.spec.ts b/packages/client/test/open-feature.spec.ts index 0ac27db9a..ec412def4 100644 --- a/packages/client/test/open-feature.spec.ts +++ b/packages/client/test/open-feature.spec.ts @@ -1,18 +1,19 @@ -import { OpenFeatureClient } from '../src'; -import { OpenFeature, OpenFeatureAPI } from '../src'; -import { Provider } from '../src'; - -const MOCK_PROVIDER: Provider = { - metadata: { - name: 'mock-events-success', - }, - initialize: jest.fn(() => { - return Promise.resolve('started'); - }), - onClose: jest.fn(() => { - return Promise.resolve('closed'); - }), -} as unknown as Provider; +import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src'; + +const mockProvider = (initialStatus?: ProviderStatus) => { + return { + metadata: { + name: 'mock-events-success', + }, + status: initialStatus || ProviderStatus.NOT_READY, + initialize: jest.fn(() => { + return Promise.resolve('started'); + }), + onClose: jest.fn(() => { + return Promise.resolve('closed'); + }), + } as unknown as Provider; +}; describe('OpenFeature', () => { afterEach(() => { @@ -34,39 +35,50 @@ describe('OpenFeature', () => { describe('Requirement 1.1.2.2', () => { it('MUST invoke the `initialize` function on the newly registered provider before using it to resolve flag values', () => { - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); - expect(MOCK_PROVIDER.initialize).toHaveBeenCalled(); + expect(provider.initialize).toHaveBeenCalled(); + }); + + it('should not invoke initialze function if the provider is not in state NOT_READY', () => { + const provider = mockProvider(ProviderStatus.READY); + OpenFeature.setProvider(provider); + expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); + expect(provider.initialize).not.toHaveBeenCalled(); }); }); describe('Requirement 1.1.2.3', () => { it("MUST invoke the `shutdown` function on the previously registered provider once it's no longer being used to resolve flag values.", () => { const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); OpenFeature.setProvider(fakeProvider); - expect(MOCK_PROVIDER.onClose).toHaveBeenCalled(); + expect(provider.onClose).toHaveBeenCalled(); }); }); }); describe('Requirement 1.1.3', () => { it('should set the default provider if no name is provided', () => { - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); const client = OpenFeature.getClient(); - expect(client.metadata.providerMetadata.name).toEqual(MOCK_PROVIDER.metadata.name); + expect(client.metadata.providerMetadata.name).toEqual(provider.metadata.name); }); it('should not change named providers when setting a new default provider', () => { const name = 'my-client'; const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); OpenFeature.setProvider(name, fakeProvider); const unnamedClient = OpenFeature.getClient(); const namedClient = OpenFeature.getClient(name); - expect(unnamedClient.metadata.providerMetadata.name).toEqual(MOCK_PROVIDER.metadata.name); + expect(unnamedClient.metadata.providerMetadata.name).toEqual(provider.metadata.name); expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name); }); @@ -74,7 +86,8 @@ describe('OpenFeature', () => { const name = 'my-client'; const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); const namedClient = OpenFeature.getClient(name); OpenFeature.setProvider(name, fakeProvider); @@ -82,8 +95,8 @@ describe('OpenFeature', () => { }); it('should close a provider if it is replaced and no other client uses it', async () => { - const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() }; - const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() }; + const provider1 = { ...mockProvider(), onClose: jest.fn() }; + const provider2 = { ...mockProvider(), onClose: jest.fn() }; OpenFeature.setProvider('client1', provider1); expect(provider1.onClose).not.toHaveBeenCalled(); @@ -92,7 +105,7 @@ describe('OpenFeature', () => { }); it('should not close provider if it is used by another client', async () => { - const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() }; + const provider1 = { ...mockProvider(), onClose: jest.fn() }; OpenFeature.setProvider('client1', provider1); OpenFeature.setProvider('client2', provider1); @@ -153,37 +166,40 @@ describe('OpenFeature', () => { describe('Requirement 1.6.1', () => { it('MUST define a `shutdown` function, which, when called, must call the respective `shutdown` function on the active provider', async () => { - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); await OpenFeature.close(); - expect(MOCK_PROVIDER.onClose).toHaveBeenCalled(); + expect(provider.onClose).toHaveBeenCalled(); }); it('runs the shutdown function on all providers for all clients', async () => { - OpenFeature.setProvider(MOCK_PROVIDER); - OpenFeature.setProvider('client1', { ...MOCK_PROVIDER }); - OpenFeature.setProvider('client2', { ...MOCK_PROVIDER }); + const provider = mockProvider(); + OpenFeature.setProvider(provider); + OpenFeature.setProvider('client1', { ...provider }); + OpenFeature.setProvider('client2', { ...provider }); - expect(OpenFeature.providerMetadata.name).toBe(MOCK_PROVIDER.metadata.name); + expect(OpenFeature.providerMetadata.name).toBe(provider.metadata.name); await OpenFeature.close(); - expect(MOCK_PROVIDER.onClose).toHaveBeenCalledTimes(3); + expect(provider.onClose).toHaveBeenCalledTimes(3); }); it('runs the shutdown function on all providers for all clients even if some fail', async () => { - const failingClose = () => { + const failingClose = jest.fn(() => { throw Error(); - }; - - OpenFeature.setProvider({ - ...MOCK_PROVIDER, - onClose: failingClose, }); - OpenFeature.setProvider('client1', { ...MOCK_PROVIDER, onClose: failingClose }); - OpenFeature.setProvider('client2', { ...MOCK_PROVIDER, onClose: jest.fn() }); - expect(OpenFeature.providerMetadata.name).toBe(MOCK_PROVIDER.metadata.name); + const provider1 = { ...mockProvider(), onClose: failingClose }; + const provider2 = { ...mockProvider(), onClose: failingClose }; + const provider3 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('client1', provider2); + OpenFeature.setProvider('client2', provider3); + + expect(OpenFeature.providerMetadata.name).toBe(provider1.metadata.name); await OpenFeature.close(); - expect(MOCK_PROVIDER.onClose).toHaveBeenCalledTimes(3); + expect(provider3.onClose).toHaveBeenCalled(); }); }); }); diff --git a/packages/server/e2e/step-definitions/evaluation.spec.ts b/packages/server/e2e/step-definitions/evaluation.spec.ts index 11d62a9d1..9cb4c254e 100644 --- a/packages/server/e2e/step-definitions/evaluation.spec.ts +++ b/packages/server/e2e/step-definitions/evaluation.spec.ts @@ -25,9 +25,7 @@ const givenAnOpenfeatureClientIsRegisteredWithCacheDisabled = ( defineFeature(feature, (test) => { beforeAll((done) => { client.addHandler(ProviderEvents.Ready, async () => { - setTimeout(() => { - done(); // TODO remove this once flagd provider properly implements readiness (for now, we add a 2s wait). - }, 2000); + done(); }); }); diff --git a/packages/server/test/open-feature.spec.ts b/packages/server/test/open-feature.spec.ts index 21f7c6110..ec412def4 100644 --- a/packages/server/test/open-feature.spec.ts +++ b/packages/server/test/open-feature.spec.ts @@ -1,17 +1,19 @@ -import { NOOP_TRANSACTION_CONTEXT_PROPAGATOR, TransactionContextPropagator } from '@openfeature/shared'; -import { OpenFeature, OpenFeatureAPI, NOOP_PROVIDER, OpenFeatureClient, Provider } from '../src'; - -const MOCK_PROVIDER: Provider = { - metadata: { - name: 'mock', - }, - initialize: jest.fn(() => { - return Promise.resolve('started'); - }), - onClose: jest.fn(() => { - return Promise.resolve('closed'); - }), -} as unknown as Provider; +import { OpenFeature, OpenFeatureAPI, OpenFeatureClient, Provider, ProviderStatus } from '../src'; + +const mockProvider = (initialStatus?: ProviderStatus) => { + return { + metadata: { + name: 'mock-events-success', + }, + status: initialStatus || ProviderStatus.NOT_READY, + initialize: jest.fn(() => { + return Promise.resolve('started'); + }), + onClose: jest.fn(() => { + return Promise.resolve('closed'); + }), + } as unknown as Provider; +}; describe('OpenFeature', () => { afterEach(() => { @@ -33,39 +35,50 @@ describe('OpenFeature', () => { describe('Requirement 1.1.2.2', () => { it('MUST invoke the `initialize` function on the newly registered provider before using it to resolve flag values', () => { - OpenFeature.setProvider(MOCK_PROVIDER); - expect(OpenFeature.providerMetadata.name).toBe(MOCK_PROVIDER.metadata.name); - expect(MOCK_PROVIDER.initialize).toHaveBeenCalled(); + const provider = mockProvider(); + OpenFeature.setProvider(provider); + expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); + expect(provider.initialize).toHaveBeenCalled(); + }); + + it('should not invoke initialze function if the provider is not in state NOT_READY', () => { + const provider = mockProvider(ProviderStatus.READY); + OpenFeature.setProvider(provider); + expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); + expect(provider.initialize).not.toHaveBeenCalled(); }); }); describe('Requirement 1.1.2.3', () => { it("MUST invoke the `shutdown` function on the previously registered provider once it's no longer being used to resolve flag values.", () => { const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); OpenFeature.setProvider(fakeProvider); - expect(MOCK_PROVIDER.onClose).toHaveBeenCalled(); + expect(provider.onClose).toHaveBeenCalled(); }); }); }); describe('Requirement 1.1.3', () => { it('should set the default provider if no name is provided', () => { - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); const client = OpenFeature.getClient(); - expect(client.metadata.providerMetadata.name).toEqual(MOCK_PROVIDER.metadata.name); + expect(client.metadata.providerMetadata.name).toEqual(provider.metadata.name); }); it('should not change named providers when setting a new default provider', () => { const name = 'my-client'; const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); OpenFeature.setProvider(name, fakeProvider); const unnamedClient = OpenFeature.getClient(); const namedClient = OpenFeature.getClient(name); - expect(unnamedClient.metadata.providerMetadata.name).toEqual(MOCK_PROVIDER.metadata.name); + expect(unnamedClient.metadata.providerMetadata.name).toEqual(provider.metadata.name); expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name); }); @@ -73,7 +86,8 @@ describe('OpenFeature', () => { const name = 'my-client'; const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(MOCK_PROVIDER); + const provider = mockProvider(); + OpenFeature.setProvider(provider); const namedClient = OpenFeature.getClient(name); OpenFeature.setProvider(name, fakeProvider); @@ -81,8 +95,8 @@ describe('OpenFeature', () => { }); it('should close a provider if it is replaced and no other client uses it', async () => { - const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() }; - const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() }; + const provider1 = { ...mockProvider(), onClose: jest.fn() }; + const provider2 = { ...mockProvider(), onClose: jest.fn() }; OpenFeature.setProvider('client1', provider1); expect(provider1.onClose).not.toHaveBeenCalled(); @@ -91,7 +105,7 @@ describe('OpenFeature', () => { }); it('should not close provider if it is used by another client', async () => { - const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() }; + const provider1 = { ...mockProvider(), onClose: jest.fn() }; OpenFeature.setProvider('client1', provider1); OpenFeature.setProvider('client2', provider1); @@ -135,58 +149,57 @@ describe('OpenFeature', () => { }); it('should return a client with the provider bound to the name', () => { - const name = 'my-client'; + const name = 'my-named-client'; const fakeProvider = { metadata: { name: 'test' } } as unknown as Provider; - OpenFeature.setProvider(name, fakeProvider); + const namedClient = OpenFeature.getClient(name); expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name); }); + + it('should be chainable', () => { + const client = OpenFeature.addHooks().clearHooks().setLogger(console).getClient(); + expect(client).toBeDefined(); + }); }); - it('should be chainable', () => { - const client = OpenFeature.addHooks() - .clearHooks() - .setContext({}) - .setLogger(console) - .setProvider(NOOP_PROVIDER) - .getClient(); + describe('Requirement 1.6.1', () => { + it('MUST define a `shutdown` function, which, when called, must call the respective `shutdown` function on the active provider', async () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + expect(OpenFeature.providerMetadata.name).toBe('mock-events-success'); + await OpenFeature.close(); + expect(provider.onClose).toHaveBeenCalled(); + }); - expect(client).toBeDefined(); - }); + it('runs the shutdown function on all providers for all clients', async () => { + const provider = mockProvider(); + OpenFeature.setProvider(provider); + OpenFeature.setProvider('client1', { ...provider }); + OpenFeature.setProvider('client2', { ...provider }); - describe('transaction context safety', () => { - it('invalid getTransactionContext is not registered', () => { - OpenFeature.setTransactionContextPropagator({ - setTransactionContext: () => undefined, - } as unknown as TransactionContextPropagator); - expect(OpenFeature['_transactionContextPropagator']).toBe(NOOP_TRANSACTION_CONTEXT_PROPAGATOR); - }); - - it('invalid setTransactionContext is not registered', () => { - OpenFeature.setTransactionContextPropagator({ - getTransactionContext: () => Object.assign({}), - } as unknown as TransactionContextPropagator); - expect(OpenFeature['_transactionContextPropagator']).toBe(NOOP_TRANSACTION_CONTEXT_PROPAGATOR); - }); - - it('throwing getTransactionContext defaults', async () => { - const mockPropagator: TransactionContextPropagator = { - getTransactionContext: jest.fn(() => { - throw Error('Transaction context error!'); - }), - setTransactionContext: jest.fn((context, callback) => { - callback(); - }), - }; - - OpenFeature.setTransactionContextPropagator(mockPropagator); - expect(OpenFeature['_transactionContextPropagator']).toBe(mockPropagator); - const client = OpenFeature.getClient(); - const result = await client.getBooleanValue('test', true); - expect(result).toEqual(true); - expect(mockPropagator.getTransactionContext).toHaveBeenCalled(); + expect(OpenFeature.providerMetadata.name).toBe(provider.metadata.name); + await OpenFeature.close(); + expect(provider.onClose).toHaveBeenCalledTimes(3); + }); + + it('runs the shutdown function on all providers for all clients even if some fail', async () => { + const failingClose = jest.fn(() => { + throw Error(); + }); + + const provider1 = { ...mockProvider(), onClose: failingClose }; + const provider2 = { ...mockProvider(), onClose: failingClose }; + const provider3 = mockProvider(); + + OpenFeature.setProvider(provider1); + OpenFeature.setProvider('client1', provider2); + OpenFeature.setProvider('client2', provider3); + + expect(OpenFeature.providerMetadata.name).toBe(provider1.metadata.name); + await OpenFeature.close(); + expect(provider3.onClose).toHaveBeenCalled(); }); }); }); diff --git a/packages/shared/src/open-feature.ts b/packages/shared/src/open-feature.ts index d4c077615..51139b5a0 100644 --- a/packages/shared/src/open-feature.ts +++ b/packages/shared/src/open-feature.ts @@ -4,7 +4,7 @@ import { InternalEventEmitter } from './events/open-feature-event-emitter'; import { isDefined } from './filter'; import { EvaluationLifeCycle, Hook } from './hooks'; import { DefaultLogger, Logger, ManageLogger, SafeLogger } from './logger'; -import { CommonProvider, ProviderMetadata } from './provider'; +import { CommonProvider, ProviderMetadata, ProviderStatus } from './provider'; import { ManageTransactionContextPropagator, NOOP_TRANSACTION_CONTEXT_PROPAGATOR, @@ -124,7 +124,15 @@ export abstract class OpenFeatureCommonAPI

{