Skip to content

Commit

Permalink
fix: only initialize NOT_READY providers (#507)
Browse files Browse the repository at this point in the history
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:
- open-feature/js-sdk-contrib#488
- open-feature/js-sdk-contrib#489 
- open-feature/js-sdk-contrib#490

Fixes: #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 <[email protected]>
  • Loading branch information
toddbaert authored Jul 31, 2023
1 parent 8734b77 commit 5e320ae
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 197 deletions.
114 changes: 52 additions & 62 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 1 addition & 4 deletions packages/client/e2e/step-definitions/evaluation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});

Expand Down
20 changes: 9 additions & 11 deletions packages/client/e2e/step-definitions/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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!');
Loading

0 comments on commit 5e320ae

Please sign in to comment.