Skip to content
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import fr.free.nrw.commons.customselector.helper.ImageHelper.CUSTOM_SELECTOR_PRE
import fr.free.nrw.commons.customselector.helper.ImageHelper.SHOW_ALREADY_ACTIONED_IMAGES_PREFERENCE_KEY
import fr.free.nrw.commons.customselector.listeners.ImageSelectListener
import fr.free.nrw.commons.customselector.model.Image
import fr.free.nrw.commons.customselector.ui.selector.ImageFileLoader
import fr.free.nrw.commons.customselector.ui.selector.ImageLoader
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -48,6 +49,7 @@ class ImageAdapter(
* ImageLoader queries images.
*/
private var imageLoader: ImageLoader,
private var imageFileLoader: ImageFileLoader,
) : RecyclerView.Adapter<ImageAdapter.ImageViewHolder>(),
FastScrollRecyclerView.SectionedAdapter {

Expand Down Expand Up @@ -537,6 +539,8 @@ class ImageAdapter(
* CleanUp function.
*/
fun cleanUp() {
//abort the device scanning process immediately
imageFileLoader.abortLoadImage()
scope.cancel()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import fr.free.nrw.commons.customselector.model.Image
import fr.free.nrw.commons.customselector.model.Result
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel

/**
Expand All @@ -21,7 +22,7 @@ class CustomSelectorViewModel(
/**
* Scope for coroutine task (image fetch).
*/
private val scope = CoroutineScope(Dispatchers.Main)
private val scope = CoroutineScope(Dispatchers.Main + SupervisorJob())

/**
* Stores selected images.
Expand All @@ -38,7 +39,9 @@ class CustomSelectorViewModel(
*/
fun fetchImages() {
result.postValue(Result(CallbackStatus.FETCHING, arrayListOf()))
scope.cancel()
// fix: instead of scope.cancel(), we call our new abort method in the loader.
//this stops the background processing while keeping the ViewModel scope alive.
imageFileLoader.abortLoadImage()
imageFileLoader.loadDeviceImages(
object : ImageLoaderListener {
override fun onImageLoaded(images: ArrayList<Image>) {
Expand All @@ -56,6 +59,8 @@ class CustomSelectorViewModel(
* Clear the coroutine task linked with context.
*/
override fun onCleared() {
//stop the loader immediately when the ViewModel is destroyed
imageFileLoader.abortLoadImage()
scope.cancel()
super.onCleared()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ import android.provider.MediaStore
import android.text.format.DateFormat
import fr.free.nrw.commons.customselector.listeners.ImageLoaderListener
import fr.free.nrw.commons.customselector.model.Image
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.coroutines.*
import java.io.File
import java.io.FileInputStream
import java.security.MessageDigest
import java.util.Calendar
import java.util.Date
import java.util.Locale
Expand All @@ -23,6 +22,12 @@ import kotlin.coroutines.CoroutineContext
class ImageFileLoader(
val context: Context,
) : CoroutineScope {

/**
* keep track of the current loading job to allow cancellation.
*/
private var loaderJob: Job? = null

/**
* Coroutine context for fetching images.
*/
Expand All @@ -45,7 +50,8 @@ class ImageFileLoader(
* Load Device Images under coroutine.
*/
fun loadDeviceImages(listener: ImageLoaderListener) {
launch(Dispatchers.Main) {
loaderJob?.cancel()
loaderJob = launch(Dispatchers.Main) {
withContext(Dispatchers.IO) {
getImages(listener)
}
Expand Down Expand Up @@ -77,18 +83,19 @@ class ImageFileLoader(
val dateColumn = cursor.getColumnIndex(MediaStore.Images.Media.DATE_ADDED)

val images = arrayListOf<Image>()
if (cursor.moveToFirst()) {
do {
if (Thread.interrupted()) {
listener.onFailed(NullPointerException())
return
}
val id = cursor.getLong(idColumn)
val name = cursor.getString(nameColumn)
val path = cursor.getString(dataColumn)
val bucketId = cursor.getLong(bucketIdColumn)
val bucketName = cursor.getString(bucketNameColumn)
val date = cursor.getLong(dateColumn)
cursor.use { c -> //using .use automatically closes the cursor
if (c.moveToFirst()) {
do {
//check if coroutine was cancelled or thread interrupted
if (!isActive || Thread.interrupted()) {
return
}
val id = c.getLong(idColumn)
val name = c.getString(nameColumn)
val path = c.getString(dataColumn)
val bucketId = c.getLong(bucketIdColumn)
val bucketName = c.getString(bucketNameColumn)
val date = c.getLong(dateColumn)

val file =
if (path == null || path.isEmpty()) {
Expand Down Expand Up @@ -117,35 +124,52 @@ class ImageFileLoader(
val dateFormat = DateFormat.getMediumDateFormat(context)
val formattedDate = dateFormat.format(date)

val image =
Image(
val sha1 = getSha1(file)
val image =
Image(
id,
name,
uri,
path,
bucketId,
bucketName,
date = (formattedDate),
date = formattedDate,
sha1 = sha1
)
images.add(image)
}
} while (cursor.moveToNext())
images.add(image)
}
} while (c.moveToNext())
}
}
//return results to the main thread via listener
launch(Dispatchers.Main) {
listener.onImageLoaded(images)
}
cursor.close()
listener.onImageLoaded(images)
}

/**
* Abort loading images.
*/
fun abortLoadImage() {
// todo Abort loading images.
loaderJob?.cancel()
}

/*
*
* TODO
* Sha1 for image (original image).
*
/**
* generates SHA-1 hash for the image file.
*/
private fun getSha1(file: File): String {
return try {
val digest = MessageDigest.getInstance("SHA-1")
val fileInputStream = FileInputStream(file)
val buffer = ByteArray(8192)
var bytesRead: Int
while (fileInputStream.read(buffer).also { bytesRead = it } != -1) {
digest.update(buffer, 0, bytesRead)
}
val sha1Bytes = digest.digest()
sha1Bytes.joinToString("") { "%02x".format(it) }
} catch (e: Exception) {
""
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ class ImageFragment :
@Inject
lateinit var uploadedStatusDao: UploadedStatusDao

/**
* ImageFileLoader to scan device files.
*/
@Inject
lateinit var imageFileLoader: ImageFileLoader

/**
* FileUtilsWrapper class to get imageSHA1 from uri
*/
Expand Down Expand Up @@ -215,7 +221,13 @@ class ImageFragment :

// ensures imageAdapter is initialized
if (!::imageAdapter.isInitialized) {
imageAdapter = ImageAdapter(requireActivity(), activity as ImageSelectListener, imageLoader!!)
//fix: Added theimageFileLoader as the 4th parameter
imageAdapter = ImageAdapter(
requireActivity(),
activity as ImageSelectListener,
imageLoader!!,
imageFileLoader
)
Timber.d("Initialized imageAdapter in onCreateView")
}
// Set single selection mode if needed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ class ImageLoader
return@launch
}

val imageSHA1: String =
// optimisation: use the SHA1 pre-calculated by ImageFileLoader.
// this avoids the redundant file I/O operations.
val imageSHA1: String = if (image.sha1.isNotEmpty()) {
image.sha1
} else {
when (mapImageSHA1[image.uri] != null) {
true -> mapImageSHA1[image.uri]!!
else ->
Expand All @@ -104,6 +108,7 @@ class ImageLoader
context.contentResolver,
)
}
}
mapImageSHA1[image.uri] = imageSHA1

if (imageSHA1.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import fr.free.nrw.commons.TestCommonsApplication
import fr.free.nrw.commons.customselector.listeners.ImageSelectListener
import fr.free.nrw.commons.customselector.model.Image
import fr.free.nrw.commons.customselector.ui.selector.CustomSelectorActivity
import fr.free.nrw.commons.customselector.ui.selector.ImageFileLoader
import fr.free.nrw.commons.customselector.ui.selector.ImageLoader
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -45,6 +46,9 @@ class ImageAdapterTest {
@Mock
private lateinit var imageLoader: ImageLoader

@Mock
private lateinit var imageFileLoader: ImageFileLoader

@Mock
private lateinit var imageSelectListener: ImageSelectListener

Expand Down Expand Up @@ -75,7 +79,7 @@ class ImageAdapterTest {
MockitoAnnotations.initMocks(this)
Dispatchers.setMain(testDispatcher)
activity = Robolectric.buildActivity(CustomSelectorActivity::class.java).get()
imageAdapter = ImageAdapter(activity, imageSelectListener, imageLoader)
imageAdapter = ImageAdapter(activity, imageSelectListener, imageLoader, imageFileLoader)
image = Image(1, "image", uri, "abc/abc", 1, "bucket1")
images = ArrayList()

Expand Down Expand Up @@ -253,6 +257,8 @@ class ImageAdapterTest {
@Test
fun cleanUp() {
imageAdapter.cleanUp()
//verify that abortLoadImage was called on the loader
Mockito.verify(imageFileLoader).abortLoadImage()
}

/**
Expand Down
Loading