diff --git a/coffee_core/src/coffee.rs b/coffee_core/src/coffee.rs index 1f0f01ca..dbb0a1f3 100644 --- a/coffee_core/src/coffee.rs +++ b/coffee_core/src/coffee.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::fmt::Debug; use std::vec::Vec; -use tokio::fs; use async_trait::async_trait; use clightningrpc_common::client::Client; @@ -12,6 +11,7 @@ use log; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::json; +use tokio::fs; use tokio::process::Command; use coffee_github::repository::Github; @@ -108,11 +108,11 @@ impl CoffeeManager { .storage .load::>("repositories") .await - .map(|item| { + .map(|mut item| { log::debug!("repositories in store {:?}", item); - item.iter().for_each(|repo| match repo.1.kind { + item.iter_mut().for_each(|(_, info)| match info.kind { Kind::Git => { - let repo = Github::from(repo.1); + let repo = Github::from(info); self.repos.insert(repo.name(), Box::new(repo)); } }); @@ -432,9 +432,10 @@ impl PluginManager for CoffeeManager { if self.repos.contains_key(name) { return Err(error!("repository with name: {name} already exists")); } - let url = URL::new(&self.config.root_path, url, name); + let url = URL::new(url, name); log::debug!("remote adding: {} {}", name, &url.url_string); - let mut repo = Github::new(name, &url); + let repo_path = format!("{}/repositories/{name}", self.config.root_path); + let mut repo = Github::new(name, &repo_path, &url); repo.init().await?; self.repos.insert(repo.name(), Box::new(repo)); log::debug!("remote added: {} {}", name, &url.url_string); @@ -447,7 +448,7 @@ impl PluginManager for CoffeeManager { match self.repos.get(name) { Some(repo) => { let remote_repo = repo.list().await?; - let repo_path = repo.url().path_string; + let repo_home = repo.home(); let plugins = self.config.plugins.clone(); for plugin in &remote_repo { if let Some(ind) = plugins @@ -461,7 +462,7 @@ impl PluginManager for CoffeeManager { } } } - fs::remove_dir_all(repo_path).await?; + fs::remove_dir_all(repo_home).await?; self.repos.remove(name); log::debug!("remote removed: {}", name); self.flush().await?; @@ -476,8 +477,8 @@ impl PluginManager for CoffeeManager { async fn list_remotes(&mut self) -> Result { let mut remote_list = Vec::new(); for repo in self.repos.values() { - let repository = git2::Repository::open(repo.url().path_string.as_str()) - .map_err(|err| error!("{}", err.message()))?; + let repository = + git2::Repository::open(repo.home()).map_err(|err| error!("{}", err.message()))?; let (commit, date) = get_repo_info!(repository); remote_list.push(CoffeeListRemote { local_name: repo.name(), @@ -587,7 +588,7 @@ impl PluginManager for CoffeeManager { // Maybe when trying to recover the repository, // we have created the folder but we were not able // to clone the repository. - let repo_path = repo.url().path_string; + let repo_path = repo.home(); // This shouldn't return an error if the repository // is not present locally. let _ = fs::remove_dir_all(repo_path).await; diff --git a/coffee_core/src/nurse/strategy.rs b/coffee_core/src/nurse/strategy.rs index a1e85d4d..9777414e 100644 --- a/coffee_core/src/nurse/strategy.rs +++ b/coffee_core/src/nurse/strategy.rs @@ -56,9 +56,9 @@ impl Handler for GitRepositoryLocallyAbsentStrategy { let mut repos: Vec = Vec::new(); let coffee_repos = &coffee.repos; for repo in coffee_repos.values() { + let repo_home = repo.home(); log::debug!("Checking if repository {} exists locally", repo.name()); - let repo_path = repo.url().path_string; - let repo_path = Path::new(&repo_path); + let repo_path = Path::new(&repo_home); if !repo_path.exists() { log::debug!("Repository {} is missing locally", repo.name()); repos.push(repo.name().to_string()); diff --git a/coffee_github/src/repository.rs b/coffee_github/src/repository.rs index be237e65..8166bbde 100644 --- a/coffee_github/src/repository.rs +++ b/coffee_github/src/repository.rs @@ -27,6 +27,8 @@ pub struct Github { /// the url of the repository to be able /// to get all the plugin information. url: URL, + /// The root path of the repository + root_path: Option, /// the name of the repository that can be used /// by coffee as repository key. name: String, @@ -54,11 +56,12 @@ fn is_hidden(entry: &DirEntry) -> bool { impl Github { /// Create a new instance of the Repository /// with a name and a url - pub fn new(name: &str, url: &URL) -> Self { + pub fn new(name: &str, path: &str, url: &URL) -> Self { debug!("creating repository: {} {}", name, url.url_string); Github { name: name.to_owned(), url: url.clone(), + root_path: Some(path.to_owned()), plugins: vec![], branch: "".to_owned(), git_head: None, @@ -69,8 +72,7 @@ impl Github { /// Index the repository to store information /// related to the plugins pub async fn index_repository(&mut self) -> Result<(), CoffeeError> { - let repo_path = &self.url.path_string; - let target_dirs = WalkDir::new(repo_path) + let target_dirs = WalkDir::new(self.home()) .max_depth(1) .into_iter() .filter_entry(|dir_entry| !is_hidden(dir_entry)); @@ -203,17 +205,25 @@ impl Github { #[async_trait] impl Repository for Github { + fn home(&self) -> String { + // SAFETY: this is safe bause the optional + // value is used only for retro-compatibility + self.root_path.clone().unwrap() + } + /// Init the repository where it is required to index /// all the plugin contained, and store somewhere the index. /// /// Where to store the index is an implementation /// details. async fn init(&mut self) -> Result<(), CoffeeError> { - debug!( + log::debug!( "initializing repository: {} {} > {}", - self.name, &self.url.url_string, &self.url.path_string, + self.name, + self.url.url_string, + self.home(), ); - let res = git2::Repository::clone(&self.url.url_string, &self.url.path_string); + let res = git2::Repository::clone(&self.url.url_string, &self.home()); match res { Ok(repo) => { self.branch = if repo.find_branch("master", git2::BranchType::Local).is_ok() { @@ -225,7 +235,7 @@ impl Repository for Github { self.git_head = Some(commit.clone()); self.last_activity = Some(date.clone()); - let clone = clone_recursive_fix(repo, &self.url).await; + let clone = clone_recursive_fix(repo, &self.home()).await; self.index_repository().await?; clone } @@ -259,7 +269,7 @@ impl Repository for Github { } } // pull the changes from the repository - let status = git_upgrade(&self.url.path_string, &self.branch, verbose).await?; + let status = git_upgrade(&self.home(), &self.branch, verbose).await?; self.git_head = Some(status.commit_id()); self.last_activity = Some(status.date()); Ok(CoffeeUpgrade { @@ -275,11 +285,11 @@ impl Repository for Github { log::debug!( "recovering repository: {} {} > {}", self.name, - &self.url.url_string, - &self.url.path_string, + self.url.url_string, + self.home(), ); // recursively clone the repository - let res = git2::Repository::clone(&self.url.url_string, &self.url.path_string); + let res = git2::Repository::clone(&self.url.url_string, &self.home()); match res { Ok(repo) => { // get the commit id @@ -298,8 +308,7 @@ impl Repository for Github { // retrieve the submodules let submodules = repo.submodules().unwrap_or_default(); for (_, sub) in submodules.iter().enumerate() { - let path = - format!("{}/{}", &self.url.path_string, sub.path().to_str().unwrap()); + let path = format!("{}/{}", &self.home(), sub.path().to_str().unwrap()); if let Err(err) = git2::Repository::clone(sub.url().unwrap(), &path) { return Err(error!("{}", err.message())); } @@ -344,7 +353,8 @@ impl Repository for Github { impl From for Github { fn from(value: StorageRepository) -> Self { Github { - url: value.url, + url: value.url.clone(), + root_path: Some(value.root_path.unwrap_or(value.url.path_string.unwrap())), name: value.name, plugins: value.plugins, branch: value.branch, @@ -354,10 +364,16 @@ impl From for Github { } } -impl From<&StorageRepository> for Github { - fn from(value: &StorageRepository) -> Self { +impl From<&mut StorageRepository> for Github { + fn from(value: &mut StorageRepository) -> Self { + let root_path = value + .root_path + .clone() + .unwrap_or(value.url.path_string.clone().unwrap()); + value.url.path_string = None; // make sure that we store a none value Github { url: value.url.to_owned(), + root_path: Some(root_path), name: value.name.to_owned(), plugins: value.plugins.to_owned(), branch: value.branch.to_owned(), @@ -377,6 +393,9 @@ impl From for StorageRepository { branch: value.branch, git_head: value.git_head, last_activity: value.last_activity, + root_path: Some(value.root_path.expect( + "this should be alway not None, otherwise there is a bug in the migration logic", + )), } } } @@ -391,6 +410,9 @@ impl From<&Github> for StorageRepository { branch: value.branch.to_owned(), git_head: value.git_head.to_owned(), last_activity: value.last_activity.to_owned(), + root_path: Some(value.root_path.clone().expect( + "this should be alway not None, otherwise there is a bug in the migration logic", + )), } } } diff --git a/coffee_github/src/utils.rs b/coffee_github/src/utils.rs index 269354a5..4a9bb7c3 100644 --- a/coffee_github/src/utils.rs +++ b/coffee_github/src/utils.rs @@ -1,17 +1,19 @@ use coffee_lib::errors::CoffeeError; use coffee_lib::macros::error; -use coffee_lib::url::URL; use coffee_lib::{commit_id, get_repo_info, sh}; use log::debug; use coffee_lib::types::response::UpgradeStatus; -pub async fn clone_recursive_fix(repo: git2::Repository, url: &URL) -> Result<(), CoffeeError> { +pub async fn clone_recursive_fix( + repo: git2::Repository, + root_path: &str, +) -> Result<(), CoffeeError> { let repository = repo.submodules().unwrap_or_default(); debug!("submodule count: {}", repository.len()); for (index, sub) in repository.iter().enumerate() { debug!("url {}: {}", index + 1, sub.url().unwrap()); - let path = format!("{}/{}", &url.path_string, sub.path().to_str().unwrap()); + let path = format!("{}/{}", root_path, sub.path().to_str().unwrap()); match git2::Repository::clone(sub.url().unwrap(), &path) { // Fix error handling Ok(_) => { diff --git a/coffee_lib/src/repository.rs b/coffee_lib/src/repository.rs index dbb9bec7..7e8d44d4 100644 --- a/coffee_lib/src/repository.rs +++ b/coffee_lib/src/repository.rs @@ -41,4 +41,6 @@ pub trait Repository: Any { fn url(&self) -> URL; fn as_any(&self) -> &dyn Any; + + fn home(&self) -> String; } diff --git a/coffee_lib/src/url.rs b/coffee_lib/src/url.rs index e5172647..f7fbc1e5 100644 --- a/coffee_lib/src/url.rs +++ b/coffee_lib/src/url.rs @@ -11,10 +11,12 @@ pub struct URL { pub name: String, /// the url string pub url_string: String, - /// the coffee path associated with the url - pub path_string: String, /// the repo name associated with the url pub repo_name: String, + #[deprecated( + note = "this value is used only for migration from db, a caller you never use this value" + )] + pub path_string: Option, } /// Handle GitHub HTTP links @@ -48,23 +50,19 @@ fn get_repo_name_from_url(url: &str) -> String { impl URL { /// Build a new URL and initialize its fields - pub fn new(local_path: &str, url: &str, remote_name: &str) -> Self { + pub fn new(url: &str, remote_name: &str) -> Self { URL { name: remote_name.to_owned(), url_string: handle_incorrect_url(url), - path_string: format!("{local_path}/repositories/{remote_name}"), repo_name: get_repo_name_from_url(url), + path_string: None, } } } impl fmt::Display for URL { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "repo_name: {}, url: {}, path: {}", - self.repo_name, self.url_string, self.path_string - ) + write!(f, "repo_name: {}, url: {}", self.repo_name, self.url_string) } } @@ -75,9 +73,8 @@ mod tests { #[test] fn test_remote() { let u = "https://github.com/lightningd/plugins"; - let url = URL::new("/tmp/", u, "lightningd_plugins"); + let url = URL::new(u, "lightningd_plugins"); assert_eq!(url.repo_name, "plugins"); assert_eq!(url.url_string, u); - println!("{}", &url); } } diff --git a/coffee_storage/src/model/repository.rs b/coffee_storage/src/model/repository.rs index 190e545d..7e4203dd 100644 --- a/coffee_storage/src/model/repository.rs +++ b/coffee_storage/src/model/repository.rs @@ -12,6 +12,10 @@ pub enum Kind { pub struct Repository { pub kind: Kind, pub name: String, + #[deprecated( + note = "make this not optional, the optional value is good just for db migration" + )] + pub root_path: Option, pub url: URL, pub plugins: Vec, pub branch: String,