From bd461dbfae27ece4c256e33bed36391322b1f264 Mon Sep 17 00:00:00 2001 From: Ryan Jennings Date: Wed, 27 Mar 2024 15:17:51 -0700 Subject: [PATCH] fix(upload manager): ensure main thread interactions from broadcast receiver save internet archive identifiers as the media server url (not a part of meta data) ensure cancelled uploads are saved to prevent new file identifiers ensure upload service coroutine scopes are cancelled on job cancel --- .../opendasharchive/openarchive/db/Project.kt | 2 +- .../openarchive/services/Conduit.kt | 1 + .../services/internetarchive/IaConduit.kt | 36 ++++++++++--------- .../upload/UploadManagerActivity.kt | 8 +++-- .../openarchive/upload/UploadService.kt | 3 +- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/net/opendasharchive/openarchive/db/Project.kt b/app/src/main/java/net/opendasharchive/openarchive/db/Project.kt index eea3d903..0e1b1d84 100644 --- a/app/src/main/java/net/opendasharchive/openarchive/db/Project.kt +++ b/app/src/main/java/net/opendasharchive/openarchive/db/Project.kt @@ -37,7 +37,7 @@ data class Project( } val isUploading - get() = collections.firstOrNull { it.isUploading } != null + get() = collections.any { it.isUploading } val collections: List get() = find(Collection::class.java, "project_id = ?", id.toString()) diff --git a/app/src/main/java/net/opendasharchive/openarchive/services/Conduit.kt b/app/src/main/java/net/opendasharchive/openarchive/services/Conduit.kt index a03f3505..a526767a 100644 --- a/app/src/main/java/net/opendasharchive/openarchive/services/Conduit.kt +++ b/app/src/main/java/net/opendasharchive/openarchive/services/Conduit.kt @@ -49,6 +49,7 @@ abstract class Conduit( open fun cancel() { mCancelled = true + mMedia.save() } diff --git a/app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt b/app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt index 22071e1c..75050c92 100644 --- a/app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt +++ b/app/src/main/java/net/opendasharchive/openarchive/services/internetarchive/IaConduit.kt @@ -41,34 +41,33 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { val client = SaveClient.get(mContext) - // TODO this should make sure we aren't accidentally using one of archive.org's metadata fields by accident - val slug = getSlug(mMedia.title) val fileName = getUploadFileName(mMedia, true) val metaJson = gson.toJson(mMedia) val proof = getProof() - var basePath = "$slug-${Util.RandomString(4).nextString()}" - val url = "$ARCHIVE_API_ENDPOINT/$basePath/$fileName" + if (mMedia.serverUrl.isBlank()) { + // TODO this should make sure we aren't accidentally using one of archive.org's metadata fields by accident + val slug = getSlug(mMedia.title) + val newIdentifier = "$slug-${Util.RandomString(4).nextString()}" + // create an identifier for the upload + mMedia.serverUrl = newIdentifier + } // upload content synchronously for progress - client.uploadContent(url, mimeType) + client.uploadContent(fileName, mimeType) // upload metadata and proofs async, and report failures - basePath = "$slug-${Util.RandomString(4).nextString()}" - client.uploadMetaData(metaJson, basePath, fileName) + client.uploadMetaData(metaJson, fileName) /// Upload ProofMode metadata, if enabled and successfully created. for (file in proof) { - client.uploadProofFiles(file, basePath) + client.uploadProofFiles(file) } - val finalPath = ARCHIVE_DETAILS_ENDPOINT + basePath - mMedia.serverUrl = finalPath - jobSucceeded() return true - } catch (e: Exception) { + } catch (e: Throwable) { jobFailed(e) } @@ -79,8 +78,11 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { // Ignored. Not used here. } - private suspend fun OkHttpClient.uploadContent(url: String, mimeType: String) { + private suspend fun OkHttpClient.uploadContent(fileName: String, mimeType: String) { val mediaUri = mMedia.originalFilePath + + val url = "${ARCHIVE_API_ENDPOINT}/${mMedia.serverUrl}/$fileName" + val requestBody = RequestBodyUtil.create( mContext.contentResolver, Uri.parse(mediaUri), @@ -101,7 +103,7 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { } @Throws(IOException::class) - private fun OkHttpClient.uploadMetaData(content: String, basePath: String, fileName: String) { + private fun OkHttpClient.uploadMetaData(content: String, fileName: String) { val requestBody = RequestBodyUtil.create( textMediaType, content.byteInputStream(), @@ -109,7 +111,7 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { createListener(cancellable = { !mCancelled }) ) - val url = "$ARCHIVE_API_ENDPOINT/$basePath/$fileName.meta.json" + val url = "${ARCHIVE_API_ENDPOINT}/${mMedia.serverUrl}/$fileName.meta.json" val request = Request.Builder() .url(url) @@ -122,7 +124,7 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { /// upload proof mode @Throws(IOException::class) - private fun OkHttpClient.uploadProofFiles(uploadFile: File, basePath: String) { + private fun OkHttpClient.uploadProofFiles(uploadFile: File) { val requestBody = RequestBodyUtil.create( mContext.contentResolver, Uri.fromFile(uploadFile), @@ -130,7 +132,7 @@ class IaConduit(media: Media, context: Context) : Conduit(media, context) { textMediaType, createListener(cancellable = { !mCancelled }) ) - val url = "$ARCHIVE_API_ENDPOINT/$basePath/${uploadFile.name}" + val url = "$ARCHIVE_API_ENDPOINT/${mMedia.serverUrl}/${uploadFile.name}" val request = Request.Builder() .url(url) diff --git a/app/src/main/java/net/opendasharchive/openarchive/upload/UploadManagerActivity.kt b/app/src/main/java/net/opendasharchive/openarchive/upload/UploadManagerActivity.kt index 72474e0c..0419c156 100644 --- a/app/src/main/java/net/opendasharchive/openarchive/upload/UploadManagerActivity.kt +++ b/app/src/main/java/net/opendasharchive/openarchive/upload/UploadManagerActivity.kt @@ -48,6 +48,8 @@ class UploadManagerActivity : BaseActivity() { } private val mMessageReceiver: BroadcastReceiver = object : BroadcastReceiver() { + private val handler = Handler(Looper.getMainLooper()) + override fun onReceive(context: Context, intent: Intent) { val action = BroadcastManager.getAction(intent) val mediaId = action?.mediaId ?: return @@ -56,10 +58,10 @@ class UploadManagerActivity : BaseActivity() { val media = Media.get(mediaId) if (action == BroadcastManager.Action.Delete || media?.sStatus == Media.Status.Uploaded) { - mFrag?.removeItem(mediaId) + handler.post { mFrag?.removeItem(mediaId) } } else { - mFrag?.updateItem(mediaId) + handler.post { mFrag?.updateItem(mediaId) } } if (media?.sStatus == Media.Status.Error) { @@ -70,7 +72,7 @@ class UploadManagerActivity : BaseActivity() { } } - Handler(Looper.getMainLooper()).post { + handler.post { updateTitle() } } diff --git a/app/src/main/java/net/opendasharchive/openarchive/upload/UploadService.kt b/app/src/main/java/net/opendasharchive/openarchive/upload/UploadService.kt index 196d8dea..f313adf0 100644 --- a/app/src/main/java/net/opendasharchive/openarchive/upload/UploadService.kt +++ b/app/src/main/java/net/opendasharchive/openarchive/upload/UploadService.kt @@ -18,6 +18,7 @@ import androidx.work.Configuration import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import net.opendasharchive.openarchive.CleanInsightsManager import net.opendasharchive.openarchive.R @@ -90,7 +91,7 @@ class UploadService : JobService() { mKeepUploading = false for (conduit in mConduits) conduit.cancel() mConduits.clear() - + scope.cancel() return true }