refactor: Add SafeUrlPart

Author: tomasol

activity/fly-http/impl/src/app.rs

@@ -1,12 +1,12 @@
 use crate::exports::obelisk_flyio::activity_fly_http::apps;
-use crate::{API_BASE_URL, AppSlug, OrgSlug, request_with_api_token};
+use crate::{API_BASE_URL, AppName, OrgSlug, request_with_api_token};
 use anyhow::anyhow;
 use serde::{Deserialize, Serialize};
 use wstd::http::request::JsonRequest as _;
 use wstd::http::{Client, Method, StatusCode};
 use wstd::runtime::block_on;
 
-async fn get(app_name: AppSlug) -> Result<Option<apps::App>, anyhow::Error> {
+async fn get(app_name: AppName) -> Result<Option<apps::App>, anyhow::Error> {
     let request = request_with_api_token()?
         .method(Method::GET)
         .uri(format!("{API_BASE_URL}/apps/{app_name}"))
@@ -28,7 +28,7 @@ async fn get(app_name: AppSlug) -> Result<Option<apps::App>, anyhow::Error> {
     }
 }
 
-async fn put(org_slug: String, app_name: String) -> Result<apps::App, anyhow::Error> {
+async fn put(org_slug: OrgSlug, app_name: AppName) -> Result<apps::App, anyhow::Error> {
     let client = Client::new();
 
     // Attempt to create the app
@@ -39,8 +39,8 @@ async fn put(org_slug: String, app_name: String) -> Result<apps::App, anyhow::Er
     }
 
     let request_body = CreateAppRequest {
-        app_name: &app_name,
-        org_slug: &org_slug,
+        app_name: app_name.as_ref(),
+        org_slug: org_slug.as_ref(),
     };
 
     let post_request = request_with_api_token()?
@@ -57,7 +57,7 @@ async fn put(org_slug: String, app_name: String) -> Result<apps::App, anyhow::Er
         }
         let app_response: AppResponse = response.body_mut().json().await?;
         return Ok(apps::App {
-            name: app_name,
+            name: app_name.to_string(),
             id: app_response.id,
         });
     }
@@ -91,7 +91,7 @@ async fn put(org_slug: String, app_name: String) -> Result<apps::App, anyhow::Er
             let app_details: AppDetails = get_response.body_mut().json().await?;
 
             // Verify the organization slug matches
