From 252678e1c8e2fb6be6d9417083f271e182de4480 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Wed, 23 Oct 2024 21:29:17 +0200 Subject: [PATCH 1/3] fix!: start column ids with 1 --- packages/storey/src/containers/column.rs | 126 +++++++++++------------ packages/storey/tests/composition.rs | 14 +-- 2 files changed, 70 insertions(+), 70 deletions(-) diff --git a/packages/storey/src/containers/column.rs b/packages/storey/src/containers/column.rs index bfb6c36..1bb1747 100644 --- a/packages/storey/src/containers/column.rs +++ b/packages/storey/src/containers/column.rs @@ -34,9 +34,9 @@ const META_LEN: &[u8] = &[1]; /// access.push(&1337).unwrap(); /// access.push(&42).unwrap(); /// -/// assert_eq!(access.get(0).unwrap(), Some(1337)); -/// assert_eq!(access.get(1).unwrap(), Some(42)); -/// assert_eq!(access.get(2).unwrap(), None); +/// assert_eq!(access.get(1).unwrap(), Some(1337)); +/// assert_eq!(access.get(2).unwrap(), Some(42)); +/// assert_eq!(access.get(3).unwrap(), None); /// ``` pub struct Column { prefix: u8, @@ -175,8 +175,8 @@ where /// let mut access = column.access(&mut storage); /// /// access.push(&1337).unwrap(); - /// assert_eq!(access.get(0).unwrap(), Some(1337)); - /// assert_eq!(access.get(1).unwrap(), None); + /// assert_eq!(access.get(1).unwrap(), Some(1337)); + /// assert_eq!(access.get(2).unwrap(), None); /// ``` pub fn get(&self, key: u32) -> Result, E::DecodeError> { self.storage @@ -205,8 +205,8 @@ where /// let mut access = column.access(&mut storage); /// /// access.push(&1337).unwrap(); - /// assert_eq!(access.try_get(0).unwrap(), 1337); - /// assert!(access.try_get(1).is_err()); + /// assert_eq!(access.try_get(1).unwrap(), 1337); + /// assert!(access.try_get(2).is_err()); /// ``` pub fn try_get(&self, key: u32) -> Result> { self.get(key)?.ok_or(TryGetError::Empty) @@ -226,9 +226,9 @@ where /// let column = Column::::new(0); /// let mut access = column.access(&mut storage); /// - /// assert_eq!(access.get_or(0, 42).unwrap(), 42); + /// assert_eq!(access.get_or(1, 42).unwrap(), 42); /// access.push(&1337).unwrap(); - /// assert_eq!(access.get_or(0, 42).unwrap(), 1337); + /// assert_eq!(access.get_or(1, 42).unwrap(), 1337); /// ``` pub fn get_or(&self, key: u32, default: T) -> Result { self.get(key).map(|value| value.unwrap_or(default)) @@ -335,7 +335,7 @@ where .map(|bytes| u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) { Some(last_ix) => last_ix.checked_add(1).ok_or(PushError::IndexOverflow)?, - None => 0, + None => 1, }; self.storage.set(&encode_ix(ix), &bytes); @@ -364,10 +364,10 @@ where /// let mut access = column.access(&mut storage); /// /// access.push(&1337).unwrap(); - /// assert_eq!(access.get(0).unwrap(), Some(1337)); + /// assert_eq!(access.get(1).unwrap(), Some(1337)); /// - /// access.update(0, &9001).unwrap(); - /// assert_eq!(access.get(0).unwrap(), Some(9001)); + /// access.update(1, &9001).unwrap(); + /// assert_eq!(access.get(1).unwrap(), Some(9001)); /// ``` pub fn update(&mut self, key: u32, value: &T) -> Result<(), UpdateError> { self.storage @@ -396,10 +396,10 @@ where /// let mut access = column.access(&mut storage); /// /// access.push(&1337).unwrap(); - /// assert_eq!(access.get(0).unwrap(), Some(1337)); + /// assert_eq!(access.get(1).unwrap(), Some(1337)); /// - /// access.remove(0).unwrap(); - /// assert_eq!(access.get(0).unwrap(), None); + /// access.remove(1).unwrap(); + /// assert_eq!(access.get(1).unwrap(), None); /// ``` pub fn remove(&mut self, key: u32) -> Result<(), RemoveError> { self.storage.remove(&encode_ix(key)); @@ -471,20 +471,20 @@ mod tests { let column = Column::::new(0); let mut access = column.access(&mut storage); - assert_eq!(access.push(&1337).unwrap(), 0); - assert_eq!(access.push(&42).unwrap(), 1); + assert_eq!(access.push(&1337).unwrap(), 1); + assert_eq!(access.push(&42).unwrap(), 2); - assert_eq!(access.get(0).unwrap(), Some(1337)); - assert_eq!(access.get(1).unwrap(), Some(42)); - assert_eq!(access.get(2).unwrap(), None); + assert_eq!(access.get(1).unwrap(), Some(1337)); + assert_eq!(access.get(2).unwrap(), Some(42)); + assert_eq!(access.get(3).unwrap(), None); assert_eq!(access.len().unwrap(), 2); - access.remove(0).unwrap(); - assert_eq!(access.update(0, &9001), Err(UpdateError::NotFound)); - access.update(1, &9001).unwrap(); + access.remove(1).unwrap(); + assert_eq!(access.update(1, &9001), Err(UpdateError::NotFound)); + access.update(2, &9001).unwrap(); - assert_eq!(access.get(0).unwrap(), None); - assert_eq!(access.get(1).unwrap(), Some(9001)); + assert_eq!(access.get(1).unwrap(), None); + assert_eq!(access.get(2).unwrap(), Some(9001)); assert_eq!(access.len().unwrap(), 1); } @@ -495,26 +495,26 @@ mod tests { let column = Column::::new(0); let mut access = column.access(&mut storage); - assert_eq!(access.push(&1337).unwrap(), 0); - assert_eq!(access.push(&42).unwrap(), 1); - assert_eq!(access.push(&17).unwrap(), 2); + assert_eq!(access.push(&1337).unwrap(), 1); + assert_eq!(access.push(&42).unwrap(), 2); + assert_eq!(access.push(&17).unwrap(), 3); assert_eq!(access.len().unwrap(), 3); // remove middle - access.remove(1).unwrap(); + access.remove(2).unwrap(); assert_eq!(access.len().unwrap(), 2); // remove first - access.remove(0).unwrap(); + access.remove(10).unwrap(); assert_eq!(access.len().unwrap(), 1); // remove last - access.remove(2).unwrap(); + access.remove(3).unwrap(); assert_eq!(access.len().unwrap(), 0); // Above removals do not reset the auto-incrementor, // such that we get a fresh key for the next push. - assert_eq!(access.push(&99).unwrap(), 3); + assert_eq!(access.push(&99).unwrap(), 4); assert_eq!(access.len().unwrap(), 1); } @@ -528,16 +528,16 @@ mod tests { access.push(&1337).unwrap(); access.push(&42).unwrap(); access.push(&9001).unwrap(); - access.remove(1).unwrap(); + access.remove(2).unwrap(); assert_eq!( access.pairs().collect::, _>>().unwrap(), - vec![(0, 1337), (2, 9001)] + vec![(1, 1337), (3, 9001)] ); assert_eq!( access.keys().collect::, _>>().unwrap(), - vec![0, 2] + vec![1, 3] ); assert_eq!( @@ -556,16 +556,16 @@ mod tests { access.push(&1337).unwrap(); access.push(&42).unwrap(); access.push(&9001).unwrap(); - access.remove(1).unwrap(); + access.remove(2).unwrap(); assert_eq!( access.rev_pairs().collect::, _>>().unwrap(), - vec![(2, 9001), (0, 1337)] + vec![(3, 9001), (1, 1337)] ); assert_eq!( access.rev_keys().collect::, _>>().unwrap(), - vec![2, 0] + vec![3, 1] ); assert_eq!( @@ -586,26 +586,26 @@ mod tests { access.push(&9001).unwrap(); access.push(&1).unwrap(); access.push(&2).unwrap(); - access.remove(2).unwrap(); + access.remove(3).unwrap(); // start and end set assert_eq!( access - .bounded_pairs(Some(1), Some(4)) + .bounded_pairs(Some(2), Some(5)) .collect::, _>>() .unwrap(), - vec![(1, 42), (3, 1)] + vec![(2, 42), (4, 1)] ); assert_eq!( access - .bounded_keys(Some(1), Some(4)) + .bounded_keys(Some(2), Some(5)) .collect::, _>>() .unwrap(), - vec![1, 3] + vec![2, 4] ); assert_eq!( access - .bounded_values(Some(1), Some(4)) + .bounded_values(Some(2), Some(5)) .collect::, _>>() .unwrap(), vec![42, 1] @@ -614,21 +614,21 @@ mod tests { // end unset assert_eq!( access - .bounded_pairs(Some(1), None) + .bounded_pairs(Some(2), None) .collect::, _>>() .unwrap(), - vec![(1, 42), (3, 1), (4, 2)] + vec![(2, 42), (4, 1), (5, 2)] ); assert_eq!( access - .bounded_keys(Some(1), None) + .bounded_keys(Some(2), None) .collect::, _>>() .unwrap(), - vec![1, 3, 4] + vec![2, 4, 5] ); assert_eq!( access - .bounded_values(Some(1), None) + .bounded_values(Some(2), None) .collect::, _>>() .unwrap(), vec![42, 1, 2] @@ -637,21 +637,21 @@ mod tests { // start unset assert_eq!( access - .bounded_pairs(None, Some(4)) + .bounded_pairs(None, Some(5)) .collect::, _>>() .unwrap(), - vec![(0, 1337), (1, 42), (3, 1)] + vec![(1, 1337), (2, 42), (4, 1)] ); assert_eq!( access - .bounded_keys(None, Some(4)) + .bounded_keys(None, Some(5)) .collect::, _>>() .unwrap(), - vec![0, 1, 3] + vec![1, 2, 4] ); assert_eq!( access - .bounded_values(None, Some(4)) + .bounded_values(None, Some(5)) .collect::, _>>() .unwrap(), vec![1337, 42, 1] @@ -670,26 +670,26 @@ mod tests { access.push(&9001).unwrap(); access.push(&1).unwrap(); access.push(&2).unwrap(); - access.remove(2).unwrap(); + access.remove(3).unwrap(); // start and end set assert_eq!( access - .bounded_rev_pairs(Some(1), Some(4)) + .bounded_rev_pairs(Some(2), Some(5)) .collect::, _>>() .unwrap(), - vec![(3, 1), (1, 42)] + vec![(4, 1), (2, 42)] ); assert_eq!( access - .bounded_rev_keys(Some(1), Some(4)) + .bounded_rev_keys(Some(2), Some(5)) .collect::, _>>() .unwrap(), - vec![3, 1] + vec![4, 2] ); assert_eq!( access - .bounded_rev_values(Some(1), Some(4)) + .bounded_rev_values(Some(2), Some(5)) .collect::, _>>() .unwrap(), vec![1, 42] @@ -698,10 +698,10 @@ mod tests { // end unset assert_eq!( access - .bounded_rev_pairs(Some(1), None) + .bounded_rev_pairs(Some(2), None) .collect::, _>>() .unwrap(), - vec![(4, 2), (3, 1), (1, 42)] + vec![(5, 2), (4, 1), (2, 42)] ); } } diff --git a/packages/storey/tests/composition.rs b/packages/storey/tests/composition.rs index 25f9fb9..96c4fee 100644 --- a/packages/storey/tests/composition.rs +++ b/packages/storey/tests/composition.rs @@ -49,21 +49,21 @@ fn map_of_column() { access.entry_mut("foo").push(&42).unwrap(); access.entry_mut("bar").push(&9001).unwrap(); - assert_eq!(access.entry("foo").get(0).unwrap(), Some(1337)); - assert_eq!(access.entry("foo").get(1).unwrap(), Some(42)); - assert_eq!(access.entry("foo").get(2).unwrap(), None); + assert_eq!(access.entry("foo").get(1).unwrap(), Some(1337)); + assert_eq!(access.entry("foo").get(2).unwrap(), Some(42)); + assert_eq!(access.entry("foo").get(3).unwrap(), None); assert_eq!(access.entry("foo").len().unwrap(), 2); - assert_eq!(access.entry("bar").get(0).unwrap(), Some(9001)); + assert_eq!(access.entry("bar").get(1).unwrap(), Some(9001)); assert_eq!(access.entry("bar").len().unwrap(), 1); let all = access.pairs().collect::, _>>().unwrap(); assert_eq!( all, vec![ - (("bar".to_string(), 0), 9001), - (("foo".to_string(), 0), 1337), - (("foo".to_string(), 1), 42) + (("bar".to_string(), 1), 9001), + (("foo".to_string(), 1), 1337), + (("foo".to_string(), 2), 42) ] ); } From bd11f22528639bdb0d63e88233252f0623290c5e Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Wed, 30 Oct 2024 13:22:24 +0100 Subject: [PATCH 2/3] column: put lowest ix in a constant --- packages/storey/src/containers/column.rs | 35 +++++++++++++++--------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/storey/src/containers/column.rs b/packages/storey/src/containers/column.rs index 1bb1747..43079fe 100644 --- a/packages/storey/src/containers/column.rs +++ b/packages/storey/src/containers/column.rs @@ -10,11 +10,17 @@ use crate::storage::{Storage, StorageMut}; use super::common::TryGetError; use super::{BoundFor, BoundedIterableAccessor, IterableAccessor, NonTerminal, Storable}; -/// The last index that has been pushed to the column. -/// This does not have to be the index of the last element as it is -/// not reset in case the last element is removed. -const META_LAST_IX: &[u8] = &[0]; -const META_LEN: &[u8] = &[1]; +/// The first (lowest) index that is pushed to the column. +const FIRST_INDEX: u32 = 1; + +/// Storage keys for metadata. +mod meta_keys { + /// The last index that has been pushed to the column. + /// This does not have to be the index of the last element as it is + /// not reset in case the last element is removed. + pub const META_LAST_IX: &[u8] = &[0]; + pub const META_LEN: &[u8] = &[1]; +} /// A collection of rows indexed by `u32` keys. This is somewhat similar to a traditional /// database table with an auto-incrementing primary key. @@ -257,7 +263,7 @@ where // TODO: bounds check + error handlinge self.storage - .get_meta(META_LEN) + .get_meta(meta_keys::META_LEN) .map(|bytes| { if bytes.len() != 4 { Err(LenError::InconsistentState) @@ -331,22 +337,24 @@ where let ix = match self .storage - .get_meta(META_LAST_IX) + .get_meta(meta_keys::META_LAST_IX) .map(|bytes| u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) { Some(last_ix) => last_ix.checked_add(1).ok_or(PushError::IndexOverflow)?, - None => 1, + None => FIRST_INDEX, }; self.storage.set(&encode_ix(ix), &bytes); - self.storage.set_meta(META_LAST_IX, &(ix).to_be_bytes()); + self.storage + .set_meta(meta_keys::META_LAST_IX, &(ix).to_be_bytes()); let len = self .storage - .get_meta(META_LEN) + .get_meta(meta_keys::META_LEN) .map(|bytes| u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) .unwrap_or(0); - self.storage.set_meta(META_LEN, &(len + 1).to_be_bytes()); + self.storage + .set_meta(meta_keys::META_LEN, &(len + 1).to_be_bytes()); Ok(ix) } @@ -406,10 +414,11 @@ where let len = self .storage - .get_meta(META_LEN) + .get_meta(meta_keys::META_LEN) .map(|bytes| u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) .ok_or(RemoveError::InconsistentState)?; - self.storage.set_meta(META_LEN, &(len - 1).to_be_bytes()); + self.storage + .set_meta(meta_keys::META_LEN, &(len - 1).to_be_bytes()); Ok(()) } From 729e083293c5818e66c1d37dfa543faf53ab7fe0 Mon Sep 17 00:00:00 2001 From: Tomasz Kurcz Date: Wed, 30 Oct 2024 18:29:04 +0100 Subject: [PATCH 3/3] improve Column API docs --- packages/storey/src/containers/column.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/storey/src/containers/column.rs b/packages/storey/src/containers/column.rs index 43079fe..c93a255 100644 --- a/packages/storey/src/containers/column.rs +++ b/packages/storey/src/containers/column.rs @@ -319,18 +319,23 @@ where { /// Append a new value to the end of the column. /// + /// Returns the key of the newly inserted value. If the column is empty, the first + /// key will be `1`. + /// /// # Example /// ``` /// # use mocks::encoding::TestEncoding; /// # use mocks::backend::TestStorage; /// use storey::containers::Column; /// + /// const COLUMN_KEY: u8 = 0; + /// /// let mut storage = TestStorage::new(); - /// let column = Column::::new(0); + /// let column = Column::::new(COLUMN_KEY); /// let mut access = column.access(&mut storage); /// - /// access.push(&1337).unwrap(); - /// access.push(&42).unwrap(); + /// assert_eq!(access.push(&1337).unwrap(), 1); + /// assert_eq!(access.push(&42).unwrap(), 2); /// ``` pub fn push(&mut self, value: &T) -> Result> { let bytes = value.encode()?;