chore: Replace assert_env_contains_canister with a getter (#204)

Author: adamspofford-dfinity

crates/icp-cli/src/commands/build.rs

@@ -1,5 +1,5 @@
-use anyhow::anyhow;
 use clap::Args;
+use futures::future::try_join_all;
 use icp::context::{Context, EnvironmentSelection, GetEnvironmentError};
 
 use crate::{
@@ -24,6 +24,9 @@ pub(crate) enum CommandError {
     #[error(transparent)]
     GetEnvironment(#[from] GetEnvironmentError),
 
+    #[error(transparent)]
+    GetEnvCanister(#[from] icp::context::GetEnvCanisterError),
+
     #[error(transparent)]
     Project(#[from] icp::LoadError),
 
@@ -35,9 +38,6 @@ pub(crate) async fn exec(ctx: &Context, args: &BuildArgs) -> Result<(), CommandE
     // Get environment selection
     let environment_selection: EnvironmentSelection = args.environment.clone().into();
 
-    // Load the project manifest
-    let p = ctx.project.load().await?;
-
     // Load target environment
     let env = ctx.get_environment(&environment_selection).await?;
 
@@ -50,26 +50,19 @@ pub(crate) async fn exec(ctx: &Context, args: &BuildArgs) -> Result<(), CommandE
         false => args.canisters.clone(),
     };
 
-    // Validate all specified canisters exist in project and environment
-    for name in &cnames {
-        ctx.assert_env_contains_canister(name, &environment_selection)
-            .await
-            .map_err(|e| anyhow!(e))?;
-    }
-
     // Skip doing any work if no canisters are targeted
     if cnames.is_empty() {
         return Ok(());
     }
 
+    let canisters_to_build = try_join_all(
+        cnames
+            .iter()
+            .map(|name| ctx.get_canister_and_path_for_env(name, &environment_selection)),
+    )
+    .await?;
     // Build the selected canisters
     let _ = ctx.term.write_line("Building canisters:");
-    let canisters_to_build = p
-        .canisters
-        .iter()
-        .filter(|(k, _)| cnames.contains(k))
-        .map(|(_, (path, canister))| (path.clone(), canister.clone()))
-        .collect::<Vec<_>>();
 
     build_many_with_progress_bar(
         canisters_to_build,

crates/icp-cli/src/commands/deploy/mod.rs

@@ -3,7 +3,7 @@ use clap::Args;
 use futures::{StreamExt, future::try_join_all, stream::FuturesOrdered};
 use ic_agent::export::Principal;
 use icp::{
-    context::{Context, EnvironmentSelection},
+    context::{Context, EnvironmentSelection, GetEnvCanisterError},
     identity::IdentitySelection,
 };
 use std::sync::Arc;
@@ -57,6 +57,9 @@ pub(crate) enum CommandError {
     #[error("project does not contain an environment named '{name}'")]
     EnvironmentNotFound { name: String },
 
+    #[error(transparent)]
+    GetEnvCanister(#[from] GetEnvCanisterError),
+
     #[error(transparent)]
     Create(#[from] create::CommandError),
 
@@ -93,25 +96,20 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), Command
         false => args.names.clone(),
     };
 
-    for name in &cnames {
-        ctx.assert_env_contains_canister(name, &environment_selection)
-            .await
-            .map_err(|e| anyhow!(e))?;
-    }
-
     // Skip doing any work if no canisters are targeted
     if cnames.is_empty() {
         return Ok(());
     }
 
+    let canisters_to_build = try_join_all(
+        cnames
+            .iter()
+            .map(|name| ctx.get_canister_and_path_for_env(name, &environment_selection)),
+    )
+    .await?;
+
     // Build the selected canisters
     let _ = ctx.term.write_line("Building canisters:");
-    let canisters_to_build = p
-        .canisters
-        .iter()
-        .filter(|(k, _)| cnames.contains(k))
-        .map(|(_, (path, canister))| (path.clone(), canister.clone()))
-        .collect::<Vec<_>>();
 
     build_many_with_progress_bar(
         canisters_to_build,

crates/icp-cli/src/commands/sync.rs

@@ -30,6 +30,12 @@ pub(crate) enum CommandError {
     #[error(transparent)]
     Project(#[from] icp::LoadError),
 
+    #[error(transparent)]
+    GetEnvCanister(#[from] icp::context::GetEnvCanisterError),
+
+    #[error(transparent)]
+    GetEnvCanisterId(#[from] icp::context::GetCanisterIdForEnvError),
+
     #[error(transparent)]
     Unexpected(#[from] anyhow::Error),
 }
@@ -54,13 +60,6 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandEr
         false => args.canisters.clone(),
     };
 
-    // Validate all specified canisters exist in project and environment
-    for name in &cnames {
-        ctx.assert_env_contains_canister(name, &environment_selection)
-            .await
-            .map_err(|e| anyhow!(e))?;
-    }
-
     // Skip doing any work if no canisters are targeted
     if cnames.is_empty() {
         return Ok(());
@@ -73,19 +72,14 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandEr
         .map_err(|e| anyhow!(e))?;
 
     // Prepare list of canisters with their info for syncing
-    let env_canisters = &env.canisters;
-    let sync_canisters = try_join_all(cnames.iter().map(|name| {
-        let environment_selection = environment_selection.clone();
-        async move {
-            let cid = ctx
-                .get_canister_id_for_env(name, &environment_selection)
-                .await
-                .map_err(|e| anyhow!(e))?;
-            let (canister_path, info) = env_canisters
-                .get(name)
-                .ok_or_else(|| anyhow!("Canister id exists but no canister info"))?;
-            Ok::<_, anyhow::Error>((cid, canister_path.clone(), info.clone()))
-        }
+    let sync_canisters = try_join_all(cnames.iter().map(|name| async {
+        let (canister_path, info) = ctx
+            .get_canister_and_path_for_env(name, &environment_selection)
+            .await?;
+        let cid = ctx
+            .get_canister_id_for_env(name, &environment_selection)
+            .await?;
+        Ok::<_, anyhow::Error>((cid, canister_path.clone(), info.clone()))
     }))
     .await?;
 

crates/icp-cli/tests/sync_tests.rs

@@ -261,17 +261,23 @@ fn sync_with_valid_principal() {
               steps:
                 - type: script
                   command: echo syncing
+        {NETWORK_RANDOM_PORT}
+        {ENVIRONMENT_RANDOM_PORT}
     "#};
 
     write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest");
 
+    // Start network
+    let _g = ctx.start_network_in(&project_dir, "my-network");
+    ctx.ping_until_healthy(&project_dir, "my-network");
+
     // Valid principal
     let principal = "aaaaa-aa";
 
     // Try to sync with principal (should fail)
     ctx.icp()
         .current_dir(&project_dir)
-        .args(["sync", principal])
+        .args(["sync", principal, "--environment", "my-environment"])
         .assert()
         .failure()
         .stderr(contains("project does not contain a canister named"));

crates/icp/src/context/mod.rs

@@ -3,11 +3,13 @@ use std::sync::Arc;
 use url::Url;
 
 use crate::{
+    Canister,
     agent::CreateAgentError,
     canister::{build::Build, sync::Synchronize},
     directories,
     identity::IdentitySelection,
     network::{Configuration as NetworkConfiguration, access::NetworkAccess},
+    prelude::*,
     project::DEFAULT_LOCAL_ENVIRONMENT_NAME,
     store_id::IdMapping,
 };
@@ -152,26 +154,26 @@ impl Context {
         }
     }
 
-    pub async fn assert_env_contains_canister(
+    pub async fn get_canister_and_path_for_env(
         &self,
         canister_name: &str,
         environment: &EnvironmentSelection,
-    ) -> Result<(), AssertEnvContainsCanisterError> {
+    ) -> Result<(PathBuf, Canister), GetEnvCanisterError> {
         let p = self.project.load().await?;
-        if !p.contains_canister(canister_name) {
-            return Err(AssertEnvContainsCanisterError::CanisterNotFoundInProject {
+        let Some((path, canister)) = p.get_canister(canister_name) else {
+            return Err(GetEnvCanisterError::CanisterNotFoundInProject {
                 canister_name: canister_name.to_owned(),
             });
-        }
+        };
 
         let env = self.get_environment(environment).await?;
         if !env.contains_canister(canister_name) {
-            return Err(AssertEnvContainsCanisterError::CanisterNotInEnv {
+            return Err(GetEnvCanisterError::CanisterNotInEnv {
                 canister_name: canister_name.to_owned(),
                 environment_name: environment.name().to_owned(),
             });
         }
-        Ok(())
+        Ok((path.clone(), canister.clone()))
     }
 
     /// Gets the canister ID for a given canister name in a specified environment.
@@ -599,7 +601,7 @@ pub enum GetIdsByEnvironmentError {
 }
 
 #[derive(Debug, Snafu)]
-pub enum AssertEnvContainsCanisterError {
+pub enum GetEnvCanisterError {
     #[snafu(transparent)]
     ProjectLoad { source: crate::LoadError },
 

crates/icp/src/lib.rs

@@ -93,8 +93,8 @@ pub struct Project {
 }
 
 impl Project {
-    pub fn contains_canister(&self, canister_name: &str) -> bool {
-        self.canisters.contains_key(canister_name)
+    pub fn get_canister(&self, canister_name: &str) -> Option<&(PathBuf, Canister)> {
+        self.canisters.get(canister_name)
     }
 }