diff --git a/crates/lib/src/api/client/repositories.rs b/crates/lib/src/api/client/repositories.rs index 11da4fc04..559d248b0 100644 --- a/crates/lib/src/api/client/repositories.rs +++ b/crates/lib/src/api/client/repositories.rs @@ -647,8 +647,7 @@ mod tests { let mock_pre_push = server.mock("POST", &path[..]).create_async().await; // Branch and commit id we're pushing - let branch = - repositories::branches::get_by_name(&local_repo, DEFAULT_BRANCH_NAME)?.unwrap(); + let branch = repositories::branches::get_by_name(&local_repo, DEFAULT_BRANCH_NAME)?; let commit_id = branch.commit_id.clone(); api::client::repositories::pre_push(&remote_repo, &branch, &commit_id).await?; diff --git a/crates/lib/src/core/v_latest/branches.rs b/crates/lib/src/core/v_latest/branches.rs index fd55c41ab..d9edc6140 100644 --- a/crates/lib/src/core/v_latest/branches.rs +++ b/crates/lib/src/core/v_latest/branches.rs @@ -144,8 +144,7 @@ pub async fn checkout( from_commit: &Option, ) -> Result<(), OxenError> { log::debug!("checkout {branch_name}"); - let branch = repositories::branches::get_by_name(repo, branch_name)? - .ok_or_else(|| OxenError::local_branch_not_found(branch_name))?; + let branch = repositories::branches::get_by_name(repo, branch_name)?; let commit = repositories::commits::get_by_id(repo, &branch.commit_id)? .ok_or_else(|| OxenError::commit_id_does_not_exist(&branch.commit_id))?; diff --git a/crates/lib/src/core/v_latest/commits.rs b/crates/lib/src/core/v_latest/commits.rs index 3d6742ec5..4f8bd79bd 100644 --- a/crates/lib/src/core/v_latest/commits.rs +++ b/crates/lib/src/core/v_latest/commits.rs @@ -131,7 +131,6 @@ fn get_commit_by_ref + Clone>( fn get_commit_by_branch(repo: &LocalRepository, branch_name: &str) -> Option { repositories::branches::get_by_name(repo, branch_name) .ok() - .flatten() .and_then(|branch| get_by_id(repo, &branch.commit_id).ok().flatten()) } diff --git a/crates/lib/src/core/v_latest/merge.rs b/crates/lib/src/core/v_latest/merge.rs index d885777fa..9eef26507 100644 --- a/crates/lib/src/core/v_latest/merge.rs +++ b/crates/lib/src/core/v_latest/merge.rs @@ -216,8 +216,7 @@ pub async fn merge( ) -> Result, OxenError> { let branch_name = branch_name.as_ref(); - let merge_branch = repositories::branches::get_by_name(repo, branch_name)? - .ok_or_else(|| OxenError::local_branch_not_found(branch_name))?; + let merge_branch = repositories::branches::get_by_name(repo, branch_name)?; let base_commit = repositories::commits::head_commit(repo)?; let merge_commit = get_commit_or_head(repo, Some(merge_branch.commit_id.clone()))?; diff --git a/crates/lib/src/core/v_latest/push.rs b/crates/lib/src/core/v_latest/push.rs index 715d5f781..e1e1f55f4 100644 --- a/crates/lib/src/core/v_latest/push.rs +++ b/crates/lib/src/core/v_latest/push.rs @@ -40,9 +40,7 @@ pub async fn push_remote_branch( // start a timer let start = std::time::Instant::now(); - let Some(local_branch) = repositories::branches::get_by_name(repo, &opts.branch)? else { - return Err(OxenError::local_branch_not_found(&opts.branch)); - }; + let local_branch = repositories::branches::get_by_name(repo, &opts.branch)?; println!( "🐂 oxen push {} {} -> {}", diff --git a/crates/lib/src/core/v_latest/workspaces/commit.rs b/crates/lib/src/core/v_latest/workspaces/commit.rs index 4a142ffe9..d56bca083 100644 --- a/crates/lib/src/core/v_latest/workspaces/commit.rs +++ b/crates/lib/src/core/v_latest/workspaces/commit.rs @@ -33,20 +33,16 @@ pub async fn commit( let repo = &workspace.base_repo; let commit = &workspace.commit; - let mut branch = repositories::branches::get_by_name(repo, branch_name)?; + let branch = match repositories::branches::get_by_name(repo, branch_name) { + Ok(branch) => branch, + Err(OxenError::BranchNotFound(_)) => { + log::debug!("commit creating branch: {branch_name}"); + repositories::branches::create(repo, branch_name, &commit.id)? + } + Err(e) => return Err(e), + }; log::debug!("commit looking up branch: {:#?}", &branch); - if branch.is_none() { - log::debug!("commit creating branch: {branch_name}"); - branch = Some(repositories::branches::create( - repo, - branch_name, - &commit.id, - )?); - } - - let branch = branch.unwrap(); - let staged_db_path = util::fs::oxen_hidden_dir(&workspace.workspace_repo.path).join(STAGED_DIR); log::debug!("workspaces::commit staged db path: {staged_db_path:?}"); @@ -108,10 +104,7 @@ pub fn mergeability( branch_name: impl AsRef, ) -> Result { let branch_name = branch_name.as_ref(); - let Some(branch) = repositories::branches::get_by_name(&workspace.base_repo, branch_name)? - else { - return Err(OxenError::BranchNotFound(branch_name.into())); - }; + let branch = repositories::branches::get_by_name(&workspace.base_repo, branch_name)?; let base = &workspace.commit; let Some(head) = repositories::commits::get_by_id(&workspace.base_repo, &branch.commit_id)? diff --git a/crates/lib/src/repositories/branches.rs b/crates/lib/src/repositories/branches.rs index 579b29496..626e08bbc 100644 --- a/crates/lib/src/repositories/branches.rs +++ b/crates/lib/src/repositories/branches.rs @@ -22,9 +22,10 @@ pub fn list_with_commits(repo: &LocalRepository) -> Result with_ref_manager(repo, |manager| manager.list_branches_with_commits()) } -/// Get a branch by name -pub fn get_by_name(repo: &LocalRepository, name: &str) -> Result, OxenError> { - with_ref_manager(repo, |manager| manager.get_branch_by_name(name)) +/// Get a branch by name, returning an error if it doesn't exist +pub fn get_by_name(repo: &LocalRepository, name: &str) -> Result { + with_ref_manager(repo, |manager| manager.get_branch_by_name(name))? + .ok_or_else(|| OxenError::local_branch_not_found(name)) } /// Get branch by name or fall back the current @@ -34,10 +35,7 @@ pub fn get_by_name_or_current( ) -> Result { if let Some(branch_name) = branch_name { let branch_name = branch_name.as_ref(); - match repositories::branches::get_by_name(repo, branch_name)? { - Some(branch) => Ok(branch), - None => Err(OxenError::local_branch_not_found(branch_name)), - } + repositories::branches::get_by_name(repo, branch_name) } else { match repositories::branches::current_branch(repo)? { Some(branch) => Ok(branch), @@ -55,11 +53,8 @@ pub fn get_commit_id(repo: &LocalRepository, name: &str) -> Result Result { - match get_by_name(repo, name)? { - Some(_) => Ok(true), - None => Ok(false), - } +pub fn exists(repo: &LocalRepository, name: &str) -> bool { + get_by_name(repo, name).is_ok() } /// Get the current branch @@ -270,8 +265,7 @@ pub fn list_entry_versions_on_branch( branch_name: &str, path: &Path, ) -> Result, OxenError> { - let branch = repositories::branches::get_by_name(local_repo, branch_name)? - .ok_or_else(|| OxenError::local_branch_not_found(branch_name))?; + let branch = repositories::branches::get_by_name(local_repo, branch_name)?; log::debug!( "get branch commits for branch {:?} -> {}", branch.name, @@ -348,7 +342,7 @@ mod tests { let commit_3 = repositories::commit(&repo, "adding test file 2")?; - let _branch = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)?.unwrap(); + let _branch = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)?; let file_versions = repositories::branches::list_entry_versions_on_branch(&repo, "main", &file_path)?; @@ -434,8 +428,8 @@ mod tests { repositories::add(&repo, &repo.path).await?; let commit_6 = repositories::commit(&repo, "adding test file 6")?; - let _main = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)?.unwrap(); - let _branch = repositories::branches::get_by_name(&repo, "test_branch")?.unwrap(); + let _main = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)?; + let _branch = repositories::branches::get_by_name(&repo, "test_branch")?; let main_versions = repositories::branches::list_entry_versions_on_branch(&repo, "main", &file_path)?; diff --git a/crates/lib/src/repositories/checkout.rs b/crates/lib/src/repositories/checkout.rs index 9e6d8f2f2..5cfa8985b 100644 --- a/crates/lib/src/repositories/checkout.rs +++ b/crates/lib/src/repositories/checkout.rs @@ -20,10 +20,10 @@ pub async fn checkout( ) -> Result, OxenError> { let value = value.as_ref(); log::debug!("--- CHECKOUT START {value} ----"); - if repositories::branches::exists(repo, value)? { + if repositories::branches::exists(repo, value) { if repositories::branches::is_checked_out(repo, value) { println!("Already on branch {value}"); - return repositories::branches::get_by_name(repo, value); + return Ok(Some(repositories::branches::get_by_name(repo, value)?)); } println!("Checkout branch: {value}"); @@ -37,7 +37,7 @@ pub async fn checkout( repositories::branches::checkout_subtrees_to_commit(repo, &commit, &subtree_paths, depth) .await?; repositories::branches::set_head(repo, value)?; - repositories::branches::get_by_name(repo, value) + Ok(Some(repositories::branches::get_by_name(repo, value)?)) } else { // If we are already on the commit, do nothing if repositories::branches::is_checked_out(repo, value) { diff --git a/crates/lib/src/repositories/entries.rs b/crates/lib/src/repositories/entries.rs index 47f28f016..f6e810ce7 100644 --- a/crates/lib/src/repositories/entries.rs +++ b/crates/lib/src/repositories/entries.rs @@ -76,7 +76,7 @@ pub fn list_directory_w_version( MinOxenVersion::V0_10_0 => panic!("v0.10.0 no longer supported"), _ => { let revision_str = revision.as_ref().to_string(); - let branch = repositories::branches::get_by_name(repo, &revision_str)?; + let branch = repositories::branches::get_by_name(repo, &revision_str).ok(); let commit = repositories::revisions::get(repo, &revision_str)?; let parsed_resource = ParsedResource { path: directory.as_ref().to_path_buf(), @@ -140,7 +140,7 @@ pub fn list_directory_w_workspace_depth( revision_str.clone() }; - let branch = repositories::branches::get_by_name(repo, &revision_str)?; + let branch = repositories::branches::get_by_name(repo, &revision_str).ok(); let commit = repositories::revisions::get(repo, &revision_str)?; let parsed_resource = ParsedResource { path: directory.as_ref().to_path_buf(), diff --git a/crates/lib/src/repositories/load.rs b/crates/lib/src/repositories/load.rs index 5c9dedb31..3d4ea6588 100644 --- a/crates/lib/src/repositories/load.rs +++ b/crates/lib/src/repositories/load.rs @@ -41,8 +41,7 @@ pub async fn load( let repo = LocalRepository::from_dir(&dest_path)?; println!("🐂 Unpacking files to working directory {dest_path:?}"); - let branch = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)? - .ok_or_else(|| OxenError::local_branch_not_found(DEFAULT_BRANCH_NAME))?; + let branch = repositories::branches::get_by_name(&repo, DEFAULT_BRANCH_NAME)?; let commit = repositories::commits::get_by_id(&repo, &branch.commit_id)? .ok_or_else(|| OxenError::commit_id_does_not_exist(&branch.commit_id))?; repositories::branches::set_working_repo_to_commit(&repo, &commit, &None).await?; diff --git a/crates/lib/src/repositories/merge.rs b/crates/lib/src/repositories/merge.rs index d61a2d181..d45317e92 100644 --- a/crates/lib/src/repositories/merge.rs +++ b/crates/lib/src/repositories/merge.rs @@ -655,9 +655,8 @@ mod tests { assert!(result.is_some()); // But now there should be conflicts when trying to merge in the human branch - let base_branch = repositories::branches::get_by_name(&repo, &og_branch.name)?.unwrap(); - let merge_branch = - repositories::branches::get_by_name(&repo, human_branch_name)?.unwrap(); + let base_branch = repositories::branches::get_by_name(&repo, &og_branch.name)?; + let merge_branch = repositories::branches::get_by_name(&repo, human_branch_name)?; // Check if there are conflicts let has_conflicts = @@ -979,8 +978,8 @@ mod tests { repositories::commit(&repo, "Adding fish to labels.txt file")?; // 4. merge main onto new branch (re-fetch branches to get current commit IDs) - let og_branch = repositories::branches::get_by_name(&repo, &og_branch.name)?.unwrap(); - let new_branch = repositories::branches::get_by_name(&repo, new_branch_name)?.unwrap(); + let og_branch = repositories::branches::get_by_name(&repo, &og_branch.name)?; + let new_branch = repositories::branches::get_by_name(&repo, new_branch_name)?; let merge_result = repositories::merge::merge_into_base(&repo, &og_branch, &new_branch).await; diff --git a/crates/lib/src/repositories/revisions.rs b/crates/lib/src/repositories/revisions.rs index 533533700..53732e7fc 100644 --- a/crates/lib/src/repositories/revisions.rs +++ b/crates/lib/src/repositories/revisions.rs @@ -18,17 +18,10 @@ pub fn get(repo: &LocalRepository, revision: impl AsRef) -> Result actix_web::Result Result { - let maybe_new_branch: Option = - repositories::branches::get_by_name(repo, &data.new_name)?; - if let Some(branch) = maybe_new_branch { - let view = BranchResponse { - status: StatusMessage::resource_found(), - branch, - }; - return Ok(HttpResponse::Ok().json(view)); + match repositories::branches::get_by_name(repo, &data.new_name) { + Ok(branch) => { + let view = BranchResponse { + status: StatusMessage::resource_found(), + branch, + }; + return Ok(HttpResponse::Ok().json(view)); + } + Err(OxenError::BranchNotFound(_)) => {} // branch doesn't exist yet, continue + Err(e) => return Err(e.into()), } - let from_branch = repositories::branches::get_by_name(repo, &data.from_name)? - .ok_or(OxenHttpError::NotFound)?; + let from_branch = repositories::branches::get_by_name(repo, &data.from_name) + .map_err(|_| OxenHttpError::NotFound)?; let new_branch = repositories::branches::create(repo, &data.new_name, from_branch.commit_id)?; @@ -226,8 +227,7 @@ pub async fn delete(req: HttpRequest) -> actix_web::Result = serde_json::from_str(&body); let data = data.map_err(|err| OxenHttpError::BadRequest(format!("{err:?}").into()))?; @@ -386,8 +385,7 @@ pub async fn list_entry_versions( // Get branch let repo = get_repo(&app_data.path, namespace.clone(), &repo_name)?; - let branch = repositories::branches::get_by_name(&repo, &branch_name)? - .ok_or_else(|| OxenError::remote_branch_not_found(&branch_name))?; + let branch = repositories::branches::get_by_name(&repo, &branch_name)?; let path = PathBuf::from(path_param(&req, "path")?); let repo = get_repo(&app_data.path, namespace, &repo_name)?; diff --git a/crates/server/src/controllers/workspaces.rs b/crates/server/src/controllers/workspaces.rs index 3ec1c3444..07d208824 100644 --- a/crates/server/src/controllers/workspaces.rs +++ b/crates/server/src/controllers/workspaces.rs @@ -63,9 +63,9 @@ pub async fn get_or_create( }; // Try to get the branch, or create it if the repo is empty - let branch = match repositories::branches::get_by_name(&repo, &data.branch_name)? { - Some(branch) => branch, - None => { + let branch = match repositories::branches::get_by_name(&repo, &data.branch_name) { + Ok(branch) => branch, + Err(OxenError::BranchNotFound(_)) => { // Branch doesn't exist - check if repo is empty if repositories::commits::head_commit_maybe(&repo)?.is_some() { // Repo has commits but branch doesn't exist - this is an error @@ -95,8 +95,8 @@ pub async fn get_or_create( // Now get the newly created branch repositories::branches::get_by_name(&repo, &data.branch_name)? - .ok_or_else(|| OxenError::basic_str("Failed to create initial branch"))? } + Err(e) => return Err(e.into()), }; // Return workspace if it already exists @@ -401,8 +401,14 @@ pub async fn commit(req: HttpRequest, body: String) -> Result branch, + Err(OxenError::BranchNotFound(_)) => { + return Ok( + HttpResponse::NotFound().json(StatusMessageDescription::not_found(branch_name)) + ); + } + Err(e) => return Err(e.into()), }; match repositories::workspaces::commit(&workspace, &data, &branch_name).await { diff --git a/crates/server/src/controllers/workspaces/data_frames.rs b/crates/server/src/controllers/workspaces/data_frames.rs index 6e13b5426..8d9c0acab 100644 --- a/crates/server/src/controllers/workspaces/data_frames.rs +++ b/crates/server/src/controllers/workspaces/data_frames.rs @@ -488,8 +488,7 @@ pub async fn get_by_branch( let page_size = query.page_size.unwrap_or(constants::DEFAULT_PAGE_SIZE); // Staged dataframes must be on a branch. - let branch = repositories::branches::get_by_name(&repo, branch_name)? - .ok_or_else(|| OxenError::remote_branch_not_found(branch_name))?; + let branch = repositories::branches::get_by_name(&repo, branch_name)?; let commit = repositories::commits::get_by_id(&repo, &branch.commit_id)? .ok_or_else(|| OxenError::resource_not_found(&branch.commit_id))?; diff --git a/crates/server/src/params.rs b/crates/server/src/params.rs index d58d64c7c..41c0eabe4 100644 --- a/crates/server/src/params.rs +++ b/crates/server/src/params.rs @@ -155,8 +155,11 @@ pub fn resolve_revision( } pub fn resolve_branch(repo: &LocalRepository, name: &str) -> Result, OxenError> { - // Lookup branch name - repositories::branches::get_by_name(repo, name) + match repositories::branches::get_by_name(repo, name) { + Ok(branch) => Ok(Some(branch)), + Err(OxenError::BranchNotFound(_)) => Ok(None), + Err(e) => Err(e), + } } fn user_cli_is_out_of_date(user_agent: &str) -> bool {