Cache per-country Geofabrik extracts so Apply doesn't redownload everything

Every region-selection change re-downloaded every selected country's
.osm.pbf from scratch, even ones already fetched by a prior build —
painful for multi-GB countries. Extracts now persist in
data/geofabrik_cache/, keyed by country, and are only re-fetched when
Geofabrik's Last-Modified for that country actually moved on (checked
with a cheap HEAD) or the file isn't cached yet.

.gitignore

@@ -12,6 +12,10 @@
 # see docs/development.adoc)
 /data/*.pmtiles
 
+# Per-country .osm.pbf cache for offline.pmtiles rebuilds (also large,
+# also local-only — see PmtilesBuilder.cached_extract)
+/data/geofabrik_cache/
+
 # Offline Wikipedia ZIM for kiwix-serve (also large, also local-only —
 # same story, see docs/development.adoc)
 /data/kiwix/*

docs/development.adoc

@@ -99,10 +99,16 @@ There's no standalone per-country file to individually delete —
 the *whole current selection*. "Remove a country" and "refresh a
 country" are both just "change the selection, then Apply" (unticking a
 country and applying rebuilds without it; nothing to separately
-delete). Every Apply re-extracts the entire selection from scratch, not
-just what changed — fine given extraction is fast (byte-range requests
-against remote archives, not per-tile fetches), but worth revisiting if
-someone selects half of Europe at once and finds it slow.
+delete). terrain.pmtiles re-extracts its selection from scratch on
+every Apply — fine given that extraction is byte-range requests
+against a remote archive, not full downloads. offline.pmtiles, by
+contrast, downloads a whole country's `.osm.pbf` from Geofabrik (can be
+multi-GB), so it keeps a per-country cache under
+`data/geofabrik_cache/` instead of re-fetching on every Apply — a
+country already cached is only re-downloaded if Geofabrik's own
+`Last-Modified` for it has moved on since (checked with a cheap `HEAD`,
+see `PmtilesBuilder.cached_extract`), so growing or shrinking the
+selection only touches whatever's actually new to it.
 
 This wraps exactly the manual workflows documented above
 (`lib/maptool/pmtiles_builder.rb`). offline.pmtiles downloads each

lib/maptool/pmtiles_builder.rb

@@ -179,17 +179,17 @@ module Maptool
     # finishes — raises BuildError (the caller turns this into an error
     # response) if nothing's selected yet, or one's already running.
     #
-    # download_extract exists as an injectable seam so tests can exercise
-    # the rest of this (scratch dir, osmium/planetiler invocation, atomic
-    # rename) without a real network call — same reasoning test_helper.rb
-    # already gives for keeping /api/plugins route tests off the real
-    # Overpass API.
-    def self.build_offline(data_dir:, download_extract: method(:download_extract))
+    # download_extract/fetch_last_modified exist as injectable seams so
+    # tests can exercise the rest of this (scratch dir, osmium/planetiler
+    # invocation, atomic rename, the per-country cache below) without a
+    # real network call — same reasoning test_helper.rb already gives for
+    # keeping /api/plugins route tests off the real Overpass API.
+    def self.build_offline(data_dir:, download_extract: method(:download_extract), fetch_last_modified: method(:fetch_geofabrik_last_modified))
       codes = selected_regions(data_dir)
       raise BuildError, "select at least one region first" if codes.empty?
 
       begin_run!(OFFLINE_MUTEX, OFFLINE_STATUS)
-      Thread.new { run_job(OFFLINE_MUTEX, OFFLINE_STATUS) { extract_offline(codes, data_dir, download_extract) } }
+      Thread.new { run_job(OFFLINE_MUTEX, OFFLINE_STATUS) { extract_offline(codes, data_dir, download_extract, fetch_last_modified) } }
     end
 
     # fetch_manifest: same injectable-seam reasoning as download_extract
@@ -228,13 +228,11 @@ module Maptool
     # tiles.openfreemap.org's Liberty style already expects, so
     # public/js/map.js's offlineStyle() can reuse ONLINE_STYLE_PATCH
     # unchanged instead of a separate protomaps-themes-base palette.
-    def self.extract_offline(codes, data_dir, download_extract)
+    def self.extract_offline(codes, data_dir, download_extract, fetch_last_modified)
       scratch_dir = Dir.mktmpdir("maptool-offline")
       sources = codes.map do |code|
         slug = GEOFABRIK_SLUGS.fetch(code) { raise BuildError, "no Geofabrik source known for region #{code}" }
-        dest = File.join(scratch_dir, "#{code}.osm.pbf")
-        last_modified = download_extract.call(geofabrik_url(slug), dest)
-        { "code" => code, "slug" => slug, "last_modified" => last_modified, "path" => dest }
+        cached_extract(code, slug, data_dir, download_extract, fetch_last_modified)
       end
 
       osm_path =
@@ -261,6 +259,41 @@ module Maptool
     end
     private_class_method :geofabrik_url
 
+    # Per-country .osm.pbf cache, kept across builds under data_dir (not
+    # scratch_dir, which is wiped every run) — changing the region
+    # selection re-downloads only the countries that are new to it, and
+    # re-running the same selection re-downloads nothing unless Geofabrik's
+    # own Last-Modified for that country moved on. A stamp file next to
+    # each cached extract records the Last-Modified it was fetched with;
+    # a HEAD check (cheap) decides whether the multi-GB GET (not cheap) is
+    # skippable.
+    def self.cached_extract(code, slug, data_dir, download_extract, fetch_last_modified)
+      cache_dir = geofabrik_cache_dir(data_dir)
+      FileUtils.mkdir_p(cache_dir)
+      path = File.join(cache_dir, "#{code}.osm.pbf")
+      stamp_path = "#{path}.last_modified"
+      url = geofabrik_url(slug)
+
+      if File.exist?(path) && File.exist?(stamp_path)
+        current = begin
+          fetch_last_modified.call(url)
+        rescue BuildError
+          nil
+        end
+        return { "code" => code, "slug" => slug, "last_modified" => current, "path" => path } if current && current == File.read(stamp_path)
+      end
+
+      last_modified = download_extract.call(url, path)
+      File.write(stamp_path, last_modified.to_s)
+      { "code" => code, "slug" => slug, "last_modified" => last_modified, "path" => path }
+    end
+    private_class_method :cached_extract
+
+    def self.geofabrik_cache_dir(data_dir)
+      File.join(data_dir, "geofabrik_cache")
+    end
+    private_class_method :geofabrik_cache_dir
+
     # Real network implementation of the download_extract seam above —
     # streams the extract to dest (some of these are multi-GB — Net::HTTP's
     # plain #get buffers the whole response body in memory first, which

test/test_pmtiles_builder.rb

@@ -102,8 +102,12 @@ class PmtilesBuilderTest < Minitest::Test
   # download_extract stub: writes a marker file (same "fake" content the
   # pmtiles/osmium/java fixtures use) instead of hitting Geofabrik, and
   # returns a fixed Last-Modified so sidecar-meta assertions are stable.
-  def fake_download(last_modified: "Wed, 10 Sep 2026 08:00:01 GMT")
-    ->(_url, dest) {
+  # `calls` (an Array, shared across builds if the caller wants it) records
+  # every URL actually downloaded, for asserting the per-country cache
+  # skipped (or didn't skip) a re-download.
+  def fake_download(last_modified: "Wed, 10 Sep 2026 08:00:01 GMT", calls: nil)
+    ->(url, dest) {
+      calls << url if calls
       File.write(dest, "fake")
       last_modified
     }
@@ -189,6 +193,73 @@ class PmtilesBuilderTest < Minitest::Test
     assert_equal %w[SK AT], codes
   end
 
+  # The actual bug report this covers: hitting "Apply" after tweaking the
+  # region selection used to re-download every selected country's .osm.pbf
+  # from scratch, even ones already fetched by an earlier build.
+  def test_build_offline_skips_redownloading_an_unchanged_cached_country
+    select_regions("SK")
+    calls = []
+    Maptool::PmtilesBuilder.build_offline(data_dir: @data_dir, download_extract: fake_download(calls: calls)).join
+    assert_equal 1, calls.size
+
+    Maptool::PmtilesBuilder.build_offline(
+      data_dir: @data_dir,
+      download_extract: fake_download(calls: calls),
+      fetch_last_modified: ->(_url) { "Wed, 10 Sep 2026 08:00:01 GMT" },
+    ).join
+
+    assert_equal 1, calls.size
+    assert_equal "done", Maptool::PmtilesBuilder.offline_status[:state]
+  end
+
+  def test_build_offline_redownloads_a_country_whose_extract_changed
+    select_regions("SK")
+    calls = []
+    Maptool::PmtilesBuilder.build_offline(data_dir: @data_dir, download_extract: fake_download(calls: calls)).join
+
+    Maptool::PmtilesBuilder.build_offline(
+      data_dir: @data_dir,
+      download_extract: fake_download(calls: calls, last_modified: "Thu, 11 Sep 2026 08:00:01 GMT"),
+      fetch_last_modified: ->(_url) { "Thu, 11 Sep 2026 08:00:01 GMT" },
+    ).join
+
+    assert_equal 2, calls.size
+  end
+
+  # A cache-freshness check that itself fails (Geofabrik unreachable) should
+  # fall back to re-downloading rather than aborting the build.
+  def test_build_offline_redownloads_when_the_cache_check_fails
+    select_regions("SK")
+    calls = []
+    Maptool::PmtilesBuilder.build_offline(data_dir: @data_dir, download_extract: fake_download(calls: calls)).join
+
+    fetch_last_modified = ->(_url) { raise Maptool::PmtilesBuilder::BuildError, "could not reach geofabrik" }
+    Maptool::PmtilesBuilder.build_offline(
+      data_dir: @data_dir,
+      download_extract: fake_download(calls: calls),
+      fetch_last_modified: fetch_last_modified,
+    ).join
+
+    assert_equal 2, calls.size
+    assert_equal "done", Maptool::PmtilesBuilder.offline_status[:state]
+  end
+
+  def test_build_offline_only_downloads_a_newly_added_country
+    select_regions("SK")
+    calls = []
+    Maptool::PmtilesBuilder.build_offline(data_dir: @data_dir, download_extract: fake_download(calls: calls)).join
+
+    select_regions("SK", "AT")
+    Maptool::PmtilesBuilder.build_offline(
+      data_dir: @data_dir,
+      download_extract: fake_download(calls: calls),
+      fetch_last_modified: ->(_url) { "Wed, 10 Sep 2026 08:00:01 GMT" },
+    ).join
+
+    assert_equal 1, calls.count { |url| url.include?("slovakia") }
+    assert_equal 1, calls.count { |url| url.include?("austria") }
+  end
+
   def test_offline_meta_is_nil_before_any_build
     assert_nil Maptool::PmtilesBuilder.offline_meta(@data_dir)
   end