diff --git a/src/internal/monitoring/logger.ts b/src/internal/monitoring/logger.ts index 7eb64219f..aaaf1bdd9 100644 --- a/src/internal/monitoring/logger.ts +++ b/src/internal/monitoring/logger.ts @@ -169,6 +169,7 @@ export interface LifecycleEventLog extends RequestLogContext { objectPath: string objectVersion: string | null bytesChanged: number + objectsChanged: number resources: string[] } diff --git a/src/storage/events/base-event.test.ts b/src/storage/events/base-event.test.ts index 63226bfd5..8c01ef956 100644 --- a/src/storage/events/base-event.test.ts +++ b/src/storage/events/base-event.test.ts @@ -193,6 +193,7 @@ function webhookPayload(): ObjectRemovedEvent { reqId: 'req-1', sbReqId: 'sb-req-1', bytesChanged: -42, + objectsChanged: -1, } } @@ -215,7 +216,7 @@ describe('BaseEvent.sendWebhook', () => { webhookSend.mockResolvedValue(undefined) }) - it('logs the billing event synchronously, before the webhook is enqueued', async () => { + it('logs the usage event synchronously, before the webhook is enqueued', async () => { const TestEvent = await loadTestEvent() await TestEvent.sendWebhook(webhookPayload(), Date.now()) @@ -228,6 +229,7 @@ describe('BaseEvent.sendWebhook', () => { objectPath: 'tenant-a/bucket-a/path/file.png', objectVersion: 'version-1', bytesChanged: -42, + objectsChanged: -1, resources: ['/tenant-a/bucket-a/path/file.png'], tenantId: 'tenant-a', project: 'tenant-a', @@ -244,7 +246,7 @@ describe('BaseEvent.sendWebhook', () => { expect(logOrder).toBeLessThan(sendOrder) }) - it('still logs the billing event exactly once when enqueuing the webhook fails', async () => { + it('still logs the usage event exactly once when enqueuing the webhook fails', async () => { webhookSend.mockRejectedValue(new Error('queue unavailable')) const TestEvent = await loadTestEvent() @@ -253,7 +255,7 @@ describe('BaseEvent.sendWebhook', () => { expect(logInfo).toHaveBeenCalledTimes(1) }) - it('still enqueues the webhook, without throwing, when logging the billing event fails', async () => { + it('still enqueues the webhook, without throwing, when logging the usage event fails', async () => { logInfo.mockImplementation(() => { throw new Error('serialization failed') }) diff --git a/src/storage/events/base-event.ts b/src/storage/events/base-event.ts index 1e4e5f5a0..d8eaabb83 100644 --- a/src/storage/events/base-event.ts +++ b/src/storage/events/base-event.ts @@ -36,6 +36,7 @@ export abstract class BaseEvent> extends name: string version: string | null bytesChanged: number + objectsChanged: number }, >(this: StaticThis, payload: Omit, applyTime: number) { if (!Webhook) { @@ -53,6 +54,7 @@ export abstract class BaseEvent> extends objectPath: path, objectVersion: payload.version, bytesChanged: payload.bytesChanged, + objectsChanged: payload.objectsChanged, resources: ['/' + path], tenantId: payload.tenant.ref, project: payload.tenant.ref, diff --git a/src/storage/events/lifecycle/object-created.ts b/src/storage/events/lifecycle/object-created.ts index 0dd67a441..2b7d6af91 100644 --- a/src/storage/events/lifecycle/object-created.ts +++ b/src/storage/events/lifecycle/object-created.ts @@ -13,6 +13,8 @@ interface ObjectCreatedEventBase extends BasePayload { uploadType: ObjectCreatedUploadType // Net bytes this event added/removed to storage bytesChanged: number + // Net objects this event added/removed. Delete markers never count + objectsChanged: number // Set only when bytesChanged includes a row this write replaced in place replacedObject?: ReplacedObjectSnapshot } diff --git a/src/storage/events/lifecycle/object-promoted.ts b/src/storage/events/lifecycle/object-promoted.ts index 4f37105f1..ad8712bda 100644 --- a/src/storage/events/lifecycle/object-promoted.ts +++ b/src/storage/events/lifecycle/object-promoted.ts @@ -7,6 +7,7 @@ export interface ObjectPromotedEvent extends BasePayload { version: string | null metadata?: Record | null bytesChanged: number + objectsChanged: number } export class ObjectPromoted extends BaseEvent { diff --git a/src/storage/events/lifecycle/object-removed.ts b/src/storage/events/lifecycle/object-removed.ts index 39827b7f8..f88ffded5 100644 --- a/src/storage/events/lifecycle/object-removed.ts +++ b/src/storage/events/lifecycle/object-removed.ts @@ -29,6 +29,8 @@ export interface ObjectRemovedEvent extends BasePayload { metadata?: Record | null // Net bytes this event added/removed to storage bytesChanged: number + // Net objects this event added/removed. Delete markers never count + objectsChanged: number // Set only when bytesChanged includes a row this write replaced in place replacedObject?: ReplacedObjectSnapshot } diff --git a/src/storage/events/objects/object-admin-delete-all-before.test.ts b/src/storage/events/objects/object-admin-delete-all-before.test.ts index 9adbd9fc3..c3e453c65 100644 --- a/src/storage/events/objects/object-admin-delete-all-before.test.ts +++ b/src/storage/events/objects/object-admin-delete-all-before.test.ts @@ -95,6 +95,58 @@ describe('ObjectAdminDeleteAllBefore.handle', () => { } }) + it('reports objectsChanged by row, counting zero-byte objects and never delete markers', async () => { + const db = { + listObjects: vi + .fn() + .mockResolvedValue([{ id: 'id-1' }, { id: 'id-2' }, { id: 'id-3' }, { id: 'id-4' }]), + deleteObjects: vi.fn().mockResolvedValue([ + { id: 'id-1', name: 'sized.txt', version: 'v1', metadata: { size: 7 } }, + { id: 'id-2', name: 'empty.txt', version: 'v1', metadata: { size: 0 } }, + { id: 'id-3', name: 'nosize.txt', version: 'v1', metadata: null }, + { id: 'id-4', name: 'sized.txt', version: 'v2', is_delete_marker: true, metadata: null }, + ]), + destroyConnection: vi.fn(), + } + const storage = { + db, + backend: { + deleteObjectsDetailed: vi.fn((_bucket: string, keys: string[]) => + Promise.resolve(keys.map((key) => ({ key, outcome: 'DELETED' as const }))) + ), + }, + location: { + getRootLocation: vi.fn(() => 'root-bucket'), + getKeyLocation: vi.fn(() => 'tenant-id/bucket/key'), + }, + } + vi.spyOn( + ObjectAdminDeleteAllBefore as unknown as { createStorage: () => Promise }, + 'createStorage' + ).mockResolvedValue(storage) + const sendWebhook = vi.spyOn(ObjectRemoved, 'sendWebhook').mockResolvedValue(undefined) + + await ObjectAdminDeleteAllBefore.handle(createJob() as never) + + const payloads = sendWebhook.mock.calls.map( + ([payload]) => + payload as { name: string; version: string; bytesChanged: number; objectsChanged: number } + ) + expect( + payloads.map(({ version, name, bytesChanged, objectsChanged }) => ({ + name, + version, + bytesChanged, + objectsChanged, + })) + ).toEqual([ + { name: 'sized.txt', version: 'v1', bytesChanged: -7, objectsChanged: -1 }, + { name: 'empty.txt', version: 'v1', bytesChanged: 0, objectsChanged: -1 }, + { name: 'nosize.txt', version: 'v1', bytesChanged: 0, objectsChanged: -1 }, + { name: 'sized.txt', version: 'v2', bytesChanged: 0, objectsChanged: 0 }, + ]) + }) + it("captures a fresh applyTime per batch instead of reusing an earlier batch's", async () => { let now = 1_700_000_000_000 const dateNowSpy = vi.spyOn(Date, 'now').mockImplementation(() => now++) diff --git a/src/storage/events/objects/object-admin-delete-all-before.ts b/src/storage/events/objects/object-admin-delete-all-before.ts index f21e52bf0..9d3b8a826 100644 --- a/src/storage/events/objects/object-admin-delete-all-before.ts +++ b/src/storage/events/objects/object-admin-delete-all-before.ts @@ -1,6 +1,6 @@ import { logger, logSchema } from '@internal/monitoring' import { BasePayload } from '@internal/queue' -import { metadataSize } from '@storage/schemas' +import { metadataSize, objectsFreed } from '@storage/schemas' import { Job, SendOptions, WorkOptions } from 'pg-boss' import { getConfig } from '../../../config' import { Storage } from '../../index' @@ -113,6 +113,7 @@ export class ObjectAdminDeleteAllBefore extends BaseEvent { version: 'v1', metadata: { size: 1 }, bytesChanged: 0, + objectsChanged: 0, }), expect.any(Number) ) diff --git a/src/storage/object.ts b/src/storage/object.ts index 8b95bd68c..d69e3f922 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -21,6 +21,7 @@ import { Obj, ObjectListEntry, ObjectListingFilterMode, + objectsFreed, } from '@storage/schemas' import { FastifyRequest } from 'fastify/types/request' import { StorageBackendAdapter } from './backend' @@ -518,6 +519,7 @@ export class ObjectStorage { sbReqId: this.db.sbReqId, metadata: eventObject.metadata, bytesChanged: -metadataSize(freed?.metadata) || 0, + objectsChanged: -objectsFreed(freed) || 0, // A marker's bytes are freed from the null-version row its insert // reused in place, which can differ from eventObject when the key has // prior history under both SUSPENDED and ENABLED versioning. @@ -552,6 +554,7 @@ export class ObjectStorage { version: promoted.version ?? null, metadata: promoted.metadata, bytesChanged: 0, + objectsChanged: 0, }, // +1 to ensure this event is applied after the remove above committedAt + 1 @@ -844,7 +847,11 @@ export class ObjectStorage { ...applied.plain.map((deleted) => { if (!deleted.is_delete_marker) { // DISABLED: the deleted row's own bytes are freed. - return { object: deleted, bytesChanged: -metadataSize(deleted.metadata) || 0 } + return { + object: deleted, + bytesChanged: -metadataSize(deleted.metadata) || 0, + objectsChanged: -objectsFreed(deleted) || 0, + } } // A marker frees only the bytes of the null-version row it replaced in place (SUSPENDED) // otherwise it archives the current row without freeing anything. @@ -853,12 +860,14 @@ export class ObjectStorage { return { object: priorPlainObject(deleted) ?? deleted, bytesChanged: -metadataSize(replaced?.metadata) || 0, + objectsChanged: -objectsFreed(replaced) || 0, ...replacedObjectField(replaced), } }), ...applied.versioned.map((deleted) => ({ object: deleted, bytesChanged: -metadataSize(deleted.metadata) || 0, + objectsChanged: -objectsFreed(deleted) || 0, })), ] @@ -910,6 +919,7 @@ export class ObjectStorage { version: event.object.version ?? null, metadata: event.object.metadata, bytesChanged: event.bytesChanged, + objectsChanged: event.objectsChanged, ...('replacedObject' in event && event.replacedObject ? { replacedObject: event.replacedObject } : {}), @@ -930,6 +940,7 @@ export class ObjectStorage { version: object.version ?? null, metadata: object.metadata, bytesChanged: 0, + objectsChanged: 0, }, // +1 to ensure this event is applied after the remove above committedAt + 1 @@ -1187,6 +1198,7 @@ export class ObjectStorage { metadata, uploadType, bytesChanged: metadataSize(metadata) - metadataSize(replaced?.metadata), + objectsChanged: 1 - objectsFreed(replaced), ...replacedObjectField(replaced), reqId: this.db.reqId, sbReqId: this.db.sbReqId, @@ -1445,6 +1457,7 @@ export class ObjectStorage { let destObject: WrittenObject let shouldDeleteSourceContent = true let sourceBytesFreed = metadataSize(sourceObject.metadata) + let sourceObjectsFreed = 1 let sourceReplacedObject: ReturnType let promotedSource: Obj | undefined const freedContent: { name: string; bucketId: string; version?: string }[] = [] @@ -1504,6 +1517,7 @@ export class ObjectStorage { shouldDeleteSourceContent = false const freed = replacedContent(deletedSource) sourceBytesFreed = metadataSize(freed?.metadata) + sourceObjectsFreed = objectsFreed(freed) sourceReplacedObject = freed if (freed) { freedContent.push({ @@ -1551,6 +1565,7 @@ export class ObjectStorage { sourceVersion: sourceObject.version, sourceMetadata: sourceObject.metadata, sourceBytesFreed, + sourceObjectsFreed, sourceReplacedObject, promotedSource, replacedDestination, @@ -1586,6 +1601,7 @@ export class ObjectStorage { version: moved.sourceVersion ?? null, metadata: moved.sourceMetadata, bytesChanged: -moved.sourceBytesFreed || 0, + objectsChanged: -moved.sourceObjectsFreed || 0, // The marker reused the null-version row's slot in place (SUSPENDED), // which can differ from the source version this event otherwise describes ...replacedObjectField(moved.sourceReplacedObject), @@ -1605,6 +1621,7 @@ export class ObjectStorage { // report its size here rather than reporting it as a separate event bytesChanged: metadataSize(metadata) - metadataSize(moved.replacedDestination?.metadata), + objectsChanged: 1 - objectsFreed(moved.replacedDestination), ...replacedObjectField(moved.replacedDestination), oldObject: { name: sourceObjectName, @@ -1634,6 +1651,7 @@ export class ObjectStorage { version: moved.promotedSource.version ?? null, metadata: moved.promotedSource.metadata, bytesChanged: 0, + objectsChanged: 0, }, // +1 to ensure this event is applied last committedAt + 1 diff --git a/src/storage/schemas/object.ts b/src/storage/schemas/object.ts index 5f8673ebf..275dbcb32 100644 --- a/src/storage/schemas/object.ts +++ b/src/storage/schemas/object.ts @@ -72,6 +72,14 @@ export function metadataSize(metadata: Obj['metadata']): number { return parseMetadataNumber(metadata?.size) ?? 0 } +/** + * How many objects a removed row accounts for: 1 for a row holding bytes, 0 for + * a delete marker or no row. Counts rows, not bytes, so zero-byte objects count. + */ +export function objectsFreed(row?: object | null): number { + return row && !(row as { is_delete_marker?: boolean | null }).is_delete_marker ? 1 : 0 +} + export const objectListEntrySchema = { $id: 'objectListEntrySchema', type: 'object', diff --git a/src/storage/uploader.ts b/src/storage/uploader.ts index 100b8f4be..c361e55a0 100644 --- a/src/storage/uploader.ts +++ b/src/storage/uploader.ts @@ -18,7 +18,7 @@ import { replacedObjectField, } from './events' import { getFileSizeLimit, isEmptyFolder } from './limits' -import { metadataSize } from './schemas' +import { metadataSize, objectsFreed } from './schemas' import { validateMimeType } from './validators/mime-type' import { validateXRobotsTag } from './validators/x-robots-tag' @@ -356,6 +356,7 @@ export class Uploader { bucketId, metadata: objectMetadata, bytesChanged: objectMetadata.size - metadataSize(replaced?.metadata), + objectsChanged: 1 - objectsFreed(replaced), ...replacedObjectField(replaced), reqId: this.db.reqId, sbReqId: this.db.sbReqId, diff --git a/src/test/object-versioning-copies.test.ts b/src/test/object-versioning-copies.test.ts index 09be15c42..d01d70755 100644 --- a/src/test/object-versioning-copies.test.ts +++ b/src/test/object-versioning-copies.test.ts @@ -64,6 +64,7 @@ describe('object versioning - copyObject', () => { expect(createdSpy).toHaveBeenCalledWith( expect.objectContaining({ bytesChanged: content.length, + objectsChanged: 1, }), expect.any(Number) ) @@ -95,6 +96,7 @@ describe('object versioning - copyObject', () => { expect.objectContaining({ metadata: expect.objectContaining({ size: content.length }), bytesChanged: content.length - overwrittenContent.length, + objectsChanged: 0, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: overwrittenContent.length }), }), diff --git a/src/test/object-versioning-moves.test.ts b/src/test/object-versioning-moves.test.ts index 22d2e0fe1..13f36ad13 100644 --- a/src/test/object-versioning-moves.test.ts +++ b/src/test/object-versioning-moves.test.ts @@ -176,7 +176,7 @@ describe('object versioning - moveObject', () => { // The source's own content is hard-deleted, so its bytes are freed, // and there's no other row to report via replacedObject. expect(removedMoveSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: -4 }), + expect.objectContaining({ bytesChanged: -4, objectsChanged: -1 }), expect.any(Number) ) // Nothing else existed under this key to promote. @@ -214,6 +214,7 @@ describe('object versioning - moveObject', () => { version: historicalVersion, metadata: expect.objectContaining({ size: 'historical content'.length }), bytesChanged: 0, + objectsChanged: 0, }), expect.any(Number) ) @@ -333,6 +334,7 @@ describe('object versioning - moveObject', () => { name: sourceName, bucketId, bytesChanged: -historicalContent.length, + objectsChanged: -1, }), expect.any(Number) ) @@ -343,6 +345,8 @@ describe('object versioning - moveObject', () => { bucketId, metadata: expect.objectContaining({ size: historicalContent.length }), bytesChanged: historicalContent.length - overwrittenContent.length, + // One object added, one replaced in place + objectsChanged: 0, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: overwrittenContent.length }), }), @@ -352,11 +356,19 @@ describe('object versioning - moveObject', () => { // The net physical change across both events is just the destroyed // row's bytes: the historical row's content moved, not grew or shrank. - const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } - const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } + const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } + const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } expect(removedPayload.bytesChanged + createdPayload.bytesChanged).toBe( -overwrittenContent.length ) + // The source row is removed and the destination replaces the null-version row in place + expect(removedPayload.objectsChanged + createdPayload.objectsChanged).toBe(-1) // Both describe the same key (a same-path restore) at the same commit // the created row is the key's actual final state, so it must always land last @@ -395,6 +407,7 @@ describe('object versioning - moveObject', () => { // ...but the bytes physically freed came from the stale row the // marker's insert reused, reported separately via replacedObject. bytesChanged: -staleContent.length, + objectsChanged: -1, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: staleContent.length }), }), @@ -418,13 +431,13 @@ describe('object versioning - moveObject', () => { // ENABLED: the source is archived behind a delete-marker, not freed, // and no other row was reused in place. expect(removedMoveSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: 0 }), + expect.objectContaining({ bytesChanged: 0, objectsChanged: 0 }), expect.any(Number) ) // The destination always receives genuinely new bytes; nothing existed // there to replace. expect(createdMoveSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: content.length }), + expect.objectContaining({ bytesChanged: content.length, objectsChanged: 1 }), expect.any(Number) ) } finally { @@ -448,9 +461,16 @@ describe('object versioning - moveObject', () => { // The destination write landed on an already-deleted row - nothing was // left for it to replace, so the two events must net to zero real bytes. - const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } - const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } + const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } + const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } expect(removedPayload.bytesChanged + createdPayload.bytesChanged).toBe(0) + expect(removedPayload.objectsChanged + createdPayload.objectsChanged).toBe(0) expect(createdPayload).not.toHaveProperty('replacedObject') // The old row's backend bytes must be queued for cleanup exactly once. @@ -487,9 +507,16 @@ describe('object versioning - moveObject', () => { // ENABLED archives rather than overwrites, so nothing is replaced and // the two events net to zero real bytes, same as the SUSPENDED case. expect(promotedSpy).not.toHaveBeenCalled() - const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } - const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { bytesChanged: number } + const removedPayload = removedMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } + const createdPayload = createdMoveSpy.mock.calls[0][0] as unknown as { + bytesChanged: number + objectsChanged: number + } expect(removedPayload.bytesChanged + createdPayload.bytesChanged).toBe(0) + expect(removedPayload.objectsChanged + createdPayload.objectsChanged).toBe(0) expect(createdPayload).not.toHaveProperty('replacedObject') } finally { removedMoveSpy.mockRestore() diff --git a/src/test/object-versioning-writes.test.ts b/src/test/object-versioning-writes.test.ts index 6e3f8c8c8..52ddbee02 100644 --- a/src/test/object-versioning-writes.test.ts +++ b/src/test/object-versioning-writes.test.ts @@ -490,6 +490,7 @@ describe('object versioning - version-aware writes', () => { expect(removedSpy).toHaveBeenCalledWith( expect.objectContaining({ bytesChanged: -70, + objectsChanged: -1, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: 70 }), }), @@ -540,6 +541,7 @@ describe('object versioning - version-aware writes', () => { // ...but the bytes physically freed came from the stale row the // marker's insert reused, reported separately via replacedObject. bytesChanged: -70, + objectsChanged: -1, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: 70 }), }), @@ -570,7 +572,7 @@ describe('object versioning - version-aware writes', () => { // Nothing existed to free: the marker's own write frees no bytes, and // there's no other row to report. expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: 0 }), + expect.objectContaining({ bytesChanged: 0, objectsChanged: 0 }), expect.any(Number) ) } finally { @@ -681,7 +683,7 @@ describe('object versioning - version-aware writes', () => { // v1's own bytes are hard-deleted, regardless of it being noncurrent; // there's no other row reused in place to report. expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: -40 }), + expect.objectContaining({ bytesChanged: -40, objectsChanged: -1 }), expect.any(Number) ) } finally { @@ -737,7 +739,7 @@ describe('object versioning - version-aware writes', () => { await Promise.all([keyHolder, currentDelete]) // v2 was the current version, hard-deleted just the same. expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: -60 }), + expect.objectContaining({ bytesChanged: -60, objectsChanged: -1 }), expect.any(Number) ) } finally { @@ -794,7 +796,7 @@ describe('object versioning - version-aware writes', () => { // and neither write reused another row's slot. expect(removedSpy).toHaveBeenCalledTimes(2) expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ bytesChanged: 0 }), + expect.objectContaining({ bytesChanged: 0, objectsChanged: 0 }), expect.any(Number) ) } finally { @@ -846,6 +848,7 @@ describe('object versioning - version-aware writes', () => { expect.objectContaining({ name: objectName, bytesChanged: -70, + objectsChanged: -1, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: 70 }), }), @@ -896,6 +899,7 @@ describe('object versioning - version-aware writes', () => { // ...but the bytes physically freed came from the stale row the // marker's insert reused, reported separately via replacedObject. bytesChanged: -70, + objectsChanged: -1, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: 70 }), }), @@ -1123,7 +1127,12 @@ describe('object versioning - version-aware writes', () => { // v2 (hard-deleted) and v1 (silently promoted) are two different rows // the promotion must not be folded into the removal event expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: objectName, version: 'v2', bytesChanged: -2 }), + expect.objectContaining({ + name: objectName, + version: 'v2', + bytesChanged: -2, + objectsChanged: -1, + }), expect.any(Number) ) expect(promotedSpy).toHaveBeenCalledWith( @@ -1133,6 +1142,7 @@ describe('object versioning - version-aware writes', () => { version: 'v1', metadata: expect.objectContaining({ size: 1 }), bytesChanged: 0, + objectsChanged: 0, }), expect.any(Number) ) @@ -1169,7 +1179,12 @@ describe('object versioning - version-aware writes', () => { // Hard-deleting the marker itself frees no bytes (markers own none), // and the content it was hiding silently becomes current again. expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: objectName, version: marker!.version, bytesChanged: 0 }), + expect.objectContaining({ + name: objectName, + version: marker!.version, + bytesChanged: 0, + objectsChanged: 0, + }), expect.any(Number) ) expect(removedSpy.mock.calls[0][0]).not.toHaveProperty('replacedObject') @@ -1180,6 +1195,7 @@ describe('object versioning - version-aware writes', () => { version: 'v1', metadata: expect.objectContaining({ size: 1 }), bytesChanged: 0, + objectsChanged: 0, }), expect.any(Number) ) @@ -1239,7 +1255,12 @@ describe('object versioning - version-aware writes', () => { await tHelper.storage.from(bucketId).deleteObjects([{ path: objectName, versionId: 'v2' }]) expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: objectName, version: 'v2', bytesChanged: -2 }), + expect.objectContaining({ + name: objectName, + version: 'v2', + bytesChanged: -2, + objectsChanged: -1, + }), expect.any(Number) ) expect(promotedSpy).toHaveBeenCalledWith( @@ -1249,6 +1270,7 @@ describe('object versioning - version-aware writes', () => { version: 'v1', metadata: expect.objectContaining({ size: 1 }), bytesChanged: 0, + objectsChanged: 0, }), expect.any(Number) ) @@ -1302,6 +1324,7 @@ describe('object versioning - version-aware writes', () => { name: objectName, bucketId, bytesChanged: -42, + objectsChanged: -1, }), expect.any(Number) ) @@ -1343,7 +1366,7 @@ describe('object versioning - version-aware writes', () => { // A plain-name entry archives the current version behind a fresh delete-marker (no bytes freed yet) // while an explicit versionId hard-deletes that exact row (its own bytes freed outright) expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: plainName, bucketId, bytesChanged: 0 }), + expect.objectContaining({ name: plainName, bucketId, bytesChanged: 0, objectsChanged: 0 }), expect.any(Number) ) expect(removedSpy).toHaveBeenCalledWith( @@ -1352,6 +1375,7 @@ describe('object versioning - version-aware writes', () => { bucketId, version: 'versioned-v1', bytesChanged: -9, + objectsChanged: -1, }), expect.any(Number) ) @@ -1392,11 +1416,21 @@ describe('object versioning - version-aware writes', () => { // Both versions for this key are gone, so there's nothing left behind either one to promote // v1 being in the same batch as the deleted current version (v2) must not make it look like a promotion expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: objectName, version: 'v1', bytesChanged: -1 }), + expect.objectContaining({ + name: objectName, + version: 'v1', + bytesChanged: -1, + objectsChanged: -1, + }), expect.any(Number) ) expect(removedSpy).toHaveBeenCalledWith( - expect.objectContaining({ name: objectName, version: 'v2', bytesChanged: -2 }), + expect.objectContaining({ + name: objectName, + version: 'v2', + bytesChanged: -2, + objectsChanged: -1, + }), expect.any(Number) ) expect(promotedSpy).not.toHaveBeenCalled() diff --git a/src/test/uploader.test.ts b/src/test/uploader.test.ts index f777bb0b1..2c07b798d 100644 --- a/src/test/uploader.test.ts +++ b/src/test/uploader.test.ts @@ -1039,7 +1039,7 @@ describe('Upload completion conflicts', () => { ) test.each([ - ['nothing replaced (archives the previous version, frees nothing)', undefined, 1], + ['nothing replaced (archives the previous version, frees nothing)', undefined, 1, 1], [ 'a real-content row replaced in place (frees its bytes)', { @@ -1049,15 +1049,17 @@ describe('Upload completion conflicts', () => { metadata: { size: 50 }, }, 1 - 50, + 0, ], [ 'a null-version delete marker replaced (frees nothing, markers own no bytes)', { id: 'old-object-id', version: 'old-version', isDeleteMarker: true }, 1, + 1, ], ] as const)( - 'completeUpload reports bytesChanged as the true byte delta when %s', - async (_mode, replaced, expectedBytesChanged) => { + 'completeUpload reports bytesChanged and objectsChanged as the true delta when %s', + async (_mode, replaced, expectedBytesChanged, expectedObjectsChanged) => { const deleteSpy = vi.spyOn(ObjectAdminDelete, 'send').mockResolvedValue(undefined) const putSpy = vi.spyOn(ObjectCreatedPutEvent, 'sendWebhook').mockResolvedValue(undefined) const postSpy = vi.spyOn(ObjectCreatedPostEvent, 'sendWebhook').mockResolvedValue(undefined) @@ -1108,6 +1110,7 @@ describe('Upload completion conflicts', () => { expect(putSpy).toHaveBeenCalledWith( expect.objectContaining({ bytesChanged: expectedBytesChanged, + objectsChanged: expectedObjectsChanged, ...(expectedReplaced ? { replacedObject: { @@ -1180,6 +1183,8 @@ describe('Upload completion conflicts', () => { expect(putSpy).toHaveBeenCalledWith( expect.objectContaining({ bytesChanged: 1, + // The replaced row still counts as an object even though it has no size + objectsChanged: 0, replacedObject: { version: 'old-version', metadata: null }, }), expect.any(Number) @@ -1253,6 +1258,7 @@ describe('Upload completion conflicts', () => { expect(sendWebhookSpy).toHaveBeenCalledWith( expect.objectContaining({ bytesChanged: 1 - 30, + objectsChanged: 0, replacedObject: { version: 'old-null-version', metadata: { size: 30 } }, }), expect.any(Number) diff --git a/src/test/webhooks.test.ts b/src/test/webhooks.test.ts index 411eddb00..4193434a9 100644 --- a/src/test/webhooks.test.ts +++ b/src/test/webhooks.test.ts @@ -97,6 +97,7 @@ describe('Webhooks', () => { name: `public/${fileName}.png`, uploadType: 'standard', bytesChanged: 3746, + objectsChanged: 1, tenant: expect.objectContaining({ ref: 'bjhaohmqunupljrqypxz', }), @@ -198,6 +199,7 @@ describe('Webhooks', () => { metadata: expect.objectContaining({ size: 3746 }), // The new upload's size, net of the replaced object's 111 bytes. bytesChanged: 3746 - 111, + objectsChanged: 0, // The replaced row is reported so bytesChanged is independently // verifiable from the payload: 3746 - 111. replacedObject: expect.objectContaining({ @@ -213,6 +215,8 @@ describe('Webhooks', () => { expect(putPayload.bytesChanged).toBe( putPayload.metadata.size - putPayload.replacedObject.metadata.size ) + // A replaced row is netted out: one object removed, one added + expect(putPayload.objectsChanged).toBe(0) }) it('emits ObjectCreated:Post, not Put, when x-upsert=true is set on a first-time create (nothing existed to replace)', async () => { @@ -238,6 +242,7 @@ describe('Webhooks', () => { expect(queuedWebhook.event.payload.oldObject).toBeUndefined() // Nothing existed to replace, so the full new size is the byte delta. expect(queuedWebhook.event.payload.bytesChanged).toBe(3746) + expect(queuedWebhook.event.payload.objectsChanged).toBe(1) expect(queuedWebhook.event.payload.replacedObject).toBeUndefined() }) @@ -275,6 +280,7 @@ describe('Webhooks', () => { version: obj.version, metadata: obj.metadata, bytesChanged: -3746, + objectsChanged: -1, tenant: { host: undefined, ref: 'bjhaohmqunupljrqypxz', @@ -340,6 +346,7 @@ describe('Webhooks', () => { version: expect.any(String), // DISABLED: the source's own content is freed by the move. bytesChanged: -3746, + objectsChanged: -1, tenant: { host: undefined, ref: 'bjhaohmqunupljrqypxz', @@ -383,6 +390,7 @@ describe('Webhooks', () => { }), name: `${obj.name}-moved`, bytesChanged: 3746, + objectsChanged: 1, oldObject: { bucketId: 'bucket6', name: obj.name, @@ -493,6 +501,7 @@ describe('Webhooks', () => { }), name: `${obj.name}-copied`, bytesChanged: 3746, + objectsChanged: 1, tenant: { host: undefined, ref: 'bjhaohmqunupljrqypxz', @@ -589,6 +598,7 @@ describe('Webhooks', () => { metadata: expect.objectContaining({ size: 3746 }), // The new copy's size, net of the replaced destination's 111 bytes. bytesChanged: 3746 - 111, + objectsChanged: 0, replacedObject: expect.objectContaining({ metadata: expect.objectContaining({ size: 111 }), }), @@ -602,6 +612,8 @@ describe('Webhooks', () => { expect(copyPayload.bytesChanged).toBe( copyPayload.metadata.size - copyPayload.replacedObject.metadata.size ) + // A replaced row is netted out: one object removed, one added + expect(copyPayload.objectsChanged).toBe(0) }) it('will emit webhooks for each deleted object during empty bucket operation', async () => { @@ -665,6 +677,7 @@ describe('Webhooks', () => { version: obj.version, metadata: obj.metadata, bytesChanged: -3746, + objectsChanged: -1, tenant: { host: undefined, ref: 'bjhaohmqunupljrqypxz', @@ -741,6 +754,7 @@ describe('Webhooks', () => { version: obj.version, metadata: obj.metadata, bytesChanged: -3746, + objectsChanged: -1, tenant: { host: undefined, ref: tenantId, @@ -826,6 +840,7 @@ describe('Webhooks', () => { version: obj.version, metadata: obj.metadata, bytesChanged: -3746, + objectsChanged: -1, tenant: { host: undefined, ref: tenantId,