per-chunk categories storage

gpx-rs/engine/src/core/gpx/categories.rs

@@ -7,13 +7,6 @@
 pub struct Categories(Vec<String>);
 
 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<String>) -> Self {
-        names.truncate(Self::CAPACITY);
-        Self(names)
-    }
-
     /// Maximum number of distinct values.
     pub const CAPACITY: usize = u8::MAX as usize;
 

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<FileRecord, DecodeError> {
     }
 }
 
-pub fn encode_trackpoints(chunk: &TrackpointChunk) -> Vec<u8> {
+/// The bytes of a chunk of trackpoints. Its surfaces, highways and scales are written as names,
-    encode(
+/// the codes of `categories` being the ones of the engine that has the chunk.
-        &chunk
+pub fn encode_trackpoints(chunk: &TrackpointChunk, categories: &TrackpointCategories) -> Vec<u8> {
-            .trkpt
+    encode(&v1::TrackpointChunk1::new(&chunk.trkpt, categories))
-            .iter()
-            .map(v1::Trackpoint1::from)
-            .collect::<Vec<_>>(),
-    )
 }
 
-pub fn decode_trackpoints(id: Uuid, bytes: &[u8]) -> Result<TrackpointChunk, DecodeError> {
+/// 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<TrackpointChunk, DecodeError> {
     let trkpt = match split(bytes)? {
-        (1, data) => parse::<Vec<v1::Trackpoint1>>(data)?
+        (1, data) => parse::<v1::TrackpointChunk1>(data)?.into_points(categories),
-            .into_iter()
-            .map(Trackpoint::from)
-            .collect(),
         (version, _) => return Err(DecodeError::UnknownVersion(version)),
     };
     Ok(TrackpointChunk {
@@ -203,22 +202,11 @@ pub fn decode_order(bytes: &[u8]) -> Result<Vec<FileId>, DecodeError> {
     }
 }
 
-pub fn encode_categories(categories: &TrackpointCategories) -> Vec<u8> {
-    encode(&v1::Categories1::from(categories))
-}
-
-pub fn decode_categories(bytes: &[u8]) -> Result<TrackpointCategories, DecodeError> {
-    match split(bytes)? {
-        (1, data) => Ok(parse::<v1::Categories1>(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),
+            // the first has the second name of the table, which comes first in the chunk
-            highway: Some(2),
+            surface: Some(if n == 0 { 1 } else { 0 }),
-            sac_scale: Some(3),
+            highway: Some(0),
-            mtb_scale: Some(4),
+            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<String>; 4] {
+        let name = |table: &crate::Categories, code: Option<u8>| {
+            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",
+        "01020667726176656c0761737068616c74010470617468010668696b696e6701046d74623003000000000000",
-        "0101020103010401000000000000001440000000000080494000000000006059400182a0abfef9620105018d",
+        "1040000000000000494000000000002059400180a0abfef9620105018c01015001fa01010001000100010001",
-        "01015001fa010101010201030104010100000000000000000000000000000000000000000000000000000000",
+        "000000000000001440000000000080494000000000006059400182a0abfef9620105018d01015001fa010101",
-        "000000000000",
+        "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!(
+        assert_eq!(chunk.trkpt.len(), expected.trkpt.len());
-            format!("{:?}", chunk.trkpt),
+        for (read, written) in chunk.trkpt.iter().zip(&expected.trkpt) {
-            format!("{:?}", 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()
-        );
     }
 }

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<u16>,
     cad: Option<u16>,
     power: Option<u16>,
+    /// The position of the name in the list of the chunk (see [`TrackpointChunk1`]).
     surface: Option<u8>,
     highway: Option<u8>,
     sac_scale: Option<u8>,
@@ -285,34 +288,107 @@ impl From<Waypoint1> 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<String>,
     highway: Vec<String>,
     sac_scale: Vec<String>,
     mtb_scale: Vec<String>,
+    points: Vec<Trackpoint1>,
 }
 
-impl From<&TrackpointCategories> for Categories1 {
+/// The names of a category used by a chunk, with their position in the list of the chunk.
-    fn from(categories: &TrackpointCategories) -> Self {
+struct LocalNames<'a> {
-        let names = |table: &Categories| table.names().to_vec();
+    table: &'a Categories,
-        Self {
+    names: Vec<String>,
-            surface: names(&categories.surface),
+    local: HashMap<u8, u8>,
-            highway: names(&categories.highway),
-            sac_scale: names(&categories.sac_scale),
-            mtb_scale: names(&categories.mtb_scale),
 }
+
+impl<'a> LocalNames<'a> {
+    fn new(table: &'a Categories) -> Self {
+        Self {
+            table,
+            names: vec![],
+            local: HashMap::new(),
         }
     }
 
-impl From<Categories1> for TrackpointCategories {
+    /// The position in the list of the chunk of the name of a code of the table. A code that
-    fn from(categories: Categories1) -> Self {
+    /// has no name has no position.
+    fn position(&mut self, code: Option<u8>) -> Option<u8> {
+        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)
+    }
+}
+
+/// 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<Option<u8>> {
+    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),
+            surface: surface.names,
-            highway: Categories::from_names(categories.highway),
+            highway: highway.names,
-            sac_scale: Categories::from_names(categories.sac_scale),
+            sac_scale: sac_scale.names,
-            mtb_scale: Categories::from_names(categories.mtb_scale),
+            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<Trackpoint> {
+        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<u8>], local: Option<u8>| {
+            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()
+    }
 }

gpx-rs/engine/src/storage/memory.rs

@@ -15,7 +15,6 @@ struct State {
     files: HashMap<FileId, Vec<u8>>,
     chunks: HashMap<ChunkKey, Vec<u8>>,
     order: Option<Vec<u8>>,
-    categories: Option<Vec<u8>>,
     settings: HashMap<String, String>,
 }
 
@@ -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!(
+        assert_eq!(data.order, Some(vec![5]));
-            (data.order, data.categories),
-            (Some(vec![5]), Some(vec![6]))
-        );
 
         // 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!(
+        assert_eq!(data.order, Some(vec![5]));
-            (data.order, data.categories),
-            (Some(vec![5]), Some(vec![6]))
-        );
 
         block_on(storage.commit(&Batch {
             delete_chunks: vec![ChunkKey::waypoints(w)],

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<u8>)>,
     /// The order of the files.
     pub order: Option<Vec<u8>>,
-    /// The categories of the trackpoints, which their chunks refer to.
-    pub categories: Option<Vec<u8>>,
 }
 
 /// 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<u8>)>,
     pub delete_chunks: Vec<ChunkKey>,
     pub order: Option<Vec<u8>>,
-    pub categories: Option<Vec<u8>>,
 }
 
 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()));
     }

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,
+    DecodeError, decode_file, decode_order, decode_trackpoints, decode_waypoints, encode_file,
-    decode_waypoints, encode_categories, encode_file, encode_order, encode_trackpoints,
+    encode_order, encode_trackpoints, encode_waypoints,
-    encode_waypoints,
 };
 use crate::{
     Batch, ChunkKey, File, FileId, StackEntry, Storage, StorageError, TrackpointCategories,
@@ -51,7 +50,6 @@ pub struct Persister {
     stored_files: HashSet<FileId>,
     chunks: HashSet<ChunkKey>,
     order: Option<Vec<FileId>>,
-    categories: Option<TrackpointCategories>,
     /// 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<TrackpointCategories> = data
+        // the names of the chunks are added to the categories of the engine as they are read
-            .categories
+        let categories: RefCell<TrackpointCategories> = Default::default();
-            .as_deref()
-            .and_then(|bytes| newer.read(decode_categories(bytes)));
         let stored_order: Option<Vec<FileId>> = 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 {
+        let mut restored = Restored::default();
-            categories: categories.clone().unwrap_or_default(),
-            ..Default::default()
-        };
         let mut files: HashMap<FileId, Rc<File>> = 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();

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<u8>)>| -> Result<Vec<(Uuid, Vec<u8>)>, StorageError> {
@@ -181,7 +168,6 @@ impl Storage for IdbStorage {
                 )
                 .collect(),
             order: order.as_ref().map(to_vec),
-            categories: categories.as_ref().map(to_vec),
         })
     }