From 58984e13782f0cfef1a6540bfae189f2a9257187 Mon Sep 17 00:00:00 2001 From: Arun Kulshreshtha Date: Thu, 2 Jul 2020 13:10:06 -0700 Subject: [PATCH] edenapi_types: add FromJson trait Summary: Add a `FromJson` trait to `edenapi_types` and use it instead of the parsing functions when parsing requests from JSON. Some design commentary: I've created a custom trait rather than using `TryFrom` for two reasons: - From a design standpoint, I'd like users to have to explicitly opt-in to this functionality by importing this `FromJson` trait, since this is different from the usual way of deserializing with serde, and it might cause confusion. - From an implementation standpoint, it turns out that using `TryFrom` in a trait bound causes difficulties with type inference in some situations (in particular, around the associated `Error` type), which necessitates type annotations, making the API less ergonomic to use. Why not just use `serde::Deserialize` directly? - The representation here doesn't actually directly match the structure of the underlying type. Instead, the JSON representation has been designed to be easy for humans to edit when writing tests. (For example, keys are replaced with simply arrays, and hashes are represented as hex rather than as byte arrays.) - In this case, we'd like to work with `serde_json::Value` rather than going directly between bytes and structs, since this allows for a level of dynamism that will be useful later in the stack. Reviewed By: markbt Differential Revision: D22320576 fbshipit-source-id: 64b6bed42e1ec599a0da61ae5d55feb7c90101a4 --- .../scm/lib/edenapi/tools/make_req/Cargo.toml | 1 + .../lib/edenapi/tools/make_req/src/main.rs | 28 +++++++++---------- eden/scm/lib/edenapi/types/src/json.rs | 22 +++++++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/eden/scm/lib/edenapi/tools/make_req/Cargo.toml b/eden/scm/lib/edenapi/tools/make_req/Cargo.toml index 5cd39a3755..026f7682f3 100644 --- a/eden/scm/lib/edenapi/tools/make_req/Cargo.toml +++ b/eden/scm/lib/edenapi/tools/make_req/Cargo.toml @@ -8,6 +8,7 @@ include = ["src/**/*.rs"] [dependencies] edenapi_types = { path = "../../types" } anyhow = "1.0" +serde = { version = "1.0", features = ["derive", "rc"] } serde_cbor = "0.11" serde_json = "1.0" structopt = "0.3.7" diff --git a/eden/scm/lib/edenapi/tools/make_req/src/main.rs b/eden/scm/lib/edenapi/tools/make_req/src/main.rs index 0be9aa4e97..1c43949cfb 100644 --- a/eden/scm/lib/edenapi/tools/make_req/src/main.rs +++ b/eden/scm/lib/edenapi/tools/make_req/src/main.rs @@ -14,15 +14,17 @@ #![deny(warnings)] +use std::fmt::Debug; use std::fs::File; use std::io::{prelude::*, stdin, stdout}; use std::path::PathBuf; use anyhow::Result; +use serde::Serialize; use serde_json::Value; use structopt::StructOpt; -use edenapi_types::json::{parse_data_req, parse_history_req, parse_tree_req}; +use edenapi_types::{json::FromJson, CompleteTreeRequest, DataRequest, HistoryRequest}; #[derive(Debug, StructOpt)] #[structopt(name = "make_req", about = "Make EdenAPI CBOR request payloads")] @@ -40,24 +42,22 @@ struct Args { output: Option, } -macro_rules! convert { - ($args:ident, $parse_fn:ident) => {{ - let json = read_input($args.input)?; - let req = $parse_fn(&json)?; - let bytes = serde_cbor::to_vec(&req)?; - eprintln!("Generated request: {:#?}", &req); - write_output($args.output, &bytes) - }}; -} - fn main() -> Result<()> { match Command::from_args() { - Command::Data(args) => convert!(args, parse_data_req), - Command::History(args) => convert!(args, parse_history_req), - Command::Tree(args) => convert!(args, parse_tree_req), + Command::Data(args) => make_req::(args), + Command::History(args) => make_req::(args), + Command::Tree(args) => make_req::(args), } } +fn make_req(args: Args) -> Result<()> { + let json = read_input(args.input)?; + let req = R::from_json(&json)?; + let bytes = serde_cbor::to_vec(&req)?; + eprintln!("Generated request: {:#?}", &req); + write_output(args.output, &bytes) +} + fn read_input(path: Option) -> Result { Ok(match path { Some(path) => { diff --git a/eden/scm/lib/edenapi/types/src/json.rs b/eden/scm/lib/edenapi/types/src/json.rs index 128bb2cf6f..082003bd38 100644 --- a/eden/scm/lib/edenapi/types/src/json.rs +++ b/eden/scm/lib/edenapi/types/src/json.rs @@ -192,3 +192,25 @@ fn make_key(path: &str, hash: &str) -> Result { let hgid = HgId::from_str(hash)?; Ok(Key::new(path, hgid)) } + +pub trait FromJson: Sized { + fn from_json(json: &Value) -> Result; +} + +impl FromJson for DataRequest { + fn from_json(json: &Value) -> Result { + parse_data_req(json) + } +} + +impl FromJson for HistoryRequest { + fn from_json(json: &Value) -> Result { + parse_history_req(json) + } +} + +impl FromJson for CompleteTreeRequest { + fn from_json(json: &Value) -> Result { + parse_tree_req(json) + } +}