# HG changeset patch
# User Georges Racinet <georges.racinet@octobus.net>
# Date 1693496872 -7200
#      Thu Aug 31 17:47:52 2023 +0200
# Branch stable
# Node ID 74c6bbd8b086214d3ba1e8bf14f0f4827c479310
# Parent  b4465e96862f36d6ecb401c4421b4ccc8a274630
rhgitaly::mercurial::DirIteratorWithoutFlatPaths

This iterator will be the core engine of `CommitService.GetTreeEntries` in
the simplest case: non-recursive without flat paths computation.

The flat paths computation is not as expensive in the Mercurial case as it is
in the Git case, because we are iterating over the entire manifest anyway (a
later version might use a binary search to find the starting point of the
requested directory, but all files within the directory will have to be scanned),
but not doing it enables this simple implementation: namely we can yield top-level
directories immediately, and hence to have at most one `TreeEntry` to yield per run
of the loop.

diff --git a/rust/rhgitaly/src/mercurial.rs b/rust/rhgitaly/src/mercurial.rs
--- a/rust/rhgitaly/src/mercurial.rs
+++ b/rust/rhgitaly/src/mercurial.rs
@@ -12,10 +12,10 @@
 use hg::revlog::{Node, NodePrefix, RevlogError, NULL_REVISION};
 use hg::utils::hg_path::HgPath;
 
-use crate::gitaly::ObjectType;
+use crate::gitaly::{tree_entry::EntryType, ObjectType, TreeEntry};
 
 use super::git;
-use super::oid::blob_oid;
+use super::oid::{blob_oid, tree_oid};
 use tracing::debug;
 
 #[derive(Debug, Default)]
@@ -274,7 +274,6 @@
 where
     IM: Iterator<Item = Result<ManifestEntry<'m>, HgError>>,
 {
-    #[allow(dead_code)]
     fn new(manifest_iter: IM, path: &'a [u8]) -> Self {
         ManifestDirIterator {
             path,
@@ -288,7 +287,6 @@
     /// This is meant so that, if `self.path` is `"foo"`, then `"foo/bar"[self.path_len()..]` is
     /// `"bar"`. It also handles the case where `self.path` is empty correctly, sparing the callers
     /// to check for this special case.
-    #[allow(dead_code)]
     fn prefix_len(&self) -> usize {
         let pl = self.path.len();
         if pl == 0 {
@@ -346,6 +344,151 @@
         None
     }
 }
+
+/// Result of splitting a path for first and last segment
+struct PathWithTopLevelDir<'a> {
+    /// The full path, for flat_path computation
+    #[allow(dead_code)]
+    path: &'a [u8],
+    /// The top level directory in the path (first segment)
+    top_level: &'a [u8],
+}
+
+/// Analyze the given `sub_path`, splitting the directory part to get its top-level.
+///
+/// The given (full) `entry_path` is for error messages only.
+///
+/// If `sub_path` is a top-level file, `None` is returned. Otherwise, the returned
+/// [`PathWithTopLevelDir`] encloses the directory part of `sub_path`.
+///
+/// The computation relies on expectations for Mercurial manifeset entries, which are reflected
+/// by the returned errors. Using the full entry path here should help with error inspection.
+fn split_dir_and_top_level<'a>(
+    sub_path: &'a [u8],
+    entry_path: &[u8],
+) -> Result<Option<PathWithTopLevelDir<'a>>, HgError> {
+    let mut rsplit = sub_path.rsplitn(2, |c| *c == b'/');
+    rsplit.next().ok_or_else(|| {
+        HgError::corrupted(format!(
+            "Manifest entry with trailing slash: {:?}",
+            entry_path
+        ))
+    })?;
+    rsplit
+        .next()
+        .map(|dir_path| {
+            let top_level = dir_path.splitn(2, |c| *c == b'/').next().ok_or_else(|| {
+                HgError::corrupted(format!(
+                    "Manifest entry with double slash: {:?}",
+                    entry_path
+                ))
+            })?;
+            Ok(PathWithTopLevelDir {
+                top_level,
+                path: dir_path,
+            })
+        })
+        .transpose()
+}
+
+/// Iterator yielding a [`TreeEntry`] without flat path for each top-level file or directory in a
+/// manifest directory.
+pub struct DirIteratorWithoutFlatPaths<'a, 'm, IM>
+where
+    IM: Iterator<Item = Result<ManifestEntry<'m>, HgError>>,
+{
+    /// Is expected to be the repetition of an incoming request "oid" field,
+    /// often something like `branch/default` and not a real OID
+    commit_oid: String,
+    changeset_node: Node,
+    root_oid: String,
+    manifest_dir_iter: ManifestDirIterator<'a, 'm, IM>,
+    current_subdir: &'m [u8],
+}
+
+impl<'a, 'm, IM> DirIteratorWithoutFlatPaths<'a, 'm, IM>
+where
+    IM: Iterator<Item = Result<ManifestEntry<'m>, HgError>>,
+{
+    pub fn new(commit_oid: String, changeset_node: Node, manifest: IM, path: &'a [u8]) -> Self {
+        let root_oid = tree_oid(&changeset_node, b"");
+        Self {
+            commit_oid,
+            changeset_node,
+            root_oid,
+            manifest_dir_iter: ManifestDirIterator::new(manifest, path),
+            current_subdir: &[],
+        }
+    }
+
+    fn tree_entry(
+        &self,
+        path: &[u8],
+        oid: String,
+        obj_type: EntryType,
+        mode: i32,
+    ) -> Result<TreeEntry, HgError> {
+        Ok(TreeEntry {
+            oid,
+            mode,
+            commit_oid: self.commit_oid.clone(),
+            root_oid: self.root_oid.clone(),
+            path: path.to_vec(),
+            r#type: obj_type as i32,
+            flat_path: Vec::new(),
+        })
+    }
+}
+
+impl<'a, 'm, IM> Iterator for DirIteratorWithoutFlatPaths<'a, 'm, IM>
+where
+    IM: Iterator<Item = Result<ManifestEntry<'m>, HgError>>,
+{
+    type Item = Result<TreeEntry, HgError>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        while let Some(entry_res) = self.manifest_dir_iter.next() {
+            match entry_res {
+                Err(e) => {
+                    return Some(Err(e));
+                }
+                Ok(entry) => {
+                    let ep = entry.path.as_bytes();
+                    let sp = &ep[self.manifest_dir_iter.prefix_len()..];
+                    match split_dir_and_top_level(sp, ep) {
+                        Ok(None) => match git_perms(&entry) {
+                            Err(e) => return Some(Err(e)),
+                            Ok(mode) => {
+                                return Some(self.tree_entry(
+                                    ep,
+                                    blob_oid(&self.changeset_node, ep),
+                                    EntryType::Blob,
+                                    mode,
+                                ));
+                            }
+                        },
+                        Ok(Some(split)) => {
+                            if split.top_level != self.current_subdir {
+                                self.current_subdir = split.top_level;
+                                let full_path = &ep
+                                    [..self.manifest_dir_iter.prefix_len() + split.top_level.len()];
+                                return Some(self.tree_entry(
+                                    full_path,
+                                    tree_oid(&self.changeset_node, full_path),
+                                    EntryType::Tree,
+                                    git::OBJECT_MODE_TREE,
+                                ));
+                            }
+                        }
+                        Err(e) => return Some(Err(e)),
+                    }
+                }
+            }
+        }
+        None
+    }
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -511,4 +654,92 @@
             ]
         );
     }
