Skip to content

Commit e27a2a1

Browse files
committed
fix(import): address PR #254 review — MacDive XML consumer compatibility
- Align MacDive tank map with UddfEntityImporter: `volume` / `workingPressure` keys and `GasMix` object (importer does `t['gasMix'] as GasMix?`, so the prior Map would throw a TypeError and `volumeL` / `workingPressureBar` were silently dropped). - Return UTC wall-time from `MacDiveXmlReader._parseDate` so dive timestamps don't drift with device timezone / DST, matching the Subsurface parser convention. - Relax MacDive format detection to match `<dives` / `<schema` as prefixes so root tags with attributes / namespaces still detect. - Skip empty `<item/>` gear elements instead of producing a phantom `"||"` equipment entity. - Use `buddyRefs` (resolved via `buddyIdMapping`) instead of `unmatchedBuddyNames` so deselected buddies don't get created unconditionally — mirrors SubsurfaceXmlParser.
1 parent fa919a6 commit e27a2a1

6 files changed

Lines changed: 109 additions & 29 deletions

File tree

lib/features/universal_import/data/parsers/macdive_xml_parser.dart

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import 'dart:convert';
22
import 'dart:typed_data';
33

4+
import 'package:submersion/features/dive_log/domain/entities/dive.dart'
5+
show GasMix;
46
import 'package:submersion/features/universal_import/data/models/import_enums.dart';
57
import 'package:submersion/features/universal_import/data/models/import_options.dart';
68
import 'package:submersion/features/universal_import/data/models/import_payload.dart';
@@ -107,22 +109,24 @@ class MacDiveXmlParser implements ImportParser {
107109
}
108110

