Refactor rates data layer and update dependencies#170
Conversation
eb53965 to
208e90c
Compare
208e90c to
cce4c46
Compare
cce4c46 to
758dadf
Compare
758dadf to
0325588
Compare
0325588 to
640aa0d
Compare
shubertm
left a comment
There was a problem hiding this comment.
@kirillt, @mdrlzy Most of currency icons I had have gone missing after updating to this branch. Is it a design choice or something is not working on my side?
Code review findings from Codex:
1.
High: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 RatesUpdatedAtCheck is persisted before the crypto/fiat fetches and DB write complete. If either remote fetch fails, updateRates() returns failure, but subsequent calls are skipped for 10 minutes because the throttle check at CurrencyRepoImpl.kt:80 now short-circuits. A transient network or endpoint failure therefore causes a forced stale-data window.
2.
Medium: core/data/src/main/java/dev/arkbuilders/rate/core/data/repo/currency/CurrencyRepoImpl.kt:99 The same throttle also applies after a “remote data not newer” result at CurrencyRepoImpl.kt:101. If upstream publishes new rates shortly after that check, the app will not pick them up until the 10-minute interval expires. This is more visible because splash now waits on updateRates() during startup in app/src/main/java/dev/arkbuilders/rate/presentation/splash/SplashViewModel.kt:37 and SplashViewModel.kt:59.
This is by design. Crypto icons are now dynamic and updated for each apk based on the top N currencies returned by the API. Here’s the command for populating the icons. |
I don’t see this as an issue. Rates are updated every 8 hours, so a potential 10-minute delay is negligible. The interval also prevents excessive retries when a request fails. I’d keep it as is |
📌 Description
updated_atchecks to avoid unnecessary API checks happening many times per minute