Skip to content

Commit

Permalink
feat: Entry refactoring to allow external creation (#1547)
Browse files Browse the repository at this point in the history
* feat: Entry refactoring to allow external creation.

* fix: Entry tests and api adapted for changes

* fix: Entry tests and api adapted for changes
  • Loading branch information
damooo authored Mar 9, 2023
1 parent f4cdc2b commit a540559
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 33 deletions.
2 changes: 1 addition & 1 deletion src/raw/oio/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@ impl Entry {
///
/// NOTE: implement this by hand to avoid leaking raw entry to end-users.
pub(crate) fn into_entry(self) -> crate::Entry {
crate::Entry::new(self.path, self.meta)
crate::Entry::new_with(self.path, self.meta)
}
}
47 changes: 24 additions & 23 deletions src/types/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,36 @@ use crate::*;
/// Entry is the file/dir entry returned by `Lister`.
#[derive(Clone, Debug)]
pub struct Entry {
/// Path of the entry.
path: String,
meta: Metadata,

/// Optional cached metadata
metadata: Option<Metadata>,
}

impl Entry {
/// Create an entry.
/// Create an entry with .
///
/// # Notes
///
/// This function is crate internal only. Users don't have public
/// methods to construct an entry. The only way to get an entry
/// methods to construct an entry with arbitrary cached metadata.
///
/// The only way to get an entry with associated cached metadata
/// is `Operator::list` or `Operator::scan`.
pub(crate) fn new(path: String, meta: Metadata) -> Self {
Self { path, meta }
pub(crate) fn new_with(path: String, metadata: Metadata) -> Self {
Self {
path,
metadata: Some(metadata),
}
}

/// Create an [`Entry`] with empty cached metadata.
pub fn new(path: &str) -> Self {
Self {
path: normalize_path(path),
metadata: None,
}
}

/// Path of entry. Path is relative to operator's root.
Expand All @@ -48,29 +64,14 @@ impl Entry {
get_basename(&self.path)
}

/// Mode of this entry.
pub fn mode(&self) -> EntryMode {
self.meta.mode()
}

/// Returns `true` if this entry is for a directory.
pub fn is_dir(&self) -> bool {
self.meta.is_dir()
}

/// Returns `true` if this entry is for a file.
pub fn is_file(&self) -> bool {
self.meta.is_file()
}

/// Get the metadata of entry.
/// Get the cached metadata of entry.
///
/// # Notes
///
/// This function is crate internal only. Because the returning
/// metadata could be incomplete. Users must use `Operator::metadata`
/// to query the cached metadata instead.
pub(crate) fn metadata(&self) -> &Metadata {
&self.meta
pub(crate) fn metadata(&self) -> &Option<Metadata> {
&self.metadata
}
}
9 changes: 6 additions & 3 deletions src/types/operator/blocking_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,14 @@ impl BlockingOperator {
/// # }
/// ```
pub fn metadata(&self, entry: &Entry, flags: impl Into<FlagSet<Metakey>>) -> Result<Metadata> {
let meta = entry.metadata();
if meta.bit().contains(flags) || meta.bit().contains(Metakey::Complete) {
return Ok(meta.clone());
// Check if cached metadata saticifies the query.
if let Some(meta) = entry.metadata() {
if meta.bit().contains(flags) || meta.bit().contains(Metakey::Complete) {
return Ok(meta.clone());
}
}

// Else request from backend..
let meta = self.stat(entry.path())?;
Ok(meta)
}
Expand Down
9 changes: 6 additions & 3 deletions src/types/operator/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,14 @@ impl Operator {
entry: &Entry,
flags: impl Into<FlagSet<Metakey>>,
) -> Result<Metadata> {
let meta = entry.metadata();
if meta.bit().contains(flags) || meta.bit().contains(Metakey::Complete) {
return Ok(meta.clone());
// Check if cached metadata saticifies the query.
if let Some(meta) = entry.metadata() {
if meta.bit().contains(flags) || meta.bit().contains(Metakey::Complete) {
return Ok(meta.clone());
}
}

// Else request from backend..
let meta = self.stat(entry.path()).await?;
Ok(meta)
}
Expand Down
1 change: 0 additions & 1 deletion tests/behavior/blocking_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ pub fn test_list_dir(op: BlockingOperator) -> Result<()> {
let meta = op.stat(de.path())?;
if de.path() == path {
assert_eq!(meta.mode(), EntryMode::FILE);
assert_eq!(de.mode(), meta.mode());

assert_eq!(meta.content_length(), size as u64);

Expand Down
2 changes: 0 additions & 2 deletions tests/behavior/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ pub async fn test_list_dir(op: Operator) -> Result<()> {
let meta = op.stat(de.path()).await?;
if de.path() == path {
assert_eq!(meta.mode(), EntryMode::FILE);
assert_eq!(de.mode(), meta.mode());

assert_eq!(meta.content_length(), size as u64);

Expand Down Expand Up @@ -201,7 +200,6 @@ pub async fn test_list_sub_dir(op: Operator) -> Result<()> {
if de.path() == path {
let meta = op.stat(&path).await?;
assert_eq!(meta.mode(), EntryMode::DIR);
assert_eq!(de.mode(), meta.mode());
assert_eq!(de.name(), path);

found = true
Expand Down

2 comments on commit a540559

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for opendal ready!

✅ Preview
https://opendal-b5cmgwqgt-databend.vercel.app

Built with commit a540559.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: a540559 Previous: f4cdc2b Ratio
service_memory_write_once/256 KiB 44321 ns/iter (± 2523) 21299 ns/iter (± 117) 2.08

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Xuanwo

Please sign in to comment.