Remote settings caching support

[bendk]

Continued from #6295. This is an attempt to expose the caching API to consumers.

Added a builder API for constructing RemoteSettings instances. Gave the builder a couple ways to enable caching support:

• Simple method: specify a cache path
• Advance method: implement the RemoteSettingsCache callback interface

If a cache is supplied then:

• get_records() will use the cache for efficient network requests
• Consumers can use the get_cached_records and sync_cached_records for extra control over when we hit the network. This is useful for settings where typically only want to get the setting from cache and not hit the network.

Pull Request checklist

• Breaking changes: This PR follows our breaking change policy
  • [ ] This PR follows the breaking change policy:
    • This PR has no breaking API changes, or
    • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
• [ ] Quality: This PR builds and tests run cleanly
  • Note:
    • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
    • If this pull request includes a breaking change, consider cutting a new release after merging.
• [ ] Tests: This PR includes thorough tests or an explanation of why it does not
• [ ] Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
  • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
• [ ] Dependencies: This PR follows our dependency management guidelines
  • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 128 lines in your changes missing coverage. Please review.

Project coverage is 22.54%. Comparing base (4874113) to head (a29068d). Report is 55 commits behind head on main.

Files with missing lines	Patch %	Lines
components/remote_settings/src/lib.rs	0.00%	53 Missing :warning:
components/remote_settings/src/cache.rs	0.00%	43 Missing :warning:
components/remote_settings/src/client.rs	0.00%	31 Missing :warning:
components/remote_settings/src/error.rs	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6328      +/-   ##
==========================================
- Coverage   22.64%   22.54%   -0.10%     
==========================================
  Files         332      333       +1     
  Lines       29830    29953     +123     
==========================================
  Hits         6754     6754              
- Misses      23076    23199     +123     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[bendk]

I would suggest to align the philosophy of this client with the one we have on desktop 🙏

Instead of having a new method get_cached_records() etc., we would just have:

* `client.get(syncIfEmpty=false)` --> returns local cache, and calls sync() if local is empty

* `client.sync(timestamp)` --> refreshes the local cache (if timestamp is None or differs from local cache).

Clients that need default data when network is off and local cache empty would provide just hardcoded values (.get().unwrap_or(...)).

+1 to this plan.

What do you think about having the clients specify default data in the constructor? That way get() wouldn't need to return an Option?

[Standard8]

What do you think about having the clients specify default data in the constructor? That way get() wouldn't need to return an Option?

In the draft specs document, @leplatrem was suggesting t to move the dumps into the component here, so that they could be read directly. This might be easier done once a-s moves into m-c though. However, this would avoid the potentially expensive operation of passing long strings across the uniffi interface.

[leplatrem]

What do you think about having the clients specify default data in the constructor? That way get() wouldn't need to return an Option?

That would work for me, but as Mark said we have to clarify how we combine this with packaged dumps. For example, if a collection has a packaged dump, then it will be its default data. If it doesn't, the default data specified during client build is used. If no default data is provided during client build, what do we return?

I suggest the Option, because in desktop we made the "mistake" to return an empty list by default. With no way for the consumer to distinguish between an empty list (eg. no ongoing experiment), a fresh profile, a faulty sync, etc.

[bendk]

That would work for me, but as Mark said we have to clarify how we combine this with packaged dumps. For example, if a collection has a packaged dump, then it will be its default data. If it doesn't, the default data specified during client build is used. If no default data is provided during client build, what do we return?

I suggest the Option, because in desktop we made the "mistake" to return an empty list by default. With no way for the consumer to distinguish between an empty list (eg. no ongoing experiment), a fresh profile, a faulty sync, etc.

You've convinced me that an option is better. I was hoping that we could guarantee that there would be fallback data, but it seems like we can't. It's means the clients will have to add some extra comments explaining how they know a value isn't null and maybe add some !! expressions, but that doesn't seem so bad.

[bendk]

I'm going to close this one since we want to update this API to bring it closer to the one used by the official client and this code is now pretty stale.

https://github.com/mozilla/application-services/pull/6378 contains a proposal for the new API, let's continue the discussion there.