diff --git a/docs/port/PROGRESS.md b/docs/port/PROGRESS.md index f558f68..3ef4248 100644 --- a/docs/port/PROGRESS.md +++ b/docs/port/PROGRESS.md @@ -317,3 +317,54 @@ well under a second with nothing booted. `TripRepositoryTest` and `MergeTest` (4 lines) should convert the same way in T09. **95 tests passing, analyze clean.** + +--- + +## T09 — Trip repository · **complete** + +`lib/src/data/trip_repository.dart`. **26 tests, green on the first run.** + +Every lifecycle transition ported: `startTrip` (adopting, never duplicating), `pauseTrip`, +`resumeTrip`, `completeTrip`, `discardTrip`, `renameTrip`, `deleteTrip`, `mergeTrips`, +`recomputeAggregates`. Each runs inside `_db.transaction` and each is idempotent, because +the platform can restart the recorder from any state. + +`Room.withTransaction` maps onto Drift's `transaction()` almost exactly, so this was the +most mechanical port so far. + +### What the tests protect + +- **Adoption over rejection.** `startTrip` on an already-active trip returns the *same* + handle rather than opening a second trip — the behaviour that makes a process kill + survivable. +- **Merge never joins segments.** The boundary between two merged rides stays a segment + boundary, exactly like a pause. The regression test puts the two rides a degree of + latitude apart and asserts the ~111 km gap never reaches `distanceM`. +- **Aggregates are recomputed, not summed** — because distance is not additive across + that gap. +- **Rename collapses empty and whitespace-only input to null**, so a stored `""` can + never diverge from the UI's date-label branch. +- **Merge rejects** self-merge, an active trip, and a missing id, and leaves no orphans. + +### One piece of speculative code removed + +A `mergeTableUpdates` helper was written to nudge Drift's stream queries after +re-parenting, then deleted before commit: Drift's own `update()` already notifies +dependent streams, so it earned nothing and would have been misleading scaffolding. + +### The instrumented-to-unit win, totalled + +`TripRepositoryTest` + `MergeTest` + `SchemaTest` were **666 lines of instrumented tests +requiring a booted emulator**. All three are now plain unit tests finishing in about two +seconds with nothing running. + +--- + +## Phase 2 complete + +**121 tests passing, `flutter analyze` clean.** The data layer is done and the domain, +statistics and export layers above it are proven equivalent to the Kotlin original. + +Next: **Phase 3, the recording engine** — the risky phase. T10's `LocationSource` seam +first, then the pipeline, then the two platform liveness stories. The +`flutter_foreground_task` deprecation warnings recorded under T01 become relevant at T12. diff --git a/lib/src/data/trip_repository.dart b/lib/src/data/trip_repository.dart new file mode 100644 index 0000000..df8c9f5 --- /dev/null +++ b/lib/src/data/trip_repository.dart @@ -0,0 +1,243 @@ +/// Owns every trip lifecycle transition. +/// +/// Ported from `com.rippr.data.TripRepository`. +/// +/// Recording state lives in the database rather than in memory. A row in `trips` with +/// `endedAt IS NULL` *is* the fact of an in-progress ride, so it survives process death — +/// which an in-memory flag could not. On Android the OS can restart the process with no +/// intent, and on iOS the app can be suspended and resumed; in both cases a flag would +/// come back `false` while a ride was genuinely underway. +/// +/// Every transition is idempotent and safe to call from an unexpected state. The platform +/// can restart the recorder at any moment, so calling resume on an already-recording trip +/// must be a no-op rather than opening a duplicate segment. +library; + +import '../domain/models.dart'; +import '../stats/ride_statistics.dart'; +import 'database.dart'; + +/// The ids the recorder stamps onto each fix. +class TripHandle { + const TripHandle(this.tripId, this.segmentId); + + final int tripId; + final int segmentId; + + @override + bool operator ==(Object other) => + other is TripHandle && + other.tripId == tripId && + other.segmentId == segmentId; + + @override + int get hashCode => Object.hash(tripId, segmentId); + + @override + String toString() => 'TripHandle(trip: $tripId, segment: $segmentId)'; +} + +class TripRepository { + TripRepository(this._db); + + final AppDatabase _db; + + AppDatabase get db => _db; + + // --- Reads --------------------------------------------------------------- + + Stream watchActiveTrip() => _db.watchActiveTrip(); + + Stream watchTrip(int id) => _db.watchTrip(id); + + Stream> watchCompletedTrips() => _db.watchCompletedTrips(); + + Future activeTrip() => _db.getActiveTrip(); + + Future tripById(int id) => _db.getTrip(id); + + Future> pointsForTrip(int id) => _db.pointsForTrip(id); + + Future> segmentsForTrip(int id) => _db.segmentsForTrip(id); + + Stream watchTripStats(int id) => _db.watchTripStats(id); + + /// The segment currently being written into, if a ride is active and not paused. + Future openSegment() async { + final trip = await _db.getActiveTrip(); + if (trip == null) return null; + return _db.openSegment(trip.id); + } + + // --- Lifecycle ----------------------------------------------------------- + + /// Starts a ride, or adopts one already in progress. + /// + /// Adoption rather than rejection is deliberate: after a process kill the trip row + /// still exists, and the restarted recorder needs to continue it, not start a second. + Future startTrip(int now) => _db.transaction(() async { + final existing = await _db.getActiveTrip(); + if (existing != null) { + return _adoptOrOpenSegment(existing, now); + } + final tripId = await _db + .insertTrip(Trip(startedAt: now, state: TripState.recording)); + final segmentId = await _db.insertSegment(tripId, now); + return TripHandle(tripId, segmentId); + }); + + /// Closes the open segment and marks the trip paused. The trip itself stays open — + /// only [completeTrip] sets `endedAt`. + Future pauseTrip(int now) => _db.transaction(() async { + final trip = await _db.getActiveTrip(); + if (trip == null) return false; + if (trip.state == TripState.paused) return true; + + final open = await _db.openSegment(trip.id); + if (open != null) await _db.closeSegment(open.id, now); + await _db.setTripState(trip.id, TripState.paused); + return true; + }); + + /// Opens a fresh segment so the pause leaves a real gap in the recorded path. + Future resumeTrip(int now) => _db.transaction(() async { + final trip = await _db.getActiveTrip(); + if (trip == null) return null; + final handle = await _adoptOrOpenSegment(trip, now); + await _db.setTripState(trip.id, TripState.recording); + return handle; + }); + + Future completeTrip(int now) => _db.transaction(() async { + final trip = await _db.getActiveTrip(); + if (trip == null) return null; + final open = await _db.openSegment(trip.id); + if (open != null) await _db.closeSegment(open.id, now); + await _db.closeTrip(trip.id, now, state: TripState.completed); + return trip.id; + }); + + /// Deletes the active trip outright. Segments and points follow via CASCADE. + Future discardTrip() => _db.transaction(() async { + final trip = await _db.getActiveTrip(); + if (trip == null) return false; + await _db.deleteTrip(trip.id); + return true; + }); + + // --- Management ---------------------------------------------------------- + + Future renameTrip(int id, String? name) { + // Empty input must collapse to null, or the UI's "derive a label from the date" + // branch and a stored "" would diverge. + final trimmed = name?.trim(); + return _db.renameTrip(id, (trimmed == null || trimmed.isEmpty) ? null : trimmed); + } + + Future deleteTrip(int id) => _db.deleteTrip(id); + + /// Combines two completed rides into one. + /// + /// The earlier trip survives, the later one's segments and points are re-parented onto + /// it, and the later row is deleted. Segments are **never joined** — the boundary + /// between the two rides becomes a segment boundary exactly like a pause, which is + /// correct: the rider genuinely was not recording in between, and joining them would + /// draw a straight line across the gap. + /// + /// Aggregates are recomputed from scratch rather than summed, because distance is not + /// additive across that gap. + /// + /// Returns the surviving trip id, or null if the merge was rejected. + Future mergeTrips(int a, int b) => _db.transaction(() async { + if (a == b) return null; + final first = await _db.getTrip(a); + if (first == null) return null; + final second = await _db.getTrip(b); + if (second == null) return null; + if (first.isActive || second.isActive) { + // Refusing to merge an active trip. + return null; + } + + // Selection order is not ride order. + final (survivor, absorbed) = first.startedAt <= second.startedAt + ? (first, second) + : (second, first); + + await _db.reparentSegments(absorbed.id, survivor.id); + await _db.reparentPoints(absorbed.id, survivor.id); + await _db.deleteTrip(absorbed.id); + + final endedAt = (survivor.endedAt ?? 0) > (absorbed.endedAt ?? 0) + ? (survivor.endedAt ?? 0) + : (absorbed.endedAt ?? 0); + await _db.closeTrip(survivor.id, endedAt, state: TripState.completed); + if (survivor.name == null && absorbed.name != null) { + await _db.renameTrip(survivor.id, absorbed.name); + } + + await recomputeAggregates(survivor.id); + return survivor.id; + }); + + /// Recomputes a trip's stored totals from the points it actually owns. + /// + /// This is the authoritative pass. The recorder's live accumulation is an estimate, so + /// running this on completion means a mid-ride process kill cannot leave permanently + /// skewed totals. + Future recomputeAggregates(int tripId) async { + final summary = computeSummary( + await _db.pointsForTrip(tripId), + segments: await _db.segmentsForTrip(tripId), + ); + await _db.updateAggregates( + id: tripId, + distanceM: summary.distanceM, + movingMillis: summary.movingMillis, + maxSpeedKmh: summary.maxSpeedKmh, + elevationGainM: summary.elevationGainM, + pointCount: summary.pointCount, + ); + } + + // --- Writes from the recorder -------------------------------------------- + + /// Appends a batch of fixes. + /// + /// Points arrive already stamped with their trip and segment ids — see the recorder. + /// They are **never** looked up at write time, so a fix still in flight when a pause + /// happens lands in the segment it actually belongs to. + Future appendPoints(List points) async { + if (points.isEmpty) return; + await _db.insertPoints(points); + } + + Future lastPointInSegment(int segmentId) => + _db.lastInSegment(segmentId); + + Future persistAggregates({ + required int tripId, + required double distanceM, + required int movingMillis, + required double maxSpeedKmh, + required double elevationGainM, + required int pointCount, + }) => + _db.updateAggregates( + id: tripId, + distanceM: distanceM, + movingMillis: movingMillis, + maxSpeedKmh: maxSpeedKmh, + elevationGainM: elevationGainM, + pointCount: pointCount, + ); + + // --- Internals ----------------------------------------------------------- + + Future _adoptOrOpenSegment(Trip trip, int now) async { + final existing = await _db.openSegment(trip.id); + if (existing != null) return TripHandle(trip.id, existing.id); + final segmentId = await _db.insertSegment(trip.id, now); + return TripHandle(trip.id, segmentId); + } +} diff --git a/test/trip_repository_test.dart b/test/trip_repository_test.dart new file mode 100644 index 0000000..727c69f --- /dev/null +++ b/test/trip_repository_test.dart @@ -0,0 +1,352 @@ +import 'package:drift/drift.dart' show driftRuntimeOptions; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:rippr/src/data/database.dart'; +import 'package:rippr/src/data/trip_repository.dart'; +import 'package:rippr/src/domain/models.dart'; + +/// Ported from `com.rippr.data.TripRepositoryTest` and `MergeTest`. +/// +/// Both were **instrumented** suites needing a device and a booted emulator. Against +/// Drift on the Dart VM they are ordinary unit tests. +/// +/// In-memory databases only — the native suite once wiped a real device's rides by +/// running against the production singleton. +void main() { + late AppDatabase db; + late TripRepository repo; + + setUp(() { + driftRuntimeOptions.dontWarnAboutMultipleDatabases = true; + db = AppDatabase(NativeDatabase.memory()); + repo = TripRepository(db); + }); + + tearDown(() async => db.close()); + + Future addPoint(TripHandle handle, int ts, + {double lat = 51.0, double speed = 40.0, double alt = 1000.0}) => + repo.appendPoints([ + TrackPoint( + tripId: handle.tripId, + segmentId: handle.segmentId, + timestamp: ts, + latitude: lat, + longitude: -114.0, + speedKmh: speed, + altitudeM: alt, + ), + ]); + + group('lifecycle', () { + test('full lifecycle produces one trip and two segments', () async { + final h1 = await repo.startTrip(1000); + await addPoint(h1, 1000); + await addPoint(h1, 2000); + + await repo.pauseTrip(3000); + final h2 = (await repo.resumeTrip(4000))!; + await addPoint(h2, 5000); + + final tripId = await repo.completeTrip(6000); + + expect(tripId, h1.tripId); + expect(h2.segmentId, isNot(h1.segmentId), + reason: 'resume must open a fresh segment'); + + final segments = await repo.segmentsForTrip(h1.tripId); + expect(segments.length, 2); + expect(segments.every((s) => !s.isOpen), isTrue, + reason: 'completing must close the open segment'); + expect((await repo.tripById(h1.tripId))!.state, TripState.completed); + expect(await repo.activeTrip(), isNull); + }); + + test('pause closes the segment but leaves the trip open', () async { + final h = await repo.startTrip(1000); + await repo.pauseTrip(2000); + + final trip = (await repo.activeTrip())!; + expect(trip.state, TripState.paused); + expect(trip.endedAt, isNull, reason: 'pause must not end the trip'); + expect(await db.openSegment(h.tripId), isNull); + }); + + test('startTrip adopts an already active trip instead of creating a second', + () async { + final first = await repo.startTrip(1000); + final second = await repo.startTrip(2000); + + expect(second.tripId, first.tripId); + expect(second.segmentId, first.segmentId); + expect(await db.countTrips(), 1); + }); + + test('resume while already recording is a no-op', () async { + final h = await repo.startTrip(1000); + final resumed = await repo.resumeTrip(2000); + + expect(resumed!.segmentId, h.segmentId, + reason: 'must not open a duplicate segment'); + expect(await db.countSegmentsForTrip(h.tripId), 1); + }); + + test('double pause is a no-op', () async { + final h = await repo.startTrip(1000); + expect(await repo.pauseTrip(2000), isTrue); + expect(await repo.pauseTrip(3000), isTrue); + expect(await db.countSegmentsForTrip(h.tripId), 1); + }); + + test('transitions with no active trip are safe no-ops', () async { + expect(await repo.pauseTrip(1000), isFalse); + expect(await repo.resumeTrip(1000), isNull); + expect(await repo.completeTrip(1000), isNull); + expect(await repo.discardTrip(), isFalse); + }); + + test('start after complete begins a fresh trip', () async { + final first = await repo.startTrip(1000); + await repo.completeTrip(2000); + final second = await repo.startTrip(3000); + + expect(second.tripId, isNot(first.tripId)); + expect(await db.countTrips(), 2); + }); + }); + + group('discard', () { + test('discard removes the trip and all its points', () async { + final h = await repo.startTrip(1000); + await addPoint(h, 1000); + await addPoint(h, 2000); + + expect(await repo.discardTrip(), isTrue); + + expect(await repo.tripById(h.tripId), isNull); + expect(await db.countPointsForTrip(h.tripId), 0); + expect(await db.countSegmentsForTrip(h.tripId), 0); + }); + + test('discard leaves earlier completed trips alone', () async { + final keep = await repo.startTrip(1000); + await addPoint(keep, 1000); + await repo.completeTrip(2000); + + final throwaway = await repo.startTrip(3000); + await addPoint(throwaway, 3000); + await repo.discardTrip(); + + expect(await repo.tripById(keep.tripId), isNotNull); + expect(await db.countPointsForTrip(keep.tripId), 1); + expect(await db.countTrips(), 1); + }); + }); + + group('streams', () { + test('active trip stream tracks the lifecycle', () async { + expect(await repo.watchActiveTrip().first, isNull); + + final h = await repo.startTrip(1000); + expect((await repo.watchActiveTrip().first)?.id, h.tripId); + + await repo.completeTrip(2000); + expect(await repo.watchActiveTrip().first, isNull); + }); + + test('completed trips stream excludes the active trip', () async { + final done = await repo.startTrip(1000); + await repo.completeTrip(2000); + await repo.startTrip(3000); + + final completed = await repo.watchCompletedTrips().first; + expect(completed.length, 1); + expect(completed.single.id, done.tripId); + }); + }); + + group('rename', () { + test('rename stores null rather than an empty string', () async { + final h = await repo.startTrip(1000); + + await repo.renameTrip(h.tripId, 'Morning loop'); + expect((await repo.tripById(h.tripId))!.name, 'Morning loop'); + + await repo.renameTrip(h.tripId, ''); + expect((await repo.tripById(h.tripId))!.name, isNull, + reason: 'an empty string would diverge from the date-label branch'); + + await repo.renameTrip(h.tripId, ' '); + expect((await repo.tripById(h.tripId))!.name, isNull); + }); + + test('rename trims surrounding whitespace', () async { + final h = await repo.startTrip(1000); + await repo.renameTrip(h.tripId, ' Sunday blast '); + expect((await repo.tripById(h.tripId))!.name, 'Sunday blast'); + }); + }); + + group('process death', () { + test('a fresh repository over the same database sees the active trip', + () async { + final h = await repo.startTrip(1000); + await addPoint(h, 1000); + + // Simulates the process being killed and rebuilt: new repository, same file. + final revived = TripRepository(db); + + final active = await revived.activeTrip(); + expect(active, isNotNull, + reason: 'recording state must survive process death'); + expect(active!.id, h.tripId); + }); + + test('restart after pause resumes into a new segment', () async { + final h = await repo.startTrip(1000); + await repo.pauseTrip(2000); + + final revived = TripRepository(db); + final resumed = (await revived.resumeTrip(3000))!; + + expect(resumed.tripId, h.tripId); + expect(resumed.segmentId, isNot(h.segmentId)); + expect((await revived.activeTrip())!.state, TripState.recording); + }); + }); + + // --------------------------------------------------------------------------- + // Merge — ported from MergeTest + // --------------------------------------------------------------------------- + + group('merge', () { + Future completedTrip({ + required int startedAt, + required int endedAt, + int points = 3, + String? name, + double lat = 51.0, + }) async { + final h = await repo.startTrip(startedAt); + for (var i = 0; i < points; i++) { + await addPoint(h, startedAt + i * 1000, lat: lat + i * 0.0001); + } + if (name != null) await repo.renameTrip(h.tripId, name); + await repo.completeTrip(endedAt); + return h.tripId; + } + + test('merge reparents everything and deletes the absorbed row', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + final b = await completedTrip(startedAt: 10000, endedAt: 15000); + + final survivor = await repo.mergeTrips(a, b); + + expect(survivor, a); + expect(await repo.tripById(b), isNull); + expect(await db.countPointsForTrip(a), 6); + expect(await db.countSegmentsForTrip(a), 2); + expect(await db.countTrips(), 1); + }); + + test('selection order does not decide the survivor', () async { + final earlier = await completedTrip(startedAt: 1000, endedAt: 5000); + final later = await completedTrip(startedAt: 10000, endedAt: 15000); + + // Passed later-first on purpose. + expect(await repo.mergeTrips(later, earlier), earlier); + }); + + test('merged aggregates are recomputed, not summed', () async { + // The two rides are a degree of latitude apart — ~111 km. If aggregates were + // summed, or if the segments were joined, that gap would appear as distance. + final a = await completedTrip(startedAt: 1000, endedAt: 5000, lat: 51.0); + final b = + await completedTrip(startedAt: 10000, endedAt: 15000, lat: 52.0); + + await repo.mergeTrips(a, b); + + final merged = (await repo.tripById(a))!; + expect(merged.pointCount, 6); + expect(merged.distanceM, lessThan(200.0), + reason: + 'the ~111 km gap leaked into distance: ${merged.distanceM} m'); + }); + + test('the join remains a segment boundary', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + final b = await completedTrip(startedAt: 10000, endedAt: 15000); + + await repo.mergeTrips(a, b); + + final segments = await repo.segmentsForTrip(a); + expect(segments.length, 2, + reason: 'segments must never be joined by a merge'); + }); + + test('endedAt becomes the later of the two', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + final b = await completedTrip(startedAt: 10000, endedAt: 15000); + + await repo.mergeTrips(a, b); + + expect((await repo.tripById(a))!.endedAt, 15000); + }); + + test('an unnamed survivor inherits the other name', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + final b = + await completedTrip(startedAt: 10000, endedAt: 15000, name: 'Part two'); + + await repo.mergeTrips(a, b); + + expect((await repo.tripById(a))!.name, 'Part two'); + }); + + test('an existing survivor name is kept', () async { + final a = await completedTrip( + startedAt: 1000, endedAt: 5000, name: 'The good one'); + final b = + await completedTrip(startedAt: 10000, endedAt: 15000, name: 'Part two'); + + await repo.mergeTrips(a, b); + + expect((await repo.tripById(a))!.name, 'The good one'); + }); + + test('merging a trip with itself is rejected', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + expect(await repo.mergeTrips(a, a), isNull); + expect(await db.countPointsForTrip(a), 3); + }); + + test('merging an active trip is rejected', () async { + final done = await completedTrip(startedAt: 1000, endedAt: 5000); + final active = await repo.startTrip(10000); + + expect(await repo.mergeTrips(done, active.tripId), isNull); + expect(await db.countTrips(), 2, reason: 'nothing may be deleted'); + }); + + test('merging a missing trip is rejected', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + expect(await repo.mergeTrips(a, 9999), isNull); + expect(await repo.tripById(a), isNotNull); + }); + + test('merge is atomic and leaves no orphans', () async { + final a = await completedTrip(startedAt: 1000, endedAt: 5000); + final b = await completedTrip(startedAt: 10000, endedAt: 15000); + + await repo.mergeTrips(a, b); + + // Every surviving point and segment must belong to the survivor. + final allPoints = await db.allPoints(); + expect(allPoints.every((p) => p.tripId == a), isTrue); + expect(allPoints.length, 6); + + final segments = await repo.segmentsForTrip(a); + expect(segments.every((s) => s.tripId == a), isTrue); + }); + }); +}