Merge pull request #25 from tweedegolf/map-remove

Implement map remove feature

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 ## Unreleased
 
+## 0.9.1 13-02-24
+
+- Added `remove_item` to map
+
 ## 0.9.0 11-02-24
 
 - *Breaking:* Storage item key must now also be clone

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sequential-storage"
-version = "0.9.0"
+version = "0.9.1"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A crate for storing data in flash with minimal erase cycles."

fuzz/fuzz_targets/map.rs

@@ -34,6 +34,7 @@ struct Input {
 enum Op {
     Store(StoreOp),
     Fetch(u8),
+    Remove(u8),
 }
 
 #[derive(Arbitrary, Debug, Clone)]
@@ -107,7 +108,11 @@ enum CacheType {
 
 fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8>) {
     let mut flash = MockFlashBase::<PAGES, WORD_SIZE, WORDS_PER_PAGE>::new(
-        WriteCountCheck::OnceOnly,
+        if ops.ops.iter().any(|op| matches!(op, Op::Remove(_))) {
+            WriteCountCheck::Twice
+        } else {
+            WriteCountCheck::OnceOnly
+        },
         Some(ops.fuel as u32),
         true,
     );
@@ -243,6 +248,62 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8>) {
                         Err(e) => panic!("{e:?}"),
                     }
                 }
+                Op::Remove(key) => {
+                    match block_on(sequential_storage::map::remove_item::<TestItem, _>(
+                        &mut flash,
+                        FLASH_RANGE,
+                        &mut cache,
+                        &mut buf.0,
+                        key,
+                    )) {
+                        Ok(()) => {
+                            map.remove(&key);
+                        }
+                        Err(MapError::Storage {
+                            value: MockFlashError::EarlyShutoff(_),
+                            backtrace: _backtrace,
+                        }) => {
+                            match block_on(sequential_storage::map::fetch_item::<TestItem, _>(
+                                &mut flash,
+                                FLASH_RANGE,
+                                &mut cache,
+                                &mut buf.0,
+                                key,
+                            )) {
+                                Ok(Some(_)) => {
+                                    #[cfg(fuzzing_repro)]
+                                    eprintln!("Early shutoff when removing item {key}! Originated from:\n{_backtrace:#}");
+                                }
+                                _ => {
+                                    // Could not fetch the item we stored...
+                                    #[cfg(fuzzing_repro)]
+                                    eprintln!("Early shutoff when removing item {key}! (but it still removed fully). Originated from:\n{_backtrace:#}");
+                                    // Even though we got a shutoff, it still managed to store well
+                                    map.remove(&key);
+                                }
+                            }
+                        }
+                        Err(MapError::Corrupted {
+                            backtrace: _backtrace,
+                        }) if !corruption_repaired => {
+                            #[cfg(fuzzing_repro)]
+                            eprintln!(
+                                "### Encountered curruption while fetching! Repairing now. Originated from:\n{_backtrace:#}"
+                            );
+
+                            block_on(sequential_storage::map::try_repair::<TestItem, _>(
+                                &mut flash,
+                                FLASH_RANGE,
+                                &mut cache,
+                                &mut buf.0,
+                            ))
+                            .unwrap();
+                            corruption_repaired = true;
+                            retry = true;
+                        }
+                        Err(e) => panic!("{e:?}"),
+                    }
+                }
             }
         }
     }

src/item.rs

@@ -47,6 +47,9 @@ impl ItemHeader {
     const LENGTH_FIELD: Range<usize> = 4..6;
     const LENGTH_CRC_FIELD: Range<usize> = 6..8;
 
+    /// Read the header from the flash at the given address.
+    ///
+    /// If the item doesn't exist or doesn't fit between the address and the end address, none is returned.
     pub async fn read_new<S: NorFlash>(
         flash: &mut S,
         address: u32,
@@ -160,6 +163,7 @@ impl ItemHeader {
             })
     }
 
