diff --git a/migrations/tenant/0063-fix-search-name-relative-to-prefix.sql b/migrations/tenant/0063-fix-search-name-relative-to-prefix.sql new file mode 100644 index 000000000..11c38a589 --- /dev/null +++ b/migrations/tenant/0063-fix-search-name-relative-to-prefix.sql @@ -0,0 +1,285 @@ +-- fixes issue with search param not returning name relative to prefix; the level input is +-- no longer used (both boundaries are now derived internally from prefix/search) but is kept +-- as an unused parameter so the signature stays backwards compatible across a rolling deploy +-- based on prefix case fix version defined in 0056 which is based on original version defined in 0050 +-- ============================================================================ +-- search: Legacy function with offset-based pagination using hybrid skip-scan +-- ============================================================================ +-- Maintains backwards compatibility with the original search function signature. +-- Uses HYBRID approach for optimal performance: +-- 1. STATIC SQL peek for folder discovery (plan cached, very fast) +-- 2. DYNAMIC SQL batch for files (overhead amortized over many rows) +-- Falls back to path_tokens approach for non-name sorting. +-- ============================================================================ +CREATE OR REPLACE FUNCTION storage.search( + prefix text, + bucketname text, + limits int DEFAULT 100, + levels int DEFAULT 1, + offsets int DEFAULT 0, + search text DEFAULT '', + sortcolumn text DEFAULT 'name', + sortorder text DEFAULT 'asc' +) +RETURNS TABLE ( + name text, + id uuid, + updated_at timestamptz, + created_at timestamptz, + last_accessed_at timestamptz, + metadata jsonb +) +SECURITY INVOKER +LANGUAGE plpgsql STABLE +AS $func$ +DECLARE + v_peek_name TEXT; + v_current RECORD; + v_common_prefix TEXT; + v_delimiter CONSTANT TEXT := '/'; + + -- Configuration + v_limit INT; + v_prefix TEXT; + v_prefix_lower TEXT; + v_prefix_len INT; + v_prefix_start INT; + v_combined_levels INT; + v_is_asc BOOLEAN; + v_order_by TEXT; + v_sort_order TEXT; + v_upper_bound TEXT; + v_file_batch_size INT; + + -- Dynamic SQL for batch query only + v_batch_query TEXT; + + -- Seek state + v_next_seek TEXT; + v_count INT := 0; + v_skipped INT := 0; +BEGIN + -- ======================================================================== + -- INITIALIZATION + -- ======================================================================== + v_limit := LEAST(coalesce(limits, 100), 1500); + v_prefix := coalesce(prefix, '') || coalesce(search, ''); + v_prefix_lower := lower(v_prefix); + v_prefix_len := length(coalesce(prefix, '')); + v_prefix_start := coalesce(array_length(string_to_array(coalesce(prefix, ''), v_delimiter), 1), 1); + v_combined_levels := coalesce(array_length(string_to_array(v_prefix, v_delimiter), 1), 1); + v_is_asc := lower(coalesce(sortorder, 'asc')) = 'asc'; + v_file_batch_size := LEAST(GREATEST(v_limit * 2, 100), 1000); + + -- Validate sort column + CASE lower(coalesce(sortcolumn, 'name')) + WHEN 'name' THEN v_order_by := 'name'; + WHEN 'updated_at' THEN v_order_by := 'updated_at'; + WHEN 'created_at' THEN v_order_by := 'created_at'; + WHEN 'last_accessed_at' THEN v_order_by := 'last_accessed_at'; + ELSE v_order_by := 'name'; + END CASE; + + v_sort_order := CASE WHEN v_is_asc THEN 'asc' ELSE 'desc' END; + + -- ======================================================================== + -- NON-NAME SORTING: Use path_tokens approach + -- ======================================================================== + IF v_order_by != 'name' THEN + RETURN QUERY EXECUTE format( + $sql$ + WITH folders AS ( + SELECT array_to_string(path_tokens[$1:$2], '/') AS folder + FROM storage.objects + WHERE objects.name ILIKE $3 || '%%' + AND bucket_id = $4 + AND array_length(objects.path_tokens, 1) <> $2 + GROUP BY folder + ORDER BY folder %s + ) + (SELECT folder AS "name", + NULL::uuid AS id, + NULL::timestamptz AS updated_at, + NULL::timestamptz AS created_at, + NULL::timestamptz AS last_accessed_at, + NULL::jsonb AS metadata FROM folders) + UNION ALL + (SELECT array_to_string(path_tokens[$1:$2], '/') AS "name", + id, updated_at, created_at, last_accessed_at, metadata + FROM storage.objects + WHERE objects.name ILIKE $3 || '%%' + AND bucket_id = $4 + AND array_length(objects.path_tokens, 1) = $2 + ORDER BY %I %s) + LIMIT $5 OFFSET $6 + $sql$, v_sort_order, v_order_by, v_sort_order + ) USING v_prefix_start, v_combined_levels, v_prefix, bucketname, v_limit, offsets; + RETURN; + END IF; + + -- ======================================================================== + -- NAME SORTING: Hybrid skip-scan with batch optimization + -- ======================================================================== + + -- Calculate upper bound for prefix filtering + IF v_prefix_lower = '' THEN + v_upper_bound := NULL; + ELSIF right(v_prefix_lower, 1) = v_delimiter THEN + v_upper_bound := left(v_prefix_lower, -1) || chr(ascii(v_delimiter) + 1); + ELSE + v_upper_bound := left(v_prefix_lower, -1) || chr(ascii(right(v_prefix_lower, 1)) + 1); + END IF; + + -- Build batch query (dynamic SQL - called infrequently, amortized over many rows) + IF v_is_asc THEN + IF v_upper_bound IS NOT NULL THEN + v_batch_query := 'SELECT o.name, o.id, o.updated_at, o.created_at, o.last_accessed_at, o.metadata ' || + 'FROM storage.objects o WHERE o.bucket_id = $1 AND lower(o.name) COLLATE "C" >= $2 ' || + 'AND lower(o.name) COLLATE "C" < $3 ORDER BY lower(o.name) COLLATE "C" ASC LIMIT $4'; + ELSE + v_batch_query := 'SELECT o.name, o.id, o.updated_at, o.created_at, o.last_accessed_at, o.metadata ' || + 'FROM storage.objects o WHERE o.bucket_id = $1 AND lower(o.name) COLLATE "C" >= $2 ' || + 'ORDER BY lower(o.name) COLLATE "C" ASC LIMIT $4'; + END IF; + ELSE + IF v_upper_bound IS NOT NULL THEN + v_batch_query := 'SELECT o.name, o.id, o.updated_at, o.created_at, o.last_accessed_at, o.metadata ' || + 'FROM storage.objects o WHERE o.bucket_id = $1 AND lower(o.name) COLLATE "C" < $2 ' || + 'AND lower(o.name) COLLATE "C" >= $3 ORDER BY lower(o.name) COLLATE "C" DESC LIMIT $4'; + ELSE + v_batch_query := 'SELECT o.name, o.id, o.updated_at, o.created_at, o.last_accessed_at, o.metadata ' || + 'FROM storage.objects o WHERE o.bucket_id = $1 AND lower(o.name) COLLATE "C" < $2 ' || + 'ORDER BY lower(o.name) COLLATE "C" DESC LIMIT $4'; + END IF; + END IF; + + -- Initialize seek position + IF v_is_asc THEN + v_next_seek := v_prefix_lower; + ELSE + -- DESC: find the last item in range first (static SQL) + IF v_upper_bound IS NOT NULL THEN + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" >= v_prefix_lower AND lower(o.name) COLLATE "C" < v_upper_bound + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + ELSIF v_prefix_lower <> '' THEN + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" >= v_prefix_lower + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + ELSE + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + END IF; + + IF v_peek_name IS NOT NULL THEN + v_next_seek := lower(v_peek_name) || v_delimiter; + ELSE + RETURN; + END IF; + END IF; + + -- ======================================================================== + -- MAIN LOOP: Hybrid peek-then-batch algorithm + -- Uses STATIC SQL for peek (hot path) and DYNAMIC SQL for batch + -- ======================================================================== + LOOP + EXIT WHEN v_count >= v_limit; + + -- STEP 1: PEEK using STATIC SQL (plan cached, very fast) + IF v_is_asc THEN + IF v_upper_bound IS NOT NULL THEN + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" >= v_next_seek AND lower(o.name) COLLATE "C" < v_upper_bound + ORDER BY lower(o.name) COLLATE "C" ASC LIMIT 1; + ELSE + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" >= v_next_seek + ORDER BY lower(o.name) COLLATE "C" ASC LIMIT 1; + END IF; + ELSE + IF v_upper_bound IS NOT NULL THEN + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" < v_next_seek AND lower(o.name) COLLATE "C" >= v_prefix_lower + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + ELSIF v_prefix_lower <> '' THEN + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" < v_next_seek AND lower(o.name) COLLATE "C" >= v_prefix_lower + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + ELSE + SELECT o.name INTO v_peek_name FROM storage.objects o + WHERE o.bucket_id = bucketname AND lower(o.name) COLLATE "C" < v_next_seek + ORDER BY lower(o.name) COLLATE "C" DESC LIMIT 1; + END IF; + END IF; + + EXIT WHEN v_peek_name IS NULL; + + -- STEP 2: Check if this is a FOLDER or FILE + v_common_prefix := storage.get_common_prefix(lower(v_peek_name), v_prefix_lower, v_delimiter); + + IF v_common_prefix IS NOT NULL THEN + -- FOLDER: Handle offset, emit if needed, skip to next folder + IF v_skipped < offsets THEN + v_skipped := v_skipped + 1; + ELSE + name := substring(rtrim(storage.get_common_prefix(v_peek_name, v_prefix, v_delimiter), v_delimiter) from v_prefix_len + 1); + id := NULL; + updated_at := NULL; + created_at := NULL; + last_accessed_at := NULL; + metadata := NULL; + RETURN NEXT; + v_count := v_count + 1; + END IF; + + -- Advance seek past the folder range + IF v_is_asc THEN + v_next_seek := lower(left(v_common_prefix, -1)) || chr(ascii(v_delimiter) + 1); + ELSE + v_next_seek := lower(v_common_prefix); + END IF; + ELSE + -- FILE: Batch fetch using DYNAMIC SQL (overhead amortized over many rows) + -- For ASC: upper_bound is the exclusive upper limit (< condition) + -- For DESC: prefix_lower is the inclusive lower limit (>= condition) + FOR v_current IN EXECUTE v_batch_query + USING bucketname, v_next_seek, + CASE WHEN v_is_asc THEN COALESCE(v_upper_bound, v_prefix_lower) ELSE v_prefix_lower END, v_file_batch_size + LOOP + v_common_prefix := storage.get_common_prefix(lower(v_current.name), v_prefix_lower, v_delimiter); + + IF v_common_prefix IS NOT NULL THEN + -- Hit a folder: exit batch, let peek handle it + v_next_seek := lower(v_current.name); + EXIT; + END IF; + + -- Handle offset skipping + IF v_skipped < offsets THEN + v_skipped := v_skipped + 1; + ELSE + -- Emit file + name := substring(v_current.name from v_prefix_len + 1); + id := v_current.id; + updated_at := v_current.updated_at; + created_at := v_current.created_at; + last_accessed_at := v_current.last_accessed_at; + metadata := v_current.metadata; + RETURN NEXT; + v_count := v_count + 1; + END IF; + + -- Advance seek past this file + IF v_is_asc THEN + v_next_seek := lower(v_current.name) || v_delimiter; + ELSE + v_next_seek := lower(v_current.name); + END IF; + + EXIT WHEN v_count >= v_limit; + END LOOP; + END IF; + END LOOP; +END; +$func$; diff --git a/src/internal/database/migrations/types.ts b/src/internal/database/migrations/types.ts index 348529aff..6dbab73d4 100644 --- a/src/internal/database/migrations/types.ts +++ b/src/internal/database/migrations/types.ts @@ -62,4 +62,5 @@ export const DBMigration = { 'optimize-existing-functions-again': 60, 'mark-filename-immutable': 61, 'object-versioning-core': 62, + 'fix-search-name-relative-to-prefix': 63, } as const diff --git a/src/storage/database/pg.ts b/src/storage/database/pg.ts index 484af8bad..098b3fa59 100644 --- a/src/storage/database/pg.ts +++ b/src/storage/database/pg.ts @@ -1438,7 +1438,7 @@ export class StoragePgDB implements Database { safePrefix, bucketId, options.limit || 100, - safePrefix.split('/').length, + (safePrefix + safeSearch).split('/').length, options.offset || 0, safeSearch, sortColumn, diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 8bc5b6189..f91a98209 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -3514,6 +3514,355 @@ describe('testing list objects', () => { tnx = undefined } }) + + test('searching a nested path with a trailing slash returns the file', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/blah/blah/blah/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: '', + search: `search-${runId}/blah/blah/blah/`, + limit: 100, + offset: 0, + sortBy: { + column: 'name', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toHaveLength(1) + expect(responseJSON[0].name).toBe(objectName) + expect(responseJSON[0].id).not.toBeNull() + expect(responseJSON[0].metadata).toMatchObject({ + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) + + test('searching a nested path with the full file name returns the file', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/blah/blah/blah/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: '', + search: `search-${runId}/blah/blah/blah/file.png`, + limit: 100, + offset: 0, + sortBy: { + column: 'name', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toHaveLength(1) + expect(responseJSON[0].name).toBe(objectName) + expect(responseJSON[0].id).not.toBeNull() + expect(responseJSON[0].metadata).toMatchObject({ + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) + + test('prefix and search combine to find a nested file', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/a/b/c/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: `search-${runId}/a`, + search: 'b/c/file.png', + limit: 100, + offset: 0, + sortBy: { + column: 'name', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toHaveLength(1) + expect(responseJSON[0].name).toBe('b/c/file.png') + expect(responseJSON[0].id).not.toBeNull() + expect(responseJSON[0].metadata).toMatchObject({ + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) + + test('prefix and search combine to return nothing without a match', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/a/b/c/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: `search-${runId}/a`, + search: 'x/y/file.png', + limit: 100, + offset: 0, + sortBy: { + column: 'name', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toHaveLength(0) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) + + test('search spanning multiple folders returns the relative folder path', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/folder/sub1/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: '', + search: `search-${runId}/folder/`, + limit: 100, + offset: 0, + sortBy: { + column: 'name', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toEqual([ + { + name: `search-${runId}/folder/sub1`, + id: null, + updated_at: null, + created_at: null, + last_accessed_at: null, + metadata: null, + }, + ]) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) + + test('prefix and search combine to find a nested file when sorting by a non-name column', async () => { + const runId = randomUUID() + const bucketName = 'bucket2' + const objectName = `search-${runId}/a/b/c/file.png` + + const seedTx = await getSuperuserPostgrestClient() + await insertObjects(seedTx, { + bucket_id: bucketName, + name: objectName, + owner: '317eadce-631a-4429-a0bb-f19a7a517b4a', + version: `${runId}-file`, + metadata: { + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }, + }) + await seedTx.commit() + tnx = undefined + + try { + const response = await appInstance.inject({ + method: 'POST', + url: '/object/list/bucket2', + payload: { + prefix: `search-${runId}/a`, + search: 'b/c/file.png', + limit: 100, + offset: 0, + sortBy: { + column: 'created_at', + order: 'asc', + }, + }, + headers: { + authorization: `Bearer ${await serviceKeyAsync}`, + }, + }) + + expect(response.statusCode).toBe(200) + const responseJSON = response.json<{ name: string; id: string | null; metadata: unknown }[]>() + expect(responseJSON).toHaveLength(1) + expect(responseJSON[0].name).toBe('b/c/file.png') + expect(responseJSON[0].id).not.toBeNull() + expect(responseJSON[0].metadata).toMatchObject({ + eTag: `${runId}-file`, + size: 42, + mimetype: 'image/png', + }) + } finally { + const cleanupTx = await getSuperuserPostgrestClient() + await withDeleteEnabled(cleanupTx, async (db) => { + await deleteObjectsByName(db, bucketName, objectName) + }) + await cleanupTx.commit() + tnx = undefined + } + }) }) describe('x-robots-tag header', () => {