diff --git a/src/access-scope.ts b/src/access-scope.ts index 12b571f..c7020d3 100644 --- a/src/access-scope.ts +++ b/src/access-scope.ts @@ -125,7 +125,35 @@ export function accessWhere( return where; } - return { AND: [...(where ? [where] : []), ...filters] }; + /* + * The caller's own conditions stay at the top level; only ours go into + * `AND`. + * + * Nesting the caller's `where` inside `AND` — which this did — is correct + * for `findMany` and silently fatal for `update`, `delete` and + * `findUnique`. Those take a `WhereUniqueInput`, and Prisma requires a + * unique field to appear at the *top level* of it: moving `id` one level + * down leaves the argument with no unique field at all, and Prisma refuses + * the call outright rather than filtering anything. Every scoped update in + * a service using this extension therefore threw a validation error, in + * scope or out — and a service that logs rather than rethrows reports that + * to its caller as a save that succeeded and changed nothing. + * + * Spreading is exactly as safe as nesting. Top-level conditions and `AND` + * conjoin, so a caller supplying their own value for a scope column gets + * ours as well and cannot widen past it; a caller's own `AND` is kept + * rather than overwritten, which is the one key that would otherwise be + * lost by spreading. + */ + const { AND: callerAnd, ...rest } = where ?? {}; + const conjoined = + callerAnd === undefined + ? [] + : Array.isArray(callerAnd) + ? callerAnd + : [callerAnd]; + + return { ...rest, AND: [...conjoined, ...filters] }; } type ReadArgs = { where?: Record }; @@ -173,8 +201,9 @@ export function accessScope({ models, resolvers }: AccessScopeOptions) { return; } const a = args as ReadArgs; - // AND our filters onto the caller's `where` (never merged by key) so the - // scope cannot be removed or overridden by caller-supplied conditions. + // AND our filters onto the caller's `where` — as a conjunction, never + // merged into their keys — so the scope cannot be removed or overridden + // by caller-supplied conditions. const scoped = accessWhere(a.where, config, resolvers); if (scoped !== a.where) { a.where = scoped; diff --git a/src/iso-dates.ts b/src/iso-dates.ts index 14782c1..d9f1c7d 100644 --- a/src/iso-dates.ts +++ b/src/iso-dates.ts @@ -24,14 +24,37 @@ import { Prisma } from '@prisma/client/extension'; -/** Recursively replace every `Date` with its ISO-8601 string, structure intact. */ -function toIsoDates(value: unknown): unknown { +/** + * Recursively replace every `Date` with its ISO-8601 string, structure intact. + * + * Exported so it can be tested without a database, like `accessWhere`: what it + * does to a shape is the whole of this extension, and the interesting cases — + * a buffer, a nested date, an array of rows — are all reachable from here. + */ +export function toIsoDates(value: unknown): unknown { if (value instanceof Date) { return value.toISOString(); } if (Array.isArray(value)) { return value.map(toIsoDates); } + /* + * Binary is returned as it came, and this is not an optimisation. + * + * The branch below rebuilds any object by walking `Object.entries`, and a + * `Buffer` walked that way becomes `{ "0": 137, "1": 80, … }` — a plain + * object with one key per byte, which is no longer a buffer, is roughly + * fifty times the size, and fails every `Buffer.isBuffer` check downstream. + * A `Bytes` column read through this extension arrived unusable and the + * failure looked like the row not existing. + * + * `ArrayBuffer.isView` covers `Buffer`, every typed array and `DataView`; + * the buffer itself is checked beside it. None of them can contain a + * `Date`, so there is nothing here to walk for. + */ + if (ArrayBuffer.isView(value) || value instanceof ArrayBuffer) { + return value; + } if (value !== null && typeof value === 'object') { const out: Record = {}; for (const [key, nested] of Object.entries(value)) { diff --git a/test/unit/access-scope.spec.ts b/test/unit/access-scope.spec.ts index 80c7928..900aeef 100644 --- a/test/unit/access-scope.spec.ts +++ b/test/unit/access-scope.spec.ts @@ -130,7 +130,8 @@ test('the caller where is preserved and AND-ed, never replaced', () => { resolvers({ user: 'u1' }), ); assert.deepEqual(out, { - AND: [{ active: true }, { OR: [{ createdBy: 'u1' }] }], + active: true, + AND: [{ OR: [{ createdBy: 'u1' }] }], }); }); @@ -142,3 +143,61 @@ test('a missing resolver for a configured level is skipped', () => { ); assert.deepEqual(out, { AND: [{ OR: [{ createdBy: 'u1' }] }] }); }); + +/* + * The regression the rest of this file exists for. + * + * `update`, `delete` and `findUnique` take a `WhereUniqueInput`, and Prisma + * requires a unique field at its *top level*. Nesting the caller's `where` + * inside `AND` — which this did — left the argument with no unique field, so + * Prisma refused the call instead of scoping it, and every scoped update in + * every service using this extension failed. + */ +test('a unique field stays at the top level, where update needs it', () => { + const out = accessWhere( + { id: 'r1' }, + { portfolio: ['portfolioId'] }, + resolvers({ portfolio: ['p1'] }), + ); + assert.deepEqual(out, { + id: 'r1', + AND: [{ OR: [{ portfolioId: { in: ['p1'] } }] }], + }); +}); + +test("the caller's own AND is conjoined rather than overwritten", () => { + const out = accessWhere( + { id: 'r1', AND: [{ active: true }] }, + { portfolio: ['portfolioId'] }, + resolvers({ portfolio: ['p1'] }), + ); + assert.deepEqual(out, { + id: 'r1', + AND: [{ active: true }, { OR: [{ portfolioId: { in: ['p1'] } }] }], + }); +}); + +test('a single-object AND from the caller is conjoined too', () => { + const out = accessWhere( + { id: 'r1', AND: { active: true } }, + { portfolio: ['portfolioId'] }, + resolvers({ portfolio: ['p1'] }), + ); + assert.deepEqual(out, { + id: 'r1', + AND: [{ active: true }, { OR: [{ portfolioId: { in: ['p1'] } }] }], + }); +}); + +test('a caller condition on a scope column is kept, so it cannot widen', () => { + const out = accessWhere( + { portfolioId: 'p9' }, + { portfolio: ['portfolioId'] }, + resolvers({ portfolio: ['p1'] }), + ); + // Both conditions apply: asking for p9 under a p1 scope matches nothing. + assert.deepEqual(out, { + portfolioId: 'p9', + AND: [{ OR: [{ portfolioId: { in: ['p1'] } }] }], + }); +}); diff --git a/test/unit/barrel.spec.ts b/test/unit/barrel.spec.ts index 8bc8189..411b37f 100644 --- a/test/unit/barrel.spec.ts +++ b/test/unit/barrel.spec.ts @@ -31,6 +31,27 @@ import { fileURLToPath } from 'node:url'; const HERE = fileURLToPath(new URL('.', import.meta.url)); const ROOT = join(HERE, '..', '..'); +/** Everything the package publishes at runtime, in sorted order. */ +const EXPORTS = [ + 'AuditAction', + 'CHANGE_NOTIFY_CHANNEL', + 'CHANGE_NOTIFY_FUNCTION_NAME', + 'CHANGE_NOTIFY_TRIGGER_NAME', + 'accessScope', + 'accessWhere', + 'audit', + 'authorship', + 'installArchiving', + 'installChangeTriggers', + 'isSqlLogSuppressed', + 'isoDates', + 'migrateDown', + 'prettifySql', + 'silently', + 'softDelete', + 'toIsoDates', +]; + // The package is ESM, but Node >= 22 lets CommonJS `require()` an ESM module — // UNLESS some module in the graph is async (top-level await), which fails hard // with ERR_REQUIRE_ASYNC_MODULE. Two things put an async module in this graph and @@ -50,13 +71,25 @@ test('the package barrel is require()-able from CommonJS', () => { [ '-e', 'const m = require(process.argv[1]);' + - 'console.log(Object.keys(m).length)', + 'console.log(Object.keys(m).sort().join(","))', join(ROOT, 'index.js'), ], { encoding: 'utf8', cwd: ROOT }, ); - assert.equal(Number(out.trim()), 16, 'expected 16 runtime exports'); + /* + * The names rather than a count, because a count cannot say what moved. + * "expected 16 runtime exports" is what this said when an export was added + * deliberately, and it sends the reader to the wrong question — whether the + * barrel broke — when the answer is simply that the surface changed and + * this list is where it is written down. + * + * Which makes the assertion do two jobs: the require() itself is the one + * that matters, since an async module anywhere in the graph fails it + * outright, and the list is the package's public surface, changed on + * purpose or not at all. + */ + assert.deepEqual(out.trim().split(','), EXPORTS); }); test('the barrel exports the same names to ESM and CommonJS', async () => { diff --git a/test/unit/iso-dates.spec.ts b/test/unit/iso-dates.spec.ts new file mode 100644 index 0000000..12234b8 --- /dev/null +++ b/test/unit/iso-dates.spec.ts @@ -0,0 +1,95 @@ +/*! + * toIsoDates() unit tests + * + * I'm Queue Software Project + * Copyright (C) 2025 imqueue.com + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + * If you want to use this code in a closed source (commercial) project, you can + * purchase a proprietary commercial license. Please contact us at + * to get commercial licensing options. + */ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { toIsoDates } from '../../index.js'; + +test('a date becomes its ISO string', () => { + assert.equal( + toIsoDates(new Date('2026-08-16T11:35:59.323Z')), + '2026-08-16T11:35:59.323Z', + ); +}); + +test('a date nested in a row is replaced where it sits', () => { + assert.deepEqual( + toIsoDates({ id: 'a', at: new Date('2026-01-02T03:04:05.000Z') }), + { id: 'a', at: '2026-01-02T03:04:05.000Z' }, + ); +}); + +test('an array of rows keeps its order and its shape', () => { + assert.deepEqual( + toIsoDates([{ at: new Date(0) }, { at: new Date(1000) }]), + [ + { at: '1970-01-01T00:00:00.000Z' }, + { at: '1970-01-01T00:00:01.000Z' }, + ], + ); +}); + +/* + * The regression. Walking an object with `Object.entries` turns a buffer into + * `{ "0": 137, "1": 80, … }` — one key per byte, roughly fifty times the size, + * and no longer something `Buffer.isBuffer` recognises. A `Bytes` column read + * through this extension arrived unusable, and the failure looked like the row + * not existing at all. + */ +test('a buffer comes back as the same buffer', () => { + const bytes = Buffer.from([0x89, 0x50, 0x4e, 0x47]); + const out = toIsoDates(bytes); + + assert.equal(out, bytes); + assert.ok(Buffer.isBuffer(out)); +}); + +test('a buffer inside a row survives the walk', () => { + const bytes = Buffer.from('a logo, more or less'); + const row = toIsoDates({ + name: 'mark.svg', + data: bytes, + at: new Date('2026-08-16T00:00:00.000Z'), + }) as { name: string; data: unknown; at: string }; + + assert.ok(Buffer.isBuffer(row.data)); + assert.equal(row.data, bytes); + assert.equal(row.at, '2026-08-16T00:00:00.000Z'); +}); + +test('every typed array is left alone, not only Buffer', () => { + for (const view of [ + new Uint8Array([1, 2]), + new Int16Array([3]), + new Float64Array([4.5]), + new DataView(new ArrayBuffer(2)), + ]) { + assert.equal(toIsoDates(view), view, view.constructor.name); + } +}); + +test('a bare ArrayBuffer is left alone too', () => { + const buffer = new ArrayBuffer(4); + + assert.equal(toIsoDates(buffer), buffer); +});