Repository: earth-mover/icechunk (language: python)

Issue:
Deleting nodes is not working properly
# Steps to reproduce
These 3 tests fail
```rust
    #[tokio::test]
    async fn test_basic_delete_after_flush() -> Result<(), Box<dyn Error>> {
        let storage: Arc<dyn Storage + Send + Sync> =
            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
        ds.add_group("/".into()).await?;
        ds.add_group("/1".into()).await?;
        ds.commit("main", "commit", None).await?;

        ds.delete_group("/1".into()).await?;
        assert!(ds.get_group(&"/".into()).await.is_ok());
        assert!(ds.get_group(&"/1".into()).await.is_err());
        assert_eq!(ds.list_nodes().await?.count(), 1);
        Ok(())
    }

    #[tokio::test]
    async fn test_commit_after_deleting_old_node() -> Result<(), Box<dyn Error>> {
        let storage: Arc<dyn Storage + Send + Sync> =
            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
        ds.add_group("/".into()).await?;
        ds.commit("main", "commit", None).await?;
        ds.delete_group("/".into()).await?;
        ds.commit("main", "commit", None).await?;
        assert_eq!(ds.list_nodes().await?.count(), 0);
        Ok(())
    }

    #[tokio::test]
    async fn test_delete_children() -> Result<(), Box<dyn Error>> {
        let storage: Arc<dyn Storage + Send + Sync> =
            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
        ds.add_group("/".into()).await?;
        ds.add_group("/a".into()).await?;
        ds.add_group("/b".into()).await?;
        ds.add_group("/b/bb".into()).await?;
        ds.delete_group("/b".into()).await?;
        assert!(ds.get_group(&"/b".into()).await.is_err());
        assert!(ds.get_group(&"/b/bb".into()).await.is_err());
        Ok(())
    }
```
# Why
* For first and second test cases: the nodes iterators have to check for deleted nodes before yielding them.
* For the third test case: we are not (currently) using a tree structure to host the nodes, so we need to iterate over children deleting them
Better `Path` type.
We were using `PathBuf`, which has platform dependent behavior. Now we use a platform independent type, and verify correctness using a smart constructor.

Closes #149 


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 596e608e..52b10eb6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1220,6 +1220,7 @@ dependencies = [
  "test-strategy",
  "thiserror",
  "tokio",
+ "typed-path",
  "url",
 ]
 
@@ -2413,6 +2414,12 @@ version = "0.2.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b"
 
+[[package]]
+name = "typed-path"
+version = "0.9.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "50c0c7479c430935701ff2532e3091e6680ec03f2f89ffcd9988b08e885b90a5"
+
 [[package]]
 name = "typenum"
 version = "1.17.0"
diff --git a/icechunk/Cargo.toml b/icechunk/Cargo.toml
index 52713ee2..5fe84b05 100644
--- a/icechunk/Cargo.toml
+++ b/icechunk/Cargo.toml
@@ -31,6 +31,7 @@ rmpv = { version = "1.3.0", features = ["serde", "with-serde"] }
 aws-sdk-s3 = "1.53.0"
 aws-config = "1.5.7"
 aws-credential-types = "1.2.1"
+typed-path = "0.9.2"
 
 [dev-dependencies]
 pretty_assertions = "1.4.1"
diff --git a/icechunk/examples/low_level_dataset.rs b/icechunk/examples/low_level_dataset.rs
index 097a07c3..063ad193 100644
--- a/icechunk/examples/low_level_dataset.rs
+++ b/icechunk/examples/low_level_dataset.rs
@@ -49,9 +49,9 @@ ds.add_group("/group2".into()).await?;
 "#,
     );
 
-    ds.add_group("/".into()).await?;
-    ds.add_group("/group1".into()).await?;
-    ds.add_group("/group2".into()).await?;
+    ds.add_group(Path::root()).await?;
+    ds.add_group("/group1".try_into().unwrap()).await?;
+    ds.add_group("/group2".try_into().unwrap()).await?;
 
     println!();
     print_nodes(&ds).await?;
@@ -129,7 +129,7 @@ ds.add_array(array1_path.clone(), zarr_meta1).await?;
             Some("t".to_string()),
         ]),
     };
-    let array1_path: Path = "/group1/array1".into();
+    let array1_path: Path = "/group1/array1".try_into().unwrap();
     ds.add_array(array1_path.clone(), zarr_meta1).await?;
     println!();
     print_nodes(&ds).await?;
@@ -292,7 +292,7 @@ async fn print_nodes(ds: &Repository) -> Result<(), StoreError> {
             format!(
                 "|{:10?}|{:15}|{:10?}\n",
                 node.node_type(),
-                node.path.to_str().unwrap(),
+                node.path.to_string(),
                 node.user_attributes,
             )
         })
