fix merging of incoming deleted objects.

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## 2.1.0
 
 - Implement permanently removing entries and groups.
+- Fix merging of files with incoming deleted objects.
 
 ## 2.0.0+1
 

lib/src/kdbx_dao.dart

@@ -1,5 +1,4 @@
 import 'package:clock/clock.dart';
-import 'package:kdbx/src/kdbx_deleted_object.dart';
 import 'package:kdbx/src/kdbx_entry.dart';
 import 'package:kdbx/src/kdbx_file.dart';
 import 'package:kdbx/src/kdbx_group.dart';

lib/src/kdbx_deleted_object.dart

@@ -18,7 +18,8 @@ class KdbxDeletedObject extends KdbxNode implements KdbxNodeContext {
   @override
   final KdbxReadWriteContext ctx;
 
-  KdbxUuid? get uuid => _uuid.get();
+  // all objects have to have a UUID.
+  KdbxUuid get uuid => _uuid.get()!;
   UuidNode get _uuid => UuidNode(this, KdbxXml.NODE_UUID);
   DateTimeUtcNode get deletionTime => DateTimeUtcNode(this, 'DeletionTime');
 }

lib/src/kdbx_format.dart

@@ -16,6 +16,7 @@ import 'package:kdbx/src/internal/crypto_utils.dart';
 import 'package:kdbx/src/internal/extension_utils.dart';
 import 'package:kdbx/src/kdbx_deleted_object.dart';
 import 'package:kdbx/src/kdbx_entry.dart';
+import 'package:kdbx/src/kdbx_group.dart';
 import 'package:kdbx/src/kdbx_header.dart';
 import 'package:kdbx/src/kdbx_xml.dart';
 import 'package:kdbx/src/utils/byte_utils.dart';
