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.

Sign up for GitHub

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

stability: remove unwrap call in volo #310

Merged
merged 5 commits into from
Jan 22, 2024
Merged

stability: remove unwrap call in volo #310

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
34f95b9
stability: remove unwrap call in volo
Ggiggle Jan 3, 2024
8657a69
format the code
Ggiggle Jan 22, 2024
c33e5b3
format the code
Ggiggle Jan 22, 2024
5107aef
remove unused error kind and fix some clippy warning
Ggiggle Jan 22, 2024
5f14594
remove unused error kind and fix some clippy warning
Ggiggle Jan 22, 2024
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
140 changes: 70 additions & 70 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions examples/src/http/simple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
async fn json_post(Json(request): Json<Person>) -> String {
let first_phone = request
.phones
.get(0)
.first()
.map(|p| p.as_str())
.unwrap_or("no number");
format!(
Expand All @@ -53,9 +53,9 @@
return Err(StatusCode::BAD_REQUEST);
}
};
let first_phone = request
.phones
.get(0)

Check warning on line 58 in examples/src/http/simple.rs

View workflow job for this annotation

GitHub Actions / clippy

accessing first element with `request .phones.get(0)` 

warning: accessing first element with `request
                 .phones.get(0)`
  --> examples/src/http/simple.rs:56:23
   |
56 |       let first_phone = request
   |  _______________________^
57 | |         .phones
58 | |         .get(0)
   | |_______________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#get_first
   = note: `#[warn(clippy::get_first)]` on by default
help: try
   |
56 ~     let first_phone = request
57 +         .phones.first()
   |
