This is an automated email from the ASF dual-hosted git repository.
xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 97974f4f5 fix(services/gdrive): fix return value of
`get_file_id_by_path` (#3801)
97974f4f5 is described below
commit 97974f4f5e85886e8e511150bd5add490f2b921c
Author: G-XD <[email protected]>
AuthorDate: Sat Dec 23 02:09:27 2023 +0800
fix(services/gdrive): fix return value of `get_file_id_by_path` (#3801)
---
core/src/services/gdrive/backend.rs | 30 +++++++++++----
core/src/services/gdrive/core.rs | 73 +++++++++++++------------------------
core/src/services/gdrive/lister.rs | 15 +++++++-
3 files changed, 63 insertions(+), 55 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..f43e80acd 100644
--- a/core/src/services/gdrive/core.rs
+++ b/core/src/services/gdrive/core.rs
@@ -24,7 +24,6 @@ use bytes;
use bytes::Bytes;
use chrono::DateTime;
use chrono::Utc;
-use futures::stream;
use http::header;
use http::Request;
use http::Response;
@@ -77,13 +76,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 +107,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 +272,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 +291,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)
@@ -308,39 +311,11 @@ impl GdriveCore {
pub async fn gdrive_list(
&self,
- path: &str,
+ file_id: &str,
page_size: i32,
next_page_token: &str,
) -> 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.
- let q = match file_id {
- Ok(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);
- }
- },
- };
-
+ let q = format!("'{}' in parents and trashed = false", file_id);
let mut url = format!(
"https://www.googleapis.com/drive/v3/files?pageSize={}&q={}";,
page_size,
@@ -364,7 +339,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 +380,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)
diff --git a/core/src/services/gdrive/lister.rs
b/core/src/services/gdrive/lister.rs
index d3ae48633..b8cf78a8e 100644
--- a/core/src/services/gdrive/lister.rs
+++ b/core/src/services/gdrive/lister.rs
@@ -40,7 +40,20 @@ impl GdriveLister {
#[async_trait]
impl oio::PageList for GdriveLister {
async fn next_page(&self, ctx: &mut oio::PageContext) -> Result<()> {
- let resp = self.core.gdrive_list(&self.path, 100, &ctx.token).await?;
+ let file_id = self.core.get_file_id_by_path(&self.path).await?;
+
+ let file_id = match file_id {
+ Some(file_id) => file_id,
+ None => {
+ ctx.done = true;
+ return Ok(());
+ }
+ };
+
+ let resp = self
+ .core
+ .gdrive_list(file_id.as_str(), 100, &ctx.token)
+ .await?;
let bytes = match resp.status() {
StatusCode::OK => resp.into_body().bytes().await?,
