From 462ece769afc0e44e901baed3d4397e003d96f4b Mon Sep 17 00:00:00 2001 From: suyanhanx Date: Fri, 17 Apr 2026 15:28:16 +0800 Subject: [PATCH 1/2] fix(services/dropbox): preserve list semantics --- core/services/dropbox/src/backend.rs | 124 ++++++++++++++++++++------- core/services/dropbox/src/core.rs | 55 ++++++++++++ core/services/dropbox/src/lister.rs | 110 ++++++++++++++++++------ core/services/dropbox/src/writer.rs | 28 +----- 4 files changed, 235 insertions(+), 82 deletions(-) diff --git a/core/services/dropbox/src/backend.rs b/core/services/dropbox/src/backend.rs index fd25f4de5900..f839e07f9ad0 100644 --- a/core/services/dropbox/src/backend.rs +++ b/core/services/dropbox/src/backend.rs @@ -29,6 +29,27 @@ use super::writer::DropboxWriter; use opendal_core::raw::*; use opendal_core::*; +fn parent_list_path(path: &str) -> String { + let parent = get_parent(path); + if parent == "/" { + String::new() + } else { + parent.to_string() + } +} + +fn dir_entry(path: &str) -> oio::Entry { + let path = if path.is_empty() || path == "/" { + "/".to_string() + } else if path.ends_with('/') { + path.to_string() + } else { + format!("{path}/") + }; + + oio::Entry::new(&path, Metadata::new(EntryMode::DIR)) +} + #[derive(Clone, Debug)] pub struct DropboxBackend { pub core: Arc, @@ -74,30 +95,7 @@ impl Access for DropboxBackend { let bytes = resp.into_body(); let decoded_response: DropboxMetadataResponse = serde_json::from_reader(bytes.reader()).map_err(new_json_deserialize_error)?; - let entry_mode: EntryMode = match decoded_response.tag.as_str() { - "file" => EntryMode::FILE, - "folder" => EntryMode::DIR, - _ => EntryMode::Unknown, - }; - - let mut metadata = Metadata::new(entry_mode); - // Only set last_modified and size if entry_mode is FILE, because Dropbox API - // returns last_modified and size only for files. - // FYI: https://www.dropbox.com/developers/documentation/http/documentation#files-get_metadata - if entry_mode == EntryMode::FILE { - let date_utc_last_modified = - decoded_response.client_modified.parse::()?; - metadata.set_last_modified(date_utc_last_modified); - - if let Some(size) = decoded_response.size { - metadata.set_content_length(size); - } else { - return Err(Error::new( - ErrorKind::Unexpected, - format!("no size found for file {path}"), - )); - } - } + let metadata = decoded_response.parse_metadata()?; Ok(RpStat::new(metadata)) } _ => Err(parse_error(resp)), @@ -139,15 +137,79 @@ impl Access for DropboxBackend { } async fn list(&self, path: &str, args: OpList) -> Result<(RpList, Self::Lister)> { - Ok(( - RpList::default(), - oio::PageLister::new(DropboxLister::new( + let recursive = args.recursive(); + let limit = args.limit(); + + let lister = if path.is_empty() || path == "/" || path.ends_with('/') { + DropboxLister::new( self.core.clone(), path.to_string(), - args.recursive(), - args.limit(), - )), - )) + recursive, + limit, + Some(dir_entry(path)), + None, + true, + ) + } else { + let resp = self.core.dropbox_get_metadata(path).await?; + match resp.status() { + StatusCode::OK => { + let bytes = resp.into_body(); + let decoded_response: DropboxMetadataResponse = + serde_json::from_reader(bytes.reader()) + .map_err(new_json_deserialize_error)?; + + if decoded_response.entry_mode().is_dir() { + DropboxLister::new( + self.core.clone(), + path.to_string(), + recursive, + limit, + Some(dir_entry(path)), + None, + recursive, + ) + } else if recursive { + DropboxLister::new( + self.core.clone(), + parent_list_path(path), + true, + limit, + None, + Some(path.to_string()), + true, + ) + } else { + DropboxLister::new( + self.core.clone(), + String::new(), + false, + limit, + Some(oio::Entry::new(path, decoded_response.parse_metadata()?)), + None, + false, + ) + } + } + _ => { + let err = parse_error(resp); + match err.kind() { + ErrorKind::NotFound => DropboxLister::new( + self.core.clone(), + parent_list_path(path), + recursive, + limit, + None, + Some(path.to_string()), + true, + ), + _ => return Err(err), + } + } + } + }; + + Ok((RpList::default(), oio::PageLister::new(lister))) } async fn copy(&self, from: &str, to: &str, _: OpCopy) -> Result { diff --git a/core/services/dropbox/src/core.rs b/core/services/dropbox/src/core.rs index b716cb923558..1aa9b1b2ba1f 100644 --- a/core/services/dropbox/src/core.rs +++ b/core/services/dropbox/src/core.rs @@ -448,6 +448,61 @@ pub struct DropboxMetadataResponse { pub size: Option, } +impl DropboxMetadataResponse { + pub(crate) fn entry_mode(&self) -> EntryMode { + match self.tag.as_str() { + "file" => EntryMode::FILE, + "folder" => EntryMode::DIR, + _ => EntryMode::Unknown, + } + } + + pub(crate) fn parse_metadata(&self) -> Result { + let entry_mode = self.entry_mode(); + let mut metadata = Metadata::new(entry_mode); + + if let Some(content_hash) = self.content_hash.as_deref() { + metadata.set_etag(content_hash); + } + if let Some(rev) = self.rev.as_deref() { + metadata.set_version(rev); + } + + if entry_mode == EntryMode::FILE { + let modified = self + .server_modified + .as_deref() + .unwrap_or(self.client_modified.as_str()); + metadata.set_last_modified(modified.parse::()?); + + if let Some(size) = self.size { + metadata.set_content_length(size); + } else { + return Err(Error::new( + ErrorKind::Unexpected, + format!("no size found for file {}", self.path_display), + )); + } + } + + Ok(metadata) + } + + pub(crate) fn entry_path(&self, root: &str) -> String { + let mut path = if self.path_display == root { + "/".to_string() + } else { + build_rel_path(root, &self.path_display) + }; + + if self.entry_mode().is_dir() && path != "/" && !path.ends_with('/') { + path.push('/'); + } + + path + } +} + #[derive(Default, Debug, Deserialize)] #[serde(default)] pub struct DropboxMetadataFileLockInfo { diff --git a/core/services/dropbox/src/lister.rs b/core/services/dropbox/src/lister.rs index bfdc07a27aa7..ad067b2d36bb 100644 --- a/core/services/dropbox/src/lister.rs +++ b/core/services/dropbox/src/lister.rs @@ -26,29 +26,55 @@ use opendal_core::*; pub struct DropboxLister { core: Arc, - path: String, + list_path: String, recursive: bool, limit: Option, + self_entry: Option, + filter_prefix: Option, + fetch_entries: bool, } impl DropboxLister { pub fn new( core: Arc, - path: String, + list_path: String, recursive: bool, limit: Option, + self_entry: Option, + filter_prefix: Option, + fetch_entries: bool, ) -> Self { Self { core, - path, + list_path, recursive, limit, + self_entry, + filter_prefix, + fetch_entries, } } + + fn build_entry(&self, entry: DropboxMetadataResponse) -> Result { + let path = entry.entry_path(self.core.root.as_str()); + let metadata = entry.parse_metadata()?; + Ok(oio::Entry::new(&path, metadata)) + } } impl oio::PageList for DropboxLister { async fn next_page(&self, ctx: &mut oio::PageContext) -> Result<()> { + if ctx.token.is_empty() { + if let Some(entry) = self.self_entry.clone() { + ctx.entries.push_back(entry); + } + + if !self.fetch_entries { + ctx.done = true; + return Ok(()); + } + } + // The token is set when obtaining entries and returning `has_more` flag. // When the token exists, we should retrieve more entries using the Dropbox continue API. // Refer: https://www.dropbox.com/developers/documentation/http/documentation#files-list_folder-continue @@ -56,7 +82,7 @@ impl oio::PageList for DropboxLister { self.core.dropbox_list_continue(&ctx.token).await? } else { self.core - .dropbox_list(&self.path, self.recursive, self.limit) + .dropbox_list(&self.list_path, self.recursive, self.limit) .await? }; @@ -79,31 +105,20 @@ impl oio::PageList for DropboxLister { serde_json::from_reader(bytes.reader()).map_err(new_json_deserialize_error)?; for entry in decoded_response.entries { - let entry_mode = match entry.tag.as_str() { - "file" => EntryMode::FILE, - "folder" => EntryMode::DIR, - _ => EntryMode::Unknown, - }; - - let mut name = entry.name; - let mut meta = Metadata::new(entry_mode); + let entry = self.build_entry(entry)?; - // Dropbox will return folder names that do not end with '/'. - if entry_mode == EntryMode::DIR && !name.ends_with('/') { - name.push('/'); + if let Some(prefix) = self.filter_prefix.as_deref() { + if !entry.path().starts_with(prefix) { + continue; + } } - - // The behavior here aligns with Dropbox's stat function. - if entry_mode == EntryMode::FILE { - let date_utc_last_modified = entry.client_modified.parse::()?; - meta.set_last_modified(date_utc_last_modified); - - if let Some(size) = entry.size { - meta.set_content_length(size); + if let Some(self_entry) = self.self_entry.as_ref() { + if self_entry.path() == entry.path() { + continue; } } - ctx.entries.push_back(oio::Entry::with(name, meta)); + ctx.entries.push_back(entry); } if decoded_response.has_more { @@ -115,3 +130,50 @@ impl oio::PageList for DropboxLister { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_entry_uses_full_relative_path_and_metadata() { + let entry = DropboxMetadataResponse { + tag: "file".to_string(), + client_modified: "2024-01-01T00:00:00Z".to_string(), + content_hash: Some("etag".to_string()), + path_display: "/workspace/nested/file.txt".to_string(), + rev: Some("version".to_string()), + server_modified: Some("2024-01-02T00:00:00Z".to_string()), + size: Some(7), + ..Default::default() + }; + + let path = entry.entry_path("/workspace/"); + let metadata = entry.parse_metadata().expect("metadata must parse"); + + assert_eq!(path, "nested/file.txt"); + assert_eq!(metadata.mode(), EntryMode::FILE); + assert_eq!(metadata.content_length(), 7); + assert_eq!(metadata.etag(), Some("etag")); + assert_eq!(metadata.version(), Some("version")); + assert_eq!( + metadata.last_modified(), + Some("2024-01-02T00:00:00Z".parse::().unwrap()) + ); + } + + #[test] + fn build_entry_preserves_directory_hierarchy() { + let entry = DropboxMetadataResponse { + tag: "folder".to_string(), + path_display: "/workspace/a/b".to_string(), + ..Default::default() + }; + + assert_eq!(entry.entry_path("/workspace/"), "a/b/"); + assert_eq!( + entry.parse_metadata().expect("metadata must parse").mode(), + EntryMode::DIR + ); + } +} diff --git a/core/services/dropbox/src/writer.rs b/core/services/dropbox/src/writer.rs index cf7b898bf2bf..2740f9bffded 100644 --- a/core/services/dropbox/src/writer.rs +++ b/core/services/dropbox/src/writer.rs @@ -35,32 +35,6 @@ impl DropboxWriter { pub fn new(core: Arc, op: OpWrite, path: String) -> Self { DropboxWriter { core, op, path } } - - fn parse_metadata(decoded_response: DropboxMetadataResponse) -> Result { - let mut metadata = Metadata::default(); - - if let Some(size) = decoded_response.size { - metadata.set_content_length(size); - } - - if let Some(content_hash) = decoded_response.content_hash { - metadata.set_etag(&content_hash); - } - - if let Some(rev) = decoded_response.rev { - metadata.set_version(&rev); - } - - if let Some(server_modified) = decoded_response.server_modified { - let date_utc = server_modified.parse::()?; - metadata.set_last_modified(date_utc); - } else { - let date_utc = decoded_response.client_modified.parse::()?; - metadata.set_last_modified(date_utc); - } - - Ok(metadata) - } } impl oio::OneShotWrite for DropboxWriter { @@ -75,7 +49,7 @@ impl oio::OneShotWrite for DropboxWriter { let bytes = resp.into_body(); let decoded_response: DropboxMetadataResponse = serde_json::from_reader(bytes.reader()).map_err(new_json_deserialize_error)?; - let metadata = DropboxWriter::parse_metadata(decoded_response)?; + let metadata = decoded_response.parse_metadata()?; Ok(metadata) } _ => Err(parse_error(resp)), From 1702c4a0f4bd3ea20312a6a18e5f98e2aea1c6a2 Mon Sep 17 00:00:00 2001 From: suyanhanx Date: Fri, 17 Apr 2026 17:06:23 +0800 Subject: [PATCH 2/2] refactor(services/dropbox): move helpers and drop lister tests --- core/services/dropbox/src/backend.rs | 42 ++++++++++++------------- core/services/dropbox/src/lister.rs | 47 ---------------------------- 2 files changed, 21 insertions(+), 68 deletions(-) diff --git a/core/services/dropbox/src/backend.rs b/core/services/dropbox/src/backend.rs index f839e07f9ad0..b311b1b84808 100644 --- a/core/services/dropbox/src/backend.rs +++ b/core/services/dropbox/src/backend.rs @@ -29,27 +29,6 @@ use super::writer::DropboxWriter; use opendal_core::raw::*; use opendal_core::*; -fn parent_list_path(path: &str) -> String { - let parent = get_parent(path); - if parent == "/" { - String::new() - } else { - parent.to_string() - } -} - -fn dir_entry(path: &str) -> oio::Entry { - let path = if path.is_empty() || path == "/" { - "/".to_string() - } else if path.ends_with('/') { - path.to_string() - } else { - format!("{path}/") - }; - - oio::Entry::new(&path, Metadata::new(EntryMode::DIR)) -} - #[derive(Clone, Debug)] pub struct DropboxBackend { pub core: Arc, @@ -246,3 +225,24 @@ impl Access for DropboxBackend { } } } + +fn parent_list_path(path: &str) -> String { + let parent = get_parent(path); + if parent == "/" { + String::new() + } else { + parent.to_string() + } +} + +fn dir_entry(path: &str) -> oio::Entry { + let path = if path.is_empty() || path == "/" { + "/".to_string() + } else if path.ends_with('/') { + path.to_string() + } else { + format!("{path}/") + }; + + oio::Entry::new(&path, Metadata::new(EntryMode::DIR)) +} diff --git a/core/services/dropbox/src/lister.rs b/core/services/dropbox/src/lister.rs index ad067b2d36bb..9d3b67dd7101 100644 --- a/core/services/dropbox/src/lister.rs +++ b/core/services/dropbox/src/lister.rs @@ -130,50 +130,3 @@ impl oio::PageList for DropboxLister { Ok(()) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn build_entry_uses_full_relative_path_and_metadata() { - let entry = DropboxMetadataResponse { - tag: "file".to_string(), - client_modified: "2024-01-01T00:00:00Z".to_string(), - content_hash: Some("etag".to_string()), - path_display: "/workspace/nested/file.txt".to_string(), - rev: Some("version".to_string()), - server_modified: Some("2024-01-02T00:00:00Z".to_string()), - size: Some(7), - ..Default::default() - }; - - let path = entry.entry_path("/workspace/"); - let metadata = entry.parse_metadata().expect("metadata must parse"); - - assert_eq!(path, "nested/file.txt"); - assert_eq!(metadata.mode(), EntryMode::FILE); - assert_eq!(metadata.content_length(), 7); - assert_eq!(metadata.etag(), Some("etag")); - assert_eq!(metadata.version(), Some("version")); - assert_eq!( - metadata.last_modified(), - Some("2024-01-02T00:00:00Z".parse::().unwrap()) - ); - } - - #[test] - fn build_entry_preserves_directory_hierarchy() { - let entry = DropboxMetadataResponse { - tag: "folder".to_string(), - path_display: "/workspace/a/b".to_string(), - ..Default::default() - }; - - assert_eq!(entry.entry_path("/workspace/"), "a/b/"); - assert_eq!( - entry.parse_metadata().expect("metadata must parse").mode(), - EntryMode::DIR - ); - } -}