Merge branch 'elias/pocket-ic-v2-delete-instance-on-drop' into 'master'

chore(VER-2515): PocketIC V2: Delete instances on drop

* delete instances on drop
* use `directory_route`s
* finish `list_instances` 

See merge request dfinity-lab/public/ic!15051

packages/pocket-ic/src/pocket_ic_v2.rs

@@ -50,6 +50,17 @@ impl PocketIcV2 {
         }
     }
 
+    pub fn list_instances() -> Vec<String> {
+        let url = crate::start_or_reuse_server().join("v2/instances").unwrap();
+        let instances: Vec<String> = reqwest::blocking::Client::new()
+            .get(url)
+            .send()
+            .expect("Failed to get result")
+            .json()
+            .expect("Failed to get json");
+        instances
+    }
+
     pub fn get_time(&self) -> SystemTime {
         let endpoint = "read/get_time";
         let result: RawTime = self.get(endpoint);
@@ -340,6 +351,15 @@ impl Default for PocketIcV2 {
     }
 }
 
+impl Drop for PocketIcV2 {
+    fn drop(&mut self) {
+        self.reqwest_client
+            .delete(self.instance_url())
+            .send()
+            .expect("Failed to send delete request");
+    }
+}
+
 /// Call a canister candid method, authenticated.
 /// The state machine executes update calls synchronously, so there is no need to poll for the result.
 pub fn call_candid_as<Input, Output>(

packages/pocket-ic/tests/tests_v2.rs

@@ -1,5 +1,4 @@
 use candid::{encode_one, Principal};
-use ic_cdk::api::management_canister::main::CreateCanisterArgument;
 use pocket_ic::{PocketIcV2, WasmResult};
 use std::time::SystemTime;
 
@@ -32,6 +31,15 @@ fn test_get_set_cycle_balance() {
     assert_eq!(balance, 69_420);
 }
 
+#[test]
+fn test_create_and_drop_instances() {
+    let pic = PocketIcV2::new();
+    assert!(PocketIcV2::list_instances().contains(&"Available".to_string()));
+    drop(pic);
+    assert!(!PocketIcV2::list_instances().contains(&"Available".to_string()));
+    assert!(PocketIcV2::list_instances().contains(&"Deleted".to_string()));
+}
+
 #[test]
 fn test_counter_canister() {
     let pic = PocketIcV2::new();
@@ -65,15 +73,7 @@ fn call_counter_can(ic: &PocketIcV2, can_id: Principal, method: &str) -> WasmRes
 #[test]
 fn test_checkpoint() {
     let pic = PocketIcV2::new();
-    let canister_id = Principal::management_canister();
-    let user = Principal::anonymous();
-
-    let _res = pic.update_call(
-        canister_id,
-        user,
-        "provisional_create_canister_with_cycles",
-        candid::encode_args((CreateCanisterArgument { settings: None },)).unwrap(),
-    );
+    let _canister_id = pic.create_canister(None);
 
     pic.create_checkpoint();
     // todo: read from graph and assert

rs/pocket_ic_server/src/main.rs

@@ -1,12 +1,11 @@
 use axum::{
     async_trait,
-    body::HttpBody,
     extract::{DefaultBodyLimit, Path, State},
     http,
     http::{HeaderMap, StatusCode},
     middleware::{self, Next},
     response::{IntoResponse, Response},
-    routing::{delete, get, post, MethodRouter},
+    routing::{delete, get, post},
     Router, Server,
 };
 use clap::Parser;
@@ -23,7 +22,7 @@ use pocket_ic::{
     Request,
 };
 use pocket_ic_server::state_api::{
-    routes::{instances_routes, status, AppState},
+    routes::{instances_routes, status, AppState, RouterExt},
     state::PocketIcApiStateBuilder,
 };
 use pocket_ic_server::{copy_dir, InstanceId};
@@ -626,22 +625,3 @@ impl From<RawCanisterCall> for ParsedCanisterCall {
         }
     }
 }
-
-trait RouterExt<S, B>
-where
-    B: HttpBody + Send + 'static,
-    S: Clone + Send + Sync + 'static,
-{
-    fn directory_route(self, path: &str, method_router: MethodRouter<S, B>) -> Self;
-}
-
-impl<S, B> RouterExt<S, B> for Router<S, B>
-where
-    B: HttpBody + Send + 'static,
-    S: Clone + Send + Sync + 'static,
-{
-    fn directory_route(self, path: &str, method_router: MethodRouter<S, B>) -> Self {
-        self.route(path, method_router.clone())
-            .route(&format!("{path}/"), method_router)
-    }
-}

rs/pocket_ic_server/src/state_api/routes.rs

@@ -13,9 +13,10 @@ use crate::pocket_ic::{
 use crate::{
     copy_dir,
     pocket_ic::{create_state_machine, PocketIc},
-    InstanceId,
+    BindOperation, BlobStore, InstanceId, Operation,
 };
-use crate::{BindOperation, BlobStore, Operation};
+use axum::body::HttpBody;
+use axum::routing::MethodRouter;
 use axum::{
     extract::{self, Path, State},
     http::StatusCode,
@@ -59,10 +60,10 @@ where
 {
     Router::new()
         // .route("root_key", get(handler_root_key))
-        .route("/query", post(handler_query))
-        .route("/get_time", get(handler_get_time))
-        .route("/get_cycles", post(handler_get_cycles))
-        .route("/get_stable_memory", post(handler_get_stable_memory))
+        .directory_route("/query", post(handler_query))
+        .directory_route("/get_time", get(handler_get_time))
+        .directory_route("/get_cycles", post(handler_get_cycles))
+        .directory_route("/get_stable_memory", post(handler_get_stable_memory))
 }
 
 pub fn instance_update_routes<S>() -> Router<S>
@@ -71,14 +72,14 @@ where
     AppState: extract::FromRef<S>,
 {
     Router::new()
-        .route(
+        .directory_route(
             "/execute_ingress_message",
             post(handler_execute_ingress_message),
         )
-        .route("/set_time", post(handler_set_time))
-        .route("/add_cycles", post(handler_add_cycles))
-        .route("/set_stable_memory", post(handler_set_stable_memory))
-        .route("/create_checkpoint", post(handler_create_checkpoint))
+        .directory_route("/set_time", post(handler_set_time))
+        .directory_route("/add_cycles", post(handler_add_cycles))
+        .directory_route("/set_stable_memory", post(handler_set_stable_memory))
+        .directory_route("/create_checkpoint", post(handler_create_checkpoint))
 }
 
 pub fn instances_routes<S>() -> Router<S>
@@ -89,15 +90,15 @@ where
     Router::new()
         //
         // List all IC instances.
-        .route("/", get(list_instances))
+        .directory_route("/", get(list_instances))
         //
         // Create a new IC instance. Returns an InstanceId.
         // If the body contains an existing checkpoint name, the instance is restored from that,
         // otherwise a new instance is created.
-        .route("/", post(create_instance))
+        .directory_route("/", post(create_instance))
         //
         // Deletes an instance.
-        .route("/:id", delete(delete_instance))
+        .directory_route("/:id", delete(delete_instance))
         //
         // All the read-only endpoints
         .nest("/:id/read", instance_read_routes())
@@ -108,7 +109,7 @@ where
         // Save this instance to a checkpoint with the given name.
         // Takes a name:String in the request body.
         // TODO: Add a function that separates those two.
-        .route(
+        .directory_route(
             "/:id/tick_and_create_checkpoint",
             post(tick_and_create_checkpoint),
         )
@@ -511,7 +512,7 @@ pub async fn list_instances(
         runtime: _,
         blob_store: _,
     }): State<AppState>,
-) -> String {
+) -> Json<Vec<String>> {
     let instances = api_state.list_instances().await;
     let instances: Vec<String> = instances
         .iter()
@@ -523,7 +524,7 @@ pub async fn list_instances(
             InstanceState::Deleted => "Deleted".to_string(),
         })
         .collect();
-    serde_json::to_string(&instances).unwrap()
+    Json(instances)
 }
 
 pub async fn list_checkpoints(
@@ -536,14 +537,14 @@ pub async fn list_checkpoints(
         runtime: _,
         blob_store: _,
     }): State<AppState>,
-) -> String {
+) -> Json<Vec<String>> {
     let checkpoints = checkpoints
         .read()
         .await
         .keys()
         .cloned()
         .collect::<Vec<String>>();
-    serde_json::to_string(&checkpoints).unwrap()
+    Json(checkpoints)
 }
 
 // TODO: Add a function that separates those two.
@@ -604,8 +605,25 @@ pub async fn delete_instance(
     }): State<AppState>,
     Path(id): Path<InstanceId>,
 ) -> StatusCode {
-    match api_state.delete_instance(id).await {
-        Ok(_) => StatusCode::OK,
-        Err(_) => StatusCode::NOT_FOUND,
+    api_state.delete_instance(id).await;
+    StatusCode::OK
+}
+
+pub trait RouterExt<S, B>
+where
+    B: HttpBody + Send + 'static,
+    S: Clone + Send + Sync + 'static,
+{
+    fn directory_route(self, path: &str, method_router: MethodRouter<S, B>) -> Self;
+}
+
+impl<S, B> RouterExt<S, B> for Router<S, B>
+where
+    B: HttpBody + Send + 'static,
+    S: Clone + Send + Sync + 'static,
+{
+    fn directory_route(self, path: &str, method_router: MethodRouter<S, B>) -> Self {
+        self.route(path, method_router.clone())
+            .route(&format!("{path}/"), method_router)
     }
 }

rs/pocket_ic_server/src/state_api/state.rs

@@ -300,8 +300,9 @@ where
         instances.len() - 1
     }
 
-    pub async fn delete_instance(&self, _instance_id: InstanceId) -> Result<(), ()> {
-        todo!();
+    pub async fn delete_instance(&self, instance_id: InstanceId) {
+        let mut instances = self.inner.instances.write().await;
+        instances[instance_id] = InstanceState::Deleted;
     }
 
     pub async fn list_instances(&self) -> Vec<InstanceState<()>> {