We should leverage networking libraries that specialize in network requests and caching rather than maintaining a custom cache implementation for pages alone. This task encompasses the work to remove PageCache and add any missing pieces needed to replace the functionality with a Retrofit OkHttp implementation. A subsequent task may investigate replacing saved pages with a Retrofit cached page that doesn't expire.
Some references:
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • Mholloway | T136322 [Dev] Rename JsonPageLoadStrategy to PageDataClient | |||
| Resolved | None | T136321 [Dev] Transition JsonPageLoadStrategy caching to Retrofit and OkHttp | |||
| Resolved | Sharvaniharan | T148685 When a page is refreshed by the user and the network is unavailable, show a message to the user | |||
| Resolved | Dbrant | T149991 Link preview does not search for articles in cache | |||
| Resolved | • Niedzielski | T148675 Replace PageCache with Retrofit OkHttp caching | |||
| Invalid | None | T156829 Remove must-revalidate cache-control header from mobile-sections-lead and mobile-sections-remaining responses |
We think that PageCache isn't really doing a whole lot now and that it can be removed without a lot of impact and OkHttp will just kind of work without any extra code. We should also always try from network as the cache is tried under the covers by OkHttp.
As I mentioned in our meeting, we need to make sure we test also cases where we use the action API for the page content. This is important since there we don't get the etag headers. Retrofit/OkHttp won't use the If-None-Match headers if it doesn't get an etag from previous responses.
Change 316852 had a related patch set uploaded (by Niedzielski):
WIP: Replace PageCache with OkHttp caching
If you haven't already, it would be good to run through the problem scenario in T152720: Pages should be cached using the canonical title and see how Retrofit handles caching -- specifically whether it's sophisticated enough to recognize and appropriately handle the redirect.
I've been meaning to dig into the Retrofit documentation on caching but haven't got to it yet.
Change 334454 had a related patch set uploaded (by Niedzielski):
Update: allow OkHttp Cache to serve stale responses
Change 334454 merged by jenkins-bot:
Update: allow OkHttp Cache to serve stale responses