Skip to content

Download blocklists conditionally and skip the reload when unchanged - #905

Merged
kasnder merged 2 commits into
masterfrom
claude/open-issues-triage-0wwibn-hosts
Sep 3, 2026
Merged

Download blocklists conditionally and skip the reload when unchanged#905
kasnder merged 2 commits into
masterfrom
claude/open-issues-triage-0wwibn-hosts

Conversation

@kasnder

@kasnder kasnder commented Sep 3, 2026

Copy link
Copy Markdown
Member

Part of #884 (item 6).

What changed

The daily HostsUpdate work 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.

  • Validators per list: Blocklist gains etag and lastModifiedHeader, persisted by BlocklistManager alongside the existing fields (read with a null fallback, so stored JSON from older versions keeps working).
  • Conditional request: when the list's file already exists, HostsDownloadWorker sends If-None-Match and If-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.
  • Skip the merge and reload when every successful list came back unchanged and the merged hosts.txt exists. 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.
  • Battery constraint: the 24-hour periodic request now also requires the battery not to be low. The manual download keeps its constraints, since the user asked for it.

The per-list download logic is extracted into a package-private static download(...) seam that takes an already-opened HttpURLConnection, so it is testable without WorkManager or a network. The mid-copy isStopped() check now throws a private StopRequestedException that doWork turns into Result.failure(), keeping the same cleanup in finally.

Tests

  • New HostsDownloadWorkerTest (plain JUnit, fake HttpURLConnection): 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.
  • Not run locally (no Android SDK in this container); CI runs the suite.

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 log Hosts unchanged, skipping merge and reload and no VPN reload should follow.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SSM3uYXrgB2Vs2ff9ZVkam


Generated by Claude Code

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
kasnder marked this pull request as ready for review September 3, 2026 06:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-03T06:03:51.782615Z e9c6b0d Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread app/src/main/java/eu/faircode/netguard/HostsDownloadWorker.java Outdated
Comment thread app/src/main/java/eu/faircode/netguard/ActivitySettings.java Outdated
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
@kasnder
kasnder merged commit ff8f3bc into master Sep 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants