Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ New table `TripDayWeather` in `lib/core/database/database.dart`:

| Column | Type | Notes |
| --- | --- | --- |
| `id` | text, pk | uuid v4 |
| `id` | text, pk | deterministic UUIDv5 over (`tripId`, day), via `tripDayWeatherRowId` |
| `tripId` | text | references `Trips(#id)` |
| `date` | int | epoch **milliseconds** at local midnight, matching what `ItineraryDayRepository` writes for `trip_itinerary_days.date` (the column comment there says "Unix timestamp", but the repository writes `millisecondsSinceEpoch`) |
| `date` | int | epoch **milliseconds** at **UTC** midnight for the calendar day, via `tripDayMillis`. Milliseconds because that is what `ItineraryDayRepository` writes for `trip_itinerary_days.date`, despite that column comment saying "Unix timestamp". UTC rather than local midnight because the value is part of the row identity: a local midnight epoch differs in every timezone, so two devices would key the same trip day differently and never converge |
| `latitude` | real | the coordinate the lookup used |
| `longitude` | real | the coordinate the lookup used |
| `airTemp` | real, nullable | celsius |
Expand All @@ -84,6 +84,15 @@ New table `TripDayWeather` in `lib/core/database/database.dart`:

Unique index on (`tripId`, `date`).

The id is **not** a v4 uuid. It is derived from the day it describes,
`UUIDv5(namespace, "$tripId|$dayMillis")`, following the same convention as
`importedDiveComputerId` and `qualityFindingId`. A per-device v4 would let two
devices store the same day under different primary keys; the serializer upserts
by primary key, so the peer's row would miss the `ON CONFLICT` target and hit
the unique index instead, throwing inside the merge transaction and aborting
the whole sync pull. The repository derives the id itself and ignores whatever
a caller passes.

The stored field set is the full `WeatherData` payload, not just the three
fields the day header renders. The API returns them in one response at no
extra cost, dive rows already store exactly this set, and a later migration to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import 'package:submersion/core/services/sync/sync_event_bus.dart';
import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'
as domain;
import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'
show tripDayMillis, tripDayWeatherRowId;
show tripDayDate, tripDayMillis, tripDayWeatherRowId;

/// Reads and writes stored per-day trip weather.
///
Expand All @@ -22,7 +22,7 @@ class TripDayWeatherRepository {
final SyncRepository _syncRepository = SyncRepository();
final _log = LoggerService.forClass(TripDayWeatherRepository);

/// Local midnight for [date], as epoch milliseconds.
/// The UTC-midnight day key for [date], as epoch milliseconds.
///
/// The day is the identity, so normalizing here is what actually enforces
/// the (trip, date) uniqueness intent. A caller that passes a DateTime with
Expand Down Expand Up @@ -50,7 +50,7 @@ class TripDayWeatherRepository {

final winners = <int, TripDayWeatherData>{};
for (final row in rows) {
final day = _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date));
final day = _dayKey(tripDayDate(row.date));
final held = winners[day];
winners[day] = held == null
? row
Expand Down Expand Up @@ -193,10 +193,12 @@ class TripDayWeatherRepository {

/// Every stored row for this trip that falls on [dayMillis]'s calendar day.
///
/// Filtered in Dart rather than SQL: local midnight is not something SQLite
/// can derive from the stored epoch millis without knowing the zone and its
/// DST history. A trip holds one row per day, so the scan is a few dozen
/// rows at most.
/// Filtered in Dart rather than SQL. It could be pushed down now that the
/// key is UTC, since the day is plain integer arithmetic on the stored
/// millis with no zone or DST to consult, but there is nothing to gain: a
/// trip holds one row per day, so the scan is a few dozen rows, and keeping
/// the rule in one Dart function is what stops it drifting from
/// [tripDayMillis].
Future<List<TripDayWeatherData>> _rowsForDay({
required String tripId,
required int dayMillis,
Expand All @@ -205,10 +207,7 @@ class TripDayWeatherRepository {
_db.tripDayWeather,
)..where((t) => t.tripId.equals(tripId))).get();
return rows
.where(
(r) =>
_dayKey(DateTime.fromMillisecondsSinceEpoch(r.date)) == dayMillis,
)
.where((r) => _dayKey(tripDayDate(r.date)) == dayMillis)
.toList();
}

Expand Down Expand Up @@ -249,9 +248,7 @@ class TripDayWeatherRepository {
// written by an older build or an out-of-date peer can still carry a
// time component, and handing that back would put time-bearing dates
// into downstream logic.
date: DateTime.fromMillisecondsSinceEpoch(
_dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)),
),
date: tripDayDate(_dayKey(tripDayDate(row.date))),
latitude: row.latitude,
longitude: row.longitude,
airTemp: row.airTemp,
Expand Down
40 changes: 36 additions & 4 deletions lib/features/trips/domain/entities/trip_day_weather.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,44 @@ import 'package:submersion/features/trips/domain/entities/trip_story_day.dart';
/// Never change: the ids already stored depend on it.
const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45';

/// Local midnight for [date], as epoch milliseconds.
/// The calendar day of [date] as epoch milliseconds at UTC midnight.
///
/// The day is the identity of a weather row, so this is what turns a
/// DateTime into one. Shared rather than reimplemented per caller: the row
/// id, the repository's storage key, and the map key reads come back under
/// all have to agree, and three copies of the same two lines would drift.
///
/// UTC, not local midnight, and the distinction is the whole point. A local
/// `DateTime(y, m, d)` has a different epoch value in every timezone, so two
/// devices looking at the same trip day would derive different keys, and
/// therefore different row ids, and never converge: each would store and
/// refetch its own copy of the day. Divers cross timezones by definition, and
/// one diver flying home is enough to trigger it.
///
/// The calendar fields are taken as given rather than converted. `toUtc()`
/// would shift a late evening onto the following day; the trip story already
/// hands over a date whose y/m/d is the day it means, matching the app's
/// wall-clock-as-UTC convention for dive timestamps.
int tripDayMillis(DateTime date) =>
DateTime(date.year, date.month, date.day).millisecondsSinceEpoch;
DateTime.utc(date.year, date.month, date.day).millisecondsSinceEpoch;

/// The instant [dayMillis] denotes, read in UTC.
///
/// The only correct way to read a stored day back. `fromMillisecondsSinceEpoch`
/// without `isUtc` returns a LOCAL DateTime, so re-extracting y/m/d from it
/// reads the calendar fields in the device's frame: on any negative UTC
/// offset, UTC midnight is the previous evening locally, and the day walks
/// backwards on every round trip.
///
/// It reads the value it is given and normalizes nothing, so it is the
/// inverse of [tripDayMillis] only for a value [tripDayMillis] produced. A
/// stored `date` is not guaranteed to be one: rows written before this branch
/// derived ids can carry a time component, and the repository deliberately
/// calls this on those raw values. Run the result back through
/// [tripDayMillis] whenever you need the normalized day rather than the
/// instant as stored.
DateTime tripDayDate(int dayMillis) =>
DateTime.fromMillisecondsSinceEpoch(dayMillis, isUtc: true);

/// Deterministic row id for one trip day.
///
Expand All @@ -27,8 +57,10 @@ int tripDayMillis(DateTime date) =>
/// target entirely and hits the index instead. That throws inside the merge
/// transaction and aborts the whole sync pull.
///
/// [dayMillis] must already be normalized to local midnight; pass it through
/// [tripDayMillis].
/// [dayMillis] must already be the normalized UTC-midnight day key; pass it
/// through [tripDayMillis]. Local midnight would defeat the purpose: its epoch
/// value differs in every timezone, so the derived id would too and the two
/// devices this exists to converge would not.
String tripDayWeatherRowId({required String tripId, required int dayMillis}) =>
const Uuid().v5(kTripDayWeatherNamespace, '$tripId|$dayMillis');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,11 @@ class _TripStoryViewState extends ConsumerState<TripStoryView>
Map<int, TripDayWeather> storedWeather,
) {
final day = story.days[index];
// Stored rows are keyed on local midnight millis.
final dayDate = DateTime(day.date.year, day.date.month, day.date.day);
final stored = storedWeather[dayDate.millisecondsSinceEpoch];
// Keyed through the same helper the repository stores under, so the
// lookup cannot drift from the write. Computing the key inline here was
// how the two came apart: it silently found nothing and every badge
// disappeared.
final stored = storedWeather[tripDayMillis(day.date)];
final showTodayDivider = todayIndex != null && index == todayIndex;
const divider = SliverPadding(
padding: EdgeInsets.symmetric(horizontal: 16),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,22 +89,23 @@ void main() {
final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(1));
final row = stored[day1.millisecondsSinceEpoch]!;
final row = stored[tripDayMillis(day1)]!;
expect(row.airTemp, 21.5);
expect(row.cloudCover, CloudCover.clear);
expect(row.windDirection, CurrentDirection.north);
expect(row.weatherCode, 0);
expect(row.weatherSource, WeatherSource.openMeteo);
expect(row.latitude, 12.16);
expect(row.date, day1);
// A calendar day in a device-independent frame, so UTC.
expect(row.date, DateTime.utc(2026, 3, 8));
});

test('a null payload field round-trips as null', () async {
await repository.upsert(sample(airTemp: null, cloudCover: null));

final row = (await repository.getForTrip(
testTripId,
))[day1.millisecondsSinceEpoch]!;
))[tripDayMillis(day1)]!;

