Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multipart/form-data and file uploads #609

Closed
wants to merge 41 commits into from
Closed
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
9d8a789
un-require unstable cargo fmt for now
ahirner Oct 19, 2023
95850e3
enable reqwest's multipart
ahirner Oct 19, 2023
92a32a2
intro BodyContentType::FormData WIP
ahirner Oct 19, 2023
2ae53f7
impl files as Into<Body> WIP
ahirner Oct 22, 2023
5bc61b0
switch binary string repr to some unit for now
ahirner Oct 22, 2023
31daaed
introduce (optional) FormPart WIP
ahirner Oct 22, 2023
7ce4a0f
cut and dry
ahirner Oct 22, 2023
4298470
generate form in method body WIP
ahirner Oct 22, 2023
0f71c90
Revert "un-require unstable cargo fmt for now"
ahirner Oct 22, 2023
e89338c
EXPECTORATE=overwrite cargo test
ahirner Oct 22, 2023
d614dff
fix Part body builder impls
ahirner Oct 22, 2023
45f099a
EXPECTORATE=overwrite cargo test
ahirner Oct 22, 2023
348a30f
cleanup
ahirner Oct 22, 2023
4c2d2b9
set params from form WIP
ahirner Oct 22, 2023
2a806f5
serialize form data body as serde strings
ahirner Oct 22, 2023
c8a9f31
Merge branch 'oxidecomputer:main' into feature/form_data
ahirner Oct 26, 2023
38d34d2
Merge branch 'main' of github.com:ahirner/progenitor into feature/for…
ahirner Oct 26, 2023
692b02a
add openapi-api sample schema
ahirner Oct 26, 2023
89b16ea
prune and describe openai-openapi.yaml
ahirner Oct 26, 2023
6489729
uses_serde_json also depends on multipart/form-data bodies
ahirner Oct 26, 2023
90f353d
no need to filter body_parts for existing types
ahirner Oct 26, 2023
6474706
explicit type requirement err
ahirner Oct 26, 2023
c3ffd79
verify_apis("openai-openapi.yaml") and EXPECTORATE=overwrite cargo test
ahirner Oct 26, 2023
2521259
Merge branch 'feature/form_data' of github.com:ahirner/progenitor int…
ahirner Oct 26, 2023
aea0b0a
don't require (cli) form part WIP
ahirner Oct 27, 2023
191105f
fix quoting form data string values
ahirner Oct 28, 2023
ba30bb1
fix to_form_string builder import
ahirner Oct 28, 2023
77ff7d2
factor out uses_request_file_part
ahirner Oct 28, 2023
716193f
let cli consumers override file request part
ahirner Oct 28, 2023
35ed72e
httpmock form data to a reasonable extent
ahirner Oct 28, 2023
0ab9911
blanket form data usage flag to remove unneeded .out diffs
ahirner Oct 28, 2023
91bea0b
cargo clippy on new code
ahirner Oct 29, 2023
fa31f6e
clarify use_form_parts comment
ahirner Oct 29, 2023
2c9cbdb
move unreachable variant of method param last
ahirner Oct 29, 2023
829199c
document new OperationParameter variants
ahirner Oct 29, 2023
5206425
docstring for (still hidden) to_form_string
ahirner Oct 29, 2023
b4e916d
TypeId based FormData bodies for text fields are still required
ahirner Oct 30, 2023
106205f
nullable binary strings in form-data hardly make sense
ahirner Oct 30, 2023
2a8e24a
no need for own dereferencing of schema objects
ahirner Oct 30, 2023
9e204ed
There is no need to treat binary strings at the type id level anymore
ahirner Oct 30, 2023
f8baafa
remove multipart/form-data structs from output if they are exclusivel…
ahirner Oct 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions cargo-progenitor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ pub fn dependencies(builder: Generator, include_client: bool) -> Vec<String> {
let mut deps = vec![
format!("bytes = \"{}\"", dependency_versions.get("bytes").unwrap()),
format!("futures-core = \"{}\"", dependency_versions.get("futures-core").unwrap()),
format!("reqwest = {{ version = \"{}\", default-features=false, features = [\"json\", \"stream\"] }}", dependency_versions.get("reqwest").unwrap()),
format!("reqwest = {{ version = \"{}\", default-features=false, features = [\"json\", \"stream\", \"multipart\"] }}", dependency_versions.get("reqwest").unwrap()),
format!("serde = {{ version = \"{}\", features = [\"derive\"] }}", dependency_versions.get("serde").unwrap()),
format!("serde_urlencoded = \"{}\"", dependency_versions.get("serde_urlencoded").unwrap()),
];
Expand Down Expand Up @@ -290,7 +290,7 @@ pub fn dependencies(builder: Generator, include_client: bool) -> Vec<String> {
dependency_versions.get("rand").unwrap()
));
}
if type_space.uses_serde_json() {
if type_space.uses_serde_json() || builder.uses_form_parts() {
deps.push(format!(
"serde_json = \"{}\"",
dependency_versions.get("serde_json").unwrap()
Expand Down
2 changes: 1 addition & 1 deletion progenitor-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ description = "An OpenAPI client generator - client support"
bytes = "1.5.0"
futures-core = "0.3.28"
percent-encoding = "2.3"
reqwest = { version = "0.11.22", default-features = false, features = ["json", "stream"] }
reqwest = { version = "0.11.22", default-features = false, features = ["json", "stream", "multipart"] }
serde = "1.0"
serde_json = "1.0"
serde_urlencoded = "0.7.1"
29 changes: 29 additions & 0 deletions progenitor-client/src/progenitor_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,3 +416,32 @@ impl<E> RequestBuilderExt<E> for RequestBuilder {
})?))
}
}

