From 2640eaa289824a34bc60c33c90ab79dd68d65feb Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 11 Aug 2025 22:00:17 +0600 Subject: [PATCH 01/14] Cmab datafile parsed --- .../java/com/optimizely/ab/config/Cmab.java | 72 +++++++++++++++++++ .../com/optimizely/ab/config/Experiment.java | 35 +++++++-- .../java/com/optimizely/ab/config/Group.java | 3 +- .../ab/config/parser/GsonHelpers.java | 38 ++++++++-- .../ab/config/parser/JsonConfigParser.java | 27 ++++++- .../config/parser/JsonSimpleConfigParser.java | 33 ++++++++- 6 files changed, 193 insertions(+), 15 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/config/Cmab.java diff --git a/core-api/src/main/java/com/optimizely/ab/config/Cmab.java b/core-api/src/main/java/com/optimizely/ab/config/Cmab.java new file mode 100644 index 000000000..738864e58 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/Cmab.java @@ -0,0 +1,72 @@ +/** + * + * Copyright 2025 Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config; + +import java.util.List; +import java.util.Objects; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +/** + * Represents the Optimizely Traffic Allocation configuration. + * + * @see Project JSON + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class Cmab { + + private final List attributeIds; + private final int trafficAllocation; + + @JsonCreator + public Cmab(@JsonProperty("attributeIds") List attributeIds, + @JsonProperty("trafficAllocation") int trafficAllocation) { + this.attributeIds = attributeIds; + this.trafficAllocation = trafficAllocation; + } + + public List getAttributeIds() { + return attributeIds; + } + + public int getTrafficAllocation() { + return trafficAllocation; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + Cmab cmab = (Cmab) obj; + return trafficAllocation == cmab.trafficAllocation && + Objects.equals(attributeIds, cmab.attributeIds); + } + + @Override + public int hashCode() { + return Objects.hash(attributeIds, trafficAllocation); + } + + @Override + public String toString() { + return "Cmab{" + + "attributeIds=" + attributeIds + + ", trafficAllocation=" + trafficAllocation + + '}'; + } +} \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java index 11530735c..1a61ce9d9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Experiment.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Experiment.java @@ -41,6 +41,7 @@ public class Experiment implements IdKeyMapped { private final String status; private final String layerId; private final String groupId; + private final Cmab cmab; private final String AND = "AND"; private final String OR = "OR"; @@ -75,7 +76,25 @@ public String toString() { @VisibleForTesting public Experiment(String id, String key, String layerId) { - this(id, key, null, layerId, Collections.emptyList(), null, Collections.emptyList(), Collections.emptyMap(), Collections.emptyList(), ""); + this(id, key, null, layerId, Collections.emptyList(), null, Collections.emptyList(), Collections.emptyMap(), Collections.emptyList(), "", null); + } + + @VisibleForTesting + public Experiment(String id, String key, String status, String layerId, + List audienceIds, Condition audienceConditions, + List variations, Map userIdToVariationKeyMap, + List trafficAllocation, String groupId) { + this(id, key, status, layerId, audienceIds, audienceConditions, variations, + userIdToVariationKeyMap, trafficAllocation, groupId, null); // Default cmab=null + } + + @VisibleForTesting + public Experiment(String id, String key, String status, String layerId, + List audienceIds, Condition audienceConditions, + List variations, Map userIdToVariationKeyMap, + List trafficAllocation) { + this(id, key, status, layerId, audienceIds, audienceConditions, variations, + userIdToVariationKeyMap, trafficAllocation, "", null); // Default groupId="" and cmab=null } @JsonCreator @@ -87,8 +106,9 @@ public Experiment(@JsonProperty("id") String id, @JsonProperty("audienceConditions") Condition audienceConditions, @JsonProperty("variations") List variations, @JsonProperty("forcedVariations") Map userIdToVariationKeyMap, - @JsonProperty("trafficAllocation") List trafficAllocation) { - this(id, key, status, layerId, audienceIds, audienceConditions, variations, userIdToVariationKeyMap, trafficAllocation, ""); + @JsonProperty("trafficAllocation") List trafficAllocation, + @JsonProperty("cmab") Cmab cmab) { + this(id, key, status, layerId, audienceIds, audienceConditions, variations, userIdToVariationKeyMap, trafficAllocation, "", cmab); } public Experiment(@Nonnull String id, @@ -100,7 +120,8 @@ public Experiment(@Nonnull String id, @Nonnull List variations, @Nonnull Map userIdToVariationKeyMap, @Nonnull List trafficAllocation, - @Nonnull String groupId) { + @Nonnull String groupId, + @Nullable Cmab cmab) { this.id = id; this.key = key; this.status = status == null ? ExperimentStatus.NOT_STARTED.toString() : status; @@ -113,6 +134,7 @@ public Experiment(@Nonnull String id, this.userIdToVariationKeyMap = userIdToVariationKeyMap; this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(variations); this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(variations); + this.cmab = cmab; } public String getId() { @@ -163,6 +185,10 @@ public String getGroupId() { return groupId; } + public Cmab getCmab() { + return cmab; + } + public boolean isActive() { return status.equals(ExperimentStatus.RUNNING.toString()) || status.equals(ExperimentStatus.LAUNCHED.toString()); @@ -281,6 +307,7 @@ public String toString() { ", variationKeyToVariationMap=" + variationKeyToVariationMap + ", userIdToVariationKeyMap=" + userIdToVariationKeyMap + ", trafficAllocation=" + trafficAllocation + + ", cmab=" + cmab + '}'; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/Group.java b/core-api/src/main/java/com/optimizely/ab/config/Group.java index afb068be4..d0d9ff364 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Group.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Group.java @@ -62,7 +62,8 @@ public Group(@JsonProperty("id") String id, experiment.getVariations(), experiment.getUserIdToVariationKeyMap(), experiment.getTrafficAllocation(), - id + id, + experiment.getCmab() ); } this.experiments.add(experiment); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java index 1399497b2..184196fc6 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java @@ -24,13 +24,8 @@ import com.google.gson.JsonParseException; import com.google.gson.reflect.TypeToken; import com.optimizely.ab.bucketing.DecisionService; -import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.*; import com.optimizely.ab.config.Experiment.ExperimentStatus; -import com.optimizely.ab.config.FeatureFlag; -import com.optimizely.ab.config.FeatureVariable; -import com.optimizely.ab.config.FeatureVariableUsageInstance; -import com.optimizely.ab.config.TrafficAllocation; -import com.optimizely.ab.config.Variation; import com.optimizely.ab.config.audience.AudienceIdCondition; import com.optimizely.ab.config.audience.Condition; import com.optimizely.ab.internal.ConditionUtils; @@ -118,6 +113,27 @@ static Condition parseAudienceConditions(JsonObject experimentJson) { } + static Cmab parseCmab(JsonObject cmabJson, JsonDeserializationContext context) { + if (cmabJson == null) { + return null; + } + + JsonArray attributeIdsJson = cmabJson.getAsJsonArray("attributeIds"); + List attributeIds = new ArrayList<>(); + if (attributeIdsJson != null) { + for (JsonElement attributeIdElement : attributeIdsJson) { + attributeIds.add(attributeIdElement.getAsString()); + } + } + + int trafficAllocation = 0; + if (cmabJson.has("trafficAllocation")) { + trafficAllocation = cmabJson.get("trafficAllocation").getAsInt(); + } + + return new Cmab(attributeIds, trafficAllocation); + } + static Experiment parseExperiment(JsonObject experimentJson, String groupId, JsonDeserializationContext context) { String id = experimentJson.get("id").getAsString(); String key = experimentJson.get("key").getAsString(); @@ -143,8 +159,16 @@ static Experiment parseExperiment(JsonObject experimentJson, String groupId, Jso List trafficAllocations = parseTrafficAllocation(experimentJson.getAsJsonArray("trafficAllocation")); + Cmab cmab = null; + if (experimentJson.has("cmab")) { + JsonObject cmabJson = experimentJson.getAsJsonObject("cmab"); + if (cmabJson != null) { + cmab = parseCmab(cmabJson, context); + } + } + return new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap, - trafficAllocations, groupId); + trafficAllocations, groupId, cmab); } static Experiment parseExperiment(JsonObject experimentJson, JsonDeserializationContext context) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index ea5101054..71d446f72 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -159,8 +159,16 @@ private List parseExperiments(JSONArray experimentJson, String group List trafficAllocations = parseTrafficAllocation(experimentObject.getJSONArray("trafficAllocation")); + Cmab cmab = null; + if (experimentObject.has("cmab")) { + JSONObject cmabObject = experimentObject.optJSONObject("cmab"); + if (cmabObject != null) { + cmab = parseCmab(cmabObject); + } + } + experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap, - trafficAllocations, groupId)); + trafficAllocations, groupId, cmab)); } return experiments; @@ -255,6 +263,23 @@ private List parseTrafficAllocation(JSONArray trafficAllocati return trafficAllocation; } + private Cmab parseCmab(JSONObject cmabObject) { + if (cmabObject == null) { + return null; + } + + JSONArray attributeIdsJson = cmabObject.optJSONArray("attributeIds"); + List attributeIds = new ArrayList(); + if (attributeIdsJson != null) { + for (int i = 0; i < attributeIdsJson.length(); i++) { + attributeIds.add(attributeIdsJson.getString(i)); + } + } + + int trafficAllocation = cmabObject.optInt("trafficAllocation", 0); + return new Cmab(attributeIds, trafficAllocation); + } + private List parseAttributes(JSONArray attributeJson) { List attributes = new ArrayList(attributeJson.length()); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index c65eb6213..a45f922b5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -166,8 +166,17 @@ private List parseExperiments(JSONArray experimentJson, String group List trafficAllocations = parseTrafficAllocation((JSONArray) experimentObject.get("trafficAllocation")); - experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap, - trafficAllocations, groupId)); + // Add cmab parsing + Cmab cmab = null; + if (experimentObject.containsKey("cmab")) { + JSONObject cmabObject = (JSONObject) experimentObject.get("cmab"); + if (cmabObject != null) { + cmab = parseCmab(cmabObject); + } + } + + experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations, + userIdToVariationKeyMap, trafficAllocations, groupId, cmab)); } return experiments; @@ -398,6 +407,26 @@ private List parseIntegrations(JSONArray integrationsJson) { return integrations; } + private Cmab parseCmab(JSONObject cmabObject) { + if (cmabObject == null) { + return null; + } + + JSONArray attributeIdsJson = (JSONArray) cmabObject.get("attributeIds"); + List attributeIds = new ArrayList<>(); + if (attributeIdsJson != null) { + for (Object idObj : attributeIdsJson) { + attributeIds.add((String) idObj); + } + } + + Object trafficAllocationObj = cmabObject.get("trafficAllocation"); + int trafficAllocation = trafficAllocationObj != null ? + ((Long) trafficAllocationObj).intValue() : 0; + + return new Cmab(attributeIds, trafficAllocation); + } + @Override public String toJson(Object src) { return JSONValue.toJSONString(src); From 2d21ee2c55f6d7220206972f0dfc4b72436ec680 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Aug 2025 22:19:42 +0600 Subject: [PATCH 02/14] Add CMAB configuration and parsing tests with cmab datafile --- .../ab/config/parser/GsonHelpers.java | 5 +- .../java/com/optimizely/ab/cmab/CmabTest.java | 160 ++++++++++++ .../ab/cmab/parser/CmabParsingTest.java | 233 ++++++++++++++++++ .../test/resources/config/cmab-config.json | 226 +++++++++++++++++ 4 files changed, 622 insertions(+), 2 deletions(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java create mode 100644 core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java create mode 100644 core-api/src/test/resources/config/cmab-config.json diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java index 74e9eecef..624f9f159 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java @@ -161,8 +161,9 @@ static Experiment parseExperiment(JsonObject experimentJson, String groupId, Jso Cmab cmab = null; if (experimentJson.has("cmab")) { - JsonObject cmabJson = experimentJson.getAsJsonObject("cmab"); - if (cmabJson != null) { + JsonElement cmabElement = experimentJson.get("cmab"); + if (!cmabElement.isJsonNull()) { + JsonObject cmabJson = cmabElement.getAsJsonObject(); cmab = parseCmab(cmabJson, context); } } diff --git a/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java b/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java new file mode 100644 index 000000000..665f015a1 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java @@ -0,0 +1,160 @@ +package com.optimizely.ab.cmab; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import org.junit.Test; + +import com.optimizely.ab.config.Cmab; + +/** + * Tests for {@link Cmab} configuration object. + */ +public class CmabTest { + + @Test + public void testCmabConstructorWithValidData() { + List attributeIds = Arrays.asList("attr1", "attr2", "attr3"); + int trafficAllocation = 4000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match", attributeIds, cmab.getAttributeIds()); + assertEquals("TrafficAllocation should match", trafficAllocation, cmab.getTrafficAllocation()); + } + + @Test + public void testCmabConstructorWithEmptyAttributeIds() { + List attributeIds = Collections.emptyList(); + int trafficAllocation = 2000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should be empty", attributeIds, cmab.getAttributeIds()); + assertTrue("AttributeIds should be empty list", cmab.getAttributeIds().isEmpty()); + assertEquals("TrafficAllocation should match", trafficAllocation, cmab.getTrafficAllocation()); + } + + @Test + public void testCmabConstructorWithSingleAttributeId() { + List attributeIds = Collections.singletonList("single_attr"); + int trafficAllocation = 3000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match", attributeIds, cmab.getAttributeIds()); + assertEquals("Should have one attribute", 1, cmab.getAttributeIds().size()); + assertEquals("Single attribute should match", "single_attr", cmab.getAttributeIds().get(0)); + assertEquals("TrafficAllocation should match", trafficAllocation, cmab.getTrafficAllocation()); + } + + @Test + public void testCmabConstructorWithZeroTrafficAllocation() { + List attributeIds = Arrays.asList("attr1", "attr2"); + int trafficAllocation = 0; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match", attributeIds, cmab.getAttributeIds()); + assertEquals("TrafficAllocation should be zero", 0, cmab.getTrafficAllocation()); + } + + @Test + public void testCmabConstructorWithMaxTrafficAllocation() { + List attributeIds = Arrays.asList("attr1"); + int trafficAllocation = 10000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match", attributeIds, cmab.getAttributeIds()); + assertEquals("TrafficAllocation should be 10000", 10000, cmab.getTrafficAllocation()); + } + + @Test + public void testCmabEqualsAndHashCode() { + List attributeIds1 = Arrays.asList("attr1", "attr2"); + List attributeIds2 = Arrays.asList("attr1", "attr2"); + List attributeIds3 = Arrays.asList("attr1", "attr3"); + + Cmab cmab1 = new Cmab(attributeIds1, 4000); + Cmab cmab2 = new Cmab(attributeIds2, 4000); + Cmab cmab3 = new Cmab(attributeIds3, 4000); + Cmab cmab4 = new Cmab(attributeIds1, 5000); + + // Test equals + assertEquals("CMAB with same data should be equal", cmab1, cmab2); + assertNotEquals("CMAB with different attributeIds should not be equal", cmab1, cmab3); + assertNotEquals("CMAB with different trafficAllocation should not be equal", cmab1, cmab4); + + // Test reflexivity + assertEquals("CMAB should equal itself", cmab1, cmab1); + + // Test null comparison + assertNotEquals("CMAB should not equal null", cmab1, null); + + // Test hashCode consistency + assertEquals("Equal objects should have same hashCode", cmab1.hashCode(), cmab2.hashCode()); + } + + @Test + public void testCmabToString() { + List attributeIds = Arrays.asList("attr1", "attr2"); + int trafficAllocation = 4000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + String result = cmab.toString(); + + assertNotNull("toString should not return null", result); + assertTrue("toString should contain attributeIds", result.contains("attributeIds")); + assertTrue("toString should contain trafficAllocation", result.contains("trafficAllocation")); + assertTrue("toString should contain attr1", result.contains("attr1")); + assertTrue("toString should contain attr2", result.contains("attr2")); + assertTrue("toString should contain 4000", result.contains("4000")); + } + + @Test + public void testCmabToStringWithEmptyAttributeIds() { + List attributeIds = Collections.emptyList(); + int trafficAllocation = 2000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + String result = cmab.toString(); + + assertNotNull("toString should not return null", result); + assertTrue("toString should contain attributeIds", result.contains("attributeIds")); + assertTrue("toString should contain trafficAllocation", result.contains("trafficAllocation")); + assertTrue("toString should contain 2000", result.contains("2000")); + } + + @Test + public void testCmabWithDuplicateAttributeIds() { + List attributeIds = Arrays.asList("attr1", "attr2", "attr1", "attr3"); + int trafficAllocation = 4000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match exactly (including duplicates)", + attributeIds, cmab.getAttributeIds()); + assertEquals("Should have 4 elements (including duplicate)", 4, cmab.getAttributeIds().size()); + } + + @Test + public void testCmabWithRealWorldAttributeIds() { + // Test with realistic attribute IDs from Optimizely + List attributeIds = Arrays.asList("808797688", "808797689", "10401066117"); + int trafficAllocation = 4000; + + Cmab cmab = new Cmab(attributeIds, trafficAllocation); + + assertEquals("AttributeIds should match", attributeIds, cmab.getAttributeIds()); + assertEquals("TrafficAllocation should match", trafficAllocation, cmab.getTrafficAllocation()); + assertTrue("Should contain first attribute ID", cmab.getAttributeIds().contains("808797688")); + assertTrue("Should contain second attribute ID", cmab.getAttributeIds().contains("808797689")); + assertTrue("Should contain third attribute ID", cmab.getAttributeIds().contains("10401066117")); + } +} \ No newline at end of file diff --git a/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java b/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java new file mode 100644 index 000000000..bcc8beafe --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java @@ -0,0 +1,233 @@ +package com.optimizely.ab.cmab.parser; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import com.google.common.base.Charsets; +import com.google.common.io.Resources; +import com.optimizely.ab.config.Cmab; +import com.optimizely.ab.config.Experiment; +import com.optimizely.ab.config.Group; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.parser.ConfigParseException; +import com.optimizely.ab.config.parser.ConfigParser; +import com.optimizely.ab.config.parser.GsonConfigParser; +import com.optimizely.ab.config.parser.JacksonConfigParser; +import com.optimizely.ab.config.parser.JsonConfigParser; +import com.optimizely.ab.config.parser.JsonSimpleConfigParser; + +/** + * Tests CMAB parsing across all config parsers using real datafile + */ +@RunWith(Parameterized.class) +public class CmabParsingTest { + + @Parameterized.Parameters(name = "{index}: {0}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {"JsonSimpleConfigParser", new JsonSimpleConfigParser()}, + {"GsonConfigParser", new GsonConfigParser()}, + {"JacksonConfigParser", new JacksonConfigParser()}, + {"JsonConfigParser", new JsonConfigParser()} + }); + } + + private final String parserName; + private final ConfigParser parser; + + public CmabParsingTest(String parserName, ConfigParser parser) { + this.parserName = parserName; + this.parser = parser; + } + + private String loadCmabDatafile() throws IOException { + return Resources.toString(Resources.getResource("config/cmab-config.json"), Charsets.UTF_8); + } + + @Test + public void testParseExperimentWithValidCmab() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + Experiment experiment = config.getExperimentKeyMapping().get("exp_with_cmab"); + assertNotNull("Experiment 'exp_with_cmab' should exist in " + parserName, experiment); + + Cmab cmab = experiment.getCmab(); + assertNotNull("CMAB should not be null for experiment with CMAB in " + parserName, cmab); + + assertEquals("Should have 2 attribute IDs in " + parserName, 2, cmab.getAttributeIds().size()); + assertTrue("Should contain attribute '10401066117' in " + parserName, + cmab.getAttributeIds().contains("10401066117")); + assertTrue("Should contain attribute '10401066170' in " + parserName, + cmab.getAttributeIds().contains("10401066170")); + assertEquals("Traffic allocation should be 4000 in " + parserName, 4000, cmab.getTrafficAllocation()); + } + + @Test + public void testParseExperimentWithoutCmab() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + Experiment experiment = config.getExperimentKeyMapping().get("exp_without_cmab"); + assertNotNull("Experiment 'exp_without_cmab' should exist in " + parserName, experiment); + assertNull("CMAB should be null when not specified in " + parserName, experiment.getCmab()); + } + + @Test + public void testParseExperimentWithEmptyAttributeIds() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + Experiment experiment = config.getExperimentKeyMapping().get("exp_with_empty_cmab"); + assertNotNull("Experiment 'exp_with_empty_cmab' should exist in " + parserName, experiment); + + Cmab cmab = experiment.getCmab(); + assertNotNull("CMAB should not be null even with empty attributeIds in " + parserName, cmab); + assertTrue("AttributeIds should be empty in " + parserName, cmab.getAttributeIds().isEmpty()); + assertEquals("Traffic allocation should be 2000 in " + parserName, 2000, cmab.getTrafficAllocation()); + } + + @Test + public void testParseExperimentWithNullCmab() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + Experiment experiment = config.getExperimentKeyMapping().get("exp_with_null_cmab"); + assertNotNull("Experiment 'exp_with_null_cmab' should exist in " + parserName, experiment); + assertNull("CMAB should be null when explicitly set to null in " + parserName, experiment.getCmab()); + } + + @Test + public void testParseGroupExperimentWithCmab() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + // Find the group experiment + Experiment groupExperiment = null; + for (Group group : config.getGroups()) { + for (Experiment exp : group.getExperiments()) { + if ("group_exp_with_cmab".equals(exp.getKey())) { + groupExperiment = exp; + break; + } + } + } + + assertNotNull("Group experiment 'group_exp_with_cmab' should exist in " + parserName, groupExperiment); + + Cmab cmab = groupExperiment.getCmab(); + assertNotNull("Group experiment CMAB should not be null in " + parserName, cmab); + assertEquals("Should have 1 attribute ID in " + parserName, 1, cmab.getAttributeIds().size()); + assertEquals("Should contain correct attribute in " + parserName, + "10401066117", cmab.getAttributeIds().get(0)); + assertEquals("Traffic allocation should be 6000 in " + parserName, 6000, cmab.getTrafficAllocation()); + } + + @Test + public void testParseAllExperimentsFromDatafile() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + // Check all expected experiments exist + assertTrue("Should have 'exp_with_cmab' in " + parserName, + config.getExperimentKeyMapping().containsKey("exp_with_cmab")); + assertTrue("Should have 'exp_without_cmab' in " + parserName, + config.getExperimentKeyMapping().containsKey("exp_without_cmab")); + assertTrue("Should have 'exp_with_empty_cmab' in " + parserName, + config.getExperimentKeyMapping().containsKey("exp_with_empty_cmab")); + assertTrue("Should have 'exp_with_null_cmab' in " + parserName, + config.getExperimentKeyMapping().containsKey("exp_with_null_cmab")); + } + + @Test + public void testParseProjectConfigStructure() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + // Verify basic project config data + assertEquals("Project ID should match in " + parserName, "10431130345", config.getProjectId()); + assertEquals("Account ID should match in " + parserName, "10367498574", config.getAccountId()); + assertEquals("Version should match in " + parserName, "4", config.getVersion()); + assertEquals("Revision should match in " + parserName, "241", config.getRevision()); + + // Verify component counts based on your cmab-config.json + assertEquals("Should have 5 experiments in " + parserName, 5, config.getExperiments().size()); + assertEquals("Should have 2 audiences in " + parserName, 2, config.getAudiences().size()); + assertEquals("Should have 2 attributes in " + parserName, 2, config.getAttributes().size()); + assertEquals("Should have 1 event in " + parserName, 1, config.getEventTypes().size()); + assertEquals("Should have 1 group in " + parserName, 1, config.getGroups().size()); + assertEquals("Should have 1 feature flag in " + parserName, 1, config.getFeatureFlags().size()); + } + + @Test + public void testCmabFieldsAreCorrectlyParsed() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + // Test experiment with full CMAB + Experiment expWithCmab = config.getExperimentKeyMapping().get("exp_with_cmab"); + Cmab cmab = expWithCmab.getCmab(); + + assertNotNull("CMAB object should exist in " + parserName, cmab); + assertEquals("CMAB should have exactly 2 attributes in " + parserName, + Arrays.asList("10401066117", "10401066170"), cmab.getAttributeIds()); + assertEquals("CMAB traffic allocation should be 4000 in " + parserName, 4000, cmab.getTrafficAllocation()); + + // Test experiment with empty CMAB + Experiment expWithEmptyCmab = config.getExperimentKeyMapping().get("exp_with_empty_cmab"); + Cmab emptyCmab = expWithEmptyCmab.getCmab(); + + assertNotNull("Empty CMAB object should exist in " + parserName, emptyCmab); + assertTrue("CMAB attributeIds should be empty in " + parserName, emptyCmab.getAttributeIds().isEmpty()); + assertEquals("Empty CMAB traffic allocation should be 2000 in " + parserName, + 2000, emptyCmab.getTrafficAllocation()); + } + + @Test + public void testExperimentIdsAndKeysMatch() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + // Verify experiment IDs and keys from your datafile + Experiment expWithCmab = config.getExperimentKeyMapping().get("exp_with_cmab"); + assertEquals("exp_with_cmab ID should match in " + parserName, "10390977673", expWithCmab.getId()); + + Experiment expWithoutCmab = config.getExperimentKeyMapping().get("exp_without_cmab"); + assertEquals("exp_without_cmab ID should match in " + parserName, "10420810910", expWithoutCmab.getId()); + + Experiment expWithEmptyCmab = config.getExperimentKeyMapping().get("exp_with_empty_cmab"); + assertEquals("exp_with_empty_cmab ID should match in " + parserName, "10420810911", expWithEmptyCmab.getId()); + + Experiment expWithNullCmab = config.getExperimentKeyMapping().get("exp_with_null_cmab"); + assertEquals("exp_with_null_cmab ID should match in " + parserName, "10420810912", expWithNullCmab.getId()); + } + + @Test + public void testCmabDoesNotAffectOtherExperimentFields() throws IOException, ConfigParseException { + String datafile = loadCmabDatafile(); + ProjectConfig config = parser.parseProjectConfig(datafile); + + Experiment expWithCmab = config.getExperimentKeyMapping().get("exp_with_cmab"); + + // Verify other fields are still parsed correctly + assertEquals("Experiment status should be parsed correctly in " + parserName, + "Running", expWithCmab.getStatus()); + assertEquals("Experiment should have correct layer ID in " + parserName, + "10420273888", expWithCmab.getLayerId()); + assertEquals("Experiment should have 2 variations in " + parserName, + 2, expWithCmab.getVariations().size()); + assertEquals("Experiment should have 1 audience in " + parserName, + 1, expWithCmab.getAudienceIds().size()); + assertEquals("Experiment should have correct audience ID in " + parserName, + "13389141123", expWithCmab.getAudienceIds().get(0)); + } +} \ No newline at end of file diff --git a/core-api/src/test/resources/config/cmab-config.json b/core-api/src/test/resources/config/cmab-config.json new file mode 100644 index 000000000..505308cda --- /dev/null +++ b/core-api/src/test/resources/config/cmab-config.json @@ -0,0 +1,226 @@ +{ + "version": "4", + "sendFlagDecisions": true, + "rollouts": [ + { + "experiments": [ + { + "audienceIds": ["13389130056"], + "forcedVariations": {}, + "id": "3332020515", + "key": "3332020515", + "layerId": "3319450668", + "status": "Running", + "trafficAllocation": [ + { + "endOfRange": 10000, + "entityId": "3324490633" + } + ], + "variations": [ + { + "featureEnabled": true, + "id": "3324490633", + "key": "3324490633", + "variables": [] + } + ] + } + ], + "id": "3319450668" + } + ], + "anonymizeIP": true, + "botFiltering": true, + "projectId": "10431130345", + "variables": [], + "featureFlags": [ + { + "experimentIds": ["10390977673"], + "id": "4482920077", + "key": "feature_1", + "rolloutId": "3319450668", + "variables": [ + { + "defaultValue": "42", + "id": "2687470095", + "key": "i_42", + "type": "integer" + } + ] + } + ], + "experiments": [ + { + "status": "Running", + "key": "exp_with_cmab", + "layerId": "10420273888", + "trafficAllocation": [ + { + "entityId": "10389729780", + "endOfRange": 10000 + } + ], + "audienceIds": ["13389141123"], + "variations": [ + { + "variables": [], + "featureEnabled": true, + "id": "10389729780", + "key": "variation_a" + }, + { + "variables": [], + "id": "10416523121", + "key": "variation_b" + } + ], + "forcedVariations": {}, + "id": "10390977673", + "cmab": { + "attributeIds": ["10401066117", "10401066170"], + "trafficAllocation": 4000 + } + }, + { + "status": "Running", + "key": "exp_without_cmab", + "layerId": "10417730432", + "trafficAllocation": [ + { + "entityId": "10418551353", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "variations": [ + { + "variables": [], + "featureEnabled": true, + "id": "10418551353", + "key": "variation_with_traffic" + }, + { + "variables": [], + "featureEnabled": false, + "id": "10418510624", + "key": "variation_no_traffic" + } + ], + "forcedVariations": {}, + "id": "10420810910" + }, + { + "status": "Running", + "key": "exp_with_empty_cmab", + "layerId": "10417730433", + "trafficAllocation": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "featureEnabled": true, + "id": "10418551354", + "key": "variation_empty_cmab" + } + ], + "forcedVariations": {}, + "id": "10420810911", + "cmab": { + "attributeIds": [], + "trafficAllocation": 2000 + } + }, + { + "status": "Running", + "key": "exp_with_null_cmab", + "layerId": "10417730434", + "trafficAllocation": [ + { + "entityId": "10418551355", + "endOfRange": 7500 + } + ], + "audienceIds": [], + "variations": [ + { + "variables": [], + "featureEnabled": true, + "id": "10418551355", + "key": "variation_null_cmab" + } + ], + "forcedVariations": {}, + "id": "10420810912", + "cmab": null + } + ], + "audiences": [ + { + "id": "13389141123", + "conditions": "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"gender\", \"type\": \"custom_attribute\", \"value\": \"f\"}]]]", + "name": "gender" + }, + { + "id": "13389130056", + "conditions": "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \"country\", \"type\": \"custom_attribute\", \"value\": \"US\"}]]]", + "name": "US" + } + ], + "groups": [ + { + "policy": "random", + "trafficAllocation": [ + { + "entityId": "10390965532", + "endOfRange": 10000 + } + ], + "experiments": [ + { + "status": "Running", + "key": "group_exp_with_cmab", + "layerId": "10420222423", + "trafficAllocation": [], + "audienceIds": [], + "variations": [ + { + "variables": [], + "featureEnabled": false, + "id": "10389752311", + "key": "group_variation_a" + } + ], + "forcedVariations": {}, + "id": "10390965532", + "cmab": { + "attributeIds": ["10401066117"], + "trafficAllocation": 6000 + } + } + ], + "id": "13142870430" + } + ], + "attributes": [ + { + "id": "10401066117", + "key": "gender" + }, + { + "id": "10401066170", + "key": "age" + } + ], + "accountId": "10367498574", + "events": [ + { + "experimentIds": [ + "10420810910" + ], + "id": "10404198134", + "key": "event1" + } + ], + "revision": "241" +} \ No newline at end of file From 0d3a88df5314654a5e30d613d07bc71e195b6d96 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 12 Aug 2025 22:24:36 +0600 Subject: [PATCH 03/14] Add copyright notice to CmabTest and CmabParsingTest files --- .../java/com/optimizely/ab/cmab/CmabTest.java | 16 ++++++++++++++++ .../ab/cmab/parser/CmabParsingTest.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java b/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java index 665f015a1..40f1340b7 100644 --- a/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java +++ b/core-api/src/test/java/com/optimizely/ab/cmab/CmabTest.java @@ -1,3 +1,19 @@ +/* + * + * Copyright 2025 Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab; import java.util.Arrays; diff --git a/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java b/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java index bcc8beafe..4a6ed8f20 100644 --- a/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java +++ b/core-api/src/test/java/com/optimizely/ab/cmab/parser/CmabParsingTest.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2025 Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab.parser; import java.io.IOException; From 50334f11aa2ca327d0f9a6e215acb2b0f66ae68d Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Aug 2025 03:59:44 +0600 Subject: [PATCH 04/14] Refactor cmab parsing logic to simplify null check in JsonConfigParser --- .../com/optimizely/ab/config/parser/JsonConfigParser.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 06d3ef4d2..10ca9685f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -176,9 +176,7 @@ private List parseExperiments(JSONArray experimentJson, String group Cmab cmab = null; if (experimentObject.has("cmab")) { JSONObject cmabObject = experimentObject.optJSONObject("cmab"); - if (cmabObject != null) { - cmab = parseCmab(cmabObject); - } + cmab = parseCmab(cmabObject); } experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap, From 8258bb04a73c9f36a506f3376bc0d4d469f5d651 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Aug 2025 04:24:15 +0600 Subject: [PATCH 05/14] update: implement remove method in DefaultLRUCache for cache entry removal --- .../main/java/com/optimizely/ab/internal/Cache.java | 1 + .../com/optimizely/ab/internal/DefaultLRUCache.java | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java index ba667ebd2..d741e316b 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java @@ -22,4 +22,5 @@ public interface Cache { void save(String key, T value); T lookup(String key); void reset(); + void remove(String key); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java index b946a65ea..6d1fb4e50 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/DefaultLRUCache.java @@ -94,6 +94,19 @@ public void reset() { } } + public void remove(String key) { + if (maxSize == 0) { + // Cache is disabled when maxSize = 0 + return; + } + lock.lock(); + try { + linkedHashMap.remove(key); + } finally { + lock.unlock(); + } + } + private class CacheEntity { public T value; public Long timestamp; From 4fa8cbe755d065e977a557d2ef130616e0bd851f Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 13 Aug 2025 07:24:52 +0600 Subject: [PATCH 06/14] add: implement remove method tests in DefaultLRUCacheTest for various scenarios --- .../ab/internal/DefaultLRUCacheTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/internal/DefaultLRUCacheTest.java b/core-api/src/test/java/com/optimizely/ab/internal/DefaultLRUCacheTest.java index 79aa96ff3..bc5a509f7 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/DefaultLRUCacheTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/DefaultLRUCacheTest.java @@ -169,4 +169,90 @@ public void whenCacheIsReset() { assertEquals(0, cache.linkedHashMap.size()); } + + @Test + public void testRemoveNonExistentKey() { + DefaultLRUCache cache = new DefaultLRUCache<>(3, 1000); + cache.save("1", 100); + cache.save("2", 200); + + cache.remove("3"); // Doesn't exist + + assertEquals(Integer.valueOf(100), cache.lookup("1")); + assertEquals(Integer.valueOf(200), cache.lookup("2")); + } + + @Test + public void testRemoveExistingKey() { + DefaultLRUCache cache = new DefaultLRUCache<>(3, 1000); + + cache.save("1", 100); + cache.save("2", 200); + cache.save("3", 300); + + assertEquals(Integer.valueOf(100), cache.lookup("1")); + assertEquals(Integer.valueOf(200), cache.lookup("2")); + assertEquals(Integer.valueOf(300), cache.lookup("3")); + + cache.remove("2"); + + assertEquals(Integer.valueOf(100), cache.lookup("1")); + assertNull(cache.lookup("2")); + assertEquals(Integer.valueOf(300), cache.lookup("3")); + } + + @Test + public void testRemoveFromZeroSizedCache() { + DefaultLRUCache cache = new DefaultLRUCache<>(0, 1000); + cache.save("1", 100); + cache.remove("1"); + + assertNull(cache.lookup("1")); + } + + @Test + public void testRemoveAndAddBack() { + DefaultLRUCache cache = new DefaultLRUCache<>(3, 1000); + cache.save("1", 100); + cache.save("2", 200); + cache.save("3", 300); + + cache.remove("2"); + cache.save("2", 201); + + assertEquals(Integer.valueOf(100), cache.lookup("1")); + assertEquals(Integer.valueOf(201), cache.lookup("2")); + assertEquals(Integer.valueOf(300), cache.lookup("3")); + } + + @Test + public void testThreadSafety() throws InterruptedException { + int maxSize = 100; + DefaultLRUCache cache = new DefaultLRUCache<>(maxSize, 1000); + + for (int i = 1; i <= maxSize; i++) { + cache.save(String.valueOf(i), i * 100); + } + + Thread[] threads = new Thread[maxSize / 2]; + for (int i = 1; i <= maxSize / 2; i++) { + final int key = i; + threads[i - 1] = new Thread(() -> cache.remove(String.valueOf(key))); + threads[i - 1].start(); + } + + for (Thread thread : threads) { + thread.join(); + } + + for (int i = 1; i <= maxSize; i++) { + if (i <= maxSize / 2) { + assertNull(cache.lookup(String.valueOf(i))); + } else { + assertEquals(Integer.valueOf(i * 100), cache.lookup(String.valueOf(i))); + } + } + + assertEquals(maxSize / 2, cache.linkedHashMap.size()); + } } From aa955ebb4601ce5b7e5028f3c3cea3a37876f8ba Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 14 Aug 2025 13:43:16 +0600 Subject: [PATCH 07/14] refactor: remove unused methods from Cache interface --- core-api/src/main/java/com/optimizely/ab/internal/Cache.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java index d741e316b..5c6f9d45e 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java @@ -21,6 +21,4 @@ public interface Cache { int DEFAULT_TIMEOUT_SECONDS = 600; void save(String key, T value); T lookup(String key); - void reset(); - void remove(String key); } From d3fc4bb9deb6f6556908cf47b02bdf7450b78503 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 14 Aug 2025 13:44:16 +0600 Subject: [PATCH 08/14] update: add reset method to Cache interface --- core-api/src/main/java/com/optimizely/ab/internal/Cache.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java index 5c6f9d45e..ba667ebd2 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/Cache.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/Cache.java @@ -21,4 +21,5 @@ public interface Cache { int DEFAULT_TIMEOUT_SECONDS = 600; void save(String key, T value); T lookup(String key); + void reset(); } From 044c2301a546ec8b6b857664ed7643a34fd93df7 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Aug 2025 19:28:26 +0600 Subject: [PATCH 09/14] add: implement CmabClient, CmabClientConfig, and RetryConfig with fetchDecision method and retry logic --- .../com/optimizely/ab/cmab/CmabClient.java | 17 ++ .../optimizely/ab/cmab/CmabClientConfig.java | 34 +++ .../com/optimizely/ab/cmab/RetryConfig.java | 105 +++++++ .../optimizely/ab/cmab/DefaultCmabClient.java | 271 ++++++++++++++++++ 4 files changed, 427 insertions(+) create mode 100644 core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java create mode 100644 core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java create mode 100644 core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java create mode 100644 core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java new file mode 100644 index 000000000..634a132a8 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java @@ -0,0 +1,17 @@ +package com.optimizely.ab.cmab; + +import java.util.Map; +import java.util.concurrent.CompletableFuture; + +public interface CmabClient { + /** + * Fetches a decision from the CMAB prediction service. + * + * @param ruleId The rule/experiment ID + * @param userId The user ID + * @param attributes User attributes + * @param cmabUuid The CMAB UUID + * @return CompletableFuture containing the variation ID as a String + */ + CompletableFuture fetchDecision(String ruleId, String userId, Map attributes, String cmabUuid); +} diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java new file mode 100644 index 000000000..f1b779f0b --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java @@ -0,0 +1,34 @@ +package com.optimizely.ab.cmab; + +import javax.annotation.Nullable; + +/** + * Configuration for CMAB client operations. + * Contains only retry configuration since HTTP client is handled separately. + */ +public class CmabClientConfig { + private final RetryConfig retryConfig; + + public CmabClientConfig(@Nullable RetryConfig retryConfig) { + this.retryConfig = retryConfig; + } + + @Nullable + public RetryConfig getRetryConfig() { + return retryConfig; + } + + /** + * Creates a config with default retry settings. + */ + public static CmabClientConfig withDefaultRetry() { + return new CmabClientConfig(RetryConfig.defaultConfig()); + } + + /** + * Creates a config with no retry. + */ + public static CmabClientConfig withNoRetry() { + return new CmabClientConfig(null); + } +} \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java new file mode 100644 index 000000000..efb725fdd --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java @@ -0,0 +1,105 @@ +package com.optimizely.ab.cmab; +/** + * Configuration for retry behavior in CMAB client operations. + */ +public class RetryConfig { + private final int maxRetries; + private final long backoffBaseMs; + private final double backoffMultiplier; + + /** + * Creates a RetryConfig with custom retry and backoff settings. + * + * @param maxRetries Maximum number of retry attempts + * @param backoffBaseMs Base delay in milliseconds for the first retry + * @param backoffMultiplier Multiplier for exponential backoff (e.g., 2.0 for doubling) + */ + public RetryConfig(int maxRetries, long backoffBaseMs, double backoffMultiplier) { + if (maxRetries < 0) { + throw new IllegalArgumentException("maxRetries cannot be negative"); + } + if (backoffBaseMs < 0) { + throw new IllegalArgumentException("backoffBaseMs cannot be negative"); + } + if (backoffMultiplier < 1.0) { + throw new IllegalArgumentException("backoffMultiplier must be >= 1.0"); + } + + this.maxRetries = maxRetries; + this.backoffBaseMs = backoffBaseMs; + this.backoffMultiplier = backoffMultiplier; + } + + /** + * Creates a RetryConfig with default backoff settings (1 second base, 2x multiplier). + * + * @param maxRetries Maximum number of retry attempts + */ + public RetryConfig(int maxRetries) { + this(maxRetries, 1000, 2.0); // Default: 1 second base, exponential backoff + } + + /** + * Creates a default RetryConfig with 3 retries and exponential backoff. + */ + public static RetryConfig defaultConfig() { + return new RetryConfig(3); + } + + /** + * Creates a RetryConfig with no retries (single attempt only). + */ + public static RetryConfig noRetry() { + return new RetryConfig(0); + } + + public int getMaxRetries() { + return maxRetries; + } + + public long getBackoffBaseMs() { + return backoffBaseMs; + } + + public double getBackoffMultiplier() { + return backoffMultiplier; + } + + /** + * Calculates the delay for a specific retry attempt. + * + * @param attemptNumber The attempt number (0-based, so 0 = first retry) + * @return Delay in milliseconds + */ + public long calculateDelay(int attemptNumber) { + if (attemptNumber < 0) { + return 0; + } + return (long) (backoffBaseMs * Math.pow(backoffMultiplier, attemptNumber)); + } + + @Override + public String toString() { + return String.format("RetryConfig{maxRetries=%d, backoffBaseMs=%d, backoffMultiplier=%.1f}", + maxRetries, backoffBaseMs, backoffMultiplier); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + + RetryConfig that = (RetryConfig) obj; + return maxRetries == that.maxRetries && + backoffBaseMs == that.backoffBaseMs && + Double.compare(that.backoffMultiplier, backoffMultiplier) == 0; + } + + @Override + public int hashCode() { + int result = maxRetries; + result = 31 * result + Long.hashCode(backoffBaseMs); + result = 31 * result + Double.hashCode(backoffMultiplier); + return result; + } +} diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java new file mode 100644 index 000000000..dca64b6c8 --- /dev/null +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java @@ -0,0 +1,271 @@ +package com.optimizely.ab.cmab; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.StringEntity; +import org.apache.http.util.EntityUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.optimizely.ab.OptimizelyHttpClient; + +public class DefaultCmabClient implements CmabClient { + + private static final Logger logger = LoggerFactory.getLogger(DefaultCmabClient.class); + // Update constants to match JS error messages format + private static final String CMAB_FETCH_FAILED = "CMAB decision fetch failed with status: %d"; + private static final String INVALID_CMAB_FETCH_RESPONSE = "Invalid CMAB fetch response"; + private static final Pattern VARIATION_ID_PATTERN = Pattern.compile("\"variation_id\"\\s*:\\s*\"?([^\"\\s,}]+)\"?"); + private static final String CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s"; + + private final OptimizelyHttpClient httpClient; + private final RetryConfig retryConfig; + private final ScheduledExecutorService executorService; + + // Main constructor: HTTP client + retry config + public DefaultCmabClient(OptimizelyHttpClient httpClient, CmabClientConfig config) { + this.httpClient = httpClient != null ? httpClient : OptimizelyHttpClient.builder().build(); + this.retryConfig = config != null ? config.getRetryConfig() : null; + this.executorService = Executors.newScheduledThreadPool(2); + } + + // Constructor with HTTP client only (no retry) + public DefaultCmabClient(OptimizelyHttpClient httpClient) { + this(httpClient, CmabClientConfig.withNoRetry()); + } + + // Constructor with just retry config (uses default HTTP client) + public DefaultCmabClient(CmabClientConfig config) { + this(null, config); + } + + // Default constructor (no retry, default HTTP client) + public DefaultCmabClient() { + this(null, CmabClientConfig.withNoRetry()); + } + + public void shutdown() { + if (executorService != null && !executorService.isShutdown()) { + executorService.shutdown(); + } + } + + @Override + public CompletableFuture fetchDecision(String ruleId, String userId, Map attributes, String cmabUuid) { + // Implementation will use this.httpClient and this.retryConfig + String url = String.format(CMAB_PREDICTION_ENDPOINT, ruleId); + String requestBody = buildRequestJson(userId, ruleId, attributes, cmabUuid); + + // Use retry logic if configured, otherwise single request + if (retryConfig != null && retryConfig.getMaxRetries() > 0) { + return doFetchWithRetry(url, requestBody, userId, ruleId, retryConfig.getMaxRetries()); + } else { + return doFetch(url, requestBody, userId, ruleId); + } + } + + private CompletableFuture doFetch(String url, String requestBody, String userId, String ruleId) { + return CompletableFuture.supplyAsync(() -> { + HttpPost request = new HttpPost(url); + request.setHeader("Content-Type", "application/json"); + + try { + request.setEntity(new StringEntity(requestBody, StandardCharsets.UTF_8)); + } catch (Exception e) { + logger.error(String.format(CMAB_FETCH_FAILED, e)); + throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, e))); + } + + CloseableHttpResponse response = null; + try { + logger.debug("Making CMAB prediction request to: {} for user: {}", url, userId); + response = httpClient.execute(request); + + int statusCode = response.getStatusLine().getStatusCode(); + + if (!isSuccessStatusCode(statusCode)) { + logger.error("CMAB fetch failed (Response code: {}) for user: {}", statusCode, userId); + // Status code error (like JS: new OptimizelyError(CMAB_FETCH_FAILED, response.statusCode)) + throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, statusCode))); + } + + String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); + logger.debug("CMAB response received for user: {}", userId); + + + if (!validateResponse(responseBody)) { + logger.error(INVALID_CMAB_FETCH_RESPONSE); + throw new CompletionException(new RuntimeException(INVALID_CMAB_FETCH_RESPONSE)); + } + + String variationId = parseVariationId(responseBody); + logger.info("CMAB returned variation '{}' for rule '{}' and user '{}'", variationId, ruleId, userId); + + return variationId; + + } catch (IOException e) { + logger.error(String.format(CMAB_FETCH_FAILED, e)); + throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, e))); + } finally { + closeHttpResponse(response); + } + }); + } + + private CompletableFuture doFetchWithRetry(String url, String requestBody, String userId, String ruleId, int retriesLeft) { + return doFetch(url, requestBody, userId, ruleId) + .handle((result, throwable) -> { + if (retriesLeft > 0 && shouldRetry(throwable)) { + logger.warn("CMAB fetch failed, retrying... ({} retries left) for user: {}", retriesLeft, userId); + + // Calculate delay using RetryConfig + int attemptNumber = retryConfig.getMaxRetries() - retriesLeft; + long delay = retryConfig.calculateDelay(attemptNumber); + + CompletableFuture future = new CompletableFuture<>(); + + // Schedule retry after delay + executorService.schedule(() -> { + doFetchWithRetry(url, requestBody, userId, ruleId, retriesLeft - 1) + .whenComplete((retryResult, retryEx) -> { + if (retryEx != null) { + future.completeExceptionally(retryEx); + } else { + future.complete(retryResult); + } + }); + }, delay, TimeUnit.MILLISECONDS); + + return future; + } else if (throwable != null) { + logger.error("CMAB fetch failed after all retries for user: {}", userId, throwable); + CompletableFuture failedFuture = new CompletableFuture<>(); + failedFuture.completeExceptionally(throwable); + return failedFuture; + } else { + // Success case + CompletableFuture successFuture = new CompletableFuture<>(); + successFuture.complete(result); + return successFuture; + } + }) + .thenCompose(future -> future); // Flatten the nested CompletableFuture + } + + private String buildRequestJson(String userId, String ruleId, Map attributes, String cmabUuid) { + StringBuilder json = new StringBuilder(); + json.append("{\"instances\":[{"); + json.append("\"visitorId\":\"").append(escapeJson(userId)).append("\","); + json.append("\"experimentId\":\"").append(escapeJson(ruleId)).append("\","); + json.append("\"cmabUUID\":\"").append(escapeJson(cmabUuid)).append("\","); + json.append("\"attributes\":["); + + boolean first = true; + for (Map.Entry entry : attributes.entrySet()) { + if (!first) { + json.append(","); + } + json.append("{\"id\":\"").append(escapeJson(entry.getKey())).append("\","); + json.append("\"value\":").append(formatJsonValue(entry.getValue())).append(","); + json.append("\"type\":\"custom_attribute\"}"); + first = false; + } + + json.append("]}]}"); + return json.toString(); + } + + private String escapeJson(String value) { + if (value == null) { + return ""; + } + return value.replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t"); + } + + private String formatJsonValue(Object value) { + if (value == null) { + return "null"; + } else if (value instanceof String) { + return "\"" + escapeJson((String) value) + "\""; + } else if (value instanceof Number || value instanceof Boolean) { + return value.toString(); + } else { + return "\"" + escapeJson(value.toString()) + "\""; + } + } + + // Helper methods + private boolean isSuccessStatusCode(int statusCode) { + return statusCode >= 200 && statusCode < 300; + } + + private boolean validateResponse(String responseBody) { + try { + return responseBody.contains("predictions") && + responseBody.contains("variation_id") && + parseVariationIdForValidation(responseBody) != null; + } catch (Exception e) { + return false; + } + } + + private boolean shouldRetry(Throwable throwable) { + if (throwable instanceof CompletionException) { + Throwable cause = throwable.getCause(); + if (cause instanceof IOException) { + return true; // Network errors - always retry + } + if (cause instanceof RuntimeException) { + String message = cause.getMessage(); + // Retry on 5xx server errors (look for "status: 5" in formatted message) + if (message != null && message.matches(".*status: 5\\d\\d")) { + return true; + } + } + } + return false; + } + + private String parseVariationIdForValidation(String jsonResponse) { + Matcher matcher = VARIATION_ID_PATTERN.matcher(jsonResponse); + if (matcher.find()) { + return matcher.group(1); + } + return null; + } + + private String parseVariationId(String jsonResponse) { + // Simple regex to extract variation_id from predictions[0].variation_id + Pattern pattern = Pattern.compile("\"predictions\"\\s*:\\s*\\[\\s*\\{[^}]*\"variation_id\"\\s*:\\s*\"?([^\"\\s,}]+)\"?"); + Matcher matcher = pattern.matcher(jsonResponse); + if (matcher.find()) { + return matcher.group(1); + } + throw new RuntimeException("Could not parse variation_id from CMAB response"); + } + + private static void closeHttpResponse(CloseableHttpResponse response) { + if (response != null) { + try { + response.close(); + } catch (IOException e) { + // Ignore close errors + } + } + } +} From 8eb694ef370c4a1082ee7a5d9c1c66e072b08dc1 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Aug 2025 19:41:34 +0600 Subject: [PATCH 10/14] update: improve error logging in DefaultCmabClient for fetchDecision method --- .../optimizely/ab/cmab/DefaultCmabClient.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java index dca64b6c8..a734cf3b3 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java @@ -24,7 +24,7 @@ public class DefaultCmabClient implements CmabClient { private static final Logger logger = LoggerFactory.getLogger(DefaultCmabClient.class); // Update constants to match JS error messages format - private static final String CMAB_FETCH_FAILED = "CMAB decision fetch failed with status: %d"; + private static final String CMAB_FETCH_FAILED = "CMAB decision fetch failed with status: %s"; private static final String INVALID_CMAB_FETCH_RESPONSE = "Invalid CMAB fetch response"; private static final Pattern VARIATION_ID_PATTERN = Pattern.compile("\"variation_id\"\\s*:\\s*\"?([^\"\\s,}]+)\"?"); private static final String CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s"; @@ -83,8 +83,9 @@ private CompletableFuture doFetch(String url, String requestBody, String try { request.setEntity(new StringEntity(requestBody, StandardCharsets.UTF_8)); } catch (Exception e) { - logger.error(String.format(CMAB_FETCH_FAILED, e)); - throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, e))); + String errorMsg = String.format(CMAB_FETCH_FAILED, "encoding error"); + logger.error(errorMsg + " for user: {}", userId, e); + throw new CompletionException(new RuntimeException(errorMsg, e)); } CloseableHttpResponse response = null; @@ -94,18 +95,20 @@ private CompletableFuture doFetch(String url, String requestBody, String int statusCode = response.getStatusLine().getStatusCode(); + // Status code error if (!isSuccessStatusCode(statusCode)) { - logger.error("CMAB fetch failed (Response code: {}) for user: {}", statusCode, userId); - // Status code error (like JS: new OptimizelyError(CMAB_FETCH_FAILED, response.statusCode)) - throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, statusCode))); + String errorMsg = String.format(CMAB_FETCH_FAILED, statusCode); + logger.error(errorMsg + " for user: {}", userId); + throw new CompletionException(new RuntimeException(errorMsg)); } String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); logger.debug("CMAB response received for user: {}", userId); + // Invalid response error if (!validateResponse(responseBody)) { - logger.error(INVALID_CMAB_FETCH_RESPONSE); + logger.error(INVALID_CMAB_FETCH_RESPONSE + " for user: {}", userId); throw new CompletionException(new RuntimeException(INVALID_CMAB_FETCH_RESPONSE)); } @@ -115,8 +118,10 @@ private CompletableFuture doFetch(String url, String requestBody, String return variationId; } catch (IOException e) { - logger.error(String.format(CMAB_FETCH_FAILED, e)); - throw new CompletionException(new RuntimeException(String.format(CMAB_FETCH_FAILED, e))); + // IO error + String errorMsg = String.format(CMAB_FETCH_FAILED, "network error"); + logger.error(errorMsg + " for user: {}", userId, e); + throw new CompletionException(new RuntimeException(errorMsg, e)); } finally { closeHttpResponse(response); } From 50e2f7d17628f73498dac51d2e09252290ce3189 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Aug 2025 20:15:06 +0600 Subject: [PATCH 11/14] add: implement unit tests for DefaultCmabClient with various scenarios and error handling --- .../ab/cmab/DefaultCmabClientTest.java | 197 ++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java new file mode 100644 index 000000000..3c0d3fd55 --- /dev/null +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java @@ -0,0 +1,197 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.cmab; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import org.apache.http.StatusLine; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.StringEntity; +import org.apache.http.util.EntityUtils; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.optimizely.ab.OptimizelyHttpClient; +import com.optimizely.ab.internal.LogbackVerifier; + +import ch.qos.logback.classic.Level; + +public class DefaultCmabClientTest { + + private static final String validCmabResponse = "{\"predictions\":[{\"variation_id\":\"treatment_1\"}]}"; + + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + + OptimizelyHttpClient mockHttpClient; + DefaultCmabClient cmabClient; + + @Before + public void setUp() throws Exception { + setupHttpClient(200); + cmabClient = new DefaultCmabClient(mockHttpClient); + } + + private void setupHttpClient(int statusCode) throws Exception { + mockHttpClient = mock(OptimizelyHttpClient.class); + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + + when(statusLine.getStatusCode()).thenReturn(statusCode); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity(validCmabResponse)); + + when(mockHttpClient.execute(any(HttpPost.class))) + .thenReturn(httpResponse); + } + + @Test + public void testBuildRequestJson() throws Exception { + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + attributes.put("browser", "chrome"); + attributes.put("isMobile", true); + String cmabUuid = "uuid_789"; + + CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + String result = future.get(); + + assertEquals("treatment_1", result); + verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); + + ArgumentCaptor request = ArgumentCaptor.forClass(HttpPost.class); + verify(mockHttpClient).execute(request.capture()); + String actualRequestBody = EntityUtils.toString(request.getValue().getEntity()); + + assertTrue(actualRequestBody.contains("\"visitorId\":\"user_456\"")); + assertTrue(actualRequestBody.contains("\"experimentId\":\"rule_123\"")); + assertTrue(actualRequestBody.contains("\"cmabUUID\":\"uuid_789\"")); + assertTrue(actualRequestBody.contains("\"browser\"")); + assertTrue(actualRequestBody.contains("\"chrome\"")); + assertTrue(actualRequestBody.contains("\"isMobile\"")); + assertTrue(actualRequestBody.contains("true")); + } + + @Test + public void returnVariationWhenStatusIs200() throws Exception { + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + attributes.put("segment", "premium"); + String cmabUuid = "uuid_789"; + + CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + String result = future.get(); + + assertEquals("treatment_1", result); + verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); + logbackVerifier.expectMessage(Level.INFO, "CMAB returned variation 'treatment_1' for rule 'rule_123' and user 'user_456'"); + } + + @Test + public void returnErrorWhenStatusIsNot200AndLogError() throws Exception { + // Create new mock for 500 error + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + when(statusLine.getStatusCode()).thenReturn(500); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity("Server Error")); + when(mockHttpClient.execute(any(HttpPost.class))).thenReturn(httpResponse); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + + try { + future.get(); + fail("Expected ExecutionException"); + } catch (ExecutionException e) { + assertTrue(e.getCause().getMessage().contains("status: 500")); + } + + verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); + logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: 500 for user: user_456"); + } + + @Test + public void returnErrorWhenInvalidResponseAndLogError() throws Exception { + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + when(statusLine.getStatusCode()).thenReturn(200); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity("{\"predictions\":[]}")); + when(mockHttpClient.execute(any(HttpPost.class))).thenReturn(httpResponse); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + + try { + future.get(); + fail("Expected ExecutionException"); + } catch (ExecutionException e) { + assertEquals("Invalid CMAB fetch response", e.getCause().getMessage()); + } + + verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); + logbackVerifier.expectMessage(Level.ERROR, "Invalid CMAB fetch response for user: user_456"); + } + + @Test + public void testNoRetryWhenNoRetryConfig() throws Exception { + when(mockHttpClient.execute(any(HttpPost.class))) + .thenThrow(new IOException("Network error")); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + + try { + future.get(); + fail("Expected ExecutionException"); + } catch (ExecutionException e) { + assertTrue(e.getCause().getMessage().contains("network error")); + } + + verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); + logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: network error for user: user_456"); + } +} \ No newline at end of file From 04bb6f4b26f9bd93fa9c5b47449fecf48b42a2a6 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Aug 2025 20:31:22 +0600 Subject: [PATCH 12/14] update: add missing license header to DefaultCmabClient.java --- .../com/optimizely/ab/cmab/DefaultCmabClient.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java index a734cf3b3..cea1d4f05 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java @@ -1,3 +1,18 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab; import java.io.IOException; From 7e9487c612f8eedd9adc52e07a066bc40384ab4f Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 20 Aug 2025 20:44:24 +0600 Subject: [PATCH 13/14] update: add missing license headers to CmabClient, CmabClientConfig, and RetryConfig classes --- .../java/com/optimizely/ab/cmab/CmabClient.java | 15 +++++++++++++++ .../com/optimizely/ab/cmab/CmabClientConfig.java | 15 +++++++++++++++ .../java/com/optimizely/ab/cmab/RetryConfig.java | 15 +++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java index 634a132a8..2d1d7d3e3 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java @@ -1,3 +1,18 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab; import java.util.Map; diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java index f1b779f0b..5c94f5155 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java @@ -1,3 +1,18 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab; import javax.annotation.Nullable; diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java index efb725fdd..2f4d40ab3 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java @@ -1,3 +1,18 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.optimizely.ab.cmab; /** * Configuration for retry behavior in CMAB client operations. From d41c775bf97f924a964cb54b48ca537b21bf3175 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 25 Aug 2025 17:46:47 +0600 Subject: [PATCH 14/14] refactor: update DefaultCmabClient to use synchronous fetchDecision method with improved error handling and retry logic --- .../ab/cmab/{ => client}/CmabClient.java | 7 +- .../cmab/{ => client}/CmabClientConfig.java | 2 +- .../ab/cmab/client/CmabFetchException.java | 28 +++ .../client/CmabInvalidResponseException.java | 27 +++ .../ab/cmab/{ => client}/RetryConfig.java | 28 ++- .../optimizely/ab/cmab/DefaultCmabClient.java | 214 ++++++++---------- .../ab/cmab/DefaultCmabClientTest.java | 139 +++++++++--- 7 files changed, 288 insertions(+), 157 deletions(-) rename core-api/src/main/java/com/optimizely/ab/cmab/{ => client}/CmabClient.java (79%) rename core-api/src/main/java/com/optimizely/ab/cmab/{ => client}/CmabClientConfig.java (97%) create mode 100644 core-api/src/main/java/com/optimizely/ab/cmab/client/CmabFetchException.java create mode 100644 core-api/src/main/java/com/optimizely/ab/cmab/client/CmabInvalidResponseException.java rename core-api/src/main/java/com/optimizely/ab/cmab/{ => client}/RetryConfig.java (78%) diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClient.java similarity index 79% rename from core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java rename to core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClient.java index 2d1d7d3e3..2deabcfb4 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClient.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClient.java @@ -13,10 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.optimizely.ab.cmab; +package com.optimizely.ab.cmab.client; import java.util.Map; -import java.util.concurrent.CompletableFuture; public interface CmabClient { /** @@ -25,8 +24,8 @@ public interface CmabClient { * @param ruleId The rule/experiment ID * @param userId The user ID * @param attributes User attributes - * @param cmabUuid The CMAB UUID + * @param cmabUUID The CMAB UUID * @return CompletableFuture containing the variation ID as a String */ - CompletableFuture fetchDecision(String ruleId, String userId, Map attributes, String cmabUuid); + String fetchDecision(String ruleId, String userId, Map attributes, String cmabUUID); } diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientConfig.java similarity index 97% rename from core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java rename to core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientConfig.java index 5c94f5155..90198d376 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/CmabClientConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabClientConfig.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.optimizely.ab.cmab; +package com.optimizely.ab.cmab.client; import javax.annotation.Nullable; diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabFetchException.java b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabFetchException.java new file mode 100644 index 000000000..d76576ea2 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabFetchException.java @@ -0,0 +1,28 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.cmab.client; + +import com.optimizely.ab.OptimizelyRuntimeException; + +public class CmabFetchException extends OptimizelyRuntimeException { + public CmabFetchException(String message) { + super(message); + } + + public CmabFetchException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabInvalidResponseException.java b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabInvalidResponseException.java new file mode 100644 index 000000000..de5550995 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/cmab/client/CmabInvalidResponseException.java @@ -0,0 +1,27 @@ +/** + * Copyright 2025, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.cmab.client; + +import com.optimizely.ab.OptimizelyRuntimeException; + +public class CmabInvalidResponseException extends OptimizelyRuntimeException{ + public CmabInvalidResponseException(String message) { + super(message); + } + public CmabInvalidResponseException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java b/core-api/src/main/java/com/optimizely/ab/cmab/client/RetryConfig.java similarity index 78% rename from core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java rename to core-api/src/main/java/com/optimizely/ab/cmab/client/RetryConfig.java index 2f4d40ab3..b5b04cfa3 100644 --- a/core-api/src/main/java/com/optimizely/ab/cmab/RetryConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/cmab/client/RetryConfig.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.optimizely.ab.cmab; +package com.optimizely.ab.cmab.client; /** * Configuration for retry behavior in CMAB client operations. */ @@ -21,15 +21,17 @@ public class RetryConfig { private final int maxRetries; private final long backoffBaseMs; private final double backoffMultiplier; - + private final int maxTimeoutMs; + /** * Creates a RetryConfig with custom retry and backoff settings. * * @param maxRetries Maximum number of retry attempts * @param backoffBaseMs Base delay in milliseconds for the first retry * @param backoffMultiplier Multiplier for exponential backoff (e.g., 2.0 for doubling) + * @param maxTimeoutMs Maximum total timeout in milliseconds for all retry attempts */ - public RetryConfig(int maxRetries, long backoffBaseMs, double backoffMultiplier) { + public RetryConfig(int maxRetries, long backoffBaseMs, double backoffMultiplier, int maxTimeoutMs) { if (maxRetries < 0) { throw new IllegalArgumentException("maxRetries cannot be negative"); } @@ -39,19 +41,23 @@ public RetryConfig(int maxRetries, long backoffBaseMs, double backoffMultiplier) if (backoffMultiplier < 1.0) { throw new IllegalArgumentException("backoffMultiplier must be >= 1.0"); } + if (maxTimeoutMs < 0) { + throw new IllegalArgumentException("maxTimeoutMs cannot be negative"); + } this.maxRetries = maxRetries; this.backoffBaseMs = backoffBaseMs; this.backoffMultiplier = backoffMultiplier; + this.maxTimeoutMs = maxTimeoutMs; } /** - * Creates a RetryConfig with default backoff settings (1 second base, 2x multiplier). + * Creates a RetryConfig with default backoff settings and timeout (1 second base, 2x multiplier, 10 second timeout). * * @param maxRetries Maximum number of retry attempts */ public RetryConfig(int maxRetries) { - this(maxRetries, 1000, 2.0); // Default: 1 second base, exponential backoff + this(maxRetries, 1000, 2.0, 10000); // Default: 1 second base, exponential backoff, 10 second timeout } /** @@ -65,7 +71,7 @@ public static RetryConfig defaultConfig() { * Creates a RetryConfig with no retries (single attempt only). */ public static RetryConfig noRetry() { - return new RetryConfig(0); + return new RetryConfig(0, 0, 1.0, 0); } public int getMaxRetries() { @@ -80,6 +86,10 @@ public double getBackoffMultiplier() { return backoffMultiplier; } + public int getMaxTimeoutMs() { + return maxTimeoutMs; + } + /** * Calculates the delay for a specific retry attempt. * @@ -95,8 +105,8 @@ public long calculateDelay(int attemptNumber) { @Override public String toString() { - return String.format("RetryConfig{maxRetries=%d, backoffBaseMs=%d, backoffMultiplier=%.1f}", - maxRetries, backoffBaseMs, backoffMultiplier); + return String.format("RetryConfig{maxRetries=%d, backoffBaseMs=%d, backoffMultiplier=%.1f, maxTimeoutMs=%d}", + maxRetries, backoffBaseMs, backoffMultiplier, maxTimeoutMs); } @Override @@ -107,6 +117,7 @@ public boolean equals(Object obj) { RetryConfig that = (RetryConfig) obj; return maxRetries == that.maxRetries && backoffBaseMs == that.backoffBaseMs && + maxTimeoutMs == that.maxTimeoutMs && Double.compare(that.backoffMultiplier, backoffMultiplier) == 0; } @@ -115,6 +126,7 @@ public int hashCode() { int result = maxRetries; result = 31 * result + Long.hashCode(backoffBaseMs); result = 31 * result + Double.hashCode(backoffMultiplier); + result = 31 * result + Integer.hashCode(maxTimeoutMs); return result; } } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java index cea1d4f05..6af4ac32a 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/cmab/DefaultCmabClient.java @@ -16,16 +16,13 @@ package com.optimizely.ab.cmab; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.io.UnsupportedEncodingException; import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.http.ParseException; +import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPost; import org.apache.http.entity.StringEntity; @@ -34,10 +31,16 @@ import org.slf4j.LoggerFactory; import com.optimizely.ab.OptimizelyHttpClient; +import com.optimizely.ab.cmab.client.CmabClient; +import com.optimizely.ab.cmab.client.CmabClientConfig; +import com.optimizely.ab.cmab.client.CmabFetchException; +import com.optimizely.ab.cmab.client.CmabInvalidResponseException; +import com.optimizely.ab.cmab.client.RetryConfig; public class DefaultCmabClient implements CmabClient { - + private static final Logger logger = LoggerFactory.getLogger(DefaultCmabClient.class); + private static final int DEFAULT_TIMEOUT_MS = 10000; // Update constants to match JS error messages format private static final String CMAB_FETCH_FAILED = "CMAB decision fetch failed with status: %s"; private static final String INVALID_CMAB_FETCH_RESPONSE = "Invalid CMAB fetch response"; @@ -46,13 +49,11 @@ public class DefaultCmabClient implements CmabClient { private final OptimizelyHttpClient httpClient; private final RetryConfig retryConfig; - private final ScheduledExecutorService executorService; - // Main constructor: HTTP client + retry config + // Primary constructor - all others delegate to this public DefaultCmabClient(OptimizelyHttpClient httpClient, CmabClientConfig config) { - this.httpClient = httpClient != null ? httpClient : OptimizelyHttpClient.builder().build(); this.retryConfig = config != null ? config.getRetryConfig() : null; - this.executorService = Executors.newScheduledThreadPool(2); + this.httpClient = httpClient != null ? httpClient : createDefaultHttpClient(); } // Constructor with HTTP client only (no retry) @@ -70,117 +71,110 @@ public DefaultCmabClient() { this(null, CmabClientConfig.withNoRetry()); } - public void shutdown() { - if (executorService != null && !executorService.isShutdown()) { - executorService.shutdown(); - } + // Extract HTTP client creation logic + private OptimizelyHttpClient createDefaultHttpClient() { + int timeoutMs = (retryConfig != null) ? retryConfig.getMaxTimeoutMs() : DEFAULT_TIMEOUT_MS; + return OptimizelyHttpClient.builder().setTimeoutMillis(timeoutMs).build(); } @Override - public CompletableFuture fetchDecision(String ruleId, String userId, Map attributes, String cmabUuid) { + public String fetchDecision(String ruleId, String userId, Map attributes, String cmabUuid) { // Implementation will use this.httpClient and this.retryConfig String url = String.format(CMAB_PREDICTION_ENDPOINT, ruleId); String requestBody = buildRequestJson(userId, ruleId, attributes, cmabUuid); // Use retry logic if configured, otherwise single request if (retryConfig != null && retryConfig.getMaxRetries() > 0) { - return doFetchWithRetry(url, requestBody, userId, ruleId, retryConfig.getMaxRetries()); + return doFetchWithRetry(url, requestBody, retryConfig.getMaxRetries()); } else { - return doFetch(url, requestBody, userId, ruleId); + return doFetch(url, requestBody); } } - private CompletableFuture doFetch(String url, String requestBody, String userId, String ruleId) { - return CompletableFuture.supplyAsync(() -> { - HttpPost request = new HttpPost(url); - request.setHeader("Content-Type", "application/json"); - - try { - request.setEntity(new StringEntity(requestBody, StandardCharsets.UTF_8)); - } catch (Exception e) { - String errorMsg = String.format(CMAB_FETCH_FAILED, "encoding error"); - logger.error(errorMsg + " for user: {}", userId, e); - throw new CompletionException(new RuntimeException(errorMsg, e)); + private String doFetch(String url, String requestBody) { + HttpPost request = new HttpPost(url); + try { + request.setEntity(new StringEntity(requestBody)); + } catch (UnsupportedEncodingException e) { + String errorMessage = String.format(CMAB_FETCH_FAILED, e.getMessage()); + logger.error(errorMessage); + throw new CmabFetchException(errorMessage); + } + request.setHeader("content-type", "application/json"); + CloseableHttpResponse response = null; + try { + response = httpClient.execute(request); + + if (!isSuccessStatusCode(response.getStatusLine().getStatusCode())) { + StatusLine statusLine = response.getStatusLine(); + String errorMessage = String.format(CMAB_FETCH_FAILED, statusLine.getReasonPhrase()); + logger.error(errorMessage); + throw new CmabFetchException(errorMessage); } - CloseableHttpResponse response = null; + String responseBody; try { - logger.debug("Making CMAB prediction request to: {} for user: {}", url, userId); - response = httpClient.execute(request); - - int statusCode = response.getStatusLine().getStatusCode(); + responseBody = EntityUtils.toString(response.getEntity()); - // Status code error - if (!isSuccessStatusCode(statusCode)) { - String errorMsg = String.format(CMAB_FETCH_FAILED, statusCode); - logger.error(errorMsg + " for user: {}", userId); - throw new CompletionException(new RuntimeException(errorMsg)); - } - - String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - logger.debug("CMAB response received for user: {}", userId); - - - // Invalid response error if (!validateResponse(responseBody)) { - logger.error(INVALID_CMAB_FETCH_RESPONSE + " for user: {}", userId); - throw new CompletionException(new RuntimeException(INVALID_CMAB_FETCH_RESPONSE)); + logger.error(INVALID_CMAB_FETCH_RESPONSE); + throw new CmabInvalidResponseException(INVALID_CMAB_FETCH_RESPONSE); } - - String variationId = parseVariationId(responseBody); - logger.info("CMAB returned variation '{}' for rule '{}' and user '{}'", variationId, ruleId, userId); - - return variationId; - - } catch (IOException e) { - // IO error - String errorMsg = String.format(CMAB_FETCH_FAILED, "network error"); - logger.error(errorMsg + " for user: {}", userId, e); - throw new CompletionException(new RuntimeException(errorMsg, e)); - } finally { - closeHttpResponse(response); + return parseVariationId(responseBody); + } catch (IOException | ParseException e) { + logger.error(CMAB_FETCH_FAILED); + throw new CmabInvalidResponseException(INVALID_CMAB_FETCH_RESPONSE); } - }); + + } catch (IOException e) { + String errorMessage = String.format(CMAB_FETCH_FAILED, e.getMessage()); + logger.error(errorMessage); + throw new CmabFetchException(errorMessage); + } finally { + closeHttpResponse(response); + } } - private CompletableFuture doFetchWithRetry(String url, String requestBody, String userId, String ruleId, int retriesLeft) { - return doFetch(url, requestBody, userId, ruleId) - .handle((result, throwable) -> { - if (retriesLeft > 0 && shouldRetry(throwable)) { - logger.warn("CMAB fetch failed, retrying... ({} retries left) for user: {}", retriesLeft, userId); - - // Calculate delay using RetryConfig - int attemptNumber = retryConfig.getMaxRetries() - retriesLeft; - long delay = retryConfig.calculateDelay(attemptNumber); - - CompletableFuture future = new CompletableFuture<>(); - - // Schedule retry after delay - executorService.schedule(() -> { - doFetchWithRetry(url, requestBody, userId, ruleId, retriesLeft - 1) - .whenComplete((retryResult, retryEx) -> { - if (retryEx != null) { - future.completeExceptionally(retryEx); - } else { - future.complete(retryResult); - } - }); - }, delay, TimeUnit.MILLISECONDS); - - return future; - } else if (throwable != null) { - logger.error("CMAB fetch failed after all retries for user: {}", userId, throwable); - CompletableFuture failedFuture = new CompletableFuture<>(); - failedFuture.completeExceptionally(throwable); - return failedFuture; - } else { - // Success case - CompletableFuture successFuture = new CompletableFuture<>(); - successFuture.complete(result); - return successFuture; + private String doFetchWithRetry(String url, String requestBody, int maxRetries) { + double backoff = retryConfig.getBackoffBaseMs(); + Exception lastException = null; + + for (int attempt = 0; attempt <= maxRetries; attempt++) { + try { + return doFetch(url, requestBody); + } catch (CmabFetchException | CmabInvalidResponseException e) { + lastException = e; + + // If this is the last attempt, don't wait - just break and throw + if (attempt >= maxRetries) { + break; } - }) - .thenCompose(future -> future); // Flatten the nested CompletableFuture + + // Log retry attempt + logger.info("Retrying CMAB request (attempt: {}) after {} ms...", + attempt + 1, (int) backoff); + + try { + Thread.sleep((long) backoff); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + String errorMessage = String.format(CMAB_FETCH_FAILED, "Request interrupted during retry"); + logger.error(errorMessage); + throw new CmabFetchException(errorMessage, ie); + } + + // Calculate next backoff using exponential backoff with multiplier + backoff = Math.min( + backoff * Math.pow(retryConfig.getBackoffMultiplier(), attempt + 1), + retryConfig.getMaxTimeoutMs() + ); + } + } + + // If we get here, all retries were exhausted + String errorMessage = String.format(CMAB_FETCH_FAILED, "Exhausted all retries for CMAB request"); + logger.error(errorMessage); + throw new CmabFetchException(errorMessage, lastException); } private String buildRequestJson(String userId, String ruleId, Map attributes, String cmabUuid) { @@ -244,21 +238,9 @@ private boolean validateResponse(String responseBody) { } } - private boolean shouldRetry(Throwable throwable) { - if (throwable instanceof CompletionException) { - Throwable cause = throwable.getCause(); - if (cause instanceof IOException) { - return true; // Network errors - always retry - } - if (cause instanceof RuntimeException) { - String message = cause.getMessage(); - // Retry on 5xx server errors (look for "status: 5" in formatted message) - if (message != null && message.matches(".*status: 5\\d\\d")) { - return true; - } - } - } - return false; + private boolean shouldRetry(Exception exception) { + return (exception instanceof CmabFetchException) || + (exception instanceof CmabInvalidResponseException); } private String parseVariationIdForValidation(String jsonResponse) { @@ -276,7 +258,7 @@ private String parseVariationId(String jsonResponse) { if (matcher.find()) { return matcher.group(1); } - throw new RuntimeException("Could not parse variation_id from CMAB response"); + throw new CmabInvalidResponseException(INVALID_CMAB_FETCH_RESPONSE); } private static void closeHttpResponse(CloseableHttpResponse response) { @@ -284,7 +266,7 @@ private static void closeHttpResponse(CloseableHttpResponse response) { try { response.close(); } catch (IOException e) { - // Ignore close errors + logger.warn(e.getLocalizedMessage()); } } } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java index 3c0d3fd55..63fca3832 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/cmab/DefaultCmabClientTest.java @@ -18,8 +18,6 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; @@ -40,6 +38,10 @@ import static org.mockito.Mockito.when; import com.optimizely.ab.OptimizelyHttpClient; +import com.optimizely.ab.cmab.client.CmabClientConfig; +import com.optimizely.ab.cmab.client.CmabFetchException; +import com.optimizely.ab.cmab.client.CmabInvalidResponseException; +import com.optimizely.ab.cmab.client.RetryConfig; import com.optimizely.ab.internal.LogbackVerifier; import ch.qos.logback.classic.Level; @@ -66,6 +68,7 @@ private void setupHttpClient(int statusCode) throws Exception { StatusLine statusLine = mock(StatusLine.class); when(statusLine.getStatusCode()).thenReturn(statusCode); + when(statusLine.getReasonPhrase()).thenReturn(statusCode == 500 ? "Internal Server Error" : "OK"); when(httpResponse.getStatusLine()).thenReturn(statusLine); when(httpResponse.getEntity()).thenReturn(new StringEntity(validCmabResponse)); @@ -82,8 +85,8 @@ public void testBuildRequestJson() throws Exception { attributes.put("isMobile", true); String cmabUuid = "uuid_789"; - CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); - String result = future.get(); + // Fixed: Direct method call instead of CompletableFuture + String result = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); assertEquals("treatment_1", result); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); @@ -109,12 +112,14 @@ public void returnVariationWhenStatusIs200() throws Exception { attributes.put("segment", "premium"); String cmabUuid = "uuid_789"; - CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); - String result = future.get(); + // Fixed: Direct method call instead of CompletableFuture + String result = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); assertEquals("treatment_1", result); verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); - logbackVerifier.expectMessage(Level.INFO, "CMAB returned variation 'treatment_1' for rule 'rule_123' and user 'user_456'"); + + // Note: Remove this line if your implementation doesn't log this specific message + // logbackVerifier.expectMessage(Level.INFO, "CMAB returned variation 'treatment_1' for rule 'rule_123' and user 'user_456'"); } @Test @@ -123,6 +128,7 @@ public void returnErrorWhenStatusIsNot200AndLogError() throws Exception { CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); when(statusLine.getStatusCode()).thenReturn(500); + when(statusLine.getReasonPhrase()).thenReturn("Internal Server Error"); when(httpResponse.getStatusLine()).thenReturn(statusLine); when(httpResponse.getEntity()).thenReturn(new StringEntity("Server Error")); when(mockHttpClient.execute(any(HttpPost.class))).thenReturn(httpResponse); @@ -132,17 +138,16 @@ public void returnErrorWhenStatusIsNot200AndLogError() throws Exception { Map attributes = new HashMap<>(); String cmabUuid = "uuid_789"; - CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); - try { - future.get(); - fail("Expected ExecutionException"); - } catch (ExecutionException e) { - assertTrue(e.getCause().getMessage().contains("status: 500")); + cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + fail("Expected CmabFetchException"); + } catch (CmabFetchException e) { + assertTrue(e.getMessage().contains("Internal Server Error")); } verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); - logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: 500 for user: user_456"); + // Fixed: Match actual log message format + logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: Internal Server Error"); } @Test @@ -159,17 +164,15 @@ public void returnErrorWhenInvalidResponseAndLogError() throws Exception { Map attributes = new HashMap<>(); String cmabUuid = "uuid_789"; - CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); - try { - future.get(); - fail("Expected ExecutionException"); - } catch (ExecutionException e) { - assertEquals("Invalid CMAB fetch response", e.getCause().getMessage()); + cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + fail("Expected CmabInvalidResponseException"); + } catch (CmabInvalidResponseException e) { + assertEquals("Invalid CMAB fetch response", e.getMessage()); } verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); - logbackVerifier.expectMessage(Level.ERROR, "Invalid CMAB fetch response for user: user_456"); + logbackVerifier.expectMessage(Level.ERROR, "Invalid CMAB fetch response"); } @Test @@ -182,16 +185,96 @@ public void testNoRetryWhenNoRetryConfig() throws Exception { Map attributes = new HashMap<>(); String cmabUuid = "uuid_789"; - CompletableFuture future = cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); - try { - future.get(); - fail("Expected ExecutionException"); - } catch (ExecutionException e) { - assertTrue(e.getCause().getMessage().contains("network error")); + cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + fail("Expected CmabFetchException"); + } catch (CmabFetchException e) { + assertTrue(e.getMessage().contains("Network error")); } verify(mockHttpClient, times(1)).execute(any(HttpPost.class)); - logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: network error for user: user_456"); + logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: Network error"); + } + + @Test + public void testRetryOnNetworkError() throws Exception { + // Create retry config + RetryConfig retryConfig = new RetryConfig(2, 50L, 1.5, 10000); + CmabClientConfig config = new CmabClientConfig(retryConfig); + DefaultCmabClient cmabClientWithRetry = new DefaultCmabClient(mockHttpClient, config); + + // Setup response for successful retry + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + when(statusLine.getStatusCode()).thenReturn(200); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity(validCmabResponse)); + + // First call fails with IOException, second succeeds + when(mockHttpClient.execute(any(HttpPost.class))) + .thenThrow(new IOException("Network error")) + .thenReturn(httpResponse); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + String result = cmabClientWithRetry.fetchDecision(ruleId, userId, attributes, cmabUuid); + + assertEquals("treatment_1", result); + verify(mockHttpClient, times(2)).execute(any(HttpPost.class)); + + // Fixed: Match actual retry log message format + logbackVerifier.expectMessage(Level.INFO, "Retrying CMAB request (attempt: 1) after 50 ms..."); + } + + @Test + public void testRetryExhausted() throws Exception { + RetryConfig retryConfig = new RetryConfig(2, 50L, 1.5, 10000); + CmabClientConfig config = new CmabClientConfig(retryConfig); + DefaultCmabClient cmabClientWithRetry = new DefaultCmabClient(mockHttpClient, config); + + // All calls fail + when(mockHttpClient.execute(any(HttpPost.class))) + .thenThrow(new IOException("Network error")); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + try { + cmabClientWithRetry.fetchDecision(ruleId, userId, attributes, cmabUuid); + fail("Expected CmabFetchException"); + } catch (CmabFetchException e) { + assertTrue(e.getMessage().contains("Exhausted all retries for CMAB request")); + } + + // Should attempt initial call + 2 retries = 3 total + verify(mockHttpClient, times(3)).execute(any(HttpPost.class)); + logbackVerifier.expectMessage(Level.ERROR, "CMAB decision fetch failed with status: Exhausted all retries for CMAB request"); + } + + @Test + public void testEmptyResponseThrowsException() throws Exception { + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + when(statusLine.getStatusCode()).thenReturn(200); + when(httpResponse.getStatusLine()).thenReturn(statusLine); + when(httpResponse.getEntity()).thenReturn(new StringEntity("")); + when(mockHttpClient.execute(any(HttpPost.class))).thenReturn(httpResponse); + + String ruleId = "rule_123"; + String userId = "user_456"; + Map attributes = new HashMap<>(); + String cmabUuid = "uuid_789"; + + try { + cmabClient.fetchDecision(ruleId, userId, attributes, cmabUuid); + fail("Expected CmabInvalidResponseException"); + } catch (CmabInvalidResponseException e) { + assertEquals("Invalid CMAB fetch response", e.getMessage()); + } } } \ No newline at end of file