Image Loader Improvements (#4516)

app/src/main/java/fr/free/nrw/commons/customselector/database/UploadedDao.kt

@@ -50,39 +50,37 @@ abstract class UploadedStatusDao {
     /**
      * Asynchronous insert into uploaded status table.
      */
-    fun insertUploaded(uploadedStatus: UploadedStatus) = runBlocking {
+    suspend fun insertUploaded(uploadedStatus: UploadedStatus) {
-        async {
+        uploadedStatus.lastUpdated = Calendar.getInstance().time as Date?
-                uploadedStatus.lastUpdated = Calendar.getInstance().time as Date?
+        insert(uploadedStatus)
-                insert(uploadedStatus)
-            }.await()
     }
 
     /**
      * Asynchronous delete from uploaded status table.
      */
-    fun deleteUploaded(uploadedStatus: UploadedStatus) = runBlocking {
+    suspend fun deleteUploaded(uploadedStatus: UploadedStatus) {
-        async { delete(uploadedStatus) }
+        delete(uploadedStatus)
     }
 
     /**
      * Asynchronous update entry in uploaded status table.
      */
-    fun updateUploaded(uploadedStatus: UploadedStatus) = runBlocking {
+    suspend fun updateUploaded(uploadedStatus: UploadedStatus) {
-        async { update(uploadedStatus) }
+        update(uploadedStatus)
     }
 
     /**
      * Asynchronous image sha1 query.
      */
-    fun getUploadedFromImageSHA1(imageSHA1: String) = runBlocking<UploadedStatus?> {
+    suspend fun getUploadedFromImageSHA1(imageSHA1: String):UploadedStatus {
-        async { getFromImageSHA1(imageSHA1) }.await()
+        return getFromImageSHA1(imageSHA1)
     }
 
     /**
      * Asynchronous modified image sha1 query.
      */
-    fun getUploadedFromModifiedImageSHA1(modifiedImageSHA1: String) = runBlocking<UploadedStatus?> {
+    suspend fun getUploadedFromModifiedImageSHA1(modifiedImageSHA1: String):UploadedStatus {
-        async { getFromModifiedImageSHA1(modifiedImageSHA1) }.await()
+        return getFromModifiedImageSHA1(modifiedImageSHA1)
     }
 
 }

app/src/main/java/fr/free/nrw/commons/customselector/ui/adapter/ImageAdapter.kt

@@ -76,7 +76,7 @@ class ImageAdapter(
         else {
             holder.itemUnselected();
         }
-        Glide.with(context).load(image.uri).into(holder.image)
+        Glide.with(context).load(image.uri).thumbnail(0.3f).into(holder.image)
         imageLoader.queryAndSetView(holder,image)
         holder.itemView.setOnClickListener {
             selectOrRemoveImage(holder, position)
@@ -99,8 +99,8 @@ class ImageAdapter(
             if(holder.isItemUploaded()){
                 Toast.makeText(context,"Already Uploaded image", Toast.LENGTH_SHORT).show()
             } else {
-            selectedImages.add(images[position])
+                selectedImages.add(images[position])
-            notifyItemChanged(position, ImageSelectedOrUpdated())
+                notifyItemChanged(position, ImageSelectedOrUpdated())
             }
         }
         imageSelectListener.onSelectedImagesChanged(selectedImages)

app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageFragment.kt

@@ -148,4 +148,12 @@ class ImageFragment: CommonsDaggerSupportFragment() {
         return 3
         // todo change span count depending on the device orientation and other factos.
     }
+
+    /**
+     * OnDestroy Cleanup the imageLoader coroutine.
+     */
+    override fun onDestroy() {
+        imageLoader?.cleanUP()
+        super.onDestroy()
+    }
 }

app/src/main/java/fr/free/nrw/commons/customselector/ui/selector/ImageLoader.kt

@@ -11,10 +11,7 @@ import fr.free.nrw.commons.filepicker.PickedFiles
 import fr.free.nrw.commons.media.MediaClient
 import fr.free.nrw.commons.upload.FileProcessor
 import fr.free.nrw.commons.upload.FileUtilsWrapper
-import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.*
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.launch
-import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.io.IOException
 import java.net.UnknownHostException
@@ -57,9 +54,17 @@ class ImageLoader @Inject constructor(
     /**
      * Maps to facilitate image query.
      */
-    private var mapImageSHA1: HashMap<Image, String> = HashMap()
+    private var mapModifiedImageSHA1: HashMap<Image, String> = HashMap()
     private var mapHolderImage : HashMap<ImageViewHolder, Image> = HashMap()
     private var mapResult: HashMap<String, Result> = HashMap()
+    private var mapImageSHA1: HashMap<Uri, String> = HashMap()
+
+    /**
+     * Coroutine Dispatchers and Scope.
+     */
+    private var defaultDispatcher = Dispatchers.Default
+    private var ioDispatcher = Dispatchers.IO
+    private val scope = MainScope()
 
     /**
      * Query image and setUp the view.
@@ -72,42 +77,43 @@ class ImageLoader @Inject constructor(
         mapHolderImage[holder] = image
         holder.itemNotUploaded()
 
-        CoroutineScope(Dispatchers.Main).launch {
+        scope.launch {
 
-            var result : Result = Result.NOTFOUND
+            var result: Result = Result.NOTFOUND
-            withContext(Dispatchers.Default) {
 
+            if (mapHolderImage[holder] != image) {
+                return@launch
+            }
+
+            val imageSHA1 = getImageSHA1(image.uri)
+            val uploadedStatus = getFromUploaded(imageSHA1)
+
+            val sha1 = uploadedStatus?.let {
+                result = getResultFromUploadedStatus(uploadedStatus)
+                uploadedStatus.modifiedImageSHA1
+            } ?: run {
                 if (mapHolderImage[holder] == image) {
-                    val imageSHA1 = getImageSHA1(image.uri)
+                    getSHA1(image)
-                    val uploadedStatus = uploadedStatusDao.getUploadedFromImageSHA1(imageSHA1)
+                } else {
+                    ""
+                }
+            }
 
-                    val sha1 = uploadedStatus?.let {
+            if (mapHolderImage[holder] != image) {
-                        result = getResultFromUploadedStatus(uploadedStatus)
+                return@launch
-                        uploadedStatus.modifiedImageSHA1
+            }
-                        } ?: run {
-                            if(mapHolderImage[holder] == image) {
-                                getSHA1(image)
-                            } else {
-                                ""
-                            }
-                        }
 
-                    if (mapHolderImage[holder] == image &&
+            if (result in arrayOf(Result.NOTFOUND, Result.INVALID) && sha1.isNotEmpty()) {
-                        result in arrayOf(Result.NOTFOUND, Result.INVALID) &&
+                // Query original image.
-                        sha1.isNotEmpty()) {
+                result = querySHA1(imageSHA1)
-                            // Query original image.
+                if (result is Result.TRUE) {
-                            result = querySHA1(imageSHA1)
+                    // Original image found.
-                            if( result is Result.TRUE ) {
+                    insertIntoUploaded(imageSHA1, sha1, result is Result.TRUE, false)
-                                // Original image found.
+                } else {
-                                insertIntoUploaded(imageSHA1, sha1, result is Result.TRUE, false)
+                    // Original image not found, query modified image.
-                            }
+                    result = querySHA1(sha1)
-                            else {
+                    if (result != Result.ERROR) {
-                                // Original image not found, query modified image.
+                        insertIntoUploaded(imageSHA1, sha1, false, result is Result.TRUE)
-                                result = querySHA1(sha1)
-                                if (result != Result.ERROR) {
-                                    insertIntoUploaded(imageSHA1, sha1, false, result is Result.TRUE)
-                                }
-                            }
                     }
                 }
             }
@@ -122,25 +128,27 @@ class ImageLoader @Inject constructor(
      *
      * @return Query result.
      */
-    private fun querySHA1(SHA1: String): Result {
+
-        mapResult[SHA1]?.let{
+    private suspend fun querySHA1(SHA1: String): Result {
-            return it
+        return withContext(ioDispatcher) {
-        }
+            mapResult[SHA1]?.let {
-        var result : Result = Result.FALSE
+                return@withContext it
-        try {
-            if (mediaClient.checkFileExistsUsingSha(SHA1).blockingGet()) {
-                mapResult[SHA1] = Result.TRUE
-                result = Result.TRUE
             }
-        } catch (e: Exception) {
+            var result: Result = Result.FALSE
-            if (e is UnknownHostException) {
+            try {
-                // Handle no network connection.
+                if (mediaClient.checkFileExistsUsingSha(SHA1).blockingGet()) {
-                Timber.e(e, "Network Connection Error")
+                    mapResult[SHA1] = Result.TRUE
+                    result = Result.TRUE
+                }
+            } catch (e: Exception) {
+                if (e is UnknownHostException) {
+                    // Handle no network connection.
+                    Timber.e(e, "Network Connection Error")
+                }
+                result = Result.ERROR
+                e.printStackTrace()
             }
-            result = Result.ERROR
+            result
-            e.printStackTrace()
-        } finally {
-               return result
         }
     }
 
@@ -149,27 +157,48 @@ class ImageLoader @Inject constructor(
      *
      * @return sha1 of the image
      */
-    private fun getSHA1(image: Image): String {
+    private suspend fun getSHA1(image: Image): String {
-        mapImageSHA1[image]?.let{
+        mapModifiedImageSHA1[image]?.let{
             return it
         }
         val sha1 = generateModifiedSHA1(image);
-        mapImageSHA1[image] = sha1;
+        mapModifiedImageSHA1[image] = sha1;
         return sha1;
     }
 
+    /**
+     * Get the uploaded status entry from the database.
+     */
+    private suspend fun getFromUploaded(imageSha1:String): UploadedStatus?{
+        return uploadedStatusDao.getUploadedFromImageSHA1(imageSha1)
+    }
+
     /**
      * Insert into uploaded status table.
      */
-    private fun insertIntoUploaded(imageSha1:String, modifiedImageSha1:String, imageResult:Boolean, modifiedImageResult: Boolean){
+    private suspend fun insertIntoUploaded(imageSha1:String, modifiedImageSha1:String, imageResult:Boolean, modifiedImageResult: Boolean){
-        uploadedStatusDao.insertUploaded(UploadedStatus(imageSha1, modifiedImageSha1, imageResult, modifiedImageResult))
+        uploadedStatusDao.insertUploaded(
+            UploadedStatus(
+                imageSha1,
+                modifiedImageSha1,
+                imageResult,
+                modifiedImageResult
+            )
+        )
     }
 
     /**
      * Get image sha1 from uri, used to retrieve the original image sha1.
      */
-    private fun getImageSHA1(uri: Uri): String {
+    private suspend fun getImageSHA1(uri: Uri): String {
-        return fileUtilsWrapper.getSHA1(context.contentResolver.openInputStream(uri))
+        return withContext(ioDispatcher) {
+            mapImageSHA1[uri]?.let{
+                return@withContext it
+            }
+            val result = fileUtilsWrapper.getSHA1(context.contentResolver.openInputStream(uri))
+            mapImageSHA1[uri] = result
+            result
+        }
     }
 
     /**
@@ -194,18 +223,28 @@ class ImageLoader @Inject constructor(
      *
      * @return modified sha1
      */
-    private fun generateModifiedSHA1(image: Image) : String {
+    private suspend fun generateModifiedSHA1(image: Image) : String {
-        val uploadableFile = PickedFiles.pickedExistingPicture(context, image.uri)
+        return withContext(defaultDispatcher) {
-        val exifInterface: ExifInterface? = try {
+            val uploadableFile = PickedFiles.pickedExistingPicture(context, image.uri)
-            ExifInterface(uploadableFile.file!!)
+            val exifInterface: ExifInterface? = try {
-        } catch (e: IOException) {
+                ExifInterface(uploadableFile.file!!)
-            Timber.e(e)
+            } catch (e: IOException) {
-            null
+                Timber.e(e)
+                null
+            }
+            fileProcessor.redactExifTags(exifInterface, fileProcessor.getExifTagsToRedact())
+            val sha1 =
+                fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(uploadableFile.filePath))
+            uploadableFile.file.delete()
+            sha1
         }
-        fileProcessor.redactExifTags(exifInterface, fileProcessor.getExifTagsToRedact())
+    }
-        val sha1 = fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(uploadableFile.filePath))
+
-        uploadableFile.file.delete()
+    /**
-        return sha1
+     * CleanUp function.
+     */
+    fun cleanUP() {
+        scope.cancel()
     }
 
     /**

app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt

@@ -28,9 +28,11 @@ import fr.free.nrw.commons.upload.UploadClient
 import fr.free.nrw.commons.upload.UploadResult
 import fr.free.nrw.commons.wikidata.WikidataEditService
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.MainScope
 import kotlinx.coroutines.flow.asFlow
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.util.*
@@ -408,7 +410,16 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) :
         contribution.contentUri?.let {
             val imageSha1 = fileUtilsWrapper.getSHA1(appContext.contentResolver.openInputStream(it))
             val modifiedSha1 = fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(contribution.localUri?.path))
-            uploadedStatusDao.insertUploaded(UploadedStatus(imageSha1, modifiedSha1, imageSha1 == modifiedSha1, true));
+            MainScope().launch {
+                uploadedStatusDao.insertUploaded(
+                    UploadedStatus(
+                        imageSha1,
+                        modifiedSha1,
+                        imageSha1 == modifiedSha1,
+                        true
+                    )
+                );
+            }
         }
     }