Skip to content

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

Merged
Farhan Anjum (FarhanAnjum-opti) merged 60 commits into
masterfrom
farhan-anjum/FSSDK-11170-decision-service-methods-for-cmab
Nov 17, 2025
Merged

Farhan Anjum (FarhanAnjum-opti) merged 60 commits into
masterfrom
farhan-anjum/FSSDK-11170-decision-service-methods-for-cmab

Conversation

@FarhanAnjum-opti

@FarhanAnjum-opti Farhan Anjum (FarhanAnjum-opti) commented Sep 24, 2025 •

Copy link
Copy Markdown
Contributor

Summary

Decision Service methods to handle CMAB

Test plan

Added unit tests

Issues

FSSDK-11170
FSSDK-12013

…ations over CMAB service decisions in DecisionService

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. Added few comments.

Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java
Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java Outdated

@jaeopt Jae Kim (jaeopt) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java Outdated
Comment thread core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java Outdated
Comment thread core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java Outdated
Comment thread core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java Outdated

@jaeopt Jae Kim (jaeopt) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/internal/CacheWithRemove.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/service/DefaultCmabService.java Outdated

@jaeopt Jae Kim (jaeopt) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/cmab/client/RetryConfig.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated
Comment thread core-api/src/main/java/com/optimizely/ab/Optimizely.java Outdated

@jaeopt Jae Kim (jaeopt) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FarhanAnjum-opti
Farhan Anjum (FarhanAnjum-opti) merged commit efbda89 into master Nov 17, 2025
12 of 17 checks passed
@FarhanAnjum-opti
Farhan Anjum (FarhanAnjum-opti) deleted the farhan-anjum/FSSDK-11170-decision-service-methods-for-cmab branch November 17, 2025 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants