diff options
2 files changed, 128 insertions, 34 deletions
diff --git a/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt b/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt index a475263c..7d658209 100644 --- a/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt +++ b/java/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractor.kt @@ -20,6 +20,7 @@ package com.android.intentresolver.contentpreview.payloadtoggle.domain.interacto import android.net.Uri import android.service.chooser.AdditionalContentContract.CursorExtraKeys.POSITION +import android.util.Log import com.android.intentresolver.contentpreview.UriMetadataReader import com.android.intentresolver.contentpreview.payloadtoggle.domain.model.CursorRow import com.android.intentresolver.contentpreview.payloadtoggle.domain.model.LoadDirection @@ -51,6 +52,8 @@ import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.mapLatest +private const val TAG = "CursorPreviewsIntr" + /** Queries data from a remote cursor, and caches it locally for presentation in Shareousel. */ class CursorPreviewsInteractor @Inject @@ -273,8 +276,7 @@ constructor( pagedCursor .getPageRows(pageNum) // TODO: what do we do if the load fails? ?.filter { it.uri !in state.merged } - ?.toPage(this, unclaimedRecords) - ?: this + ?.toPage(this, unclaimedRecords) ?: this private suspend fun <M : MutablePreviewMap> Sequence<CursorRow>.toPage( destination: M, @@ -288,26 +290,32 @@ constructor( private fun createPreviewModel( row: CursorRow, unclaimedRecords: MutableUnclaimedMap, - ): PreviewModel = uriMetadataReader.getMetadata(row.uri).let { metadata -> - val size = - row.previewSize - ?: metadata.previewUri?.let { uriMetadataReader.readPreviewSize(it) } - PreviewModel( - uri = row.uri, - previewUri = metadata.previewUri, - mimeType = metadata.mimeType, - aspectRatio = size.aspectRatioOrDefault(1f), - order = row.position, - ) - }.also { updated -> - if (unclaimedRecords.remove(row.uri) != null) { - // unclaimedRecords contains initially shared (and thus selected) items with unknown - // cursor position. Update selection records when any of those items is encountered - // in the cursor to maintain proper selection order should other items also be - // selected. - selectionInteractor.updateSelection(updated) + ): PreviewModel = + uriMetadataReader + .getMetadata(row.uri) + .let { metadata -> + val size = + row.previewSize + ?: metadata.previewUri?.let { uriMetadataReader.readPreviewSize(it) } + PreviewModel( + uri = row.uri, + previewUri = metadata.previewUri, + mimeType = metadata.mimeType, + aspectRatio = size.aspectRatioOrDefault(1f), + order = row.position, + ) + } + .also { updated -> + if (unclaimedRecords.remove(row.uri) != null) { + // unclaimedRecords contains initially shared (and thus selected) items with + // unknown + // cursor position. Update selection records when any of those items is + // encountered + // in the cursor to maintain proper selection order should other items also be + // selected. + selectionInteractor.updateSelection(updated) + } } - } private fun <M : MutablePreviewMap> M.putAllUnclaimedRight(unclaimed: UnclaimedMap): M = putAllUnclaimedWhere(unclaimed) { it >= focusedItemIdx } @@ -343,7 +351,28 @@ private fun <M : MutablePreviewMap> M.putAllUnclaimedWhere( .toMap(this) private fun PagedCursor<CursorRow?>.getPageRows(pageNum: Int): Sequence<CursorRow>? = - get(pageNum)?.filterNotNull() + runCatching { get(pageNum) } + .onFailure { Log.e(TAG, "Failed to read additional content cursor page #$pageNum", it) } + .getOrNull() + ?.asSafeSequence() + ?.filterNotNull() + +private fun <T> Sequence<T>.asSafeSequence(): Sequence<T> { + return if (this is SafeSequence) this else SafeSequence(this) +} + +private class SafeSequence<T>(private val sequence: Sequence<T>) : Sequence<T> { + override fun iterator(): Iterator<T> = + sequence.iterator().let { if (it is SafeIterator) it else SafeIterator(it) } +} + +private class SafeIterator<T>(private val iterator: Iterator<T>) : Iterator<T> by iterator { + override fun hasNext(): Boolean { + return runCatching { iterator.hasNext() } + .onFailure { Log.e(TAG, "Failed to read cursor", it) } + .getOrDefault(false) + } +} @Qualifier @MustBeDocumented @Retention(AnnotationRetention.RUNTIME) annotation class PageSize diff --git a/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt b/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt index 48e43190..c4ba8105 100644 --- a/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt +++ b/tests/unit/src/com/android/intentresolver/contentpreview/payloadtoggle/domain/interactor/CursorPreviewsInteractorTest.kt @@ -18,10 +18,13 @@ package com.android.intentresolver.contentpreview.payloadtoggle.domain.interactor +import android.database.Cursor import android.database.MatrixCursor import android.net.Uri import android.provider.MediaStore.MediaColumns.HEIGHT import android.provider.MediaStore.MediaColumns.WIDTH +import android.service.chooser.AdditionalContentContract.Columns.URI +import android.service.chooser.AdditionalContentContract.CursorExtraKeys.POSITION import android.util.Size import androidx.core.os.bundleOf import com.android.intentresolver.contentpreview.FileInfo @@ -39,6 +42,7 @@ import com.android.intentresolver.util.cursor.CursorView import com.android.intentresolver.util.cursor.viewBy import com.android.intentresolver.util.runTest import com.android.systemui.kosmos.Kosmos +import com.google.common.truth.Correspondence import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch @@ -93,9 +97,9 @@ class CursorPreviewsInteractorTest { private val cursorSizes: Map<Int, Size>, ) { val cursor: CursorView<CursorRow?> = - MatrixCursor(arrayOf("uri", WIDTH, HEIGHT)) + MatrixCursor(arrayOf(URI, WIDTH, HEIGHT)) .apply { - extras = bundleOf("position" to cursorStartPosition) + extras = bundleOf(POSITION to cursorStartPosition) for (i in cursorRange) { val size = cursorSizes[i] addRow( @@ -279,22 +283,83 @@ class CursorPreviewsInteractorTest { ) { deps -> previewSelectionsRepository.selections.value = PreviewModel( - uri = uri(1), - mimeType = "image/png", - order = 0, - ).let { mapOf(it.uri to it) } + uri = uri(1), + mimeType = "image/png", + order = 0, + ) + .let { mapOf(it.uri to it) } backgroundScope.launch { cursorPreviewsInteractor.launch(deps.cursor, deps.initialPreviews) } runCurrent() - assertThat(previewSelectionsRepository.selections.value.values).containsExactly( - PreviewModel( - uri = uri(1), - mimeType = "image/bitmap", - order = 1, + assertThat(previewSelectionsRepository.selections.value.values) + .containsExactly( + PreviewModel( + uri = uri(1), + mimeType = "image/bitmap", + order = 1, + ) + ) + } + + @Test + fun testReadFailedPages() = + runTestWithDeps( + initialSelection = listOf(4), + cursor = emptyList(), + cursorStartPosition = 0, + pageSize = 2, + maxLoadedPages = 5, + ) { deps -> + val cursor = + MatrixCursor(arrayOf(URI)).apply { + extras = bundleOf(POSITION to 4) + for (i in 0 until 10) { + addRow(arrayOf(uri(i))) + } + } + val failingPositions = setOf(1, 5, 8) + val failingCursor = + object : Cursor by cursor { + override fun move(offset: Int): Boolean = moveToPosition(position + offset) + + override fun moveToPosition(position: Int): Boolean { + if (failingPositions.contains(position)) { + throw RuntimeException( + "A test exception when moving the cursor to position $position" + ) + } + return cursor.moveToPosition(position) + } + + override fun moveToFirst(): Boolean = moveToPosition(0) + + override fun moveToLast(): Boolean = moveToPosition(count - 1) + + override fun moveToNext(): Boolean = move(1) + + override fun moveToPrevious(): Boolean = move(-1) + } + .viewBy { + getString(0)?.let { uriStr -> + CursorRow(Uri.parse(uriStr), readSize(), position) + } + } + backgroundScope.launch { + cursorPreviewsInteractor.launch(failingCursor, deps.initialPreviews) + } + runCurrent() + + assertThat(cursorPreviewsRepository.previewsModel.value).isNotNull() + assertThat(cursorPreviewsRepository.previewsModel.value!!.previewModels) + .comparingElementsUsing<PreviewModel, Uri>( + Correspondence.transforming({ it.uri }, "has a Uri of") + ) + .containsExactlyElementsIn( + (0..7).filterNot { failingPositions.contains(it) }.map { uri(it) } ) - ) + .inOrder() } } |