Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ticsService contract Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是否完成了任务中要求的所有开发并且不再有类似的问题 |
…ls to match contract Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
经过全面审查，发现
已在 a42a8d5 中移除这两处多余的 |
|
@copilot 14:34:35.528 @objectstack/spec:build: ✓ data/TenantDatabaseLifecycle.json |
…error Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in 3d0775e — after removing the |
There was a problem hiding this comment.
Pull request overview
This PR fixes a method-naming misalignment between HttpDispatcher's analytics handling and the IAnalyticsService contract defined in packages/spec. The dispatcher was calling getMetadata() (non-existent) and passing extra { request: context.request } arguments to query() and generateSql(), neither of which matches the contract interface. The PR renames the call to getMeta(), removes the extra arguments, and updates tests and the ROADMAP accordingly.
Changes:
handleAnalyticsinhttp-dispatcher.ts: renamegetMetadata→getMeta, remove non-contract{ request: context.request }arguments from all three analytics service calls, and prefix unusedcontextparam as_context.http-dispatcher.test.ts: rename two mock method definitions fromgetMetadatatogetMeta.ROADMAP.md: add a fix record entry documenting the naming correction.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/runtime/src/http-dispatcher.ts |
Fixes getMetadata → getMeta, removes extra context args, renames unused param to _context |
packages/runtime/src/http-dispatcher.test.ts |
Updates two mock definitions from getMetadata to getMeta to match corrected dispatcher |
ROADMAP.md |
Documents the analytics naming fix in the project roadmap |
| const mockAnalytics = { | ||
| query: vi.fn().mockResolvedValue({ rows: [{ id: 1 }], total: 1 }), | ||
| getMetadata: vi.fn().mockResolvedValue({ tables: ['t1'] }), | ||
| getMeta: vi.fn().mockResolvedValue({ tables: ['t1'] }), |
There was a problem hiding this comment.
Same issue as line 248: the mock at line 219 returns { tables: ['t1'] } (an object), while the IAnalyticsService.getMeta() contract specifies Promise<CubeMeta[]> (an array). The mock should use the correct contract-conformant return type: an array of CubeMeta objects.
| if (subPath === 'sql' && m === 'POST') { | ||
| // Assuming service has generateSql method | ||
| const result = await analyticsService.generateSql(body, { request: context.request }); | ||
| const result = await analyticsService.generateSql(body); |
There was a problem hiding this comment.
The generateSql method is declared optional in IAnalyticsService (generateSql?(query: AnalyticsQuery): ...), but it is called unconditionally here without a null/undefined guard. If an analytics service only implements the required query and getMeta methods (which is a valid implementation per the contract), a POST /analytics/sql request will throw TypeError: analyticsService.generateSql is not a function at runtime.
The fix should check for the method's existence before calling it, for example: if the method doesn't exist, return a 501 Not Implemented response or { handled: false }.
| it('should handle GET /analytics/meta with async service', async () => { | ||
| const mockAnalytics = { | ||
| getMetadata: vi.fn().mockResolvedValue({ tables: ['users', 'orders'] }), | ||
| getMeta: vi.fn().mockResolvedValue({ tables: ['users', 'orders'] }), |
There was a problem hiding this comment.
The mock at line 248 returns { tables: ['users', 'orders'] } (a plain object with a tables key), but the IAnalyticsService.getMeta() contract specifies the return type as Promise<CubeMeta[]> — an array of CubeMeta objects. The same non-conformant mock shape also appears at line 219.
This means the test assertion at line 255 (result.response?.body?.data?.tables) is validating an artificial response shape rather than the actual contract. The mock should return a proper CubeMeta[] array (e.g., [{ name: 'users', measures: [], dimensions: [] }, { name: 'orders', measures: [], dimensions: [] }]) and the assertion should validate the returned array directly.
http-dispatcher.ts: renamegetMetadata→getMetato matchIAnalyticsServicecontracthttp-dispatcher.ts: remove extra{ request: context.request }fromquery()andgenerateSql()callshttp-dispatcher.ts: prefix unusedcontextparam with_to fix TS6133 build error (follows existing_contextconvention inhandleI18n/handleUi)http-dispatcher.test.ts: rename mock methodgetMetadata→getMetaROADMAP.mdwith fix recordOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.