Skip to content

Commit

Permalink
[FSSDK-9785] handle duplicate experiment keys with a warning (#535)
Browse files Browse the repository at this point in the history
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}"
  • Loading branch information
jaeopt authored Nov 27, 2023
1 parent ca2a894 commit e59c595
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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. *
Expand All @@ -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.*;

Expand All @@ -31,6 +33,8 @@ public class OptimizelyConfigService {
private Map<String, List<FeatureVariable>> featureIdToVariablesMap = new HashMap<>();
private Map<String, OptimizelyExperiment> 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());
Expand Down Expand Up @@ -125,6 +129,11 @@ Map<String, OptimizelyExperiment> 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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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. *
Expand All @@ -15,23 +15,29 @@
***************************************************************************/
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 {

private ProjectConfig projectConfig;
private OptimizelyConfigService optimizelyConfigService;
private OptimizelyConfig expectedConfig;

@Rule
public LogbackVerifier logbackVerifier = new LogbackVerifier();

@Before
public void initialize() {
projectConfig = generateOptimizelyConfig();
Expand All @@ -46,6 +52,33 @@ public void testGetExperimentsMap() {
assertEquals(expectedConfig.getExperimentsMap(), optimizelyExperimentMap);
}

@Test
public void testGetExperimentsMapWithDuplicateKeys() {
List<Experiment> experiments = Arrays.asList(
new Experiment(
"first",
"duplicate_key",
null, null, Collections.<String>emptyList(), null,
Collections.<Variation>emptyList(), Collections.<String, String>emptyMap(), Collections.<TrafficAllocation>emptyList()
),
new Experiment(
"second",
"duplicate_key",
null, null, Collections.<String>emptyList(), null,
Collections.<Variation>emptyList(), Collections.<String, String>emptyMap(), Collections.<TrafficAllocation>emptyList()
)
);

ProjectConfig projectConfig = mock(ProjectConfig.class);
OptimizelyConfigService optimizelyConfigService = new OptimizelyConfigService(projectConfig);
when(projectConfig.getExperiments()).thenReturn(experiments);

Map<String, OptimizelyExperiment> 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();
Expand Down
2 changes: 1 addition & 1 deletion java-quickstart/src/main/java/com/optimizely/Example.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private void processVisitor(String userId, Map<String, Object> attributes) {

public static void main(String[] args) throws InterruptedException {
Optimizely optimizely = OptimizelyFactory.newDefaultInstance("BX9Y3bTa4YErpHZEMpAwHm");

Example example = new Example(optimizely);
Random random = new Random();

Expand Down

0 comments on commit e59c595

Please sign in to comment.