Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Add list_folders Function #1

Open
PLLX76 opened this issue Dec 3, 2024 · 3 comments
Open

Feature Request: Add list_folders Function #1

PLLX76 opened this issue Dec 3, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@PLLX76
Copy link

PLLX76 commented Dec 3, 2024

Feature Request: Add list_folders Function

Hello,
Here is my feature request. I’d like to mention that it was written with the help of ChatGPT, as I’m not very confident in my English. I hope that’s okay with you.

Is your feature request related to a problem? Please describe.

In my current project, I need to list folders within a given directory using the Supabase storage client implemented in this library. While attempting to use the list_files function on a directory containing only folders, I encountered the following error:

called `Result::unwrap()` on an `Err` value: StorageError { status: 200, message: "[{\"name\":\"104\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"19\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"240\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"4\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"5\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"51\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"52\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"6\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null},{\"name\":\"85\",\"id\":null,\"updated_at\":null,\"created_at\":null,\"last_accessed_at\":null,\"metadata\":null}]" }

This behavior suggests a potential bug in handling directories that contain only folders. As a result, the current implementation of list_files is not sufficient for listing folders.

Describe the solution you'd like

I propose adding a dedicated list_folders function to the library. This function would:

  1. List all folders within a given directory.
  2. Avoid errors like the one above by explicitly supporting folder-only structures.
  3. Return folder metadata in a structured and consistent format (e.g., names, creation dates, etc.).

This feature would enhance the library's usability and align it with the goal of providing a complete and easy-to-use API for Supabase storage.

Describe alternatives you've considered

To work around the issue, I implemented a simplified version of the list_folders function using a custom trait. While this solution works for now, it is limited in scope and lacks the robustness and maintainability of a built-in library function.

Additional context

The issue appears to stem from how the list_files function interprets and handles folder metadata. Here's an example of the data returned in the error:

[{"name":"104","id":null,"updated_at":null,"created_at":null,"last_accessed_at":null,"metadata":null},...]

A dedicated list_folders function could resolve such ambiguities and ensure compatibility with folder-specific use cases.

Let me know if additional details or code examples would be helpful.

@PLLX76 PLLX76 added the enhancement New feature or request label Dec 3, 2024
Proziam added a commit that referenced this issue Dec 4, 2024
Previously, list_files would fail to parse folders due to missing
fields.
Proziam added a commit that referenced this issue Dec 4, 2024
Proziam added a commit that referenced this issue Dec 4, 2024
@Proziam
Copy link
Collaborator

Proziam commented Dec 4, 2024

Thanks for the excellent report!

Version 0.1.5 should fix the error. This was caused when parsing folders, which had empty fields which weren't optional on the models. I've updated the test to catch cases like this in the future as well.

You can create a list of folders from your returned Vec<FileObject> by looking for objects that have only a name, with all other fields being empty. If you find that to be inconvenient I'll take a look at adding another method or a helper function.

@PLLX76
Copy link
Author

PLLX76 commented Dec 4, 2024

Thank you for the quick fix !
I don’t mind performing the verification myself for the folders, but it might not be very intuitive for new users.

@Proziam
Copy link
Collaborator

Proziam commented Dec 5, 2024

Understood. I'll add some docs explaining this for now while I think about what kind of solution would be the least cumbersome.

Perhaps a few others will offer some suggestions as to what they think would be the most user friendly option, so I'll leave this open for discussion.

Proziam added a commit that referenced this issue Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants