Better update implementation

app/src/main/java/com/kunzisoft/keepass/database/action/node/UpdateEntryRunnable.kt

@@ -34,54 +34,52 @@ class UpdateEntryRunnable constructor(
         afterActionNodesFinish: AfterActionNodesFinish?)
     : ActionNodeDatabaseRunnable(context, database, afterActionNodesFinish, save) {
 
-    // Keep backup of original values in case save fails
-    private var mBackupEntryHistory: Entry = Entry(mOldEntry)
-
     override fun nodeAction() {
-        // WARNING : Re attribute parent removed in entry edit activity to save memory
-        mNewEntry.addParentFrom(mOldEntry)
+        if (mOldEntry.nodeId == mNewEntry.nodeId) {
+            // WARNING : Re attribute parent removed in entry edit activity to save memory
+            mNewEntry.addParentFrom(mOldEntry)
 
-        // Build oldest attachments
-        val oldEntryAttachments = mOldEntry.getAttachments(database.attachmentPool, true)
-        val newEntryAttachments = mNewEntry.getAttachments(database.attachmentPool, true)
-        val attachmentsToRemove = ArrayList<Attachment>(oldEntryAttachments)
-        // Not use equals because only check name
-        newEntryAttachments.forEach { newAttachment ->
-            oldEntryAttachments.forEach { oldAttachment ->
-                if (oldAttachment.name == newAttachment.name
-                        && oldAttachment.binaryData == newAttachment.binaryData)
-                    attachmentsToRemove.remove(oldAttachment)
+            // Build oldest attachments
+            val oldEntryAttachments = mOldEntry.getAttachments(database.attachmentPool, true)
+            val newEntryAttachments = mNewEntry.getAttachments(database.attachmentPool, true)
+            val attachmentsToRemove = ArrayList<Attachment>(oldEntryAttachments)
+            // Not use equals because only check name
+            newEntryAttachments.forEach { newAttachment ->
+                oldEntryAttachments.forEach { oldAttachment ->
+                    if (oldAttachment.name == newAttachment.name
+                        && oldAttachment.binaryData == newAttachment.binaryData
+                    )
+                        attachmentsToRemove.remove(oldAttachment)
+                }
             }
-        }
 
-        // Update entry with new values
-        mOldEntry.updateWith(mNewEntry)
-        mNewEntry.touch(modified = true, touchParents = true)
+            // Update entry with new values
+            mNewEntry.touch(modified = true, touchParents = true)
 
-        // Create an entry history (an entry history don't have history)
-        mOldEntry.addEntryToHistory(Entry(mBackupEntryHistory, copyHistory = false))
-        database.removeOldestEntryHistory(mOldEntry, database.attachmentPool)
+            // Create an entry history (an entry history don't have history)
+            mNewEntry.addEntryToHistory(Entry(mOldEntry, copyHistory = false))
+            database.removeOldestEntryHistory(mNewEntry, database.attachmentPool)
 
-        // Only change data in index
-        database.updateEntry(mOldEntry)
+            // Only change data in index
+            database.updateEntry(mNewEntry)
 
-        // Remove oldest attachments
-        attachmentsToRemove.forEach {
-            database.removeAttachmentIfNotUsed(it)
+            // Remove oldest attachments
+            attachmentsToRemove.forEach {
+                database.removeAttachmentIfNotUsed(it)
+            }
         }
     }
 
     override fun nodeFinish(): ActionNodesValues {
         if (!result.isSuccess) {
-            mOldEntry.updateWith(mBackupEntryHistory)
             // If we fail to save, back out changes to global structure
             database.updateEntry(mOldEntry)
         }
 
         val oldNodesReturn = ArrayList<Node>()
-        oldNodesReturn.add(mBackupEntryHistory)
+        oldNodesReturn.add(mOldEntry)
         val newNodesReturn = ArrayList<Node>()
-        newNodesReturn.add(mOldEntry)
+        newNodesReturn.add(mNewEntry)
         return ActionNodesValues(oldNodesReturn, newNodesReturn)
     }
 }

app/src/main/java/com/kunzisoft/keepass/database/action/node/UpdateGroupRunnable.kt

@@ -33,33 +33,30 @@ class UpdateGroupRunnable constructor(
         afterActionNodesFinish: AfterActionNodesFinish?)
     : ActionNodeDatabaseRunnable(context, database, afterActionNodesFinish, save) {
 
-    // Keep backup of original values in case save fails
-    private val mBackupGroup: Group = Group(mOldGroup)
-
     override fun nodeAction() {
-        // WARNING : Re attribute parent and children removed in group activity to save memory
-        mNewGroup.addParentFrom(mOldGroup)
-        mNewGroup.addChildrenFrom(mOldGroup)
+        if (mOldGroup.nodeId == mNewGroup.nodeId) {
+            // WARNING : Re attribute parent and children removed in group activity to save memory
+            mNewGroup.addParentFrom(mOldGroup)
+            mNewGroup.addChildrenFrom(mOldGroup)
 
-        // Update group with new values
-        mOldGroup.updateWith(mNewGroup)
-        mOldGroup.touch(modified = true, touchParents = true)
+            // Update group with new values
+            mNewGroup.touch(modified = true, touchParents = true)
 
-        // Only change data in index
-        database.updateGroup(mOldGroup)
+            // Only change data in index
+            database.updateGroup(mNewGroup)
+        }
     }
 
     override fun nodeFinish(): ActionNodesValues {
         if (!result.isSuccess) {
             // If we fail to save, back out changes to global structure
-            mOldGroup.updateWith(mBackupGroup)
             database.updateGroup(mOldGroup)
         }
 
         val oldNodesReturn = ArrayList<Node>()
-        oldNodesReturn.add(mBackupGroup)
+        oldNodesReturn.add(mOldGroup)
         val newNodesReturn = ArrayList<Node>()
-        newNodesReturn.add(mOldGroup)
+        newNodesReturn.add(mNewGroup)
         return ActionNodesValues(oldNodesReturn, newNodesReturn)
     }
 }

app/src/main/java/com/kunzisoft/keepass/database/element/Entry.kt

@@ -45,15 +45,6 @@ class Entry : Node, EntryVersionedInterface<Group> {
     var entryKDBX: EntryKDBX? = null
         private set
 
-    fun updateWith(entry: Entry, copyHistory: Boolean = true) {
-        entry.entryKDB?.let {
-            this.entryKDB?.updateWith(it)
-        }
-        entry.entryKDBX?.let {
-            this.entryKDBX?.updateWith(it, copyHistory)
-        }
-    }
-
     /**
      * Use this constructor to copy an Entry with exact same values
      */
@@ -64,7 +55,12 @@ class Entry : Node, EntryVersionedInterface<Group> {
         if (entry.entryKDBX != null) {
             this.entryKDBX = EntryKDBX()
         }
-        updateWith(entry, copyHistory)
+        entry.entryKDB?.let {
+            this.entryKDB?.updateWith(it)
+        }
+        entry.entryKDBX?.let {
+            this.entryKDBX?.updateWith(it, copyHistory)
+        }
     }
 
     constructor(entry: EntryKDB) {

app/src/main/java/com/kunzisoft/keepass/database/element/Group.kt

@@ -340,6 +340,24 @@ class Group : Node, GroupVersionedInterface<Group, Entry> {
         }
     }
 
+    override fun updateChildGroup(group: Group) {
+        group.groupKDB?.let {
+            groupKDB?.updateChildGroup(it)
+        }
+        group.groupKDBX?.let {
+            groupKDBX?.updateChildGroup(it)
+        }
+    }
+
+    override fun updateChildEntry(entry: Entry) {
+        entry.entryKDB?.let {
+            groupKDB?.updateChildEntry(it)
+        }
+        entry.entryKDBX?.let {
+            groupKDBX?.updateChildEntry(it)
+        }
+    }
+
     override fun removeChildGroup(group: Group) {
         group.groupKDB?.let {
             groupKDB?.removeChildGroup(it)

app/src/main/java/com/kunzisoft/keepass/database/element/database/DatabaseVersioned.kt

@@ -242,13 +242,6 @@ abstract class DatabaseVersioned<
         }
     }
 
-    fun updateGroupIndex(group: Group) {
-        val groupId = group.nodeId
-        if (groupIndexes.containsKey(groupId)) {
-            groupIndexes[groupId] = group
-        }
-    }
-
     fun removeGroupIndex(group: Group) {
         this.groupIndexes.remove(group.nodeId)
     }
@@ -291,13 +284,6 @@ abstract class DatabaseVersioned<
         }
     }
 
-    fun updateEntryIndex(entry: Entry) {
-        val entryId = entry.nodeId
-        if (entryIndexes.containsKey(entryId)) {
-            entryIndexes[entryId] = entry
-        }
-    }
-
     fun removeEntryIndex(entry: Entry) {
         this.entryIndexes.remove(entry.nodeId)
     }
@@ -329,7 +315,10 @@ abstract class DatabaseVersioned<
     }
 
     fun updateGroup(group: Group) {
-        updateGroupIndex(group)
+        val groupId = group.nodeId
+        if (groupIndexes.containsKey(groupId)) {
+            groupIndexes[groupId] = group
+        }
     }
 
     fun removeGroupFrom(groupToRemove: Group, parent: Group?) {
@@ -346,7 +335,10 @@ abstract class DatabaseVersioned<
     }
 
     open fun updateEntry(entry: Entry) {
-        updateEntryIndex(entry)
+        val entryId = entry.nodeId
+        if (entryIndexes.containsKey(entryId)) {
+            entryIndexes[entryId] = entry
+        }
     }
 
     open fun removeEntryFrom(entryToRemove: Entry, parent: Group?) {

app/src/main/java/com/kunzisoft/keepass/database/element/group/GroupVersioned.kt

@@ -98,6 +98,22 @@ abstract class GroupVersioned
         this.childEntries.add(entry)
     }
 
+    override fun updateChildGroup(group: Group) {
+        val index = this.childGroups.indexOfFirst { it.nodeId == group.nodeId }
+        if (index >= 0) {
+            this.childGroups.removeAt(index)
+            this.childGroups.add(index, group)
+        }
+    }
+
+    override fun updateChildEntry(entry: Entry) {
+        val index = this.childEntries.indexOfFirst { it.nodeId == entry.nodeId }
+        if (index >= 0) {
+            this.childEntries.removeAt(index)
+            this.childEntries.add(index, entry)
+        }
+    }
+
     override fun removeChildGroup(group: Group) {
         this.childGroups.remove(group)
     }

app/src/main/java/com/kunzisoft/keepass/database/element/group/GroupVersionedInterface.kt

@@ -32,6 +32,10 @@ interface GroupVersionedInterface<Group: GroupVersionedInterface<Group, Entry>,
 
     fun addChildEntry(entry: Entry)
 
+    fun updateChildGroup(group: Group)
+
+    fun updateChildEntry(entry: Entry)
+
     fun removeChildGroup(group: Group)
 
     fun removeChildEntry(entry: Entry)