Skip to content

fix: prevent downloads sheet crash during active downloads #817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvanhorn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: prevent downloads sheet crash during active downloads #817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions app/src/main/java/fulguris/settings/fragment/DownloadData.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package fulguris.settings.fragment

import android.app.DownloadManager
import android.database.Cursor
import androidx.core.database.getIntOrNull
import androidx.core.database.getLongOrNull
import androidx.core.database.getStringOrNull

internal data class DownloadData(
val id: Long,
val title: String?,
val status: Int,
val localUri: String?,
val uri: String?,
val bytesDownloaded: Long,
val totalSize: Long,
val lastModified: Long,
val mimeType: String?
) {
companion object {
fun fromCursor(cursor: Cursor): DownloadData? {
val id = cursor.requiredLong(DownloadManager.COLUMN_ID)?.takeIf { it >= 0L }
?: return null
val status = cursor.requiredInt(DownloadManager.COLUMN_STATUS) ?: return null

return DownloadData(
id = id,
title = cursor.optionalString(DownloadManager.COLUMN_TITLE),
status = status,
localUri = cursor.optionalString(DownloadManager.COLUMN_LOCAL_URI),
uri = cursor.optionalString(DownloadManager.COLUMN_URI),
bytesDownloaded = cursor.optionalLong(
DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR,
0L
),
totalSize = cursor.optionalLong(DownloadManager.COLUMN_TOTAL_SIZE_BYTES, -1L),
lastModified = cursor.optionalLong(
DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP,
0L
),
mimeType = cursor.optionalString(DownloadManager.COLUMN_MEDIA_TYPE)
)
}

private fun Cursor.requiredLong(columnName: String): Long? {
val index = getColumnIndex(columnName)
return if (index >= 0) getLongOrNull(index) else null
}

private fun Cursor.requiredInt(columnName: String): Int? {
val index = getColumnIndex(columnName)
return if (index >= 0) getIntOrNull(index) else null
}

private fun Cursor.optionalLong(columnName: String, defaultValue: Long): Long {
val index = getColumnIndex(columnName)
return if (index >= 0) getLongOrNull(index) ?: defaultValue else defaultValue
}

private fun Cursor.optionalString(columnName: String): String? {
val index = getColumnIndex(columnName)
return if (index >= 0) getStringOrNull(index) else null
}
}
}
123 changes: 41 additions & 82 deletions app/src/main/java/fulguris/settings/fragment/DownloadsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.google.android.material.dialog.MaterialAlertDialogBuilder
import dagger.hilt.android.AndroidEntryPoint
import fulguris.R
import fulguris.extensions.copyToClipboard
import fulguris.extensions.firstOrNullMap
import fulguris.extensions.toast
import fulguris.utils.Utils
import timber.log.Timber
Expand Down Expand Up @@ -340,12 +341,11 @@ class DownloadsFragment : PreferenceFragmentCompat() {
try {
processedCount++

val id = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_ID))
seenDownloadIds.add(id)

// Use the shared updateDownload logic (doesn't update actions/count)
updateDownloadFromCursor(cursor)
//
val downloadData = decodeDownloadData(cursor)
if (downloadData != null) {
seenDownloadIds.add(downloadData.id)
updateDownload(downloadData)
}
updateDownloadCount()

// Schedule next download
Expand Down Expand Up @@ -403,36 +403,12 @@ class DownloadsFragment : PreferenceFragmentCompat() {
}
}

/**
* Data class to hold download information extracted from cursor
*/
private data class DownloadData(
val id: Long,
val title: String?,
val status: Int,
val localUri: String?,
val uri: String?,
val bytesDownloaded: Long,
val totalSize: Long,
val lastModified: Long,
val mimeType: String?
)

/**
* Create DownloadData from cursor position
*/
private fun createDownloadData(cursor: Cursor): DownloadData {
return DownloadData(
id = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_ID)),
title = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TITLE)),
status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS)),
localUri = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LOCAL_URI)),
uri = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_URI)),
bytesDownloaded = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR)),
totalSize = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TOTAL_SIZE_BYTES)),
lastModified = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP)),
mimeType = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_MEDIA_TYPE))
)
private fun decodeDownloadData(cursor: Cursor): DownloadData? {
return DownloadData.fromCursor(cursor).also { data ->
if (data == null) {
Timber.w("Skipping download row without a valid ID or status")
}
}
}