-            if app_details.organization.slug == org_slug {
+            if app_details.organization.slug == org_slug.as_ref() {
                 // Idempotency success: App exists and is in the correct org.
                 return Ok(apps::App {
                     id: app_details.id,
@@ -116,7 +116,7 @@ async fn put(org_slug: String, app_name: String) -> Result<apps::App, anyhow::Er
     ))
 }
 
-async fn list(org_slug: String) -> Result<Vec<apps::App>, anyhow::Error> {
+async fn list(org_slug: OrgSlug) -> Result<Vec<apps::App>, anyhow::Error> {
     let request = request_with_api_token()?
         .method(Method::GET)
         .uri(format!("{API_BASE_URL}/apps?org_slug={org_slug}"))
@@ -140,7 +140,7 @@ async fn list(org_slug: String) -> Result<Vec<apps::App>, anyhow::Error> {
     }
 }
 
-async fn delete(app_name: String, force: bool) -> Result<(), anyhow::Error> {
+async fn delete(app_name: AppName, force: bool) -> Result<(), anyhow::Error> {
     let mut url = format!("{API_BASE_URL}/apps/{app_name}");
     if force {
         url.push_str("?force=true");
@@ -166,21 +166,34 @@ async fn delete(app_name: String, force: bool) -> Result<(), anyhow::Error> {
 impl apps::Guest for crate::Component {
     fn get(app_name: String) -> Result<Option<apps::App>, String> {
         (|| {
-            let app_slug = AppSlug::new(app_name)?;
-            block_on(get(app_slug))
+            let app_name = AppName::new(app_name)?;
+            block_on(get(app_name))
         })()
         .map_err(|err| err.to_string())
     }
 
     fn put(org_slug: String, app_name: String) -> Result<apps::App, String> {
-        block_on(put(org_slug, app_name)).map_err(|err| err.to_string())
+        (|| {
+            let org_slug = OrgSlug::new(org_slug)?;
+            let app_name = AppName::new(app_name)?;
+            block_on(put(org_slug, app_name))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn list(org_slug: String) -> Result<Vec<apps::App>, String> {
-        block_on(list(org_slug)).map_err(|err| err.to_string())
+        (|| {
+            let org_slug = OrgSlug::new(org_slug)?;
+            block_on(list(org_slug))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn delete(app_name: String, force: bool) -> Result<(), String> {
-        block_on(delete(app_name, force)).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            block_on(delete(app_name, force))
+        })()
+        .map_err(|err| err.to_string())
     }
 }

activity/fly-http/impl/src/lib.rs

@@ -24,16 +24,14 @@ fn request_with_api_token() -> Result<request::Builder, anyhow::Error> {
 
 #[derive(derive_more::Display)]
 #[display("{value}")]
-struct SlugOf<T> {
+struct SafeUrlPart<T> {
     value: String,
     _phantom_data: PhantomData<T>,
 }
-impl<T> SlugOf<T> {
-    fn new(s: String) -> Result<SlugOf<T>, anyhow::Error> {
-        if s.chars()
-            .all(|c| c.is_ascii_lowercase() || c.is_numeric() || c == '-')
-        {
-            Ok(SlugOf {
+impl<T> SafeUrlPart<T> {
+    fn new(s: String) -> Result<SafeUrlPart<T>, anyhow::Error> {
+        if s.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') {
+            Ok(SafeUrlPart {
                 value: s,
                 _phantom_data: PhantomData::default(),
             })
@@ -42,8 +40,19 @@ impl<T> SlugOf<T> {
         }
     }
 }
+impl<T> AsRef<str> for SafeUrlPart<T> {
+    fn as_ref(&self) -> &str {
+        &self.value
+    }
+}
 
 struct AppMarker;
-type AppSlug = SlugOf<AppMarker>;
+type AppName = SafeUrlPart<AppMarker>;
 struct OrgMarker;
-type OrgSlug = SlugOf<OrgMarker>;
+type OrgSlug = SafeUrlPart<OrgMarker>;
+struct SecretKeyMarker;
+type SecretKey = SafeUrlPart<SecretKeyMarker>;
+struct VolumeIdMarker;
+type VolumeId = SafeUrlPart<VolumeIdMarker>;
+struct MachineIdMarker;
+type MachineId = SafeUrlPart<MachineIdMarker>;

activity/fly-http/impl/src/machine.rs

@@ -5,7 +5,7 @@ use crate::obelisk_flyio::activity_fly_http::regions::Region;
 
 use crate::machine::ser::MachineSer;
 use crate::serde::KebabWrapper;
-use crate::{API_BASE_URL, Component, request_with_api_token};
+use crate::{API_BASE_URL, AppName, Component, MachineId, request_with_api_token};
 use anyhow::{Context, anyhow, bail, ensure};
 use ser::{
     ExecResponseSer, MachineConfigSer, MachineCreateRequestSer, MachineCreateResponseSer,
@@ -313,7 +313,7 @@ pub(crate) mod ser {
     }
 }
 
-async fn list(app_name: String) -> Result<Vec<Machine>, anyhow::Error> {
+async fn list(app_name: AppName) -> Result<Vec<Machine>, anyhow::Error> {
     let url = format!("{API_BASE_URL}/apps/{app_name}/machines");
     let request = request_with_api_token()?
         .method(Method::GET)
@@ -337,7 +337,7 @@ async fn list(app_name: String) -> Result<Vec<Machine>, anyhow::Error> {
     }
 }
 
-async fn get(app_name: String, machine_id: String) -> Result<Option<Machine>, anyhow::Error> {
+async fn get(app_name: AppName, machine_id: MachineId) -> Result<Option<Machine>, anyhow::Error> {
     let url = format!("{API_BASE_URL}/apps/{app_name}/machines/{machine_id}");
     let request = request_with_api_token()?
         .method(Method::GET)
@@ -364,7 +364,7 @@ async fn get(app_name: String, machine_id: String) -> Result<Option<Machine>, an
 }
 
 async fn create(
-    app_name: String,
+    app_name: AppName,
     machine_name: String,
     machine_config: MachineConfig,
     region: Option<Region>,
@@ -420,8 +420,8 @@ async fn create(
 }
 
 async fn update(
-    app_name: String,
-    machine_id: String,
+    app_name: AppName,
+    machine_id: MachineId,
     machine_config: MachineConfig,
     region: Option<Region>,
 ) -> Result<(), anyhow::Error> {
@@ -449,7 +449,7 @@ async fn update(
                     )
                 })?;
             ensure!(
-                resp.id == machine_id,
+                resp.id == machine_id.as_ref(),
                 "unexpected id returned, expected {machine_id} got {id}",
                 id = resp.id
             );
@@ -462,8 +462,8 @@ async fn update(
 }
 
 async fn exec(
-    app_name: String,
-    machine_id: String,
+    app_name: AppName,
+    machine_id: MachineId,
     command: Vec<String>,
 ) -> Result<ExecResponse, anyhow::Error> {
     let url = format!("{API_BASE_URL}/apps/{app_name}/machines/{machine_id}/exec");
@@ -493,14 +493,23 @@ async fn exec(
 }
 
 async fn change_machine(
-    app_name: String,
-    machine_id: String,
+    app_name: AppName,
+    machine_id: MachineId,
     url_suffix: &'static str,
 ) -> Result<(), anyhow::Error> {
     let url = format!("{API_BASE_URL}/apps/{app_name}/machines/{machine_id}/{url_suffix}");
     send_request(url, Method::POST).await
 }
 
+async fn delete(
+    app_name: AppName,
+    machine_id: MachineId,
+    force: bool,
+) -> Result<(), anyhow::Error> {
+    let url = format!("{API_BASE_URL}/apps/{app_name}/machines/{machine_id}?force={force}");
+    send_request(url, Method::DELETE).await
+}
+
 async fn send_request(url: String, method: Method) -> Result<(), anyhow::Error> {
     let request = request_with_api_token()?
         .method(method)
@@ -514,7 +523,6 @@ async fn send_request(url: String, method: Method) -> Result<(), anyhow::Error>
     } else {
         let error_status = response.status();
         let error_body = response.into_body().bytes().await?;
-        eprintln!("Got error status {error_status}");
         Err(anyhow!(
             "failed with status {error_status}: {}",
             String::from_utf8_lossy(&error_body)
@@ -525,11 +533,20 @@ async fn send_request(url: String, method: Method) -> Result<(), anyhow::Error>
 // Implementation of the vm interface for the component.
 impl Guest for Component {
     fn list(app_name: String) -> Result<Vec<Machine>, String> {
-        block_on(list(app_name)).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            block_on(list(app_name))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn get(app_name: String, machine_id: String) -> Result<Option<Machine>, String> {
-        block_on(get(app_name, machine_id)).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(get(app_name, machine_id))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn create(
@@ -538,8 +555,11 @@ impl Guest for Component {
         machine_config: MachineConfig,
         region: Option<Region>,
     ) -> Result<String, String> {
-        block_on(create(app_name, machine_name, machine_config, region))
-            .map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            block_on(create(app_name, machine_name, machine_config, region))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn update(
@@ -548,37 +568,70 @@ impl Guest for Component {
         machine_config: MachineConfig,
         region: Option<Region>,
     ) -> Result<(), String> {
-        block_on(update(app_name, machine_id, machine_config, region))
-            .map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(update(app_name, machine_id, machine_config, region))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn stop(app_name: String, machine_id: String) -> Result<(), String> {
-        block_on(change_machine(app_name, machine_id, "stop")).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(change_machine(app_name, machine_id, "stop"))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn suspend(app_name: String, machine_id: String) -> Result<(), String> {
-        block_on(change_machine(app_name, machine_id, "suspend")).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(change_machine(app_name, machine_id, "suspend"))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn start(app_name: String, machine_id: String) -> Result<(), String> {
-        block_on(change_machine(app_name, machine_id, "start")).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(change_machine(app_name, machine_id, "start"))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn restart(app_name: String, machine_id: String) -> Result<(), String> {
-        block_on(change_machine(app_name, machine_id, "restart")).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(change_machine(app_name, machine_id, "restart"))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn delete(app_name: String, machine_id: String, force: bool) -> Result<(), String> {
-        let url = format!("{API_BASE_URL}/apps/{app_name}/machines/{machine_id}?force={force}");
-        block_on(send_request(url, Method::DELETE)).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(delete(app_name, machine_id, force))
+        })()
+        .map_err(|err| err.to_string())
     }
 
     fn exec(
         app_name: String,
         machine_id: String,
         command: Vec<String>,
     ) -> Result<ExecResponse, String> {
-        block_on(exec(app_name, machine_id, command)).map_err(|err| err.to_string())
+        (|| {
+            let app_name = AppName::new(app_name)?;
+            let machine_id = MachineId::new(machine_id)?;
+            block_on(exec(app_name, machine_id, command))
+        })()
+        .map_err(|err| err.to_string())
     }
 }