Skip to content

Commit

Permalink
fix: Ensure files are fully downloaded before sharing (#482)
Browse files Browse the repository at this point in the history 
The previous code would share media by either:

a. If it was an image, downloading the image using Glide in to a bitmap,
then recompressing as a PNG, saving, and sharing the resulting file.

b. Otherwise, create a temporary file, enqueue a DownloadManager request
to download the media in to the file, and immediately start sharing,
hoping that the download had completed in time.

Both approaches have problems:

In the "image" case the image was being downloaded (or retrieved from
the Glide cache), decompressed to a bitmap, then recompressed as a PNG.
This uses more memory, and doesn't share the original contents of the
file. E.g., if the original file was a JPEG that's lost (and the PNG
might well be larger than the source image).

In the second case the DownloadManager download is not guaranteed to
have completed (or even started) before the user chooses the share
destination. The destination could receive a partial or even empty file.

Fix both of those cases by always fully downloading the file before
sending the share intent. This guarantees the file is available to
share, and in its original format. Since this uses the same OkHttpClient
as the rest of the app the content is highly likely to be in the OkHttp
cache, so there will no extra network traffic because of this.
  • Loading branch information
@nikclayton
nikclayton committed Mar 1, 2024
1 parent fcae441 commit b9512e4
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 86 deletions.
152 changes: 66 additions & 86 deletions app/src/main/java/app/pachli/ViewMediaActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import android.content.ClipData
import android.content.ClipboardManager
import android.content.Context
import android.content.pm.PackageManager
import android.graphics.Bitmap
import android.graphics.Color
import android.net.Uri
import android.os.Build
Expand All @@ -40,35 +39,35 @@ import android.widget.Toast
import androidx.core.app.ShareCompat
import androidx.core.content.FileProvider
import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.viewpager2.adapter.FragmentStateAdapter
import androidx.viewpager2.widget.ViewPager2
import app.pachli.BuildConfig.APPLICATION_ID
import app.pachli.core.activity.BaseActivity
import app.pachli.core.common.extensions.hide
import app.pachli.core.common.extensions.show
import app.pachli.core.common.extensions.viewBinding
import app.pachli.core.navigation.AttachmentViewData
import app.pachli.core.navigation.ViewMediaActivityIntent
import app.pachli.core.navigation.ViewThreadActivityIntent
import app.pachli.core.network.model.Attachment
import app.pachli.databinding.ActivityViewMediaBinding
import app.pachli.fragment.ViewImageFragment
import app.pachli.fragment.ViewVideoFragment
import app.pachli.pager.ImagePagerAdapter
import app.pachli.pager.SingleImagePagerAdapter
import app.pachli.util.getTemporaryMediaFilename
import autodispose2.androidx.lifecycle.AndroidLifecycleScopeProvider
import autodispose2.autoDispose
import com.bumptech.glide.Glide
import com.bumptech.glide.request.FutureTarget
import com.google.android.material.snackbar.Snackbar
import dagger.hilt.android.AndroidEntryPoint
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers
import io.reactivex.rxjava3.core.Single
import io.reactivex.rxjava3.schedulers.Schedulers
import java.io.File
import java.io.FileNotFoundException
import java.io.FileOutputStream
import java.io.IOException
import java.util.Locale
import javax.inject.Inject
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.launch
import okhttp3.OkHttpClient
import okhttp3.Request
import okio.buffer
import okio.sink
import timber.log.Timber

typealias ToolbarVisibilityListener = (isVisible: Boolean) -> Unit
Expand All @@ -78,6 +77,9 @@ typealias ToolbarVisibilityListener = (isVisible: Boolean) -> Unit
*/
@AndroidEntryPoint
class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener, ViewVideoFragment.VideoActionsListener {
@Inject
lateinit var okHttpClient: OkHttpClient

private val binding by viewBinding(ActivityViewMediaBinding::inflate)

val toolbar: View
Expand All @@ -90,6 +92,9 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
private val toolbarVisibilityListeners = mutableListOf<ToolbarVisibilityListener>()
private var imageUrl: String? = null

/** True if a download to share media is in progress */
private var isDownloading: Boolean = false

fun addToolbarVisibilityListener(listener: ToolbarVisibilityListener): Function0<Boolean> {
this.toolbarVisibilityListeners.add(listener)
listener(isToolbarVisible)
Expand Down Expand Up @@ -170,7 +175,7 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
}

override fun onPrepareOptionsMenu(menu: Menu?): Boolean {
menu?.findItem(R.id.action_share_media)?.isEnabled = !isCreating
menu?.findItem(R.id.action_share_media)?.isEnabled = !isDownloading
return true
}

Expand Down Expand Up @@ -254,98 +259,73 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
}

private fun shareMedia() {
val directory = applicationContext.getExternalFilesDir("Pachli")
val directory = applicationContext.getExternalFilesDir(null)
if (directory == null || !(directory.exists())) {
Timber.e("Error obtaining directory to save temporary media.")
return
}

if (imageUrl != null) {
shareImage(directory, imageUrl!!)
shareMediaFile(directory, imageUrl!!)
} else {
val attachment = attachments!![binding.viewPager.currentItem].attachment
when (attachment.type) {
Attachment.Type.IMAGE -> shareImage(directory, attachment.url)
Attachment.Type.AUDIO,
Attachment.Type.VIDEO,
Attachment.Type.GIFV,
-> shareMediaFile(directory, attachment.url)
else -> Timber.e("Unknown media format for sharing.")
}
shareMediaFile(directory, attachment.url)
}
}

private fun shareFile(file: File, mimeType: String?) {
ShareCompat.IntentBuilder(this)
.setType(mimeType)
.addStream(FileProvider.getUriForFile(applicationContext, "$APPLICATION_ID.fileprovider", file))
.setChooserTitle(R.string.send_media_to)
.startChooser()
}

private var isCreating: Boolean = false

private fun shareImage(directory: File, url: String) {
isCreating = true
binding.progressBarShare.visibility = View.VISIBLE
/**
* Share media by downloading it to a temporary file and then sharing that
* file.
*
* [DownloadManager] is not used for this as it is not guaranteed to start
* downloading the file expediently, and the user may wait a long time.
*/
private fun shareMediaFile(directory: File, url: String) {
isDownloading = true
binding.progressBarShare.show()
invalidateOptionsMenu()
val file = File(directory, getTemporaryMediaFilename("png"))
val futureTask: FutureTarget<Bitmap> =
Glide.with(applicationContext).asBitmap().load(Uri.parse(url)).submit()
Single.fromCallable {
val bitmap = futureTask.get()
try {
val stream = FileOutputStream(file)
bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream)
stream.close()
return@fromCallable true
} catch (fnfe: FileNotFoundException) {
Timber.e("Error writing temporary media.")
} catch (ioe: IOException) {
Timber.e("Error writing temporary media.")
}
return@fromCallable false
}
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.doOnDispose {
futureTask.cancel(true)
}
.autoDispose(AndroidLifecycleScopeProvider.from(this, Lifecycle.Event.ON_DESTROY))
.subscribe(
{ result ->
Timber.d("Download image result: %s", result)
isCreating = false
invalidateOptionsMenu()
binding.progressBarShare.visibility = View.GONE
if (result) {
shareFile(file, "image/png")
}
},
{ error ->
isCreating = false
invalidateOptionsMenu()
binding.progressBarShare.visibility = View.GONE
Timber.e(error, "Failed to download image")
},
)
}

private fun shareMediaFile(directory: File, url: String) {
val uri = Uri.parse(url)
val mimeTypeMap = MimeTypeMap.getSingleton()
val extension = MimeTypeMap.getFileExtensionFromUrl(url)
val mimeType = mimeTypeMap.getMimeTypeFromExtension(extension)
val filename = getTemporaryMediaFilename(extension)
val file = File(directory, filename)

val downloadManager = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager
val request = DownloadManager.Request(uri)
request.setDestinationUri(Uri.fromFile(file))
request.setVisibleInDownloadsUi(false)
downloadManager.enqueue(request)
lifecycleScope.launch {
val request = Request.Builder().url(url).build()
val response = async(Dispatchers.IO) {
val response = okHttpClient.newCall(request).execute()
response.body?.let { body ->
file.sink().buffer().use { it.writeAll(body.source()) }
}
return@async response
}.await()

isDownloading = false
binding.progressBarShare.hide()
invalidateOptionsMenu()

if (!response.isSuccessful) {
Snackbar.make(
binding.root,
getString(R.string.error_media_download, url, response.code, response.message),
Snackbar.LENGTH_INDEFINITE,
).show()
return@launch
}

shareFile(file, mimeType)
ShareCompat.IntentBuilder(this@ViewMediaActivity)
.setType(mimeType)
.addStream(
FileProvider.getUriForFile(
applicationContext,
"$APPLICATION_ID.fileprovider",
file,
),
)
.setChooserTitle(R.string.send_media_to)
.startChooser()
}
}
}

Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,4 +719,5 @@
<string name="pref_title_update_check_now">Check for update now</string>
<string name="pref_update_check_no_updates">There are no updates available</string>
<string name="pref_update_next_scheduled_check">Next scheduled check: %1$s</string>
<string name="error_media_download">Could not download %1$s: %2$d %3$s</string>
</resources>

0 comments on commit b9512e4

Please sign in to comment.