chore: adds connection mode resolution and source factories#265
chore: adds connection mode resolution and source factories#265tanderson-ld wants to merge 7 commits intomainfrom
Conversation
Implements the four building blocks Phase 3 needs from Stream A: - calculate_poll_delay.dart: pure helper. Given freshness and interval, returns the time remaining in the interval; zero when overdue; full interval when there's no prior freshness or the freshness is in the future (clock skew clamp). - polling_initializer.dart: one-shot Initializer. Calls the injected PollFunction up to 3 times. ChangeSetResult and terminal status results return immediately. Interrupted results retry after a 1s delay. After 3 interrupted attempts the last is escalated to terminalError so the orchestrator stops retrying at this layer. close() interrupts a pending retry delay and yields shutdown. - polling_synchronizer.dart: long-lived Synchronizer. Single- subscription StreamController that polls immediately on subscribe, then schedules subsequent polls via calculatePollDelay over the freshness of the most recent successful result. Interrupted results pass through but do not advance freshness, so a transient failure does not delay the catch-up poll. Injected TimerFactory and now() for deterministic tests. - cache_initializer.dart: Initializer that reads the persistence cache via an injected CachedFlagsReader. Cache hit emits a full ChangeSetResult with persist=false and an empty selector (the cache does not track server-side selector state). Cache miss or reader exception emits a none-type ChangeSetResult so the initializer chain advances. The reader typedef leaves the actual persistence wiring to the orchestrator (Phase C1). PollFunction, DelayFunction, TimerFactory, and CachedFlagsReader are typedefs rather than concrete dependencies so the orchestrator can wire real implementations and tests can inject scripted ones, mirroring the abstraction style established in SDK-2183.
- polling_synchronizer: replace `bool _closed` with `Completer<void> _stoppedSignal` to match the polling initializer and have a single source of truth for closed-ness. Drop the redundant `_controller.isClosed` checks (the signal is set before the controller is closed, so the signal check covers both cases). Replace `Future<void>.microtask(_doPoll)` in onListen with `unawaited(_doPoll())` -- the microtask wrapping was unnecessary; fire-and-forget of an async function is what unawaited expresses. - polling_synchronizer: fix typo in the unexpected-throw message: "raised unexpectedly" -> "raised error unexpectedly". - calculate_poll_delay: split the run-on docstring describing the null-freshness and overdue-freshness cases into two separate sentences for readability. - cache_initializer: reword the awkward parenthetical "(the data came from the cache; writing it back is a no-op)" to "(the data is already cached)".
…king branch 'origin' into ta/SDK-2187/connection-mode-and-resolution
| this.disableAutomaticNetworkHandling = false}); | ||
| ConnectionManagerConfig({ | ||
| this.foregroundConnectionMode = ConnectionMode.streaming, | ||
| this.backgroundConnectionMode = ConnectionMode.offline, |
There was a problem hiding this comment.
Need to double check if this is a breaking change.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ 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 b6bd1de. Configure here.
| if (_offline) { | ||
| resolved = ConnectionMode.offline; | ||
| } else if (_modeOverride != null) { | ||
| resolved = _modeOverride!; |
There was a problem hiding this comment.
Bang operator used instead of Dart 3 case pattern
Low Severity
The _modeOverride nullable field is checked with != null and then accessed with the ! bang operator. The codebase's preferred idiom (and the one used in other files like requestor.dart and flag_eval_mapper.dart) is Dart 3 case-pattern narrowing, e.g. else if (_modeOverride case final override?), which avoids the bang operator and leverages compiler flow analysis.
Triggered by learned rule: Prefer Dart 3 case-pattern null narrowing over bang operator
Reviewed by Cursor Bugbot for commit b6bd1de. Configure here.
| dataSourceConfig: pollingDataSourceConfig, | ||
| httpProperties: httpProperties); | ||
| }, | ||
| ConnectionMode.background: streaming, |
There was a problem hiding this comment.
Background connection mode incorrectly mapped to streaming factory
Medium Severity
ConnectionMode.background is mapped to the streaming data source factory, which creates a persistent StreamingDataSource. This contradicts the design intent of background mode, which per BuiltInModes.background and its fdv1Fallback config specifies reduced-rate polling (3600s interval). If a user configures backgroundConnectionMode: ConnectionMode.background, the SDK would open a full streaming connection in the background instead of using a resource-efficient polling approach.
Reviewed by Cursor Bugbot for commit b6bd1de. Configure here.
|
Need to bench test to confirm no regressions in connection manager wrt event flushing. |
2 participants
Requirements
Note
Medium Risk
Changes connection-mode selection logic across the Flutter SDK (foreground/background/network) and adds a new
ConnectionMode.background, which can alter when the client goes offline, flushes, and sends events. Also introduces new FDv2 mode/source-factory plumbing that affects how polling endpoints and intervals are constructed.Overview
Adds a new
ConnectionMode.backgroundand wires it throughLDCommonClient/DataSourceManagerfactories (including offlineNullDataSourcemappings) so the SDK can represent a distinct background connection behavior.Refactors Flutter
ConnectionManagerto use a table-driven automatic resolver (resolveConnectionMode+flutterDefaultResolutionTable) with separate foreground and background mode slots, plus a nullablesetModeoverride to temporarily bypass automatic selection; it also flushes on backgrounding when online and disables event sending when offline or backgrounded withoutrunInBackground.Introduces FDv2 mode definitions and factory builders (
ModeDefinition, built-in modes,SourceFactoryContext,entry_factories) to construct polling/cache initializers/synchronizers with per-entry endpoint overrides; streaming entry factories are explicitly unsupported for now. Test coverage is added/updated for the new mode, resolver behavior, and factory-building logic.Reviewed by Cursor Bugbot for commit 69ad8d4. Bugbot is set up for automated code reviews on this repo. Configure here.