Skip to content

Commit 351fd28

Browse files
authored
Image Loader Improvements (#4516)
1 parent 404a79b commit 351fd28

File tree

5 files changed

+146
-90
lines changed

5 files changed

+146
-90
lines changed

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,39 +50,37 @@ abstract class UploadedStatusDao {
5050
/**
5151
* Asynchronous insert into uploaded status table.
5252
*/
53-
fun insertUploaded(uploadedStatus: UploadedStatus) = runBlocking {
54-
async {
55-
uploadedStatus.lastUpdated = Calendar.getInstance().time as Date?
56-
insert(uploadedStatus)
57-
}.await()
53+
suspend fun insertUploaded(uploadedStatus: UploadedStatus) {
54+
uploadedStatus.lastUpdated = Calendar.getInstance().time as Date?
55+
insert(uploadedStatus)
5856
}
5957

6058
/**
6159
* Asynchronous delete from uploaded status table.
6260
*/
63-
fun deleteUploaded(uploadedStatus: UploadedStatus) = runBlocking {
64-
async { delete(uploadedStatus) }
61+
suspend fun deleteUploaded(uploadedStatus: UploadedStatus) {
62+
delete(uploadedStatus)
6563
}
6664

6765
/**
6866
* Asynchronous update entry in uploaded status table.
6967
*/
70-
fun updateUploaded(uploadedStatus: UploadedStatus) = runBlocking {
71-
async { update(uploadedStatus) }
68+
suspend fun updateUploaded(uploadedStatus: UploadedStatus) {
69+
update(uploadedStatus)
7270
}
7371

7472
/**
7573
* Asynchronous image sha1 query.
7674
*/
77-
fun getUploadedFromImageSHA1(imageSHA1: String) = runBlocking<UploadedStatus?> {
78-
async { getFromImageSHA1(imageSHA1) }.await()
75+
suspend fun getUploadedFromImageSHA1(imageSHA1: String):UploadedStatus {
76+
return getFromImageSHA1(imageSHA1)
7977
}
8078

8179
/**
8280
* Asynchronous modified image sha1 query.
8381
*/
84-
fun getUploadedFromModifiedImageSHA1(modifiedImageSHA1: String) = runBlocking<UploadedStatus?> {
85-
async { getFromModifiedImageSHA1(modifiedImageSHA1) }.await()
82+
suspend fun getUploadedFromModifiedImageSHA1(modifiedImageSHA1: String):UploadedStatus {
83+
return getFromModifiedImageSHA1(modifiedImageSHA1)
8684
}
8785

8886
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class ImageAdapter(
7676
else {
7777
holder.itemUnselected();
7878
}
79-
Glide.with(context).load(image.uri).into(holder.image)
79+
Glide.with(context).load(image.uri).thumbnail(0.3f).into(holder.image)
8080
imageLoader.queryAndSetView(holder,image)
8181
holder.itemView.setOnClickListener {
8282
selectOrRemoveImage(holder, position)
@@ -99,8 +99,8 @@ class ImageAdapter(
9999
if(holder.isItemUploaded()){
100100
Toast.makeText(context,"Already Uploaded image", Toast.LENGTH_SHORT).show()
101101
} else {
102-
selectedImages.add(images[position])
103-
notifyItemChanged(position, ImageSelectedOrUpdated())
102+
selectedImages.add(images[position])
103+
notifyItemChanged(position, ImageSelectedOrUpdated())
104104
}
105105
}
106106
imageSelectListener.onSelectedImagesChanged(selectedImages)

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,12 @@ class ImageFragment: CommonsDaggerSupportFragment() {
148148
return 3
149149
// todo change span count depending on the device orientation and other factos.
150150
}
151+
152+
/**
153+
* OnDestroy Cleanup the imageLoader coroutine.
154+
*/
155+
override fun onDestroy() {
156+
imageLoader?.cleanUP()
157+
super.onDestroy()
158+
}
151159
}

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

Lines changed: 112 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ import fr.free.nrw.commons.filepicker.PickedFiles
1111
import fr.free.nrw.commons.media.MediaClient
1212
import fr.free.nrw.commons.upload.FileProcessor
1313
import fr.free.nrw.commons.upload.FileUtilsWrapper
14-
import kotlinx.coroutines.CoroutineScope
15-
import kotlinx.coroutines.Dispatchers
16-
import kotlinx.coroutines.launch
17-
import kotlinx.coroutines.withContext
14+
import kotlinx.coroutines.*
1815
import timber.log.Timber
1916
import java.io.IOException
2017
import java.net.UnknownHostException
@@ -57,9 +54,17 @@ class ImageLoader @Inject constructor(
5754
/**
5855
* Maps to facilitate image query.
5956
*/
60-
private var mapImageSHA1: HashMap<Image, String> = HashMap()
57+
private var mapModifiedImageSHA1: HashMap<Image, String> = HashMap()
6158
private var mapHolderImage : HashMap<ImageViewHolder, Image> = HashMap()
6259
private var mapResult: HashMap<String, Result> = HashMap()
60+
private var mapImageSHA1: HashMap<Uri, String> = HashMap()
61+
62+
/**
63+
* Coroutine Dispatchers and Scope.
64+
*/
65+
private var defaultDispatcher = Dispatchers.Default
66+
private var ioDispatcher = Dispatchers.IO
67+
private val scope = MainScope()
6368

6469
/**
6570
* Query image and setUp the view.
@@ -72,42 +77,43 @@ class ImageLoader @Inject constructor(
7277
mapHolderImage[holder] = image
7378
holder.itemNotUploaded()
7479

75-
CoroutineScope(Dispatchers.Main).launch {
80+
scope.launch {
7681

77-
var result : Result = Result.NOTFOUND
78-
withContext(Dispatchers.Default) {
82+
var result: Result = Result.NOTFOUND
7983

84+
if (mapHolderImage[holder] != image) {
85+
return@launch
86+
}
87+
88+
val imageSHA1 = getImageSHA1(image.uri)
89+
val uploadedStatus = getFromUploaded(imageSHA1)
90+
91+
val sha1 = uploadedStatus?.let {
92+
result = getResultFromUploadedStatus(uploadedStatus)
93+
uploadedStatus.modifiedImageSHA1
94+
} ?: run {
8095
if (mapHolderImage[holder] == image) {
81-
val imageSHA1 = getImageSHA1(image.uri)
82-
val uploadedStatus = uploadedStatusDao.getUploadedFromImageSHA1(imageSHA1)
83-
84-
val sha1 = uploadedStatus?.let {
85-
result = getResultFromUploadedStatus(uploadedStatus)
86-
uploadedStatus.modifiedImageSHA1
87-
} ?: run {
88-
if(mapHolderImage[holder] == image) {
89-
getSHA1(image)
90-
} else {
91-
""
92-
}
93-
}
94-
95-
if (mapHolderImage[holder] == image &&
96-
result in arrayOf(Result.NOTFOUND, Result.INVALID) &&
97-
sha1.isNotEmpty()) {
98-
// Query original image.
99-
result = querySHA1(imageSHA1)
100-
if( result is Result.TRUE ) {
101-
// Original image found.
102-
insertIntoUploaded(imageSHA1, sha1, result is Result.TRUE, false)
103-
}
104-
else {
105-
// Original image not found, query modified image.
106-
result = querySHA1(sha1)
107-
if (result != Result.ERROR) {
108-
insertIntoUploaded(imageSHA1, sha1, false, result is Result.TRUE)
109-
}
110-
}
96+
getSHA1(image)
97+
} else {
98+
""
99+
}
100+
}
101+
102+
if (mapHolderImage[holder] != image) {
103+
return@launch
104+
}
105+
106+
if (result in arrayOf(Result.NOTFOUND, Result.INVALID) && sha1.isNotEmpty()) {
107+
// Query original image.
108+
result = querySHA1(imageSHA1)
109+
if (result is Result.TRUE) {
110+
// Original image found.
111+
insertIntoUploaded(imageSHA1, sha1, result is Result.TRUE, false)
112+
} else {
113+
// Original image not found, query modified image.
114+
result = querySHA1(sha1)
115+
if (result != Result.ERROR) {
116+
insertIntoUploaded(imageSHA1, sha1, false, result is Result.TRUE)
111117
}
112118
}
113119
}
@@ -122,25 +128,27 @@ class ImageLoader @Inject constructor(
122128
*
123129
* @return Query result.
124130
*/
125-
private fun querySHA1(SHA1: String): Result {
126-
mapResult[SHA1]?.let{
127-
return it
128-
}
129-
var result : Result = Result.FALSE
130-
try {
131-
if (mediaClient.checkFileExistsUsingSha(SHA1).blockingGet()) {
132-
mapResult[SHA1] = Result.TRUE
133-
result = Result.TRUE
131+
132+
private suspend fun querySHA1(SHA1: String): Result {
133+
return withContext(ioDispatcher) {
134+
mapResult[SHA1]?.let {
135+
return@withContext it
134136
}
135-
} catch (e: Exception) {
136-
if (e is UnknownHostException) {
137-
// Handle no network connection.
138-
Timber.e(e, "Network Connection Error")
137+
var result: Result = Result.FALSE
138+
try {
139+
if (mediaClient.checkFileExistsUsingSha(SHA1).blockingGet()) {
140+
mapResult[SHA1] = Result.TRUE
141+
result = Result.TRUE
142+
}
143+
} catch (e: Exception) {
144+
if (e is UnknownHostException) {
145+
// Handle no network connection.
146+
Timber.e(e, "Network Connection Error")
147+
}
148+
result = Result.ERROR
149+
e.printStackTrace()
139150
}
140-
result = Result.ERROR
141-
e.printStackTrace()
142-
} finally {
143-
return result
151+
result
144152
}
145153
}
146154

@@ -149,27 +157,48 @@ class ImageLoader @Inject constructor(
149157
*
150158
* @return sha1 of the image
151159
*/
152-
private fun getSHA1(image: Image): String {
153-
mapImageSHA1[image]?.let{
160+
private suspend fun getSHA1(image: Image): String {
161+
mapModifiedImageSHA1[image]?.let{
154162
return it
155163
}
156164
val sha1 = generateModifiedSHA1(image);
157-
mapImageSHA1[image] = sha1;
165+
mapModifiedImageSHA1[image] = sha1;
158166
return sha1;
159167
}
160168

169+
/**
170+
* Get the uploaded status entry from the database.
171+
*/
172+
private suspend fun getFromUploaded(imageSha1:String): UploadedStatus?{
173+
return uploadedStatusDao.getUploadedFromImageSHA1(imageSha1)
174+
}
175+
161176
/**
162177
* Insert into uploaded status table.
163178
*/
164-
private fun insertIntoUploaded(imageSha1:String, modifiedImageSha1:String, imageResult:Boolean, modifiedImageResult: Boolean){
165-
uploadedStatusDao.insertUploaded(UploadedStatus(imageSha1, modifiedImageSha1, imageResult, modifiedImageResult))
179+
private suspend fun insertIntoUploaded(imageSha1:String, modifiedImageSha1:String, imageResult:Boolean, modifiedImageResult: Boolean){
180+
uploadedStatusDao.insertUploaded(
181+
UploadedStatus(
182+
imageSha1,
183+
modifiedImageSha1,
184+
imageResult,
185+
modifiedImageResult
186+
)
187+
)
166188
}
167189

168190
/**
169191
* Get image sha1 from uri, used to retrieve the original image sha1.
170192
*/
171-
private fun getImageSHA1(uri: Uri): String {
172-
return fileUtilsWrapper.getSHA1(context.contentResolver.openInputStream(uri))
193+
private suspend fun getImageSHA1(uri: Uri): String {
194+
return withContext(ioDispatcher) {
195+
mapImageSHA1[uri]?.let{
196+
return@withContext it
197+
}
198+
val result = fileUtilsWrapper.getSHA1(context.contentResolver.openInputStream(uri))
199+
mapImageSHA1[uri] = result
200+
result
201+
}
173202
}
174203

175204
/**
@@ -194,18 +223,28 @@ class ImageLoader @Inject constructor(
194223
*
195224
* @return modified sha1
196225
*/
197-
private fun generateModifiedSHA1(image: Image) : String {
198-
val uploadableFile = PickedFiles.pickedExistingPicture(context, image.uri)
199-
val exifInterface: ExifInterface? = try {
200-
ExifInterface(uploadableFile.file!!)
201-
} catch (e: IOException) {
202-
Timber.e(e)
203-
null
226+
private suspend fun generateModifiedSHA1(image: Image) : String {
227+
return withContext(defaultDispatcher) {
228+
val uploadableFile = PickedFiles.pickedExistingPicture(context, image.uri)
229+
val exifInterface: ExifInterface? = try {
230+
ExifInterface(uploadableFile.file!!)
231+
} catch (e: IOException) {
232+
Timber.e(e)
233+
null
234+
}
235+
fileProcessor.redactExifTags(exifInterface, fileProcessor.getExifTagsToRedact())
236+
val sha1 =
237+
fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(uploadableFile.filePath))
238+
uploadableFile.file.delete()
239+
sha1
204240
}
205-
fileProcessor.redactExifTags(exifInterface, fileProcessor.getExifTagsToRedact())
206-
val sha1 = fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(uploadableFile.filePath))
207-
uploadableFile.file.delete()
208-
return sha1
241+
}
242+
243+
/**
244+
* CleanUp function.
245+
*/
246+
fun cleanUP() {
247+
scope.cancel()
209248
}
210249

211250
/**

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ import fr.free.nrw.commons.upload.UploadClient
2828
import fr.free.nrw.commons.upload.UploadResult
2929
import fr.free.nrw.commons.wikidata.WikidataEditService
3030
import kotlinx.coroutines.Dispatchers
31+
import kotlinx.coroutines.MainScope
3132
import kotlinx.coroutines.flow.asFlow
3233
import kotlinx.coroutines.flow.collect
3334
import kotlinx.coroutines.flow.map
35+
import kotlinx.coroutines.launch
3436
import kotlinx.coroutines.withContext
3537
import timber.log.Timber
3638
import java.util.*
@@ -408,7 +410,16 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) :
408410
contribution.contentUri?.let {
409411
val imageSha1 = fileUtilsWrapper.getSHA1(appContext.contentResolver.openInputStream(it))
410412
val modifiedSha1 = fileUtilsWrapper.getSHA1(fileUtilsWrapper.getFileInputStream(contribution.localUri?.path))
411-
uploadedStatusDao.insertUploaded(UploadedStatus(imageSha1, modifiedSha1, imageSha1 == modifiedSha1, true));
413+
MainScope().launch {
414+
uploadedStatusDao.insertUploaded(
415+
UploadedStatus(
416+
imageSha1,
417+
modifiedSha1,
418+
imageSha1 == modifiedSha1,
419+
true
420+
)
421+
);
422+
}
412423
}
413424
}
414425

0 commit comments

Comments
 (0)