Skip to content

Commit 06235da

Browse files
rich7420richXuanwo
authored
refactor: Deprecate remove_all API in favor of RFC-3911: Deleter API (#6785)
* try 3922 * decrease explanation * allow * fix fmt error * update * fix bindings * fix errors * fix format error * allow * update * fix import * fmt error * fix error from behavior test local_fs * remove redundant part * modify ci error toml-format * remove redundant part * Format code --------- Co-authored-by: rich <rich@MacBook-Pro-di-rich.local> Co-authored-by: Xuanwo <github@xuanwo.io>
1 parent 1ba746d commit 06235da

File tree

13 files changed

+132
-44
lines changed

13 files changed

+132
-44
lines changed

bindings/dart/rust/src/frb_generated.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
use crate::api::opendal_api::*;
2929
use flutter_rust_bridge::for_generated::byteorder::{NativeEndian, ReadBytesExt, WriteBytesExt};
30-
use flutter_rust_bridge::for_generated::{Lifetimeable, Lockable, transform_result_dco};
30+
use flutter_rust_bridge::for_generated::{transform_result_dco, Lifetimeable, Lockable};
3131
use flutter_rust_bridge::{Handler, IntoIntoDart};
3232

3333
// Section: boilerplate
@@ -1491,7 +1491,7 @@ mod io {
14911491
use flutter_rust_bridge::for_generated::byteorder::{
14921492
NativeEndian, ReadBytesExt, WriteBytesExt,
14931493
};
1494-
use flutter_rust_bridge::for_generated::{Lifetimeable, Lockable, transform_result_dco};
1494+
use flutter_rust_bridge::for_generated::{transform_result_dco, Lifetimeable, Lockable};
14951495
use flutter_rust_bridge::{Handler, IntoIntoDart};
14961496

14971497
// Section: boilerplate
@@ -1544,7 +1544,7 @@ mod web {
15441544
};
15451545
use flutter_rust_bridge::for_generated::wasm_bindgen;
15461546
use flutter_rust_bridge::for_generated::wasm_bindgen::prelude::*;
1547-
use flutter_rust_bridge::for_generated::{Lifetimeable, Lockable, transform_result_dco};
1547+
use flutter_rust_bridge::for_generated::{transform_result_dco, Lifetimeable, Lockable};
15481548
use flutter_rust_bridge::{Handler, IntoIntoDart};
15491549

15501550
// Section: boilerplate

bindings/haskell/src/lib.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -706,9 +706,18 @@ pub unsafe extern "C" fn blocking_remove_all(
706706
}
707707
};
708708

709-
let res = match op.remove_all(path_str) {
710-
Ok(()) => FFIResult::ok(()),
711-
Err(e) => FFIResult::err_with_source("Failed to remove all", e),
709+
let res = {
710+
use od::options::DeleteOptions;
711+
match op.delete_options(
712+
path_str,
713+
DeleteOptions {
714+
recursive: true,
715+
..Default::default()
716+
},
717+
) {
718+
Ok(()) => FFIResult::ok(()),
719+
Err(e) => FFIResult::err_with_source("Failed to remove all", e),
720+
}
712721
};
713722

714723
*result = res;

bindings/java/src/async_operator.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,8 @@ fn intern_remove_all(
469469

470470
executor_or_default(env, executor)?.spawn(async move {
471471
let result = op
472-
.remove_all(&path)
472+
.delete_with(&path)
473+
.recursive(true)
473474
.await
474475
.map(|_| JValueOwned::Void)
475476
.map_err(Into::into);

bindings/java/src/operator.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,9 +283,16 @@ pub unsafe extern "system" fn Java_org_apache_opendal_Operator_removeAll(
283283
}
284284

285285
fn intern_remove_all(env: &mut JNIEnv, op: &mut blocking::Operator, path: JString) -> Result<()> {
286+
use opendal::options::DeleteOptions;
286287
let path = jstring_to_string(env, &path)?;
287288

288-
Ok(op.remove_all(&path)?)
289+
Ok(op.delete_options(
290+
&path,
291+
DeleteOptions {
292+
recursive: true,
293+
..Default::default()
294+
},
295+
)?)
289296
}
290297

291298
/// # Safety

bindings/nodejs/src/lib.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,8 @@ impl Operator {
516516
#[napi]
517517
pub async fn remove_all(&self, path: String) -> Result<()> {
518518
self.async_op
519-
.remove_all(&path)
519+
.delete_with(&path)
520+
.recursive(true)
520521
.await
521522
.map_err(format_napi_error)
522523
}
@@ -532,8 +533,15 @@ impl Operator {
532533
/// ```
533534
#[napi]
534535
pub fn remove_all_sync(&self, path: String) -> Result<()> {
536+
use opendal::options::DeleteOptions;
535537
self.blocking_op
536-
.remove_all(&path)
538+
.delete_options(
539+
&path,
540+
DeleteOptions {
541+
recursive: true,
542+
..Default::default()
543+
},
544+
)
537545
.map_err(format_napi_error)
538546
}
539547

bindings/ocaml/src/operator/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,14 @@ pub fn blocking_remove(operator: &mut Operator, path: Vec<String>) -> Result<(),
148148
#[ocaml::func]
149149
#[ocaml::sig("operator -> string -> (unit, string) Result.t ")]
150150
pub fn blocking_remove_all(operator: &mut Operator, path: String) -> Result<(), String> {
151-
map_res_error(operator.0.remove_all(path.as_str()))
151+
use opendal::options::DeleteOptions;
152+
map_res_error(operator.0.delete_options(
153+
path.as_str(),
154+
DeleteOptions {
155+
recursive: true,
156+
..Default::default()
157+
},
158+
))
152159
}
153160

154161
#[ocaml::func]

bindings/python/src/operator.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,17 @@ impl Operator {
484484
/// path : str
485485
/// The path to remove.
486486
pub fn remove_all(&self, path: PathBuf) -> PyResult<()> {
487+
use ocore::options::DeleteOptions;
487488
let path = path.to_string_lossy().to_string();
488-
self.core.remove_all(&path).map_err(format_pyerr)
489+
self.core
490+
.delete_options(
491+
&path,
492+
DeleteOptions {
493+
recursive: true,
494+
..Default::default()
495+
},
496+
)
497+
.map_err(format_pyerr)
489498
}
490499

491500
/// Create a directory at the given path.
@@ -1219,7 +1228,10 @@ impl AsyncOperator {
12191228
let this = self.core.clone();
12201229
let path = path.to_string_lossy().to_string();
12211230
future_into_py(py, async move {
1222-
this.remove_all(&path).await.map_err(format_pyerr)
1231+
this.delete_with(&path)
1232+
.recursive(true)
1233+
.await
1234+
.map_err(format_pyerr)
12231235
})
12241236
}
12251237

bindings/ruby/src/operator.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,16 @@ impl Operator {
200200
/// @param path [String]
201201
/// @return [nil]
202202
fn remove_all(ruby: &Ruby, rb_self: &Self, path: String) -> Result<(), Error> {
203+
use ocore::options::DeleteOptions;
203204
rb_self
204205
.blocking_op
205-
.remove_all(&path)
206+
.delete_options(
207+
&path,
208+
DeleteOptions {
209+
recursive: true,
210+
..Default::default()
211+
},
212+
)
206213
.map_err(|err| Error::new(ruby.exception_runtime_error(), err.to_string()))
207214
}
208215

core/core/src/blocking/operator.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,9 +555,31 @@ impl Operator {
555555

556556
/// Remove the path and all nested dirs and files recursively.
557557
///
558+
/// # Deprecated
559+
///
560+
/// This method is deprecated since v0.55.0. Use [`blocking::Operator::delete_options`] with
561+
/// `recursive: true` instead.
562+
///
563+
/// ## Migration Example
564+
///
565+
/// Instead of:
566+
/// ```ignore
567+
/// op.remove_all("path/to/dir")?;
568+
/// ```
569+
///
570+
/// Use:
571+
/// ```ignore
572+
/// use opendal_core::options::DeleteOptions;
573+
/// op.delete_options("path/to/dir", DeleteOptions {
574+
/// recursive: true,
575+
/// ..Default::default()
576+
/// })?;
577+
/// ```
578+
///
558579
/// # Notes
559580
///
560-
/// We don't support batch delete now.
581+
/// If underlying services support delete in batch, we will use batch
582+
/// delete instead.
561583
///
562584
/// # Examples
563585
///
@@ -571,8 +593,19 @@ impl Operator {
571593
/// # Ok(())
572594
/// # }
573595
/// ```
596+
#[deprecated(
597+
since = "0.55.0",
598+
note = "Use `delete_options` with `recursive: true` instead"
599+
)]
600+
#[allow(deprecated)]
574601
pub fn remove_all(&self, path: &str) -> Result<()> {
575-
self.handle.block_on(self.op.remove_all(path))
602+
self.delete_options(
603+
path,
604+
options::DeleteOptions {
605+
recursive: true,
606+
..Default::default()
607+
},
608+
)
576609
}
577610

578611
/// List entries whose paths start with the given prefix `path`.

core/core/src/types/operator/operator.rs

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,23 @@ impl Operator {
13751375

13761376
/// Remove the path and all nested dirs and files recursively.
13771377
///
1378+
/// # Deprecated
1379+
///
1380+
/// This method is deprecated since v0.55.0. Use [`Operator::delete_with`] with
1381+
/// `recursive(true)` instead.
1382+
///
1383+
/// ## Migration Example
1384+
///
1385+
/// Instead of:
1386+
/// ```ignore
1387+
/// op.remove_all("path/to/dir").await?;
1388+
/// ```
1389+
///
1390+
/// Use:
1391+
/// ```ignore
1392+
/// op.delete_with("path/to/dir").recursive(true).await?;
1393+
/// ```
1394+
///
13781395
/// # Notes
13791396
///
13801397
/// If underlying services support delete in batch, we will use batch
@@ -1392,28 +1409,12 @@ impl Operator {
13921409
/// # Ok(())
13931410
/// # }
13941411
/// ```
1412+
#[deprecated(
1413+
since = "0.55.0",
1414+
note = "Use `delete_with` with `recursive(true)` instead"
1415+
)]
13951416
pub async fn remove_all(&self, path: &str) -> Result<()> {
1396-
match self.stat(path).await {
1397-
// If object exists.
1398-
Ok(metadata) => {
1399-
// If the object is a file, we can delete it.
1400-
if metadata.mode() != EntryMode::DIR {
1401-
self.delete(path).await?;
1402-
// There may still be objects prefixed with the path in some backend, so we can't return here.
1403-
}
1404-
}
1405-
1406-
// If dir not found, it may be a prefix in object store like S3,
1407-
// and we still need to delete objects under the prefix.
1408-
Err(e) if e.kind() == ErrorKind::NotFound => {}
1409-
1410-
// Pass on any other error.
1411-
Err(e) => return Err(e),
1412-
};
1413-
1414-
let lister = self.lister_with(path).recursive(true).await?;
1415-
self.delete_try_stream(lister).await?;
1416-
Ok(())
1417+
self.delete_with(path).recursive(true).await
14171418
}
14181419

14191420
/// List entries whose paths start with the given prefix `path`.

0 commit comments

Comments
 (0)