core: remove repo path from url

Signed-off-by: Vincenzo Palazzo <[email protected]>

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::<HashMap<RepoName, RepositoryInfo>>("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<CoffeeRemote, CoffeeError> {
         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;

coffee_core/src/nurse/strategy.rs

@@ -56,9 +56,9 @@ impl Handler for GitRepositoryLocallyAbsentStrategy {
         let mut repos: Vec<String> = 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());

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<String>,
     /// 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<StorageRepository> 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<StorageRepository> 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<Github> 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",
+            )),
         }
     }
 }

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(_) => {

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;
 }

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<String>,
 }
 
 /// 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);
     }
 }

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<String>,
     pub url: URL,
     pub plugins: Vec<Plugin>,
     pub branch: String,