-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: replace custom MSW handlers with MSWPlugin + protocol broker shim #860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Simple MSW Integration Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * MSW Integration Tests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Minimal test to verify MSW server is working | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Verifies the MSW server (powered by MSWPlugin) correctly handles: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Basic CRUD operations via the ObjectStack protocol | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Query parameters: filter, sort, top, skip | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * MSWPlugin routes all requests through the ObjectStack protocol stack, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ensuring behaviour is identical to server mode (HonoServerPlugin). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { describe, it, expect, beforeAll, afterAll } from 'vitest'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -40,4 +45,54 @@ describe('MSW Server Integration', () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(newContact.name).toBe('Test User'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(newContact.email).toBe('test@example.com'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ── Protocol-level query tests (filter / sort / top) ─────────────────── | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // These tests hit the MSW HTTP layer via fetch, which MSWPlugin routes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // through HttpDispatcher → ObjectStack protocol. If the protocol handles | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // filter/sort/top correctly, these will pass. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('should support top (limit) via HTTP', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch('http://localhost/api/v1/data/contact?top=3'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(res.ok).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // HttpDispatcher wraps in { success, data: { value: [...] } } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = body.data ?? body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const records = data.value ?? data.records ?? data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(Array.isArray(records)).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(records.length).toBeLessThanOrEqual(3); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('should support sort via HTTP', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch('http://localhost/api/v1/data/contact?sort=name'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(res.ok).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = body.data ?? body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const records = data.value ?? data.records ?? data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(Array.isArray(records)).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(records.length).toBeGreaterThan(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify records are sorted by name ascending | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const names = records.map((r: any) => r.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sorted = [...names].sort((a: string, b: string) => a.localeCompare(b)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(names).toEqual(sorted); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('should support filter via HTTP', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Filter contacts where priority = "high" using tuple format | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filter = JSON.stringify(['priority', '=', 'high']); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch(`http://localhost/api/v1/data/contact?filter=${encodeURIComponent(filter)}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(res.ok).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = await res.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = body.data ?? body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const records = data.value ?? data.records ?? data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(Array.isArray(records)).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(records.length).toBeGreaterThan(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // All returned records should have priority 'high' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const record of records) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(record.priority).toBe('high'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | |
| }); | |
| it('should support skip (offset) via HTTP', async () => { | |
| // Use sort=name to ensure a deterministic ordering, then apply skip=2 | |
| const baseRes = await fetch('http://localhost/api/v1/data/contact?sort=name'); | |
| expect(baseRes.ok).toBe(true); | |
| const baseBody = await baseRes.json(); | |
| const baseData = baseBody.data ?? baseBody; | |
| const baseRecords = baseData.value ?? baseData.records ?? baseData; | |
| expect(Array.isArray(baseRecords)).toBe(true); | |
| expect(baseRecords.length).toBeGreaterThan(2); | |
| const skipRes = await fetch('http://localhost/api/v1/data/contact?sort=name&skip=2'); | |
| expect(skipRes.ok).toBe(true); | |
| const skipBody = await skipRes.json(); | |
| const skipData = skipBody.data ?? skipBody; | |
| const skipRecords = skipData.value ?? skipData.records ?? skipData; | |
| expect(Array.isArray(skipRecords)).toBe(true); | |
| expect(skipRecords.length).toBeGreaterThan(0); | |
| // After skipping 2 records, the first record in the skip result | |
| // should match the third record from the base (index 2) | |
| expect(skipRecords[0].id).toBe(baseRecords[2].id); | |
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,24 +4,94 @@ | |||||||||||||||||||||||
| * Creates a fully bootstrapped ObjectStack kernel for use in both | ||||||||||||||||||||||||
| * browser (MSW setupWorker) and test (MSW setupServer) environments. | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * Uses MSWPlugin from @objectstack/plugin-msw to expose the full | ||||||||||||||||||||||||
| * ObjectStack protocol via MSW. This ensures filter/sort/top/pagination | ||||||||||||||||||||||||
| * work identically to server mode. | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * Follows the same pattern as @objectstack/studio's createKernel — | ||||||||||||||||||||||||
| * see https://github.com/objectstack-ai/spec | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { ObjectKernel, DriverPlugin, AppPlugin } from '@objectstack/runtime'; | ||||||||||||||||||||||||
| import { ObjectQLPlugin } from '@objectstack/objectql'; | ||||||||||||||||||||||||
| import { InMemoryDriver } from '@objectstack/driver-memory'; | ||||||||||||||||||||||||
| import { MSWPlugin } from '@objectstack/plugin-msw'; | ||||||||||||||||||||||||
| import type { MSWPluginOptions } from '@objectstack/plugin-msw'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export interface KernelOptions { | ||||||||||||||||||||||||
| /** Application configuration (defineStack output) */ | ||||||||||||||||||||||||
| appConfig: any; | ||||||||||||||||||||||||
| /** Whether to skip system validation (useful in browser) */ | ||||||||||||||||||||||||
| skipSystemValidation?: boolean; | ||||||||||||||||||||||||
| /** MSWPlugin options; when provided, MSWPlugin is added to the kernel. */ | ||||||||||||||||||||||||
| mswOptions?: MSWPluginOptions; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export interface KernelResult { | ||||||||||||||||||||||||
| kernel: ObjectKernel; | ||||||||||||||||||||||||
| driver: InMemoryDriver; | ||||||||||||||||||||||||
| /** The MSWPlugin instance (if mswOptions was provided). */ | ||||||||||||||||||||||||
| mswPlugin?: MSWPlugin; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Install a lightweight broker shim on the kernel so that | ||||||||||||||||||||||||
| * HttpDispatcher (used by MSWPlugin) can route data/metadata | ||||||||||||||||||||||||
| * calls through the ObjectStack protocol service. | ||||||||||||||||||||||||
| * | ||||||||||||||||||||||||
| * A full Moleculer-based broker is only available in server mode | ||||||||||||||||||||||||
| * (HonoServerPlugin). In MSW/browser mode we bridge the gap with | ||||||||||||||||||||||||
| * this thin adapter that delegates to the protocol service. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| async function installBrokerShim(kernel: ObjectKernel): Promise<void> { | ||||||||||||||||||||||||
| let protocol: any; | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| protocol = await kernel.getService('protocol'); | ||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if (!protocol) return; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| (kernel as any).broker = { | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| async call(action: string, params: any = {}) { | ||||||||||||||||||||||||
| const [service, method] = action.split('.'); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (service === 'data') { | ||||||||||||||||||||||||
| switch (method) { | ||||||||||||||||||||||||
| case 'query': | ||||||||||||||||||||||||
| return protocol.findData({ object: params.object, query: params.query ?? params }); | ||||||||||||||||||||||||
| case 'get': | ||||||||||||||||||||||||
| return protocol.getData({ object: params.object, id: params.id }); | ||||||||||||||||||||||||
| case 'create': | ||||||||||||||||||||||||
| return protocol.createData({ object: params.object, data: params.data }); | ||||||||||||||||||||||||
| case 'update': | ||||||||||||||||||||||||
| return protocol.updateData({ object: params.object, id: params.id, data: params.data }); | ||||||||||||||||||||||||
| case 'delete': | ||||||||||||||||||||||||
| return protocol.deleteData({ object: params.object, id: params.id }); | ||||||||||||||||||||||||
| case 'batch': | ||||||||||||||||||||||||
| return protocol.batchData?.({ object: params.object, ...params }) ?? { results: [] }; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| return protocol.batchData?.({ object: params.object, ...params }) ?? { results: [] }; | |
| return protocol.batchData?.({ object: params.object, ...params }) ?? { results: [] }; | |
| default: | |
| throw new Error(`[BrokerShim] Unhandled data method: ${method}`); |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug in metadata routing: The fallback at line 89 assumes any unhandled method in the 'metadata' service should be treated as a list-style call (metadata.objects, metadata.apps, etc.). However, this could incorrectly handle typos or invalid method names, returning misleading results instead of throwing an error.
Consider adding explicit handling for known list methods (objects, apps, dashboards, pages, reports) and throwing an error for unrecognized methods, similar to the throw statement at line 92 for the data service.
| return protocol.getMetaItems({ type: method, packageId: params.packageId }); | |
| switch (method) { | |
| case 'objects': | |
| case 'apps': | |
| case 'dashboards': | |
| case 'pages': | |
| case 'reports': | |
| return protocol.getMetaItems({ type: method, packageId: params.packageId }); | |
| default: | |
| throw new Error(`[BrokerShim] Unhandled metadata method: ${method} in action: ${action}`); | |
| } |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling: If protocol methods throw errors (e.g., findData, getData, createData), they will propagate uncaught. While this might be intentional to let MSWPlugin handle errors, the broker shim should at least ensure errors are properly wrapped or logged for debugging. Consider adding try-catch blocks around protocol method calls and wrapping errors with additional context (e.g., which action and params caused the error).
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The broker shim is installed twice: before MSWPlugin is added (line 121) and after bootstrap (lines 131-132). While the comment explains this is to ensure the protocol service is fully initialized, this pattern is fragile and could mask initialization order issues. Consider one of these alternatives:
- Only install the shim after bootstrap (remove line 121), which should be sufficient since MSWPlugin's handlers are only registered during bootstrap.
- Add a check in installBrokerShim to avoid reinstalling if the broker already exists and is functional.
- Document why both installations are necessary with a more detailed explanation of what changes between the two calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test validates that records have priority='high' but doesn't verify that records with other priorities (e.g., 'medium', 'low') are excluded. Consider adding an assertion that verifies the total count matches expected filtered results, or seed known data with multiple priorities and verify only 'high' priority records are returned.
For example, you could check that the returned count is less than the total count of contacts, or verify specific known 'high' priority contacts are present while known 'medium' ones are absent.