Copilot commented on code in PR #6283:
URL: https://github.com/apache/opendal/pull/6283#discussion_r2184992488
##########
integrations/object_store/src/backend/lister.rs:
##########
@@ -0,0 +1,74 @@
+use std::future::Future;
+use std::sync::Arc;
+use tokio::sync::Mutex;
+
+use futures::{stream::BoxStream, StreamExt};
+use object_store::{ObjectMeta, ObjectStore};
+use opendal::raw::*;
+use opendal::*;
+
+use super::error::parse_error;
+
+pub struct ObjectStoreLister {
+ stream: Arc<Mutex<BoxStream<'static, object_store::Result<ObjectMeta>>>>,
+}
+
+impl ObjectStoreLister {
+ pub(crate) async fn new(
+ store: Arc<dyn ObjectStore + 'static>,
+ path: &str,
+ args: OpList,
+ ) -> Result<Self> {
+ // If start_after is specified, use list_with_offset
+ let mut stream = if let Some(start_after) = args.start_after() {
+ store
+ .list_with_offset(
+ Some(&object_store::path::Path::from(path)),
+ &object_store::path::Path::from(start_after),
+ )
+ .boxed()
+ } else {
+ store
+ .list(Some(&object_store::path::Path::from(path)))
+ .boxed()
+ };
+
+ // Process listing arguments
+ if let Some(limit) = args.limit() {
+ stream = stream.take(limit).boxed();
+ }
+
+ Ok(Self {
+ stream: Arc::new(Mutex::new(stream)),
+ })
+ }
+}
+
+impl oio::List for ObjectStoreLister {
+ fn next(&mut self) -> impl Future<Output = Result<Option<oio::Entry>>> +
MaybeSend {
+ let stream = self.stream.clone();
+ async move {
+ let next_item = {
+ let mut stream = stream.lock().await;
+ stream.next().await
+ };
+ match next_item {
+ Some(Ok(meta)) => {
+ let mut metadata = Metadata::new(EntryMode::FILE);
+ let entry = oio::Entry::new(meta.location.as_ref(),
metadata.clone());
Review Comment:
The entry is built with a cloned metadata before setting content_length,
last_modified, etag, and version. Move the metadata.clone() after populating
all fields, or set fields directly on the entry's metadata to ensure the
returned entry carries correct metadata.
```suggestion
```
##########
integrations/object_store/src/backend/deleter.rs:
##########
@@ -0,0 +1,54 @@
+use std::collections::VecDeque;
+use std::future::Future;
+use std::sync::Arc;
+
+use futures::FutureExt;
+use object_store::ObjectStore;
+use opendal::raw::*;
+use opendal::*;
+
+use super::error::parse_error;
+
+pub struct ObjectStoreDeleter {
+ store: Arc<dyn ObjectStore + 'static>,
+ paths: VecDeque<object_store::path::Path>,
+}
+
+impl ObjectStoreDeleter {
+ pub(crate) fn new(store: Arc<dyn ObjectStore + 'static>) -> Self {
+ Self {
+ store,
+ paths: VecDeque::new(),
+ }
+ }
+}
+
+impl oio::Delete for ObjectStoreDeleter {
+ fn delete(&mut self, path: &str, _: OpDelete) -> Result<()> {
+ self.paths.push_back(object_store::path::Path::from(path));
+ Ok(())
+ }
+
+ fn flush(&mut self) -> impl Future<Output = Result<usize>> + MaybeSend {
+ async move {
+ if self.paths.is_empty() {
+ return Ok(0);
+ }
+
+ let mut count = 0;
+ while let Some(path) = self.paths.front() {
+ match self.store.delete(path).await {
+ Ok(_) => {
+ self.paths.pop_front();
Review Comment:
[nitpick] The deletion loop uses `front()` followed by `pop_front()`. You
can simplify this to `while let Some(path) = self.paths.pop_front()` to
streamline the queue processing.
```suggestion
while let Some(path) = self.paths.pop_front() {
match self.store.delete(&path).await {
Ok(_) => {
```
--
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: commits-unsubscr...@opendal.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
