From ed6f337a486cfaa77867bc8f8886296d667b3343 Mon Sep 17 00:00:00 2001 From: Arash Outadi Date: Wed, 8 Jul 2020 09:49:27 -0700 Subject: [PATCH] Refactor Fetch command (No logic changes) (#2131) * Refactoring unrelated to Streaming... Mainly keeping code DRY * Remove cli as dependency --- crates/nu_plugin_fetch/src/fetch.rs | 199 ++++++++++----------------- crates/nu_plugin_fetch/src/nu/mod.rs | 4 +- 2 files changed, 75 insertions(+), 128 deletions(-) diff --git a/crates/nu_plugin_fetch/src/fetch.rs b/crates/nu_plugin_fetch/src/fetch.rs index b9e2d61b0e..c384c24229 100644 --- a/crates/nu_plugin_fetch/src/fetch.rs +++ b/crates/nu_plugin_fetch/src/fetch.rs @@ -56,7 +56,7 @@ impl Fetch { } } -pub async fn fetch_helper( +pub async fn fetch( path: &Value, has_raw: bool, user: Option, @@ -65,12 +65,12 @@ pub async fn fetch_helper( let path_str = path.as_string()?; let path_span = path.tag.span; - let result = fetch(&path_str, path_span, has_raw, user, password).await; + let result = helper(&path_str, path_span, has_raw, user, password).await; if let Err(e) = result { return Err(e); } - let (file_extension, contents, contents_tag) = result?; + let (file_extension, value) = result?; let file_extension = if has_raw { None @@ -80,32 +80,32 @@ pub async fn fetch_helper( file_extension.or_else(|| path_str.split('.').last().map(String::from)) }; - let tagged_contents = contents.retag(&contents_tag); - if let Some(extension) = file_extension { Ok(ReturnSuccess::Action(CommandAction::AutoConvert( - tagged_contents, - extension, + value, extension, ))) } else { - ReturnSuccess::value(tagged_contents) + ReturnSuccess::value(value) } } -pub async fn fetch( +// Helper function that actually goes to retrieve the resource from the url given +// The Option return a possible file extension which can be used in AutoConvert commands +async fn helper( location: &str, span: Span, has_raw: bool, user: Option, password: Option, -) -> Result<(Option, UntaggedValue, Tag), ShellError> { - if url::Url::parse(location).is_err() { +) -> Result<(Option, Value), ShellError> { + if let Err(e) = url::Url::parse(location) { return Err(ShellError::labeled_error( - "Incomplete or incorrect url", + format!("Incomplete or incorrect url:\n{:?}", e), "expected a full url", span, )); } + let login = match (user, password) { (Some(user), Some(password)) => Some(encode(&format!("{}:{}", user, password))), (Some(user), _) => Some(encode(&format!("{}:", user))), @@ -115,6 +115,17 @@ pub async fn fetch( if let Some(login) = login { response = response.set_header("Authorization", format!("Basic {}", login)); } + let generate_error = |t: &str, e: Box, span: &Span| { + ShellError::labeled_error( + format!("Could not load {} from remote url: {:?}", t, e), + "could not load", + span, + ) + }; + let tag = Tag { + span, + anchor: Some(AnchorLocation::Url(location.to_string())), + }; match response.await { Ok(mut r) => match r.headers().get("content-type") { Some(content_type) => { @@ -128,93 +139,56 @@ pub async fn fetch( match (content_type.type_(), content_type.subtype()) { (mime::APPLICATION, mime::XML) => Ok(( Some("xml".to_string()), - UntaggedValue::string(r.body_string().await.map_err(|_| { - ShellError::labeled_error( - "Could not load text from remote url", - "could not load", - span, - ) - })?), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string( + r.body_string() + .await + .map_err(|e| generate_error("text", e, &span))?, + ) + .into_value(tag), )), (mime::APPLICATION, mime::JSON) => Ok(( Some("json".to_string()), - UntaggedValue::string(r.body_string().await.map_err(|_| { - ShellError::labeled_error( - "Could not load text from remote url", - "could not load", - span, - ) - })?), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string( + r.body_string() + .await + .map_err(|e| generate_error("text", e, &span))?, + ) + .into_value(tag), )), (mime::APPLICATION, mime::OCTET_STREAM) => { - let buf: Vec = r.body_bytes().await.map_err(|_| { - ShellError::labeled_error( - "Could not load binary file", - "could not load", - span, - ) - })?; - Ok(( - None, - UntaggedValue::binary(buf), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, - )) + let buf: Vec = r + .body_bytes() + .await + .map_err(|e| generate_error("binary", Box::new(e), &span))?; + Ok((None, UntaggedValue::binary(buf).into_value(tag))) } (mime::IMAGE, mime::SVG) => Ok(( Some("svg".to_string()), - UntaggedValue::string(r.body_string().await.map_err(|_| { - ShellError::labeled_error( - "Could not load svg from remote url", - "could not load", - span, - ) - })?), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string( + r.body_string() + .await + .map_err(|e| generate_error("svg", e, &span))?, + ) + .into_value(tag), )), (mime::IMAGE, image_ty) => { - let buf: Vec = r.body_bytes().await.map_err(|_| { - ShellError::labeled_error( - "Could not load image file", - "could not load", - span, - ) - })?; + let buf: Vec = r + .body_bytes() + .await + .map_err(|e| generate_error("image", Box::new(e), &span))?; Ok(( Some(image_ty.to_string()), - UntaggedValue::binary(buf), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::binary(buf).into_value(tag), )) } (mime::TEXT, mime::HTML) => Ok(( Some("html".to_string()), - UntaggedValue::string(r.body_string().await.map_err(|_| { - ShellError::labeled_error( - "Could not load text from remote url", - "could not load", - span, - ) - })?), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string( + r.body_string() + .await + .map_err(|e| generate_error("text", e, &span))?, + ) + .into_value(tag), )), (mime::TEXT, mime::PLAIN) => { let path_extension = url::Url::parse(location) @@ -236,17 +210,12 @@ pub async fn fetch( Ok(( path_extension, - UntaggedValue::string(r.body_string().await.map_err(|_| { - ShellError::labeled_error( - "Could not load text from remote url", - "could not load", - span, - ) - })?), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string( + r.body_string() + .await + .map_err(|e| generate_error("text", e, &span))?, + ) + .into_value(tag), )) } (_ty, _sub_ty) if has_raw => { @@ -255,44 +224,22 @@ pub async fn fetch( // For unsupported MIME types, we do not know if the data is UTF-8, // so we get the raw body bytes and try to convert to UTF-8 if possible. match std::str::from_utf8(&raw_bytes) { - Ok(response_str) => Ok(( - None, - UntaggedValue::string(response_str), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, - )), - Err(_) => Ok(( - None, - UntaggedValue::binary(raw_bytes), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, - )), + Ok(response_str) => { + Ok((None, UntaggedValue::string(response_str).into_value(tag))) + } + Err(_) => Ok((None, UntaggedValue::binary(raw_bytes).into_value(tag))), } } - (ty, sub_ty) => Ok(( - None, - UntaggedValue::string(format!( - "Not yet supported MIME type: {} {}", - ty, sub_ty - )), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, - )), + (ty, sub_ty) => Err(ShellError::unimplemented(format!( + "Not yet supported MIME type: {} {}", + ty, sub_ty + ))), } } + // TODO: Should this return "nothing" or Err? None => Ok(( None, - UntaggedValue::string("No content type found".to_owned()), - Tag { - span, - anchor: Some(AnchorLocation::Url(location.to_string())), - }, + UntaggedValue::string("No content type found".to_owned()).into_value(tag), )), }, Err(_) => Err(ShellError::labeled_error( diff --git a/crates/nu_plugin_fetch/src/nu/mod.rs b/crates/nu_plugin_fetch/src/nu/mod.rs index 5989646807..b891cce727 100644 --- a/crates/nu_plugin_fetch/src/nu/mod.rs +++ b/crates/nu_plugin_fetch/src/nu/mod.rs @@ -3,7 +3,7 @@ use nu_errors::ShellError; use nu_plugin::Plugin; use nu_protocol::{CallInfo, ReturnValue, Signature, SyntaxShape}; -use crate::fetch::fetch_helper; +use crate::fetch::fetch; use crate::Fetch; impl Plugin for Fetch { @@ -33,7 +33,7 @@ impl Plugin for Fetch { fn begin_filter(&mut self, callinfo: CallInfo) -> Result, ShellError> { self.setup(callinfo)?; - Ok(vec![block_on(fetch_helper( + Ok(vec![block_on(fetch( &self.path.clone().ok_or_else(|| { ShellError::labeled_error("internal error: path not set", "path not set", &self.tag) })?,