diff --git a/src/lib/server/storage/hdfs-provider.ts b/src/lib/server/storage/hdfs-provider.ts index 5861498a..faa5c07c 100644 --- a/src/lib/server/storage/hdfs-provider.ts +++ b/src/lib/server/storage/hdfs-provider.ts @@ -10,6 +10,10 @@ import type { StoragePage, StorageMetadata } from '$lib/storage/types.js'; export class HDFSStorageProvider implements StorageProvider { constructor(_config: HDFSConfig) {} + listContainers(): Promise { + throw new Error('HDFS not implemented'); + } + listObjects( _prefix: string, _pageSize: number, diff --git a/src/lib/server/storage/preview/stream.test.ts b/src/lib/server/storage/preview/stream.test.ts index 382f19c7..458c16c8 100644 --- a/src/lib/server/storage/preview/stream.test.ts +++ b/src/lib/server/storage/preview/stream.test.ts @@ -7,6 +7,7 @@ const mockLog = { info: vi.fn(), debug: vi.fn(), warn: vi.fn() } as unknown as p function makeProvider(overrides: Partial = {}): StorageProvider { return { + listContainers: vi.fn(), listObjects: vi.fn(), getObject: vi .fn() diff --git a/src/lib/server/storage/provider.ts b/src/lib/server/storage/provider.ts index 6156762d..2977e238 100644 --- a/src/lib/server/storage/provider.ts +++ b/src/lib/server/storage/provider.ts @@ -12,6 +12,8 @@ export interface ObjectDownload { /** Backend-agnostic interface for a bucket-scoped storage provider. */ export interface StorageProvider { + /** List all buckets accessible with the current connection credentials. */ + listContainers(): Promise; /** * List objects using cursor-based pagination. `continuationToken` is the * provider-specific opaque token returned from a previous call. When diff --git a/src/lib/server/storage/s3-provider.test.ts b/src/lib/server/storage/s3-provider.test.ts index d67d613a..7856b1f6 100644 --- a/src/lib/server/storage/s3-provider.test.ts +++ b/src/lib/server/storage/s3-provider.test.ts @@ -509,3 +509,184 @@ describe('S3StorageProvider constructor', () => { ); }); }); + +// --------------------------------------------------------------------------- +// listContainers +// --------------------------------------------------------------------------- + +describe('S3StorageProvider.listContainers', () => { + let provider: S3StorageProvider; + let send: ReturnType; + + beforeEach(() => { + ({ provider, send } = makeProvider()); + }); + + it('returns bucket names', async () => { + send.mockResolvedValue({ Buckets: [{ Name: 'a' }, { Name: 'b' }] }); + expect(await provider.listContainers()).toEqual(['a', 'b']); + }); + + it('filters out buckets with no name', async () => { + send.mockResolvedValue({ Buckets: [{ Name: 'a' }, { Name: undefined }, { Name: '' }] }); + expect(await provider.listContainers()).toEqual(['a']); + }); + + it('handles null Buckets in response', async () => { + send.mockResolvedValue({ Buckets: null }); + expect(await provider.listContainers()).toEqual([]); + }); + + it('maps AccessDenied S3 error to HTTP 403', async () => { + send.mockRejectedValue(makeS3Error('AccessDenied', 403)); + await expect(provider.listContainers()).rejects.toMatchObject({ status: 403 }); + }); + + it('re-throws non-S3 errors', async () => { + const err = new Error('network'); + send.mockRejectedValue(err); + await expect(provider.listContainers()).rejects.toThrow('network'); + }); +}); + +// --------------------------------------------------------------------------- +// Error handling — S3ServiceException → HTTP error mapping +// --------------------------------------------------------------------------- + +describe('S3StorageProvider error handling', () => { + let provider: S3StorageProvider; + let send: ReturnType; + + beforeEach(() => { + ({ provider, send } = makeProvider()); + }); + + it('listObjects: maps AccessDenied to HTTP 403', async () => { + send.mockRejectedValue(makeS3Error('AccessDenied', 403)); + await expect(provider.listObjects('', 10)).rejects.toMatchObject({ status: 403 }); + }); + + it('listObjects: maps NoSuchBucket to HTTP 404', async () => { + send.mockRejectedValue(makeS3Error('NoSuchBucket', 404)); + await expect(provider.listObjects('', 10)).rejects.toMatchObject({ status: 404 }); + }); + + it('listObjects: re-throws non-S3 errors', async () => { + send.mockRejectedValue(new Error('timeout')); + await expect(provider.listObjects('', 10)).rejects.toThrow('timeout'); + }); + + it('getObject: maps NoSuchKey to HTTP 404', async () => { + send.mockRejectedValue(makeS3Error('NoSuchKey', 404)); + await expect(provider.getObject('k')).rejects.toMatchObject({ status: 404 }); + }); + + it('getObject: maps AccessDenied to HTTP 403', async () => { + send.mockRejectedValue(makeS3Error('AccessDenied', 403)); + await expect(provider.getObject('k')).rejects.toMatchObject({ status: 403 }); + }); + + it('getObject: re-throws non-S3 errors', async () => { + send.mockRejectedValue(new Error('gone')); + await expect(provider.getObject('k')).rejects.toThrow('gone'); + }); + + it('getObjectRange: maps S3 error to HTTP error', async () => { + send.mockRejectedValue(makeS3Error('NoSuchKey', 404)); + await expect(provider.getObjectRange('k', 0, 100)).rejects.toMatchObject({ status: 404 }); + }); + + it('getObjectRange: re-throws non-S3 errors', async () => { + send.mockRejectedValue(new Error('network')); + await expect(provider.getObjectRange('k', 0, 100)).rejects.toThrow('network'); + }); + + it('getMetadata: maps NoSuchKey to HTTP 404', async () => { + send.mockRejectedValue(makeS3Error('NoSuchKey', 404)); + await expect(provider.getMetadata('k')).rejects.toMatchObject({ status: 404 }); + }); + + it('getMetadata: re-throws non-S3 errors', async () => { + send.mockRejectedValue(new Error('gone')); + await expect(provider.getMetadata('k')).rejects.toThrow('gone'); + }); + + it('putObject: maps AccessDenied to HTTP 403', async () => { + mockUploadDone.mockRejectedValue(makeS3Error('AccessDenied', 403)); + MockUpload.mockImplementationOnce(function (this: { done: typeof mockUploadDone }) { + this.done = mockUploadDone; + }); + await expect(provider.putObject('k', Buffer.from('x'), 'text/plain')).rejects.toMatchObject({ + status: 403 + }); + }); + + it('putObject: re-throws non-S3 errors', async () => { + mockUploadDone.mockRejectedValue(new Error('disk full')); + MockUpload.mockImplementationOnce(function (this: { done: typeof mockUploadDone }) { + this.done = mockUploadDone; + }); + await expect(provider.putObject('k', Buffer.from('x'), 'text/plain')).rejects.toThrow( + 'disk full' + ); + }); + + it('deleteObjects: maps AccessDenied to HTTP 403', async () => { + send.mockRejectedValue(makeS3Error('AccessDenied', 403)); + await expect(provider.deleteObjects(['file.txt'])).rejects.toMatchObject({ status: 403 }); + }); + + it('deleteObjects: re-throws non-S3 errors', async () => { + send.mockRejectedValue(new Error('network')); + await expect(provider.deleteObjects(['file.txt'])).rejects.toThrow('network'); + }); +}); + +// --------------------------------------------------------------------------- +// deleteObjects — directory expansion +// --------------------------------------------------------------------------- + +describe('S3StorageProvider.deleteObjects directory expansion', () => { + let provider: S3StorageProvider; + let send: ReturnType; + + beforeEach(() => { + ({ provider, send } = makeProvider()); + }); + + it('expands directory prefixes and deletes all contained keys', async () => { + // First call: ListObjectsV2 for dir/ + send.mockResolvedValueOnce({ + Contents: [{ Key: 'dir/a.txt' }, { Key: 'dir/b.txt' }], + IsTruncated: false + }); + // Second call: DeleteObjects + send.mockResolvedValueOnce({ Errors: [] }); + + const result = await provider.deleteObjects(['file.txt', 'dir/']); + + const deleteCall = send.mock.calls[1][0]; + expect(deleteCall.input.Delete.Objects).toEqual([ + { Key: 'file.txt' }, + { Key: 'dir/a.txt' }, + { Key: 'dir/b.txt' } + ]); + expect(result).toEqual({ failed: [] }); + }); + + it('uses directory key itself when no children found', async () => { + send.mockResolvedValueOnce({ IsTruncated: false }); // listAllKeys returns [] + send.mockResolvedValueOnce({ Errors: [] }); // deleteObjects + + await provider.deleteObjects(['empty/']); + + const deleteCall = send.mock.calls[1][0]; + expect(deleteCall.input.Delete.Objects).toEqual([{ Key: 'empty/' }]); + }); + + it('returns empty result for no keys', async () => { + const result = await provider.deleteObjects([]); + expect(result).toEqual({ failed: [] }); + expect(send).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/server/storage/s3-provider.ts b/src/lib/server/storage/s3-provider.ts index e6fac66f..594030bf 100644 --- a/src/lib/server/storage/s3-provider.ts +++ b/src/lib/server/storage/s3-provider.ts @@ -1,6 +1,7 @@ import { S3Client, S3ServiceException, + ListBucketsCommand, ListObjectsV2Command, GetObjectCommand, HeadObjectCommand, @@ -13,9 +14,22 @@ import type { S3Config } from './types.js'; import type { StoragePage, StorageObject, StorageMetadata } from '$lib/storage/types.js'; import { logger } from '$lib/server/logging'; import { createS3Client } from './s3-client.js'; +import { mapS3ErrorToHttp } from './s3-errors.js'; const log = logger.child({ module: 's3-provider' }); +/** Runs `fn` and maps any S3ServiceException to an HTTP error via `mapS3ErrorToHttp`. */ +async function withS3Errors( + fn: () => Promise, + context: Parameters[1] +): Promise { + try { + return await fn(); + } catch (err) { + mapS3ErrorToHttp(err, context); + } +} + export class S3StorageProvider implements StorageProvider { private readonly client: S3Client; private readonly bucket: string; @@ -25,21 +39,37 @@ export class S3StorageProvider implements StorageProvider { this.client = client ?? createS3Client(config); } + async listContainers(): Promise { + log.trace({ bucket: this.bucket }, 'S3 ListBuckets'); + return withS3Errors( + async () => { + const output = await this.client.send(new ListBucketsCommand({})); + const buckets = (output.Buckets ?? []).map((b) => b.Name ?? '').filter(Boolean); + log.debug({ bucket_count: buckets.length }, 'listed buckets'); + return buckets; + }, + { operation: 'listBuckets' } + ); + } + async listObjects( prefix: string, pageSize: number, continuationToken?: string | null ): Promise { log.trace({ bucket: this.bucket, prefix, page_size: pageSize }, 'S3 ListObjectsV2'); - - const output: ListObjectsV2CommandOutput = await this.client.send( - new ListObjectsV2Command({ - Bucket: this.bucket, - Prefix: prefix || undefined, - Delimiter: '/', - MaxKeys: pageSize, - ContinuationToken: continuationToken ?? undefined - }) + const output = await withS3Errors( + () => + this.client.send( + new ListObjectsV2Command({ + Bucket: this.bucket, + Prefix: prefix || undefined, + Delimiter: '/', + MaxKeys: pageSize, + ContinuationToken: continuationToken ?? undefined + }) + ), + { bucket: this.bucket, operation: 'listObjects' } ); const objects: StorageObject[] = [ @@ -74,39 +104,58 @@ export class S3StorageProvider implements StorageProvider { async getObject(key: string): Promise { log.trace({ bucket: this.bucket, key }, 'S3 GetObject'); - const output = await this.client.send(new GetObjectCommand({ Bucket: this.bucket, Key: key })); - if (!output.Body) { - throw new Error(`Object ${key} has no body`); - } - return { - stream: output.Body.transformToWebStream(), - contentType: output.ContentType, - contentLength: output.ContentLength, - etag: output.ETag - }; + return withS3Errors( + async () => { + const output = await this.client.send( + new GetObjectCommand({ Bucket: this.bucket, Key: key }) + ); + if (!output.Body) { + throw new Error(`Object ${key} has no body`); + } + return { + stream: output.Body.transformToWebStream(), + contentType: output.ContentType, + contentLength: output.ContentLength, + etag: output.ETag + }; + }, + { bucket: this.bucket, key, operation: 'getObject' } + ); } async getObjectRange(key: string, start: number, end: number): Promise { log.trace({ bucket: this.bucket, key, start, end }, 'S3 GetObject (range)'); - const output = await this.client.send( - new GetObjectCommand({ Bucket: this.bucket, Key: key, Range: `bytes=${start}-${end}` }) + return withS3Errors( + async () => { + const output = await this.client.send( + new GetObjectCommand({ Bucket: this.bucket, Key: key, Range: `bytes=${start}-${end}` }) + ); + if (!output.Body) { + throw new Error(`Object ${key} has no body`); + } + return output.Body.transformToWebStream(); + }, + { bucket: this.bucket, key, operation: 'getObjectRange' } ); - if (!output.Body) { - throw new Error(`Object ${key} has no body`); - } - return output.Body.transformToWebStream(); } async getMetadata(key: string): Promise { log.trace({ bucket: this.bucket, key }, 'S3 HeadObject'); - const output = await this.client.send(new HeadObjectCommand({ Bucket: this.bucket, Key: key })); - return { - size: output.ContentLength ?? 0, - lastModified: output.LastModified ?? new Date(0), - contentType: output.ContentType, - etag: output.ETag, - customMetadata: output.Metadata - }; + return withS3Errors( + async () => { + const output = await this.client.send( + new HeadObjectCommand({ Bucket: this.bucket, Key: key }) + ); + return { + size: output.ContentLength ?? 0, + lastModified: output.LastModified ?? new Date(0), + contentType: output.ContentType, + etag: output.ETag, + customMetadata: output.Metadata + }; + }, + { bucket: this.bucket, key, operation: 'getMetadata' } + ); } async exists(key: string): Promise { @@ -146,32 +195,57 @@ export class S3StorageProvider implements StorageProvider { ...(contentLength !== undefined ? { ContentLength: contentLength } : {}) } }); - await upload.done(); + await withS3Errors(() => upload.done(), { bucket: this.bucket, key, operation: 'putObject' }); } async deleteObjects(keys: string[]): Promise { log.trace({ bucket: this.bucket, key_count: keys.length }, 'S3 DeleteObjects'); - const output = await this.client.send( - new DeleteObjectsCommand({ - Bucket: this.bucket, - Delete: { - Objects: keys.map((key) => ({ Key: key })), - Quiet: true + + if (keys.length === 0) { + return { failed: [] }; + } + + // Expand directory prefixes to their contained keys + const resolvedKeys: string[] = []; + for (const key of keys) { + if (key.endsWith('/')) { + const children = await this.listAllKeys(key); + if (children.length > 0) { + resolvedKeys.push(...children); + } else { + resolvedKeys.push(key); } - }) - ); - const failed = (output.Errors ?? []).map((e) => ({ - key: e.Key ?? '', - code: e.Code, - message: e.Message - })); - if (failed.length > 0) { - log.warn( - { bucket: this.bucket, failed_count: failed.length }, - 'some objects failed to delete' - ); + } else { + resolvedKeys.push(key); + } } - return { failed }; + + return withS3Errors( + async () => { + const output = await this.client.send( + new DeleteObjectsCommand({ + Bucket: this.bucket, + Delete: { + Objects: resolvedKeys.map((key) => ({ Key: key })), + Quiet: true + } + }) + ); + const failed = (output.Errors ?? []).map((e) => ({ + key: e.Key ?? '', + code: e.Code, + message: e.Message + })); + if (failed.length > 0) { + log.warn( + { bucket: this.bucket, failed_count: failed.length }, + 'some objects failed to delete' + ); + } + return { failed }; + }, + { bucket: this.bucket, operation: 'deleteObjects' } + ); } async listAllKeys(prefix: string): Promise { diff --git a/src/lib/server/storage/service.test.ts b/src/lib/server/storage/service.test.ts deleted file mode 100644 index fa4be91f..00000000 --- a/src/lib/server/storage/service.test.ts +++ /dev/null @@ -1,247 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { faker } from '@faker-js/faker'; -import { S3ServiceException } from '@aws-sdk/client-s3'; - -vi.mock('$lib/server/logging', () => ({ - logger: { child: () => ({ info: vi.fn(), debug: vi.fn(), warn: vi.fn() }) } -})); - -const mockSend = vi.fn(); -vi.mock('./s3-client.js', () => ({ - createS3Client: () => ({ send: (...args: unknown[]) => mockSend(...args) }) -})); - -const mockProvider = { - listObjects: vi.fn(), - getObject: vi.fn(), - getObjectRange: vi.fn(), - getMetadata: vi.fn(), - exists: vi.fn(), - putObject: vi.fn(), - deleteObjects: vi.fn(), - listAllKeys: vi.fn() -}; -vi.mock('./utils.js', () => ({ - getProvider: () => mockProvider -})); - -// eslint-disable-next-line @typescript-eslint/no-unused-vars -const mockMapS3ErrorToHttp = vi.fn((err: unknown, _context?: unknown) => { - throw err; -}); -vi.mock('./s3-errors.js', () => ({ - mapS3ErrorToHttp: (err: unknown, context: unknown) => mockMapS3ErrorToHttp(err, context) -})); - -function makeS3Error(name = 'NoSuchKey'): S3ServiceException { - return new S3ServiceException({ name, message: name, $fault: 'client', $metadata: {} }); -} - -import { - listBuckets, - listObjects, - downloadObject, - getObjectMetadata, - uploadObject, - deleteObjects -} from './service.js'; -import type { S3ConnectionConfig } from './types.js'; - -const config: S3ConnectionConfig = { - type: 's3', - region: faker.location.countryCode() -}; - -describe('storage service', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - describe('listBuckets', () => { - it('returns bucket names', async () => { - mockSend.mockResolvedValue({ Buckets: [{ Name: 'a' }, { Name: 'b' }] }); - expect(await listBuckets(config)).toEqual(['a', 'b']); - }); - - it('filters out buckets with no name', async () => { - mockSend.mockResolvedValue({ Buckets: [{ Name: 'a' }, { Name: undefined }, { Name: '' }] }); - expect(await listBuckets(config)).toEqual(['a']); - }); - - it('handles null Buckets in response', async () => { - mockSend.mockResolvedValue({ Buckets: null }); - expect(await listBuckets(config)).toEqual([]); - }); - }); - - describe('listObjects', () => { - it('proxies to provider', async () => { - const page = { objects: [], prefixes: [], nextToken: null }; - mockProvider.listObjects.mockResolvedValue(page); - const result = await listObjects(config, 'bucket', 'prefix/', 100); - expect(mockProvider.listObjects).toHaveBeenCalledWith('prefix/', 100, undefined); - expect(result).toEqual(page); - }); - - it('passes continuation token when provided', async () => { - const page = { objects: [], prefixes: [], nextToken: null }; - mockProvider.listObjects.mockResolvedValue(page); - await listObjects(config, 'bucket', '', 10, 'tok'); - expect(mockProvider.listObjects).toHaveBeenCalledWith('', 10, 'tok'); - }); - - it('passes undefined when continuation token is null', async () => { - const page = { objects: [], prefixes: [], nextToken: null }; - mockProvider.listObjects.mockResolvedValue(page); - await listObjects(config, 'bucket', '', 10, null); - expect(mockProvider.listObjects).toHaveBeenCalledWith('', 10, undefined); - }); - - it('throws S3ServiceException via mapS3ErrorToHttp', async () => { - const err = makeS3Error('AccessDenied'); - mockProvider.listObjects.mockRejectedValue(err); - await expect(listObjects(config, 'bucket', '', 10)).rejects.toThrow(err); - expect(mockMapS3ErrorToHttp).toHaveBeenCalledWith(err, { - bucket: 'bucket', - operation: 'listObjects' - }); - }); - - it('rethrows non-S3 errors', async () => { - const err = new Error('network'); - mockProvider.listObjects.mockRejectedValue(err); - await expect(listObjects(config, 'bucket', '', 10)).rejects.toThrow('network'); - expect(mockMapS3ErrorToHttp).not.toHaveBeenCalled(); - }); - }); - - describe('downloadObject', () => { - it('returns download from provider', async () => { - const download = { stream: new ReadableStream(), contentType: 'text/plain' }; - mockProvider.getObject.mockResolvedValue(download); - const result = await downloadObject(config, 'bucket', 'key.txt'); - expect(result).toEqual(download); - }); - - it('throws S3ServiceException via mapS3ErrorToHttp', async () => { - const err = makeS3Error('NoSuchKey'); - mockProvider.getObject.mockRejectedValue(err); - await expect(downloadObject(config, 'bucket', 'key.txt')).rejects.toThrow(err); - expect(mockMapS3ErrorToHttp).toHaveBeenCalledWith(err, { - bucket: 'bucket', - key: 'key.txt', - operation: 'getObject' - }); - }); - - it('rethrows non-S3 errors', async () => { - const err = new Error('timeout'); - mockProvider.getObject.mockRejectedValue(err); - await expect(downloadObject(config, 'bucket', 'k')).rejects.toThrow('timeout'); - expect(mockMapS3ErrorToHttp).not.toHaveBeenCalled(); - }); - }); - - describe('getObjectMetadata', () => { - it('proxies to provider', async () => { - const meta = { contentType: 'text/plain', contentLength: 42 }; - mockProvider.getMetadata.mockResolvedValue(meta); - expect(await getObjectMetadata(config, 'bucket', 'k')).toEqual(meta); - }); - - it('throws S3ServiceException via mapS3ErrorToHttp', async () => { - const err = makeS3Error('NoSuchKey'); - mockProvider.getMetadata.mockRejectedValue(err); - await expect(getObjectMetadata(config, 'bucket', 'k')).rejects.toThrow(err); - expect(mockMapS3ErrorToHttp).toHaveBeenCalledWith(err, { - bucket: 'bucket', - key: 'k', - operation: 'getMetadata' - }); - }); - - it('rethrows non-S3 errors', async () => { - const err = new Error('gone'); - mockProvider.getMetadata.mockRejectedValue(err); - await expect(getObjectMetadata(config, 'bucket', 'k')).rejects.toThrow('gone'); - expect(mockMapS3ErrorToHttp).not.toHaveBeenCalled(); - }); - }); - - describe('uploadObject', () => { - it('proxies to provider', async () => { - mockProvider.putObject.mockResolvedValue(undefined); - await uploadObject(config, 'bucket', 'k', Buffer.from('x'), 'text/plain', 1); - expect(mockProvider.putObject).toHaveBeenCalledWith('k', expect.any(Buffer), 'text/plain', 1); - }); - - it('throws S3ServiceException via mapS3ErrorToHttp', async () => { - const err = makeS3Error('AccessDenied'); - mockProvider.putObject.mockRejectedValue(err); - await expect( - uploadObject(config, 'bucket', 'k', Buffer.from('x'), 'text/plain') - ).rejects.toThrow(err); - expect(mockMapS3ErrorToHttp).toHaveBeenCalledWith(err, { - bucket: 'bucket', - key: 'k', - operation: 'putObject' - }); - }); - - it('rethrows non-S3 errors', async () => { - const err = new Error('disk full'); - mockProvider.putObject.mockRejectedValue(err); - await expect( - uploadObject(config, 'bucket', 'k', Buffer.from('x'), 'text/plain') - ).rejects.toThrow('disk full'); - expect(mockMapS3ErrorToHttp).not.toHaveBeenCalled(); - }); - }); - - describe('deleteObjects', () => { - it('expands directory prefixes and deletes all', async () => { - mockProvider.listAllKeys.mockResolvedValue(['dir/a.txt', 'dir/b.txt']); - mockProvider.deleteObjects.mockResolvedValue({ failed: [] }); - const result = await deleteObjects(config, 'bucket', ['file.txt', 'dir/']); - expect(mockProvider.listAllKeys).toHaveBeenCalledWith('dir/'); - expect(mockProvider.deleteObjects).toHaveBeenCalledWith([ - 'file.txt', - 'dir/a.txt', - 'dir/b.txt' - ]); - expect(result).toEqual({ failed: [] }); - }); - - it('uses directory key itself when no children found', async () => { - mockProvider.listAllKeys.mockResolvedValue([]); - mockProvider.deleteObjects.mockResolvedValue({ failed: [] }); - await deleteObjects(config, 'bucket', ['empty/']); - expect(mockProvider.deleteObjects).toHaveBeenCalledWith(['empty/']); - }); - - it('returns empty result for no keys', async () => { - mockProvider.listAllKeys.mockResolvedValue([]); - mockProvider.deleteObjects.mockResolvedValue({ failed: [] }); - const result = await deleteObjects(config, 'bucket', []); - expect(result).toEqual({ failed: [] }); - }); - - it('throws S3ServiceException via mapS3ErrorToHttp', async () => { - const err = makeS3Error('AccessDenied'); - mockProvider.listAllKeys.mockResolvedValue([]); - mockProvider.deleteObjects.mockRejectedValue(err); - await expect(deleteObjects(config, 'bucket', ['file.txt'])).rejects.toThrow(err); - expect(mockMapS3ErrorToHttp).toHaveBeenCalledWith(err, { - bucket: 'bucket', - operation: 'deleteObjects' - }); - }); - - it('rethrows non-S3 errors', async () => { - const err = new Error('network'); - mockProvider.deleteObjects.mockRejectedValue(err); - await expect(deleteObjects(config, 'bucket', ['file.txt'])).rejects.toThrow('network'); - expect(mockMapS3ErrorToHttp).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/src/lib/server/storage/service.ts b/src/lib/server/storage/service.ts deleted file mode 100644 index c27b6532..00000000 --- a/src/lib/server/storage/service.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { S3ServiceException, ListBucketsCommand } from '@aws-sdk/client-s3'; -import { createS3Client } from './s3-client.js'; -import { mapS3ErrorToHttp } from './s3-errors.js'; -import { getProvider } from './utils.js'; -import type { StoragePage, StorageMetadata } from '$lib/storage/types.js'; -import type { ObjectDownload, DeleteObjectsResult } from './provider.js'; -import type { S3ConnectionConfig } from './types.js'; -import { logger } from '$lib/server/logging'; - -const log = logger.child({ module: 'storage-service' }); - -/** List all buckets accessible with the given connection config. */ -export async function listBuckets(config: S3ConnectionConfig): Promise { - const client = createS3Client(config); - - const output = await client.send(new ListBucketsCommand({})); - const buckets = (output.Buckets ?? []).map((b) => b.Name ?? '').filter(Boolean); - log.debug({ storage_type: config.type, bucket_count: buckets.length }, 'listed buckets'); - return buckets; -} - -/** List objects at the given bucket/prefix. Throws on S3 errors. */ -export async function listObjects( - config: S3ConnectionConfig, - bucket: string, - prefix: string, - pageSize: number, - continuationToken?: string | null -): Promise { - const provider = getProvider(config, bucket); - - try { - return await provider.listObjects(prefix, pageSize, continuationToken ?? undefined); - } catch (err) { - if (err instanceof S3ServiceException) { - mapS3ErrorToHttp(err, { bucket, operation: 'listObjects' }); - } - throw err; - } -} - -/** Download a single object from the bucket, returning a stream and metadata for the HTTP response. */ -export async function downloadObject( - config: S3ConnectionConfig, - bucket: string, - key: string -): Promise { - const provider = getProvider(config, bucket); - - try { - log.debug({ bucket, key }, 'downloading object'); - const download = await provider.getObject(key); - log.info({ bucket, key }, 'object download started'); - return download; - } catch (err) { - if (err instanceof S3ServiceException) { - mapS3ErrorToHttp(err, { bucket, key, operation: 'getObject' }); - } - throw err; - } -} - -/** Fetch metadata for a single object — used for lightweight pre-flight checks. */ -export async function getObjectMetadata( - config: S3ConnectionConfig, - bucket: string, - key: string -): Promise { - const provider = getProvider(config, bucket); - - try { - log.debug({ bucket, key }, 'getting object metadata'); - return await provider.getMetadata(key); - } catch (err) { - if (err instanceof S3ServiceException) { - mapS3ErrorToHttp(err, { bucket, key, operation: 'getMetadata' }); - } - throw err; - } -} - -/** Upload an object to the bucket, using multipart upload for large files. */ -export async function uploadObject( - config: S3ConnectionConfig, - bucket: string, - key: string, - body: ReadableStream | Buffer, - contentType: string, - contentLength?: number -): Promise { - const provider = getProvider(config, bucket); - - try { - log.debug({ bucket, key, content_type: contentType }, 'uploading object'); - await provider.putObject(key, body, contentType, contentLength); - log.info( - { bucket, key, content_type: contentType, content_length: contentLength }, - 'object uploaded' - ); - } catch (err) { - if (err instanceof S3ServiceException) { - mapS3ErrorToHttp(err, { bucket, key, operation: 'putObject' }); - } - throw err; - } -} - -/** Delete one or more objects from the bucket. Returns a result listing any keys that failed. - * Directory keys (ending with '/') are expanded to all contained objects before deletion. */ -export async function deleteObjects( - config: S3ConnectionConfig, - bucket: string, - keys: string[] -): Promise { - const provider = getProvider(config, bucket); - - try { - // Expand any directory prefixes (keys ending with '/') to their contents. - const dirPrefixes = keys.filter((k) => k.endsWith('/')); - const fileKeys = keys.filter((k) => !k.endsWith('/')); - - let allKeys = [...fileKeys]; - for (const prefix of dirPrefixes) { - log.debug({ bucket, prefix }, 'expanding directory prefix for deletion'); - const children = await provider.listAllKeys(prefix); - allKeys = allKeys.concat(children.length > 0 ? children : [prefix]); - } - - if (allKeys.length === 0) { - return { failed: [] }; - } - - log.debug({ bucket, key_count: allKeys.length }, 'deleting objects'); - const result = await provider.deleteObjects(allKeys); - log.info( - { bucket, key_count: allKeys.length, failed_count: result.failed.length }, - 'objects delete completed' - ); - return result; - } catch (err) { - if (err instanceof S3ServiceException) { - mapS3ErrorToHttp(err, { bucket, operation: 'deleteObjects' }); - } - throw err; - } -} diff --git a/src/lib/server/storage/utils.ts b/src/lib/server/storage/utils.ts index a0ab2223..7efb2d95 100644 --- a/src/lib/server/storage/utils.ts +++ b/src/lib/server/storage/utils.ts @@ -14,3 +14,16 @@ export function getProvider(config: S3ConnectionConfig, bucket: string): Storage return StorageProviderFactory.create({ ...config, bucket }); } + +/** + * Construct a connection-scoped storage provider for operations that do not + * require a specific bucket (e.g. listing all buckets). + * Throws a 400 HTTP error if the connection type is not supported. + */ +export function getConnectionProvider(config: S3ConnectionConfig): StorageProvider { + if (config.type !== 's3') { + throw error(400, 'Storage backend not supported'); + } + + return StorageProviderFactory.create({ ...config, bucket: '' }); +} diff --git a/src/routes/(app)/storage/+page.server.ts b/src/routes/(app)/storage/+page.server.ts index b383aec3..55cab9d9 100644 --- a/src/routes/(app)/storage/+page.server.ts +++ b/src/routes/(app)/storage/+page.server.ts @@ -3,7 +3,7 @@ import { superValidate, message } from 'sveltekit-superforms'; import { zod4 as zod } from 'sveltekit-superforms/adapters'; import type { Actions, PageServerLoad } from './$types'; import { StorageConnectionSchema } from '$lib/storage/schemas.js'; -import { listBuckets } from '$lib/server/storage/service.js'; +import { getConnectionProvider } from '$lib/server/storage/utils.js'; import type { S3ConnectionConfig } from '$lib/server/storage/types.js'; export const load: PageServerLoad = async ({ locals }) => { @@ -38,7 +38,7 @@ export const actions: Actions = { }; try { - await listBuckets(config); + await getConnectionProvider(config).listContainers(); log.info({ storage_type: type }, 'user storage connection verified'); } catch (err) { log.warn({ err }, 'storage connection test failed'); diff --git a/src/routes/(app)/storage/api/buckets/+server.ts b/src/routes/(app)/storage/api/buckets/+server.ts index 54994560..dadfa8e1 100644 --- a/src/routes/(app)/storage/api/buckets/+server.ts +++ b/src/routes/(app)/storage/api/buckets/+server.ts @@ -1,4 +1,4 @@ -import { listBuckets } from '$lib/server/storage/service.js'; +import { getConnectionProvider } from '$lib/server/storage/utils.js'; import type { RequestHandler } from './$types'; /** @@ -10,7 +10,7 @@ import type { RequestHandler } from './$types'; * in hooks.server.ts before this handler runs. */ export const GET: RequestHandler = async ({ locals }) => { - const buckets = await listBuckets(locals.storageConfig!); + const buckets = await getConnectionProvider(locals.storageConfig!).listContainers(); locals.logger.debug({ bucket_count: buckets.length }, 'bucket list returned'); return Response.json(buckets); }; diff --git a/src/routes/(app)/storage/api/delete/+server.ts b/src/routes/(app)/storage/api/delete/+server.ts index 43369e83..58eea526 100644 --- a/src/routes/(app)/storage/api/delete/+server.ts +++ b/src/routes/(app)/storage/api/delete/+server.ts @@ -1,6 +1,6 @@ import type { RequestHandler } from './$types'; import { error } from '@sveltejs/kit'; -import { deleteObjects } from '$lib/server/storage/service.js'; +import { getProvider } from '$lib/server/storage/utils.js'; import { requireBucket } from '../params.js'; /** @@ -22,7 +22,7 @@ export const DELETE: RequestHandler = async ({ locals, url }) => { locals.logger.debug({ bucket, key_count: keys.length }, 'delete request received'); - const result = await deleteObjects(locals.storageConfig!, bucket, keys); + const result = await getProvider(locals.storageConfig!, bucket).deleteObjects(keys); locals.logger.info( { bucket, key_count: keys.length, failed_count: result.failed.length }, diff --git a/src/routes/(app)/storage/api/delete/server.test.ts b/src/routes/(app)/storage/api/delete/server.test.ts index 7d2dcd9e..4133230f 100644 --- a/src/routes/(app)/storage/api/delete/server.test.ts +++ b/src/routes/(app)/storage/api/delete/server.test.ts @@ -1,12 +1,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { faker } from '@faker-js/faker'; -vi.mock('$lib/server/storage/service.js', () => ({ - deleteObjects: vi.fn() +const mockProvider = { deleteObjects: vi.fn() }; +vi.mock('$lib/server/storage/utils.js', () => ({ + getProvider: () => mockProvider })); import { DELETE } from './+server.js'; -import { deleteObjects } from '$lib/server/storage/service.js'; const CONNECTION_HEADER = { 'x-storage-connection': btoa(JSON.stringify({ type: 's3', region: 'us-east-1' })) @@ -50,12 +50,10 @@ describe('DELETE /storage/api/delete', () => { it('deletes objects and returns JSON result', async () => { const keys = [faker.system.fileName(), faker.system.fileName()]; const result = { deleted: keys, failed: [] }; - vi.mocked(deleteObjects).mockResolvedValue( - result as unknown as Awaited> - ); + mockProvider.deleteObjects.mockResolvedValue(result); const response = await DELETE(mockEvent({ bucket: 'b1', keys })); - expect(deleteObjects).toHaveBeenCalledWith(expect.objectContaining({ type: 's3' }), 'b1', keys); + expect(mockProvider.deleteObjects).toHaveBeenCalledWith(keys); expect(await response.json()).toEqual(result); }); }); diff --git a/src/routes/(app)/storage/api/download/+server.ts b/src/routes/(app)/storage/api/download/+server.ts index d8e09bb8..53f28612 100644 --- a/src/routes/(app)/storage/api/download/+server.ts +++ b/src/routes/(app)/storage/api/download/+server.ts @@ -1,5 +1,5 @@ import type { RequestHandler } from './$types'; -import { downloadObject, getObjectMetadata } from '$lib/server/storage/service.js'; +import { getProvider } from '$lib/server/storage/utils.js'; import { requireBucketKey } from '../params.js'; /** Derive the bare filename from a (possibly path-prefixed) object key. */ @@ -23,7 +23,7 @@ export const GET: RequestHandler = async ({ locals, url }) => { locals.logger.debug({ bucket, key }, 'download request received'); - const download = await downloadObject(locals.storageConfig!, bucket, key); + const download = await getProvider(locals.storageConfig!, bucket).getObject(key); const filename = filenameFromKey(key); // RFC 5987 encoding for non-ASCII filenames in Content-Disposition @@ -64,7 +64,7 @@ export const HEAD: RequestHandler = async ({ locals, url }) => { locals.logger.debug({ bucket, key }, 'download pre-flight check'); - const meta = await getObjectMetadata(locals.storageConfig!, bucket, key); + const meta = await getProvider(locals.storageConfig!, bucket).getMetadata(key); return new Response(null, { status: 200, diff --git a/src/routes/(app)/storage/api/download/server.test.ts b/src/routes/(app)/storage/api/download/server.test.ts index 545b581b..bc602d23 100644 --- a/src/routes/(app)/storage/api/download/server.test.ts +++ b/src/routes/(app)/storage/api/download/server.test.ts @@ -1,12 +1,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -vi.mock('$lib/server/storage/service.js', () => ({ - downloadObject: vi.fn(), - getObjectMetadata: vi.fn() +const mockProvider = { getObject: vi.fn(), getMetadata: vi.fn() }; +vi.mock('$lib/server/storage/utils.js', () => ({ + getProvider: () => mockProvider })); import { GET, HEAD } from './+server.js'; -import { downloadObject, getObjectMetadata } from '$lib/server/storage/service.js'; const CONNECTION_HEADER = { 'x-storage-connection': btoa(JSON.stringify({ type: 's3', region: 'us-east-1' })) @@ -30,12 +29,12 @@ describe('GET /storage/api/download', () => { it('streams download with correct headers', async () => { const stream = new ReadableStream(); - vi.mocked(downloadObject).mockResolvedValue({ + mockProvider.getObject.mockResolvedValue({ stream, contentType: 'text/csv', contentLength: 1234, etag: '"abc"' - } as unknown as Awaited>); + }); const res = await GET(mockEvent('bucket=b1&key=path/data.csv')); @@ -47,12 +46,12 @@ describe('GET /storage/api/download', () => { }); it('uses application/octet-stream when no content type', async () => { - vi.mocked(downloadObject).mockResolvedValue({ + mockProvider.getObject.mockResolvedValue({ stream: new ReadableStream(), contentType: undefined, contentLength: undefined, etag: undefined - } as unknown as Awaited>); + }); const res = await GET(mockEvent('bucket=b1&key=file.bin')); expect(res.headers.get('Content-Type')).toBe('application/octet-stream'); @@ -64,10 +63,10 @@ describe('HEAD /storage/api/download', () => { beforeEach(() => vi.clearAllMocks()); it('returns 200 with metadata headers', async () => { - vi.mocked(getObjectMetadata).mockResolvedValue({ + mockProvider.getMetadata.mockResolvedValue({ contentType: 'application/json', size: 999 - } as unknown as Awaited>); + }); const res = await HEAD(mockEvent('bucket=b1&key=data.json')); diff --git a/src/routes/(app)/storage/api/objects/+server.ts b/src/routes/(app)/storage/api/objects/+server.ts index 463a5b63..1973354e 100644 --- a/src/routes/(app)/storage/api/objects/+server.ts +++ b/src/routes/(app)/storage/api/objects/+server.ts @@ -1,5 +1,5 @@ import { error } from '@sveltejs/kit'; -import { listObjects } from '$lib/server/storage/service.js'; +import { getProvider } from '$lib/server/storage/utils.js'; import type { RequestHandler } from './$types'; /** @@ -24,12 +24,10 @@ export const GET: RequestHandler = async ({ url, locals }) => { 'listing objects' ); - const page = await listObjects( - locals.storageConfig!, - bucket, + const page = await getProvider(locals.storageConfig!, bucket).listObjects( prefix, pageSize, - continuationToken + continuationToken ?? undefined ); return Response.json(page); }; diff --git a/src/routes/(app)/storage/api/upload/+server.ts b/src/routes/(app)/storage/api/upload/+server.ts index b6202da4..6aacf360 100644 --- a/src/routes/(app)/storage/api/upload/+server.ts +++ b/src/routes/(app)/storage/api/upload/+server.ts @@ -1,5 +1,5 @@ import { error } from '@sveltejs/kit'; -import { uploadObject } from '$lib/server/storage/service.js'; +import { getProvider } from '$lib/server/storage/utils.js'; import { requireBucketKey } from '../params.js'; import type { RequestHandler } from './$types'; @@ -33,7 +33,12 @@ export const POST: RequestHandler = async ({ locals, url, request }) => { 'upload request received' ); - await uploadObject(locals.storageConfig!, bucket, key, request.body, contentType, contentLength); + await getProvider(locals.storageConfig!, bucket).putObject( + key, + request.body, + contentType, + contentLength + ); log.info( { bucket, key, content_type: contentType, content_length: contentLength }, diff --git a/src/routes/(app)/storage/api/upload/server.test.ts b/src/routes/(app)/storage/api/upload/server.test.ts index 17644c13..ff4b42b8 100644 --- a/src/routes/(app)/storage/api/upload/server.test.ts +++ b/src/routes/(app)/storage/api/upload/server.test.ts @@ -1,11 +1,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -vi.mock('$lib/server/storage/service.js', () => ({ - uploadObject: vi.fn() +const mockProvider = { putObject: vi.fn() }; +vi.mock('$lib/server/storage/utils.js', () => ({ + getProvider: () => mockProvider })); import { POST } from './+server.js'; -import { uploadObject } from '$lib/server/storage/service.js'; const CONNECTION_HEADER = { 'x-storage-connection': btoa(JSON.stringify({ type: 's3', region: 'us-east-1' })) @@ -50,7 +50,7 @@ describe('POST /storage/api/upload', () => { }); it('uploads and returns 201', async () => { - vi.mocked(uploadObject).mockResolvedValue(undefined); + mockProvider.putObject.mockResolvedValue(undefined); const body = new ReadableStream(); const res = await POST( @@ -65,22 +65,15 @@ describe('POST /storage/api/upload', () => { ); expect(res.status).toBe(201); - expect(uploadObject).toHaveBeenCalledWith( - expect.objectContaining({ type: 's3' }), - 'b1', - 'file.txt', - body, - 'image/png', - 100 - ); + expect(mockProvider.putObject).toHaveBeenCalledWith('file.txt', body, 'image/png', 100); }); it('defaults content type to application/octet-stream', async () => { - vi.mocked(uploadObject).mockResolvedValue(undefined); + mockProvider.putObject.mockResolvedValue(undefined); const res = await POST(mockEvent({ headers: { ...CONNECTION_HEADER } })); expect(res.status).toBe(201); - expect(vi.mocked(uploadObject).mock.calls[0][4]).toBe('application/octet-stream'); + expect(mockProvider.putObject.mock.calls[0][2]).toBe('application/octet-stream'); }); }); diff --git a/src/routes/(app)/storage/page.server.test.ts b/src/routes/(app)/storage/page.server.test.ts index 207b214c..d3e4926e 100644 --- a/src/routes/(app)/storage/page.server.test.ts +++ b/src/routes/(app)/storage/page.server.test.ts @@ -1,7 +1,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -vi.mock('$lib/server/storage/service.js', () => ({ - listBuckets: vi.fn() +const mockConnectionProvider = { listContainers: vi.fn() }; +vi.mock('$lib/server/storage/utils.js', () => ({ + getConnectionProvider: () => mockConnectionProvider })); vi.mock('$lib/storage/schemas.js', () => ({ @@ -18,7 +19,6 @@ vi.mock('sveltekit-superforms/adapters', () => ({ })); import { load, actions } from './+page.server.js'; -import { listBuckets } from '$lib/server/storage/service.js'; import { superValidate } from 'sveltekit-superforms'; function mockLocals() { @@ -87,7 +87,7 @@ describe('storage page actions', () => { secretAccessKey: 'sk' } } as unknown as Awaited>); - vi.mocked(listBuckets).mockRejectedValue(new Error('connection refused')); + mockConnectionProvider.listContainers.mockRejectedValue(new Error('connection refused')); const result = await actions.connect({ request: new Request('http://localhost', { method: 'POST' }), @@ -111,7 +111,7 @@ describe('storage page actions', () => { secretAccessKey: 'sk' } } as unknown as Awaited>); - vi.mocked(listBuckets).mockResolvedValue(['b1']); + mockConnectionProvider.listContainers.mockResolvedValue(['b1']); await expect( actions.connect({