Skip to content

Commit

Permalink
Fix crashes (#89)
Browse files Browse the repository at this point in the history
* Make mediaCursor non null in MediaGalleryAdapter

* Remove unnecessary getItemId override

* Improve MimeType fromValue function

* Make columns and value get null safe

* Remove unused import

* Add DEFAULT_ID to adapter when id may be null

* Add error message for invalid id

* Set mime type for camera capture to null

* Open camera after getting permission

* Improve error logging

* Remove usage of isDataValid because cursor is no longer nullable in adapter

* Use getStringOrNull
  • Loading branch information
kinnerapriyap authored Feb 16, 2021
1 parent 0179da1 commit 10244a8
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import android.content.Intent
import android.content.pm.PackageManager
import android.os.Build
import android.os.Bundle
import android.os.Handler
import android.os.Looper
import android.view.View
import android.widget.AdapterView
import android.widget.Toast
Expand All @@ -21,7 +23,6 @@ import com.kinnerapriyap.sugar.databinding.ActivityShergilBinding
import com.kinnerapriyap.sugar.mediagallery.album.MediaGalleryAlbumSpinnerAdapter
import com.kinnerapriyap.sugar.mediagallery.album.toBucketDisplayName
import com.kinnerapriyap.sugar.resultlauncher.ResultLauncherHandler
import java.util.ArrayList

internal class ShergilActivity :
AppCompatActivity(),
Expand Down Expand Up @@ -194,7 +195,9 @@ internal class ShergilActivity :
viewModel.resetCameraCaptureUri()
observer?.cameraCapture(viewModel.getCameraCaptureUri())
} else {
navController.navigate(NavGraphDirections.actionGlobalCameraFragment())
Handler(Looper.getMainLooper()).post {
navController.navigate(NavGraphDirections.actionGlobalCameraFragment())
}
}
}

Expand Down
32 changes: 21 additions & 11 deletions sher-gil/src/main/java/com/kinnerapriyap/sugar/ShergilViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import android.graphics.Bitmap
import android.net.Uri
import android.os.Build
import android.provider.MediaStore
import androidx.core.database.getStringOrNull
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
Expand All @@ -18,6 +19,7 @@ import com.kinnerapriyap.sugar.mediagallery.MediaGalleryHandler.Companion.ALL_AL
import com.kinnerapriyap.sugar.mediagallery.album.MediaGalleryAlbum
import com.kinnerapriyap.sugar.mediagallery.cell.MediaCellDisplayModel
import com.kinnerapriyap.sugar.mediagallery.cell.MediaCellUpdateModel
import com.kinnerapriyap.sugar.mediagallery.media.DEFAULT_ID
import com.kinnerapriyap.sugar.mediagallery.media.MediaGalleryCursorWrapper
import java.io.FileNotFoundException
import java.io.FileOutputStream
Expand Down Expand Up @@ -95,19 +97,27 @@ class ShergilViewModel(application: Application) : AndroidViewModel(application)

