Skip to content

Commit d771400

Browse files
committed
chore: apply Kirills comments partially
1 parent 2e7b38c commit d771400

3 files changed

Lines changed: 47 additions & 29 deletions

File tree

packages/db-lib/src/commondao/common.dao.model.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,6 @@ export interface CommonDaoHooks<
6767
*/
6868
beforeSave?: (dbm: DBM) => void
6969

70-
/**
71-
* Invoked when the `__compressed` Buffer for a row exceeds the
72-
* `compress.warnSizeBytes` threshold. No-op unless both `compress.warnSizeBytes`
73-
* is set and this hook is provided.
74-
*
75-
* Consumer decides how to surface the warning (Sentry, logger, metrics, etc.).
76-
*/
77-
onOversizeWarning?: (info: { table: string; id: string; size: number; threshold: number }) => void
78-
7970
/**
8071
* If hook is defined - allows to prevent or modify the error thrown.
8172
* Return `false` to prevent throwing an error.
@@ -234,13 +225,21 @@ export interface CommonDaoCfg<
234225
*/
235226
level?: Integer
236227
/**
237-
* If set, the `onOversizeWarning` hook is invoked whenever the resulting
238-
* `__compressed` Buffer exceeds this size in bytes. Useful for monitoring
239-
* rows approaching DB row-size limits (e.g. Datastore's 1MB per entity).
228+
* If set, `onOversizeWarning` is invoked whenever the resulting `__compressed`
229+
* Buffer exceeds this size in bytes. Useful for monitoring rows approaching
230+
* DB row-size limits (e.g. Datastore's 1MB per entity).
240231
*
241-
* No-op unless `hooks.onOversizeWarning` is also provided.
232+
* No-op unless `onOversizeWarning` is also provided.
242233
*/
243234
warnSizeBytes?: number
235+
/**
236+
* Invoked when the `__compressed` Buffer exceeds `warnSizeBytes`.
237+
*
238+
* Called with a shallow clone of the DBM (since the original is mutated
239+
* later in the compression step) and the resulting compressed size in bytes.
240+
* The consumer decides how to surface the warning (Sentry, logger, metrics, etc.).
241+
*/
242+
onOversizeWarning?: (dbm: DBM, size: number) => void
244243
}
245244
}
246245

packages/db-lib/src/commondao/common.dao.test.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,7 @@ describe('auto compression', () => {
12761276

12771277
describe('warnSizeBytes', () => {
12781278
type OnOversizeWarning = NonNullable<
1279-
NonNullable<CommonDaoCfg<Item>['hooks']>['onOversizeWarning']
1279+
NonNullable<CommonDaoCfg<Item>['compress']>['onOversizeWarning']
12801280
>
12811281

12821282
test('fires onOversizeWarning when compressed payload exceeds threshold', async () => {
@@ -1287,20 +1287,42 @@ describe('auto compression', () => {
12871287
compress: {
12881288
keys: ['obj', 'shu'],
12891289
warnSizeBytes: 1,
1290+
onOversizeWarning,
12901291
},
1291-
hooks: { onOversizeWarning },
12921292
})
12931293

12941294
await dao.save({ id: 'id1', obj: { objId: 'objId1' }, shu: 'shu1' })
12951295

12961296
expect(onOversizeWarning).toHaveBeenCalledTimes(1)
1297-
expect(onOversizeWarning).toHaveBeenCalledWith({
1298-
table: TEST_TABLE,
1297+
const [dbm, size] = onOversizeWarning.mock.calls[0]!
1298+
expect(dbm).toMatchObject({
12991299
id: 'id1',
1300-
size: expect.any(Number),
1301-
threshold: 1,
1300+
obj: { objId: 'objId1' },
1301+
shu: 'shu1',
1302+
})
1303+
// hook receives pre-mutation snapshot (no __compressed yet, source keys still present)
1304+
expect(dbm).not.toHaveProperty('__compressed')
1305+
expect(size).toBeGreaterThan(1)
1306+
})
1307+
1308+
test('hook receives a clone, not a reference to the mutated dbm', async () => {
1309+
let capturedDbm: Item | undefined
1310+
const dao = new CommonDao<Item>({
1311+
table: TEST_TABLE,
1312+
db,
1313+
compress: {
1314+
keys: ['obj', 'shu'],
1315+
warnSizeBytes: 1,
1316+
onOversizeWarning: dbm => {
1317+
capturedDbm = dbm
1318+
},
1319+
},
13021320
})
1303-
expect(onOversizeWarning.mock.calls[0]![0].size).toBeGreaterThan(1)
1321+
1322+
await dao.save({ id: 'id1', obj: { objId: 'objId1' }, shu: 'shu1' })
1323+
1324+
expect(capturedDbm).toMatchObject({ obj: { objId: 'objId1' }, shu: 'shu1' })
1325+
expect(capturedDbm).not.toHaveProperty('__compressed')
13041326
})
13051327

13061328
test('does not fire when compressed payload is under threshold', async () => {
@@ -1311,8 +1333,8 @@ describe('auto compression', () => {
13111333
compress: {
13121334
keys: ['obj', 'shu'],
13131335
warnSizeBytes: 1_000_000,
1336+
onOversizeWarning,
13141337
},
1315-
hooks: { onOversizeWarning },
13161338
})
13171339

13181340
await dao.save({ id: 'id1', obj: { objId: 'objId1' }, shu: 'shu1' })

packages/db-lib/src/commondao/common.dao.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -849,19 +849,16 @@ export class CommonDao<
849849
private async compress(dbm: DBM): Promise<void> {
850850
if (!this.cfg.compress?.keys.length) return // No compression requested
851851

852-
const { keys, level = 1, warnSizeBytes } = this.cfg.compress
852+
const { keys, level = 1, warnSizeBytes, onOversizeWarning } = this.cfg.compress
853853
const properties = _pick(dbm, keys)
854854
const bufferString = JSON.stringify(properties)
855855
// Unlike `decompress`, we're testing to use async zstd compression.
856856
// Async Decompression leaks memory severely. But Compression seems fine.
857857
const __compressed = await zip2.zstdCompress(bufferString, level)
858-
if (warnSizeBytes && __compressed.byteLength > warnSizeBytes) {
859-
this.cfg.hooks?.onOversizeWarning?.({
860-
table: this.cfg.table,
861-
id: dbm.id,
862-
size: __compressed.byteLength,
863-
threshold: warnSizeBytes,
864-
})
858+
if (warnSizeBytes && onOversizeWarning && __compressed.byteLength > warnSizeBytes) {
859+
// Shallow-clone: `dbm` is mutated below (omit source keys + assign __compressed),
860+
// so we hand the hook a snapshot of the pre-mutation state.
861+
onOversizeWarning({ ...dbm }, __compressed.byteLength)
865862
}
866863
_omitWithUndefined(dbm as any, _objectKeys(properties), { mutate: true })
867864
Object.assign(dbm, { __compressed })

0 commit comments

Comments
 (0)