Skip to content

Commit 524ffc0

Browse files
committed
fix(macdive-sqlite): address PR #256 review (UID width, reader guards, UUID scoping)
- bplist: UID now reads 1-4 bytes big-endian per spec (low nibble = byteCount - 1); regression tests added for 1/2/4-byte indexes and the BPlistUID docstring now matches the decoder. - MacDive DB reader: wrap optional-table reads (_readBuddies / _readTags / _readGear / _readTanks) in _selectOrEmpty so missing tables yield empty collections, matching the docstring; docstring tightened to call out the required/optional split explicitly. - dive_repository.getSourceUuidByDiveId: optional diverId parameter that joins to dives.diver_id, matching the scope the universal_adapter already uses for existingDives. 4 targeted tests. - CHANGELOG: rewrote the MacDive SQLite entry to describe only what actually ships (dives/sites/buddies/tags/gear plus per-dive tank/gas linkage); added a Known Limitations bullet for the critters/events/service-records/certifications gap. Inline code comments in the mapper document why dive dateTime follows M2's absolute-UTC convention and why diveToGearPks is collected-but-not- emitted, both tracked as cross-parser follow-up work. - real-sample test: replaced hard-coded dev path with MACDIVE_SQLITE_REAL_SAMPLE_PATH env var; group-level skip with a clear reason when unset; StateError guard for --run-skipped. - synthetic fixture: ZTANKANDGAS UUIDs renamed tag-uuid-* to tankandgas-uuid-*. - Regenerated 9 mock files for the new diverId parameter.
1 parent 2c44857 commit 524ffc0

20 files changed

Lines changed: 271 additions & 70 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ All notable changes to Submersion are documented in this file.
2020
- **MacDive (XML) source override** in the import wizard's detected-source
2121
dropdown, alongside the existing MacDive (CSV) option.
2222
- **MacDive SQLite import.** Direct import from MacDive's Core Data
23-
SQLite database the most complete metadata path. Captures dives,
24-
sites, buddies, tags (as with XML), gear inventory, critters/marine
25-
life sightings, dive events, service records, and certifications.
26-
Cross-format deduplication via source UUIDs: if you've already
27-
imported the same dives via MacDive UDDF or XML, re-importing from
28-
SQLite won't create duplicates.
23+
SQLite database. Captures the same entity set as MacDive XML — dives,
24+
sites, buddies, tags, and gear inventory — plus per-dive tank and gas
25+
mix linkage drawn from the `ZTANKANDGAS` join table. Cross-format
26+
deduplication via source UUIDs: if you've already imported the same
27+
dives via MacDive UDDF or XML, re-importing from SQLite won't create
28+
duplicates.
2929
- **MacDive (SQLite) source override** in the import wizard's
3030
detected-source dropdown, alongside MacDive (CSV) and MacDive (XML).
3131
- Cross-format import deduplication: stable per-dive UUIDs from MacDive,
@@ -59,6 +59,13 @@ All notable changes to Submersion are documented in this file.
5959
and isn't standard bplist or any common compression. Users who
6060
need time-series profile data should use the MacDive UDDF import
6161
path instead, which decodes MacDive's UDDF profile correctly.
62+
- The MacDive SQLite path reads critters (marine-life sightings),
63+
dive events, service records, and certifications into its typed
64+
row graph, but these are not yet emitted into the import payload —
65+
the unified importer doesn't have entity types for critters,
66+
events, or service records, and certification emission is scoped
67+
for a follow-up. For now, only the dive/site/buddy/tag/gear subset
68+
is persisted.
6269

6370

6471
## 1.4.6 (2026-04-22)

lib/core/utils/bplist/bplist_decoder.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,9 @@ class BPlistDecoder {
122122
}
123123
return BPlistArray(refs.map(_readObject).toList(growable: false));
124124

125-
case 0x8: // UID — CFKeyedArchiver reference (always 1 byte)
126-
return BPlistUID(_bytes[offset + 1]);
125+
case 0x8: // UID — CFKeyedArchiver reference (1..4 bytes; info encodes byteCount - 1)
126+
final byteCount = info + 1;
127+
return BPlistUID(_readBigEndianInt(_bytes, offset + 1, byteCount));
127128

128129
case 0xD: // dict
129130
final li = _readLenAndStart(offset, info);

lib/core/utils/bplist/bplist_object.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,11 @@ class BPlistDict extends BPlistObject {
5959
const BPlistDict(this.value);
6060
}
6161