fun setMediaChecked(displayModel: MediaCellDisplayModel) {
val selected = selectedMediaCellDisplayModels.value ?: mutableListOf()
if (selected.any { it.id == displayModel.id }) {
selected.removeAll { it.id == displayModel.id }
} else {
if (isSelectedOverMax()) {
when {
displayModel.id == DEFAULT_ID -> {
errorMessage.value =
getApplication<Application>().resources.getString(
R.string.max_selectable_error,
choiceSpec.maxSelectable
)
getApplication<Application>().resources.getString(R.string.invalid_id)
return
}
if (!allowMultipleSelection()) selected.removeAll(selected)
selected.add(displayModel.copy(isChecked = true))
selected.any { it.id == displayModel.id } -> {
selected.removeAll { it.id == displayModel.id }
}
else -> {
if (isSelectedOverMax()) {
errorMessage.value =
getApplication<Application>().resources.getString(
R.string.max_selectable_error,
choiceSpec.maxSelectable
)
return
}
if (!allowMultipleSelection()) selected.removeAll(selected)
selected.add(displayModel.copy(isChecked = true))
}
}
updatedMediaCellPositions = Pair(displayModel.position, updatedMediaCellPositions.first)
selectedMediaCellDisplayModels.value = selected
Expand All @@ -130,7 +140,7 @@ class ShergilViewModel(application: Application) : AndroidViewModel(application)
while (!cursor.isAfterLast) {
val bucketColumnIndex = cursor.getColumnIndex(MediaGalleryHandler.BUCKET_DISPLAY_NAME)
if (bucketColumnIndex == -1) break
val name = cursor.getString(bucketColumnIndex)
val name = cursor.getStringOrNull(bucketColumnIndex)
cursor.moveToNext()
if (name == null || name == ALL_ALBUM_BUCKET_DISPLAY_NAME) continue
else if (!addedNamesCount.contains(name)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package com.kinnerapriyap.sugar.choice
* Reference for MIME types supported by Android:
* See [MediaFile.java](https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/media/java/android/media/MediaFile.java/)
*/
@Suppress("unused")
enum class MimeType(val value: String) {
// Images
JPEG("image/jpeg"),
Expand All @@ -20,6 +21,6 @@ enum class MimeType(val value: String) {
val IMAGES: List<MimeType> = values().toList()

private val map = values().associateBy(MimeType::value)
fun fromValue(value: String): MimeType? = map[value]
fun fromValue(value: String?): MimeType? = map.getOrDefault(value, null)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ class MediaGalleryHandler(private val contentResolver: ContentResolver) {
mimeTypes: List<MimeType>,
showDisallowedMimeTypes: Boolean,
allowCamera: Boolean
): Cursor? {
): Cursor {
val extras = MatrixCursor(PROJECTION)
if (allowCamera) {
extras.addRow(
arrayOf(
CAMERA_CAPTURE_ID.toString(),
MimeType.IMAGES,
null,
ALL_ALBUM_BUCKET_DISPLAY_NAME
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import android.widget.Filter
import android.widget.FilterQueryProvider
import android.widget.Filterable
import androidx.appcompat.app.AppCompatActivity
import androidx.core.database.getLongOrNull
import androidx.core.database.getStringOrNull
import androidx.recyclerview.widget.RecyclerView
import com.kinnerapriyap.sugar.choice.MimeType
import com.kinnerapriyap.sugar.databinding.ViewMediaCellBinding
Expand All @@ -20,8 +22,10 @@ import com.kinnerapriyap.sugar.mediagallery.cell.MediaCellDisplayModel
import com.kinnerapriyap.sugar.mediagallery.cell.MediaCellUpdateModel
import com.kinnerapriyap.sugar.mediagallery.cell.bindMediaUri

const val DEFAULT_ID = -1L

class MediaGalleryAdapter(
private var mediaCursor: Cursor?,
private var mediaCursor: Cursor,
private var selectedMediaCellDisplayModels: List<MediaCellDisplayModel>,
private val onMediaCellClicked: ((MediaCellDisplayModel) -> Unit),
private val mimeTypes: List<MimeType>,
Expand All @@ -30,8 +34,6 @@ class MediaGalleryAdapter(
Filterable,
MediaGalleryCursorFilterListener {

private var isDataValid = mediaCursor != null

var mediaCellUpdateModel: MediaCellUpdateModel =
MediaCellUpdateModel(Pair(-1, -1), listOf())
set(value) {
Expand All @@ -58,13 +60,13 @@ class MediaGalleryAdapter(
* getColumnIndexOrThrow is used since _ID column exists in [BaseColumns]
*/
private var idColumnIndex =
mediaCursor?.getColumnIndexOrThrow(MediaStore.MediaColumns._ID) ?: -1
mediaCursor.getColumnIndexOrThrow(MediaStore.MediaColumns._ID)

private val bucketDisplayNameColumnIndex =
mediaCursor?.getColumnIndex(MediaGalleryHandler.BUCKET_DISPLAY_NAME) ?: -1
mediaCursor.getColumnIndex(MediaGalleryHandler.BUCKET_DISPLAY_NAME)

private val mimeTypeColumnIndex =
mediaCursor?.getColumnIndex(MediaStore.MediaColumns.MIME_TYPE) ?: -1
mediaCursor.getColumnIndex(MediaStore.MediaColumns.MIME_TYPE)

override fun onCreateViewHolder(
parent: ViewGroup,
Expand All @@ -78,34 +80,36 @@ class MediaGalleryAdapter(
return MediaCellHolder(binding)
}

override fun getItemCount(): Int = mediaCursor?.count ?: 0
override fun getItemCount(): Int = mediaCursor.count

/**
* [mediaCursor] is moved to the correct position
* so it is used to get the data
*/
override fun onBindViewHolder(holder: MediaCellHolder, position: Int) {
if (mediaCursor?.moveToPosition(position) == false ||
!isDataValid ||
idColumnIndex == -1 ||
if (!mediaCursor.moveToPosition(position)) {
throw IllegalStateException("onBind position:$position")
} else if (idColumnIndex == -1 ||
bucketDisplayNameColumnIndex == -1 ||
mimeTypeColumnIndex == -1
) {
throw IllegalStateException("onBind $position")
throw IllegalStateException(
"onBind invalid column index $idColumnIndex " +
"$bucketDisplayNameColumnIndex $mimeTypeColumnIndex"
)
}
val cursor = mediaCursor ?: throw IllegalStateException("invalid cursor")

/**
* Get a URI representing the media item and
* append the id from the projection column to the base URI
*/
val id = cursor.getLong(idColumnIndex)
val id = mediaCursor.getLongOrNull(idColumnIndex) ?: DEFAULT_ID
val contentUri: Uri = ContentUris.withAppendedId(
MediaStore.Images.Media.EXTERNAL_CONTENT_URI,
id
)
val bucketDisplayName = cursor.getString(bucketDisplayNameColumnIndex)
val mimeType = MimeType.fromValue(cursor.getString(mimeTypeColumnIndex))
val bucketDisplayName = mediaCursor.getStringOrNull(bucketDisplayNameColumnIndex)
val mimeType = MimeType.fromValue(mediaCursor.getStringOrNull(mimeTypeColumnIndex))
val displayModel = MediaCellDisplayModel(
position = position,
id = id,
Expand All @@ -120,11 +124,6 @@ class MediaGalleryAdapter(

private fun isCameraCapture(id: Long) = id == CAMERA_CAPTURE_ID

override fun getItemId(position: Int): Long =
if (isDataValid && mediaCursor?.moveToPosition(position) == true) {
mediaCursor?.getLong(idColumnIndex) ?: 0
} else RecyclerView.NO_ID

inner class MediaCellHolder(
private val binding: ViewMediaCellBinding
) : RecyclerView.ViewHolder(binding.root) {
Expand All @@ -149,10 +148,10 @@ class MediaGalleryAdapter(
* @param constraint to filter the query
* @return [Cursor] for query results
*/
override fun fetchMediaAsync(constraint: CharSequence?): Cursor? =
override fun fetchMediaAsync(constraint: CharSequence?): Cursor =
filterQueryProvider?.runQuery(constraint) ?: getCursor()

override fun getCursor(): Cursor? = mediaCursor
override fun getCursor(): Cursor = mediaCursor

/**
* Update to new cursor with [swapCursor]
Expand All @@ -173,11 +172,12 @@ class MediaGalleryAdapter(
*/
private fun swapCursor(newCursor: Cursor?) {
if (newCursor === mediaCursor) return
mediaCursor = newCursor
isDataValid = newCursor != null
idColumnIndex =
newCursor?.getColumnIndex(MediaStore.MediaColumns._ID) ?: -1
notifyDataSetChanged()
if (newCursor != null) {
mediaCursor = newCursor
idColumnIndex =
mediaCursor.getColumnIndexOrThrow(MediaStore.MediaColumns._ID)
notifyDataSetChanged()
}
}

override fun getFilter(): Filter = cursorFilter
Expand Down
1 change: 1 addition & 0 deletions sher-gil/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<string name="preview">Preview</string>
<string name="gallery_image">Gallery image</string>
<string name="max_selectable_error">You can only select up to %d media files</string>
<string name="invalid_id">This media file has an invalid id</string>
<string name="take_photo">Take Photo</string>
<string name="switch_camera">Switch camera</string>
<string name="open_gallery">Open gallery</string>
Expand Down

0 comments on commit 10244a8

Please sign in to comment.