diff --git a/.changeset/seven-brooms-applaud.md b/.changeset/seven-brooms-applaud.md new file mode 100644 index 0000000000..5402b08f46 --- /dev/null +++ b/.changeset/seven-brooms-applaud.md @@ -0,0 +1,5 @@ +--- +'@directus/storage-driver-supabase': patch +--- + +Fixed `list` method to recursively list all files under a prefix diff --git a/packages/storage-driver-supabase/src/index.test.ts b/packages/storage-driver-supabase/src/index.test.ts index a547170789..bc04cc4e0a 100644 --- a/packages/storage-driver-supabase/src/index.test.ts +++ b/packages/storage-driver-supabase/src/index.test.ts @@ -10,6 +10,7 @@ import { randPastDate, randText, randGitShortSha as randUnique, + randFileName, } from '@ngneat/falso'; import { Response, fetch } from 'undici'; import { Readable } from 'node:stream'; @@ -49,7 +50,7 @@ beforeEach(() => { serviceRole: randAlphaNumeric({ length: 40 }).join(''), bucket: randBucket(), projectId: randAlphaNumeric({ length: 10 }).join(''), - root: randDirectoryPath(), + root: randUnique() + randDirectoryPath(), endpoint: randDomainName(), }, path: { @@ -104,7 +105,8 @@ describe('#constructor', () => { test('Saves passed config to local property', () => { const driver = new DriverSupabase(sample.config); - expect(driver['config']).toBe(sample.config); + + expect(driver['config']).toStrictEqual(sample.config); }); test('Creates shared client', () => { @@ -114,7 +116,7 @@ describe('#constructor', () => { }); test('Defaults root to empty string', () => { - expect(driver['config'].root).toBe(undefined); + expect(driver['config'].root).toBe(''); }); }); @@ -170,7 +172,7 @@ describe('#getClient', () => { }); }); -describe('#getFullPath', () => { +describe('#fullPath', () => { test('Returns the input value if no root is given', () => { const driver = new DriverSupabase({ serviceRole: sample.config.serviceRole, @@ -178,7 +180,7 @@ describe('#getFullPath', () => { endpoint: sample.config.endpoint, }); - const result = driver['getFullPath'](sample.path.input); + const result = driver['fullPath'](sample.path.input); expect(result).toBe(sample.path.input); }); @@ -190,7 +192,7 @@ describe('#getFullPath', () => { root: sample.config.root, }); - const result = driver['getFullPath'](sample.path.input); + const result = driver['fullPath'](sample.path.input); expect(result).toBe(`${sample.config.root}/${sample.path.input}`); }); }); @@ -462,11 +464,11 @@ describe('#write', () => { }); }); - test('Ensures input is passed to getFullPath', async () => { - driver['getFullPath'] = vi.fn(); + test('Ensures input is passed to fullPath', async () => { + driver['fullPath'] = vi.fn(); await driver.write(sample.path.input, sample.stream); - expect(driver['getFullPath']).toHaveBeenCalledWith(sample.path.input); + expect(driver['fullPath']).toHaveBeenCalledWith(sample.path.input); }); test('Optionally sets ContentType', async () => { @@ -482,44 +484,49 @@ describe('#write', () => { }); describe('#delete', () => { - test('Ensures input is passed to getFullPath', async () => { + test('Ensures input is passed to fullPath', async () => { driver['bucket'] = { remove: vi.fn(), } as any; - driver['getFullPath'] = vi.fn(); + driver['fullPath'] = vi.fn(); await driver.delete(sample.path.input); - expect(driver['getFullPath']).toHaveBeenCalledWith(sample.path.input); + expect(driver['fullPath']).toHaveBeenCalledWith(sample.path.input); }); }); describe('#list', () => { test('Constructs list objects params based on input prefix', async () => { + const sampleFile = randFileName(); + const sampleDirectory = randDirectoryPath(); + const fullSample = `${sampleDirectory}/${sampleFile}`; + // TODO: Probably a better way to do this? driver['bucket'] = { list: vi.fn().mockReturnValue({ data: [], error: null }), } as any; - await driver.list(sample.path.input).next(); + await driver.list(fullSample)[Symbol.asyncIterator]().next(); - expect(driver['bucket'].list).toHaveBeenCalledWith(undefined, { + expect(driver['bucket'].list).toHaveBeenCalledWith(sampleDirectory, { + search: sampleFile, limit: 1000, offset: 0, - search: sample.path.input, }); }); - test('Yields file name omitting root', async () => { + test('Yields file name omitting root if prefix is the full file path', async () => { const sampleRoot = randDirectoryPath(); - const sampleFile = randFilePath(); + const sampleFile = randFileName(); + const sampleFull = `${sample.path.input}/${sampleFile}`; - // TODO: Probably a better way to do this? driver['bucket'] = { - list: vi.fn().mockReturnValue({ + list: vi.fn().mockResolvedValueOnce({ data: [ { name: sampleFile, + id: randUnique(), }, ], error: null, @@ -528,20 +535,142 @@ describe('#list', () => { driver['config'].root = sampleRoot; - const iterator = driver.list(sample.path.input); + const iterator = driver.list(sampleFull); const output: string[] = []; for await (const filepath of iterator) { output.push(filepath); } - expect(driver['bucket'].list).toHaveBeenCalledWith(sampleRoot, { - limit: 1000, - offset: 0, - search: sample.path.input, - }); + expect(output).toStrictEqual([sampleFull]); + }); - expect(output).toStrictEqual([sampleFile]); + test('Yields file name omitting root if prefix is the parent directory', async () => { + const sampleRoot = randDirectoryPath(); + const sampleFile = randFileName(); + const sampleParentDir = randUnique(); + const sampleInput = `${sample.path.input}/${sampleParentDir}`; + const sampleFull = `${sampleInput}/${sampleFile}`; + + driver['bucket'] = { + list: vi + .fn() + .mockResolvedValueOnce({ + data: [ + { + name: sampleParentDir, + id: null, + }, + ], + error: null, + }) + .mockResolvedValueOnce({ + data: [ + { + name: sampleFile, + id: randUnique(), + }, + ], + error: null, + }), + } as any; + + driver['config'].root = sampleRoot; + + const iterator = driver.list(sampleInput); + const output: string[] = []; + + for await (const filepath of iterator) { + output.push(filepath); + } + + expect(driver['bucket'].list).toHaveBeenCalledTimes(2); + expect(output).toStrictEqual([sampleFull]); + }); + + test('Yields file name omitting root if prefix is part of the file name', async () => { + const sampleRoot = randDirectoryPath(); + const sampleFilePrefix = randFileName(); + const sampleFiles = [1, 2, 3].map((i) => `${sampleFilePrefix}_postfix${i}`); + const sampleInput = `${sample.path.input}/${sampleFilePrefix}`; + const sampleFilesFull = sampleFiles.map((name) => `${sample.path.input}/${name}`); + + driver['bucket'] = { + list: vi.fn().mockResolvedValueOnce({ + data: sampleFiles.map((name) => ({ + name, + id: randUnique(), + })), + error: null, + }), + } as any; + + driver['config'].root = sampleRoot; + + const iterator = driver.list(sampleInput); + const output: string[] = []; + + for await (const filepath of iterator) { + output.push(filepath); + } + + expect(output).toStrictEqual(sampleFilesFull); + }); + + test('Recursively fetches all nested directories and yields only the files', async () => { + const sampleRoot = randUnique() + randDirectoryPath(); + const samplePrefixBase = randUnique() + randDirectoryPath(); + const samplePrefixLastDir = randUnique(); + const samplePrefix = `${samplePrefixBase}/${samplePrefixLastDir}`; + + /* + sampleFile + sampleDirectory/ + ├─ sampleFileNested + */ + const sampleDirectory = randUnique(); + const sampleFile = randFileName(); + const sampleFileNested = randFileName(); + + const fullSampleDirectory = `${samplePrefix}/${sampleDirectory}`; + const fullSampleFile = `${samplePrefix}/${sampleFile}`; + const fullSampleFileNested = `${fullSampleDirectory}/${sampleFileNested}`; + + driver['bucket'] = { + list: vi.fn(async (path, options): Promise => { + // query for parent dir, return the contained dir + if (path === `${sampleRoot}/${samplePrefixBase}` && options?.search === samplePrefixLastDir) + return { data: [{ name: samplePrefixLastDir, id: null }], error: null }; + // query for the contents of the samplePrefix, return file and directory + if (path === `${sampleRoot}/${samplePrefix}/` && options?.search === '') + return { + data: [ + { name: sampleDirectory, id: null }, + { name: sampleFile, id: randUnique() }, + ], + error: null, + }; + // query for the contents of the sampleDirectory, return the nested file + if (path === `${sampleRoot}/${fullSampleDirectory}/` && options?.search === '') + return { + data: [{ name: sampleFileNested, id: randUnique() }], + error: null, + }; + throw Error(); + }), + } as any; + + driver['config'].root = sampleRoot; + + const iterator = driver.list(samplePrefix); + const output: string[] = []; + + for await (const filepath of iterator) { + output.push(filepath); + } + + expect(driver['bucket'].list).toHaveBeenCalledTimes(3); + expect(output).toStrictEqual([fullSampleFileNested, fullSampleFile]); }); test('Continuously fetches until all pages are returned', async () => { diff --git a/packages/storage-driver-supabase/src/index.ts b/packages/storage-driver-supabase/src/index.ts index 40e49a1d46..1c17a54f18 100644 --- a/packages/storage-driver-supabase/src/index.ts +++ b/packages/storage-driver-supabase/src/index.ts @@ -10,18 +10,22 @@ export type DriverSupabaseConfig = { bucket: string; serviceRole: string; projectId?: string; - // Allows a custom Supabase endpoint incase self-hosting + /** Allows a custom Supabase endpoint for self-hosting */ endpoint?: string; root?: string; }; export class DriverSupabase implements Driver { - private config: DriverSupabaseConfig; + private config: DriverSupabaseConfig & { root: string }; private client: StorageClient; private bucket: ReturnType; constructor(config: DriverSupabaseConfig) { - this.config = config; + this.config = { + ...config, + root: normalizePath(config.root ?? '', { removeLeading: true }), + }; + this.client = this.getClient(); this.bucket = this.getBucket(); } @@ -53,12 +57,15 @@ export class DriverSupabase implements Driver { return this.client.from(this.config.bucket); } - private getFullPath(filepath: string) { - return this.config.root ? normalizePath(join(this.config.root, filepath)) : filepath; + private fullPath(filepath: string) { + const path = join(this.config.root, filepath); + // Supabase expects an empty string for current directory + if (path === '.') return ''; + return normalizePath(path); } private getAuthenticatedUrl(filepath: string) { - return `${this.endpoint}/${join('object/authenticated', this.config.bucket, this.getFullPath(filepath))}`; + return `${this.endpoint}/${join('object/authenticated', this.config.bucket, this.fullPath(filepath))}`; } async read(filepath: string, range?: Range) { @@ -115,15 +122,15 @@ export class DriverSupabase implements Driver { } async move(src: string, dest: string) { - await this.bucket.move(this.getFullPath(src), this.getFullPath(dest)); + await this.bucket.move(this.fullPath(src), this.fullPath(dest)); } async copy(src: string, dest: string) { - await this.bucket.copy(this.getFullPath(src), this.getFullPath(dest)); + await this.bucket.copy(this.fullPath(src), this.fullPath(dest)); } async write(filepath: string, content: Readable, type?: string) { - await this.bucket.upload(this.getFullPath(filepath), content, { + await this.bucket.upload(this.fullPath(filepath), content, { contentType: type ?? '', cacheControl: '3600', upsert: true, @@ -132,16 +139,52 @@ export class DriverSupabase implements Driver { } async delete(filepath: string) { - await this.bucket.remove([this.getFullPath(filepath)]); + await this.bucket.remove([this.fullPath(filepath)]); } - async *list(prefix = '') { + list(prefix = ''): AsyncIterable { + const fullPrefix = this.fullPath(prefix); + return this.listGenerator(fullPrefix); + } + + async *listGenerator(prefix: string): AsyncIterable { const limit = 1000; let offset = 0; let itemCount = 0; + /* + * The Supabase API only returns the directories and files directly within the queried location + * (called prefix in their API) matching the search query. Directories can be identified by the id being null. + * + * Since it's unknown whether the last part of the prefix param is a directory, a file or part of a directory or file name, + * the first query will be the largest common denominator (the parent directory part of the prefix) + * and the results then filtered with the remaining part of the query. + * + * This can lead to the following outcomes: + * 1. The full prefix is a file and is split up into parentdir/filename, the API is queried with + * { prefix: parentdir, search: filename } and returns one result { name: filename, id: "..." }. + * The filename is yielded in that case. + * 2. The full prefix is a directory and is split up into parentdir/dir, the API is queried with + * { prefix: parentdir, search: dir } and returns one result { name: dir, id: null } and importantly, not the + * contents of the directory. Then the contents of the directory must be listed recursively by calling this + * function with the prefix and a trailing "/". + * 2.1. The prefix ends with a "/", the API is queried with { prefix: prefix, search: "" } + * and returns all files and directories within the prefix, which are yielded and recursively listed. + * 3. Special case of 1. and 2.: The prefix is part of a filename/directory name and could result in more than one result + * that all match the search query. The API is queried with { prefix: parentdir, search: part of filename } + * and returns all files and directories in parentdir that match the search query. Filenames are yielded and + * directories are recursively listed. + */ + const isDirectory = prefix.endsWith('/'); + const prefixDirectory = isDirectory ? prefix : dirname(prefix); + const search = isDirectory ? '' : prefix.split('/').pop() ?? ''; + do { - const { data, error } = await this.bucket.list(this.config.root, { limit, offset, search: prefix }); + const { data, error } = await this.bucket.list(prefixDirectory, { + limit, + offset, + search, + }); if (!data || error) { break; @@ -151,10 +194,26 @@ export class DriverSupabase implements Driver { offset += itemCount; for (const item of data) { - yield item.name; + // Since the API only returns the filename, the file path is constructed by joining the prefix with the item name + const filePath = normalizePath(join(prefixDirectory, item.name)); + + if (item.id !== null) { + // Remove the root from the path and yield the filename + yield filePath.substring(this.config.root ? this.config.root.length + 1 : 0); + } else { + // This is a directory, recursively list it + yield* this.listGenerator(`${filePath}/`); + } } } while (itemCount === limit); } } export default DriverSupabase; + +/** + * dirname implementation that always uses '/' to split and returns '' in case of no separator present. + */ +function dirname(path: string) { + return path.split('/').slice(0, -1).join('/'); +}