diff --git a/Cargo.lock b/Cargo.lock index d8c019f5ad..336e62147a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2095,6 +2095,7 @@ dependencies = [ "abstutil", "anyhow", "contour", + "flate2", "fs-err", "geo", "geojson", @@ -2107,6 +2108,7 @@ dependencies = [ "map_model", "maplit", "raw_map", + "regex", "serde", "serde_json", "structopt", diff --git a/abstio/src/abst_paths.rs b/abstio/src/abst_paths.rs index 655b161188..d933001263 100644 --- a/abstio/src/abst_paths.rs +++ b/abstio/src/abst_paths.rs @@ -398,7 +398,7 @@ pub fn path_all_edits(name: &MapName) -> String { pub fn path_ltn_proposals(name: &MapName, proposal_name: &str) -> String { path(format!( - "player/ltn_proposals/{}/{}/{}/{}.bin", + "player/ltn_proposals/{}/{}/{}/{}.json.gz", name.city.country, name.city.city, name.map, proposal_name )) } diff --git a/abstio/src/io_native.rs b/abstio/src/io_native.rs index 04aafc876d..a129ae8890 100644 --- a/abstio/src/io_native.rs +++ b/abstio/src/io_native.rs @@ -95,6 +95,12 @@ pub fn write_binary(path: String, obj: &T) { info!("Wrote {}", path); } +pub fn write_raw(path: String, bytes: &[u8]) -> Result<()> { + let mut file = BufWriter::new(File::create(path)?); + file.write_all(bytes)?; + Ok(()) +} + /// Idempotent pub fn delete_file>(path: I) { let path = path.as_ref(); diff --git a/abstio/src/io_web.rs b/abstio/src/io_web.rs index 9f965b2eae..19b181a4b3 100644 --- a/abstio/src/io_web.rs +++ b/abstio/src/io_web.rs @@ -82,17 +82,18 @@ pub fn list_dir(dir: String) -> Vec { pub fn slurp_file>(path: I) -> Result> { let path = path.as_ref(); - debug!( - "slurping file: {}, trimmed_path: {}", - path, - path.trim_start_matches("../data/system/") - ); if let Some(raw) = SYSTEM_DATA.get_file(path.trim_start_matches("../data/system/")) { Ok(raw.contents().to_vec()) } else if path.starts_with(&path_player("")) { let string = read_local_storage(path)?; - Ok(string.into_bytes()) + // TODO Hack: if it probably wasn't written with write_json, do the base64 decoding. This + // may not always be appropriate... + if path.ends_with(".json") { + Ok(string.into_bytes()) + } else { + base64::decode(string).map_err(|err| err.into()) + } } else { bail!("Can't slurp_file {}, it doesn't exist", path) } @@ -122,17 +123,23 @@ pub fn write_json(path: String, obj: &T) { } pub fn write_binary(path: String, obj: &T) { + write_raw(path, &abstutil::to_binary(obj)).unwrap(); +} + +pub fn write_raw(path: String, bytes: &[u8]) -> Result<()> { // Only save for data/player, for now if !path.starts_with(&path_player("")) { - warn!("Not saving {}", path); - return; + bail!("Not saving {}", path); } let window = web_sys::window().unwrap(); let storage = window.local_storage().unwrap().unwrap(); // Local storage only supports strings, so base64 encoding needed - let encoded = base64::encode(abstutil::to_binary(obj)); - storage.set_item(&path, &encoded).unwrap(); + let encoded = base64::encode(bytes); + storage + .set_item(&path, &encoded) + .map_err(|err| anyhow!(err.as_string().unwrap_or("set_item failed".to_string())))?; + Ok(()) } pub fn delete_file>(path: I) { diff --git a/apps/ltn/Cargo.toml b/apps/ltn/Cargo.toml index 57f6b826a1..0fd721c3bc 100644 --- a/apps/ltn/Cargo.toml +++ b/apps/ltn/Cargo.toml @@ -16,6 +16,7 @@ abstio = { path = "../../abstio" } abstutil = { path = "../../abstutil" } anyhow = "1.0.38" contour = "0.4.0" +flate2 = "1.0.20" fs-err = "2.6.0" geo = "0.19" geojson = { version = "0.22.2", features = ["geo-types"] } @@ -28,6 +29,7 @@ maplit = "1.0.2" map_gui = { path = "../../map_gui" } map_model = { path = "../../map_model" } raw_map = { path = "../../raw_map" } +regex = "1.5.4" serde = "1.0.123" serde_json = "1.0.61" synthpop = { path = "../../synthpop" } diff --git a/apps/ltn/src/filters/mod.rs b/apps/ltn/src/filters/mod.rs index 9834b105cc..83f85d614a 100644 --- a/apps/ltn/src/filters/mod.rs +++ b/apps/ltn/src/filters/mod.rs @@ -5,6 +5,7 @@ use std::collections::{BTreeMap, BTreeSet}; use serde::{Deserialize, Serialize}; +use abstutil::{deserialize_btreemap, serialize_btreemap}; use geom::{Circle, Distance, Line}; use map_model::{IntersectionID, Map, RoadID, RoutingParams, TurnID}; use widgetry::mapspace::{DrawUnzoomedShapes, ToggleZoomed}; @@ -16,8 +17,17 @@ use crate::{after_edit, colors, App}; /// Stored in App session state. Before making any changes, call `before_edit`. #[derive(Clone, Default, Serialize, Deserialize)] pub struct ModalFilters { + // We use serialize_btreemap so that save::perma can detect and transform IDs /// For filters placed along a road, where is the filter located? + #[serde( + serialize_with = "serialize_btreemap", + deserialize_with = "deserialize_btreemap" + )] pub roads: BTreeMap, + #[serde( + serialize_with = "serialize_btreemap", + deserialize_with = "deserialize_btreemap" + )] pub intersections: BTreeMap, /// Edit history is preserved recursively diff --git a/apps/ltn/src/save.rs b/apps/ltn/src/save/mod.rs similarity index 80% rename from apps/ltn/src/save.rs rename to apps/ltn/src/save/mod.rs index 18ec023dd3..17ace948ee 100644 --- a/apps/ltn/src/save.rs +++ b/apps/ltn/src/save/mod.rs @@ -1,8 +1,10 @@ +mod perma; + use anyhow::Result; use serde::{Deserialize, Serialize}; use abstio::MapName; -use abstutil::{Counter, Timer}; +use abstutil::Counter; use map_gui::tools::{ChooseSomething, PromptInput}; use map_model::PathRequest; use widgetry::tools::PopupMsg; @@ -11,11 +13,8 @@ use widgetry::{Choice, EventCtx, Key, Line, State, Widget}; use crate::partition::BlockID; use crate::{App, BrowseNeighborhoods, ModalFilters, Partitioning, Transition}; -/// Captures all of the edits somebody makes to a map in the LTN tool. Note this separate from +/// Captures all of the edits somebody makes to a map in the LTN tool. Note this is separate from /// `map_model::MapEdits`. -/// -/// TODO Note this format isn't future-proof at all. Changes to the LTN blockfinding algorithm or -/// map data (like RoadIDs) will probably break someone's edits. #[derive(Serialize, Deserialize)] pub struct Proposal { pub map: MapName, @@ -51,22 +50,27 @@ impl Proposal { /// Try to load a proposal. If it fails, returns a popup message state. pub fn load(ctx: &mut EventCtx, app: &mut App, name: &str) -> Option>> { - ctx.loading_screen( - "load existing proposal", - |ctx, mut timer| match Self::inner_load(ctx, app, name, &mut timer) { - Ok(()) => None, - Err(err) => Some(PopupMsg::new_state( - ctx, - "Error", - vec![format!("Couldn't load proposal {}", name), err.to_string()], - )), - }, - ) + match Self::inner_load(ctx, app, name) { + Ok(()) => None, + Err(err) => Some(PopupMsg::new_state( + ctx, + "Error", + vec![ + format!("Couldn't load proposal {}", name), + err.to_string(), + "The format of saved proposals recently changed.".to_string(), + "Contact dabreegster@gmail.com if you need help restoring a file.".to_string(), + ], + )), + } } - fn inner_load(ctx: &mut EventCtx, app: &mut App, name: &str, timer: &mut Timer) -> Result<()> { - let proposal: Proposal = - abstio::maybe_read_binary(abstio::path_ltn_proposals(app.map.get_name(), name), timer)?; + fn inner_load(ctx: &mut EventCtx, app: &mut App, name: &str) -> Result<()> { + let bytes = abstio::slurp_file(abstio::path_ltn_proposals(app.map.get_name(), name))?; + let decoder = flate2::read::GzDecoder::new(&bytes[..]); + let value = serde_json::from_reader(decoder)?; + let proposal = perma::from_permanent(&app.map, value)?; + // TODO We could try to detect if the file's partitioning (road IDs and such) still matches // this version of the map or not @@ -125,16 +129,35 @@ fn save_ui(ctx: &mut EventCtx, app: &App, preserve_state: PreserveState) -> Box< // and file state are not synchronized / auto-saved. app.session.proposal_name = Some(name.clone()); - let path = abstio::path_ltn_proposals(app.map.get_name(), &name); - let proposal = Proposal::from_app(app); - abstio::write_binary(path, &proposal); - - // If we changed the name, we'll want to recreate the panel - preserve_state.switch_to_state(ctx, app) + match inner_save(app) { + // If we changed the name, we'll want to recreate the panel + Ok(()) => preserve_state.switch_to_state(ctx, app), + Err(err) => Transition::Multi(vec![ + preserve_state.switch_to_state(ctx, app), + Transition::Push(PopupMsg::new_state( + ctx, + "Error", + vec![format!("Couldn't save proposal: {}", err)], + )), + ]), + } }), ) } +fn inner_save(app: &App) -> Result<()> { + let proposal = Proposal::from_app(app); + let path = abstio::path_ltn_proposals(app.map.get_name(), &proposal.name); + + let json_value = perma::to_permanent(&app.map, &proposal)?; + let mut output_buffer = Vec::new(); + let mut encoder = + flate2::write::GzEncoder::new(&mut output_buffer, flate2::Compression::best()); + serde_json::to_writer(&mut encoder, &json_value)?; + encoder.finish()?; + abstio::write_raw(path, &output_buffer) +} + fn load_picker_ui( ctx: &mut EventCtx, app: &App, @@ -145,9 +168,15 @@ fn load_picker_ui( ChooseSomething::new_state( ctx, "Load which proposal?", - Choice::strings(abstio::list_all_objects(abstio::path_all_ltn_proposals( - app.map.get_name(), - ))), + // basename (and thus list_all_objects) turn "foo.json.gz" into "foo.json", so further + // strip out the extension. + // TODO Fix basename, but make sure nothing downstream breaks + Choice::strings( + abstio::list_all_objects(abstio::path_all_ltn_proposals(app.map.get_name())) + .into_iter() + .map(abstutil::basename) + .collect(), + ), Box::new(|name, ctx, app| match Proposal::load(ctx, app, &name) { Some(err_state) => Transition::Replace(err_state), None => preserve_state.switch_to_state(ctx, app), diff --git a/apps/ltn/src/save/perma.rs b/apps/ltn/src/save/perma.rs new file mode 100644 index 0000000000..5aa2b128f8 --- /dev/null +++ b/apps/ltn/src/save/perma.rs @@ -0,0 +1,128 @@ +//! The Proposal struct references IntersectionIDs and RoadIDs, which won't survive OSM updates. +//! Similar to the MapEdits <-> PermanentMapEdits strategy, transform those IDs before saving. +//! +//! Unlike PermanentMapEdits, we don't define a PermanentProposal struct, because to do so for +//! everything it includes would be a nightmare. In particular, Partitioning includes Blocks, which +//! nest RoadIDs deep inside. Instead, play a "runtime reflection" trick: +//! +//! 1) Serialize the Proposal with RoadIDs to JSON +//! 2) Dynamically walk the JSON +//! 3) When the path of a value matches the hardcoded list of patterns in is_road_id and +//! is_intersection_id, transform to a permanent ID +//! 4) Save the proposal as JSON with that ID instead +//! 5) Do the inverse to later load +//! +//! In practice, this attempt to keep proposals compatible with future basemap updates might be +//! futile. We're embedding loads of details about the partitioning, but not checking that they +//! remain valid after loading. Even splitting one road in two anywhere in the map would likely +//! break things kind of silently. Absolute worst case, we also record an abst_version field so we +//! could manually load the proposal in the correct version, and do something to manually recover +//! an old proposal. +//! +//! Also, the JSON blobs are massive because of the partitioning, so compress everything. + +use anyhow::Result; +use lazy_static::lazy_static; +use regex::Regex; +use serde_json::Value; + +use map_model::{IntersectionID, Map, RoadID}; +use raw_map::osm::NodeID; +use raw_map::OriginalRoad; + +use super::Proposal; + +pub fn to_permanent(map: &Map, proposal: &Proposal) -> Result { + let mut proposal_value = serde_json::to_value(proposal)?; + walk("", &mut proposal_value, &|path, value| { + if is_road_id(path) { + let replace_with = map.get_r(RoadID(value.as_u64().unwrap() as usize)).orig_id; + *value = serde_json::to_value(&replace_with)?; + } else if is_intersection_id(path) { + let replace_with = map + .get_i(IntersectionID(value.as_u64().unwrap() as usize)) + .orig_id; + *value = serde_json::to_value(&replace_with)?; + } + Ok(()) + })?; + Ok(proposal_value) +} + +pub fn from_permanent(map: &Map, mut proposal_value: Value) -> Result { + walk("", &mut proposal_value, &|path, value| { + if is_road_id(path) { + let orig_id: OriginalRoad = serde_json::from_value(value.clone())?; + let replace_with = map.find_r_by_osm_id(orig_id)?; + *value = serde_json::to_value(&replace_with)?; + } else if is_intersection_id(path) { + let orig_id: NodeID = serde_json::from_value(value.clone())?; + let replace_with = map.find_i_by_osm_id(orig_id)?; + *value = serde_json::to_value(&replace_with)?; + } + Ok(()) + })?; + let result = serde_json::from_value(proposal_value)?; + Ok(result) +} + +fn is_road_id(path: &str) -> bool { + lazy_static! { + static ref PATTERNS: Vec = vec![ + Regex::new(r"^/modal_filters/roads/\d+/0$").unwrap(), + Regex::new(r"^/modal_filters/intersections/\d+/1/r1$").unwrap(), + Regex::new(r"^/modal_filters/intersections/\d+/1/r2$").unwrap(), + Regex::new(r"^/modal_filters/intersections/\d+/1/group1/y$").unwrap(), + Regex::new(r"^/modal_filters/intersections/\d+/1/group2/y$").unwrap(), + // First place a Block is stored + Regex::new(r"^/partitioning/single_blocks/\d+/perimeter/interior/\d+$").unwrap(), + Regex::new(r"^/partitioning/single_blocks/\d+/perimeter/roads/\d+/road$").unwrap(), + // The other + Regex::new(r"^/partitioning/neighborhoods/\d+/0/perimeter/interior/\d+$").unwrap(), + Regex::new(r"^/partitioning/neighborhoods/\d+/0/perimeter/roads/\d+/road$").unwrap(), + ]; + } + + PATTERNS.iter().any(|re| re.is_match(path)) +} + +fn is_intersection_id(path: &str) -> bool { + lazy_static! { + static ref PATTERNS: Vec = vec![ + Regex::new(r"^/modal_filters/intersections/\d+/0$").unwrap(), + Regex::new(r"^/modal_filters/intersections/\d+/1/i$").unwrap(), + ]; + } + + PATTERNS.iter().any(|re| re.is_match(path)) +} + +// Note there's no chance to transform keys in a map. So use serialize_btreemap elsewhere to force +// into a list of pairs +fn walk Result<()>>( + path: &str, + value: &mut Value, + transform: &F, +) -> Result<()> { + match value { + Value::Array(list) => { + for (idx, x) in list.into_iter().enumerate() { + walk(&format!("{}/{}", path, idx), x, transform)?; + } + transform(path, value)?; + } + Value::Object(map) => { + for (key, val) in map { + walk(&format!("{}/{}", path, key), val, transform)?; + } + // After recursing, possibly transform this. We turn a number into an object, so to + // reverse that... + transform(path, value)?; + } + _ => { + transform(path, value)?; + // The value may have been transformed into an array or object, but don't walk it. + } + } + Ok(()) +} diff --git a/map_model/src/edits/mod.rs b/map_model/src/edits/mod.rs index 3ce40e04fa..84cf41ad5c 100644 --- a/map_model/src/edits/mod.rs +++ b/map_model/src/edits/mod.rs @@ -225,8 +225,7 @@ impl MapEdits { Err(_) => { // The JSON format may have changed, so attempt backwards compatibility. let bytes = abstio::slurp_file(path)?; - let contents = std::str::from_utf8(&bytes)?; - let value = serde_json::from_str(contents)?; + let value = serde_json::from_slice(&bytes)?; compat::upgrade(value, map)? } }; diff --git a/tests/src/main.rs b/tests/src/main.rs index b4b42b1140..d99677c0b1 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -354,7 +354,11 @@ fn bus_route_test() -> Result<()> { )); let mut f = File::create(path)?; for tr in map.all_transit_routes() { - writeln!(f, "{} ({}) from {} to {:?}", tr.gtfs_id, tr.short_name, tr.start, tr.end_border)?; + writeln!( + f, + "{} ({}) from {} to {:?}", + tr.gtfs_id, tr.short_name, tr.start, tr.end_border + )?; for ts in &tr.stops { let ts = map.get_ts(*ts); writeln!(