Skip to content

refactor: split json parsing/translation for future-proofing client/server distinction#525

Merged
beekld merged 8 commits intomainfrom
beeklimt/SDK-2238
Apr 24, 2026
Merged

refactor: split json parsing/translation for future-proofing client/server distinction#525
beekld merged 8 commits intomainfrom
beeklimt/SDK-2238

Conversation

@beekld
Copy link
Copy Markdown
Contributor

@beekld beekld commented Apr 22, 2026

As discussed in a previous PR, this splits apart the parsing and translation for FDv2 changesets, so that the server-specific code is separate from the shared code. This will be needed once we add client code later.

I went with passing around JSON objects everywhere, rather than passing in a translator, because 1) it's simpler, and 2) it's more like Java, which has advantages for understanding code across codebases.


Note

Medium Risk
Moderate risk because it changes the FDv2 ingestion pipeline and error/forward-compatibility behavior (unknown kinds and JSON deserialization), which could affect store updates and sync reliability.

Overview
Splits FDv2 event handling into two phases: the shared FDv2ProtocolHandler now only accumulates raw changes (kind, key, version, and JSON object) and emits an FDv2ChangeSet without deserializing flags/segments.

Adds a server-side TranslateChangeSet step that converts raw FDv2 changes into typed ItemDescriptor<Flag|Segment> changes for application, warning/skip on unknown kinds and aborting the whole changeset on deserialization failure.

Introduces a generic data_model::ChangeSet<T> + shared ChangeSetType, updates MemoryStore::Apply and FDv2SourceResult to consume ChangeSet<ChangeSetData>, and wires translation into FDv2 polling (including a new translation-failure interrupted error). Tests are updated and new translator unit tests are added, including the behavior change that unknown kinds now pass through the protocol layer.

Reviewed by Cursor Bugbot for commit 854edbd. Bugbot is set up for automated code reviews on this repo. Configure here.

@beekld beekld marked this pull request as ready for review April 22, 2026 23:48
@beekld beekld requested a review from a team as a code owner April 22, 2026 23:48
Comment thread libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp Outdated
return false;
}
if (!result->has_value()) {
LD_LOG(logger, LogLevel::kWarn)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is a change from a JSON parse error.

if (!result->has_value()) {
    return tl::make_unexpected(Error::JsonParseError(
        "flag '" + put.key + "' object was null"));
}

Where I still think we would want the data source level to treat this as malformed JSON for reconnect triggering purposes/status reporting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is the same behavior as Java:

https://github.com/launchdarkly/java-core/blob/f4719ecf55d11f2e16a34b904349f98c469213ba/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/FDv2ChangeSetTranslator.java#L74

I don't really think either one is more "correct" (unless the spec covers this case). But I'm happy to change it if you feel strongly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am trying to think what really should happen if this case was encountered. I am not sure either is amazing. I am fine with this for the moment.

Comment thread libs/server-sdk/src/data_systems/fdv2/fdv2_changeset_translation.cpp Outdated
@beekld beekld requested a review from kinyoklion April 24, 2026 17:42
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 854edbd. Configure here.

@beekld beekld merged commit e7411d0 into main Apr 24, 2026
45 checks passed
@beekld beekld deleted the beeklimt/SDK-2238 branch April 24, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants