From 069b8c80b0fc134949b9ec43b33cd80571e34be9 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 5 Oct 2026 18:20:42 +0200 Subject: [PATCH 1/5] test(agent-bff): pin composite primary key mapping per agent stack --- .../test/data/composite-primary-key.test.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 packages/agent-bff/test/data/composite-primary-key.test.ts diff --git a/packages/agent-bff/test/data/composite-primary-key.test.ts b/packages/agent-bff/test/data/composite-primary-key.test.ts new file mode 100644 index 0000000000..92d1113fe6 --- /dev/null +++ b/packages/agent-bff/test/data/composite-primary-key.test.ts @@ -0,0 +1,73 @@ +import type { ForestSchemaCollection } from '@forestadmin/forestadmin-client'; + +import { mapListResponse } from '../../src/data/response-mappers'; +import ReadModel from '../../src/read-model/read-model'; + +const COLLECTION = 'EdgeCompositePk'; + +function schemaWithKeys(keys: string[]): ForestSchemaCollection[] { + return [ + { + name: COLLECTION, + fields: [ + { field: 'created_at', type: 'Date', isPrimaryKey: false }, + { field: 'payload', type: 'String', isPrimaryKey: false }, + { field: 'seq', type: 'Number', isPrimaryKey: keys.includes('seq') }, + { field: 'tenant_id', type: 'String', isPrimaryKey: keys.includes('tenant_id') }, + ], + }, + ] as unknown as ForestSchemaCollection[]; +} + +const ROWS = [ + { tenantId: 'acme', seq: 1 }, + { tenantId: 'acme', seq: 2 }, + { tenantId: 'globex', seq: 1 }, +]; + +function listKeys(keys: string[], idOf: (row: (typeof ROWS)[number]) => string) { + const primaryKeys = new ReadModel(schemaWithKeys(keys)).getPrimaryKeys(COLLECTION); + const records = ROWS.map(row => ({ id: idOf(row), ...row, payload: 'p' })); + + return mapListResponse(COLLECTION, records, primaryKeys).data.map( + ({ __forest: { primaryKey } }) => primaryKey, + ); +} + +describe('a (tenant_id, seq) composite primary key, per agent stack', () => { + it('should unfold both columns on forest-express-sequelize, which packs in declaration order', () => { + expect(listKeys(['seq', 'tenant_id'], row => `${row.tenantId}|${row.seq}`)).toEqual([ + { seq: 1, tenant_id: 'acme' }, + { seq: 2, tenant_id: 'acme' }, + { seq: 1, tenant_id: 'globex' }, + ]); + }); + + it('should unfold both columns on a v2 agent, which packs in schema order', () => { + expect(listKeys(['seq', 'tenant_id'], row => `${row.seq}|${row.tenantId}`)).toEqual([ + { seq: 1, tenant_id: 'acme' }, + { seq: 2, tenant_id: 'acme' }, + { seq: 1, tenant_id: 'globex' }, + ]); + }); + + it('should report the single key forest_liana declares when its model narrows the key to one column', () => { + expect(listKeys(['tenant_id'], row => row.tenantId)).toEqual([ + { tenant_id: 'acme' }, + { tenant_id: 'acme' }, + { tenant_id: 'globex' }, + ]); + }); + + it('should fail the whole list on the JSON array id forest_liana serializes for a composite model key', () => { + expect(() => + listKeys(['seq', 'tenant_id'], row => JSON.stringify([row.tenantId, row.seq])), + ).toThrow( + expect.objectContaining({ + type: 'mapping_error', + status: 500, + message: 'Cannot build primary key: expected 2 values, found 1', + }), + ); + }); +}); From 615a573063bca23d90b45687ef7dfa09193c0997 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 5 Oct 2026 18:34:55 +0200 Subject: [PATCH 2/5] fix(agent-bff): read the json array composite id forest_liana serializes --- packages/agent-bff/src/data/pack-id.ts | 92 +++++++++++++---- .../agent-bff/src/openapi/record-schemas.ts | 5 +- packages/agent-bff/src/openapi/schemas.ts | 3 +- .../agent-bff/src/openapi/unfolded-paths.ts | 11 +-- .../test/data/composite-primary-key.test.ts | 16 ++- packages/agent-bff/test/data/pack-id.test.ts | 98 +++++++++++++++++++ .../test/openapi/openapi-unfolded.test.ts | 5 +- 7 files changed, 189 insertions(+), 41 deletions(-) diff --git a/packages/agent-bff/src/data/pack-id.ts b/packages/agent-bff/src/data/pack-id.ts index 92255533bc..f465171da9 100644 --- a/packages/agent-bff/src/data/pack-id.ts +++ b/packages/agent-bff/src/data/pack-id.ts @@ -4,7 +4,7 @@ import { recordKey } from '@forestadmin/agent-client'; import { mappingError } from '../http/bff-local-errors'; -export const PACKED_ID_SEPARATOR = '|'; +const PACKED_ID_SEPARATOR = '|'; // The only column type unpacked to a number, mirroring the agent's `IdUtils.unpackId`. const NUMBER_COLUMN_TYPE = 'Number'; @@ -73,13 +73,11 @@ function comparableValue(record: Record, key: PrimaryKeyField): * * With no record, the values are returned whole and the pairing is the positional one. */ -function segmentsByKey( +function matchedByRecord( values: string[], primaryKeys: PrimaryKeyField[], - record?: Record, -): string[] { - if (!record) return values; - + record: Record, +): string[] | null { const claimed = values.map(() => false); const matched = primaryKeys.map(key => { const wanted = comparableValue(record, key); @@ -91,16 +89,81 @@ function segmentsByKey( return values[index]; }); - if (matched.filter(value => value === null).length > 1) return values; + if (matched.filter(value => value === null).length > 1) return null; const leftovers = values.filter((_, index) => !claimed[index]); return matched.map(value => value ?? (leftovers.shift() as string)); } +function segmentsByKey( + values: string[], + primaryKeys: PrimaryKeyField[], + record?: Record, +): string[] { + if (!record) return values; + + return matchedByRecord(values, primaryKeys, record) ?? values; +} + +function parseJsonArrayId(packedId: string, keyCount: number): unknown[] | null { + if (keyCount < 2 || !packedId.startsWith('[') || !packedId.endsWith(']')) return null; + + try { + const parsed: unknown = JSON.parse(packedId); + + return Array.isArray(parsed) && parsed.length === keyCount ? parsed : null; + } catch { + return null; + } +} + +function jsonArrayValue(element: unknown): string { + if (typeof element === 'string') return element; + if (typeof element === 'number' && Number.isSafeInteger(element)) return String(element); + + const kind = element === null ? 'null' : typeof element; + + throw mappingError(`Cannot build primary key: unsupported ${kind} value in a composite id`); +} + +function jsonArraySegments( + elements: unknown[], + primaryKeys: PrimaryKeyField[], + record?: Record, +): string[] { + const values = elements.map(jsonArrayValue); + const segments = record ? matchedByRecord(values, primaryKeys, record) : null; + + if (!segments) { + throw mappingError( + 'Cannot build primary key: the record does not say which composite id value is whose', + ); + } + + return segments; +} + +function pipeSegments( + packedId: string, + primaryKeys: PrimaryKeyField[], + record?: Record, +): string[] { + const values = packedId.split(PACKED_ID_SEPARATOR); + + if (values.length !== primaryKeys.length) { + throw mappingError( + `Cannot build primary key: expected ${primaryKeys.length} values, found ${values.length}`, + ); + } + + return segmentsByKey(values, primaryKeys, record); +} + /** * Rebuild the structured primary key of a record from its opaque packed id, mirroring the agent's - * `IdUtils.packId`/`unpackId` (`|`-joined values, `Number` columns cast back to numbers). Returns a + * `IdUtils.packId`/`unpackId` (`|`-joined values, `Number` columns cast back to numbers), or the + * JSON array `forest_liana` serializes a composite key as, paired by the record only. Returns a * `{ pkField: value }` map for `__forest.primaryKey`. Throws a mapping error rather than emitting a * malformed key when the schema lacks key metadata or the packed id shape does not match it. * @@ -126,15 +189,10 @@ export default function unpackPrimaryKey( }; } - const values = packedId.split(PACKED_ID_SEPARATOR); - - if (values.length !== primaryKeys.length) { - throw mappingError( - `Cannot build primary key: expected ${primaryKeys.length} values, found ${values.length}`, - ); - } - - const segments = segmentsByKey(values, primaryKeys, record); + const elements = parseJsonArrayId(packedId, primaryKeys.length); + const segments = elements + ? jsonArraySegments(elements, primaryKeys, record) + : pipeSegments(packedId, primaryKeys, record); const result: Record = {}; primaryKeys.forEach(({ name, type }, index) => { diff --git a/packages/agent-bff/src/openapi/record-schemas.ts b/packages/agent-bff/src/openapi/record-schemas.ts index ff0042f132..527533e22f 100644 --- a/packages/agent-bff/src/openapi/record-schemas.ts +++ b/packages/agent-bff/src/openapi/record-schemas.ts @@ -5,7 +5,6 @@ import { groupByRecordKey } from '@forestadmin/agent-client'; import toFieldSchema from './field-schemas'; import { quoted } from './names'; -import { PACKED_ID_SEPARATOR } from '../data/pack-id'; // The flat id is the JSON:API resource id, which is a string by specification whatever the key // column holds. `__forest.primaryKey` is the same id unpacked and typed, so the two forms of one @@ -15,8 +14,8 @@ const ID_SCHEMA: SchemaObject = { description: 'The record id, always a string — the agent serializes it as the JSON:API resource id, even ' + 'when the key column is a Number. `__forest.primaryKey` carries the same id TYPED, so ' + - `comparing the two without coercion fails. A composite key is its values joined by ` + - `${quoted(PACKED_ID_SEPARATOR)}.`, + "comparing the two without coercion fails. A composite key is packed in the agent's own " + + 'format, so pass it back as is rather than splitting it.', }; function isReference(schema: SchemaObject | ReferenceObject): schema is ReferenceObject { diff --git a/packages/agent-bff/src/openapi/schemas.ts b/packages/agent-bff/src/openapi/schemas.ts index d922431180..eb774df420 100644 --- a/packages/agent-bff/src/openapi/schemas.ts +++ b/packages/agent-bff/src/openapi/schemas.ts @@ -1,7 +1,6 @@ import { allOperators } from '@forestadmin/datasource-toolkit'; import { z } from './zod-openapi'; -import { PACKED_ID_SEPARATOR } from '../data/pack-id'; import { CountFlatInputs, ListFlatInputs, @@ -303,7 +302,7 @@ export const ListResponseSchema = z .openapi('ListResponse', { description: 'Records are flat, each carrying a `__forest` envelope. A record always holds `id`, the ' + - `agent id as a string — a composite key is its values joined by \`${PACKED_ID_SEPARATOR}\` — ` + + "agent id as a string — a composite key packed in the agent's own format — " + 'while `__forest.primaryKey` holds that same id typed and split per column — with the one ' + 'exception `ForestRecordMeta` describes, where the name it carries is not a column. ' + 'The list never ' + diff --git a/packages/agent-bff/src/openapi/unfolded-paths.ts b/packages/agent-bff/src/openapi/unfolded-paths.ts index b46371cc6d..3ff41ea134 100644 --- a/packages/agent-bff/src/openapi/unfolded-paths.ts +++ b/packages/agent-bff/src/openapi/unfolded-paths.ts @@ -31,7 +31,6 @@ import { SortClauseSchema, TimezoneSchema, } from './schemas'; -import { PACKED_ID_SEPARATOR } from '../data/pack-id'; import { NON_BLANK_PATTERN } from '../data/request-schemas'; import { enumOptionsOf } from '../read-model/field-type'; @@ -443,7 +442,7 @@ const PARENT_ID_SHAPE = { * whatever the key's type is, and forwards it to the agent as a string. So the SHAPE stays that * union — narrowing a numeric key to `number` would make a generated client reject `"123"`, which * the endpoint accepts. What the read-model knows goes in the description: which column the id - * belongs to, and, for a composite key, the `|`-joined order `unpackPrimaryKey` expects. + * belongs to, and, for a composite key, that it is packed in the agent's own format. */ function parentIdSchema( pool: ComponentPool, @@ -461,11 +460,9 @@ function parentIdSchema( // would send a client to build an id the agent unpacks onto the wrong columns. description: `The composite id of the parent ${quoted(parent)} record, taken verbatim from that ` + - `record's own id: the values of ${primaryKeys.map(key => key.name).join(', ')} joined by ` + - `${quoted( - PACKED_ID_SEPARATOR, - )}, in the order the agent packs them. Copy it from a listed ` + - `record rather than assembling it.`, + `record's own id: the values of ${primaryKeys.map(key => key.name).join(', ')} packed ` + + `in the agent's own format and order. Copy it from a listed record rather than ` + + `assembling it.`, }; } diff --git a/packages/agent-bff/test/data/composite-primary-key.test.ts b/packages/agent-bff/test/data/composite-primary-key.test.ts index 92d1113fe6..cc70cfd040 100644 --- a/packages/agent-bff/test/data/composite-primary-key.test.ts +++ b/packages/agent-bff/test/data/composite-primary-key.test.ts @@ -59,15 +59,11 @@ describe('a (tenant_id, seq) composite primary key, per agent stack', () => { ]); }); - it('should fail the whole list on the JSON array id forest_liana serializes for a composite model key', () => { - expect(() => - listKeys(['seq', 'tenant_id'], row => JSON.stringify([row.tenantId, row.seq])), - ).toThrow( - expect.objectContaining({ - type: 'mapping_error', - status: 500, - message: 'Cannot build primary key: expected 2 values, found 1', - }), - ); + it('should unfold both columns on forest_liana, which serializes a composite key as a JSON array', () => { + expect(listKeys(['seq', 'tenant_id'], row => JSON.stringify([row.tenantId, row.seq]))).toEqual([ + { seq: 1, tenant_id: 'acme' }, + { seq: 2, tenant_id: 'acme' }, + { seq: 1, tenant_id: 'globex' }, + ]); }); }); diff --git a/packages/agent-bff/test/data/pack-id.test.ts b/packages/agent-bff/test/data/pack-id.test.ts index beb18753f3..813e102d36 100644 --- a/packages/agent-bff/test/data/pack-id.test.ts +++ b/packages/agent-bff/test/data/pack-id.test.ts @@ -256,4 +256,102 @@ describe('unpackPrimaryKey', () => { }); }); }); + + describe('when a forest_liana agent serializes its composite id as a JSON array', () => { + const LIANA_KEYS = [ + { name: 'seq', type: 'Number' }, + { name: 'tenant_id', type: 'String' }, + ]; + const MAPPING_ERROR = expect.objectContaining({ type: 'mapping_error', status: 500 }); + + it('should place each value on its key by the record, the array following the model order', () => { + expect( + unpackPrimaryKey('["acme",1]', LIANA_KEYS, { tenantId: 'acme', seq: 1, id: '["acme",1]' }), + ).toEqual({ seq: 1, tenant_id: 'acme' }); + }); + + it('should give the one key the record cannot read the value left over', () => { + expect(unpackPrimaryKey('["acme",1]', LIANA_KEYS, { tenantId: 'acme' })).toEqual({ + seq: 1, + tenant_id: 'acme', + }); + }); + + it('should throw rather than pair by position when two keys go unread', () => { + expect(() => unpackPrimaryKey('["acme",1]', LIANA_KEYS, {})).toThrow(MAPPING_ERROR); + expect(() => unpackPrimaryKey('["acme",1]', LIANA_KEYS)).toThrow(MAPPING_ERROR); + }); + + it('should throw on an integer past the safe range instead of rounding it', () => { + expect(() => + unpackPrimaryKey('["acme",9007199254740993]', LIANA_KEYS, { + tenantId: 'acme', + seq: 9007199254740992, + }), + ).toThrow(MAPPING_ERROR); + }); + + it.each([ + ['a boolean', '["acme",true]'], + ['a null', '["acme",null]'], + ['an object', '["acme",{"seq":1}]'], + ['a nested array', '["acme",[1]]'], + ['a fractional number', '["acme",1.5]'], + ])('should throw on %s element rather than build a key from it', (_, packedId) => { + expect(() => unpackPrimaryKey(packedId, LIANA_KEYS, { tenantId: 'acme', seq: 1 })).toThrow( + MAPPING_ERROR, + ); + }); + + it('should keep the segment count error when the array length does not match the keys', () => { + expect(() => + unpackPrimaryKey('["acme",1,2]', LIANA_KEYS, { tenantId: 'acme', seq: 1 }), + ).toThrow( + expect.objectContaining({ + message: 'Cannot build primary key: expected 2 values, found 1', + }), + ); + }); + + it('should read a JSON array whose string value holds the pipe separator', () => { + expect(unpackPrimaryKey('["a|b",1]', LIANA_KEYS, { tenantId: 'a|b', seq: 1 })).toEqual({ + seq: 1, + tenant_id: 'a|b', + }); + }); + + it('should throw on a pipe id that happens to parse as a JSON array the record does not back', () => { + expect(() => + unpackPrimaryKey('["a|b",1]', LIANA_KEYS, { tenantId: '["a', seq: 'b",1]' }), + ).toThrow(MAPPING_ERROR); + }); + + it('should leave a pipe id on the pipe path', () => { + expect(unpackPrimaryKey('acme|1', LIANA_KEYS, { tenantId: 'acme', seq: 1 })).toEqual({ + seq: 1, + tenant_id: 'acme', + }); + }); + + it('should leave a bracketed pipe id that is not JSON on the pipe path', () => { + expect( + unpackPrimaryKey('[x|y]', [ + { name: 'a', type: 'String' }, + { name: 'b', type: 'String' }, + ]), + ).toEqual({ a: '[x', b: 'y]' }); + }); + + it('should keep a single key whole, even when its id looks like a JSON array', () => { + expect(unpackPrimaryKey('["a"]', [{ name: 'code', type: 'String' }])).toEqual({ + code: '["a"]', + }); + }); + + it('should keep a derived key whole, even when its id is a JSON array', () => { + expect( + unpackPrimaryKey('["acme",1]', [{ name: 'id', type: 'String', derived: true }]), + ).toEqual({ id: '["acme",1]' }); + }); + }); }); diff --git a/packages/agent-bff/test/openapi/openapi-unfolded.test.ts b/packages/agent-bff/test/openapi/openapi-unfolded.test.ts index 198ad038c8..ab7466df0d 100644 --- a/packages/agent-bff/test/openapi/openapi-unfolded.test.ts +++ b/packages/agent-bff/test/openapi/openapi-unfolded.test.ts @@ -481,8 +481,9 @@ describe('the unfolded document', () => { }; expect(request.properties.parentId.type).toBe('string'); - expect(request.properties.parentId.description).toContain('shop, number joined by "|"'); - expect(request.properties.parentId.description).toContain('in the order the agent packs them'); + expect(request.properties.parentId.description).toContain( + "shop, number packed in the agent's own format and order", + ); expect(request.properties.parentId.description).toContain( 'Copy it from a listed record rather than assembling it', ); From 00aae4d628162bc2816c131eca0b6c2f583218e1 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 5 Oct 2026 18:42:36 +0200 Subject: [PATCH 3/5] refactor(agent-bff): reduce returns in the record key matcher --- packages/agent-bff/src/data/pack-id.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/agent-bff/src/data/pack-id.ts b/packages/agent-bff/src/data/pack-id.ts index f465171da9..45acfd937f 100644 --- a/packages/agent-bff/src/data/pack-id.ts +++ b/packages/agent-bff/src/data/pack-id.ts @@ -83,10 +83,9 @@ function matchedByRecord( const wanted = comparableValue(record, key); const index = wanted === null ? -1 : values.findIndex((v, i) => !claimed[i] && v === wanted); - if (index === -1) return null; - claimed[index] = true; + if (index !== -1) claimed[index] = true; - return values[index]; + return index === -1 ? null : values[index]; }); if (matched.filter(value => value === null).length > 1) return null; From c705b1f909066d4f29bb439c9b19d48365baeb04 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 5 Oct 2026 18:51:15 +0200 Subject: [PATCH 4/5] fix(agent-bff): require the record to fully back an ambiguous composite id --- packages/agent-bff/src/data/pack-id.ts | 39 +++++++++++++++----- packages/agent-bff/test/data/pack-id.test.ts | 37 +++++++++++++++++-- 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/packages/agent-bff/src/data/pack-id.ts b/packages/agent-bff/src/data/pack-id.ts index 45acfd937f..92ccc06cfe 100644 --- a/packages/agent-bff/src/data/pack-id.ts +++ b/packages/agent-bff/src/data/pack-id.ts @@ -77,6 +77,7 @@ function matchedByRecord( values: string[], primaryKeys: PrimaryKeyField[], record: Record, + unreadAllowed = 1, ): string[] | null { const claimed = values.map(() => false); const matched = primaryKeys.map(key => { @@ -88,7 +89,7 @@ function matchedByRecord( return index === -1 ? null : values[index]; }); - if (matched.filter(value => value === null).length > 1) return null; + if (matched.filter(value => value === null).length > unreadAllowed) return null; const leftovers = values.filter((_, index) => !claimed[index]); @@ -117,22 +118,42 @@ function parseJsonArrayId(packedId: string, keyCount: number): unknown[] | null } } -function jsonArrayValue(element: unknown): string { - if (typeof element === 'string') return element; - if (typeof element === 'number' && Number.isSafeInteger(element)) return String(element); +function jsonArrayValues(elements: unknown[]): string[] | null { + const values = elements.map(element => { + if (typeof element === 'string') return element; - const kind = element === null ? 'null' : typeof element; + return typeof element === 'number' && Number.isSafeInteger(element) ? String(element) : null; + }); + + return values.every((value): value is string => value !== null) ? values : null; +} + +function recordBackedSegments( + packedId: string, + elements: unknown[], + primaryKeys: PrimaryKeyField[], + record: Record, +): string[] | null { + const values = jsonArrayValues(elements); + const pipeValues = packedId.split(PACKED_ID_SEPARATOR); - throw mappingError(`Cannot build primary key: unsupported ${kind} value in a composite id`); + if (pipeValues.length !== primaryKeys.length) { + return values && matchedByRecord(values, primaryKeys, record); + } + + return ( + matchedByRecord(pipeValues, primaryKeys, record, 0) ?? + (values && matchedByRecord(values, primaryKeys, record, 0)) + ); } function jsonArraySegments( + packedId: string, elements: unknown[], primaryKeys: PrimaryKeyField[], record?: Record, ): string[] { - const values = elements.map(jsonArrayValue); - const segments = record ? matchedByRecord(values, primaryKeys, record) : null; + const segments = record ? recordBackedSegments(packedId, elements, primaryKeys, record) : null; if (!segments) { throw mappingError( @@ -190,7 +211,7 @@ export default function unpackPrimaryKey( const elements = parseJsonArrayId(packedId, primaryKeys.length); const segments = elements - ? jsonArraySegments(elements, primaryKeys, record) + ? jsonArraySegments(packedId, elements, primaryKeys, record) : pipeSegments(packedId, primaryKeys, record); const result: Record = {}; diff --git a/packages/agent-bff/test/data/pack-id.test.ts b/packages/agent-bff/test/data/pack-id.test.ts index 813e102d36..2953d0b10e 100644 --- a/packages/agent-bff/test/data/pack-id.test.ts +++ b/packages/agent-bff/test/data/pack-id.test.ts @@ -320,10 +320,39 @@ describe('unpackPrimaryKey', () => { }); }); - it('should throw on a pipe id that happens to parse as a JSON array the record does not back', () => { - expect(() => - unpackPrimaryKey('["a|b",1]', LIANA_KEYS, { tenantId: '["a', seq: 'b",1]' }), - ).toThrow(MAPPING_ERROR); + describe('when the id reads both as a JSON array and as pipe segments', () => { + const STRING_KEYS = [ + { name: 'k1', type: 'String' }, + { name: 'k2', type: 'String' }, + ]; + + it('should keep the pipe reading when the record backs every segment', () => { + expect(unpackPrimaryKey('[")a|b",1]', STRING_KEYS, { k1: '[")a', k2: 'b",1]' })).toEqual({ + k1: '[")a', + k2: 'b",1]', + }); + }); + + it('should keep the pipe reading over a JSON reading the record only partly backs', () => { + const packedId = '["a|b","[\\"a"]'; + + expect(unpackPrimaryKey(packedId, STRING_KEYS, { k1: '["a', k2: 'b","[\\"a"]' })).toEqual({ + k1: '["a', + k2: 'b","[\\"a"]', + }); + }); + + it('should throw rather than give a key the leftover value when neither reading is fully backed', () => { + expect(() => unpackPrimaryKey('["a|b",1]', STRING_KEYS, { k1: '["a' })).toThrow( + MAPPING_ERROR, + ); + }); + + it('should throw when only a JSON reading with an unread key is left', () => { + expect(() => unpackPrimaryKey('["a|b",1]', LIANA_KEYS, { tenantId: 'a|b' })).toThrow( + MAPPING_ERROR, + ); + }); }); it('should leave a pipe id on the pipe path', () => { From ed0d132e0375e8650ea2d62deefa8e4a60702316 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Mon, 5 Oct 2026 18:59:31 +0200 Subject: [PATCH 5/5] refactor(agent-bff): return unread count from the record key matcher --- packages/agent-bff/src/data/pack-id.ts | 53 ++++++++++++-------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/agent-bff/src/data/pack-id.ts b/packages/agent-bff/src/data/pack-id.ts index 92ccc06cfe..6f71a8f0df 100644 --- a/packages/agent-bff/src/data/pack-id.ts +++ b/packages/agent-bff/src/data/pack-id.ts @@ -77,8 +77,7 @@ function matchedByRecord( values: string[], primaryKeys: PrimaryKeyField[], record: Record, - unreadAllowed = 1, -): string[] | null { +): { segments: string[]; unread: number } { const claimed = values.map(() => false); const matched = primaryKeys.map(key => { const wanted = comparableValue(record, key); @@ -89,11 +88,12 @@ function matchedByRecord( return index === -1 ? null : values[index]; }); - if (matched.filter(value => value === null).length > unreadAllowed) return null; - const leftovers = values.filter((_, index) => !claimed[index]); - return matched.map(value => value ?? (leftovers.shift() as string)); + return { + segments: matched.map(value => value ?? (leftovers.shift() as string)), + unread: matched.filter(value => value === null).length, + }; } function segmentsByKey( @@ -103,7 +103,9 @@ function segmentsByKey( ): string[] { if (!record) return values; - return matchedByRecord(values, primaryKeys, record) ?? values; + const { segments, unread } = matchedByRecord(values, primaryKeys, record); + + return unread > 1 ? values : segments; } function parseJsonArrayId(packedId: string, keyCount: number): unknown[] | null { @@ -128,32 +130,26 @@ function jsonArrayValues(elements: unknown[]): string[] | null { return values.every((value): value is string => value !== null) ? values : null; } -function recordBackedSegments( +function jsonArraySegments( packedId: string, - elements: unknown[], primaryKeys: PrimaryKeyField[], - record: Record, -): string[] | null { - const values = jsonArrayValues(elements); + record?: Record, +): string[] { + const values = jsonArrayValues(parseJsonArrayId(packedId, primaryKeys.length) ?? []); const pipeValues = packedId.split(PACKED_ID_SEPARATOR); - if (pipeValues.length !== primaryKeys.length) { - return values && matchedByRecord(values, primaryKeys, record); - } + const read = (candidate: string[] | null, unreadAllowed: number): string[] | null => { + if (!candidate || !record) return null; - return ( - matchedByRecord(pipeValues, primaryKeys, record, 0) ?? - (values && matchedByRecord(values, primaryKeys, record, 0)) - ); -} + const { segments, unread } = matchedByRecord(candidate, primaryKeys, record); -function jsonArraySegments( - packedId: string, - elements: unknown[], - primaryKeys: PrimaryKeyField[], - record?: Record, -): string[] { - const segments = record ? recordBackedSegments(packedId, elements, primaryKeys, record) : null; + return unread <= unreadAllowed ? segments : null; + }; + + const segments = + pipeValues.length === primaryKeys.length + ? read(pipeValues, 0) ?? read(values, 0) + : read(values, 1); if (!segments) { throw mappingError( @@ -209,9 +205,8 @@ export default function unpackPrimaryKey( }; } - const elements = parseJsonArrayId(packedId, primaryKeys.length); - const segments = elements - ? jsonArraySegments(packedId, elements, primaryKeys, record) + const segments = parseJsonArrayId(packedId, primaryKeys.length) + ? jsonArraySegments(packedId, primaryKeys, record) : pipeSegments(packedId, primaryKeys, record); const result: Record = {};