.map(|p| p.as_str())
.unwrap_or("no number");
Ok(format!(
Expand Down Expand Up @@ -223,7 +223,7 @@
// ```
async fn tracing_from_fn(
uri: Uri,
peer: Address,
peer: Option<Address>,
cookie_jar: CookieJar,
cx: &mut ServerContext,
req: BodyIncoming,
Expand All @@ -237,7 +237,7 @@
let resp = next.run(cx, req).await;
let elapsed = start.elapsed();

tracing::info!("seq: {count}: {peer} request {uri}, cost {elapsed:?}");
tracing::info!("seq: {count}: {peer:?} request {uri}, cost {elapsed:?}");

(
(
Expand Down
51 changes: 32 additions & 19 deletions volo-build/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::{
use anyhow::{bail, Context};
use itertools::Itertools;
use lazy_static::lazy_static;
use serde::de::Error;

use crate::model::{Config, GitSource, Idl, Source};

Expand All @@ -30,10 +31,24 @@ pub fn ensure_cache_path() -> std::io::Result<()> {
}

pub fn read_config_from_file(f: &File) -> Result<Config, serde_yaml::Error> {
if f.metadata().unwrap().len() != 0 {
serde_yaml::from_reader(f)
} else {
Ok(Config::new())
match f.metadata() {
Ok(metadata) => {
if metadata.len() == 0 {
Ok(Config::new())
} else {
serde_yaml::from_reader(f)
}
}
Err(e) => {
if e.kind() == std::io::ErrorKind::NotFound {
Ok(Config::new())
} else {
Err(serde_yaml::Error::custom(format!(
"failed to read config file, err: {}",
e
)))
}
}
}
}

Expand Down Expand Up @@ -206,16 +221,16 @@ impl Task {
}

pub fn get_repo_latest_commit_id(repo: &str, r#ref: &str) -> anyhow::Result<String> {
let commit_list = unsafe {
String::from_utf8_unchecked(
Command::new("git")
.arg("ls-remote")
.arg(repo)
.arg(r#ref)
.output()
.unwrap()
.stdout,
)
let commit_list = match Command::new("git")
.arg("ls-remote")
.arg(repo)
.arg(r#ref)
.output()
{
Ok(output) => unsafe { String::from_utf8_unchecked(output.stdout) },
Err(e) => {
bail!("git ls-remote {} {} err:{}", repo, r#ref, e);
}
};
let commit_list: Vec<_> = commit_list
.split('\n')
Expand Down Expand Up @@ -300,11 +315,9 @@ pub fn git_repo_init(path: &Path) -> anyhow::Result<()> {
}

pub fn strip_slash_prefix(p: &Path) -> PathBuf {
if p.starts_with("/") {
// remove the "/" prefix to the start of idl path
p.strip_prefix("/").unwrap().to_path_buf()
} else {
p.to_path_buf()
match p.strip_prefix("/") {
Ok(p) => p.to_path_buf(),
Err(_) => p.to_path_buf(),
}
}

Expand Down
38 changes: 27 additions & 11 deletions volo-build/src/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,20 @@ where
{
pub fn gen(self) {
let work_dir = std::env::current_dir().unwrap();
let config = std::fs::read(work_dir.join("volo.workspace.yml")).unwrap();
let config = serde_yaml::from_slice::<WorkspaceConfig>(&config).unwrap();

let config = match std::fs::read(work_dir.join("volo.workspace.yml")) {
Ok(config) => config,
Err(e) => {
eprintln!("failed to read volo.workspace.yml file, err: {}", e);
std::process::exit(1);
}
};
let config = match serde_yaml::from_slice::<WorkspaceConfig>(&config) {
Ok(config) => config,
Err(e) => {
eprintln!("failed to parse volo.workspace.yml, err: {}", e);
std::process::exit(1);
}
};
let services = config
.services
.into_iter()
Expand All @@ -60,14 +71,19 @@ where
config: s.config,
})
})
.collect::<Result<Vec<_>, _>>()
.unwrap();

self.ignore_unused(!config.touch_all)
.dedup(config.dedup_list)
.nonstandard_snake_case(config.nonstandard_snake_case)
.pilota_builder
.compile_with_config(services, pilota_build::Output::Workspace(work_dir));
.collect::<Result<Vec<_>, _>>();
match services {
Ok(services) => {
self.ignore_unused(!config.touch_all)
.dedup(config.dedup_list)
.pilota_builder
.compile_with_config(services, pilota_build::Output::Workspace(work_dir));
}
Err(e) => {
eprintln!("failed to get or download idl, err: {}", e);
std::process::exit(1);
}
}
yukiiiteru marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn plugin(mut self, plugin: impl Plugin + 'static) -> Self {
Expand Down
35 changes: 24 additions & 11 deletions volo-cli/src/idl/add.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,36 @@ impl CliCommand for Add {
if idl.path != self.idl {
continue;
}
match idl.source {
Source::Git(ref mut source) if self.git.as_ref() == Some(&source.repo) => {
if let Some(r#ref) = self.r#ref.as_ref() {
let _ = source.r#ref.insert(r#ref.clone());
if let Source::Git(ref git) = new_idl.source {
let _ = source.lock.insert(git.lock.clone().unwrap());
}
}

let git_source = match idl.source {
Source::Git(ref mut git_source)
if self.git.as_ref() == Some(&git_source.repo) =>
{
has_found_idl = true;
break;
git_source
}
Source::Local if self.git.is_none() => {
Source::Git(_) => continue,
Source::Local => {
has_found_idl = true;
break;
}
_ => {}
};

if let Some(r#ref) = self.r#ref.as_ref() {
let _ = git_source.r#ref.insert(r#ref.clone());
} else {
unreachable!("git ref should be Some if git source is git")
}
let git = match new_idl.source {
Source::Git(ref git) => git,
_ => unreachable!("git source should be git if idl source is git"),
};
if let Some(lock) = git.lock.clone() {
let _ = git_source.lock.insert(lock);
} else {
unreachable!("git lock should be Some if idl source is git")
}
break;
}

if !has_found_idl {
Expand Down
26 changes: 19 additions & 7 deletions volo-cli/src/idl/update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ impl CliCommand for Update {
std::process::exit(1);
}

let entry = config.entries.get_mut(&cx.entry_name).unwrap();
let entry = match config.entries.get_mut(&cx.entry_name) {
Some(entry) => entry,
None => {
eprintln!("entry {} not found", cx.entry_name);
std::process::exit(1);
}
};
let mut exists = HashSet::new();

entry.idls.iter_mut().for_each(|idl| {
idl.path = strip_slash_prefix(idl.path.as_path());
if let Source::Git(ref git) = idl.source {
Expand All @@ -46,12 +51,12 @@ impl CliCommand for Update {
.iter()
.filter_map(|repo| {
entry.idls.iter_mut().find_map(|config_idl| {
if let Source::Git(git_source) = &mut config_idl.source {
if *repo == git_source.repo {
return Some(git_source as *mut _);
match &mut config_idl.source {
Source::Git(git_source) if *repo == git_source.repo => {
Some(git_source as *mut _)
}
_ => None,
}
None
})
})
.collect()
Expand All @@ -72,7 +77,14 @@ impl CliCommand for Update {

should_update_gits
.into_iter()
.try_for_each(|git_source| unsafe { git_source.as_mut().unwrap().update() })?;
.try_for_each(|git_source| unsafe {
if let Some(git_source) = git_source.as_mut() {
git_source.update()
} else {
eprintln!("git source is null");
std::process::exit(1);
}
})?;

Ok(())
})
Expand Down
45 changes: 26 additions & 19 deletions volo-cli/src/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ impl CliCommand for Init {
lock,
});
}

if self.git.is_some() {
idl.path = strip_slash_prefix(&self.idl);
} else {
Expand All @@ -202,6 +203,7 @@ impl CliCommand for Init {
touch_all: false,
nonstandard_snake_case: false,
};

if self.is_grpc_project() {
self.copy_grpc_template(entry.clone())?;
} else {
Expand All @@ -210,17 +212,18 @@ impl CliCommand for Init {

if self.git.as_ref().is_none() {
// we will move volo.yml to volo-gen, so we need to add .. to includes and idl path
let idl = entry.idls.get_mut(0).unwrap();
if let Some(includes) = &mut idl.includes {
for i in includes {
if i.is_absolute() {
continue;
if let Some(idl) = entry.idls.get_mut(0) {
if let Some(includes) = &mut idl.includes {
for i in includes {
if i.is_absolute() {
continue;
}
*i = PathBuf::new().join("../").join(i.clone());
}
*i = PathBuf::new().join("../").join(i.clone());
}
}
if !idl.path.is_absolute() {
idl.path = PathBuf::new().join("../").join(self.idl.clone());
if !idl.path.is_absolute() {
idl.path = PathBuf::new().join("../").join(self.idl.clone());
}
}
}

Expand All @@ -233,24 +236,28 @@ impl CliCommand for Init {
if self.idl != idl.path {
continue;
}
match idl.source {
let (repo, r#ref) = match idl.source {
Source::Git(GitSource {
ref mut repo,
ref mut r#ref,
..
}) if self.git.is_some() => {
}) => {
// found the desired idl, update it
found = true;
if self.git.is_some() {
*repo = self.git.as_ref().unwrap().clone();
if self.r#ref.is_some() {
*r#ref = self.r#ref.clone();
}
}
break;
(repo, r#ref)
}
_ => continue,
};

if let Some(git) = self.git.as_ref() {
*repo = git.clone();
if self.r#ref.is_some() {
*r#ref = self.r#ref.clone();
}
_ => {}
} else {
unreachable!()
}
break;
}

if !found {
Expand Down
4 changes: 2 additions & 2 deletions volo-grpc/src/codec/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ impl<T: Message + Default> RecvStream<T> {
Err(_frame) => {
// **unreachable** because the `frame` cannot be `Frame::Data` here
debug!("[VOLO] unexpected data from stream");
return Err(Status::new(
Err(Status::new(
Code::Internal,
"Unexpected data from stream.".to_string(),
));
))
}
},
Some(Err(err)) => Err(Status::from_error(Box::new(err))),
Expand Down
2 changes: 1 addition & 1 deletion volo-grpc/src/status.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl Status {
.expect("Invalid status header, expected base64 encoded value")
})
.map(Bytes::from)
.unwrap_or_else(Bytes::new);
.unwrap_or_default();

// must remove these redundant message from the header map
let mut other_headers = header_map.clone();
Expand Down
4 changes: 3 additions & 1 deletion volo-http/src/extract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@
pub struct MaybeInvalid<T>(Vec<u8>, PhantomData<T>);

impl MaybeInvalid<String> {
pub unsafe fn assume_valid(self) -> String {

Check warning on line 60 in volo-http/src/extract.rs

View workflow job for this annotation

GitHub Actions / clippy

unsafe function's docs miss `# Safety` section 

warning: unsafe function's docs miss `# Safety` section
  --> volo-http/src/extract.rs:60:5
   |
60 |     pub unsafe fn assume_valid(self) -> String {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
   = note: `#[warn(clippy::missing_safety_doc)]` on by default
String::from_utf8_unchecked(self.0)
}
}

impl MaybeInvalid<FastStr> {
pub unsafe fn assume_valid(self) -> FastStr {

Check warning on line 66 in volo-http/src/extract.rs

View workflow job for this annotation

GitHub Actions / clippy

unsafe function's docs miss `# Safety` section 

warning: unsafe function's docs miss `# Safety` section
  --> volo-http/src/extract.rs:66:5
   |
66 |     pub unsafe fn assume_valid(self) -> FastStr {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
FastStr::from_vec_u8_unchecked(self.0)
}
}
Expand All @@ -81,7 +81,7 @@
}

impl<S: Sync> FromContext<S> for Address {
type Rejection = Infallible;
type Rejection = RejectionError;

async fn from_context(cx: &mut ServerContext, _state: &S) -> Result<Address, Self::Rejection> {
Ok(cx.peer.clone())
Expand Down Expand Up @@ -309,6 +309,7 @@
JsonRejection(crate::json::Error),
QueryRejection(serde_urlencoded::de::Error),
FormRejection(serde_html_form::de::Error),
EmptyAddressError,
}

unsafe impl Send for RejectionError {}
Expand All @@ -322,6 +323,7 @@
Self::JsonRejection(_) => StatusCode::BAD_REQUEST,
Self::QueryRejection(_) => StatusCode::BAD_REQUEST,
Self::FormRejection(_) => StatusCode::BAD_REQUEST,
Self::EmptyAddressError => StatusCode::BAD_REQUEST,
};

status.into_response()
Expand Down
Loading
Loading