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:49 UTC
(opendal) 01/05: fix(services/onedrive): align versioning semantics
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)
+ }
}