Remove APIClient dependency on the Settings model

sources/api/apiclient/src/apply.rs

@@ -31,7 +31,7 @@ where
     let get_request_stream = stream::iter(get_requests).buffered(4);
     let get_responses: Vec<(&String, Result<String>)> = get_request_stream.collect().await;
 
-    // Reformat the responses to (model-verified) JSON we can send to the API.
+    // Reformat the responses to JSON we can send to the API.
     let mut changes = Vec::with_capacity(get_responses.len());
     for (input_source, get_response) in get_responses {
         let response = get_response?;
@@ -44,16 +44,12 @@ where
 
     // Send the settings changes to the server in the same transaction.  (They're quick local
     // requests, so don't add the complexity of making them run concurrently.)
-    for (input_source, json) in changes {
+    for (_input_source, json) in changes {
         let uri = format!("/settings?tx={}", transaction);
         let method = "PATCH";
         let (_status, _body) = crate::raw_request(&socket_path, &uri, method, Some(json))
             .await
-            .context(error::PatchSnafu {
-                input_source,
-                uri,
-                method,
-            })?;
+            .context(error::PatchSnafu)?;
     }
 
     // Commit the transaction and apply it to the system.
@@ -109,7 +105,7 @@ where
     }
 }
 
-/// Takes a string of TOML or JSON settings data, verifies that it fits the model, and reserializes
+/// Takes a string of TOML or JSON settings data and reserializes
 /// it to JSON for sending to the API.
 fn format_change(input: &str, input_source: &str) -> Result<String> {
     // Try to parse the input as (arbitrary) TOML.  If that fails, try to parse it as JSON.
@@ -138,11 +134,6 @@ fn format_change(input: &str, input_source: &str) -> Result<String> {
     let json_inner = json_object
         .remove("settings")
         .context(error::MissingSettingsSnafu { input_source })?;
-
-    // Deserialize into the model to confirm the settings are valid.
-    let _settings = model::Settings::deserialize(&json_inner)
-        .context(error::ModelDeserializeSnafu { input_source })?;
-
     // Return JSON text we can send to the API.
     serde_json::to_string(&json_inner).context(error::JsonSerializeSnafu { input_source })
 }
@@ -198,30 +189,11 @@ mod error {
         ))]
         MissingSettings { input_source: String },
 
-        #[snafu(display(
-            "Failed to deserialize settings from '{}' into this variant's model: {}",
-            input_source,
-            source
-        ))]
-        ModelDeserialize {
-            input_source: String,
-            source: serde_json::Error,
-        },
-
         #[snafu(display("Settings from '{}' are not a TOML table / JSON object", input_source))]
         ModelType { input_source: String },
 
-        #[snafu(display(
-            "Failed to {} settings from '{}' to '{}': {}",
-            method,
-            input_source,
-            uri,
-            source
-        ))]
+        #[snafu(display("{}", source))]
         Patch {
-            input_source: String,
-            uri: String,
-            method: String,
             #[snafu(source(from(crate::Error, Box::new)))]
             source: Box<crate::Error>,
         },

sources/api/apiclient/src/lib.rs

@@ -17,7 +17,7 @@ use hyper::{body, header, Body, Client, Request};
 use hyper_unix_connector::{UnixClient, Uri};
 use rand::{distributions::Alphanumeric, thread_rng, Rng};
 use snafu::{ensure, ResultExt};
-use std::path::Path;
+use std::{fmt, fmt::Display, path::Path};
 
 pub mod apply;
 pub mod exec;
@@ -39,6 +39,9 @@ mod error {
         #[snafu(display("Failed to send request: {}", source))]
         RequestSend { source: hyper::Error },
 
+        #[snafu(display("{}", body))]
+        CliResponseStatus { body: String },
+
         #[snafu(display("Status {} when {}ing {}: {}", code.as_str(), method, uri, body))]
         ResponseStatus {
             method: String,
@@ -86,15 +89,7 @@ where
     let (status, body) = raw_request_unchecked(&socket_path, &uri, &method, data).await?;
 
     // Error if the response status is in not in the 2xx range.
-    ensure!(
-        status.is_success(),
-        error::ResponseStatusSnafu {
-            method: method.as_ref(),
-            code: status,
-            uri: uri.as_ref(),
-            body,
-        }
-    );
+    ensure!(status.is_success(), error::CliResponseStatusSnafu { body });
 
     Ok((status, body))
 }
@@ -156,3 +151,19 @@ pub(crate) fn rando() -> String {
         .map(char::from)
         .collect()
 }
+
+/// Different input types supported by the Settings API.
+#[derive(Debug)]
+pub enum SettingsInput {
+    KeyPair(String),
+    Json(String),
+}
+
+impl Display for SettingsInput {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            SettingsInput::KeyPair(value) => write!(f, "{}", value),
+            SettingsInput::Json(value) => write!(f, "{}", value),
+        }
+    }
+}

sources/api/apiclient/src/main.rs

