From 925d7b0376bb0c46f524edf9e89104937a326ea6 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Tue, 18 Oct 2022 19:28:48 -0700 Subject: [PATCH 01/20] Reduce number of clones --- src/generator/templates/api/lib/mbuild.mako | 42 ++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 3bccacdcc6..5d1a5e9ddf 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -525,14 +525,13 @@ match result { use std::io::{Read, Seek}; use hyper::header::{CONTENT_TYPE, CONTENT_LENGTH, AUTHORIZATION, USER_AGENT, LOCATION}; use client::ToParts; + use std::borrow::Cow; + let mut dd = client::DefaultDelegate; - let mut dlg: &mut dyn client::Delegate = match ${delegate} { - Some(d) => d, - None => &mut dd - }; + let mut dlg: &mut dyn client::Delegate = ${delegate}.unwrap_or(&mut dd); dlg.begin(client::MethodInfo { id: "${m.id}", http_method: ${method_name_to_variant(m.httpMethod)} }); - let mut params: Vec<(&str, String)> = Vec::with_capacity(${len(params) + len(reserved_params)} + ${paddfields}.len()); + let mut params: Vec<(&str, Cow)> = Vec::with_capacity(${len(params) + len(reserved_params)} + ${paddfields}.len()); <% if media_params and 'mediaUpload' in m: upload_type_map = dict() @@ -540,7 +539,7 @@ match result { if mp.protocol == 'simple': upload_type_map[mp.protocol] = m.mediaUpload.protocols.simple.multipart and 'multipart' or 'media' break - # for each meadia param + # for each media param # end build media param map %>\ % for p in field_params: @@ -569,15 +568,15 @@ match result { % if p.get('repeated', False): if ${pname}.len() > 0 { for f in ${pname}.iter() { - params.push(("${p.name}", ${to_string_impl("f")})); + params.push(("${p.name}", ${to_string_impl("f")}.into())); } } % elif not is_required_property(p): if let Some(value) = ${pname}.as_ref() { - params.push(("${p.name}", ${to_string_impl("value")})); + params.push(("${p.name}", ${to_string_impl("value")}.into())); } % else: - params.push(("${p.name}", ${to_string_impl(pname)})); + params.push(("${p.name}", ${to_string_impl(pname)}.into())); % endif % endfor ## Additional params - may not overlap with optional params @@ -587,9 +586,9 @@ match result { return Err(client::Error::FieldClash(field)); } } - for (name, value) in ${paddfields}.iter() { - params.push((&name, value.clone())); - } + + params.extend(${paddfields}.iter().map(|(k, v)| (k.as_str(), v.into()))); + % if response_schema: % if supports_download: @@ -606,10 +605,10 @@ match result { (field_missing, enable) }; if json_field_missing { - params.push(("alt", "json".to_string())); + params.push(("alt", "json".into())); } % else: - params.push(("alt", "json".to_string())); + params.push(("alt", "json".into())); % endif ## supportsMediaDownload % endif ## response schema @@ -628,7 +627,7 @@ protocol == "${mp.protocol}" { else { unreachable!() }; - params.push(("uploadType", upload_type.to_string())); + params.push(("uploadType", upload_type.into())); % else: let mut url = self.hub._base_url.clone() + "${m.path}"; % endif @@ -637,9 +636,8 @@ else { <% assert 'key' in parameters, "Expected 'key' parameter if there are no scopes" %> - let key = dlg.api_key(); - match key { - Some(value) => params.push(("key", value)), + match dlg.api_key() { + Some(value) => params.push(("key", value.into())), None => { ${delegate_finish}(false); return Err(client::Error::MissingAPIKey) @@ -652,7 +650,7 @@ else { } % endif - ## Hanlde URI Tempates + ## Handle URI Templates % if replacements: for &(find_this, param_name) in [${', '.join('("%s", "%s")' % r for r in replacements)}].iter() { <% @@ -754,8 +752,10 @@ else { % endif let client = &self.hub.client; dlg.pre_request(); - let mut req_builder = hyper::Request::builder().method(${method_name_to_variant(m.httpMethod)}).uri(url.clone().into_string()) - .header(USER_AGENT, self.hub._user_agent.clone()); + let mut req_builder = hyper::Request::builder() + .method(${method_name_to_variant(m.httpMethod)}) + .uri(url.as_str()) + .header(USER_AGENT, self.hub._user_agent.clone()); % if default_scope: if let Some(token) = token.as_ref() { From eb072087de24753982d851fe619b71f241f0d23e Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Tue, 18 Oct 2022 20:04:09 -0700 Subject: [PATCH 02/20] Use Cow for parameter replacement --- src/generator/templates/api/lib/mbuild.mako | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 5d1a5e9ddf..74ea16a77e 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -658,21 +658,18 @@ else { replace_assign = 'Some(value)' url_replace_arg = 'replace_with.expect("to find substitution value in params")' if URL_ENCODE in special_cases: - replace_init = ' = String::new()' - replace_assign = 'value.to_string()' + replace_init = ': Cow = "".into()' + replace_assign = 'Cow::from(value.as_ref())' url_replace_arg = '&replace_with' # end handle url encoding %>\ let mut replace_with${replace_init}; - for &(name, ref value) in params.iter() { - if name == param_name { - replace_with = ${replace_assign}; - break; - } + if let Some((_, value)) = params.iter().find(|(name, _)| name == ¶m_name) { + replace_with = ${replace_assign}; } % if URL_ENCODE in special_cases: if find_this.as_bytes()[1] == '+' as u8 { - replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET).to_string(); + replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET).to_string().into(); } % endif url = url.replace(find_this, ${url_replace_arg}); From ae3e6a232ba43ca6814dde699a7ee428fcdf8142 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:00:16 -0700 Subject: [PATCH 03/20] Simplify search for alt=json param --- src/generator/templates/api/lib/mbuild.mako | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 74ea16a77e..546196310b 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -592,19 +592,14 @@ match result { % if response_schema: % if supports_download: - let (json_field_missing, enable_resource_parsing) = { - let mut enable = true; - let mut field_missing = true; - for &(name, ref value) in params.iter() { - if name == "alt" { - field_missing = false; - enable = value == "json"; - break; - } + let (alt_field_missing, enable_resource_parsing) = { + if let Some((_, value)) = params.iter().find(|(name, _)| name == &"alt") { + (false, value == "json") + } else { + (true, true) } - (field_missing, enable) }; - if json_field_missing { + if alt_field_missing { params.push(("alt", "json".into())); } % else: From cfa6958aa09b2e7ae9ad7da551abc011f38ccd79 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:13:20 -0700 Subject: [PATCH 04/20] Add UploadProtocol enum to remove string types --- google-apis-common/src/lib.rs | 6 ++++++ src/generator/templates/api/lib/mbuild.mako | 18 +++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/google-apis-common/src/lib.rs b/google-apis-common/src/lib.rs index 5380e041a2..f0ae5e9553 100644 --- a/google-apis-common/src/lib.rs +++ b/google-apis-common/src/lib.rs @@ -41,6 +41,12 @@ pub enum Retry { After(Duration), } +#[derive(PartialEq, Eq)] +pub enum UploadProtocol { + Simple, + Resumable, +} + /// Identifies the Hub. There is only one per library, this trait is supposed /// to make intended use more explicit. /// The hub allows to access all resource methods more easily. diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 546196310b..81877e0bc4 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -14,6 +14,11 @@ is_repeated_property, setter_fn_name, ADD_SCOPE_FN, ADD_SCOPES_FN, rust_doc_sanitize, CLEAR_SCOPES_FN, items, string_impl) + PROTOCOL_TYPE_MAP = { + "simple": "client::UploadProtocol::Simple", + "resumable": "client::UploadProtocol::Resumable" + } + def get_parts(part_prop): if not part_prop: return list() @@ -450,7 +455,7 @@ match result { type_params = '<%s>' % mtype_param qualifier = '' where = '\n\t\twhere ' + mtype_param + ': client::ReadSeek' - add_args = (', mut reader: %s, reader_mime_type: mime::Mime' % mtype_param) + ", protocol: &'static str" + add_args = (', mut reader: %s, reader_mime_type: mime::Mime' % mtype_param) + ", protocol: client::UploadProtocol" for p in media_params: if p.protocol == 'simple': simple_media_param = p @@ -606,7 +611,6 @@ match result { params.push(("alt", "json".into())); % endif ## supportsMediaDownload % endif ## response schema - % if media_params: let (mut url, upload_type) = % for mp in media_params: @@ -615,7 +619,7 @@ match result { % else: else if \ % endif -protocol == "${mp.protocol}" { +protocol == ${PROTOCOL_TYPE_MAP[mp.protocol]} { (self.hub._root_url.clone() + "${mp.path.lstrip('/')}", "${upload_type_map.get(mp.protocol, mp.protocol)}") } \ % endfor @@ -757,7 +761,7 @@ else { % if resumable_media_param: upload_url_from_server = true; - if protocol == "${resumable_media_param.protocol}" { + if protocol == ${PROTOCOL_TYPE_MAP[resumable_media_param.protocol]} { req_builder = req_builder.header("X-Upload-Content-Type", format!("{}", reader_mime_type)); } % endif @@ -777,7 +781,7 @@ else { % endif ## not simple_media_param % else: % if simple_media_param: - let request = if protocol == "${simple_media_param.protocol}" { + let request = if protocol == ${PROTOCOL_TYPE_MAP[simple_media_param.protocol]} { ${READER_SEEK | indent_all_but_first_by(4)} let mut bytes = Vec::with_capacity(size as usize); reader.read_to_end(&mut bytes)?; @@ -833,7 +837,7 @@ else { } } % if resumable_media_param: - if protocol == "${resumable_media_param.protocol}" { + if protocol == ${PROTOCOL_TYPE_MAP[resumable_media_param.protocol]} { ${READER_SEEK | indent_all_but_first_by(6)} let upload_result = { let url_str = &res.headers().get("Location").expect("LOCATION header is part of protocol").to_str().unwrap(); @@ -919,7 +923,7 @@ if enable_resource_parsing \ % endfor pub async fn ${upload_action_fn(api.terms.upload_action, p.type.suffix)}<${mtype_param}>(self, ${p.type.arg_name}: ${mtype_param}, mime_type: mime::Mime) -> ${rtype} where ${mtype_param}: client::ReadSeek { - self.${api.terms.action}(${p.type.arg_name}, mime_type, "${p.protocol}").await + self.${api.terms.action}(${p.type.arg_name}, mime_type, ${PROTOCOL_TYPE_MAP[p.protocol]}).await } % endfor From a6e763f49556145cfb98554008a0ba416857824f Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:17:08 -0700 Subject: [PATCH 05/20] Use top-level constants for upload protocols --- src/generator/templates/api/lib/mbuild.mako | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 81877e0bc4..69353798ba 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -14,9 +14,11 @@ is_repeated_property, setter_fn_name, ADD_SCOPE_FN, ADD_SCOPES_FN, rust_doc_sanitize, CLEAR_SCOPES_FN, items, string_impl) + SIMPLE = "simple" + RESUMABLE = "resumable" PROTOCOL_TYPE_MAP = { - "simple": "client::UploadProtocol::Simple", - "resumable": "client::UploadProtocol::Resumable" + SIMPLE: "client::UploadProtocol::Simple", + RESUMABLE: "client::UploadProtocol::Resumable" } def get_parts(part_prop): @@ -457,9 +459,9 @@ match result { where = '\n\t\twhere ' + mtype_param + ': client::ReadSeek' add_args = (', mut reader: %s, reader_mime_type: mime::Mime' % mtype_param) + ", protocol: client::UploadProtocol" for p in media_params: - if p.protocol == 'simple': + if p.protocol == SIMPLE: simple_media_param = p - elif p.protocol == 'resumable': + elif p.protocol == RESUMABLE: resumable_media_param = p # end handle media params @@ -541,7 +543,7 @@ match result { if media_params and 'mediaUpload' in m: upload_type_map = dict() for mp in media_params: - if mp.protocol == 'simple': + if mp.protocol == SIMPLE: upload_type_map[mp.protocol] = m.mediaUpload.protocols.simple.multipart and 'multipart' or 'media' break # for each media param From 9fa31bd034f63424ab8910d998d9afd8a4411cd5 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:44:48 -0700 Subject: [PATCH 06/20] Don't call .to_string() on string values --- src/generator/lib/util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/generator/lib/util.py b/src/generator/lib/util.py index d25476c56d..7e766a2e17 100644 --- a/src/generator/lib/util.py +++ b/src/generator/lib/util.py @@ -1222,8 +1222,9 @@ def string_impl(p): "byte": lambda x: f"::client::serde::urlsafe_base64::to_string(&{x})", "google-datetime": lambda x: f"::client::serde::datetime_to_string(&{x})", "date-time": lambda x: f"::client::serde::datetime_to_string(&{x})", - "google-fieldmask": lambda x: f"{x}.to_string()" - }.get(p.get("format"), lambda x: f"{x}.to_string()") + "google-fieldmask": lambda x: f"{x}.to_string()", + "string": lambda x: x + }.get(p.get("format", p["type"]), lambda x: f"{x}.to_string()") if __name__ == '__main__': From 0ad3b1258f55318cbfa9c4fc121291471cb65dca Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 17:05:26 -0700 Subject: [PATCH 07/20] Refactor Params into external struct Reduces file size of generated library: ``` cargo build --release Compiling google-compute1 v5.0.1+20220224 (/home/philippe/PycharmProjects/google-apis-rs/gen/compute1) Finished release [optimized] target(s) in 35.15s ``` 164 MB resulting lib (4MB reduction) --- google-apis-common/Cargo.toml | 1 + google-apis-common/src/lib.rs | 1 + google-apis-common/src/url.rs | 71 +++++++++++++++++++++ src/generator/templates/api/lib/mbuild.mako | 51 ++++----------- 4 files changed, 87 insertions(+), 37 deletions(-) create mode 100644 google-apis-common/src/url.rs diff --git a/google-apis-common/Cargo.toml b/google-apis-common/Cargo.toml index ff2249c544..e7bbf1838d 100644 --- a/google-apis-common/Cargo.toml +++ b/google-apis-common/Cargo.toml @@ -23,6 +23,7 @@ serde_json = "^ 1.0" base64 = "0.13.0" chrono = { version = "0.4.22", features = ["serde"] } +url = "= 1.7" yup-oauth2 = { version = "^ 7.0", optional = true } itertools = "^ 0.10" diff --git a/google-apis-common/src/lib.rs b/google-apis-common/src/lib.rs index f0ae5e9553..d3b57acc67 100644 --- a/google-apis-common/src/lib.rs +++ b/google-apis-common/src/lib.rs @@ -1,6 +1,7 @@ pub mod auth; pub mod field_mask; pub mod serde; +pub mod url; use std::error; use std::error::Error as StdError; diff --git a/google-apis-common/src/url.rs b/google-apis-common/src/url.rs new file mode 100644 index 0000000000..5fdeefcd73 --- /dev/null +++ b/google-apis-common/src/url.rs @@ -0,0 +1,71 @@ +use std::borrow::Cow; + +use ::url::percent_encoding::{percent_encode, DEFAULT_ENCODE_SET}; +use ::url::Url; + +pub struct Params<'a> { + params: Vec<(&'a str, Cow<'a, str>)>, +} + +impl<'a> Params<'a> { + pub fn with_capacity(capacity: usize) -> Self { + Self { + params: Vec::with_capacity(capacity), + } + } + + pub fn push>>(&mut self, param: &'a str, value: I) { + self.params.push((param, value.into())) + } + + pub fn extend, IC: Into>>( + &mut self, + params: I, + ) { + self.params + .extend(params.map(|(k, v)| (k.as_str(), v.into()))) + } + + pub fn get(&self, param_name: &str) -> Option<&str> { + self.params + .iter() + .find(|(name, _)| name == ¶m_name) + .map(|(_, param)| param.as_ref()) + } + + pub fn uri_replacement( + &self, + url: String, + param: &str, + from: &str, + url_encode: bool, + ) -> String { + if url_encode { + let mut replace_with: Cow = self.get(param).unwrap_or("").into(); + if from.as_bytes()[1] == '+' as u8 { + replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET) + .to_string() + .into(); + } + url.replace(from, &replace_with) + } else { + let replace_with = self + .get(param) + .expect("to find substitution value in params"); + + url.replace(from, replace_with) + } + } + + pub fn remove_params(&mut self, to_remove: &[&str]) { + self.params.retain(|(n, _)| !to_remove.contains(n)) + } + + pub fn inner_mut(&mut self) -> &mut Vec<(&'a str, Cow<'a, str>)> { + self.params.as_mut() + } + + pub fn parse_with_url(&self, url: &str) -> Url { + Url::parse_with_params(&url, &self.params).unwrap() + } +} diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 69353798ba..dd7b4e8e4c 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -526,19 +526,16 @@ match result { /// Perform the operation you have build so far. % endif ${action_fn} { - % if URL_ENCODE in special_cases: - use url::percent_encoding::{percent_encode, DEFAULT_ENCODE_SET}; - % endif use std::io::{Read, Seek}; use hyper::header::{CONTENT_TYPE, CONTENT_LENGTH, AUTHORIZATION, USER_AGENT, LOCATION}; - use client::ToParts; + use client::{ToParts, url::Params}; use std::borrow::Cow; let mut dd = client::DefaultDelegate; let mut dlg: &mut dyn client::Delegate = ${delegate}.unwrap_or(&mut dd); dlg.begin(client::MethodInfo { id: "${m.id}", http_method: ${method_name_to_variant(m.httpMethod)} }); - let mut params: Vec<(&str, Cow)> = Vec::with_capacity(${len(params) + len(reserved_params)} + ${paddfields}.len()); + let mut params = Params::with_capacity(${len(params) + len(reserved_params)} + ${paddfields}.len()); <% if media_params and 'mediaUpload' in m: upload_type_map = dict() @@ -575,15 +572,15 @@ match result { % if p.get('repeated', False): if ${pname}.len() > 0 { for f in ${pname}.iter() { - params.push(("${p.name}", ${to_string_impl("f")}.into())); + params.push("${p.name}", ${to_string_impl("f")}); } } % elif not is_required_property(p): if let Some(value) = ${pname}.as_ref() { - params.push(("${p.name}", ${to_string_impl("value")}.into())); + params.push("${p.name}", ${to_string_impl("value")}); } % else: - params.push(("${p.name}", ${to_string_impl(pname)}.into())); + params.push("${p.name}", ${to_string_impl(pname)}); % endif % endfor ## Additional params - may not overlap with optional params @@ -594,23 +591,22 @@ match result { } } - params.extend(${paddfields}.iter().map(|(k, v)| (k.as_str(), v.into()))); - + params.extend(${paddfields}.iter()); % if response_schema: % if supports_download: let (alt_field_missing, enable_resource_parsing) = { - if let Some((_, value)) = params.iter().find(|(name, _)| name == &"alt") { + if let Some(value) = params.get("alt") { (false, value == "json") } else { (true, true) } }; if alt_field_missing { - params.push(("alt", "json".into())); + params.push("alt", "json"); } % else: - params.push(("alt", "json".into())); + params.push("alt", "json"); % endif ## supportsMediaDownload % endif ## response schema % if media_params: @@ -628,7 +624,7 @@ protocol == ${PROTOCOL_TYPE_MAP[mp.protocol]} { else { unreachable!() }; - params.push(("uploadType", upload_type.into())); + params.push("uploadType", upload_type); % else: let mut url = self.hub._base_url.clone() + "${m.path}"; % endif @@ -638,7 +634,7 @@ else { assert 'key' in parameters, "Expected 'key' parameter if there are no scopes" %> match dlg.api_key() { - Some(value) => params.push(("key", value.into())), + Some(value) => params.push("key", value), None => { ${delegate_finish}(false); return Err(client::Error::MissingAPIKey) @@ -654,35 +650,16 @@ else { ## Handle URI Templates % if replacements: for &(find_this, param_name) in [${', '.join('("%s", "%s")' % r for r in replacements)}].iter() { -<% - replace_init = ': Option<&str> = None' - replace_assign = 'Some(value)' - url_replace_arg = 'replace_with.expect("to find substitution value in params")' - if URL_ENCODE in special_cases: - replace_init = ': Cow = "".into()' - replace_assign = 'Cow::from(value.as_ref())' - url_replace_arg = '&replace_with' - # end handle url encoding -%>\ - let mut replace_with${replace_init}; - if let Some((_, value)) = params.iter().find(|(name, _)| name == ¶m_name) { - replace_with = ${replace_assign}; - } - % if URL_ENCODE in special_cases: - if find_this.as_bytes()[1] == '+' as u8 { - replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET).to_string().into(); - } - % endif - url = url.replace(find_this, ${url_replace_arg}); + url = params.uri_replacement(url, param_name, find_this, ${"true" if URL_ENCODE in special_cases else "false"}); } ## Remove all used parameters { let to_remove = [${', '.join(reversed(['"%s"' % r[1] for r in replacements]))}]; - params.retain(|(n, _)| !to_remove.contains(n)); + params.remove_params(&to_remove); } % endif - let url = url::Url::parse_with_params(&url, params).unwrap(); + let url = params.parse_with_url(&url); % if request_value: let mut json_mime_type = mime::APPLICATION_JSON; From 83007472c94d9ec7cb4f594433fbc5c0ebc3b47d Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 18:25:58 -0700 Subject: [PATCH 08/20] Fix stringly typed upload protocol --- src/generator/templates/api/lib/mbuild.mako | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index dd7b4e8e4c..b1e1ef8aeb 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -715,7 +715,7 @@ else { % if request_value and simple_media_param: let mut mp_reader: client::MultiPartReader = Default::default(); let (mut body_reader, content_type) = match protocol { - "${simple_media_param.protocol}" => { + ${PROTOCOL_TYPE_MAP[simple_media_param.protocol]} => { mp_reader.reserve_exact(2); ${READER_SEEK | indent_all_but_first_by(5)} mp_reader.add_part(&mut request_value_reader, request_size, json_mime_type.clone()) From 852bd70ecb64e3de9336d75ae5ec433bd3d97d58 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 18:42:15 -0700 Subject: [PATCH 09/20] Move error checking earlier --- src/generator/templates/api/lib/mbuild.mako | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index b1e1ef8aeb..f6fd75d157 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -535,6 +535,16 @@ match result { let mut dlg: &mut dyn client::Delegate = ${delegate}.unwrap_or(&mut dd); dlg.begin(client::MethodInfo { id: "${m.id}", http_method: ${method_name_to_variant(m.httpMethod)} }); + + ## TODO: Should go into validation function? + ## Additional params - may not overlap with optional params + for &field in [${', '.join(enclose_in('"', reserved_params + [p.name for p in field_params]))}].iter() { + if ${paddfields}.contains_key(field) { + ${delegate_finish}(false); + return Err(client::Error::FieldClash(field)); + } + } + let mut params = Params::with_capacity(${len(params) + len(reserved_params)} + ${paddfields}.len()); <% if media_params and 'mediaUpload' in m: @@ -583,13 +593,6 @@ match result { params.push("${p.name}", ${to_string_impl(pname)}); % endif % endfor - ## Additional params - may not overlap with optional params - for &field in [${', '.join(enclose_in('"', reserved_params + [p.name for p in field_params]))}].iter() { - if ${paddfields}.contains_key(field) { - ${delegate_finish}(false); - return Err(client::Error::FieldClash(field)); - } - } params.extend(${paddfields}.iter()); From d9fea508ead5a000d9f08a1c8e751e78bd6032bb Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 20 Oct 2022 11:03:56 +0800 Subject: [PATCH 10/20] enable clippy and make it to fail CI on warnings. This should improve overall code quality and reduce guess work I typically do, being used to running `clippy` on my other projects. --- .github/workflows/rust.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 16f6f28d45..e434f713d4 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -7,6 +7,14 @@ on: branches: [main] jobs: + clippy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: Swatinem/rust-cache@v2 + - name: Run clippy + run: | + cargo clippy -- -D warnings build-and-test: runs-on: ubuntu-latest env: From 4115ede2292de5f2b16ad9336b279eaff1391471 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 20 Oct 2022 11:08:20 +0800 Subject: [PATCH 11/20] Also test CLI generation, which was notably absent from CI previously It's about time this is (re)added as a regression already occurred. --- .github/workflows/rust.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index e434f713d4..a16e19469d 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -25,9 +25,11 @@ jobs: - name: Run tests run: | source ~/.profile + cargo test make test-gen make gen-all-cli cargo-api ARGS=test make cargo-api ARGS='check --no-default-features' + make cargo-api ARGS=check make cargo-api ARGS=doc + make cargo-cli ARGS=check make docs-all - cargo test From 63793b55a69a30aea17d87b29ab8b19d6d43ebdd Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:29:39 -0700 Subject: [PATCH 12/20] Add FromStr impl for FieldMask This improves compatibility with the CLI crates --- google-apis-common/src/field_mask.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/google-apis-common/src/field_mask.rs b/google-apis-common/src/field_mask.rs index c20c8b32a2..900edf11c0 100644 --- a/google-apis-common/src/field_mask.rs +++ b/google-apis-common/src/field_mask.rs @@ -1,3 +1,5 @@ +use std::str::FromStr; + use serde::{Deserialize, Deserializer, Serialize, Serializer}; fn titlecase(source: &str, dest: &mut String) { @@ -46,12 +48,13 @@ impl<'de> Deserialize<'de> for FieldMask { D: Deserializer<'de>, { let s: &str = Deserialize::deserialize(deserializer)?; - Ok(FieldMask::from_str(s)) + Ok(FieldMask::from_str(s).unwrap()) } } -impl FieldMask { - fn from_str(s: &str) -> FieldMask { +impl FromStr for FieldMask { + type Err = std::convert::Infallible; + fn from_str(s: &str) -> Result { let mut in_quotes = false; let mut prev_ind = 0; let mut paths = Vec::new(); @@ -66,9 +69,11 @@ impl FieldMask { } } paths.push(snakecase(&s[prev_ind..])); - FieldMask(paths) + Ok(FieldMask(paths)) } +} +impl FieldMask { pub fn to_string(&self) -> String { let mut repr = String::new(); for path in &self.0 { From 9c9c17b9235a66183892f0cb826e41a2b033be45 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:30:04 -0700 Subject: [PATCH 13/20] Fix default values in CLI crates --- src/generator/lib/cli.py | 15 +--- src/generator/lib/types.py | 97 +++++++++++++++++++++ src/generator/lib/util.py | 64 +------------- src/generator/templates/cli/lib/engine.mako | 11 +-- src/generator/templates/cli/main.rs.mako | 3 +- 5 files changed, 109 insertions(+), 81 deletions(-) create mode 100644 src/generator/lib/types.py diff --git a/src/generator/lib/cli.py b/src/generator/lib/cli.py index ef50b19185..9ad2be4263 100644 --- a/src/generator/lib/cli.py +++ b/src/generator/lib/cli.py @@ -6,6 +6,8 @@ import collections from copy import deepcopy from random import (randint, random, choice) +from generator.lib import types + SPLIT_START = '>>>>>>>' SPLIT_END = '<<<<<<<' @@ -56,7 +58,7 @@ JSON_TYPE_RND_MAP = {'boolean': lambda: str(bool(randint(0, 1))).lower(), 'number' : lambda: random(), 'int32' : lambda: randint(-101, -1), 'int64' : lambda: randint(-101, -1), - 'string': lambda: '%s' % choice(util.words).lower()} + 'string': lambda: '%s' % choice(types.WORDS).lower()} JSON_TYPE_TO_ENUM_MAP = {'boolean' : 'Boolean', 'integer' : 'Int', @@ -74,17 +76,6 @@ CTYPE_TO_ENUM_MAP = {CTYPE_POD: 'Pod', CTYPE_ARRAY: 'Vec', CTYPE_MAP: 'Map'} -JSON_TYPE_VALUE_MAP = {'boolean': 'false', - 'integer' : '-0', - 'uint32' : '0', - 'uint64' : '0', - 'float' : '0.0', - 'double' : '0.0', - 'number' : '0.0', - 'int32' : '-0', - 'int64' : '-0', - 'string': ''} - assert len(set(JSON_TYPE_RND_MAP.keys()) ^ POD_TYPES) == 0 def new_method_context(resource, method, c): diff --git a/src/generator/lib/types.py b/src/generator/lib/types.py new file mode 100644 index 0000000000..fcf2bbd608 --- /dev/null +++ b/src/generator/lib/types.py @@ -0,0 +1,97 @@ +from .rust_type import Base, HashMap, Vec +from random import randint, random, choice, seed + +seed(1337) + +WORDS = [ + w.strip(',') for w in + "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.".split( + ' ')] + + +def chrono_date(y=None, m=None, d=None): + y = randint(1, 9999) if y is None else y + m = randint(1, 12) if m is None else m + d = randint(1, 31) if d is None else d + return f"chrono::NaiveDate::from_ymd({y}, {m}, {d})" + + +CHRONO_PATH = "client::chrono" +CHRONO_DATETIME = f"{CHRONO_PATH}::DateTime<{CHRONO_PATH}::offset::Utc>" +CHRONO_DATE = f"{CHRONO_PATH}::NaiveDate" +USE_FORMAT = 'use_format_field' +CHRONO_UTC_NOW = "chrono::Utc::now()" + +RUST_TYPE_MAP = { + 'boolean': Base("bool"), + 'integer': USE_FORMAT, + 'number': USE_FORMAT, + 'uint32': Base("u32"), + 'double': Base("f64"), + 'float': Base("f32"), + 'int32': Base("i32"), + 'any': Base("String"), # TODO: Figure out how to handle it. It's 'interface' in Go ... + 'int64': Base("i64"), + 'uint64': Base("u64"), + 'array': Vec(None), + 'string': Base("String"), + 'object': HashMap(None, None), + # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/timestamp.proto + # In JSON format, the Timestamp type is encoded as a string in the [RFC 3339] format + 'google-datetime': Base(CHRONO_DATETIME), + # Per .json files: RFC 3339 timestamp + 'date-time': Base(CHRONO_DATETIME), + # Per .json files: A date in RFC 3339 format with only the date part + # e.g. "2013-01-15" + 'date': Base(CHRONO_DATE), + # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/duration.proto + 'google-duration': Base(f"{CHRONO_PATH}::Duration"), + # guessing bytes is universally url-safe b64 + "byte": Vec(Base("u8")), + # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/field_mask.proto + "google-fieldmask": Base("client::FieldMask") +} + +RUST_TYPE_RND_MAP = { + 'bool': lambda: str(bool(randint(0, 1))).lower(), + 'u32': lambda: randint(0, 100), + 'u64': lambda: randint(0, 100), + 'f64': lambda: random(), + 'f32': lambda: random(), + 'i32': lambda: randint(-101, -1), + 'i64': lambda: randint(-101, -1), + 'String': lambda: '"%s"' % choice(WORDS), + '&str': lambda: '"%s"' % choice(WORDS), + '&Vec': lambda: '&vec!["%s".into()]' % choice(WORDS), + "Vec": lambda: f"vec![0, 1, 2, 3]", + # why a reference to Vec? Because it works. Should be slice, but who knows how typing works here. + "&Vec": lambda: f"&vec![0, 1, 2, 3]", + # TODO: styling this + f"{CHRONO_PATH}::Duration": lambda: f"chrono::Duration::seconds({randint(0, 9999999)})", + CHRONO_DATE: chrono_date, + CHRONO_DATETIME: lambda: CHRONO_UTC_NOW, + "FieldMask": lambda: f"FieldMask(vec![{choice(WORDS)}])", +} + +JSON_TO_RUST_DEFAULT = { + 'boolean': 'false', + 'uint32': '0', + 'uint64': '0', + 'int32': "-0", + 'int64': "-0", + 'float': '0.0', + 'double': '0.0', + 'string': "\"\"", + 'google-datetime': CHRONO_UTC_NOW, + 'date-time': CHRONO_UTC_NOW, + 'date': chrono_date(2000, 1, 1), + # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/duration.proto + 'google-duration': "chrono::Duration::seconds(0)", + # guessing bytes is universally url-safe b64 + "byte": "b\"hello world\"", + # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/field_mask.proto + "google-fieldmask": "FieldMask::default()" +} + + +assert set(JSON_TO_RUST_DEFAULT.keys()).issubset(set(RUST_TYPE_MAP.keys())) \ No newline at end of file diff --git a/src/generator/lib/util.py b/src/generator/lib/util.py index 7e766a2e17..275f1247a1 100644 --- a/src/generator/lib/util.py +++ b/src/generator/lib/util.py @@ -3,12 +3,10 @@ import re import subprocess from dataclasses import dataclass -from random import (randint, random, choice, seed) from typing import Any, Dict, List, Mapping, Tuple from copy import deepcopy from .rust_type import Base, Box, HashMap, Vec, Option, RustType - -seed(1337) +from .types import RUST_TYPE_MAP, RUST_TYPE_RND_MAP re_linestart = re.compile('^', flags=re.MULTILINE) re_spaces_after_newline = re.compile('^ {4}', flags=re.MULTILINE) @@ -20,40 +18,7 @@ re_desc_parts = re.compile( re_find_replacements = re.compile(r"\{[/\+]?\w+\*?\}") HTTP_METHODS = set(("OPTIONS", "GET", "POST", "PUT", "DELETE", "HEAD", "TRACE", "CONNECT", "PATCH")) -CHRONO_PATH = "client::chrono" -CHRONO_DATETIME = f"{CHRONO_PATH}::DateTime<{CHRONO_PATH}::offset::Utc>" -CHRONO_DATE = f"{CHRONO_PATH}::NaiveDate" -USE_FORMAT = 'use_format_field' -RUST_TYPE_MAP = { - 'boolean': Base("bool"), - 'integer': USE_FORMAT, - 'number': USE_FORMAT, - 'uint32': Base("u32"), - 'double': Base("f64"), - 'float': Base("f32"), - 'int32': Base("i32"), - 'any': Base("String"), # TODO: Figure out how to handle it. It's 'interface' in Go ... - 'int64': Base("i64"), - 'uint64': Base("u64"), - 'array': Vec(None), - 'string': Base("String"), - 'object': HashMap(None, None), - # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/timestamp.proto - # In JSON format, the Timestamp type is encoded as a string in the [RFC 3339] format - 'google-datetime': Base(CHRONO_DATETIME), - # Per .json files: RFC 3339 timestamp - 'date-time': Base(CHRONO_DATETIME), - # Per .json files: A date in RFC 3339 format with only the date part - # e.g. "2013-01-15" - 'date': Base(CHRONO_DATE), - # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/duration.proto - 'google-duration': Base(f"{CHRONO_PATH}::Duration"), - # guessing bytes is universally url-safe b64 - "byte": Vec(Base("u8")), - # https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/field_mask.proto - "google-fieldmask": Base("client::FieldMask") -} RESERVED_WORDS = set(('abstract', 'alignof', 'as', 'become', 'box', 'break', 'const', 'continue', 'crate', 'do', 'else', 'enum', 'extern', 'false', 'final', 'fn', 'for', 'if', 'impl', 'in', 'let', 'loop', @@ -61,35 +26,8 @@ RESERVED_WORDS = set(('abstract', 'alignof', 'as', 'become', 'box', 'break', 'co 'return', 'sizeof', 'static', 'self', 'struct', 'super', 'true', 'trait', 'type', 'typeof', 'unsafe', 'unsized', 'use', 'virtual', 'where', 'while', 'yield')) -words = [w.strip(',') for w in - "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.".split( - ' ')] -def chrono_date(): - return f"chrono::NaiveDate::from_ymd({randint(1, 9999)}, {randint(1, 12)}, {randint(1, 31)})" - - -RUST_TYPE_RND_MAP = { - 'bool': lambda: str(bool(randint(0, 1))).lower(), - 'u32': lambda: randint(0, 100), - 'u64': lambda: randint(0, 100), - 'f64': lambda: random(), - 'f32': lambda: random(), - 'i32': lambda: randint(-101, -1), - 'i64': lambda: randint(-101, -1), - 'String': lambda: '"%s"' % choice(words), - '&str': lambda: '"%s"' % choice(words), - '&Vec': lambda: '&vec!["%s".into()]' % choice(words), - "Vec": lambda: f"vec![0, 1, 2, 3]", - # why a reference to Vec? Because it works. Should be slice, but who knows how typing works here. - "&Vec": lambda: f"&vec![0, 1, 2, 3]", - # TODO: styling this - f"{CHRONO_PATH}::Duration": lambda: f"chrono::Duration::seconds({randint(0, 9999999)})", - CHRONO_DATE: chrono_date, - CHRONO_DATETIME: lambda: f"chrono::Utc::now()", - "FieldMask": lambda: f"FieldMask(vec![{choice(words)}])", -} TREF = '$ref' IO_RESPONSE = 'response' IO_REQUEST = 'request' diff --git a/src/generator/templates/cli/lib/engine.mako b/src/generator/templates/cli/lib/engine.mako index 01cf023541..953695f141 100644 --- a/src/generator/templates/cli/lib/engine.mako +++ b/src/generator/templates/cli/lib/engine.mako @@ -5,12 +5,12 @@ ADD_SCOPE_FN, TREF, enclose_in) from generator.lib.cli import (mangle_subcommand, new_method_context, PARAM_FLAG, STRUCT_FLAG, OUTPUT_FLAG, VALUE_ARG, CONFIG_DIR, SCOPE_FLAG, is_request_value_property, FIELD_SEP, docopt_mode, FILE_ARG, MIME_ARG, OUT_ARG, - call_method_ident, POD_TYPES, opt_value, ident, JSON_TYPE_VALUE_MAP, + call_method_ident, POD_TYPES, opt_value, ident, KEY_VALUE_ARG, to_cli_schema, SchemaEntry, CTYPE_POD, actual_json_type, CTYPE_MAP, CTYPE_ARRAY, application_secret_path, CONFIG_DIR_FLAG, req_value, MODE_ARG, opt_values, SCOPE_ARG, CONFIG_DIR_ARG, DEFAULT_MIME, field_vec, comma_sep_fields, JSON_TYPE_TO_ENUM_MAP, CTYPE_TO_ENUM_MAP) - + from generator.lib.types import JSON_TO_RUST_DEFAULT v_arg = '<%s>' % VALUE_ARG SOPT = 'self.opt' @@ -226,8 +226,9 @@ for parg in ${opt_values(VALUE_ARG)} { match key { % for p in optional_props: <% - ptype = actual_json_type(p.name, p.type) - value_unwrap = 'value.unwrap_or("%s")' % JSON_TYPE_VALUE_MAP[ptype] + ptype = actual_json_type(p.name, p.get("format", p.type)) + default_value = JSON_TO_RUST_DEFAULT[ptype] + value_unwrap = f"value.unwrap_or({default_value})" %>\ "${mangle_subcommand(p.name)}" => { % if p.name == 'alt': @@ -237,7 +238,7 @@ for parg in ${opt_values(VALUE_ARG)} { % endif call = call.${mangle_ident(setter_fn_name(p))}(\ % if ptype != 'string': -arg_from_str(${value_unwrap}, err, "${mangle_subcommand(p.name)}", "${ptype}")\ + value.map(|v| arg_from_str(v, err, "${mangle_subcommand(p.name)}", "${ptype}")).unwrap_or(${default_value})\ % else: ${value_unwrap}\ % endif # handle conversion diff --git a/src/generator/templates/cli/main.rs.mako b/src/generator/templates/cli/main.rs.mako index fcdc9d9108..b286c9a452 100644 --- a/src/generator/templates/cli/main.rs.mako +++ b/src/generator/templates/cli/main.rs.mako @@ -21,7 +21,8 @@ use std::env; use std::io::{self, Write}; use clap::{App, SubCommand, Arg}; -use ${to_extern_crate_name(library_to_crate_name(library_name(name, version), make.depends_on_suffix))}::{api, Error, oauth2}; +use ${to_extern_crate_name(library_to_crate_name(library_name(name, version), make.depends_on_suffix))}::{api, Error, oauth2, client::chrono, FieldMask}; + use google_clis_common as client; From 05fc10b0c4c15713d916ec4524f13f166d172586 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:30:19 -0700 Subject: [PATCH 14/20] Fix clippy lint in preproc --- src/rust/preproc/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/preproc/src/main.rs b/src/rust/preproc/src/main.rs index d58150c144..251388994e 100644 --- a/src/rust/preproc/src/main.rs +++ b/src/rust/preproc/src/main.rs @@ -31,5 +31,5 @@ fn main() { None, ) .unwrap(); - io::stdout().write_all(&output.as_bytes()).unwrap(); + io::stdout().write_all(output.as_bytes()).unwrap(); } From 4cca633f9236dc3cc1a799f562931c9f12026c9f Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:31:31 -0700 Subject: [PATCH 15/20] Apply cargo clippy --fix --- google-apis-common/src/field_mask.rs | 2 +- google-apis-common/src/lib.rs | 18 +++++++++--------- google-apis-common/src/serde.rs | 6 +++--- google-apis-common/src/url.rs | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/google-apis-common/src/field_mask.rs b/google-apis-common/src/field_mask.rs index 900edf11c0..88fa240f98 100644 --- a/google-apis-common/src/field_mask.rs +++ b/google-apis-common/src/field_mask.rs @@ -30,7 +30,7 @@ fn snakecase(source: &str) -> String { } /// A `FieldMask` as defined in `https://github.com/protocolbuffers/protobuf/blob/ec1a70913e5793a7d0a7b5fbf7e0e4f75409dd41/src/google/protobuf/field_mask.proto#L180` -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct FieldMask(Vec); impl Serialize for FieldMask { diff --git a/google-apis-common/src/lib.rs b/google-apis-common/src/lib.rs index d3b57acc67..194ef55201 100644 --- a/google-apis-common/src/lib.rs +++ b/google-apis-common/src/lib.rs @@ -24,7 +24,7 @@ use serde_json as json; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::time::sleep; -use tower_service; + pub use auth::{GetToken, NoToken}; pub use chrono; @@ -372,7 +372,7 @@ impl<'a> MultiPartReader<'a> { let mut headers = HeaderMap::new(); headers.insert( CONTENT_TYPE, - hyper::header::HeaderValue::from_str(&mime_type.to_string()).unwrap(), + hyper::header::HeaderValue::from_str(mime_type.as_ref()).unwrap(), ); headers.insert(CONTENT_LENGTH, size.into()); self.raw_parts.push((headers, reader)); @@ -481,7 +481,7 @@ impl<'a> Read for MultiPartReader<'a> { /// /// Generated via rustc --pretty expanded -Z unstable-options, and manually /// processed to be more readable. -#[derive(PartialEq, Debug, Clone)] +#[derive(PartialEq, Eq, Debug, Clone)] pub struct XUploadContentType(pub Mime); impl ::std::ops::Deref for XUploadContentType { @@ -501,7 +501,7 @@ impl Display for XUploadContentType { } } -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug)] pub struct Chunk { pub first: u64, pub last: u64, @@ -537,7 +537,7 @@ impl FromStr for Chunk { } /// Implements the Content-Range header, for serialization only -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug)] pub struct ContentRange { pub range: Option, pub total_length: u64, @@ -556,7 +556,7 @@ impl ContentRange { } } -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug)] pub struct RangeResponseHeader(pub Chunk); impl RangeResponseHeader { @@ -565,7 +565,7 @@ impl RangeResponseHeader { if let Ok(s) = std::str::from_utf8(raw) { const PREFIX: &str = "bytes "; if let Some(stripped) = s.strip_prefix(PREFIX) { - if let Ok(c) = ::from_str(&stripped) { + if let Ok(c) = ::from_str(stripped) { return RangeResponseHeader(c); } } @@ -778,7 +778,7 @@ mod test_api { use std::str::FromStr; use ::serde::{Deserialize, Serialize}; - use mime; + use serde_json as json; #[test] @@ -809,7 +809,7 @@ mod test_api { json::to_string(&::default()).unwrap(); let j = "{\"snooSnoo\":\"foo\"}"; - let b: Bar = json::from_str(&j).unwrap(); + let b: Bar = json::from_str(j).unwrap(); assert_eq!(b.snoo_snoo, "foo"); // We can't have unknown fields with structs. diff --git a/google-apis-common/src/serde.rs b/google-apis-common/src/serde.rs index e7de0b1c4e..d2f1ef6922 100644 --- a/google-apis-common/src/serde.rs +++ b/google-apis-common/src/serde.rs @@ -61,9 +61,9 @@ pub mod duration { }; let (seconds, nanoseconds) = if let Some((seconds, nanos)) = value.split_once('.') { - let is_neg = seconds.starts_with("-"); + let is_neg = seconds.starts_with('-'); let seconds = i64::from_str(seconds)?; - let nano_magnitude = nanos.chars().filter(|c| c.is_digit(10)).count() as u32; + let nano_magnitude = nanos.chars().filter(|c| c.is_ascii_digit()).count() as u32; if nano_magnitude > 9 { // not enough precision to model the remaining digits return Err(ParseDurationError::NanosTooSmall); @@ -263,7 +263,7 @@ mod test { serde_json::from_str(r#"{"bytes": "aGVsbG8gd29ybGQ="}"#).unwrap(); assert_eq!( Some(b"hello world".as_slice()), - wrapper.bytes.as_ref().map(Vec::as_slice) + wrapper.bytes.as_deref() ); } diff --git a/google-apis-common/src/url.rs b/google-apis-common/src/url.rs index 5fdeefcd73..0cd831fff7 100644 --- a/google-apis-common/src/url.rs +++ b/google-apis-common/src/url.rs @@ -42,7 +42,7 @@ impl<'a> Params<'a> { ) -> String { if url_encode { let mut replace_with: Cow = self.get(param).unwrap_or("").into(); - if from.as_bytes()[1] == '+' as u8 { + if from.as_bytes()[1] == b'+' { replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET) .to_string() .into(); @@ -66,6 +66,6 @@ impl<'a> Params<'a> { } pub fn parse_with_url(&self, url: &str) -> Url { - Url::parse_with_params(&url, &self.params).unwrap() + Url::parse_with_params(url, &self.params).unwrap() } } From 85e0d284d1551d1809e6883a3a18b8ce62f7b831 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:35:56 -0700 Subject: [PATCH 16/20] Update mime dependency in google-clis-common --- google-clis-common/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-clis-common/Cargo.toml b/google-clis-common/Cargo.toml index ab0f740737..130b73a2f8 100644 --- a/google-clis-common/Cargo.toml +++ b/google-clis-common/Cargo.toml @@ -16,7 +16,7 @@ edition = "2021" doctest = false [dependencies] -mime = "0.2" +mime = "^ 0.3" yup-oauth2 = "^ 7.0" serde = "1" serde_json = "1" From f4317a2968238eb0837bb88e65de4cee387d500f Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:39:28 -0700 Subject: [PATCH 17/20] cargo clippy --fix google-clis-common --- google-apis-common/src/url.rs | 2 +- google-clis-common/src/lib.rs | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/google-apis-common/src/url.rs b/google-apis-common/src/url.rs index 0cd831fff7..e175c3aa49 100644 --- a/google-apis-common/src/url.rs +++ b/google-apis-common/src/url.rs @@ -41,7 +41,7 @@ impl<'a> Params<'a> { url_encode: bool, ) -> String { if url_encode { - let mut replace_with: Cow = self.get(param).unwrap_or("").into(); + let mut replace_with: Cow = self.get(param).unwrap_or_default().into(); if from.as_bytes()[1] == b'+' { replace_with = percent_encode(replace_with.as_bytes(), DEFAULT_ENCODE_SET) .to_string() diff --git a/google-clis-common/src/lib.rs b/google-clis-common/src/lib.rs index f1a74813ea..2bdb12a627 100644 --- a/google-clis-common/src/lib.rs +++ b/google-clis-common/src/lib.rs @@ -50,11 +50,11 @@ pub fn remove_json_null_values(value: &mut Value) { Value::Object(ref mut map) => { let mut for_removal = Vec::new(); - for (key, mut value) in map.iter_mut() { + for (key, value) in map.iter_mut() { if value.is_null() { for_removal.push(key.clone()); } else { - remove_json_null_values(&mut value); + remove_json_null_values(value); } } @@ -221,9 +221,9 @@ impl FieldCursor { let push_field = |fs: &mut String, f: &mut String| { if !f.is_empty() { - fs.push_str(match did_you_mean(&f, possible_values) { + fs.push_str(match did_you_mean(f, possible_values) { Some(candidate) => candidate, - None => &f, + None => f, }); f.truncate(0); } @@ -264,7 +264,7 @@ impl FieldCursor { for field in &self.0[..self.0.len() - 1] { let tmp = object; object = match *tmp { - Value::Object(ref mut mapping) => assure_entry(mapping, &field), + Value::Object(ref mut mapping) => assure_entry(mapping, field), _ => panic!("We don't expect non-object Values here ..."), }; } @@ -276,18 +276,18 @@ impl FieldCursor { |value: &str, jtype: JsonType, err: &mut InvalidOptionsError| -> Value { match jtype { JsonType::Boolean => { - Value::Bool(arg_from_str(value, err, &field, "boolean")) + Value::Bool(arg_from_str(value, err, field, "boolean")) } JsonType::Int => Value::Number( - json::Number::from_f64(arg_from_str(value, err, &field, "int")) + json::Number::from_f64(arg_from_str(value, err, field, "int")) .expect("valid f64"), ), JsonType::Uint => Value::Number( - json::Number::from_f64(arg_from_str(value, err, &field, "uint")) + json::Number::from_f64(arg_from_str(value, err, field, "uint")) .expect("valid f64"), ), JsonType::Float => Value::Number( - json::Number::from_f64(arg_from_str(value, err, &field, "float")) + json::Number::from_f64(arg_from_str(value, err, field, "float")) .expect("valid f64"), ), JsonType::String => Value::String(value.to_owned()), @@ -315,7 +315,7 @@ impl FieldCursor { let (key, value) = parse_kv_arg(value, err, true); let jval = to_jval(value.unwrap_or(""), type_info.jtype, err); - match *assure_entry(mapping, &field) { + match *assure_entry(mapping, field) { Value::Object(ref mut value_map) => { if value_map.insert(key.to_owned(), jval).is_some() { err.issues.push(CLIError::Field(FieldError::Duplicate( @@ -590,7 +590,7 @@ impl fmt::Display for CLIError { arg_name, value, type_name, err_desc ), CLIError::UnknownParameter(ref param_name, ref possible_values) => { - let suffix = match did_you_mean(param_name, &possible_values) { + let suffix = match did_you_mean(param_name, possible_values) { Some(v) => format!(" Did you mean '{}' ?", v), None => String::new(), }; From 9285942f3d06039f85be8d60a19fcff12e5efdf2 Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:42:47 -0700 Subject: [PATCH 18/20] impl std::fmt::Display for FieldMask --- google-apis-common/src/field_mask.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/google-apis-common/src/field_mask.rs b/google-apis-common/src/field_mask.rs index 88fa240f98..f84dcf4f10 100644 --- a/google-apis-common/src/field_mask.rs +++ b/google-apis-common/src/field_mask.rs @@ -1,3 +1,4 @@ +use std::fmt::{Display, Formatter}; use std::str::FromStr; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -73,15 +74,15 @@ impl FromStr for FieldMask { } } -impl FieldMask { - pub fn to_string(&self) -> String { +impl Display for FieldMask { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let mut repr = String::new(); for path in &self.0 { titlecase(path, &mut repr); repr.push(','); } repr.pop(); - repr + f.write_str(&repr) } } From 4bdd77a52ffa57c6cf6649f7973f5f1eee9e9d6e Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:42:56 -0700 Subject: [PATCH 19/20] cargo fmt --- google-apis-common/src/lib.rs | 3 +-- google-apis-common/src/serde.rs | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/google-apis-common/src/lib.rs b/google-apis-common/src/lib.rs index 194ef55201..71907cb8d2 100644 --- a/google-apis-common/src/lib.rs +++ b/google-apis-common/src/lib.rs @@ -25,7 +25,6 @@ use serde_json as json; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::time::sleep; - pub use auth::{GetToken, NoToken}; pub use chrono; pub use field_mask::FieldMask; @@ -778,7 +777,7 @@ mod test_api { use std::str::FromStr; use ::serde::{Deserialize, Serialize}; - + use serde_json as json; #[test] diff --git a/google-apis-common/src/serde.rs b/google-apis-common/src/serde.rs index d2f1ef6922..29277f3f1a 100644 --- a/google-apis-common/src/serde.rs +++ b/google-apis-common/src/serde.rs @@ -261,10 +261,7 @@ mod test { fn urlsafe_base64_de_success_cases() { let wrapper: Base64Wrapper = serde_json::from_str(r#"{"bytes": "aGVsbG8gd29ybGQ="}"#).unwrap(); - assert_eq!( - Some(b"hello world".as_slice()), - wrapper.bytes.as_deref() - ); + assert_eq!(Some(b"hello world".as_slice()), wrapper.bytes.as_deref()); } #[test] From b706de7a9569f182d022b097f8eb42677815808a Mon Sep 17 00:00:00 2001 From: philippeitis <33013301+philippeitis@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:47:19 -0700 Subject: [PATCH 20/20] Add default impl to InvalidOptionsError --- google-clis-common/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/google-clis-common/src/lib.rs b/google-clis-common/src/lib.rs index 2bdb12a627..36ebec625a 100644 --- a/google-clis-common/src/lib.rs +++ b/google-clis-common/src/lib.rs @@ -620,6 +620,15 @@ pub struct InvalidOptionsError { pub exit_code: i32, } +impl Default for InvalidOptionsError { + fn default() -> Self { + InvalidOptionsError { + issues: Vec::new(), + exit_code: 1, + } + } +} + impl fmt::Display for InvalidOptionsError { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { for issue in &self.issues { @@ -638,10 +647,7 @@ impl InvalidOptionsError { } pub fn new() -> InvalidOptionsError { - InvalidOptionsError { - issues: Vec::new(), - exit_code: 1, - } + Default::default() } }