Background
HookHandler::onSkinTemplateNavigation__Universal() currently performs a central-ID lookup and calls ReadingListRepository::getDefaultListIdForUser(). (it also happens before checking if the page view is eligible to have the bookmark button)
We no longer do anything with the default list id other than using it to gate the bloom filter bookmark lookup. It would be better to consolidate this and only have the bloom filter bookmark lookup, and have it handle the case of a user who never interacted with ReadingLists in a more optimal way.
For the bloom filter lookup, on cache miss, we could first perform the default list DB lookup, which uses the existing rl_user_default index:
- If the user has no active default list, store a compact EMPTY state in the existing per-project Bloom cache and return a definite negative without performing the exact page lookup or enqueueing a rebuild.
- If the user has an active default list, then do the existing bloom filter lookup.
Technical notes
- It is safe to introduce the EMPTY state for the bloom filter cache, without bumping the cache version
Acceptance criteria
- Check the bloom filter cache first. If there is a Bloom filter or EMPTY result, then do not check if the user has a default list.
- If a bloom filter cache miss, then check if the user has a default list.
- If no default list, store EMPTY and skip the exact page lookup and Bloom rebuild.
- If the user has a default list, then continue with the bloom filter rebuild and fallback bookmark lookup via DB.
- A rebuild with no saved pages stores EMPTY, and this is invalidated when a user saves their first page. (this should already be what happens)
- EMPTY fills and EMPTY cache hits are visible in the existing bloom filter metrics (new result/reason labels), and the dashboard is updated.
QA
- With a new account that never used ReadingLists, on first page view, a cache entry with EMPTY should be saved, and subsequently the the page save / unsave flow works and the user has a bloom filter cached.
- Immediately after saving the first page, subsequent page views show the saved state (a stale EMPTY is not served).
- For users with pages saved, that the bloom filter cache and rebuild flow works the same as before.
- For users with a default list but no pages saved in any reading list, then they should also get a cache entry with EMPTY.