109111
if (dive.buddies.isNotEmpty) {
110-
final names = <String>[];
112+
final buddyRefs = <String>[];
111113
for (final buddy in dive.buddies) {
112114
final trimmed = buddy.trim();
113115
if (trimmed.isEmpty) continue;
114-
names.add(trimmed);
116+
buddyRefs.add(trimmed);
115117
buddiesByName.putIfAbsent(
116118
trimmed,
117119
() => <String, dynamic>{'name': trimmed, 'uddfId': trimmed},
118120
);
119121
}
120-
if (names.isNotEmpty) {
121-
// `unmatchedBuddyNames` is the key UddfEntityImporter uses for
122-
// inline buddy names that should be created on demand and linked
123-
// to the dive. This keeps one-pipe compatibility with the UDDF
124-
// importer without introducing a second key name.
125-
diveMap['unmatchedBuddyNames'] = names;
122+
if (buddyRefs.isNotEmpty) {
123+
// `buddyRefs` matches the `uddfId` values of the buddy entities we
124+
// just added to `buddiesByName`, so `UddfEntityImporter` resolves
125+
// them via `buddyIdMapping`. That mapping only has entries for
126+
// buddies the user actually selected for import — using
127+
// `unmatchedBuddyNames` here would bypass that selection and create
128+
// buddies unconditionally. This mirrors SubsurfaceXmlParser.
129+
diveMap['buddyRefs'] = buddyRefs;
126130
}
127131
}
128132

@@ -222,10 +226,12 @@ class MacDiveXmlParser implements ImportParser {
222226
// MacDive rating is a 0.0-5.0 float; Submersion stores 0-5 int.
223227
if (d.rating != null) map['rating'] = d.rating!.clamp(0.0, 5.0).round();
224228

225-
// Tanks: each <gas> becomes a tank map. gasMix is nested as a Map with
226-
// o2/he expressed as fractions (0.0-1.0) so the downstream importer can
227-
// construct a GasMix and link tank -> gas without having to re-derive
228-
// the mix. This mirrors what the UDDF pipeline produces for its tanks.
229+
// Tanks: each <gas> becomes a tank map using the same key conventions as
230+
// the Subsurface and UDDF parsers so `UddfEntityImporter._buildTanks` can
231+
// consume MacDive tanks unchanged — keys `volume` / `workingPressure`
232+
// (not `volumeL` / `workingPressureBar`), and `gasMix` as a `GasMix`
233+
// object (the importer casts `t['gasMix'] as GasMix?`). `GasMix` stores
234+
// o2/he as percentages 0-100, which matches what the reader already emits.
229235
final tanks = <Map<String, dynamic>>[];
230236
for (var i = 0; i < d.gases.length; i++) {
231237
final g = d.gases[i];
@@ -234,17 +240,17 @@ class MacDiveXmlParser implements ImportParser {
234240
tank['startPressure'] = g.pressureStartBar;
235241
}
236242
if (g.pressureEndBar != null) tank['endPressure'] = g.pressureEndBar;
237-
if (g.tankSizeLiters != null) tank['volumeL'] = g.tankSizeLiters;
243+
if (g.tankSizeLiters != null) tank['volume'] = g.tankSizeLiters;
238244
if (g.workingPressureBar != null) {
239-
tank['workingPressureBar'] = g.workingPressureBar;
245+
tank['workingPressure'] = g.workingPressureBar;
240246
}
241247
if (g.tankName != null) tank['name'] = g.tankName;
242248
if (g.supplyType != null) tank['supplyType'] = g.supplyType;
243249
if (g.duration != null) tank['runtime'] = g.duration;
244-
tank['gasMix'] = <String, dynamic>{
245-
if (g.oxygenPercent != null) 'o2': g.oxygenPercent! / 100.0,
246-
if (g.heliumPercent != null) 'he': g.heliumPercent! / 100.0,
247-
};
250+
tank['gasMix'] = GasMix(
251+
o2: g.oxygenPercent ?? 21.0,
252+
he: g.heliumPercent ?? 0.0,
253+
);
248254
tanks.add(tank);
249255
}
250256
if (tanks.isNotEmpty) map['tanks'] = tanks;
@@ -292,6 +298,13 @@ class MacDiveXmlParser implements ImportParser {
292298
}
293299

294300
String _gearKey(MacDiveXmlGearItem g) {
295-
return '${g.manufacturer ?? ''}|${g.name ?? ''}|${g.serial ?? ''}';
301+
final manufacturer = g.manufacturer?.trim() ?? '';
302+
final name = g.name?.trim() ?? '';
303+
final serial = g.serial?.trim() ?? '';
304+
// Empty `<item/>` elements would otherwise collapse to a `"||"` key and
305+
// show up as a phantom equipment entity. Skip them by returning an
306+
// empty key; the caller (`_gearKey(...).isEmpty`) drops the item.
307+
if (manufacturer.isEmpty && name.isEmpty && serial.isEmpty) return '';
308+
return '$manufacturer|$name|$serial';
296309
}
297310
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,11 @@ class FormatDetector {
145145

146146
// MacDive native XML: root <dives>, DOCTYPE macdive_logbook.dtd, <schema>.
147147
// Must precede the UDDF check because both are XML but MacDive's native
148-
// XML is a different format entirely.
148+
// XML is a different format entirely. Match opening tags as prefixes so
149+
// attributes/namespace declarations (`<dives xmlns=...>`) or trailing
150+
// whitespace (`<dives >`) don't defeat detection.
149151
if (lower.contains('mac-dive.com/macdive_logbook.dtd') ||
150-
(lower.contains('<dives>') && lower.contains('<schema>'))) {
152+
(lower.contains('<dives') && lower.contains('<schema'))) {
151153
return const DetectionResult(
152154
format: ImportFormat.macdiveXml,
153155
sourceApp: SourceApp.macdive,

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,31 @@ class MacDiveXmlReader {
208208

209209
static DateTime? _parseDate(String? raw) {
210210
if (raw == null || raw.isEmpty) return null;
211+
// MacDive XML carries no timezone info — treat the timestamp as a wall
212+
// clock and encode it in UTC so dedup and display don't drift when the
213+
// device's timezone changes (travel, DST). Matches the Subsurface XML
214+
// parser's convention (see SubsurfaceXmlParser._parseDive).
211215
try {
212-
return _dateFormat.parseStrict(raw);
216+
return _asUtcWallTime(_dateFormat.parseStrict(raw));
213217
} catch (_) {
214-
// Fallback: ISO-8601 style (2024-06-01T09:00:00)
215-
return DateTime.tryParse(raw.replaceFirst(' ', 'T'));
218+
final parsed = DateTime.tryParse(raw.replaceFirst(' ', 'T'));
219+
return parsed == null ? null : _asUtcWallTime(parsed);
216220
}
217221
}
218222

223+
static DateTime _asUtcWallTime(DateTime value) {
224+
return DateTime.utc(
225+
value.year,
226+
value.month,
227+
value.day,
228+
value.hour,
229+
value.minute,
230+
value.second,
231+
value.millisecond,
232+
value.microsecond,
233+
);
234+
}
235+
219236
static Duration? _durationSeconds(int? seconds) {
220237
if (seconds == null) return null;
221238
return Duration(seconds: seconds);

test/features/universal_import/data/parsers/macdive_xml_parser_test.dart

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import 'dart:typed_data';
44

55
import 'package:flutter_test/flutter_test.dart';
66

7+
import 'package:submersion/features/dive_log/domain/entities/dive.dart'
8+
show GasMix;
79
import 'package:submersion/features/universal_import/data/models/import_enums.dart';
810
import 'package:submersion/features/universal_import/data/models/import_warning.dart';
911
import 'package:submersion/features/universal_import/data/parsers/macdive_xml_parser.dart';
@@ -76,10 +78,17 @@ void main() {
7678
final tank = tanks.first as Map<String, dynamic>;
7779
expect(tank['startPressure'], 200);
7880
expect(tank['endPressure'], 60);
79-
expect(tank['gasMix'], isNotNull);
80-
final gasMix = tank['gasMix'] as Map<String, dynamic>;
81-
expect(gasMix['o2'], closeTo(0.32, 0.01));
82-
expect(gasMix['he'], closeTo(0.0, 0.01));
81+
// Keys match the UDDF/Subsurface tank-map convention so
82+
// UddfEntityImporter._buildTanks can consume them directly.
83+
expect(tank['volume'], 12);
84+
expect(tank['workingPressure'], 232);
85+
// gasMix must be a `GasMix` object, not a Map — the importer does
86+
// `t['gasMix'] as GasMix?` and a Map cast would throw.
87+
expect(tank['gasMix'], isA<GasMix>());
88+
final gasMix = tank['gasMix'] as GasMix;
89+
// GasMix stores o2/he as percentages 0-100 (not 0-1 fractions).
90+
expect(gasMix.o2, closeTo(32.0, 0.01));
91+
expect(gasMix.he, closeTo(0.0, 0.01));
8392
});
8493

8594
test('dive has profile samples with timestamp+depth', () async {
@@ -183,6 +192,30 @@ void main() {
183192
);
184193
});
185194

195+
test(
196+
'empty <item/> gear elements are skipped (no phantom entity)',
197+
() async {
198+
const xml = '''<?xml version="1.0"?>
199+
<dives><units>Metric</units><schema>2.2.0</schema>
200+
<dive>
201+
<date>2024-01-01 09:00:00</date><identifier>d1</identifier>
202+
<maxDepth>20</maxDepth><duration>1800</duration>
203+
<gear>
204+
<item/>
205+
<item><manufacturer> </manufacturer><name></name><serial/></item>
206+
<item><manufacturer>Test</manufacturer><name>BCD1</name></item>
207+
</gear>
208+
<samples/>
209+
</dive>
210+
</dives>''';
211+
final bytes = Uint8List.fromList(utf8.encode(xml));
212+
final payload = await const MacDiveXmlParser().parse(bytes);
213+
final equipment = payload.entitiesOf(ImportEntityType.equipment);
214+
expect(equipment.length, 1, reason: 'only the populated item survives');
215+
expect(equipment.first['name'], 'BCD1');
216+
},
217+
);
218+
186219
test('multiple dives with overlapping buddies dedup', () async {
187220
const xml = '''<?xml version="1.0"?>
188221
<dives><units>Metric</units><schema>2.2.0</schema>

test/features/universal_import/data/services/format_detector_test.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,18 @@ void main() {
151151
final result = detector.detect(Uint8List.fromList(utf8.encode(xml)));
152152
expect(result.format, isNot(ImportFormat.macdiveXml));
153153
});
154+
155+
test('detects MacDive XML when root tags carry attributes', () {
156+
const xml =
157+
'<?xml version="1.0"?>'
158+
'<dives xmlns="http://example.com/mac">'
159+
'<schema version="2.2.0">2.2.0</schema>'
160+
'<dive/>'
161+
'</dives>';
162+
final result = detector.detect(Uint8List.fromList(utf8.encode(xml)));
163+
expect(result.format, ImportFormat.macdiveXml);
164+
expect(result.sourceApp, SourceApp.macdive);
165+
});
154166
});
155167

156168
group('CSV detection', () {

test/features/universal_import/data/services/macdive_xml_reader_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ void main() {
2525
test('parses identifier, date, diveNumber', () {
2626
final dive = MacDiveXmlReader.parse(content).dives.first;
2727
expect(dive.identifier, '20240601090000-ABC123');
28-
expect(dive.date, DateTime(2024, 6, 1, 9, 0, 0));
28+
// MacDive XML has no timezone; we encode the wall clock in UTC so
29+
// timestamps don't drift across DST or travel. See _parseDate.
30+
expect(dive.date, DateTime.utc(2024, 6, 1, 9, 0, 0));
31+
expect(dive.date!.isUtc, isTrue);
2932
expect(dive.diveNumber, 42);
3033
});
3134

0 commit comments

Comments
 (0)