diff --git a/app.rb b/app.rb index 4746a3d..c5fd4ee 100644 --- a/app.rb +++ b/app.rb @@ -193,18 +193,17 @@ module Maptool get "/api/gpx/file/*" do begin - path = GPX_STORE.resolve_file(params[:splat].first) + body = GPX_STORE.geojson(params[:splat].first) rescue GpxStore::InvalidPath halt 400 - end - halt 404 unless File.file?(path) - - begin - geojson = path.downcase.end_with?(".fit") ? Fit.to_geojson(path) : Gpx.to_geojson(path) - json geojson + rescue Errno::ENOENT + halt 404 rescue Gpx::ParseError, Fit::ParseError halt 422, json(error: "could not parse file") end + + content_type :json + body end post "/api/gpx/folder" do diff --git a/docs/architecture.adoc b/docs/architecture.adoc index e15583f..ccc3061 100644 --- a/docs/architecture.adoc +++ b/docs/architecture.adoc @@ -251,7 +251,12 @@ Rendering:: Checking a file fetches `GET /api/gpx/file/`, routed by extension to `Maptool::Gpx` or `Maptool::Fit` (`lib/maptool/gpx.rb`/`fit.rb`) for parsing, and both convert to the same GeoJSON `FeatureCollection` shape so the frontend doesn't need to -know which format it got. `Gpx` parses the GPX with REXML: `` +know which format it got. `GpxStore#geojson` caches that conversion +alongside the source file (`.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: `` waypoints become `Point` features (with `name`, `desc` and `time` carried through as properties), each `` becomes one `MultiLineString` feature (one line per ``), and each `` diff --git a/lib/maptool/gpx_store.rb b/lib/maptool/gpx_store.rb index f037541..d50a0cf 100644 --- a/lib/maptool/gpx_store.rb +++ b/lib/maptool/gpx_store.rb @@ -1,4 +1,5 @@ require "fileutils" +require "json" require_relative "gpx" require_relative "fit" @@ -58,6 +59,7 @@ module Maptool raise Errno::ENOENT, path unless File.file?(path) File.delete(path) + File.delete(geojson_cache_path(path)) if File.file?(geojson_cache_path(path)) end def delete_folder(relative_path) @@ -79,10 +81,30 @@ module Maptool dest_dir = resolve_dir(to_folder.to_s) FileUtils.mkdir_p(dest_dir) name = unique_filename(dest_dir, File.basename(source)) - FileUtils.mv(source, File.join(dest_dir, name)) + dest = File.join(dest_dir, name) + FileUtils.mv(source, dest) + FileUtils.mv(geojson_cache_path(source), geojson_cache_path(dest)) if File.file?(geojson_cache_path(source)) to_folder.to_s.empty? ? name : "#{to_folder}/#{name}" end + # The GeoJSON conversion (Gpx.to_geojson/Fit.to_geojson) re-parses the + # whole file with REXML every call — fine for a small track, but a + # several-thousand-waypoint POI export (e.g. a park4night area export) + # can take tens of seconds, every single time the file is toggled + # visible. Cached alongside the source file and invalidated by mtime, + # so only the first read after a save (or edit) pays that cost. + def geojson(relative_path) + path = resolve_file(relative_path) + raise Errno::ENOENT, path unless File.file?(path) + + cache = geojson_cache_path(path) + if !File.file?(cache) || File.mtime(cache) < File.mtime(path) + data = path.downcase.end_with?(".fit") ? Fit.to_geojson(path) : Gpx.to_geojson(path) + File.write(cache, JSON.generate(data)) + end + File.read(cache) + end + private def resolve_dir(folder) @@ -96,6 +118,10 @@ module Maptool path == @root || path.start_with?(@root + File::SEPARATOR) end + def geojson_cache_path(path) + "#{path}.geojson" + end + TRACK_EXTENSIONS = %w[.gpx .fit].freeze def safe_filename(filename) diff --git a/test/test_gpx_store.rb b/test/test_gpx_store.rb index 5827dd9..0077256 100644 --- a/test/test_gpx_store.rb +++ b/test/test_gpx_store.rb @@ -67,4 +67,59 @@ class GpxStoreTest < Minitest::Test assert_equal "archive/ride-1.gpx", new_path assert_empty @store.tree["children"].reject { |c| c["name"] == "archive" } end + + def test_geojson_is_cached_alongside_the_source_file + 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. + File.write(cache_path, first.sub("waypoint", "cached-waypoint")) + assert_includes @store.geojson(path), "cached-waypoint" + 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) + + File.write(@store.resolve_file(path), sample_gpx.sub("Waypoint", "Renamed")) + FileUtils.touch(@store.resolve_file(path), mtime: Time.now + 1) + + assert_includes @store.geojson(path), "Renamed" + end + + def test_delete_file_removes_the_geojson_cache_too + path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx)) + @store.geojson(path) + cache_path = @store.resolve_file(path) + ".geojson" + + @store.delete_file(path) + + refute File.file?(cache_path) + end + + def test_move_relocates_the_geojson_cache_too + path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx)) + @store.geojson(path) + + new_path = @store.move(from: path, to_folder: "archive") + + assert File.file?(@store.resolve_file(new_path) + ".geojson") + end + + def test_geojson_raises_enoent_for_a_missing_file + assert_raises(Errno::ENOENT) { @store.geojson("nope.gpx") } + end + + private + + def sample_gpx + <<~GPX + Waypoint + GPX + end end