Skip to content

Commit fffa428

Browse files
hotlonghotlong
authored
Merge pull request #853 from objectstack-ai/copilot/fix-populate-expand-lookup-issues
2 parents 87669b4 + 9fbf5bf commit fffa428

8 files changed

Lines changed: 517 additions & 4 deletions

File tree

‎apps/studio/src/mocks/createKernel.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export async function createKernel(options: KernelOptions) {
5454
const ql = (kernel as any).context?.getService('objectql');
5555

5656
if (service === 'data') {
57+
// Delegate to protocol service when available for proper expand/populate support
58+
const protocol = (kernel as any).context?.getService('protocol');
5759
// All data responses conform to protocol.zod.ts schemas:
5860
// CreateDataResponse = { object, id, record }
5961
// GetDataResponse = { object, id, record }
@@ -66,6 +68,10 @@ export async function createKernel(options: KernelOptions) {
6668
return { object: params.object, id: record.id || record._id, record };
6769
}
6870
if (method === 'get') {
71+
// Delegate to protocol for proper expand/select support
72+
if (protocol) {
73+
return await protocol.getData({ object: params.object, id: params.id, expand: params.expand, select: params.select });
74+
}
6975
let all = await ql.find(params.object);
7076
if (!all) all = [];
7177
const match = all.find((i: any) => i.id === params.id || i._id === params.id);
@@ -110,6 +116,10 @@ export async function createKernel(options: KernelOptions) {
110116
}
111117
}
112118
if (method === 'find' || method === 'query') {
119+
// Delegate to protocol for proper expand/populate support
120+
if (protocol) {
121+
return await protocol.findData({ object: params.object, query: params.query || params.filters });
122+
}
113123
let all = await ql.find(params.object);
114124

115125
// DEBUG SHIM

‎packages/client/src/client.hono.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,34 @@ describe('ObjectStackClient (with Hono Server)', () => {
3131

3232
if (service === 'data') {
3333
const ql = kernel.getService<any>('objectql'); // Use 'objectql' service name for clarity
34+
// Delegate to protocol service when available for proper expand/populate support
35+
let protocol: any;
36+
try { protocol = kernel.getService<any>('protocol'); } catch { /* not registered */ }
3437
if (method === 'create') {
3538
const res = await ql.insert(params.object, params.data);
3639
const record = { ...params.data, ...res };
3740
return { object: params.object, id: record.id || record._id, record };
3841
}
3942
// Params from HttpDispatcher: { object, id, ...query }
4043
if (method === 'get') {
44+
if (protocol) {
45+
return await protocol.getData({ object: params.object, id: params.id, expand: params.expand, select: params.select });
46+
}
4147
const record = await ql.findOne(params.object, { where: { id: params.id } });
4248
return record ? { object: params.object, id: params.id, record } : null;
4349
}
4450
// Params from HttpDispatcher: { object, filters }
4551
if (method === 'query') {
52+
if (protocol) {
53+
return await protocol.findData({ object: params.object, query: params.query || params.filters });
54+
}
4655
const records = await ql.find(params.object, { filter: params.filters });
4756
return { object: params.object, records, total: records.length };
4857
}
4958
if (method === 'find') {
59+
if (protocol) {
60+
return await protocol.findData({ object: params.object, query: params.query || params.filters });
61+
}
5062
const records = await ql.find(params.object, { filter: params.filters });
5163
return { object: params.object, records, total: records.length };
5264
}

‎packages/client/src/client.msw.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,33 @@ describe('ObjectStackClient (with MSW Plugin)', () => {
3737

3838
if (service === 'data') {
3939
const ql = kernel.getService<any>('objectql');
40+
// Delegate to protocol service when available for proper expand/populate support
41+
let protocol: any;
42+
try { protocol = kernel.getService<any>('protocol'); } catch { /* not registered */ }
4043
if (method === 'create') {
4144
const res = await ql.insert(params.object, params.data);
4245
const record = { ...params.data, ...res };
4346
return { object: params.object, id: record.id || record._id, record };
4447
}
4548
if (method === 'get') {
46-
// Ensure we search by 'id' explicitly for InMemoryDriver
49+
if (protocol) {
50+
return await protocol.getData({ object: params.object, id: params.id, expand: params.expand, select: params.select });
51+
}
4752
const record = await ql.findOne(params.object, { where: { id: params.id } });
4853
return record ? { object: params.object, id: params.id, record } : null;
4954
}
5055
if (method === 'query') {
56+
if (protocol) {
57+
return await protocol.findData({ object: params.object, query: params.query });
58+
}
5159
const queryOpts = params.query || {};
5260
const records = await ql.find(params.object, { filter: queryOpts.filters || queryOpts.filter });
5361
return { object: params.object, records, total: records.length };
5462
}
5563
if (method === 'find') {
64+
if (protocol) {
65+
return await protocol.findData({ object: params.object, query: params.query });
66+
}
5667
const queryOpts = params.query || {};
5768
const records = await ql.find(params.object, { filter: queryOpts.filters || queryOpts.filter });
5869
return { object: params.object, records, total: records.length };

‎packages/objectql/src/protocol-data.test.ts‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi, beforeEach } from 'vitest';
4+
import { ObjectStackProtocolImplementation } from './protocol.js';
5+
6+
/**
7+
* Tests for the Protocol Implementation's data methods (findData, getData).
8+
* Validates that expand/populate/select parameters are correctly normalized
9+
* and forwarded to the underlying engine.
10+
*/
11+
describe('ObjectStackProtocolImplementation - Data Operations', () => {
12+
let protocol: ObjectStackProtocolImplementation;
13+
let mockEngine: any;
14+
15+
beforeEach(() => {
16+
mockEngine = {
17+
find: vi.fn().mockResolvedValue([]),
18+
findOne: vi.fn().mockResolvedValue(null),
19+
};
20+
protocol = new ObjectStackProtocolImplementation(mockEngine);
21+
});
22+
23+
// ═══════════════════════════════════════════════════════════════
24+
// findData — expand/populate normalization
25+
// ═══════════════════════════════════════════════════════════════
26+
27+
describe('findData', () => {
28+
it('should normalize expand string to populate array', async () => {
29+
await protocol.findData({ object: 'order_item', query: { expand: 'order,product' } });
30+
31+
expect(mockEngine.find).toHaveBeenCalledWith(
32+
'order_item',
33+
expect.objectContaining({
34+
populate: ['order', 'product'],
35+
}),
36+
);
37+
// expand should be deleted from options
38+
const callArgs = mockEngine.find.mock.calls[0][1];
39+
expect(callArgs.expand).toBeUndefined();
40+
expect(callArgs.$expand).toBeUndefined();
41+
});
42+
43+
it('should normalize $expand (OData) to populate array', async () => {
44+
await protocol.findData({ object: 'task', query: { $expand: 'assignee,project' } });
45+
46+
expect(mockEngine.find).toHaveBeenCalledWith(
47+
'task',
48+
expect.objectContaining({
49+
populate: ['assignee', 'project'],
50+
}),
51+
);
52+
});
53+
54+
it('should pass populate array as-is if already an array', async () => {
55+
await protocol.findData({ object: 'task', query: { populate: ['assignee'] } });
56+
57+
expect(mockEngine.find).toHaveBeenCalledWith(
58+
'task',
59+
expect.objectContaining({
60+
populate: ['assignee'],
61+
}),
62+
);
63+
});
64+
65+
it('should normalize populate string to array', async () => {
66+
await protocol.findData({ object: 'task', query: { populate: 'assignee,project' } });
67+
68+
expect(mockEngine.find).toHaveBeenCalledWith(
69+
'task',
70+
expect.objectContaining({
71+
populate: ['assignee', 'project'],
72+
}),
73+
);
74+
});
75+
76+
it('should prefer explicit populate over expand', async () => {
77+
await protocol.findData({
78+
object: 'task',
79+
query: { populate: ['assignee'], expand: 'project' },
80+
});
81+
82+
// populate takes precedence; expand is not converted
83+
expect(mockEngine.find).toHaveBeenCalledWith(
84+
'task',
85+
expect.objectContaining({
86+
populate: ['assignee'],
87+
}),
88+
);
89+
});
90+
91+
it('should normalize expand array to populate array', async () => {
92+
await protocol.findData({ object: 'task', query: { expand: ['owner', 'team'] } });
93+
94+
expect(mockEngine.find).toHaveBeenCalledWith(
95+
'task',
96+
expect.objectContaining({
97+
populate: ['owner', 'team'],
98+
}),
99+
);
100+
});
101+
102+
it('should normalize select string to array', async () => {
103+
await protocol.findData({ object: 'task', query: { select: 'name,status,assignee' } });
104+
105+
expect(mockEngine.find).toHaveBeenCalledWith(
106+
'task',
107+
expect.objectContaining({
108+
select: ['name', 'status', 'assignee'],
109+
}),
110+
);
111+
});
112+
113+
it('should pass numeric pagination params correctly', async () => {
114+
await protocol.findData({ object: 'task', query: { top: '10', skip: '20' } });
115+
116+
expect(mockEngine.find).toHaveBeenCalledWith(
117+
'task',
118+
expect.objectContaining({
119+
top: 10,
120+
skip: 20,
121+
}),
122+
);
123+
});
124+
125+
it('should work with no query options', async () => {
126+
await protocol.findData({ object: 'task' });
127+
128+
expect(mockEngine.find).toHaveBeenCalledWith('task', {});
129+
});
130+
131+
it('should return records and standard response shape', async () => {
132+
mockEngine.find.mockResolvedValue([{ _id: 't1', name: 'Task 1' }]);
133+
134+
const result = await protocol.findData({ object: 'task', query: {} });
135+
136+
expect(result).toEqual(
137+
expect.objectContaining({
138+
object: 'task',
139+
records: [{ _id: 't1', name: 'Task 1' }],
140+
total: 1,
141+
}),
142+
);
143+
});
144+
});
145+
146+
// ═══════════════════════════════════════════════════════════════
147+
// getData — expand/select normalization
148+
// ═══════════════════════════════════════════════════════════════
149+
150+
describe('getData', () => {
151+
it('should convert expand string to populate array', async () => {
152+
mockEngine.findOne.mockResolvedValue({ _id: 'oi_1', name: 'Item 1' });
153+
154+
await protocol.getData({ object: 'order_item', id: 'oi_1', expand: 'order,product' });
155+
156+
expect(mockEngine.findOne).toHaveBeenCalledWith(
157+
'order_item',
158+
expect.objectContaining({
159+
filter: { _id: 'oi_1' },
160+
populate: ['order', 'product'],
161+
}),
162+
);
163+
});
164+
165+
it('should convert expand array to populate array', async () => {
166+
mockEngine.findOne.mockResolvedValue({ _id: 't1' });
167+
168+
await protocol.getData({ object: 'task', id: 't1', expand: ['assignee', 'project'] });
169+
170+
expect(mockEngine.findOne).toHaveBeenCalledWith(
171+
'task',
172+
expect.objectContaining({
173+
populate: ['assignee', 'project'],
174+
}),
175+
);
176+
});
177+
178+
it('should convert select string to array', async () => {
179+
mockEngine.findOne.mockResolvedValue({ _id: 't1', name: 'Test' });
180+
181+
await protocol.getData({ object: 'task', id: 't1', select: 'name,status' });
182+
183+
expect(mockEngine.findOne).toHaveBeenCalledWith(
184+
'task',
185+
expect.objectContaining({
186+
select: ['name', 'status'],
187+
}),
188+
);
189+
});
190+
191+
it('should pass both expand and select together', async () => {
192+
mockEngine.findOne.mockResolvedValue({ _id: 'oi_1' });
193+
194+
await protocol.getData({
195+
object: 'order_item',
196+
id: 'oi_1',
197+
expand: 'order',
198+
select: ['name', 'total'],
199+
});
200+
201+
expect(mockEngine.findOne).toHaveBeenCalledWith(
202+
'order_item',
203+
expect.objectContaining({
204+
filter: { _id: 'oi_1' },
205+
populate: ['order'],
206+
select: ['name', 'total'],
207+
}),
208+
);
209+
});
210+
211+
it('should work without expand or select', async () => {
212+
mockEngine.findOne.mockResolvedValue({ _id: 't1' });
213+
214+
await protocol.getData({ object: 'task', id: 't1' });
215+
216+
expect(mockEngine.findOne).toHaveBeenCalledWith(
217+
'task',
218+
{ filter: { _id: 't1' } },
219+
);
220+
});
221+
222+
it('should return standard GetDataResponse shape', async () => {
223+
mockEngine.findOne.mockResolvedValue({ _id: 'oi_1', name: 'Item 1' });
224+
225+
const result = await protocol.getData({ object: 'order_item', id: 'oi_1' });
226+
227+
expect(result).toEqual({
228+
object: 'order_item',
229+
id: 'oi_1',
230+
record: { _id: 'oi_1', name: 'Item 1' },
231+
});
232+
});
233+
234+
it('should throw when record not found', async () => {
235+
mockEngine.findOne.mockResolvedValue(null);
236+
237+
await expect(
238+
protocol.getData({ object: 'task', id: 'missing_id' })
239+
).rejects.toThrow('not found');
240+
});
241+
});
242+
});

‎packages/plugins/plugin-msw/src/msw-plugin.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,8 +371,8 @@ export class ObjectStackServer {
371371
return { data: body, status: 200 };
372372
}
373373

374-
static async getData(objectName: string, id: string) {
375-
const body = await this.getProtocol().getData({ object: objectName, id });
374+
static async getData(objectName: string, id: string, options?: { expand?: string | string[]; select?: string | string[] }) {
375+
const body = await this.getProtocol().getData({ object: objectName, id, ...options });
376376
return { data: body, status: 200 };
377377
}
378378

‎packages/rest/src/rest-server.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,9 +480,12 @@ export class RestServer {
480480
path: `${dataPath}/:object/:id`,
481481
handler: async (req: any, res: any) => {
482482
try {
483+
const { select, expand } = req.query || {};
483484
const result = await this.protocol.getData({
484485
object: req.params.object,
485-
id: req.params.id
486+
id: req.params.id,
487+
...(select != null ? { select } : {}),
488+
...(expand != null ? { expand } : {}),
486489
});
487490
res.json(result);
488491
} catch (error: any) {

0 commit comments

Comments
 (0)