feat(app): catalogo offline (Room) + cache copertine persistente + refresh all'apertura#9
Open
fabiodalez-dev wants to merge 1 commit into
Open
feat(app): catalogo offline (Room) + cache copertine persistente + refresh all'apertura#9fabiodalez-dev wants to merge 1 commit into
fabiodalez-dev wants to merge 1 commit into
Conversation
…fresh on open Reduces server load and lets the catalog work offline. - Room cache (data/local/): CachedBook entity + CatalogDao (Flow-backed) + AppDatabase store the catalog snapshot locally. CatalogRepository exposes observeCachedCatalog() (offline-first) and refreshCatalog() (fetch page 1, replace the snapshot atomically; a network failure keeps the existing cache). - Home is offline-first: it renders the "Available now" shelf from the cached snapshot immediately (works with no network) and refreshes in the background. - Covers: a persistent 256 MB Coil ImageLoader with respectCacheHeaders(false), so covers are downloaded once and reused instead of re-fetched every time. - Refresh on app open: ProcessLifecycle ON_START refreshes the cache when logged in, so the offline catalog stays current without hammering the server. - First unit-test suite for the app (29 tests): Room CatalogDao (Robolectric, in-memory), entity↔summary mappers, BookSummary availability, reservation cancellable rules, and the URL-derivation helpers. - versionCode 3 -> 4, versionName 1.1.1 -> 1.2.0. Verified: assembleDebug builds; testDebugUnitTest green (29/29).
WalkthroughViene introdotta una cache locale offline-first basata su Room per il catalogo libri. Sono aggiunti l'entità ChangesCache offline del catalogo e aggiornamento in foreground
Sequence Diagram(s)sequenceDiagram
participant App as PinakesApplication
participant PLO as ProcessLifecycleOwner
participant Repo as CatalogRepository
participant DAO as CatalogDao
participant Net as NetworkModule
rect rgba(33, 150, 243, 0.5)
note over App,PLO: Avvio in foreground
PLO->>App: onStart()
App->>Repo: refreshCatalog()
Repo->>Net: search(page=1, limit=40)
Net-->>Repo: ApiResult.Success(books)
Repo->>DAO: replaceAll(books.toCached())
end
rect rgba(76, 175, 80, 0.5)
note over App,DAO: HomeViewModel osserva cache
DAO-->>Repo: Flow~List~CachedBook~~
Repo-->>App: observeCachedCatalog() Flow~List~BookSummary~~
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/pinakes/app/PinakesApplication.kt (1)
30-35: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRefresh duplicato all'avvio con utente loggato.
All'apertura dell'app con sessione attiva,
ProcessLifecycleOwneronStartlanciarefreshCatalog()e, in parallelo,HomeViewModel.initchiama anch'essorefresh(). Si ottengono due richieste di rete e duereplaceAllconcorrenti (serializzati da Room, ma comunque ridondanti). Valutare un meccanismo di de-duplicazione/throttle (es. saltare se un refresh recente è già in corso) per evitare traffico e scritture superflue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/pinakes/app/PinakesApplication.kt` around lines 30 - 35, The startup refresh path is triggering duplicate catalog loads when a user is already logged in: `PinakesApplication`’s `ProcessLifecycleOwner` observer and `HomeViewModel.init` both call refresh logic. Update the refresh flow around `services.catalogRepository.refreshCatalog()` and the `HomeViewModel` refresh entrypoint so only one in-flight or very recent refresh can run, using a shared de-duplication/throttle guard. Keep the change localized to the lifecycle observer and the view-model refresh trigger so repeated app foregrounding or initialization does not issue redundant network requests or `replaceAll` writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/pinakes/app/ui/screens/home/HomeViewModel.kt`:
- Around line 47-54: In HomeViewModel.observeCache(), the state currently sets
loading to false on every cache emission, including Room’s initial empty list,
which hides the initial spinner too early. Update the flow handling so loading
stays true while the cache is still empty and only gets cleared when refresh()
completes or when a non-empty catalog is observed; keep using the existing
observeCache(), refresh(), and _state.update symbols to locate the change.
---
Nitpick comments:
In `@app/src/main/java/com/pinakes/app/PinakesApplication.kt`:
- Around line 30-35: The startup refresh path is triggering duplicate catalog
loads when a user is already logged in: `PinakesApplication`’s
`ProcessLifecycleOwner` observer and `HomeViewModel.init` both call refresh
logic. Update the refresh flow around
`services.catalogRepository.refreshCatalog()` and the `HomeViewModel` refresh
entrypoint so only one in-flight or very recent refresh can run, using a shared
de-duplication/throttle guard. Keep the change localized to the lifecycle
observer and the view-model refresh trigger so repeated app foregrounding or
initialization does not issue redundant network requests or `replaceAll` writes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a68e91a4-b969-4a10-b00d-d45568fc4cde
📒 Files selected for processing (14)
app/build.gradle.ktsapp/src/main/java/com/pinakes/app/PinakesApplication.ktapp/src/main/java/com/pinakes/app/data/local/AppDatabase.ktapp/src/main/java/com/pinakes/app/data/local/CachedBook.ktapp/src/main/java/com/pinakes/app/data/local/CatalogDao.ktapp/src/main/java/com/pinakes/app/data/repository/CatalogRepository.ktapp/src/main/java/com/pinakes/app/di/ServiceLocator.ktapp/src/main/java/com/pinakes/app/ui/screens/home/HomeViewModel.ktapp/src/test/java/com/pinakes/app/BookSummaryTest.ktapp/src/test/java/com/pinakes/app/CachedBookMapperTest.ktapp/src/test/java/com/pinakes/app/CatalogDaoTest.ktapp/src/test/java/com/pinakes/app/NetworkUrlTest.ktapp/src/test/java/com/pinakes/app/StatusMappingTest.ktgradle/libs.versions.toml
Comment on lines
+47
to
+54
| private fun observeCache() { | ||
| viewModelScope.launch { | ||
| // "Available now" shelf: full catalog filtered to currently-loanable copies. | ||
| when (val res = catalog.search(SearchFilters(availableOnly = true), limit = 20)) { | ||
| is ApiResult.Success -> _state.update { | ||
| it.copy(available = res.data.items, loading = false, error = null) | ||
| } | ||
| is ApiResult.Failure -> _state.update { | ||
| it.copy(loading = false, error = res.message.takeIf { m -> m.isNotBlank() }) | ||
| catalog.observeCachedCatalog().collectLatest { books -> | ||
| val available = books.filter { it.available } | ||
| _state.update { it.copy(available = available, loading = false, error = null) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Lo spinner iniziale viene perso al primo avvio con cache vuota.
observeCache() imposta loading = false ad ogni emissione del Flow, inclusa la prima emissione di lista vuota che Room produce immediatamente. Su un'installazione pulita (cache vuota) lo stato diventa subito isEmpty = true mentre refresh() è ancora in corso, mostrando lo stato "catalogo vuoto" invece dell'indicatore di caricamento. Conviene mantenere loading finché la cache è vuota e lasciare che sia refresh() a chiuderlo, oppure abbassare loading solo quando arriva una lista non vuota.
🩹 Possibile correzione
private fun observeCache() {
viewModelScope.launch {
catalog.observeCachedCatalog().collectLatest { books ->
val available = books.filter { it.available }
- _state.update { it.copy(available = available, loading = false, error = null) }
+ _state.update {
+ it.copy(
+ available = available,
+ loading = if (available.isEmpty()) it.loading else false,
+ error = null,
+ )
+ }
}
}
}📝 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.
Suggested change
| private fun observeCache() { | |
| viewModelScope.launch { | |
| // "Available now" shelf: full catalog filtered to currently-loanable copies. | |
| when (val res = catalog.search(SearchFilters(availableOnly = true), limit = 20)) { | |
| is ApiResult.Success -> _state.update { | |
| it.copy(available = res.data.items, loading = false, error = null) | |
| } | |
| is ApiResult.Failure -> _state.update { | |
| it.copy(loading = false, error = res.message.takeIf { m -> m.isNotBlank() }) | |
| catalog.observeCachedCatalog().collectLatest { books -> | |
| val available = books.filter { it.available } | |
| _state.update { it.copy(available = available, loading = false, error = null) } | |
| } | |
| } | |
| } | |
| private fun observeCache() { | |
| viewModelScope.launch { | |
| catalog.observeCachedCatalog().collectLatest { books -> | |
| val available = books.filter { it.available } | |
| _state.update { | |
| it.copy( | |
| available = available, | |
| loading = if (available.isEmpty()) it.loading else false, | |
| error = null, | |
| ) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/pinakes/app/ui/screens/home/HomeViewModel.kt` around
lines 47 - 54, In HomeViewModel.observeCache(), the state currently sets loading
to false on every cache emission, including Room’s initial empty list, which
hides the initial spinner too early. Update the flow handling so loading stays
true while the cache is still empty and only gets cleared when refresh()
completes or when a non-empty catalog is observed; keep using the existing
observeCache(), refresh(), and _state.update symbols to locate the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementa una cache locale per non sovraccaricare il server e far funzionare il catalogo offline (richiesta esplicita).
Cosa fa
data/local/):CachedBook+CatalogDao(Flow) +AppDatabasesalvano uno snapshot del catalogo.CatalogRepositoryesponeobserveCachedCatalog()(offline-first) erefreshCatalog()(scarica la prima pagina e rimpiazza lo snapshot in modo atomico; se la rete fallisce, la cache resta).ImageLoaderCoil persistente da 256 MB conrespectCacheHeaders(false)→ le copertine si scaricano una volta e si riusano, invece di rifarle ogni volta.ProcessLifecycle ON_STARTaggiorna la cache quando sei loggato → il catalogo offline resta aggiornato senza martellare il server.Test (prima suite di test dell'app — 29 test, tutti verdi)
CatalogDaoTest(Robolectric, Room in-memory): insert/observe/ordinamento/replaceAllatomico/clear.CachedBookMapperTest: round-trip entity↔summary.BookSummaryTest: disponibilità derivata.StatusMappingTest: regole di annullabilità prenotazione.NetworkUrlTest: derivazione URL istanza../gradlew testDebugUnitTest→ 29/29 verde.assembleDebugbuilda. versionCode 4 / 1.2.0.Note
Summary by CodeRabbit
Nuove funzionalità
Miglioramenti
Test