diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt index 3a2d6f70..ad0e2e5f 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafIo.kt @@ -272,7 +272,8 @@ private fun MainActivity.readMetadataFromUri( uri: Uri, displayNameHint: String? = null, fallbackExt: String? = null, - acceptDirect: (JSONObject) -> Boolean = { true }, + acceptRead: (JSONObject) -> Boolean = { true }, + retryDirectOnFailure: Boolean = false, read: (String, String) -> JSONObject?, ): JSONObject? { val displayName = buildUriDisplayName(uri, displayNameHint, fallbackExt) @@ -280,17 +281,19 @@ private fun MainActivity.readMetadataFromUri( directRead = { contentResolver.openFileDescriptor(uri, "r")?.use { descriptor -> if (!isSeekableSafDescriptor(descriptor)) return@use null - read("/proc/self/fd/${descriptor.fd}", displayName)?.takeIf(acceptDirect) + read("/proc/self/fd/${descriptor.fd}", displayName) } }, fallbackRead = { - val tempPath = copyUriToTemp(uri, fallbackExt) + val tempPath = copyUriToTemp(uri, fallbackExt, displayName = displayName) if (tempPath == null) null else try { read(tempPath, displayName) } finally { try { File(tempPath).delete() } catch (_: Exception) {} } }, + accept = acceptRead, + retryDirectOnFailure = retryDirectOnFailure, ) } @@ -301,7 +304,17 @@ internal fun MainActivity.readAudioMetadataFromUri( coverCacheKey: String = "", ): JSONObject? = readMetadataFromUri( uri, displayNameHint, fallbackExt, - acceptDirect = { !it.optBoolean("metadataFromFilename", false) }, + acceptRead = { + val fromFilename = it.optBoolean("metadataFromFilename", false) + if (fromFilename) { + android.util.Log.w( + "SpotiFLAC", + "SAF metadata tags unreadable; ignoring filename-derived metadata", + ) + } + !fromFilename + }, + retryDirectOnFailure = true, ) { path, name -> if (name.endsWith(".dsf", true) || name.endsWith(".dff", true) || name.endsWith(".wv", true)) { DsdSource.open(path)?.use { source -> diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/SafMetadataReadPolicy.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/SafMetadataReadPolicy.kt index 84ca629e..3b0d38a2 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/SafMetadataReadPolicy.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/SafMetadataReadPolicy.kt @@ -7,10 +7,20 @@ import java.io.File internal fun readSafMetadataWithFallback( directRead: () -> T?, fallbackRead: () -> T?, + accept: (T) -> Boolean = { true }, + retryDirectOnFailure: Boolean = false, ): T? { - val direct = try { directRead() } catch (_: Exception) { null } + fun attempt(read: () -> T?): T? = try { + read()?.takeIf(accept) + } catch (_: Exception) { null } + + val direct = attempt(directRead) if (direct != null) return direct - return try { fallbackRead() } catch (_: Exception) { null } + val copied = attempt(fallbackRead) + if (copied != null) return copied + // A remote provider may have completed its download/cache by now. Reopen + // once; never repeatedly copy whole songs or publish filename guesses. + return if (retryDirectOnFailure) attempt(directRead) else null } internal fun readLyricsWithSafCopy( diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/SafMetadataReadPolicyTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/SafMetadataReadPolicyTest.kt index 84e3e760..f8d5df63 100644 --- a/android/app/src/test/kotlin/com/zarz/spotiflac/SafMetadataReadPolicyTest.kt +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/SafMetadataReadPolicyTest.kt @@ -8,6 +8,55 @@ import org.junit.Test import java.io.File class SafMetadataReadPolicyTest { + @Test fun rejectsTemporaryWorkspaceMetadataAndRetriesDescriptorOnce() { + val guessed = mapOf( + "metadataFromFilename" to true, + "albumName" to "native-media-work", + "trackName" to "Artist", + "artistName" to "Song", + ) + val tags = mapOf("trackName" to "Song", "artistName" to "Artist", "albumName" to "Album") + var reads = 0 + var copies = 0 + val result = readSafMetadataWithFallback( + directRead = { if (++reads == 1) guessed else tags }, + fallbackRead = { copies++; guessed }, + accept = { it["metadataFromFilename"] != true }, + retryDirectOnFailure = true, + ) + assertEquals(tags, result) + assertEquals(2, reads) + assertEquals(1, copies) + } + + @Test fun persistentNasReadFailureDoesNotPublishGuessedMetadataOrLoop() { + var reads = 0 + var copies = 0 + val guessed = mapOf("metadataFromFilename" to true, "albumName" to "native-media-work") + val result = readSafMetadataWithFallback( + directRead = { reads++; guessed }, + fallbackRead = { copies++; guessed }, + accept = { it["metadataFromFilename"] != true }, + retryDirectOnFailure = true, + ) + assertNull(result) + assertEquals(2, reads) + assertEquals(1, copies) + } + + @Test fun validCopiedTagsDoNotTriggerAnotherRemoteRead() { + var reads = 0 + val tags = mapOf("trackName" to "Song", "albumName" to "Album") + val result = readSafMetadataWithFallback( + directRead = { reads++; throw java.io.IOException("Provider not ready") }, + fallbackRead = { tags }, + accept = { it["metadataFromFilename"] != true }, + retryDirectOnFailure = true, + ) + assertEquals(tags, result) + assertEquals(1, reads) + } + @Test fun lyricsReadUsesSafCopyAndDeletesItAfterSuccessOrFailure() { for (fails in listOf(false, true)) { val temporary = File.createTempFile("lyrics_", ".flac")