Skip to content

Commit 3a09324

Browse files
committed
#1655 fix: do not crash when restoring key map groups
1 parent 24872b5 commit 3a09324

3 files changed

Lines changed: 98 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
- Inputting key events with Shizuku does not crash the app if a Key Mapper keyboard is being used at the same time. And latency when inputting key events has been improved in some apps.
1717
- #1646 disabling Bluetooth clears the list of connected devices
18+
- #1655 do not crash when restoring key map groups
1819

1920
## [3.0.0](https://github.com/sds100/KeyMapper/releases/tag/v3.0.0)
2021

app/src/main/java/io/github/sds100/keymapper/backup/BackupManager.kt

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ import kotlinx.coroutines.withContext
7070
import timber.log.Timber
7171
import java.io.IOException
7272
import java.io.InputStream
73+
import java.util.LinkedList
7374
import java.util.UUID
7475

7576
/**
@@ -410,7 +411,12 @@ class BackupManagerImpl(
410411
val soundFiles = soundDir.listFiles() ?: emptyList() // null if dir doesn't exist
411412

412413
return@then parseBackupContent(inputStream).then { backupContent ->
413-
restore(restoreType, backupContent, soundFiles)
414+
restore(
415+
restoreType,
416+
backupContent,
417+
soundFiles,
418+
currentTime = System.currentTimeMillis(),
419+
)
414420
}
415421
}
416422
}
@@ -441,25 +447,45 @@ class BackupManagerImpl(
441447
}
442448
}
443449

444-
private suspend fun restore(
450+
suspend fun restore(
445451
restoreType: RestoreType,
446452
backupContent: BackupContent,
447453
soundFiles: List<IFile>,
454+
currentTime: Long,
448455
): Result<*> {
449456
try {
450457
// MUST come before restoring key maps so it is possible to
451458
// validate that each key map's group exists in the repository.
452459
if (backupContent.groups != null) {
453-
val groupUids = backupContent.groups.map { it.uid }.toMutableSet()
454-
455460
val existingGroupUids = groupRepository.getAllGroups().first()
456461
.map { it.uid }
457462
.toSet()
458-
.also { groupUids.addAll(it) }
459463

460-
val currentTime = System.currentTimeMillis()
464+
val groupUids = backupContent.groups.map { it.uid }.toMutableSet()
465+
466+
groupUids.addAll(existingGroupUids)
467+
468+
// Group parents must be restored first so an SqliteConstraintException
469+
// is not thrown when restoring a child group.
470+
val groupsToRestoreMap = backupContent.groups.associateBy { it.uid }.toMutableMap()
471+
val groupRestoreQueue = LinkedList<GroupEntity>()
461472

473+
// Order the groups into a queue such that a parent is always before a child.
462474
for (group in backupContent.groups) {
475+
if (groupsToRestoreMap.containsKey(group.uid)) {
476+
groupRestoreQueue.addFirst(group)
477+
}
478+
479+
var parent = groupsToRestoreMap[group.parentUid]
480+
481+
while (parent != null) {
482+
groupRestoreQueue.addFirst(parent)
483+
groupsToRestoreMap.remove(parent.uid)
484+
parent = groupsToRestoreMap[parent.parentUid]
485+
}
486+
}
487+
488+
for (group in groupRestoreQueue) {
463489
// Set the last opened date to now so that the imported group
464490
// shows as the most recent.
465491
var modifiedGroup = group.copy(lastOpenedDate = currentTime)

app/src/test/java/io/github/sds100/keymapper/BackupManagerTest.kt

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import com.google.gson.Gson
55
import com.google.gson.JsonParser
66
import io.github.sds100.keymapper.actions.sound.SoundFileInfo
77
import io.github.sds100.keymapper.actions.sound.SoundsManager
8+
import io.github.sds100.keymapper.backup.BackupContent
89
import io.github.sds100.keymapper.backup.BackupManagerImpl
910
import io.github.sds100.keymapper.backup.RestoreType
1011
import io.github.sds100.keymapper.data.db.AppDatabase
@@ -46,10 +47,12 @@ import org.junit.Rule
4647
import org.junit.Test
4748
import org.junit.rules.TemporaryFolder
4849
import org.junit.runner.RunWith
50+
import org.mockito.ArgumentMatchers
4951
import org.mockito.junit.MockitoJUnitRunner
5052
import org.mockito.kotlin.any
5153
import org.mockito.kotlin.anyVararg
5254
import org.mockito.kotlin.doReturn
55+
import org.mockito.kotlin.inOrder
5356
import org.mockito.kotlin.mock
5457
import org.mockito.kotlin.never
5558
import org.mockito.kotlin.times
@@ -79,6 +82,7 @@ class BackupManagerTest {
7982
private lateinit var fakeFileAdapter: FakeFileAdapter
8083
private lateinit var fakePreferenceRepository: PreferenceRepository
8184
private lateinit var mockKeyMapRepository: KeyMapRepository
85+
private lateinit var mockGroupRepository: GroupRepository
8286
private lateinit var mockSoundsManager: SoundsManager
8387
private lateinit var mockUuidGenerator: UuidGenerator
8488

@@ -94,6 +98,10 @@ class BackupManagerTest {
9498
fakePreferenceRepository = FakePreferenceRepository()
9599

96100
mockKeyMapRepository = mock()
101+
mockGroupRepository = mock<GroupRepository> {
102+
on { getAllGroups() } doReturn MutableStateFlow(emptyList())
103+
on { getGroupsByParent(ArgumentMatchers.any()) }.thenReturn(MutableStateFlow(emptyList()))
104+
}
97105

98106
fakeFileAdapter = FakeFileAdapter(temporaryFolder)
99107

@@ -116,9 +124,7 @@ class BackupManagerTest {
116124
floatingLayoutRepository = mock<FloatingLayoutRepository> {
117125
on { layouts } doReturn MutableStateFlow(State.Data(emptyList()))
118126
},
119-
groupRepository = mock<GroupRepository> {
120-
on { getAllGroups() } doReturn MutableStateFlow(emptyList())
121-
},
127+
groupRepository = mockGroupRepository,
122128
)
123129

124130
parser = JsonParser()
@@ -130,6 +136,62 @@ class BackupManagerTest {
130136
Dispatchers.resetMain()
131137
}
132138

139+
/**
140+
* Issue #1655. If the list of groups in the backup has a child before the parent then the
141+
* parent must be restored first. Otherwise the SqliteConstraintException will be thrown.
142+
*/
143+
@Test
144+
fun `restore groups breadth first so parents exist before children are restored`() = runTest(testDispatcher) {
145+
val parentGroup1 = GroupEntity(
146+
uid = "parent_group_1_uid",
147+
name = "parent_group_1_name",
148+
parentUid = null,
149+
lastOpenedDate = 0L,
150+
)
151+
152+
val parentGroup2 = GroupEntity(
153+
uid = "parent_group_2_uid",
154+
name = "parent_group_2_name",
155+
parentUid = null,
156+
lastOpenedDate = 0L,
157+
)
158+
159+
val childGroup = GroupEntity(
160+
uid = "child_group_uid",
161+
name = "child_group_name",
162+
parentUid = parentGroup1.uid,
163+
lastOpenedDate = 0L,
164+
)
165+
166+
val grandChildGroup = GroupEntity(
167+
uid = "grand_child_group_uid",
168+
name = "grand_child_group_name",
169+
parentUid = childGroup.uid,
170+
lastOpenedDate = 0L,
171+
)
172+
173+
val backupContent = BackupContent(
174+
appVersion = Constants.VERSION_CODE,
175+
dbVersion = AppDatabase.DATABASE_VERSION,
176+
groups = listOf(parentGroup2, grandChildGroup, childGroup, parentGroup1),
177+
)
178+
179+
inOrder(mockGroupRepository) {
180+
backupManager.restore(
181+
RestoreType.REPLACE,
182+
backupContent,
183+
emptyList(),
184+
currentTime = 0L,
185+
)
186+
187+
verify(mockGroupRepository).insert(parentGroup1)
188+
verify(mockGroupRepository).insert(childGroup)
189+
verify(mockGroupRepository).insert(grandChildGroup)
190+
verify(mockGroupRepository).insert(parentGroup2)
191+
verify(mockGroupRepository, never()).update(any())
192+
}
193+
}
194+
133195
@Test
134196
fun `when backing up everything include layouts that are not in the list of key maps`() = runTest(testDispatcher) {
135197
val layoutWithButtons = FloatingLayoutEntityWithButtons(

0 commit comments

Comments
 (0)