From dd272ffb8afec778faaac603ee155ae28edee8fb Mon Sep 17 00:00:00 2001
From: Roman Priesol <roman@priesol.net>
Date: Mon, 7 Sep 2026 22:33:29 +0200
Subject: [PATCH] Convert GPX/FIT to GeoJSON at upload time, not first view
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GpxStore#save now calls geojson() itself right after writing the
file, so the (potentially tens-of-seconds, for a large POI export)
conversion cost is paid during the upload — covered by the frontend's
existing "Uploading…" status — instead of on whichever checkbox
toggle happens to be first, which otherwise reads as a silent hang
indistinguishable from broken. An unparseable file still saves fine;
the error just surfaces on first view same as before.
---
 docs/architecture.adoc   | 10 ++++++++--
 lib/maptool/gpx_store.rb | 14 ++++++++++++++
 test/test_gpx_store.rb   | 18 ++++++++++++------
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/docs/architecture.adoc b/docs/architecture.adoc
index b8558b1..f11c7d9 100644
--- a/docs/architecture.adoc
+++ b/docs/architecture.adoc
@@ -269,8 +269,14 @@ know which format it got. `GpxStore#geojson` caches that conversion
 alongside the source file (`<path>.geojson`, invalidated by mtime) —
 REXML parsing a several-thousand-waypoint POI export (a park4night
 area export, say) can take tens of seconds, and without a cache that
-cost repeats on every single toggle rather than just the first one
-after a save. `Gpx` parses the GPX with REXML: `<wpt>`
+cost repeats on every single toggle. `GpxStore#save` also calls
+`geojson` itself right after writing the file, so that cost is paid
+once during the upload (covered by the frontend's own "Uploading…"
+status) instead of on whichever toggle happens to be first — a silent
+multi-second hang on a checkbox click reads as broken in a way the
+same wait during an upload doesn't. A file that fails to parse still
+saves fine either way; the error just surfaces on first view instead.
+`Gpx` parses the GPX with REXML: `<wpt>`
 waypoints become `Point` features (with `name`, `desc` and `time`
 carried through as properties), each `<trk>` becomes one
 `MultiLineString` feature (one line per `<trkseg>`), and each `<rte>`
diff --git a/lib/maptool/gpx_store.rb b/lib/maptool/gpx_store.rb
index d50a0cf..b319e71 100644
--- a/lib/maptool/gpx_store.rb
+++ b/lib/maptool/gpx_store.rb
@@ -30,12 +30,26 @@ module Maptool
     # Saves `io`'s contents as a .gpx/.fit file under `folder` (created if
     # needed), deriving a safe filename from `filename`. Returns the
     # stored file's path relative to the store root.
+    #
+    # Converts to GeoJSON and caches it right away (see `geojson` below)
+    # rather than waiting for the first time someone checks the file's box —
+    # a several-thousand-waypoint POI export is slow enough to parse that
+    # doing it lazily meant the *first* toggle silently hung (indistinguishable
+    # from broken), instead of the upload's own "Uploading…" status covering
+    # that wait like it already does for the upload itself. A file that fails
+    # to parse still saves fine — same as before, it just surfaces the error
+    # on first view instead of here.
     def save(folder:, filename:, io:)
       dir = resolve_dir(folder.to_s)
       FileUtils.mkdir_p(dir)
       name = unique_filename(dir, safe_filename(filename))
       File.binwrite(File.join(dir, name), io.read)
       relative = folder.to_s.empty? ? name : "#{folder}/#{name}"
+      begin
+        geojson(relative)
+      rescue Gpx::ParseError, Fit::ParseError
+        nil
+      end
       relative
     end
 
diff --git a/test/test_gpx_store.rb b/test/test_gpx_store.rb
index 0077256..6724d8d 100644
--- a/test/test_gpx_store.rb
+++ b/test/test_gpx_store.rb
@@ -68,20 +68,26 @@ class GpxStoreTest < Minitest::Test
     assert_empty @store.tree["children"].reject { |c| c["name"] == "archive" }
   end
 
-  def test_geojson_is_cached_alongside_the_source_file
+  def test_save_eagerly_caches_the_geojson_conversion
     path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx))
     cache_path = @store.resolve_file(path) + ".geojson"
-    refute File.file?(cache_path)
-
-    first = @store.geojson(path)
     assert File.file?(cache_path)
 
-    # Corrupt the cache directly to prove the second call reused it instead
-    # of re-parsing the (still-valid) source file.
+    first = @store.geojson(path)
+
+    # Corrupt the cache directly to prove the call above reused it instead of
+    # re-parsing the (still-valid) source file.
     File.write(cache_path, first.sub("waypoint", "cached-waypoint"))
     assert_includes @store.geojson(path), "cached-waypoint"
   end
 
+  def test_save_of_an_unparseable_file_still_succeeds
+    path = @store.save(folder: "", filename: "bad.gpx", io: StringIO.new("not gpx at all"))
+
+    refute File.file?(@store.resolve_file(path) + ".geojson")
+    assert_raises(Maptool::Gpx::ParseError) { @store.geojson(path) }
+  end
+
   def test_geojson_cache_is_invalidated_when_the_file_is_resaved
     path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx))
     @store.geojson(path)