expect(row.airTemp, isNull);
expect(row.cloudCover, isNull);
Expand All @@ -120,21 +121,21 @@ void main() {
final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(1));
expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25);
expect(stored[tripDayMillis(day1)]!.airTemp, 25);
});

test('a date with a time component is stored under local midnight', () {
test('a date with a time component is stored under UTC midnight', () {
// The repository owns the (trip, date) uniqueness invariant, so it
// normalizes rather than trusting every caller to. A row keyed on a
// stray time would be invisible to midnight-keyed lookups and would
// refetch forever.
// stray time would be invisible to day-keyed lookups and would refetch
// forever. UTC, so the key does not move with the device timezone.
return () async {
await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30)));

final stored = await repository.getForTrip(testTripId);

expect(stored.keys.single, day1.millisecondsSinceEpoch);
expect(stored[day1.millisecondsSinceEpoch]!.date, day1);
expect(stored.keys.single, tripDayMillis(day1));
expect(stored[tripDayMillis(day1)]!.date, DateTime.utc(2026, 3, 8));
}();
});

Expand All @@ -147,7 +148,7 @@ void main() {
final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(1));
expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25);
expect(stored[tripDayMillis(day1)]!.airTemp, 25);
});

test('two different days both persist', () async {
Expand All @@ -157,7 +158,7 @@ void main() {
final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(2));
expect(stored[day2.millisecondsSinceEpoch]!.airTemp, 19);
expect(stored[tripDayMillis(day2)]!.airTemp, 19);
});

test('getForTrip is scoped to one trip', () async {
Expand Down Expand Up @@ -271,7 +272,7 @@ void main() {

final rows = await allRows();
expect(rows, hasLength(1));
expect(rows.single.date, day1.millisecondsSinceEpoch);
expect(rows.single.date, tripDayMillis(day1));
expect(rows.single.airTemp, 25);
});

Expand Down Expand Up @@ -333,28 +334,31 @@ void main() {
final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(1));
expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25);
expect(stored[tripDayMillis(day1)]!.airTemp, 25);
},
);

test('getForTrip falls back to the most recently updated stray', () async {
// Off-midnight strays within the same UTC day. "Same day" is a UTC
// question now: a row stored at 23:00 local on a negative offset falls
// on the following UTC day and is genuinely a different day's row.
await insertRaw(
id: 'peer-a',
date: DateTime(2026, 3, 8, 6),
date: DateTime.utc(2026, 3, 8, 6),
airTemp: 10,
updatedAt: 100,
);
await insertRaw(
id: 'peer-b',
date: DateTime(2026, 3, 8, 23),
date: DateTime.utc(2026, 3, 8, 23),
airTemp: 20,
updatedAt: 200,
);

final stored = await repository.getForTrip(testTripId);

expect(stored, hasLength(1));
expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 20);
expect(stored[tripDayMillis(day1)]!.airTemp, 20);
});

test('getForTrip does not write while resolving strays', () async {
Expand Down
44 changes: 44 additions & 0 deletions test/features/trips/domain/entities/trip_day_weather_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,50 @@ void main() {
);
}

group('tripDayMillis', () {
test('is the calendar day in UTC, not the device local midnight', () {
// The key must not depend on where the device is standing. A local
// DateTime(y, m, d) has a different epoch value in every timezone, so
// two devices would derive different row ids for the same calendar day
// and never converge.
expect(
tripDayMillis(DateTime(2026, 3, 8, 17, 30)),
DateTime.utc(2026, 3, 8).millisecondsSinceEpoch,
);
});

test('takes the calendar fields as given, never shifting the day', () {
// Guards the wrong fix: converting with toUtc() would move a late
// evening local time onto the following calendar day.
expect(
tripDayMillis(DateTime(2026, 3, 8, 23, 59)),
tripDayMillis(DateTime.utc(2026, 3, 8, 0, 1)),
);
});

test('distinct days stay distinct', () {
expect(
tripDayMillis(DateTime(2026, 3, 8)),
isNot(tripDayMillis(DateTime(2026, 3, 9))),
);
});
});

group('tripDayWeatherRowId', () {
test('is stable for one calendar day regardless of the time given', () {
expect(
tripDayWeatherRowId(
tripId: 't1',
dayMillis: tripDayMillis(DateTime(2026, 3, 8, 1)),
),
tripDayWeatherRowId(
tripId: 't1',
dayMillis: tripDayMillis(DateTime(2026, 3, 8, 22)),
),
);
});
});

group('hasRenderableWeather', () {
test('air temperature alone counts', () {
expect(weather(airTemp: 24).hasRenderableWeather, isTrue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ void main() {
tester,
story,
tripDayWeather: {
surfaceDate.millisecondsSinceEpoch: TripDayWeather(
tripDayMillis(surfaceDate): TripDayWeather(
id: 'w1',
tripId: trip.id,
date: surfaceDate,
Expand Down