[FSSDK-13023] Fix excludeTargetedDeliveries parsing in Holdout config parsers - #638
Merged
Conversation
Adds isExcludeTargetedDeliveries() to verifyHoldouts() and a dedicated 0%-traffic holdout (holdout_etd_parser_coverage) to the shared holdout fixture, so all 4 ConfigParser implementations are checked against a true value through the existing parseProjectConfigHoldoutV4 tests. GSON, org.json, and JSON-simple only recognize the snake_case "exclude_targeted_deliveries" key, while the rest of the datafile schema is camelCase; this exposes that GSON/org.json/JSON-simple silently default to false on the camelCase "excludeTargetedDeliveries" key while Jackson parses it correctly, so those 3 parser tests are expected to fail until the parsers are fixed to match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s camelCase GSON, org.json, and JSON-simple looked for the snake_case "exclude_targeted_deliveries" key while every other datafile field is camelCase, so a real datafile using the conventional camelCase spelling silently parsed as false in those 3 parsers. Jackson already matched the conventional spelling. Aligns all 4 parsers on "excludeTargetedDeliveries", turning the previously-red parser tests green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a schema-key mismatch in the Holdout config parsing logic where 3 parsers were reading excludeTargetedDeliveries using a snake_case key, causing standard camelCase datafiles to silently parse the flag as false. It aligns all parsers (and the Jackson model annotation) on the camelCase key and adds regression coverage across config parsers.
Changes:
- Updated GSON, org.json, and JSON-simple holdout parsers to read
excludeTargetedDeliveries(camelCase). - Updated
Holdout’s Jackson@JsonPropertyto the camelCase key for consistency with the datafile schema. - Added a dedicated holdout fixture + assertion coverage to verify correct parsing across all config parser implementations.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core-api/src/test/resources/config/holdouts-project-config.json | Adds explicit excludeTargetedDeliveries values (false/true) plus a dedicated holdout to exercise parsing. |
| core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java | Introduces a dedicated 0% traffic holdout constant and includes it in the generated holdout config expectations. |
| core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTestUtils.java | Extends holdout verification to assert excludeTargetedDeliveries matches expected values. |
| core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java | Switches holdout parsing to camelCase excludeTargetedDeliveries. |
| core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java | Switches holdout parsing to camelCase excludeTargetedDeliveries. |
| core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java | Switches holdout parsing to camelCase excludeTargetedDeliveries. |
| core-api/src/main/java/com/optimizely/ab/config/Holdout.java | Updates Jackson constructor annotation to excludeTargetedDeliveries (camelCase) and retains null→false defaulting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
raju-opti
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Holdout config parsers (GSON, org.json, JSON-simple) looked for the
excludeTargetedDeliveriesflag using a snake_case key while every other datafile field uses camelCase, so datafiles using the standard camelCase spelling silently parsed the flag asfalsein 3 of 4 parsers. This aligns all parsers on the camelCase key and adds coverage so a regression is caught across every config parser going forward.Changes
excludeTargetedDeliveriesparses correctly across all 4 config parsersexcludeTargetedDeliverieskey, matching Jackson and the rest of the datafile schemaHoldout's Jackson@JsonPropertyannotation to the camelCase key name for consistencyJira Ticket
FSSDK-13023
🤖 Generated with Claude Code