Skip to content

Commit 82e7fb2

Browse files
frettclaude
frett
and
claude
committed
Prefer CDN when downloading published translation files, falling back to API
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c778f34 commit 82e7fb2

2 files changed

Lines changed: 177 additions & 22 deletions

File tree

‎library/download-manager/src/main/kotlin/org/cru/godtools/downloadmanager/GodToolsDownloadManager.kt‎

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import kotlinx.coroutines.flow.transformLatest
4343
import kotlinx.coroutines.launch
4444
import kotlinx.coroutines.sync.withLock
4545
import kotlinx.coroutines.withContext
46+
import okhttp3.ResponseBody
4647
import okio.ByteString.Companion.decodeHex
4748
import okio.HashingSource.Companion.sha256
4849
import okio.buffer
@@ -53,6 +54,7 @@ import org.ccci.gto.android.common.kotlin.coroutines.flow.EmptyStateFlow
5354
import org.ccci.gto.android.common.kotlin.coroutines.flow.combineTransformLatest
5455
import org.ccci.gto.android.common.kotlin.coroutines.withLock
5556
import org.cru.godtools.api.AttachmentsApi
57+
import org.cru.godtools.api.CdnApi
5658
import org.cru.godtools.api.TranslationsApi
5759
import org.cru.godtools.base.Settings
5860
import org.cru.godtools.base.ToolFileSystem
@@ -70,6 +72,7 @@ import org.cru.godtools.model.Translation
7072
import org.cru.godtools.model.TranslationKey
7173
import org.cru.godtools.shared.tool.parser.ManifestParser
7274
import org.cru.godtools.shared.tool.parser.ParserResult
75+
import retrofit2.Response
7376

7477
@VisibleForTesting
7578
internal const val CLEANUP_DELAY = 30_000L
@@ -78,6 +81,7 @@ internal const val CLEANUP_DELAY = 30_000L
7881
class GodToolsDownloadManager @VisibleForTesting internal constructor(
7982
private val attachmentsApi: AttachmentsApi,
8083
private val attachmentsRepository: AttachmentsRepository,
84+
private val cdnApi: CdnApi,
8185
private val downloadedFilesRepository: DownloadedFilesRepository,
8286
private val fs: ToolFileSystem,
8387
private val manifestParser: ManifestParser,
@@ -91,6 +95,7 @@ class GodToolsDownloadManager @VisibleForTesting internal constructor(
9195
internal constructor(
9296
attachmentsApi: AttachmentsApi,
9397
attachmentsRepository: AttachmentsRepository,
98+
cdnApi: CdnApi,
9499
downloadedFilesRepository: DownloadedFilesRepository,
95100
fs: ToolFileSystem,
96101
manifestParser: ManifestParser,
@@ -100,6 +105,7 @@ class GodToolsDownloadManager @VisibleForTesting internal constructor(
100105
) : this(
101106
attachmentsApi,
102107
attachmentsRepository,
108+
cdnApi,
103109
downloadedFilesRepository,
104110
fs,
105111
manifestParser,
@@ -284,7 +290,7 @@ class GodToolsDownloadManager @VisibleForTesting internal constructor(
284290
private suspend fun downloadTranslationFiles(translation: Translation): Boolean = filesystemMutex.read.withLock {
285291
// download manifest if necessary
286292
val manifestFileName = translation.manifestFileName ?: return false
287-
if (!downloadTranslationFileIfNecessary(manifestFileName)) return false
293+
if (!downloadPublishedFileIfNecessary(manifestFileName)) return false
288294

289295
// parse manifest
290296
val parserResult = manifestParser.parseManifest(
@@ -301,7 +307,11 @@ class GodToolsDownloadManager @VisibleForTesting internal constructor(
301307
relatedFiles.map { file ->
302308
async {
303309
val src = file.src ?: return@async true
304-
downloadTranslationFileIfNecessary(src, sha256 = file.checksumSha256, size = file.size).also {
310+
downloadPublishedFileIfNecessary(
311+
fileName = src,
312+
sha256 = file.checksumSha256,
313+
size = file.size?.toLong()
314+
).also {
305315
do {
306316
val completed = completedFiles.get()
307317
updateProgress(key, completed + 1, relatedFiles.size.toLong())
@@ -322,35 +332,53 @@ class GodToolsDownloadManager @VisibleForTesting internal constructor(
322332
return true
323333
}
324334

325-
private suspend fun downloadTranslationFileIfNecessary(
335+
private suspend fun downloadPublishedFileIfNecessary(
326336
fileName: String,
327337
sha256: String? = null,
328338
size: Long? = null,
329-
): Boolean = filesMutex.withLock(fileName) {
339+
): Boolean = filesMutex[fileName].withLock {
330340
if (downloadedFilesRepository.findDownloadedFile(fileName) != null) return true
331341

332-
return withContext(Dispatchers.IO) {
333-
try {
334-
val body = translationsApi.downloadFile(fileName).takeIf { it.isSuccessful }?.body()
335-
?: return@withContext false
342+
withContext(ioDispatcher) {
343+
downloadPublishedFileFromCdn(fileName, sha256, size) ||
344+
downloadPublishedFileFromApi(fileName, sha256, size)
345+
}
346+
}
336347

337-
val downloadedFile = DownloadedFile(fileName)
338-
downloadedFile.bufferedSink().use { sink ->
339-
body.source().use { source ->
340-
val digest = sha256(source)
341-
val bytesWritten = sink.writeAll(digest)
348+
private suspend fun downloadPublishedFileFromCdn(fileName: String, sha256: String?, size: Long?) = try {
349+
cdnApi.downloadPublishedFile(fileName).storeFile(fileName, sha256, size)
350+
} catch (_: IOException) {
351+
false
352+
}
342353

343-
if (size != null && size != bytesWritten) return@withContext false
344-
if (sha256 != null && sha256.decodeHex() != digest.hash) return@withContext false
345-
}
346-
}
354+
private suspend fun downloadPublishedFileFromApi(fileName: String, sha256: String?, size: Long?) = try {
355+
translationsApi.downloadFile(fileName).storeFile(fileName, sha256, size)
356+
} catch (_: IOException) {
357+
false
358+
}
347359

348-
downloadedFilesRepository.insertOrIgnore(downloadedFile)
349-
true
350-
} catch (_: IOException) {
351-
false
360+
private suspend fun Response<ResponseBody>.storeFile(fileName: String, sha256: String?, size: Long?): Boolean {
361+
val body = body().takeIf { isSuccessful } ?: return false
362+
363+
val downloadedFile = DownloadedFile(fileName)
364+
val valid = downloadedFile.bufferedSink().use { sink ->
365+
body.source().use { source ->
366+
val digest = sha256(source)
367+
val bytesWritten = sink.writeAll(digest)
368+
when {
369+
size != null && size != bytesWritten -> false
370+
sha256 != null && sha256.decodeHex() != digest.hash -> false
371+
else -> true
372+
}
352373
}
353374
}
375+
if (!valid) {
376+
downloadedFile.getFile(fs).delete()
377+
return false
378+
}
379+
380+
downloadedFilesRepository.insertOrIgnore(downloadedFile)
381+
return true
354382
}
355383

356384
private suspend fun downloadTranslationZip(translation: Translation) = try {

‎library/download-manager/src/test/kotlin/org/cru/godtools/downloadmanager/GodToolsDownloadManagerTest.kt‎

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,14 @@ import kotlinx.coroutines.test.UnconfinedTestDispatcher
4141
import kotlinx.coroutines.test.advanceTimeBy
4242
import kotlinx.coroutines.test.runCurrent
4343
import kotlinx.coroutines.test.runTest
44+
import okhttp3.ResponseBody.Companion.toResponseBody
4445
import okhttp3.internal.http.RealResponseBody
4546
import okio.Buffer
47+
import okio.ByteString.Companion.toByteString
4648
import okio.buffer
4749
import okio.source
4850
import org.cru.godtools.api.AttachmentsApi
51+
import org.cru.godtools.api.CdnApi
4952
import org.cru.godtools.api.TranslationsApi
5053
import org.cru.godtools.base.ToolFileSystem
5154
import org.cru.godtools.db.repository.AttachmentsRepository
@@ -55,6 +58,7 @@ import org.cru.godtools.downloadmanager.work.WORK_NAME_DOWNLOAD_ATTACHMENT
5558
import org.cru.godtools.model.Attachment
5659
import org.cru.godtools.model.DownloadedFile
5760
import org.cru.godtools.model.DownloadedTranslationFile
61+
import org.cru.godtools.model.Translation
5862
import org.cru.godtools.model.TranslationKey
5963
import org.cru.godtools.model.randomTranslation
6064
import org.cru.godtools.shared.tool.parser.ManifestParser
@@ -75,6 +79,9 @@ class GodToolsDownloadManagerTest {
7579

7680
private val attachmentsApi = mockk<AttachmentsApi>()
7781
private val attachmentsRepository: AttachmentsRepository = mockk(relaxUnitFun = true)
82+
private val cdnApi: CdnApi = mockk {
83+
coEvery { downloadPublishedFile(any()) } returns Response.error(404, "".toResponseBody())
84+
}
7885
private val downloadedFilesRepository: DownloadedFilesRepository = mockk(relaxUnitFun = true) {
7986
coEvery { findDownloadedFile(any()) } returns null
8087
coEvery { getDownloadedFiles() } returns emptyList()
@@ -90,9 +97,13 @@ class GodToolsDownloadManagerTest {
9097
every { defaultConfig } returns ParserConfig()
9198
excludeRecords { defaultConfig }
9299
}
93-
private val translationsApi = mockk<TranslationsApi>()
100+
private val translationsApi: TranslationsApi = mockk {
101+
coEvery { download(any()) } returns Response.error(404, "".toResponseBody())
102+
coEvery { downloadFile(any()) } returns Response.error(404, "".toResponseBody())
103+
}
94104
private val translationsRepository: TranslationsRepository = mockk {
95105
coEvery { markStaleTranslationsAsNotDownloaded() } returns false
106+
coEvery { markTranslationDownloaded(any(), any()) } just Runs
96107
}
97108
private val workManager: WorkManager = mockk {
98109
every { enqueueUniqueWork(any(), any(), any<OneTimeWorkRequest>()) } returns mockk()
@@ -102,6 +113,7 @@ class GodToolsDownloadManagerTest {
102113
private val downloadManager = GodToolsDownloadManager(
103114
attachmentsApi,
104115
attachmentsRepository,
116+
cdnApi = cdnApi,
105117
downloadedFilesRepository,
106118
fs,
107119
manifestParser,
@@ -434,6 +446,121 @@ class GodToolsDownloadManagerTest {
434446
}
435447
// endregion downloadLatestPublishedTranslation()
436448

449+
// region downloadPublishedFileIfNecessary
450+
private fun setupTranslationFilesDownload(translation: Translation, manifest: Manifest) {
451+
coEvery { translationsRepository.findLatestTranslation(translation.toolCode, translation.languageCode) }
452+
.returns(translation)
453+
coEvery { manifestParser.parseManifest(translation.manifestFileName!!, any()) }
454+
.returns(ParserResult.Data(manifest))
455+
coEvery { translationsApi.downloadFile(translation.manifestFileName!!) }
456+
.returns(Response.success(RealResponseBody(null, 0, Buffer().writeUtf8("manifest"))))
457+
}
458+
459+
@Test
460+
fun `downloadPublishedFileIfNecessary - prefers CDN over API`() = testScope.runTest {
461+
downloadManager.cleanupActor.close()
462+
val fileContent = "a".repeat(1024)
463+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
464+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml")))
465+
setupTranslationFilesDownload(translation, manifest)
466+
coEvery { cdnApi.downloadPublishedFile("file.xml") } returns Response.success(fileContent.toResponseBody())
467+
468+
assertTrue(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
469+
assertEquals(fileContent, files["file.xml"]!!.readText())
470+
coVerify { cdnApi.downloadPublishedFile("file.xml") }
471+
coVerify(exactly = 0) { translationsApi.downloadFile("file.xml") }
472+
}
473+
474+
@Test
475+
fun `downloadPublishedFileIfNecessary - falls back to API on CDN 404`() = testScope.runTest {
476+
downloadManager.cleanupActor.close()
477+
val fileContent = "a".repeat(1024)
478+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
479+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml")))
480+
setupTranslationFilesDownload(translation, manifest)
481+
coEvery { translationsApi.downloadFile("file.xml") } returns Response.success(fileContent.toResponseBody())
482+
483+
assertTrue(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
484+
assertEquals(fileContent, files["file.xml"]!!.readText())
485+
coVerify { cdnApi.downloadPublishedFile("file.xml") }
486+
coVerify { translationsApi.downloadFile("file.xml") }
487+
}
488+
489+
@Test
490+
fun `downloadPublishedFileIfNecessary - falls back to API on CDN IOException`() = testScope.runTest {
491+
downloadManager.cleanupActor.close()
492+
val fileContent = "a".repeat(1024)
493+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
494+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml")))
495+
setupTranslationFilesDownload(translation, manifest)
496+
coEvery { cdnApi.downloadPublishedFile("file.xml") } throws IOException()
497+
coEvery { translationsApi.downloadFile("file.xml") } returns Response.success(fileContent.toResponseBody())
498+
499+
assertTrue(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
500+
assertEquals(fileContent, files["file.xml"]!!.readText())
501+
coVerify { cdnApi.downloadPublishedFile("file.xml") }
502+
coVerify { translationsApi.downloadFile("file.xml") }
503+
}
504+
505+
@Test
506+
fun `downloadPublishedFileIfNecessary - returns false when both CDN and API fail`() = testScope.runTest {
507+
downloadManager.cleanupActor.close()
508+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
509+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml")))
510+
setupTranslationFilesDownload(translation, manifest)
511+
512+
assertFalse(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
513+
coVerify { cdnApi.downloadPublishedFile("file.xml") }
514+
coVerify { translationsApi.downloadFile("file.xml") }
515+
}
516+
517+
@Test
518+
fun `downloadPublishedFileIfNecessary - accepts file with matching sha256`() = testScope.runTest {
519+
downloadManager.cleanupActor.close()
520+
val fileContent = "a".repeat(1024)
521+
val sha256Hex = fileContent.toByteArray().toByteString().sha256().hex()
522+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
523+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml", checksumSha256 = sha256Hex)))
524+
setupTranslationFilesDownload(translation, manifest)
525+
coEvery { cdnApi.downloadPublishedFile("file.xml") } returns Response.success(fileContent.toResponseBody())
526+
527+
assertTrue(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
528+
coVerify { downloadedFilesRepository.insertOrIgnore(DownloadedFile("file.xml")) }
529+
}
530+
531+
@Test
532+
fun `downloadPublishedFileIfNecessary - rejects file with wrong sha256`() = testScope.runTest {
533+
downloadManager.cleanupActor.close()
534+
val fileContent = "a".repeat(1024)
535+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
536+
val manifest = Manifest(
537+
pageXmlFiles = listOf(XmlFile("file.xml", "file.xml", checksumSha256 = "00".repeat(32))),
538+
)
539+
setupTranslationFilesDownload(translation, manifest)
540+
coEvery { cdnApi.downloadPublishedFile("file.xml") } returns Response.success(fileContent.toResponseBody())
541+
coEvery { translationsApi.downloadFile("file.xml") } returns Response.success(fileContent.toResponseBody())
542+
543+
assertFalse(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
544+
coVerify(exactly = 0) { downloadedFilesRepository.insertOrIgnore(DownloadedFile("file.xml")) }
545+
assertFalse(files["file.xml"]!!.exists())
546+
}
547+
548+
@Test
549+
fun `downloadPublishedFileIfNecessary - rejects file with wrong size`() = testScope.runTest {
550+
downloadManager.cleanupActor.close()
551+
val fileContent = "a".repeat(1024)
552+
val translation = randomTranslation(manifestFileName = "manifest.xml", isDownloaded = false)
553+
val manifest = Manifest(pageXmlFiles = listOf(XmlFile("file.xml", "file.xml", size = 9999)))
554+
setupTranslationFilesDownload(translation, manifest)
555+
coEvery { cdnApi.downloadPublishedFile("file.xml") } returns Response.success(fileContent.toResponseBody())
556+
coEvery { translationsApi.downloadFile("file.xml") } returns Response.success(fileContent.toResponseBody())
557+
558+
assertFalse(downloadManager.downloadLatestPublishedTranslation(TranslationKey(translation)))
559+
coVerify(exactly = 0) { downloadedFilesRepository.insertOrIgnore(DownloadedFile("file.xml")) }
560+
assertFalse(files["file.xml"]!!.exists())
561+
}
562+
// endregion downloadPublishedFileIfNecessary
563+
437564
@Test
438565
fun verifyImportTranslation() = testScope.runTest {
439566
coEvery { translationsRepository.findLatestTranslation(any(), any(), any()) } returns null

0 commit comments

Comments
 (0)