suyanhanx commented on code in PR #3651:
URL:
https://github.com/apache/incubator-opendal/pull/3651#discussion_r1402940695
##########
core/src/raw/oio/list/api.rs:
##########
@@ -90,6 +94,35 @@ impl<P: List> List for Option<P> {
}
}
+/// Impl ListExt for all T: List
+impl<T: List> ListExt for T {}
+
+/// Extension of [`Read`] to make it easier for use.
Review Comment:
Read?
##########
core/src/types/operator/operator.rs:
##########
@@ -158,16 +158,36 @@ impl Operator {
}
}
- /// Get current path's metadata.
+ /// Get given path's metadata.
///
/// # Notes
///
/// For fetch metadata of entries returned by [`Lister`], it's better to
use [`list_with`] and
/// [`lister_with`] with `metakey` query like `Metakey::ContentLength |
Metakey::LastModified`
- /// so that we can avoid extra requests.
+ /// so that we can avoid extra stat requests.
+ ///
+ /// # Behavior
+ ///
+ /// `test` and `test/` may vary in some services such as S3. However, on a
local file system,
+ /// they're identical. Therefore, the behavior of `stat("test")` and
`stat("test/")` might differ
+ /// in certain edge cases. Always use `stat("test/")` when you need to
access a directory if possible.
+ ///
Review Comment:
The essence may still need to be noted here. Those services rely on
prefixes.Or like WebDAV, which locates resources by their url, there is no way
to determine if it is a folder or not before accessing them. That's why this
happens.
##########
core/tests/behavior/write.rs:
##########
@@ -282,23 +291,64 @@ pub async fn test_stat_file(op: Operator) -> Result<()> {
assert_eq!(meta.mode(), EntryMode::FILE);
assert_eq!(meta.content_length(), size as u64);
+ // Stat a file with trailing slash should return `NotFound`.
+ if op.info().full_capability().create_dir {
+ let result = op.stat(&format!("{path}/")).await;
+ assert!(result.is_err());
+ assert_eq!(result.unwrap_err().kind(), ErrorKind::NotFound);
+ }
+
op.delete(&path).await.expect("delete must succeed");
Ok(())
}
/// Stat existing file should return metadata
pub async fn test_stat_dir(op: Operator) -> Result<()> {
+ if !op.info().full_capability().create_dir {
+ return Ok(());
+ }
+
let path = format!("{}/", uuid::Uuid::new_v4());
op.create_dir(&path).await.expect("write must succeed");
let meta = op.stat(&path).await?;
assert_eq!(meta.mode(), EntryMode::DIR);
+ // Stat a dir without trailing slash could have two behavior.
+ let result = op.stat(path.trim_end_matches('/')).await;
+ match result {
+ Ok(meta) => assert_eq!(meta.mode(), EntryMode::DIR),
+ Err(err) => assert_eq!(err.kind(), ErrorKind::NotFound),
+ }
+
op.delete(&path).await.expect("delete must succeed");
Ok(())
}
+/// Stat the parent dir of existing dir should return metadata
+pub async fn test_stat_nested_parent_dir(op: Operator) -> Result<()> {
+ if !op.info().full_capability().create_dir {
+ return Ok(());
+ }
+
+ let parent = format!("{}", uuid::Uuid::new_v4());
+ let file = format!("{}", uuid::Uuid::new_v4());
+ let (content, _) = gen_bytes(op.info().full_capability());
+
+ op.write(&format!("{parent}/{file}"), content.clone())
+ .await
+ .expect("write must succeed");
+
+ let meta = op.stat(&format!("{parent}/")).await?;
+ assert_eq!(meta.mode(), EntryMode::DIR);
+
+ op.delete(&format!("{parent}/{file}"))
+ .await
+ .expect("delete must succeed");
+ Ok(())
+}
Review Comment:
We may delete the parent folder too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
