From 77929cfa0499b6168cf72d9346d24c98c91e4517 Mon Sep 17 00:00:00 2001 From: vcoppe Date: Fri, 9 Oct 2026 17:55:07 +0200 Subject: [PATCH] per-chunk categories storage --- gpx-rs/engine/src/core/gpx/categories.rs | 7 - gpx-rs/engine/src/storage/codec/mod.rs | 212 ++++++++++++++++------- gpx-rs/engine/src/storage/codec/v1.rs | 106 ++++++++++-- gpx-rs/engine/src/storage/memory.rs | 16 +- gpx-rs/engine/src/storage/mod.rs | 12 +- gpx-rs/engine/src/storage/persister.rs | 101 ++++++++--- gpx-rs/wasm/src/idb.rs | 16 +- 7 files changed, 327 insertions(+), 143 deletions(-) diff --git a/gpx-rs/engine/src/core/gpx/categories.rs b/gpx-rs/engine/src/core/gpx/categories.rs index 5c8440ff4..ad0f3097d 100644 --- a/gpx-rs/engine/src/core/gpx/categories.rs +++ b/gpx-rs/engine/src/core/gpx/categories.rs @@ -7,13 +7,6 @@ pub struct Categories(Vec); impl Categories { - /// A table with these names, in the order of their codes (at most [`Self::CAPACITY`] of them - /// are kept). - pub fn from_names(mut names: Vec) -> Self { - names.truncate(Self::CAPACITY); - Self(names) - } - /// Maximum number of distinct values. pub const CAPACITY: usize = u8::MAX as usize; diff --git a/gpx-rs/engine/src/storage/codec/mod.rs b/gpx-rs/engine/src/storage/codec/mod.rs index 3a3797344..0b4a00412 100644 --- a/gpx-rs/engine/src/storage/codec/mod.rs +++ b/gpx-rs/engine/src/storage/codec/mod.rs @@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::{ - File, FileId, FileInfo, Track, TrackId, TrackInfo, TrackSegment, TrackSegmentId, Trackpoint, + File, FileId, FileInfo, Track, TrackId, TrackInfo, TrackSegment, TrackSegmentId, TrackpointCategories, TrackpointChunk, TrackpointChunkId, Waypoint, WaypointChunk, WaypointChunkId, Waypoints, }; @@ -144,22 +144,21 @@ pub fn decode_file(bytes: &[u8]) -> Result { } } -pub fn encode_trackpoints(chunk: &TrackpointChunk) -> Vec { - encode( - &chunk - .trkpt - .iter() - .map(v1::Trackpoint1::from) - .collect::>(), - ) +/// The bytes of a chunk of trackpoints. Its surfaces, highways and scales are written as names, +/// the codes of `categories` being the ones of the engine that has the chunk. +pub fn encode_trackpoints(chunk: &TrackpointChunk, categories: &TrackpointCategories) -> Vec { + encode(&v1::TrackpointChunk1::new(&chunk.trkpt, categories)) } -pub fn decode_trackpoints(id: Uuid, bytes: &[u8]) -> Result { +/// A chunk of trackpoints from its bytes. The names it holds are added to `categories` if it does +/// not know them yet, and the trackpoints refer to them by the codes of `categories`. +pub fn decode_trackpoints( + id: Uuid, + bytes: &[u8], + categories: &mut TrackpointCategories, +) -> Result { let trkpt = match split(bytes)? { - (1, data) => parse::>(data)? - .into_iter() - .map(Trackpoint::from) - .collect(), + (1, data) => parse::(data)?.into_points(categories), (version, _) => return Err(DecodeError::UnknownVersion(version)), }; Ok(TrackpointChunk { @@ -203,22 +202,11 @@ pub fn decode_order(bytes: &[u8]) -> Result, DecodeError> { } } -pub fn encode_categories(categories: &TrackpointCategories) -> Vec { - encode(&v1::Categories1::from(categories)) -} - -pub fn decode_categories(bytes: &[u8]) -> Result { - match split(bytes)? { - (1, data) => Ok(parse::(data)?.into()), - (version, _) => Err(DecodeError::UnknownVersion(version)), - } -} - #[cfg(test)] mod tests { use std::collections::HashMap; - use crate::{Author, Link, LngLat}; + use crate::{Author, Chunk, Link, LngLat, Trackpoint}; use super::*; @@ -246,10 +234,11 @@ mod tests { hr: Some(140 + u16::from(n)), cad: Some(80), power: Some(250), - surface: Some(1), - highway: Some(2), - sac_scale: Some(3), - mtb_scale: Some(4), + // the first has the second name of the table, which comes first in the chunk + surface: Some(if n == 0 { 1 } else { 0 }), + highway: Some(0), + sac_scale: Some(0), + mtb_scale: Some(0), anchor: Some(n), } } @@ -439,12 +428,34 @@ mod tests { assert!(built.trk.is_empty() && built.wpt.is_empty()); } + /// The names of the categories of a trackpoint. + fn names(point: &Trackpoint, categories: &TrackpointCategories) -> [Option; 4] { + let name = |table: &crate::Categories, code: Option| { + code.and_then(|code| table.name(code)).map(str::to_owned) + }; + [ + name(&categories.surface, point.surface), + name(&categories.highway, point.highway), + name(&categories.sac_scale, point.sac_scale), + name(&categories.mtb_scale, point.mtb_scale), + ] + } + #[test] fn test_chunks_keep_their_identity_and_items() { let chunk = &sample().trk[0].trkseg[0].chunks()[0].clone(); - let decoded = decode_trackpoints(chunk.id.0, &encode_trackpoints(chunk)).unwrap(); + let categories = categories(); + let mut reader = categories.clone(); + let decoded = decode_trackpoints( + chunk.id.0, + &encode_trackpoints(chunk, &categories), + &mut reader, + ) + .unwrap(); assert_eq!(decoded.id, chunk.id); assert_eq!(decoded.trkpt.len(), 3); + // read by an engine that has the same table, the trackpoints are the same + assert_eq!(reader, categories); for (a, b) in decoded.trkpt.iter().zip(&chunk.trkpt) { assert_eq!(format!("{a:?}"), format!("{b:?}")); } @@ -459,14 +470,99 @@ mod tests { } #[test] - fn test_order_and_categories_round_trip() { + fn test_a_chunk_means_the_same_to_an_engine_with_another_table() { + let chunk = &sample().trk[0].trkseg[0].chunks()[0].clone(); + let writer = categories(); + let bytes = encode_trackpoints(chunk, &writer); + + // an engine that learned other names first, and the same ones in another order + let mut reader = TrackpointCategories::default(); + reader.surface.code("sand"); + reader.surface.code("gravel"); + reader.surface.code("asphalt"); + reader.highway.code("track"); + let decoded = decode_trackpoints(chunk.id.0, &bytes, &mut reader).unwrap(); + + for (read, written) in decoded.trkpt.iter().zip(&chunk.trkpt) { + assert_eq!(names(read, &reader), names(written, &writer)); + } + // the codes are the ones of the reader, which learned what it did not know + assert_eq!(decoded.trkpt[0].surface, Some(1)); + assert_eq!(decoded.trkpt[1].surface, Some(2)); + assert_eq!(reader.highway.names(), ["track", "path"]); + assert_eq!(decoded.trkpt[0].highway, Some(1)); + assert_eq!(reader.sac_scale.names(), ["hiking"]); + // and an empty engine learns them in the order they come + let mut empty = TrackpointCategories::default(); + decode_trackpoints(chunk.id.0, &bytes, &mut empty).unwrap(); + assert_eq!(empty.surface.names(), ["gravel", "asphalt"]); + } + + #[test] + fn test_a_chunk_only_carries_the_names_it_uses() { + let mut writer = TrackpointCategories::default(); + for i in 0..10 { + writer.surface.code(&format!("surface-{i}")); + } + let mut chunk = sample().trk[0].trkseg[0].chunks()[0].trkpt.clone(); + chunk.iter_mut().for_each(|p| p.surface = Some(7)); + let chunk = TrackpointChunk::new(chunk); + let bytes = encode_trackpoints(&chunk, &writer); + + let mut reader = TrackpointCategories::default(); + let decoded = decode_trackpoints(chunk.id.0, &bytes, &mut reader).unwrap(); + assert_eq!(reader.surface.names(), ["surface-7"]); + assert!(decoded.trkpt.iter().all(|p| p.surface == Some(0))); + } + + #[test] + fn test_codes_without_a_name_are_not_stored_and_names_without_room_are_lost() { + let mut writer = categories(); + let mut chunk = sample().trk[0].trkseg[0].chunks()[0].trkpt.clone(); + // a code that the table does not know + chunk[1].surface = Some(200); + let chunk = TrackpointChunk::new(chunk); + let bytes = encode_trackpoints(&chunk, &writer); + let decoded = + decode_trackpoints(chunk.id.0, &bytes, &mut TrackpointCategories::default()).unwrap(); + assert_eq!(decoded.trkpt[1].surface, None); + assert_eq!(decoded.trkpt[0].surface, Some(0)); + + // an engine whose table is full keeps the names it has and loses the other ones + let mut full = TrackpointCategories::default(); + for i in 0..crate::Categories::CAPACITY { + full.surface.code(&format!("other-{i}")); + } + writer.surface.code("other-3"); + let mut chunk = chunk.trkpt.clone(); + chunk[0].surface = writer.surface.code("other-3"); + chunk[1].surface = writer.surface.code("gravel"); + let chunk = TrackpointChunk::new(chunk); + let decoded = + decode_trackpoints(chunk.id.0, &encode_trackpoints(&chunk, &writer), &mut full) + .unwrap(); + assert_eq!(decoded.trkpt[0].surface, Some(3)); + assert_eq!(decoded.trkpt[1].surface, None); + } + + #[test] + fn test_chunks_read_one_after_the_other_share_the_table() { + let writer = categories(); + let chunk = sample().trk[0].trkseg[0].chunks()[0].clone(); + let bytes = encode_trackpoints(&chunk, &writer); + let mut reader = TrackpointCategories::default(); + let a = decode_trackpoints(id(1), &bytes, &mut reader).unwrap(); + let b = decode_trackpoints(id(2), &bytes, &mut reader).unwrap(); + for (a, b) in a.trkpt.iter().zip(&b.trkpt) { + assert_eq!((a.surface, a.highway), (b.surface, b.highway)); + } + assert_eq!(reader.surface.names().len(), 2); + } + + #[test] + fn test_order_round_trips() { let order = vec![FileId(id(9)), FileId(id(8))]; assert_eq!(decode_order(&encode_order(&order)).unwrap(), order); - let categories = categories(); - assert_eq!( - decode_categories(&encode_categories(&categories)).unwrap(), - categories - ); } #[test] @@ -486,17 +582,13 @@ mod tests { Err(DecodeError::UnknownVersion(_)) )); assert!(matches!( - decode_trackpoints(id(1), &bytes), + decode_trackpoints(id(1), &bytes, &mut Default::default()), Err(DecodeError::UnknownVersion(_)) )); assert!(matches!( decode_waypoints(id(1), &bytes), Err(DecodeError::UnknownVersion(_)) )); - assert!(matches!( - decode_categories(&bytes), - Err(DecodeError::UnknownVersion(_)) - )); // nothing at all, or a known version with something else assert_eq!(decode_order(&[]), Err(DecodeError::Corrupt)); @@ -505,10 +597,9 @@ mod tests { Some(DecodeError::Corrupt) ); assert_eq!( - decode_trackpoints(id(1), &[LATEST, 9, 9, 9]).err(), + decode_trackpoints(id(1), &[LATEST, 9, 9, 9], &mut Default::default()).err(), Some(DecodeError::Corrupt) ); - assert_eq!(decode_categories(&[LATEST]), Err(DecodeError::Corrupt)); } // The stored forms of version 1, as they were written when the version was made. They must @@ -529,10 +620,10 @@ mod tests { "000000000000000000000000000600011000000000000000000000000000000005", ); const TRACKPOINTS_V1: &str = concat!( - "01030000000000001040000000000000494000000000002059400180a0abfef9620105018c01015001fa0101", - "0101020103010401000000000000001440000000000080494000000000006059400182a0abfef9620105018d", - "01015001fa010101010201030104010100000000000000000000000000000000000000000000000000000000", - "000000000000", + "01020667726176656c0761737068616c74010470617468010668696b696e6701046d74623003000000000000", + "1040000000000000494000000000002059400180a0abfef9620105018c01015001fa01010001000100010001", + "000000000000001440000000000080494000000000006059400182a0abfef9620105018d01015001fa010101", + "010001000100010100000000000000000000000000000000000000000000000000000000000000000000", ); const WAYPOINTS_V1: &str = concat!( "0102100000000000000000000000000000000700000000000016400000000000c04940000000000010744001", @@ -545,15 +636,16 @@ mod tests { ); const ORDER_V1: &str = "010210000000000000000000000000000000091000000000000000000000000000000008"; - const CATEGORIES_V1: &str = - "01020761737068616c740667726176656c010470617468010668696b696e6701046d746230"; #[test] fn test_version_1_is_still_written_as_it_was() { let file = sample(); assert_eq!(hex(&encode_file(&file)), FILE_V1); assert_eq!( - hex(&encode_trackpoints(&file.trk[0].trkseg[0].chunks()[0])), + hex(&encode_trackpoints( + &file.trk[0].trkseg[0].chunks()[0], + &categories() + )), TRACKPOINTS_V1 ); assert_eq!(hex(&encode_waypoints(&file.wpt.chunks()[0])), WAYPOINTS_V1); @@ -561,7 +653,6 @@ mod tests { hex(&encode_order(&[FileId(id(9)), FileId(id(8))])), ORDER_V1 ); - assert_eq!(hex(&encode_categories(&categories())), CATEGORIES_V1); } #[test] @@ -571,12 +662,15 @@ mod tests { &rebuilt(&file, decode_file(&unhex(FILE_V1)).unwrap()).unwrap(), &file, ); - let chunk = decode_trackpoints(id(4), &unhex(TRACKPOINTS_V1)).unwrap(); + let mut reader = TrackpointCategories::default(); + let chunk = decode_trackpoints(id(4), &unhex(TRACKPOINTS_V1), &mut reader).unwrap(); let expected = &file.trk[0].trkseg[0].chunks()[0]; - assert_eq!( - format!("{:?}", chunk.trkpt), - format!("{:?}", expected.trkpt) - ); + assert_eq!(chunk.trkpt.len(), expected.trkpt.len()); + for (read, written) in chunk.trkpt.iter().zip(&expected.trkpt) { + assert_eq!(names(read, &reader), names(written, &categories())); + assert_eq!(read.hr, written.hr); + assert_eq!(read.anchor, written.anchor); + } let chunk = decode_waypoints(id(5), &unhex(WAYPOINTS_V1)).unwrap(); assert_eq!(chunk.wpt.len(), 2); assert!(chunk.wpt[0].same_content(&waypoint())); @@ -585,9 +679,5 @@ mod tests { decode_order(&unhex(ORDER_V1)).unwrap(), vec![FileId(id(9)), FileId(id(8))] ); - assert_eq!( - decode_categories(&unhex(CATEGORIES_V1)).unwrap(), - categories() - ); } } diff --git a/gpx-rs/engine/src/storage/codec/v1.rs b/gpx-rs/engine/src/storage/codec/v1.rs index 82a72e202..d6917e5d4 100644 --- a/gpx-rs/engine/src/storage/codec/v1.rs +++ b/gpx-rs/engine/src/storage/codec/v1.rs @@ -2,6 +2,8 @@ //! compares the bytes they give with the ones of a fixture). A change of the form is a new //! version, next to this one, that the decoders convert from. +use std::collections::HashMap; + use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -183,6 +185,7 @@ pub struct Trackpoint1 { hr: Option, cad: Option, power: Option, + /// The position of the name in the list of the chunk (see [`TrackpointChunk1`]). surface: Option, highway: Option, sac_scale: Option, @@ -285,34 +288,107 @@ impl From for Waypoint { } } -/// The names of each table of categories, in the order of their codes. +/// A chunk of trackpoints. It carries the names its categories refer to: the surface, highway, +/// SAC scale and MTB scale of its trackpoints are the positions of the names in its own lists, +/// which only hold the names it uses. So it means the same whatever the table of categories of +/// the engine that reads it is, and it is not changed by what other chunks learn. #[derive(Serialize, Deserialize)] -pub struct Categories1 { +pub struct TrackpointChunk1 { surface: Vec, highway: Vec, sac_scale: Vec, mtb_scale: Vec, + points: Vec, } -impl From<&TrackpointCategories> for Categories1 { - fn from(categories: &TrackpointCategories) -> Self { - let names = |table: &Categories| table.names().to_vec(); +/// The names of a category used by a chunk, with their position in the list of the chunk. +struct LocalNames<'a> { + table: &'a Categories, + names: Vec, + local: HashMap, +} + +impl<'a> LocalNames<'a> { + fn new(table: &'a Categories) -> Self { Self { - surface: names(&categories.surface), - highway: names(&categories.highway), - sac_scale: names(&categories.sac_scale), - mtb_scale: names(&categories.mtb_scale), + table, + names: vec![], + local: HashMap::new(), } } + + /// The position in the list of the chunk of the name of a code of the table. A code that + /// has no name has no position. + fn position(&mut self, code: Option) -> Option { + let code = code?; + if let Some(position) = self.local.get(&code) { + return Some(*position); + } + let name = self.table.name(code)?; + let position = u8::try_from(self.names.len()).ok()?; + self.names.push(name.to_owned()); + self.local.insert(code, position); + Some(position) + } } -impl From for TrackpointCategories { - fn from(categories: Categories1) -> Self { +/// The codes of the table for the names of a chunk, which are added to it if it does not know +/// them. A name that does not fit in a full table has no code. +fn codes_of(names: &[String], table: &mut Categories) -> Vec> { + names.iter().map(|name| table.code(name)).collect() +} + +impl TrackpointChunk1 { + pub fn new(points: &[Trackpoint], categories: &TrackpointCategories) -> Self { + let mut surface = LocalNames::new(&categories.surface); + let mut highway = LocalNames::new(&categories.highway); + let mut sac_scale = LocalNames::new(&categories.sac_scale); + let mut mtb_scale = LocalNames::new(&categories.mtb_scale); + let points = points + .iter() + .map(|point| { + let mut stored = Trackpoint1::from(point); + stored.surface = surface.position(point.surface); + stored.highway = highway.position(point.highway); + stored.sac_scale = sac_scale.position(point.sac_scale); + stored.mtb_scale = mtb_scale.position(point.mtb_scale); + stored + }) + .collect(); Self { - surface: Categories::from_names(categories.surface), - highway: Categories::from_names(categories.highway), - sac_scale: Categories::from_names(categories.sac_scale), - mtb_scale: Categories::from_names(categories.mtb_scale), + surface: surface.names, + highway: highway.names, + sac_scale: sac_scale.names, + mtb_scale: mtb_scale.names, + points, } } + + /// The trackpoints, with the codes of `categories` for the names of the chunk. + pub fn into_points(self, categories: &mut TrackpointCategories) -> Vec { + let surface = codes_of(&self.surface, &mut categories.surface); + let highway = codes_of(&self.highway, &mut categories.highway); + let sac_scale = codes_of(&self.sac_scale, &mut categories.sac_scale); + let mtb_scale = codes_of(&self.mtb_scale, &mut categories.mtb_scale); + let global = |codes: &[Option], local: Option| { + local.and_then(|local| codes.get(usize::from(local)).copied().flatten()) + }; + self.points + .into_iter() + .map(|stored| { + let (s, h, sac, mtb) = ( + global(&surface, stored.surface), + global(&highway, stored.highway), + global(&sac_scale, stored.sac_scale), + global(&mtb_scale, stored.mtb_scale), + ); + let mut point = Trackpoint::from(stored); + point.surface = s; + point.highway = h; + point.sac_scale = sac; + point.mtb_scale = mtb; + point + }) + .collect() + } } diff --git a/gpx-rs/engine/src/storage/memory.rs b/gpx-rs/engine/src/storage/memory.rs index ad784d756..61c181828 100644 --- a/gpx-rs/engine/src/storage/memory.rs +++ b/gpx-rs/engine/src/storage/memory.rs @@ -15,7 +15,6 @@ struct State { files: HashMap>, chunks: HashMap>, order: Option>, - categories: Option>, settings: HashMap, } @@ -47,7 +46,6 @@ impl Storage for MemoryStorage { files: state.files.iter().map(|(k, v)| (*k, v.clone())).collect(), chunks: state.chunks.iter().map(|(k, v)| (*k, v.clone())).collect(), order: state.order.clone(), - categories: state.categories.clone(), }) } @@ -64,9 +62,6 @@ impl Storage for MemoryStorage { if let Some(order) = &batch.order { state.order = Some(order.clone()); } - if let Some(categories) = &batch.categories { - state.categories = Some(categories.clone()); - } Ok(()) } @@ -140,7 +135,6 @@ mod tests { (ChunkKey::waypoints(w), vec![4]), ], order: Some(vec![5]), - categories: Some(vec![6]), ..Default::default() })) .unwrap(); @@ -157,10 +151,7 @@ mod tests { (ChunkKey::waypoints(w), vec![4]) ] ); - assert_eq!( - (data.order, data.categories), - (Some(vec![5]), Some(vec![6])) - ); + assert_eq!(data.order, Some(vec![5])); // a put replaces, a delete removes, what is not mentioned stays block_on(storage.commit(&Batch { @@ -173,10 +164,7 @@ mod tests { assert_eq!(storage.counts(), counts(1, 0, 1)); let data = block_on(storage.load()).unwrap(); assert_eq!(data.files, vec![(a, vec![10])]); - assert_eq!( - (data.order, data.categories), - (Some(vec![5]), Some(vec![6])) - ); + assert_eq!(data.order, Some(vec![5])); block_on(storage.commit(&Batch { delete_chunks: vec![ChunkKey::waypoints(w)], diff --git a/gpx-rs/engine/src/storage/mod.rs b/gpx-rs/engine/src/storage/mod.rs index 408fb39af..393d09527 100644 --- a/gpx-rs/engine/src/storage/mod.rs +++ b/gpx-rs/engine/src/storage/mod.rs @@ -7,6 +7,11 @@ //! [`crate::Chunked`]) that are written once, under their identity, and a file is a small record //! that lists the chunks it is made of. An edit only writes the new chunks and the records of the //! files that changed; the chunks that nothing refers to anymore are deleted at the same time. +//! +//! The surface, highway and scales of the trackpoints are stored as names, in the chunk that has +//! them: the codes that the engine gives them (see [`crate::TrackpointCategories`]) depend on the +//! order in which it learned the names, so they mean nothing outside of it. A storage that two +//! engines (tabs, a server) write to therefore has nothing to agree on but the chunks. mod codec; mod memory; @@ -68,8 +73,6 @@ pub struct StoredData { pub chunks: Vec<(ChunkKey, Vec)>, /// The order of the files. pub order: Option>, - /// The categories of the trackpoints, which their chunks refer to. - pub categories: Option>, } /// A change of the stored data, to be applied all at once: a stored state is always one that the @@ -81,7 +84,6 @@ pub struct Batch { pub put_chunks: Vec<(ChunkKey, Vec)>, pub delete_chunks: Vec, pub order: Option>, - pub categories: Option>, } impl Batch { @@ -155,10 +157,6 @@ mod tests { order: Some(vec![]), ..Default::default() }, - Batch { - categories: Some(vec![]), - ..Default::default() - }, ]; assert!(holds.iter().all(|batch| !batch.is_empty())); } diff --git a/gpx-rs/engine/src/storage/persister.rs b/gpx-rs/engine/src/storage/persister.rs index 69c135457..c79204d57 100644 --- a/gpx-rs/engine/src/storage/persister.rs +++ b/gpx-rs/engine/src/storage/persister.rs @@ -5,9 +5,8 @@ use std::rc::Rc; use uuid::Uuid; use super::codec::{ - DecodeError, decode_categories, decode_file, decode_order, decode_trackpoints, - decode_waypoints, encode_categories, encode_file, encode_order, encode_trackpoints, - encode_waypoints, + DecodeError, decode_file, decode_order, decode_trackpoints, decode_waypoints, encode_file, + encode_order, encode_trackpoints, encode_waypoints, }; use crate::{ Batch, ChunkKey, File, FileId, StackEntry, Storage, StorageError, TrackpointCategories, @@ -51,7 +50,6 @@ pub struct Persister { stored_files: HashSet, chunks: HashSet, order: Option>, - categories: Option, /// Set by `restore`, see [`Restored::read_only`]. read_only: bool, } @@ -88,10 +86,8 @@ impl Persister { // what comes from a newer version is not corrupt: it must not be overwritten let newer = Newer::default(); - let categories: Option = data - .categories - .as_deref() - .and_then(|bytes| newer.read(decode_categories(bytes))); + // the names of the chunks are added to the categories of the engine as they are read + let categories: RefCell = Default::default(); let stored_order: Option> = data .order .as_deref() @@ -108,7 +104,13 @@ impl Persister { .entry(id.0) .or_insert_with(|| { let bytes = chunk_bytes.get(&ChunkKey::trackpoints(id.0))?; - newer.read(decode_trackpoints(id.0, bytes)).map(Rc::new) + newer + .read(decode_trackpoints( + id.0, + bytes, + &mut categories.borrow_mut(), + )) + .map(Rc::new) }) .clone() }; @@ -123,10 +125,7 @@ impl Persister { .clone() }; - let mut restored = Restored { - categories: categories.clone().unwrap_or_default(), - ..Default::default() - }; + let mut restored = Restored::default(); let mut files: HashMap> = HashMap::new(); for (id, bytes) in &data.files { self.stored_files.insert(*id); @@ -156,7 +155,7 @@ impl Persister { self.files = files; self.chunks = chunk_bytes.into_keys().collect(); self.order = stored_order; - self.categories = categories; + restored.categories = categories.into_inner(); self.read_only = newer.0.get(); restored.read_only = self.read_only; Ok(restored) @@ -182,7 +181,6 @@ impl Persister { self.files = plan.snapshot.files; self.chunks = plan.chunks; self.order = Some(plan.snapshot.order); - self.categories = Some(plan.snapshot.categories); Ok(()) } @@ -200,7 +198,9 @@ impl Persister { for chunk in segments.flat_map(|segment| segment.chunks()) { let key = ChunkKey::trackpoints(chunk.id.0); if chunks.insert(key) && changed && !self.chunks.contains(&key) { - batch.put_chunks.push((key, encode_trackpoints(chunk))); + batch + .put_chunks + .push((key, encode_trackpoints(chunk, &snapshot.categories))); } } for chunk in file.wpt.chunks() { @@ -224,9 +224,6 @@ impl Persister { if self.order.as_ref() != Some(&snapshot.order) { batch.order = Some(encode_order(&snapshot.order)); } - if self.categories.as_ref() != Some(&snapshot.categories) { - batch.categories = Some(encode_categories(&snapshot.categories)); - } Plan { batch, @@ -375,7 +372,7 @@ mod tests { assert_eq!(batch.put_files.len(), 1); assert!(batch.put_chunks.is_empty()); assert!(batch.delete_chunks.is_empty()); - assert!(batch.order.is_none() && batch.categories.is_none()); + assert!(batch.order.is_none()); } assert_eq!(storage.inner.counts(), counts); @@ -482,7 +479,7 @@ mod tests { #[test] fn test_what_a_newer_version_wrote_is_left_alone() { type Damage = (&'static str, fn(&Snapshot, &mut Batch)); - let damages: [Damage; 4] = [ + let damages: [Damage; 3] = [ ("a file", |snapshot, batch| { batch.put_files.push((snapshot.order[0], from_the_future())); }), @@ -496,9 +493,6 @@ mod tests { ("the order", |_, batch| { batch.order = Some(from_the_future()) }), - ("the categories", |_, batch| { - batch.categories = Some(from_the_future()) - }), ]; for (what, damage) in damages { let (storage, snapshot) = stored_with(damage); @@ -533,6 +527,65 @@ mod tests { assert_eq!(clean.inner.counts().files, FILES.len()); } + #[test] + fn test_files_saved_by_engines_with_other_categories_read_the_same() { + // two engines that share a storage (two tabs) learn the names in their own order + let storage = Recording::default(); + let mut first = snapshot_of(&["data/with_surface.gpx", "data/with_highway.gpx"]); + let mut second = snapshot_of(&["data/with_highway.gpx", "data/with_surface.gpx"]); + assert_ne!(first.categories, second.categories); + for snapshot in [&first, &second] { + block_on(Persister::default().save(&storage, snapshot.clone())).unwrap(); + } + + let mut persister = Persister::default(); + let restored = block_on(persister.restore(&storage)).unwrap(); + assert_eq!(restored.unreadable, 0); + assert_eq!(restored.files.len(), 4); + // every file means what it did for the engine that saved it + first.files.extend(second.files.drain()); + for file in &restored.files { + let original = &first.files[&file.id]; + let in_first = first.order.contains(&file.id); + let categories = if in_first { + &first.categories + } else { + &second.categories + }; + assert_eq!(gpx(file, &restored.categories), gpx(original, categories)); + } + } + + #[test] + fn test_chunks_shared_by_files_still_are_after_the_names_are_read() { + let storage = Recording::default(); + let mut snapshot = snapshot_of(&["data/with_surface.gpx"]); + let original = snapshot.files[&snapshot.order[0]].clone(); + let mut copy = (*original).clone(); + copy.id = FileId::default(); + snapshot.order.push(copy.id); + snapshot.files.insert(copy.id, Rc::new(copy)); + block_on(Persister::default().save(&storage, snapshot)).unwrap(); + + let restored = block_on(Persister::default().restore(&storage)).unwrap(); + assert_eq!(restored.files.len(), 2); + assert!(Rc::ptr_eq( + &restored.files[0].trk[0].trkseg[0].chunks()[0], + &restored.files[1].trk[0].trkseg[0].chunks()[0] + )); + assert!(!restored.categories.surface.names().is_empty()); + } + + #[test] + fn test_nothing_but_the_files_and_the_order_is_stored() { + let storage = Recording::default(); + block_on(Persister::default().save(&storage, snapshot_of(&FILES))).unwrap(); + // the categories are in the chunks: a storage with no more than these is complete + let data = block_on(storage.load()).unwrap(); + assert_eq!(data.files.len(), FILES.len()); + assert!(data.order.is_some()); + } + #[test] fn test_an_engine_starts_from_what_is_restored() { let storage = Recording::default(); diff --git a/gpx-rs/wasm/src/idb.rs b/gpx-rs/wasm/src/idb.rs index baf901638..f435e5276 100644 --- a/gpx-rs/wasm/src/idb.rs +++ b/gpx-rs/wasm/src/idb.rs @@ -14,13 +14,12 @@ use wasm_bindgen::JsValue; const FILES: &str = "files"; const TRACKPOINT_CHUNKS: &str = "trackpoint_chunks"; const WAYPOINT_CHUNKS: &str = "waypoint_chunks"; -/// The order of the files and the categories of the trackpoints, under their own names. +/// The order of the files, under its own name. const META: &str = "meta"; const SETTINGS: &str = "settings"; const STORES: [&str; 5] = [FILES, TRACKPOINT_CHUNKS, WAYPOINT_CHUNKS, META, SETTINGS]; const ORDER_KEY: &str = "order"; -const CATEGORIES_KEY: &str = "categories"; pub struct IdbStorage { database: Database, @@ -111,13 +110,6 @@ impl IdbStorage { .map_err(error)?, ); } - if let Some(categories) = &batch.categories { - let value = bytes(categories); - puts.push( - meta.put(&value, Some(&JsValue::from_str(CATEGORIES_KEY))) - .map_err(error)?, - ); - } for FileId(id) in &batch.delete_files { deletes.push(files.delete(Query::Key(key(*id))).map_err(error)?); } @@ -153,11 +145,6 @@ impl Storage for IdbStorage { .map_err(error)? .await .map_err(error)?; - let categories = meta - .get(JsValue::from_str(CATEGORIES_KEY)) - .map_err(error)? - .await - .map_err(error)?; transaction.await.map_err(error)?; let ids = |records: Vec<(JsValue, Vec)>| -> Result)>, StorageError> { @@ -181,7 +168,6 @@ impl Storage for IdbStorage { ) .collect(), order: order.as_ref().map(to_vec), - categories: categories.as_ref().map(to_vec), }) }