This is an automated email from the ASF dual-hosted git repository. gxd pushed a commit to branch services_gdrive in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
commit 211dc9db18230769488e012452312180863093ab Author: G-XD <[email protected]> AuthorDate: Fri Dec 22 00:59:06 2023 +0800 fix(services/gdrive): fix return value of get_file_id_by_path --- core/src/services/gdrive/backend.rs | 30 ++++++++++---- core/src/services/gdrive/core.rs | 82 ++++++++++++++++++++----------------- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/core/src/services/gdrive/backend.rs b/core/src/services/gdrive/backend.rs index eb38e75d5..fd83d96fa 100644 --- a/core/src/services/gdrive/backend.rs +++ b/core/src/services/gdrive/backend.rs @@ -212,7 +212,11 @@ impl Accessor for GdriveBackend { } async fn copy(&self, from: &str, to: &str, _args: OpCopy) -> Result<RpCopy> { - let from_file_id = self.core.get_file_id_by_path(from).await?; + let from_file_id = self + .core + .get_file_id_by_path(from) + .await? + .ok_or(Error::new(ErrorKind::NotFound, "invalid 'from' path"))?; // split `to` into parent and name according to the last `/` let mut to_path_items: Vec<&str> = to.split('/').filter(|&x| !x.is_empty()).collect(); @@ -225,14 +229,26 @@ impl Accessor for GdriveBackend { let to_parent = to_path_items.join("/") + "/"; - if to_parent != "/" { - self.create_dir(&to_parent, OpCreateDir::new()).await?; - } - - let to_parent_id = self.core.get_file_id_by_path(to_parent.as_str()).await?; + let to_parent_id = + if let Some(id) = self.core.get_file_id_by_path(to_parent.as_str()).await? { + id + } else { + self.create_dir(&to_parent, OpCreateDir::new()).await?; + self.core + .get_file_id_by_path(to_parent.as_str()) + .await? + .ok_or_else(|| { + Error::new(ErrorKind::Unexpected, "create to's parent folder failed") + })? + }; // copy will overwrite `to`, delete it if exist - if self.core.get_file_id_by_path(to).await.is_ok() { + if self + .core + .get_file_id_by_path(to) + .await + .is_ok_and(|id| id.is_some()) + { self.delete(to, OpDelete::new()).await?; } diff --git a/core/src/services/gdrive/core.rs b/core/src/services/gdrive/core.rs index f6517419d..bf52ce2a1 100644 --- a/core/src/services/gdrive/core.rs +++ b/core/src/services/gdrive/core.rs @@ -77,13 +77,13 @@ impl GdriveCore { /// - A path is a sequence of file names separated by slashes. /// - A file only knows its parent id, but not its name. /// - To find the file id of a file, we need to traverse the path from the root to the file. - pub(crate) async fn get_file_id_by_path(&self, file_path: &str) -> Result<String> { + pub(crate) async fn get_file_id_by_path(&self, file_path: &str) -> Result<Option<String>> { let path = build_abs_path(&self.root, file_path); let mut cache = self.path_cache.lock().await; if let Some(id) = cache.get(&path) { - return Ok(id.to_owned()); + return Ok(Some(id.to_owned())); } let mut parent_id = "root".to_owned(); @@ -108,15 +108,11 @@ impl GdriveCore { parent_id = id; cache.insert(path_part, parent_id.clone()); } else { - // TODO: return None instead of error. - return Err(Error::new( - ErrorKind::NotFound, - &format!("path not found: {}", item), - )); + return Ok(None); }; } - Ok(parent_id) + Ok(Some(parent_id)) } /// Ensure the parent path exists. @@ -277,13 +273,16 @@ impl GdriveCore { } pub async fn gdrive_stat(&self, path: &str) -> Result<Response<IncomingAsyncBody>> { - let path_id = self.get_file_id_by_path(path).await?; + let path_id = self.get_file_id_by_path(path).await?.ok_or(Error::new( + ErrorKind::NotFound, + &format!("path not found: {}", path), + ))?; // The file metadata in the Google Drive API is very complex. // For now, we only need the file id, name, mime type and modified time. let mut req = Request::get(&format!( "https://www.googleapis.com/drive/v3/files/{}?fields=id,name,mimeType,size,modifiedTime";, - path_id.as_str() + path_id )) .body(AsyncBody::Empty) .map_err(new_request_build_error)?; @@ -293,9 +292,14 @@ impl GdriveCore { } pub async fn gdrive_get(&self, path: &str) -> Result<Response<IncomingAsyncBody>> { + let path_id = self.get_file_id_by_path(path).await?.ok_or(Error::new( + ErrorKind::NotFound, + &format!("path not found: {}", path), + ))?; + let url: String = format!( "https://www.googleapis.com/drive/v3/files/{}?alt=media";, - self.get_file_id_by_path(path).await? + path_id ); let mut req = Request::get(&url) @@ -314,31 +318,29 @@ impl GdriveCore { ) -> Result<Response<IncomingAsyncBody>> { let file_id = self.get_file_id_by_path(path).await; - // when list over a no exist dir, `get_file_id_by_path` will return a NotFound Error, we should return a empty list in this case. + // when list over a no exist dir, we should return a empty list in this case. let q = match file_id { - Ok(file_id) => { + Ok(Some(file_id)) => { format!("'{}' in parents and trashed = false", file_id) } - Err(e) => match e.kind() { - ErrorKind::NotFound => { - return Response::builder() - .status(StatusCode::OK) - .body(IncomingAsyncBody::new( - Box::new(oio::into_stream(stream::empty())), - Some(0), - )) - .map_err(|e| { - Error::new( - ErrorKind::Unexpected, - &format!("failed to create a empty response for list: {}", e), - ) - .set_source(e) - }); - } - _ => { - return Err(e); - } - }, + Ok(None) => { + return Response::builder() + .status(StatusCode::OK) + .body(IncomingAsyncBody::new( + Box::new(oio::into_stream(stream::empty())), + Some(0), + )) + .map_err(|e| { + Error::new( + ErrorKind::Unexpected, + &format!("failed to create a empty response for list: {}", e), + ) + .set_source(e) + }); + } + Err(e) => { + return Err(e); + } }; let mut url = format!( @@ -364,7 +366,10 @@ impl GdriveCore { source: &str, target: &str, ) -> Result<Response<IncomingAsyncBody>> { - let file_id = self.get_file_id_by_path(source).await?; + let file_id = self.get_file_id_by_path(source).await?.ok_or(Error::new( + ErrorKind::NotFound, + &format!("source path not found: {}", source), + ))?; let parent = self.ensure_parent_path(target).await?; @@ -402,10 +407,11 @@ impl GdriveCore { } pub async fn gdrive_delete(&self, path: &str) -> Result<Response<IncomingAsyncBody>> { - let url = format!( - "https://www.googleapis.com/drive/v3/files/{}";, - self.get_file_id_by_path(path).await? - ); + let file_id = self.get_file_id_by_path(path).await?.ok_or(Error::new( + ErrorKind::NotFound, + &format!("path not found: {}", path), + ))?; + let url = format!("https://www.googleapis.com/drive/v3/files/{}";, file_id); let mut req = Request::delete(&url) .body(AsyncBody::Empty)