@@ -338,7 +339,25 @@ class KdbxBody extends KdbxNode {
 
     // remove deleted objects
     for (final incomingDelete in incomingDeleted.values) {
-      final object = mergeContext.objectIndex![incomingDelete.uuid!];
+      final object = mergeContext.objectIndex[incomingDelete.uuid];
+      if (object == null) {
+        mergeContext.trackWarning(
+            'Incoming deleted object not found locally ${incomingDelete.uuid}');
+        continue;
+      }
+      final parent = object.parent;
+      if (parent == null) {
+        mergeContext.trackWarning('Unable to delete object $object - '
+            'already deleted? (${incomingDelete.uuid})');
+        continue;
+      }
+      if (object is KdbxGroup) {
+        parent.internalRemoveGroup(object);
+      } else if (object is KdbxEntry) {
+        parent.internalRemoveEntry(object);
+      } else {
+        throw StateError('Invalid object type $object');
+      }
       mergeContext.trackChange(object, debug: 'was deleted.');
     }
 
@@ -347,11 +366,11 @@ class KdbxBody extends KdbxNode {
     _logger.info('Finished merging:\n${mergeContext.debugChanges()}');
     final incomingObjects = other._createObjectIndex();
     _logger.info('Merged: ${mergeContext.merged} vs. '
-        '(local objects: ${mergeContext.objectIndex!.length}, '
+        '(local objects: ${mergeContext.objectIndex.length}, '
         'incoming objects: ${incomingObjects.length})');
 
     // sanity checks
-    if (mergeContext.merged.keys.length != mergeContext.objectIndex!.length) {
+    if (mergeContext.merged.keys.length != mergeContext.objectIndex.length) {
       // TODO figure out what went wrong.
     }
     return mergeContext;
@@ -429,12 +448,24 @@ class MergeChange {
   }
 }
 
+class MergeWarning {
+  MergeWarning(this.debug);
+
+  final String debug;
+
+  @override
+  String toString() {
+    return debug;
+  }
+}
+
 class MergeContext implements OverwriteContext {
-  MergeContext({this.objectIndex, this.deletedObjects});
+  MergeContext({required this.objectIndex, required this.deletedObjects});
-  final Map<KdbxUuid, KdbxObject>? objectIndex;
+  final Map<KdbxUuid, KdbxObject> objectIndex;
-  final Map<KdbxUuid?, KdbxDeletedObject>? deletedObjects;
+  final Map<KdbxUuid?, KdbxDeletedObject> deletedObjects;
   final Map<KdbxUuid, KdbxObject> merged = {};
   final List<MergeChange> changes = [];
+  final List<MergeWarning> warnings = [];
 
   void markAsMerged(KdbxObject object) {
     if (merged.containsKey(object.uuid)) {
@@ -453,6 +484,11 @@ class MergeContext implements OverwriteContext {
     ));
   }
 
+  void trackWarning(String warning) {
+    _logger.warning(warning, StackTrace.current);
+    warnings.add(MergeWarning(warning));
+  }
+
   String debugChanges() {
     final group =
         changes.groupBy((element) => element.object, valueTransform: (x) => x);

lib/src/kdbx_group.dart

@@ -149,7 +149,7 @@ class KdbxGroup extends KdbxObject {
       if (meObj == null) {
         // moved or deleted.
 
-        final movedObj = mergeContext.objectIndex![otherObj.uuid];
+        final movedObj = mergeContext.objectIndex[otherObj.uuid];
         if (movedObj == null) {
           // item was created in the other file. we have to import it
           final newMeObject = importToHere(otherObj);

test/kdbx_binaries_test.dart

@@ -2,8 +2,6 @@ import 'dart:convert';
 import 'dart:typed_data';
 
 import 'package:kdbx/kdbx.dart';
-import 'package:logging/logging.dart';
-import 'package:logging_appenders/logging_appenders.dart';
 import 'package:test/test.dart';
 
 import 'internal/test_utils.dart';

test/kdbx_test.dart

@@ -5,7 +5,6 @@ import 'dart:typed_data';
 import 'package:kdbx/kdbx.dart';
 import 'package:kdbx/src/crypto/protected_salt_generator.dart';
 import 'package:logging/logging.dart';
-import 'package:logging_appenders/logging_appenders.dart';
 import 'package:synchronized/synchronized.dart';
 import 'package:test/test.dart';
 

test/merge/kdbx_merge_test.dart

@@ -1,10 +1,13 @@
 import 'package:clock/clock.dart';
 import 'package:kdbx/kdbx.dart';
+import 'package:kdbx/src/kdbx_xml.dart';
 import 'package:kdbx/src/utils/print_utils.dart';
 import 'package:logging/logging.dart';
 import 'package:test/test.dart';
+import 'package:xml/xml.dart';
 
 import '../internal/test_utils.dart';
+import '../kdbx_test.dart';
 
 final _logger = Logger('kdbx_merge_test');
 
@@ -97,6 +100,32 @@ void main() {
             hasLength(2));
       }),
     );
+    test(
+      'permanently delete an entry',
+      () async => await withClock(fakeClock, () async {
+        final file = await createSimpleFile();
+        final objCount = file.body.rootGroup.getAllGroupsAndEntries().length;
+        final fileMod = await testUtil.saveAndRead(file);
+        final entryDelete = fileMod.body.rootGroup.entries.first;
+        fileMod.deletePermanently(entryDelete);
+        expect(fileMod.body.rootGroup.getAllGroupsAndEntries(),
+            hasLength(objCount - 1));
+
+        final file2 = await testUtil.saveAndRead(fileMod);
+        final merge = file.merge(file2);
+        _logger.info('Merged file:\n'
+            '${KdbxPrintUtils().catGroupToString(file.body.rootGroup)}');
+        expect(merge.deletedObjects, hasLength(1));
+        expect(
+            file.body.rootGroup.getAllGroupsAndEntries().length, objCount - 1);
+        final xml = file.body.generateXml(FakeProtectedSaltGenerator());
+        final deleted = xml.findAllElements(KdbxXml.NODE_DELETED_OBJECT);
+        expect(deleted, hasLength(1));
+        expect(
+            deleted.first.findAllElements(KdbxXml.NODE_UUID).map((e) => e.text),
+            [entryDelete.uuid.uuid]);
+      }),
+    );
   });
 }