+    /// Erase this item by setting the crc to none and overwriting the header with it
     pub async fn erase_data<S: MultiwriteNorFlash>(
         mut self,
         flash: &mut S,
@@ -173,10 +177,12 @@ impl ItemHeader {
         Ok(self)
     }
 
+    /// Get the address of the start of the data for this item
     pub fn data_address<S: NorFlash>(address: u32) -> u32 {
         address + round_up_to_alignment::<S>(Self::LENGTH as u32)
     }
 
+    /// Get the location of the next item in flash
     pub fn next_item_address<S: NorFlash>(&self, address: u32) -> u32 {
         let data_address = ItemHeader::data_address::<S>(address);
         data_address + round_up_to_alignment::<S>(self.length as u32)

src/map.rs

@@ -115,9 +115,14 @@
 //! # });
 //! ```
 
+use embedded_storage_async::nor_flash::MultiwriteNorFlash;
+
 use crate::item::{find_next_free_item_spot, Item, ItemHeader, ItemIter};
 
-use self::cache::{KeyCacheImpl, PrivateKeyCacheImpl};
+use self::{
+    cache::{KeyCacheImpl, PrivateKeyCacheImpl},
+    item::ItemHeaderIter,
+};
 
 use super::*;
 
@@ -476,6 +481,87 @@ pub async fn store_item<I: StorageItem, S: NorFlash>(
     }
 }
 
+/// Fully remove an item. Additional calls to fetch with the same key will return None until
+/// a new one is stored again.
+///
+/// <div class="warning">
+/// This is really slow!
+///
+/// All items in flash have to be read and deserialized to find the items with the key.
+/// This is unlikely to be cached well.
+/// </div>
+pub async fn remove_item<I: StorageItem, S: MultiwriteNorFlash>(
+    flash: &mut S,
+    flash_range: Range<u32>,
+    mut cache: impl KeyCacheImpl<I::Key>,
+    data_buffer: &mut [u8],
+    search_key: I::Key,
+) -> Result<(), MapError<I::Error, S::Error>> {
+    cache.notice_key_erased(&search_key);
+
+    // Search for the last used page. We're gonna erase from the one after this first.
+    // If we get an early shutoff or cancellation, this will make it so that we don't return
+    // an old version of the key on the next fetch.
+    let last_used_page = find_first_page(
+        flash,
+        flash_range.clone(),
+        &mut cache,
+        0,
+        PageState::PartialOpen,
+    )
+    .await?
+    .unwrap_or_default();
+
+    // Go through all the pages
+    for page_index in get_pages::<S>(
+        flash_range.clone(),
+        next_page::<S>(flash_range.clone(), last_used_page),
+    ) {
+        if get_page_state(flash, flash_range.clone(), &mut cache, page_index)
+            .await?
+            .is_open()
+        {
+            // This page is open, we don't have to check it
+            continue;
+        }
+
+        let page_data_start_address =
+            calculate_page_address::<S>(flash_range.clone(), page_index) + S::WORD_SIZE as u32;
+        let page_data_end_address =
+            calculate_page_end_address::<S>(flash_range.clone(), page_index) - S::WORD_SIZE as u32;
+
+        // Go through all items on the page
+        let mut item_headers = ItemHeaderIter::new(page_data_start_address, page_data_end_address);
+
+        while let (Some(item_header), item_address) = item_headers.next(flash).await? {
+            let item = item_header
+                .read_item(flash, data_buffer, item_address, page_data_end_address)
+                .await?;
+
+            match item {
+                item::MaybeItem::Corrupted(_, _) => continue,
+                item::MaybeItem::Erased(_, _) => continue,
+                item::MaybeItem::Present(item) => {
+                    let item_key = I::deserialize_key_only(item.data()).map_err(MapError::Item)?;
+
+                    // If this item has the same key as the key we're trying to erase, then erase the item.
+                    // But keep going! We need to erase everything.
+                    if item_key == search_key {
+                        item.header
+                            .erase_data(flash, flash_range.clone(), &mut cache, item_address)
+                            .await?;
+                    }
+                }
+            }
+        }
+    }
+
+    // We're done, we now know the cache is in a good state
+    cache.unmark_dirty();
+
+    Ok(())
+}
+
 /// A way of serializing and deserializing items in the storage.
 ///
 /// Serialized items must not be 0 bytes and may not be longer than [u16::MAX].
@@ -1134,4 +1220,87 @@ mod tests {
             assert_eq!(item.value, vec![i as u8; LENGHT_PER_KEY[i]]);
         }
     }
+
+    #[test]
+    async fn remove_items() {
+        let mut flash = mock_flash::MockFlashBase::<4, 1, 4096>::new(
+            mock_flash::WriteCountCheck::Twice,
+            None,
+            true,
+        );
+        let mut data_buffer = AlignedBuf([0; 128]);
+        const FLASH_RANGE: Range<u32> = 0x0000..0x4000;
+
+        // Add some data to flash
+        for j in 0..10 {
+            for i in 0..24 {
+                let item = MockStorageItem {
+                    key: i as u8,
+                    value: vec![i as u8; j + 2],
+                };
+
+                store_item::<_, _>(
+                    &mut flash,
+                    FLASH_RANGE,
+                    cache::NoCache::new(),
+                    &mut data_buffer,
+                    &item,
+                )
+                .await
+                .unwrap();
+            }
+        }
+
+        for j in (0..24).rev() {
+            // Are all things still in flash that we expect?
+            for i in 0..=j {
+                assert!(fetch_item::<MockStorageItem, _>(
+                    &mut flash,
+                    FLASH_RANGE,
+                    cache::NoCache::new(),
+                    &mut data_buffer,
+                    i
+                )
+                .await
+                .unwrap()
+                .is_some());
+            }
+
+            // Remove the item
+            remove_item::<MockStorageItem, _>(
+                &mut flash,
+                FLASH_RANGE,
+                cache::NoCache::new(),
+                &mut data_buffer,
+                j,
+            )
+            .await
+            .unwrap();
+
+            // Are all things still in flash that we expect?
+            for i in 0..j {
+                assert!(fetch_item::<MockStorageItem, _>(
+                    &mut flash,
+                    FLASH_RANGE,
+                    cache::NoCache::new(),
+                    &mut data_buffer,
+                    i
+                )
+                .await
+                .unwrap()
+                .is_some());
+            }
+
+            assert!(fetch_item::<MockStorageItem, _>(
+                &mut flash,
+                FLASH_RANGE,
+                cache::NoCache::new(),
+                &mut data_buffer,
+                j
+            )
+            .await
+            .unwrap()
+            .is_none());
+        }
+    }
 }