Skip to content

Commit

Permalink
fix(core): store the repository globally
Browse files Browse the repository at this point in the history
While working with coffee, we noted that there was a
limitation on how we store the repositories.

In fact, we store them once per network, and this causes problem
when we try to work with multiple networks,
and installing the same plugins.

Now with this commit, we store the repositories globally,
outside of the configuration network, so in this way we
can download once the remote is on and use
it in multiple directories.

Link: #172
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
  • Loading branch information
vincenzopalazzo committed Jul 18, 2023
1 parent 3ff3213 commit a104871
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 53 deletions.
48 changes: 28 additions & 20 deletions coffee_core/src/coffee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ impl From<&CoffeeManager> for CoffeeStorageInfo {

pub struct CoffeeManager {
config: config::CoffeeConf,
#[serde(skip_serializing, skip_deerializing)]
repos: HashMap<String, Box<dyn Repository + Send + Sync>>,
/// Core lightning configuration managed by coffee
coffee_cln_config: CLNConf,
Expand All @@ -68,7 +67,7 @@ pub struct CoffeeManager {
cln_config: Option<CLNConf>,
/// storage instance to make all the plugin manager
/// information persistent on disk
storage: Box<dyn StorageManager<CoffeeStorageInfo, Err = CoffeeError> + Send + Sync>,
storage: NoSQlStorage,
/// core lightning rpc connection
rpc: Option<Client>,
}
Expand All @@ -80,7 +79,7 @@ impl CoffeeManager {
config: conf.clone(),
coffee_cln_config: CLNConf::new(conf.config_path, true),
repos: HashMap::new(),
storage: Box::new(NoSQlStorage::new(&conf.root_path).await?),
storage: NoSQlStorage::new(&conf.root_path).await?,
cln_config: None,
rpc: None,
};
Expand All @@ -91,23 +90,28 @@ impl CoffeeManager {
/// when coffee is configured, run an inventory to collect all the necessary information
/// about the coffee ecosystem.
async fn inventory(&mut self) -> Result<(), CoffeeError> {
let Ok(store) = self.storage.load(&self.config.network).await else {
log::debug!("storage do not exist");
return Ok(());
};
// this is really needed? I think no, because coffee at this point
// have a new conf loading
self.config = store.config;
let repositories = self.storage.load::<>("repository")?;
store
.repositories
.iter()
.for_each(|repo| match repo.1.kind {
Kind::Git => {
let repo = Github::from(repo.1);
self.repos.insert(repo.name(), Box::new(repo));
}
self.storage

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (stable)

unused `Result` that must be used

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (stable)

unused `Result` that must be used

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (beta)

unused `Result` that must be used

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (beta)

unused `Result` that must be used

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (nightly)

unused `Result` that must be used

Check warning on line 93 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (nightly)

unused `Result` that must be used
.load::<CoffeeStorageInfo>(&self.config.network)
.await
.and_then(|store| {
self.config = store.config;
Ok(())
});
// FIXME: check if this exist in a better wai
self.storage

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (stable)

unused `Result` that must be used

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (stable)

unused `Result` that must be used

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (beta)

unused `Result` that must be used

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (beta)

unused `Result` that must be used

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (nightly)

unused `Result` that must be used

Check warning on line 101 in coffee_core/src/coffee.rs

View workflow job for this annotation

GitHub Actions / Build (nightly)

unused `Result` that must be used
.load::<HashMap<RepoName, RepositoryInfo>>("repositories")
.await
.and_then(|item| {
log::debug!("repositories in store {:?}", item);
item.iter().for_each(|repo| match repo.1.kind {
Kind::Git => {
let repo = Github::from(repo.1);
self.repos.insert(repo.name(), Box::new(repo));
}
});
Ok(())
});

if let Err(err) = self.coffee_cln_config.parse() {
log::error!("{}", err.cause);
}
Expand Down Expand Up @@ -151,8 +155,12 @@ impl CoffeeManager {
}

pub async fn flush(&self) -> Result<(), CoffeeError> {
let store_info = self.storage_info();
self.storage
.store(&self.config.network, &store_info)
.await?;
self.storage
.store(&self.config.network, &self.storage_info())
.store("repositories", &store_info.repositories)
.await?;
Ok(())
}
Expand Down
4 changes: 3 additions & 1 deletion coffee_github/src/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ impl Github {
}

let Some(exec_path) = exec_path else {
return Err(error!("exec path not known, but we should know at this point."));
return Err(error!(
"exec path not known, but we should know at this point."
));
};

debug!("exec path is {exec_path}");
Expand Down
2 changes: 1 addition & 1 deletion coffee_lib/src/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
/// This struct will make sure our URLs are of the
/// correct format and will also check correctness
/// of associated fields
#[derive(Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct URL {
/// the url name in case of remote
pub name: String,
Expand Down
6 changes: 3 additions & 3 deletions coffee_storage/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ impl FileStorage {
}

#[async_trait]
impl<T> StorageManager<T> for FileStorage {
impl StorageManager for FileStorage {
type Err = CoffeeError;

async fn load<'c>(&self, _: &str) -> Result<T, Self::Err>
async fn load<T>(&self, _: &str) -> Result<T, Self::Err>
where
T: DeserializeOwned + Send + Sync,
{
Expand All @@ -50,7 +50,7 @@ impl<T> StorageManager<T> for FileStorage {
Ok(val)
}

async fn store(&self, _: &str, to_store: &T) -> Result<(), Self::Err>
async fn store<T>(&self, _: &str, to_store: &T) -> Result<(), Self::Err>
where
T: Serialize + Send + Sync,
{
Expand Down
4 changes: 2 additions & 2 deletions coffee_storage/src/model/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
use coffee_lib::{plugin::Plugin, url::URL};
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize)]
#[derive(Debug, Serialize, Deserialize)]
pub enum Kind {
Git,
}

#[derive(Serialize, Deserialize)]
#[derive(Debug, Serialize, Deserialize)]
pub struct Repository {
pub kind: Kind,
pub name: String,
Expand Down
6 changes: 3 additions & 3 deletions coffee_storage/src/nosql_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ impl NoSQlStorage {
}

#[async_trait]
impl<T> StorageManager<T> for NoSQlStorage {
impl StorageManager for NoSQlStorage {
type Err = CoffeeError;

async fn load<'c>(&self, key: &str) -> Result<T, Self::Err>
async fn load<T>(&self, key: &str) -> Result<T, Self::Err>
where
T: serde::de::DeserializeOwned + Send + Sync,
{
Expand All @@ -40,7 +40,7 @@ impl<T> StorageManager<T> for NoSQlStorage {
Ok(value)
}

async fn store(&self, key: &str, to_store: &T) -> Result<(), Self::Err>
async fn store<T>(&self, key: &str, to_store: &T) -> Result<(), Self::Err>
where
T: serde::Serialize + Send + Sync,
{
Expand Down
6 changes: 3 additions & 3 deletions coffee_storage/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ use async_trait::async_trait;
use serde::{de::DeserializeOwned, Serialize};

#[async_trait]
pub trait StorageManager<T> {
pub trait StorageManager {
type Err;

/// async call to persist the information
/// on disk.
async fn store(&self, key: &str, to_store: &T) -> Result<(), Self::Err>
async fn store<T>(&self, key: &str, to_store: &T) -> Result<(), Self::Err>
where
T: Serialize + Send + Sync;

/// async call to load the data that was made persistent
/// from the previous `store` call.
async fn load<'c>(&self, key: &str) -> Result<T, Self::Err>
async fn load<T>(&self, key: &str) -> Result<T, Self::Err>
where
T: DeserializeOwned + Send + Sync;
}
9 changes: 5 additions & 4 deletions coffee_testing/src/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@ impl Drop for BtcNode {
fn drop(&mut self) {
for process in self.process.iter() {
let Some(child) = process.id() else {
continue;
continue;
};
let Ok(mut kill) = std::process::Command::new("kill")
.args(["-s", "SIGKILL", &child.to_string()])
.spawn() else {
continue;
};
.spawn()
else {
continue;
};
let _ = kill.wait();
}

Expand Down
9 changes: 5 additions & 4 deletions coffee_testing/src/cln.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,14 @@ impl Drop for Node {
fn drop(&mut self) {
for process in self.process.iter() {
let Some(child) = process.id() else {
continue;
continue;
};
let Ok(mut kill) = std::process::Command::new("kill")
.args(["-s", "SIGKILL", &child.to_string()])
.spawn() else {
continue;
};
.spawn()
else {
continue;
};
let _ = kill.wait();
}

Expand Down
13 changes: 7 additions & 6 deletions coffee_testing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,14 @@ pub struct CoffeeHTTPDTesting {
impl Drop for CoffeeHTTPDTesting {
fn drop(&mut self) {
let Some(child) = self.httpd_pid.id() else {
return;
};
return;
};
let Ok(mut kill) = std::process::Command::new("kill")
.args(["-s", "SIGKILL", &child.to_string()])
.spawn() else {
return;
};
.args(["-s", "SIGKILL", &child.to_string()])
.spawn()
else {
return;
};
let _ = kill.wait();
}
}
Expand Down
16 changes: 10 additions & 6 deletions tests/src/coffee_integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,8 @@ pub async fn install_plugin_in_two_networks() -> anyhow::Result<()> {
};
let mut manager = CoffeeTesting::tmp_with_args(&args, dir.clone()).await?;
let root_path = manager.root_path().to_owned();
manager
.coffee()
.setup(&lightning_regtest_dir)
.await
.unwrap();
let result = manager.coffee().setup(&lightning_regtest_dir).await;
assert!(result.is_ok(), "{:?}", result);
// Add lightningd remote repository
manager
.coffee()
Expand All @@ -333,6 +330,7 @@ pub async fn install_plugin_in_two_networks() -> anyhow::Result<()> {

// dropping the first coffee instance, but without delete the dir
drop(manager);
log::info!("------ second run -------");
// initialize a lightning node in testnet network
let mut cln = Node::tmp("testnet").await.unwrap();
let lightning_dir = cln.rpc().getinfo().unwrap().ligthning_dir;
Expand All @@ -345,12 +343,18 @@ pub async fn install_plugin_in_two_networks() -> anyhow::Result<()> {
};
let mut manager = CoffeeTesting::tmp_with_args(&new_args, dir.clone()).await?;
let new_root_path = manager.root_path().to_owned();
assert_eq!(root_path.path(), new_root_path.path());
assert_eq!(dir.path(), new_root_path.path());
manager
.coffee()
.setup(&lightning_testnet_dir)
.await
.unwrap();

let result = manager
.coffee()
.add_remote("lightningd", "https://github.com/lightningd/plugins.git")
.await;
assert!(result.is_err(), "{:?}", result);
// Install summary plugin
// This should install summary plugin for testnet network
manager
Expand Down

0 comments on commit a104871

Please sign in to comment.