Merge pull request #1332 from cloudflare/alewis/commands-refactor-kv

Alewis/commands refactor kv

src/commands/kv/bulk/delete.rs

@@ -4,18 +4,16 @@ use std::fs;
 use std::fs::metadata;
 use std::path::Path;
 
-use cloudflare::endpoints::workerskv::delete_bulk::DeleteBulk;
 use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair;
-use cloudflare::framework::apiclient::ApiClient;
 
 use crate::commands::kv;
-use crate::commands::kv::bulk::MAX_PAIRS;
+use crate::kv::bulk::delete;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::interactive;
 use crate::terminal::message;
 
-pub fn delete(
+pub fn run(
     target: &Target,
     user: &GlobalUser,
     namespace_id: &str,
@@ -50,38 +48,9 @@ pub fn delete(
 
     let keys: Vec<String> = pairs?.iter().map(|kv| kv.key.to_owned()).collect();
 
-    match delete_bulk(target, user, namespace_id, keys) {
+    match delete(target, user, namespace_id, keys) {
         Ok(_) => message::success("Success"),
         Err(e) => print!("{}", e),
     }
     Ok(())
 }
-
-pub fn delete_bulk(
-    target: &Target,
-    user: &GlobalUser,
-    namespace_id: &str,
-    keys: Vec<String>,
-) -> Result<(), failure::Error> {
-    let client = kv::api_client(user)?;
-
-    // Check number of pairs is under limit
-    if keys.len() > MAX_PAIRS {
-        failure::bail!(
-            "Number of keys to delete ({}) exceeds max of {}",
-            keys.len(),
-            MAX_PAIRS
-        );
-    }
-
-    let response = client.request(&DeleteBulk {
-        account_identifier: &target.account_id,
-        namespace_identifier: namespace_id,
-        bulk_keys: keys,
-    });
-
-    match response {
-        Ok(_) => Ok(()),
-        Err(e) => failure::bail!("{}", kv::format_error(e)),
-    }
-}

src/commands/kv/bulk/mod.rs

@@ -1,7 +1,28 @@
 pub mod delete;
 pub mod put;
 
-pub use delete::delete;
-pub use put::put;
+pub use delete::run as delete;
+pub use put::run as put;
 
-const MAX_PAIRS: usize = 10000;
+use std::time::Duration;
+
+use cloudflare::framework::auth::Credentials;
+use cloudflare::framework::{Environment, HttpApiClient, HttpApiClientConfig};
+
+use crate::http::feature::headers;
+use crate::settings::global_user::GlobalUser;
+
+// Create a special API client that has a longer timeout than usual, given that KV operations
+// can be lengthy if payloads are large.
+fn bulk_api_client(user: &GlobalUser) -> Result<HttpApiClient, failure::Error> {
+    let config = HttpApiClientConfig {
+        http_timeout: Duration::from_secs(5 * 60),
+        default_headers: headers(None),
+    };
+
+    HttpApiClient::new(
+        Credentials::from(user.to_owned()),
+        config,
+        Environment::Production,
+    )
+}

src/commands/kv/bulk/put.rs

@@ -5,23 +5,22 @@ use std::fs::metadata;
 use std::path::Path;
 
 use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair;
-use cloudflare::endpoints::workerskv::write_bulk::WriteBulk;
-use cloudflare::framework::apiclient::ApiClient;
-use cloudflare::framework::response::{ApiFailure, ApiSuccess};
 
-use crate::commands::kv;
-use crate::commands::kv::bulk::MAX_PAIRS;
+use crate::commands::kv::validate_target;
+use crate::kv::bulk::put;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::message;
 
-pub fn put(
+use super::bulk_api_client;
+
+pub fn run(
     target: &Target,
     user: &GlobalUser,
     namespace_id: &str,
     filename: &Path,
 ) -> Result<(), failure::Error> {
-    kv::validate_target(target)?;
+    validate_target(target)?;
 
     let pairs: Vec<KeyValuePair> = match &metadata(filename) {
         Ok(file_type) if file_type.is_file() => {
@@ -39,32 +38,9 @@ pub fn put(
         Err(e) => Err(failure::format_err!("{}", e)),
     }?;
 
-    // Validate that bulk upload is within size constraints
-    if pairs.len() > MAX_PAIRS {
-        failure::bail!(
-            "Number of key-value pairs to upload ({}) exceeds max of {}",
-            pairs.len(),
-            MAX_PAIRS
-        );
-    }
+    let client = bulk_api_client(user)?;
+    put(&client, target, namespace_id, &pairs)?;
+    message::success("Success");
 
-    let client = kv::api_client(user)?;
-    match call_api(&client, target, namespace_id, &pairs) {
-        Ok(_) => message::success("Success"),
-        Err(e) => failure::bail!("{}", kv::format_error(e)),
-    }
     Ok(())
 }
-
-pub fn call_api(
-    client: &impl ApiClient,
-    target: &Target,
-    namespace_id: &str,
-    pairs: &[KeyValuePair],
-) -> Result<ApiSuccess<()>, ApiFailure> {
-    client.request(&WriteBulk {
-        account_identifier: &target.account_id,
-        namespace_identifier: namespace_id,
-        bulk_key_value_pairs: pairs.to_owned(),
-    })
-}

src/commands/kv/key/delete.rs

@@ -1,7 +1,8 @@
 use cloudflare::endpoints::workerskv::delete_key::DeleteKey;
 use cloudflare::framework::apiclient::ApiClient;
 
-use crate::commands::kv;
+use crate::commands::kv::{format_error, validate_target};
+use crate::http;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::interactive;
@@ -13,8 +14,8 @@ pub fn delete(
     id: &str,
     key: &str,
 ) -> Result<(), failure::Error> {
-    kv::validate_target(target)?;
-    let client = kv::api_client(user)?;
+    validate_target(target)?;
+    let client = http::cf_v4_client(user)?;
 
     match interactive::delete(&format!("Are you sure you want to delete key \"{}\"?", key)) {
         Ok(true) => (),
@@ -36,7 +37,7 @@ pub fn delete(
 
     match response {
         Ok(_) => message::success("Success"),
-        Err(e) => print!("{}", kv::format_error(e)),
+        Err(e) => print!("{}", format_error(e)),
     }
 
     Ok(())

src/commands/kv/key/list.rs

@@ -1,7 +1,8 @@
 extern crate serde_json;
 
 use crate::commands::kv;
-use crate::commands::kv::key::KeyList;
+use crate::http;
+use crate::kv::key::KeyList;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 
@@ -15,7 +16,8 @@ pub fn list(
     prefix: Option<&str>,
 ) -> Result<(), failure::Error> {
     kv::validate_target(target)?;
-    let key_list = KeyList::new(target, user, namespace_id, prefix)?;
+    let client = http::cf_v4_client(&user)?;
+    let key_list = KeyList::new(target, client, namespace_id, prefix)?;
 
     print!("["); // Open json list bracket
 

src/commands/kv/key/mod.rs

@@ -1,11 +1,9 @@
 mod delete;
 mod get;
-mod key_list;
 mod list;
 mod put;
 
 pub use delete::delete;
 pub use get::get;
-pub use key_list::KeyList;
 pub use list::list;
 pub use put::{put, KVMetaData};

src/commands/kv/mod.rs

@@ -1,38 +1,18 @@
 use std::collections::HashSet;
-use std::time::Duration;
 
-use cloudflare::framework::auth::Credentials;
 use cloudflare::framework::response::ApiFailure;
-use cloudflare::framework::{Environment, HttpApiClient, HttpApiClientConfig};
 
 use percent_encoding::{utf8_percent_encode, CONTROLS};
 
-use crate::settings::global_user::GlobalUser;
+use crate::http;
 use crate::settings::toml::Target;
 
-use crate::http::{self, feature::headers};
-
-pub mod bucket;
 pub mod bulk;
 pub mod key;
 pub mod namespace;
 
-// Create a special API client that has a longer timeout than usual, given that KV operations
-// can be lengthy if payloads are large.
-fn api_client(user: &GlobalUser) -> Result<HttpApiClient, failure::Error> {
-    let config = HttpApiClientConfig {
-        http_timeout: Duration::from_secs(5 * 60),
-        default_headers: headers(None),
-    };
-
-    HttpApiClient::new(
-        Credentials::from(user.to_owned()),
-        config,
-        Environment::Production,
-    )
-}
-
-fn format_error(e: ApiFailure) -> String {
+// TODO: callers outside this module should write their own error handling (lookin at you sites)
+pub fn format_error(e: ApiFailure) -> String {
     http::format_error(e, Some(&kv_help))
 }
 

src/commands/kv/namespace/create.rs

@@ -1,16 +1,13 @@
-use cloudflare::endpoints::workerskv::create_namespace::CreateNamespace;
-use cloudflare::endpoints::workerskv::create_namespace::CreateNamespaceParams;
-use cloudflare::endpoints::workerskv::WorkersKvNamespace;
-use cloudflare::framework::apiclient::ApiClient;
-use cloudflare::framework::response::{ApiFailure, ApiSuccess};
+use regex::Regex;
 
 use crate::commands::kv;
+use crate::http;
+use crate::kv::namespace::create;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::message;
-use regex::Regex;
 
-pub fn create(
+pub fn run(
     target: &Target,
     env: Option<&str>,
     user: &GlobalUser,
@@ -23,8 +20,8 @@ pub fn create(
     let msg = format!("Creating namespace with title \"{}\"", title);
     message::working(&msg);
 
-    let client = kv::api_client(user)?;
-    let result = call_api(&client, target, &title);
+    let client = http::cf_v4_client(user)?;
+    let result = create(&client, target, &title);
 
     match result {
         Ok(success) => {
@@ -64,19 +61,6 @@ pub fn create(
     Ok(())
 }
 
-pub fn call_api(
-    client: &impl ApiClient,
-    target: &Target,
-    title: &str,
-) -> Result<ApiSuccess<WorkersKvNamespace>, ApiFailure> {
-    client.request(&CreateNamespace {
-        account_identifier: &target.account_id,
-        params: CreateNamespaceParams {
-            title: title.to_string(),
-        },
-    })
-}
-
 fn validate_binding(binding: &str) -> Result<(), failure::Error> {
     let re = Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").unwrap();
     if !re.is_match(binding) {

src/commands/kv/namespace/delete.rs

@@ -1,15 +1,14 @@
-use cloudflare::endpoints::workerskv::remove_namespace::RemoveNamespace;
-use cloudflare::framework::apiclient::ApiClient;
-
 use crate::commands::kv;
+use crate::http;
+use crate::kv::namespace::delete;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::interactive;
 use crate::terminal::message;
 
-pub fn delete(target: &Target, user: &GlobalUser, id: &str) -> Result<(), failure::Error> {
+pub fn run(target: &Target, user: &GlobalUser, id: &str) -> Result<(), failure::Error> {
     kv::validate_target(target)?;
-    let client = kv::api_client(user)?;
+    let client = http::cf_v4_client(user)?;
 
     match interactive::delete(&format!(
         "Are you sure you want to delete namespace {}?",
@@ -26,11 +25,7 @@ pub fn delete(target: &Target, user: &GlobalUser, id: &str) -> Result<(), failur
     let msg = format!("Deleting namespace {}", id);
     message::working(&msg);
 
-    let response = client.request(&RemoveNamespace {
-        account_identifier: &target.account_id,
-        namespace_identifier: id,
-    });
-
+    let response = delete(client, target, id);
     match response {
         Ok(_) => {
             message::success("Success");

src/commands/kv/namespace/list.rs

@@ -1,23 +1,16 @@
 extern crate serde_json;
 
-use cloudflare::endpoints::workerskv::list_namespaces::ListNamespaces;
-use cloudflare::endpoints::workerskv::list_namespaces::ListNamespacesParams;
-use cloudflare::endpoints::workerskv::WorkersKvNamespace;
-use cloudflare::framework::apiclient::ApiClient;
-use cloudflare::framework::response::{ApiFailure, ApiSuccess};
-
 use crate::commands::kv;
+use crate::http;
+use crate::kv::namespace::list;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 
-const MAX_NAMESPACES_PER_PAGE: u32 = 100;
-const PAGE_NUMBER: u32 = 1;
-
-pub fn list(target: &Target, user: &GlobalUser) -> Result<(), failure::Error> {
+pub fn run(target: &Target, user: &GlobalUser) -> Result<(), failure::Error> {
     kv::validate_target(target)?;
 
-    let client = kv::api_client(user)?;
-    let result = call_api(&client, target);
+    let client = http::cf_v4_client(user)?;
+    let result = list(&client, target);
     match result {
         Ok(success) => {
             let namespaces = success.result;
@@ -27,18 +20,3 @@ pub fn list(target: &Target, user: &GlobalUser) -> Result<(), failure::Error> {
     }
     Ok(())
 }
-
-pub fn call_api(
-    client: &impl ApiClient,
-    target: &Target,
-) -> Result<ApiSuccess<Vec<WorkersKvNamespace>>, ApiFailure> {
-    let params = ListNamespacesParams {
-        page: Some(PAGE_NUMBER),
-        per_page: Some(MAX_NAMESPACES_PER_PAGE),
-    };
-
-    client.request(&ListNamespaces {
-        account_identifier: &target.account_id,
-        params,
-    })
-}