Skip to content

feat: add FDv1AdapterSynchronizer wrapping IDataSynchronizer as IFDv2Synchronizer#540

Open
beekld wants to merge 23 commits into
mainfrom
beeklimt/SDK-2379-4
Open

feat: add FDv1AdapterSynchronizer wrapping IDataSynchronizer as IFDv2Synchronizer#540
beekld wants to merge 23 commits into
mainfrom
beeklimt/SDK-2379-4

Conversation

@beekld

@beekld beekld commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #539. Adds an adapter that wraps any FDv1 IDataSynchronizer (e.g. the existing StreamingDataSource) and presents it as an IFDv2Synchronizer. FDv1 Init/Upsert callbacks delivered through an internal IDestination are translated into FDv2SourceResult::ChangeSet results (kFull for Init, kPartial for Upsert), with empty selectors and fdv1_fallback=false.

The orchestrator picks this up after SwitchToFDv1Fallback() once a factory whose IsFDv1Fallback() returns true is configured. Wiring the adapter into the public config builders is a separate concern, deferred until the Configuration API work.

Test plan

  • 8 unit tests cover: lazy start, no-restart on second Next, Init→kFull, Upsert→kPartial (both overloads), pending-Next-resolved-as-Shutdown-on-close, ShutdownAsync called once on close (and not at all if never started), Next-after-Close
  • Full server suite green (486 tests)

Note

Medium Risk
Touches core FDv2 data-sync orchestration semantics (result types, close/shutdown, status→error mapping); risk is mitigated by isolation behind IFDv2Synchronizer and broad unit tests, but incorrect adapter behavior could affect fallback flag delivery once wired.

Overview
Adds FDv1AdapterSynchronizer, which implements IFDv2Synchronizer by wrapping an FDv1 IDataSynchronizer built via an injectable SourceBuilder. The first Next() lazily calls StartAsync on the wrapped source with an internal IDestination that turns FDv1 Init/Upsert into FDv2SourceResult::ChangeSet (kFull / kPartial, empty selector, no fdv1_fallback directive).

Lifecycle and async behavior: a thread-safe queue/promise State serializes results to outstanding Next() calls; Close() races with pending Next() via WhenAny and resolves waiters with Shutdown, shutting down the FDv1 source only if it was started. FDv1 DataSourceStatus errors map to Interrupted (initializing/interrupted) or TerminalError (kOff).

The new sources are registered in CMakeLists.txt. Unit tests exercise start-once semantics, conversion, FIFO queuing, status mapping, and close paths. Public config/orchestrator factory wiring is not in this diff (intended for a follow-up).

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

@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from 9f04fa3 to ea3b92b Compare May 28, 2026 18:22
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 1780156 to 479e599 Compare May 28, 2026 18:24
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from ea3b92b to 142b2ff Compare May 28, 2026 20:19
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 479e599 to 403bdff Compare May 28, 2026 20:20
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from 142b2ff to 7ea2f7c Compare May 28, 2026 23:22
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 403bdff to 20da492 Compare May 28, 2026 23:22
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from 7ea2f7c to e2824ec Compare May 28, 2026 23:52
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 20da492 to 95351be Compare May 28, 2026 23:53
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from e2824ec to f0b2aab Compare May 29, 2026 00:32
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 95351be to 7202ca6 Compare May 29, 2026 00:32
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from f0b2aab to f94ce01 Compare May 29, 2026 21:17
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 7202ca6 to 1bcd458 Compare May 29, 2026 21:17
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from f94ce01 to eb28df0 Compare May 29, 2026 21:20
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 1bcd458 to a183532 Compare May 29, 2026 21:20
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from eb28df0 to 3e76ae5 Compare May 29, 2026 21:45
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from a183532 to e0396ff Compare May 29, 2026 21:45
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from e0396ff to cce4172 Compare June 2, 2026 20:42
@beekld beekld mentioned this pull request Jun 2, 2026
2 tasks
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from cce4172 to 458539d Compare June 4, 2026 00:53
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from af2d13f to bfe0441 Compare June 4, 2026 00:53
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 458539d to 5aace2f Compare June 4, 2026 21:54
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch from bfe0441 to af660a0 Compare June 4, 2026 21:54
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 5aace2f to 5988ce1 Compare June 4, 2026 23:43
@beekld beekld force-pushed the beeklimt/SDK-2379-3 branch 2 times, most recently from 18951d8 to 35aa744 Compare June 4, 2026 23:51
@beekld beekld force-pushed the beeklimt/SDK-2379-4 branch from 5988ce1 to 88190bc Compare June 4, 2026 23:51
beekld added a commit that referenced this pull request Jun 9, 2026
beekld added a commit that referenced this pull request Jun 9, 2026
beekld added a commit that referenced this pull request Jun 11, 2026
beekld added a commit that referenced this pull request Jun 11, 2026
## Summary

