feat: add FDv1AdapterSynchronizer wrapping IDataSynchronizer as IFDv2Synchronizer#540
feat: add FDv1AdapterSynchronizer wrapping IDataSynchronizer as IFDv2Synchronizer#540beekld wants to merge 23 commits into
Conversation
9f04fa3 to
ea3b92b
Compare
1780156 to
479e599
Compare
ea3b92b to
142b2ff
Compare
479e599 to
403bdff
Compare
142b2ff to
7ea2f7c
Compare
403bdff to
20da492
Compare
7ea2f7c to
e2824ec
Compare
20da492 to
95351be
Compare
e2824ec to
f0b2aab
Compare
95351be to
7202ca6
Compare
f0b2aab to
f94ce01
Compare
7202ca6 to
1bcd458
Compare
f94ce01 to
eb28df0
Compare
1bcd458 to
a183532
Compare
eb28df0 to
3e76ae5
Compare
a183532 to
e0396ff
Compare
e0396ff to
cce4172
Compare
cce4172 to
458539d
Compare
af2d13f to
bfe0441
Compare
458539d to
5aace2f
Compare
bfe0441 to
af660a0
Compare
5aace2f to
5988ce1
Compare
18951d8 to
35aa744
Compare
5988ce1 to
88190bc
Compare
## 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 -->
| // ----- FDv1AdapterSynchronizer ----- | ||
|
|
||
| FDv1AdapterSynchronizer::FDv1AdapterSynchronizer( | ||
| std::unique_ptr<data_interfaces::IDataSynchronizer> fdv1_source, |
There was a problem hiding this comment.
Is this ultimately going to be the correct pointer type? The data sources use shared_from_this and weak_from_this?
There was a problem hiding this comment.
Fixed. And I filed a task to add a contract test that would've caught this.
| if (!error) { | ||
| return; | ||
| } | ||
| switch (status.State()) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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.

Summary
Stacked on #539. Adds an adapter that wraps any FDv1
IDataSynchronizer(e.g. the existingStreamingDataSource) and presents it as anIFDv2Synchronizer. FDv1Init/Upsertcallbacks delivered through an internalIDestinationare translated intoFDv2SourceResult::ChangeSetresults (kFullforInit,kPartialforUpsert), with empty selectors andfdv1_fallback=false.The orchestrator picks this up after
SwitchToFDv1Fallback()once a factory whoseIsFDv1Fallback()returns true is configured. Wiring the adapter into the public config builders is a separate concern, deferred until the Configuration API work.Test plan
Note
Medium Risk
Touches core FDv2 data-sync orchestration semantics (result types, close/shutdown, status→error mapping); risk is mitigated by isolation behind
IFDv2Synchronizerand broad unit tests, but incorrect adapter behavior could affect fallback flag delivery once wired.Overview
Adds
FDv1AdapterSynchronizer, which implementsIFDv2Synchronizerby wrapping an FDv1IDataSynchronizerbuilt via an injectableSourceBuilder. The firstNext()lazily callsStartAsyncon the wrapped source with an internalIDestinationthat turns FDv1Init/UpsertintoFDv2SourceResult::ChangeSet(kFull/kPartial, empty selector, nofdv1_fallbackdirective).Lifecycle and async behavior: a thread-safe queue/promise
Stateserializes results to outstandingNext()calls;Close()races with pendingNext()viaWhenAnyand resolves waiters withShutdown, shutting down the FDv1 source only if it was started. FDv1DataSourceStatuserrors map toInterrupted(initializing/interrupted) orTerminalError(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.