Refactor HTTP api to better adhere to REST guidelines

Cargo.toml

@@ -192,6 +192,7 @@ nohash-hasher = "0.2"
 once_cell = "1.16"
 parking_lot = { version = "0.12.1", features = ["send_guard", "arc_lock"] }
 paste = "1.0"
+percent-encoding = "2.3"
 petgraph = { version = "0.6.5", default-features = false }
 pin-project-lite = "0.2.9"
 postgres-types = "0.2.5"
@@ -249,6 +250,8 @@ tokio-util = { version = "0.7.4", features = ["time"] }
 toml = "0.8"
 toml_edit = "0.22.22"
 tower-http = { version = "0.5", features = ["cors"] }
+tower-layer = "0.3"
+tower-service = "0.3"
 tracing = "0.1.37"
 tracing-appender = "0.2.2"
 tracing-core = "0.1.31"

crates/cli/Cargo.toml

@@ -47,6 +47,7 @@ itertools.workspace = true
 indicatif.workspace = true
 jsonwebtoken.workspace = true
 mimalloc.workspace = true
+percent-encoding.workspace = true
 regex.workspace = true
 reqwest.workspace = true
 rustyline.workspace = true

crates/cli/src/api.rs

@@ -7,7 +7,7 @@ use spacetimedb_lib::de::serde::DeserializeWrapper;
 use spacetimedb_lib::sats::ProductType;
 use spacetimedb_lib::Identity;
 
-use crate::util::AuthHeader;
+use crate::util::{AuthHeader, ResponseExt};
 
 static APP_USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"),);
 
