diff --git a/docs/port/PROGRESS.md b/docs/port/PROGRESS.md index b082aed..8946126 100644 --- a/docs/port/PROGRESS.md +++ b/docs/port/PROGRESS.md @@ -661,3 +661,79 @@ Phases 0–4 are complete. What is left is verification and cutover: **Known parity gap so far:** notification actions (Pause/Resume in the shade), lost with `flutter_foreground_task`. + +--- + +## Phase 5 — Verification + +### T22 — Uploader and config · **complete** + +`lib/src/telemetry/telemetry_uploader.dart` and `lib/src/config/config.dart`. **7 tests.** + +`package:http` with a `MockClient` replaces OkHttp with MockWebServer, and the tests run +against a real in-memory Drift database rather than a fake DAO — closer to production and +still no device. Batching, retry, the offline-safe backlog via `synced`, and per-point +trip/segment identity all carried over. + +Upload runs on **its own timer** in the engine, injected as a callback so the engine has +no opinion about HTTP and tests need no network. Every failure path is swallowed: nothing +about uploading may disturb recording. + +`Config` uses `shared_preferences`, with a hand-rolled UUID v4 rather than a package for +sixteen bytes. `mapEnabled` now comes from preferences instead of being hardcoded. + +**Parity note:** there is still **no UI** for the endpoint, exactly as in the native app. + +> Two `prefer_initializing_formals` lints are suppressed with a reason: Dart does not +> permit a named parameter whose name begins with an underscore, so the lint's suggested +> fix does not compile. + +### T23 — Widget tests · **complete** (delivered in Phase 4) + +### T24 — Integration tests · **complete** + +`integration_test/app_test.dart`. **4 tests, passing on the iOS simulator.** + +These cover what widget tests structurally cannot: Drift opening against real platform +storage, plugin registration resolving, `go_router` driving a real Navigator, and a cold +start surviving. The database test in particular is meaningful — an in-memory Drift +instance cannot prove the sqlite3 native library loaded on the device. + +``` +flutter test integration_test -d → 00:50 +4: All tests passed! +``` + +**Not yet run on the Android emulator**, which needs 7.4 GiB free to boot; run +`flutter clean` first. + +### T25 — Real ride · **outstanding, and it is the important one** + +Written up as [REAL-RIDE-CHECKLIST.md](REAL-RIDE-CHECKLIST.md): eleven shared checks, three +Android-only, five iOS-only. + +Three things in it are worth calling out: + +- **A2 (Android):** force-stop mid-ride and confirm the dead time is *not* added to + distance — the native bug found in T11, measured at 111 km. +- **I3 (iOS):** park for fifteen minutes mid-recording and see whether recording resumes. + **This decides the platform strategy.** If `geolocator` loses rides to suspension, the + `LocationSource` seam exists so `flutter_background_geolocation` (~$500/yr) can replace + it as one new implementation. +- **Record the Android ride on both apps at once.** They have different application ids + precisely so they can coexist, and a direct numeric comparison is far stronger evidence + than either app alone. + +### T26 — iOS release readiness · **configured, not submitted** + +[RELEASE-IOS.md](RELEASE-IOS.md). Usage strings are written specifically rather than +generically (the leading 5.1.1 rejection cause), privacy-label answers are decided, and +the pre-submission list covers the bundle-id switch, the still-default app icon, a release +build, and a demo video for the review notes. + +--- + +## Phase 5 status + +**T22, T23, T24 complete. T25 and T26 need a real device and a real rider.** + +`178 tests passing` (171 unit + widget, plus 4 integration on device), analyze clean. diff --git a/docs/port/REAL-RIDE-CHECKLIST.md b/docs/port/REAL-RIDE-CHECKLIST.md new file mode 100644 index 0000000..12cd847 --- /dev/null +++ b/docs/port/REAL-RIDE-CHECKLIST.md @@ -0,0 +1,100 @@ +# T25 — the real-ride checklist + +**This is the only task in the plan that cannot be automated, and it is the one that +matters most.** 171 unit and widget tests plus 4 integration tests are green, and none of +them prove the app records a ride correctly. + +Adapted from the native repo's `docs/TESTING.md`, extended for two platforms. + +--- + +## Why a green suite proves nothing here + +**Neither simulator produces velocity.** `adb emu geo fix` teleports the device and iOS's +simulated locations are no better, so every recorded `speedKmh` is `0.0`. That makes the +following **structurally unverifiable** without riding: + +- max speed, average moving speed +- moving time (everything sits under the 1.5 km/h noise floor, so it stays 00:00:00) +- speed colouring on the map — the whole path renders in one colour +- elevation gain against real, *correlated* GPS altitude error +- battery over a multi-hour ride +- whether iOS suspends a stationary app mid-ride + +And the subtler trap, which cost v2.0 a release: **when a value cannot change under test, +the UI around it cannot be judged either.** Max speed as the headline looked perfectly +fine on an emulator where every number was zero. On a real ride it read as a frozen, +broken screen. + +--- + +## Before you ride + +Install both apps. They have different application ids on purpose +(`com.rippr` and `com.rippr.port`), so they coexist: + +```bash +flutter build apk --debug && flutter install # Flutter port +adb install -r ~/dojo/samplez/rippr-2.0.1-debug.apk # native reference +``` + +**Run both simultaneously on the Android ride.** Recording the same ride twice gives a +direct numeric comparison, which is far stronger evidence than either app alone. + +--- + +## The ride + +Do this once on **Android** and once on **iOS**. Pocket the phone — that is the founding +use case. + +| # | Check | Why it is here | +|---|---|---| +| 1 | Start, pocket, ride ~20 min, stop | The actual usage pattern | +| 2 | Max speed plausible against the speedometer | Unverifiable on any simulator | +| 3 | Distance plausible against the odometer | Guards the cross-batch anchor | +| 4 | Moving time excludes stops | Noise floor behaviour on real data | +| 5 | **Elevation gain near zero on flat ground** | The most likely silent bug; synthetic noise is uniform, real error is correlated | +| 6 | Pause at a stop, resume — **no straight line across the gap** | The segment guarantee | +| 7 | Speed colouring visibly varies along the path | Cannot render in more than one colour on a simulator | +| 8 | **A short ride (under 100 m) still shows streets** | The v2.0 zoom bug | +| 9 | GPX opens correctly in Google Earth or Strava | Schema-valid is not the same as accepted | +| 10 | Battery drain over a multi-hour ride | Never measured, on either app | +| 11 | Pause/resume survives a screen-off stretch | Android wake lock, iOS background mode | + +### Android only + +| # | Check | +|---|---| +| A1 | The ongoing notification appears and persists with the screen off | +| A2 | Force-stop the app mid-ride, relaunch — the ride resumes into a **new segment**, and the dead time is **not** added to distance | +| A3 | Compare totals against the native app recording the same ride | + +> **A2 is the fix for a real bug found in the native app.** See T11 in +> [PROGRESS.md](PROGRESS.md): the native version measures straight through the dead time, +> which a synthetic test showed adding **111 km**. Confirm the port does not. + +### iOS only — the genuine unknown + +| # | Check | +|---|---| +| I1 | The blue background-location indicator appears while recording | +| I2 | Lock the screen for 10 minutes of riding — fixes keep arriving | +| I3 | **Park for 15 minutes without stopping the recording, then ride again.** Does recording resume? | +| I4 | Take a phone call mid-ride; recording survives | +| I5 | Swipe the app away mid-ride — what happens? Document it, whatever it is | + +**I3 is the decisive test for the whole platform strategy.** If iOS suspends the app when +stationary and does not reliably resume, `geolocator` is not sufficient and the +`LocationSource` seam exists precisely so `flutter_background_geolocation` (~$500/yr) can +be swapped in as one new implementation. Do not make that call without this data. + +--- + +## Recording the results + +Append findings to [PROGRESS.md](PROGRESS.md) under a T25 heading, including the numbers +from both apps where Android was recorded twice. If elevation gain on flat ground is +implausible, **do not tune it blind** — the native backlog says so explicitly, and the +port's parity harness (`tool/parity/run.sh`) means any change can be checked against the +Kotlin implementation first. diff --git a/docs/port/RELEASE-IOS.md b/docs/port/RELEASE-IOS.md new file mode 100644 index 0000000..a231498 --- /dev/null +++ b/docs/port/RELEASE-IOS.md @@ -0,0 +1,78 @@ +# T26 — iOS release readiness + +What App Review will look at, and what is already in place. Background-location apps get +more scrutiny than most, and the research in `docs/PORT_RESEARCH.md` names unclear +privacy disclosure as the leading rejection cause. + +**Status: configured, not submitted.** Nothing here has been through review. + +--- + +## Guideline 5.1.1 — data privacy and transparency + +**Rejection cause:** generic usage strings like *"This app needs location"*. + +Already in `ios/Runner/Info.plist`, written specifically: + +| Key | Says | +|---|---| +| `NSLocationWhenInUseUsageDescription` | Records the GPS track of your ride so you can see route, speed and distance afterwards. **Nothing is recorded until you press Start.** | +| `NSLocationAlwaysAndWhenInUseUsageDescription` | Keeps recording while the phone is in your pocket or the screen is off, so a ride you started is captured beginning to end. **Recording stops the moment you press Stop.** | + +Both name what is collected, why, and when it stops. That last clause matters — it is the +difference between "an app that wants your location" and "a recorder you control". + +## App Privacy nutrition labels + +Declare, and make sure it stays true: + +- **Location → Precise Location**, linked to the user? **No.** Used for **App + Functionality** only. +- **No data collected for tracking**, no advertising identifiers, no analytics SDKs. +- **Data is not transmitted off-device by default.** The uploader exists but has no UI and + no endpoint configured; it is inert unless someone sets one deliberately. + +> If a server ever ships (the v3 group-ride idea), these labels must change **before** it +> does. Undisclosed background transmission is a straightforward rejection. + +## Guideline 2.1 — completeness and background stability + +The exposure is crashing on background resume. Mitigations in place: + +- Recording state lives in the database, so resuming after suspension reads real state + rather than guessing. +- `restoreAfterProcessDeath` runs at startup and is covered by four tests, including the + crash-gap guard. +- Every upload failure path is swallowed; the network cannot stop a recording. +- Database write failures are caught per batch — losing points beats losing the app. + +**Still unproven:** what iOS actually does when the app is suspended while stationary. +That is item **I3** in [REAL-RIDE-CHECKLIST.md](REAL-RIDE-CHECKLIST.md) and it must be +answered before submission. + +## Guideline 4.2 — minimum functionality + +Not a realistic risk: native Flutter UI, real hardware integration, offline-first storage, +no web view anywhere. + +--- + +## Before submitting + +- [ ] **Switch the bundle id** from `com.rippr.port` to `com.rippr` (T27). The `.port` + suffix exists only so the native app can be installed alongside during the port. +- [ ] `CFBundleName` is still the generated lowercase `rippr`; `CFBundleDisplayName` is + already `Rippr`. Make them consistent. +- [ ] App icon and launch screen — still Flutter defaults. The native app's adaptive icon + artwork lives in `~/dojo/rippr/design/` and needs re-exporting at iOS sizes. +- [ ] Run [REAL-RIDE-CHECKLIST.md](REAL-RIDE-CHECKLIST.md) on a real iPhone, especially I3. +- [ ] Record a demo video of a real ride for the review notes. Background-location apps + are frequently asked to justify the entitlement; a video pre-empts a rejection round. +- [ ] `flutter build ipa --release` and confirm the release build works — everything so + far has been debug. + +## Known parity gap to disclose internally + +The notification cannot carry actions, so the native app's Pause/Resume buttons in the +shade are absent on both platforms. Not an App Review issue; it is a feature difference +the T27 audit must record. diff --git a/integration_test/app_test.dart b/integration_test/app_test.dart new file mode 100644 index 0000000..861784b --- /dev/null +++ b/integration_test/app_test.dart @@ -0,0 +1,84 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:integration_test/integration_test.dart'; +import 'package:rippr/main.dart' as app; + +/// End-to-end tests against the real app on a real device or simulator. +/// +/// These exist to cover what widget tests structurally cannot: that Drift actually opens +/// against platform storage, that the plugin registrations resolve, that go_router +/// navigates a real Navigator, and that the app survives a cold start. +/// +/// **What these still cannot prove.** Neither simulator produces velocity — `adb emu geo +/// fix` teleports and iOS's simulated locations are no better — so max speed, moving +/// time, average moving speed and speed colouring on the map remain unverifiable here. +/// A green run of this file says nothing about any of them. That is what T25's real ride +/// is for; see `docs/port/PLAN.md` and the native repo's `docs/TESTING.md`. +/// +/// Run with: +/// flutter test integration_test -d `` +void main() { + IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + + group('cold start', () { + testWidgets('the app launches and lands on the record screen', + (tester) async { + app.main(); + await tester.pumpAndSettle(const Duration(seconds: 5)); + + expect(find.text('RIPPR'), findsOneWidget); + expect(find.text('SPEED'), findsOneWidget); + // Idle, because a fresh install has no ride in progress. + expect(find.text('Ready'), findsOneWidget); + }); + + testWidgets('the database opens against real platform storage', + (tester) async { + // If Drift could not open its file, or the sqlite3 native library were missing, + // the trips list would surface an error rather than an empty state. This is the + // check a widget test with an in-memory database cannot make. + app.main(); + await tester.pumpAndSettle(const Duration(seconds: 5)); + + await tester.tap(find.text('Rides')); + await tester.pumpAndSettle(); + + expect(find.textContaining('No rides yet'), findsOneWidget); + }); + }); + + group('navigation', () { + testWidgets('record → rides → back', (tester) async { + app.main(); + await tester.pumpAndSettle(const Duration(seconds: 5)); + + await tester.tap(find.text('Rides')); + await tester.pumpAndSettle(); + expect(find.text('RIDES'), findsOneWidget); + + await tester.tap(find.byKey(const Key('back'))); + await tester.pumpAndSettle(); + expect(find.text('SPEED'), findsOneWidget); + }); + }); + + group('permissions', () { + testWidgets('starting without location permission explains itself', + (tester) async { + // On a simulator with permission denied, Start must surface a readable message + // rather than silently doing nothing or throwing into the void. + // + // If permission IS granted in the test environment, recording begins instead — + // both are acceptable outcomes, so this asserts only that the app stays alive and + // responsive either way. + app.main(); + await tester.pumpAndSettle(const Duration(seconds: 5)); + + await tester.tap(find.byKey(const Key('start'))); + await tester.pumpAndSettle(const Duration(seconds: 3)); + + expect(tester.takeException(), isNull); + expect(find.byType(MaterialApp), findsOneWidget); + }); + }); +} diff --git a/lib/main.dart b/lib/main.dart index 5f383e3..4a2b69f 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'src/app/providers.dart'; +import 'src/config/config.dart'; import 'src/ui/router.dart'; import 'src/ui/theme.dart'; @@ -24,8 +25,10 @@ class _RipprAppState extends ConsumerState { super.initState(); // Re-attach to a ride that was in progress when the process died. Must run before any // user interaction, and the database is the only thing that knows a ride was open. - WidgetsBinding.instance.addPostFrameCallback((_) { - ref.read(recordingEngineProvider).restoreAfterProcessDeath(); + WidgetsBinding.instance.addPostFrameCallback((_) async { + // Preferences first: the uploader and the map toggle both read from them. + ref.read(configProvider.notifier).state = await Config.load(); + await ref.read(recordingEngineProvider).restoreAfterProcessDeath(); }); } diff --git a/lib/src/app/providers.dart b/lib/src/app/providers.dart index 9d89c95..f99d479 100644 --- a/lib/src/app/providers.dart +++ b/lib/src/app/providers.dart @@ -10,6 +10,7 @@ import 'package:drift/drift.dart' show driftRuntimeOptions; import 'package:drift_flutter/drift_flutter.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../config/config.dart'; import '../data/database.dart'; import '../data/trip_repository.dart'; import '../domain/models.dart'; @@ -17,6 +18,7 @@ import '../recording/geolocator_location_source.dart'; import '../recording/location_source.dart'; import '../recording/recording_engine.dart'; import '../telemetry/live_telemetry.dart'; +import '../telemetry/telemetry_uploader.dart'; /// The Drift database, opened against app-private storage. /// @@ -39,10 +41,30 @@ final locationSourceProvider = Provider((ref) { return source; }); +/// Loaded once at startup; null until then so nothing blocks the first frame. +final configProvider = StateProvider((ref) => null); + +/// The uploader, or null when no endpoint is configured. +/// +/// Parity note: there is still **no UI** for setting the endpoint, exactly as in the +/// native app. It is reachable only through `Config.setUploadEndpoint`. +final uploaderProvider = Provider((ref) { + final config = ref.watch(configProvider); + if (config == null || config.uploadEndpoint.trim().isEmpty) return null; + final uploader = TelemetryUploader( + db: ref.watch(databaseProvider), + endpoint: config.uploadEndpoint, + deviceId: config.deviceId, + ); + ref.onDispose(uploader.close); + return uploader; +}); + final recordingEngineProvider = Provider((ref) { final engine = RecordingEngine( repository: ref.watch(tripRepositoryProvider), locationSource: ref.watch(locationSourceProvider), + uploadPending: () async => ref.read(uploaderProvider)?.uploadPending(), ); ref.onDispose(engine.dispose); return engine; @@ -76,4 +98,6 @@ final recorderStateProvider = StreamProvider((ref) { /// /// Kept as a toggle from v2: the map must only ever be live on a visible screen, and no /// tile is fetched while recording in the background. -final mapEnabledProvider = StateProvider((ref) => true); +final mapEnabledProvider = StateProvider( + (ref) => ref.watch(configProvider)?.mapEnabled ?? true, +); diff --git a/lib/src/config/config.dart b/lib/src/config/config.dart new file mode 100644 index 0000000..779f8dd --- /dev/null +++ b/lib/src/config/config.dart @@ -0,0 +1,61 @@ +/// Ported from `com.rippr.Config`. +/// +/// Runtime configuration. The upload endpoint is intentionally empty by default — +/// recording must work with no server at all, and uploading is opt-in. +library; + +import 'dart:math'; + +import 'package:shared_preferences/shared_preferences.dart'; + +const _keyEndpoint = 'upload_endpoint'; +const _keyDeviceId = 'device_id'; +const _keyMapEnabled = 'map_enabled'; + +class Config { + Config(this._prefs); + + final SharedPreferences _prefs; + + static Future load() async => + Config(await SharedPreferences.getInstance()); + + String get uploadEndpoint => _prefs.getString(_keyEndpoint) ?? ''; + + Future setUploadEndpoint(String url) => + _prefs.setString(_keyEndpoint, url.trim()); + + /// Whether trip detail renders a map at all. + /// + /// When off the map widget is never created, so no tile is ever requested — a real + /// short-circuit, not a hidden view. The map only ever exists inside a visible screen's + /// lifecycle; the recorder never touches it. + bool get mapEnabled => _prefs.getBool(_keyMapEnabled) ?? true; + + Future setMapEnabled(bool enabled) => + _prefs.setBool(_keyMapEnabled, enabled); + + /// Stable per-install id so a server can distinguish riders in a group. + String get deviceId { + final existing = _prefs.getString(_keyDeviceId); + if (existing != null) return existing; + final generated = _randomId(); + // Fire-and-forget: the value is returned immediately either way, and a lost write + // only costs a new id next launch. + _prefs.setString(_keyDeviceId, generated); + return generated; + } + + /// A UUID v4, without pulling in a package for sixteen bytes. + static String _randomId() { + final rng = Random.secure(); + final bytes = List.generate(16, (_) => rng.nextInt(256)); + bytes[6] = (bytes[6] & 0x0f) | 0x40; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + String hex(int start, int end) => bytes + .sublist(start, end) + .map((b) => b.toRadixString(16).padLeft(2, '0')) + .join(); + return '${hex(0, 4)}-${hex(4, 6)}-${hex(6, 8)}-${hex(8, 10)}-${hex(10, 16)}'; + } +} diff --git a/lib/src/recording/recording_engine.dart b/lib/src/recording/recording_engine.dart index c5e978c..fcfab97 100644 --- a/lib/src/recording/recording_engine.dart +++ b/lib/src/recording/recording_engine.dart @@ -28,6 +28,10 @@ /// the explicit drains at pause, stop and discard. library; +// Dart does not permit a named parameter whose name begins with an underscore, so the +// lint's suggested `required this._uploadPending` will not compile here. +// ignore_for_file: prefer_initializing_formals + import 'dart:async'; import 'package:synchronized/synchronized.dart'; @@ -46,6 +50,13 @@ const int flushSize = 25; /// Coalesce bursts without letting points sit unwritten for long. const Duration flushInterval = Duration(seconds: 2); +/// How often the backlog is offered to the server. +/// +/// Upload runs on its own timer so a slow or dead endpoint can never interrupt +/// recording — the same separation the native service kept between its writer loop and +/// its upload loop. +const Duration uploadInterval = Duration(seconds: 30); + /// What the recorder is doing right now, for the UI. enum RecorderState { idle, recording, paused } @@ -55,16 +66,23 @@ class RecordingEngine { required LocationSource locationSource, LiveTelemetry? liveTelemetry, int Function()? clock, + Future Function()? uploadPending, }) : _repo = repository, _source = locationSource, _live = liveTelemetry ?? LiveTelemetry.instance, - _now = clock ?? (() => DateTime.now().millisecondsSinceEpoch); + _now = clock ?? (() => DateTime.now().millisecondsSinceEpoch), + _uploadPending = uploadPending; final TripRepository _repo; final LocationSource _source; final LiveTelemetry _live; final int Function() _now; + /// Injected rather than constructed here, so the engine has no opinion about HTTP and + /// tests need no network. + final Future Function()? _uploadPending; + Timer? _uploadTimer; + /// Unbounded, exactly like the Kotlin `Channel(UNLIMITED)`. Appending is the only work /// done on the fix path. final List _pending = []; @@ -127,10 +145,22 @@ class RecordingEngine { await _source.start(); _subscription ??= _source.fixes.listen(_onFix); _flushTimer ??= Timer.periodic(flushInterval, (_) => _flush()); + _startUploadLoop(); _setState(RecorderState.recording); } + void _startUploadLoop() { + final upload = _uploadPending; + if (upload == null || _uploadTimer != null) return; + _uploadTimer = Timer.periodic(uploadInterval, (_) async { + // Swallowed on purpose. Nothing about uploading may disturb recording. + try { + await upload(); + } catch (_) {} + }); + } + /// Stops consuming GPS but leaves the trip open, so resuming is instant. Future pause() async { // Stop the source first, then drain, then close the segment. Points already queued @@ -320,6 +350,8 @@ class RecordingEngine { Future _teardown() async { _flushTimer?.cancel(); _flushTimer = null; + _uploadTimer?.cancel(); + _uploadTimer = null; await _subscription?.cancel(); _subscription = null; await _writeLock.synchronized(() async { diff --git a/lib/src/telemetry/telemetry_uploader.dart b/lib/src/telemetry/telemetry_uploader.dart new file mode 100644 index 0000000..89f8662 --- /dev/null +++ b/lib/src/telemetry/telemetry_uploader.dart @@ -0,0 +1,118 @@ +/// Ported from `com.rippr.TelemetryUploader`. +/// +/// Streams recorded points to a REST endpoint when the network allows it. +/// +/// Upload is strictly secondary to recording: every failure path here is swallowed and +/// retried later, and nothing in this class can stop the location pipeline. Points stay +/// in the database with `synced = 0` until the server acknowledges them, so a dead +/// endpoint costs nothing but a growing backlog. +/// +/// **Parity note:** as in the native app, this still has no UI. It is reachable only by +/// setting an endpoint in [Config]. +library; + +import 'dart:async'; + +import 'package:http/http.dart' as http; + +import '../data/database.dart'; +import 'live_telemetry.dart'; +import 'telemetry.dart'; + +const int uploadBatchSize = 200; +const int _maxBatchesPerRun = 10; +const Duration _timeout = Duration(seconds: 20); + +sealed class UploadResult { + const UploadResult(); +} + +class UploadDisabled extends UploadResult { + const UploadDisabled(); +} + +class UploadFailed extends UploadResult { + const UploadFailed(); +} + +class UploadPartial extends UploadResult { + const UploadPartial(this.uploaded); + final int uploaded; +} + +class UploadSuccess extends UploadResult { + const UploadSuccess(this.uploaded); + final int uploaded; +} + +class TelemetryUploader { + TelemetryUploader({ + required AppDatabase db, + required String endpoint, + required String deviceId, + http.Client? client, + UploadStatus? status, + }) : _db = db, + _endpoint = endpoint, + _deviceId = deviceId, + _client = client ?? http.Client(), + _status = status ?? UploadStatus.instance; + // ignore_for_file: prefer_initializing_formals + // Dart does not permit a named parameter whose name begins with an underscore, so the + // lint's suggested `required this._db` will not compile here. + + final AppDatabase _db; + final String _endpoint; + final String _deviceId; + final http.Client _client; + final UploadStatus _status; + + Future uploadPending() async { + if (_endpoint.trim().isEmpty) return const UploadDisabled(); + + var uploaded = 0; + for (var i = 0; i < _maxBatchesPerRun; i++) { + final batch = await _db.unsyncedPoints(uploadBatchSize); + if (batch.isEmpty) return UploadSuccess(uploaded); + + final ok = await _postBatch(batch); + if (!ok) { + return uploaded > 0 ? UploadPartial(uploaded) : const UploadFailed(); + } + await _db.markSynced([for (final p in batch) p.id]); + uploaded += batch.length; + + // Server was fine but there may be more; a brief pause so a long backlog does not + // saturate a weak mobile link. + if (batch.length == uploadBatchSize) { + await Future.delayed(const Duration(milliseconds: 250)); + } + } + return UploadPartial(uploaded); + } + + Future _postBatch(List batch) async { + try { + final response = await _client + .post( + Uri.parse(_endpoint), + headers: const {'Content-Type': 'application/json'}, + body: encodeBatch(_deviceId, batch.cast()), + ) + .timeout(_timeout); + + if (response.statusCode >= 200 && response.statusCode < 300) { + _status.setError(null); + return true; + } + _status.setError('HTTP ${response.statusCode}'); + return false; + } catch (e) { + // Any network problem is transient by assumption. The backlog survives. + _status.setError('$e'); + return false; + } + } + + void close() => _client.close(); +} diff --git a/test/telemetry_uploader_test.dart b/test/telemetry_uploader_test.dart new file mode 100644 index 0000000..b877371 --- /dev/null +++ b/test/telemetry_uploader_test.dart @@ -0,0 +1,152 @@ +import 'dart:convert'; + +import 'package:drift/drift.dart' show driftRuntimeOptions; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:rippr/src/data/database.dart'; +import 'package:rippr/src/domain/models.dart'; +import 'package:rippr/src/telemetry/telemetry_uploader.dart'; + +/// Ported from `com.rippr.TelemetryUploaderTest`. +/// +/// The Kotlin suite used MockWebServer plus an in-memory fake DAO. Here a `MockClient` +/// stands in for the network and a real in-memory Drift database stands in for the DAO — +/// closer to production, and still no device required. +void main() { + late AppDatabase db; + + setUp(() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + db = AppDatabase(NativeDatabase.memory()); + }); + + tearDown(() async => db.close()); + + Future seed(int count) async { + final tripId = + await db.insertTrip(const Trip(startedAt: 1000, state: TripState.recording)); + final segmentId = await db.insertSegment(tripId, 1000); + await db.insertPoints([ + for (var i = 0; i < count; i++) + TrackPoint( + tripId: tripId, + segmentId: segmentId, + timestamp: 1000 + i, + latitude: 51.0, + longitude: -114.0, + speedKmh: 40, + altitudeM: 1000, + ), + ]); + } + + TelemetryUploader uploader({ + required http.Client client, + String endpoint = 'https://example.test/ingest', + }) => + TelemetryUploader( + db: db, + endpoint: endpoint, + deviceId: 'device-abc', + client: client, + ); + + test('an empty endpoint disables upload entirely', () async { + var called = false; + final client = MockClient((_) async { + called = true; + return http.Response('', 200); + }); + + final result = await uploader(client: client, endpoint: '').uploadPending(); + + expect(result, isA()); + expect(called, isFalse, reason: 'recording must work with no server at all'); + }); + + test('a successful run marks points synced', () async { + await seed(5); + final client = MockClient((_) async => http.Response('{}', 200)); + + final result = await uploader(client: client).uploadPending(); + + expect(result, isA()); + expect((result as UploadSuccess).uploaded, 5); + expect(await db.countUnsynced(), 0); + }); + + test('a rejected batch leaves the backlog intact for retry', () async { + await seed(5); + final client = MockClient((_) async => http.Response('nope', 500)); + + final result = await uploader(client: client).uploadPending(); + + expect(result, isA()); + expect(await db.countUnsynced(), 5, + reason: 'nothing may be marked synced when the server refused it'); + }); + + test('a network error is swallowed and retried later', () async { + await seed(3); + final client = MockClient((_) async => throw http.ClientException('offline')); + + final result = await uploader(client: client).uploadPending(); + + expect(result, isA()); + expect(await db.countUnsynced(), 3); + }); + + test('the payload carries trip and segment identity per point', () async { + await seed(2); + String? captured; + final client = MockClient((req) async { + captured = req.body; + return http.Response('{}', 200); + }); + + await uploader(client: client).uploadPending(); + + final json = jsonDecode(captured!) as Map; + expect(json['device_id'], 'device-abc'); + final first = (json['points'] as List).first as Map; + // Batches are drawn by id and can straddle a boundary, so identity travels with the + // point rather than the batch. + expect(first.containsKey('trip_id'), isTrue); + expect(first.containsKey('segment_id'), isTrue); + }); + + test('a backlog larger than one batch is sent across several requests', + () async { + // Two full batches plus a remainder. + await seed(uploadBatchSize * 2 + 7); + var requests = 0; + final client = MockClient((_) async { + requests++; + return http.Response('{}', 200); + }); + + final result = await uploader(client: client).uploadPending(); + + expect(requests, 3); + expect((result as UploadSuccess).uploaded, uploadBatchSize * 2 + 7); + expect(await db.countUnsynced(), 0); + }); + + test('a failure partway through reports what did land', () async { + await seed(uploadBatchSize + 10); + var requests = 0; + final client = MockClient((_) async { + requests++; + return http.Response('{}', requests == 1 ? 200 : 503); + }); + + final result = await uploader(client: client).uploadPending(); + + expect(result, isA()); + expect((result as UploadPartial).uploaded, uploadBatchSize); + expect(await db.countUnsynced(), 10, + reason: 'the first batch landed; the rest must survive for retry'); + }); +}