@@ -6,14 +6,13 @@
 // library calls based on the given flags, etc.)  The library modules contain the code for talking
 // to the API, which is intended to be reusable by other crates.
 
-use apiclient::{apply, exec, get, reboot, report, set, update};
-use datastore::{serialize_scalar, Key, KeyType};
+use apiclient::{apply, exec, get, reboot, report, set, update, SettingsInput};
 use log::{info, log_enabled, trace, warn};
+use serde::{Deserialize, Serialize};
 use simplelog::{
     ColorChoice, ConfigBuilder as LogConfigBuilder, LevelFilter, TermLogger, TerminalMode,
 };
 use snafu::ResultExt;
-use std::collections::HashMap;
 use std::env;
 use std::ffi::OsString;
 use std::process;
@@ -84,10 +83,16 @@ struct RawArgs {
 #[derive(Debug)]
 struct RebootArgs {}
 
+/// Stores a vector of user-supplied key-value pairs for the 'set' subcommand.
+#[derive(Serialize, Deserialize)]
+pub struct SetKeyPairSettings {
+    request_payload: Vec<String>,
+}
+
 /// Stores user-supplied arguments for the 'set' subcommand.
 #[derive(Debug)]
 enum SetArgs {
-    Simple(HashMap<Key, String>),
+    Simple(Vec<String>),
     Json(serde_json::Value),
 }
 
@@ -449,7 +454,7 @@ fn parse_reboot_args(args: Vec<String>) -> Subcommand {
 // what formats to accept.  This code currently makes it as convenient as possible to set settings,
 // by adding/removing a "settings" prefix as necessary.
 fn parse_set_args(args: Vec<String>) -> Subcommand {
-    let mut simple = HashMap::new();
+    let mut simple = Vec::new();
     let mut json = None;
 
     let mut iter = args.into_iter();
@@ -491,22 +496,8 @@ fn parse_set_args(args: Vec<String>) -> Subcommand {
             }
 
             x if x.contains('=') => {
-                let (raw_key, value) = x.split_once('=').unwrap();
-
-                let mut key = Key::new(KeyType::Data, raw_key).unwrap_or_else(|_| {
-                    usage_msg(format!("Given key '{}' is not a valid format", raw_key))
-                });
-
-                // Add "settings" prefix if the user didn't give a known prefix, to ease usage
-                let key_prefix = &key.segments()[0];
-                if key_prefix != "settings" {
-                    let mut segments = key.segments().clone();
-                    segments.insert(0, "settings".to_string());
-                    key = Key::from_segments(KeyType::Data, &segments)
-                        .expect("Adding prefix to key resulted in invalid key?!");
-                }
-
-                simple.insert(key, value.to_string());
+                // Push each key=value pair to vector.
+                simple.push(x.to_string());
             }
 
             x => usage_msg(format!("Unknown argument '{}'", x)),
@@ -698,41 +689,6 @@ async fn check(args: &Args) -> Result<String> {
     Ok(output)
 }
 
-/// We want the key=val form of 'set' to be as simple as possible; we don't want users to have to
-/// annotate or structure their input too much just to tell us the data type, but unfortunately
-/// knowledge of the data type is required to deserialize with the current datastore ser/de code.
-///
-/// To simplify usage, we use some heuristics to determine the type of each input.  We try to parse
-/// each value as a number and boolean, and if those fail, we assume a string.  (API communication
-/// is in JSON form, limiting the set of types; the API doesn't allow arrays or null, and "objects"
-/// (maps) are represented natively through our nested tree-like settings structure.)
-///
-/// If this goes wrong -- for example the user wants a string "42" -- we'll get a deserialization
-/// error, and can print a clear error and request the user use JSON input form to handle
-/// situations with more complex types.
-///
-/// If you have an idea for how to improve deserialization so we don't have to do this, please say!
-fn massage_set_input(input_map: HashMap<Key, String>) -> Result<HashMap<Key, String>> {
-    // Deserialize the given value into the matching Rust type.  When we find a matching type, we
-    // serialize back out to the data store format, which is required to build a Settings object
-    // through the data store deserialization code.
-    let mut massaged_map = HashMap::with_capacity(input_map.len());
-    for (key, in_val) in input_map {
-        let serialized = if let Ok(b) = serde_json::from_str::<bool>(&in_val) {
-            serialize_scalar(&b).context(error::SerializeSnafu)?
-        } else if let Ok(u) = serde_json::from_str::<u64>(&in_val) {
-            serialize_scalar(&u).context(error::SerializeSnafu)?
-        } else if let Ok(f) = serde_json::from_str::<f64>(&in_val) {
-            serialize_scalar(&f).context(error::SerializeSnafu)?
-        } else {
-            // No deserialization, already a string, just serialize
-            serialize_scalar(&in_val).context(error::SerializeSnafu)?
-        };
-        massaged_map.insert(key, serialized);
-    }
-    Ok(massaged_map)
-}
-
 // =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=
 // Main dispatch
 
@@ -803,25 +759,24 @@ async fn run() -> Result<()> {
 
         Subcommand::Set(set) => {
             let settings = match set {
-                SetArgs::Simple(input_map) => {
-                    // For key=val, we need some type information to deserialize into a Settings.
-                    trace!("Original key=value input: {:#?}", input_map);
-                    let massaged_map = massage_set_input(input_map)?;
-                    trace!("Massaged key=value input: {:#?}", massaged_map);
-
-                    // The data store deserialization code understands how to turn the key names
-                    // (a.b.c) and serialized values into the nested Settings structure.
-                    datastore::deserialization::from_map(&massaged_map)
-                        .context(error::DeserializeMapSnafu)?
+                SetArgs::Simple(simple) => {
+                    trace!("User supplied Key Value settings {:#?}", simple);
+                    // Construct the Key Pair struct.
+                    let set_key_pair = SetKeyPairSettings {
+                        request_payload: simple,
+                    };
+                    let settings_string =
+                        serde_json::to_string(&set_key_pair).context(error::SerializeSnafu)?;
+                    SettingsInput::KeyPair(settings_string)
                 }
                 SetArgs::Json(json) => {
-                    // No processing to do on JSON input; the format determines the types.  serde
-                    // can turn a Value into the nested Settings structure itself.
-                    serde_json::from_value(json).context(error::DeserializeJsonSnafu)?
+                    trace!("User supplied Json settings {:#?}", json);
+                    // Convert JSON Value to a string.
+                    SettingsInput::Json(json.to_string())
                 }
             };
 
-            set::set(&args.socket_path, &settings)
+            set::set(&args.socket_path, settings)
                 .await
                 .context(error::SetSnafu)?;
         }
@@ -930,17 +885,6 @@ mod error {
         #[snafu(display("Failed to apply settings: {}", source))]
         Apply { source: apply::Error },
 
-        #[snafu(display("Unable to deserialize input JSON into model: {}", source))]
-        DeserializeJson { source: serde_json::Error },
-
-        // This is an important error, it's shown when the user uses 'apiclient set' with the
-        // key=value form and we don't have enough data to deserialize the value.  It's not the
-        // user's fault and so we want to be very clear and give an alternative.
-        #[snafu(display("Unable to match your input to the data model.  We may not have enough type information.  Please try the --json input form.  Cause: {}", source))]
-        DeserializeMap {
-            source: datastore::deserialization::Error,
-        },
-
         #[snafu(display("Failed to exec: {}", source))]
         Exec { source: exec::Error },
 

sources/api/apiclient/src/set.rs

@@ -1,32 +1,34 @@
-use crate::rando;
+use crate::{rando, SettingsInput};
 use snafu::ResultExt;
 use std::path::Path;
 
 /// Changes the requested settings through the API, then commits and applies the transaction
 /// containing those changes.  The given Settings only has to be populated (i.e. Option::Some) with
 /// the settings you want to change.  If you're deserializing a request from a user, for example,
 /// the created Settings will only have the requested keys populated.
-pub async fn set<P>(socket_path: P, settings: &model::Settings) -> Result<()>
+pub async fn set<P>(socket_path: P, settings: SettingsInput) -> Result<()>
 where
     P: AsRef<Path>,
 {
     // We use a specific transaction ID so we don't commit any other changes that may be pending.
     let transaction = format!("apiclient-set-{}", rando());
 
     // Send the settings changes to the server.
-    let uri = format!("/settings?tx={}", transaction);
+    let (uri, settings_data) = match settings {
+        SettingsInput::KeyPair(value) => (format!("/settings/keypair?tx={}", transaction), value),
+        SettingsInput::Json(value) => (format!("/settings?tx={}", transaction), value),
+    };
     let method = "PATCH";
-    let request_body = serde_json::to_string(&settings).context(error::SerializeSnafu)?;
-    let (_status, _body) = crate::raw_request(&socket_path, &uri, method, Some(request_body))
+    let (_status, _body) = crate::raw_request(&socket_path, &uri, method, Some(settings_data))
         .await
-        .context(error::RequestSnafu { uri, method })?;
+        .context(error::RequestSnafu)?;
 
     // Commit the transaction and apply it to the system.
     let uri = format!("/tx/commit_and_apply?tx={}", transaction);
     let method = "POST";
     let (_status, _body) = crate::raw_request(&socket_path, &uri, method, None)
         .await
-        .context(error::RequestSnafu { uri, method })?;
+        .context(error::RequestSnafu)?;
 
     Ok(())
 }
@@ -40,10 +42,8 @@ mod error {
         #[snafu(display("Unable to serialize data: {}", source))]
         Serialize { source: serde_json::Error },
 
-        #[snafu(display("Failed {} request to '{}': {}", method, uri, source))]
+        #[snafu(display("{}", source))]
         Request {
-            method: String,
-            uri: String,
             #[snafu(source(from(crate::Error, Box::new)))]
             source: Box<crate::Error>,
         },