+
+    fn testing_blob_flat_path(cs_node: &Node, path: &[u8], flat_path: &[u8]) -> TreeEntry {
+        TreeEntry {
+            path: path.to_vec(),
+            r#type: EntryType::Blob as i32,
+            mode: git::OBJECT_MODE_NON_EXECUTABLE,
+            commit_oid: "branch/test".to_owned(),
+            root_oid: tree_oid(cs_node, b""),
+            oid: blob_oid(cs_node, path),
+            flat_path: flat_path.to_vec(),
+        }
+    }
+
+    fn testing_blob(cs_node: &Node, path: &[u8]) -> TreeEntry {
+        testing_blob_flat_path(cs_node, path, &[])
+    }
+
+    fn testing_tree_flat_path(cs_node: &Node, path: &[u8], flat_path: &[u8]) -> TreeEntry {
+        TreeEntry {
+            path: path.to_vec(),
+            r#type: EntryType::Tree as i32,
+            mode: git::OBJECT_MODE_TREE,
+            commit_oid: "branch/test".to_owned(),
+            root_oid: tree_oid(cs_node, b""),
+            oid: tree_oid(cs_node, path),
+            flat_path: flat_path.to_vec(),
+        }
+    }
+
+    fn testing_tree(cs_node: &Node, path: &[u8]) -> TreeEntry {
+        testing_tree_flat_path(cs_node, path, &[])
+    }
+
+    #[test]
+    fn test_dir_iterator_without_flat_paths() {
+        let cs_node = Node::from_hex(b"1234567812345678123456781234567812345678").unwrap();
+
+        let manifest = paths_manifest(vec!["foo/a"]);
+        let iter = DirIteratorWithoutFlatPaths::new(
+            "branch/test".to_owned(),
+            cs_node,
+            manifest.into_iter(),
+            &[],
+        );
+        let res: Vec<TreeEntry> = iter.map(|r| r.unwrap()).collect();
+        assert_eq!(res, vec![testing_tree(&cs_node, b"foo"),]);
+
+        let manifest = paths_manifest(vec![
+            "foo.",
+            "foo/a",
+            "foo/sub/a",
+            "foo/sub/b",
+            "foo/subb",
+            "foo0",
+        ]);
+        let iter = DirIteratorWithoutFlatPaths::new(
+            "branch/test".to_owned(),
+            cs_node,
+            manifest.into_iter(),
+            b"foo",
+        );
+        let res: Vec<TreeEntry> = iter.map(|r| r.unwrap()).collect();
+        assert_eq!(
+            res,
+            vec![
+                testing_blob(&cs_node, b"foo/a"),
+                testing_tree(&cs_node, b"foo/sub"),
+                testing_blob(&cs_node, b"foo/subb"),
+            ]
+        );
+
+        let manifest = paths_manifest(vec!["foo.", "foo/a", "foo/sub/ssub/b", "foo/subb", "foo0"]);
+        let iter = DirIteratorWithoutFlatPaths::new(
+            "branch/test".to_owned(),
+            cs_node,
+            manifest.into_iter(),
+            b"foo",
+        );
+        let res: Vec<TreeEntry> = iter.map(|r| r.unwrap()).collect();
+        assert_eq!(
+            res,
+            vec![
+                testing_blob(&cs_node, b"foo/a"),
+                testing_tree(&cs_node, b"foo/sub"),
+                testing_blob(&cs_node, b"foo/subb"),
+            ]
+        );
+    }
 }
diff --git a/rust/rhgitaly/src/util.rs b/rust/rhgitaly/src/util.rs
--- a/rust/rhgitaly/src/util.rs
+++ b/rust/rhgitaly/src/util.rs
@@ -38,8 +38,9 @@
                 None => {
                     if *fc == b'/' {
                         latest_slash = Some(i);
+                    } else {
+                        break;
                     }
-                    else{ break;}
                 }
                 Some(sc) => {
                     if *fc != *sc {