Skip to content

Commit 4e2c479

Browse files
raju-optiraju-opti
committed
update ups batching
1 parent 767f2ce commit 4e2c479

File tree

4 files changed

+89
-37
lines changed

4 files changed

+89
-37
lines changed

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

+9-5
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,10 @@ Optional<FeatureDecision> getForcedDecision(@Nonnull String flagKey,
13171317
OptimizelyDecision decide(@Nonnull OptimizelyUserContext user,
13181318
@Nonnull String key,
13191319
@Nonnull List<OptimizelyDecideOption> options) {
1320+
ProjectConfig projectConfig = getProjectConfig();
1321+
if (projectConfig == null) {
1322+
return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason());
1323+
}
13201324
return decideForKeys(user, Arrays.asList(key), options).get(key);
13211325

13221326
// ProjectConfig projectConfig = getProjectConfig();
@@ -1509,15 +1513,12 @@ Map<String, OptimizelyDecision> decideForKeys(@Nonnull OptimizelyUserContext use
15091513

15101514
ProjectConfig projectConfig = getProjectConfig();
15111515
if (projectConfig == null) {
1512-
logger.error("Optimizely instance is not valid, failing isFeatureEnabled call.");
1516+
logger.error("Optimizely instance is not valid, failing decideForKeys call.");
15131517
return decisionMap;
15141518
}
15151519

15161520
if (keys.isEmpty()) return decisionMap;
15171521

1518-
String userId = user.getUserId();
1519-
Map<String, Object> attributes = user.getAttributes();
1520-
Boolean decisionEventDispatched = false;
15211522
List<OptimizelyDecideOption> allOptions = getAllOptions(options);
15221523

15231524
Map<String, FeatureDecision> flagDecisions = new HashMap<>();
@@ -1561,7 +1562,10 @@ Map<String, OptimizelyDecision> decideForKeys(@Nonnull OptimizelyUserContext use
15611562
OptimizelyDecision optimizelyDecision = createOptimizelyDecision(
15621563
user, key, flagDecision, decisionReasons, allOptions, projectConfig
15631564
);
1564-
decisionMap.put(key, optimizelyDecision);
1565+
1566+
if (!allOptions.contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || optimizelyDecision.getEnabled()) {
1567+
decisionMap.put(key, optimizelyDecision);
1568+
}
15651569
}
15661570

15671571
return decisionMap;

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

+35-19
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
8383
@Nonnull OptimizelyUserContext user,
8484
@Nonnull ProjectConfig projectConfig,
8585
@Nonnull List<OptimizelyDecideOption> options,
86-
@Nonnull UserProfile userProfile,
86+
@Nullable UserProfileTracker userProfileTracker,
8787
@Nullable DecisionReasons reasons) {
8888
if (reasons == null) {
8989
reasons = DefaultDecisionReasons.newInstance();
@@ -111,8 +111,8 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
111111
return new DecisionResponse(variation, reasons);
112112
}
113113

114-
if (userProfile != null) {
115-
decisionVariation = getStoredVariation(experiment, userProfile, projectConfig);
114+
if (userProfileTracker != null) {
115+
decisionVariation = getStoredVariation(experiment, userProfileTracker.userProfile, projectConfig);
116116
reasons.merge(decisionVariation.getReasons());
117117
variation = decisionVariation.getResult();
118118
// return the stored variation if it exists
@@ -131,8 +131,9 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
131131
variation = decisionVariation.getResult();
132132

133133
if (variation != null) {
134-
if (userProfile != null) {
135-
updateUserProfile(experiment, variation, userProfile);
134+
if (userProfileTracker != null) {
135+
updateUserProfile(experiment, variation, userProfileTracker.userProfile);
136+
userProfileTracker.profileUpdated = true;
136137
} else {
137138
logger.debug("This decision will not be saved since the UserProfileService is null.");
138139
}
@@ -164,16 +165,19 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
164165

165166
// fetch the user profile map from the user profile service
166167
boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE);
167-
UserProfile userProfile = null;
168+
// UserProfile userProfile = null;
169+
UserProfileTracker userProfileTracker = null;
168170

169171
if (userProfileService != null && !ignoreUPS) {
170-
userProfile = getUserProfile(user.getUserId(), reasons);
172+
UserProfile userProfile = getUserProfile(user.getUserId(), reasons);
173+
userProfileTracker = new UserProfileTracker(userProfile, false);
171174
}
172175

173-
DecisionResponse<Variation> response = getVariation(experiment, user, projectConfig, options, userProfile, reasons);
174176

175-
if(userProfileService != null && !ignoreUPS) {
176-
saveUserProfile(userProfile);
177+
DecisionResponse<Variation> response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons);
178+
179+
if(userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) {
180+
saveUserProfile(userProfileTracker.userProfile);
177181
}
178182
return response;
179183
}
@@ -254,6 +258,16 @@ private UserProfile getUserProfile(String userId, DecisionReasons reasons) {
254258
return userProfile;
255259
}
256260

261+
static class UserProfileTracker {
262+
public UserProfile userProfile;
263+
public boolean profileUpdated;
264+
265+
UserProfileTracker(UserProfile userProfile, boolean profileUpdated) {
266+
this.userProfile = userProfile;
267+
this.profileUpdated = profileUpdated;
268+
}
269+
}
270+
257271
/**
258272
* Get the variation the user is bucketed into for the FeatureFlag
259273
*
@@ -271,10 +285,11 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
271285
DecisionReasons upsReasons = DefaultDecisionReasons.newInstance();
272286

273287
boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE);
274-
UserProfile userProfile = null;
288+
UserProfileTracker userProfileTracker = null;
275289

276290
if (userProfileService != null && !ignoreUPS) {
277-
userProfile = getUserProfile(user.getUserId(), upsReasons);
291+
UserProfile userProfile = getUserProfile(user.getUserId(), upsReasons);
292+
userProfileTracker = new UserProfileTracker(userProfile, false);
278293
}
279294

280295
List<DecisionResponse<FeatureDecision>> decisions = new ArrayList<>();
@@ -283,7 +298,7 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
283298
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
284299
reasons.merge(upsReasons);
285300

286-
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfile);
301+
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker);
287302
reasons.merge(decisionVariationResponse.getReasons());
288303

289304
FeatureDecision decision = decisionVariationResponse.getResult();
@@ -309,8 +324,8 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
309324
decisions.add(new DecisionResponse(decision, reasons));
310325
}
311326

312-
if (userProfileService != null && !ignoreUPS) {
313-
saveUserProfile(userProfile);
327+
if (userProfileService != null && !ignoreUPS && userProfileTracker.profileUpdated) {
328+
saveUserProfile(userProfileTracker.userProfile);
314329
}
315330

316331
return decisions;
@@ -360,13 +375,14 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
360375
@Nonnull FeatureFlag featureFlag,
361376
@Nonnull OptimizelyUserContext user,
362377
@Nonnull List<OptimizelyDecideOption> options,
363-
@Nullable UserProfile userProfile) {
378+
@Nullable UserProfileTracker userProfileTracker) {
364379
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
365380
if (!featureFlag.getExperimentIds().isEmpty()) {
366381
for (String experimentId : featureFlag.getExperimentIds()) {
367382
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
368383

369-
DecisionResponse<Variation> decisionVariation = getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfile);
384+
DecisionResponse<Variation> decisionVariation =
385+
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker);
370386
reasons.merge(decisionVariation.getReasons());
371387
Variation variation = decisionVariation.getResult();
372388

@@ -777,7 +793,7 @@ public DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proje
777793
@Nonnull Experiment rule,
778794
@Nonnull OptimizelyUserContext user,
779795
@Nonnull List<OptimizelyDecideOption> options,
780-
@Nullable UserProfile userProfile) {
796+
@Nullable UserProfileTracker userProfileTracker) {
781797
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
782798

783799
String ruleKey = rule != null ? rule.getKey() : null;
@@ -792,7 +808,7 @@ public DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proje
792808
return new DecisionResponse(variation, reasons);
793809
}
794810
//regular decision
795-
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfile, null);
811+
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null);
796812
reasons.merge(decisionResponse.getReasons());
797813

798814
variation = decisionResponse.getResult();

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

+26-5
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@
2323
import com.optimizely.ab.bucketing.UserProfileService;
2424
import com.optimizely.ab.config.*;
2525
import com.optimizely.ab.config.parser.ConfigParseException;
26+
import com.optimizely.ab.event.EventHandler;
27+
import com.optimizely.ab.event.EventProcessor;
2628
import com.optimizely.ab.event.ForwardingEventProcessor;
29+
import com.optimizely.ab.event.internal.ImpressionEvent;
2730
import com.optimizely.ab.event.internal.payload.DecisionMetadata;
2831
import com.optimizely.ab.internal.LogbackVerifier;
2932
import com.optimizely.ab.notification.NotificationCenter;
@@ -37,7 +40,10 @@
3740
import org.junit.Before;
3841
import org.junit.Rule;
3942
import org.junit.Test;
43+
import org.mockito.ArgumentCaptor;
4044
import org.mockito.Mockito;
45+
import org.slf4j.Logger;
46+
import org.slf4j.LoggerFactory;
4147

4248
import java.util.*;
4349
import java.util.concurrent.CountDownLatch;
@@ -345,9 +351,11 @@ public void decideAll_twoFlags() {
345351

346352
@Test
347353
public void decideAll_allFlags() {
354+
EventProcessor mockEventProcessor = mock(EventProcessor.class);
355+
348356
optimizely = new Optimizely.Builder()
349357
.withDatafile(datafile)
350-
.withEventProcessor(new ForwardingEventProcessor(eventHandler, null))
358+
.withEventProcessor(mockEventProcessor)
351359
.build();
352360

353361
String flagKey1 = "feature_1";
@@ -361,7 +369,6 @@ public void decideAll_allFlags() {
361369

362370
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
363371
Map<String, OptimizelyDecision> decisions = user.decideAll();
364-
365372
assertTrue(decisions.size() == 3);
366373

367374
assertEquals(
@@ -395,9 +402,23 @@ public void decideAll_allFlags() {
395402
user,
396403
Collections.emptyList()));
397404

398-
eventHandler.expectImpression("10390977673", "10389729780", userId, attributes);
399-
eventHandler.expectImpression("10420810910", "10418551353", userId, attributes);
400-
eventHandler.expectImpression(null, "", userId, attributes);
405+
ArgumentCaptor<ImpressionEvent> argumentCaptor = ArgumentCaptor.forClass(ImpressionEvent.class);
406+
verify(mockEventProcessor, times(3)).process(argumentCaptor.capture());
407+
408+
List<ImpressionEvent> sentEvents = argumentCaptor.getAllValues();
409+
assertEquals(sentEvents.size(), 3);
410+
411+
assertEquals(sentEvents.get(2).getExperimentKey(), "exp_with_audience");
412+
assertEquals(sentEvents.get(2).getVariationKey(), "a");
413+
assertEquals(sentEvents.get(2).getUserContext().getUserId(), userId);
414+
415+
416+
assertEquals(sentEvents.get(1).getExperimentKey(), "exp_no_audience");
417+
assertEquals(sentEvents.get(1).getVariationKey(), "variation_with_traffic");
418+
assertEquals(sentEvents.get(1).getUserContext().getUserId(), userId);
419+
420+
assertEquals(sentEvents.get(0).getExperimentKey(), "");
421+
assertEquals(sentEvents.get(0).getUserContext().getUserId(), userId);
401422
}
402423

403424
@Test

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

+19-8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.optimizely.ab.error.ErrorHandler;
2525
import com.optimizely.ab.internal.ControlAttribute;
2626
import com.optimizely.ab.internal.LogbackVerifier;
27+
import com.optimizely.ab.optimizelydecision.DecisionReasons;
2728
import com.optimizely.ab.optimizelydecision.DecisionResponse;
2829
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2930
import org.junit.Before;
@@ -297,7 +298,9 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment
297298
any(Experiment.class),
298299
any(OptimizelyUserContext.class),
299300
any(ProjectConfig.class),
300-
anyObject()
301+
anyObject(),
302+
anyObject(),
303+
any(DecisionReasons.class)
301304
);
302305
// do not bucket to any rollouts
303306
doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(null, null, null))).when(decisionService).getVariationForFeatureInRollout(
@@ -315,9 +318,9 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment
315318
assertNull(featureDecision.variation);
316319
assertNull(featureDecision.decisionSource);
317320

318-
// logbackVerifier.expectMessage(Level.INFO,
319-
// "The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" +
320-
// FEATURE_MULTI_VARIATE_FEATURE_KEY + "\".");
321+
logbackVerifier.expectMessage(Level.INFO,
322+
"The user \"" + genericUserId + "\" was not bucketed into a rollout for feature flag \"" +
323+
FEATURE_MULTI_VARIATE_FEATURE_KEY + "\".");
321324

322325
verify(spyFeatureFlag, times(2)).getExperimentIds();
323326
verify(spyFeatureFlag, times(2)).getKey();
@@ -381,7 +384,9 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout()
381384
eq(featureExperiment),
382385
any(OptimizelyUserContext.class),
383386
any(ProjectConfig.class),
384-
anyObject()
387+
anyObject(),
388+
anyObject(),
389+
any(DecisionReasons.class)
385390
);
386391

387392
// return variation for rollout
@@ -413,7 +418,9 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout()
413418
any(Experiment.class),
414419
any(OptimizelyUserContext.class),
415420
any(ProjectConfig.class),
416-
anyObject()
421+
anyObject(),
422+
anyObject(),
423+
any(DecisionReasons.class)
417424
);
418425
}
419426

@@ -438,7 +445,9 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails
438445
eq(featureExperiment),
439446
any(OptimizelyUserContext.class),
440447
any(ProjectConfig.class),
441-
anyObject()
448+
anyObject(),
449+
anyObject(),
450+
any(DecisionReasons.class)
442451
);
443452

444453
// return variation for rollout
@@ -470,7 +479,9 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails
470479
any(Experiment.class),
471480
any(OptimizelyUserContext.class),
472481
any(ProjectConfig.class),
473-
anyObject()
482+
anyObject(),
483+
anyObject(),
484+
any(DecisionReasons.class)
474485
);
475486

476487
logbackVerifier.expectMessage(

0 commit comments

Comments
 (0)