WeatherApp icon indicating copy to clipboard operation
WeatherApp copied to clipboard

Cache and Eviction Strategy: Offline First

Open CalebKL opened this issue 2 years ago • 6 comments

Related Issue

#21

Description

Added offline First features. fetchWeather function from the repositiory is responsible for fetching weather data. It first checks if there is cached weather data available and if it is still valid. If so, it emits the cached data as a ApiResult.Success result. If not, it makes an API request to fetch the weather data. If the API response is successful, it converts the response to a WeatherEntity, inserts it into the local cache using the weatherDao, and emits the converted data as a ApiResult.Success result. Finally, in the onCompletion block, it schedules a background worker (UpdateWeatherWorker) using WorkManager to refresh the weather data periodically after 15 Minutes.

How Can It Be Tested

The Test function passes and emits ApiResult.Success when fetchWeather contains the expected mapped weather data.

Screenshots (If Applicable)

N/A

Additional Comments

I also added a notification utility, so when there's a newly refreshed weather update, client gets a notification.

Checklist

  • [ x] New tests were added/Existing Modified

CalebKL avatar May 17 '23 15:05 CalebKL

I have ran the tests bitrise is warning about and they are passing. Not sure why but I added Roboelectric test runner and they pass.

CalebKL avatar May 17 '23 15:05 CalebKL

@odaridavid I had updated the changes as you had requested. Kindly review. @jumaallan I have removed gson and updated the entities to use relations, however, I am not able to populate the hourly and daily lists, I can only view the current weather. Kindly check and the entities. There' entities is a bit complex since we have about three nested entities(Weather-Listof(Dailly)-Listof(WeatherResponse). The same applies to the hourly. I used type converters to convert the list of weather response then have relations with the weather and daily list/hourly list.

CalebKL avatar May 30 '23 14:05 CalebKL

Codecov Report

Patch coverage: 31.30% and project coverage change: -11.96 :warning:

Comparison is base (1370359) 58.76% compared to head (bf60f85) 46.80%.

:exclamation: Current head bf60f85 differs from pull request most recent head 15c0363. Consider uploading reports for the commit 15c0363 to get more accurate results

Additional details and impacted files
@@              Coverage Diff               @@
##             develop      #41       +/-   ##
==============================================
- Coverage      58.76%   46.80%   -11.96%     
- Complexity        31       42       +11     
==============================================
  Files             18       25        +7     
  Lines            388      611      +223     
  Branches          30       37        +7     
==============================================
+ Hits             228      286       +58     
- Misses           149      311      +162     
- Partials          11       14        +3     
Impacted Files Coverage Δ
...ava/com/github/odaridavid/weatherapp/WeatherApp.kt 0.00% <0.00%> (ø)
...erapp/data/weather/DefaultRefreshWeatherUseCase.kt 0.00% <0.00%> (ø)
...d/weatherapp/data/weather/local/WeatherDatabase.kt 0.00% <0.00%> (ø)
...herapp/data/weather/local/converters/Converters.kt 0.00% <0.00%> (ø)
...hub/odaridavid/weatherapp/ui/home/HomeViewModel.kt 92.72% <ø> (ø)
...hub/odaridavid/weatherapp/util/NotificationUtil.kt 0.00% <0.00%> (ø)
...aridavid/weatherapp/worker/UpdatedWeatherWorker.kt 0.00% <0.00%> (ø)
...idavid/weatherapp/worker/WeatherUpdateScheduler.kt 0.00% <0.00%> (ø)
...therapp/data/weather/local/entity/WeatherEntity.kt 39.34% <39.34%> (ø)
...thub/odaridavid/weatherapp/data/weather/Mappers.kt 31.40% <47.22%> (-10.60%) :arrow_down:
... and 1 more

... and 2 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar May 30 '23 20:05 codecov[bot]

There seems to be an issue when I run the app, I'll hold off on the merge for now

odaridavid avatar Jun 10 '23 10:06 odaridavid

@odaridavid yes. There's a problem with the room relations. I am waiting for @jumaallan to help me out. Alternatively, can I use Typeconverters for this PR and then work on the room relations in a separate PR? what do you think?

CalebKL avatar Jun 10 '23 10:06 CalebKL

@odaridavid I fixed the error on Workmanager. Also I had to add latitude and longitude to the weather models so I can use it on my entity. I will explain why? The Populated weather would have consisted of Current, Hourly list and daily list. But since the current weather has a list of weather info responses, we would need to define another entity(Weather Entity) that would have the latitude and longitude parameters. The ID(weatherId) on this entity would then need to the parent entity on the PopulatedWeather as I have done. Here is the resource I used. https://developer.android.com/training/data-storage/room/relationships (At the bottom there's the implementation on defining nested relationships).

This also fixes the review @jumaallan had initially requested and removed the type converters. Have a look and let me know if there's anything I should update on this PR.

CalebKL avatar Jun 19 '23 09:06 CalebKL