diff --git a/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsManager.kt b/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsManager.kt index faa2c5a..8e7780d 100644 --- a/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsManager.kt +++ b/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsManager.kt @@ -107,13 +107,18 @@ internal class CustomLyricsManager( } /** - * Streams a bounded ZIP backup from [input] into a merge-restore: backup - * entries overwrite same-ID current entries, current-only IDs are kept, - * every restored entry gets a fresh remote fileId. TTML bodies are - * validated and written one at a time, never all at once. Consumes and - * closes [input]. + * Streams a bounded ZIP backup from [input] into a merge-restore. Under + * [CustomLyricsRestorePolicy.OVERWRITE] backup entries overwrite same-ID + * current entries; under [CustomLyricsRestorePolicy.KEEP_EXISTING] + * same-ID conflicts keep the current entry. Current-only IDs are kept and + * backup-only IDs are appended under either policy; every restored entry + * gets a fresh remote fileId. TTML bodies are validated and written one + * at a time, never all at once. Consumes and closes [input]. */ - fun restore(input: InputStream): CustomLyricsRestoreResult = synchronized(mutationLock) { + fun restore( + input: InputStream, + policy: CustomLyricsRestorePolicy = CustomLyricsRestorePolicy.OVERWRITE, + ): CustomLyricsRestoreResult = synchronized(mutationLock) { if (!isWritable()) return CustomLyricsRestoreResult.Failed("libxposed remote file 服务不可用") val state = configStore.indexState(snapshot) return CustomLyricsRestoreTransaction( @@ -123,7 +128,7 @@ internal class CustomLyricsManager( deleteRemoteFile = { fileId -> if (ModuleApplication.isCurrentSnapshot(snapshot)) snapshot.deleteRemoteFile(fileId) }, - ).merge(state.manifest) { onFile -> CustomLyricsBackupCodec.decode(input, onFile) } + ).merge(state.manifest, policy) { onFile -> CustomLyricsBackupCodec.decode(input, onFile) } } private fun readRemoteFile(fileId: String): ByteArray? { diff --git a/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransaction.kt b/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransaction.kt index b6b6eb4..aa440d4 100644 --- a/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransaction.kt +++ b/app/src/main/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransaction.kt @@ -10,14 +10,30 @@ internal sealed interface CustomLyricsRestoreResult { } /** - * Merge-restore semantics: backup entries overwrite current entries with the - * same Apple Music ID, current-only IDs are kept. The backup's files arrive - * one at a time through the caller-supplied stream; every one is written to a - * fresh remote fileId as it arrives, the merged manifest is published once - * after the whole backup scans, then old files of overwritten IDs are - * deleted. Any scan, write, or publish failure rolls back only the new files - * and leaves the old manifest and old files untouched. An empty backup is a - * successful no-op. + * Conflict strategy applied per Apple Music ID when a restore meets both a + * current entry and a backup entry with the same ID. + */ +internal enum class CustomLyricsRestorePolicy { + /** Same-ID conflicts take the backup entry; the overwritten current file is retired. */ + OVERWRITE, + + /** Same-ID conflicts keep the current entry; the written backup file is dropped after publish. */ + KEEP_EXISTING, +} + +/** + * Merge-restore semantics: under [CustomLyricsRestorePolicy.OVERWRITE] backup + * entries overwrite current entries with the same Apple Music ID; under + * [CustomLyricsRestorePolicy.KEEP_EXISTING] same-ID conflicts keep the + * current entry. Current-only IDs are kept and backup-only IDs are appended + * under either policy. The backup's files arrive one at a time through the + * caller-supplied stream; every one is written to a fresh remote fileId as it + * arrives, the merged manifest is published once after the whole backup + * scans, then files that no longer reference any entry are deleted: old files + * of overwritten IDs under OVERWRITE, written-but-dropped backup files under + * KEEP_EXISTING. Any scan, write, or publish failure rolls back only the new + * files and leaves the old manifest and old files untouched. An empty backup + * is a successful no-op. */ internal class CustomLyricsRestoreTransaction( private val fileIdFactory: () -> String, @@ -27,6 +43,7 @@ internal class CustomLyricsRestoreTransaction( ) { fun merge( oldManifest: CustomLyricsManifest, + policy: CustomLyricsRestorePolicy = CustomLyricsRestorePolicy.OVERWRITE, streamBackup: (onFile: (fileId: String, bytes: ByteArray) -> Unit) -> CustomLyricsBackupDecodeResult, ): CustomLyricsRestoreResult { val allocatedFileIds = oldManifest.entries.mapTo(mutableSetOf(), CustomLyricsEntry::fileId) @@ -65,34 +82,46 @@ internal class CustomLyricsRestoreTransaction( } is CustomLyricsBackupDecodeResult.Decoded -> { val backup = scan.backup - if (backup.manifest.entries.isEmpty()) { - return CustomLyricsRestoreResult.Restored(oldManifest) - } val error = writeError if (error != null) { written.forEach { runCatching { deleteRemoteFile(it) } } return CustomLyricsRestoreResult.Failed(error) } + if (backup.manifest.entries.isEmpty()) { + written.forEach { runCatching { deleteRemoteFile(it) } } + return CustomLyricsRestoreResult.Restored(oldManifest) + } val rebuilt = mutableListOf() + val incomingById = linkedMapOf() for (incoming in backup.manifest.entries) { val fileId = newFileIds[incoming.fileId] if (fileId == null) { written.forEach { runCatching { deleteRemoteFile(it) } } return CustomLyricsRestoreResult.Failed("备份内容缺失") } - rebuilt += incoming.copy(fileId = fileId) + val entry = incoming.copy(fileId = fileId) + if (incomingById.putIfAbsent(entry.appleMusicId, entry) != null) { + written.forEach { runCatching { deleteRemoteFile(it) } } + return CustomLyricsRestoreResult.Failed("备份条目重复") + } + rebuilt += entry } - val incomingById = rebuilt.associateBy(CustomLyricsEntry::appleMusicId) val currentIds = oldManifest.entries.mapTo(mutableSetOf(), CustomLyricsEntry::appleMusicId) val mergedEntries = mutableListOf() val retiredFileIds = mutableListOf() + val droppedFileIds = mutableListOf() oldManifest.entries.forEach { current -> val replacement = incomingById[current.appleMusicId] - if (replacement != null) { - mergedEntries += replacement - retiredFileIds += current.fileId - } else { - mergedEntries += current + when { + replacement == null -> mergedEntries += current + policy == CustomLyricsRestorePolicy.OVERWRITE -> { + mergedEntries += replacement + retiredFileIds += current.fileId + } + else -> { + mergedEntries += current + droppedFileIds += replacement.fileId + } } } rebuilt.forEach { entry -> @@ -103,7 +132,7 @@ internal class CustomLyricsRestoreTransaction( written.forEach { runCatching { deleteRemoteFile(it) } } return CustomLyricsRestoreResult.Failed("无法发布歌词映射") } - retiredFileIds.forEach { runCatching { deleteRemoteFile(it) } } + (retiredFileIds + droppedFileIds).forEach { runCatching { deleteRemoteFile(it) } } CustomLyricsRestoreResult.Restored(merged) } } diff --git a/app/src/main/java/dev/amenhancer/module/ui/SettingsActivity.kt b/app/src/main/java/dev/amenhancer/module/ui/SettingsActivity.kt index 9db92d1..9a314b7 100644 --- a/app/src/main/java/dev/amenhancer/module/ui/SettingsActivity.kt +++ b/app/src/main/java/dev/amenhancer/module/ui/SettingsActivity.kt @@ -48,6 +48,7 @@ import dev.amenhancer.module.lyrics.CustomLyricsMutationResult import dev.amenhancer.module.lyrics.CustomLyricsOnlineImportResult import dev.amenhancer.module.lyrics.CustomLyricsOnlineImporter import dev.amenhancer.module.lyrics.CustomLyricsRestoreResult +import dev.amenhancer.module.lyrics.CustomLyricsRestorePolicy import dev.amenhancer.module.lyrics.CustomLyricsSaveResult import dev.amenhancer.module.model.CustomLyricsEntry import dev.amenhancer.module.model.CustomLyricsManifest @@ -761,22 +762,27 @@ class SettingsActivity : Activity() { private fun confirmRestoreCustomLyrics(uri: android.net.Uri) { AlertDialog.Builder(this) - .setTitle("合并恢复歌词备份") - .setMessage( - "同 Apple Music ID 的歌词将使用备份版本；" + - "当前独有歌词会保留；总开关不会改变。是否继续？", - ) + .setTitle("恢复歌词备份") + .setMessage("覆盖：冲突歌词使用备份版本；不覆盖：冲突歌词保留当前版本。") .setNegativeButton("取消", null) - .setPositiveButton("恢复") { _, _ -> restoreCustomLyrics(uri) } + .setNeutralButton("不覆盖") { _, _ -> + restoreCustomLyrics(uri, CustomLyricsRestorePolicy.KEEP_EXISTING) + } + .setPositiveButton("覆盖") { _, _ -> + restoreCustomLyrics(uri, CustomLyricsRestorePolicy.OVERWRITE) + } .show() } - private fun restoreCustomLyrics(uri: android.net.Uri) { + private fun restoreCustomLyrics( + uri: android.net.Uri, + policy: CustomLyricsRestorePolicy, + ) { val snapshot = ModuleApplication.serviceSnapshot backgroundExecutor.execute { val result = runCatching { contentResolver.openInputStream(uri)?.use { input -> - CustomLyricsManager(snapshot, store).restore(input) + CustomLyricsManager(snapshot, store).restore(input, policy) } ?: CustomLyricsRestoreResult.Failed("无法读取备份文件") }.getOrElse { CustomLyricsRestoreResult.Failed("读取备份失败") } runOnUiThread { diff --git a/app/src/test/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransactionTest.kt b/app/src/test/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransactionTest.kt index 861098b..591170c 100644 --- a/app/src/test/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransactionTest.kt +++ b/app/src/test/java/dev/amenhancer/module/lyrics/CustomLyricsRestoreTransactionTest.kt @@ -31,7 +31,7 @@ class CustomLyricsRestoreTransactionTest { events += "publish" published = manifest true - }.merge(current, streamBackup(backup, files)) + }.merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertTrue(result is CustomLyricsRestoreResult.Restored) assertEquals( @@ -46,6 +46,106 @@ class CustomLyricsRestoreTransactionTest { assertEquals(listOf("write:lyrics_new1", "write:lyrics_new2", "publish", "delete:lyrics_b"), events) } + @Test + fun `overwrite policy explicitly replaces the same id and retires the current file`() { + val events = mutableListOf() + var published = CustomLyricsManifest.empty() + val current = CustomLyricsManifest( + listOf( + existingEntry(appleMusicId = 1L, fileId = "lyrics_a"), + existingEntry(appleMusicId = 2L, fileId = "lyrics_b"), + ), + ) + val (backup, files) = backup( + backupEntry(appleMusicId = 2L, fileId = "lyrics_bb", displayName = "B new") to ttmlBytes(), + ) + + val result = transaction(events) { manifest -> + events += "publish" + published = manifest + true + }.merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) + + assertTrue(result is CustomLyricsRestoreResult.Restored) + assertEquals(listOf(1L, 2L), published.entries.map(CustomLyricsEntry::appleMusicId)) + assertEquals(listOf("lyrics_a", "lyrics_new1"), published.entries.map(CustomLyricsEntry::fileId)) + assertEquals("B new", published.entries[1].displayName) + assertEquals(listOf("write:lyrics_new1", "publish", "delete:lyrics_b"), events) + } + + @Test + fun `keep existing policy keeps the conflict entry appends new ids and deletes dropped backup files after publish`() { + val events = mutableListOf() + var published = CustomLyricsManifest.empty() + val current = CustomLyricsManifest( + listOf( + existingEntry(appleMusicId = 1L, fileId = "lyrics_a"), + existingEntry(appleMusicId = 2L, fileId = "lyrics_b"), + ), + ) + val (backup, files) = backup( + backupEntry(appleMusicId = 2L, fileId = "lyrics_bb", displayName = "B new") to ttmlBytes(), + backupEntry(appleMusicId = 3L, fileId = "lyrics_cc") to ttmlBytes(), + ) + + val result = transaction(events) { manifest -> + events += "publish" + published = manifest + true + }.merge(current, CustomLyricsRestorePolicy.KEEP_EXISTING, streamBackup(backup, files)) + + assertTrue(result is CustomLyricsRestoreResult.Restored) + assertEquals(listOf(1L, 2L, 3L), published.entries.map(CustomLyricsEntry::appleMusicId)) + assertEquals(listOf("lyrics_a", "lyrics_b", "lyrics_new2"), published.entries.map(CustomLyricsEntry::fileId)) + assertEquals("Old song 2", published.entries[1].displayName) + assertEquals( + listOf("write:lyrics_new1", "write:lyrics_new2", "publish", "delete:lyrics_new1"), + events, + ) + } + + @Test + fun `keep existing policy without conflicts appends backup only ids and deletes nothing`() { + val events = mutableListOf() + var published = CustomLyricsManifest.empty() + val current = CustomLyricsManifest(listOf(existingEntry(appleMusicId = 1L, fileId = "lyrics_a"))) + val (backup, files) = backup(backupEntry(appleMusicId = 2L, fileId = "lyrics_bb") to ttmlBytes()) + + val result = transaction(events) { manifest -> + events += "publish" + published = manifest + true + }.merge(current, CustomLyricsRestorePolicy.KEEP_EXISTING, streamBackup(backup, files)) + + assertTrue(result is CustomLyricsRestoreResult.Restored) + assertEquals(listOf(1L, 2L), published.entries.map(CustomLyricsEntry::appleMusicId)) + assertEquals(listOf("write:lyrics_new1", "publish"), events) + } + + @Test + fun `keep existing policy rolls back every new file when publish fails`() { + val events = mutableListOf() + val current = CustomLyricsManifest( + listOf( + existingEntry(appleMusicId = 1L, fileId = "lyrics_a"), + existingEntry(appleMusicId = 2L, fileId = "lyrics_b"), + ), + ) + val (backup, files) = backup( + backupEntry(appleMusicId = 2L, fileId = "lyrics_bb") to ttmlBytes(), + backupEntry(appleMusicId = 3L, fileId = "lyrics_cc") to ttmlBytes(), + ) + + val result = transaction(events) { events += "publish"; false } + .merge(current, CustomLyricsRestorePolicy.KEEP_EXISTING, streamBackup(backup, files)) + + assertEquals(CustomLyricsRestoreResult.Failed("无法发布歌词映射"), result) + assertEquals( + listOf("write:lyrics_new1", "write:lyrics_new2", "publish", "delete:lyrics_new1", "delete:lyrics_new2"), + events, + ) + } + @Test fun `a write failure deletes only the new files written so far`() { val events = mutableListOf() @@ -66,7 +166,7 @@ class CustomLyricsRestoreTransactionTest { events += "write:$fileId" fileId != "lyrics_new2" }, - ) { events += "publish"; true }.merge(current, streamBackup(backup, files)) + ) { events += "publish"; true }.merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertEquals(CustomLyricsRestoreResult.Failed("无法写入共享歌词文件"), result) assertEquals( @@ -94,7 +194,8 @@ class CustomLyricsRestoreTransactionTest { backupEntry(appleMusicId = 3L, fileId = "lyrics_cc") to ttmlBytes(), ) - val result = transaction(events) { events += "publish"; false }.merge(current, streamBackup(backup, files)) + val result = transaction(events) { events += "publish"; false } + .merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertEquals(CustomLyricsRestoreResult.Failed("无法发布歌词映射"), result) assertEquals( @@ -109,13 +210,103 @@ class CustomLyricsRestoreTransactionTest { val current = CustomLyricsManifest(listOf(existingEntry(appleMusicId = 1L, fileId = "lyrics_a"))) val result = transaction(events) { events += "publish"; true } - .merge(current, streamBackup(CustomLyricsBackup(CustomLyricsManifest.empty()), emptyMap())) + .merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(CustomLyricsBackup(CustomLyricsManifest.empty()), emptyMap())) assertTrue(result is CustomLyricsRestoreResult.Restored) assertEquals(current, (result as CustomLyricsRestoreResult.Restored).manifest) assertTrue(events.isEmpty()) } + @Test + fun `an empty backup manifest with stray files written before decode cleans them up`() { + val events = mutableListOf() + val current = CustomLyricsManifest(listOf(existingEntry(appleMusicId = 1L, fileId = "lyrics_a"))) + + val result = transaction(events) { events += "publish"; true } + .merge(current, CustomLyricsRestorePolicy.OVERWRITE) { onFile -> + onFile("lyrics_bb", ttmlBytes()) + onFile("lyrics_cc", ttmlBytes()) + CustomLyricsBackupDecodeResult.Decoded(CustomLyricsBackup(CustomLyricsManifest.empty())) + } + + assertTrue(result is CustomLyricsRestoreResult.Restored) + assertEquals(current, (result as CustomLyricsRestoreResult.Restored).manifest) + assertEquals( + listOf( + "write:lyrics_new1", + "write:lyrics_new2", + "delete:lyrics_new1", + "delete:lyrics_new2", + ), + events, + ) + } + + @Test + fun `a write failure with an empty backup manifest fails and rolls back`() { + val events = mutableListOf() + val current = CustomLyricsManifest(listOf(existingEntry(appleMusicId = 1L, fileId = "lyrics_a"))) + + val result = transaction( + events, + write = { fileId, _ -> + events += "write:$fileId" + fileId != "lyrics_new2" + }, + ) { events += "publish"; true } + .merge(current, CustomLyricsRestorePolicy.OVERWRITE) { onFile -> + onFile("lyrics_bb", ttmlBytes()) + onFile("lyrics_cc", ttmlBytes()) + CustomLyricsBackupDecodeResult.Decoded(CustomLyricsBackup(CustomLyricsManifest.empty())) + } + + assertEquals(CustomLyricsRestoreResult.Failed("无法写入共享歌词文件"), result) + assertEquals( + listOf( + "write:lyrics_new1", + "write:lyrics_new2", + "delete:lyrics_new2", + "delete:lyrics_new1", + ), + events, + ) + } + + @Test + fun `a backup manifest with duplicate apple music ids fails and rolls back`() { + val events = mutableListOf() + val declared = CustomLyricsBackup( + CustomLyricsManifest( + listOf( + backupEntry(2L, "lyrics_bb"), + backupEntry(2L, "lyrics_cc"), + backupEntry(3L, "lyrics_dd"), + ), + ), + ) + + val result = transaction(events) { events += "publish"; true } + .merge(CustomLyricsManifest.empty(), CustomLyricsRestorePolicy.OVERWRITE) { onFile -> + onFile("lyrics_bb", ttmlBytes()) + onFile("lyrics_cc", ttmlBytes()) + onFile("lyrics_dd", ttmlBytes()) + CustomLyricsBackupDecodeResult.Decoded(declared) + } + + assertEquals(CustomLyricsRestoreResult.Failed("备份条目重复"), result) + assertEquals( + listOf( + "write:lyrics_new1", + "write:lyrics_new2", + "write:lyrics_new3", + "delete:lyrics_new1", + "delete:lyrics_new2", + "delete:lyrics_new3", + ), + events, + ) + } + @Test fun `merging a backup with more than 32 entries publishes every entry`() { val events = mutableListOf() @@ -130,7 +321,7 @@ class CustomLyricsRestoreTransactionTest { events += "publish" published = manifest true - }.merge(current, streamBackup(backup, files)) + }.merge(current, CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertTrue(result is CustomLyricsRestoreResult.Restored) assertEquals(41, (result as CustomLyricsRestoreResult.Restored).manifest.entries.size) @@ -196,7 +387,7 @@ class CustomLyricsRestoreTransactionTest { val (backup, files) = backup(backupEntry(appleMusicId = 3L, fileId = "lyrics_cc") to ttmlBytes()) val result = transaction(events, fileIdFactory = { "bad file id!" }) { events += "publish"; true } - .merge(CustomLyricsManifest.empty(), streamBackup(backup, files)) + .merge(CustomLyricsManifest.empty(), CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertEquals(CustomLyricsRestoreResult.Failed("无法生成歌词文件 ID"), result) assertTrue(events.isEmpty()) @@ -208,7 +399,7 @@ class CustomLyricsRestoreTransactionTest { val (backup, files) = backup(backupEntry(appleMusicId = 3L, fileId = "lyrics_cc") to ttmlBytes()) val result = transaction(events, fileIdFactory = { "lyrics_taken" }) { events += "publish"; true } - .merge(CustomLyricsManifest(listOf(existingEntry(1L, "lyrics_taken"))), streamBackup(backup, files)) + .merge(CustomLyricsManifest(listOf(existingEntry(1L, "lyrics_taken"))), CustomLyricsRestorePolicy.OVERWRITE, streamBackup(backup, files)) assertEquals(CustomLyricsRestoreResult.Failed("无法生成唯一歌词文件 ID"), result) assertTrue(events.isEmpty()) diff --git a/app/src/test/java/dev/amenhancer/module/ui/SettingsUiStructuralRegressionTest.kt b/app/src/test/java/dev/amenhancer/module/ui/SettingsUiStructuralRegressionTest.kt index bc7cbbc..2f909ed 100644 --- a/app/src/test/java/dev/amenhancer/module/ui/SettingsUiStructuralRegressionTest.kt +++ b/app/src/test/java/dev/amenhancer/module/ui/SettingsUiStructuralRegressionTest.kt @@ -187,10 +187,16 @@ class SettingsUiStructuralRegressionTest { assertTrue(activity.contains("application/x-zip-compressed")) assertTrue(activity.contains("application/octet-stream")) assertTrue(activity.contains("CustomLyricsManager(snapshot, store).backup(output)")) - assertTrue(activity.contains("CustomLyricsManager(snapshot, store).restore(input)")) - assertTrue(activity.contains("同 Apple Music ID 的歌词将使用备份版本")) - assertTrue(activity.contains("当前独有歌词会保留")) - assertTrue(activity.contains("总开关不会改变")) + assertTrue(activity.contains("CustomLyricsManager(snapshot, store).restore(input, policy)")) + assertFalse(activity.contains("setSingleChoiceItems(")) + assertTrue(activity.contains("setNegativeButton(\"取消\", null)")) + assertTrue(activity.contains("setNeutralButton(\"不覆盖\")")) + assertTrue(activity.contains("setPositiveButton(\"覆盖\")")) + assertTrue(activity.contains("CustomLyricsRestorePolicy.OVERWRITE")) + assertTrue(activity.contains("CustomLyricsRestorePolicy.KEEP_EXISTING")) + assertTrue(activity.contains("restoreCustomLyrics(uri, CustomLyricsRestorePolicy.KEEP_EXISTING)")) + assertTrue(activity.contains("restoreCustomLyrics(uri, CustomLyricsRestorePolicy.OVERWRITE)")) + assertTrue(activity.contains("覆盖：冲突歌词使用备份版本；不覆盖：冲突歌词保留当前版本。")) assertTrue(activity.contains("backgroundExecutor.execute")) assertTrue(activity.contains("contentResolver.delete(uri, null, null)")) assertFalse(activity.contains("takePersistableUriPermission"))