/**
Expand All @@ -444,11 +420,7 @@ class DownloadsFragment : PreferenceFragmentCompat() {
val cursor = downloadManager.query(query)

return cursor?.use {
if (it.moveToFirst()) {
createDownloadData(it)
} else {
null
}
it.firstOrNullMap(::decodeDownloadData)
}
}

Expand All @@ -467,7 +439,7 @@ class DownloadsFragment : PreferenceFragmentCompat() {

if (cursor != null && cursor.moveToFirst()) {
try {
updateDownloadFromCursor(cursor)
decodeDownloadData(cursor)?.let(::updateDownload)
// Update action states and download count after updating the preference
updateDownloadCount()
} finally {
Expand All @@ -479,15 +451,10 @@ class DownloadsFragment : PreferenceFragmentCompat() {
}
}

/**
* Update a download from cursor data. This is the core implementation used by both
* list population and individual updates. Does not call updateActionStates/updateDownloadCount
* so caller can batch those calls.
*/
private fun updateDownloadFromCursor(cursor: Cursor) {
val id = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_ID))
val status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS))
val localUri = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LOCAL_URI))
private fun updateDownload(downloadData: DownloadData) {
val id = downloadData.id
val status = downloadData.status
val localUri = downloadData.localUri

// Update flags based on this download's status
hasAnyDownloads = true
Expand Down Expand Up @@ -516,14 +483,12 @@ class DownloadsFragment : PreferenceFragmentCompat() {

if (downloadPref != null) {
// Update existing preference
val downloadData = createDownloadData(cursor)
downloadPref.updateFromDownloadData(downloadData)
// Update icon in case status or file type changed
setDownloadIcon(downloadPref, downloadData.status, downloadData.mimeType, downloadData.localUri)
Timber.d("updateDownloadFromCursor: Updated existing preference for download $id")
} else {
// Create new preference
val downloadData = createDownloadData(cursor)
downloadPref = createDownloadPreference(downloadData)
downloadsListCategory.addPreference(downloadPref)
Timber.d("updateDownloadFromCursor: Created new preference for download $id")
Expand Down Expand Up @@ -1747,16 +1712,18 @@ class DownloadsFragment : PreferenceFragmentCompat() {

var isActive = false
if (cursor.moveToFirst()) {
val status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS))

isActive = status == DownloadManager.STATUS_RUNNING ||
status == DownloadManager.STATUS_PENDING ||
status == DownloadManager.STATUS_PAUSED

val downloadData = DownloadData.fromCursor(cursor)
if (downloadData == null) {
Timber.w("updateProgress($downloadId): Invalid cursor data")
} else {
isActive = downloadData.status == DownloadManager.STATUS_RUNNING ||
downloadData.status == DownloadManager.STATUS_PENDING ||
downloadData.status == DownloadManager.STATUS_PAUSED

// Only update if this download is active or needs a summary refresh
if (isActive || summary == null) {
updateFromCursor(cursor)
// Only update if this download is active or needs a summary refresh
if (isActive || summary == null) {
updateSummary(downloadData)
}
}
} else {
Timber.w("updateProgress($downloadId): No cursor data found")
Expand All @@ -1766,26 +1733,12 @@ class DownloadsFragment : PreferenceFragmentCompat() {
return isActive
}

/**
* Update the preference summary from cursor data.
* This only updates the text, not the entire preference UI.
*/
fun updateFromCursor(cursor: Cursor) {
val status = cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS))
val bytesDownloaded = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR))
val bytesTotal = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_TOTAL_SIZE_BYTES))
val lastModified = cursor.getLong(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP))
val localUri = cursor.getString(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_LOCAL_URI))

summary = formatSummary(status, bytesDownloaded, bytesTotal, lastModified, localUri)
}

/**
* Initialize the preference summary from DownloadData.
* Called once during preference creation.
*/
fun initializeFromDownloadData(data: DownloadData) {
summary = formatSummary(data.status, data.bytesDownloaded, data.totalSize, data.lastModified, data.localUri)
updateSummary(data)
}

/**
Expand All @@ -1799,7 +1752,17 @@ class DownloadsFragment : PreferenceFragmentCompat() {
}

// Update summary
summary = formatSummary(data.status, data.bytesDownloaded, data.totalSize, data.lastModified, data.localUri)
updateSummary(data)
}

private fun updateSummary(data: DownloadData) {
summary = formatSummary(
data.status,
data.bytesDownloaded,
data.totalSize,
data.lastModified,
data.localUri
)
}

/**
Expand Down Expand Up @@ -1867,7 +1830,3 @@ class DownloadsFragment : PreferenceFragmentCompat() {







Loading
Loading