diff --git a/src/__tests__/services/S3ServiceMultipart.test.ts b/src/__tests__/services/S3ServiceMultipart.test.ts index f033fb2..921986f 100644 --- a/src/__tests__/services/S3ServiceMultipart.test.ts +++ b/src/__tests__/services/S3ServiceMultipart.test.ts @@ -224,59 +224,33 @@ describe('S3Service.getPresignedPartUrl — wire contract', () => { }); /** - * Multipart made books over 5 GiB possible, and a single CopyObject can't copy - * them into the `deleted_` support prefix. The delete must still happen. + * A delete removes the object and nothing else: the `deleted_` support copy it + * used to make first held the request (and the apps' serial sync queue) for a + * full server-side copy of the book. */ -describe('S3Service.deleteFile — books beyond the 5 GiB copy limit', () => { +describe('S3Service.deleteFile', () => { let service: S3Service; let sendMock: jest.Mock<(command: any) => Promise>; - let headMock: jest.Mock<(input: any) => Promise>; - const GiB = 1024 * 1024 * 1024; beforeEach(() => { process.env.S3_BUCKET = 'test-bucket'; service = new S3Service(); sendMock = jest.fn(async () => ({})); - headMock = jest.fn(async () => ({ ContentLength: 6 * GiB })); (service as any).clientObject = { send: sendMock }; - (service as any).client = { headObject: headMock }; (service as any)._logger = mockLoggerService; mockLoggerService.log.mockClear(); }); - const commandNames = () => sendMock.mock.calls.map((call) => call[0].constructor.name); - - it('deletes a book too large to copy, without the support copy', async () => { - sendMock.mockRejectedValueOnce(s3Error('InvalidRequest', 400)); - - await expect(service.deleteFile('p/root/big.m4b')).resolves.toBe(true); + it('deletes the object with one request, and copies nothing', async () => { + await expect(service.deleteFile('p/root/book.m4b')).resolves.toBe(true); - expect(commandNames()).toEqual(['CopyObjectCommand', 'DeleteObjectCommand']); - expect(sendMock.mock.calls[1][0].input.Key).toBe('p/root/big.m4b'); + expect(sendMock.mock.calls.map((call) => call[0].constructor.name)).toEqual(['DeleteObjectCommand']); + expect(sendMock.mock.calls[0][0].input).toEqual({ Bucket: 'test-bucket', Key: 'p/root/book.m4b' }); }); - it('keeps the old behaviour for any other copy failure: nothing is deleted without its copy', async () => { + it('answers null when S3 refuses the delete', async () => { sendMock.mockRejectedValueOnce(s3Error('InternalError', 500)); - headMock.mockResolvedValueOnce({ ContentLength: 3 * GiB }); await expect(service.deleteFile('p/root/book.m4b')).resolves.toBeNull(); - - expect(commandNames()).toEqual(['CopyObjectCommand']); - }); - - it('keeps the old behaviour when it cannot tell the size', async () => { - sendMock.mockRejectedValueOnce(s3Error('InvalidRequest', 400)); - headMock.mockRejectedValueOnce(s3Error('NotFound', 404)); - - await expect(service.deleteFile('p/root/gone.m4b')).resolves.toBeNull(); - - expect(commandNames()).toEqual(['CopyObjectCommand']); - }); - - it('never looks at the size when the copy succeeds', async () => { - await expect(service.deleteFile('p/root/book.m4b')).resolves.toBe(true); - - expect(headMock).not.toHaveBeenCalled(); - expect(commandNames()).toEqual(['CopyObjectCommand', 'DeleteObjectCommand']); }); }); diff --git a/src/__tests__/services/S3ServiceStorageClass.test.ts b/src/__tests__/services/S3ServiceStorageClass.test.ts index b25568f..25925ff 100644 --- a/src/__tests__/services/S3ServiceStorageClass.test.ts +++ b/src/__tests__/services/S3ServiceStorageClass.test.ts @@ -55,14 +55,4 @@ describe('S3Service — storage class on writes', () => { expect(copy.input.Key).toBe('prefix/root/b.m4b'); expect(copy.input.StorageClass).toBe('INTELLIGENT_TIERING'); }); - - it('leaves the support copy of a deleted object in STANDARD', async () => { - // `remove-deleted-items` expires this prefix within days, well before - // Intelligent-Tiering earns back its monitoring charge. - await service.deleteFile('prefix/root/a.m4b'); - - const copy = sentCommand(0); - expect(copy.input.Key).toBe('deleted_prefix/root/a.m4b'); - expect(copy.input.StorageClass).toBeUndefined(); - }); }); diff --git a/src/services/S3Service.ts b/src/services/S3Service.ts index cf5a580..3490e61 100644 --- a/src/services/S3Service.ts +++ b/src/services/S3Service.ts @@ -51,9 +51,6 @@ import { stripStoragePrefix } from '../utils'; */ const WRITE_STORAGE_CLASS = StorageClass.INTELLIGENT_TIERING; -// S3 refuses a single CopyObject from a source larger than this. -const MAX_SINGLE_COPY_SIZE = 5 * 1024 * 1024 * 1024; - // SigV4's ceiling. The effective lifetime is shorter in production: URLs signed // with the ECS task role's temporary credentials die when those rotate, which // is why clients request part URLs just before sending each window. @@ -467,33 +464,11 @@ export class S3Service { } } + /// No support copy: copying the file to a `deleted_` prefix first made a + /// delete wait on a full server-side copy of the book (about a minute for + /// 3 GB), and the apps' serial sync queue waited with it. async deleteFile(sourceKey: string): Promise { try { - /// Keep a copy for support purposes; `remove-deleted-items` expires the - /// `deleted_` prefix after 3 days. A week was the original intent — which - /// retention is right is still an open product question. - try { - await this.clientObject.send( - new CopyObjectCommand({ - Bucket: process.env.S3_BUCKET, - Key: `deleted_${sourceKey}`, - CopySource: `${process.env.S3_BUCKET}/${sourceKey}`, - // Deliberately left in STANDARD: at 3 days this copy is gone well - // before Intelligent-Tiering could earn back its monitoring charge. - }), - ); - } catch (copyError) { - // A single CopyObject stops at 5 GiB, and multipart uploads made books - // past that possible. Without this, the failed copy would skip the - // delete below and leave the book billed forever with no row pointing - // at it. Such books go without the 3-day support copy instead. - if (!(await this.exceedsSingleCopyLimit(sourceKey))) throw copyError; - this._logger.log({ - origin: 'S3: deleteFile', - message: 'Deleting without a support copy: object exceeds the 5 GiB copy limit', - data: { key: stripStoragePrefix(sourceKey) }, - }, 'warn'); - } await this.clientObject.send( new DeleteObjectCommand({ Bucket: process.env.S3_BUCKET, @@ -510,19 +485,6 @@ export class S3Service { return null; } } - /** Only a definite answer counts: a failed HEAD keeps the delete's old behaviour. */ - private async exceedsSingleCopyLimit(key: string): Promise { - try { - const head = await this.client.headObject({ - Bucket: process.env.S3_BUCKET, - Key: key, - }); - return (head.ContentLength ?? 0) > MAX_SINGLE_COPY_SIZE; - } catch { - return false; - } - } - async calculateFolderSize(folderKey: string): Promise { try { let totalSize = 0; diff --git a/src/services/StorageService.ts b/src/services/StorageService.ts index adb5870..8a62435 100644 --- a/src/services/StorageService.ts +++ b/src/services/StorageService.ts @@ -156,9 +156,6 @@ export class StorageService { origin?: StorageOrigin; }): Promise { try { - /// Keep a copy for support purposes; S3Service.deleteFile writes it to - /// the `deleted_` prefix, which `remove-deleted-items` expires after 3 - /// days. const { sourceKey, origin } = params; const storageOrigin = origin || StorageOrigin.S3; let deleted = false;