[FSSDK-13023] Fix excludeTargetedDeliveries parsing in Holdout config parsers (#638)

Author: jaeopt

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

@@ -108,7 +108,7 @@ public Holdout(@JsonProperty("id") @Nonnull String id,
             @JsonProperty("variations") @Nonnull List<Variation> variations,
             @JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation,
             @JsonProperty("includedRules") @Nullable List<String> includedRules,
-            @JsonProperty("exclude_targeted_deliveries") @Nullable Boolean excludeTargetedDeliveries) {
+            @JsonProperty("excludeTargetedDeliveries") @Nullable Boolean excludeTargetedDeliveries) {
         this.id = id;
         this.key = key;
         this.status = status;

core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java

@@ -213,8 +213,8 @@ static Holdout parseHoldout(JsonObject holdoutJson, JsonDeserializationContext c
         }
 
         boolean excludeTargetedDeliveries = false;
-        if (holdoutJson.has("exclude_targeted_deliveries") && !holdoutJson.get("exclude_targeted_deliveries").isJsonNull()) {
-            excludeTargetedDeliveries = holdoutJson.get("exclude_targeted_deliveries").getAsBoolean();
+        if (holdoutJson.has("excludeTargetedDeliveries") && !holdoutJson.get("excludeTargetedDeliveries").isJsonNull()) {
+            excludeTargetedDeliveries = holdoutJson.get("excludeTargetedDeliveries").getAsBoolean();
         }
 
         return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedRules, excludeTargetedDeliveries);

core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java

@@ -239,8 +239,8 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
             }
 
             boolean excludeTargetedDeliveries = false;
-            if (holdoutObject.has("exclude_targeted_deliveries") && !holdoutObject.isNull("exclude_targeted_deliveries")) {
-                excludeTargetedDeliveries = holdoutObject.getBoolean("exclude_targeted_deliveries");
+            if (holdoutObject.has("excludeTargetedDeliveries") && !holdoutObject.isNull("excludeTargetedDeliveries")) {
+                excludeTargetedDeliveries = holdoutObject.getBoolean("excludeTargetedDeliveries");
             }
 
             holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,

core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java

@@ -258,8 +258,8 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
             }
 
             boolean excludeTargetedDeliveries = false;
-            if (hoObject.containsKey("exclude_targeted_deliveries") && hoObject.get("exclude_targeted_deliveries") != null) {
-                excludeTargetedDeliveries = (Boolean) hoObject.get("exclude_targeted_deliveries");
+            if (hoObject.containsKey("excludeTargetedDeliveries") && hoObject.get("excludeTargetedDeliveries") != null) {
+                excludeTargetedDeliveries = (Boolean) hoObject.get("excludeTargetedDeliveries");
             }
 
             holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,

core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTestUtils.java

@@ -546,6 +546,7 @@ private static void verifyHoldouts(List<Holdout> actual, List<Holdout> expected)
             assertThat(actualHoldout.getAudienceConditions(), is(expectedHoldout.getAudienceConditions()));
             assertThat(actualHoldout.getIncludedRules(), is(expectedHoldout.getIncludedRules()));
             assertThat(actualHoldout.isGlobal(), is(expectedHoldout.isGlobal()));
+            assertThat(actualHoldout.isExcludeTargetedDeliveries(), is(expectedHoldout.isExcludeTargetedDeliveries()));
             verifyVariations(actualHoldout.getVariations(), expectedHoldout.getVariations());
             verifyTrafficAllocations(actualHoldout.getTrafficAllocation(),
                 expectedHoldout.getTrafficAllocation());

core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java

@@ -570,6 +570,28 @@ public class ValidProjectConfigV4 {
             )
     );
 
+    // Dedicated 0% traffic holdout used solely to verify excludeTargetedDeliveries parsing
+    // across all 4 ConfigParser implementations, without affecting decision-path tests
+    // (no user is ever bucketed into a 0%-traffic holdout).
+    public static final Holdout HOLDOUT_ETD_PARSER_COVERAGE = new Holdout(
+            "1007532345431",
+            "holdout_etd_parser_coverage",
+            Holdout.HoldoutStatus.RUNNING.toString(),
+            Collections.<String>emptyList(),
+            null,
+            DatafileProjectConfigTestUtils.createListOfObjects(
+                VARIATION_HOLDOUT_VARIATION_OFF
+            ),
+            DatafileProjectConfigTestUtils.createListOfObjects(
+                new TrafficAllocation(
+                        "$opt_dummy_variation_id",
+                        0
+                )
+            ),
+            null,
+            true
+    );
+
 
     public static final Holdout HOLDOUT_TYPEDAUDIENCE_HOLDOUT = new Holdout(
             "10075323429",
@@ -1685,6 +1707,7 @@ public static ProjectConfig generateValidProjectConfigV4_holdout() {
         holdouts.add(HOLDOUT_ZERO_TRAFFIC_HOLDOUT);
         holdouts.add(HOLDOUT_BASIC_HOLDOUT);
         holdouts.add(HOLDOUT_TYPEDAUDIENCE_HOLDOUT);
+        holdouts.add(HOLDOUT_ETD_PARSER_COVERAGE);
         holdouts.add(HOLDOUT_LOCAL_FOR_BASIC_EXPERIMENT_PARSER);
 
         // list featureFlags

core-api/src/test/resources/config/holdouts-project-config.json

@@ -511,7 +511,8 @@
           "id": "$opt_dummy_variation_id",
           "key": "ho_off_key"
         }
-      ]
+      ],
+      "excludeTargetedDeliveries": false
     },
     {
       "id": "10075323429",
@@ -532,6 +533,26 @@
       ],
       "audienceIds": ["3468206643", "3468206644", "3468206646", "3468206645"],
       "audienceConditions" : ["or", "3468206643", "3468206644", "3468206646", "3468206645"]
+    },
+    {
+      "audienceIds": [],
+      "id": "1007532345431",
+      "key": "holdout_etd_parser_coverage",
+      "status": "Running",
+      "trafficAllocation": [
+        {
+          "endOfRange": 0,
+          "entityId": "$opt_dummy_variation_id"
+        }
+      ],
+      "variations": [
+        {
+          "featureEnabled": false,
+          "id": "$opt_dummy_variation_id",
+          "key": "ho_off_key"
+        }
+      ],
+      "excludeTargetedDeliveries": true
     }
   ],
   "localHoldouts": [