diff --git a/README.md b/README.md index 67ac9e583..08ab9f5ad 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![Coveralls](https://img.shields.io/coveralls/optimizely/javascript-sdk.svg)](https://coveralls.io/github/optimizely/javascript-sdk) [![license](https://img.shields.io/github/license/optimizely/javascript-sdk.svg)](https://choosealicense.com/licenses/apache-2.0/) -This repository houses the JavaScript SDK for use with Optimizely Feature Experimentation and Optimizely Full Stack (legacy). The SDK now features a modular architecture for greater flexibility and control. If you're upgrading from a previous version, see our [Migration Guide](MIGRATION.md). +This is the official JavaScript and TypeScript SDK for use with Optimizely Feature Experimentation and Optimizely Full Stack (legacy). The SDK now features a modular architecture for greater flexibility and control. If you're upgrading from a previous version, see our [Migration Guide](MIGRATION.md). Optimizely Feature Experimentation is an A/B testing and feature management tool for product development teams that enables you to experiment at every step. Using Optimizely Feature Experimentation allows for every feature on your roadmap to be an opportunity to discover hidden insights. Learn more at [Optimizely.com](https://www.optimizely.com/products/experiment/feature-experimentation/), or see the [developer documentation](https://docs.developers.optimizely.com/feature-experimentation/docs/introduction). @@ -16,9 +16,8 @@ Optimizely Rollouts is [free feature flags](https://www.optimizely.com/free-feat ## Get Started -> For **Browser** applications, refer to the [JavaScript SDK's developer documentation](https://docs.developers.optimizely.com/feature-experimentation/docs/javascript-sdk) for detailed instructions on getting started with using the SDK within client-side applications. +> Refer to the [JavaScript SDK's developer documentation](https://docs.developers.optimizely.com/feature-experimentation/docs/javascript-sdk) for detailed instructions on getting started with using the SDK. -> For **Node.js** applications, refer to the [JavaScript (Node) variant of the developer documentation](https://docs.developers.optimizely.com/feature-experimentation/docs/javascript-node-sdk). > For **Edge Functions**, we provide starter kits that utilize the Optimizely JavaScript SDK for the following platforms: > @@ -28,7 +27,7 @@ Optimizely Rollouts is [free feature flags](https://www.optimizely.com/free-feat > - [Fastly Compute@Edge](https://github.com/optimizely/fastly-compute-starter-kit) > - [Vercel Edge Middleware](https://github.com/optimizely/vercel-examples/tree/main/edge-middleware/feature-flag-optimizely) > -> Note: We recommend using the **Lite** version of the sdk for edge platforms. These starter kits also use the **Lite** variant of the JavaScript SDK which excludes the datafile manager and event processor packages. +> Note: We recommend using the **Lite** entrypoint (for version < 6) / **Universal** entrypoint (for version >=6) of the sdk for edge platforms. These starter kits also use the **Lite** variant of the JavaScript SDK. ### Prerequisites @@ -73,7 +72,7 @@ import optimizely from 'npm:@optimizely/optimizely-sdk'; ## Use the JavaScript SDK -See the [Optimizely Feature Experimentation developer documentation for JavaScript](https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/javascript-sdk) to learn how to set up your first JavaScript project and use the SDK for client-side applications. +See the [JavaScript SDK's developer documentation](https://docs.developers.optimizely.com/feature-experimentation/docs/javascript-sdk) to learn how to set up your first JavaScript project using the SDK. The SDK uses a modular architecture with dedicated components for project configuration, event processing, and more. The examples below demonstrate the recommended initialization pattern. @@ -121,17 +120,19 @@ optimizelyClient }); ``` -### Initialization (Using HTML) +### Initialization (Using HTML script tag) The package has different entry points for different environments. The browser entry point is an ES module, which can be used with an appropriate bundler like **Webpack** or **Rollup**. Additionally, for ease of use during initial evaluations you can include a standalone umd bundle of the SDK in your web page by fetching it from [unpkg](https://unpkg.com/): ```html - + - + ``` +> ⚠️ **Warning**: Always include a specific version number (such as @6) when using CDN URLs like the `unpkg` example above. If you use a URL without a version, your application may automatically receive breaking changes when a new major version is released, which can lead to unexpected issues. + When evaluated, that bundle assigns the SDK's exports to `window.optimizelySdk`. If you wish to use the asset locally (for example, if unpkg is down), you can find it in your local copy of the package at dist/optimizely.browser.umd.min.js. We do not recommend using this method in production settings as it introduces a third-party performance dependency. As `window.optimizelySdk` should be a global variable at this point, you can continue to use it like so: @@ -175,21 +176,49 @@ As `window.optimizelySdk` should be a global variable at this point, you can con ``` -Regarding `EventDispatcher`s: In Node.js environment, the default `EventDispatcher` is powered by the [`http/s`](https://nodejs.org/api/http.html) module. +### Closing the SDK Instance + +Depending on the sdk configuration, the client instance might schedule tasks in the background. If the instance has background tasks scheduled, +then the instance will not be garbage collected even though there are no more references to the instance in the code. (Basically, the background tasks will still hold references to the instance). Therefore, it's important to close it to properly clean up resources. + +```javascript +// Close the Optimizely client when you're done using it +optimizelyClient.close() +``` +Using the following settings will cause background tasks to be scheduled + +- Polling Datafile Manager +- Batch Event Processor with batchSize > 1 +- ODP manager with eventBatchSize > 1 + + + +> ⚠️ **Warning**: Failure to close SDK instances when they're no longer needed may result in memory leaks. This is particularly important for applications that create multiple instances over time. For some environment like SSR applications, it might not be convenient to close each instance, in which case, the `disposable` option of `createInstance` can be used to disable all background tasks on the server side, allowing the instance to be garbage collected. + + +## Special Notes + +### Migration Guides + +If you're updating your SDK version, please check the appropriate migration guide: + +- **Migrating from 5.x or lower to 6.x**: See our [Migration Guide](MIGRATION.md) for detailed instructions on updating to the new modular architecture. +- **Migrating from 4.x or lower to 5.x**: Please refer to the [Changelog](CHANGELOG.md#500---january-19-2024) for details on these breaking changes. ## SDK Development ### Unit Tests -There is a mix of testing paradigms used within the JavaScript SDK which include Mocha, Chai, Karma, and Jest, indicated by their respective `*.tests.js` and `*.spec.ts` filenames. +There is a mix of testing paradigms used within the JavaScript SDK which include Mocha, Chai, Karma, and Vitest, indicated by their respective `*.tests.js` and `*.spec.ts` filenames. When contributing code to the SDK, aim to keep the percentage of code test coverage at the current level ([![Coveralls](https://img.shields.io/coveralls/optimizely/javascript-sdk.svg)](https://coveralls.io/github/optimizely/javascript-sdk)) or above. -To run unit tests on the primary JavaScript SDK package source code, you can take the following steps: +To run unit tests, you can take the following steps: -1. On your command line or terminal, navigate to the `~/javascript-sdk/packages/optimizely-sdk` directory. -2. Ensure that you have run `npm install` to install all project dependencies. -3. Run `npm test` to run all test files. +1. Ensure that you have run `npm install` to install all project dependencies. +2. Run `npm test` to run all test files. +3. Run `npm run test-vitest` to run only tests written using Vitest. +4. Run `npm run test-mocha` to run only tests written using Mocha. 4. (For cross-browser testing) Run `npm run test-xbrowser` to run tests in many browsers via BrowserStack. 5. Resolve any tests that fail before continuing with your contribution. @@ -215,14 +244,6 @@ npm run test-xbrowser For more information regarding contributing to the Optimizely JavaScript SDK, please read [Contributing](CONTRIBUTING.md). -## Special Notes - -### Migration Guides - -If you're updating your SDK version, please check the appropriate migration guide: - -- **Migrating from 5.x to 6.x**: See our [Migration Guide](MIGRATION.md) for detailed instructions on updating to the new modular architecture. -- **Migrating from 4.x to 5.x**: Please refer to the [Changelog](CHANGELOG.md#500---january-19-2024) for details on these breaking changes. ### Feature Management access @@ -232,7 +253,7 @@ To access the Feature Management configuration in the Optimizely dashboard, plea `@optimizely/optimizely-sdk` is developed and maintained by [Optimizely](https://optimizely.com) and many [contributors](https://github.com/optimizely/javascript-sdk/graphs/contributors). If you're interested in learning more about what Optimizely Feature Experimentation can do for your company you can visit the [official Optimizely Feature Experimentation product page here](https://www.optimizely.com/products/experiment/feature-experimentation/) to learn more. -First-party code (under `packages/optimizely-sdk/lib/`, `packages/datafile-manager/lib`, `packages/datafile-manager/src`, `packages/datafile-manager/__test__`, `packages/event-processor/src`, `packages/event-processor/__tests__`, `packages/logging/src`, `packages/logging/__tests__`, `packages/utils/src`, `packages/utils/__tests__`) is copyright Optimizely, Inc. and contributors, licensed under Apache 2.0. +First-party code (under `lib/`) is copyright Optimizely, Inc., licensed under Apache 2.0. ### Other Optimizely SDKs diff --git a/lib/core/bucketer/index.spec.ts b/lib/core/bucketer/index.spec.ts index b3aac5158..942295356 100644 --- a/lib/core/bucketer/index.spec.ts +++ b/lib/core/bucketer/index.spec.ts @@ -80,6 +80,7 @@ describe('excluding groups', () => { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: mockLogger, + validateEntity: true, }; vi.spyOn(bucketValueGenerator, 'generateBucketValue') @@ -127,6 +128,7 @@ describe('including groups: random', () => { groupIdMap: configObj.groupIdMap, logger: mockLogger, userId: 'testUser', + validateEntity: true, }; }); @@ -228,6 +230,7 @@ describe('including groups: overlapping', () => { groupIdMap: configObj.groupIdMap, logger: mockLogger, userId: 'testUser', + validateEntity: true, }; }); @@ -280,6 +283,7 @@ describe('bucket value falls into empty traffic allocation ranges', () => { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: mockLogger, + validateEntity: true, }; }); @@ -329,6 +333,7 @@ describe('traffic allocation has invalid variation ids', () => { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: mockLogger, + validateEntity: true, }; }); @@ -359,6 +364,7 @@ describe('testBucketWithBucketingId', () => { variationIdMap: configObj.variationIdMap, experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, + validateEntity: true, }; }); diff --git a/lib/core/bucketer/index.tests.js b/lib/core/bucketer/index.tests.js index 0bdf62f4a..a1e046088 100644 --- a/lib/core/bucketer/index.tests.js +++ b/lib/core/bucketer/index.tests.js @@ -74,6 +74,7 @@ describe('lib/core/bucketer', function () { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: createdLogger, + validateEntity: true, }; sinon .stub(bucketValueGenerator, 'generateBucketValue') @@ -115,6 +116,7 @@ describe('lib/core/bucketer', function () { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: createdLogger, + validateEntity: true, }; bucketerStub = sinon.stub(bucketValueGenerator, 'generateBucketValue'); }); @@ -135,6 +137,7 @@ describe('lib/core/bucketer', function () { groupIdMap: configObj.groupIdMap, logger: createdLogger, userId: 'testUser', + validateEntity: true, }; }); @@ -225,6 +228,7 @@ describe('lib/core/bucketer', function () { groupIdMap: configObj.groupIdMap, logger: createdLogger, userId: 'testUser', + validateEntity: true, }; }); @@ -269,6 +273,7 @@ describe('lib/core/bucketer', function () { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: createdLogger, + validateEntity: true, }; }); @@ -316,6 +321,7 @@ describe('lib/core/bucketer', function () { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: createdLogger, + validateEntity: true, }; }); @@ -365,6 +371,7 @@ describe('lib/core/bucketer', function () { experimentIdMap: configObj.experimentIdMap, groupIdMap: configObj.groupIdMap, logger: createdLogger, + validateEntity: true, }; }); diff --git a/lib/core/bucketer/index.ts b/lib/core/bucketer/index.ts index 686f49abd..e31c8df4b 100644 --- a/lib/core/bucketer/index.ts +++ b/lib/core/bucketer/index.ts @@ -56,7 +56,8 @@ export const bucket = function(bucketerParams: BucketerParams): DecisionResponse const decideReasons: DecisionReason[] = []; // Check if user is in a random group; if so, check if user is bucketed into a specific experiment const experiment = bucketerParams.experimentIdMap[bucketerParams.experimentId]; - const groupId = experiment['groupId']; + // Optional chaining skips groupId check for holdout experiments; Holdout experimentId is not in experimentIdMap + const groupId = experiment?.['groupId']; if (groupId) { const group = bucketerParams.groupIdMap[groupId]; if (!group) { @@ -138,17 +139,16 @@ export const bucket = function(bucketerParams: BucketerParams): DecisionResponse ]); const entityId = _findBucket(bucketValue, bucketerParams.trafficAllocationConfig); - if (entityId !== null) { - if (!bucketerParams.variationIdMap[entityId]) { - if (entityId) { - bucketerParams.logger?.warn(INVALID_VARIATION_ID); - decideReasons.push([INVALID_VARIATION_ID]); - } - return { - result: null, - reasons: decideReasons, - }; + + if (bucketerParams.validateEntity && entityId !== null && !bucketerParams.variationIdMap[entityId]) { + if (entityId) { + bucketerParams.logger?.warn(INVALID_VARIATION_ID); + decideReasons.push([INVALID_VARIATION_ID]); } + return { + result: null, + reasons: decideReasons, + }; } return { diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 975653611..9fc9d89a1 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -13,25 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { describe, it, expect, vi, MockInstance, beforeEach } from 'vitest'; +import { describe, it, expect, vi, MockInstance, beforeEach, afterEach } from 'vitest'; import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.'; import { getMockLogger } from '../../tests/mock/mock_logger'; import OptimizelyUserContext from '../../optimizely_user_context'; import { bucket } from '../bucketer'; import { getTestProjectConfig, getTestProjectConfigWithFeatures } from '../../tests/test_data'; import { createProjectConfig, ProjectConfig } from '../../project_config/project_config'; -import { BucketerParams, Experiment, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types'; +import { BucketerParams, Experiment, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types'; import { CONTROL_ATTRIBUTES, DECISION_SOURCES } from '../../utils/enums'; import { getDecisionTestDatafile } from '../../tests/decision_test_datafile'; import { Value } from '../../utils/promise/operation_value'; - import { USER_HAS_NO_FORCED_VARIATION, VALID_BUCKETING_ID, SAVED_USER_VARIATION, SAVED_VARIATION_NOT_FOUND, } from 'log_message'; - import { EXPERIMENT_NOT_RUNNING, RETURNING_STORED_VARIATION, @@ -48,7 +46,6 @@ import { NO_ROLLOUT_EXISTS, USER_MEETS_CONDITIONS_FOR_TARGETING_RULE, } from '../decision_service/index'; - import { BUCKETING_ID_NOT_STRING, USER_PROFILE_LOOKUP_ERROR, USER_PROFILE_SAVE_ERROR } from 'error_message'; type MockLogger = ReturnType; @@ -117,11 +114,74 @@ vi.mock('../bucketer', () => ({ bucket: mockBucket, })); +// Mock the feature toggle for holdout tests +const mockHoldoutToggle = vi.hoisted(() => vi.fn()); + +vi.mock('../../feature_toggle', () => ({ + holdout: mockHoldoutToggle, +})); + + const cloneDeep = (d: any) => JSON.parse(JSON.stringify(d)); const testData = getTestProjectConfig(); const testDataWithFeatures = getTestProjectConfigWithFeatures(); +// Utility function to create test datafile with holdout configurations +const getHoldoutTestDatafile = () => { + const datafile = getDecisionTestDatafile(); + + // Add holdouts to the datafile + datafile.holdouts = [ + { + id: 'holdout_running_id', + key: 'holdout_running', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: ['4001'], // age_22 audience + audienceConditions: ['or', '4001'], + variations: [ + { + id: 'holdout_variation_running_id', + key: 'holdout_variation_running', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_running_id', + endOfRange: 5000 + } + ] + }, + { + id: "holdout_not_bucketed_id", + key: "holdout_not_bucketed", + status: "Running", + includedFlags: [], + excludedFlags: [], + audienceIds: ['4002'], + audienceConditions: ['or', '4002'], + variations: [ + { + id: 'holdout_not_bucketed_variation_id', + key: 'holdout_not_bucketed_variation', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_not_bucketed_variation_id', + endOfRange: 0, + } + ] + }, + ]; + + return datafile; +}; + const verifyBucketCall = ( call: number, projectConfig: ProjectConfig, @@ -1841,6 +1901,359 @@ describe('DecisionService', () => { expect(userProfileServiceAsync?.lookup).not.toHaveBeenCalled(); expect(userProfileServiceAsync?.save).not.toHaveBeenCalled(); }); + + describe('holdout', () => { + beforeEach(async() => { + mockHoldoutToggle.mockReturnValue(true); + const actualBucketModule = (await vi.importActual('../bucketer')) as { bucket: typeof bucket }; + mockBucket.mockImplementation(actualBucketModule.bucket); + }); + + it('should return holdout variation when user is bucketed into running holdout', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 20, + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it("should consider global holdout even if local holdout is present", async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + const newEntry = { + id: 'holdout_included_id', + key: 'holdout_included', + status: 'Running', + includedFlags: ['1001'], + excludedFlags: [], + audienceIds: ['4002'], // age_40 audience + audienceConditions: ['or', '4002'], + variations: [ + { + id: 'holdout_variation_included_id', + key: 'holdout_variation_included', + variables: [], + }, + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_included_id', + endOfRange: 5000, + }, + ], + }; + datafile.holdouts = [newEntry, ...datafile.holdouts]; + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 20, // satisfies both global holdout (age_22) and included holdout (age_40) audiences + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it("should consider local holdout if misses global holdout", async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts.push({ + id: 'holdout_included_specific_id', + key: 'holdout_included_specific', + status: 'Running', + includedFlags: ['1001'], + excludedFlags: [], + audienceIds: ['4002'], // age_60 audience (age <= 60) + audienceConditions: ['or', '4002'], + variations: [ + { + id: 'holdout_variation_included_specific_id', + key: 'holdout_variation_included_specific', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_included_specific_id', + endOfRange: 5000 + } + ] + }); + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'test_holdout_user', + attributes: { + age: 50, // Does not satisfy global holdout (age_22, age <= 22) but satisfies included holdout (age_60, age <= 60) + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_included_specific_id'], + variation: config.variationIdMap['holdout_variation_included_specific_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should fallback to experiment when holdout status is not running', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts = datafile.holdouts.map((holdout: Holdout) => { + if(holdout.id === 'holdout_running_id') { + return { + ...holdout, + status: "Draft" + } + } + return holdout; + }); + + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_1'], + variation: config.variationIdMap['5001'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to experiment when user does not meet holdout audience conditions', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 30, // does not satisfy age_22 audience condition for holdout_running + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to experiment when user is not bucketed into holdout traffic', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(getHoldoutTestDatafile()); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 50, + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_2'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should fallback to rollout when no holdout or experiment matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + // Modify the datafile to create proper audience conditions for this test + // Make exp_1 and exp_2 use age conditions that won't match our test user + datafile.audiences = datafile.audiences.map((audience: any) => { + if (audience.id === '4001') { // age_22 + return { + ...audience, + conditions: JSON.stringify(["or", {"match": "exact", "name": "age", "type": "custom_attribute", "value": 22}]) + }; + } + if (audience.id === '4002') { // age_60 + return { + ...audience, + conditions: JSON.stringify(["or", {"match": "exact", "name": "age", "type": "custom_attribute", "value": 60}]) + }; + } + return audience; + }); + + // Make exp_2 use a different audience so it won't conflict with delivery_2 + datafile.experiments = datafile.experiments.map((experiment: any) => { + if (experiment.key === 'exp_2') { + return { + ...experiment, + audienceIds: ['4001'], // Change from 4002 to 4001 (age_22) + audienceConditions: ['or', '4001'] + }; + } + return experiment; + }); + + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 60, // matches audience 4002 (age_60) used by delivery_2, but not experiments + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['delivery_2'], + variation: config.variationIdMap['5005'], + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('should skip holdouts excluded for specific flags', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts = datafile.holdouts.map((holdout: any) => { + if(holdout.id === 'holdout_running_id') { + return { + ...holdout, + excludedFlags: ['1001'] + } + } + return holdout; + }); + + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 15, // satisfies age_22 audience condition (age <= 22) for global holdout, but holdout excludes flag_1 + }, + }); + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['exp_1'], + variation: config.variationIdMap['5001'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); + }); + + it('should handle multiple holdouts and use first matching one', async () => { + const { decisionService } = getDecisionService(); + const datafile = getHoldoutTestDatafile(); + + datafile.holdouts.push({ + id: 'holdout_second_id', + key: 'holdout_second', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], // no audience requirements + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_second_id', + key: 'holdout_variation_second', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_second_id', + endOfRange: 5000 + } + ] + }); + + const config = createProjectConfig(datafile); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + age: 20, // satisfies audience for holdout_running + }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + }); }); describe('resolveVariationForFeatureList - sync', () => { diff --git a/lib/core/decision_service/index.tests.js b/lib/core/decision_service/index.tests.js index cdc4dc7c7..346814857 100644 --- a/lib/core/decision_service/index.tests.js +++ b/lib/core/decision_service/index.tests.js @@ -653,6 +653,7 @@ describe('lib/core/decision_service', function() { experimentIdMap: configObj.experimentIdMap, experimentKeyMap: configObj.experimentKeyMap, groupIdMap: configObj.groupIdMap, + validateEntity: true, }; assert.deepEqual(bucketerParams, expectedParams); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 5d5e57da9..057a0e129 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -23,7 +23,6 @@ import { } from '../../utils/enums'; import { getAudiencesById, - getExperimentAudienceConditions, getExperimentFromId, getExperimentFromKey, getFlagVariationByKey, @@ -32,7 +31,7 @@ import { getVariationKeyFromId, isActive, ProjectConfig, - getTrafficAllocation, + getHoldoutsForFlag, } from '../../project_config/project_config'; import { AudienceEvaluator, createAudienceEvaluator } from '../audience_evaluator'; import * as stringValidator from '../../utils/string_value_validator'; @@ -41,10 +40,11 @@ import { DecisionResponse, Experiment, ExperimentBucketMap, + ExperimentCore, FeatureFlag, + Holdout, OptimizelyDecideOption, OptimizelyUserContext, - TrafficAllocation, UserAttributes, UserProfile, UserProfileService, @@ -75,6 +75,7 @@ import { OptimizelyError } from '../../error/optimizly_error'; import { CmabService } from './cmab/cmab_service'; import { Maybe, OpType, OpValue } from '../../utils/type'; import { Value } from '../../utils/promise/operation_value'; +import * as featureToggle from '../../feature_toggle'; export const EXPERIMENT_NOT_RUNNING = 'Experiment %s is not running.'; export const RETURNING_STORED_VARIATION = @@ -112,9 +113,14 @@ export const USER_HAS_FORCED_DECISION_WITH_NO_RULE_SPECIFIED_BUT_INVALID = export const CMAB_NOT_SUPPORTED_IN_SYNC = 'CMAB is not supported in sync mode.'; export const CMAB_FETCH_FAILED = 'Failed to fetch CMAB data for experiment %s.'; export const CMAB_FETCHED_VARIATION_INVALID = 'Fetched variation %s for cmab experiment %s is invalid.'; +export const HOLDOUT_NOT_RUNNING = 'Holdout %s is not running.'; +export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for holdout %s.'; +export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.'; +export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.'; +export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.'; export interface DecisionObj { - experiment: Experiment | null; + experiment: Experiment | Holdout | null; variation: Variation | null; decisionSource: DecisionSource; cmabUuid?: string; @@ -540,14 +546,15 @@ export class DecisionService { */ private checkIfUserIsInAudience( configObj: ProjectConfig, - experiment: Experiment, + experiment: ExperimentCore, evaluationAttribute: string, user: OptimizelyUserContext, loggingKey?: string | number, ): DecisionResponse { const decideReasons: DecisionReason[] = []; - const experimentAudienceConditions = getExperimentAudienceConditions(configObj, experiment.id); + const experimentAudienceConditions = experiment.audienceConditions || experiment.audienceIds; const audiencesById = getAudiencesById(configObj); + this.logger?.debug( EVALUATING_AUDIENCES_COMBINED, evaluationAttribute, @@ -560,7 +567,9 @@ export class DecisionService { loggingKey || experiment.key, JSON.stringify(experimentAudienceConditions), ]); + const result = this.audienceEvaluator.evaluate(experimentAudienceConditions, audiencesById, user); + this.logger?.info( AUDIENCE_EVALUATION_RESULT_COMBINED, evaluationAttribute, @@ -590,16 +599,21 @@ export class DecisionService { */ private buildBucketerParams( configObj: ProjectConfig, - experiment: Experiment, + experiment: Experiment | Holdout, bucketingId: string, userId: string ): BucketerParams { - let trafficAllocationConfig: TrafficAllocation[] = getTrafficAllocation(configObj, experiment.id); - if (experiment.cmab) { + let validateEntity = true; + + let trafficAllocationConfig = experiment.trafficAllocation; + + if ('cmab' in experiment && experiment.cmab) { trafficAllocationConfig = [{ entityId: CMAB_DUMMY_ENTITY_ID, endOfRange: experiment.cmab.trafficAllocation }]; + + validateEntity = false; } return { @@ -613,7 +627,101 @@ export class DecisionService { trafficAllocationConfig, userId, variationIdMap: configObj.variationIdMap, + validateEntity, + } + } + + /** + * Determines if a user should be bucketed into a holdout variation. + * @param {ProjectConfig} configObj - The parsed project configuration object. + * @param {Holdout} holdout - The holdout to evaluate. + * @param {OptimizelyUserContext} user - The user context. + * @returns {DecisionResponse} - DecisionResponse containing holdout decision and reasons. + */ + private getVariationForHoldout( + configObj: ProjectConfig, + holdout: Holdout, + user: OptimizelyUserContext, + ): DecisionResponse { + const userId = user.getUserId(); + const decideReasons: DecisionReason[] = []; + + if (holdout.status !== 'Running') { + const reason: DecisionReason = [HOLDOUT_NOT_RUNNING, holdout.key]; + decideReasons.push(reason); + this.logger?.info(HOLDOUT_NOT_RUNNING, holdout.key); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + + const audienceResult = this.checkIfUserIsInAudience( + configObj, + holdout, + AUDIENCE_EVALUATION_TYPES.EXPERIMENT, + user + ); + decideReasons.push(...audienceResult.reasons); + + if (!audienceResult.result) { + const reason: DecisionReason = [USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT, userId, holdout.key]; + decideReasons.push(reason); + this.logger?.info(USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT, userId, holdout.key); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } + + const reason: DecisionReason = [USER_MEETS_CONDITIONS_FOR_HOLDOUT, userId, holdout.key]; + decideReasons.push(reason); + this.logger?.info(USER_MEETS_CONDITIONS_FOR_HOLDOUT, userId, holdout.key); + + const attributes = user.getAttributes(); + const bucketingId = this.getBucketingId(userId, attributes); + const bucketerParams = this.buildBucketerParams(configObj, holdout, bucketingId, userId); + const bucketResult = bucket(bucketerParams); + + decideReasons.push(...bucketResult.reasons); + + if (bucketResult.result) { + const variation = configObj.variationIdMap[bucketResult.result]; + if (variation) { + const bucketReason: DecisionReason = [USER_BUCKETED_INTO_HOLDOUT_VARIATION, userId, holdout.key, variation.key]; + decideReasons.push(bucketReason); + this.logger?.info(USER_BUCKETED_INTO_HOLDOUT_VARIATION, userId, holdout.key, variation.key); + + return { + result: { + experiment: holdout, + variation: variation, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; + } } + + const noBucketReason: DecisionReason = [USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION, userId]; + decideReasons.push(noBucketReason); + this.logger?.info(USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION, userId); + return { + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.HOLDOUT + }, + reasons: decideReasons + }; } /** @@ -830,6 +938,21 @@ export class DecisionService { reasons: decideReasons, }); } + if (featureToggle.holdout()) { + const holdouts = getHoldoutsForFlag(configObj, feature.key); + + for (const holdout of holdouts) { + const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); + decideReasons.push(...holdoutDecision.reasons); + + if (holdoutDecision.result.variation) { + return Value.of(op, { + result: holdoutDecision.result, + reasons: decideReasons, + }); + } + } + } return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { if (experimentDecision.error || experimentDecision.result.variation !== null) { diff --git a/lib/event_processor/batch_event_processor.ts b/lib/event_processor/batch_event_processor.ts index b573ca6aa..6ad19eaf8 100644 --- a/lib/event_processor/batch_event_processor.ts +++ b/lib/event_processor/batch_event_processor.ts @@ -266,6 +266,10 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { } private async readEventCountInStore(store: Store): Promise { + if (this.eventCountInStore !== undefined) { + return; + } + try { const keys = await store.getKeys(); this.eventCountInStore = keys.length; diff --git a/lib/event_processor/event_builder/log_event.spec.ts b/lib/event_processor/event_builder/log_event.spec.ts index 54a9c2acf..ad3b22b94 100644 --- a/lib/event_processor/event_builder/log_event.spec.ts +++ b/lib/event_processor/event_builder/log_event.spec.ts @@ -16,15 +16,16 @@ import { describe, it, expect } from 'vitest'; import { - buildConversionEventV1, - buildImpressionEventV1, makeEventBatch, + buildLogEvent, } from './log_event'; -import { ImpressionEvent, ConversionEvent } from './user_event'; +import { ImpressionEvent, ConversionEvent, UserEvent } from './user_event'; +import { Region } from '../../project_config/project_config'; -describe('buildImpressionEventV1', () => { - it('should build an ImpressionEventV1 when experiment and variation are defined', () => { + +describe('makeEventBatch', () => { + it('should build a batch with single impression event when experiment and variation are defined', () => { const impressionEvent: ImpressionEvent = { type: 'impression', timestamp: 69, @@ -65,7 +66,7 @@ describe('buildImpressionEventV1', () => { enabled: true, } - const result = buildImpressionEventV1(impressionEvent) + const result = makeEventBatch([impressionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -123,7 +124,7 @@ describe('buildImpressionEventV1', () => { }) }) - it('should build an ImpressionEventV1 when experiment and variation are not defined', () => { + it('should build a batch with simlge impression event when experiment and variation are not defined', () => { const impressionEvent: ImpressionEvent = { type: 'impression', timestamp: 69, @@ -164,7 +165,7 @@ describe('buildImpressionEventV1', () => { enabled: true, } - const result = buildImpressionEventV1(impressionEvent) + const result = makeEventBatch([impressionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -220,11 +221,9 @@ describe('buildImpressionEventV1', () => { }, ], }) - }) -}) + }); -describe('buildConversionEventV1', () => { - it('should build a ConversionEventV1 when tags object is defined', () => { + it('should build a batch with single conversion event when tags object is defined', () => { const conversionEvent: ConversionEvent = { type: 'conversion', timestamp: 69, @@ -260,7 +259,7 @@ describe('buildConversionEventV1', () => { value: 123, } - const result = buildConversionEventV1(conversionEvent) + const result = makeEventBatch([conversionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -311,7 +310,7 @@ describe('buildConversionEventV1', () => { }) }) - it('should build a ConversionEventV1 when tags object is undefined', () => { + it('should build a batch with single conversion event when when tags object is undefined', () => { const conversionEvent: ConversionEvent = { type: 'conversion', timestamp: 69, @@ -343,7 +342,7 @@ describe('buildConversionEventV1', () => { value: 123, } - const result = buildConversionEventV1(conversionEvent) + const result = makeEventBatch([conversionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -390,7 +389,7 @@ describe('buildConversionEventV1', () => { }) }) - it('should build a ConversionEventV1 when event id is null', () => { + it('should build a batch with single conversion event when event id is null', () => { const conversionEvent: ConversionEvent = { type: 'conversion', timestamp: 69, @@ -422,7 +421,7 @@ describe('buildConversionEventV1', () => { value: 123, } - const result = buildConversionEventV1(conversionEvent) + const result = makeEventBatch([conversionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -469,7 +468,7 @@ describe('buildConversionEventV1', () => { }) }) - it('should include revenue and value if they are 0', () => { + it('should include revenue and value for conversion events if they are 0', () => { const conversionEvent: ConversionEvent = { type: 'conversion', timestamp: 69, @@ -505,7 +504,7 @@ describe('buildConversionEventV1', () => { value: 0, } - const result = buildConversionEventV1(conversionEvent) + const result = makeEventBatch([conversionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -591,7 +590,7 @@ describe('buildConversionEventV1', () => { value: 123, } - const result = buildConversionEventV1(conversionEvent) + const result = makeEventBatch([conversionEvent]) expect(result).toEqual({ client_name: 'node-sdk', client_version: '3.0.0', @@ -635,9 +634,7 @@ describe('buildConversionEventV1', () => { ], }) }) -}) -describe('makeEventBatch', () => { it('should batch Conversion and Impression events together', () => { const conversionEvent: ConversionEvent = { type: 'conversion', @@ -810,3 +807,64 @@ describe('makeEventBatch', () => { }) }) +describe('buildLogEvent', () => { + it('should select the correct URL based on the event context region', () => { + const baseEvent: ImpressionEvent = { + type: 'impression', + timestamp: 69, + uuid: 'uuid', + context: { + accountId: 'accountId', + projectId: 'projectId', + clientName: 'node-sdk', + clientVersion: '3.0.0', + revision: 'revision', + botFiltering: true, + anonymizeIP: true + }, + user: { + id: 'userId', + attributes: [] + }, + layer: { + id: 'layerId' + }, + experiment: { + id: 'expId', + key: 'expKey' + }, + variation: { + id: 'varId', + key: 'varKey' + }, + ruleKey: 'expKey', + flagKey: 'flagKey1', + ruleType: 'experiment', + enabled: true + }; + + // Test for US region + const usEvent = { + ...baseEvent, + context: { + ...baseEvent.context, + region: 'US' as Region + } + }; + + const usResult = buildLogEvent([usEvent]); + expect(usResult.url).toBe('https://logx.optimizely.com/v1/events'); + + // Test for EU region + const euEvent = { + ...baseEvent, + context: { + ...baseEvent.context, + region: 'EU' as Region + } + }; + + const euResult = buildLogEvent([euEvent]); + expect(euResult.url).toBe('https://eu.logx.optimizely.com/v1/events'); + }); +}); diff --git a/lib/event_processor/event_builder/log_event.ts b/lib/event_processor/event_builder/log_event.ts index 8e65d6ba1..d3ec940fa 100644 --- a/lib/event_processor/event_builder/log_event.ts +++ b/lib/event_processor/event_builder/log_event.ts @@ -19,10 +19,16 @@ import { CONTROL_ATTRIBUTES } from '../../utils/enums'; import { LogEvent } from '../event_dispatcher/event_dispatcher'; import { EventTags } from '../../shared_types'; +import { Region } from '../../project_config/project_config'; const ACTIVATE_EVENT_KEY = 'campaign_activated' const CUSTOM_ATTRIBUTE_FEATURE_TYPE = 'custom' +export const logxEndpoint: Record = { + US: 'https://logx.optimizely.com/v1/events', + EU: 'https://eu.logx.optimizely.com/v1/events', +} + export type EventBatch = { account_id: string project_id: string @@ -214,51 +220,12 @@ function makeVisitor(data: ImpressionEvent | ConversionEvent): Visitor { return visitor } -/** - * Event for usage with v1 logtier - * - * @export - * @interface EventBuilderV1 - */ -export function buildImpressionEventV1(data: ImpressionEvent): EventBatch { - const visitor = makeVisitor(data) - visitor.snapshots.push(makeDecisionSnapshot(data)) - - return { - client_name: data.context.clientName, - client_version: data.context.clientVersion, - - account_id: data.context.accountId, - project_id: data.context.projectId, - revision: data.context.revision, - anonymize_ip: data.context.anonymizeIP, - enrich_decisions: true, - - visitors: [visitor], - } -} - -export function buildConversionEventV1(data: ConversionEvent): EventBatch { - const visitor = makeVisitor(data) - visitor.snapshots.push(makeConversionSnapshot(data)) - - return { - client_name: data.context.clientName, - client_version: data.context.clientVersion, - - account_id: data.context.accountId, - project_id: data.context.projectId, - revision: data.context.revision, - anonymize_ip: data.context.anonymizeIP, - enrich_decisions: true, - - visitors: [visitor], - } -} - export function buildLogEvent(events: UserEvent[]): LogEvent { + const region = events[0]?.context.region || 'US'; + const url = logxEndpoint[region]; + return { - url: 'https://logx.optimizely.com/v1/events', + url, httpVerb: 'POST', params: makeEventBatch(events), } diff --git a/lib/event_processor/event_builder/user_event.spec.ts b/lib/event_processor/event_builder/user_event.spec.ts new file mode 100644 index 000000000..e8cb373b3 --- /dev/null +++ b/lib/event_processor/event_builder/user_event.spec.ts @@ -0,0 +1,81 @@ +import { describe, it, expect, vi } from 'vitest'; +import { buildImpressionEvent, buildConversionEvent } from './user_event'; +import { createProjectConfig, ProjectConfig } from '../../project_config/project_config'; +import { DecisionObj } from '../../core/decision_service'; +import testData from '../../tests/test_data'; + +describe('buildImpressionEvent', () => { + it('should use correct region from projectConfig in event context', () => { + const projectConfig = createProjectConfig( + testData.getTestProjectConfig(), + ) + + const experiment = projectConfig.experiments[0]; + const variation = experiment.variations[0]; + + const decisionObj = { + experiment, + variation, + decisionSource: 'experiment', + } as DecisionObj; + + + const impressionEvent = buildImpressionEvent({ + configObj: projectConfig, + decisionObj, + userId: 'test_user', + flagKey: 'test_flag', + enabled: true, + clientEngine: 'node-sdk', + clientVersion: '1.0.0', + }); + + expect(impressionEvent.context.region).toBe('US'); + + projectConfig.region = 'EU'; + + const impressionEventEU = buildImpressionEvent({ + configObj: projectConfig, + decisionObj, + userId: 'test_user', + flagKey: 'test_flag', + enabled: true, + clientEngine: 'node-sdk', + clientVersion: '1.0.0', + }); + + expect(impressionEventEU.context.region).toBe('EU'); + }); +}); + +describe('buildConversionEvent', () => { + it('should use correct region from projectConfig in event context', () => { + const projectConfig = createProjectConfig( + testData.getTestProjectConfig(), + ) + + const conversionEvent = buildConversionEvent({ + configObj: projectConfig, + userId: 'test_user', + eventKey: 'test_event', + eventTags: { revenue: 1000 }, + clientEngine: 'node-sdk', + clientVersion: '1.0.0', + }); + + expect(conversionEvent.context.region).toBe('US'); + + projectConfig.region = 'EU'; + + const conversionEventEU = buildConversionEvent({ + configObj: projectConfig, + userId: 'test_user', + eventKey: 'test_event', + eventTags: { revenue: 1000 }, + clientEngine: 'node-sdk', + clientVersion: '1.0.0', + }); + + expect(conversionEventEU.context.region).toBe('EU'); + }); +}); diff --git a/lib/event_processor/event_builder/user_event.tests.js b/lib/event_processor/event_builder/user_event.tests.js index 19964e931..30f271d0e 100644 --- a/lib/event_processor/event_builder/user_event.tests.js +++ b/lib/event_processor/event_builder/user_event.tests.js @@ -26,6 +26,7 @@ describe('user_event', function() { beforeEach(function() { configObj = { + region: 'US', accountId: 'accountId', projectId: 'projectId', revision: '69', @@ -106,6 +107,7 @@ describe('user_event', function() { timestamp: 100, uuid: 'uuid', context: { + region: 'US', accountId: 'accountId', projectId: 'projectId', revision: '69', @@ -200,6 +202,7 @@ describe('user_event', function() { timestamp: 100, uuid: 'uuid', context: { + region: 'US', accountId: 'accountId', projectId: 'projectId', revision: '69', @@ -270,6 +273,7 @@ describe('user_event', function() { timestamp: 100, uuid: 'uuid', context: { + region: 'US', accountId: 'accountId', projectId: 'projectId', revision: '69', @@ -336,6 +340,7 @@ describe('user_event', function() { timestamp: 100, uuid: 'uuid', context: { + region: 'US', accountId: 'accountId', projectId: 'projectId', revision: '69', diff --git a/lib/event_processor/event_builder/user_event.ts b/lib/event_processor/event_builder/user_event.ts index c6c6c5446..ae33d65da 100644 --- a/lib/event_processor/event_builder/user_event.ts +++ b/lib/event_processor/event_builder/user_event.ts @@ -23,10 +23,12 @@ import { getEventId, getLayerId, ProjectConfig, + Region, } from '../../project_config/project_config'; import { EventTags, UserAttributes } from '../../shared_types'; import { LoggerFacade } from '../../logging/logger'; +import { DECISION_SOURCES } from '../../common_exports'; export type VisitorAttribute = { entityId: string @@ -35,6 +37,7 @@ export type VisitorAttribute = { } type EventContext = { + region?: Region; accountId: string; projectId: string; revision: string; @@ -44,8 +47,11 @@ type EventContext = { botFiltering?: boolean; } -export type BaseUserEvent = { - type: 'impression' | 'conversion'; +type EventType = 'impression' | 'conversion'; + + +export type BaseUserEvent = { + type: T; timestamp: number; uuid: string; context: EventContext; @@ -55,9 +61,7 @@ export type BaseUserEvent = { }; }; -export type ImpressionEvent = BaseUserEvent & { - type: 'impression'; - +export type ImpressionEvent = BaseUserEvent<'impression'> & { layer: { id: string | null; } | null; @@ -79,9 +83,7 @@ export type ImpressionEvent = BaseUserEvent & { cmabUuid?: string; }; -export type ConversionEvent = BaseUserEvent & { - type: 'conversion'; - +export type ConversionEvent = BaseUserEvent<'conversion'> & { event: { id: string | null; key: string; @@ -97,7 +99,12 @@ export type UserEvent = ImpressionEvent | ConversionEvent; export const areEventContextsEqual = (eventA: UserEvent, eventB: UserEvent): boolean => { const contextA = eventA.context const contextB = eventB.context + + const regionA: Region = contextA.region || 'US'; + const regionB: Region = contextB.region || 'US'; + return ( + regionA === regionB && contextA.accountId === contextB.accountId && contextA.projectId === contextB.projectId && contextA.clientName === contextB.clientName && @@ -108,6 +115,43 @@ export const areEventContextsEqual = (eventA: UserEvent, eventB: UserEvent): boo ) } +const buildBaseEvent = ({ + configObj, + userId, + userAttributes, + clientEngine, + clientVersion, + type, +}: { + configObj: ProjectConfig; + userId: string; + userAttributes?: UserAttributes; + clientEngine: string; + clientVersion: string; + type: T; +}): BaseUserEvent => { + return { + type, + timestamp: fns.currentTimestamp(), + uuid: fns.uuid(), + context: { + region: configObj.region, + accountId: configObj.accountId, + projectId: configObj.projectId, + revision: configObj.revision, + clientName: clientEngine, + clientVersion: clientVersion, + anonymizeIP: configObj.anonymizeIP || false, + botFiltering: configObj.botFiltering, + }, + user: { + id: userId, + attributes: buildVisitorAttributes(configObj, userAttributes), + }, + }; + +} + export type ImpressionConfig = { decisionObj: DecisionObj; userId: string; @@ -119,7 +163,6 @@ export type ImpressionConfig = { configObj: ProjectConfig; } - /** * Creates an ImpressionEvent object from decision data * @param {ImpressionConfig} config @@ -142,28 +185,18 @@ export const buildImpressionEvent = function({ const variationKey = decision.getVariationKey(decisionObj); const variationId = decision.getVariationId(decisionObj); const cmabUuid = decisionObj.cmabUuid; - - const layerId = experimentId !== null ? getLayerId(configObj, experimentId) : null; + const layerId = + experimentId !== null ? (ruleType === DECISION_SOURCES.HOLDOUT ? '' : getLayerId(configObj, experimentId)) : null; return { - type: 'impression', - timestamp: fns.currentTimestamp(), - uuid: fns.uuid(), - - user: { - id: userId, - attributes: buildVisitorAttributes(configObj, userAttributes), - }, - - context: { - accountId: configObj.accountId, - projectId: configObj.projectId, - revision: configObj.revision, - clientName: clientEngine, - clientVersion: clientVersion, - anonymizeIP: configObj.anonymizeIP || false, - botFiltering: configObj.botFiltering, - }, + ...buildBaseEvent({ + configObj, + userId, + userAttributes, + clientEngine, + clientVersion, + type: 'impression', + }), layer: { id: layerId, @@ -218,24 +251,14 @@ export const buildConversionEvent = function({ const eventValue = eventTags ? eventTagUtils.getEventValue(eventTags, logger) : null; return { - type: 'conversion', - timestamp: fns.currentTimestamp(), - uuid: fns.uuid(), - - user: { - id: userId, - attributes: buildVisitorAttributes(configObj, userAttributes), - }, - - context: { - accountId: configObj.accountId, - projectId: configObj.projectId, - revision: configObj.revision, - clientName: clientEngine, - clientVersion: clientVersion, - anonymizeIP: configObj.anonymizeIP || false, - botFiltering: configObj.botFiltering, - }, + ...buildBaseEvent({ + configObj, + userId, + userAttributes, + clientEngine, + clientVersion, + type: 'conversion', + }), event: { id: eventId, diff --git a/lib/feature_toggle.ts b/lib/feature_toggle.ts new file mode 100644 index 000000000..0a647c169 --- /dev/null +++ b/lib/feature_toggle.ts @@ -0,0 +1,34 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ + +/** + * This module contains feature flags that control the availability of features under development. + * Each flag represents a feature that is not yet ready for production release. These flags + * serve multiple purposes in our development workflow: + * + * When a new feature is in development, it can be safely merged into the main branch + * while remaining disabled in production. This allows continuous integration without + * affecting the stability of production releases. The feature code will be automatically + * removed in production builds through tree-shaking when the flag is disabled. + * + * During development and testing, these flags can be easily mocked to enable/disable + * specific features. Once a feature is complete and ready for release, its corresponding + * flag and all associated checks can be removed from the codebase. + */ + +export const holdout = () => true; diff --git a/lib/notification_center/type.ts b/lib/notification_center/type.ts index b433c0121..cbf8467a4 100644 --- a/lib/notification_center/type.ts +++ b/lib/notification_center/type.ts @@ -15,7 +15,15 @@ */ import { LogEvent } from '../event_processor/event_dispatcher/event_dispatcher'; -import { EventTags, Experiment, FeatureVariableValue, UserAttributes, VariableType, Variation } from '../shared_types'; +import { + EventTags, + Experiment, + FeatureVariableValue, + Holdout, + UserAttributes, + VariableType, + Variation, +} from '../shared_types'; import { DecisionSource } from '../utils/enums'; import { Nullable } from '../utils/type'; @@ -25,7 +33,7 @@ export type UserEventListenerPayload = { } export type ActivateListenerPayload = UserEventListenerPayload & { - experiment: Experiment | null; + experiment: Experiment | Holdout | null; variation: Variation | null; logEvent: LogEvent; } diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 165fae41b..0c7e2fc79 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -29,6 +29,35 @@ import { DECISION_SOURCES } from '../utils/enums'; import OptimizelyUserContext from '../optimizely_user_context'; import { newErrorDecision } from '../optimizely_decision'; import { ImpressionEvent } from '../event_processor/event_builder/user_event'; +import { OptimizelyDecideOption } from '../shared_types'; +import { NOTIFICATION_TYPES, DECISION_NOTIFICATION_TYPES } from '../notification_center/type'; + + +const holdoutData = [ + { + id: 'holdout_test_id', + key: 'holdout_test_key', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id', + key: 'holdout_variation_key', + variables: [], + featureEnabled: false, + }, + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id', + endOfRange: 10000, + }, + ], + }, +]; describe('Optimizely', () => { const eventDispatcher = { @@ -212,5 +241,588 @@ describe('Optimizely', () => { const event = processSpy.mock.calls[0][0] as ImpressionEvent; expect(event.cmabUuid).toBe('uuid-cmab'); }); + + + }); + + describe('holdout tests', () => { + let projectConfig: any; + let optimizely: any; + let decisionService: any; + let notificationSpy: any; + let eventProcessor: any; + + beforeEach(() => { + const datafile = getDecisionTestDatafile(); + datafile.holdouts = JSON.parse(JSON.stringify(holdoutData)); // Deep copy to avoid mutations + projectConfig = createProjectConfig(datafile); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + + optimizely = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + decisionService = optimizely.decisionService; + + // Setup notification spy + notificationSpy = vi.fn(); + optimizely.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpy + ); + }); + + it('should dispatch impression event for holdout decision', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.flagKey).toBe('flag_1'); + expect(decision.variationKey).toBe('holdout_variation_key'); + expect(decision.enabled).toBe(false); + + expect(eventProcessor.process).toHaveBeenCalledOnce(); + + const event = processSpy.mock.calls[0][0] as ImpressionEvent; + + expect(event.type).toBe('impression'); + expect(event.ruleKey).toBe('holdout_test_key'); + expect(event.ruleType).toBe('holdout'); + expect(event.enabled).toBe(false); + }); + + it('should not dispatch impression event for holdout when DISABLE_DECISION_EVENT is used', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.enabled).toBe(false); + expect(processSpy).not.toHaveBeenCalled(); + }); + + it('should dispatch impression event for holdout decision for isFeatureEnabled', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + + vi.spyOn(decisionService, 'getVariationForFeature').mockReturnValue({ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }); + + const result = optimizely.isFeatureEnabled('flag_1', 'test_user', {}); + + expect(result).toBe(false); + + expect(eventProcessor.process).toHaveBeenCalledOnce(); + const event = processSpy.mock.calls[0][0] as ImpressionEvent; + + expect(event.type).toBe('impression'); + expect(event.ruleKey).toBe('holdout_test_key'); + expect(event.ruleType).toBe('holdout'); + expect(event.enabled).toBe(false); + }); + + it('should send correct decision notification for holdout decision', async () => { + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.flagKey).toBe('flag_1'); + expect(decision.enabled).toBe(false); + expect(decision.variationKey).toBe('holdout_variation_key'); + expect(decision.ruleKey).toBe('holdout_test_key'); + + // Verify decision notification was sent + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + variationKey: 'holdout_variation_key', + ruleKey: 'holdout_test_key', + variables: expect.any(Object), + reasons: expect.any(Array), + decisionEventDispatched: true, + }), + }); + }); + + it('should handle holdout with included flags', async () => { + // Modify holdout to include specific flag + const modifiedHoldout = { ...projectConfig.holdouts[0] }; + modifiedHoldout.includedFlags = ['1001']; // flag_1 ID from test datafile + projectConfig.holdouts = [modifiedHoldout]; + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: modifiedHoldout.variations[0], + experiment: modifiedHoldout, + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key'); + expect(decision.variationKey).toBe('holdout_variation_key'); + + // Verify notification shows holdout details + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + ruleKey: 'holdout_test_key', + }), + }); + }); + + it('should handle holdout with excluded flags', async () => { + // Modify holdout to exclude specific flag + const modifiedHoldout = { ...projectConfig.holdouts[0] }; + modifiedHoldout.excludedFlags = ['1001']; // flag_1 ID from test datafile + projectConfig.holdouts = [modifiedHoldout]; + + // Mock normal feature test behavior for excluded flag + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.variationIdMap['5003'], + experiment: projectConfig.experimentKeyMap['exp_3'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'BD', age: 80 }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(true); + expect(decision.ruleKey).toBe('exp_3'); + expect(decision.variationKey).toBe('variation_3'); + + // Verify notification shows normal experiment details (not holdout) + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'BD', age: 80 }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: true, + ruleKey: 'exp_3', + }), + }); + }); + + it('should handle multiple holdouts with correct priority', async () => { + // Setup multiple holdouts + const holdout1 = { ...projectConfig.holdouts[0] }; + holdout1.excludedFlags = ['1001']; // exclude flag_1 + + const holdout2 = { + id: 'holdout_test_id_2', + key: 'holdout_test_key_2', + status: 'Running', + includedFlags: ['1001'], // include flag_1 + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'holdout_variation_id_2', + key: 'holdout_variation_key_2', + variables: [], + featureEnabled: false, + }, + ], + trafficAllocation: [ + { + entityId: 'holdout_variation_id_2', + endOfRange: 10000, + }, + ], + }; + + projectConfig.holdouts = [holdout1, holdout2]; + + // Mock that holdout2 takes priority due to includedFlags + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: holdout2.variations[0], + experiment: holdout2, + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key_2'); + expect(decision.variationKey).toBe('holdout_variation_key_2'); + + // Verify notification shows details of selected holdout + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + flagKey: 'flag_1', + enabled: false, + ruleKey: 'holdout_test_key_2', + }), + }); + }); + + it('should respect sendFlagDecisions setting for holdout events - false', async () => { + // Set sendFlagDecisions to false + projectConfig.sendFlagDecisions = false; + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithConfig = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithConfig.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionServiceLocal = optimizelyWithConfig.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithConfig, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + await optimizelyWithConfig.decideAsync(user, 'flag_1', []); + + // Impression event should still be dispatched for holdouts even when sendFlagDecisions is false + expect(processSpy).toHaveBeenCalledOnce(); + + // Verify notification shows decisionEventDispatched: true + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: true, + }), + }); + }); + + it('should respect sendFlagDecisions setting for holdout events - true', async () => { + // Set sendFlagDecisions to true + projectConfig.sendFlagDecisions = true; + + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithConfig = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithConfig.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionServiceLocal = optimizelyWithConfig.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithConfig, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + await optimizelyWithConfig.decideAsync(user, 'flag_1', []); + + // Impression event should be dispatched for holdouts + expect(processSpy).toHaveBeenCalledOnce(); + + // Verify notification shows decisionEventDispatched: true + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: true, + }), + }); + }); + + it('should return correct variable values for holdout decision', async () => { + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizely.decideAsync(user, 'flag_1', []); + + expect(decision.enabled).toBe(false); + expect(decision.variables).toBeDefined(); + expect(typeof decision.variables).toBe('object'); + + // Verify notification includes variable information + expect(notificationSpy).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + variables: expect.any(Object), + flagKey: 'flag_1', + enabled: false, + }), + }); + }); + + it('should handle disable decision event option for holdout', async () => { + const mockEventDispatcher = { + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }; + const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const projectConfigManager = getMockProjectConfigManager({ + initConfig: projectConfig, + }); + + const optimizelyWithEventProcessor = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + + // Add notification listener + const notificationSpyLocal = vi.fn(); + optimizelyWithEventProcessor.notificationCenter.addNotificationListener( + NOTIFICATION_TYPES.DECISION, + notificationSpyLocal + ); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const decisionServiceLocal = optimizelyWithEventProcessor.decisionService; + vi.spyOn(decisionServiceLocal, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: projectConfig.holdouts[0].variations[0], + experiment: projectConfig.holdouts[0], + decisionSource: DECISION_SOURCES.HOLDOUT, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely: optimizelyWithEventProcessor, + userId: 'test_user', + attributes: { country: 'US' }, + }); + + const decision = await optimizelyWithEventProcessor.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBe('holdout_test_key'); + + // No impression event should be dispatched + expect(processSpy).not.toHaveBeenCalled(); + + // Verify notification shows decisionEventDispatched: false + expect(notificationSpyLocal).toHaveBeenCalledWith({ + type: DECISION_NOTIFICATION_TYPES.FLAG, + userId: 'test_user', + attributes: { country: 'US' }, + decisionInfo: expect.objectContaining({ + decisionEventDispatched: false, + }), + }); + }); }); }); diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index c84d6a4cb..2f3e3277f 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -652,7 +652,7 @@ export default class Optimizely extends BaseService implements Client { let featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj); - if (decisionSource === DECISION_SOURCES.FEATURE_TEST) { + if (decisionSource === DECISION_SOURCES.FEATURE_TEST || decisionSource === DECISION_SOURCES.HOLDOUT) { sourceInfo = { experimentKey: experimentKey, variationKey: variationKey, @@ -661,6 +661,7 @@ export default class Optimizely extends BaseService implements Client { if ( decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj)) ) { this.sendImpressionEvent(decisionObj, feature.key, userId, featureEnabled, attributes); @@ -1503,6 +1504,7 @@ export default class Optimizely extends BaseService implements Client { if ( !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT] && (decisionSource === DECISION_SOURCES.FEATURE_TEST || + decisionSource === DECISION_SOURCES.HOLDOUT || (decisionSource === DECISION_SOURCES.ROLLOUT && projectConfig.getSendFlagDecisionsValue(configObj))) ) { this.sendImpressionEvent(decisionObj, key, userId, flagEnabled, attributes); diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 5a0259ee4..21d18940c 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -13,10 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { describe, it, expect, beforeEach, afterEach, vi, assert, Mock } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi, assert, Mock, beforeAll, afterAll } from 'vitest'; import { sprintf } from '../utils/fns'; import { keyBy } from '../utils/fns'; -import projectConfig, { ProjectConfig } from './project_config'; +import projectConfig, { ProjectConfig, getHoldoutsForFlag } from './project_config'; import { FEATURE_VARIABLE_TYPES, LOG_LEVEL } from '../utils/enums'; import testDatafile from '../tests/test_data'; import configValidator from '../utils/config_validator'; @@ -32,14 +32,44 @@ import { import { getMockLogger } from '../tests/mock/mock_logger'; import { VariableType } from '../shared_types'; import { OptimizelyError } from '../error/optimizly_error'; +import { mock } from 'node:test'; const buildLogMessageFromArgs = (args: any[]) => sprintf(args[1], ...args.splice(2)); const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj)); const logger = getMockLogger(); +const mockHoldoutToggle = vi.hoisted(() => vi.fn()); + +vi.mock('../feature_toggle', () => { + return { + holdout: mockHoldoutToggle, + }; +}); + describe('createProjectConfig', () => { let configObj: ProjectConfig; + it('should use US region when no region is specified in datafile', () => { + const datafile = testDatafile.getTestProjectConfig(); + const config = projectConfig.createProjectConfig(datafile); + + expect(config.region).toBe('US'); + }); + + it('should parse region specified in datafile correctly', () => { + const datafileUs = testDatafile.getTestProjectConfig(); + datafileUs.region = 'US'; + + const configUs = projectConfig.createProjectConfig(datafileUs); + expect(configUs.region).toBe('US'); + + const datafileEu = testDatafile.getTestProjectConfig(); + datafileEu.region = 'EU'; + const configEu = projectConfig.createProjectConfig(datafileEu); + + expect(configEu.region).toBe('EU'); + }); + it('should set properties correctly when createProjectConfig is called', () => { const testData: Record = testDatafile.getTestProjectConfig(); configObj = projectConfig.createProjectConfig(testData as JSON); @@ -277,6 +307,191 @@ describe('createProjectConfig - cmab experiments', () => { }); }); +const getHoldoutDatafile = () => { + const datafile = testDatafile.getTestDecideProjectConfig(); + + // Add holdouts to the datafile + datafile.holdouts = [ + { + id: 'holdout_id_1', + key: 'holdout_1', + status: 'Running', + includedFlags: [], + excludedFlags: [], + audienceIds: ['13389130056'], + audienceConditions: ['or', '13389130056'], + variations: [ + { + id: 'var_id_1', + key: 'holdout_variation_1', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'var_id_1', + endOfRange: 5000 + } + ] + }, + { + id: 'holdout_id_2', + key: 'holdout_2', + status: 'Running', + includedFlags: [], + excludedFlags: ['44829230000'], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'var_id_2', + key: 'holdout_variation_2', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'var_id_2', + endOfRange: 1000 + } + ] + }, + { + id: 'holdout_id_3', + key: 'holdout_3', + status: 'Draft', + includedFlags: ['4482920077'], + excludedFlags: [], + audienceIds: [], + audienceConditions: [], + variations: [ + { + id: 'var_id_2', + key: 'holdout_variation_2', + variables: [] + } + ], + trafficAllocation: [ + { + entityId: 'var_id_2', + endOfRange: 1000 + } + ] + } + ]; + + return datafile; +} + +describe('createProjectConfig - holdouts, feature toggle is on', () => { + beforeAll(() => { + mockHoldoutToggle.mockReturnValue(true); + }); + + afterAll(() => { + mockHoldoutToggle.mockReset(); + }); + + it('should populate holdouts fields correctly', function() { + const datafile = getHoldoutDatafile(); + + mockHoldoutToggle.mockReturnValue(true); + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + expect(configObj.holdouts).toHaveLength(3); + configObj.holdouts.forEach((holdout, i) => { + expect(holdout).toEqual(expect.objectContaining(datafile.holdouts[i])); + expect(holdout.variationKeyMap).toEqual( + keyBy(datafile.holdouts[i].variations, 'key') + ); + }); + + expect(configObj.holdoutIdMap).toEqual({ + holdout_id_1: configObj.holdouts[0], + holdout_id_2: configObj.holdouts[1], + holdout_id_3: configObj.holdouts[2], + }); + + expect(configObj.globalHoldouts).toHaveLength(2); + expect(configObj.globalHoldouts).toEqual([ + configObj.holdouts[0], // holdout_1 has empty includedFlags + configObj.holdouts[1] // holdout_2 has empty includedFlags + ]); + + expect(configObj.includedHoldouts).toEqual({ + feature_1: [configObj.holdouts[2]], // holdout_3 includes feature_1 (ID: 4482920077) + }); + + expect(configObj.excludedHoldouts).toEqual({ + feature_3: [configObj.holdouts[1]] // holdout_2 excludes feature_3 (ID: 44829230000) + }); + + expect(configObj.flagHoldoutsMap).toEqual({}); + }); + + it('should handle empty holdouts array', function() { + const datafile = testDatafile.getTestProjectConfig(); + + const configObj = projectConfig.createProjectConfig(datafile); + + expect(configObj.holdouts).toEqual([]); + expect(configObj.holdoutIdMap).toEqual({}); + expect(configObj.globalHoldouts).toEqual([]); + expect(configObj.includedHoldouts).toEqual({}); + expect(configObj.excludedHoldouts).toEqual({}); + expect(configObj.flagHoldoutsMap).toEqual({}); + }); + + it('should handle undefined includedFlags and excludedFlags in holdout', function() { + const datafile = getHoldoutDatafile(); + datafile.holdouts[0].includedFlags = undefined; + datafile.holdouts[0].excludedFlags = undefined; + + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + expect(configObj.holdouts).toHaveLength(3); + expect(configObj.holdouts[0].includedFlags).toEqual([]); + expect(configObj.holdouts[0].excludedFlags).toEqual([]); + }); +}); + +describe('getHoldoutsForFlag: feature toggle is on', () => { + beforeAll(() => { + mockHoldoutToggle.mockReturnValue(true); + }); + + afterAll(() => { + mockHoldoutToggle.mockReset(); + }); + + it('should return all applicable holdouts for a flag', () => { + const datafile = getHoldoutDatafile(); + const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile))); + + const feature1Holdouts = getHoldoutsForFlag(configObj, 'feature_1'); + expect(feature1Holdouts).toHaveLength(3); + expect(feature1Holdouts).toEqual([ + configObj.holdouts[0], + configObj.holdouts[1], + configObj.holdouts[2], + ]); + + const feature2Holdouts = getHoldoutsForFlag(configObj, 'feature_2'); + expect(feature2Holdouts).toHaveLength(2); + expect(feature2Holdouts).toEqual([ + configObj.holdouts[0], + configObj.holdouts[1], + ]); + + const feature3Holdouts = getHoldoutsForFlag(configObj, 'feature_3'); + expect(feature3Holdouts).toHaveLength(1); + expect(feature3Holdouts).toEqual([ + configObj.holdouts[0], + ]); + }); +}); + describe('getExperimentId', () => { let testData: Record; let configObj: ProjectConfig; diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index e91c4743a..efb50301e 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -34,6 +34,7 @@ import { VariationVariable, Integration, FeatureVariableValue, + Holdout, } from '../shared_types'; import { OdpConfig, OdpIntegrationConfig } from '../odp/odp_config'; import { Transformer } from '../utils/type'; @@ -51,6 +52,7 @@ import { } from 'error_message'; import { SKIPPING_JSON_VALIDATION, VALID_DATAFILE } from 'log_message'; import { OptimizelyError } from '../error/optimizly_error'; +import * as featureToggle from '../feature_toggle'; interface TryCreatingProjectConfigConfig { // TODO[OASIS-6649]: Don't use object type @@ -70,7 +72,10 @@ interface VariableUsageMap { [id: string]: VariationVariable; } +export type Region = 'US' | 'EU'; + export interface ProjectConfig { + region: Region; revision: string; projectId: string; sdkKey: string; @@ -107,6 +112,12 @@ export interface ProjectConfig { integrations: Integration[]; integrationKeyMap?: { [key: string]: Integration }; odpIntegrationConfig: OdpIntegrationConfig; + holdouts: Holdout[]; + holdoutIdMap?: { [id: string]: Holdout }; + globalHoldouts: Holdout[]; + includedHoldouts: { [key: string]: Holdout[]; } + excludedHoldouts: { [key: string]: Holdout[]; } + flagHoldoutsMap: { [key: string]: Holdout[]; } } const EXPERIMENT_RUNNING_STATUS = 'Running'; @@ -155,6 +166,10 @@ function createMutationSafeDatafileCopy(datafile: any): ProjectConfig { export const createProjectConfig = function(datafileObj?: JSON, datafileStr: string | null = null): ProjectConfig { const projectConfig = createMutationSafeDatafileCopy(datafileObj); + if (!projectConfig.region) { + projectConfig.region = 'US'; // Default to US region if not specified + } + projectConfig.__datafileStr = datafileStr === null ? JSON.stringify(datafileObj) : datafileStr; /* @@ -328,9 +343,85 @@ export const createProjectConfig = function(datafileObj?: JSON, datafileStr: str projectConfig.flagVariationsMap[flagKey] = variations; }); + parseHoldoutsConfig(projectConfig); + return projectConfig; }; +const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => { + if (!featureToggle.holdout()) { + return; + } + + projectConfig.holdouts = projectConfig.holdouts || []; + projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id'); + projectConfig.globalHoldouts = []; + projectConfig.includedHoldouts = {}; + projectConfig.excludedHoldouts = {}; + projectConfig.flagHoldoutsMap = {}; + + const featureFlagIdMap = keyBy(projectConfig.featureFlags, 'id'); + + projectConfig.holdouts.forEach((holdout) => { + if (!holdout.includedFlags) { + holdout.includedFlags = []; + } + + if (!holdout.excludedFlags) { + holdout.excludedFlags = []; + } + + holdout.variationKeyMap = keyBy(holdout.variations, 'key'); + + projectConfig.variationIdMap = { + ...projectConfig.variationIdMap, + ...keyBy(holdout.variations, 'id'), + }; + + if (holdout.includedFlags.length === 0) { + projectConfig.globalHoldouts.push(holdout); + + holdout.excludedFlags.forEach((flagId: string) => { + const flag = featureFlagIdMap[flagId]; + if (flag) { + const flagKey = flag.key; + if (!projectConfig.excludedHoldouts[flagKey]) { + projectConfig.excludedHoldouts[flagKey] = []; + } + projectConfig.excludedHoldouts[flagKey].push(holdout); + } + }); + } else { + holdout.includedFlags.forEach((flagId: string) => { + const flag = featureFlagIdMap[flagId]; + if (flag) { + const flagKey = flag.key; + if (!projectConfig.includedHoldouts[flagKey]) { + projectConfig.includedHoldouts[flagKey] = []; + } + projectConfig.includedHoldouts[flagKey].push(holdout); + } + }) + } + }); +} + +export const getHoldoutsForFlag = (projectConfig: ProjectConfig, flagKey: string): Holdout[] => { + if (projectConfig.flagHoldoutsMap[flagKey]) { + return projectConfig.flagHoldoutsMap[flagKey]; + } + + const flagHoldouts: Holdout[] = [ + ...projectConfig.globalHoldouts.filter((holdout) => { + return !(projectConfig.excludedHoldouts[flagKey] || []).includes(holdout); + }), + ...(projectConfig.includedHoldouts[flagKey] || []), + ]; + + projectConfig.flagHoldoutsMap[flagKey] = flagHoldouts; + return flagHoldouts; +} + /** * Extract all audience segments used in this audience's conditions * @param {Audience} audience Object representing the audience being parsed diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 93d5d4524..65b9594b2 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -64,6 +64,7 @@ export interface BucketerParams { variationIdMap: { [id: string]: Variation }; logger?: LoggerFacade; bucketingId: string; + validateEntity?: boolean; } export interface DecisionResponse { @@ -150,17 +151,20 @@ export interface Variation { variables?: VariationVariable[]; } -export interface Experiment { +export interface ExperimentCore { id: string; key: string; variations: Variation[]; variationKeyMap: { [key: string]: Variation }; - groupId?: string; - layerId: string; - status: string; audienceConditions: Array; audienceIds: string[]; trafficAllocation: TrafficAllocation[]; +} + +export interface Experiment extends ExperimentCore { + layerId: string; + groupId?: string; + status: string; forcedVariations?: { [key: string]: string }; isRollout?: boolean; cmab?: { @@ -169,6 +173,14 @@ export interface Experiment { }; } +export type HoldoutStatus = 'Draft' | 'Running' | 'Concluded' | 'Archived'; + +export interface Holdout extends ExperimentCore { + status: HoldoutStatus; + includedFlags: string[]; + excludedFlags: string[]; +} + export enum VariableType { BOOLEAN = 'boolean', DOUBLE = 'double', @@ -346,7 +358,7 @@ export interface Client { } export interface ActivateListenerPayload extends ListenerPayload { - experiment: import('./shared_types').Experiment; + experiment: import('./shared_types').ExperimentCore; variation: import('./shared_types').Variation; logEvent: Event; } diff --git a/lib/utils/enums/index.ts b/lib/utils/enums/index.ts index 892bff837..103cdac73 100644 --- a/lib/utils/enums/index.ts +++ b/lib/utils/enums/index.ts @@ -53,6 +53,7 @@ export const DECISION_SOURCES = { FEATURE_TEST: 'feature-test', ROLLOUT: 'rollout', EXPERIMENT: 'experiment', + HOLDOUT: 'holdout', } as const; export type DecisionSource = typeof DECISION_SOURCES[keyof typeof DECISION_SOURCES];