@@ -23,10 +23,10 @@ impl Connection {
     pub fn db_uri(&self, endpoint: &str) -> String {
         [
             &self.host,
-            "/database/",
-            endpoint,
-            "/",
+            "/v1/database/",
             &self.database_identity.to_hex(),
+            "/",
+            endpoint,
         ]
         .concat()
     }
@@ -66,9 +66,8 @@ impl ClientApi {
             .get(self.con.db_uri("schema"))
             .query(&[("version", "9")])
             .send()
-            .await?
-            .error_for_status()?;
-        let DeserializeWrapper(module_def) = res.json().await?;
+            .await?;
+        let DeserializeWrapper(module_def) = res.json_or_error().await?;
         Ok(module_def)
     }
 

crates/cli/src/errors.rs

@@ -1,22 +1,7 @@
-use reqwest::{Response, StatusCode};
 use thiserror::Error;
 
-#[derive(Debug)]
-pub enum RequestSource {
-    Client,
-    Server,
-}
-
 #[derive(Error, Debug)]
 pub enum CliError {
-    #[error("HTTP status {kind:?} error ({status}): {msg}")]
-    Request {
-        msg: String,
-        kind: RequestSource,
-        status: StatusCode,
-    },
-    #[error(transparent)]
-    ReqWest(#[from] reqwest::Error),
     #[error("Config error: The option `{key}` not found")]
     Config { key: String },
     #[error("Config error: The option `{key}` is not a `{kind}`, found: `{type}: {value}`",
@@ -29,19 +14,3 @@ pub enum CliError {
         found: Box<toml_edit::Item>,
     },
 }
-
-/// Turn a response into an error if the server returned an error.
-pub async fn error_for_status(response: Response) -> Result<Response, CliError> {
-    let status = response.status();
-    if let Some(kind) = status
-        .is_client_error()
-        .then_some(RequestSource::Client)
-        // Anything that is not a success is an error for the client, even a redirect that is not followed.
-        .or_else(|| (!status.is_success()).then_some(RequestSource::Server))
-    {
-        let msg = response.text().await?;
-        return Err(CliError::Request { kind, msg, status });
-    }
-
-    Ok(response)
-}

crates/cli/src/subcommands/delete.rs

@@ -23,7 +23,7 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
 
     let identity = database_identity(&config, database, server).await?;
 
-    let builder = reqwest::Client::new().post(format!("{}/database/delete/{}", config.get_host_url(server)?, identity));
+    let builder = reqwest::Client::new().delete(format!("{}/v1/database/{}", config.get_host_url(server)?, identity));
     let auth_header = get_auth_header(&mut config, false, server, !force).await?;
     let builder = add_auth_header_opt(builder, &auth_header);
     builder.send().await?.error_for_status()?;

crates/cli/src/subcommands/dns.rs

@@ -1,13 +1,10 @@
 use crate::common_args;
 use crate::config::Config;
-use crate::util::{
-    add_auth_header_opt, decode_identity, get_auth_header, get_login_token_or_log_in, spacetime_register_tld,
-};
+use crate::util::{add_auth_header_opt, decode_identity, get_auth_header, get_login_token_or_log_in, ResponseExt};
 use clap::ArgMatches;
 use clap::{Arg, Command};
-use reqwest::Url;
 
-use spacetimedb_client_api_messages::name::{InsertDomainResult, RegisterTldResult};
+use spacetimedb_client_api_messages::name::{DomainName, InsertDomainResult};
 
 pub fn cli() -> Command {
     Command::new("rename")
@@ -37,32 +34,17 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
     let identity = decode_identity(&token)?;
     let auth_header = get_auth_header(&mut config, false, server, !force).await?;
 
-    match spacetime_register_tld(&mut config, domain, server, !force).await? {
-        RegisterTldResult::Success { domain } => {
-            println!("Registered domain: {}", domain);
-        }
-        RegisterTldResult::Unauthorized { domain } => {
-            return Err(anyhow::anyhow!("Domain is already registered by another: {}", domain));
-        }
-        RegisterTldResult::AlreadyRegistered { domain } => {
-            println!("Domain is already registered by the identity you provided: {}", domain);
-        }
-    }
+    let domain: DomainName = domain.parse()?;
 
-    let builder = reqwest::Client::new().get(Url::parse_with_params(
-        format!("{}/database/set_name", config.get_host_url(server)?).as_str(),
-        [
-            ("domain", domain.clone()),
-            ("database_identity", database_identity.clone()),
-            ("register_tld", "true".to_string()),
-        ],
-    )?);
+    let builder = reqwest::Client::new()
+        .post(format!(
+            "{}/v1/database/{database_identity}/names",
+            config.get_host_url(server)?
+        ))
+        .body(String::from(domain));
     let builder = add_auth_header_opt(builder, &auth_header);
 
-    let res = builder.send().await?.error_for_status()?;
-    let bytes = res.bytes().await.unwrap();
-    println!("{}", String::from_utf8_lossy(&bytes[..]));
-    let result: InsertDomainResult = serde_json::from_slice(&bytes[..]).unwrap();
+    let result = builder.send().await?.json_or_error().await?;
     match result {
         InsertDomainResult::Success {
             domain,

crates/cli/src/subcommands/energy.rs

@@ -60,7 +60,7 @@ async fn exec_status(mut config: Config, args: &ArgMatches) -> Result<(), anyhow
     };
 
     let status = reqwest::Client::new()
-        .get(format!("{}/energy/{}", config.get_host_url(server)?, identity))
+        .get(format!("{}/v1/energy/{}", config.get_host_url(server)?, identity))
         .send()
         .await?
         .error_for_status()?

crates/cli/src/subcommands/list.rs

@@ -1,10 +1,11 @@
 use crate::common_args;
 use crate::util;
 use crate::util::get_login_token_or_log_in;
+use crate::util::ResponseExt;
 use crate::util::UNSTABLE_WARNING;
 use crate::Config;
+use anyhow::Context;
 use clap::{ArgMatches, Command};
-use reqwest::StatusCode;
 use serde::Deserialize;
 use spacetimedb::Identity;
 use tabled::{
@@ -44,22 +45,18 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
     let client = reqwest::Client::new();
     let res = client
         .get(format!(
-            "{}/identity/{}/databases",
+            "{}/v1/identity/{}/databases",
             config.get_host_url(server)?,
             identity
         ))
         .bearer_auth(token)
         .send()
         .await?;
 
-    if res.status() != StatusCode::OK {
-        return Err(anyhow::anyhow!(format!(
-            "Unable to retrieve databases for identity: {}",
-            res.status()
-        )));
-    }
-
-    let result: DatabasesResult = res.json().await?;
+    let result: DatabasesResult = res
+        .json_or_error()
+        .await
+        .context("unable to retrieve databases for identity")?;
 
     if !result.identities.is_empty() {
         let mut table = Table::new(result.identities);

crates/cli/src/subcommands/login.rs

@@ -273,6 +273,12 @@ struct LocalLoginResponse {
 
 async fn spacetimedb_direct_login(host: &Url) -> Result<String, anyhow::Error> {
     let client = reqwest::Client::new();
-    let response: LocalLoginResponse = client.post(host.join("identity")?).send().await?.json().await?;
+    let response: LocalLoginResponse = client
+        .post(host.join("/v1/identity")?)
+        .send()
+        .await?
+        .error_for_status()?
+        .json()
+        .await?;
     Ok(response.token)
 }

crates/cli/src/subcommands/logs.rs

@@ -130,7 +130,7 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
 
     let host_url = config.get_host_url(server)?;
 
-    let builder = reqwest::Client::new().get(format!("{}/database/logs/{}", host_url, database_identity));
+    let builder = reqwest::Client::new().get(format!("{}/v1/database/{}/logs", host_url, database_identity));
     let builder = add_auth_header_opt(builder, &auth_header);
     let mut res = builder.query(&query_parms).send().await?;
     let status = res.status();

crates/cli/src/subcommands/publish.rs

@@ -1,4 +1,3 @@
-use anyhow::bail;
 use clap::Arg;
 use clap::ArgAction::{Set, SetTrue};
 use clap::ArgMatches;
@@ -9,7 +8,7 @@ use std::fs;
 use std::path::PathBuf;
 
 use crate::config::Config;
-use crate::util::{add_auth_header_opt, get_auth_header};
+use crate::util::{add_auth_header_opt, get_auth_header, ResponseExt};
 use crate::util::{decode_identity, unauth_error_context, y_or_n};
 use crate::{build, common_args};
 
@@ -82,18 +81,19 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
     //  easily create a new identity with an email
     let auth_header = get_auth_header(&mut config, anon_identity, server, !force).await?;
 
-    let mut query_params = Vec::<(&str, &str)>::new();
-    query_params.push(("host_type", "wasm"));
-    query_params.push(("register_tld", "true"));
+    let client = reqwest::Client::new();
 
     // If a domain or identity was provided, we should locally make sure it looks correct and
-    // append it as a query parameter
-    if let Some(name_or_identity) = name_or_identity {
+    let mut builder = if let Some(name_or_identity) = name_or_identity {
         if !is_identity(name_or_identity) {
             parse_domain_name(name_or_identity)?;
         }
-        query_params.push(("name_or_identity", name_or_identity.as_str()));
-    }
+        let encode_set = const { &percent_encoding::NON_ALPHANUMERIC.remove(b'_').remove(b'-') };
+        let domain = percent_encoding::percent_encode(name_or_identity.as_bytes(), encode_set);
+        client.put(format!("{database_host}/v1/database/{domain}"))
+    } else {
+        client.post(format!("{database_host}/v1/database"))
+    };
 
     if !path_to_project.exists() {
         return Err(anyhow::anyhow!(
@@ -145,16 +145,11 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
             println!("Aborting");
             return Ok(());
         }
-        query_params.push(("clear", "true"));
+        builder = builder.query(&[("clear", true)]);
     }
 
     println!("Publishing module...");
 
-    let mut builder = reqwest::Client::new().post(Url::parse_with_params(
-        format!("{}/database/publish", database_host).as_str(),
-        query_params,
-    )?);
-
     builder = add_auth_header_opt(builder, &auth_header);
 
     let res = builder.body(program_bytes).send().await?;
@@ -169,13 +164,8 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
             config.server_nick_or_host(server)?,
         );
     }
-    if res.status().is_client_error() || res.status().is_server_error() {
-        let err = res.text().await?;
-        bail!(err)
-    }
-    let bytes = res.bytes().await.unwrap();
 
-    let response: PublishResult = serde_json::from_slice(&bytes[..]).unwrap();
+    let response: PublishResult = res.json_or_error().await?;
     match response {
         PublishResult::Success {
             domain,
@@ -192,45 +182,21 @@ pub async fn exec(mut config: Config, args: &ArgMatches) -> Result<(), anyhow::E
                 println!("{} database with identity: {}", op, database_identity);
             }
         }
-        PublishResult::TldNotRegistered { domain } => {
-            return Err(anyhow::anyhow!(
-                "The top level domain that you provided is not registered.\n\
-            This tld is not yet registered to any identity: {}",
-                domain.tld()
-            ));
-        }
-        PublishResult::PermissionDenied { domain } => {
+        PublishResult::PermissionDenied { name } => {
             if anon_identity {
-                anyhow::bail!(
-                    "You need to be logged in as the owner of {} to publish to {}",
-                    domain.tld(),
-                    domain.tld()
-                );
+                anyhow::bail!("You need to be logged in as the owner of {name} to publish to {name}",);
             }
             // If we're not in the `anon_identity` case, then we have already forced the user to log in above (using `get_auth_header`), so this should be safe to unwrap.
             let token = config.spacetimedb_token().unwrap();
             let identity = decode_identity(token)?;
             //TODO(jdetter): Have a nice name generator here, instead of using some abstract characters
             // we should perhaps generate fun names like 'green-fire-dragon' instead
             let suggested_tld: String = identity.chars().take(12).collect();
-            if let Some(sub_domain) = domain.sub_domain() {
-                return Err(anyhow::anyhow!(
-                    "The top level domain {} is not registered to the identity you provided.\n\
-                We suggest you publish to a domain that starts with a TLD owned by you, or publish to a new domain like:\n\
-                \tspacetime publish {}/{}\n",
-                    domain.tld(),
-                    suggested_tld,
-                    sub_domain
-                ));
-            } else {
-                return Err(anyhow::anyhow!(
-                    "The top level domain {} is not registered to the identity you provided.\n\
+            return Err(anyhow::anyhow!(
+                "The database {name} is not registered to the identity you provided.\n\
                 We suggest you push to either a domain owned by you, or a new domain like:\n\
-                \tspacetime publish {}\n",
-                    domain.tld(),
-                    suggested_tld
-                ));
-            }
+                \tspacetime publish {suggested_tld}\n",
+            ));
         }
     }