[FSSDK-11170] update: decision service methods for cmab #583

FarhanAnjum-opti · 2025-09-24T15:46:52Z

Summary

Decision Service methods to handle CMAB

Test plan

Added unit tests

Issues

FSSDK-11170

…vice

…e to support CMAB UUID handling

…and include CMAB UUIDs in responses

…ils for CMAB configuration

…ations over CMAB service decisions in DecisionService

…rvice failures

…information

…tly verify error state

…izely and DecisionService

…out CMAB logic

…text and related fetcher classes

…serContextTest

… documentation

muzahidul-opti

Changes look good to me. Added few comments.

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

core-api/src/main/java/com/optimizely/ab/optimizelydecision/DecisionResponse.java

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

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

core-api/src/main/java/com/optimizely/ab/optimizelydecision/AsyncDecisionsFetcher.java

…aking logic

…ecision responses

…ent key retrieval

…atureDecision

…eation in CMAB client

…hance decision handling

…lientHelper

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

… backward compatibility for mobile apps

… decisions in OptimizelyUserContext

…rContextTest for consistency

…od for consistency

…ecision handling

jaeopt

A few more suggestions at the top level.

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

core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java

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

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

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

…larity and maintainability

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

…ckage-private and add copyright notice to DecisionPath

…and OptimizelyUserContext with corresponding tests

…ndency and adjust related tests

…and enhance builder methods for cache configuration

jaeopt

I see it's now more aligned with android-sdk. I added more comments. Let's discuss.

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

jaeopt · 2025-10-28T00:46:00Z

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

+
            if (decision != null) {
-                decisions.add(new DecisionResponse(decision, reasons));
+                decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUUID));


Do we have any case we need pass cmabUUID separately though it's already in decision?

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

core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java

core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

…nService and adjust tests accordingly

…ervice to utilize it

…djust instance creation logic

…on and enhance cache handling logging

jaeopt

Bucket cleaning up looks good! A few more changes suggested.

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

core-api/src/main/java/com/optimizely/ab/internal/CacheWithRemove.java

core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java

…ache size and timeout, and remove CacheWithRemove interface

…in OptimizelyFactory

…in newDefaultInstance method of OptimizelyFactory

jaeopt

All changes look good to me!
Added a few suggestions - it must be the final round :)

jaeopt · 2025-11-10T21:10:39Z

core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java


