Download blocklists conditionally and skip the reload when unchanged - #905
Merged
Conversation
The daily hosts update fetched every enabled blocklist in full, merged them and called ServiceSinkhole.reload, which on the native restart path closes every live connection, whether or not anything had changed. Store each list's ETag and Last-Modified and send If-None-Match and If-Modified-Since when the list's file already exists. A 304 leaves the file alone and counts as success; a missing file is always fetched unconditionally so a stale validator can never leave the app without a list. When every list came back unchanged and the merged hosts file exists, record the check time and return without merging or reloading. The periodic request now also requires the battery not to be low. The manual download keeps its constraints, since the user asked for it. Part of #884 (item 6). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSM3uYXrgB2Vs2ff9ZVkam
kasnder
marked this pull request as ready for review
September 3, 2026 06:01
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9c6b0d5c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two follow-ups from review of the conditional download. A run that replaced a list file but died before the merge, or whose merge failed while an old merged file was still present, left the new validators persisted. Every later run then answered 304 for each list and treated the existing hosts.txt as current, so blocking stayed stale until an upstream list happened to change. Record a durable merge-pending flag as soon as a list file is replaced, clear it only once the merge succeeds, and require it to be clear before skipping the merge. The battery constraint reached only users who toggled the setting after upgrading, since the periodic work is enqueued from the preference listener alone. Move the scheduling into the worker and reconcile it once at startup, keyed to a schedule version to bump whenever the constraints or the period change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SSM3uYXrgB2Vs2ff9ZVkam
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #884 (item 6).
What changed
The daily
HostsUpdatework fetched every enabled blocklist in full, merged them and calledServiceSinkhole.reload, which on the native restart path closes every live connection, whether or not anything had changed.BlocklistgainsetagandlastModifiedHeader, persisted byBlocklistManageralongside the existing fields (read with a null fallback, so stored JSON from older versions keeps working).HostsDownloadWorkersendsIf-None-MatchandIf-Modified-Since. A 304 leaves the file alone and counts as success. A 200 downloads as before and stores the response validators, clearing them if the server stopped sending them. A missing file is always fetched unconditionally, so a stale validator can never leave the app without a list.hosts.txtexists. The last-download preference is still updated so the settings screen shows the check. Any change, or a missing merged file, takes the existing merge and reload path.The per-list download logic is extracted into a package-private static
download(...)seam that takes an already-openedHttpURLConnection, so it is testable without WorkManager or a network. The mid-copyisStopped()check now throws a privateStopRequestedExceptionthatdoWorkturns intoResult.failure(), keeping the same cleanup infinally.Tests
HostsDownloadWorkerTest(plain JUnit, fakeHttpURLConnection): 304 with a stored ETag leaves the file untouched and marks success; a missing target sends no conditional headers even with stored validators; 200 replaces the file and stores the validators; 200 without validator headers clears stale ones.Device check worth doing
Run the manual download twice in a row against the default StevenBlack list and watch
logcat -s TrackerControl.Hosts: the second run should logHosts unchanged, skipping merge and reloadand no VPN reload should follow.🤖 Generated with Claude Code
https://claude.ai/code/session_01SSM3uYXrgB2Vs2ff9ZVkam
Generated by Claude Code