From 7c6694eaa67811877adce8bb2d48838fca7e0999 Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Tue, 7 Apr 2026 23:48:11 -0700 Subject: [PATCH 01/12] Enhance ImageWell UI with vertical push animation and sticky content --- .../ui/components/capture/ImageWell.kt | 73 ++++++++++++------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/ImageWell.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/ImageWell.kt index d98b9e437..56f8f7758 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/ImageWell.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/ImageWell.kt @@ -18,22 +18,25 @@ package com.google.jetpackcamera.ui.components.capture import android.net.Uri import androidx.compose.animation.AnimatedContent import androidx.compose.animation.ExperimentalAnimationApi -import androidx.compose.animation.core.spring -import androidx.compose.animation.expandHorizontally -import androidx.compose.animation.fadeIn -import androidx.compose.animation.fadeOut -import androidx.compose.animation.scaleIn +import androidx.compose.animation.core.tween +import androidx.compose.animation.slideInVertically +import androidx.compose.animation.slideOutVertically import androidx.compose.animation.togetherWith import androidx.compose.foundation.Image import androidx.compose.foundation.border import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.IconButtonDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color @@ -52,7 +55,7 @@ import com.google.jetpackcamera.ui.uistate.capture.ImageWellUiState * A composable that displays thumbnail image that can be clicked to open the full media in * post-capture * - * @param imageWellUiState the [ImageWellUiState.LastCapture] for this component + * @param imageWellUiState the [ImageWellUiState] for this component * @param onClick the callback for when the image well is clicked * @param modifier the modifier for this component * @param shape the shape of the image well @@ -61,13 +64,18 @@ import com.google.jetpackcamera.ui.uistate.capture.ImageWellUiState @OptIn(ExperimentalMaterial3ExpressiveApi::class, ExperimentalAnimationApi::class) @Composable fun ImageWell( - imageWellUiState: ImageWellUiState.Content, + imageWellUiState: ImageWellUiState, onClick: () -> Unit, modifier: Modifier = Modifier, shape: Shape = RoundedCornerShape(16.dp), enabled: Boolean = true ) { - val lastCapture = imageWellUiState.mediaDescriptor + val currentContent = (imageWellUiState as? ImageWellUiState.Content)?.mediaDescriptor + var lastValidContent by remember { mutableStateOf(null) } + + if (currentContent != null) { + lastValidContent = currentContent + } Box( modifier = modifier @@ -77,24 +85,37 @@ fun ImageWell( .clip(shape) .clickable(onClick = onClick, enabled = enabled) ) { - AnimatedContent( - targetState = lastCapture, - label = "ImageWellAnimation", - transitionSpec = { - ( - fadeIn() + expandHorizontally() + - scaleIn(animationSpec = spring(0.8f)) - ).togetherWith(fadeOut()) - } - ) { contentDesc -> - contentDesc.thumbnail?.let { - Image( - bitmap = it.asImageBitmap(), - contentDescription = stringResource( - id = R.string.image_well_content_description - ), - contentScale = ContentScale.Crop - ) + lastValidContent?.let { targetContent -> + AnimatedContent( + targetState = targetContent, + modifier = Modifier.fillMaxSize(), + label = "ImageWellAnimation", + contentKey = { it.uri }, + transitionSpec = { + val enter = slideInVertically( + initialOffsetY = { -it }, + animationSpec = tween(300) + ) + val exit = slideOutVertically( + targetOffsetY = { it }, + animationSpec = tween(300) + ) + enter.togetherWith(exit).apply { + targetContentZIndex = 1f + } + } + ) { contentDesc -> + contentDesc.thumbnail?.let { bitmap -> + val imageBitmap = remember(bitmap) { bitmap.asImageBitmap() } + Image( + bitmap = imageBitmap, + modifier = Modifier.fillMaxSize(), + contentDescription = stringResource( + id = R.string.image_well_content_description + ), + contentScale = ContentScale.Crop + ) + } } } } From 56e46c7286fee664fc58f3ee36786dda5ddfd059 Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Thu, 11 Jun 2026 23:33:21 -0700 Subject: [PATCH 02/12] Optimize MediaStore repository with reactive flow, URI filtering, and deletion fallback --- app/build.gradle.kts | 1 + .../data/media/LocalMediaRepository.kt | 231 +++++-- .../data/media/MediaRepository.kt | 14 +- .../media/FakeContentProvider.kt | 136 +++- .../media/LocalMediaRepositoryTest.kt | 608 ++++++++++-------- .../data/media/testing/FakeMediaRepository.kt | 43 +- 6 files changed, 708 insertions(+), 325 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index be7fcd1ed..437592577 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -152,6 +152,7 @@ dependencies { // Access settings & model data implementation(project(":data:settings")) + implementation(project(":data:media")) implementation(project(":core:model")) // Camera Preview diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt index 4950fa147..d0f48981d 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt @@ -19,6 +19,7 @@ import android.content.ContentResolver import android.content.ContentUris import android.content.ContentValues import android.content.Context +import android.database.ContentObserver import android.graphics.Bitmap import android.graphics.BitmapFactory import android.graphics.ImageDecoder @@ -39,8 +40,16 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.mapLatest +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.withContext @@ -57,8 +66,83 @@ class LocalMediaRepository private val repositoryScope = CoroutineScope(iODispatcher + SupervisorJob()) private val _currentMedia = MutableStateFlow(MediaDescriptor.None) + private var thumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { uri, collectionUri -> + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + context.contentResolver.loadThumbnail(uri, Size(640, 480), null) + } else { + if (collectionUri == MediaStore.Images.Media.EXTERNAL_CONTENT_URI) { + MediaStore.Images.Thumbnails.getThumbnail( + context.contentResolver, + ContentUris.parseId(uri), + MediaStore.Images.Thumbnails.MINI_KIND, + null + ) + } else { // Video + MediaStore.Video.Thumbnails.getThumbnail( + context.contentResolver, + ContentUris.parseId(uri), + MediaStore.Video.Thumbnails.MINI_KIND, + null + ) + } + } + } + + /** + * Sets a custom thumbnail loader. Primarily used for testing. + */ + internal fun setThumbnailLoader(loader: suspend (Uri, Uri) -> Bitmap?) { + thumbnailLoader = loader + } + override val currentMedia = _currentMedia.asStateFlow() + /** + * A [StateFlow] that emits the most recently captured media (image or video) from the MediaStore. + */ + @OptIn(ExperimentalCoroutinesApi::class) + override val lastCapturedMedia: StateFlow = + mediaStoreChangesFlow(context.contentResolver) + .mapLatest { changedUri -> + val targetUri = when { + changedUri == null || isCollectionUri(changedUri) -> findLatestAppSpecificUri() + isAppSpecificUri(changedUri) -> changedUri + else -> null + } + + if (targetUri != null) { + getCapturedMedia(targetUri) + } else { + val currentCachedUri = cachedUri + if (currentCachedUri != null && !exists(currentCachedUri)) { + // The current media was deleted. Find the next most recent one. + val fallbackUri = findLatestAppSpecificUri() + if (fallbackUri != null) { + getCapturedMedia(fallbackUri) + } else { + cachedUri = null + cachedMediaDescriptor = null + MediaDescriptor.None + } + } else { + // Something else changed, but our current media is still valid. + cachedMediaDescriptor ?: MediaDescriptor.None + } + } + } + .distinctUntilChanged() + .stateIn( + scope = repositoryScope, + started = SharingStarted.Eagerly, + initialValue = MediaDescriptor.None + ) + + private fun isCollectionUri(uri: Uri): Boolean { + val segments = uri.pathSegments + return (segments.contains("images") || segments.contains("video")) && + segments.lastOrNull() == "media" + } + /** * Sets the current media descriptor. * @@ -66,7 +150,6 @@ class LocalMediaRepository */ override suspend fun setCurrentMedia(pendingMedia: MediaDescriptor) { _currentMedia.update { pendingMedia } - Log.d(TAG, "set new media $pendingMedia") } /** @@ -128,12 +211,84 @@ class LocalMediaRepository return@withContext false } + private var cachedUri: Uri? = null + private var cachedMediaDescriptor: MediaDescriptor? = null + /** - * Returns the most recent captured media (image or video) from the MediaStore. + * Returns the [MediaDescriptor] for the given [Uri] from the MediaStore. * - * @return The [MediaDescriptor] of the last captured media, or [MediaDescriptor.None] if no media is found. + * @param uri The [Uri] of the media to retrieve. + * @return The [MediaDescriptor] of the media, or [MediaDescriptor.None] if no media is found. */ - override suspend fun getLastCapturedMedia(): MediaDescriptor { + private suspend fun getCapturedMedia(uri: Uri): MediaDescriptor { + val cachedDesc = cachedMediaDescriptor + if (uri == cachedUri && + cachedDesc is MediaDescriptor.Content && + cachedDesc.thumbnail != null + ) { + return cachedDesc + } + + val descriptor = if (uri.toString().contains("video")) { + getVideoMediaDescriptor(uri) + } else { + getImageMediaDescriptor(uri) + } + + if (descriptor is MediaDescriptor.Content && descriptor.thumbnail != null) { + cachedUri = uri + cachedMediaDescriptor = descriptor + return descriptor + } + + if (cachedDesc != null && cachedDesc is MediaDescriptor.Content) { + // The new thumbnail isn't ready yet, so return the old cached image to prevent transient unmounting. + return cachedDesc + } + + return descriptor + } + + /** + * Checks if the given [Uri] belongs to the app. + * + * @param uri The [Uri] to check. + * @return `true` if the URI is app-specific, `false` otherwise. + */ + private suspend fun isAppSpecificUri(uri: Uri): Boolean = withContext(iODispatcher) { + val projection = mutableListOf(MediaStore.MediaColumns.DISPLAY_NAME) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + projection.add(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) + } + + try { + context.contentResolver.query(uri, projection.toTypedArray(), null, null, null) + ?.use { cursor -> + if (cursor.moveToFirst()) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + val ownerColumn = + cursor.getColumnIndex(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) + if (ownerColumn != -1 && !cursor.isNull(ownerColumn)) { + val owner = cursor.getString(ownerColumn) + if (owner == context.packageName) return@withContext true + } + } + val nameColumn = + cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME) + val name = cursor.getString(nameColumn) + return@withContext name?.startsWith("JCA") == true + } + } + } catch (e: Exception) { + Log.e(TAG, "Error checking app specificity for $uri", e) + } + false + } + + /** + * Returns the most recent app-specific media URI from the MediaStore. + */ + private suspend fun findLatestAppSpecificUri(): Uri? = withContext(iODispatcher) { val imagePair = getLastSavedMediaUriWithDate( context.contentResolver, @@ -145,23 +300,13 @@ class LocalMediaRepository MediaStore.Video.Media.EXTERNAL_CONTENT_URI ) - return if (imagePair != null && videoPair != null) { - // Case 1: BOTH exist. Compare dates. - if (imagePair.second >= videoPair.second) { - getImageMediaDescriptor(imagePair.first) - } else { - getVideoMediaDescriptor(videoPair.first) - } - } else if (imagePair != null) { - // Case 2: Only image exists - getImageMediaDescriptor(imagePair.first) - } else if (videoPair != null) { - // Case 3: Only video exists - getVideoMediaDescriptor(videoPair.first) + val latestPair = if (imagePair != null && videoPair != null) { + if (imagePair.second >= videoPair.second) imagePair else videoPair } else { - // Case 4: Neither exist - MediaDescriptor.None + imagePair ?: videoPair } + + latestPair?.first } /** @@ -432,28 +577,10 @@ class LocalMediaRepository withContext(iODispatcher) { if (uri.scheme != ContentResolver.SCHEME_CONTENT) { Log.e(TAG, "URI is not managed by a content provider") - return@withContext null + null } else { - return@withContext try { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - context.contentResolver.loadThumbnail(uri, Size(640, 480), null) - } else { - if (collectionUri == MediaStore.Images.Media.EXTERNAL_CONTENT_URI) { - MediaStore.Images.Thumbnails.getThumbnail( - context.contentResolver, - ContentUris.parseId(uri), - MediaStore.Images.Thumbnails.MINI_KIND, - null - ) - } else { // Video - MediaStore.Video.Thumbnails.getThumbnail( - context.contentResolver, - ContentUris.parseId(uri), - MediaStore.Video.Thumbnails.MINI_KIND, - null - ) - } - } + try { + thumbnailLoader(uri, collectionUri) } catch (e: Exception) { Log.e(TAG, "Error retrieving thumbnail: ${e.message}", e) null @@ -509,3 +636,27 @@ class LocalMediaRepository return null } } + +private fun mediaStoreChangesFlow(contentResolver: ContentResolver): Flow = callbackFlow { + val observer = object : ContentObserver(null) { + override fun onChange(selfChange: Boolean, uri: Uri?) { + trySend(uri) + } + } + contentResolver.registerContentObserver( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + true, + observer + ) + contentResolver.registerContentObserver( + MediaStore.Video.Media.EXTERNAL_CONTENT_URI, + true, + observer + ) + // Trigger initial emission + trySend(null) + + awaitClose { + contentResolver.unregisterContentObserver(observer) + } +} diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/MediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/MediaRepository.kt index a93a20123..429b7e0dd 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/MediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/MediaRepository.kt @@ -24,8 +24,16 @@ import kotlinx.coroutines.flow.StateFlow */ interface MediaRepository { val currentMedia: StateFlow + + /** + * A [StateFlow] that emits the most recently captured media (image or video) from the MediaStore. + * + * The flow will emit a new [MediaDescriptor] whenever a new media item is saved to the + * MediaStore. The initial value is the most recent media available at the time of collection. + */ + val lastCapturedMedia: StateFlow + suspend fun setCurrentMedia(pendingMedia: MediaDescriptor) - suspend fun getLastCapturedMedia(): MediaDescriptor suspend fun deleteMedia(mediaDescriptor: MediaDescriptor.Content): Boolean @@ -50,13 +58,13 @@ sealed interface MediaDescriptor { val thumbnail: Bitmap? val isCached: Boolean - class Image( + data class Image( override val uri: Uri, override val thumbnail: Bitmap?, override val isCached: Boolean = false ) : Content - class Video( + data class Video( override val uri: Uri, override val thumbnail: Bitmap?, override val isCached: Boolean = false diff --git a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt index b28301ef8..34c8d434c 100644 --- a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt +++ b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt @@ -45,7 +45,8 @@ import java.io.OutputStream */ class FakeContentProvider : ContentProvider() { - private val mediaStore: MutableMap = mutableMapOf() + private val mediaStore: MutableMap = mutableMapOf() + private val thumbnailFailures = mutableSetOf() private var nextId = 1L private var failNextInsert = false @@ -53,6 +54,19 @@ class FakeContentProvider : ContentProvider() { failNextInsert = fail } + /** + * Toggles whether thumbnail generation (via openFile) should fail for a specific URI. + */ + fun setThumbnailFail(uri: Uri, fail: Boolean) { + if (fail) { + thumbnailFailures.add( + uri.toString() + ) + } else { + thumbnailFailures.remove(uri.toString()) + } + } + override fun onCreate(): Boolean { return true } @@ -66,43 +80,74 @@ class FakeContentProvider : ContentProvider() { ): Cursor { val resolvedProjection = projection ?: arrayOf() val cursor = MatrixCursor(resolvedProjection) + val uriString = uri.toString() - // Case 1: Direct URI lookup (e.g., content://media/external/images/media/123) - if (mediaStore.containsKey(uri)) { - mediaStore[uri]?.let { values -> + // Case 1: Direct URI lookup + if (mediaStore.containsKey(uriString)) { + mediaStore[uriString]?.let { values -> cursor.addRow(createRow(resolvedProjection, values, uri)) } return cursor } - // Case 2: Collection URI lookup (e.g., content://media/external/images/media) - val isImageQuery = uri == MediaStore.Images.Media.EXTERNAL_CONTENT_URI - val isVideoQuery = uri == MediaStore.Video.Media.EXTERNAL_CONTENT_URI + // Case 2: Collection URI lookup + val segments = uri.pathSegments + val isImageCollection = segments.contains("images") && segments.last() == "media" + val isVideoCollection = segments.contains("video") && segments.last() == "media" - if (isImageQuery || isVideoQuery) { - val relevantMediaStore = mediaStore.entries.filter { - val keyString = it.key.toString() - if (isImageQuery) { - keyString.contains("images") + if (isImageCollection || isVideoCollection) { + var filteredMedia = mediaStore.entries.filter { + val keyUri = Uri.parse(it.key) + if (isImageCollection) { + keyUri.pathSegments.contains("images") } else { - keyString.contains("video") + keyUri.pathSegments.contains("video") + } + } + + // Simple support for DISPLAY_NAME LIKE ? + if (selection != null && selection.contains( + MediaStore.MediaColumns.DISPLAY_NAME + ) && selectionArgs != null + ) { + val pattern = selectionArgs[0].replace("%", ".*").replace("_", ".") + val regex = Regex(pattern) + filteredMedia = filteredMedia.filter { + val name = it.value.getAsString(MediaStore.MediaColumns.DISPLAY_NAME) ?: "" + regex.matches(name) } } - val sortedMedia = relevantMediaStore - .sortedByDescending { it.value.getAsLong(MediaStore.MediaColumns.DATE_ADDED) } + val sortedMedia = filteredMedia + .sortedByDescending { + it.value.getAsLong(MediaStore.MediaColumns.DATE_ADDED) ?: 0L + } - for ((itemUri, values) in sortedMedia) { - cursor.addRow(createRow(resolvedProjection, values, itemUri)) + for ((itemUriString, values) in sortedMedia) { + cursor.addRow(createRow(resolvedProjection, values, Uri.parse(itemUriString))) } + return cursor } + + // If it's a specific URI that wasn't found in Case 1, return empty cursor return cursor } private fun createRow(projection: Array, values: ContentValues, uri: Uri): Array { + val packageName = context?.packageName return projection.map { proj -> when (proj) { MediaStore.MediaColumns._ID -> uri.lastPathSegment?.toLong() + MediaStore.MediaColumns.OWNER_PACKAGE_NAME -> { + values.getAsString( + proj + ) ?: if (values.containsKey(MediaStore.MediaColumns.DISPLAY_NAME)) { + val name = values.getAsString(MediaStore.MediaColumns.DISPLAY_NAME) + if (name?.startsWith("JCA") == true) packageName else "com.other.app" + } else { + packageName + } + } else -> values.get(proj) } }.toTypedArray() @@ -119,13 +164,35 @@ class FakeContentProvider : ContentProvider() { } if (values == null) return null val newUri = Uri.withAppendedPath(uri, nextId.toString()) - mediaStore[newUri] = values + mediaStore[newUri.toString()] = values + + // Proactively create the file and write a dummy bitmap so loadThumbnail succeeds + context?.let { ctx -> + val file = File(ctx.cacheDir, newUri.lastPathSegment ?: "tempfile") + if (!file.exists() || file.length() == 0L) { + file.createNewFile() + val bitmap = android.graphics.Bitmap.createBitmap( + 1, + 1, + android.graphics.Bitmap.Config.ARGB_8888 + ) + file.outputStream().use { out -> + bitmap.compress(android.graphics.Bitmap.CompressFormat.JPEG, 100, out) + } + } + } + nextId++ return newUri } override fun delete(uri: Uri, selection: String?, selectionArgs: Array?): Int { - return if (mediaStore.remove(uri) != null) 1 else 0 + val uriString = uri.toString() + context?.let { ctx -> + val file = File(ctx.cacheDir, uri.lastPathSegment ?: "tempfile") + if (file.exists()) file.delete() + } + return if (mediaStore.remove(uriString) != null) 1 else 0 } override fun update( @@ -134,24 +201,49 @@ class FakeContentProvider : ContentProvider() { selection: String?, selectionArgs: Array? ): Int { - if (mediaStore.containsKey(uri) && values != null) { - mediaStore[uri]?.putAll(values) + val uriString = uri.toString() + if (mediaStore.containsKey(uriString) && values != null) { + mediaStore[uriString]?.putAll(values) return 1 } return 0 } fun get(uri: Uri): ContentValues? { - return mediaStore[uri] + return mediaStore[uri.toString()] + } + + override fun openTypedAssetFile( + uri: Uri, + mimeTypeFilter: String, + opts: android.os.Bundle?, + signal: android.os.CancellationSignal? + ): android.content.res.AssetFileDescriptor? { + val pfd = openFile(uri, "r") + val file = File(context?.cacheDir, uri.lastPathSegment ?: "tempfile") + return pfd?.let { android.content.res.AssetFileDescriptor(it, 0, file.length()) } } override fun openFile(uri: Uri, mode: String): android.os.ParcelFileDescriptor? { val context = context ?: return null val file = File(context.cacheDir, uri.lastPathSegment ?: "tempfile") try { + if (thumbnailFailures.contains(uri.toString())) return null + if (!file.exists()) { file.createNewFile() + if (mode == "r") { + val bitmap = android.graphics.Bitmap.createBitmap( + 1, + 1, + android.graphics.Bitmap.Config.ARGB_8888 + ) + file.outputStream().use { out -> + bitmap.compress(android.graphics.Bitmap.CompressFormat.JPEG, 100, out) + } + } } + val accessMode = android.os.ParcelFileDescriptor.parseMode(mode) return android.os.ParcelFileDescriptor.open(file, accessMode) } catch (e: FileNotFoundException) { diff --git a/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt b/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt index 6974b6ec8..73d773700 100644 --- a/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt +++ b/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt @@ -32,7 +32,7 @@ import com.google.jetpackcamera.data.media.MediaDescriptor import java.io.File import java.io.FileOutputStream import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Assert.fail import org.junit.Before @@ -51,9 +51,14 @@ class LocalMediaRepositoryTest { private lateinit var context: Context private lateinit var contentResolver: ContentResolver private lateinit var repository: LocalMediaRepository - private val testDispatcher = StandardTestDispatcher() + private val testDispatcher = UnconfinedTestDispatcher() private lateinit var fakeContentProvider: FakeContentProvider + // Reliable fake thumbnail loader for tests + private val fakeThumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { _, _ -> + Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + } + @Before fun setup() { context = ApplicationProvider.getApplicationContext() @@ -68,15 +73,79 @@ class LocalMediaRepositoryTest { context, testDispatcher, FakeFilePathGenerator() - ) + ).apply { + setThumbnailLoader(fakeThumbnailLoader) + } } @Test - fun setCurrentMedia_updatesStateFlow() = runTest(testDispatcher) { + fun lastCapturedMedia_initialValueIsLatest() = runTest { // Given - val initialMedia = repository.currentMedia.value - assertThat(initialMedia).isEqualTo(MediaDescriptor.None) + val olderImageTime = 1000L + val newerVideoTime = 5000L + val imageValues = ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, olderImageTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + val videoValues = ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, newerVideoTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") + } + fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! + val videoUrl = + fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! + + // When initializing a new repository + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) + } + val result = newRepo.lastCapturedMedia.value + + // Then + assertThat(result).isInstanceOf(MediaDescriptor.Content.Video::class.java) + assertThat((result as MediaDescriptor.Content.Video).uri).isEqualTo(videoUrl) + } + + @Test + fun lastCapturedMedia_emitsNewImageOnContentChange() = runTest { + // When a new image is added + val imageValues = ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 6000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image_New.jpg") + } + val imageUrl = + fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! + + // Notify change and let coroutines process + contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) + + // Then + val result = repository.lastCapturedMedia.value + assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) + assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) + } + + @Test + fun lastCapturedMedia_emitsNewVideoOnContentChange() = runTest { + // When a new video is added + val videoValues = ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 7000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video_New.mp4") + } + val videoUrl = + fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! + + // Notify change and let coroutines process + contentResolver.notifyChange(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, null) + + // Then + val result = repository.lastCapturedMedia.value + assertThat(result).isInstanceOf(MediaDescriptor.Content.Video::class.java) + assertThat((result as MediaDescriptor.Content.Video).uri).isEqualTo(videoUrl) + } + @Test + fun setCurrentMedia_updatesStateFlow() = runTest { // When val newMedia = MediaDescriptor.Content.Image( Uri.parse("content://media/external/images/media/1"), @@ -90,28 +159,17 @@ class LocalMediaRepositoryTest { } @Test - fun loadImage_succeeds_returnsImageMedia() = runTest(testDispatcher) { - // 1. Create a real, decodable Bitmap + fun loadImage_succeeds_returnsImageMedia() = runTest { val bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) - - // 2. Given a valid image URI val sourceFile = File(context.cacheDir, "temp.jpg") - - // Write the actual Bitmap data as a compressed JPEG try { FileOutputStream(sourceFile).use { outputStream -> - // Use compress to write a valid image format bitmap.compress(Bitmap.CompressFormat.JPEG, 100, outputStream) } } catch (e: Exception) { - // Handle potential IO exceptions if necessary fail("Failed to write mock image data: ${e.message}") } - // Check if the file was created and is non-empty - assertThat(sourceFile.exists()).isTrue() - assertThat(sourceFile.length()).isGreaterThan(0) - val imageUri = Uri.fromFile(sourceFile) val mediaDescriptor = MediaDescriptor.Content.Image(imageUri, null, true) @@ -123,20 +181,9 @@ class LocalMediaRepositoryTest { } @Test - fun loadVideo_succeeds_returnsVideoMedia() = runTest(testDispatcher) { - // 1. Setup: Create a temporary file in the cache directory + fun loadVideo_succeeds_returnsVideoMedia() = runTest { val sourceFile = File(context.cacheDir, "temp_video.mp4") - - // 2. Write a small amount of data to make it non-empty. - // Unlike images, video content doesn't need to be fully valid to pass the existence check. - // However, if the repository eventually uses a video decoder for metadata/thumbnail, - // a small amount of non-zero data ensures the file exists and is readable. sourceFile.writeText("fake video content") - - // Ensure the file exists before proceeding - assertThat(sourceFile.exists()).isTrue() - - // 3. Given a valid video URI (file:// pointing to the real file) val videoUri = Uri.fromFile(sourceFile) val mediaDescriptor = MediaDescriptor.Content.Video(videoUri, null, true) @@ -149,8 +196,7 @@ class LocalMediaRepositoryTest { } @Test - fun loadImage_fails_returnsError() = runTest(testDispatcher) { - // Given an invalid image URI + fun loadImage_fails_returnsError() = runTest { val invalidImageUri = Uri.parse("file:///nonexistent/image.jpg") val mediaDescriptor = MediaDescriptor.Content.Image(invalidImageUri, null, true) @@ -162,103 +208,67 @@ class LocalMediaRepositoryTest { } @Test - fun loadVideo_fails_returnsError() = runTest(testDispatcher) { - val nonExistentPath = "/nonexistent/path/video_not_here.mp4" - val nonExistentUri = Uri.parse("file://$nonExistentPath") - - // Explicitly verify file does not exist (for robust setup assertion) - assertThat(File(nonExistentPath).exists()).isFalse() + fun loadVideo_fails_returnsError() = runTest { + val nonExistentUri = Uri.parse("file:///nonexistent/video.mp4") + val mediaDescriptor = MediaDescriptor.Content.Video(nonExistentUri, null, true) - val mediaDescriptor = MediaDescriptor.Content.Video( - uri = nonExistentUri, - thumbnail = null, - isCached = true - ) - - // 2. When: The repository attempts to load the non-existent video. + // When val result = repository.load(mediaDescriptor) - // 3. Then: The result should be Media.Error because the existence check failed. + // Then assertThat(result).isEqualTo(Media.Error) } @Test - fun load_none_returnsNone() = runTest(testDispatcher) { - // When + fun load_none_returnsNone() = runTest { val result = repository.load(MediaDescriptor.None) - // Then assertThat(result).isEqualTo(Media.None) } @Test - fun deleteMedia_savedMedia_callsContentResolverDelete() = runTest(testDispatcher) { - val baseUri = MediaStore.Images.Media.EXTERNAL_CONTENT_URI - - // Insert test data and capture the URI *returned* by the fake ContentProvider - val insertedUri = fakeContentProvider.insert(baseUri, ContentValues())!! - - // 2. Create the MediaDescriptor using the URI returned by the insert - val mediaToDelete = MediaDescriptor.Content.Image( - insertedUri, // Use the URI with the correct generated ID - thumbnail = null, - isCached = false - ) + fun deleteMedia_savedMedia_callsContentResolverDelete() = runTest { + val insertedUri = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_To_Delete.jpg") + } + )!! - // Verify it exists before deleting - var cursor = fakeContentProvider.query(insertedUri, null, null, null, null) - assertThat(cursor.count).isEqualTo(1) + val mediaToDelete = MediaDescriptor.Content.Image(insertedUri, null, false) - // 3. When + // When repository.deleteMedia(mediaToDelete) - // 4. Then - // Query using the correct, inserted URI - cursor = fakeContentProvider.query(insertedUri, null, null, null, null) + // Then + val cursor = fakeContentProvider.query(insertedUri, null, null, null, null) assertThat(cursor.count).isEqualTo(0) } @Test - fun deleteMedia_cachedMedia_deletesRealFile() = runTest(testDispatcher) { - // 1. Setup: Create a REAL temporary file in the app's cache directory - // ApplicationProvider gives us a working context for file ops in Robolectric - val cacheDir = ApplicationProvider.getApplicationContext().cacheDir - if (!cacheDir.exists()) cacheDir.mkdirs() - - val tempFile = File(cacheDir, "temp_test_video.mp4") - tempFile.createNewFile() // Actually creates the empty file on disk - + fun deleteMedia_cachedMedia_deletesRealFile() = runTest { + val tempFile = File(context.cacheDir, "temp_to_delete.mp4") + tempFile.createNewFile() assertThat(tempFile.exists()).isTrue() - // 2. Create the descriptor pointing to this real file - // Uri.fromFile() creates a "file://" URI, which is what your app likely uses for cached media - val cachedUri = Uri.fromFile(tempFile) - val mediaToDelete = MediaDescriptor.Content.Video( - cachedUri, - thumbnail = null, - isCached = true - ) + val mediaToDelete = MediaDescriptor.Content.Video(Uri.fromFile(tempFile), null, true) - // 3. Act: Call deleteMedia + // When repository.deleteMedia(mediaToDelete) - // 4. Assert: Verify the file is physically gone + // Then assertThat(tempFile.exists()).isFalse() } @Test - fun deleteMedia_currentMedia_resetsToNone() = runTest(testDispatcher) { - // Given a media item that is currently set as the active media + fun deleteMedia_currentMedia_resetsToNone() = runTest { val returnedUri = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues() + ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Active.jpg") + } )!! - val mediaToDelete = MediaDescriptor.Content.Image( - returnedUri, - thumbnail = null, - isCached = false - ) + val mediaToDelete = MediaDescriptor.Content.Image(returnedUri, null, false) repository.setCurrentMedia(mediaToDelete) - assertThat(repository.currentMedia.value).isEqualTo(mediaToDelete) // When repository.deleteMedia(mediaToDelete) @@ -268,237 +278,327 @@ class LocalMediaRepositoryTest { } @Test - fun getLastCapturedMedia_videoIsNewer_returnsVideo() = runTest(testDispatcher) { - // Given + fun lastCapturedMedia_videoIsNewer_returnsVideo() = runTest { val olderImageTime = 1000L val newerVideoTime = 5000L + fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, olderImageTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + ) + val videoUrl = fakeContentProvider.insert( + MediaStore.Video.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, newerVideoTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") + } + )!! - // Insert mock data into the fake provider - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, olderImageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, newerVideoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) } - fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! - val videoUrl = - fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! - - // When - val result = repository.getLastCapturedMedia() + val result = newRepo.lastCapturedMedia.value - // Then assertThat(result).isInstanceOf(MediaDescriptor.Content.Video::class.java) assertThat((result as MediaDescriptor.Content.Video).uri).isEqualTo(videoUrl) } @Test - fun getLastCapturedMedia_imageIsNewer_returnsImage() = runTest(testDispatcher) { - // Given + fun lastCapturedMedia_imageIsNewer_returnsImage() = runTest { val newerImageTime = 9000L val olderVideoTime = 2000L + val imageUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, newerImageTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + )!! + fakeContentProvider.insert( + MediaStore.Video.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, olderVideoTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") + } + ) - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, newerImageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) } - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, olderVideoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - val imageUrl = - fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! - fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! - - // When - val result = repository.getLastCapturedMedia() + val result = newRepo.lastCapturedMedia.value - // Then assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) } @Test - fun getLastCapturedMedia_nothingFound_returnsNone() = runTest(testDispatcher) { - // Given an empty provider - // When - val result = repository.getLastCapturedMedia() - // Then - assertThat(result).isEqualTo(MediaDescriptor.None) + fun lastCapturedMedia_nothingFound_returnsNone() = runTest { + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) + } + assertThat(newRepo.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) } @Test - fun getLastCapturedMedia_equalTimestamps_returnsImage() = runTest(testDispatcher) { - // Given + fun lastCapturedMedia_equalTimestamps_returnsImage() = runTest { val sameTime = 9999L - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, sameTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, sameTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - val imageUrl = - fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! - fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! - - // When - val result = repository.getLastCapturedMedia() - - // Then - assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) - assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) - } + val imageUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, sameTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + )!! + fakeContentProvider.insert( + MediaStore.Video.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, sameTime) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") + } + ) - @Test - fun getLastCapturedMedia_onlyImageExists_returnsImage() = runTest(testDispatcher) { - // Given - val imageTime = 10000L - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, imageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) } - val imageUrl = - fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! - - // When - val result = repository.getLastCapturedMedia() + val result = newRepo.lastCapturedMedia.value - // Then assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) } @Test - fun getLastCapturedMedia_onlyVideoExists_returnsVideo() = runTest(testDispatcher) { - // Given - val videoTime = 11000L - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, videoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - val videoUrl = - fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! - - // When - val result = repository.getLastCapturedMedia() - - // Then - assertThat(result).isInstanceOf(MediaDescriptor.Content.Video::class.java) - assertThat((result as MediaDescriptor.Content.Video).uri).isEqualTo(videoUrl) - } - - @Test - fun deleteMedia_nonExistentUri_doesNotThrow() = runTest(testDispatcher) { - // Given a URI that does not exist in the provider + fun deleteMedia_nonExistentUri_doesNotThrow() = runTest { val nonExistentUri = ContentUris.withAppendedId( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 999L ) - val mediaToDelete = MediaDescriptor.Content.Image( - nonExistentUri, - thumbnail = null, - isCached = false - ) - - // When & Then (The test passes if no exception is thrown) + val mediaToDelete = MediaDescriptor.Content.Image(nonExistentUri, null, false) repository.deleteMedia(mediaToDelete) } @Test - fun saveToMediaStore_video_success_returnsNewUri() = runTest(testDispatcher) { - // Given + fun saveToMediaStore_video_success_returnsNewUri() = runTest { val sourceFile = File(context.cacheDir, "temp.mp4") sourceFile.writeText("fake video data") - val sourceUri = Uri.fromFile(sourceFile) - - val mediaDescriptor = MediaDescriptor.Content.Video( - sourceUri, - thumbnail = null, - isCached = true - ) - - // When - val result = repository.saveToMediaStore( + val mediaDescriptor = MediaDescriptor.Content.Video(Uri.fromFile(sourceFile), null, true) - mediaDescriptor, - "my_video.mp4" - ) + val result = repository.saveToMediaStore(mediaDescriptor, "my_video.mp4") - // Then assertThat(result).isNotNull() - // Check that the media is in the fake provider with the correct name val values = fakeContentProvider.get(result!!) assertThat(values?.get(MediaStore.MediaColumns.DISPLAY_NAME)).isEqualTo("my_video.mp4") } @Test - fun saveToMediaStore_success_returnsNewUri() = runTest(testDispatcher) { - // Given + fun saveToMediaStore_success_returnsNewUri() = runTest { val sourceFile = File(context.cacheDir, "temp.jpg") sourceFile.writeText("fake image data") - val sourceUri = Uri.fromFile(sourceFile) - val mediaDescriptor = MediaDescriptor.Content.Image( - sourceUri, - thumbnail = null, - isCached = true - ) - - // When - val result = repository.saveToMediaStore( + val mediaDescriptor = MediaDescriptor.Content.Image(Uri.fromFile(sourceFile), null, true) - mediaDescriptor, - "my_photo.jpg" - ) + val result = repository.saveToMediaStore(mediaDescriptor, "my_photo.jpg") - // Then assertThat(result).isNotNull() val values = fakeContentProvider.get(result!!) assertThat(values?.get(MediaStore.MediaColumns.DISPLAY_NAME)).isEqualTo("my_photo.jpg") } @Test - fun saveToMediaStore_insertFails_returnsNull() = runTest(testDispatcher) { - // Given + fun saveToMediaStore_insertFails_returnsNull() = runTest { val sourceFile = File(context.cacheDir, "temp.jpg") sourceFile.writeText("fake image data") - val sourceUri = Uri.fromFile(sourceFile) - val mediaDescriptor = MediaDescriptor.Content.Image( - sourceUri, - thumbnail = null, - isCached = true - ) - // Simulate an insert failure + val mediaDescriptor = MediaDescriptor.Content.Image(Uri.fromFile(sourceFile), null, true) fakeContentProvider.setFailNextInsert(true) - // When - val result = repository.saveToMediaStore( - - mediaDescriptor, - "my_photo.jpg" - ) + val result = repository.saveToMediaStore(mediaDescriptor, "my_photo.jpg") - // Then assertThat(result).isNull() } @Test - fun saveToMediaStore_copyFails_returnsNull() = runTest(testDispatcher) { - // Given a source URI that points to a non-existent file + fun saveToMediaStore_copyFails_returnsNull() = runTest { val sourceUri = Uri.parse("file:///nonexistent/file.jpg") - val mediaDescriptor = MediaDescriptor.Content.Image( - sourceUri, - thumbnail = null, - isCached = true - ) + val mediaDescriptor = MediaDescriptor.Content.Image(sourceUri, null, true) - // When val result = repository.saveToMediaStore(mediaDescriptor, "broken.jpg") - // Then assertThat(result).isNull() } + + @Test + fun lastCapturedMedia_ignoresNonAppMediaStoreChange() = runTest { + // Given a JCA file is current + val jcaUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, context.packageName) + } + )!! + contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) + + val jcaDescriptor = repository.lastCapturedMedia.value + assertThat(jcaDescriptor).isInstanceOf(MediaDescriptor.Content::class.java) + + // When a non-JCA file from a different app is inserted and notified + val otherUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 5000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "OTHER_Image.jpg") + put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, "com.other.app") + } + )!! + contentResolver.notifyChange(otherUrl, null) + + // Then the flow still points to the JCA file + assertThat(repository.lastCapturedMedia.value).isEqualTo(jcaDescriptor) + } + + @Test + fun lastCapturedMedia_initialLoad_ignoresNonAppFiles() = runTest { + val jcaUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, context.packageName) + } + )!! + fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 5000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "OTHER_Image.jpg") + put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, "com.other.app") + } + )!! + + val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + setThumbnailLoader(fakeThumbnailLoader) + } + val result = newRepo.lastCapturedMedia.value + + assertThat(result).isInstanceOf(MediaDescriptor.Content::class.java) + assertThat((result as MediaDescriptor.Content).uri).isEqualTo(jcaUrl) + } + + @Test + fun lastCapturedMedia_multipleEventsForSameUri_emitsSameObjectReference() = runTest { + val jcaUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + )!! + contentResolver.notifyChange(jcaUrl, null) + val firstEmission = repository.lastCapturedMedia.value + + contentResolver.notifyChange(jcaUrl, null) + contentResolver.notifyChange(jcaUrl, null) + + assertThat(repository.lastCapturedMedia.value).isSameInstanceAs(firstEmission) + } + + @Test + fun lastCapturedMedia_onDeletion_fallsBackToNextLatest() = runTest { + val urlA = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_A.jpg") + } + )!! + val urlB = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 2000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_B.jpg") + } + )!! + + contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) + assertThat( + (repository.lastCapturedMedia.value as MediaDescriptor.Content).uri + ).isEqualTo(urlB) + + fakeContentProvider.delete(urlB, null, null) + contentResolver.notifyChange(urlB, null) + + val result = repository.lastCapturedMedia.value + assertThat(result).isInstanceOf(MediaDescriptor.Content::class.java) + assertThat((result as MediaDescriptor.Content).uri).isEqualTo(urlA) + } + + @Test + fun lastCapturedMedia_onLastItemDeletion_emitsNone() = runTest { + val jcaUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") + } + )!! + contentResolver.notifyChange(jcaUrl, null) + assertThat(repository.lastCapturedMedia.value).isNotEqualTo(MediaDescriptor.None) + + fakeContentProvider.delete(jcaUrl, null, null) + contentResolver.notifyChange(jcaUrl, null) + + assertThat(repository.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) + } + + @Test + fun lastCapturedMedia_newUriWithNullThumbnail_holdsPreviousMedia() = runTest { + // We use a custom repo here so we can control the thumbnail loader specifically for this test + var shouldFailThumbnail = false + val customThumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { _, _ -> + if (shouldFailThumbnail) null else Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + } + val customRepo = LocalMediaRepository( + context, + testDispatcher, + FakeFilePathGenerator() + ).apply { + setThumbnailLoader(customThumbnailLoader) + } + + val oldUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 1000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Old.jpg") + } + )!! + contentResolver.notifyChange(oldUrl, null) + val initialResult = customRepo.lastCapturedMedia.value + assertThat((initialResult as MediaDescriptor.Content).uri).isEqualTo(oldUrl) + + val newUrl = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + ContentValues().apply { + put(MediaStore.MediaColumns.DATE_ADDED, 2000L) + put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_New.jpg") + } + )!! + + // Trigger thumbnail failure + shouldFailThumbnail = true + contentResolver.notifyChange(newUrl, null) + + // Then it holds the previous media + assertThat(customRepo.lastCapturedMedia.value).isEqualTo(initialResult) + + // Thumbnail becomes ready + shouldFailThumbnail = false + contentResolver.notifyChange(newUrl, null) + + // Finally emits new media + val finalResult = customRepo.lastCapturedMedia.value + assertThat(finalResult).isInstanceOf(MediaDescriptor.Content::class.java) + assertThat((finalResult as MediaDescriptor.Content).uri).isEqualTo(newUrl) + } } diff --git a/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt b/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt index e84203bf7..1df48cac6 100644 --- a/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt +++ b/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt @@ -15,21 +15,51 @@ */ package com.google.jetpackcamera.data.media.testing +import android.graphics.Bitmap import android.net.Uri import androidx.core.net.toUri import com.google.jetpackcamera.data.media.Media import com.google.jetpackcamera.data.media.MediaDescriptor import com.google.jetpackcamera.data.media.MediaRepository +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update class FakeMediaRepository : MediaRepository { private val _currentMedia = MutableStateFlow(MediaDescriptor.None) - override val currentMedia = _currentMedia.asStateFlow() - var loadHandler: (MediaDescriptor) -> Media = { Media.None } + private val _lastCapturedMedia = MutableStateFlow(MediaDescriptor.None) + override val lastCapturedMedia: StateFlow = _lastCapturedMedia.asStateFlow() + .filter { + when (it) { + is MediaDescriptor.None -> true + is MediaDescriptor.Content -> it.thumbnail != null + } + } + .distinctUntilChanged() + .stateIn( + scope = CoroutineScope(Dispatchers.Main), + started = SharingStarted.Eagerly, + initialValue = MediaDescriptor.None + ) + + var loadHandler: (MediaDescriptor) -> Media = { mediaDescriptor -> + when (mediaDescriptor) { + is MediaDescriptor.Content.Image -> Media.Image( + Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + ) + is MediaDescriptor.Content.Video -> Media.Video(mediaDescriptor.uri) + else -> Media.None + } + } var saveToMediaStoreHandler: (MediaDescriptor.Content) -> Uri? = { mediaDescriptor -> when (mediaDescriptor) { is MediaDescriptor.Content.Image -> "img.jpg".toUri() @@ -42,10 +72,6 @@ class FakeMediaRepository : MediaRepository { _currentMedia.update { pendingMedia } } - override suspend fun getLastCapturedMedia(): MediaDescriptor { - return MediaDescriptor.None - } - override suspend fun load(mediaDescriptor: MediaDescriptor): Media { return loadHandler(mediaDescriptor) } @@ -67,4 +93,9 @@ class FakeMediaRepository : MediaRepository { override suspend fun copyToUri(mediaDescriptor: MediaDescriptor.Content, destinationUri: Uri) { } + + // Helper for testing + fun setLastCapturedMedia(mediaDescriptor: MediaDescriptor) { + _lastCapturedMedia.value = mediaDescriptor + } } From 761e5d086617f61919bee1fe0c4262a96102ede6 Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Fri, 12 Jun 2026 00:51:26 -0700 Subject: [PATCH 03/12] Address code review: Fix race condition, refine ownership check, and simplify fake repository --- .../data/media/LocalMediaRepository.kt | 63 +++++++++++-------- .../data/media/testing/FakeMediaRepository.kt | 18 ------ 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt index d0f48981d..96e8d1ba5 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt @@ -51,6 +51,8 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext private const val TAG = "LocalMediaRepository" @@ -66,6 +68,10 @@ class LocalMediaRepository private val repositoryScope = CoroutineScope(iODispatcher + SupervisorJob()) private val _currentMedia = MutableStateFlow(MediaDescriptor.None) + private val cacheMutex = Mutex() + private var cachedUri: Uri? = null + private var cachedMediaDescriptor: MediaDescriptor? = null + private var thumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { uri, collectionUri -> if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { context.contentResolver.loadThumbnail(uri, Size(640, 480), null) @@ -104,29 +110,32 @@ class LocalMediaRepository override val lastCapturedMedia: StateFlow = mediaStoreChangesFlow(context.contentResolver) .mapLatest { changedUri -> - val targetUri = when { - changedUri == null || isCollectionUri(changedUri) -> findLatestAppSpecificUri() - isAppSpecificUri(changedUri) -> changedUri - else -> null - } + cacheMutex.withLock { + val targetUri = when { + changedUri == null || isCollectionUri(changedUri) -> + findLatestAppSpecificUri() + isAppSpecificUri(changedUri) -> changedUri + else -> null + } - if (targetUri != null) { - getCapturedMedia(targetUri) - } else { - val currentCachedUri = cachedUri - if (currentCachedUri != null && !exists(currentCachedUri)) { - // The current media was deleted. Find the next most recent one. - val fallbackUri = findLatestAppSpecificUri() - if (fallbackUri != null) { - getCapturedMedia(fallbackUri) + if (targetUri != null) { + getCapturedMediaInternal(targetUri) + } else { + val currentCachedUri = cachedUri + if (currentCachedUri != null && !exists(currentCachedUri)) { + // The current media was deleted. Find the next most recent one. + val fallbackUri = findLatestAppSpecificUri() + if (fallbackUri != null) { + getCapturedMediaInternal(fallbackUri) + } else { + cachedUri = null + cachedMediaDescriptor = null + MediaDescriptor.None + } } else { - cachedUri = null - cachedMediaDescriptor = null - MediaDescriptor.None + // Something else changed, but our current media is still valid. + cachedMediaDescriptor ?: MediaDescriptor.None } - } else { - // Something else changed, but our current media is still valid. - cachedMediaDescriptor ?: MediaDescriptor.None } } } @@ -211,16 +220,20 @@ class LocalMediaRepository return@withContext false } - private var cachedUri: Uri? = null - private var cachedMediaDescriptor: MediaDescriptor? = null - /** * Returns the [MediaDescriptor] for the given [Uri] from the MediaStore. * * @param uri The [Uri] of the media to retrieve. * @return The [MediaDescriptor] of the media, or [MediaDescriptor.None] if no media is found. */ - private suspend fun getCapturedMedia(uri: Uri): MediaDescriptor { + private suspend fun getCapturedMedia(uri: Uri): MediaDescriptor = cacheMutex.withLock { + getCapturedMediaInternal(uri) + } + + /** + * Internal implementation of getCapturedMedia that assumes the [cacheMutex] is already held. + */ + private suspend fun getCapturedMediaInternal(uri: Uri): MediaDescriptor { val cachedDesc = cachedMediaDescriptor if (uri == cachedUri && cachedDesc is MediaDescriptor.Content && @@ -270,7 +283,7 @@ class LocalMediaRepository cursor.getColumnIndex(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) if (ownerColumn != -1 && !cursor.isNull(ownerColumn)) { val owner = cursor.getString(ownerColumn) - if (owner == context.packageName) return@withContext true + return@withContext owner == context.packageName } } val nameColumn = diff --git a/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt b/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt index 1df48cac6..28afd8532 100644 --- a/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt +++ b/data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt @@ -21,15 +21,9 @@ import androidx.core.net.toUri import com.google.jetpackcamera.data.media.Media import com.google.jetpackcamera.data.media.MediaDescriptor import com.google.jetpackcamera.data.media.MediaRepository -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update class FakeMediaRepository : MediaRepository { @@ -38,18 +32,6 @@ class FakeMediaRepository : MediaRepository { private val _lastCapturedMedia = MutableStateFlow(MediaDescriptor.None) override val lastCapturedMedia: StateFlow = _lastCapturedMedia.asStateFlow() - .filter { - when (it) { - is MediaDescriptor.None -> true - is MediaDescriptor.Content -> it.thumbnail != null - } - } - .distinctUntilChanged() - .stateIn( - scope = CoroutineScope(Dispatchers.Main), - started = SharingStarted.Eagerly, - initialValue = MediaDescriptor.None - ) var loadHandler: (MediaDescriptor) -> Media = { mediaDescriptor -> when (mediaDescriptor) { From 690535ec139be8929791d6ae8a1a2aa2c88cc3ee Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Fri, 12 Jun 2026 09:56:05 -0700 Subject: [PATCH 04/12] Address code review and fix b/522926012: Dynamic prefix for media search --- .../com/google/jetpackcamera/JcaFilePathGenerator.kt | 7 +++++-- .../jetpackcamera/core/common/FilePathGenerator.kt | 5 +++++ .../core/common/testing/FakeFilePathGenerator.kt | 6 ++++-- .../jetpackcamera/data/media/LocalMediaRepository.kt | 9 +++++---- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt b/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt index 4debee011..ad58c1e2d 100644 --- a/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt +++ b/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt @@ -25,6 +25,9 @@ class JcaFilePathGenerator : FilePathGenerator { private val RELATIVE_OUTPUT_PATH: String = "${Environment.DIRECTORY_DCIM}${File.separator}Camera" } + + override val prefix: String = "JCA" + override val relativeImageOutputPath: String = RELATIVE_OUTPUT_PATH override val relativeVideoOutputPath: String = RELATIVE_OUTPUT_PATH @@ -64,7 +67,7 @@ class JcaFilePathGenerator : FilePathGenerator { override fun generateImageFilename(suffixText: String?, fileExtension: String?): String { return constructFilename( - "JCA-photo", + "$prefix-photo", createTimestamp().toString(), suffixText, fileExtension @@ -73,7 +76,7 @@ class JcaFilePathGenerator : FilePathGenerator { override fun generateVideoFilename(suffixText: String?, fileExtension: String?): String { return constructFilename( - "JCA-recording", + "$prefix-recording", createTimestamp().toString(), suffixText, fileExtension diff --git a/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt b/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt index 5df44a8c7..fb0205011 100644 --- a/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt +++ b/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt @@ -20,6 +20,11 @@ package com.google.jetpackcamera.core.common */ interface FilePathGenerator { + /** + * The prefix used for generated filenames. + */ + val prefix: String + /** * Provides the relative output path for an image file, used for MediaStore. * diff --git a/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt b/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt index 3c7a65097..0d09c9fb0 100644 --- a/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt +++ b/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt @@ -26,6 +26,8 @@ class FakeFilePathGenerator : FilePathGenerator { "${Environment.DIRECTORY_DCIM}${File.separator}Camera" } + override val prefix: String = "JCA" + override val relativeImageOutputPath: String = RELATIVE_OUTPUT_PATH override val relativeVideoOutputPath: String = RELATIVE_OUTPUT_PATH @@ -63,7 +65,7 @@ class FakeFilePathGenerator : FilePathGenerator { override fun generateImageFilename(suffixText: String?, fileExtension: String?): String { return constructFilename( - "JCA-test-photo", + "$prefix-test-photo", createTimestamp().toString(), suffixText, fileExtension @@ -72,7 +74,7 @@ class FakeFilePathGenerator : FilePathGenerator { override fun generateVideoFilename(suffixText: String?, fileExtension: String?): String { return constructFilename( - "JCA-test-recording", + "$prefix-test-recording", createTimestamp().toString(), suffixText, fileExtension diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt index 96e8d1ba5..f02b2f422 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt @@ -289,7 +289,7 @@ class LocalMediaRepository val nameColumn = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME) val name = cursor.getString(nameColumn) - return@withContext name?.startsWith("JCA") == true + return@withContext name?.startsWith(filePathGenerator.prefix) == true } } } catch (e: Exception) { @@ -603,7 +603,8 @@ class LocalMediaRepository /** * This function queries the MediaStore for media files that have a display name starting with - * "JCA". It returns the URI and date added for the most recently added file. + * the prefix from [filePathGenerator]. It returns the URI and date added for the most recently + * added file. * * @param contentResolver The [ContentResolver] to query the MediaStore. * @param collectionUri The [Uri] of the media collection to query (e.g., [MediaStore.Images.Media.EXTERNAL_CONTENT_URI] or [MediaStore.Video.Media.EXTERNAL_CONTENT_URI]). @@ -618,9 +619,9 @@ class LocalMediaRepository MediaStore.MediaColumns.DATE_ADDED ) - // Filter by filenames starting with "JCA" + // Filter by filenames starting with the prefix from filePathGenerator val selection = "${MediaStore.MediaColumns.DISPLAY_NAME} LIKE ?" - val selectionArgs = arrayOf("JCA%") + val selectionArgs = arrayOf("${filePathGenerator.prefix}%") // Sort the results so that the most recently added media appears first. val sortOrder = "${MediaStore.MediaColumns.DATE_ADDED} DESC" From 20fd15d7bfa316181bccfac617f22f0363dcb6d8 Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Fri, 12 Jun 2026 11:19:03 -0700 Subject: [PATCH 05/12] Address code review and fix b/522926012: Robust RELATIVE_PATH media filtering --- .../jetpackcamera/JcaFilePathGenerator.kt | 2 + .../core/common/FilePathGenerator.kt | 6 +++ .../common/testing/FakeFilePathGenerator.kt | 2 + .../data/media/LocalMediaRepository.kt | 53 +++++++++++++++---- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt b/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt index ad58c1e2d..c0c3d99bf 100644 --- a/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt +++ b/app/src/main/java/com/google/jetpackcamera/JcaFilePathGenerator.kt @@ -28,6 +28,8 @@ class JcaFilePathGenerator : FilePathGenerator { override val prefix: String = "JCA" + override val baseRelativePath: String = RELATIVE_OUTPUT_PATH + override val relativeImageOutputPath: String = RELATIVE_OUTPUT_PATH override val relativeVideoOutputPath: String = RELATIVE_OUTPUT_PATH diff --git a/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt b/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt index fb0205011..a80f22967 100644 --- a/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt +++ b/core/common/src/main/java/com/google/jetpackcamera/core/common/FilePathGenerator.kt @@ -25,6 +25,12 @@ interface FilePathGenerator { */ val prefix: String + /** + * The base relative path where media is stored (e.g., "DCIM/Camera"). + * Used for filtering in the MediaStore on API 29+. + */ + val baseRelativePath: String + /** * Provides the relative output path for an image file, used for MediaStore. * diff --git a/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt b/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt index 0d09c9fb0..044933603 100644 --- a/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt +++ b/core/common/testing/src/main/java/com/google/jetpackcamera/core/common/testing/FakeFilePathGenerator.kt @@ -28,6 +28,8 @@ class FakeFilePathGenerator : FilePathGenerator { override val prefix: String = "JCA" + override val baseRelativePath: String = RELATIVE_OUTPUT_PATH + override val relativeImageOutputPath: String = RELATIVE_OUTPUT_PATH override val relativeVideoOutputPath: String = RELATIVE_OUTPUT_PATH diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt index f02b2f422..720953bcc 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt @@ -281,11 +281,29 @@ class LocalMediaRepository if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { val ownerColumn = cursor.getColumnIndex(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) + val pathColumn = + cursor.getColumnIndex(MediaStore.MediaColumns.RELATIVE_PATH) + + // 1. Check Owner Package if (ownerColumn != -1 && !cursor.isNull(ownerColumn)) { val owner = cursor.getString(ownerColumn) - return@withContext owner == context.packageName + if (owner != context.packageName) return@withContext false + } + + // 2. Check Relative Path (Ensure it's in the camera directory) + if (pathColumn != -1 && !cursor.isNull(pathColumn)) { + val path = cursor.getString(pathColumn) + if (path != null && + !path.startsWith(filePathGenerator.baseRelativePath) + ) { + return@withContext false + } } + + if (ownerColumn != -1 || pathColumn != -1) return@withContext true } + + // 3. Fallback to Display Name Prefix (API 28 or missing modern columns) val nameColumn = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME) val name = cursor.getString(nameColumn) @@ -602,8 +620,8 @@ class LocalMediaRepository } /** - * This function queries the MediaStore for media files that have a display name starting with - * the prefix from [filePathGenerator]. It returns the URI and date added for the most recently + * This function queries the MediaStore for media files that belong to this app and are stored in + * the standard camera directory. It returns the URI and date added for the most recently * added file. * * @param contentResolver The [ContentResolver] to query the MediaStore. @@ -614,14 +632,31 @@ class LocalMediaRepository contentResolver: ContentResolver, collectionUri: Uri ): Pair? { - val projection = arrayOf( + val projection = mutableListOf( MediaStore.MediaColumns._ID, - MediaStore.MediaColumns.DATE_ADDED + MediaStore.MediaColumns.DATE_ADDED, + MediaStore.MediaColumns.DISPLAY_NAME ) - // Filter by filenames starting with the prefix from filePathGenerator - val selection = "${MediaStore.MediaColumns.DISPLAY_NAME} LIKE ?" - val selectionArgs = arrayOf("${filePathGenerator.prefix}%") + val selection: String + val selectionArgs: Array + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + projection.add(MediaStore.MediaColumns.RELATIVE_PATH) + projection.add(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) + + // Primary Filter: Relative Path + App Ownership + selection = "${MediaStore.MediaColumns.RELATIVE_PATH} LIKE ? AND " + + "${MediaStore.MediaColumns.OWNER_PACKAGE_NAME} = ?" + selectionArgs = arrayOf( + "${filePathGenerator.baseRelativePath}%", + context.packageName + ) + } else { + // Legacy Filter: Filename Prefix + selection = "${MediaStore.MediaColumns.DISPLAY_NAME} LIKE ?" + selectionArgs = arrayOf("${filePathGenerator.prefix}%") + } // Sort the results so that the most recently added media appears first. val sortOrder = "${MediaStore.MediaColumns.DATE_ADDED} DESC" @@ -629,7 +664,7 @@ class LocalMediaRepository // Perform the query on the MediaStore. contentResolver.query( collectionUri, - projection, + projection.toTypedArray(), selection, selectionArgs, sortOrder From 76ac88abc8e1eb425de047f295648e20d63c9fa7 Mon Sep 17 00:00:00 2001 From: Trevor McGuire Date: Fri, 12 Jun 2026 11:29:20 -0700 Subject: [PATCH 06/12] Address code review and fix b/522926012: Update unit tests for robust filtering --- .../media/LocalMediaRepositoryTest.kt | 545 +++++------------- 1 file changed, 157 insertions(+), 388 deletions(-) diff --git a/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt b/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt index 73d773700..48ad81a5c 100644 --- a/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt +++ b/data/media/src/test/java/com/google/jetpackcamera/media/LocalMediaRepositoryTest.kt @@ -16,7 +16,6 @@ package com.google.jetpackcamera.media import android.content.ContentResolver -import android.content.ContentUris import android.content.ContentValues import android.content.Context import android.graphics.Bitmap @@ -25,6 +24,7 @@ import android.os.Build import android.provider.MediaStore import androidx.test.core.app.ApplicationProvider import com.google.common.truth.Truth.assertThat +import com.google.jetpackcamera.core.common.FilePathGenerator import com.google.jetpackcamera.core.common.testing.FakeFilePathGenerator import com.google.jetpackcamera.data.media.LocalMediaRepository import com.google.jetpackcamera.data.media.Media @@ -53,6 +53,7 @@ class LocalMediaRepositoryTest { private lateinit var repository: LocalMediaRepository private val testDispatcher = UnconfinedTestDispatcher() private lateinit var fakeContentProvider: FakeContentProvider + private val filePathGenerator: FilePathGenerator = FakeFilePathGenerator() // Reliable fake thumbnail loader for tests private val fakeThumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { _, _ -> @@ -72,31 +73,43 @@ class LocalMediaRepositoryTest { repository = LocalMediaRepository( context, testDispatcher, - FakeFilePathGenerator() + filePathGenerator ).apply { setThumbnailLoader(fakeThumbnailLoader) } } + private fun createContentValues( + displayName: String = "${filePathGenerator.prefix}_Image.jpg", + dateAdded: Long = 1000L, + relativePath: String = filePathGenerator.baseRelativePath, + ownerPackageName: String = context.packageName + ) = ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, displayName) + put(MediaStore.MediaColumns.DATE_ADDED, dateAdded) + put(MediaStore.MediaColumns.RELATIVE_PATH, relativePath) + put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, ownerPackageName) + } + @Test fun lastCapturedMedia_initialValueIsLatest() = runTest { // Given val olderImageTime = 1000L val newerVideoTime = 5000L - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, olderImageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, newerVideoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } + val imageValues = createContentValues( + displayName = "${filePathGenerator.prefix}_Image.jpg", + dateAdded = olderImageTime + ) + val videoValues = createContentValues( + displayName = "${filePathGenerator.prefix}_Video.mp4", + dateAdded = newerVideoTime + ) fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! val videoUrl = fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! // When initializing a new repository - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + val newRepo = LocalMediaRepository(context, testDispatcher, filePathGenerator).apply { setThumbnailLoader(fakeThumbnailLoader) } val result = newRepo.lastCapturedMedia.value @@ -109,10 +122,10 @@ class LocalMediaRepositoryTest { @Test fun lastCapturedMedia_emitsNewImageOnContentChange() = runTest { // When a new image is added - val imageValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 6000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image_New.jpg") - } + val imageValues = createContentValues( + displayName = "${filePathGenerator.prefix}_Image_New.jpg", + dateAdded = 6000L + ) val imageUrl = fakeContentProvider.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, imageValues)!! @@ -128,10 +141,10 @@ class LocalMediaRepositoryTest { @Test fun lastCapturedMedia_emitsNewVideoOnContentChange() = runTest { // When a new video is added - val videoValues = ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 7000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video_New.mp4") - } + val videoValues = createContentValues( + displayName = "${filePathGenerator.prefix}_Video_New.mp4", + dateAdded = 7000L + ) val videoUrl = fakeContentProvider.insert(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, videoValues)!! @@ -145,379 +158,101 @@ class LocalMediaRepositoryTest { } @Test - fun setCurrentMedia_updatesStateFlow() = runTest { - // When - val newMedia = MediaDescriptor.Content.Image( - Uri.parse("content://media/external/images/media/1"), - null, - false - ) - repository.setCurrentMedia(newMedia) - - // Then - assertThat(repository.currentMedia.value).isEqualTo(newMedia) - } - - @Test - fun loadImage_succeeds_returnsImageMedia() = runTest { - val bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) - val sourceFile = File(context.cacheDir, "temp.jpg") - try { - FileOutputStream(sourceFile).use { outputStream -> - bitmap.compress(Bitmap.CompressFormat.JPEG, 100, outputStream) - } - } catch (e: Exception) { - fail("Failed to write mock image data: ${e.message}") - } - - val imageUri = Uri.fromFile(sourceFile) - val mediaDescriptor = MediaDescriptor.Content.Image(imageUri, null, true) - - // When - val result = repository.load(mediaDescriptor) - - // Then - assertThat(result).isInstanceOf(Media.Image::class.java) - } - - @Test - fun loadVideo_succeeds_returnsVideoMedia() = runTest { - val sourceFile = File(context.cacheDir, "temp_video.mp4") - sourceFile.writeText("fake video content") - val videoUri = Uri.fromFile(sourceFile) - val mediaDescriptor = MediaDescriptor.Content.Video(videoUri, null, true) - - // When - val result = repository.load(mediaDescriptor) - - // Then - assertThat(result).isInstanceOf(Media.Video::class.java) - assertThat((result as Media.Video).uri).isEqualTo(videoUri) - } - - @Test - fun loadImage_fails_returnsError() = runTest { - val invalidImageUri = Uri.parse("file:///nonexistent/image.jpg") - val mediaDescriptor = MediaDescriptor.Content.Image(invalidImageUri, null, true) - - // When - val result = repository.load(mediaDescriptor) - - // Then - assertThat(result).isEqualTo(Media.Error) - } - - @Test - fun loadVideo_fails_returnsError() = runTest { - val nonExistentUri = Uri.parse("file:///nonexistent/video.mp4") - val mediaDescriptor = MediaDescriptor.Content.Video(nonExistentUri, null, true) - - // When - val result = repository.load(mediaDescriptor) - - // Then - assertThat(result).isEqualTo(Media.Error) - } - - @Test - fun load_none_returnsNone() = runTest { - val result = repository.load(MediaDescriptor.None) - assertThat(result).isEqualTo(Media.None) - } - - @Test - fun deleteMedia_savedMedia_callsContentResolverDelete() = runTest { - val insertedUri = fakeContentProvider.insert( + fun lastCapturedMedia_ignoresNonAppMediaStoreChange() = runTest { + // Given an app-specific file is current + val appUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_To_Delete.jpg") - } + createContentValues(dateAdded = 1000L) )!! + contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) - val mediaToDelete = MediaDescriptor.Content.Image(insertedUri, null, false) - - // When - repository.deleteMedia(mediaToDelete) - - // Then - val cursor = fakeContentProvider.query(insertedUri, null, null, null, null) - assertThat(cursor.count).isEqualTo(0) - } - - @Test - fun deleteMedia_cachedMedia_deletesRealFile() = runTest { - val tempFile = File(context.cacheDir, "temp_to_delete.mp4") - tempFile.createNewFile() - assertThat(tempFile.exists()).isTrue() - - val mediaToDelete = MediaDescriptor.Content.Video(Uri.fromFile(tempFile), null, true) - - // When - repository.deleteMedia(mediaToDelete) - - // Then - assertThat(tempFile.exists()).isFalse() - } + val appDescriptor = repository.lastCapturedMedia.value + assertThat(appDescriptor).isInstanceOf(MediaDescriptor.Content::class.java) - @Test - fun deleteMedia_currentMedia_resetsToNone() = runTest { - val returnedUri = fakeContentProvider.insert( + // When a file from a different app is inserted + val otherUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Active.jpg") - } + createContentValues( + displayName = "OTHER_Image.jpg", + dateAdded = 5000L, + ownerPackageName = "com.other.app" + ) )!! - val mediaToDelete = MediaDescriptor.Content.Image(returnedUri, null, false) - repository.setCurrentMedia(mediaToDelete) - - // When - repository.deleteMedia(mediaToDelete) + contentResolver.notifyChange(otherUrl, null) - // Then - assertThat(repository.currentMedia.value).isEqualTo(MediaDescriptor.None) + // Then the flow still points to our app's file + assertThat(repository.lastCapturedMedia.value).isEqualTo(appDescriptor) } @Test - fun lastCapturedMedia_videoIsNewer_returnsVideo() = runTest { - val olderImageTime = 1000L - val newerVideoTime = 5000L + fun lastCapturedMedia_ignoresWrongPath() = runTest { + // Given a file in the wrong directory fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, olderImageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - ) - val videoUrl = fakeContentProvider.insert( - MediaStore.Video.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, newerVideoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - )!! - - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { - setThumbnailLoader(fakeThumbnailLoader) - } - val result = newRepo.lastCapturedMedia.value - - assertThat(result).isInstanceOf(MediaDescriptor.Content.Video::class.java) - assertThat((result as MediaDescriptor.Content.Video).uri).isEqualTo(videoUrl) - } - - @Test - fun lastCapturedMedia_imageIsNewer_returnsImage() = runTest { - val newerImageTime = 9000L - val olderVideoTime = 2000L - val imageUrl = fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, newerImageTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } + createContentValues( + displayName = "${filePathGenerator.prefix}_External.jpg", + dateAdded = 5000L, + relativePath = "Download/" + ) )!! - fakeContentProvider.insert( - MediaStore.Video.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, olderVideoTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - ) - - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { - setThumbnailLoader(fakeThumbnailLoader) - } - val result = newRepo.lastCapturedMedia.value + contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) - assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) - assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) + // Then it should be ignored + assertThat(repository.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) } @Test - fun lastCapturedMedia_nothingFound_returnsNone() = runTest { - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { - setThumbnailLoader(fakeThumbnailLoader) + fun lastCapturedMedia_initialLoad_usesDynamicPrefixAndPath() = runTest { + // Given a custom generator with different prefix and path + val customGenerator = object : FilePathGenerator by filePathGenerator { + override val prefix: String = "GPH" + override val baseRelativePath: String = "DCIM/Photos" } - assertThat(newRepo.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) - } - @Test - fun lastCapturedMedia_equalTimestamps_returnsImage() = runTest { - val sameTime = 9999L - val imageUrl = fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, sameTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - )!! + // Add a JCA file (should be ignored by the custom repo) fakeContentProvider.insert( - MediaStore.Video.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, sameTime) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Video.mp4") - } - ) - - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { - setThumbnailLoader(fakeThumbnailLoader) - } - val result = newRepo.lastCapturedMedia.value - - assertThat(result).isInstanceOf(MediaDescriptor.Content.Image::class.java) - assertThat((result as MediaDescriptor.Content.Image).uri).isEqualTo(imageUrl) - } - - @Test - fun deleteMedia_nonExistentUri_doesNotThrow() = runTest { - val nonExistentUri = ContentUris.withAppendedId( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - 999L - ) - val mediaToDelete = MediaDescriptor.Content.Image(nonExistentUri, null, false) - repository.deleteMedia(mediaToDelete) - } - - @Test - fun saveToMediaStore_video_success_returnsNewUri() = runTest { - val sourceFile = File(context.cacheDir, "temp.mp4") - sourceFile.writeText("fake video data") - val mediaDescriptor = MediaDescriptor.Content.Video(Uri.fromFile(sourceFile), null, true) - - val result = repository.saveToMediaStore(mediaDescriptor, "my_video.mp4") - - assertThat(result).isNotNull() - val values = fakeContentProvider.get(result!!) - assertThat(values?.get(MediaStore.MediaColumns.DISPLAY_NAME)).isEqualTo("my_video.mp4") - } - - @Test - fun saveToMediaStore_success_returnsNewUri() = runTest { - val sourceFile = File(context.cacheDir, "temp.jpg") - sourceFile.writeText("fake image data") - val mediaDescriptor = MediaDescriptor.Content.Image(Uri.fromFile(sourceFile), null, true) - - val result = repository.saveToMediaStore(mediaDescriptor, "my_photo.jpg") - - assertThat(result).isNotNull() - val values = fakeContentProvider.get(result!!) - assertThat(values?.get(MediaStore.MediaColumns.DISPLAY_NAME)).isEqualTo("my_photo.jpg") - } - - @Test - fun saveToMediaStore_insertFails_returnsNull() = runTest { - val sourceFile = File(context.cacheDir, "temp.jpg") - sourceFile.writeText("fake image data") - val mediaDescriptor = MediaDescriptor.Content.Image(Uri.fromFile(sourceFile), null, true) - fakeContentProvider.setFailNextInsert(true) - - val result = repository.saveToMediaStore(mediaDescriptor, "my_photo.jpg") - - assertThat(result).isNull() - } - - @Test - fun saveToMediaStore_copyFails_returnsNull() = runTest { - val sourceUri = Uri.parse("file:///nonexistent/file.jpg") - val mediaDescriptor = MediaDescriptor.Content.Image(sourceUri, null, true) - - val result = repository.saveToMediaStore(mediaDescriptor, "broken.jpg") - - assertThat(result).isNull() - } - - @Test - fun lastCapturedMedia_ignoresNonAppMediaStoreChange() = runTest { - // Given a JCA file is current - val jcaUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, context.packageName) - } + createContentValues( + displayName = "JCA_Image.jpg", + relativePath = "DCIM/Camera", + ownerPackageName = context.packageName + ) )!! - contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) - - val jcaDescriptor = repository.lastCapturedMedia.value - assertThat(jcaDescriptor).isInstanceOf(MediaDescriptor.Content::class.java) - // When a non-JCA file from a different app is inserted and notified - val otherUrl = fakeContentProvider.insert( + // Add a GPH file (should be found by the custom repo) + val gphUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 5000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "OTHER_Image.jpg") - put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, "com.other.app") - } + createContentValues( + displayName = "GPH_Image.jpg", + relativePath = "DCIM/Photos", + ownerPackageName = context.packageName + ) )!! - contentResolver.notifyChange(otherUrl, null) - - // Then the flow still points to the JCA file - assertThat(repository.lastCapturedMedia.value).isEqualTo(jcaDescriptor) - } - @Test - fun lastCapturedMedia_initialLoad_ignoresNonAppFiles() = runTest { - val jcaUrl = fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, context.packageName) - } - )!! - fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 5000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "OTHER_Image.jpg") - put(MediaStore.MediaColumns.OWNER_PACKAGE_NAME, "com.other.app") - } - )!! - - val newRepo = LocalMediaRepository(context, testDispatcher, FakeFilePathGenerator()).apply { + val customRepo = LocalMediaRepository(context, testDispatcher, customGenerator).apply { setThumbnailLoader(fakeThumbnailLoader) } - val result = newRepo.lastCapturedMedia.value + val result = customRepo.lastCapturedMedia.value assertThat(result).isInstanceOf(MediaDescriptor.Content::class.java) - assertThat((result as MediaDescriptor.Content).uri).isEqualTo(jcaUrl) - } - - @Test - fun lastCapturedMedia_multipleEventsForSameUri_emitsSameObjectReference() = runTest { - val jcaUrl = fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - )!! - contentResolver.notifyChange(jcaUrl, null) - val firstEmission = repository.lastCapturedMedia.value - - contentResolver.notifyChange(jcaUrl, null) - contentResolver.notifyChange(jcaUrl, null) - - assertThat(repository.lastCapturedMedia.value).isSameInstanceAs(firstEmission) + assertThat((result as MediaDescriptor.Content).uri).isEqualTo(gphUrl) } @Test fun lastCapturedMedia_onDeletion_fallsBackToNextLatest() = runTest { val urlA = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_A.jpg") - } + createContentValues( + displayName = "${filePathGenerator.prefix}_A.jpg", + dateAdded = 1000L + ) )!! val urlB = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 2000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_B.jpg") - } + createContentValues( + displayName = "${filePathGenerator.prefix}_B.jpg", + dateAdded = 2000L + ) )!! contentResolver.notifyChange(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, null) @@ -533,27 +268,8 @@ class LocalMediaRepositoryTest { assertThat((result as MediaDescriptor.Content).uri).isEqualTo(urlA) } - @Test - fun lastCapturedMedia_onLastItemDeletion_emitsNone() = runTest { - val jcaUrl = fakeContentProvider.insert( - MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Image.jpg") - } - )!! - contentResolver.notifyChange(jcaUrl, null) - assertThat(repository.lastCapturedMedia.value).isNotEqualTo(MediaDescriptor.None) - - fakeContentProvider.delete(jcaUrl, null, null) - contentResolver.notifyChange(jcaUrl, null) - - assertThat(repository.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) - } - @Test fun lastCapturedMedia_newUriWithNullThumbnail_holdsPreviousMedia() = runTest { - // We use a custom repo here so we can control the thumbnail loader specifically for this test var shouldFailThumbnail = false val customThumbnailLoader: suspend (Uri, Uri) -> Bitmap? = { _, _ -> if (shouldFailThumbnail) null else Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) @@ -561,17 +277,17 @@ class LocalMediaRepositoryTest { val customRepo = LocalMediaRepository( context, testDispatcher, - FakeFilePathGenerator() + filePathGenerator ).apply { setThumbnailLoader(customThumbnailLoader) } val oldUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 1000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_Old.jpg") - } + createContentValues( + displayName = "${filePathGenerator.prefix}_Old.jpg", + dateAdded = 1000L + ) )!! contentResolver.notifyChange(oldUrl, null) val initialResult = customRepo.lastCapturedMedia.value @@ -579,26 +295,79 @@ class LocalMediaRepositoryTest { val newUrl = fakeContentProvider.insert( MediaStore.Images.Media.EXTERNAL_CONTENT_URI, - ContentValues().apply { - put(MediaStore.MediaColumns.DATE_ADDED, 2000L) - put(MediaStore.MediaColumns.DISPLAY_NAME, "JCA_New.jpg") - } + createContentValues( + displayName = "${filePathGenerator.prefix}_New.jpg", + dateAdded = 2000L + ) )!! - // Trigger thumbnail failure shouldFailThumbnail = true contentResolver.notifyChange(newUrl, null) - - // Then it holds the previous media assertThat(customRepo.lastCapturedMedia.value).isEqualTo(initialResult) - // Thumbnail becomes ready shouldFailThumbnail = false contentResolver.notifyChange(newUrl, null) - - // Finally emits new media val finalResult = customRepo.lastCapturedMedia.value - assertThat(finalResult).isInstanceOf(MediaDescriptor.Content::class.java) assertThat((finalResult as MediaDescriptor.Content).uri).isEqualTo(newUrl) } + + @Test + fun setCurrentMedia_updatesStateFlow() = runTest { + val newMedia = MediaDescriptor.Content.Image( + Uri.parse("content://media/external/images/media/1"), + null, + false + ) + repository.setCurrentMedia(newMedia) + assertThat(repository.currentMedia.value).isEqualTo(newMedia) + } + + @Test + fun loadImage_succeeds_returnsImageMedia() = runTest { + val bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + val sourceFile = File(context.cacheDir, "temp.jpg") + try { + FileOutputStream(sourceFile).use { outputStream -> + bitmap.compress(Bitmap.CompressFormat.JPEG, 100, outputStream) + } + } catch (e: Exception) { + fail("Failed to write mock image data: ${e.message}") + } + + val imageUri = Uri.fromFile(sourceFile) + val mediaDescriptor = MediaDescriptor.Content.Image(imageUri, null, true) + val result = repository.load(mediaDescriptor) + assertThat(result).isInstanceOf(Media.Image::class.java) + } + + @Test + fun loadVideo_succeeds_returnsVideoMedia() = runTest { + val sourceFile = File(context.cacheDir, "temp_video.mp4") + sourceFile.writeText("fake video content") + val videoUri = Uri.fromFile(sourceFile) + val mediaDescriptor = MediaDescriptor.Content.Video(videoUri, null, true) + val result = repository.load(mediaDescriptor) + assertThat(result).isInstanceOf(Media.Video::class.java) + assertThat((result as Media.Video).uri).isEqualTo(videoUri) + } + + @Test + fun deleteMedia_savedMedia_callsContentResolverDelete() = runTest { + val insertedUri = fakeContentProvider.insert( + MediaStore.Images.Media.EXTERNAL_CONTENT_URI, + createContentValues(displayName = "${filePathGenerator.prefix}_Delete.jpg") + )!! + val mediaToDelete = MediaDescriptor.Content.Image(insertedUri, null, false) + repository.deleteMedia(mediaToDelete) + val cursor = fakeContentProvider.query(insertedUri, null, null, null, null) + assertThat(cursor.count).isEqualTo(0) + } + + @Test + fun lastCapturedMedia_nothingFound_returnsNone() = runTest { + val newRepo = LocalMediaRepository(context, testDispatcher, filePathGenerator).apply { + setThumbnailLoader(fakeThumbnailLoader) + } + assertThat(newRepo.lastCapturedMedia.value).isEqualTo(MediaDescriptor.None) + } } From 6ca114b118dafabb8275952f0d2540796e0dde06 Mon Sep 17 00:00:00 2001 From: Jetski Date: Wed, 22 Jul 2026 07:46:01 +0000 Subject: [PATCH 07/12] Fix CameraAppSettingsViewModelTest mock for ConcurrentCamera --- .../settings/CameraAppSettingsViewModelTest.kt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/feature/settings/src/androidTest/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt b/feature/settings/src/androidTest/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt index 6b12a1d19..739831c90 100644 --- a/feature/settings/src/androidTest/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt +++ b/feature/settings/src/androidTest/java/com/google/jetpackcamera/settings/CameraAppSettingsViewModelTest.kt @@ -295,7 +295,17 @@ internal class CameraAppSettingsViewModelTest { val customViewModel = createViewModelWithConstraints( systemConstraints = TYPICAL_SYSTEM_CONSTRAINTS.copy( - concurrentCamerasSupported = true + concurrentCamerasSupported = true, + perLensConstraints = buildMap { + for (lensFacing in listOf(LensFacing.FRONT, LensFacing.BACK)) { + put( + lensFacing, + TYPICAL_SYSTEM_CONSTRAINTS.perLensConstraints[lensFacing]!!.copy( + supportedEffects = setOf(SingleStreamEffectKey.id) + ) + ) + } + } ) ) advanceUntilIdle() From 0a50b3e3dad48fa5581b086dbe7d1cc5b47bafd0 Mon Sep 17 00:00:00 2001 From: Jetski Date: Thu, 23 Jul 2026 00:06:09 +0000 Subject: [PATCH 08/12] Fix PreviewViewModel unresolved reference to getLastCapturedMedia() --- .../google/jetpackcamera/feature/preview/PreviewViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt index a54fe3988..1ad16158f 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt +++ b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt @@ -188,7 +188,7 @@ class PreviewViewModel @Inject constructor( updateLastCapturedMediaCallback = { viewModelScope.launch { trackedCaptureUiState.update { old -> - old.copy(recentCapturedMedia = mediaRepository.getLastCapturedMedia()) + old.copy(recentCapturedMedia = mediaRepository.lastCapturedMedia.value) } } }, From 53802d4c9a8bddadc9a24205487cda7b96b1fbc8 Mon Sep 17 00:00:00 2001 From: Jetski Date: Thu, 23 Jul 2026 00:13:56 +0000 Subject: [PATCH 09/12] Refactor ImageWellController to fully embrace reactive StateFlow from MediaRepository --- build_err.txt | 307 ++++++++++++++++++ .../feature/preview/PreviewScreen.kt | 1 - .../feature/preview/PreviewViewModel.kt | 15 +- gh_comments.json | 1 + pr_line_comments.json | 1 + .../controller/impl/CaptureControllerImpl.kt | 4 +- .../impl/ImageWellControllerImpl.kt | 5 - .../ui/controller/ImageWellController.kt | 4 - .../testing/FakeImageWellController.kt | 5 - .../testing/FakeImageWellControllerTest.kt | 8 +- 10 files changed, 320 insertions(+), 31 deletions(-) create mode 100644 build_err.txt create mode 100644 gh_comments.json create mode 100644 pr_line_comments.json diff --git a/build_err.txt b/build_err.txt new file mode 100644 index 000000000..5c97d2172 --- /dev/null +++ b/build_err.txt @@ -0,0 +1,307 @@ +WARNING: A restricted method in java.lang.System has been called +WARNING: java.lang.System::load has been called by net.rubygrapefruit.platform.internal.NativeLibraryLoader in an unnamed module (file:/usr/local/google/home/trevormcguire/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/native-platform-0.22-milestone-28.jar) +WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module +WARNING: Restricted methods will be blocked in a future release unless native access is enabled + +Parallel Configuration Cache is an incubating feature. +Calculating task graph as configuration cache cannot be reused because JVM has changed. + +FAILURE: Build failed with an exception. + +* What went wrong: +26.0.1 + +* Try: +> Run with --info or --debug option to get more log output. +> Run with --scan to get full insights. +> Get more help at https://help.gradle.org. + +* Exception is: +java.lang.IllegalArgumentException: 26.0.1 + at org.jetbrains.kotlin.com.intellij.util.lang.JavaVersion.parse(JavaVersion.java:307) + at org.jetbrains.kotlin.com.intellij.util.lang.JavaVersion.current(JavaVersion.java:176) + at org.jetbrains.kotlin.cli.jvm.modules.JavaVersionUtilsKt.isAtLeastJava9(javaVersionUtils.kt:11) + at org.jetbrains.kotlin.cli.jvm.modules.CoreJrtFileSystem.globalJrtFsCache$lambda$2(CoreJrtFileSystem.kt:83) + at org.jetbrains.kotlin.cli.jvm.modules.CoreJrtFileSystem.globalJrtFsCache$lambda$3(CoreJrtFileSystem.kt:74) + at org.jetbrains.kotlin.com.intellij.util.containers.ConcurrentFactoryMap$2.create(ConcurrentFactoryMap.java:174) + at org.jetbrains.kotlin.com.intellij.util.containers.ConcurrentFactoryMap.get(ConcurrentFactoryMap.java:40) + at org.jetbrains.kotlin.cli.jvm.modules.CoreJrtFileSystem.roots$lambda$0(CoreJrtFileSystem.kt:34) + at org.jetbrains.kotlin.cli.jvm.modules.CoreJrtFileSystem.roots$lambda$1(CoreJrtFileSystem.kt:33) + at org.jetbrains.kotlin.com.intellij.util.containers.ConcurrentFactoryMap$2.create(ConcurrentFactoryMap.java:174) + at org.jetbrains.kotlin.com.intellij.util.containers.ConcurrentFactoryMap.get(ConcurrentFactoryMap.java:40) + at org.jetbrains.kotlin.cli.jvm.modules.CoreJrtFileSystem.findFileByPath(CoreJrtFileSystem.kt:42) + at org.jetbrains.kotlin.cli.jvm.modules.CliJavaModuleFinder.(CliJavaModuleFinder.kt:44) + at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment.(KotlinCoreEnvironment.kt:243) + at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment.(KotlinCoreEnvironment.kt) + at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment$Companion.createForProduction(KotlinCoreEnvironment.kt:468) + at org.gradle.kotlin.dsl.support.KotlinCompilerKt$kotlinCoreEnvironmentFor$1.create(KotlinCompiler.kt:514) + at org.gradle.kotlin.dsl.support.KotlinCompilerKt$kotlinCoreEnvironmentFor$1.create(KotlinCompiler.kt:510) + at org.gradle.internal.SystemProperties.withSystemProperty(SystemProperties.java:123) + at org.gradle.kotlin.dsl.support.KotlinCompilerKt.kotlinCoreEnvironmentFor(KotlinCompiler.kt:510) + at org.gradle.kotlin.dsl.support.KotlinCompilerKt.compileKotlinScriptModuleTo(KotlinCompiler.kt:231) + at org.gradle.kotlin.dsl.support.KotlinCompilerKt.compileKotlinScriptToDirectory(KotlinCompiler.kt:198) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$compileScript$1.invoke(ResidualProgramCompiler.kt:713) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$compileScript$1.invoke(ResidualProgramCompiler.kt:712) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator$InterpreterHost$runCompileBuildOperation$1.call(KotlinScriptEvaluator.kt:209) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator$InterpreterHost$runCompileBuildOperation$1.call(KotlinScriptEvaluator.kt:206) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:210) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:205) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:54) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator$InterpreterHost.runCompileBuildOperation(KotlinScriptEvaluator.kt:206) + at org.gradle.kotlin.dsl.execution.Interpreter$compile$1$1$1$1.invoke(Interpreter.kt:334) + at org.gradle.kotlin.dsl.execution.Interpreter$compile$1$1$1$1.invoke(Interpreter.kt:334) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.compileScript-C5AE47M(ResidualProgramCompiler.kt:712) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.compileStage1-EfyMToc(ResidualProgramCompiler.kt:695) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.emitStage1Sequence(ResidualProgramCompiler.kt:247) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.emitStage1Sequence(ResidualProgramCompiler.kt:238) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.emit(ResidualProgramCompiler.kt:198) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.emit(ResidualProgramCompiler.kt:182) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.access$emit(ResidualProgramCompiler.kt:83) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$emitDynamicProgram$1$1.invoke(ResidualProgramCompiler.kt:123) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$emitDynamicProgram$1$1.invoke(ResidualProgramCompiler.kt:121) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$overrideExecute$1.invoke(ResidualProgramCompiler.kt:548) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$overrideExecute$1.invoke(ResidualProgramCompiler.kt:547) + at org.gradle.kotlin.dsl.support.bytecode.AsmExtensionsKt.method(AsmExtensions.kt:133) + at org.gradle.kotlin.dsl.support.bytecode.AsmExtensionsKt.publicMethod(AsmExtensions.kt:116) + at org.gradle.kotlin.dsl.support.bytecode.AsmExtensionsKt.publicMethod$default(AsmExtensions.kt:108) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.overrideExecute(ResidualProgramCompiler.kt:547) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.access$overrideExecute(ResidualProgramCompiler.kt:83) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$emitDynamicProgram$1.invoke(ResidualProgramCompiler.kt:121) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$emitDynamicProgram$1.invoke(ResidualProgramCompiler.kt:119) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$program$3.invoke(ResidualProgramCompiler.kt:673) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler$program$3.invoke(ResidualProgramCompiler.kt:671) + at org.gradle.kotlin.dsl.support.bytecode.AsmExtensionsKt.publicClass-7y5yvvE(AsmExtensions.kt:41) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.program-5oOsWEo(ResidualProgramCompiler.kt:671) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.access$program-5oOsWEo(ResidualProgramCompiler.kt:83) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.emitDynamicProgram(ResidualProgramCompiler.kt:798) + at org.gradle.kotlin.dsl.execution.ResidualProgramCompiler.compile(ResidualProgramCompiler.kt:102) + at org.gradle.kotlin.dsl.execution.Interpreter$compile$1.invoke(Interpreter.kt:337) + at org.gradle.kotlin.dsl.execution.Interpreter$compile$1.invoke(Interpreter.kt:301) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator$KotlinScriptCompilationAndInstrumentation.compile(KotlinScriptEvaluator.kt:440) + at org.gradle.internal.scripts.BuildScriptCompilationAndInstrumentation.execute(BuildScriptCompilationAndInstrumentation.java:136) + at org.gradle.internal.execution.steps.ExecuteStep.executeInternal(ExecuteStep.java:105) + at org.gradle.internal.execution.steps.ExecuteStep.access$000(ExecuteStep.java:44) + at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:59) + at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:56) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:210) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:205) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:54) + at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:56) + at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:44) + at org.gradle.internal.execution.steps.CancelExecutionStep.execute(CancelExecutionStep.java:42) + at org.gradle.internal.execution.steps.TimeoutStep.executeWithoutTimeout(TimeoutStep.java:75) + at org.gradle.internal.execution.steps.TimeoutStep.execute(TimeoutStep.java:55) + at org.gradle.internal.execution.steps.PreCreateOutputParentsStep.execute(PreCreateOutputParentsStep.java:50) + at org.gradle.internal.execution.steps.PreCreateOutputParentsStep.execute(PreCreateOutputParentsStep.java:28) + at org.gradle.internal.execution.steps.BroadcastChangingOutputsStep.execute(BroadcastChangingOutputsStep.java:61) + at org.gradle.internal.execution.steps.BroadcastChangingOutputsStep.execute(BroadcastChangingOutputsStep.java:26) + at org.gradle.internal.execution.steps.NoInputChangesStep.execute(NoInputChangesStep.java:30) + at org.gradle.internal.execution.steps.NoInputChangesStep.execute(NoInputChangesStep.java:21) + at org.gradle.internal.execution.steps.CaptureOutputsAfterExecutionStep.execute(CaptureOutputsAfterExecutionStep.java:69) + at org.gradle.internal.execution.steps.CaptureOutputsAfterExecutionStep.execute(CaptureOutputsAfterExecutionStep.java:46) + at org.gradle.internal.execution.steps.BuildCacheStep.executeWithoutCache(BuildCacheStep.java:189) + at org.gradle.internal.execution.steps.BuildCacheStep.lambda$execute$1(BuildCacheStep.java:75) + at org.gradle.internal.Either$Right.fold(Either.java:175) + at org.gradle.internal.execution.caching.CachingState.fold(CachingState.java:62) + at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:73) + at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:48) + at org.gradle.internal.execution.steps.NeverUpToDateStep.execute(NeverUpToDateStep.java:34) + at org.gradle.internal.execution.steps.NeverUpToDateStep.execute(NeverUpToDateStep.java:22) + at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:37) + at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:27) + at org.gradle.internal.execution.steps.ResolveNonIncrementalCachingStateStep.executeDelegate(ResolveNonIncrementalCachingStateStep.java:50) + at org.gradle.internal.execution.steps.AbstractResolveCachingStateStep.execute(AbstractResolveCachingStateStep.java:71) + at org.gradle.internal.execution.steps.AbstractResolveCachingStateStep.execute(AbstractResolveCachingStateStep.java:39) + at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:107) + at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:56) + at org.gradle.internal.execution.steps.AbstractCaptureStateBeforeExecutionStep.execute(AbstractCaptureStateBeforeExecutionStep.java:64) + at org.gradle.internal.execution.steps.AbstractCaptureStateBeforeExecutionStep.execute(AbstractCaptureStateBeforeExecutionStep.java:43) + at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsStartedStep.execute(MarkSnapshottingInputsStartedStep.java:38) + at org.gradle.internal.execution.steps.AssignImmutableWorkspaceStep.lambda$executeInTemporaryWorkspace$3(AssignImmutableWorkspaceStep.java:209) + at org.gradle.internal.execution.workspace.impl.CacheBasedImmutableWorkspaceProvider$1.withTemporaryWorkspace(CacheBasedImmutableWorkspaceProvider.java:116) + at org.gradle.internal.execution.steps.AssignImmutableWorkspaceStep.executeInTemporaryWorkspace(AssignImmutableWorkspaceStep.java:199) + at org.gradle.internal.execution.steps.AssignImmutableWorkspaceStep.lambda$execute$0(AssignImmutableWorkspaceStep.java:121) + at org.gradle.internal.execution.steps.AssignImmutableWorkspaceStep.execute(AssignImmutableWorkspaceStep.java:121) + at org.gradle.internal.execution.steps.AssignImmutableWorkspaceStep.execute(AssignImmutableWorkspaceStep.java:90) + at org.gradle.internal.execution.steps.ChoosePipelineStep.execute(ChoosePipelineStep.java:38) + at org.gradle.internal.execution.steps.ChoosePipelineStep.execute(ChoosePipelineStep.java:23) + at org.gradle.internal.execution.steps.ExecuteWorkBuildOperationFiringStep.lambda$execute$2(ExecuteWorkBuildOperationFiringStep.java:67) + at org.gradle.internal.execution.steps.ExecuteWorkBuildOperationFiringStep.execute(ExecuteWorkBuildOperationFiringStep.java:67) + at org.gradle.internal.execution.steps.ExecuteWorkBuildOperationFiringStep.execute(ExecuteWorkBuildOperationFiringStep.java:39) + at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:46) + at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:34) + at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:48) + at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:35) + at org.gradle.internal.execution.impl.DefaultExecutionEngine$1.execute(DefaultExecutionEngine.java:61) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator$InterpreterHost.cachedDirFor(KotlinScriptEvaluator.kt:304) + at org.gradle.kotlin.dsl.execution.Interpreter.compile(Interpreter.kt:301) + at org.gradle.kotlin.dsl.execution.Interpreter.emitSpecializedProgramFor(Interpreter.kt:267) + at org.gradle.kotlin.dsl.execution.Interpreter.eval(Interpreter.kt:199) + at org.gradle.kotlin.dsl.provider.StandardKotlinScriptEvaluator.evaluate(KotlinScriptEvaluator.kt:133) + at org.gradle.kotlin.dsl.provider.KotlinScriptPluginFactory$create$1.invoke(KotlinScriptPluginFactory.kt:61) + at org.gradle.kotlin.dsl.provider.KotlinScriptPluginFactory$create$1.invoke(KotlinScriptPluginFactory.kt:52) + at org.gradle.kotlin.dsl.provider.KotlinScriptPlugin.apply(KotlinScriptPlugin.kt:35) + at org.gradle.configuration.BuildOperationScriptPlugin$1.run(BuildOperationScriptPlugin.java:68) + at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:30) + at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:27) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.run(DefaultBuildOperationRunner.java:48) + at org.gradle.configuration.BuildOperationScriptPlugin.lambda$apply$0(BuildOperationScriptPlugin.java:65) + at org.gradle.internal.code.DefaultUserCodeApplicationContext.apply(DefaultUserCodeApplicationContext.java:44) + at org.gradle.configuration.BuildOperationScriptPlugin.apply(BuildOperationScriptPlugin.java:65) + at org.gradle.initialization.ScriptEvaluatingSettingsProcessor.applySettingsScript(ScriptEvaluatingSettingsProcessor.java:75) + at org.gradle.initialization.ScriptEvaluatingSettingsProcessor.process(ScriptEvaluatingSettingsProcessor.java:68) + at org.gradle.initialization.SettingsEvaluatedCallbackFiringSettingsProcessor.process(SettingsEvaluatedCallbackFiringSettingsProcessor.java:34) + at org.gradle.initialization.RootBuildCacheControllerSettingsProcessor.process(RootBuildCacheControllerSettingsProcessor.java:47) + at org.gradle.initialization.BuildOperationSettingsProcessor$2.call(BuildOperationSettingsProcessor.java:49) + at org.gradle.initialization.BuildOperationSettingsProcessor$2.call(BuildOperationSettingsProcessor.java:46) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:210) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:205) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:54) + at org.gradle.initialization.BuildOperationSettingsProcessor.process(BuildOperationSettingsProcessor.java:46) + at org.gradle.initialization.DefaultSettingsLoader.findSettingsAndLoadIfAppropriate(DefaultSettingsLoader.java:183) + at org.gradle.initialization.DefaultSettingsLoader.findAndLoadSettings(DefaultSettingsLoader.java:86) + at org.gradle.initialization.SettingsAttachingSettingsLoader.findAndLoadSettings(SettingsAttachingSettingsLoader.java:33) + at org.gradle.internal.composite.CommandLineIncludedBuildSettingsLoader.findAndLoadSettings(CommandLineIncludedBuildSettingsLoader.java:35) + at org.gradle.internal.composite.ChildBuildRegisteringSettingsLoader.findAndLoadSettings(ChildBuildRegisteringSettingsLoader.java:44) + at org.gradle.internal.composite.CompositeBuildSettingsLoader.findAndLoadSettings(CompositeBuildSettingsLoader.java:35) + at org.gradle.initialization.InitScriptHandlingSettingsLoader.findAndLoadSettings(InitScriptHandlingSettingsLoader.java:33) + at org.gradle.api.internal.initialization.CacheConfigurationsHandlingSettingsLoader.findAndLoadSettings(CacheConfigurationsHandlingSettingsLoader.java:36) + at org.gradle.initialization.GradlePropertiesHandlingSettingsLoader.findAndLoadSettings(GradlePropertiesHandlingSettingsLoader.java:38) + at org.gradle.initialization.DefaultSettingsPreparer.prepareSettings(DefaultSettingsPreparer.java:31) + at org.gradle.initialization.BuildOperationFiringSettingsPreparer$LoadBuild.doLoadBuild(BuildOperationFiringSettingsPreparer.java:71) + at org.gradle.initialization.BuildOperationFiringSettingsPreparer$LoadBuild.run(BuildOperationFiringSettingsPreparer.java:66) + at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:30) + at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:27) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.run(DefaultBuildOperationRunner.java:48) + at org.gradle.initialization.BuildOperationFiringSettingsPreparer.prepareSettings(BuildOperationFiringSettingsPreparer.java:54) + at org.gradle.initialization.VintageBuildModelController.lambda$prepareSettings$1(VintageBuildModelController.java:80) + at org.gradle.internal.model.StateTransitionController.lambda$doTransition$14(StateTransitionController.java:255) + at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:266) + at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:254) + at org.gradle.internal.model.StateTransitionController.lambda$transitionIfNotPreviously$11(StateTransitionController.java:213) + at org.gradle.internal.work.DefaultSynchronizer.withLock(DefaultSynchronizer.java:36) + at org.gradle.internal.model.StateTransitionController.transitionIfNotPreviously(StateTransitionController.java:209) + at org.gradle.initialization.VintageBuildModelController.prepareSettings(VintageBuildModelController.java:80) + at org.gradle.initialization.VintageBuildModelController.prepareToScheduleTasks(VintageBuildModelController.java:70) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildModelController.prepareToScheduleTasks(ConfigurationCacheAwareBuildModelController.kt:49) + at org.gradle.internal.build.DefaultBuildLifecycleController.lambda$prepareToScheduleTasks$6(DefaultBuildLifecycleController.java:175) + at org.gradle.internal.model.StateTransitionController.lambda$doTransition$14(StateTransitionController.java:255) + at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:266) + at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:254) + at org.gradle.internal.model.StateTransitionController.lambda$maybeTransition$9(StateTransitionController.java:190) + at org.gradle.internal.work.DefaultSynchronizer.withLock(DefaultSynchronizer.java:36) + at org.gradle.internal.model.StateTransitionController.maybeTransition(StateTransitionController.java:186) + at org.gradle.internal.build.DefaultBuildLifecycleController.prepareToScheduleTasks(DefaultBuildLifecycleController.java:173) + at org.gradle.internal.buildtree.DefaultBuildTreeWorkPreparer.scheduleRequestedTasks(DefaultBuildTreeWorkPreparer.java:36) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildTreeWorkController$scheduleAndRunRequestedTasks$executionResult$1$result$1.invoke(ConfigurationCacheAwareBuildTreeWorkController.kt:48) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildTreeWorkController$scheduleAndRunRequestedTasks$executionResult$1$result$1.invoke(ConfigurationCacheAwareBuildTreeWorkController.kt:45) + at org.gradle.internal.cc.impl.DefaultConfigurationCache$loadOrScheduleRequestedTasks$1.invoke(DefaultConfigurationCache.kt:241) + at org.gradle.internal.cc.impl.DefaultConfigurationCache$loadOrScheduleRequestedTasks$1.invoke(DefaultConfigurationCache.kt:240) + at org.gradle.internal.cc.impl.DefaultConfigurationCache.runWorkThatContributesToCacheEntry(DefaultConfigurationCache.kt:552) + at org.gradle.internal.cc.impl.DefaultConfigurationCache.loadOrScheduleRequestedTasks(DefaultConfigurationCache.kt:240) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildTreeWorkController$scheduleAndRunRequestedTasks$executionResult$1.apply(ConfigurationCacheAwareBuildTreeWorkController.kt:45) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildTreeWorkController$scheduleAndRunRequestedTasks$executionResult$1.apply(ConfigurationCacheAwareBuildTreeWorkController.kt:44) + at org.gradle.composite.internal.DefaultIncludedBuildTaskGraph.withNewWorkGraph(DefaultIncludedBuildTaskGraph.java:112) + at org.gradle.internal.cc.impl.ConfigurationCacheAwareBuildTreeWorkController.scheduleAndRunRequestedTasks(ConfigurationCacheAwareBuildTreeWorkController.kt:44) + at org.gradle.internal.buildtree.DefaultBuildTreeLifecycleController.lambda$scheduleAndRunTasks$1(DefaultBuildTreeLifecycleController.java:77) + at org.gradle.internal.buildtree.DefaultBuildTreeLifecycleController.lambda$runBuild$4(DefaultBuildTreeLifecycleController.java:120) + at org.gradle.internal.model.StateTransitionController.lambda$transition$6(StateTransitionController.java:169) + at org.gradle.internal.model.StateTransitionController.doTransition(StateTransitionController.java:266) + at org.gradle.internal.model.StateTransitionController.lambda$transition$7(StateTransitionController.java:169) + at org.gradle.internal.work.DefaultSynchronizer.withLock(DefaultSynchronizer.java:46) + at org.gradle.internal.model.StateTransitionController.transition(StateTransitionController.java:169) + at org.gradle.internal.buildtree.DefaultBuildTreeLifecycleController.runBuild(DefaultBuildTreeLifecycleController.java:117) + at org.gradle.internal.buildtree.DefaultBuildTreeLifecycleController.scheduleAndRunTasks(DefaultBuildTreeLifecycleController.java:77) + at org.gradle.internal.buildtree.DefaultBuildTreeLifecycleController.scheduleAndRunTasks(DefaultBuildTreeLifecycleController.java:72) + at org.gradle.tooling.internal.provider.ExecuteBuildActionRunner.run(ExecuteBuildActionRunner.java:31) + at org.gradle.launcher.exec.ChainingBuildActionRunner.run(ChainingBuildActionRunner.java:35) + at org.gradle.internal.buildtree.ProblemReportingBuildActionRunner.run(ProblemReportingBuildActionRunner.java:49) + at org.gradle.launcher.exec.BuildOutcomeReportingBuildActionRunner.run(BuildOutcomeReportingBuildActionRunner.java:71) + at org.gradle.tooling.internal.provider.FileSystemWatchingBuildActionRunner.run(FileSystemWatchingBuildActionRunner.java:135) + at org.gradle.launcher.exec.BuildCompletionNotifyingBuildActionRunner.run(BuildCompletionNotifyingBuildActionRunner.java:41) + at org.gradle.launcher.exec.RootBuildLifecycleBuildActionExecutor.lambda$execute$0(RootBuildLifecycleBuildActionExecutor.java:54) + at org.gradle.composite.internal.DefaultRootBuildState.run(DefaultRootBuildState.java:130) + at org.gradle.launcher.exec.RootBuildLifecycleBuildActionExecutor.execute(RootBuildLifecycleBuildActionExecutor.java:54) + at org.gradle.internal.buildtree.InitDeprecationLoggingActionExecutor.execute(InitDeprecationLoggingActionExecutor.java:62) + at org.gradle.internal.buildtree.InitProblems.execute(InitProblems.java:36) + at org.gradle.internal.buildtree.DefaultBuildTreeContext.execute(DefaultBuildTreeContext.java:40) + at org.gradle.launcher.exec.BuildTreeLifecycleBuildActionExecutor.lambda$execute$0(BuildTreeLifecycleBuildActionExecutor.java:71) + at org.gradle.internal.buildtree.BuildTreeState.run(BuildTreeState.java:60) + at org.gradle.launcher.exec.BuildTreeLifecycleBuildActionExecutor.execute(BuildTreeLifecycleBuildActionExecutor.java:71) + at org.gradle.launcher.exec.RunAsBuildOperationBuildActionExecutor$2.call(RunAsBuildOperationBuildActionExecutor.java:67) + at org.gradle.launcher.exec.RunAsBuildOperationBuildActionExecutor$2.call(RunAsBuildOperationBuildActionExecutor.java:63) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:210) + at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:205) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:67) + at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:167) + at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:60) + at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:54) + at org.gradle.launcher.exec.RunAsBuildOperationBuildActionExecutor.execute(RunAsBuildOperationBuildActionExecutor.java:63) + at org.gradle.launcher.exec.RunAsWorkerThreadBuildActionExecutor.lambda$execute$0(RunAsWorkerThreadBuildActionExecutor.java:36) + at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:263) + at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:127) + at org.gradle.launcher.exec.RunAsWorkerThreadBuildActionExecutor.execute(RunAsWorkerThreadBuildActionExecutor.java:36) + at org.gradle.tooling.internal.provider.continuous.ContinuousBuildActionExecutor.execute(ContinuousBuildActionExecutor.java:110) + at org.gradle.tooling.internal.provider.SubscribableBuildActionExecutor.execute(SubscribableBuildActionExecutor.java:64) + at org.gradle.internal.session.DefaultBuildSessionContext.execute(DefaultBuildSessionContext.java:46) + at org.gradle.internal.buildprocess.execution.BuildSessionLifecycleBuildActionExecutor$ActionImpl.apply(BuildSessionLifecycleBuildActionExecutor.java:92) + at org.gradle.internal.buildprocess.execution.BuildSessionLifecycleBuildActionExecutor$ActionImpl.apply(BuildSessionLifecycleBuildActionExecutor.java:80) + at org.gradle.internal.session.BuildSessionState.run(BuildSessionState.java:73) + at org.gradle.internal.buildprocess.execution.BuildSessionLifecycleBuildActionExecutor.execute(BuildSessionLifecycleBuildActionExecutor.java:62) + at org.gradle.internal.buildprocess.execution.BuildSessionLifecycleBuildActionExecutor.execute(BuildSessionLifecycleBuildActionExecutor.java:41) + at org.gradle.internal.buildprocess.execution.StartParamsValidatingActionExecutor.execute(StartParamsValidatingActionExecutor.java:64) + at org.gradle.internal.buildprocess.execution.StartParamsValidatingActionExecutor.execute(StartParamsValidatingActionExecutor.java:32) + at org.gradle.internal.buildprocess.execution.SessionFailureReportingActionExecutor.execute(SessionFailureReportingActionExecutor.java:51) + at org.gradle.internal.buildprocess.execution.SessionFailureReportingActionExecutor.execute(SessionFailureReportingActionExecutor.java:39) + at org.gradle.internal.buildprocess.execution.SetupLoggingActionExecutor.execute(SetupLoggingActionExecutor.java:47) + at org.gradle.internal.buildprocess.execution.SetupLoggingActionExecutor.execute(SetupLoggingActionExecutor.java:31) + at org.gradle.launcher.daemon.server.exec.ExecuteBuild.doBuild(ExecuteBuild.java:70) + at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:37) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.WatchForDisconnection.execute(WatchForDisconnection.java:39) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.ResetDeprecationLogger.execute(ResetDeprecationLogger.java:29) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.RequestStopIfSingleUsedDaemon.execute(RequestStopIfSingleUsedDaemon.java:35) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.ForwardClientInput.lambda$execute$0(ForwardClientInput.java:40) + at org.gradle.internal.daemon.clientinput.ClientInputForwarder.forwardInput(ClientInputForwarder.java:80) + at org.gradle.launcher.daemon.server.exec.ForwardClientInput.execute(ForwardClientInput.java:37) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.LogAndCheckHealth.execute(LogAndCheckHealth.java:64) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.LogToClient.doBuild(LogToClient.java:63) + at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:37) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.EstablishBuildEnvironment.doBuild(EstablishBuildEnvironment.java:84) + at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:37) + at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:104) + at org.gradle.launcher.daemon.server.exec.StartBuildOrRespondWithBusy$1.run(StartBuildOrRespondWithBusy.java:52) + at org.gradle.launcher.daemon.server.DaemonStateCoordinator.lambda$runCommand$0(DaemonStateCoordinator.java:321) + at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64) + at org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:48) + + +BUILD FAILED in 778ms +Configuration cache entry stored. diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt index eff65479a..8bf772865 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt +++ b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt @@ -228,7 +228,6 @@ fun PreviewScreen( if (Build.VERSION.SDK_INT > Build.VERSION_CODES.P || readStoragePermission.status.isGranted ) { - viewModel.imageWellController.updateLastCapturedMedia() } } } diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt index 1ad16158f..a7dfd6d06 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt +++ b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt @@ -185,16 +185,17 @@ class PreviewViewModel @Inject constructor( val imageWellController: ImageWellController = ImageWellControllerImpl( mediaRepository = mediaRepository, - updateLastCapturedMediaCallback = { - viewModelScope.launch { - trackedCaptureUiState.update { old -> - old.copy(recentCapturedMedia = mediaRepository.lastCapturedMedia.value) - } - } - }, coroutineContext = viewModelScope.coroutineContext ) + init { + viewModelScope.launch { + mediaRepository.lastCapturedMedia.collect { media -> + trackedCaptureUiState.update { old -> old.copy(recentCapturedMedia = media) } + } + } + } + val cameraController: CameraController = CameraControllerImpl( initializationDeferred = initializationDeferred, captureUiState = captureUiState, diff --git a/gh_comments.json b/gh_comments.json new file mode 100644 index 000000000..d16d9b0d9 --- /dev/null +++ b/gh_comments.json @@ -0,0 +1 @@ +{"comments":[{"id":"IC_kwDOIzt9z87fXR8S","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"/gemini summary","createdAt":"2026-01-14T02:37:54Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/google/jetpack-camera-app/pull/461#issuecomment-3747421970","viewerDidAuthor":true},{"id":"IC_kwDOIzt9z87fXSmA","author":{"login":"gemini-code-assist"},"authorAssociation":"CONTRIBUTOR","body":"## Summary of Changes\n\n\n\nThis pull request refines the `CaptureButton` component by implementing several visual enhancements and behavioral corrections. The changes aim to align the button's appearance and interaction more closely with design mocks, improving user experience and visual clarity. Key updates include a new semi-transparent background, precise sizing adjustments, and fixes for recording state indicators and capture animations, alongside a streamlined preview setup.\n\n### Highlights\n\n* **Visual Updates**: A 50% semi-transparent black background has been added to the capture button ring for improved contrast. The capture button's overall size, ring stroke width, and nucleus dimensions have been adjusted to match design specifications. Compose Previews now utilize a gradient background (Gray to Dark Gray) and force a White content color, ensuring the semi-transparent background and button details are clearly visible in preview modes.\n* **Behavioral Fixes**: In `VIDEO_ONLY` mode, the nucleus now correctly appears white when idle and only turns red when pressed or actively recording, addressing a previous issue where it was always red. The capture animation has been fixed to trigger reliably when capturing via volume buttons. The pressed state color for `IMAGE_ONLY` mode has also been corrected.\n* **Refactoring**: The preview logic has been simplified and updated to directly use the real `CaptureButton` implementation, improving consistency and reducing redundancy in preview definitions.\n\n🧠 **New Feature in Public Preview:** You can now enable **Memory** to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**\n\n
\nChangelog\n\n* **ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureButtonComponents.kt**\n * Added new animation-related imports: `animateColor`, `animateDp`, `snap`, `updateTransition`.\n * Introduced `MutableInteractionSource`, `PressInteraction`, and `collectIsPressedAsState` for enhanced interaction handling.\n * Imported `MaterialTheme`, `darkColorScheme`, and `CompositionLocalProvider` for theme and content color management in previews.\n * Added `Brush` import for gradient backgrounds.\n * Decreased `DEFAULT_CAPTURE_BUTTON_SIZE` from 80f to 76f.\n * Modified the `CaptureButton` composable to accept an `interactionSource` parameter.\n * Implemented `PressInteraction` emission for key events to provide visual feedback.\n * Removed redundant `Log.d` calls.\n * Updated the private `CaptureButton` composable to use the new `interactionSource` and `isPressedInteraction`.\n * Adjusted the `animatedColor` animation specification based on the `isVisuallyDisabled` state.\n * Refactored `onPress` and `awaitRelease` logic to emit `PressInteraction` events.\n * Passed `isVisuallyDisabled` and `isPressed` parameters to `LockSwitchCaptureButtonNucleus` and `CaptureButtonNucleus`.\n * Reduced `CaptureButtonRing` `borderWidth` from 4f to 3f.\n * Added a 50% semi-transparent black background to the `CaptureButtonRing`.\n * Introduced `NucleusState` enum (`Disabled`, `Idle`, `Pressed`) for clearer state management.\n * Adjusted `idleImageCaptureScale` to .74f and `idleVideoCaptureScale` to .26f.\n * Added `isVisuallyDisabled` parameter to `CaptureButtonNucleus`.\n * Renamed `centerShapeSize` to `standardShapeSize` and implemented a new `centerShapeSize` driven by `pressTransition` for image capture mode.\n * Reworked the nucleus color animation using `updateTransition` and `NucleusState` to provide more granular control over color changes based on the button's state and capture mode.\n * Removed the explicit alpha modification logic from `CaptureButtonNucleus` as it is now handled by the new color transitions.\n * Refactored all `@Preview` composables to use a new `PreviewCaptureButton` wrapper, which configures a `MaterialTheme` with `darkColorScheme`, `LocalContentColor` as `Color.White`, and a gradient background for improved preview visibility.\n * Updated `LockSwitchCaptureButtonNucleus` previews to correctly pass `isVisuallyDisabled` and `isPressed`.\n
\n\n\n\n
\nActivity\n\n* The pull request author, temcguir, requested a summary of the changes.\n* A bot, gemini-code-assist[bot], provided a review comment suggesting to simplify the `isPressableImageMode` logic using a `when` expression for better readability, citing a style guide rule.\n
\n\n\n\n\n\n\n\n","createdAt":"2026-01-14T02:39:24Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/google/jetpack-camera-app/pull/461#issuecomment-3747424640","viewerDidAuthor":false}],"reviews":[{"id":"PRR_kwDOIzt9z87aEgHK","author":{"login":"gemini-code-assist"},"authorAssociation":"CONTRIBUTOR","body":"## Code Review\n\nThis pull request does a great job of updating the `CaptureButton` component's appearance and behavior. The changes align well with the design mocks, and the fixes for `VIDEO_ONLY` mode and volume button capture are significant improvements. The refactoring of the animation logic using `updateTransition` and the introduction of `InteractionSource` are excellent choices that follow Compose best practices. The simplification of the preview composables is also a welcome change that improves maintainability.\n\nI've left a couple of comments: one to address a logic regression that could cause `onRelease` to be called twice, and another with a suggestion to simplify a boolean check for better readability. Overall, this is a high-quality contribution.","submittedAt":"2026-01-14T02:39:08Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"d9398ce81729d10e5558514f003645f5574fa97f"}},{"id":"PRR_kwDOIzt9z87aRh8D","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-01-14T17:51:32Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"d9398ce81729d10e5558514f003645f5574fa97f"}},{"id":"PRR_kwDOIzt9z87aRiM8","author":{"login":"gemini-code-assist"},"authorAssociation":"CONTRIBUTOR","body":"","submittedAt":"2026-01-14T17:51:44Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"d9398ce81729d10e5558514f003645f5574fa97f"}},{"id":"PRR_kwDOIzt9z87aRotQ","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-01-14T17:57:49Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"d9398ce81729d10e5558514f003645f5574fa97f"}},{"id":"PRR_kwDOIzt9z87aRpBs","author":{"login":"gemini-code-assist"},"authorAssociation":"CONTRIBUTOR","body":"","submittedAt":"2026-01-14T17:58:08Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"d9398ce81729d10e5558514f003645f5574fa97f"}},{"id":"PRR_kwDOIzt9z87eXVaC","author":{"login":"Kimblebee"},"authorAssociation":"COLLABORATOR","body":"some duplicate comments regarding readibility, otherwise I think the PR looks great","submittedAt":"2026-01-30T23:01:37Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"647522bb7eb0468ff56b2e7f447640b26e0131a7"}},{"id":"PRR_kwDOIzt9z88AAAABFZFUpA","author":{"login":"Kimblebee"},"authorAssociation":"COLLABORATOR","body":"","submittedAt":"2026-07-09T17:41:51Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaKzJQ","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:13:34Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaK69g","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:13:54Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaLEaA","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:14:17Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaN1oA","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:21:12Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaOB-Q","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:21:44Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGaSeAg","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-17T18:33:33Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"68cc38184160f8c1879bddec42e21ca346f28dea"}},{"id":"PRR_kwDOIzt9z88AAAABGljLqQ","author":{"login":"Kimblebee"},"authorAssociation":"COLLABORATOR","body":"","submittedAt":"2026-07-20T18:57:44Z","includesCreatedEdit":false,"reactionGroups":[],"state":"APPROVED","commit":{"oid":"5a4ee7dd470e054d0d45f918fea879866f33f195"}},{"id":"PRR_kwDOIzt9z88AAAABGyLxxA","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-22T01:33:24Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"5a4ee7dd470e054d0d45f918fea879866f33f195"}},{"id":"PRR_kwDOIzt9z88AAAABGyL0aw","author":{"login":"temcguir"},"authorAssociation":"MEMBER","body":"","submittedAt":"2026-07-22T01:33:37Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"5a4ee7dd470e054d0d45f918fea879866f33f195"}}]} diff --git a/pr_line_comments.json b/pr_line_comments.json new file mode 100644 index 000000000..3e8b505cf --- /dev/null +++ b/pr_line_comments.json @@ -0,0 +1 @@ +[{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246804","pull_request_review_id":4483142380,"id":3401246804,"node_id":"PRRC_kwDOIzt9z87KuuhU","diff_hunk":"@@ -128,12 +211,84 @@ class LocalMediaRepository\n return@withContext false\n }\n \n+ private var cachedUri: Uri? = null\n+ private var cachedMediaDescriptor: MediaDescriptor? = null","path":"data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt","commit_id":"56e46c7286fee664fc58f3ee36786dda5ddfd059","original_commit_id":"56e46c7286fee664fc58f3ee36786dda5ddfd059","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![high](https://www.gstatic.com/codereviewagent/high-priority.svg)\n\nThe mutable properties `cachedUri` and `cachedMediaDescriptor` are accessed and modified concurrently within the `lastCapturedMedia` flow (which runs on `iODispatcher` / `repositoryScope`). Because `mapLatest` uses cooperative cancellation, a cancelled collection block can continue running concurrently with a new block after a suspension point. Since there is no synchronization, this can lead to race conditions and thread-visibility issues. Consider using a `Mutex` to synchronize access to these cached properties.\n\n```suggestion\n private val cacheMutex = kotlinx.coroutines.sync.Mutex()\n private var cachedUri: Uri? = null\n private var cachedMediaDescriptor: MediaDescriptor? = null\n```","created_at":"2026-06-12T06:42:11Z","updated_at":"2026-06-12T06:42:11Z","html_url":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246804","pull_request_url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531","_links":{"self":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246804"},"html":{"href":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246804"},"pull_request":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531"}},"reactions":{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246804/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":214,"start_side":"RIGHT","line":null,"original_line":215,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":122,"position":1,"subject_type":"line"},{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246820","pull_request_review_id":4483142380,"id":3401246820,"node_id":"PRRC_kwDOIzt9z87Kuuhk","diff_hunk":"@@ -128,12 +211,84 @@ class LocalMediaRepository\n return@withContext false\n }\n \n+ private var cachedUri: Uri? = null\n+ private var cachedMediaDescriptor: MediaDescriptor? = null\n+\n /**\n- * Returns the most recent captured media (image or video) from the MediaStore.\n+ * Returns the [MediaDescriptor] for the given [Uri] from the MediaStore.\n *\n- * @return The [MediaDescriptor] of the last captured media, or [MediaDescriptor.None] if no media is found.\n+ * @param uri The [Uri] of the media to retrieve.\n+ * @return The [MediaDescriptor] of the media, or [MediaDescriptor.None] if no media is found.\n */\n- override suspend fun getLastCapturedMedia(): MediaDescriptor {\n+ private suspend fun getCapturedMedia(uri: Uri): MediaDescriptor {\n+ val cachedDesc = cachedMediaDescriptor\n+ if (uri == cachedUri &&\n+ cachedDesc is MediaDescriptor.Content &&\n+ cachedDesc.thumbnail != null\n+ ) {\n+ return cachedDesc\n+ }\n+\n+ val descriptor = if (uri.toString().contains(\"video\")) {\n+ getVideoMediaDescriptor(uri)\n+ } else {\n+ getImageMediaDescriptor(uri)\n+ }\n+\n+ if (descriptor is MediaDescriptor.Content && descriptor.thumbnail != null) {\n+ cachedUri = uri\n+ cachedMediaDescriptor = descriptor\n+ return descriptor\n+ }\n+\n+ if (cachedDesc != null && cachedDesc is MediaDescriptor.Content) {\n+ // The new thumbnail isn't ready yet, so return the old cached image to prevent transient unmounting.\n+ return cachedDesc\n+ }\n+\n+ return descriptor\n+ }\n+\n+ /**\n+ * Checks if the given [Uri] belongs to the app.\n+ *\n+ * @param uri The [Uri] to check.\n+ * @return `true` if the URI is app-specific, `false` otherwise.\n+ */\n+ private suspend fun isAppSpecificUri(uri: Uri): Boolean = withContext(iODispatcher) {\n+ val projection = mutableListOf(MediaStore.MediaColumns.DISPLAY_NAME)\n+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {\n+ projection.add(MediaStore.MediaColumns.OWNER_PACKAGE_NAME)\n+ }\n+\n+ try {\n+ context.contentResolver.query(uri, projection.toTypedArray(), null, null, null)\n+ ?.use { cursor ->\n+ if (cursor.moveToFirst()) {\n+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {\n+ val ownerColumn =\n+ cursor.getColumnIndex(MediaStore.MediaColumns.OWNER_PACKAGE_NAME)\n+ if (ownerColumn != -1 && !cursor.isNull(ownerColumn)) {\n+ val owner = cursor.getString(ownerColumn)\n+ if (owner == context.packageName) return@withContext true\n+ }\n+ }\n+ val nameColumn =\n+ cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME)\n+ val name = cursor.getString(nameColumn)\n+ return@withContext name?.startsWith(\"JCA\") == true","path":"data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt","commit_id":"761e5d086617f61919bee1fe0c4262a96102ede6","original_commit_id":"56e46c7286fee664fc58f3ee36786dda5ddfd059","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)\n\nOn Android Q and above, the `OWNER_PACKAGE_NAME` is the definitive source of truth for app ownership. If the owner package name is present but does not match our package name, we should return `false` immediately rather than falling back to checking the display name prefix (which could result in false positives if another app creates a file starting with \"JCA\").\n\n```suggestion\n if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {\n val ownerColumn =\n cursor.getColumnIndex(MediaStore.MediaColumns.OWNER_PACKAGE_NAME)\n if (ownerColumn != -1 && !cursor.isNull(ownerColumn)) {\n val owner = cursor.getString(ownerColumn)\n return@withContext owner == context.packageName\n }\n }\n val nameColumn =\n cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME)\n val name = cursor.getString(nameColumn)\n return@withContext name?.startsWith(\"JCA\") == true\n```","created_at":"2026-06-12T06:42:11Z","updated_at":"2026-06-12T06:42:11Z","html_url":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246820","pull_request_url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531","_links":{"self":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246820"},"html":{"href":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246820"},"pull_request":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531"}},"reactions":{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246820/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":268,"start_side":"RIGHT","line":null,"original_line":279,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":189,"position":1,"subject_type":"line"},{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246826","pull_request_review_id":4483142380,"id":3401246826,"node_id":"PRRC_kwDOIzt9z87Kuuhq","diff_hunk":"@@ -15,21 +15,51 @@\n */\n package com.google.jetpackcamera.data.media.testing\n \n+import android.graphics.Bitmap\n import android.net.Uri\n import androidx.core.net.toUri\n import com.google.jetpackcamera.data.media.Media\n import com.google.jetpackcamera.data.media.MediaDescriptor\n import com.google.jetpackcamera.data.media.MediaRepository\n+import kotlinx.coroutines.CoroutineScope\n+import kotlinx.coroutines.Dispatchers\n import kotlinx.coroutines.flow.MutableStateFlow\n+import kotlinx.coroutines.flow.SharingStarted\n+import kotlinx.coroutines.flow.StateFlow\n import kotlinx.coroutines.flow.asStateFlow\n+import kotlinx.coroutines.flow.distinctUntilChanged\n+import kotlinx.coroutines.flow.filter\n+import kotlinx.coroutines.flow.stateIn\n import kotlinx.coroutines.flow.update\n \n class FakeMediaRepository : MediaRepository {\n private val _currentMedia = MutableStateFlow(MediaDescriptor.None)\n-\n override val currentMedia = _currentMedia.asStateFlow()\n \n- var loadHandler: (MediaDescriptor) -> Media = { Media.None }\n+ private val _lastCapturedMedia = MutableStateFlow(MediaDescriptor.None)\n+ override val lastCapturedMedia: StateFlow = _lastCapturedMedia.asStateFlow()\n+ .filter {\n+ when (it) {\n+ is MediaDescriptor.None -> true\n+ is MediaDescriptor.Content -> it.thumbnail != null\n+ }\n+ }\n+ .distinctUntilChanged()\n+ .stateIn(\n+ scope = CoroutineScope(Dispatchers.Main),\n+ started = SharingStarted.Eagerly,\n+ initialValue = MediaDescriptor.None\n+ )","path":"data/media/testing/src/main/kotlin/com/google/jetpackcamera/data/media/testing/FakeMediaRepository.kt","commit_id":"56e46c7286fee664fc58f3ee36786dda5ddfd059","original_commit_id":"56e46c7286fee664fc58f3ee36786dda5ddfd059","user":{"login":"gemini-code-assist[bot]","id":176961590,"node_id":"BOT_kgDOCow4Ng","avatar_url":"https://avatars.githubusercontent.com/in/956858?v=4","gravatar_id":"","url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D","html_url":"https://github.com/apps/gemini-code-assist","followers_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/followers","following_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/repos","events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/gemini-code-assist%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"body":"![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)\n\nThe fake repository is using a complex flow pipeline with `filter`, `distinctUntilChanged`, and `stateIn` using an unmanaged `CoroutineScope(Dispatchers.Main)`. This introduces unnecessary complexity, can cause coroutine leaks, and will fail in pure JUnit tests where `Dispatchers.Main` is not available. Since this is a fake, we should simplify it to a direct `StateFlow` backed by a `MutableStateFlow` without unmanaged scopes or main thread dependencies.\n\n```suggestion\n private val _lastCapturedMedia = MutableStateFlow(MediaDescriptor.None)\n override val lastCapturedMedia: StateFlow = _lastCapturedMedia.asStateFlow()\n```\n\n
\nReferences\n\n1. Simplify Complex Logic: Look for needlessly complex code. If a multi-line block of logic can be condensed into a more concise and readable idiomatic expression, suggest the simplification. ([link](https://github.com/google/jetpack-camera-app/blob/main/.gemini/styleguide.md))\n2. Testing Strategy: Use Fakes Over Mocks... Fakes lead to more robust and maintainable tests by focusing on behavior rather than implementation details... ([link](https://github.com/google/jetpack-camera-app/blob/main/.gemini/styleguide.md))\n
","created_at":"2026-06-12T06:42:11Z","updated_at":"2026-06-12T06:42:11Z","html_url":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246826","pull_request_url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531","_links":{"self":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246826"},"html":{"href":"https://github.com/google/jetpack-camera-app/pull/531#discussion_r3401246826"},"pull_request":{"href":"https://api.github.com/repos/google/jetpack-camera-app/pulls/531"}},"reactions":{"url":"https://api.github.com/repos/google/jetpack-camera-app/pulls/comments/3401246826/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"start_line":null,"original_start_line":39,"start_side":"RIGHT","line":null,"original_line":52,"side":"RIGHT","author_association":"CONTRIBUTOR","original_position":40,"position":1,"subject_type":"line"}] \ No newline at end of file diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt index 90c1a4af3..a121d1cfe 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt @@ -113,7 +113,7 @@ class CaptureControllerImpl( } } if (saveLocation !is SaveLocation.Cache) { - imageWellController.updateLastCapturedMedia() + // MediaRepository's flow will automatically emit the new item } else { savedUri?.let { scope.launch { @@ -164,7 +164,7 @@ class CaptureControllerImpl( } if (saveLocation !is SaveLocation.Cache) { - imageWellController.updateLastCapturedMedia() + // MediaRepository's flow will automatically emit the new item } else { scope.launch { postCurrentMediaToMediaRepository( diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/ImageWellControllerImpl.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/ImageWellControllerImpl.kt index ea9b21402..b1652e243 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/ImageWellControllerImpl.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/ImageWellControllerImpl.kt @@ -30,12 +30,10 @@ import kotlinx.coroutines.launch * Implementation of [ImageWellController] that handles image well actions. * * @param mediaRepository The [MediaRepository] for accessing media. - * @param updateLastCapturedMediaCallback Callback to update the last captured media. * @param coroutineContext The [CoroutineContext] for launching coroutines. */ class ImageWellControllerImpl( private val mediaRepository: MediaRepository, - private val updateLastCapturedMediaCallback: () -> Unit, coroutineContext: CoroutineContext ) : ImageWellController { private val job = Job(parent = coroutineContext[Job.Key]) @@ -48,9 +46,6 @@ class ImageWellControllerImpl( ) } } - override fun updateLastCapturedMedia() { - updateLastCapturedMediaCallback() - } /** * Initiates the cancellation of this controller's scope and returns its Job. diff --git a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt index 7ae893aaa..8b4c87840 100644 --- a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt +++ b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt @@ -29,8 +29,4 @@ interface ImageWellController { */ fun imageWellToRepository(mediaDescriptor: MediaDescriptor) - /** - * Updates the image well to display the most recently captured media from the repository. - */ - fun updateLastCapturedMedia() } diff --git a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt index e10c8e4b4..a96045eae 100644 --- a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt +++ b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt @@ -22,17 +22,12 @@ import com.google.jetpackcamera.ui.controller.ImageWellController * A fake implementation of [ImageWellController] that allows for configuring actions for its methods. * * @param imageWellToRepositoryAction The action to perform when [imageWellToRepository] is called. - * @param updateLastCapturedMediaAction The action to perform when [updateLastCapturedMedia] is called. */ class FakeImageWellController( var imageWellToRepositoryAction: (MediaDescriptor) -> Unit = {}, - var updateLastCapturedMediaAction: () -> Unit = {} ) : ImageWellController { override fun imageWellToRepository(mediaDescriptor: MediaDescriptor) { imageWellToRepositoryAction(mediaDescriptor) } - override fun updateLastCapturedMedia() { - updateLastCapturedMediaAction() - } } diff --git a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt index 08e8bcbee..1dd1c3b71 100644 --- a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt +++ b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt @@ -35,11 +35,5 @@ class FakeImageWellControllerTest { assertThat(calledDescriptor).isEqualTo(descriptor) } - @Test - fun updateLastCapturedMedia_invokesAction() { - var called = false - val controller = FakeImageWellController(updateLastCapturedMediaAction = { called = true }) - controller.updateLastCapturedMedia() - assertThat(called).isTrue() - } } + From 34b088eacbbdb922f89d34b5107f1d251ab0c2e2 Mon Sep 17 00:00:00 2001 From: Jetski Date: Thu, 23 Jul 2026 00:19:04 +0000 Subject: [PATCH 10/12] Apply spotless formatting --- .../google/jetpackcamera/ui/controller/ImageWellController.kt | 1 - .../ui/controller/testing/FakeImageWellController.kt | 3 +-- .../ui/controller/testing/FakeImageWellControllerTest.kt | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt index 8b4c87840..9fcf18232 100644 --- a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt +++ b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/ImageWellController.kt @@ -28,5 +28,4 @@ interface ImageWellController { * @param mediaDescriptor The media descriptor to be set. */ fun imageWellToRepository(mediaDescriptor: MediaDescriptor) - } diff --git a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt index a96045eae..6994ce5a6 100644 --- a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt +++ b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellController.kt @@ -24,10 +24,9 @@ import com.google.jetpackcamera.ui.controller.ImageWellController * @param imageWellToRepositoryAction The action to perform when [imageWellToRepository] is called. */ class FakeImageWellController( - var imageWellToRepositoryAction: (MediaDescriptor) -> Unit = {}, + var imageWellToRepositoryAction: (MediaDescriptor) -> Unit = {} ) : ImageWellController { override fun imageWellToRepository(mediaDescriptor: MediaDescriptor) { imageWellToRepositoryAction(mediaDescriptor) } - } diff --git a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt index 1dd1c3b71..72c88ad55 100644 --- a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt +++ b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeImageWellControllerTest.kt @@ -34,6 +34,4 @@ class FakeImageWellControllerTest { controller.imageWellToRepository(descriptor) assertThat(calledDescriptor).isEqualTo(descriptor) } - } - From e27982b4e58fddacca72afd1e5e66dc3f4d99e71 Mon Sep 17 00:00:00 2001 From: Jetski Date: Thu, 23 Jul 2026 00:22:14 +0000 Subject: [PATCH 11/12] Fix LocalMediaRepositoryTest failures caused by recent main merge - Added RELATIVE_PATH to MediaStore projection in LocalMediaRepository.isAppSpecificUri - Updated FakeContentProvider to support RELATIVE_PATH and OWNER_PACKAGE_NAME filtering --- .../data/media/LocalMediaRepository.kt | 1 + .../media/FakeContentProvider.kt | 29 ++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt index 720953bcc..2734a8b5c 100644 --- a/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt +++ b/data/media/src/main/kotlin/com/google/jetpackcamera/data/media/LocalMediaRepository.kt @@ -272,6 +272,7 @@ class LocalMediaRepository val projection = mutableListOf(MediaStore.MediaColumns.DISPLAY_NAME) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { projection.add(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) + projection.add(MediaStore.MediaColumns.RELATIVE_PATH) } try { diff --git a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt index 34c8d434c..44ba69cc6 100644 --- a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt +++ b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt @@ -105,16 +105,25 @@ class FakeContentProvider : ContentProvider() { } } - // Simple support for DISPLAY_NAME LIKE ? - if (selection != null && selection.contains( - MediaStore.MediaColumns.DISPLAY_NAME - ) && selectionArgs != null - ) { - val pattern = selectionArgs[0].replace("%", ".*").replace("_", ".") - val regex = Regex(pattern) - filteredMedia = filteredMedia.filter { - val name = it.value.getAsString(MediaStore.MediaColumns.DISPLAY_NAME) ?: "" - regex.matches(name) + // Simple support for RELATIVE_PATH LIKE ? AND OWNER_PACKAGE_NAME = ? + // or DISPLAY_NAME LIKE ? + if (selection != null) { + if (selection.contains(MediaStore.MediaColumns.RELATIVE_PATH) && selectionArgs != null && selectionArgs.size >= 2) { + val pathPattern = selectionArgs[0].replace("%", ".*").replace("_", ".") + val ownerPattern = selectionArgs[1] + val pathRegex = Regex(pathPattern) + filteredMedia = filteredMedia.filter { + val path = it.value.getAsString(MediaStore.MediaColumns.RELATIVE_PATH) ?: "" + val owner = it.value.getAsString(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) ?: context?.packageName + pathRegex.matches(path) && owner == ownerPattern + } + } else if (selection.contains(MediaStore.MediaColumns.DISPLAY_NAME) && selectionArgs != null) { + val pattern = selectionArgs[0].replace("%", ".*").replace("_", ".") + val regex = Regex(pattern) + filteredMedia = filteredMedia.filter { + val name = it.value.getAsString(MediaStore.MediaColumns.DISPLAY_NAME) ?: "" + regex.matches(name) + } } } From 58726f8b66c709df70f49ee3fcf5e154cc45d84d Mon Sep 17 00:00:00 2001 From: Jetski Date: Thu, 23 Jul 2026 00:22:42 +0000 Subject: [PATCH 12/12] Apply spotless formatting\n\nTAG=agy\nCONV=c141a4e9-d7bd-4552-afba-b09d4c36e5d0 --- .../jetpackcamera/media/FakeContentProvider.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt index 44ba69cc6..d47302be8 100644 --- a/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt +++ b/data/media/src/test/java/com/google/jetpackcamera/media/FakeContentProvider.kt @@ -108,16 +108,24 @@ class FakeContentProvider : ContentProvider() { // Simple support for RELATIVE_PATH LIKE ? AND OWNER_PACKAGE_NAME = ? // or DISPLAY_NAME LIKE ? if (selection != null) { - if (selection.contains(MediaStore.MediaColumns.RELATIVE_PATH) && selectionArgs != null && selectionArgs.size >= 2) { + if (selection.contains( + MediaStore.MediaColumns.RELATIVE_PATH + ) && selectionArgs != null && selectionArgs.size >= 2 + ) { val pathPattern = selectionArgs[0].replace("%", ".*").replace("_", ".") val ownerPattern = selectionArgs[1] val pathRegex = Regex(pathPattern) filteredMedia = filteredMedia.filter { val path = it.value.getAsString(MediaStore.MediaColumns.RELATIVE_PATH) ?: "" - val owner = it.value.getAsString(MediaStore.MediaColumns.OWNER_PACKAGE_NAME) ?: context?.packageName + val owner = it.value.getAsString( + MediaStore.MediaColumns.OWNER_PACKAGE_NAME + ) ?: context?.packageName pathRegex.matches(path) && owner == ownerPattern } - } else if (selection.contains(MediaStore.MediaColumns.DISPLAY_NAME) && selectionArgs != null) { + } else if (selection.contains( + MediaStore.MediaColumns.DISPLAY_NAME + ) && selectionArgs != null + ) { val pattern = selectionArgs[0].replace("%", ".*").replace("_", ".") val regex = Regex(pattern) filteredMedia = filteredMedia.filter {