/// Returns a string from any json value which is expected
/// to work as `multipart/form-data` text. In particular, a
/// [`serde_json::Value::String`] remains unquoted.
///
/// # Example
/// ```
/// # use progenitor_client::to_form_string;
/// use serde_json::json;
/// assert_eq!(to_form_string::<()>(&json!("a")).unwrap(), "a");
/// assert_eq!(to_form_string::<()>(&json!(101)).unwrap(), "101");
/// assert_eq!(
/// to_form_string::<()>(&json!({"a": 202})).unwrap(),
/// "{\"a\":202}"
/// );
/// ```
#[doc(hidden)]
ahirner marked this conversation as resolved.
Show resolved Hide resolved
pub fn to_form_string<E>(
value: &serde_json::Value,
) -> Result<String, Error<E>> {
match value.as_str() {
// Take unquoted String Value
Some(v) => Ok(v.to_string()),
// otherwise convert to quoted json
None => serde_json::to_string(value).map_err(|_| {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this make serde_json into a mandatory dependency for generated crates?

Error::InvalidRequest("failed to serialize Value".to_string())
}),
}
}
126 changes: 108 additions & 18 deletions progenitor-impl/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use typify::{Type, TypeEnumVariant, TypeSpaceImpl, TypeStructPropInfo};

