Convert GPX/FIT to GeoJSON at upload time, not first view
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.
This commit is contained in:
1 parent
96065b1237
commit
dd272ffb8a
3 files changed
+34
-8
No files matched your search
@@ -269,8 +269,14 @@ know which format it got. `GpxStore#geojson` caches that conversion
|
|||||||
alongside the source file (`<path>.geojson`, invalidated by mtime) —
|
alongside the source file (`<path>.geojson`, invalidated by mtime) —
|
||||||
REXML parsing a several-thousand-waypoint POI export (a park4night
|
REXML parsing a several-thousand-waypoint POI export (a park4night
|
||||||
area export, say) can take tens of seconds, and without a cache that
|
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
|
cost repeats on every single toggle. `GpxStore#save` also calls
|
||||||
after a save. `Gpx` parses the GPX with REXML: `<wpt>`
|
`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`
|
waypoints become `Point` features (with `name`, `desc` and `time`
|
||||||
carried through as properties), each `<trk>` becomes one
|
carried through as properties), each `<trk>` becomes one
|
||||||
`MultiLineString` feature (one line per `<trkseg>`), and each `<rte>`
|
`MultiLineString` feature (one line per `<trkseg>`), and each `<rte>`
|
||||||
|
|||||||
@@ -30,12 +30,26 @@ module Maptool
|
|||||||
# Saves `io`'s contents as a .gpx/.fit file under `folder` (created if
|
# Saves `io`'s contents as a .gpx/.fit file under `folder` (created if
|
||||||
# needed), deriving a safe filename from `filename`. Returns the
|
# needed), deriving a safe filename from `filename`. Returns the
|
||||||
# stored file's path relative to the store root.
|
# 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:)
|
def save(folder:, filename:, io:)
|
||||||
dir = resolve_dir(folder.to_s)
|
dir = resolve_dir(folder.to_s)
|
||||||
FileUtils.mkdir_p(dir)
|
FileUtils.mkdir_p(dir)
|
||||||
name = unique_filename(dir, safe_filename(filename))
|
name = unique_filename(dir, safe_filename(filename))
|
||||||
File.binwrite(File.join(dir, name), io.read)
|
File.binwrite(File.join(dir, name), io.read)
|
||||||
relative = folder.to_s.empty? ? name : "#{folder}/#{name}"
|
relative = folder.to_s.empty? ? name : "#{folder}/#{name}"
|
||||||
|
begin
|
||||||
|
geojson(relative)
|
||||||
|
rescue Gpx::ParseError, Fit::ParseError
|
||||||
|
nil
|
||||||
|
end
|
||||||
relative
|
relative
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
+12
-6
@@ -68,20 +68,26 @@ class GpxStoreTest < Minitest::Test
|
|||||||
assert_empty @store.tree["children"].reject { |c| c["name"] == "archive" }
|
assert_empty @store.tree["children"].reject { |c| c["name"] == "archive" }
|
||||||
end
|
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))
|
path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx))
|
||||||
cache_path = @store.resolve_file(path) + ".geojson"
|
cache_path = @store.resolve_file(path) + ".geojson"
|
||||||
refute File.file?(cache_path)
|
|
||||||
|
|
||||||
first = @store.geojson(path)
|
|
||||||
assert File.file?(cache_path)
|
assert File.file?(cache_path)
|
||||||
|
|
||||||
# Corrupt the cache directly to prove the second call reused it instead
|
first = @store.geojson(path)
|
||||||
# of re-parsing the (still-valid) source file.
|
|
||||||
|
# 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"))
|
File.write(cache_path, first.sub("waypoint", "cached-waypoint"))
|
||||||
assert_includes @store.geojson(path), "cached-waypoint"
|
assert_includes @store.geojson(path), "cached-waypoint"
|
||||||
end
|
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
|
def test_geojson_cache_is_invalidated_when_the_file_is_resaved
|
||||||
path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx))
|
path = @store.save(folder: "", filename: "ride.gpx", io: StringIO.new(sample_gpx))
|
||||||
@store.geojson(path)
|
@store.geojson(path)
|
||||||
|
|||||||
Reference in new issue
Block a user