Merge pull request #446 from splitio/review-changes

review changes and add fallbacklabel to avoid impression

Author: ZamoraEmmanuel

src/evaluator/fallbackTreatmentsCalculator/__tests__/fallback-calculator.spec.ts

@@ -131,20 +131,4 @@ describe('FallbackTreatmentsCalculator' , () => {
       label: 'label by noFallback',
     });
   });
-
-  test('returns undefined label if no label provided', () => {
-    const config: FallbackTreatmentConfiguration = {
-      byFlag: {
-        'featureB': { treatment: 'TREATMENT_B', config: '{ value: 1 }' },
-      },
-    };
-    const calculator = new FallbackTreatmentsCalculator(loggerMock, config);
-    const result = calculator.resolve('featureB');
-
-    expect(result).toEqual({
-      treatment: 'TREATMENT_B',
-      config: '{ value: 1 }',
-      label: '',
-    });
-  });
 });

src/evaluator/fallbackTreatmentsCalculator/fallbackSanitizer/index.ts

@@ -1,4 +1,4 @@
-import { FallbackTreatment, Treatment, TreatmentWithConfig } from '../../../../types/splitio';
+import { Treatment, TreatmentWithConfig } from '../../../../types/splitio';
 import { ILogger } from '../../../logger/types';
 import { isObject, isString } from '../../../utils/lang';
 import { FallbackDiscardReason } from '../constants';
@@ -12,7 +12,7 @@ export class FallbacksSanitizer {
     return name.length <= 100 && !name.includes(' ');
   }
 
