Skip to content

Commit 7347f06

Browse files
mikeproeng37mikeproeng37
authored
refactor(init): build invalid instance if datafile cannot be parsed. (#209)
1 parent 4ecd765 commit 7347f06

File tree

7 files changed

+250
-39
lines changed

7 files changed

+250
-39
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
local.properties
1919

2020
**/build
21+
bin
2122
out
2223
classes
2324

‎core-api/src/main/java/com/optimizely/ab/Optimizely.java‎

Lines changed: 101 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import com.optimizely.ab.config.ProjectConfig;
3030
import com.optimizely.ab.config.Variation;
3131
import com.optimizely.ab.config.parser.ConfigParseException;
32-
import com.optimizely.ab.config.parser.DefaultConfigParser;
3332
import com.optimizely.ab.error.ErrorHandler;
3433
import com.optimizely.ab.error.NoOpErrorHandler;
3534
import com.optimizely.ab.event.EventHandler;
@@ -80,33 +79,65 @@ public class Optimizely {
8079

8180
private static final Logger logger = LoggerFactory.getLogger(Optimizely.class);
8281

83-
@VisibleForTesting final DecisionService decisionService;
82+
@VisibleForTesting DecisionService decisionService;
8483
@VisibleForTesting final EventFactory eventFactory;
85-
@VisibleForTesting final ProjectConfig projectConfig;
84+
@VisibleForTesting ProjectConfig projectConfig;
8685
@VisibleForTesting final EventHandler eventHandler;
8786
@VisibleForTesting final ErrorHandler errorHandler;
87+
private boolean isValid;
8888
public final NotificationCenter notificationCenter = new NotificationCenter();
8989

9090
@Nullable private final UserProfileService userProfileService;
9191

92-
private Optimizely(@Nonnull ProjectConfig projectConfig,
93-
@Nonnull DecisionService decisionService,
94-
@Nonnull EventHandler eventHandler,
92+
private Optimizely(@Nonnull EventHandler eventHandler,
9593
@Nonnull EventFactory eventFactory,
9694
@Nonnull ErrorHandler errorHandler,
95+
@Nullable DecisionService decisionService,
9796
@Nullable UserProfileService userProfileService) {
98-
this.projectConfig = projectConfig;
9997
this.decisionService = decisionService;
10098
this.eventHandler = eventHandler;
10199
this.eventFactory = eventFactory;
102100
this.errorHandler = errorHandler;
103101
this.userProfileService = userProfileService;
104102
}
105103

106-
// Do work here that should be done once per Optimizely lifecycle
104+
/**
105+
* Initializes the SDK state. Can conceivably re-use this in the future with datafile sync where
106+
* we can re-initialize the SDK instead of re-instantiating.
107+
*/
107108
@VisibleForTesting
108-
void initialize() {
109+
void initialize(@Nonnull String datafile, @Nullable ProjectConfig projectConfig) {
110+
if (projectConfig == null) {
111+
try {
112+
projectConfig = new ProjectConfig.Builder()
113+
.withDatafile(datafile)
114+
.build();
115+
isValid = true;
116+
logger.info("Datafile is valid");
117+
} catch (ConfigParseException ex) {
118+
logger.error("Unable to parse the datafile", ex);
119+
logger.info("Datafile is invalid");
120+
errorHandler.handleError(new OptimizelyRuntimeException(ex));
121+
}
122+
} else {
123+
isValid = true;
124+
}
125+
126+
this.projectConfig = projectConfig;
127+
if (decisionService == null) {
128+
Bucketer bucketer = new Bucketer(projectConfig);
129+
decisionService = new DecisionService(bucketer, errorHandler, projectConfig, userProfileService);
130+
}
131+
}
109132

133+
/**
134+
* Determine if the instance of the Optimizely client is valid. An instance can be deemed invalid if it was not
135+
* initialized properly due to an invalid datafile being passed in.
136+
* @return True if the Optimizely instance is valid.
137+
* False if the Optimizely instance is not valid.
138+
*/
139+
public boolean isValid() {
140+
return isValid;
110141
}
111142

112143
//======== activate calls ========//
@@ -121,6 +152,10 @@ Variation activate(@Nonnull String experimentKey,
121152
Variation activate(@Nonnull String experimentKey,
122153
@Nonnull String userId,
123154
@Nonnull Map<String, ?> attributes) throws UnknownExperimentException {
155+
if (!isValid) {
156+
logger.error("Optimizely instance is not valid, failing activate call.");
157+
return null;
158+
}
124159

125160
if (experimentKey == null) {
126161
logger.error("The experimentKey parameter must be nonnull.");
@@ -165,6 +200,10 @@ Variation activate(@Nonnull ProjectConfig projectConfig,
165200
@Nonnull Experiment experiment,
166201
@Nonnull String userId,
167202
@Nonnull Map<String, ?> attributes) {
203+
if (!isValid) {
204+
logger.error("Optimizely instance is not valid, failing activate call.");
205+
return null;
206+
}
168207

169208
if (!validateUserId(userId)){
170209
logger.info("Not activating user \"{}\" for experiment \"{}\".", userId, experiment.getKey());
@@ -236,6 +275,10 @@ public void track(@Nonnull String eventName,
236275
@Nonnull String userId,
237276
@Nonnull Map<String, ?> attributes,
238277
@Nonnull Map<String, ?> eventTags) throws UnknownEventTypeException {
278+
if (!isValid) {
279+
logger.error("Optimizely instance is not valid, failing track call.");
280+
return;
281+
}
239282

240283
if (!validateUserId(userId)) {
241284
logger.info("Not tracking event \"{}\".", eventName);
@@ -345,6 +388,11 @@ public void track(@Nonnull String eventName,
345388
public @Nonnull Boolean isFeatureEnabled(@Nonnull String featureKey,
346389
@Nonnull String userId,
347390
@Nonnull Map<String, ?> attributes) {
391+
if (!isValid) {
392+
logger.error("Optimizely instance is not valid, failing isFeatureEnabled call.");
393+
return false;
394+
}
395+
348396
if (featureKey == null) {
349397
logger.warn("The featureKey parameter must be nonnull.");
350398
return false;
@@ -411,6 +459,11 @@ else if (userId == null) {
411459
@Nonnull String variableKey,
412460
@Nonnull String userId,
413461
@Nonnull Map<String, ?> attributes) {
462+
if (!isValid) {
463+
logger.error("Optimizely instance is not valid, failing getFeatureVariableBoolean call.");
464+
return null;
465+
}
466+
414467
String variableValue = getFeatureVariableValueForType(
415468
featureKey,
416469
variableKey,
@@ -451,6 +504,11 @@ else if (userId == null) {
451504
@Nonnull String variableKey,
452505
@Nonnull String userId,
453506
@Nonnull Map<String, ?> attributes) {
507+
if (!isValid) {
508+
logger.error("Optimizely instance is not valid, failing getFeatureVariableDouble call.");
509+
return null;
510+
}
511+
454512
String variableValue = getFeatureVariableValueForType(
455513
featureKey,
456514
variableKey,
@@ -496,6 +554,11 @@ else if (userId == null) {
496554
@Nonnull String variableKey,
497555
@Nonnull String userId,
498556
@Nonnull Map<String, ?> attributes) {
557+
if (!isValid) {
558+
logger.error("Optimizely instance is not valid, failing getFeatureVariableInteger call.");
559+
return null;
560+
}
561+
499562
String variableValue = getFeatureVariableValueForType(
500563
featureKey,
501564
variableKey,
@@ -541,6 +604,11 @@ else if (userId == null) {
541604
@Nonnull String variableKey,
542605
@Nonnull String userId,
543606
@Nonnull Map<String, ?> attributes) {
607+
if (!isValid) {
608+
logger.error("Optimizely instance is not valid, failing getFeatureVariableString call.");
609+
return null;
610+
}
611+
544612
return getFeatureVariableValueForType(
545613
featureKey,
546614
variableKey,
@@ -617,6 +685,11 @@ else if (userId == null) {
617685
public List<String> getEnabledFeatures(@Nonnull String userId, @Nonnull Map<String, ?> attributes) {
618686
List<String> enabledFeaturesList = new ArrayList<String>();
619687

688+
if (!isValid) {
689+
logger.error("Optimizely instance is not valid, failing getEnabledFeatures call.");
690+
return enabledFeaturesList;
691+
}
692+
620693
if (!validateUserId(userId)){
621694
return enabledFeaturesList;
622695
}
@@ -660,6 +733,11 @@ Variation getVariation(@Nonnull String experimentKey,
660733
Variation getVariation(@Nonnull String experimentKey,
661734
@Nonnull String userId,
662735
@Nonnull Map<String, ?> attributes) {
736+
if (!isValid) {
737+
logger.error("Optimizely instance is not valid, failing getVariation call.");
738+
return null;
739+
}
740+
663741
if (!validateUserId(userId)) {
664742
return null;
665743
}
@@ -697,7 +775,10 @@ Variation getVariation(@Nonnull String experimentKey,
697775
public boolean setForcedVariation(@Nonnull String experimentKey,
698776
@Nonnull String userId,
699777
@Nullable String variationKey) {
700-
778+
if (!isValid) {
779+
logger.error("Optimizely instance is not valid, failing setForcedVariation call.");
780+
return false;
781+
}
701782

702783
return projectConfig.setForcedVariation(experimentKey, userId, variationKey);
703784
}
@@ -715,6 +796,11 @@ public boolean setForcedVariation(@Nonnull String experimentKey,
715796
*/
716797
public @Nullable Variation getForcedVariation(@Nonnull String experimentKey,
717798
@Nonnull String userId) {
799+
if (!isValid) {
800+
logger.error("Optimizely instance is not valid, failing getForcedVariation call.");
801+
return null;
802+
}
803+
718804
return projectConfig.getForcedVariation(experimentKey, userId);
719805
}
720806

@@ -869,17 +955,7 @@ protected Builder withConfig(ProjectConfig projectConfig) {
869955
return this;
870956
}
871957

872-
public Optimizely build() throws ConfigParseException {
873-
if (projectConfig == null) {
874-
projectConfig = new ProjectConfig.Builder()
875-
.withDatafile(datafile)
876-
.build();
877-
}
878-
879-
if (bucketer == null) {
880-
bucketer = new Bucketer(projectConfig);
881-
}
882-
958+
public Optimizely build() {
883959
if (clientEngine == null) {
884960
clientEngine = ClientEngine.JAVA_SDK;
885961
}
@@ -897,12 +973,13 @@ public Optimizely build() throws ConfigParseException {
897973
errorHandler = new NoOpErrorHandler();
898974
}
899975

900-
if (decisionService == null) {
976+
// Used for convenience while unit testing to override/mock bucketing. This interface is NOT public and should be refactored out.
977+
if (bucketer != null && decisionService == null) {
901978
decisionService = new DecisionService(bucketer, errorHandler, projectConfig, userProfileService);
902979
}
903980

904-
Optimizely optimizely = new Optimizely(projectConfig, decisionService, eventHandler, eventFactory, errorHandler, userProfileService);
905-
optimizely.initialize();
981+
Optimizely optimizely = new Optimizely(eventHandler, eventFactory, errorHandler, decisionService, userProfileService);
982+
optimizely.initialize(datafile, projectConfig);
906983
return optimizely;
907984
}
908985
}

‎core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public class DecisionService {
6262
*/
6363
public DecisionService(@Nonnull Bucketer bucketer,
6464
@Nonnull ErrorHandler errorHandler,
65-
@Nonnull ProjectConfig projectConfig,
65+
@Nullable ProjectConfig projectConfig,
6666
@Nullable UserProfileService userProfileService) {
6767
this.bucketer = bucketer;
6868
this.errorHandler = errorHandler;

‎core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ public Builder withDatafile(String datafile) {
614614
/**
615615
* @return a {@link ProjectConfig} instance given a JSON string datafile
616616
*/
617-
public ProjectConfig build() throws ConfigParseException{
617+
public ProjectConfig build() throws ConfigParseException {
618618
if (datafile == null) {
619619
throw new ConfigParseException("Unable to parse null datafile.");
620620
}

‎core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,10 @@
3333
import org.mockito.junit.MockitoJUnit;
3434
import org.mockito.junit.MockitoRule;
3535

36-
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV2;
37-
import static com.optimizely.ab.config.ProjectConfigTestUtils.validConfigJsonV3;
38-
import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV2;
39-
import static com.optimizely.ab.config.ProjectConfigTestUtils.validProjectConfigV3;
36+
import static com.optimizely.ab.config.ProjectConfigTestUtils.*;
4037
import static org.hamcrest.CoreMatchers.instanceOf;
4138
import static org.hamcrest.CoreMatchers.is;
39+
import static org.junit.Assert.assertFalse;
4240
import static org.junit.Assert.assertThat;
4341
import static org.mockito.Mockito.mock;
4442

@@ -154,20 +152,31 @@ public void withCustomClientVersion() throws Exception {
154152

155153
@SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
156154
@Test
157-
public void builderThrowsConfigParseExceptionForNullDatafile() throws Exception {
158-
thrown.expect(ConfigParseException.class);
159-
Optimizely.builder(null, mockEventHandler).build();
155+
public void nullDatafileResultsInInvalidOptimizelyInstance() throws Exception {
156+
Optimizely optimizelyClient = Optimizely.builder(null, mockEventHandler).build();
157+
158+
assertFalse(optimizelyClient.isValid());
160159
}
161160

162161
@Test
163-
public void builderThrowsConfigParseExceptionForEmptyDatafile() throws Exception {
164-
thrown.expect(ConfigParseException.class);
165-
Optimizely.builder("", mockEventHandler).build();
162+
public void emptyDatafileResultsInInvalidOptimizelyInstance() throws Exception {
163+
Optimizely optimizelyClient = Optimizely.builder("", mockEventHandler).build();
164+
165+
assertFalse(optimizelyClient.isValid());
166166
}
167167

168168
@Test
169-
public void builderThrowsConfigParseExceptionForInvalidDatafile() throws Exception {
170-
thrown.expect(ConfigParseException.class);
171-
Optimizely.builder("{invalidDatafile}", mockEventHandler).build();
169+
public void invalidDatafileResultsInInvalidOptimizelyInstance() throws Exception {
170+
Optimizely optimizelyClient = Optimizely.builder("{invalidDatafile}", mockEventHandler).build();
171+
172+
assertFalse(optimizelyClient.isValid());
173+
}
174+
175+
@Test
176+
public void unsupportedDatafileResultsInInvalidOptimizelyInstance() throws Exception {
177+
Optimizely optimizelyClient = Optimizely.builder(invalidProjectConfigV5(), mockEventHandler)
178+
.build();
179+
180+
assertFalse(optimizelyClient.isValid());
172181
}
173182
}

0 commit comments

Comments
 (0)