mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-20 06:28:27 +02:00
fix(library): reject filename guesses from failed NAS metadata reads
This commit is contained in:
1 parent
cc3fe42df2
commit
115602b84e
3 files changed
+78
-6
No files matched your search
@@ -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 ->
|
||||
|
||||
@@ -7,10 +7,20 @@ import java.io.File
|
||||
internal fun <T> 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 <T> readLyricsWithSafCopy(
|
||||
|
||||
@@ -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<String, Any>("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")
|
||||
|
||||
Reference in new issue
Block a user