diff --git a/src/btreemap.rs b/src/btreemap.rs index 02f1f0f8..b5532522 100644 --- a/src/btreemap.rs +++ b/src/btreemap.rs @@ -1188,9 +1188,13 @@ where self.range_internal(key_range).into() } - /// Returns an iterator pointing to the first element below the given bound. - /// Returns an empty iterator if there are no keys below the given bound. - pub fn iter_upper_bound(&self, bound: &K) -> Iter { + /// Returns an iterator starting just before the given key. + /// + /// Finds the largest key strictly less than `bound` and starts from it. + /// Useful when `range(bound..)` skips the previous element. + /// + /// Returns an empty iterator if no smaller key exists. + pub fn iter_from_prev_key(&self, bound: &K) -> Iter { if let Some((start_key, _)) = self.range(..bound).next_back() { IterInternal::new_in_range(self, (Bound::Included(start_key), Bound::Unbounded)).into() } else { @@ -1198,6 +1202,19 @@ where } } + /// **Deprecated**: use [`iter_from_prev_key`] instead. + /// + /// The name `iter_upper_bound` was misleading — it suggested an inclusive + /// upper bound. In reality, it starts from the largest key strictly less + /// than the given bound. + /// + /// The new name, [`iter_from_prev_key`], better reflects this behavior and + /// improves code clarity. + #[deprecated(note = "use `iter_from_prev_key` instead")] + pub fn iter_upper_bound(&self, bound: &K) -> Iter { + self.iter_from_prev_key(bound) + } + /// Returns an iterator over the keys of the map. pub fn keys(&self) -> KeysIter { self.iter_internal().into() @@ -2955,28 +2972,28 @@ mod test { } btree_test!(test_bruteforce_range_search, bruteforce_range_search); - fn test_iter_upper_bound() { + fn test_iter_from_prev_key() { let (key, value) = (K::build, V::build); run_btree_test(|mut btree| { for j in 0..100 { btree.insert(key(j), value(j)); for i in 0..=j { assert_eq!( - btree.iter_upper_bound(&key(i + 1)).next(), + btree.iter_from_prev_key(&key(i + 1)).next(), Some((key(i), value(i))), "failed to get an upper bound for key({})", i + 1 ); } assert_eq!( - btree.iter_upper_bound(&key(0)).next(), + btree.iter_from_prev_key(&key(0)).next(), None, "key(0) must not have an upper bound" ); } }); } - btree_test!(test_test_iter_upper_bound, test_iter_upper_bound); + btree_test!(test_test_iter_from_prev_key, test_iter_from_prev_key); // A buggy implementation of storable where the max_size is smaller than the serialized size. #[derive(Clone, Ord, PartialOrd, Eq, PartialEq)] diff --git a/src/btreemap/proptests.rs b/src/btreemap/proptests.rs index 020f1bcb..16c7cc99 100644 --- a/src/btreemap/proptests.rs +++ b/src/btreemap/proptests.rs @@ -141,12 +141,12 @@ fn map_min_max(#[strategy(pvec(any::(), 10..100))] keys: Vec) { } #[proptest] -fn map_upper_bound_iter(#[strategy(pvec(0u64..u64::MAX -1 , 10..100))] keys: Vec) { +fn map_iter_from_prev_key(#[strategy(pvec(0u64..u64::MAX -1 , 10..100))] keys: Vec) { run_btree_test(|mut map| { for k in keys.iter() { map.insert(*k, ()); - prop_assert_eq!(Some((*k, ())), map.iter_upper_bound(&(k + 1)).next()); + prop_assert_eq!(Some((*k, ())), map.iter_from_prev_key(&(k + 1)).next()); } Ok(()) diff --git a/src/btreeset.rs b/src/btreeset.rs index 8be5ba99..b356d346 100644 --- a/src/btreeset.rs +++ b/src/btreeset.rs @@ -510,31 +510,6 @@ where Iter::new(self.map.range(key_range)) } - /// Returns an iterator pointing to the first element strictly below the given bound. - /// Returns an empty iterator if there are no keys strictly below the given bound. - /// - /// # Complexity - /// O(log n) for creating the iterator, where n is the number of elements in the set. - /// - /// # Example - /// - /// ```rust - /// use ic_stable_structures::{BTreeSet, DefaultMemoryImpl}; - /// use ic_stable_structures::memory_manager::{MemoryId, MemoryManager}; - /// - /// let mem_mgr = MemoryManager::init(DefaultMemoryImpl::default()); - /// let mut set: BTreeSet = BTreeSet::new(mem_mgr.get(MemoryId::new(0))); - /// set.insert(1); - /// set.insert(2); - /// set.insert(3); - /// - /// let upper_bound: Option = set.iter_upper_bound(&3).next(); - /// assert_eq!(upper_bound, Some(2)); - /// ``` - pub fn iter_upper_bound(&self, bound: &K) -> Iter { - Iter::new(self.map.iter_upper_bound(bound)) - } - /// Returns an iterator over the union of this set and another. /// /// The union of two sets is a set containing all elements that are in either set. @@ -1110,31 +1085,6 @@ mod test { assert!(!btreeset.contains(&1u32)); } - #[test] - fn test_iter_upper_bound() { - let mem = make_memory(); - let mut btreeset = BTreeSet::new(mem); - - for i in 0u32..100 { - btreeset.insert(i); - - // Test that `iter_upper_bound` returns the largest element strictly below the bound. - for j in 1u32..=i { - assert_eq!( - btreeset.iter_upper_bound(&(j + 1)).next(), - Some(j), - "failed to get an upper bound for {}", - j + 1 - ); - } - assert_eq!( - btreeset.iter_upper_bound(&0).next(), - None, - "0 must not have an upper bound" - ); - } - } - #[test] fn test_iter() { let mem = make_memory(); @@ -1234,20 +1184,6 @@ mod test { assert_eq!(elements[999], 999); } - #[test] - fn test_iter_upper_bound_large_set() { - let mem = make_memory(); - let mut btreeset: BTreeSet = BTreeSet::new(mem); - - for i in 0u32..1000 { - btreeset.insert(i); - } - - assert_eq!(btreeset.iter_upper_bound(&500).next(), Some(499)); - assert_eq!(btreeset.iter_upper_bound(&0).next(), None); - assert_eq!(btreeset.iter_upper_bound(&1000).next(), Some(999)); - } - #[test] fn test_range_large_set() { let mem = make_memory(); @@ -1304,14 +1240,6 @@ mod test { assert_eq!(btreeset.pop_last(), None); } - #[test] - fn test_iter_upper_bound_empty() { - let mem = make_memory(); - let btreeset: BTreeSet = BTreeSet::new(mem); - - assert_eq!(btreeset.iter_upper_bound(&42u32).next(), None); - } - #[test] fn test_range_empty() { let mem = make_memory(); @@ -1414,20 +1342,6 @@ mod test { assert!(btreeset.is_empty()); } - #[test] - fn test_iter_upper_bound_edge_cases() { - let mem = make_memory(); - let mut btreeset: BTreeSet = BTreeSet::new(mem); - - for i in 1..=10 { - btreeset.insert(i); - } - - assert_eq!(btreeset.iter_upper_bound(&1).next(), None); // No element strictly below 1 - assert_eq!(btreeset.iter_upper_bound(&5).next(), Some(4)); // Largest element below 5 - assert_eq!(btreeset.iter_upper_bound(&11).next(), Some(10)); // Largest element below 11 - } - #[test] fn test_is_disjoint_with_disjoint_sets() { let mem1 = make_memory(); diff --git a/src/btreeset/proptests.rs b/src/btreeset/proptests.rs index 0a2b4978..a698b9fb 100644 --- a/src/btreeset/proptests.rs +++ b/src/btreeset/proptests.rs @@ -67,19 +67,6 @@ fn set_min_max(#[strategy(pvec(any::(), 10..100))] keys: Vec) { }); } -#[proptest] -fn set_upper_bound_iter(#[strategy(pvec(0u64..u64::MAX - 1, 10..100))] keys: Vec) { - crate::btreeset::test::run_btree_test(|mut set| { - for k in keys.iter() { - set.insert(*k); - - prop_assert_eq!(Some(*k), set.iter_upper_bound(&(k + 1)).next()); - } - - Ok(()) - }); -} - // Given an operation, executes it on the given stable btreeset and standard btreeset, verifying // that the result of the operation is equal in both btrees. fn execute_operation( diff --git a/src/lib.rs b/src/lib.rs index e528dd4b..e096b11e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,12 +3,11 @@ mod base_vec; pub mod btreemap; pub mod cell; pub use cell::{Cell as StableCell, Cell}; +pub mod btreeset; pub mod file_mem; #[cfg(target_arch = "wasm32")] mod ic0_memory; // Memory API for canisters. pub mod log; -pub use log::{Log as StableLog, Log}; -pub mod btreeset; pub mod memory_manager; pub mod min_heap; pub mod reader; @@ -17,20 +16,22 @@ pub mod storable; mod tests; mod types; pub mod vec; -pub use min_heap::{MinHeap, MinHeap as StableMinHeap}; -pub use vec::{Vec as StableVec, Vec}; pub mod vec_mem; pub mod writer; + pub use btreemap::{BTreeMap, BTreeMap as StableBTreeMap}; pub use btreeset::{BTreeSet, BTreeSet as StableBTreeSet}; pub use file_mem::FileMemory; #[cfg(target_arch = "wasm32")] pub use ic0_memory::Ic0StableMemory; +pub use log::{Log as StableLog, Log}; +pub use min_heap::{MinHeap, MinHeap as StableMinHeap}; use std::error; use std::fmt::{Display, Formatter}; use std::mem::MaybeUninit; pub use storable::Storable; use types::Address; +pub use vec::{Vec as StableVec, Vec}; pub use vec_mem::VectorMemory; #[cfg(target_arch = "wasm32")] diff --git a/tests/api_confomrance.rs b/tests/api_confomrance.rs new file mode 100644 index 00000000..7b5cb568 --- /dev/null +++ b/tests/api_confomrance.rs @@ -0,0 +1,325 @@ +use ic_stable_structures::btreemap::BTreeMap; +use ic_stable_structures::btreeset::BTreeSet; +use ic_stable_structures::min_heap::MinHeap; +use ic_stable_structures::vec::Vec as StableVec; +use std::cell::RefCell; +use std::cmp::Reverse; +use std::collections::BinaryHeap; +use std::rc::Rc; + +pub fn make_memory() -> Rc>> { + Rc::new(RefCell::new(Vec::new())) +} + +#[test] +fn api_conformance_btreemap() { + let mem = make_memory(); + let mut stable = BTreeMap::new(mem); + let mut std = std::collections::BTreeMap::new(); + let n = 10_u32; + + // Insert keys and values. + for i in 0..n { + stable.insert(i, format!("{i}")); + std.insert(i, format!("{i}")); + } + + // Get and contains. + // Note: stable.get returns owned value (Option), std.get returns reference (Option<&V>). + assert_eq!(stable.get(&1), std.get(&1).cloned()); + assert_eq!(stable.contains_key(&1), std.contains_key(&1)); + + // Remove. + // Same return behavior as get: stable returns owned value, std returns owned too. + assert_eq!(stable.remove(&1), std.remove(&1)); + + // Length and is_empty. + // Note: stable.len() returns u64, std.len() returns usize. + assert_eq!(stable.len(), std.len() as u64); + assert_eq!(stable.is_empty(), std.is_empty()); + + // Clear. + // Note: stable uses clear_new(); std uses clear(). + stable.clear_new(); + std.clear(); + assert_eq!(stable.len(), std.len() as u64); + assert_eq!(stable.is_empty(), std.is_empty()); + + // Re-insert to test iteration-related methods. + for i in 0..n { + let v = i.to_string(); + stable.insert(i, v.clone()); + std.insert(i, v); + } + + // Iterators. + // Note: stable.iter() yields (K, V), std.iter() yields (&K, &V) + let stable_items: std::vec::Vec<_> = stable.iter().collect(); + let std_items: std::vec::Vec<_> = std.iter().map(|(k, v)| (*k, v.clone())).collect(); + assert_eq!(stable_items, std_items); + + let stable_keys: std::vec::Vec<_> = stable.keys().collect(); + let std_keys: std::vec::Vec<_> = std.keys().copied().collect(); + assert_eq!(stable_keys, std_keys); + + let stable_values: std::vec::Vec<_> = stable.values().collect(); + let std_values: std::vec::Vec<_> = std.values().cloned().collect(); + assert_eq!(stable_values, std_values); + + // First and last. + // Note: stable returns (K, V), std returns (&K, &V) + let stable_first = stable.first_key_value(); + let std_first = std.first_key_value().map(|(k, v)| (*k, v.clone())); + assert_eq!(stable_first, std_first); + + let stable_last = stable.last_key_value(); + let std_last = std.last_key_value().map(|(k, v)| (*k, v.clone())); + assert_eq!(stable_last, std_last); + + // Pop first and last. + // Note: both stable and std return Option<(K, V)> + let stable_pop_first = stable.pop_first(); + let std_pop_first = std.pop_first(); + assert_eq!(stable_pop_first, std_pop_first); + + let stable_pop_last = stable.pop_last(); + let std_pop_last = std.pop_last(); + assert_eq!(stable_pop_last, std_pop_last); + + // Range. + let range_start = 3; + let range_end = 7; + + let stable_range: std::vec::Vec<_> = stable.range(range_start..range_end).collect(); + let std_range: std::vec::Vec<_> = std + .range(range_start..range_end) + .map(|(k, v)| (*k, v.clone())) + .collect(); + assert_eq!(stable_range, std_range); + + // keys_range + // Note: stable has a dedicated keys_range() method; std uses map.range().map(|(k, _)| ...) + let stable_keys_range: std::vec::Vec<_> = stable.keys_range(range_start..range_end).collect(); + let std_keys_range: std::vec::Vec<_> = + std.range(range_start..range_end).map(|(k, _)| *k).collect(); + assert_eq!(stable_keys_range, std_keys_range); + + // values_range + // Note: stable has a dedicated values_range() method; std uses map.range().map(|(_, v)| ...) + let stable_values_range: std::vec::Vec<_> = + stable.values_range(range_start..range_end).collect(); + let std_values_range: std::vec::Vec<_> = std + .range(range_start..range_end) + .map(|(_, v)| v.clone()) + .collect(); + assert_eq!(stable_values_range, std_values_range); + + // iter_from_prev_key + // Note: stable.iter_from_prev_key(bound) is a custom method. + // It starts from the largest key strictly less than `bound`, and continues forward. + // To simulate in std: use .range(..bound).next_back() to get the largest < bound, + // then iterate from that key forward using .range(start..). + let bound = 5; + let stable_result: std::vec::Vec<_> = stable.iter_from_prev_key(&bound).collect(); + let std_result: std::vec::Vec<_> = if let Some((start, _)) = std.range(..bound).next_back() { + std.range(start..).map(|(k, v)| (*k, v.clone())).collect() + } else { + Vec::new() + }; + assert_eq!(stable_result, std_result); +} + +#[test] +fn api_conformance_btreeset() { + let mem = make_memory(); + let mut stable = BTreeSet::new(mem); + let mut std = std::collections::BTreeSet::new(); + let n = 10_u32; + + // Insert elements. + for i in 0..n { + assert_eq!(stable.insert(i), std.insert(i)); + } + + // Contains. + for i in 0..n { + assert_eq!(stable.contains(&i), std.contains(&i)); + } + + // is_empty and len. + // Note: stable.len() returns u64, std.len() returns usize. + assert_eq!(stable.is_empty(), std.is_empty()); + assert_eq!(stable.len(), std.len() as u64); + + // First and last. + // Note: stable.first()/last() returns Option, std returns Option<&T>. + assert_eq!(stable.first(), std.first().copied()); + assert_eq!(stable.last(), std.last().copied()); + + // Iteration. + // Note: stable.iter() yields T, std.iter() yields &T. + let stable_items: std::vec::Vec<_> = stable.iter().collect(); + let std_items: std::vec::Vec<_> = std.iter().copied().collect(); + assert_eq!(stable_items, std_items); + + // Range. + let range_start = 3; + let range_end = 7; + let stable_range: std::vec::Vec<_> = stable.range(range_start..range_end).collect(); + let std_range: std::vec::Vec<_> = std.range(range_start..range_end).copied().collect(); + assert_eq!(stable_range, std_range); + + // pop_first / pop_last. + let mem2 = make_memory(); + let mut stable_temp = BTreeSet::new(mem2); + let mut std_temp = std::collections::BTreeSet::new(); + for i in 0..n { + assert_eq!(stable_temp.insert(i), std_temp.insert(i)); + } + + assert_eq!(stable_temp.pop_first(), std_temp.pop_first()); + assert_eq!(stable_temp.pop_last(), std_temp.pop_last()); + + // Remove elements. + for i in 0..n { + assert_eq!(stable.remove(&i), std.remove(&i)); + } + assert!(stable.is_empty()); + assert!(std.is_empty()); + + // Clear. + for i in 0..n { + stable.insert(i); + std.insert(i); + } + stable.clear(); + std.clear(); + assert!(stable.is_empty()); + assert!(std.is_empty()); + + // Reinsert for set operations. + for i in 0..n { + if i % 2 == 0 { + stable.insert(i); + std.insert(i); + } + } + + let mem2 = make_memory(); + let mut stable2 = BTreeSet::new(mem2); + let mut std2 = std::collections::BTreeSet::new(); + + for i in 0..n { + if i % 3 == 0 { + stable2.insert(i); + std2.insert(i); + } + } + + // is_disjoint, is_subset, is_superset. + assert_eq!(stable.is_disjoint(&stable2), std.is_disjoint(&std2)); + assert_eq!(stable.is_subset(&stable2), std.is_subset(&std2)); + assert_eq!(stable.is_superset(&stable2), std.is_superset(&std2)); + + // union + let stable_union: std::vec::Vec<_> = stable.union(&stable2).collect(); + let std_union: std::vec::Vec<_> = std.union(&std2).copied().collect(); + assert_eq!(stable_union, std_union); + + // intersection + let stable_inter: std::vec::Vec<_> = stable.intersection(&stable2).collect(); + let std_inter: std::vec::Vec<_> = std.intersection(&std2).copied().collect(); + assert_eq!(stable_inter, std_inter); + + // symmetric_difference + let stable_diff: std::vec::Vec<_> = stable.symmetric_difference(&stable2).collect(); + let std_diff: std::vec::Vec<_> = std.symmetric_difference(&std2).copied().collect(); + assert_eq!(stable_diff, std_diff); +} + +#[test] +fn api_conformance_min_heap() { + let mem = make_memory(); + let mut stable = MinHeap::new(mem).unwrap(); + let mut std = BinaryHeap::new(); + let n = 10_u32; + + // Push elements. + for i in 0..n { + stable.push(&i).expect("push failed"); + std.push(Reverse(i)); + } + + // Length and is_empty. + // Note: stable.len() returns u64, std.len() returns usize. + assert_eq!(stable.len(), std.len() as u64); + assert_eq!(stable.is_empty(), std.is_empty()); + + // Peek (min element). + // Note: stable.peek() returns Option, std.peek() returns Option<&Reverse>. + assert_eq!(stable.peek(), std.peek().map(|r| r.0)); + + // Pop all elements, should match in ascending order. + let mut stable_popped = Vec::new(); + let mut std_popped = Vec::new(); + while let Some(v) = stable.pop() { + stable_popped.push(v); + } + while let Some(Reverse(v)) = std.pop() { + std_popped.push(v); + } + assert_eq!(stable_popped, std_popped); + + // After popping everything, both should be empty. + assert_eq!(stable.len(), 0); + assert!(stable.is_empty()); + assert!(std.is_empty()); +} + +#[test] +fn api_conformance_vec() { + let mem = make_memory(); + let stable = StableVec::new(mem).unwrap(); + let mut std = Vec::new(); + let n = 10_u32; + + // Push elements. + for i in 0..n { + stable.push(&i).expect("push failed"); + std.push(i); + } + + // Length and is_empty. + // Note: stable.len() returns u64, std.len() returns usize. + assert_eq!(stable.len(), std.len() as u64); + assert_eq!(stable.is_empty(), std.is_empty()); + + // Get by index. + // Note: stable.get returns Option<&T>, std returns Option<&T>. + for i in 0..n { + assert_eq!(stable.get(i as u64), Some(std[i as usize])); + } + + // Set by index. + // Note: stable.set uses &T, std uses indexing. + for i in 0..n { + let value = i * 10; + stable.set(i as u64, &value); + std[i as usize] = value; + } + + // Confirm updated values. + for i in 0..n { + assert_eq!(stable.get(i as u64), Some(std[i as usize])); + } + + // Pop elements. + // Note: stable.pop() and std.pop() both return Option. + for _ in 0..n { + assert_eq!(stable.pop(), std.pop()); + } + + // After popping everything, both should be empty. + assert_eq!(stable.len(), std.len() as u64); + assert_eq!(stable.is_empty(), std.is_empty()); +}