From 51d882f95d983390b149cf62e0aa92acca9a722a Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Thu, 30 Jan 2025 22:03:55 +0600 Subject: [PATCH 1/2] [FSSDK-11101] refactor handling of no config availability --- lib/message/error_message.ts | 2 +- lib/optimizely/index.ts | 107 ++++++++++++++--------------------- 2 files changed, 43 insertions(+), 66 deletions(-) diff --git a/lib/message/error_message.ts b/lib/message/error_message.ts index 0dcb28567..5b12a5f68 100644 --- a/lib/message/error_message.ts +++ b/lib/message/error_message.ts @@ -71,7 +71,7 @@ export const UNKNOWN_CONDITION_TYPE = export const UNKNOWN_MATCH_TYPE = 'Audience condition %s uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK.'; export const UNRECOGNIZED_DECIDE_OPTION = 'Unrecognized decide option %s provided.'; -export const INVALID_OBJECT = 'Optimizely object is not valid. Failing %s.'; +export const NO_PROJECT_CONFIG_FAILURE = 'No project config available. Failing %s.'; export const EVENT_KEY_NOT_FOUND = 'Event key %s is not in datafile.'; export const NOT_TRACKING_USER = 'Not tracking user %s.'; export const VARIABLE_REQUESTED_WITH_WRONG_TYPE = diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index e7cb921de..8cd8a6df6 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -22,6 +22,7 @@ import { OdpManager } from '../odp/odp_manager'; import { VuidManager } from '../vuid/vuid_manager'; import { OdpEvent } from '../odp/event_manager/odp_event'; import { OptimizelySegmentOption } from '../odp/segment_manager/optimizely_segment_option'; +import { BaseService } from '../service'; import { UserAttributes, @@ -71,7 +72,7 @@ import { ODP_EVENT_FAILED_ODP_MANAGER_MISSING, UNABLE_TO_GET_VUID_VUID_MANAGER_NOT_AVAILABLE, UNRECOGNIZED_DECIDE_OPTION, - INVALID_OBJECT, + NO_PROJECT_CONFIG_FAILURE, EVENT_KEY_NOT_FOUND, NOT_TRACKING_USER, VARIABLE_REQUESTED_WITH_WRONG_TYPE, @@ -267,11 +268,9 @@ export default class Optimizely implements Client { /** * Returns a truthy value if this instance currently has a valid project config - * object, and the initial configuration object that was passed into the - * constructor was also valid. * @return {boolean} */ - isValidInstance(): boolean { + private hasProjectConfig(): boolean { return !!this.projectConfigManager.getConfig(); } @@ -284,8 +283,9 @@ export default class Optimizely implements Client { */ activate(experimentKey: string, userId: string, attributes?: UserAttributes): string | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'activate'); + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'activate'); return null; } @@ -293,11 +293,6 @@ export default class Optimizely implements Client { return this.notActivatingExperiment(experimentKey, userId); } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { - return null; - } - try { const variationKey = this.getVariation(experimentKey, userId, attributes); if (variationKey === null) { @@ -394,8 +389,9 @@ export default class Optimizely implements Client { return; } - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'track'); + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'track'); return; } @@ -403,11 +399,6 @@ export default class Optimizely implements Client { return; } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { - return; - } - if (!projectConfig.eventWithKeyExists(configObj, eventKey)) { this.logger?.warn(EVENT_KEY_NOT_FOUND, eventKey); @@ -453,8 +444,9 @@ export default class Optimizely implements Client { */ getVariation(experimentKey: string, userId: string, attributes?: UserAttributes): string | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getVariation'); + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getVariation'); return null; } @@ -463,11 +455,6 @@ export default class Optimizely implements Client { return null; } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { - return null; - } - const experiment = configObj.experimentKeyMap[experimentKey]; if (!experiment || experiment.isRollout) { this.logger?.debug(INVALID_EXPERIMENT_KEY_INFO, experimentKey); @@ -624,8 +611,9 @@ export default class Optimizely implements Client { */ isFeatureEnabled(featureKey: string, userId: string, attributes?: UserAttributes): boolean { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'isFeatureEnabled'); + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'isFeatureEnabled'); return false; } @@ -633,11 +621,6 @@ export default class Optimizely implements Client { return false; } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { - return false; - } - const feature = projectConfig.getFeatureFromKey(configObj, featureKey, this.logger); if (!feature) { return false; @@ -704,17 +687,14 @@ export default class Optimizely implements Client { getEnabledFeatures(userId: string, attributes?: UserAttributes): string[] { try { const enabledFeatures: string[] = []; - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getEnabledFeatures'); - return enabledFeatures; - } - if (!this.validateInputs({ user_id: userId })) { + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getEnabledFeatures'); return enabledFeatures; } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { + if (!this.validateInputs({ user_id: userId })) { return enabledFeatures; } @@ -752,8 +732,8 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): FeatureVariableValue { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariable'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariable'); return null; } return this.getFeatureVariableForType(featureKey, variableKey, null, userId, attributes); @@ -946,8 +926,8 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): boolean | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariableBoolean'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableBoolean'); return null; } return this.getFeatureVariableForType( @@ -984,8 +964,8 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): number | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariableDouble'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableDouble'); return null; } return this.getFeatureVariableForType( @@ -1022,8 +1002,8 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): number | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariableInteger'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableInteger'); return null; } return this.getFeatureVariableForType( @@ -1060,8 +1040,8 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): string | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariableString'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableString'); return null; } return this.getFeatureVariableForType( @@ -1093,8 +1073,8 @@ export default class Optimizely implements Client { */ getFeatureVariableJSON(featureKey: string, variableKey: string, userId: string, attributes: UserAttributes): unknown { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getFeatureVariableJSON'); + if (!this.hasProjectConfig()) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableJSON'); return null; } return this.getFeatureVariableForType(featureKey, variableKey, FEATURE_VARIABLE_TYPES.JSON, userId, attributes); @@ -1120,17 +1100,14 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): { [variableKey: string]: unknown } | null { try { - if (!this.isValidInstance()) { - this.logger?.error(INVALID_OBJECT, 'getAllFeatureVariables'); - return null; - } + const configObj = this.projectConfigManager.getConfig(); - if (!this.validateInputs({ feature_key: featureKey, user_id: userId }, attributes)) { + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getAllFeatureVariables'); return null; } - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { + if (!this.validateInputs({ feature_key: featureKey, user_id: userId }, attributes)) { return null; } @@ -1425,8 +1402,8 @@ export default class Optimizely implements Client { decide(user: OptimizelyUserContext, key: string, options: OptimizelyDecideOption[] = []): OptimizelyDecision { const configObj = this.projectConfigManager.getConfig(); - if (!this.isValidInstance() || !configObj) { - this.logger?.error(INVALID_OBJECT, 'decide'); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decide'); return newErrorDecision(key, user, [DECISION_MESSAGES.SDK_NOT_READY]); } @@ -1570,8 +1547,8 @@ export default class Optimizely implements Client { const configObj = this.projectConfigManager.getConfig() - if (!this.isValidInstance() || !configObj) { - this.logger?.error(INVALID_OBJECT, 'decideForKeys'); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decideForKeys'); return decisionMap; } if (keys.length === 0) { @@ -1638,10 +1615,10 @@ export default class Optimizely implements Client { user: OptimizelyUserContext, options: OptimizelyDecideOption[] = [] ): { [key: string]: OptimizelyDecision } { - const configObj = this.projectConfigManager.getConfig(); const decisionMap: { [key: string]: OptimizelyDecision } = {}; - if (!this.isValidInstance() || !configObj) { - this.logger?.error(INVALID_OBJECT, 'decideAll'); + const configObj = this.projectConfigManager.getConfig(); + if (!configObj) { + this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decideAll'); return decisionMap; } From 534177b6ccd769740cfb7e01677682ef3cf0f42d Mon Sep 17 00:00:00 2001 From: Raju Ahmed Date: Mon, 3 Feb 2025 21:42:46 +0600 Subject: [PATCH 2/2] review fix --- lib/optimizely/index.ts | 54 ++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 8cd8a6df6..416b4f3c8 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -266,14 +266,6 @@ export default class Optimizely implements Client { return this.projectConfigManager.getConfig() || null; } - /** - * Returns a truthy value if this instance currently has a valid project config - * @return {boolean} - */ - private hasProjectConfig(): boolean { - return !!this.projectConfigManager.getConfig(); - } - /** * Buckets visitor and sends impression event to Optimizely. * @param {string} experimentKey @@ -283,7 +275,7 @@ export default class Optimizely implements Client { */ activate(experimentKey: string, userId: string, attributes?: UserAttributes): string | null { try { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'activate'); return null; @@ -348,7 +340,7 @@ export default class Optimizely implements Client { return; } - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return; } @@ -389,7 +381,7 @@ export default class Optimizely implements Client { return; } - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'track'); return; @@ -444,7 +436,7 @@ export default class Optimizely implements Client { */ getVariation(experimentKey: string, userId: string, attributes?: UserAttributes): string | null { try { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getVariation'); return null; @@ -504,7 +496,7 @@ export default class Optimizely implements Client { return false; } - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return false; } @@ -528,7 +520,7 @@ export default class Optimizely implements Client { return null; } - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return null; } @@ -611,7 +603,7 @@ export default class Optimizely implements Client { */ isFeatureEnabled(featureKey: string, userId: string, attributes?: UserAttributes): boolean { try { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'isFeatureEnabled'); return false; @@ -688,7 +680,7 @@ export default class Optimizely implements Client { try { const enabledFeatures: string[] = []; - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getEnabledFeatures'); return enabledFeatures; @@ -732,7 +724,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): FeatureVariableValue { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariable'); return null; } @@ -776,7 +768,7 @@ export default class Optimizely implements Client { return null; } - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return null; } @@ -862,7 +854,7 @@ export default class Optimizely implements Client { variable: FeatureVariable, userId: string ): FeatureVariableValue { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return null; } @@ -926,7 +918,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): boolean | null { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableBoolean'); return null; } @@ -964,7 +956,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): number | null { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableDouble'); return null; } @@ -1002,7 +994,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): number | null { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableInteger'); return null; } @@ -1040,7 +1032,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): string | null { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableString'); return null; } @@ -1073,7 +1065,7 @@ export default class Optimizely implements Client { */ getFeatureVariableJSON(featureKey: string, variableKey: string, userId: string, attributes: UserAttributes): unknown { try { - if (!this.hasProjectConfig()) { + if (!this.getProjectConfig()) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getFeatureVariableJSON'); return null; } @@ -1100,7 +1092,7 @@ export default class Optimizely implements Client { attributes?: UserAttributes ): { [variableKey: string]: unknown } | null { try { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'getAllFeatureVariables'); @@ -1201,7 +1193,7 @@ export default class Optimizely implements Client { */ getOptimizelyConfig(): OptimizelyConfig | null { try { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { return null; } @@ -1400,7 +1392,7 @@ export default class Optimizely implements Client { } decide(user: OptimizelyUserContext, key: string, options: OptimizelyDecideOption[] = []): OptimizelyDecision { - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decide'); @@ -1545,7 +1537,7 @@ export default class Optimizely implements Client { const flagsWithoutForcedDecision = []; const validKeys = []; - const configObj = this.projectConfigManager.getConfig() + const configObj = this.getProjectConfig() if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decideForKeys'); @@ -1616,7 +1608,7 @@ export default class Optimizely implements Client { options: OptimizelyDecideOption[] = [] ): { [key: string]: OptimizelyDecision } { const decisionMap: { [key: string]: OptimizelyDecision } = {}; - const configObj = this.projectConfigManager.getConfig(); + const configObj = this.getProjectConfig(); if (!configObj) { this.errorReporter.report(NO_PROJECT_CONFIG_FAILURE, 'decideAll'); return decisionMap; @@ -1631,7 +1623,7 @@ export default class Optimizely implements Client { * Updates ODP Config with most recent ODP key, host, pixelUrl, and segments from the project config */ private updateOdpSettings(): void { - const projectConfig = this.projectConfigManager.getConfig(); + const projectConfig = this.getProjectConfig(); if (!projectConfig) { return; @@ -1673,7 +1665,7 @@ export default class Optimizely implements Client { * @returns { boolean } `true` if ODP settings were found in the datafile otherwise `false` */ public isOdpIntegrated(): boolean { - return this.projectConfigManager.getConfig()?.odpIntegrationConfig?.integrated ?? false; + return this.getProjectConfig()?.odpIntegrationConfig?.integrated ?? false; } /**