fix: deleting project canister by id will clean up canister id store. (…

…#3442)

`dfx canister delete <by id>` would leave entries (canister name -> canister id) in the canister id store.  A subsequent `dfx deploy` would then fail because it would try to install to the deleted canister rather than creating a new one.

Fixes https://dfinity.atlassian.net/browse/SDK-1143

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 # UNRELEASED
 
+=== fix: `dfx canister delete <canister id>` removes the related entry from the canister id store
+
+Previously, deleting a canister in the project by id rather than by name
+would leave the canister id in the canister id store. This would cause
+`dfx deploy` to fail.
+
 === fix: dfx extension install can no longer create a corrupt cache directory
 
 Running `dfx cache delete && dfx extension install nns` would previously

e2e/tests-dfx/delete.bash

@@ -14,6 +14,19 @@ teardown() {
   standard_teardown
 }
 
+@test "delete by canister id cleans up canister id store" {
+  dfx_start
+  dfx deploy e2e_project_backend
+  id=$(dfx canister id e2e_project_backend)
+  dfx canister stop e2e_project_backend
+  assert_command dfx canister delete "$id"
+  assert_command_fail dfx canister info e2e_project_backend
+  assert_contains "Cannot find canister id. Please issue 'dfx canister create e2e_project_backend'."
+  assert_command_fail dfx canister status "$id"
+  assert_contains "Canister $id not found"
+  assert_command dfx deploy
+}
+
 @test "delete can be used to delete a canister" {
   dfx_start
   dfx deploy e2e_project_backend

src/dfx-core/src/config/model/canister_id_store.rs

@@ -188,10 +188,14 @@ impl CanisterIdStore {
         self.remote_ids
             .as_ref()
             .and_then(|remote_ids| self.get_name_in(canister_id, remote_ids))
-            .or_else(|| self.get_name_in(canister_id, &self.ids))
+            .or_else(|| self.get_name_in_project(canister_id))
             .or_else(|| self.get_name_in_pull_ids(canister_id))
     }
 
+    pub fn get_name_in_project(&self, canister_id: &str) -> Option<&String> {
+        self.get_name_in(canister_id, &self.ids)
+    }
+
     pub fn get_name_in<'a>(
         &'a self,
         canister_id: &str,

src/dfx/src/commands/canister/delete.rs

@@ -91,10 +91,15 @@ async fn delete_canister(
 ) -> DfxResult {
     let log = env.get_logger();
     let mut canister_id_store = env.get_canister_id_store()?;
+    let (canister_id, canister_name_to_delete) = match Principal::from_text(canister) {
+        Ok(canister_id) => (
+            canister_id,
+            canister_id_store.get_name_in_project(canister).cloned(),
+        ),
+        Err(_) => (canister_id_store.get(canister)?, Some(canister.to_string())),
+    };
 
     if !env.get_network_descriptor().is_playground() {
-        let canister_id =
-            Principal::from_text(canister).or_else(|_| canister_id_store.get(canister))?;
         let mut call_sender = call_sender;
         let to_dank = withdraw_cycles_to_dank || withdraw_cycles_to_dank_principal.is_some();
 
@@ -268,7 +273,10 @@ async fn delete_canister(
 
         canister::delete_canister(env, canister_id, call_sender).await?;
     }
-    canister_id_store.remove(canister)?;
+
+    if let Some(canister_name) = canister_name_to_delete {
+        canister_id_store.remove(&canister_name)?;
+    }
 
     Ok(())
 }