Cache and Eviction Strategy: Offline First

[CalebKL]

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]

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.

[jumaallan]

use something modern like moshi or kotlinx.serialization

[CalebKL]

Okay. On it

[jumaallan]

Just curious

Why do you want to save a list of entities as a string - is it not better to basically use relationships?

Also, avoid gson as suggested earlier

Use typeconverters only when converting data types not supported by room natively

[CalebKL]

My bad. Relationships would be a better approach. Let me make the adjustments and remove gson.

[odaridavid]

Thanks for the contribution 🙌, I'll leave some comments on the go

[odaridavid]

I'd suggest more meaningful names and description on what kind of notificiations this channel provides e.g Weather Updates or something in that line

[CalebKL]

okay.

[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.

[codecov]

Codecov Report

Patch coverage: 31.30% and project coverage change: -11.96 ⚠️

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

❗ 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%)	⬇️
... and 1 more

... and 2 files with indirect coverage changes

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

[odaridavid]

Looks fine to me ✅, great work on pushing this.

My final comment would be the UseCase implementation should ideally sit in the core/domain and can be done in such a way that the platform-specific needs are implemented in the presentation layer, but that can be followed up later like so

class DefaultRefreshWeatherUseCase @Inject constructor(
    private val weatherRepository: WeatherRepository
) : RefreshWeatherUseCase {
    override operator fun invoke(
        defaultLocation: DefaultLocation,
        language: String,
        units: String,
        onSuccess: (data) -> Unit
        onError: (errorType) -> Unit
    ): Flow<Result<Weather>>  = flow{
        weatherRepository.fetchWeatherData(
            defaultLocation = defaultLocation,
            language = language,
            units =units
        ).collect{ result->
            when (result) {
                is Result.Success -> {
                     onSuccess(result.data)
                }
                is Result.Error -> {
                    onError(result.errorType)
                }
            }
        }

    }
}

[odaridavid]

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

[odaridavid]

What I see on the log cat

[CalebKL]

@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]

@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.