Episode downloads, with the YouTube path kept separate

Two sources with no fallback between them. A YouTube subscription's media URL
is a watch page, not audio: fetching it over plain HTTP would store an HTML
document under an .mp3 name that fails to play with nothing to explain why.
The subscription kind decides the route.

Audio is written to a .part name and renamed only once complete. A truncated
file recorded as DOWNLOADED plays as a cut-off episode and reads like the
publisher's fault. On failure the partial is removed and localPath cleared,
so a FAILED row never points at a file something downstream might try to play.

A download recorded as complete whose file has since been deleted is fetched
again rather than trusted, because otherwise playback fails on a file the
database insists exists.

File names are derived from the episode id with separators stripped. Ids come
from feeds, and a guid containing a path separator would otherwise write
outside the library directory.

The Chaquopy-backed source sits behind an interface so the downloader's own
logic stays testable off-device; nothing in the real source can run without an
Android Python runtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

4 files changed, 329 insertions, 1 deletions

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)
+    }
+}