You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by su...@apache.org on 2026/04/17 07:28:43 UTC

(opendal) branch fix/dropbox-list-semantics created (now 462ece769)

This is an automated email from the ASF dual-hosted git repository.

suyanhanx pushed a change to branch fix/dropbox-list-semantics
in repository https://gitbox.apache.org/repos/asf/opendal.git


      at 462ece769 fix(services/dropbox): preserve list semantics

This branch includes the following new commits:

     new 462ece769 fix(services/dropbox): preserve list semantics

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(opendal) 01/01: fix(services/dropbox): preserve list semantics

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

suyanhanx pushed a commit to branch fix/dropbox-list-semantics
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit 462ece769afc0e44e901baed3d4397e003d96f4b
Author: suyanhanx <su...@gmail.com>
AuthorDate: Fri Apr 17 15:28:16 2026 +0800

    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 fd25f4de5..f839e07f9 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<DropboxCore>,
@@ -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::<Timestamp>()?;
-                    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<RpCopy> {
diff --git a/core/services/dropbox/src/core.rs b/core/services/dropbox/src/core.rs
index b716cb923..1aa9b1b2b 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<u64>,
 }
 
+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<Metadata> {
+        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::<Timestamp>()?);
+
+            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 bfdc07a27..ad067b2d3 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<DropboxCore>,
-    path: String,
+    list_path: String,
     recursive: bool,
     limit: Option<usize>,
+    self_entry: Option<oio::Entry>,
+    filter_prefix: Option<String>,
+    fetch_entries: bool,
 }
 
 impl DropboxLister {
     pub fn new(
         core: Arc<DropboxCore>,
-        path: String,
+        list_path: String,
         recursive: bool,
         limit: Option<usize>,
+        self_entry: Option<oio::Entry>,
+        filter_prefix: Option<String>,
+        fetch_entries: bool,
     ) -> Self {
         Self {
             core,
-            path,
+            list_path,
             recursive,
             limit,
+            self_entry,
+            filter_prefix,
+            fetch_entries,
         }
     }
+
+    fn build_entry(&self, entry: DropboxMetadataResponse) -> Result<oio::Entry> {
+        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::<Timestamp>()?;
-                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::<Timestamp>().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 cf7b898bf..2740f9bff 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<DropboxCore>, op: OpWrite, path: String) -> Self {
         DropboxWriter { core, op, path }
     }
-
-    fn parse_metadata(decoded_response: DropboxMetadataResponse) -> Result<Metadata> {
-        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::<Timestamp>()?;
-            metadata.set_last_modified(date_utc);
-        } else {
-            let date_utc = decoded_response.client_modified.parse::<Timestamp>()?;
-            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)),