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

Fix Supabase storage driver list operation #22322

Merged
merged 7 commits into from
Apr 28, 2024

Conversation

hanneskuettner
Copy link
Contributor

Scope

The Driver.list method is expected to yield all files and nested files in the storage device matching the prefix parameter. Up until this point the Supabase storage driver only returned the folder level files and directories that matched with prefix, effectively breaking as soon as prefix contained a path and not just a single filename prefix (and also returning directories, which is not expected in the first place). This worked fine until the need for synching extensions

So instead of querying the API once we need to recursively query the API multiple times to yield all files. This is what Supabase does in their CLI as well to recursively list a directory.1

What's changed:

  • Recursively query the Supabase
  • Added more test cases based on the cases outlined in the large comment in the driver itself

Potential Risks / Drawbacks

None, hopefully. The increased number of API queries are sadly necessary.

Review Notes / Questions

  • I tested it with their hosted service and created a free account, hit me up if you need my credentials for testing.
  • To force a re-sync of your (marketplace) extensions remove the node_modules/.directus/ directory and restart directus.

Fixes #21098

Footnotes

  1. https://github.com/supabase/cli/blob/b5e0e172e1a3a40d4d6f518490c68609941c4f41/internal/storage/ls/ls.go#L44

Copy link

changeset-bot bot commented Apr 26, 2024

🦋 Changeset detected

Latest commit: b030af4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@directus/storage-driver-supabase Patch
@directus/api Patch
directus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@paescuj paescuj self-assigned this Apr 27, 2024
@paescuj paescuj force-pushed the fix-21098-supabase-recursive-list branch from 4c29a8b to b030af4 Compare April 28, 2024 22:56
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Great work!

@paescuj paescuj merged commit 0e19bce into main Apr 28, 2024
4 checks passed
@paescuj paescuj deleted the fix-21098-supabase-recursive-list branch April 28, 2024 23:02
@github-actions github-actions bot added this to the Next Release milestone Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Loading extensions from external storage Supabase not working
2 participants