From f875b03e79e3e50e5a0c28349e7b97202db22efc Mon Sep 17 00:00:00 2001 From: Zennmn <176552149+Zennmn@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:17:39 +0800 Subject: [PATCH 1/3] fix: preserve lyric blur around instrumental rows --- .../module/hook/BidirectionalBlurPolicy.kt | 17 +++- .../module/hook/LyricBlurRenderer.kt | 1 - .../module/hook/LyricHighlightSession.kt | 27 ++++++ .../module/hook/OpenSourceLyricBlurPort.kt | 2 + .../hook/BidirectionalBlurPolicyTest.kt | 73 +++++++++++++++ ...cBlurClearAlphaStructuralRegressionTest.kt | 43 +++++++++ .../module/hook/LyricHighlightSessionTest.kt | 93 +++++++++++++++++++ 7 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/dev/amenhancer/module/hook/LyricBlurClearAlphaStructuralRegressionTest.kt diff --git a/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt b/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt index 4a99a35..27bfd51 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt @@ -28,10 +28,25 @@ internal object BidirectionalBlurPolicy { fun selectInstrumentalGapAnchor( active: Set, isGap: Boolean, + isOpeningHighlight: Boolean, instrumentalPositions: List, + visiblePositions: List, ): Int { - if (active.isNotEmpty() && !isGap) return -1 val referencePosition = active.maxOrNull() + if (active.isNotEmpty() && !isGap) { + if (!isOpeningHighlight) return -1 + val earliestVisible = visiblePositions + .asSequence() + .filter { position -> position >= 0 } + .minOrNull() + ?: return -1 + if (!active.contains(earliestVisible)) return -1 + return instrumentalPositions + .asSequence() + .filter { position -> position >= 0 && position < earliestVisible } + .maxOrNull() + ?: -1 + } return instrumentalPositions .asSequence() .filter { position -> position >= 0 } diff --git a/app/src/main/java/dev/amenhancer/module/hook/LyricBlurRenderer.kt b/app/src/main/java/dev/amenhancer/module/hook/LyricBlurRenderer.kt index 6162b59..2774843 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/LyricBlurRenderer.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/LyricBlurRenderer.kt @@ -71,7 +71,6 @@ internal class LyricBlurRenderer { fun clear(view: View) { transitions.remove(view) view.setRenderEffect(null) - view.alpha = 1f } fun clearAll() { diff --git a/app/src/main/java/dev/amenhancer/module/hook/LyricHighlightSession.kt b/app/src/main/java/dev/amenhancer/module/hook/LyricHighlightSession.kt index 76f9513..b484e05 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/LyricHighlightSession.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/LyricHighlightSession.kt @@ -6,6 +6,11 @@ internal class LyricHighlightSession { private val highlightedLineIds = mutableSetOf() private val completedOverlapLineIds = mutableSetOf() private var gap = false + private var openingHighlight = false + + // Explicit "first non-empty highlight since enter" state; the highlighted set alone + // would conflate "never highlighted" with "set empty at this moment". + private var hasReceivedNonEmptyHighlight = false @Synchronized fun enter(newToken: Any): Boolean { @@ -14,6 +19,8 @@ internal class LyricHighlightSession { highlightedLineIds.clear() completedOverlapLineIds.clear() gap = false + openingHighlight = false + hasReceivedNonEmptyHighlight = false return true } @@ -25,6 +32,12 @@ internal class LyricHighlightSession { } gap = false if (incoming == highlightedLineIds) return snapshotLocked() + if (!hasReceivedNonEmptyHighlight) { + hasReceivedNonEmptyHighlight = true + openingHighlight = true + } else { + openingHighlight = false + } val completedOverlap = if ( highlightedLineIds.size > 1 && highlightedLineIds.containsAll(incoming) ) { @@ -45,6 +58,17 @@ internal class LyricHighlightSession { @Synchronized fun replace(lineId: Int) { gap = false + val isRepeat = highlightedLineIds.size == 1 && + completedOverlapLineIds.isEmpty() && + highlightedLineIds.contains(lineId) + if (!isRepeat) { + if (!hasReceivedNonEmptyHighlight) { + hasReceivedNonEmptyHighlight = true + openingHighlight = true + } else { + openingHighlight = false + } + } completedOverlapLineIds.clear() highlightedLineIds.clear() highlightedLineIds.add(lineId) @@ -56,5 +80,8 @@ internal class LyricHighlightSession { @Synchronized fun isGap(): Boolean = gap + @Synchronized + fun isOpeningHighlight(): Boolean = openingHighlight + private fun snapshotLocked(): Set = highlightedLineIds + completedOverlapLineIds } diff --git a/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt b/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt index b3c053f..e5c2c5c 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt @@ -250,7 +250,9 @@ internal class OpenSourceLyricBlurPort( val gapAnchorPosition = BidirectionalBlurPolicy.selectInstrumentalGapAnchor( active = activeIds, isGap = highlightSession.isGap(), + isOpeningHighlight = highlightSession.isOpeningHighlight(), instrumentalPositions = instrumentalRows.map { (_, position) -> position }, + visiblePositions = visibleRows.map { (_, position) -> position }, ) val effectiveIds = BidirectionalBlurPolicy.resolveDisplayHighlights( active = activeIds, diff --git a/app/src/test/java/dev/amenhancer/module/hook/BidirectionalBlurPolicyTest.kt b/app/src/test/java/dev/amenhancer/module/hook/BidirectionalBlurPolicyTest.kt index 366229d..bbbabe6 100644 --- a/app/src/test/java/dev/amenhancer/module/hook/BidirectionalBlurPolicyTest.kt +++ b/app/src/test/java/dev/amenhancer/module/hook/BidirectionalBlurPolicyTest.kt @@ -49,7 +49,9 @@ class BidirectionalBlurPolicyTest { BidirectionalBlurPolicy.selectInstrumentalGapAnchor( active = emptySet(), isGap = false, + isOpeningHighlight = false, instrumentalPositions = listOf(0), + visiblePositions = emptyList(), ), ) assertEquals( @@ -57,7 +59,9 @@ class BidirectionalBlurPolicyTest { BidirectionalBlurPolicy.selectInstrumentalGapAnchor( active = setOf(4), isGap = true, + isOpeningHighlight = false, instrumentalPositions = listOf(5), + visiblePositions = listOf(3, 4, 6), ), ) assertEquals( @@ -65,7 +69,76 @@ class BidirectionalBlurPolicyTest { BidirectionalBlurPolicy.selectInstrumentalGapAnchor( active = setOf(4), isGap = false, + isOpeningHighlight = false, instrumentalPositions = listOf(5), + visiblePositions = listOf(3, 4, 6), + ), + ) + } + + @Test + fun `opening highlight before the earliest visible lyric anchors the three-dot intro`() { + assertEquals( + 0, + BidirectionalBlurPolicy.selectInstrumentalGapAnchor( + active = setOf(1), + isGap = false, + isOpeningHighlight = true, + instrumentalPositions = listOf(0), + visiblePositions = listOf(1, 2, 3), + ), + ) + assertEquals( + -1, + BidirectionalBlurPolicy.selectInstrumentalGapAnchor( + active = setOf(1), + isGap = false, + isOpeningHighlight = true, + instrumentalPositions = emptyList(), + visiblePositions = listOf(1, 2, 3), + ), + ) + } + + @Test + fun `interlude geometry alone never anchors a mid-song first line`() { + assertEquals( + -1, + BidirectionalBlurPolicy.selectInstrumentalGapAnchor( + active = setOf(9), + isGap = false, + isOpeningHighlight = false, + instrumentalPositions = listOf(8), + visiblePositions = listOf(9, 10, 11), + ), + ) + } + + @Test + fun `opening state alone never anchors when the active line is not earliest visible`() { + assertEquals( + -1, + BidirectionalBlurPolicy.selectInstrumentalGapAnchor( + active = setOf(11), + isGap = false, + isOpeningHighlight = true, + instrumentalPositions = listOf(8), + visiblePositions = listOf(9, 10, 11), + ), + ) + } + + @Test + fun `mid-song instrumentals never anchor a non-gap active highlight`() { + assertEquals( + -1, + BidirectionalBlurPolicy.selectInstrumentalGapAnchor( + active = setOf(13), + isGap = false, + isOpeningHighlight = false, + instrumentalPositions = listOf(12), + // Visible lyric rows exclude the instrumental row (see applyBlur partitioning). + visiblePositions = listOf(13, 14, 15), ), ) } diff --git a/app/src/test/java/dev/amenhancer/module/hook/LyricBlurClearAlphaStructuralRegressionTest.kt b/app/src/test/java/dev/amenhancer/module/hook/LyricBlurClearAlphaStructuralRegressionTest.kt new file mode 100644 index 0000000..c906770 --- /dev/null +++ b/app/src/test/java/dev/amenhancer/module/hook/LyricBlurClearAlphaStructuralRegressionTest.kt @@ -0,0 +1,43 @@ +package dev.amenhancer.module.hook + +import java.io.File +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * STRUCTURAL EXPERIMENT TEST (no Android View test dependencies in this project): + * guards the single-variable experiment that [LyricBlurRenderer.clear] must stop + * force-writing `view.alpha = 1f` on instrumental rows. Hypothesis: that write + * stomps the instrumental interlude three-dot animation, which animates row alpha. + * [clearAll] (fragment destruction) is deliberately out of scope and keeps its write. + */ +class LyricBlurClearAlphaStructuralRegressionTest { + private val rendererSource: String by lazy { + sourceFile("LyricBlurRenderer.kt").readText() + } + + private val clearBody: String by lazy { + rendererSource.substringAfter("fun clear(view: View)") + .substringBefore("fun clearAll()") + } + + @Test + fun `instrumental row clear path stops force-writing alpha`() { + assertFalse( + "LyricBlurRenderer.clear() must not write view.alpha = 1f; it may stomp the interlude dot animation", + clearBody.contains("view.alpha = 1f"), + ) + } + + @Test + fun `clear path keeps its blur state and render effect duties`() { + assertTrue(clearBody.contains("transitions.remove(view)")) + assertTrue(clearBody.contains("view.setRenderEffect(null)")) + } + + private fun sourceFile(name: String): File = sequenceOf( + File("src/main/java/dev/amenhancer/module/hook/$name"), + File("app/src/main/java/dev/amenhancer/module/hook/$name"), + ).firstOrNull(File::isFile) ?: error("$name was not found from the unit-test working directory") +} diff --git a/app/src/test/java/dev/amenhancer/module/hook/LyricHighlightSessionTest.kt b/app/src/test/java/dev/amenhancer/module/hook/LyricHighlightSessionTest.kt index ac95770..094723b 100644 --- a/app/src/test/java/dev/amenhancer/module/hook/LyricHighlightSessionTest.kt +++ b/app/src/test/java/dev/amenhancer/module/hook/LyricHighlightSessionTest.kt @@ -112,6 +112,99 @@ class LyricHighlightSessionTest { assertEquals(setOf(47), session.snapshot()) } + @Test + fun `initial empty callbacks do not count as the opening highlight`() { + val session = LyricHighlightSession() + val song = Any() + + assertTrue(session.enter(song)) + assertFalse(session.isOpeningHighlight()) + session.update(emptySet()) + assertFalse(session.isOpeningHighlight()) + session.update(emptySet()) + assertFalse(session.isOpeningHighlight()) + } + + @Test + fun `the first non-empty update marks the opening highlight`() { + val session = LyricHighlightSession() + + session.update(emptySet()) + assertFalse(session.isOpeningHighlight()) + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + } + + @Test + fun `repeating the first snapshot keeps the opening highlight`() { + val session = LyricHighlightSession() + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + + session.update(emptySet()) + assertTrue(session.isOpeningHighlight()) + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + } + + @Test + fun `the next distinct snapshot clears the opening highlight`() { + val session = LyricHighlightSession() + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + + session.update(setOf(47)) + assertFalse(session.isOpeningHighlight()) + } + + @Test + fun `fallback replacement as the first highlight marks the opening`() { + val session = LyricHighlightSession() + + session.update(emptySet()) + session.replace(47) + assertTrue(session.isOpeningHighlight()) + + session.replace(47) + assertTrue(session.isOpeningHighlight()) + } + + @Test + fun `fallback replacement after a real highlight clears the opening`() { + val session = LyricHighlightSession() + + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + + session.replace(47) + assertFalse(session.isOpeningHighlight()) + } + + @Test + fun `entering another song resets the opening highlight`() { + val session = LyricHighlightSession() + val firstSong = Any() + val nextSong = Any() + + session.enter(firstSong) + session.update(setOf(46)) + assertTrue(session.isOpeningHighlight()) + + assertTrue(session.enter(nextSong)) + assertFalse(session.isOpeningHighlight()) + session.update(emptySet()) + assertFalse(session.isOpeningHighlight()) + session.update(setOf(1)) + assertTrue(session.isOpeningHighlight()) + } + @Test fun `song boundaries use pointer identity rather than value equality`() { val session = LyricHighlightSession() From 96a6b2dd0557cd9549617f68ed435eca74fd1b5e Mon Sep 17 00:00:00 2001 From: Zennmn <176552149+Zennmn@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:17:13 +0800 Subject: [PATCH 2/3] fix: blur lyrics writers credits rows --- .../AppleMusicBidirectionalLyricBlurTarget.kt | 2 + .../module/hook/BidirectionalBlurPolicy.kt | 11 +- .../module/hook/CreditsRowIdentity.kt | 28 +++ .../module/hook/FeatureInstallation.kt | 5 +- .../module/hook/FutureLyricBlurFeature.kt | 11 ++ .../module/hook/OpenSourceLyricBlurPort.kt | 33 +++- ...yricCreditsRowsStructuralRegressionTest.kt | 177 ++++++++++++++++++ 7 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 app/src/main/java/dev/amenhancer/module/hook/CreditsRowIdentity.kt create mode 100644 app/src/test/java/dev/amenhancer/module/hook/LyricCreditsRowsStructuralRegressionTest.kt diff --git a/app/src/main/java/dev/amenhancer/module/hook/AppleMusicBidirectionalLyricBlurTarget.kt b/app/src/main/java/dev/amenhancer/module/hook/AppleMusicBidirectionalLyricBlurTarget.kt index 73854d6..5d59cd8 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/AppleMusicBidirectionalLyricBlurTarget.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/AppleMusicBidirectionalLyricBlurTarget.kt @@ -239,6 +239,8 @@ internal class AppleMusicLyricBlurTargetAccess( override fun isInstrumentalRow(view: View): Boolean = InstrumentalRowIdentity.matches(view) + override fun isCreditsRow(view: View): Boolean = CreditsRowIdentity.matches(view) + override fun adapterPosition(view: View): Int = try { (adapterPositionAccessor?.invoke(null, view) as? Int) ?: -1 } catch (_: Throwable) { diff --git a/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt b/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt index 27bfd51..0adcda3 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/BidirectionalBlurPolicy.kt @@ -5,9 +5,10 @@ import kotlin.math.abs import kotlin.math.roundToInt internal object BidirectionalBlurPolicy { - private const val BLUR_MAX = 22f - private val PAST_RADII_BY_DISTANCE = floatArrayOf(0f, 13f, 17f, BLUR_MAX) - private val FUTURE_RADII_BY_DISTANCE = floatArrayOf(0f, 8f, 13f, 17f, BLUR_MAX) + /** Largest radius any visible lyric row may receive; credits fall back to it only when no lyric row is visible. */ + const val MAX_BLUR_RADIUS = 22f + private val PAST_RADII_BY_DISTANCE = floatArrayOf(0f, 13f, 17f, MAX_BLUR_RADIUS) + private val FUTURE_RADII_BY_DISTANCE = floatArrayOf(0f, 8f, 13f, 17f, MAX_BLUR_RADIUS) const val TRANSITION_DURATION_MS = 300L fun resolveDisplayHighlights( @@ -56,7 +57,7 @@ internal object BidirectionalBlurPolicy { } fun targetRadius(position: Int, highlighted: Set): Float { - if (highlighted.isEmpty()) return BLUR_MAX + if (highlighted.isEmpty()) return MAX_BLUR_RADIUS if (position in highlighted) return 0f return highlighted.minOf { highlightedPosition -> val offset = position - highlightedPosition @@ -65,7 +66,7 @@ internal object BidirectionalBlurPolicy { } else { FUTURE_RADII_BY_DISTANCE } - radii.getOrElse(abs(offset)) { BLUR_MAX } + radii.getOrElse(abs(offset)) { MAX_BLUR_RADIUS } } } diff --git a/app/src/main/java/dev/amenhancer/module/hook/CreditsRowIdentity.kt b/app/src/main/java/dev/amenhancer/module/hook/CreditsRowIdentity.kt new file mode 100644 index 0000000..0c79451 --- /dev/null +++ b/app/src/main/java/dev/amenhancer/module/hook/CreditsRowIdentity.kt @@ -0,0 +1,28 @@ +package dev.amenhancer.module.hook + +import android.view.View +import java.util.Collections +import java.util.WeakHashMap + +/** + * Process-local semantic identity for inflated lyric writers-credits rows. + * + * Credits rows are direct RecyclerView items that the generic lyrics-line heuristic rejects + * (dynamic roots contain an ImageView; static roots are a custom TextView), so the blur runtime + * classifies them through this identity instead. Deliberately independent from + * [InstrumentalRowIdentity]: credits are never three-dot anchors. + */ +internal object CreditsRowIdentity { + val layoutNames: List = listOf( + "lyrics_line_writers_credits", + "lyrics_line_static_writers_credits", + ) + + private val rows = Collections.synchronizedMap(WeakHashMap()) + + fun mark(view: View) { + rows[view] = true + } + + fun matches(view: View): Boolean = rows.containsKey(view) +} diff --git a/app/src/main/java/dev/amenhancer/module/hook/FeatureInstallation.kt b/app/src/main/java/dev/amenhancer/module/hook/FeatureInstallation.kt index 709107f..7d631b8 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/FeatureInstallation.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/FeatureInstallation.kt @@ -206,7 +206,10 @@ private fun productionFeatureInstallationModule(): FeatureInstallationModule { feature = PhoneLiquidGlassFeature(), registerResources = PhoneLiquidGlassResourceHook::install, ), - FeatureInstallationPlan(feature = FutureLyricBlurFeature()), + FeatureInstallationPlan( + feature = FutureLyricBlurFeature(), + registerResources = { LyricCreditsRowResourceHook.install() }, + ), FeatureInstallationPlan( feature = LyricsTypefaceFeature(), registerResources = lyricsTypefaceSession::registerResources, diff --git a/app/src/main/java/dev/amenhancer/module/hook/FutureLyricBlurFeature.kt b/app/src/main/java/dev/amenhancer/module/hook/FutureLyricBlurFeature.kt index caeb0d3..9712c42 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/FutureLyricBlurFeature.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/FutureLyricBlurFeature.kt @@ -16,3 +16,14 @@ internal class FutureLyricBlurFeature : FeatureHook { return context.target.bidirectionalLyricBlur.install().toFeatureInstallResult() } } + +/** Registers the writers-credits lyric rows on the blur feature's own resource path. */ +internal object LyricCreditsRowResourceHook { + fun install() { + CreditsRowIdentity.layoutNames.forEach { layoutName -> + LayoutInflationRegistry.register(layoutName) { root -> + CreditsRowIdentity.mark(root) + } + } + } +} diff --git a/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt b/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt index e5c2c5c..5fe5dc8 100644 --- a/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt +++ b/app/src/main/java/dev/amenhancer/module/hook/OpenSourceLyricBlurPort.kt @@ -235,10 +235,15 @@ internal class OpenSourceLyricBlurPort( val rv = getRv() as? ViewGroup ?: return val visibleRows = ArrayList>(rv.childCount) val instrumentalRows = ArrayList>(1) + val creditsRows = ArrayList>(2) for (i in 0 until rv.childCount) { val child = rv.getChildAt(i) ?: continue val adapterPos = targetAccess.adapterPosition(child) + if (targetAccess.isCreditsRow(child)) { + creditsRows += child to adapterPos + continue + } if (targetAccess.isInstrumentalRow(child)) { instrumentalRows += child to adapterPos continue @@ -260,7 +265,15 @@ internal class OpenSourceLyricBlurPort( gapAnchorPosition = gapAnchorPosition, ) val useTabletEdges = TabletModeQualifier.isEligible(rv.context) - val targets = LinkedHashMap(visibleRows.size) + val targets = LinkedHashMap(visibleRows.size + creditsRows.size) + var lastLyricFocusBlur = if (includeFocus) { + BidirectionalBlurPolicy.applyRadiusOffset( + radius = BidirectionalBlurPolicy.MAX_BLUR_RADIUS, + offsetPx = blurRadiusOffsetPx, + ) + } else { + 0f + } visibleRows.forEach { (child, adapterPos) -> val focusBlur = if (includeFocus) { BidirectionalBlurPolicy.applyRadiusOffset( @@ -270,6 +283,7 @@ internal class OpenSourceLyricBlurPort( } else { 0f } + lastLyricFocusBlur = focusBlur val edgeBlur = if (useTabletEdges) { TabletLyricVisualPolicy.edgeBlurRadius( rowCenterPx = (child.top + child.bottom) / 2f, @@ -284,6 +298,22 @@ internal class OpenSourceLyricBlurPort( isHighlighted = includeFocus && adapterPos in effectiveIds, ) } + creditsRows.forEach { (child, _) -> + val focusBlur = if (includeFocus) lastLyricFocusBlur else 0f + val edgeBlur = if (useTabletEdges) { + TabletLyricVisualPolicy.edgeBlurRadius( + rowCenterPx = (child.top + child.bottom) / 2f, + viewportHeightPx = rv.height.toFloat(), + ) + } else { + 0f + } + targets[child] = TabletLyricVisualPolicy.mergeBlurRadius( + focusBlurRadius = focusBlur, + edgeBlurRadius = edgeBlur, + isHighlighted = false, + ) + } instrumentalRows.forEach { (view, _) -> blurRenderer.clear(view) } if (immediate) { blurRenderer.applyImmediately(targets) @@ -321,5 +351,6 @@ internal interface LyricBlurRuntime { internal interface LyricBlurTargetAccess { fun isRecyclerView(view: View): Boolean fun isInstrumentalRow(view: View): Boolean + fun isCreditsRow(view: View): Boolean fun adapterPosition(view: View): Int } diff --git a/app/src/test/java/dev/amenhancer/module/hook/LyricCreditsRowsStructuralRegressionTest.kt b/app/src/test/java/dev/amenhancer/module/hook/LyricCreditsRowsStructuralRegressionTest.kt new file mode 100644 index 0000000..19fc835 --- /dev/null +++ b/app/src/test/java/dev/amenhancer/module/hook/LyricCreditsRowsStructuralRegressionTest.kt @@ -0,0 +1,177 @@ +package dev.amenhancer.module.hook + +import java.io.File +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Guards the writers-credits row partition seam of the lyric blur runtime. + * + * Credits rows are RecyclerView items that the generic [OpenSourceLyricBlurPort.isLyricsLine] + * heuristic rejects (dynamic roots contain an ImageView; static roots are a custom TextView), so + * they must be classified through their own layout-identity path and inherit the last visible + * lyric row's focus blur instead of being skipped or rendering as the clear focus. + */ +class LyricCreditsRowsStructuralRegressionTest { + private val portSource: String by lazy { + sourceFile("OpenSourceLyricBlurPort.kt").readText() + } + private val featureSource: String by lazy { + sourceFile("FutureLyricBlurFeature.kt").readText() + } + private val installationSource: String by lazy { + sourceFile("FeatureInstallation.kt").readText() + } + private val targetSource: String by lazy { + sourceFile("AppleMusicBidirectionalLyricBlurTarget.kt").readText() + } + private val identitySource: String by lazy { + sourceFile("CreditsRowIdentity.kt").readText() + } + private val instrumentalSource: String by lazy { + sourceFile("InstrumentalRowIdentity.kt").readText() + } + private val contractSource: String by lazy { + sourceFile("LyricsTypefaceSession.kt").readText() + } + + @Test + fun `both credits layouts register on the blur feature's own resource path`() { + assertEquals( + listOf("lyrics_line_writers_credits", "lyrics_line_static_writers_credits"), + CreditsRowIdentity.layoutNames, + ) + assertTrue(featureSource.contains("CreditsRowIdentity.layoutNames.forEach")) + assertTrue(featureSource.contains("LayoutInflationRegistry.register")) + val blurPlan = installationSource + .substringAfter("feature = FutureLyricBlurFeature()") + .substringBefore("FeatureInstallationPlan(") + assertTrue(blurPlan.contains("registerResources")) + assertTrue(blurPlan.contains("LyricCreditsRowResourceHook.install")) + } + + @Test + fun `credits classification keeps a dedicated identity seam`() { + assertTrue(identitySource.contains("fun mark(view: View)")) + assertTrue(identitySource.contains("fun matches(view: View): Boolean")) + assertFalse(instrumentalSource.contains("lyrics_line_writers_credits")) + assertFalse(instrumentalSource.contains("lyrics_line_static_writers_credits")) + } + + @Test + fun `credits stay out of the twelve layout typeface contract`() { + assertEquals(12, LyricsTypefaceLayoutContract.layoutNames.size) + assertFalse(contractSource.contains("lyrics_line_writers_credits")) + assertFalse(contractSource.contains("lyrics_line_static_writers_credits")) + } + + @Test + fun `target access classifies credits through the identity only`() { + assertTrue(portSource.contains("fun isCreditsRow(view: View): Boolean")) + val override = targetSource + .substringAfter("override fun isCreditsRow(") + .substringBefore("\n") + assertTrue(override.contains("CreditsRowIdentity.matches")) + assertFalse(override.contains("java.lang.reflect")) + assertFalse(override.contains("Class<")) + } + + @Test + fun `port partitions credits before the lyrics line heuristic`() { + val applyBlur = portSource.substringAfter("private fun applyBlur(") + val partition = applyBlur + .substringAfter("for (i in 0 until rv.childCount)") + .substringBefore("val activeIds") + assertTrue(partition.contains("if (targetAccess.isCreditsRow(child)) {")) + assertTrue(partition.contains("creditsRows += child to adapterPos")) + assertTrue(partition.contains("if (targetAccess.isInstrumentalRow(child)) {")) + assertTrue(partition.contains("instrumentalRows += child to adapterPos")) + assertTrue(partition.contains("if (!isLyricsLine(child)) continue")) + assertTrue(partition.contains("visibleRows += child to adapterPos")) + assertTrue(partition.indexOf("isCreditsRow") < partition.indexOf("isLyricsLine")) + } + + @Test + fun `credits never reach gap anchors or visible highlight resolution`() { + val applyBlur = portSource.substringAfter("private fun applyBlur(") + val anchors = applyBlur + .substringAfter("selectInstrumentalGapAnchor(") + .substringBefore("val effectiveIds") + assertFalse(anchors.contains("credits")) + val resolution = applyBlur + .substringAfter("resolveDisplayHighlights(") + .substringBefore("val useTabletEdges") + assertFalse(resolution.contains("credits")) + } + + @Test + fun `credits inherit the last visible lyric focus blur without their own radius policy`() { + val creditsBlock = portSource + .substringAfter("creditsRows.forEach { (child, _) ->") + .substringBefore("instrumentalRows.forEach") + assertTrue(creditsBlock.contains("if (includeFocus) lastLyricFocusBlur else 0f")) + assertTrue(creditsBlock.contains("lastLyricFocusBlur")) + assertFalse(creditsBlock.contains("applyRadiusOffset(")) + assertFalse(creditsBlock.contains("MAX_BLUR_RADIUS")) + assertTrue(creditsBlock.contains("0f")) + assertTrue(creditsBlock.contains("TabletLyricVisualPolicy.mergeBlurRadius(")) + assertTrue(creditsBlock.contains("isHighlighted = false")) + assertTrue(portSource.contains("applyBlur(includeFocus = false, immediate = true)")) + } + + @Test + fun `credits focus reuses the last visible lyric row state`() { + val applyBlur = portSource.substringAfter("private fun applyBlur(") + val lyricLoop = applyBlur + .substringAfter("visibleRows.forEach { (child, adapterPos) ->") + .substringBefore("creditsRows.forEach") + assertTrue(lyricLoop.contains("lastLyricFocusBlur = focusBlur")) + assertTrue(lyricLoop.contains("BidirectionalBlurPolicy.applyRadiusOffset(")) + assertTrue(lyricLoop.contains("BidirectionalBlurPolicy.targetRadius(adapterPos, effectiveIds)")) + assertFalse(lyricLoop.contains("MAX_BLUR_RADIUS")) + } + + @Test + fun `credits fall back to maximum blur through the offset policy when no lyric row is visible`() { + val applyBlur = portSource.substringAfter("private fun applyBlur(") + val fallback = applyBlur + .substringAfter("val targets") + .substringBefore("visibleRows.forEach") + assertTrue(fallback.contains("lastLyricFocusBlur")) + assertTrue(fallback.contains("if (includeFocus) {")) + assertTrue(fallback.contains("BidirectionalBlurPolicy.MAX_BLUR_RADIUS")) + assertTrue(fallback.contains("BidirectionalBlurPolicy.applyRadiusOffset(")) + assertTrue(fallback.contains("offsetPx = blurRadiusOffsetPx")) + } + + @Test + fun `credits radius constant is the policy maximum and honors the offset cap`() { + assertEquals(22f, BidirectionalBlurPolicy.MAX_BLUR_RADIUS, FLOAT_TOLERANCE) + assertEquals( + 22f, + BidirectionalBlurPolicy.applyRadiusOffset(BidirectionalBlurPolicy.MAX_BLUR_RADIUS, 0), + FLOAT_TOLERANCE, + ) + assertEquals( + 32f, + BidirectionalBlurPolicy.applyRadiusOffset(BidirectionalBlurPolicy.MAX_BLUR_RADIUS, 10), + FLOAT_TOLERANCE, + ) + assertEquals( + 12f, + BidirectionalBlurPolicy.applyRadiusOffset(BidirectionalBlurPolicy.MAX_BLUR_RADIUS, -10), + FLOAT_TOLERANCE, + ) + } + + private fun sourceFile(name: String): File = sequenceOf( + File("src/main/java/dev/amenhancer/module/hook/$name"), + File("app/src/main/java/dev/amenhancer/module/hook/$name"), + ).firstOrNull(File::isFile) ?: error("$name was not found from the unit-test working directory") + + private companion object { + const val FLOAT_TOLERANCE = 0.0001f + } +} From 38018f9727ee8388877f740a48cfcfaff40e1722 Mon Sep 17 00:00:00 2001 From: Zennmn <176552149+Zennmn@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:30:35 +0800 Subject: [PATCH 3/3] fix: add custom lyrics restore conflict choices --- .../module/lyrics/CustomLyricsManager.kt | 19 +- .../lyrics/CustomLyricsRestoreTransaction.kt | 67 ++++-- .../amenhancer/module/ui/SettingsActivity.kt | 22 +- .../CustomLyricsRestoreTransactionTest.kt | 205 +++++++++++++++++- .../ui/SettingsUiStructuralRegressionTest.kt | 14 +- 5 files changed, 282 insertions(+), 45 deletions(-) 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"))