refactor(core): Deprecate OpList::version and add versions instead (#5481)

Co-authored-by: Xuanwo <[email protected]>

Author: geetanshjuneja

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,

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())
     }
 }

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
     }
 }
 

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,

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

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))
     }
 }

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![];