You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2026/05/23 15:12:07 UTC
[PR] feat(binding/go): Add ListOptions with recursive support [opendal]
FrankYang0529 opened a new pull request, #7605:
URL: https://github.com/apache/opendal/pull/7605
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #7604
# Rationale for this change
The Go binding's op.List(path) had no way to perform a recursive listing. Users who needed to enumerate all objects under a prefix recursively had to implement their own traversal.
# What changes are included in this PR?
Add golang binding `ListWith` and `ListOptions`.
# Are there any user-facing changes?
Users can now perform recursive listings via:
```
opts := opendal.ListOptions{}.WithRecursive(true)
lister, err := op.ListWith("path/to/prefix/", opts)
```
The existing `op.List(path)` API is unchanged.
# AI Usage Statement
This PR was developed with the assistance of OpenCode using the claude-sonnet-4.6 model.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #7605:
URL: https://github.com/apache/opendal/pull/7605#issuecomment-4564200025
Thanks @yuchanns and @dentiny 's review. I will continue working on other option fields.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "yuchanns (via GitHub)" <gi...@apache.org>.
yuchanns commented on PR #7605:
URL: https://github.com/apache/opendal/pull/7605#issuecomment-4528583288
Can we use options pattern here? So List and ListWith can be merged.
```go
func List(path string, opts ...WithListFn)
```
Sorry for inaccurate description. Sent by my phone.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "dentiny (via GitHub)" <gi...@apache.org>.
dentiny commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293235022
##########
bindings/c/tests/test_suites_list.cpp:
##########
@@ -347,6 +347,126 @@ void test_entry_metadata(opendal_test_context* ctx)
opendal_operator_delete(ctx->config->operator_instance, dir_path);
}
+// Test: list_with default options (null opts behaves like list)
+void test_list_with_default_options(opendal_test_context* ctx)
+{
+ const char* dir_path = "test_list_with_default/";
+ const char* file_path = "test_list_with_default/file.txt";
+
+ opendal_error* error = opendal_operator_create_dir(ctx->config->operator_instance, dir_path);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create dir should succeed");
+
+ opendal_bytes data;
+ data.data = (uint8_t*)"content";
+ data.len = 7;
+ data.capacity = 7;
+ error = opendal_operator_write(ctx->config->operator_instance, file_path, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write should succeed");
+
+ // Pass NULL opts — should behave identically to opendal_operator_list
+ opendal_result_list list_result = opendal_operator_list_with(
+ ctx->config->operator_instance, dir_path, NULL);
+ OPENDAL_ASSERT_NO_ERROR(list_result.error, "list_with(NULL opts) should succeed");
+ OPENDAL_ASSERT_NOT_NULL(list_result.lister, "Lister should not be null");
+
+ bool found_file = false;
+ while (true) {
+ opendal_result_lister_next next = opendal_lister_next(list_result.lister);
+ if (next.error) {
+ OPENDAL_ASSERT_NO_ERROR(next.error, "lister_next should not fail");
+ break;
+ }
+ if (!next.entry) break;
+
+ char* path = opendal_entry_path(next.entry);
+ if (strcmp(path, file_path) == 0) {
+ found_file = true;
+ }
+ opendal_string_free(path);
+ opendal_entry_free(next.entry);
+ }
+
+ OPENDAL_ASSERT(found_file, "Should find the file with null opts");
+
+ opendal_lister_free(list_result.lister);
+ opendal_operator_delete(ctx->config->operator_instance, file_path);
+ opendal_operator_delete(ctx->config->operator_instance, dir_path);
+}
+
+// Test: list_with recursive=true returns entries in nested directories
+void test_list_with_recursive(opendal_test_context* ctx)
+{
+ const char* base_dir = "test_list_recursive/";
+ const char* sub_dir = "test_list_recursive/sub/";
+ const char* deep_dir = "test_list_recursive/sub/deep/";
+ const char* file_top = "test_list_recursive/top.txt";
+ const char* file_sub = "test_list_recursive/sub/mid.txt";
+ const char* file_deep = "test_list_recursive/sub/deep/bottom.txt";
+
+ opendal_bytes data;
+ data.data = (uint8_t*)"x";
+ data.len = 1;
+ data.capacity = 1;
+
+ opendal_error* error;
+ error = opendal_operator_create_dir(ctx->config->operator_instance, base_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create base dir should succeed");
+ error = opendal_operator_create_dir(ctx->config->operator_instance, sub_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create sub dir should succeed");
+ error = opendal_operator_create_dir(ctx->config->operator_instance, deep_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create deep dir should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_top, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write top file should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_sub, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write sub file should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_deep, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write deep file should succeed");
+
+ // List recursively from base_dir
+ opendal_list_options* opts = opendal_list_options_new();
+ OPENDAL_ASSERT_NOT_NULL(opts, "list_options_new should not return NULL");
+ opendal_list_options_set_recursive(opts, true);
+
+ opendal_result_list list_result = opendal_operator_list_with(
+ ctx->config->operator_instance, base_dir, opts);
+ opendal_list_options_free(opts);
+
+ OPENDAL_ASSERT_NO_ERROR(list_result.error, "Recursive list should succeed");
+ OPENDAL_ASSERT_NOT_NULL(list_result.lister, "Lister should not be null");
+
+ std::set<std::string> found_paths;
Review Comment:
```suggestion
std::unordered_set<std::string> found_paths;
```
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "dentiny (via GitHub)" <gi...@apache.org>.
dentiny commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293223904
##########
bindings/c/include/opendal.h:
##########
@@ -1227,6 +1246,33 @@ struct opendal_result_stat opendal_operator_stat(const struct opendal_operator *
struct opendal_result_list opendal_operator_list(const struct opendal_operator *op,
const char *path);
+/**
+ * \brief Blocking list the objects in `path` with options.
+ *
+ * List the objects in `path` with the provided `opendal_list_options`. This is
+ * similar to `opendal_operator_list` but allows passing options such as
+ * `recursive` to control the listing behavior.
+ *
+ * @param op The opendal_operator created previously
+ * @param path The designated path you want to list
+ * @param opts The options for the list operation; pass NULL to use defaults
+ * @see opendal_lister
+ * @see opendal_list_options
+ * @return Returns opendal_result_list, containing a lister and an opendal_error.
+ *
+ * # Safety
+ *
+ * * The memory pointed to by `path` must contain a valid nul terminator at the end of
Review Comment:
```suggestion
* * The memory pointed to by `path` must contain a valid null terminator at the end of
```
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "dentiny (via GitHub)" <gi...@apache.org>.
dentiny commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293241742
##########
bindings/go/types.go:
##########
@@ -306,6 +306,10 @@ type opendalResultList struct {
type opendalLister struct{}
+type opendalListOptions struct {
+ recursive uint8
Review Comment:
curious why do we use uint8 instead of bool here?
##########
bindings/c/tests/test_suites_list.cpp:
##########
@@ -347,6 +347,126 @@ void test_entry_metadata(opendal_test_context* ctx)
opendal_operator_delete(ctx->config->operator_instance, dir_path);
}
+// Test: list_with default options (null opts behaves like list)
+void test_list_with_default_options(opendal_test_context* ctx)
+{
+ const char* dir_path = "test_list_with_default/";
+ const char* file_path = "test_list_with_default/file.txt";
+
+ opendal_error* error = opendal_operator_create_dir(ctx->config->operator_instance, dir_path);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create dir should succeed");
+
+ opendal_bytes data;
+ data.data = (uint8_t*)"content";
+ data.len = 7;
+ data.capacity = 7;
+ error = opendal_operator_write(ctx->config->operator_instance, file_path, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write should succeed");
+
+ // Pass NULL opts — should behave identically to opendal_operator_list
+ opendal_result_list list_result = opendal_operator_list_with(
+ ctx->config->operator_instance, dir_path, NULL);
+ OPENDAL_ASSERT_NO_ERROR(list_result.error, "list_with(NULL opts) should succeed");
+ OPENDAL_ASSERT_NOT_NULL(list_result.lister, "Lister should not be null");
+
+ bool found_file = false;
+ while (true) {
+ opendal_result_lister_next next = opendal_lister_next(list_result.lister);
+ if (next.error) {
+ OPENDAL_ASSERT_NO_ERROR(next.error, "lister_next should not fail");
+ break;
+ }
+ if (!next.entry) break;
+
+ char* path = opendal_entry_path(next.entry);
+ if (strcmp(path, file_path) == 0) {
+ found_file = true;
+ }
+ opendal_string_free(path);
+ opendal_entry_free(next.entry);
+ }
+
+ OPENDAL_ASSERT(found_file, "Should find the file with null opts");
+
+ opendal_lister_free(list_result.lister);
+ opendal_operator_delete(ctx->config->operator_instance, file_path);
+ opendal_operator_delete(ctx->config->operator_instance, dir_path);
+}
+
+// Test: list_with recursive=true returns entries in nested directories
+void test_list_with_recursive(opendal_test_context* ctx)
+{
+ const char* base_dir = "test_list_recursive/";
+ const char* sub_dir = "test_list_recursive/sub/";
+ const char* deep_dir = "test_list_recursive/sub/deep/";
+ const char* file_top = "test_list_recursive/top.txt";
+ const char* file_sub = "test_list_recursive/sub/mid.txt";
+ const char* file_deep = "test_list_recursive/sub/deep/bottom.txt";
+
+ opendal_bytes data;
+ data.data = (uint8_t*)"x";
+ data.len = 1;
+ data.capacity = 1;
+
+ opendal_error* error;
+ error = opendal_operator_create_dir(ctx->config->operator_instance, base_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create base dir should succeed");
+ error = opendal_operator_create_dir(ctx->config->operator_instance, sub_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create sub dir should succeed");
+ error = opendal_operator_create_dir(ctx->config->operator_instance, deep_dir);
+ OPENDAL_ASSERT_NO_ERROR(error, "Create deep dir should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_top, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write top file should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_sub, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write sub file should succeed");
+ error = opendal_operator_write(ctx->config->operator_instance, file_deep, &data);
+ OPENDAL_ASSERT_NO_ERROR(error, "Write deep file should succeed");
+
+ // List recursively from base_dir
+ opendal_list_options* opts = opendal_list_options_new();
+ OPENDAL_ASSERT_NOT_NULL(opts, "list_options_new should not return NULL");
+ opendal_list_options_set_recursive(opts, true);
+
+ opendal_result_list list_result = opendal_operator_list_with(
+ ctx->config->operator_instance, base_dir, opts);
+ opendal_list_options_free(opts);
+
+ OPENDAL_ASSERT_NO_ERROR(list_result.error, "Recursive list should succeed");
+ OPENDAL_ASSERT_NOT_NULL(list_result.lister, "Lister should not be null");
+
+ std::set<std::string> found_paths;
Review Comment:
```suggestion
std::unordered_map<std::string> found_paths;
```
##########
bindings/go/lister.go:
##########
@@ -87,8 +87,7 @@ func (op *Operator) Check() (err error) {
//
// # Notes
//
-// 1. List is a wrapper around the C-binding function `opendal_operator_list`. Recursive listing is not currently supported.
-// 2. Returned entries do not include metadata information. Use op.Stat to fetch metadata for individual entries.
+// 1. Returned entries do not include metadata information. Use op.Stat to fetch metadata for individual entries.
Review Comment:
No need, I think I've added support last week :)
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "yuchanns (via GitHub)" <gi...@apache.org>.
yuchanns commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3304226968
##########
bindings/go/lister.go:
##########
@@ -74,69 +74,88 @@ func (op *Operator) Check() (err error) {
// List returns a Lister to iterate over entries that start with the given path in the parent directory.
//
-// This function creates a new Lister to enumerate entries in the specified path.
+// WithListFn is a functional option for the List operation.
+type WithListFn func(*listOptions)
+
+// WithRecursive sets the recursive flag for the list operation.
+//
+// When recursive is true, the list operation will descend into sub-directories.
+func WithRecursive(recursive bool) WithListFn {
+ return func(o *listOptions) {
+ o.recursive = recursive
+ }
+}
+
+// listOptions holds the options for a list operation.
+type listOptions struct {
+ recursive bool
+}
+
+func (o *listOptions) toInner() opendalListOptions {
+ return opendalListOptions{recursive: o.recursive}
Review Comment:
Can we use `opendal_list_options_set_recursive` here instead of constructing `opendalListOptions` directly?
I'm afraid mirroring the C struct layout in Go could introduce potential memory/layout risks if `opendal_list_options` changes later. IMO using the C setter `opendal_list_options_set_recursive` would keep the ownership and initialization details on the C side.
##########
bindings/go/lister.go:
##########
@@ -74,69 +74,88 @@ func (op *Operator) Check() (err error) {
// List returns a Lister to iterate over entries that start with the given path in the parent directory.
//
-// This function creates a new Lister to enumerate entries in the specified path.
+// WithListFn is a functional option for the List operation.
+type WithListFn func(*listOptions)
+
+// WithRecursive sets the recursive flag for the list operation.
+//
+// When recursive is true, the list operation will descend into sub-directories.
+func WithRecursive(recursive bool) WithListFn {
Review Comment:
I'd prefer `ListWithRecursive` here.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3317798758
##########
bindings/go/lister.go:
##########
@@ -74,69 +74,88 @@ func (op *Operator) Check() (err error) {
// List returns a Lister to iterate over entries that start with the given path in the parent directory.
//
-// This function creates a new Lister to enumerate entries in the specified path.
+// WithListFn is a functional option for the List operation.
+type WithListFn func(*listOptions)
+
+// WithRecursive sets the recursive flag for the list operation.
+//
+// When recursive is true, the list operation will descend into sub-directories.
+func WithRecursive(recursive bool) WithListFn {
+ return func(o *listOptions) {
+ o.recursive = recursive
+ }
+}
+
+// listOptions holds the options for a list operation.
+type listOptions struct {
+ recursive bool
+}
+
+func (o *listOptions) toInner() opendalListOptions {
+ return opendalListOptions{recursive: o.recursive}
Review Comment:
Thanks for the great suggestion. Updated it.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293938403
##########
bindings/c/include/opendal.h:
##########
@@ -426,6 +426,25 @@ typedef struct opendal_result_list {
struct opendal_error *error;
} opendal_result_list;
+/**
+ * \brief The options for the list operation.
+ *
+ * This struct carries the options for the list operation, including whether to
+ * list recursively. Use `opendal_list_options_new()` to construct and
+ * `opendal_list_options_free()` to free.
+ *
+ * @see opendal_operator_list_with
+ * @see opendal_list_options_new
+ * @see opendal_list_options_free
+ * @see opendal_list_options_set_recursive
+ */
+typedef struct opendal_list_options {
+ /**
Review Comment:
I would prefer to do each function in separate PR. WDYT?
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293949560
##########
bindings/go/types.go:
##########
@@ -306,6 +306,10 @@ type opendalResultList struct {
type opendalLister struct{}
+type opendalListOptions struct {
+ recursive uint8
Review Comment:
I think it's better to use bool here. Updated it.
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "dentiny (via GitHub)" <gi...@apache.org>.
dentiny commented on code in PR #7605:
URL: https://github.com/apache/opendal/pull/7605#discussion_r3293219795
##########
bindings/c/include/opendal.h:
##########
@@ -426,6 +426,25 @@ typedef struct opendal_result_list {
struct opendal_error *error;
} opendal_result_list;
+/**
+ * \brief The options for the list operation.
+ *
+ * This struct carries the options for the list operation, including whether to
+ * list recursively. Use `opendal_list_options_new()` to construct and
+ * `opendal_list_options_free()` to free.
+ *
+ * @see opendal_operator_list_with
+ * @see opendal_list_options_new
+ * @see opendal_list_options_free
+ * @see opendal_list_options_set_recursive
+ */
+typedef struct opendal_list_options {
+ /**
Review Comment:
There're a few list options in opendal core, I'm wondering why don't expose them all?
https://opendal.incubator.apache.org/docs/rust/opendal/options/struct.ListOptions.html
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(binding/go): Add ListOptions with recursive support [opendal]
Posted by "yuchanns (via GitHub)" <gi...@apache.org>.
yuchanns merged PR #7605:
URL: https://github.com/apache/opendal/pull/7605
--
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-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org