Skip to content

Refactor MerkleNodeDb to use repo's Path directly #523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
malcolmgreaves merged 1 commit into from
May 5, 2026
Merged

Refactor MerkleNodeDb to use repo's Path directly #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions crates/lib/src/command/migrate/m20250111083535_add_child_counts_to_nodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fn run_on_commit(repository: &LocalRepository, commit: &Commit) -> Result<(), Ox
// Write a new commit db
let commit_node = CommitNode::from_commit(commit.clone());
let mut root_commit_db =
MerkleNodeDB::open_read_write(&old_repo, &commit_node, root_node.parent_id)?;
MerkleNodeDB::open_read_write(&old_repo.path, &commit_node, root_node.parent_id)?;
root_commit_db.add_child(&dir_node)?;

let current_path = Path::new("");
Expand Down Expand Up @@ -190,7 +190,8 @@ fn rewrite_nodes(
let mut dir_node_opts = dir.get_opts();
dir_node_opts.num_entries = total_children as u64;
let dir = DirNode::new(new_repo, dir_node_opts)?;
let mut dir_db = MerkleNodeDB::open_read_write(old_repo, &dir, node.parent_id)?;
let mut dir_db =
MerkleNodeDB::open_read_write(&old_repo.path, &dir, node.parent_id)?;

// log::debug!(
// "rewrite_nodes {} VNodes for {} children in {} with vnode size {}",
Expand Down Expand Up @@ -253,8 +254,11 @@ fn rewrite_nodes(
// log::debug!("rewrite_nodes adding VNode to DirNode! {:?}", vnode_obj);
dir_db.add_child(&vnode_obj)?;

let mut vnode_db =
MerkleNodeDB::open_read_write(new_repo, &vnode_obj, Some(dir_db.node_id))?;
let mut vnode_db = MerkleNodeDB::open_read_write(
&new_repo.path,
&vnode_obj,
Some(dir_db.node_id),
)?;

// log::debug!("rewrite_nodes count entries {}", entries.len());
for entry in entries {
Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/core/db/merkle_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
pub mod merkle_node_db;

pub use merkle_node_db::MerkleNodeDB;
pub(crate) use merkle_node_db::MerkleNodeDB;
156 changes: 28 additions & 128 deletions crates/lib/src/core/db/merkle_node/merkle_node_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ use std::path::{Path, PathBuf};

use crate::constants;
use crate::error::OxenError;
use crate::model::LocalRepository;
use crate::model::MerkleHash;
use crate::model::merkle_tree::node_type::InvalidMerkleTreeNodeType;
use crate::util;
Expand All @@ -69,9 +68,9 @@ const NODE_FILE: &str = "node";
const CHILDREN_FILE: &str = "children";

/// An absolute path to the directory for the Merkle node's `node` and `children` files.
pub fn node_db_path(repo: &LocalRepository, hash: &MerkleHash) -> PathBuf {
pub(crate) fn node_db_path(repo_path: &Path, hash: &MerkleHash) -> PathBuf {
let dir_prefix = hash.to_hex_hash().node_db_prefix();
repo.path
repo_path
.join(constants::OXEN_HIDDEN_DIR)
.join(constants::TREE_DIR)
.join(constants::NODES_DIR)
Expand Down Expand Up @@ -117,13 +116,13 @@ impl MerkleDbError {
}
}

pub struct MerkleNodeLookup {
pub data_type: u8,
pub parent_id: u128,
pub data: Vec<u8>,
pub num_children: u64,
// hash -> (dtype, offset, length)
pub offsets: Vec<(u128, (u8, u64, u64))>,
struct MerkleNodeLookup {
data_type: u8,
parent_id: u128,
data: Vec<u8>,
num_children: u64,
/// hash -> (dtype, offset, length)
offsets: Vec<(u128, (u8, u64, u64))>,
}

impl MerkleNodeLookup {
Expand Down Expand Up @@ -220,29 +219,19 @@ impl MerkleNodeLookup {
}
}

pub struct MerkleNodeDB {
pub(crate) struct MerkleNodeDB {
pub dtype: MerkleTreeNodeType,
pub node_id: MerkleHash,
pub parent_id: Option<MerkleHash>,
read_only: bool,
path: PathBuf,
node_file: Option<File>,
children_file: Option<File>,
lookup: Option<MerkleNodeLookup>,
data: Vec<u8>,
num_children: u64,
data_offset: u64,
}

impl MerkleNodeDB {
pub fn num_children(&self) -> u64 {
if let Some(lookup) = &self.lookup {
return lookup.num_children;
}

self.num_children
}

pub fn data(&self) -> Vec<u8> {
if let Some(lookup) = &self.lookup {
return lookup.data.to_owned();
Expand All @@ -256,7 +245,7 @@ impl MerkleNodeDB {
Ok(node)
}

pub fn to_node(
fn to_node(
dtype: MerkleTreeNodeType,
data: &[u8],
) -> Result<EMerkleTreeNode, rmp_serde::decode::Error> {
Expand All @@ -273,44 +262,39 @@ impl MerkleNodeDB {
}
}

pub fn path(&self) -> PathBuf {
self.path.to_owned()
}

pub fn exists(repo: &LocalRepository, hash: &MerkleHash) -> bool {
let db_path = node_db_path(repo, hash);
pub(crate) fn exists(repo_path: &Path, hash: &MerkleHash) -> bool {
let db_path = node_db_path(repo_path, hash);
db_path.join(NODE_FILE).exists() && db_path.join(CHILDREN_FILE).exists()
}

pub fn open_read_only(
repo: &LocalRepository,
pub(crate) fn open_read_only(
repo_path: &Path,
hash: &MerkleHash,
) -> Result<Self, MerkleDbError> {
let path = node_db_path(repo, hash);
Self::open(path, true)
let path = node_db_path(repo_path, hash);
Self::open(path, true, *hash)
}

pub fn open_read_write<N: TMerkleTreeNode>(
repo: &LocalRepository,
pub(crate) fn open_read_write<N: TMerkleTreeNode>(
repo_path: &Path,
node: &N,
parent_id: Option<MerkleHash>,
) -> Result<Self, MerkleDbError> {
let path = node_db_path(repo, &node.hash());
let path = node_db_path(repo_path, &node.hash());
if !path.exists() {
util::fs::create_dir_all(&path).map_err(MerkleDbError::dir_create)?;
}
log::debug!("open_read_write merkle node db at {}", path.display());
let mut db = Self::open(path, false)?;
let mut db = Self::open(path, false, node.hash())?;
db.write_node(node, parent_id)?;
Ok(db)
}

pub fn open(path: impl AsRef<Path>, read_only: bool) -> Result<Self, MerkleDbError> {
let path = path.as_ref();

/// The `node_id: MerkleHash` **MUST** be the same one that the `path` was derived from!
fn open(path: PathBuf, read_only: bool, node_id: MerkleHash) -> Result<Self, MerkleDbError> {
// mkdir if not exists
if !path.exists() {
util::fs::create_dir_all(path).map_err(MerkleDbError::dir_create)?;
util::fs::create_dir_all(&path).map_err(MerkleDbError::dir_create)?;
}
Comment thread
malcolmgreaves marked this conversation as resolved.

let node_path = path.join(NODE_FILE);
Expand Down Expand Up @@ -349,22 +333,20 @@ impl MerkleNodeDB {
let parent_id = lookup.as_ref().map(|l| l.parent_id);
Ok(Self {
read_only,
path: path.to_path_buf(),
node_file,
children_file,
lookup,
data: vec![],
num_children: 0,
dtype,
node_id: MerkleHash::new(0),
node_id,
parent_id: parent_id.map(MerkleHash::new),
data_offset: 0,
})
}

/// Closes the open node and children file handles.
/// WARNING: Sets the internal node_file, children_file, and lookup to None.
pub fn close(&mut self) -> Result<(), MerkleDbError> {
pub(crate) fn close(&mut self) -> Result<(), MerkleDbError> {
if let Some(node_file) = &mut self.node_file {
node_file.flush()?;
node_file.sync_data()?;
Expand Down Expand Up @@ -435,7 +417,7 @@ impl MerkleNodeDB {
Ok(())
}

pub fn add_child<N: TMerkleTreeNode>(&mut self, item: &N) -> Result<(), MerkleDbError> {
pub(crate) fn add_child<N: TMerkleTreeNode>(&mut self, item: &N) -> Result<(), MerkleDbError> {
if self.read_only {
return Err(MerkleDbError::ReadOnly);
}
Expand Down Expand Up @@ -469,46 +451,7 @@ impl MerkleNodeDB {
Ok(())
}

/*
pub fn get<D>(&self, hash: u128) -> Result<D, OxenError>
where
D: TMerkleTreeNode + de::DeserializeOwned,
{
let Some(lookup) = self.lookup.as_ref() else {
return Err(MerkleError::ReadBeforeOpen);
};

let Some(mut children_file) = self.children_file.as_ref() else {
return Err(MerkleError::WriteBeforeOpen);
};

// Find the offset and length of the data
let Some(offset) = lookup.offsets.get(&hash) else {
let err_str = format!(
"Cannot find hash in merkle node db: {:x} in {} offsets",
hash,
lookup.offsets.len()
);
return Err(OxenError::basic_str(err_str));
};

// Read from the data table at the offset
// Allocate the exact amount of data
let mut data = vec![0; offset.2 as usize];
children_file.seek(SeekFrom::Start(offset.1))?;
children_file.read_exact(&mut data)?;

let val: D = rmp_serde::from_slice(&data).map_err(|e| {
OxenError::basic_str(format!(
"MerkleNodeDB.get({}): Error deserializing data: {:?}",
hash, e
))
})?;
Ok(val)
}
*/

pub fn map(&mut self) -> Result<Vec<(MerkleHash, MerkleTreeNode)>, MerkleDbError> {
pub(crate) fn map(&mut self) -> Result<Vec<(MerkleHash, MerkleTreeNode)>, MerkleDbError> {
// log::debug!("Loading merkle node db map");
let Some(lookup) = self.lookup.as_ref() else {
return Err(MerkleDbError::ReadBeforeOpen);
Expand Down Expand Up @@ -551,46 +494,3 @@ impl MerkleNodeDB {
Ok(ret)
}
}

#[cfg(test)]
mod tests {
use super::*;

use crate::test;

#[test]
fn test_merkle_node_db() -> Result<(), OxenError> {
test::run_empty_dir_test(|_dir| {
/*
let vnode = VNode {
id: 1234,
dtype: MerkleTreeNodeType::VNode,
};
let mut writer_db = MerkleNodeDB::open_read_write(dir, &vnode, 2)?;

let node_1 = DirNode {
path: "test".to_string(),
};
writer_db.add_child(1234, MerkleTreeNodeType::Dir, &node_1)?;

let node_2 = DirNode {
path: "image".to_string(),
};
writer_db.add_child(5678, MerkleTreeNodeType::Dir, &node_2)?;
writer_db.close()?;

let reader_db = MerkleNodeDB::open_read_only(dir)?;

let size = reader_db.num_children();
assert_eq!(size, 2);

let data: DirNode = reader_db.get(1234)?;
assert_eq!(data, node_1);

let data: DirNode = reader_db.get(5678)?;
assert_eq!(data, node_2);
*/
Ok(())
})
}
}
4 changes: 2 additions & 2 deletions crates/lib/src/core/v_latest/commits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub fn create_empty_commit(
)?;

let parent_id = Some(existing_node.hash);
let mut commit_db = MerkleNodeDB::open_read_write(repo, &commit_node, parent_id)?;
let mut commit_db = MerkleNodeDB::open_read_write(&repo.path, &commit_node, parent_id)?;
// There should always be one child, the root directory
let dir_node = existing_node.children.first().unwrap().dir()?;
commit_db.add_child(&dir_node)?;
Expand Down Expand Up @@ -363,7 +363,7 @@ pub fn create_initial_commit(
)?;

// Open the commit database and add the root directory
let mut commit_db = MerkleNodeDB::open_read_write(repo, &commit_node, None)?;
let mut commit_db = MerkleNodeDB::open_read_write(&repo.path, &commit_node, None)?;
commit_db.add_child(&dir_node)?;

// Initialize the dir_hash_db with the root directory hash
Expand Down
7 changes: 4 additions & 3 deletions crates/lib/src/core/v_latest/entries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,8 @@ fn traverse_and_update_sizes_and_counts(
&mut local_sizes,
num_bytes,
)?;
let mut dir_db = MerkleNodeDB::open_read_write(repo, commit_node, node.parent_id)?;
let mut dir_db =
MerkleNodeDB::open_read_write(&repo.path, commit_node, node.parent_id)?;
add_children_to_db(&mut dir_db, &node.children)?;
}
EMerkleTreeNode::VNode(vnode) => {
Expand All @@ -584,7 +585,7 @@ fn traverse_and_update_sizes_and_counts(
&mut local_sizes,
num_bytes,
)?;
let mut dir_db = MerkleNodeDB::open_read_write(repo, vnode, node.parent_id)?;
let mut dir_db = MerkleNodeDB::open_read_write(&repo.path, vnode, node.parent_id)?;
add_children_to_db(&mut dir_db, &node.children)?;
}
EMerkleTreeNode::Directory(dir_node) => {
Expand All @@ -598,7 +599,7 @@ fn traverse_and_update_sizes_and_counts(
)?;
dir_node.set_data_type_counts(local_counts.clone());
dir_node.set_data_type_sizes(local_sizes.clone());
let mut dir_db = MerkleNodeDB::open_read_write(repo, dir_node, node.parent_id)?;
let mut dir_db = MerkleNodeDB::open_read_write(&repo.path, dir_node, node.parent_id)?;
add_children_to_db(&mut dir_db, &node.children)?;
}
EMerkleTreeNode::File(file_node) => {
Expand Down
Loading
Loading