You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by xu...@apache.org on 2024/12/31 10:01:53 UTC
(opendal) branch main updated: refactor(core): Deprecate OpList::version and add versions instead (#5481)
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/opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 386b8de81 refactor(core): Deprecate OpList::version and add versions instead (#5481)
386b8de81 is described below
commit 386b8de81c96fae931c5d825f61aa69e1fa6d948
Author: Geetansh Juneja <ro...@gmail.com>
AuthorDate: Tue Dec 31 15:31:47 2024 +0530
refactor(core): Deprecate OpList::version and add versions instead (#5481)
Co-authored-by: Xuanwo <gi...@xuanwo.io>
---
bindings/ruby/src/capability.rs | 4 ++--
core/src/layers/capability_check.rs | 12 ++++++------
core/src/raw/ops.rs | 21 +++++++++++++++++----
core/src/services/s3/backend.rs | 4 ++--
core/src/types/capability.rs | 11 +++++++----
core/src/types/operator/operator_futures.rs | 28 ++++++++++++++++++++++++++--
core/tests/behavior/async_list.rs | 24 ++++++++++++------------
7 files changed, 72 insertions(+), 32 deletions(-)
diff --git a/bindings/ruby/src/capability.rs b/bindings/ruby/src/capability.rs
index 03116534d..b3a7309f9 100644
--- a/bindings/ruby/src/capability.rs
+++ b/bindings/ruby/src/capability.rs
@@ -89,7 +89,7 @@ define_accessors!(Capability, {
list_with_limit: bool,
list_with_start_after: bool,
list_with_recursive: bool,
- list_with_version: bool,
+ list_with_versions: bool,
presign: bool,
presign_read: bool,
presign_stat: bool,
@@ -137,7 +137,7 @@ pub fn include(gem_module: &RModule) -> Result<(), Error> {
list_with_limit,
list_with_start_after,
list_with_recursive,
- list_with_version,
+ list_with_versions,
presign,
presign_read,
presign_stat,
diff --git a/core/src/layers/capability_check.rs b/core/src/layers/capability_check.rs
index 7a5064bc6..449bf9ce7 100644
--- a/core/src/layers/capability_check.rs
+++ b/core/src/layers/capability_check.rs
@@ -30,7 +30,7 @@ use std::sync::Arc;
///
/// There are two main differences between this checker with the `CorrectnessChecker`:
/// 1. This checker provides additional checks for capabilities like write_with_content_type and
-/// list_with_version, among others. These capabilities do not affect data integrity, even if
+/// list_with_versions, among others. These capabilities do not affect data integrity, even if
/// the underlying storage services do not support them.
///
/// 2. OpenDAL doesn't apply this checker by default. Users can enable this layer if they want to
@@ -131,7 +131,7 @@ impl<A: Access> LayeredAccess for CapabilityAccessor<A> {
async fn list(&self, path: &str, args: OpList) -> crate::Result<(RpList, Self::Lister)> {
let capability = self.info.full_capability();
- if !capability.list_with_version && args.version() {
+ if !capability.list_with_versions && args.versions() {
return Err(new_unsupported_error(
self.info.as_ref(),
Operation::List,
@@ -191,7 +191,7 @@ impl<A: Access> LayeredAccess for CapabilityAccessor<A> {
args: OpList,
) -> crate::Result<(RpList, Self::BlockingLister)> {
let capability = self.info.full_capability();
- if !capability.list_with_version && args.version() {
+ if !capability.list_with_versions && args.versions() {
return Err(new_unsupported_error(
self.info.as_ref(),
Operation::BlockingList,
@@ -289,16 +289,16 @@ mod tests {
list: true,
..Default::default()
});
- let res = op.list_with("path/").version(true).await;
+ let res = op.list_with("path/").versions(true).await;
assert!(res.is_err());
assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported);
let op = new_test_operator(Capability {
list: true,
- list_with_version: true,
+ list_with_versions: true,
..Default::default()
});
- let res = op.lister_with("path/").version(true).await;
+ let res = op.lister_with("path/").versions(true).await;
assert!(res.is_ok())
}
}
diff --git a/core/src/raw/ops.rs b/core/src/raw/ops.rs
index 3b196d7b2..2620bb541 100644
--- a/core/src/raw/ops.rs
+++ b/core/src/raw/ops.rs
@@ -111,7 +111,7 @@ pub struct OpList {
/// by the underlying service
///
/// Default to `false`
- version: bool,
+ versions: bool,
}
impl Default for OpList {
@@ -121,7 +121,7 @@ impl Default for OpList {
start_after: None,
recursive: false,
concurrent: 1,
- version: false,
+ versions: false,
}
}
}
@@ -184,14 +184,27 @@ impl OpList {
}
/// Change the version of this list operation
+ #[deprecated(since = "0.51.1", note = "use with_versions instead")]
pub fn with_version(mut self, version: bool) -> Self {
- self.version = version;
+ self.versions = version;
+ self
+ }
+
+ /// Change the version of this list operation
+ pub fn with_versions(mut self, versions: bool) -> Self {
+ self.versions = versions;
self
}
/// Get the version of this list operation
+ #[deprecated(since = "0.51.1", note = "use versions instead")]
pub fn version(&self) -> bool {
- self.version
+ self.versions
+ }
+
+ /// Get the version of this list operation
+ pub fn versions(&self) -> bool {
+ self.versions
}
}
diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs
index 2d8ba5058..24f441f36 100644
--- a/core/src/services/s3/backend.rs
+++ b/core/src/services/s3/backend.rs
@@ -982,7 +982,7 @@ impl Access for S3Backend {
list_with_limit: true,
list_with_start_after: true,
list_with_recursive: true,
- list_with_version: self.core.enable_versioning,
+ list_with_versions: self.core.enable_versioning,
list_has_etag: true,
list_has_content_md5: true,
list_has_content_length: true,
@@ -1060,7 +1060,7 @@ impl Access for S3Backend {
}
async fn list(&self, path: &str, args: OpList) -> Result<(RpList, Self::Lister)> {
- let l = if args.version() {
+ let l = if args.versions() {
TwoWays::Two(PageLister::new(S3ObjectVersionsLister::new(
self.core.clone(),
path,
diff --git a/core/src/types/capability.rs b/core/src/types/capability.rs
index 76bbc0961..2c4d3faf4 100644
--- a/core/src/types/capability.rs
+++ b/core/src/types/capability.rs
@@ -76,7 +76,7 @@ pub struct Capability {
pub stat_with_override_content_disposition: bool,
/// Indicates if Content-Type header override is supported during stat operations.
pub stat_with_override_content_type: bool,
- /// Indicates if versioned stat operations are supported.
+ /// Indicates if versions stat operations are supported.
pub stat_with_version: bool,
/// Indicates whether cache control information is available in stat response
pub stat_has_cache_control: bool,
@@ -113,7 +113,7 @@ pub struct Capability {
pub read_with_override_content_disposition: bool,
/// Indicates if Content-Type header override is supported during read operations.
pub read_with_override_content_type: bool,
- /// Indicates if versioned read operations are supported.
+ /// Indicates if versions read operations are supported.
pub read_with_version: bool,
/// Indicates if the operator supports write operations.
@@ -155,7 +155,7 @@ pub struct Capability {
/// Indicates if delete operations are supported.
pub delete: bool,
- /// Indicates if versioned delete operations are supported.
+ /// Indicates if versions delete operations are supported.
pub delete_with_version: bool,
/// Maximum size supported for single delete operations.
pub delete_max_size: Option<usize>,
@@ -174,8 +174,11 @@ pub struct Capability {
pub list_with_start_after: bool,
/// Indicates if recursive listing is supported.
pub list_with_recursive: bool,
- /// Indicates if versioned listing is supported.
+ /// Indicates if versions listing is supported.
+ #[deprecated(since = "0.51.1", note = "use with_versions instead")]
pub list_with_version: bool,
+ /// Indicates if versions listing is supported.
+ pub list_with_versions: bool,
/// Indicates whether cache control information is available in list response
pub list_has_cache_control: bool,
/// Indicates whether content disposition information is available in list response
diff --git a/core/src/types/operator/operator_futures.rs b/core/src/types/operator/operator_futures.rs
index 2d1190557..6c9e2aa42 100644
--- a/core/src/types/operator/operator_futures.rs
+++ b/core/src/types/operator/operator_futures.rs
@@ -486,8 +486,20 @@ impl<F: Future<Output = Result<Vec<Entry>>>> FutureList<F> {
/// by the underlying service
///
/// Default to `false`
+ #[deprecated(since = "0.51.1", note = "use versions instead")]
pub fn version(self, v: bool) -> Self {
- self.map(|args| args.with_version(v))
+ self.map(|args| args.with_versions(v))
+ }
+
+ /// The version is used to control whether the object versions should be returned.
+ ///
+ /// - If `false`, list operation will not return with object versions
+ /// - If `true`, list operation will return with object versions if object versioning is supported
+ /// by the underlying service
+ ///
+ /// Default to `false`
+ pub fn versions(self, v: bool) -> Self {
+ self.map(|args| args.with_versions(v))
}
}
@@ -528,7 +540,19 @@ impl<F: Future<Output = Result<Lister>>> FutureLister<F> {
/// by the underlying service
///
/// Default to `false`
+ #[deprecated(since = "0.51.1", note = "use versions instead")]
pub fn version(self, v: bool) -> Self {
- self.map(|args| args.with_version(v))
+ self.map(|args| args.with_versions(v))
+ }
+
+ /// The version is used to control whether the object versions should be returned.
+ ///
+ /// - If `false`, list operation will not return with object versions
+ /// - If `true`, list operation will return with object versions if object versioning is supported
+ /// by the underlying service
+ ///
+ /// Default to `false`
+ pub fn versions(self, v: bool) -> Self {
+ self.map(|args| args.with_versions(v))
}
}
diff --git a/core/tests/behavior/async_list.rs b/core/tests/behavior/async_list.rs
index 9e9367b1a..6a0aa2ffb 100644
--- a/core/tests/behavior/async_list.rs
+++ b/core/tests/behavior/async_list.rs
@@ -47,9 +47,9 @@ pub fn tests(op: &Operator, tests: &mut Vec<Trial>) {
test_list_file_with_recursive,
test_list_root_with_recursive,
test_remove_all,
- test_list_files_with_version,
- test_list_with_version_and_limit,
- test_list_with_version_and_start_after
+ test_list_files_with_versions,
+ test_list_with_versions_and_limit,
+ test_list_with_versions_and_start_after
))
}
@@ -575,8 +575,8 @@ pub async fn test_list_only(op: Operator) -> Result<()> {
Ok(())
}
-pub async fn test_list_files_with_version(op: Operator) -> Result<()> {
- if !op.info().full_capability().list_with_version {
+pub async fn test_list_files_with_versions(op: Operator) -> Result<()> {
+ if !op.info().full_capability().list_with_versions {
return Ok(());
}
@@ -586,7 +586,7 @@ pub async fn test_list_files_with_version(op: Operator) -> Result<()> {
op.write(file_path.as_str(), "1").await?;
op.write(file_path.as_str(), "2").await?;
- let mut ds = op.list_with(parent.as_str()).version(true).await?;
+ let mut ds = op.list_with(parent.as_str()).versions(true).await?;
ds.retain(|de| de.path() != parent.as_str());
assert_eq!(ds.len(), 2);
@@ -608,13 +608,13 @@ pub async fn test_list_files_with_version(op: Operator) -> Result<()> {
}
// listing a directory with version, which contains more object versions than a page can take
-pub async fn test_list_with_version_and_limit(op: Operator) -> Result<()> {
+pub async fn test_list_with_versions_and_limit(op: Operator) -> Result<()> {
// Gdrive think that this test is an abuse of their service and redirect us
// to an infinite loop. Let's ignore this test for gdrive.
if op.info().scheme() == Scheme::Gdrive {
return Ok(());
}
- if !op.info().full_capability().list_with_version {
+ if !op.info().full_capability().list_with_versions {
return Ok(());
}
@@ -633,7 +633,7 @@ pub async fn test_list_with_version_and_limit(op: Operator) -> Result<()> {
.collect();
expected.push(parent.to_string());
- let mut objects = op.lister_with(parent).version(true).limit(5).await?;
+ let mut objects = op.lister_with(parent).versions(true).limit(5).await?;
let mut actual = vec![];
while let Some(o) = objects.try_next().await? {
let path = o.path().to_string();
@@ -648,8 +648,8 @@ pub async fn test_list_with_version_and_limit(op: Operator) -> Result<()> {
Ok(())
}
-pub async fn test_list_with_version_and_start_after(op: Operator) -> Result<()> {
- if !op.info().full_capability().list_with_version {
+pub async fn test_list_with_versions_and_start_after(op: Operator) -> Result<()> {
+ if !op.info().full_capability().list_with_versions {
return Ok(());
}
@@ -672,7 +672,7 @@ pub async fn test_list_with_version_and_start_after(op: Operator) -> Result<()>
let mut objects = op
.lister_with(dir)
- .version(true)
+ .versions(true)
.start_after(&given[2])
.await?;
let mut actual = vec![];