Fix updating regions

[JRWilding]

Marking a region as invalid has a number of problems.
Tiles/Resources marked as offline that expire are not useable.

This PR does the following:

1. Track the online status in OnlineFileSource.

2. Adds canRequestNow to file sources. While canRequest checks the validity of the resource (to ensure it is the correct type of request for the given file source), it does not check whether or not the request can actually be performed. canRequestNow should check the environment to ensure that the request should be completable. This is currently only implemented for OnlineFileSource to check that a network connection is available, but could be expanded upon to ensure files are writable etc.
   2a. In DatabaseFileSource.request when the response is not useable (!offlineResponse->isUsable()) - if the network is not available to revalidate / update the expired resource than allow the use of the old data because otherwise the map will show nothing.
   2b. In MainResourceLoader.request if the database response is not usable but there is no network available, reply to the callback with the database version so that it can be used while the online request is scheduled to update the data when the network is available again.

3. OfflineDownload.ensureResource checks whether the resource exists in the database, but doesn't check whether it is usable. If a region is invalidated, and then re-downloaded it doesn't schedule any network to request the new data because it thinks everything is fine. Now, check isUsable and ensure the etag, prior data etc is included in the request so that it can revalidate without having to download needlessly (server can respond 304 if nothing has changed)

4. SpriteLoader.load previously asserted if the data was the same, with the change in 3 above, it can be -> so now only emit if something actually changed, but don't assert

This fixes
#410
and removes the need for #409

[louwers]

It sounds like a sensible change. I have to study this part of the codebase a bit better.

online_file_source.test.cpp contains quite extensive tests. One or more new tests should be added there. You may need to update the helper HTTP server for the tests as well.

[github-actions]

Benchmark Results ⚡

Benchmark                                                     Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------
OVERALL_GEOMEAN                                            -0.0117         -0.0112             0             0             0             0

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-2517-compared-to-main.txt

[github-actions]

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +12.4Ki  +0.0% +3.71Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2517-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +28% +32.6Mi  +426% +25.4Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2517-compared-to-legacy.txt

[github-actions]

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +480  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-2517-compared-to-main.txt

[louwers]

I think the whole point of must-revalidate is that caches should not use stale data when it is present. https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.4 In my opinion if you are OK with stale data being used, you should ask your tile server operator not to include must-revalidate.

[JRWilding]

I think the whole point of must-revalidate is that caches should not use stale data when it is present. https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.4 In my opinion if you are OK with stale data being used, you should ask your tile server operator not to include must-revalidate.

OfflineDatabase::invalidateRegion sets must-revalidate=1 as well, which comes from client side only call to OfflineRegion.invalidate.

Say you have a region for a city saved for offline purposes. After a couple of weeks, you want to make sure you've got the latest tiles for the entire city.

There isn't a specific API to refresh a region like that, but invalidating it and telling it to download again makes sense. That should reuse existing data and avoid unnecessary downloads by using the etags for tiles which haven't changed, while downloading anything that has.

Problem is OfflineDownload::ensureResource just looks to see if the tile is in the database, and not whether it is expired/invalidated. If the tile is there, it doesn't bother scheduling the network request to check if there is new data for the tile.

So invalidating a region currently makes all the data stored locally useless, until you manually look at each tile on a map (it'll then get a new version (or get 304 back) from the server). If you're offline I think the majority of the time stale data is better than no data.

Will push the suggested changes

[louwers]

Maybe adding support for stale-while-revalidate would be a good solution for the use case you describe: https://web.dev/articles/stale-while-revalidate

[louwers]

@JRWilding I think setting stale-while-revalidate with OfflineDatabase::invalidateRegion would be a cleaner solution. The must-revalidate header from the server is honored, while downloaded tiles can still be used while offline.

Would you be available for a quick call sometime to discuss this issue?

[JRWilding]

@louwers sorry for the late reply. Yes that sounds like it might be a cleaner solution. Although it would involve db version update and migration to add that to the table, which is a bit more involved than I have capacity for at the moment.