+        // If no cmabService provided, create default one
+        if (cmabService == null) {
+            DefaultCmabClient defaultCmabClient = new DefaultCmabClient(CmabClientConfig.withDefaultRetry());


Can we change the DefaultCmabClient to use defaultRetry by default, so we can use simpler

Suggested change

DefaultCmabClient defaultCmabClient = new DefaultCmabClient(CmabClientConfig.withDefaultRetry());

DefaultCmabClient defaultCmabClient = new DefaultCmabClient();

jaeopt · 2025-11-10T21:13:27Z

core-api/src/main/java/com/optimizely/ab/optimizelydecision/OptimizelyDecisionsCallback.java

@@ -0,0 +1,32 @@
+/**
+ * Copyright 2024, Optimizely and contributors


Suggested change

* Copyright 2024, Optimizely and contributors

* Copyright 2025, Optimizely and contributors

jaeopt · 2025-11-10T21:26:05Z

core-api/src/main/java/com/optimizely/ab/optimizelydecision/AsyncDecisionFetcher.java

+        // This mirrors how the sync methods handle errors
+        return OptimizelyDecision.newErrorDecision(key, userContext, "Async decision error: " + errorMessage);
+    }
+}


I see this "no-new-line-at-the-end" in several places. Can we clean them up?

jaeopt · 2025-11-10T21:34:07Z

core-api/src/main/java/com/optimizely/ab/cmab/client/RetryConfig.java

    }

    /**
     * Creates a default RetryConfig with 3 retries and exponential backoff.


Suggested change

* Creates a default RetryConfig with 3 retries and exponential backoff.

* Creates a default RetryConfig with 1 retry and exponential backoff.

jaeopt · 2025-11-10T21:41:07Z

core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientHelper.java

+import java.util.regex.Pattern;
+
+public class CmabClientHelper {
+    public static final String CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s";


remove "final" so that they can override it.
Do you allow it configurable in other way?

jaeopt · 2025-11-10T21:51:39Z

core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientConfig.java

    /**
     * Creates a config with default retry settings.
+     * 
+     * @return A default cmab client config


We can have this as default.

jaeopt · 2025-11-10T22:30:10Z

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

                           @Nonnull ErrorHandler errorHandler,
-                           @Nullable UserProfileService userProfileService) {
+                           @Nullable UserProfileService userProfileService,
+                           @Nonnull CmabService cmabService) {


Don't we need to support old constructor for backward compatibility?
DecisionService(bucketer, errorHandler, userProfileService)

jaeopt · 2025-11-10T23:18:33Z

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

+        Map<String, OptimizelyDecision> decisionMap = new HashMap<>();
+
+        ProjectConfig projectConfig = getProjectConfig();
+        if (projectConfig == null) {
+            logger.error("Optimizely instance is not valid, failing decideAllSync call.");
+            return decisionMap;
+        }
+
+        List<FeatureFlag> allFlags = projectConfig.getFeatureFlags();
+        List<String> allFlagKeys = new ArrayList<>();
+        for (int i = 0; i < allFlags.size(); i++) allFlagKeys.add(allFlags.get(i).getKey());
+
+        return decideForKeysSync(user, allFlagKeys, options);


Can't we reuse the code for decideAll - the same logic repeated here.

jaeopt · 2025-11-10T23:20:09Z

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

+        ProjectConfig projectConfig = getProjectConfig();
+        if (projectConfig == null) {
+            return OptimizelyDecision.newErrorDecision(key, user, DecisionMessage.SDK_NOT_READY.reason());
+        }
+
+        List<OptimizelyDecideOption> allOptions = getAllOptions(options);
+        allOptions.remove(OptimizelyDecideOption.ENABLED_FLAGS_ONLY);
+
+        return decideForKeysSync(user, Arrays.asList(key), allOptions, true).get(key);


Same here - Can't we reuse the code for decide - the same logic repeated here.

FarhanAnjum-opti added 17 commits September 19, 2025 16:18

update: add CmabService to Optimizely class and builder

26e6393

update: integrate CMAB service into OptimizelyFactory

ad63201

update: change CmabService field to non-nullable in Optimizely class

fbed362

update: add CmabService to DecisionService and its tests

53d754a

update: implement CMAB traffic allocation in Bucketer and DecisionSer…

9905026

…vice

update: enhance DecisionService, FeatureDecision, and DecisionRespons…

78f45bf

…e to support CMAB UUID handling

update: enhance DecisionService and DecisionMessage to handle errors …

9757d49

…and include CMAB UUIDs in responses

update: add validConfigJsonCMAB method to DatafileProjectConfigTestUt…

ecf9199

…ils for CMAB configuration

update: add tests to verify precedence of whitelisted and forced vari…

5e0808f

…ations over CMAB service decisions in DecisionService

update: add test to verify error handling in getVariation for CMAB se…

36d2b4c

…rvice failures

update: modify DecisionResponse to include additional error handling …

5796cb7

…information

update: fix error handling assertion in DecisionServiceTest to correc…

d8b0134

…tly verify error state

update: add tests for CMAB experiment variations in DecisionService

b2f270f

update: implement decision-making methods to skip CMAB logic in Optim…

a4c3f1c

…izely and DecisionService

update: add methods to OptimizelyUserContext for decision-making with…

e4fe788

…out CMAB logic

update: add asynchronous decision-making methods in OptimizelyUserCon…

e75693d

…text and related fetcher classes

update: add decision-making methods without CMAB logic in OptimizelyU…

af210d8

…serContextTest

optimizely-codereview-bot assigned FarhanAnjum-opti Sep 24, 2025

update: remove unused parameter 'useCmab' from DecisionService method…

42053e4

… documentation

muzahidul-opti requested changes Sep 25, 2025

View reviewed changes

FarhanAnjum-opti added 10 commits September 26, 2025 03:00

update: rename methods to use 'Sync' suffix for clarity in decision-m…

9a12d72

…aking logic

update: return cmab error decision whenever found

416bcbd

update: enhance error handling by specifying CMAB error messages in d…

64f378f

…ecision responses

update: improve error handling by checking for null values in experim…

8539166

…ent key retrieval

update: fix CMAB error handling by providing a valid Experiment in Fe…

3cee65c

…atureDecision

update: add Javadoc comments for async decision methods and config cr…

47c65b5

…eation in CMAB client

update: refactor build to use cmabClient instead of default service

fe75a85

update: refactor DefaultCmabClient to utilize CmabClientHelper

b0d5090

update: refactor AsyncDecisionsFetcher to AsyncDecisionFetcher and en…

6db2e88

…hance decision handling

update: add missing copyright notice and license information to CmabC…

6fc6446

…lientHelper

jaeopt reviewed Oct 13, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java Outdated Show resolved Hide resolved

FarhanAnjum-opti added 5 commits October 15, 2025 18:28

update: enhance CMAB handling in bucketing and decision services, add…

a80c0d3

… backward compatibility for mobile apps

update: add backward compatibility support for Android sync and async…

a9ae805

… decisions in OptimizelyUserContext

update: add empty list parameter to decision methods in OptimizelyUse…

f25f824

…rContextTest for consistency

update: replace null with empty list parameter in async decision meth…

7363a2f

…od for consistency

update: add useCmab parameter to decideForKeys methods for enhanced d…

1c52366

…ecision handling

jaeopt requested changes Oct 17, 2025

View reviewed changes

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

b2dcf9e

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

jaeopt reviewed Oct 17, 2025

View reviewed changes

core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated Show resolved Hide resolved

FarhanAnjum-opti and others added 7 commits October 23, 2025 20:54

update: refactor decision-making logic to use DecisionPath enum for c…

89771bc

…larity and maintainability

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

73d5673

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

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

690379c

Co-authored-by: Jae Kim <45045038+jaeopt@users.noreply.github.com>

update: modify OptimizelyUserContext to change optimizely field to pa…

9c8dd8f

…ckage-private and add copyright notice to DecisionPath

update: implement asynchronous decision-making methods in Optimizely …

a17becd

…and OptimizelyUserContext with corresponding tests

update: refactor DefaultCmabService to remove CmabServiceOptions depe…

ba575ce

…ndency and adjust related tests

update: refactor DefaultCmabService to use a generic Cache interface …

ad474c8

…and enhance builder methods for cache configuration

jaeopt requested changes Oct 28, 2025

View reviewed changes

jaeopt and others added 7 commits October 30, 2025 14:04

fix to support android-sdk

9f7ea59

clean up

93cdaba

update: refactor bucketing logic to remove CMAB handling from Decisio…

e459321

…nService and adjust tests accordingly

update: introduce CacheWithRemove interface and refactor DefaultCmabS…

8c96222

…ervice to utilize it

update: implement CacheWithRemove interface in DefaultLRUCache class

7c97734

update: refactor OptimizelyFactory to remove CMAB cache methods and a…

4feb857

…djust instance creation logic

update: refactor DefaultCmabService to streamline logger initializati…

4b35768

…on and enhance cache handling logging

FarhanAnjum-opti requested a review from jaeopt November 6, 2025 07:35

jaeopt requested changes Nov 7, 2025

View reviewed changes

FarhanAnjum-opti added 3 commits November 10, 2025 09:23

update: refactor DefaultCmabService to use Cache interface, enhance c…

effbd74

…ache size and timeout, and remove CacheWithRemove interface

update: change parameter type from DefaultCmabService to CmabService …

ebe27be

…in OptimizelyFactory

update: change parameter type from DefaultCmabService to CmabService …

219b322

…in newDefaultInstance method of OptimizelyFactory

jaeopt requested changes Nov 10, 2025

View reviewed changes

	DefaultCmabClient defaultCmabClient = new DefaultCmabClient(CmabClientConfig.withDefaultRetry());
	DefaultCmabClient defaultCmabClient = new DefaultCmabClient();

		@@ -0,0 +1,32 @@
		/**
		* Copyright 2024, Optimizely and contributors

	* Copyright 2024, Optimizely and contributors
	* Copyright 2025, Optimizely and contributors

	* Creates a default RetryConfig with 3 retries and exponential backoff.
	* Creates a default RetryConfig with 1 retry and exponential backoff.

[FSSDK-11170] update: decision service methods for cmab #583

Are you sure you want to change the base?

[FSSDK-11170] update: decision service methods for cmab #583

Conversation

FarhanAnjum-opti commented Sep 24, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Issues

Uh oh!

muzahidul-opti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

FarhanAnjum-opti commented Sep 24, 2025 •

edited by atlassian bot

Loading