diff --git a/icechunk/src/change_set.rs b/icechunk/src/change_set.rs
index 6306b618..e91c09e0 100644
--- a/icechunk/src/change_set.rs
+++ b/icechunk/src/change_set.rs
@@ -27,8 +27,8 @@ pub struct ChangeSet {
     updated_attributes: HashMap<NodeId, Option<UserAttributes>>,
     // FIXME: issue with too many inline chunks kept in mem
     set_chunks: HashMap<NodeId, HashMap<ChunkIndices, Option<ChunkPayload>>>,
-    deleted_groups: HashSet<NodeId>,
-    deleted_arrays: HashSet<NodeId>,
+    deleted_groups: HashSet<Path>,
+    deleted_arrays: HashSet<Path>,
 }
 
 impl ChangeSet {
@@ -48,14 +48,43 @@ impl ChangeSet {
         self.new_arrays.get(path)
     }
 
-    pub fn delete_group(&mut self, path: &Path, node_id: NodeId) {
-        let new_node_id = self.new_groups.remove(path);
-        let is_new_group = new_node_id.is_some();
-        debug_assert!(!is_new_group || new_node_id == Some(node_id));
-
+    pub fn delete_group(&mut self, path: Path, node_id: NodeId) {
         self.updated_attributes.remove(&node_id);
-        if !is_new_group {
-            self.deleted_groups.insert(node_id);
+        match self.new_groups.remove(&path) {
+            Some(deleted_node_id) => {
+                // the group was created in this session
+                // so we delete it directly, no need to flag as deleted
+                debug_assert!(deleted_node_id == node_id);
+                self.delete_children(&path);
+            }
+            None => {
+                // it's an old group, we need to flag it as deleted
+                self.deleted_groups.insert(path);
+            }
+        }
+    }
+
+    fn delete_children(&mut self, path: &Path) {
+        let groups_to_delete: Vec<_> = self
+            .new_groups
+            .iter()
+            .filter(|(child_path, _)| child_path.starts_with(path))
+            .map(|(k, v)| (k.clone(), *v))
+            .collect();
+
+        for (path, node) in groups_to_delete {
+            self.delete_group(path, node);
+        }
+
+        let arrays_to_delete: Vec<_> = self
+            .new_arrays
+            .iter()
+            .filter(|(child_path, _)| child_path.starts_with(path))
+            .map(|(k, (node, _))| (k.clone(), *node))
+            .collect();
+
+        for (path, node) in arrays_to_delete {
+            self.delete_array(path, node);
         }
     }
 
@@ -72,10 +101,10 @@ impl ChangeSet {
         self.updated_arrays.insert(node_id, metadata);
     }
 
-    pub fn delete_array(&mut self, path: &Path, node_id: NodeId) {
+    pub fn delete_array(&mut self, path: Path, node_id: NodeId) {
         // if deleting a new array created in this session, just remove the entry
         // from new_arrays
-        let node_and_meta = self.new_arrays.remove(path);
+        let node_and_meta = self.new_arrays.remove(&path);
         let is_new_array = node_and_meta.is_some();
         debug_assert!(!is_new_array || node_and_meta.map(|n| n.0) == Some(node_id));
 
@@ -83,12 +112,14 @@ impl ChangeSet {
         self.updated_attributes.remove(&node_id);
         self.set_chunks.remove(&node_id);
         if !is_new_array {
-            self.deleted_arrays.insert(node_id);
+            self.deleted_arrays.insert(path);
         }
     }
 
-    pub fn is_deleted(&self, node_id: &NodeId) -> bool {
-        self.deleted_groups.contains(node_id) || self.deleted_arrays.contains(node_id)
+    pub fn is_deleted(&self, path: &Path) -> bool {
+        self.deleted_groups.contains(path)
+            || self.deleted_arrays.contains(path)
+            || path.ancestors().skip(1).any(|parent| self.is_deleted(&parent))
     }
 
     pub fn has_updated_attributes(&self, node_id: &NodeId) -> bool {
@@ -309,29 +340,32 @@ impl ChangeSet {
         &self,
         node: NodeSnapshot,
         new_manifests: Option<Vec<ManifestRef>>,
-    ) -> NodeSnapshot {
+    ) -> Option<NodeSnapshot> {
+        if self.is_deleted(&node.path) {
+            return None;
+        }
+
         let session_atts = self
             .get_user_attributes(node.id)
             .cloned()
             .map(|a| a.map(UserAttributesSnapshot::Inline));
         let new_atts = session_atts.unwrap_or(node.user_attributes);
         match node.node_data {
-            NodeData::Group => NodeSnapshot { user_attributes: new_atts, ..node },
+            NodeData::Group => Some(NodeSnapshot { user_attributes: new_atts, ..node }),
             NodeData::Array(old_zarr_meta, _) => {
                 let new_zarr_meta = self
                     .get_updated_zarr_metadata(node.id)
                     .cloned()
                     .unwrap_or(old_zarr_meta);
 
-                NodeSnapshot {
-                    // FIXME: bad option type, change
+                Some(NodeSnapshot {
                     node_data: NodeData::Array(
                         new_zarr_meta,
                         new_manifests.unwrap_or_default(),
                     ),
                     user_attributes: new_atts,
                     ..node
-                }
+                })
             }
         }
     }
diff --git a/icechunk/src/format/mod.rs b/icechunk/src/format/mod.rs
index d7168756..e8366f51 100644
--- a/icechunk/src/format/mod.rs
+++ b/icechunk/src/format/mod.rs
@@ -4,14 +4,15 @@ use std::{
     hash::Hash,
     marker::PhantomData,
     ops::Bound,
-    path::PathBuf,
 };
 
 use bytes::Bytes;
 use itertools::Itertools;
 use rand::{thread_rng, Rng};
 use serde::{Deserialize, Deserializer, Serialize};
+use serde_with::{serde_as, TryFromInto};
 use thiserror::Error;
+use typed_path::Utf8UnixPathBuf;
 
 use crate::metadata::DataType;
 
@@ -19,7 +20,9 @@ pub mod attributes;
 pub mod manifest;
 pub mod snapshot;
 
-pub type Path = PathBuf;
+#[serde_as]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Serialize, Deserialize)]
+pub struct Path(#[serde_as(as = "TryFromInto<String>")] Utf8UnixPathBuf);
 
 #[allow(dead_code)]
 pub trait FileTypeTag {}
@@ -232,6 +235,70 @@ pub mod format_constants {
     pub const LATEST_ICECHUNK_SNAPSHOT_VERSION_METADATA_KEY: &str = "ic-sna-fmt-ver";
 }
 
+impl Display for Path {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+#[derive(Debug, Clone, Error, PartialEq, Eq)]
+pub enum PathError {
+    #[error("path must start with a `/` character")]
+    NotAbsolute,
+    #[error(r#"path must be cannonic, cannot include "." or "..""#)]
+    NotCanonic,
+}
+
+impl Path {
+    pub fn root() -> Path {
+        Path(Utf8UnixPathBuf::from("/".to_string()))
+    }
+
+    pub fn new(path: &str) -> Result<Path, PathError> {
+        let buf = Utf8UnixPathBuf::from(path);
+        if !buf.is_absolute() {
+            return Err(PathError::NotAbsolute);
+        }
+
+        if buf.normalize() != buf {
+            return Err(PathError::NotCanonic);
+        }
+        Ok(Path(buf))
+    }
+
+    pub fn starts_with(&self, other: &Path) -> bool {
+        self.0.starts_with(&other.0)
+    }
+
+    pub fn ancestors(&self) -> impl Iterator<Item = Path> + '_ {
+        self.0.ancestors().map(|p| Path(p.to_owned()))
+    }
+}
+
+impl TryFrom<&str> for Path {
+    type Error = PathError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Self::new(value)
+    }
+}
+
+impl TryFrom<&String> for Path {
+    type Error = PathError;
+
+    fn try_from(value: &String) -> Result<Self, Self::Error> {
+        value.as_str().try_into()
+    }
+}
+
+impl TryFrom<String> for Path {
+    type Error = PathError;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        value.as_str().try_into()
+    }
+}
+
 #[cfg(test)]
 #[allow(clippy::panic, clippy::unwrap_used, clippy::expect_used)]
 mod tests {
diff --git a/icechunk/src/format/snapshot.rs b/icechunk/src/format/snapshot.rs
index ac76ed8c..a7930e7a 100644
--- a/icechunk/src/format/snapshot.rs
+++ b/icechunk/src/format/snapshot.rs
@@ -313,25 +313,25 @@ mod tests {
         let oid = ObjectId::random();
         let nodes = vec![
             NodeSnapshot {
-                path: "/".into(),
+                path: Path::root(),
                 id: 1,
                 user_attributes: None,
                 node_data: NodeData::Group,
             },
             NodeSnapshot {
-                path: "/a".into(),
+                path: "/a".try_into().unwrap(),
                 id: 2,
                 user_attributes: None,
                 node_data: NodeData::Group,
             },
             NodeSnapshot {
-                path: "/b".into(),
+                path: "/b".try_into().unwrap(),
                 id: 3,
                 user_attributes: None,
                 node_data: NodeData::Group,
             },
             NodeSnapshot {
-                path: "/b/c".into(),
+                path: "/b/c".try_into().unwrap(),
                 id: 4,
                 user_attributes: Some(UserAttributesSnapshot::Inline(
                     UserAttributes::try_new(br#"{"foo": "some inline"}"#).unwrap(),
@@ -339,7 +339,7 @@ mod tests {
                 node_data: NodeData::Group,
             },
             NodeSnapshot {
-                path: "/b/array1".into(),
+                path: "/b/array1".try_into().unwrap(),
                 id: 5,
                 user_attributes: Some(UserAttributesSnapshot::Ref(UserAttributesRef {
                     object_id: oid.clone(),
@@ -351,13 +351,13 @@ mod tests {
                 ),
             },
             NodeSnapshot {
-                path: "/array2".into(),
+                path: "/array2".try_into().unwrap(),
                 id: 6,
                 user_attributes: None,
                 node_data: NodeData::Array(zarr_meta2.clone(), vec![]),
             },
             NodeSnapshot {
-                path: "/b/array3".into(),
+                path: "/b/array3".try_into().unwrap(),
                 id: 7,
                 user_attributes: None,
                 node_data: NodeData::Array(zarr_meta3.clone(), vec![]),
@@ -377,15 +377,17 @@ mod tests {
         let st = Snapshot::from_iter(&initial, None, manifests, vec![], nodes);
 
         assert_eq!(
-            st.get_node(&"/nonexistent".into()),
-            Err(IcechunkFormatError::NodeNotFound { path: "/nonexistent".into() })
+            st.get_node(&"/nonexistent".try_into().unwrap()),
+            Err(IcechunkFormatError::NodeNotFound {
+                path: "/nonexistent".try_into().unwrap()
+            })
         );
 
-        let node = st.get_node(&"/b/c".into());
+        let node = st.get_node(&"/b/c".try_into().unwrap());
         assert_eq!(
             node,
             Ok(&NodeSnapshot {
-                path: "/b/c".into(),
+                path: "/b/c".try_into().unwrap(),
                 id: 4,
                 user_attributes: Some(UserAttributesSnapshot::Inline(
                     UserAttributes::try_new(br#"{"foo": "some inline"}"#).unwrap(),
@@ -393,21 +395,21 @@ mod tests {
                 node_data: NodeData::Group,
             }),
         );
-        let node = st.get_node(&"/".into());
+        let node = st.get_node(&Path::root());
         assert_eq!(
             node,
             Ok(&NodeSnapshot {
-                path: "/".into(),
+                path: Path::root(),
                 id: 1,
                 user_attributes: None,
                 node_data: NodeData::Group,
             }),
         );
-        let node = st.get_node(&"/b/array1".into());
+        let node = st.get_node(&"/b/array1".try_into().unwrap());
         assert_eq!(
             node,
             Ok(&NodeSnapshot {
-                path: "/b/array1".into(),
+                path: "/b/array1".try_into().unwrap(),
                 id: 5,
                 user_attributes: Some(UserAttributesSnapshot::Ref(UserAttributesRef {
                     object_id: oid,
@@ -416,21 +418,21 @@ mod tests {
                 node_data: NodeData::Array(zarr_meta1.clone(), vec![man_ref1, man_ref2]),
             }),
         );
-        let node = st.get_node(&"/array2".into());
+        let node = st.get_node(&"/array2".try_into().unwrap());
         assert_eq!(
             node,
             Ok(&NodeSnapshot {
-                path: "/array2".into(),
+                path: "/array2".try_into().unwrap(),
                 id: 6,
                 user_attributes: None,
                 node_data: NodeData::Array(zarr_meta2.clone(), vec![]),
             }),
         );
-        let node = st.get_node(&"/b/array3".into());
+        let node = st.get_node(&"/b/array3".try_into().unwrap());
         assert_eq!(
             node,
             Ok(&NodeSnapshot {
-                path: "/b/array3".into(),
+                path: "/b/array3".try_into().unwrap(),
                 id: 7,
                 user_attributes: None,
                 node_data: NodeData::Array(zarr_meta3.clone(), vec![]),
diff --git a/icechunk/src/repository.rs b/icechunk/src/repository.rs
index 6851e2c6..13772173 100644
--- a/icechunk/src/repository.rs
+++ b/icechunk/src/repository.rs
@@ -1,7 +1,6 @@
 use std::{
     collections::{BTreeMap, HashMap, HashSet},
     iter::{self},
-    path::PathBuf,
     pin::Pin,
     sync::Arc,
 };
@@ -319,7 +318,7 @@ impl Repository {
     pub async fn delete_group(&mut self, path: Path) -> RepositoryResult<()> {
         self.get_group(&path)
             .await
-            .map(|node| self.change_set.delete_group(&node.path, node.id))
+            .map(|node| self.change_set.delete_group(node.path, node.id))
     }
 
     /// Add an array to the store.
@@ -360,7 +359,7 @@ impl Repository {
     pub async fn delete_array(&mut self, path: Path) -> RepositoryResult<()> {
         self.get_array(&path)
             .await
-            .map(|node| self.change_set.delete_array(&node.path, node.id))
+            .map(|node| self.change_set.delete_array(node.path, node.id))
     }
 
     /// Record the write or delete of user attributes to array or group
@@ -410,11 +409,17 @@ impl Repository {
 
     pub async fn get_node(&self, path: &Path) -> RepositoryResult<NodeSnapshot> {
         // We need to look for nodes in self.change_set and the snapshot file
+        if self.change_set.is_deleted(path) {
+            return Err(RepositoryError::NodeNotFound {
+                path: path.clone(),
+                message: "getting node".to_string(),
+            });
+        }
         match self.change_set.get_new_node(path) {
             Some(node) => Ok(node),
             None => {
                 let node = self.get_existing_node(path).await?;
-                if self.change_set.is_deleted(&node.id) {
+                if self.change_set.is_deleted(&node.path) {
                     Err(RepositoryError::NodeNotFound {
                         path: path.clone(),
                         message: "getting node".to_string(),
@@ -643,7 +648,7 @@ impl Repository {
     /// Warning: The presence of a single error may mean multiple missing items
     async fn updated_chunk_iterator(
         &self,
-    ) -> RepositoryResult<impl Stream<Item = RepositoryResult<(PathBuf, ChunkInfo)>> + '_>
+    ) -> RepositoryResult<impl Stream<Item = RepositoryResult<(Path, ChunkInfo)>> + '_>
     {
         let snapshot = self.storage.fetch_snapshot(&self.snapshot_id).await?;
         let nodes = futures::stream::iter(snapshot.iter_arc());
@@ -744,7 +749,7 @@ impl Repository {
 
     pub async fn all_chunks(
         &self,
-    ) -> RepositoryResult<impl Stream<Item = RepositoryResult<(PathBuf, ChunkInfo)>> + '_>
+    ) -> RepositoryResult<impl Stream<Item = RepositoryResult<(Path, ChunkInfo)>> + '_>
     {
         let existing_array_chunks = self.updated_chunk_iterator().await?;
         let new_array_chunks =
@@ -965,7 +970,7 @@ async fn updated_existing_nodes<'a>(
     // TODO: solve this duplication, there is always the possibility of this being the first
     // version
     let updated_nodes =
-        storage.fetch_snapshot(parent_id).await?.iter_arc().map(move |node| {
+        storage.fetch_snapshot(parent_id).await?.iter_arc().filter_map(move |node| {
             let new_manifests = if node.node_type() == NodeType::Array {
                 //FIXME: it could be none for empty arrays
                 Some(vec![ManifestRef {
@@ -1096,7 +1101,7 @@ async fn distributed_flush<I: IntoIterator<Item = ChangeSet>>(
 #[allow(clippy::panic, clippy::unwrap_used, clippy::expect_used)]
 mod tests {
 
-    use std::{error::Error, num::NonZeroU64, path::PathBuf};
+    use std::{error::Error, num::NonZeroU64};
 
     use crate::{
         format::manifest::ChunkInfo,
@@ -1276,10 +1281,10 @@ mod tests {
             object_id: manifest_id.clone(),
             extents: ManifestExtents(vec![]),
         };
-        let array1_path: PathBuf = "/array1".to_string().into();
+        let array1_path: Path = "/array1".try_into().unwrap();
         let nodes = vec![
             NodeSnapshot {
-                path: "/".into(),
+                path: Path::root(),
                 id: 1,
                 user_attributes: None,
                 node_data: NodeData::Group,
@@ -1317,13 +1322,13 @@ mod tests {
         assert_eq!(nodes.get(1).unwrap(), &node);
 
         let group_name = "/tbd-group".to_string();
-        ds.add_group(group_name.clone().into()).await?;
-        ds.delete_group(group_name.clone().into()).await?;
-        assert!(ds.delete_group(group_name.clone().into()).await.is_err());
-        assert!(ds.get_node(&group_name.into()).await.is_err());
+        ds.add_group(group_name.clone().try_into().unwrap()).await?;
+        ds.delete_group(group_name.clone().try_into().unwrap()).await?;
+        assert!(ds.delete_group(group_name.clone().try_into().unwrap()).await.is_err());
+        assert!(ds.get_node(&group_name.try_into().unwrap()).await.is_err());
 
         // add a new array and retrieve its node
-        ds.add_group("/group".to_string().into()).await?;
+        ds.add_group("/group".try_into().unwrap()).await?;
 
         let zarr_meta2 = ZarrArrayMetadata {
             shape: vec![3],
@@ -1339,7 +1344,7 @@ mod tests {
             dimension_names: Some(vec![Some("t".to_string())]),
         };
 
-        let new_array_path: PathBuf = "/group/array2".to_string().into();
+        let new_array_path: Path = "/group/array2".to_string().try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta2.clone()).await?;
 
         ds.delete_array(new_array_path.clone()).await?;
@@ -1371,7 +1376,7 @@ mod tests {
         assert_eq!(
             node.ok(),
             Some(NodeSnapshot {
-                path: "/group/array2".into(),
+                path: "/group/array2".try_into().unwrap(),
                 id: 6,
                 user_attributes: Some(UserAttributesSnapshot::Inline(
                     UserAttributes::try_new(br#"{"n":42}"#).unwrap()
@@ -1437,7 +1442,7 @@ mod tests {
         .await?;
         assert_eq!(chunk, Some(data));
 
-        let path: Path = "/group/array2".into();
+        let path: Path = "/group/array2".try_into().unwrap();
         let node = ds.get_node(&path).await;
         assert!(ds.change_set.has_updated_attributes(&node.as_ref().unwrap().id));
         assert!(ds.delete_array(path.clone()).await.is_ok());
@@ -1473,8 +1478,8 @@ mod tests {
             ]),
         };
 
-        change_set.add_array("foo/bar".into(), 1, zarr_meta.clone());
-        change_set.add_array("foo/baz".into(), 2, zarr_meta);
+        change_set.add_array("/foo/bar".try_into().unwrap(), 1, zarr_meta.clone());
+        change_set.add_array("/foo/baz".try_into().unwrap(), 2, zarr_meta);
         assert_eq!(None, change_set.new_arrays_chunk_iterator().next());
 
         change_set.set_chunk_ref(1, ChunkIndices(vec![0, 1]), None);
@@ -1508,7 +1513,7 @@ mod tests {
                 .collect();
             let expected_chunks: Vec<_> = [
                 (
-                    "foo/baz".into(),
+                    "/foo/baz".try_into().unwrap(),
                     ChunkInfo {
                         node: 2,
                         coord: ChunkIndices(vec![0]),
@@ -1516,7 +1521,7 @@ mod tests {
                     },
                 ),
                 (
-                    "foo/baz".into(),
+                    "/foo/baz".try_into().unwrap(),
                     ChunkInfo {
                         node: 2,
                         coord: ChunkIndices(vec![1]),
@@ -1524,7 +1529,7 @@ mod tests {
                     },
                 ),
                 (
-                    "foo/bar".into(),
+                    "/foo/bar".try_into().unwrap(),
                     ChunkInfo {
                         node: 1,
                         coord: ChunkIndices(vec![1, 0]),
@@ -1532,7 +1537,7 @@ mod tests {
                     },
                 ),
                 (
-                    "foo/bar".into(),
+                    "/foo/bar".try_into().unwrap(),
                     ChunkInfo {
                         node: 1,
                         coord: ChunkIndices(vec![1, 1]),
@@ -1557,35 +1562,35 @@ mod tests {
         let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
 
         // add a new array and retrieve its node
-        ds.add_group("/".into()).await?;
+        ds.add_group(Path::root()).await?;
         let snapshot_id = ds.flush("commit", SnapshotProperties::default()).await?;
 
         assert_eq!(snapshot_id, ds.snapshot_id);
         assert_eq!(
-            ds.get_node(&"/".into()).await.ok(),
+            ds.get_node(&Path::root()).await.ok(),
             Some(NodeSnapshot {
                 id: 1,
-                path: "/".into(),
+                path: Path::root(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
         );
-        ds.add_group("/group".into()).await?;
+        ds.add_group("/group".try_into().unwrap()).await?;
         let _snapshot_id = ds.flush("commit", SnapshotProperties::default()).await?;
         assert_eq!(
-            ds.get_node(&"/".into()).await.ok(),
+            ds.get_node(&Path::root()).await.ok(),
             Some(NodeSnapshot {
                 id: 1,
-                path: "/".into(),
+                path: Path::root(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
         );
         assert_eq!(
-            ds.get_node(&"/group".into()).await.ok(),
+            ds.get_node(&"/group".try_into().unwrap()).await.ok(),
             Some(NodeSnapshot {
                 id: 2,
-                path: "/group".into(),
+                path: "/group".try_into().unwrap(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
@@ -1604,7 +1609,7 @@ mod tests {
             dimension_names: Some(vec![Some("t".to_string())]),
         };
 
-        let new_array_path: PathBuf = "/group/array1".to_string().into();
+        let new_array_path: Path = "/group/array1".try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta.clone()).await?;
 
         // wo commit to test the case of a chunkless array
@@ -1620,19 +1625,19 @@ mod tests {
 
         let _snapshot_id = ds.flush("commit", SnapshotProperties::default()).await?;
         assert_eq!(
-            ds.get_node(&"/".into()).await.ok(),
+            ds.get_node(&Path::root()).await.ok(),
             Some(NodeSnapshot {
                 id: 1,
-                path: "/".into(),
+                path: Path::root(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
         );
         assert_eq!(
-            ds.get_node(&"/group".into()).await.ok(),
+            ds.get_node(&"/group".try_into().unwrap()).await.ok(),
             Some(NodeSnapshot {
                 id: 2,
-                path: "/group".into(),
+                path: "/group".try_into().unwrap(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
@@ -1733,6 +1738,83 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn test_basic_delete_and_flush() -> Result<(), Box<dyn Error>> {
+        let storage: Arc<dyn Storage + Send + Sync> =
+            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
+        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
+        ds.add_group(Path::root()).await?;
+        ds.add_group("/1".try_into().unwrap()).await?;
+        ds.delete_group("/1".try_into().unwrap()).await?;
+        assert_eq!(ds.list_nodes().await?.count(), 1);
+        ds.commit("main", "commit", None).await?;
+        assert!(ds.get_group(&Path::root()).await.is_ok());
+        assert!(ds.get_group(&"/1".try_into().unwrap()).await.is_err());
+        assert_eq!(ds.list_nodes().await?.count(), 1);
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_basic_delete_after_flush() -> Result<(), Box<dyn Error>> {
+        let storage: Arc<dyn Storage + Send + Sync> =
+            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
+        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
+        ds.add_group(Path::root()).await?;
+        ds.add_group("/1".try_into().unwrap()).await?;
+        ds.commit("main", "commit", None).await?;
+
+        ds.delete_group("/1".try_into().unwrap()).await?;
+        assert!(ds.get_group(&Path::root()).await.is_ok());
+        assert!(ds.get_group(&"/1".try_into().unwrap()).await.is_err());
+        assert_eq!(ds.list_nodes().await?.count(), 1);
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_commit_after_deleting_old_node() -> Result<(), Box<dyn Error>> {
+        let storage: Arc<dyn Storage + Send + Sync> =
+            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
+        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
+        ds.add_group(Path::root()).await?;
+        ds.commit("main", "commit", None).await?;
+        ds.delete_group(Path::root()).await?;
+        ds.commit("main", "commit", None).await?;
+        assert_eq!(ds.list_nodes().await?.count(), 0);
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_delete_children() -> Result<(), Box<dyn Error>> {
+        let storage: Arc<dyn Storage + Send + Sync> =
+            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
+        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
+        ds.add_group(Path::root()).await?;
+        ds.add_group("/a".try_into().unwrap()).await?;
+        ds.add_group("/b".try_into().unwrap()).await?;
+        ds.add_group("/b/bb".try_into().unwrap()).await?;
+        ds.delete_group("/b".try_into().unwrap()).await?;
+        assert!(ds.get_group(&"/b".try_into().unwrap()).await.is_err());
+        assert!(ds.get_group(&"/b/bb".try_into().unwrap()).await.is_err());
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_delete_children_of_old_nodes() -> Result<(), Box<dyn Error>> {
+        let storage: Arc<dyn Storage + Send + Sync> =
+            Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
+        let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
+        ds.add_group(Path::root()).await?;
+        ds.add_group("/a".try_into().unwrap()).await?;
+        ds.add_group("/b".try_into().unwrap()).await?;
+        ds.add_group("/b/bb".try_into().unwrap()).await?;
+        ds.commit("main", "commit", None).await?;
+
+        ds.delete_group("/b".try_into().unwrap()).await?;
+        assert!(ds.get_group(&"/b".try_into().unwrap()).await.is_err());
+        assert!(ds.get_group(&"/b/bb".try_into().unwrap()).await.is_err());
+        Ok(())
+    }
+
     #[tokio::test(flavor = "multi_thread")]
     async fn test_all_chunks_iterator() -> Result<(), Box<dyn Error>> {
         let storage: Arc<dyn Storage + Send + Sync> =
@@ -1740,7 +1822,7 @@ mod tests {
         let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
 
         // add a new array and retrieve its node
-        ds.add_group("/".into()).await?;
+        ds.add_group(Path::root()).await?;
         let zarr_meta = ZarrArrayMetadata {
             shape: vec![1, 1, 2],
             data_type: DataType::Int32,
@@ -1755,7 +1837,7 @@ mod tests {
             dimension_names: Some(vec![Some("t".to_string())]),
         };
 
-        let new_array_path: PathBuf = "/array".to_string().into();
+        let new_array_path: Path = "/array".try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta.clone()).await?;
         // we 3 chunks
         ds.set_chunk_ref(
@@ -1804,7 +1886,7 @@ mod tests {
         let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
 
         // add a new array and retrieve its node
-        ds.add_group("/".into()).await?;
+        ds.add_group(Path::root()).await?;
         let new_snapshot_id =
             ds.commit(Ref::DEFAULT_BRANCH, "first commit", None).await?;
         assert_eq!(
@@ -1819,10 +1901,10 @@ mod tests {
         assert_eq!(new_snapshot_id, ref_data.snapshot);
 
         assert_eq!(
-            ds.get_node(&"/".into()).await.ok(),
+            ds.get_node(&Path::root()).await.ok(),
             Some(NodeSnapshot {
                 id: 1,
-                path: "/".into(),
+                path: Path::root(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
@@ -1831,10 +1913,10 @@ mod tests {
         let mut ds =
             Repository::from_branch_tip(Arc::clone(&storage), "main").await?.build();
         assert_eq!(
-            ds.get_node(&"/".into()).await.ok(),
+            ds.get_node(&Path::root()).await.ok(),
             Some(NodeSnapshot {
                 id: 1,
-                path: "/".into(),
+                path: Path::root(),
                 user_attributes: None,
                 node_data: NodeData::Group
             })
@@ -1853,7 +1935,7 @@ mod tests {
             dimension_names: Some(vec![Some("t".to_string())]),
         };
 
-        let new_array_path: PathBuf = "/array1".to_string().into();
+        let new_array_path: Path = "/array1".try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta.clone()).await?;
         ds.set_chunk_ref(
             new_array_path.clone(),
@@ -1890,8 +1972,8 @@ mod tests {
         let mut ds2 =
             Repository::from_branch_tip(Arc::clone(&storage), "main").await?.build();
 
-        ds1.add_group("a".into()).await?;
-        ds2.add_group("b".into()).await?;
+        ds1.add_group("/a".try_into().unwrap()).await?;
+        ds2.add_group("/b".try_into().unwrap()).await?;
 
         let barrier = Arc::new(Barrier::new(2));
         let barrier_c = Arc::clone(&barrier);
@@ -2059,6 +2141,8 @@ mod tests {
                                 "Attempting to delete a non-existent path: {path}",
                             );
                         state.groups.swap_remove(index);
+                        state.groups.retain(|group| !group.starts_with(path));
+                        state.arrays.retain(|array, _| !array.starts_with(path));
                     }
                     _ => panic!(),
                 }
diff --git a/icechunk/src/strategies.rs b/icechunk/src/strategies.rs
index 0f9bfcbc..6c306378 100644
--- a/icechunk/src/strategies.rs
+++ b/icechunk/src/strategies.rs
@@ -1,7 +1,7 @@
 use std::num::NonZeroU64;
-use std::path::PathBuf;
 use std::sync::Arc;
 
+use prop::string::string_regex;
 use proptest::prelude::*;
 use proptest::{collection::vec, option, strategy::Strategy};
 
@@ -15,7 +15,10 @@ use crate::{ObjectStorage, Repository};
 
 pub fn node_paths() -> impl Strategy<Value = Path> {
     // FIXME: Add valid paths
-    any::<PathBuf>()
+    #[allow(clippy::expect_used)]
+    vec(string_regex("[a-zA-Z0-9]*").expect("invalid regex"), 0..10).prop_map(|v| {
+        format!("/{}", v.join("/")).try_into().expect("invalid Path string")
+    })
 }
 
 prop_compose! {
diff --git a/icechunk/src/zarr.rs b/icechunk/src/zarr.rs
index 002faa9c..48ba01b6 100644
--- a/icechunk/src/zarr.rs
+++ b/icechunk/src/zarr.rs
@@ -1,5 +1,6 @@
 use std::{
     collections::HashSet,
+    fmt::Display,
     iter,
     num::NonZeroU64,
     ops::{Deref, DerefMut},
@@ -816,10 +817,8 @@ impl Store {
             for node in repository.list_nodes().await? {
                 // TODO: handle non-utf8?
                 let meta_key = Key::Metadata { node_path: node.path }.to_string();
-                if let Some(key) = meta_key {
-                    if key.starts_with(prefix) {
-                        yield key;
-                    }
+                if meta_key.starts_with(prefix) {
+                    yield meta_key;
                 }
             }
         };
@@ -840,10 +839,8 @@ impl Store {
                 match maybe_path_chunk {
                     Ok((path,chunk)) => {
                         let chunk_key = Key::Chunk { node_path: path, coords: chunk.coord }.to_string();
-                        if let Some(key) = chunk_key {
-                            if key.starts_with(prefix) {
-                                yield key;
-                            }
+                        if chunk_key.starts_with(prefix) {
+                            yield chunk_key;
                         }
                     }
                     Err(err) => Err(err)?
@@ -974,7 +971,7 @@ impl Key {
         fn parse_chunk(key: &str) -> Result<Key, StoreError> {
             if key == "c" {
                 return Ok(Key::Chunk {
-                    node_path: "/".into(),
+                    node_path: Path::root(),
                     coords: ChunkIndices(vec![]),
                 });
             }
@@ -982,10 +979,15 @@ impl Key {
                 let path = path.strip_suffix('/').unwrap_or(path);
                 if coords.is_empty() {
                     Ok(Key::Chunk {
-                        node_path: ["/", path].iter().collect(),
+                        node_path: format!("/{path}").try_into().map_err(|_| {
+                            StoreError::InvalidKey { key: key.to_string() }
+                        })?,
                         coords: ChunkIndices(vec![]),
                     })
                 } else {
+                    let absolute = format!("/{path}")
+                        .try_into()
+                        .map_err(|_| StoreError::InvalidKey { key: key.to_string() })?;
                     coords
                         .strip_prefix('/')
                         .ok_or(StoreError::InvalidKey { key: key.to_string() })?
@@ -993,7 +995,7 @@ impl Key {
                         .map(|s| s.parse::<u32>())
                         .collect::<Result<Vec<_>, _>>()
                         .map(|coords| Key::Chunk {
-                            node_path: ["/", path].iter().collect(),
+                            node_path: absolute,
                             coords: ChunkIndices(coords),
                         })
                         .map_err(|_| StoreError::InvalidKey { key: key.to_string() })
@@ -1004,30 +1006,37 @@ impl Key {
         }
 
         if key == Key::ROOT_KEY {
-            Ok(Key::Metadata { node_path: "/".into() })
+            Ok(Key::Metadata { node_path: Path::root() })
         } else if let Some(path) = key.strip_suffix(Key::METADATA_SUFFIX) {
             // we need to be careful indexing into utf8 strings
-            Ok(Key::Metadata { node_path: ["/", path].iter().collect() })
+            Ok(Key::Metadata {
+                node_path: format!("/{path}")
+                    .try_into()
+                    .map_err(|_| StoreError::InvalidKey { key: key.to_string() })?,
+            })
         } else {
             parse_chunk(key)
         }
     }
+}
 
-    fn to_string(&self) -> Option<String> {
+impl Display for Key {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Key::Metadata { node_path } => node_path.as_path().to_str().map(|s| {
-                format!("{}{}", &s[1..], Key::METADATA_SUFFIX)
-                    .trim_start_matches('/')
-                    .to_string()
-            }),
+            Key::Metadata { node_path } => {
+                let s =
+                    format!("{}{}", &node_path.to_string()[1..], Key::METADATA_SUFFIX)
+                        .trim_start_matches('/')
+                        .to_string();
+                f.write_str(s.as_str())
+            }
             Key::Chunk { node_path, coords } => {
-                node_path.as_path().to_str().map(|path| {
-                    let coords = coords.0.iter().map(|c| c.to_string()).join("/");
-                    [path[1..].to_string(), "c".to_string(), coords]
-                        .iter()
-                        .filter(|s| !s.is_empty())
-                        .join("/")
-                })
+                let coords = coords.0.iter().map(|c| c.to_string()).join("/");
+                let s = [node_path.to_string()[1..].to_string(), "c".to_string(), coords]
+                    .iter()
+                    .filter(|s| !s.is_empty())
+                    .join("/");
+                f.write_str(s.as_str())
             }
         }
     }
@@ -1362,91 +1371,100 @@ mod tests {
     fn test_parse_key() {
         assert!(matches!(
             Key::parse("zarr.json"),
-            Ok(Key::Metadata { node_path}) if node_path.to_str() == Some("/")
+            Ok(Key::Metadata { node_path}) if node_path.to_string() == "/"
         ));
         assert!(matches!(
             Key::parse("a/zarr.json"),
-            Ok(Key::Metadata { node_path }) if node_path.to_str() == Some("/a")
+            Ok(Key::Metadata { node_path }) if node_path.to_string() == "/a"
         ));
         assert!(matches!(
             Key::parse("a/b/c/zarr.json"),
-            Ok(Key::Metadata { node_path }) if node_path.to_str() == Some("/a/b/c")
+            Ok(Key::Metadata { node_path }) if node_path.to_string() == "/a/b/c"
         ));
         assert!(matches!(
             Key::parse("foo/c"),
-            Ok(Key::Chunk { node_path, coords }) if node_path.to_str() == Some("/foo") && coords == ChunkIndices(vec![])
+            Ok(Key::Chunk { node_path, coords }) if node_path.to_string() == "/foo" && coords == ChunkIndices(vec![])
         ));
         assert!(matches!(
             Key::parse("foo/bar/c"),
-            Ok(Key::Chunk { node_path, coords}) if node_path.to_str() == Some("/foo/bar") && coords == ChunkIndices(vec![])
+            Ok(Key::Chunk { node_path, coords}) if node_path.to_string() == "/foo/bar" && coords == ChunkIndices(vec![])
         ));
         assert!(matches!(
             Key::parse("foo/c/1/2/3"),
             Ok(Key::Chunk {
                 node_path,
                 coords,
-            }) if node_path.to_str() == Some("/foo") && coords == ChunkIndices(vec![1,2,3])
+            }) if node_path.to_string() == "/foo" && coords == ChunkIndices(vec![1,2,3])
         ));
         assert!(matches!(
             Key::parse("foo/bar/baz/c/1/2/3"),
             Ok(Key::Chunk {
                 node_path,
                 coords,
-            }) if node_path.to_str() == Some("/foo/bar/baz") && coords == ChunkIndices(vec![1,2,3])
+            }) if node_path.to_string() == "/foo/bar/baz" && coords == ChunkIndices(vec![1,2,3])
         ));
         assert!(matches!(
             Key::parse("c"),
-            Ok(Key::Chunk { node_path, coords}) if node_path.to_str() == Some("/") && coords == ChunkIndices(vec![])
+            Ok(Key::Chunk { node_path, coords}) if node_path.to_string() == "/" && coords == ChunkIndices(vec![])
         ));
         assert!(matches!(
             Key::parse("c/0/0"),
-            Ok(Key::Chunk { node_path, coords}) if node_path.to_str() == Some("/") && coords == ChunkIndices(vec![0,0])
+            Ok(Key::Chunk { node_path, coords}) if node_path.to_string() == "/" && coords == ChunkIndices(vec![0,0])
         ));
     }
 
     #[test]
     fn test_format_key() {
         assert_eq!(
-            Key::Metadata { node_path: "/".into() }.to_string(),
-            Some("zarr.json".to_string())
+            Key::Metadata { node_path: Path::root() }.to_string(),
+            "zarr.json".to_string()
         );
         assert_eq!(
-            Key::Metadata { node_path: "/a".into() }.to_string(),
-            Some("a/zarr.json".to_string())
+            Key::Metadata { node_path: "/a".try_into().unwrap() }.to_string(),
+            "a/zarr.json".to_string()
         );
         assert_eq!(
-            Key::Metadata { node_path: "/a/b/c".into() }.to_string(),
-            Some("a/b/c/zarr.json".to_string())
+            Key::Metadata { node_path: "/a/b/c".try_into().unwrap() }.to_string(),
+            "a/b/c/zarr.json".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/".into(), coords: ChunkIndices(vec![]) }
+            Key::Chunk { node_path: Path::root(), coords: ChunkIndices(vec![]) }
                 .to_string(),
-            Some("c".to_string())
+            "c".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/".into(), coords: ChunkIndices(vec![0]) }
+            Key::Chunk { node_path: Path::root(), coords: ChunkIndices(vec![0]) }
                 .to_string(),
-            Some("c/0".to_string())
+            "c/0".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/".into(), coords: ChunkIndices(vec![1, 2]) }
+            Key::Chunk { node_path: Path::root(), coords: ChunkIndices(vec![1, 2]) }
                 .to_string(),
-            Some("c/1/2".to_string())
+            "c/1/2".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/a".into(), coords: ChunkIndices(vec![]) }
-                .to_string(),
-            Some("a/c".to_string())
+            Key::Chunk {
+                node_path: "/a".try_into().unwrap(),
+                coords: ChunkIndices(vec![])
+            }
+            .to_string(),
+            "a/c".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/a".into(), coords: ChunkIndices(vec![1]) }
-                .to_string(),
-            Some("a/c/1".to_string())
+            Key::Chunk {
+                node_path: "/a".try_into().unwrap(),
+                coords: ChunkIndices(vec![1])
+            }
+            .to_string(),
+            "a/c/1".to_string()
         );
         assert_eq!(
-            Key::Chunk { node_path: "/a".into(), coords: ChunkIndices(vec![1, 2]) }
-                .to_string(),
-            Some("a/c/1/2".to_string())
+            Key::Chunk {
+                node_path: "/a".try_into().unwrap(),
+                coords: ChunkIndices(vec![1, 2])
+            }
+            .to_string(),
+            "a/c/1/2".to_string()
         );
     }
 
@@ -1569,7 +1587,7 @@ mod tests {
 
         assert!(matches!(
             store.get("zarr.json", &ByteRange::ALL).await,
-            Err(StoreError::NotFound(KeyNotFoundError::NodeNotFound {path})) if path.to_str() == Some("/")
+            Err(StoreError::NotFound(KeyNotFoundError::NodeNotFound {path})) if path.to_string() == "/"
         ));
 
         store
@@ -1633,14 +1651,14 @@ mod tests {
         assert!(matches!(
             store.get("array/zarr.json", &ByteRange::ALL).await,
             Err(StoreError::NotFound(KeyNotFoundError::NodeNotFound { path }))
-                if path.to_str() == Some("/array"),
+                if path.to_string() == "/array",
         ));
         store.set("array/zarr.json", zarr_meta.clone()).await.unwrap();
         store.delete("array/zarr.json").await.unwrap();
         assert!(matches!(
             store.get("array/zarr.json", &ByteRange::ALL).await,
             Err(StoreError::NotFound(KeyNotFoundError::NodeNotFound { path } ))
-                if path.to_str() == Some("/array"),
+                if path.to_string() == "/array",
         ));
         store.set("array/zarr.json", Bytes::copy_from_slice(group_data)).await.unwrap();
 
@@ -1779,7 +1797,7 @@ mod tests {
         assert!(matches!(
             store.get("array/c/0/1/0", &ByteRange::ALL).await,
             Err(StoreError::NotFound(KeyNotFoundError::ChunkNotFound { key, path, coords }))
-                if key == "array/c/0/1/0" && path.to_str() == Some("/array") && coords == ChunkIndices([0, 1, 0].to_vec())
+                if key == "array/c/0/1/0" && path.to_string() == "/array" && coords == ChunkIndices([0, 1, 0].to_vec())
         ));
         assert!(matches!(
             store.delete("array/foo").await,


Test patch:
diff --git a/icechunk/tests/test_concurrency.rs b/icechunk/tests/test_concurrency.rs
index 2e07b2cb..3e9ed546 100644
--- a/icechunk/tests/test_concurrency.rs
+++ b/icechunk/tests/test_concurrency.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::expect_used)]
+#![allow(clippy::expect_used, clippy::unwrap_used)]
 use bytes::Bytes;
 use icechunk::{
     format::{ByteRange, ChunkIndices, Path},
@@ -33,7 +33,7 @@ async fn test_concurrency() -> Result<(), Box<dyn std::error::Error>> {
         Arc::new(ObjectStorage::new_in_memory_store(Some("prefix".into())));
     let mut ds = Repository::init(Arc::clone(&storage), false).await?.build();
 
-    ds.add_group("/".into()).await?;
+    ds.add_group(Path::root()).await?;
     let zarr_meta = ZarrArrayMetadata {
         shape: vec![N as u64, N as u64],
         data_type: DataType::Float64,
@@ -51,7 +51,7 @@ async fn test_concurrency() -> Result<(), Box<dyn std::error::Error>> {
         dimension_names: Some(vec![Some("x".to_string()), Some("y".to_string())]),
     };
 
-    let new_array_path: Path = "/array".to_string().into();
+    let new_array_path: Path = "/array".try_into().unwrap();
     ds.add_array(new_array_path.clone(), zarr_meta.clone()).await?;
 
     let ds = Arc::new(RwLock::new(ds));
@@ -101,7 +101,11 @@ async fn write_task(ds: Arc<RwLock<Repository>>, x: u32, y: u32) {
 
     ds.write()
         .await
-        .set_chunk_ref("/array".into(), ChunkIndices(vec![x, y]), Some(payload))
+        .set_chunk_ref(
+            "/array".try_into().unwrap(),
+            ChunkIndices(vec![x, y]),
+            Some(payload),
+        )
         .await
         .expect("Failed to write chunk ref");
 }
@@ -114,7 +118,7 @@ async fn read_task(ds: Arc<RwLock<Repository>>, x: u32, y: u32, barrier: Arc<Bar
         let readable_ds = ds.read().await;
         let chunk_reader = readable_ds
             .get_chunk_reader(
-                &"/array".into(),
+                &"/array".try_into().unwrap(),
                 &ChunkIndices(vec![x, y]),
                 &ByteRange::ALL,
             )
@@ -159,7 +163,7 @@ async fn list_task(ds: Arc<RwLock<Repository>>, barrier: Arc<Barrier>) {
             .list_nodes()
             .await
             .expect("list_nodes failed")
-            .map(|n| n.path.as_path().to_string_lossy().into_owned())
+            .map(|n| n.path.to_string())
             .collect::<HashSet<_>>();
         assert_eq!(expected_nodes, nodes);
 
diff --git a/icechunk/tests/test_distributed_writes.rs b/icechunk/tests/test_distributed_writes.rs
index f602b2c4..a737641e 100644
--- a/icechunk/tests/test_distributed_writes.rs
+++ b/icechunk/tests/test_distributed_writes.rs
@@ -1,3 +1,4 @@
+#![allow(clippy::unwrap_used)]
 use pretty_assertions::assert_eq;
 use std::{num::NonZeroU64, ops::Range, sync::Arc};
 
@@ -66,14 +67,17 @@ async fn write_chunks(
                 .collect();
             let payload =
                 repo.get_chunk_writer()(Bytes::copy_from_slice(bytes.as_slice())).await?;
-            repo.set_chunk_ref("/array".into(), ChunkIndices(vec![x, y]), Some(payload))
-                .await?;
+            repo.set_chunk_ref(
+                "/array".try_into().unwrap(),
+                ChunkIndices(vec![x, y]),
+                Some(payload),
+            )
+            .await?;
         }
     }
     Ok(repo)
 }
 
-#[allow(clippy::unwrap_used)]
 async fn verify(
     repo: Repository,
 ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
@@ -81,7 +85,7 @@ async fn verify(
         for y in 0..(SIZE / 2) as u32 {
             let bytes = get_chunk(
                 repo.get_chunk_reader(
-                    &"/array".into(),
+                    &"/array".try_into().unwrap(),
                     &ChunkIndices(vec![x, y]),
                     &ByteRange::ALL,
                 )
@@ -132,7 +136,7 @@ async fn test_distributed_writes() -> Result<(), Box<dyn std::error::Error + Sen
         dimension_names: None,
     };
 
-    let new_array_path: Path = "/array".to_string().into();
+    let new_array_path: Path = "/array".try_into().unwrap();
     repo1.add_array(new_array_path.clone(), zarr_meta.clone()).await?;
     repo1.commit("main", "create array", None).await?;
 
diff --git a/icechunk/tests/test_virtual_refs.rs b/icechunk/tests/test_virtual_refs.rs
index dc4e6c50..5da2e7ce 100644
--- a/icechunk/tests/test_virtual_refs.rs
+++ b/icechunk/tests/test_virtual_refs.rs
@@ -4,7 +4,7 @@ mod tests {
     use icechunk::{
         format::{
             manifest::{VirtualChunkLocation, VirtualChunkRef},
-            ByteRange, ChunkId, ChunkIndices,
+            ByteRange, ChunkId, ChunkIndices, Path,
         },
         metadata::{ChunkKeyEncoding, ChunkShape, DataType, FillValue},
         repository::{get_chunk, ChunkPayload, ZarrArrayMetadata},
@@ -16,8 +16,8 @@ mod tests {
         zarr::AccessMode,
         Repository, Storage, Store,
     };
-    use std::{error::Error, num::NonZeroU64, path::PathBuf};
-    use std::{path::Path, sync::Arc};
+    use std::{error::Error, num::NonZeroU64};
+    use std::{path::Path as StdPath, sync::Arc};
     use tempfile::TempDir;
 
     use bytes::Bytes;
@@ -67,7 +67,7 @@ mod tests {
                 .expect(&format!("putting chunk to {} failed", &path));
         }
     }
-    async fn create_local_repository(path: &Path) -> Repository {
+    async fn create_local_repository(path: &StdPath) -> Repository {
         let storage: Arc<dyn Storage + Send + Sync> = Arc::new(
             ObjectStorage::new_local_store(path).expect("Creating local storage failed"),
         );
@@ -153,7 +153,7 @@ mod tests {
             length: 5,
         });
 
-        let new_array_path: PathBuf = "/array".to_string().into();
+        let new_array_path: Path = "/array".try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta.clone()).await.unwrap();
 
         ds.set_chunk_ref(
@@ -263,7 +263,7 @@ mod tests {
             length: 5,
         });
 
-        let new_array_path: PathBuf = "/array".to_string().into();
+        let new_array_path: Path = "/array".try_into().unwrap();
         ds.add_array(new_array_path.clone(), zarr_meta.clone()).await.unwrap();
 
         ds.set_chunk_ref(