62-
/// CFKeyedArchiver UID: a 1-byte reference to another object in the
63-
/// archive's object table. Used in NSKeyedArchiver-encoded Core Data
64-
/// blobs to represent cross-references between objects.
62+
/// CFKeyedArchiver UID: a big-endian reference to another object in the
63+
/// archive's object table. The bplist marker's low nibble encodes
64+
/// `byteCount - 1`, so the index field is 1..4 bytes wide. Used in
65+
/// NSKeyedArchiver-encoded Core Data blobs to represent cross-references
66+
/// between objects.
6567
class BPlistUID extends BPlistObject {
6668
final int index;
6769
const BPlistUID(this.index);

lib/features/dive_log/data/repositories/dive_repository_impl.dart

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3561,21 +3561,35 @@ class DiveRepository {
35613561
/// has multiple data sources (multi-computer), the primary row's UUID wins;
35623562
/// otherwise the most recently created row's UUID is used. Dives with no
35633563
/// source_uuid on any row are absent from the map.
3564-
Future<Map<String, String>> getSourceUuidByDiveId() async {
3564+
///
3565+
/// When [diverId] is provided, the result is restricted to that diver's
3566+
/// dives — callers that have already scoped `existingDives` to a single
3567+
/// diver should pass it here so the UUID map shares the same scope.
3568+
Future<Map<String, String>> getSourceUuidByDiveId({String? diverId}) async {
35653569
try {
3566-
final query = _db.select(_db.diveDataSources)
3567-
..where((t) => t.sourceUuid.isNotNull())
3568-
..orderBy([
3569-
(t) => OrderingTerm.desc(t.isPrimary),
3570-
(t) => OrderingTerm.desc(t.createdAt),
3571-
]);
3572-
final rows = await query.get();
3570+
final sql = StringBuffer('SELECT s.dive_id, s.source_uuid ')
3571+
..write('FROM dive_data_sources s ');
3572+
final variables = <Variable<Object>>[];
3573+
if (diverId != null) {
3574+
sql.write('INNER JOIN dives d ON d.id = s.dive_id ');
3575+
}
3576+
sql.write('WHERE s.source_uuid IS NOT NULL ');
3577+
if (diverId != null) {
3578+
sql.write('AND d.diver_id = ? ');
3579+
variables.add(Variable<Object>(diverId));
3580+
}
3581+
sql.write('ORDER BY s.is_primary DESC, s.created_at DESC');
3582+
3583+
final rows = await _db
3584+
.customSelect(sql.toString(), variables: variables)
3585+
.get();
35733586
final result = <String, String>{};
35743587
for (final row in rows) {
3575-
final uuid = row.sourceUuid;
3588+
final diveId = row.read<String>('dive_id');
3589+
final uuid = row.read<String?>('source_uuid');
35763590
if (uuid == null || uuid.isEmpty) continue;
35773591
// First row wins (ordered by isPrimary DESC then createdAt DESC).
3578-
result.putIfAbsent(row.diveId, () => uuid);
3592+
result.putIfAbsent(diveId, () => uuid);
35793593
}
35803594
return result;
35813595
} catch (e, stackTrace) {

lib/features/import_wizard/data/adapters/universal_adapter.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,9 @@ class UniversalAdapter implements ImportSourceAdapter {
296296
final existingDiveTypes = await _ref.refresh(diveTypesProvider.future);
297297
final diveRepo = _ref.read(diveRepositoryProvider);
298298
final existingDives = await diveRepo.getAllDives(diverId: diverId);
299-
final existingSourceUuidByDiveId = await diveRepo.getSourceUuidByDiveId();
299+
final existingSourceUuidByDiveId = await diveRepo.getSourceUuidByDiveId(
300+
diverId: diverId,
301+
);
300302

301303
final dupResult = checker.check(
302304
payload: payload,

lib/features/universal_import/data/services/macdive_db_reader.dart

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@ class MacDiveDbReader {
4444
}
4545

4646
/// Reads every MacDive table relevant to the import pipeline and
47-
/// returns a fully populated [MacDiveRawLogbook]. Missing/unpopulated
48-
/// tables (e.g. `ZCRITTER` when the user logs no marine-life
49-
/// sightings) produce empty collections rather than errors.
47+
/// returns a fully populated [MacDiveRawLogbook]. The four tables in
48+
/// [_requiredTables] are validated up-front and must exist. For every
49+
/// other table (e.g. `ZCRITTER`, `ZBUDDY`, `ZGEARITEM`), a missing or
50+
/// unpopulated table produces an empty collection rather than an
51+
/// error — some MacDive schema versions omit tables the user has
52+
/// never touched.
5053
static Future<MacDiveRawLogbook> readAll(Uint8List bytes) async {
5154
final tmpPath = _tmpPath();
5255
final tmpFile = File(tmpPath);
@@ -155,28 +158,34 @@ class MacDiveDbReader {
155158
}
156159

157160
static List<MacDiveRawBuddy> _readBuddies(Database db) {
158-
return db.select('SELECT Z_PK, ZUUID, ZNAME FROM ZBUDDY').map((r) {
159-
return MacDiveRawBuddy(
161+
return _selectOrEmpty<MacDiveRawBuddy>(
162+
db,
163+
'SELECT Z_PK, ZUUID, ZNAME FROM ZBUDDY',
164+
(r) => MacDiveRawBuddy(
160165
pk: r['Z_PK'] as int,
161166
uuid: _str(r['ZUUID']) ?? '',
162167
name: _str(r['ZNAME']),
163-
);
164-
}).toList();
168+
),
169+
);
165170
}
166171

167172
static List<MacDiveRawTag> _readTags(Database db) {
168-
return db.select('SELECT Z_PK, ZUUID, ZNAME FROM ZTAG').map((r) {
169-
return MacDiveRawTag(
173+
return _selectOrEmpty<MacDiveRawTag>(
174+
db,
175+
'SELECT Z_PK, ZUUID, ZNAME FROM ZTAG',
176+
(r) => MacDiveRawTag(
170177
pk: r['Z_PK'] as int,
171178
uuid: _str(r['ZUUID']) ?? '',
172179
name: _str(r['ZNAME']),
173-
);
174-
}).toList();
180+
),
181+
);
175182
}
176183

177184
static List<MacDiveRawGear> _readGear(Database db) {
178-
return db.select('SELECT * FROM ZGEARITEM').map((r) {
179-
return MacDiveRawGear(
185+
return _selectOrEmpty<MacDiveRawGear>(
186+
db,
187+
'SELECT * FROM ZGEARITEM',
188+
(r) => MacDiveRawGear(
180189
pk: r['Z_PK'] as int,
181190
uuid: _str(r['ZUUID']) ?? '',
182191
name: _str(r['ZNAME']),
@@ -191,21 +200,23 @@ class MacDiveDbReader {
191200
notes: _str(r['ZNOTES']),
192201
url: _str(r['ZURL']),
193202
warranty: _str(r['ZWARRANTY']),
194-
);
195-
}).toList();
203+
),
204+
);
196205
}
197206

198207
static List<MacDiveRawTank> _readTanks(Database db) {
199-
return db.select('SELECT * FROM ZTANK').map((r) {
200-
return MacDiveRawTank(
208+
return _selectOrEmpty<MacDiveRawTank>(
209+
db,
210+
'SELECT * FROM ZTANK',
211+
(r) => MacDiveRawTank(
201212
pk: r['Z_PK'] as int,
202213
uuid: _str(r['ZUUID']) ?? '',
203214
name: _str(r['ZNAME']),
204215
size: _double(r['ZSIZE']),
205216
workingPressure: _double(r['ZWORKINGPRESSURE']),
206217
type: _str(r['ZTYPE']),
207-
);
208-
}).toList();
218+
),
219+
);
209220
}
210221

211222
static List<MacDiveRawGas> _readGases(Database db) {

lib/features/universal_import/data/services/macdive_dive_mapper.dart

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ class MacDiveDiveMapper {
159159

160160
if (d.uuid.isNotEmpty) map['sourceUuid'] = d.uuid;
161161
if (d.identifier != null) map['sourceIdentifier'] = d.identifier;
162+
// `rawDate` is an absolute UTC DateTime derived from ZRAWDATE (NSDate
163+
// reference seconds). MacDive stores the per-dive zone separately in
164+
// `ZTIMEZONE` as an NSKeyedArchiver-encoded NSTimeZone. Emitting
165+
// rawDate directly matches M2 (`macdive_xml_parser.dart`) and reads
166+
// back correctly as long as the diver views the dive from the same
167+
// zone in which they dove. A cross-parser move to the wall-time-as-UTC
168+
// convention (cf. `subsurface_xml_parser.dart`) requires NSTimeZone
169+
// extraction for M3 and structured-date emission for M1/M2 — tracked
170+
// as follow-up work, not folded into this PR.
162171
if (d.rawDate != null) map['dateTime'] = d.rawDate;
163172
if (d.diveNumber != null) map['diveNumber'] = d.diveNumber;
164173
if (d.repetitiveDiveNumber != null) {
@@ -245,6 +254,14 @@ class MacDiveDiveMapper {
245254
];
246255
if (buddyNames.isNotEmpty) map['unmatchedBuddyNames'] = buddyNames;
247256

257+
// Per-dive gear linkage (`logbook.diveToGearPks`) is read but not
258+
// emitted: the unified import payload currently has no dive→gear
259+
// reference convention (`equipmentRefs` isn't recognised by the
260+
// entity importer), and M2 behaves identically — gear is imported
261+
// as standalone equipment entities dedup'd across all dives. Wiring
262+
// dive↔gear links end-to-end is a cross-parser change tracked as
263+
// follow-up work.
264+
248265
// Tags - emit names under `tagRefs`.
249266
final tagPks = logbook.diveToTagPks[d.pk] ?? const <int>[];
250267
final tagNames = <String>[

test/core/utils/bplist/bplist_decoder_test.dart

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,54 @@ void main() {
113113
});
114114
});
115115

116+
group('BPlistDecoder — multi-byte UID markers', () {
117+
// Synthetic bplist: root = UID(index). The decoder must honor the
118+
// bplist spec's `byteCount = (marker & 0x0F) + 1` rule so archives
119+
// with > 255 objects (e.g. large NSKeyedArchiver graphs) decode
120+
// correctly. Regression test for the 1-byte-only bug.
121+
Uint8List buildBplistWithRootUid(List<int> uidBytes) {
122+
final marker = 0x80 | (uidBytes.length - 1);
123+
final body = <int>[marker, ...uidBytes];
124+
const bodyOffset = 8;
125+
final offsetTableOffset = bodyOffset + body.length;
126+
const numObjects = 1;
127+
const topObject = 0;
128+
final trailer = <int>[
129+
0, 0, 0, 0, 0, 0, // unused / sortVersion
130+
1, // offsetIntSize
131+
1, // objectRefSize
132+
0, 0, 0, 0, 0, 0, 0, numObjects, // numObjects (u64 BE)
133+
0, 0, 0, 0, 0, 0, 0, topObject, // topObject (u64 BE)
134+
0, 0, 0, 0, 0, 0, 0, offsetTableOffset, // offsetTableOffset (u64 BE)
135+
];
136+
return Uint8List.fromList([
137+
0x62, 0x70, 0x6C, 0x69, 0x73, 0x74, 0x30, 0x30, // "bplist00"
138+
...body,
139+
bodyOffset, // offset table: one entry pointing at body
140+
...trailer,
141+
]);
142+
}
143+
144+
test('decodes 2-byte UID index (e.g. object 256)', () {
145+
final bytes = buildBplistWithRootUid(const [0x01, 0x00]);
146+
final result = BPlistDecoder.decode(bytes);
147+
expect(result, isA<BPlistUID>());
148+
expect((result as BPlistUID).index, 256);
149+
});
150+
151+
test('decodes 4-byte UID index', () {
152+
final bytes = buildBplistWithRootUid(const [0x00, 0x01, 0x00, 0x00]);
153+
final result = BPlistDecoder.decode(bytes);
154+
expect((result as BPlistUID).index, 0x00010000);
155+
});
156+
157+
test('still decodes the common 1-byte case', () {
158+
final bytes = buildBplistWithRootUid(const [0x2A]);
159+
final result = BPlistDecoder.decode(bytes);
160+
expect((result as BPlistUID).index, 42);
161+
});
162+
});
163+
116164
group('BPlistDecoder — real MacDive ZTIMEZONE BLOB', () {
117165
late BPlistObject root;
118166

test/features/dive_computer/data/services/dive_import_service_test.mocks.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -993,9 +993,9 @@ class MockDiveRepository extends _i1.Mock implements _i4.DiveRepository {
993993
as _i7.Future<List<_i16.DiveDataSource>>);
994994

995995
@override
996-
_i7.Future<Map<String, String>> getSourceUuidByDiveId() =>
996+
_i7.Future<Map<String, String>> getSourceUuidByDiveId({String? diverId}) =>
997997
(super.noSuchMethod(
998-
Invocation.method(#getSourceUuidByDiveId, []),
998+
Invocation.method(#getSourceUuidByDiveId, [], {#diverId: diverId}),
999999
returnValue: _i7.Future<Map<String, String>>.value(
10001000
<String, String>{},
10011001
),

test/features/dive_import/data/services/uddf_entity_importer_test.mocks.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2058,9 +2058,9 @@ class MockDiveRepository extends _i1.Mock implements _i13.DiveRepository {
20582058
as _i17.Future<List<_i35.DiveDataSource>>);
20592059

20602060
@override
2061-
_i17.Future<Map<String, String>> getSourceUuidByDiveId() =>
2061+
_i17.Future<Map<String, String>> getSourceUuidByDiveId({String? diverId}) =>
20622062
(super.noSuchMethod(
2063-
Invocation.method(#getSourceUuidByDiveId, []),
2063+
Invocation.method(#getSourceUuidByDiveId, [], {#diverId: diverId}),
20642064
returnValue: _i17.Future<Map<String, String>>.value(
20652065
<String, String>{},
20662066
),

0 commit comments

Comments
 (0)