Conversation
- 페이징 데이터 흐름(Flow) 반환 구조로 리팩토링한 코드에 맞춰 수정
- add paging3
📝 WalkthroughWalkthroughThis PR adds Android Paging3 support for gallery images: new PagingData API on the repository, a GalleryPagingSource implementation, a GalleryImageModel, build/deps updates, and refactors the home feature to consume cached PagingData flows and render via LazyPagingItems. Changes
Sequence DiagramsequenceDiagram
participant HomeUi as Feature:HomeUi
participant Presenter as Feature:HomePresenter
participant Repo as Core:GalleryRepository
participant Pager as Paging3:Pager
participant Source as Core:GalleryPagingSource
participant MediaStore as Android:MediaStore
HomeUi->>Presenter: collect galleryPagingFlow (cachedIn)
Presenter->>Repo: getGalleryPagingData()
Repo->>Pager: create Pager(flow) with GalleryPagingSource
Pager->>Source: load(params)
Source->>MediaStore: query(_ID, DATE_ADDED, offset/limit)
MediaStore-->>Source: Cursor(rows)
Source->>Source: map Cursor -> GalleryImageModel
Source-->>Pager: LoadResult.Page(data, prevKey, nextKey)
Pager-->>Repo: emits PagingData<GalleryImageModel>
Repo-->>Presenter: flow emits PagingData
Presenter-->>HomeUi: HomeUi collects as LazyPagingItems
HomeUi->>HomeUi: render items, check LoadState (Loading/NotLoading/Error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@core/data/impl/src/main/java/com/example/metasearch/core/data/impl/datasource/GalleryPagingSource.kt:
- Around line 54-58: The prevKey calculation in GalleryPagingSource's
LoadResult.Page can produce negative keys when offset < limit; update the logic
that sets prevKey (currently "if (offset == 0) null else offset - limit") to
clamp or guard against negatives — e.g., compute prev = offset - limit and set
prevKey = null if prev <= 0 (or prevKey = max(0, prev) if your loader accepts 0)
so prevKey is never negative; adjust in the same method where LoadResult.Page is
returned.
In @feature/home/src/main/java/com/example/metasearch/feature/home/HomeUi.kt:
- Around line 62-63: HomeUi currently reuses the incoming modifier when
composing MetaSearchMainBottomBar (passing modifier = modifier.padding(...)),
which can apply callers' modifiers to the bottom bar; change the call to use a
fresh Modifier instance instead (e.g., pass modifier =
Modifier.padding(MetaSearchTheme.spacing.spacing3)) so MetaSearchMainBottomBar
gets only its intended local modifiers; locate the usage in HomeUi where
MetaSearchMainBottomBar is invoked and replace the reused `modifier` with a new
`Modifier` combined with the padding.
🧹 Nitpick comments (4)
gradle/libs.versions.toml (2)
24-26: Clarify the scope: WorkManager and Hilt-Work additions.The PR objectives mention adding paging functionality for gallery images, but this change introduces WorkManager (
workRuntime) and commented-out Hilt-Work dependencies. Are these related to the paging feature, or should they be in a separate PR? If they're for the ImageAnalysisWorker (which already exists in the codebase), consider clarifying this in the commit message.Also applies to: 102-104
129-129: Remove or translate the Korean comment.The inline comment
# 이 줄을 추가하세요(translation: "Add this line") appears to be a development note that should be removed before merging, or translated to English for consistency with the rest of the codebase.♻️ Proposed fix
-okhttp = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okhttp" } # 이 줄을 추가하세요 +okhttp = { group = "com.squareup.okhttp3", name = "okhttp", version.ref = "okhttp" }core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/GalleryRepositoryImpl.kt (1)
39-39: Inconsistent dispatcher usage.The injected
ioDispatcheris used forGalleryPagingSource, but other methods likegetAllGalleryImagesstill use hardcodedDispatchers.IO. Consider using the injected dispatcher consistently for better testability.♻️ Suggested fix
- override suspend fun getAllGalleryImages(): List<Uri> = withContext(Dispatchers.IO) { + override suspend fun getAllGalleryImages(): List<Uri> = withContext(ioDispatcher) {Apply the same change to
getFileName,findMatchedUri, andfindMatchedUrismethods.feature/home/src/main/java/com/example/metasearch/feature/home/HomeUi.kt (1)
174-191: Paging integration looks correct.The usage of
items(count, key)withlazyPagingItems.itemKey { it.id }is the recommended Paging3 Compose pattern. The null check on line 179 properly handles placeholder scenarios (even though placeholders are disabled).Consider also showing a loading indicator during append operations for a smoother UX when scrolling to load more items:
💡 Optional: Add append loading indicator
if (lazyPagingItems.loadState.refresh is LoadState.Loading) { MetaSearchLoadingIndicator(modifier = Modifier.align(Alignment.Center)) } // Optional: Show a smaller indicator at bottom during append if (lazyPagingItems.loadState.append is LoadState.Loading) { // Add a bottom-aligned loading indicator }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.mdcore/data/api/build.gradle.ktscore/data/api/src/main/java/com/example/metasearch/core/data/api/repository/GalleryRepository.ktcore/data/impl/src/main/java/com/example/metasearch/core/data/impl/datasource/GalleryPagingSource.ktcore/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/GalleryRepositoryImpl.ktcore/model/src/main/java/com/example/metasearch/core/model/GalleryImageModel.ktcore/model/src/main/java/com/example/metasearch/core/model/HomeModel.ktfeature/home/src/main/java/com/example/metasearch/feature/home/HomePresenter.ktfeature/home/src/main/java/com/example/metasearch/feature/home/HomeUi.ktfeature/home/src/main/java/com/example/metasearch/feature/home/HomeUiState.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- core/model/src/main/java/com/example/metasearch/core/model/HomeModel.kt
🧰 Additional context used
🧬 Code graph analysis (2)
core/data/api/build.gradle.kts (1)
build-logic/src/main/java/com/example/metasearch/convention/Dependencies.kt (1)
api(11-13)
feature/home/src/main/java/com/example/metasearch/feature/home/HomeUi.kt (2)
feature/screens/src/main/java/com/example/metasearch/feature/screens/component/MetaSearchMainBottomBar.kt (1)
MetaSearchMainBottomBar(36-86)core/ui/src/main/java/com/example/metasearch/core/ui/component/MetaSearchSquareImage.kt (1)
MetaSearchSquareImage(14-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (10)
core/data/api/build.gradle.kts (1)
13-13: LGTM! Correct use ofapidependency.The Paging library is correctly exposed via
apisince the repository interface returnsFlow<PagingData<GalleryImageModel>>, which requires consumers to have access to the Paging types.README.md (1)
50-50: LGTM! Documentation accurately reflects the new Paging3 integration.The addition of Paging3 to the Jetpack Libraries section correctly documents the new dependency introduced in this PR.
feature/home/src/main/java/com/example/metasearch/feature/home/HomePresenter.kt (1)
62-64: LGTM! Proper paging implementation with caching.The
cachedIn(scope)call ensures that the paging data survives configuration changes and prevents redundant data loads. Therememberblock correctly creates the flow only once during the lifecycle of this presenter.core/model/src/main/java/com/example/metasearch/core/model/GalleryImageModel.kt (1)
1-10: LGTM! Well-designed data model.The
GalleryImageModelis properly structured:
@Stableannotation optimizes Compose recomposition- Immutable properties align with best practices
idfield clearly maps to MediaStore._IDdateAddedenables chronological sortinggradle/libs.versions.toml (1)
22-22: Version 3.3.6 is the current latest stable release (February 2025) with no known security vulnerabilities or advisories.No action needed; the dependency is already up-to-date.
feature/home/src/main/java/com/example/metasearch/feature/home/HomeUiState.kt (1)
16-16: Flow in UI state - ensure it's properly cached.Storing
Flow<PagingData<...>>directly in the UI state data class is a valid pattern with Circuit. Ensure this flow is created withcachedIn(viewModelScope/presenterScope)in the presenter to survive configuration changes and avoid redundant loads.core/data/api/src/main/java/com/example/metasearch/core/data/api/repository/GalleryRepository.kt (1)
8-14: LGTM!The repository interface is well-defined. The
getGalleryPagingData()correctly returns a coldFlow(non-suspend) which is the standard pattern for Paging3 integration.core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/GalleryRepositoryImpl.kt (1)
28-37: LGTM!The Paging implementation is correct. The
pagingSourceFactoryproperly creates a newGalleryPagingSourceinstance for each invalidation, which is the expected pattern for Paging3.core/data/impl/src/main/java/com/example/metasearch/core/data/impl/datasource/GalleryPagingSource.kt (2)
65-70: LGTM!The
getRefreshKeyimplementation follows the standard Paging3 pattern for calculating the refresh position based on the anchor position and nearby page keys.
30-35: No API level compatibility issue: The code is compatible with the project's minSdk of 30, which exceeds the API 26 requirement forContentResolver.QUERY_ARG_*constants. No fallback needed.Likely an incorrect or invalid review comment.
| LoadResult.Page( | ||
| data = imageList, | ||
| prevKey = if (offset == 0) null else offset - limit, | ||
| nextKey = if (imageList.size < limit) null else offset + imageList.size, | ||
| ) |
There was a problem hiding this comment.
Potential negative prevKey value.
When offset < limit (e.g., on initial load with initialLoadSize=60 but pageSize=30), prevKey = offset - limit would produce a negative value. This should be clamped to avoid invalid page keys.
🐛 Proposed fix
LoadResult.Page(
data = imageList,
- prevKey = if (offset == 0) null else offset - limit,
+ prevKey = if (offset == 0) null else (offset - limit).coerceAtLeast(0).takeIf { it < offset },
nextKey = if (imageList.size < limit) null else offset + imageList.size,
)Alternatively, a simpler approach:
- prevKey = if (offset == 0) null else offset - limit,
+ prevKey = if (offset == 0) null else maxOf(0, offset - limit),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LoadResult.Page( | |
| data = imageList, | |
| prevKey = if (offset == 0) null else offset - limit, | |
| nextKey = if (imageList.size < limit) null else offset + imageList.size, | |
| ) | |
| LoadResult.Page( | |
| data = imageList, | |
| prevKey = if (offset == 0) null else (offset - limit).coerceAtLeast(0).takeIf { it < offset }, | |
| nextKey = if (imageList.size < limit) null else offset + imageList.size, | |
| ) |
🤖 Prompt for AI Agents
In
@core/data/impl/src/main/java/com/example/metasearch/core/data/impl/datasource/GalleryPagingSource.kt
around lines 54 - 58, The prevKey calculation in GalleryPagingSource's
LoadResult.Page can produce negative keys when offset < limit; update the logic
that sets prevKey (currently "if (offset == 0) null else offset - limit") to
clamp or guard against negatives — e.g., compute prev = offset - limit and set
prevKey = null if prev <= 0 (or prevKey = max(0, prev) if your loader accepts 0)
so prevKey is never negative; adjust in the same method where LoadResult.Page is
returned.
| modifier = modifier | ||
| .padding(bottom = MetaSearchTheme.spacing.spacing3), |
There was a problem hiding this comment.
Avoid reusing the outer modifier parameter.
The modifier parameter from HomeUi is being passed to MetaSearchMainBottomBar. This can cause unintended side effects since the caller's modifiers (e.g., fillMaxSize()) would be applied to the bottom bar. Use a fresh Modifier instead.
🐛 Proposed fix
MetaSearchMainBottomBar(
- modifier = modifier
+ modifier = Modifier
.padding(bottom = MetaSearchTheme.spacing.spacing3),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| modifier = modifier | |
| .padding(bottom = MetaSearchTheme.spacing.spacing3), | |
| modifier = Modifier | |
| .padding(bottom = MetaSearchTheme.spacing.spacing3), |
🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/example/metasearch/feature/home/HomeUi.kt
around lines 62 - 63, HomeUi currently reuses the incoming modifier when
composing MetaSearchMainBottomBar (passing modifier = modifier.padding(...)),
which can apply callers' modifiers to the bottom bar; change the call to use a
fresh Modifier instance instead (e.g., pass modifier =
Modifier.padding(MetaSearchTheme.spacing.spacing3)) so MetaSearchMainBottomBar
gets only its intended local modifiers; locate the usage in HomeUi where
MetaSearchMainBottomBar is invoked and replace the reused `modifier` with a new
`Modifier` combined with the padding.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Android CI Summary
Step Results:
- Unit Test: ❌ Failure (0s)
- Debug Build: ⏭️ Skipped (0s)
- Code Style Check: ✅ Success (4m 0s)
Total Time: 4m 0s
See the Actions Log for details.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.