From 3b9f78fad77153259059133fa6d028a6405742d2 Mon Sep 17 00:00:00 2001 From: Eli Bishop <35503443+eli-darkly@users.noreply.github.com> Date: Thu, 30 Aug 2018 12:56:02 -0700 Subject: [PATCH] prepare 5.4.0 release (#118) --- CHANGELOG.md | 7 ++ evaluate_flag.js | 146 ++++++++++++++++++++-------------- event_processor.js | 3 + flags_state.js | 8 +- index.d.ts | 107 ++++++++++++++++++++++++- index.js | 124 ++++++++++++++++++----------- package-lock.json | 2 +- package.json | 2 +- test/LDClient-test.js | 149 +++++++++++++++++++++++++++++++++++ test/evaluate_flag-test.js | 123 ++++++++++++++++++++--------- test/event_processor-test.js | 17 ++++ 11 files changed, 540 insertions(+), 148 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cf3d8a..16c2170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to the LaunchDarkly Node.js SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [5.4.0] - 2018-08-30 +### Added: +- The new `LDClient` method `variationDetail` allows you to evaluate a feature flag (using the same parameters as you would for `variation`) and receive more information about how the value was calculated. This information is returned in an object that contains both the result value and a "reason" object which will tell you, for instance, if the user was individually targeted for the flag or was matched by one of the flag's rules, or if the flag returned the default value due to an error. + +### Fixed: +- Evaluating a prerequisite feature flag did not produce an analytics event if the prerequisite flag was off. + ## [5.3.2] - 2018-08-29 ### Fixed: - Fixed TypeScript syntax errors in `index.d.ts`. We are now running the TypeScript compiler in our automated builds to avoid such problems. (Thanks, [PsychicCat](https://github.com/launchdarkly/node-client/pull/116)!) diff --git a/evaluate_flag.js b/evaluate_flag.js index c89e418..d985778 100644 --- a/evaluate_flag.js +++ b/evaluate_flag.js @@ -8,90 +8,93 @@ var builtins = ['key', 'ip', 'country', 'email', 'firstName', 'lastName', 'avata var noop = function(){}; +// Callback receives (err, detail, events) where detail has the properties "value", "variationIndex", and "reason"; +// detail will never be null even if there's an error. function evaluate(flag, user, featureStore, cb) { cb = cb || noop; if (!user || user.key === null || user.key === undefined) { - cb(null, null, null, null); + cb(null, errorResult('USER_NOT_SPECIFIED'), []); return; } if (!flag) { - cb(null, null, null, null); + cb(null, errorResult('FLAG_NOT_FOUND'), []); return; } + var events = []; + evalInternal(flag, user, featureStore, events, function(err, detail) { + cb(err, detail, events); + }); +} + +function evalInternal(flag, user, featureStore, events, cb) { + // If flag is off, return the off variation if (!flag.on) { - // Return the off variation if defined and valid - cb(null, flag.offVariation, getVariation(flag, flag.offVariation), null); + getOffResult(flag, { kind: 'OFF' }, function(err, detail) { + cb(err, detail); + }); return; } - evalInternal(flag, user, featureStore, [], function(err, variation, value, events) { - if (err) { - cb(err, variation, value, events); - return; - } - - if (variation === null) { - // Return the off variation if defined and valid - cb(null, flag.offVariation, getVariation(flag, flag.offVariation), events); + checkPrerequisites(flag, user, featureStore, events, function(err, failureReason) { + if (err != null || failureReason != null) { + getOffResult(flag, failureReason, cb); } else { - cb(err, variation, value, events); + evalRules(flag, user, featureStore, cb); } }); - return; } -function evalInternal(flag, user, featureStore, events, cb) { - // Evaluate prerequisites, if any +// Callback receives (err, reason) where reason is null if successful, or a "prerequisite failed" reason +function checkPrerequisites(flag, user, featureStore, events, cb) { if (flag.prerequisites) { async.mapSeries(flag.prerequisites, function(prereq, callback) { featureStore.get(dataKind.features, prereq.key, function(f) { // If the flag does not exist in the store or is not on, the prerequisite // is not satisfied - if (!f || !f.on) { - callback(new Error("Unsatisfied prerequisite"), null); + if (!f) { + callback({ key: prereq.key, err: new Error("Could not retrieve prerequisite feature flag \"" + prereq.key + "\"") }); return; } - evalInternal(f, user, featureStore, events, function(err, variation, value) { + evalInternal(f, user, featureStore, events, function(err, detail) { // If there was an error, the value is null, the variation index is out of range, // or the value does not match the indexed variation the prerequisite is not satisfied - events.push(createFlagEvent(f.key, f, user, variation, value, null, flag.key)); - if (err || value === null || variation != prereq.variation) { - callback(new Error("Unsatisfied prerequisite"), null) + events.push(createFlagEvent(f.key, f, user, detail, null, flag.key, true)); + if (err) { + callback({ key: prereq.key, err: err }); + } else if (!f.on || detail.variationIndex != prereq.variation) { + // Note that if the prerequisite flag is off, we don't consider it a match no matter what its + // off variation was. But we still evaluate it and generate an event. + callback({ key: prereq.key }); } else { // The prerequisite was satisfied - callback(null, null); + callback(null); } }); }); }, - function(err, results) { - // If the error is that prerequisites weren't satisfied, we don't return an error, - // because we want to serve the 'offVariation' - if (err) { - cb(null, null, null, events); - return; - } - evalRules(flag, user, featureStore, function(e, variation, value) { - cb(e, variation, value, events); - }); - }) + function(errInfo) { + if (errInfo) { + cb(errInfo.err, { 'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': errInfo.key }); + } else { + cb(null, null); + } + }); } else { - evalRules(flag, user, featureStore, function(e, variation, value) { - cb(e, variation, value, events); - }); + cb(null, null); } } +// Callback receives (err, detail) function evalRules(flag, user, featureStore, cb) { var i, j; var target; var variation; var rule; // Check target matches - for (i = 0; i < flag.targets.length; i++) { + for (i = 0; i < (flag.targets || []).length; i++) { target = flag.targets[i]; if (!target.values) { @@ -100,32 +103,30 @@ function evalRules(flag, user, featureStore, cb) { for (j = 0; j < target.values.length; j++) { if (user.key === target.values[j]) { - value = getVariation(flag, target.variation); - cb(value === null ? new Error("Undefined variation for flag " + flag.key) : null, - target.variation, value); + getVariation(flag, target.variation, { kind: 'TARGET_MATCH' }, cb); return; } } } - async.mapSeries(flag.rules, + i = 0; + async.mapSeries(flag.rules || [], function(rule, callback) { ruleMatchUser(rule, user, featureStore, function(matched) { - setImmediate(callback, matched ? rule : null, null); + var match = matched ? { index: i, rule: rule } : null; + setImmediate(callback, match, null); }); }, function(err, results) { // we use the "error" value to indicate that a rule was successfully matched (since we only care // about the first match, and mapSeries terminates on the first "error") if (err) { - var rule = err; - variation = variationForUser(rule, user, flag); + var reason = { kind: 'RULE_MATCH', ruleIndex: err.index, ruleId: err.rule.id }; + getResultForVariationOrRollout(err.rule, user, flag, reason, cb); } else { // no rule matched; check the fallthrough - variation = variationForUser(flag.fallthrough, user, flag); + getResultForVariationOrRollout(flag.fallthrough, user, flag, { kind: 'FALLTHROUGH' }, cb); } - cb(variation === null ? new Error("Undefined variation for flag " + flag.key) : null, - variation, getVariation(flag, variation)); } ); } @@ -255,16 +256,39 @@ function matchAny(matchFn, value, values) { return false; } -// Given an index, return the variation value, or null if -// the index is invalid -function getVariation(flag, index) { - if (index === null || index === undefined || index >= flag.variations.length) { - return null; +function getVariation(flag, index, reason, cb) { + if (index === null || index === undefined || index < 0 || index >= flag.variations.length) { + cb(new Error('Invalid variation index in flag', errResult('MALFORMED_FLAG'))); } else { - return flag.variations[index]; + cb(null, { value: flag.variations[index], variationIndex: index, reason: reason }); } } +function getOffResult(flag, reason, cb) { + if (flag.offVariation === null || flag.offVariation === undefined) { + cb(null, { value: null, variationIndex: null, reason: reason }); + } else { + getVariation(flag, flag.offVariation, reason, cb); + } +} + +function getResultForVariationOrRollout(r, user, flag, reason, cb) { + if (!r) { + cb(new Error('Fallthrough variation undefined'), errResult('MALFORMED_FLAG')); + } else { + var index = variationForUser(r, user, flag); + if (index === null) { + cb(new Error('Variation/rollout object with no variation or rollout'), errResult('MALFORMED_FLAG')); + } else { + getVariation(flag, index, reason, cb); + } + } +} + +function errorResult(errorKind) { + return { value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: errorKind }}; +} + // Given a variation or rollout 'r', select // the variation for the given user function variationForUser(r, user, flag) { @@ -337,13 +361,13 @@ function bucketableStringValue(value) { return null; } -function createFlagEvent(key, flag, user, variation, value, defaultVal, prereqOf) { - return { +function createFlagEvent(key, flag, user, detail, defaultVal, prereqOf, includeReason) { + var e = { "kind": "feature", "key": key, "user": user, - "variation": variation, - "value": value, + "variation": detail.variationIndex, + "value": detail.value, "default": defaultVal, "creationDate": new Date().getTime(), "version": flag ? flag.version : null, @@ -351,6 +375,10 @@ function createFlagEvent(key, flag, user, variation, value, defaultVal, prereqOf "trackEvents": flag ? flag.trackEvents : null, "debugEventsUntilDate": flag ? flag.debugEventsUntilDate : null }; + if (includeReason) { + e['reason'] = detail.reason; + } + return e; } module.exports = {evaluate: evaluate, bucketUser: bucketUser, createFlagEvent: createFlagEvent}; \ No newline at end of file diff --git a/event_processor.js b/event_processor.js index ab453b1..42bfdb2 100644 --- a/event_processor.js +++ b/event_processor.js @@ -59,6 +59,9 @@ function EventProcessor(sdkKey, config, errorReporter) { if (event.version) { out.version = event.version; } + if (event.reason) { + out.reason = event.reason; + } if (config.inlineUsersInEvents || debug) { out.user = userFilter.filterUser(event.user); } else { diff --git a/flags_state.js b/flags_state.js index 10f15c6..5a7a2eb 100644 --- a/flags_state.js +++ b/flags_state.js @@ -4,7 +4,7 @@ function FlagsStateBuilder(valid) { var flagValues = {}; var flagMetadata = {}; - builder.addFlag = function(flag, value, variation) { + builder.addFlag = function(flag, value, variation, reason) { flagValues[flag.key] = value; var meta = { version: flag.version, @@ -16,6 +16,9 @@ function FlagsStateBuilder(valid) { if (flag.debugEventsUntilDate !== undefined && flag.debugEventsUntilDate !== null) { meta.debugEventsUntilDate = flag.debugEventsUntilDate; } + if (reason) { + meta.reason = reason; + } flagMetadata[flag.key] = meta; }; @@ -24,6 +27,9 @@ function FlagsStateBuilder(valid) { valid: valid, allValues: function() { return flagValues; }, getFlagValue: function(key) { return flagValues[key]; }, + getFlagReason: function(key) { + return flagMetadata[key] ? flagMetadata[key].reason : null; + }, toJSON: function() { return Object.assign({}, flagValues, { $flagsState: flagMetadata, $valid: valid }); } diff --git a/index.d.ts b/index.d.ts index ea534d0..ab78012 100644 --- a/index.d.ts +++ b/index.d.ts @@ -53,7 +53,7 @@ declare module 'ldclient-node' { */ export interface LDFlagsState { /** - * True if this object contains a valid snapshot of feature flag state, or false if the + * True if this object contains a valid snapshot of feature flag state, or false if the * state could not be computed (for instance, because the client was offline or there * was no user). */ @@ -66,6 +66,13 @@ declare module 'ldclient-node' { */ getFlagValue: (key: string) => LDFlagValue; + /** + * Returns the evaluation reason for a feature flag at the time the state was recorded. + * It will be null if reasons were not recorded, or if there was no such flag. + * @param key the flag key + */ + getFlagReason: (key: string) => LDEvaluationReason; + /** * Returns a map of feature flag keys to values. If a flag would have evaluated to the * default value, its value will be null. @@ -86,6 +93,74 @@ declare module 'ldclient-node' { toJSON: () => object; } + /** + * Describes the reason that a flag evaluation produced a particular value. This is + * part of the LDEvaluationDetail object returned by variationDetail(). + */ + export type LDEvaluationReason = { + /** + * The general category of the reason: + * + * 'OFF': the flag was off and therefore returned its configured off value + * + * 'FALLTHROUGH': the flag was on but the user did not match any targets or rules + * + * 'TARGET_MATCH': the user key was specifically targeted for this flag + * + * 'RULE_MATCH': the user matched one of the flag's rules + * + * 'PREREQUISITE_FAILED': the flag was considered off because it had at least one + * prerequisite flag that either was off or did not return the desired variation + * + * 'ERROR': the flag could not be evaluated, e.g. because it does not exist or due + * to an unexpected error + */ + kind: string; + + /** + * A further description of the error condition, if the kind was 'ERROR'. + */ + errorKind?: string; + + /** + * The index of the matched rule (0 for the first), if the kind was 'RULE_MATCH'. + */ + ruleIndex?: number; + + /** + * The unique identifier of the matched rule, if the kind was 'RULE_MATCH'. + */ + ruleId?: string; + + /** + * The key of the failed prerequisite flag, if the kind was 'PREREQUISITE_FAILED'. + */ + prerequisiteKey?: string; + }; + + /** + * An object returned by LDClient.variationDetail(), combining the result of a feature flag + * evaluation with information about how it was calculated. + */ + export type LDEvaluationDetail = { + /** + * The result of the flag evaluation. This will be either one of the flag's variations or + * the default value that was passed to variationDetail(). + */ + value: LDFlagValue; + + /** + * The index of the returned value within the flag's list of variations, e.g. 0 for the + * first variation - or null if the default value was returned. + */ + variationIndex?: number; + + /** + * An object describing the main factor that influenced the flag evaluation value. + */ + reason: LDEvaluationReason; + }; + /** * LaunchDarkly initialization options. */ @@ -524,6 +599,36 @@ declare module 'ldclient-node' { callback?: (err: any, res: LDFlagValue) => void ) => Promise; + /** + * Retrieves a flag's value, along with information about how it was calculated, in the form + * of an LDEvaluationDetail object. + * + * The reason property of the result will also be included in analytics events, if you are + * capturing detailed event data for this flag. + * + * @param key + * The key of the flag for which to retrieve the corresponding value. + * @param user + * The user for the variation. + * + * The variation call will automatically create a user in LaunchDarkly if a user with that user key doesn't exist already. + * + * @param defaultValue + * The value to use if the flag is not available (for example, if the + * user is offline or a flag is requested that does not exist). + * + * @param callback + * The callback to receive the result. + * + * @returns a Promise containing the flag value and explanation + */ + variationDetail: ( + key: string, + user: LDUser, + defaultValue: LDFlagValue, + callback?: (err: any, res: LDEvaluationDetail) => void + ) => Promise; + toggle: ( key: string, user: LDUser, diff --git a/index.js b/index.js index d70f12b..79a97c5 100644 --- a/index.js +++ b/index.js @@ -152,54 +152,78 @@ var newClient = function(sdkKey, config) { client.variation = function(key, user, defaultVal, callback) { return wrapPromiseCallback(new Promise(function(resolve, reject) { - sanitizeUser(user); - var variationErr; + evaluateIfPossible(key, user, defaultVal, false, + function(detail) { + resolve(detail.value) + }, + reject); + }.bind(this)), callback); + }; - if (this.isOffline()) { - config.logger.info("Variation called in offline mode. Returning default value."); - return resolve(defaultVal); - } + client.variationDetail = function(key, user, defaultVal, callback) { + return wrapPromiseCallback(new Promise(function(resolve, reject) { + evaluateIfPossible(key, user, defaultVal, true, resolve, reject); + }.bind(this)), callback); + }; - else if (!key) { - variationErr = new errors.LDClientError('No feature flag key specified. Returning default value.'); - maybeReportError(variationError); - sendFlagEvent(key, null, user, null, defaultVal, defaultVal); - return resolve(defaultVal); - } + function errorResult(errorKind, defaultVal) { + return { value: defaultVal, variationIndex: null, reason: { kind: 'ERROR', errorKind: errorKind } }; + }; - else if (user && user.key === "") { - config.logger.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly"); - } + function evaluateIfPossible(key, user, defaultVal, includeReasonsInEvents, resolve, reject) { + if (!initComplete) { + config.featureStore.initialized(function(storeInited) { + if (storeInited) { + config.logger.warn("Variation called before LaunchDarkly client initialization completed (did you wait for the 'ready' event?) - using last known values from feature store") + variationInternal(key, user, defaultVal, includeReasonsInEvents, resolve, reject); + } else { + var err = new errors.LDClientError("Variation called before LaunchDarkly client initialization completed (did you wait for the 'ready' event?) - using default value"); + maybeReportError(variationErr); + var result = errorResult('CLIENT_NOT_READY', defaultVal); + sendFlagEvent(key, null, user, result, defaultVal, includeReasonsInEvents); + return resolve(result); + } + }); + } else { + variationInternal(key, user, defaultVal, includeReasonsInEvents, resolve, reject); + } + } - if (!initComplete) { - config.featureStore.initialized(function(storeInited) { - if (storeInited) { - config.logger.warn("Variation called before LaunchDarkly client initialization completed (did you wait for the 'ready' event?) - using last known values from feature store") - variationInternal(key, user, defaultVal, resolve, reject); - } else { - variationErr = new errors.LDClientError("Variation called before LaunchDarkly client initialization completed (did you wait for the 'ready' event?) - using default value"); - maybeReportError(variationErr); - sendFlagEvent(key, null, user, null, defaultVal, defaultVal); - return resolve(defaultVal); - } - }); - return; - } + // resolves to a "detail" object with properties "value", "variationIndex", "reason" + function variationInternal(key, user, defaultVal, includeReasonsInEvents, resolve, reject) { + if (client.isOffline()) { + config.logger.info("Variation called in offline mode. Returning default value."); + return resolve(errorResult('CLIENT_NOT_READY', defaultVal)); + } - variationInternal(key, user, defaultVal, resolve, reject); - }.bind(this)), callback); - } + else if (!key) { + err = new errors.LDClientError('No feature flag key specified. Returning default value.'); + maybeReportError(variationError); + return resolve(errorResult('FLAG_NOT_FOUND', defaultVal)); + } + + sanitizeUser(user); + if (user && user.key === "") { + config.logger.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly"); + } - function variationInternal(key, user, defaultVal, resolve, reject) { config.featureStore.get(dataKind.features, key, function(flag) { if (!user) { variationErr = new errors.LDClientError('No user specified. Returning default value.'); maybeReportError(variationErr); - sendFlagEvent(key, flag, user, null, defaultVal, defaultVal); - return resolve(defaultVal); + var result = errorResult('USER_NOT_SPECIFIED', defaultVal); + sendFlagEvent(key, flag, user, result, defaultVal, includeReasonsInEvents); + return resolve(result); } - evaluate.evaluate(flag, user, config.featureStore, function(err, variation, value, events) { + if (!flag) { + maybeReportError(new errors.LDClientError('Unknown feature flag "' + key + '"; returning default value')); + var result = errorResult('FLAG_NOT_FOUND', defaultVal); + sendFlagEvent(key, null, user, result, defaultVal, includeReasonsInEvents); + return resolve(result); + } + + evaluate.evaluate(flag, user, config.featureStore, function(err, detail, events) { var i; var version = flag ? flag.version : null; @@ -211,18 +235,20 @@ var newClient = function(sdkKey, config) { // have already been constructed, so we just have to push them onto the queue. if (events) { for (i = 0; i < events.length; i++) { - eventProcessor.sendEvent(events[i]); + var e = events[i]; + if (!includeReasonsInEvents) { + delete e['reason']; + } + eventProcessor.sendEvent(e); } } - if (value === null) { + if (detail.variationIndex === null) { config.logger.debug("Result value is null in variation"); - sendFlagEvent(key, flag, user, null, defaultVal, defaultVal); - return resolve(defaultVal); - } else { - sendFlagEvent(key, flag, user, variation, value, defaultVal); - return resolve(value); + detail.value = defaultVal; } + sendFlagEvent(key, flag, user, detail, defaultVal, includeReasonsInEvents); + return resolve(detail); }); }); } @@ -263,14 +289,18 @@ var newClient = function(sdkKey, config) { var builder = FlagsStateBuilder(true); var clientOnly = options.clientSideOnly; + var withReasons = options.withReasons; config.featureStore.all(dataKind.features, function(flags) { async.forEachOf(flags, function(flag, key, iterateeCb) { if (clientOnly && !flag.clientSide) { setImmediate(iterateeCb); } else { // At the moment, we don't send any events here - evaluate.evaluate(flag, user, config.featureStore, function(err, variation, value, events) { - builder.addFlag(flag, value, variation); + evaluate.evaluate(flag, user, config.featureStore, function(err, detail, events) { + if (err != null) { + maybeReportError(new Error('Error for feature flag "' + flag.key + '" while evaluating all flags: ' + err)); + } + builder.addFlag(flag, detail.value, detail.variationIndex, withReasons ? detail.reason : null); setImmediate(iterateeCb); }); } @@ -327,8 +357,8 @@ var newClient = function(sdkKey, config) { return eventProcessor.flush(callback); }; - function sendFlagEvent(key, flag, user, variation, value, defaultVal) { - var event = evaluate.createFlagEvent(key, flag, user, variation, value, defaultVal); + function sendFlagEvent(key, flag, user, detail, defaultVal, includeReasonsInEvents) { + var event = evaluate.createFlagEvent(key, flag, user, detail, defaultVal, null, includeReasonsInEvents); eventProcessor.sendEvent(event); } diff --git a/package-lock.json b/package-lock.json index b6d1c07..49d1345 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "ldclient-node", - "version": "5.3.2", + "version": "5.4.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 2287690..7d2d860 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ldclient-node", - "version": "5.3.2", + "version": "5.4.0", "description": "LaunchDarkly SDK for Node.js", "main": "index.js", "scripts": { diff --git a/test/LDClient-test.js b/test/LDClient-test.js index e297e45..a18023f 100644 --- a/test/LDClient-test.js +++ b/test/LDClient-test.js @@ -143,6 +143,86 @@ describe('LDClient', function() { }); }); + it('returns default from variation() for unknown flag', function(done) { + var client = createOnlineClientWithFlags({ }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variation('flagkey', user, 'default', function(err, result) { + expect(err).toBeNull(); + expect(result).toEqual('default'); + done(); + }); + }); + }); + + it('returns default from variation() for flag that evaluates to null', function(done) { + var flag = { + key: 'flagkey', + on: false, + offVariation: null + }; + var client = createOnlineClientWithFlags({ flagkey: flag }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variation(flag.key, user, 'default', function(err, result) { + expect(err).toBeNull(); + expect(result).toEqual('default'); + done(); + }); + }); + }); + + it('evaluates a flag with variationDetail()', function(done) { + var flag = { + key: 'flagkey', + version: 1, + on: true, + targets: [], + fallthrough: { variation: 1 }, + variations: ['a', 'b'], + trackEvents: true + }; + var client = createOnlineClientWithFlags({ flagkey: flag }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variationDetail(flag.key, user, 'c', function(err, result) { + expect(err).toBeNull(); + expect(result).toMatchObject({ value: 'b', variationIndex: 1, reason: { kind: 'FALLTHROUGH' } }); + done(); + }); + }); + }); + + it('returns default from variationDetail() for unknown flag', function(done) { + var client = createOnlineClientWithFlags({ }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variationDetail('flagkey', user, 'default', function(err, result) { + expect(err).toBeNull(); + expect(result).toMatchObject({ value: 'default', variationIndex: null, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' } }); + done(); + }); + }); + }); + + it('returns default from variationDetail() for flag that evaluates to null', function(done) { + var flag = { + key: 'flagkey', + on: false, + offVariation: null + }; + var client = createOnlineClientWithFlags({ flagkey: flag }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variationDetail(flag.key, user, 'default', function(err, result) { + expect(err).toBeNull(); + expect(result).toMatchObject({ value: 'default', variationIndex: null, reason: { kind: 'OFF' } }); + done(); + }); + }); + }); + it('generates an event for an existing feature', function(done) { var flag = { key: 'flagkey', @@ -174,6 +254,38 @@ describe('LDClient', function() { }); }); + it('generates an event for an existing feature with reason', function(done) { + var flag = { + key: 'flagkey', + version: 1, + on: true, + targets: [], + fallthrough: { variation: 1 }, + variations: ['a', 'b'], + trackEvents: true + }; + var client = createOnlineClientWithFlags({ flagkey: flag }); + var user = { key: 'user' }; + client.on('ready', function() { + client.variationDetail(flag.key, user, 'c', function(err, result) { + expect(eventProcessor.events).toHaveLength(1); + var e = eventProcessor.events[0]; + expect(e).toMatchObject({ + kind: 'feature', + key: 'flagkey', + version: 1, + user: user, + variation: 1, + value: 'b', + default: 'c', + reason: { kind: 'FALLTHROUGH' }, + trackEvents: true + }); + done(); + }); + }); + }); + it('generates an event for an unknown feature', function(done) { var client = createOnlineClientWithFlags({}); var user = { key: 'user' }; @@ -347,6 +459,43 @@ describe('LDClient', function() { }); }); + it('can include reasons in allFlagsState()', function(done) { + var flag = { + key: 'feature', + version: 100, + on: true, + targets: [], + fallthrough: { variation: 1 }, + variations: ['a', 'b'], + trackEvents: true, + debugEventsUntilDate: 1000 + }; + var client = createOnlineClientWithFlags({ feature: flag }); + var user = { key: 'user' }; + client.on('ready', function() { + client.allFlagsState(user, { withReasons: true }, function(err, state) { + expect(err).toBeNull(); + expect(state.valid).toEqual(true); + expect(state.allValues()).toEqual({feature: 'b'}); + expect(state.getFlagValue('feature')).toEqual('b'); + expect(state.toJSON()).toEqual({ + feature: 'b', + $flagsState: { + feature: { + version: 100, + variation: 1, + reason: { kind: 'FALLTHROUGH' }, + trackEvents: true, + debugEventsUntilDate: 1000 + } + }, + $valid: true + }); + done(); + }); + }); + }); + it('should not overflow the call stack when evaluating a huge number of flags', function(done) { var flagCount = 5000; var flags = {}; diff --git a/test/evaluate_flag-test.js b/test/evaluate_flag-test.js index d4c3cf3..ca6ce35 100644 --- a/test/evaluate_flag-test.js +++ b/test/evaluate_flag-test.js @@ -58,8 +58,9 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'x' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe('b'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'b', variationIndex: 1, reason: { kind: 'OFF' } }); + expect(events).toMatchObject([]); done(); }); }); @@ -72,8 +73,9 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'x' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(null); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: null, variationIndex: null, reason: { kind: 'OFF' } }); + expect(events).toMatchObject([]); done(); }); }); @@ -89,8 +91,9 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'x' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe('a'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'a', variationIndex: 0, reason: { kind: 'FALLTHROUGH' } }); + expect(events).toMatchObject([]); done(); }); }); @@ -105,12 +108,51 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'x' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe('b'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'b', variationIndex: 1, + reason: { kind: 'PREREQUISITE_FAILED', prerequisiteKey: 'badfeature' } }); + expect(events).toMatchObject([]); done(); }); }); + it('returns off variation and event if prerequisite is off', function(done) { + var flag = { + key: 'feature0', + on: true, + prerequisites: [{key: 'feature1', variation: 1}], + fallthrough: { variation: 0 }, + offVariation: 1, + targets: [], + rules: [], + variations: ['a', 'b', 'c'], + version: 1 + }; + var flag1 = { + key: 'feature1', + on: false, + offVariation: 1, + // note that even though it returns the desired variation, it is still off and therefore not a match + fallthrough: { variation: 0 }, + targets: [], + rules: [], + variations: ['d', 'e'], + version: 2 + }; + defineFeatures([flag, flag1], function() { + var user = { key: 'x' }; + var eventsShouldBe = [ + { kind: 'feature', key: 'feature1', variation: 1, value: 'e', version: 2, prereqOf: 'feature0' } + ]; + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'b', variationIndex: 1, + reason: { kind: 'PREREQUISITE_FAILED', prerequisiteKey: 'feature1' } }); + expect(events).toMatchObject(eventsShouldBe); + done(); + }); + }); + }); + it('returns off variation and event if prerequisite is not met', function(done) { var flag = { key: 'feature0', @@ -137,8 +179,9 @@ describe('evaluate', function() { var eventsShouldBe = [ { kind: 'feature', key: 'feature1', variation: 0, value: 'd', version: 2, prereqOf: 'feature0' } ]; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result, events) { - expect(result).toBe('b'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'b', variationIndex: 1, + reason: { kind: 'PREREQUISITE_FAILED', prerequisiteKey: 'feature1' } }); expect(events).toMatchObject(eventsShouldBe); done(); }); @@ -171,8 +214,8 @@ describe('evaluate', function() { var eventsShouldBe = [ { kind: 'feature', key: 'feature1', variation: 1, value: 'e', version: 2, prereqOf: 'feature0' } ]; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result, events) { - expect(result).toBe('a'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'a', variationIndex: 0, reason: { kind: 'FALLTHROUGH' } }); expect(events).toMatchObject(eventsShouldBe); done(); }); @@ -185,6 +228,7 @@ describe('evaluate', function() { on: true, rules: [ { + id: 'id', clauses: [ { attribute: 'key', @@ -201,8 +245,10 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'userkey' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe('c'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'c', variationIndex: 2, + reason: { kind: 'RULE_MATCH', ruleIndex: 0, ruleId: 'id' } }); + expect(events).toMatchObject([]); done(); }); }); @@ -223,16 +269,17 @@ describe('evaluate', function() { variations: ['a', 'b', 'c'] }; var user = { key: 'userkey' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe('c'); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail).toMatchObject({ value: 'c', variationIndex: 2, reason: { kind: 'TARGET_MATCH' } }); + expect(events).toMatchObject([]); done(); }); }); function testClauseMatch(clause, user, shouldBe, done) { var flag = makeBooleanFlagWithOneClause(clause); - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(shouldBe); + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail.value).toBe(shouldBe); done(); }); } @@ -270,8 +317,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(true); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(true); done(); }); }); @@ -286,8 +333,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(false); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(false); done(); }); }); @@ -302,8 +349,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'bar' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(false); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(false); done(); }); }); @@ -319,8 +366,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(true); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(true); done(); }); }); @@ -346,8 +393,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo', email: 'test@example.com' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(true); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(true); done(); }); }); @@ -373,8 +420,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo', email: 'test@example.com' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(false); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(false); done(); }); }); @@ -404,8 +451,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo', email: 'test@example.com', name: 'bob' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(true); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(true); done(); }); }); @@ -435,8 +482,8 @@ describe('evaluate', function() { defineSegment(segment, function() { var flag = makeFlagWithSegmentMatch(segment); var user = { key: 'foo', email: 'test@example.com', name: 'bob' }; - evaluate.evaluate(flag, user, featureStore, function(err, variation, result) { - expect(result).toBe(false); + evaluate.evaluate(flag, user, featureStore, function(err, detail) { + expect(detail.value).toBe(false); done(); }); }); @@ -463,9 +510,9 @@ describe('evaluate', function() { rules.push({ clauses: [clause], variation: 1 }); } flag.rules = rules; - evaluate.evaluate(flag, {key: 'user'}, featureStore, function(err, variation, value) { + evaluate.evaluate(flag, {key: 'user'}, featureStore, function(err, detail) { expect(err).toEqual(null); - expect(value).toEqual(false); + expect(detail.value).toEqual(false); done(); }); }); @@ -492,9 +539,9 @@ describe('evaluate', function() { } var rule = { clauses: clauses, variation: 1 }; flag.rules = [rule]; - evaluate.evaluate(flag, {key: 'user'}, featureStore, function(err, variation, value) { + evaluate.evaluate(flag, {key: 'user'}, featureStore, function(err, detail) { expect(err).toEqual(null); - expect(value).toEqual(true); + expect(detail.value).toEqual(true); done(); }); }); diff --git a/test/event_processor-test.js b/test/event_processor-test.js index 6ae8014..eb0f8f8 100644 --- a/test/event_processor-test.js +++ b/test/event_processor-test.js @@ -66,6 +66,7 @@ describe('EventProcessor', function() { expect(e.variation).toEqual(source.variation); expect(e.value).toEqual(source.value); expect(e.default).toEqual(source.default); + expect(e.reason).toEqual(source.reason); if (inlineUser) { expect(e.user).toEqual(inlineUser); } else { @@ -184,6 +185,22 @@ describe('EventProcessor', function() { }); }); + it('can include reason in feature event', function(done) { + var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); + ep = EventProcessor(sdkKey, config); + var e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: true, + reason: { kind: 'FALLTHROUGH' } }; + ep.sendEvent(e); + + flushAndGetRequest(function(output) { + expect(output.length).toEqual(2); + checkFeatureEvent(output[0], e, false, user); + checkSummaryEvent(output[1]); + done(); + }); + }); + it('still generates index event if inlineUsers is true but feature event is not tracked', function(done) { var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); ep = EventProcessor(sdkKey, config);