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/16 14:38:48 UTC

(opendal) branch fix/onedrive-api-alignment created (now ac9b72df7)

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

suyanhanx pushed a change to branch fix/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git


      at ac9b72df7 build(core): refresh lockfile

This branch includes the following new commits:

     new e4117c5d0 fix(services/onedrive): align versioning semantics
     new 6384aaade fix(services/onedrive): align primitive semantics
     new 7e4d62d1e test(services/onedrive): cover recursive ensure dir
     new f1eb34298 test(services/onedrive): drop ensure dir unit test
     new ac9b72df7 build(core): refresh lockfile

The 5 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) 03/05: test(services/onedrive): cover recursive ensure dir

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

suyanhanx pushed a commit to branch fix/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit 7e4d62d1e3a70bddf79f6f0b7cf237e8535a6ee9
Author: suyanhanx <su...@gmail.com>
AuthorDate: Thu Apr 16 22:30:06 2026 +0800

    test(services/onedrive): cover recursive ensure dir
---
 core/Cargo.lock                    |   1 +
 core/services/onedrive/Cargo.toml  |   1 +
 core/services/onedrive/src/core.rs | 156 +++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+)

diff --git a/core/Cargo.lock b/core/Cargo.lock
index 457528b48..6c71e05c2 100644
--- a/core/Cargo.lock
+++ b/core/Cargo.lock
@@ -7047,6 +7047,7 @@ name = "opendal-service-onedrive"
 version = "0.55.0"
 dependencies = [
  "bytes",
+ "futures",
  "http 1.4.0",
  "log",
  "mea",
diff --git a/core/services/onedrive/Cargo.toml b/core/services/onedrive/Cargo.toml
index 1a5f04c70..b10724b90 100644
--- a/core/services/onedrive/Cargo.toml
+++ b/core/services/onedrive/Cargo.toml
@@ -41,4 +41,5 @@ serde_json = { workspace = true }
 tokio = { workspace = true, features = ["time"] }
 
 [dev-dependencies]
+futures = { workspace = true }
 tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
diff --git a/core/services/onedrive/src/core.rs b/core/services/onedrive/src/core.rs
index 17df75a19..0df3f111a 100644
--- a/core/services/onedrive/src/core.rs
+++ b/core/services/onedrive/src/core.rs
@@ -719,3 +719,159 @@ impl OneDriveSigner {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ONEDRIVE_SCHEME;
+    use std::collections::VecDeque;
+    use std::sync::Mutex as StdMutex;
+
+    use bytes::Bytes;
+    use futures::stream;
+
+    use super::*;
+
+    #[derive(Clone)]
+    struct MockHttpClient {
+        requests: Arc<StdMutex<Vec<String>>>,
+        responses: Arc<StdMutex<VecDeque<(StatusCode, &'static str)>>>,
+    }
+
+    impl MockHttpClient {
+        fn new(responses: Vec<(StatusCode, &'static str)>) -> Self {
+            Self {
+                requests: Arc::new(StdMutex::new(Vec::new())),
+                responses: Arc::new(StdMutex::new(VecDeque::from(responses))),
+            }
+        }
+
+        fn requests(&self) -> Vec<String> {
+            self.requests.lock().expect("lock poisoned").clone()
+        }
+    }
+
+    impl HttpFetch for MockHttpClient {
+        async fn fetch(&self, req: Request<Buffer>) -> Result<Response<HttpBody>> {
+            self.requests.lock().expect("lock poisoned").push(format!(
+                "{} {}",
+                req.method(),
+                req.uri()
+            ));
+
+            let (status, body) = self
+                .responses
+                .lock()
+                .expect("lock poisoned")
+                .pop_front()
+                .expect("response queue must not be empty");
+            let bs = Bytes::from(body.to_string());
+            let size = bs.len() as u64;
+            let body = HttpBody::new(stream::iter(vec![Ok(Buffer::from(bs))]), Some(size));
+
+            Ok(Response::builder().status(status).body(body).unwrap())
+        }
+    }
+
+    fn folder_item_json(name: &str, parent_path: &str, id: &str) -> String {
+        format!(
+            r#"{{
+                "id": "{id}",
+                "name": "{name}",
+                "lastModifiedDateTime": "2025-02-16T19:49:05Z",
+                "eTag": "\"etag-{id}\"",
+                "size": 0,
+                "parentReference": {{
+                    "driveId": "drive-id",
+                    "id": "parent-{id}",
+                    "path": "{parent_path}"
+                }},
+                "folder": {{
+                    "childCount": 0
+                }}
+            }}"#
+        )
+    }
+
+    fn create_test_core(mock: MockHttpClient) -> OneDriveCore {
+        let info = AccessorInfo::default();
+        info.set_scheme(ONEDRIVE_SCHEME).set_root("/");
+        info.update_http_client(|_| HttpClient::with(mock));
+
+        let info = Arc::new(info);
+        let mut signer = OneDriveSigner::new(info.clone());
+        signer.access_token = "access_token".to_string();
+        signer.expires_in = Timestamp::MAX;
+
+        OneDriveCore {
+            info,
+            root: "/".to_string(),
+            signer: Arc::new(Mutex::new(signer)),
+        }
+    }
+
+    #[tokio::test]
+    async fn test_ensure_directory_creates_missing_ancestors() {
+        let root = Box::leak(folder_item_json("root", "/drive/root:", "root").into_boxed_str());
+        let a = Box::leak(folder_item_json("a", "/drive/root:", "a").into_boxed_str());
+        let ab = Box::leak(folder_item_json("b", "/drive/root:/a", "ab").into_boxed_str());
+        let abc = Box::leak(folder_item_json("c", "/drive/root:/a/b", "abc").into_boxed_str());
+
+        let mock = MockHttpClient::new(vec![
+            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
+            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
+            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
+            (StatusCode::OK, root),
+            (StatusCode::CREATED, a),
+            (StatusCode::CREATED, ab),
+            (StatusCode::CREATED, abc),
+        ]);
+        let core = create_test_core(mock.clone());
+
+        let item = core
+            .ensure_directory("a/b/c/")
+            .await
+            .expect("ensure directory must succeed");
+
+        assert_eq!(item.id, "abc");
+        assert_eq!(
+            mock.requests(),
+            vec![
+                format!(
+                    "GET {}:/a/b/c?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "GET {}:/a/b?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "GET {}:/a?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "GET {}?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "POST {}:/children?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "POST {}:/a:/children?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+                format!(
+                    "POST {}:/a/b:/children?{}",
+                    OneDriveCore::DRIVE_ROOT_URL,
+                    GENERAL_SELECT_PARAM
+                ),
+            ]
+        );
+    }
+}


(opendal) 02/05: fix(services/onedrive): align primitive 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/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit 6384aaade783dfe16a20f8604fdda8d545a63b25
Author: suyanhanx <su...@gmail.com>
AuthorDate: Thu Apr 16 12:10:06 2026 +0800

    fix(services/onedrive): align primitive semantics
---
 core/services/onedrive/src/backend.rs | 15 ++++--
 core/services/onedrive/src/core.rs    | 98 +++++++++++++++++++++--------------
 core/services/onedrive/src/deleter.rs |  9 +++-
 core/services/onedrive/src/lister.rs  | 19 +++----
 core/services/onedrive/src/writer.rs  |  7 +++
 5 files changed, 90 insertions(+), 58 deletions(-)

diff --git a/core/services/onedrive/src/backend.rs b/core/services/onedrive/src/backend.rs
index e6ecb8150..6fbcffa40 100644
--- a/core/services/onedrive/src/backend.rs
+++ b/core/services/onedrive/src/backend.rs
@@ -50,11 +50,9 @@ impl Access for OnedriveBackend {
             return Ok(RpCreateDir::default());
         }
 
-        let response = self.core.onedrive_create_dir(path).await?;
-        match response.status() {
-            StatusCode::CREATED | StatusCode::OK => Ok(RpCreateDir::default()),
-            _ => Err(parse_error(response)),
-        }
+        self.core.ensure_directory(path).await?;
+
+        Ok(RpCreateDir::default())
     }
 
     async fn stat(&self, path: &str, args: OpStat) -> Result<RpStat> {
@@ -96,6 +94,13 @@ impl Access for OnedriveBackend {
     }
 
     async fn copy(&self, from: &str, to: &str, _args: OpCopy) -> Result<RpCopy> {
+        if _args.if_not_exists() {
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "copy with if_not_exists is not supported by OneDrive",
+            ));
+        }
+
         let monitor_url = self.core.initialize_copy(from, to).await?;
         self.core.wait_until_complete(monitor_url).await?;
         Ok(RpCopy::default())
diff --git a/core/services/onedrive/src/core.rs b/core/services/onedrive/src/core.rs
index 2a7d4ac92..17df75a19 100644
--- a/core/services/onedrive/src/core.rs
+++ b/core/services/onedrive/src/core.rs
@@ -108,28 +108,55 @@ impl OneDriveCore {
     ///
     /// * `path` - a relative folder path
     pub(crate) async fn ensure_directory(&self, path: &str) -> Result<OneDriveItem> {
-        let response = self.onedrive_get_stat_plain(path).await?;
-        let item: OneDriveItem = match response.status() {
-            StatusCode::OK => {
-                let bytes = response.into_body();
-                serde_json::from_reader(bytes.reader()).map_err(new_json_deserialize_error)?
-            }
-            StatusCode::NOT_FOUND => {
-                // We must create directory for the destination
-                let response = self.onedrive_create_dir(path).await?;
-                match response.status() {
-                    StatusCode::CREATED | StatusCode::OK => {
-                        let bytes = response.into_body();
-                        serde_json::from_reader(bytes.reader())
-                            .map_err(new_json_deserialize_error)?
+        let mut pending: Vec<String> = Vec::new();
+        let mut current = path;
+
+        loop {
+            let response = self.onedrive_get_stat_plain(current).await?;
+            match response.status() {
+                StatusCode::OK => {
+                    let bytes = response.into_body();
+                    let item: OneDriveItem = serde_json::from_reader(bytes.reader())
+                        .map_err(new_json_deserialize_error)?;
+
+                    if !matches!(item.item_type, ItemType::Folder { .. }) {
+                        return Err(Error::new(
+                            ErrorKind::NotADirectory,
+                            "path exists but is not a directory",
+                        )
+                        .with_context("path", current));
                     }
-                    _ => return Err(parse_error(response)),
+
+                    let mut ensured = item;
+                    for missing in pending.into_iter().rev() {
+                        let response = self.onedrive_create_dir(&missing).await?;
+                        ensured = match response.status() {
+                            StatusCode::CREATED | StatusCode::OK => {
+                                let bytes = response.into_body();
+                                serde_json::from_reader(bytes.reader())
+                                    .map_err(new_json_deserialize_error)?
+                            }
+                            _ => return Err(parse_error(response)),
+                        };
+                    }
+
+                    return Ok(ensured);
+                }
+                StatusCode::NOT_FOUND => {
+                    pending.push(current.to_string());
+                    let parent = get_parent(current);
+                    if parent == current {
+                        return Err(Error::new(
+                            ErrorKind::Unexpected,
+                            "failed to resolve parent while creating directory",
+                        )
+                        .with_context("path", path));
+                    }
+                    current = parent;
                 }
+                _ => return Err(parse_error(response)),
             }
-            _ => return Err(parse_error(response)),
-        };
-
-        Ok(item)
+        }
     }
 
     pub(crate) async fn sign<T>(&self, request: &mut Request<T>) -> Result<()> {
@@ -158,13 +185,15 @@ impl OneDriveCore {
     ///
     /// See also [`onedrive_get_stat_plain()`].
     pub(crate) async fn onedrive_stat(&self, path: &str, args: OpStat) -> Result<Metadata> {
-        let mut url: String = self.onedrive_item_url(path, true);
         if args.version().is_some() {
-            url += "?$expand=versions(";
-            url += VERSION_SELECT_PARAM;
-            url += ")";
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "stat with version is not supported by OneDrive",
+            ));
         }
 
+        let url: String = self.onedrive_item_url(path, true);
+
         let mut request = Request::get(&url);
         if let Some(etag) = args.if_none_match() {
             request = request.header(header::IF_NONE_MATCH, etag);
@@ -195,22 +224,6 @@ impl OneDriveCore {
             .with_etag(decoded_response.e_tag)
             .with_content_length(decoded_response.size.max(0) as u64);
 
-        if let Some(version) = args.version() {
-            for item_version in decoded_response.versions.as_deref().unwrap_or_default() {
-                if item_version.id == version {
-                    meta.set_version(version);
-                    break; // early exit
-                }
-            }
-
-            if meta.version().is_none() {
-                return Err(Error::new(
-                    ErrorKind::NotFound,
-                    "cannot find this version of the item",
-                ));
-            }
-        }
-
         let last_modified = decoded_response.last_modified_date_time;
         let date_utc_last_modified = last_modified.parse::<Timestamp>()?;
         meta.set_last_modified(date_utc_last_modified);
@@ -277,6 +290,13 @@ impl OneDriveCore {
         path: &str,
         args: &OpRead,
     ) -> Result<Response<HttpBody>> {
+        if args.version().is_some() {
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "read with version is not supported by OneDrive",
+            ));
+        }
+
         // We can't "select" the OneDrive API response fields when reading because "select" shadows not found error
         let url: String = format!("{}:/content", self.onedrive_item_url(path, true));
 
diff --git a/core/services/onedrive/src/deleter.rs b/core/services/onedrive/src/deleter.rs
index 30e477021..6671115f3 100644
--- a/core/services/onedrive/src/deleter.rs
+++ b/core/services/onedrive/src/deleter.rs
@@ -38,7 +38,14 @@ impl OneDriveDeleter {
 }
 
 impl oio::OneShotDelete for OneDriveDeleter {
-    async fn delete_once(&self, path: String, _: OpDelete) -> Result<()> {
+    async fn delete_once(&self, path: String, args: OpDelete) -> Result<()> {
+        if args.version().is_some() {
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "delete with version is not supported by OneDrive",
+            ));
+        }
+
         let response = self.core.onedrive_delete(&path).await?;
         match response.status() {
             StatusCode::NO_CONTENT | StatusCode::NOT_FOUND => Ok(()),
diff --git a/core/services/onedrive/src/lister.rs b/core/services/onedrive/src/lister.rs
index 64e2a248b..001e0e0e3 100644
--- a/core/services/onedrive/src/lister.rs
+++ b/core/services/onedrive/src/lister.rs
@@ -135,25 +135,18 @@ impl oio::PageList for OneDriveLister {
             // N+1 is horrendous but we can't do any better without OneDrive's API support.
             // When OneDrive supports listing with versions API, remove this.
             if list_with_versions && entry_mode == EntryMode::FILE {
-                let versions = self.core.onedrive_list_versions(&path).await?;
-                if versions.is_empty() {
-                    let entry = oio::Entry::new(&normalized_path, meta);
-                    ctx.entries.push_back(entry);
-                    continue;
-                }
+                meta.set_is_current(true);
+                let current_entry = oio::Entry::new(&normalized_path, meta);
+                ctx.entries.push_back(current_entry);
 
-                for (idx, version) in versions.into_iter().enumerate() {
+                let versions = self.core.onedrive_list_versions(&path).await?;
+                for version in versions {
                     let mut version_meta =
                         Metadata::new(entry_mode).with_content_length(version.size.max(0) as u64);
                     version_meta
                         .set_last_modified(version.last_modified_date_time.parse::<Timestamp>()?);
                     version_meta.set_version(&version.id);
-                    version_meta.set_is_current(idx == 0);
-
-                    // OneDrive only exposes the current item's etag from the children API.
-                    if idx == 0 {
-                        version_meta.set_etag(meta.etag().unwrap_or_default());
-                    }
+                    version_meta.set_is_current(false);
 
                     let entry = oio::Entry::new(&normalized_path, version_meta);
                     ctx.entries.push_back(entry);
diff --git a/core/services/onedrive/src/writer.rs b/core/services/onedrive/src/writer.rs
index eefd16558..5e6c3fb7a 100644
--- a/core/services/onedrive/src/writer.rs
+++ b/core/services/onedrive/src/writer.rs
@@ -52,6 +52,13 @@ impl OneDriveWriter {
 // use `OneShotWrite` instead of `MultipartWrite`.
 impl oio::OneShotWrite for OneDriveWriter {
     async fn write_once(&self, bs: Buffer) -> Result<Metadata> {
+        if self.op.if_none_match().is_some() || self.op.if_not_exists() {
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "write with if_none_match/if_not_exists is not supported by OneDrive",
+            ));
+        }
+
         let size = bs.len();
 
         let meta = if size <= Self::MAX_SIMPLE_SIZE {


(opendal) 01/05: fix(services/onedrive): align versioning 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/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit e4117c5d044fbc57ebef805bb08018730e887a51
Author: suyanhanx <su...@gmail.com>
AuthorDate: Thu Apr 16 11:59:55 2026 +0800

    fix(services/onedrive): align versioning semantics
---
 core/services/onedrive/src/builder.rs |  4 ++-
 core/services/onedrive/src/core.rs    | 23 +++++++++++----
 core/services/onedrive/src/docs.md    |  5 +++-
 core/services/onedrive/src/lister.rs  | 31 ++++++++++++++++----
 core/services/onedrive/src/writer.rs  | 54 ++++++++++++++---------------------
 5 files changed, 72 insertions(+), 45 deletions(-)

diff --git a/core/services/onedrive/src/builder.rs b/core/services/onedrive/src/builder.rs
index 8d2371a71..96ec400a3 100644
--- a/core/services/onedrive/src/builder.rs
+++ b/core/services/onedrive/src/builder.rs
@@ -132,7 +132,9 @@ impl Builder for OnedriveBuilder {
 
                 stat: true,
                 stat_with_if_none_match: true,
-                stat_with_version: self.config.enable_versioning,
+                // OneDrive's version API doesn't preserve complete metadata for historical versions,
+                // so we only advertise list-based version discovery for now.
+                stat_with_version: false,
 
                 delete: true,
                 create_dir: true,
diff --git a/core/services/onedrive/src/core.rs b/core/services/onedrive/src/core.rs
index aa85145f6..2a7d4ac92 100644
--- a/core/services/onedrive/src/core.rs
+++ b/core/services/onedrive/src/core.rs
@@ -242,6 +242,10 @@ impl OneDriveCore {
         self.sign(&mut request).await?;
 
         let response = self.info.http_client().send(request).await?;
+        if !response.status().is_success() {
+            return Err(parse_error(response));
+        }
+
         let decoded_response: GraphApiOneDriveVersionsResponse =
             serde_json::from_reader(response.into_body().reader())
                 .map_err(new_json_deserialize_error)?;
@@ -528,20 +532,29 @@ impl OneDriveCore {
 
     pub(crate) async fn wait_until_complete(&self, monitor_url: String) -> Result<()> {
         for _attempt in 0..MAX_MONITOR_ATTEMPT {
-            let mut request = Request::get(monitor_url.to_string())
+            let request = Request::get(monitor_url.as_str())
                 .header(header::CONTENT_TYPE, "application/json")
                 .extension(Operation::Copy)
                 .body(Buffer::new())
                 .map_err(new_request_build_error)?;
 
-            self.sign(&mut request).await?;
-
             let response = self.info.http_client().send(request).await?;
+            if !response.status().is_success() {
+                return Err(parse_error(response));
+            }
+
             let status: OneDriveMonitorStatus =
                 serde_json::from_reader(response.into_body().reader())
                     .map_err(new_json_deserialize_error)?;
-            if status.status == "completed" {
-                return Ok(());
+            match status.status.as_str() {
+                "completed" => return Ok(()),
+                "notStarted" | "inProgress" => {}
+                _ => {
+                    return Err(Error::new(
+                        ErrorKind::Unexpected,
+                        format!("OneDrive copy operation returned status {}", status.status),
+                    ));
+                }
             }
 
             tokio::time::sleep(Duration::from_secs(MONITOR_WAIT_SECOND)).await;
diff --git a/core/services/onedrive/src/docs.md b/core/services/onedrive/src/docs.md
index c7a2786d6..b432b35f9 100644
--- a/core/services/onedrive/src/docs.md
+++ b/core/services/onedrive/src/docs.md
@@ -47,7 +47,10 @@ You should consider [`RetryLayer`] and monitor your operations carefully.
 - `client_id`: Set the client ID for a Microsoft Graph API application (available though Azure's registration portal)
 - `client_secret`: Set the client secret for a Microsoft Graph API application
 - `root`: Set the work directory for OneDrive backend
-- `enable_versioning`: Enable versioning support for OneDrive items
+- `enable_versioning`: Enable version listing support for OneDrive items
+
+Historical OneDrive versions don't preserve the full item metadata surface, so OpenDAL only
+advertises `list_with_versions` when versioning is enabled.
 
 The configuration for tokens is one of the following:
 
diff --git a/core/services/onedrive/src/lister.rs b/core/services/onedrive/src/lister.rs
index 98d3abd53..64e2a248b 100644
--- a/core/services/onedrive/src/lister.rs
+++ b/core/services/onedrive/src/lister.rs
@@ -134,15 +134,34 @@ impl oio::PageList for OneDriveLister {
             // Thus, `list_with_versions` induces N+1 requests. This N+1 is intentional.
             // N+1 is horrendous but we can't do any better without OneDrive's API support.
             // When OneDrive supports listing with versions API, remove this.
-            if list_with_versions {
+            if list_with_versions && entry_mode == EntryMode::FILE {
                 let versions = self.core.onedrive_list_versions(&path).await?;
-                if let Some(version) = versions.first() {
-                    meta.set_version(&version.id);
+                if versions.is_empty() {
+                    let entry = oio::Entry::new(&normalized_path, meta);
+                    ctx.entries.push_back(entry);
+                    continue;
                 }
-            }
 
-            let entry = oio::Entry::new(&normalized_path, meta);
-            ctx.entries.push_back(entry)
+                for (idx, version) in versions.into_iter().enumerate() {
+                    let mut version_meta =
+                        Metadata::new(entry_mode).with_content_length(version.size.max(0) as u64);
+                    version_meta
+                        .set_last_modified(version.last_modified_date_time.parse::<Timestamp>()?);
+                    version_meta.set_version(&version.id);
+                    version_meta.set_is_current(idx == 0);
+
+                    // OneDrive only exposes the current item's etag from the children API.
+                    if idx == 0 {
+                        version_meta.set_etag(meta.etag().unwrap_or_default());
+                    }
+
+                    let entry = oio::Entry::new(&normalized_path, version_meta);
+                    ctx.entries.push_back(entry);
+                }
+            } else {
+                let entry = oio::Entry::new(&normalized_path, meta);
+                ctx.entries.push_back(entry)
+            }
         }
 
         Ok(())
diff --git a/core/services/onedrive/src/writer.rs b/core/services/onedrive/src/writer.rs
index adf2364b1..eefd16558 100644
--- a/core/services/onedrive/src/writer.rs
+++ b/core/services/onedrive/src/writer.rs
@@ -72,20 +72,7 @@ impl OneDriveWriter {
             .await?;
 
         match response.status() {
-            StatusCode::CREATED | StatusCode::OK => {
-                let item: OneDriveItem = serde_json::from_reader(response.into_body().reader())
-                    .map_err(new_json_deserialize_error)?;
-
-                let mut meta = Metadata::new(EntryMode::FILE)
-                    .with_etag(item.e_tag)
-                    .with_content_length(item.size.max(0) as u64);
-
-                let last_modified = item.last_modified_date_time;
-                let date_utc_last_modified = last_modified.parse::<Timestamp>()?;
-                meta.set_last_modified(date_utc_last_modified);
-
-                Ok(meta)
-            }
+            StatusCode::CREATED | StatusCode::OK => Self::parse_uploaded_item(response),
             _ => Err(parse_error(response)),
         }
     }
@@ -125,30 +112,18 @@ impl OneDriveWriter {
             match response.status() {
                 // Typical response code: 202 Accepted
                 // Reference: https://learn.microsoft.com/en-us/onedrive/developer/rest-api/api/driveitem_put_content?view=odsp-graph-online#response
-                StatusCode::ACCEPTED | StatusCode::OK => {} // skip, in the middle of upload
-                StatusCode::CREATED => {
-                    // last trunk
-                    let item: OneDriveItem = serde_json::from_reader(response.into_body().reader())
-                        .map_err(new_json_deserialize_error)?;
-
-                    let mut meta = Metadata::new(EntryMode::FILE)
-                        .with_etag(item.e_tag)
-                        .with_content_length(item.size.max(0) as u64);
-
-                    let last_modified = item.last_modified_date_time;
-                    let date_utc_last_modified = last_modified.parse::<Timestamp>()?;
-                    meta.set_last_modified(date_utc_last_modified);
-                    return Ok(meta);
-                }
+                StatusCode::ACCEPTED => {} // still uploading
+                StatusCode::CREATED | StatusCode::OK => return Self::parse_uploaded_item(response),
                 _ => return Err(parse_error(response)),
             }
 
             offset += OneDriveWriter::CHUNK_SIZE_FACTOR;
         }
 
-        debug_assert!(false, "should have returned");
-
-        Ok(Metadata::default()) // should not happen, but start with handling this gracefully - do nothing, but return the default metadata
+        Err(Error::new(
+            ErrorKind::Unexpected,
+            "OneDrive upload session finished without a terminal response",
+        ))
     }
 
     async fn create_upload_session(&self) -> Result<OneDriveUploadSessionCreationResponseBody> {
@@ -166,4 +141,19 @@ impl OneDriveWriter {
             _ => Err(parse_error(response)),
         }
     }
+
+    fn parse_uploaded_item(response: http::Response<Buffer>) -> Result<Metadata> {
+        let item: OneDriveItem = serde_json::from_reader(response.into_body().reader())
+            .map_err(new_json_deserialize_error)?;
+
+        let mut meta = Metadata::new(EntryMode::FILE)
+            .with_etag(item.e_tag)
+            .with_content_length(item.size.max(0) as u64);
+
+        let last_modified = item.last_modified_date_time;
+        let date_utc_last_modified = last_modified.parse::<Timestamp>()?;
+        meta.set_last_modified(date_utc_last_modified);
+
+        Ok(meta)
+    }
 }


(opendal) 05/05: build(core): refresh lockfile

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

suyanhanx pushed a commit to branch fix/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit ac9b72df77ecd496eebf585d8697d07b955b5bcc
Author: suyanhanx <su...@gmail.com>
AuthorDate: Thu Apr 16 22:37:46 2026 +0800

    build(core): refresh lockfile
---
 core/Cargo.lock | 1 -
 1 file changed, 1 deletion(-)

diff --git a/core/Cargo.lock b/core/Cargo.lock
index 6c71e05c2..457528b48 100644
--- a/core/Cargo.lock
+++ b/core/Cargo.lock
@@ -7047,7 +7047,6 @@ name = "opendal-service-onedrive"
 version = "0.55.0"
 dependencies = [
  "bytes",
- "futures",
  "http 1.4.0",
  "log",
  "mea",


(opendal) 04/05: test(services/onedrive): drop ensure dir unit test

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

suyanhanx pushed a commit to branch fix/onedrive-api-alignment
in repository https://gitbox.apache.org/repos/asf/opendal.git

commit f1eb3429854a6ea2bafd3e6a778ce301efed2cad
Author: suyanhanx <su...@gmail.com>
AuthorDate: Thu Apr 16 22:33:31 2026 +0800

    test(services/onedrive): drop ensure dir unit test
---
 core/services/onedrive/Cargo.toml  |   1 -
 core/services/onedrive/src/core.rs | 156 -------------------------------------
 2 files changed, 157 deletions(-)

diff --git a/core/services/onedrive/Cargo.toml b/core/services/onedrive/Cargo.toml
index b10724b90..1a5f04c70 100644
--- a/core/services/onedrive/Cargo.toml
+++ b/core/services/onedrive/Cargo.toml
@@ -41,5 +41,4 @@ serde_json = { workspace = true }
 tokio = { workspace = true, features = ["time"] }
 
 [dev-dependencies]
-futures = { workspace = true }
 tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
diff --git a/core/services/onedrive/src/core.rs b/core/services/onedrive/src/core.rs
index 0df3f111a..17df75a19 100644
--- a/core/services/onedrive/src/core.rs
+++ b/core/services/onedrive/src/core.rs
@@ -719,159 +719,3 @@ impl OneDriveSigner {
         Ok(())
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::ONEDRIVE_SCHEME;
-    use std::collections::VecDeque;
-    use std::sync::Mutex as StdMutex;
-
-    use bytes::Bytes;
-    use futures::stream;
-
-    use super::*;
-
-    #[derive(Clone)]
-    struct MockHttpClient {
-        requests: Arc<StdMutex<Vec<String>>>,
-        responses: Arc<StdMutex<VecDeque<(StatusCode, &'static str)>>>,
-    }
-
-    impl MockHttpClient {
-        fn new(responses: Vec<(StatusCode, &'static str)>) -> Self {
-            Self {
-                requests: Arc::new(StdMutex::new(Vec::new())),
-                responses: Arc::new(StdMutex::new(VecDeque::from(responses))),
-            }
-        }
-
-        fn requests(&self) -> Vec<String> {
-            self.requests.lock().expect("lock poisoned").clone()
-        }
-    }
-
-    impl HttpFetch for MockHttpClient {
-        async fn fetch(&self, req: Request<Buffer>) -> Result<Response<HttpBody>> {
-            self.requests.lock().expect("lock poisoned").push(format!(
-                "{} {}",
-                req.method(),
-                req.uri()
-            ));
-
-            let (status, body) = self
-                .responses
-                .lock()
-                .expect("lock poisoned")
-                .pop_front()
-                .expect("response queue must not be empty");
-            let bs = Bytes::from(body.to_string());
-            let size = bs.len() as u64;
-            let body = HttpBody::new(stream::iter(vec![Ok(Buffer::from(bs))]), Some(size));
-
-            Ok(Response::builder().status(status).body(body).unwrap())
-        }
-    }
-
-    fn folder_item_json(name: &str, parent_path: &str, id: &str) -> String {
-        format!(
-            r#"{{
-                "id": "{id}",
-                "name": "{name}",
-                "lastModifiedDateTime": "2025-02-16T19:49:05Z",
-                "eTag": "\"etag-{id}\"",
-                "size": 0,
-                "parentReference": {{
-                    "driveId": "drive-id",
-                    "id": "parent-{id}",
-                    "path": "{parent_path}"
-                }},
-                "folder": {{
-                    "childCount": 0
-                }}
-            }}"#
-        )
-    }
-
-    fn create_test_core(mock: MockHttpClient) -> OneDriveCore {
-        let info = AccessorInfo::default();
-        info.set_scheme(ONEDRIVE_SCHEME).set_root("/");
-        info.update_http_client(|_| HttpClient::with(mock));
-
-        let info = Arc::new(info);
-        let mut signer = OneDriveSigner::new(info.clone());
-        signer.access_token = "access_token".to_string();
-        signer.expires_in = Timestamp::MAX;
-
-        OneDriveCore {
-            info,
-            root: "/".to_string(),
-            signer: Arc::new(Mutex::new(signer)),
-        }
-    }
-
-    #[tokio::test]
-    async fn test_ensure_directory_creates_missing_ancestors() {
-        let root = Box::leak(folder_item_json("root", "/drive/root:", "root").into_boxed_str());
-        let a = Box::leak(folder_item_json("a", "/drive/root:", "a").into_boxed_str());
-        let ab = Box::leak(folder_item_json("b", "/drive/root:/a", "ab").into_boxed_str());
-        let abc = Box::leak(folder_item_json("c", "/drive/root:/a/b", "abc").into_boxed_str());
-
-        let mock = MockHttpClient::new(vec![
-            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
-            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
-            (StatusCode::NOT_FOUND, r#"{"error":"not found"}"#),
-            (StatusCode::OK, root),
-            (StatusCode::CREATED, a),
-            (StatusCode::CREATED, ab),
-            (StatusCode::CREATED, abc),
-        ]);
-        let core = create_test_core(mock.clone());
-
-        let item = core
-            .ensure_directory("a/b/c/")
-            .await
-            .expect("ensure directory must succeed");
-
-        assert_eq!(item.id, "abc");
-        assert_eq!(
-            mock.requests(),
-            vec![
-                format!(
-                    "GET {}:/a/b/c?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "GET {}:/a/b?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "GET {}:/a?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "GET {}?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "POST {}:/children?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "POST {}:/a:/children?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-                format!(
-                    "POST {}:/a/b:/children?{}",
-                    OneDriveCore::DRIVE_ROOT_URL,
-                    GENERAL_SELECT_PARAM
-                ),
-            ]
-        );
-    }
-}