-  private static isValidTreatment(t?: Treatment | FallbackTreatment): boolean {
+  private static isValidTreatment(t?: Treatment | TreatmentWithConfig): boolean {
     const treatment = isObject(t) ? (t as TreatmentWithConfig).treatment : t;
 
     if (!isString(treatment) || treatment.length > 100) {
@@ -21,7 +21,7 @@ export class FallbacksSanitizer {
     return FallbacksSanitizer.pattern.test(treatment);
   }
 
-  static sanitizeGlobal(logger: ILogger, treatment?: string | FallbackTreatment): string | FallbackTreatment | undefined {
+  static sanitizeGlobal(logger: ILogger, treatment?: Treatment | TreatmentWithConfig): Treatment | TreatmentWithConfig | undefined {
     if (!this.isValidTreatment(treatment)) {
       logger.error(
         `Fallback treatments - Discarded fallback: ${FallbackDiscardReason.Treatment}`
@@ -33,9 +33,9 @@ export class FallbacksSanitizer {
 
   static sanitizeByFlag(
     logger: ILogger,
-    byFlagFallbacks: Record<string, string | FallbackTreatment>
-  ): Record<string, string | FallbackTreatment> {
-    const sanitizedByFlag: Record<string, string | FallbackTreatment> = {};
+    byFlagFallbacks: Record<string, Treatment | TreatmentWithConfig>
+  ): Record<string, Treatment | TreatmentWithConfig> {
+    const sanitizedByFlag: Record<string, Treatment | TreatmentWithConfig> = {};
 
     const entries = Object.keys(byFlagFallbacks);
     entries.forEach((flag) => {

src/evaluator/fallbackTreatmentsCalculator/index.ts

@@ -1,15 +1,16 @@
-import { FallbackTreatmentConfiguration, FallbackTreatment } from '../../../types/splitio';
+import { FallbackTreatmentConfiguration, Treatment, TreatmentWithConfig } from '../../../types/splitio';
 import { FallbacksSanitizer } from './fallbackSanitizer';
 import { CONTROL } from '../../utils/constants';
 import { isString } from '../../utils/lang';
 import { ILogger } from '../../logger/types';
 
 export type IFallbackTreatmentsCalculator = {
-  resolve(flagName: string, label?: string): FallbackTreatment & { label?: string };
+  resolve(flagName: string, label: string): TreatmentWithConfig & { label: string };
 }
 
+export const FALLBACK_PREFIX = 'fallback - ';
+
 export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculator {
-  private readonly labelPrefix = 'fallback - ';
   private readonly fallbacks: FallbackTreatmentConfiguration;
 
   constructor(logger: ILogger, fallbacks?: FallbackTreatmentConfiguration) {
@@ -21,7 +22,7 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
     };
   }
 
-  resolve(flagName: string, label?: string): FallbackTreatment & { label?: string } {
+  resolve(flagName: string, label: string): TreatmentWithConfig & { label: string } {
     const treatment = this.fallbacks.byFlag?.[flagName];
     if (treatment) {
       return this.copyWithLabel(treatment, label);
@@ -38,24 +39,19 @@ export class FallbackTreatmentsCalculator implements IFallbackTreatmentsCalculat
     };
   }
 
-  private copyWithLabel(fallback: string | FallbackTreatment, label?: string): FallbackTreatment & { label: string } {
+  private copyWithLabel(fallback: Treatment | TreatmentWithConfig, label: string): TreatmentWithConfig & { label: string } {
     if (isString(fallback)) {
       return {
         treatment: fallback,
         config: null,
-        label: this.resolveLabel(label),
+        label: `${FALLBACK_PREFIX}${label}`,
       };
     }
 
     return {
       treatment: fallback.treatment,
       config: fallback.config,
-      label: this.resolveLabel(label),
+      label: `${FALLBACK_PREFIX}${label}`,
     };
   }
-
-  private resolveLabel(label?: string): string {
-    return label ? `${this.labelPrefix}${label}` : '';
-  }
-
 }

src/sdkClient/client.ts

@@ -145,15 +145,16 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
 
     const { changeNumber, impressionsDisabled } = evaluation;
     let { treatment, label, config = null } = evaluation;
-    log.info(IMPRESSION, [featureFlagName, matchingKey, treatment, label]);
 
     if (treatment === CONTROL) {
       const fallbackTreatment = fallbackTreatmentsCalculator.resolve(featureFlagName, label);
       treatment = fallbackTreatment.treatment;
-      label = fallbackTreatment.label ? fallbackTreatment.label : label;
+      label = fallbackTreatment.label;
       config = fallbackTreatment.config;
     }
 
+    log.info(IMPRESSION, [featureFlagName, matchingKey, treatment, label]);
+
     if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) {
       log.info(IMPRESSION_QUEUEING);
       queue.push({

src/utils/inputValidation/splitExistence.ts

@@ -1,4 +1,4 @@
-import { SPLIT_NOT_FOUND } from '../labels';
+import { FALLBACK_SPLIT_NOT_FOUND, SPLIT_NOT_FOUND } from '../labels';
 import { IReadinessManager } from '../../readiness/types';
 import { ILogger } from '../../logger/types';
 import { WARN_NOT_EXISTENT_SPLIT } from '../../logger/constants';
@@ -9,7 +9,7 @@ import { WARN_NOT_EXISTENT_SPLIT } from '../../logger/constants';
  */
 export function validateSplitExistence(log: ILogger, readinessManager: IReadinessManager, splitName: string, labelOrSplitObj: any, method: string): boolean {
   if (readinessManager.isReady()) { // Only if it's ready we validate this, otherwise it may just be that the SDK is not ready yet.
-    if (labelOrSplitObj === SPLIT_NOT_FOUND || labelOrSplitObj == null) {
+    if (labelOrSplitObj === SPLIT_NOT_FOUND || labelOrSplitObj == null || labelOrSplitObj === FALLBACK_SPLIT_NOT_FOUND) {
       log.warn(WARN_NOT_EXISTENT_SPLIT, [method, splitName]);
       return false;
     }

src/utils/labels/index.ts

@@ -1,3 +1,5 @@
+import { FALLBACK_PREFIX } from '../../evaluator/fallbackTreatmentsCalculator';
+
 export const SPLIT_KILLED = 'killed';
 export const NO_CONDITION_MATCH = 'default rule';
 export const SPLIT_NOT_FOUND = 'definition not found';
@@ -7,3 +9,4 @@ export const SPLIT_ARCHIVED = 'archived';
 export const NOT_IN_SPLIT = 'not in split';
 export const UNSUPPORTED_MATCHER_TYPE = 'targeting rule type unsupported by sdk';
 export const PREREQUISITES_NOT_MET = 'prerequisites not met';
+export const FALLBACK_SPLIT_NOT_FOUND = FALLBACK_PREFIX + SPLIT_NOT_FOUND;

types/splitio.d.ts

@@ -1232,17 +1232,13 @@ declare namespace SplitIO {
    * User consent status.
    */
   type ConsentStatus = 'GRANTED' | 'DECLINED' | 'UNKNOWN';
-  /**
-   * Fallback treatment can be either a string (Treatment) or an object with treatment and config (TreatmentWithConfig).
-   */
-  type FallbackTreatment = TreatmentWithConfig;
   /**
    * Fallback treatments to be used when the SDK is not ready or the flag is not found.
    */
   type FallbackTreatmentConfiguration = {
-    global?: string | FallbackTreatment,
+    global?: Treatment | TreatmentWithConfig,
     byFlag?: {
-      [key: string]: string | FallbackTreatment
+      [featureFlagName: string]: Treatment | TreatmentWithConfig
     }
   }
   /**