use crate::{
method::{
OperationParameterKind, OperationParameterType, OperationResponseStatus,
BodyContentType, OperationParameterKind, OperationParameterType,
OperationResponseStatus,
},
to_schema::ToSchema,
util::{sanitize, Case},
Expand All @@ -21,6 +22,7 @@ struct CliOperation {
cli_fn: TokenStream,
execute_fn: TokenStream,
execute_trait: TokenStream,
uses_request_file_part: bool,
}

impl Generator {
Expand Down Expand Up @@ -100,6 +102,26 @@ impl Generator {
})
.collect::<Vec<_>>();

let request_file = methods.iter().any(|method|method.uses_request_file_part).then(|| {
quote! {
fn request_file_part(
&self,
path: &std::path::PathBuf,
) -> Result<reqwest::multipart::Part, String> {
use std::io::Read;
let mut file = std::fs::File::open(&path).map_err(|e| e.to_string())?;
let mut value = Vec::new();
file.read_to_end(&mut value).map_err(|e| e.to_string())?;
let part = reqwest::multipart::Part::bytes(value);
Ok(if let Some(file_name) = path.file_name() {
part.file_name(file_name.to_string_lossy().into_owned())
} else {
part
})
}
}
});

let crate_ident = format_ident!("{}", crate_name);

let code = quote! {
Expand Down Expand Up @@ -151,6 +173,7 @@ impl Generator {

pub trait CliOverride {
#(#trait_ops)*
#request_file
}

impl CliOverride for () {}
Expand Down Expand Up @@ -182,6 +205,7 @@ impl Generator {
let CliArg {
parser: parser_args,
consumer: consumer_args,
uses_request_file_part,
} = self.cli_method_args(method);

let about = method.summary.as_ref().map(|summary| {
Expand Down Expand Up @@ -315,6 +339,7 @@ impl Generator {
cli_fn,
execute_fn,
execute_trait,
uses_request_file_part,
}
}

Expand Down Expand Up @@ -390,23 +415,18 @@ impl Generator {
}
};

args.add_arg(arg_name, CliArg { parser, consumer })
args.add_arg(arg_name, CliArg::new(parser, consumer))
}

let maybe_body_type_id = method
.params
.iter()
.find(|param| {
matches!(&param.kind, OperationParameterKind::Body(_))
})
.and_then(|param| match &param.typ {
// TODO not sure how to deal with raw bodies, but we definitely
// need **some** input so we shouldn't just ignore it... as we
// are currently...
OperationParameterType::RawBody => None,

OperationParameterType::Type(body_type_id) => {
Some(body_type_id)
// args that destructure a structured body
let maybe_body_type_id =
method.params.iter().find_map(|param| {
match (&param.kind, &param.typ) {
(
OperationParameterKind::Body(_),
OperationParameterType::Type(body_type_id),
) => Some(body_type_id),
_ => None,
}
});

Expand All @@ -430,6 +450,56 @@ impl Generator {
}
}

// args to send binary files
let body_file_args = method
.params
.iter()
.filter_map(|param| match (&param.kind, &param.typ) {
// TODO: impl, which should be pretty
// straight forward with a custom consumer
( _, OperationParameterType::RawBody) => None,
// TODO: impl, which should be pretty
// straight forward with a custom consumer
( OperationParameterKind::Body(BodyContentType::OctetStream),
_,
) => None,
(
OperationParameterKind::Body(BodyContentType::FormData(
required,
)),
OperationParameterType::FormPart,
) => Some((param, *required)),
_ => None,
})
.map(|(param, required)| {
let help = if let Some(description) = &param.description {
quote! { .help(#description) }
} else {
quote! { .help("Path to file.") }
};
let arg_name = param.api_name.to_kebab_case();
let request_field = format_ident!("{}", param.name);
let parser = quote! {
clap::Arg::new(#arg_name)
.required(#required)
.value_parser(clap::value_parser!(std::path::PathBuf))
#help
};
let part = if required {
quote! { part }
} else {
quote ! { Some(part) }
};
let consumer = quote! {
if let Some(path) = matches.get_one::<std::path::PathBuf>(#arg_name) {
let part = self.over.request_file_part(path).unwrap();
request = request.#request_field(#part);
}
};
(param.api_name.clone(), CliArg { parser, consumer, uses_request_file_part: true })
});
args.args.extend(body_file_args);

let parser_args =
args.args.values().map(|CliArg { parser, .. }| parser);

Expand Down Expand Up @@ -497,7 +567,14 @@ impl Generator {
#body_json_consumer
};

CliArg { parser, consumer }
CliArg {
parser,
consumer,
uses_request_file_part: args
.args
.values()
.any(|arg| arg.uses_request_file_part),
}
}

fn cli_method_body_arg(
Expand Down Expand Up @@ -570,7 +647,7 @@ impl Generator {
})
}
};
args.add_arg(prop_name, CliArg { parser, consumer })
args.add_arg(prop_name, CliArg::new(parser, consumer))
} else if required {
args.body_required()
}
Expand Down Expand Up @@ -674,6 +751,19 @@ struct CliArg {

/// Code to consume the argument
consumer: TokenStream,

/// Whether request_file_part is used to consume the argument
uses_request_file_part: bool,
}

impl CliArg {
fn new(parser: TokenStream, consumer: TokenStream) -> Self {
Self {
parser,
consumer,
uses_request_file_part: false,
}
}
}

#[derive(Debug, Default, PartialEq, Eq)]
Expand Down
42 changes: 42 additions & 0 deletions progenitor-impl/src/httpmock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,21 @@ impl Generator {
},
_ => unreachable!(),
},
OperationParameterType::FormPart => match kind {
OperationParameterKind::Body(
BodyContentType::FormData(required),
) => {
// No data can be accessed from a reqwest::multiform::Part
// Hence, only form-data headers will be checked
// automatically.
if *required {
quote! { () }
} else {
quote! { Option<()> }
}
}
_ => unreachable!(),
},
};

let name_ident = format_ident!("{}", name);
Expand Down Expand Up @@ -257,6 +272,33 @@ impl Generator {
_ => unreachable!(),
}
}
// no binary data checks supported:
// https://github.com/alexliesenfeld/httpmock/issues/39#issuecomment-983140233
// you should check ascii text manually
OperationParameterType::FormPart => {
if let BodyContentType::FormData(required) =
body_content_type
{
let f =
format!("form-data; name=\"{}\"", name);
if *required {
quote! {
Self(self.0.body_contains(#f))
}
} else {
quote! {
if let Some(()) = value {
Self(self.0.body_contains(#f))
}
else {
self
}
}
}
} else {
unreachable!()
}
}
}
}
};
Expand Down
Loading