Skip to content

Commit 9f098ac

Browse files
committed
Refactor downloader to support unique download IDs and prevent duplicate downloads
- Introduced a unique `downloadId` (UUID) for each download session in both `AndroidDownloader` and `DesktopDownloader`. - Added a `ConcurrentHashMap` to track active file names and prevent multiple concurrent downloads of the same file. - Updated `activeDownloads` to map by `downloadId` instead of file name to improve tracking accuracy. - Enhanced `cancelDownload` logic to look up and remove downloads using the new ID-based mapping. - Added validation checks to throw an `IllegalStateException` if a download for a specific file is already in progress. - Cleaned up file existence checks and logging across both Android and JVM implementations.
1 parent def0a55 commit 9f098ac

2 files changed

Lines changed: 44 additions & 36 deletions

File tree

core/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.kt

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class AndroidDownloader(
2828
) : Downloader {
2929

3030
private val activeDownloads = ConcurrentHashMap<String, Call>()
31+
private val activeFileNames = ConcurrentHashMap<String, String>()
3132

3233
private fun buildClient(): OkHttpClient {
3334
Authenticator.setDefault(null)
@@ -81,21 +82,25 @@ class AndroidDownloader(
8182
"Invalid file name: $rawName"
8283
}
8384

84-
val destination = File(dir, safeName)
85+
check(!activeFileNames.containsKey(safeName)) {
86+
"A download for '$safeName' is already in progress"
87+
}
88+
89+
val downloadId = UUID.randomUUID().toString()
8590

91+
val destination = File(dir, safeName)
8692
if (destination.exists()) {
8793
Logger.d { "Deleting existing file before download: ${destination.absolutePath}" }
8894
destination.delete()
8995
}
9096

91-
Logger.d { "Starting download: $url" }
92-
93-
val request = Request.Builder()
94-
.url(url)
95-
.build()
97+
Logger.d { "Starting download: $url (id=$downloadId)" }
9698

99+
val request = Request.Builder().url(url).build()
97100
val call = client.newCall(request)
98-
activeDownloads[safeName] = call
101+
102+
activeDownloads[downloadId] = call
103+
activeFileNames[safeName] = downloadId
99104

100105
try {
101106
call.execute().use { response ->
@@ -132,13 +137,13 @@ class AndroidDownloader(
132137
throw IllegalStateException("File not ready after download: ${destination.absolutePath}")
133138
}
134139
}
135-
136140
} catch (e: Exception) {
137141
destination.delete()
138142
Logger.e(e) { "Download failed" }
139143
throw e
140144
} finally {
141-
activeDownloads.remove(safeName)
145+
activeDownloads.remove(downloadId)
146+
activeFileNames.remove(safeName)
142147
}
143148
}.flowOn(Dispatchers.IO)
144149

@@ -178,16 +183,19 @@ class AndroidDownloader(
178183

179184
override suspend fun cancelDownload(fileName: String): Boolean =
180185
withContext(Dispatchers.IO) {
181-
182186
var cancelled = false
183187
var deleted = false
184188

185-
activeDownloads[fileName]?.let { call: Call ->
186-
if (!call.isCanceled()) {
187-
call.cancel()
188-
cancelled = true
189+
val downloadId = activeFileNames[fileName]
190+
if (downloadId != null) {
191+
activeDownloads[downloadId]?.let { call: Call ->
192+
if (!call.isCanceled()) {
193+
call.cancel()
194+
cancelled = true
195+
}
196+
activeDownloads.remove(downloadId)
189197
}
190-
activeDownloads.remove(fileName)
198+
activeFileNames.remove(fileName)
191199
}
192200

193201
val file = File(files.appDownloadsDir(), fileName)

core/data/src/jvmMain/kotlin/zed/rainxch/core/data/services/DesktopDownloader.kt

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class DesktopDownloader(
2626
private val files: FileLocationsProvider,
2727
private val proxyManager: ProxyManager = ProxyManager
2828
) : Downloader {
29-
3029
private val activeDownloads = ConcurrentHashMap<String, Call>()
30+
private val nameToId = ConcurrentHashMap<String, String>()
3131

3232
private fun buildClient(): OkHttpClient {
3333
Authenticator.setDefault(null)
@@ -80,21 +80,23 @@ class DesktopDownloader(
8080
"Invalid file name: $rawName"
8181
}
8282

83-
val destination = File(dir, safeName)
83+
val downloadId = UUID.randomUUID().toString()
84+
val previous = nameToId.putIfAbsent(safeName, downloadId)
85+
if (previous != null) {
86+
throw IllegalStateException("A download for '$safeName' is already in progress")
87+
}
8488

89+
val destination = File(dir, safeName)
8590
if (destination.exists()) {
8691
Logger.d { "Deleting existing file before download: ${destination.absolutePath}" }
8792
destination.delete()
8893
}
8994

9095
Logger.d { "Starting download: $url" }
9196

92-
val request = Request.Builder()
93-
.url(url)
94-
.build()
95-
97+
val request = Request.Builder().url(url).build()
9698
val call = client.newCall(request)
97-
activeDownloads[safeName] = call
99+
activeDownloads[downloadId] = call
98100

99101
try {
100102
call.execute().use { response ->
@@ -136,7 +138,8 @@ class DesktopDownloader(
136138
Logger.e(e) { "Download failed" }
137139
throw e
138140
} finally {
139-
activeDownloads.remove(safeName)
141+
activeDownloads.remove(downloadId)
142+
nameToId.remove(safeName)
140143
}
141144
}.flowOn(Dispatchers.IO)
142145

@@ -151,7 +154,6 @@ class DesktopDownloader(
151154
}
152155

153156
val file = File(files.userDownloadsDir(), safeName)
154-
155157
if (file.exists()) {
156158
Logger.d { "Deleting existing file before download: ${file.absolutePath}" }
157159
file.delete()
@@ -166,26 +168,24 @@ class DesktopDownloader(
166168
override suspend fun getDownloadedFilePath(fileName: String): String? =
167169
withContext(Dispatchers.IO) {
168170
val file = File(files.userDownloadsDir(), fileName)
169-
170-
if (file.exists() && file.length() > 0) {
171-
file.absolutePath
172-
} else {
173-
null
174-
}
171+
if (file.exists() && file.length() > 0) file.absolutePath else null
175172
}
176173

177174
override suspend fun cancelDownload(fileName: String): Boolean =
178175
withContext(Dispatchers.IO) {
179-
180176
var cancelled = false
181177
var deleted = false
182178

183-
activeDownloads[fileName]?.let { call: Call ->
184-
if (!call.isCanceled()) {
185-
call.cancel()
186-
cancelled = true
179+
val downloadId = nameToId[fileName]
180+
if (downloadId != null) {
181+
activeDownloads[downloadId]?.let { call ->
182+
if (!call.isCanceled()) {
183+
call.cancel()
184+
cancelled = true
185+
}
187186
}
188-
activeDownloads.remove(fileName)
187+
activeDownloads.remove(downloadId)
188+
nameToId.remove(fileName)
189189
}
190190

191191
val file = File(files.userDownloadsDir(), fileName)

0 commit comments

Comments
 (0)