Skip to content

Commit 1b8e8e1

Browse files
committed
test(objectql): pin duplicatePackage's refusal of a renamed reference that meets a held name; changeset
Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude <noreply@anthropic.com>
1 parent ff195fa commit 1b8e8e1

2 files changed

Lines changed: 263 additions & 0 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): `duplicatePackage` refuses a copied item whose renamed object reference meets a name the item already holds, instead of merging the two
6+
7+
Clause-②: no
8+
9+
`duplicatePackage` (ADR-0070 D4, the package duplicate route's method) renames every copied object into the target namespace and rewrites each reference to it where a property names an object: a string, an array entry, or a record key (a permission set's `objects`, a translation's `objects`).
10+
11+
**FROM.** When a copied item also named the object the copy is about to be called, the renamed name landed on a name the item already held, and the copy merged the two with `failed: []`. A permission set holding `objects: { crm_customer: { allowRead: true }, crm2_customer: { allowRead: true, allowEdit: true, allowDelete: true } }`, duplicated into namespace `crm2`, was copied as `objects: { crm2_customer: { allowRead: true, allowEdit: true, allowDelete: true } }`: whichever entry came last was kept, so one grant was dropped, and here the copied object gained edit and delete the source set never granted on its own object. A translation's `objects` dropped one object's labels the same way. A hook whose `object` list held both names was copied as `['crm2_customer', 'crm2_customer']`, so its handler ran twice per event on the copied object.
12+
13+
**TO.** Such an item is listed in `failed[]`, and nothing of it is written (no item row, no history entry). The entry names the item, the property, the name that moved and the name it met, and the source edit that clears it, for example: "Permission set 'crm_access' is not copied: its `objects` key 'crm_customer' follows the copied object to 'crm2_customer', a key it already holds there, which this copy does not rename. … Remove 'crm_customer' or 'crm2_customer' on the source, then duplicate again." Remove one of the two names on the source item, then duplicate again.
14+
15+
**Unchanged.** An item with no such collision copies as before, its references following the copied objects. An entry the source item itself repeats is copied as stored. The source package is never modified.
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0070 D4 — `duplicatePackage` never merges a renamed reference into a
5+
* name the copied item already holds there.
6+
*
7+
* ## The defect
8+
*
9+
* At an object-reference property, `duplicatePackage` renames a name whole —
10+
* a string, an array element, a record KEY. When the source names the object
11+
* the copy is about to be called (`dupdst_customer`, not yet an object) beside
12+
* the package's own (`dupsrc_customer`), the rename lands on a name the item
13+
* already holds:
14+
*
15+
* - a permission set's `objects` kept whichever entry came last, so one grant
16+
* was dropped with `failed: []`, and the copied object could gain edit and
17+
* delete the source set never granted on its own object;
18+
* - a translation's `objects` dropped one object's labels the same way;
19+
* - a hook's `object` list named the copied object twice, so its handler ran
20+
* twice per event there.
21+
*
22+
* Now the item is refused in `failed[]`, naming the path, both names and the
23+
* source edit that clears it, and nothing of it is written.
24+
*
25+
* ## Why a real kernel
26+
*
27+
* `ObjectQLPlugin` binds the real protocol to the real engine and registry, so
28+
* every pin reads the stored rows out of the driver, never a payload.
29+
*/
30+
31+
import { randomUUID } from 'node:crypto';
32+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
33+
import { ObjectKernel } from '@objectstack/core';
34+
import { ObjectQL } from './engine.js';
35+
import { ObjectQLPlugin } from './plugin.js';
36+
37+
type Stores = Map<string, Map<string, Record<string, unknown>>>;
38+
39+
/** In-memory driver, copied from `protocol-duplicate-package-permission-field-keys.test.ts`; equality-only WHERE. */
40+
function makeMemoryDriver(stores: Stores) {
41+
const storeFor = (obj: string) => {
42+
let s = stores.get(obj);
43+
if (!s) { s = new Map(); stores.set(obj, s); }
44+
return s;
45+
};
46+
// `$and` / `$or` are conjoined WITH their sibling keys, the way a real
47+
// driver ANDs them.
48+
const matchesWhere = (row: Record<string, unknown>, where: any): boolean => {
49+
if (!where || typeof where !== 'object') return true;
50+
for (const [k, v] of Object.entries(where)) {
51+
if (k === '$and' && Array.isArray(v)) {
52+
if (!v.every((w: any) => matchesWhere(row, w))) return false;
53+
continue;
54+
}
55+
if (k === '$or' && Array.isArray(v)) {
56+
if (!v.some((w: any) => matchesWhere(row, w))) return false;
57+
continue;
58+
}
59+
if (k.startsWith('$')) continue;
60+
const rowVal = row[k];
61+
const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
62+
const a = rowVal === undefined ? null : rowVal;
63+
const b = expected === undefined ? null : expected;
64+
if (a !== b) return false;
65+
}
66+
return true;
67+
};
68+
const driver: any = {
69+
name: 'memory', version: '0.0.0', supports: {} as any,
70+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
71+
async execute() { return null; },
72+
async find(object: string, ast: any) {
73+
const rows = Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where));
74+
// The caller's bound, after the filter, by presence (`check:objectql-double-limit`).
75+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
76+
},
77+
async findOne(object: string, ast: any) {
78+
for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r;
79+
return null;
80+
},
81+
async create(object: string, data: Record<string, unknown>) {
82+
const id = (data.id as string) ?? `r_${randomUUID()}`;
83+
const row = { ...data, id };
84+
storeFor(object).set(id, row);
85+
return row;
86+
},
87+
async update(object: string, id: string, data: Record<string, unknown>) {
88+
const s = storeFor(object);
89+
const cur = s.get(id);
90+
if (!cur) throw new Error(`not found: ${object}/${id}`);
91+
const updated = { ...cur, ...data, id };
92+
s.set(id, updated);
93+
return updated;
94+
},
95+
async upsert(object: string, data: Record<string, unknown>) {
96+
const id = data.id as string | undefined;
97+
if (id && storeFor(object).has(id)) return this.update(object, id, data);
98+
return this.create(object, data);
99+
},
100+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
101+
async count(object: string, ast: any) { return (await this.find(object, ast)).length; },
102+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
103+
return Promise.all(rows.map((r) => this.create(object, r)));
104+
},
105+
async bulkUpdate() { return []; }, async bulkDelete() {},
106+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
107+
async commit() {}, async rollback() {},
108+
};
109+
return driver;
110+
}
111+
112+
/** Source package; `installPackage` derives its namespace `dupsrc`. */
113+
const P = 'com.qa.dupsrc';
114+
/** Target package; namespace `dupdst`, derived the same way. */
115+
const Q = 'com.qa.dupdst';
116+
117+
const NARROW = { allowRead: true };
118+
const WIDE = { allowRead: true, allowEdit: true, allowDelete: true };
119+
120+
const customerBody = {
121+
name: 'dupsrc_customer', label: 'Customer', sharingModel: 'private',
122+
fields: { full_name: { name: 'full_name', type: 'text', label: 'Full name' } },
123+
};
124+
125+
describe('ADR-0070 D4 — duplicatePackage refuses a renamed reference that meets a name the item holds', () => {
126+
let kernel: ObjectKernel;
127+
let stores: Stores;
128+
let engine: ObjectQL;
129+
let protocol: any;
130+
131+
beforeEach(async () => {
132+
// `logger.level` is the real config key, and gracefulShutdown:false keeps
133+
// process signal handlers out of vitest's fork workers (#4250).
134+
kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
135+
stores = new Map();
136+
engine = new ObjectQL();
137+
engine.registerDriver(makeMemoryDriver(stores), true);
138+
await kernel.use(new ObjectQLPlugin({ ql: engine }));
139+
await kernel.bootstrap();
140+
protocol = kernel.getService('protocol');
141+
});
142+
afterEach(async () => {
143+
if (kernel.getState() === 'running') await kernel.shutdown();
144+
});
145+
146+
const storedBody = (type: string, name: string, packageId: string): any => {
147+
const row = Array.from(stores.get('sys_metadata')?.values() ?? [])
148+
.find((r) => r.type === type && r.name === name && r.package_id === packageId);
149+
if (!row) return undefined;
150+
return typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata;
151+
};
152+
/** Every table the copy writes (the item, its history, commit and audit rows) that mentions `name`. */
153+
const tablesMentioning = (name: string): string[] =>
154+
Array.from(stores.entries())
155+
.filter(([, rows]) => Array.from(rows.values()).some((row) => JSON.stringify(row).includes(`"${name}"`)))
156+
.map(([table]) => table)
157+
.sort();
158+
159+
async function save(type: string, item: { name: string }): Promise<void> {
160+
const res = await protocol.saveMetaItem({ type, name: item.name, packageId: P, item });
161+
expect(res.success).toBe(true);
162+
}
163+
async function seed(type: string, item: { name: string }): Promise<void> {
164+
await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'application' } });
165+
expect(engine.registry.getPackage(P)?.manifest?.namespace).toBe('dupsrc');
166+
await save('object', customerBody);
167+
await save(type, item);
168+
}
169+
170+
for (const [order, objects] of [
171+
['the renamed key first', { dupsrc_customer: NARROW, dupdst_customer: WIDE }],
172+
['the key it meets first', { dupdst_customer: WIDE, dupsrc_customer: NARROW }],
173+
] as const) {
174+
it(`a permission set whose renamed \`objects\` key meets one it holds is refused, never merged (${order})`, async () => {
175+
await seed('permission', { name: 'dupsrc_objs', label: 'Objects', objects } as { name: string });
176+
177+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
178+
179+
expect(res.failed).toHaveLength(1);
180+
const [refusal] = res.failed;
181+
expect(refusal).toMatchObject({ type: 'permission', name: 'dupsrc_objs' });
182+
// Located: the set, the property, the key that moved, the key it met, and the remedy.
183+
expect(refusal.error).toContain("Permission set 'dupsrc_objs' is not copied");
184+
expect(refusal.error).toContain("`objects` key 'dupsrc_customer' follows the copied object to 'dupdst_customer'");
185+
expect(refusal.error).toContain("Remove 'dupsrc_customer' or 'dupdst_customer' on the source, then duplicate again.");
186+
// Nothing of the set was written: no item row, no history, commit or audit row.
187+
expect(storedBody('permission', 'dupdst_objs', Q)).toBeUndefined();
188+
expect(tablesMentioning('dupdst_objs')).toEqual([]);
189+
// NEGATIVE CONTROL: the object still copied, and the scan finds what a copy writes.
190+
expect(res.copied).toEqual([{ type: 'object', name: 'dupdst_customer' }]);
191+
expect(tablesMentioning('dupdst_customer')).toEqual(expect.arrayContaining(['sys_metadata', 'sys_metadata_history']));
192+
// The source set is untouched.
193+
expect(storedBody('permission', 'dupsrc_objs', P).objects).toEqual(objects);
194+
});
195+
}
196+
197+
it('NEGATIVE CONTROL: the same set without the second key copies, and its one grant follows the renamed object', async () => {
198+
await seed('permission', { name: 'dupsrc_objs', label: 'Objects', objects: { dupsrc_customer: NARROW } } as { name: string });
199+
200+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
201+
202+
expect(res.failed).toEqual([]);
203+
expect(res.copied).toEqual(expect.arrayContaining([
204+
{ type: 'object', name: 'dupdst_customer' },
205+
{ type: 'permission', name: 'dupdst_objs' },
206+
]));
207+
expect(storedBody('permission', 'dupdst_objs', Q).objects).toEqual({ dupdst_customer: NARROW });
208+
expect(storedBody('permission', 'dupsrc_objs', P).objects).toEqual({ dupsrc_customer: NARROW });
209+
});
210+
211+
it('a translation whose renamed `objects` key meets one it holds is refused, never merged', async () => {
212+
const objects = { dupsrc_customer: { label: 'Customer A' }, dupdst_customer: { label: 'Customer B' } };
213+
await seed('translation', { name: 'dupsrc_zh', locale: 'zh-CN', objects } as { name: string });
214+
215+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
216+
217+
expect(res.failed).toHaveLength(1);
218+
expect(res.failed[0]).toMatchObject({ type: 'translation', name: 'dupsrc_zh' });
219+
expect(res.failed[0].error).toContain("Translation 'dupsrc_zh' is not copied");
220+
expect(res.failed[0].error).toContain("`objects` key 'dupsrc_customer' follows the copied object to 'dupdst_customer'");
221+
expect(tablesMentioning('dupdst_zh')).toEqual([]);
222+
// NEGATIVE CONTROL: the object still copied; the source is untouched.
223+
expect(res.copied).toEqual([{ type: 'object', name: 'dupdst_customer' }]);
224+
expect(storedBody('translation', 'dupsrc_zh', P).objects).toEqual(objects);
225+
});
226+
227+
it('a hook whose renamed `object` entry meets one it holds is refused; without the second entry it copies', async () => {
228+
const hook = (object: string[]) => ({
229+
name: 'dupsrc_stamp', object, events: ['beforeInsert'],
230+
body: { language: 'js', source: "ctx.input.full_name = 'stamped';" },
231+
});
232+
await seed('hook', hook(['dupsrc_customer', 'dupdst_customer']));
233+
234+
const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q });
235+
236+
expect(res.failed).toHaveLength(1);
237+
expect(res.failed[0]).toMatchObject({ type: 'hook', name: 'dupsrc_stamp' });
238+
expect(res.failed[0].error).toContain("Hook 'dupsrc_stamp' is not copied");
239+
expect(res.failed[0].error).toContain("`object` entry 'dupsrc_customer' follows the copied object to 'dupdst_customer'");
240+
expect(tablesMentioning('dupdst_stamp')).toEqual([]);
241+
242+
// NEGATIVE CONTROL: the same hook without the second entry copies, and its entry follows the renamed object.
243+
await save('hook', hook(['dupsrc_customer']));
244+
const again = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: 'com.qa.dupdst2' });
245+
expect(again.failed).toEqual([]);
246+
expect(storedBody('hook', 'dupdst2_stamp', 'com.qa.dupdst2').object).toEqual(['dupdst2_customer']);
247+
});
248+
});

0 commit comments

Comments
 (0)