Implements the FDv1 fallback directive in the FDv2 data system with
TTL-based recovery.

- `FDv2SourceResult` carries `std::optional<FDv1FallbackDirective>` with
a TTL (default 1h, 0 = indefinite).
- Sources parse `X-LD-FD-Fallback-TTL` (header) and
`protocolFallbackTTL` (goodbye).
- On directive: switch to FDv1 if configured, otherwise disconnect
(`kInterrupted`). Schedule retry after TTL.
- On TTL elapse: `SourceManager::SwitchBackToFDv2()` re-enables FDv2
(including previously terminal-error-blocked factories), blocks FDv1,
restarts synchronizers.

End-to-end fallback will work once an FDv1 streaming source is wrapped
as an `IFDv2Synchronizer` (#540).

## Test plan

- [x] `SourceManager` tests cover the new block/unblock semantics
- [x] Source-level tests cover TTL parsing (HTTP + goodbye), default,
malformed
- [x] Orchestrator tests cover directive flows in
initializer/synchronizer paths, TTL elapse rebuild, FDv2 recovery
- [x] Full server suite green

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes core flag-data orchestration and protocol fallback behavior;
mis-handling could leave clients on the wrong protocol or retry timing,
though behavior is heavily tested.
> 
> **Overview**
> Adds **server-directed FDv1 fallback** for the FDv2 data path: results
now carry an optional `FDv1FallbackDirective` (TTL, default 1h; **0 =
stay on FDv1 indefinitely**) instead of a boolean flag.
> 
> **Parsing:** Goodbye JSON gains `protocolFallbackTTL`; polling and
streaming read `X-LD-FD-Fallback` / `X-LD-FD-Fallback-TTL`, with
body/goodbye directives overriding headers.
> 
> **Orchestration:** On directive, `SourceManager` blocks FDv2 factories
and selects an `IsFDv1Fallback()` synchronizer; missing adapter →
`kInterrupted` without forcing `kOff`. A cancellable timer calls
`SwitchBackToFDv2()` (re-enabling FDv2 factories blocked for terminal
errors) and restarts synchronizers. Destructor/`Close()` no longer sets
status to `kOff`; retry timers are cancelled on close.
> 
> **Other:** Logging tweaks (initializer/synchronizer start, deduped
synchronizer interrupts). Broad unit/orchestrator tests cover TTL
parsing, switching, recovery, and edge cases.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
81b1c7f. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Base automatically changed from beeklimt/SDK-2379-3 to main June 11, 2026 17:56
beekld added a commit that referenced this pull request Jun 11, 2026
@beekld beekld marked this pull request as ready for review June 11, 2026 17:58
@beekld beekld requested a review from a team as a code owner June 11, 2026 17:58
Comment thread libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp Outdated
Comment thread libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp
Comment thread libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp
beekld added a commit that referenced this pull request Jun 11, 2026
// ----- FDv1AdapterSynchronizer -----

FDv1AdapterSynchronizer::FDv1AdapterSynchronizer(
std::unique_ptr<data_interfaces::IDataSynchronizer> fdv1_source,

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.

Is this ultimately going to be the correct pointer type? The data sources use shared_from_this and weak_from_this?

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.

Fixed. And I filed a task to add a contract test that would've caught this.

if (!error) {
return;
}
switch (status.State()) {

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.

Do we need to be concerned with error only status changes? Sometimes we update the error but leave the state the same. So it wouldn't affect the flow of fallback/recovery, but the granularity of the reported status.

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.

Fixed.

Comment thread libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 using default effort 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 2f68933. Configure here.

Comment thread libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp
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