From e59c5953f03bff3a3bcce1cd1e9716464d0f395b Mon Sep 17 00:00:00 2001 From: Jae Kim <45045038+jaeopt@users.noreply.github.com> Date: Mon, 27 Nov 2023 09:38:14 -0800 Subject: [PATCH] [FSSDK-9785] handle duplicate experiment keys with a warning (#535) When duplicate experiment keys are found in a datafile, SDK returns a correct experimentMap in OptimizelyConfig: - the experimentMap will contain the experiment later in the datafile experiments list. - add a warning log about "Duplicate experiment keys found in datafile: {key-name}" --- .../OptimizelyConfigService.java | 11 +++++- .../OptimizelyConfigServiceTest.java | 39 +++++++++++++++++-- .../src/main/java/com/optimizely/Example.java | 2 +- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java b/core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java index 8937d8572..c1ec93c01 100644 --- a/core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java +++ b/core-api/src/main/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigService.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2020-2021, Optimizely, Inc. and contributors * + * Copyright 2020-2021, 2023, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -18,6 +18,8 @@ import com.optimizely.ab.annotations.VisibleForTesting; import com.optimizely.ab.config.*; import com.optimizely.ab.config.audience.Audience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.*; @@ -31,6 +33,8 @@ public class OptimizelyConfigService { private Map> featureIdToVariablesMap = new HashMap<>(); private Map experimentMapByExperimentId = new HashMap<>(); + private static final Logger logger = LoggerFactory.getLogger(OptimizelyConfigService.class); + public OptimizelyConfigService(ProjectConfig projectConfig) { this.projectConfig = projectConfig; this.audiences = getAudiencesList(projectConfig.getTypedAudiences(), projectConfig.getAudiences()); @@ -125,6 +129,11 @@ Map getExperimentsMap() { experiment.serializeConditions(this.audiencesMap) ); + if (featureExperimentMap.containsKey(experiment.getKey())) { + // continue with this warning, so the later experiment will be used. + logger.warn("Duplicate experiment keys found in datafile: {}", experiment.getKey()); + } + featureExperimentMap.put(experiment.getKey(), optimizelyExperiment); experimentMapByExperimentId.put(experiment.getId(), optimizelyExperiment); } diff --git a/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java b/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java index 29cbe3695..418cb2494 100644 --- a/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/optimizelyconfig/OptimizelyConfigServiceTest.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2020-2021, Optimizely, Inc. and contributors * + * Copyright 2020-2021, 2023, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -15,16 +15,19 @@ ***************************************************************************/ package com.optimizely.ab.optimizelyconfig; +import ch.qos.logback.classic.Level; import com.optimizely.ab.config.*; import com.optimizely.ab.config.audience.Audience; +import com.optimizely.ab.internal.LogbackVerifier; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; import java.util.*; import static java.util.Arrays.asList; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class OptimizelyConfigServiceTest { @@ -32,6 +35,9 @@ public class OptimizelyConfigServiceTest { private OptimizelyConfigService optimizelyConfigService; private OptimizelyConfig expectedConfig; + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + @Before public void initialize() { projectConfig = generateOptimizelyConfig(); @@ -46,6 +52,33 @@ public void testGetExperimentsMap() { assertEquals(expectedConfig.getExperimentsMap(), optimizelyExperimentMap); } + @Test + public void testGetExperimentsMapWithDuplicateKeys() { + List experiments = Arrays.asList( + new Experiment( + "first", + "duplicate_key", + null, null, Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyMap(), Collections.emptyList() + ), + new Experiment( + "second", + "duplicate_key", + null, null, Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyMap(), Collections.emptyList() + ) + ); + + ProjectConfig projectConfig = mock(ProjectConfig.class); + OptimizelyConfigService optimizelyConfigService = new OptimizelyConfigService(projectConfig); + when(projectConfig.getExperiments()).thenReturn(experiments); + + Map optimizelyExperimentMap = optimizelyConfigService.getExperimentsMap(); + assertEquals("Duplicate keys should be overwritten", optimizelyExperimentMap.size(), 1); + assertEquals("Duplicate keys should be overwritten", optimizelyExperimentMap.get("duplicate_key").getId(), "second"); + logbackVerifier.expectMessage(Level.WARN, "Duplicate experiment keys found in datafile: duplicate_key"); + } + @Test public void testRevision() { String revision = optimizelyConfigService.getConfig().getRevision(); diff --git a/java-quickstart/src/main/java/com/optimizely/Example.java b/java-quickstart/src/main/java/com/optimizely/Example.java index 04d7f78da..e3bccd483 100644 --- a/java-quickstart/src/main/java/com/optimizely/Example.java +++ b/java-quickstart/src/main/java/com/optimizely/Example.java @@ -56,7 +56,7 @@ private void processVisitor(String userId, Map attributes) { public static void main(String[] args) throws InterruptedException { Optimizely optimizely = OptimizelyFactory.newDefaultInstance("BX9Y3bTa4YErpHZEMpAwHm"); - + Example example = new Example(optimizely); Random random = new Random();