diff options
5 files changed, 330 insertions, 2 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index 0186ab3..36d5f03 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -81.69 +80.96 diff --git a/app/src/main/java/org/terst/listen/download/ChaquopyYouTubeSource.kt b/app/src/main/java/org/terst/listen/download/ChaquopyYouTubeSource.kt new file mode 100644 index 0000000..267cc3e --- /dev/null +++ b/app/src/main/java/org/terst/listen/download/ChaquopyYouTubeSource.kt @@ -0,0 +1,22 @@ +package org.terst.listen.download + +import android.content.Context +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.terst.listen.youtube.YouTubeAudio +import java.io.File + +/** + * The real YouTube source, backed by yt-dlp on the embedded interpreter. + * + * Isolated behind [YouTubeDownloadSource] so the downloader's own logic stays testable off + * device -- nothing here can run without an Android Python runtime. + */ +class ChaquopyYouTubeSource(private val context: Context) : YouTubeDownloadSource { + + override suspend fun download(url: String, targetDir: File): File = withContext(Dispatchers.IO) { + // Idempotent, and cheap after the first call. + YouTubeAudio.start(context) + YouTubeAudio.download(url, targetDir) + } +} diff --git a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt new file mode 100644 index 0000000..d07e026 --- /dev/null +++ b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt @@ -0,0 +1,136 @@ +package org.terst.listen.download + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient +import okhttp3.Request +import org.terst.listen.data.DownloadState +import org.terst.listen.data.EpisodeDao +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.SubscriptionKind +import java.io.File +import java.io.IOException + +/** + * Fetches an episode's audio for offline playback. + * + * Two sources, because a YouTube subscription's media URL is a watch page rather than audio. + * Fetching one over plain HTTP would store an HTML document under an .mp3 name that fails to + * play with no indication why -- so the kind decides the route, and there is no fallback + * between them. + */ +class EpisodeDownloader( + private val client: OkHttpClient, + private val episodes: EpisodeDao, + private val libraryDir: File, + private val youtube: YouTubeDownloadSource? = null, + private val now: () -> Long = System::currentTimeMillis, +) { + + suspend fun download(episode: EpisodeEntity, kind: SubscriptionKind): Result<File> { + alreadyOnDisk(episode)?.let { return Result.success(it) } + + episodes.updateDownload(episode.id, DownloadState.DOWNLOADING, null, null) + + val result = when (kind) { + SubscriptionKind.YOUTUBE_CHANNEL -> downloadViaYouTube(episode) + SubscriptionKind.PODCAST -> downloadOverHttp(episode) + } + + result.fold( + onSuccess = { episodes.updateDownload(episode.id, DownloadState.DOWNLOADED, it.absolutePath, now()) }, + // localPath is cleared: a FAILED row pointing at a path invites something downstream + // to try playing a file that is not there. + onFailure = { episodes.updateDownload(episode.id, DownloadState.FAILED, null, null) }, + ) + return result + } + + /** + * A recorded download whose file has since been deleted has to be fetched again, not treated + * as present -- otherwise playback fails on a file the database insists exists. + */ + private fun alreadyOnDisk(episode: EpisodeEntity): File? { + if (episode.downloadState != DownloadState.DOWNLOADED) return null + val path = episode.localPath ?: return null + return File(path).takeIf { it.isFile && it.length() > 0 } + } + + private suspend fun downloadViaYouTube(episode: EpisodeEntity): Result<File> { + val source = youtube + ?: return Result.failure(IllegalStateException("No YouTube downloader configured")) + return runCatching { source.download(episode.mediaUrl, libraryDir) } + } + + private suspend fun downloadOverHttp(episode: EpisodeEntity): Result<File> = + withContext(Dispatchers.IO) { + val target = targetFile(episode) + // Written to a partial name and renamed on success. A truncated file recorded as + // downloaded plays as a cut-off episode and reads like the publisher's fault. + val partial = File(target.absolutePath + ".part") + partial.parentFile?.mkdirs() + + try { + val request = Request.Builder().url(episode.mediaUrl).header("User-Agent", USER_AGENT).build() + client.newCall(request).execute().use { response -> + if (!response.isSuccessful) { + partial.delete() + return@withContext Result.failure( + IOException("HTTP ${response.code} ${response.message}".trim()) + ) + } + val body = response.body ?: run { + partial.delete() + return@withContext Result.failure(IOException("Empty response")) + } + body.byteStream().use { input -> + partial.outputStream().use { output -> input.copyTo(output) } + } + } + if (!partial.renameTo(target)) { + partial.delete() + return@withContext Result.failure(IOException("Could not store ${target.name}")) + } + Result.success(target) + } catch (e: IOException) { + partial.delete() + Result.failure(e) + } + } + + /** + * Names the file from the episode id, sanitised. + * + * Ids come from feeds, and a guid containing path separators would otherwise write outside + * the library. + */ + private fun targetFile(episode: EpisodeEntity): File { + val safeId = episode.id.map { if (it.isLetterOrDigit() || it == '-' || it == '_') it else '_' } + .joinToString("") + .ifBlank { "episode" } + .take(MAX_NAME_LENGTH) + val extension = extensionFor(episode.mediaType) + return File(libraryDir, "$safeId$extension") + } + + private fun extensionFor(mediaType: String) = when (mediaType.substringBefore(';').trim().lowercase()) { + "audio/mpeg", "audio/mp3" -> ".mp3" + "audio/mp4", "audio/x-m4a", "audio/aac" -> ".m4a" + "audio/opus" -> ".opus" + "audio/ogg" -> ".ogg" + "audio/flac" -> ".flac" + "audio/wav", "audio/x-wav" -> ".wav" + "video/mp4" -> ".mp4" + else -> ".audio" + } + + private companion object { + const val USER_AGENT = "Listen/1.0 (Android podcast client)" + const val MAX_NAME_LENGTH = 120 + } +} + +/** Indirection so the Chaquopy-backed downloader can be swapped out in tests. */ +interface YouTubeDownloadSource { + suspend fun download(url: String, targetDir: File): File +} diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt index e28786c..821e1b2 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -49,6 +49,9 @@ class FakeSubscriptionDao : SubscriptionDao { class FakeEpisodeDao : EpisodeDao { val rows = mutableListOf<EpisodeEntity>() + /** Lets a test observe intermediate states that the final row no longer shows. */ + var onUpdate: ((EpisodeEntity) -> Unit)? = null + override fun observeForSubscription(subscriptionId: String): Flow<List<EpisodeEntity>> = flowOf(rows.filter { it.subscriptionId == subscriptionId }) @@ -105,7 +108,10 @@ class FakeEpisodeDao : EpisodeDao { private fun replace(id: String, transform: (EpisodeEntity) -> EpisodeEntity) { val index = rows.indexOfFirst { it.id == id } - if (index >= 0) rows[index] = transform(rows[index]) + if (index >= 0) { + rows[index] = transform(rows[index]) + onUpdate?.invoke(rows[index]) + } } } diff --git a/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt b/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt new file mode 100644 index 0000000..d5e971b --- /dev/null +++ b/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt @@ -0,0 +1,164 @@ +package org.terst.listen.download + +import kotlinx.coroutines.test.runTest +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.terst.listen.data.DownloadState +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.FakeEpisodeDao +import org.terst.listen.data.SubscriptionKind +import java.io.File + +class EpisodeDownloaderTest { + + @get:Rule val temp = TemporaryFolder() + + private lateinit var server: MockWebServer + private lateinit var dao: FakeEpisodeDao + private lateinit var downloader: EpisodeDownloader + private lateinit var library: File + + private fun episode(id: String = "ep-1", url: String = server.url("/a.mp3").toString()) = + EpisodeEntity( + id = id, + subscriptionId = "s1", + guid = id, + title = "Episode One", + mediaUrl = url, + mediaType = "audio/mpeg", + discoveredAtMs = 0, + ) + + @Before + fun setUp() { + server = MockWebServer() + server.start() + dao = FakeEpisodeDao() + library = temp.newFolder("audio") + downloader = EpisodeDownloader(OkHttpClient(), dao, library, null) { 1_000L } + } + + @After + fun tearDown() = server.shutdown() + + @Test + fun `downloads an episode and records it`() = runTest { + server.enqueue(MockResponse().setBody("audio-bytes")) + val ep = episode().also { dao.rows += it } + + val result = downloader.download(ep, SubscriptionKind.PODCAST) + assertTrue(result.isSuccess) + + val stored = dao.rows.single() + assertEquals(DownloadState.DOWNLOADED, stored.downloadState) + assertEquals(1_000L, stored.downloadedAtMs) + assertEquals("audio-bytes", File(stored.localPath!!).readText()) + } + + /** + * A part-written file that gets recorded as DOWNLOADED plays as a truncated episode and + * looks like the publisher's fault. Nothing lands under its real name until it is complete. + */ + @Test + fun `a failed download leaves no playable file`() = runTest { + server.enqueue(MockResponse().setResponseCode(500)) + val ep = episode().also { dao.rows += it } + + val result = downloader.download(ep, SubscriptionKind.PODCAST) + assertTrue(result.isFailure) + + val stored = dao.rows.single() + assertEquals(DownloadState.FAILED, stored.downloadState) + assertNull(stored.localPath) + assertEquals("nothing should be left in the library", 0, library.listFiles()!!.size) + } + + @Test + fun `no partial files are left behind after a failure`() = runTest { + server.enqueue(MockResponse().setResponseCode(404)) + val ep = episode().also { dao.rows += it } + downloader.download(ep, SubscriptionKind.PODCAST) + assertTrue(library.walkTopDown().none { it.name.endsWith(".part") }) + } + + @Test + fun `an unreachable host fails without throwing`() = runTest { + val url = server.url("/a.mp3").toString() + server.shutdown() + val ep = episode(url = url).also { dao.rows += it } + assertTrue(downloader.download(ep, SubscriptionKind.PODCAST).isFailure) + assertEquals(DownloadState.FAILED, dao.rows.single().downloadState) + } + + /** Re-downloading something already present should be free. */ + @Test + fun `an already-downloaded episode is not fetched again`() = runTest { + server.enqueue(MockResponse().setBody("audio-bytes")) + val ep = episode().also { dao.rows += it } + downloader.download(ep, SubscriptionKind.PODCAST) + + val requestsBefore = server.requestCount + downloader.download(dao.rows.single(), SubscriptionKind.PODCAST) + assertEquals(requestsBefore, server.requestCount) + } + + /** A recorded download whose file has been deleted must re-fetch, not fail to play. */ + @Test + fun `a missing local file is downloaded again`() = runTest { + server.enqueue(MockResponse().setBody("audio-bytes")) + val ep = episode().also { dao.rows += it } + downloader.download(ep, SubscriptionKind.PODCAST) + File(dao.rows.single().localPath!!).delete() + + server.enqueue(MockResponse().setBody("audio-bytes")) + val result = downloader.download(dao.rows.single(), SubscriptionKind.PODCAST) + assertTrue(result.isSuccess) + assertTrue(File(dao.rows.single().localPath!!).exists()) + } + + @Test + fun `the file name cannot escape the library directory`() = runTest { + server.enqueue(MockResponse().setBody("x")) + val ep = episode(id = "../../escape").also { dao.rows += it } + downloader.download(ep, SubscriptionKind.PODCAST) + + val path = dao.rows.single().localPath + if (path != null) { + assertTrue("escaped the library: $path", + File(path).canonicalPath.startsWith(library.canonicalPath)) + } + assertFalse(File(library.parentFile, "escape").exists()) + } + + @Test + fun `marks the episode as downloading while in flight`() = runTest { + server.enqueue(MockResponse().setBody("audio-bytes")) + val ep = episode().also { dao.rows += it } + val seen = mutableListOf<DownloadState>() + dao.onUpdate = { seen += it.downloadState } + + downloader.download(ep, SubscriptionKind.PODCAST) + assertTrue("progress should be visible, saw $seen", seen.contains(DownloadState.DOWNLOADING)) + } + + /** + * A YouTube subscription's media URL is a watch page, not audio. Fetching it directly would + * store an HTML document with an .mp3 name that fails to play with no clue why. + */ + @Test + fun `a youtube episode is not fetched over plain http`() = runTest { + val ep = episode(url = "https://www.youtube.com/watch?v=abc").also { dao.rows += it } + downloader.download(ep, SubscriptionKind.YOUTUBE_CHANNEL) + assertEquals("no plain HTTP request should have been made", 0, server.requestCount) + } +} |