diff --git a/.changeset/i18n-success-envelope-conformance.md b/.changeset/i18n-success-envelope-conformance.md new file mode 100644 index 0000000000..93faf7aaeb --- /dev/null +++ b/.changeset/i18n-success-envelope-conformance.md @@ -0,0 +1,62 @@ +--- +"@objectstack/spec": minor +"@objectstack/runtime": patch +"@objectstack/service-i18n": patch +--- + +fix(runtime,i18n)!: `/i18n/locales` answers in one shape — plus the +success-envelope conformance gate that found it + +Follow-up to #3676 / #3833 / #3847. Those three were each a body that did not +match the schema declaring it, and each survived a green suite because **every +test asserted the emitted body against a hand-written literal**. Comparing +output to a literal proves the code does what the test author believed; it +cannot prove the code does what the contract declares. Nothing had ever put the +emitted value and the declared schema in the same assertion. + +This adds that assertion as a suite — `i18n-success-envelope.conformance.test.ts` +in `runtime`, the missing success-path twin of service-i18n's +`error-envelope.conformance.test.ts` and the same pairing storage got in #3689. +Every `/i18n` success body is parsed against `BaseResponseSchema` and against +the schema `plugin-rest-api` names for that route (`responseSchema: +'GetLocalesResponseSchema'`, …), imported rather than restated. + +**It found a fourth gap on its first run.** `GET /i18n/locales` passed +`getLocales()`'s raw `string[]` straight through the dispatcher, while +`GetLocalesResponseSchema` declares `{ code, label, isDefault }[]` — and +service-i18n, the *other* provider of this identical route, already emitted +descriptors. One endpoint, two shapes, decided by which plugin mounted it, with +the dispatcher's form contradicting the SDK's own `GetLocalesResponse` type. + +That is the same split #3833 found in the field-labels derivation, one route +over, and it happened for the same reason: two surfaces, one mapping, kept +twice. So the mapping is now shared as `toLocaleDescriptors` in +`packages/spec/src/system/i18n-resolver.ts`, next to `resolveObjectFieldLabels`, +and both surfaces call it. `label` is the locale code — no display-name source +exists in the tree and the schema requires the field; inventing an ICU +display-name table here would be a product decision, not an implementation +detail. + +The gate was verified the same way #3833's was: the fix was reverted and the +suite confirmed to fail on it — + +``` +locales body does not match its declared schema: + [{"expected":"object","code":"invalid_type","path":["locales",0], + "message":"Invalid input: expected object, received string"}, …] +``` + +— rather than merely passing once written. Five existing tests pinned the bare +`string[]`; they now assert on `.map(l => l.code)`, so the codes stay pinned +while the shape is owned by the schema. + +BREAKING: `GET /i18n/locales` served by the dispatcher now returns +`[{ code, label, isDefault }]` instead of `['en', …]`. Callers on the +service-i18n mount already received this shape, and the SDK's published +`GetLocalesResponse` type has always described it, so this ends a divergence +rather than starting one. + +Worth generalizing beyond `/i18n`: `plugin-rest-api.zod.ts` already carries a +`responseSchema` name on essentially every route (29 declarations across 28 +handlers), so the route → declaring-schema mapping needed to run this check +repo-wide exists today and is unused. diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 6c6b01bb73..b676ec3b94 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -126,7 +126,7 @@ describe('HttpDispatcher domain registry (D11 step ③)', () => { const i18n = { getLocales: vi.fn().mockReturnValue(['en', 'zh-CN']), getTranslations: vi.fn().mockReturnValue({}) }; const result = await makeDispatcher({ i18n }).dispatch('GET', '/i18n/locales', undefined, {}, {} as any); expect(result.response?.status).toBe(200); - expect(result.response?.body?.data?.locales).toEqual(['en', 'zh-CN']); + expect(result.response?.body?.data?.locales.map((l: any) => l.code)).toEqual(['en', 'zh-CN']); }); it('/analytics bridges to the analytics service exactly as the legacy branch did', async () => { diff --git a/packages/runtime/src/domains/i18n.ts b/packages/runtime/src/domains/i18n.ts index 8a2dcf06f6..2a0b1e1117 100644 --- a/packages/runtime/src/domains/i18n.ts +++ b/packages/runtime/src/domains/i18n.ts @@ -17,7 +17,7 @@ */ import { resolveLocale } from '@objectstack/core'; -import { CoreServiceName, resolveObjectFieldLabels } from '@objectstack/spec/system'; +import { CoreServiceName, resolveObjectFieldLabels, toLocaleDescriptors } from '@objectstack/spec/system'; import type { TranslationData } from '@objectstack/spec/system'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -48,7 +48,16 @@ export async function handleI18nRequest( // GET /i18n/locales if (parts[0] === 'locales' && parts.length === 1) { - const locales = i18nService.getLocales(); + // Descriptors, not the raw `string[]` `getLocales()` hands back — + // that is what `GetLocalesResponseSchema` declares and what + // service-i18n already emitted for this same route. Passing the bare + // array through made one endpoint answer in two shapes depending on + // which provider mounted it, the dispatcher's contradicting the SDK's + // own `GetLocalesResponse` type. + const locales = toLocaleDescriptors( + i18nService.getLocales(), + typeof i18nService.getDefaultLocale === 'function' ? i18nService.getDefaultLocale() : undefined, + ); return { handled: true, response: deps.success({ locales }) }; } diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 1055a58002..b997143878 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -1854,7 +1854,9 @@ describe('HttpDispatcher', () => { const result = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); - expect(result.response?.body?.data?.locales).toEqual(['en', 'zh-CN', 'ja']); + // Descriptors, not bare codes — the shape `GetLocalesResponseSchema` + // declares and both surfaces now emit (#3859 follow-up). + expect(result.response?.body?.data?.locales.map((l: any) => l.code)).toEqual(['en', 'zh-CN', 'ja']); expect(mockI18nService.getLocales).toHaveBeenCalled(); }); @@ -2015,7 +2017,7 @@ describe('HttpDispatcher', () => { it('should dispatch /i18n routes via dispatch()', async () => { const result = await dispatcher.dispatch('GET', '/i18n/locales', undefined, {}, { request: {} }); expect(result.handled).toBe(true); - expect(result.response?.body?.data?.locales).toEqual(['en', 'zh-CN', 'ja']); + expect(result.response?.body?.data?.locales.map((l: any) => l.code)).toEqual(['en', 'zh-CN', 'ja']); }); it('should resolve locale via fallback (zh → zh-CN) for translations', async () => { @@ -2107,7 +2109,7 @@ describe('HttpDispatcher', () => { const result = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); - expect(result.response?.body?.data?.locales).toEqual(['en', 'fr']); + expect(result.response?.body?.data?.locales.map((l: any) => l.code)).toEqual(['en', 'fr']); }); it('should populate locale from actual i18n service', async () => { @@ -2295,7 +2297,7 @@ describe('HttpDispatcher', () => { // MSW-style dispatch: full path stripped to relative const localesResult = await dispatcher.dispatch('GET', '/i18n/locales', undefined, {}, { request: {} }); expect(localesResult.handled).toBe(true); - expect(localesResult.response?.body?.data?.locales).toEqual(['en', 'de']); + expect(localesResult.response?.body?.data?.locales.map((l: any) => l.code)).toEqual(['en', 'de']); const translationsResult = await dispatcher.dispatch('GET', '/i18n/translations/de', undefined, {}, { request: {} }); expect(translationsResult.handled).toBe(true); diff --git a/packages/runtime/src/i18n-success-envelope.conformance.test.ts b/packages/runtime/src/i18n-success-envelope.conformance.test.ts new file mode 100644 index 0000000000..412a33046a --- /dev/null +++ b/packages/runtime/src/i18n-success-envelope.conformance.test.ts @@ -0,0 +1,161 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Success-envelope conformance for the dispatcher's `/i18n` domain — the + * missing twin of `service-i18n`'s `error-envelope.conformance.test.ts`, and + * the same pairing storage got in #3689. + * + * WHY THIS SUITE EXISTS + * + * Three consecutive defects landed on this one route family, and every one of + * them was a body that did not match the schema declaring it: + * + * #3676 the request declared `namespace`/`keys` filters no server read + * #3833 the field-labels derivation scanned a retired key dialect and so + * returned `{}` for every provider + * #3847 `labels` entries were emitted as bare strings under a schema + * declaring `{ label, help?, options? }` + * + * All three survived a green test suite, because every test asserted the + * emitted body against a HAND-WRITTEN LITERAL. Comparing output to a literal + * proves the code does what the test author believed; it cannot prove the code + * does what the contract declares. Nothing had ever put the emitted value and + * the declared schema in the same assertion. + * + * So these assertions parse each response with the schema `plugin-rest-api` + * names for that route (`responseSchema: 'GetLocalesResponseSchema'`, …), + * imported rather than restated. A body that parses is a body the SDK's + * published return type does not lie about. + * + * The first thing the suite caught, on the day it was written: `GET + * /i18n/locales` passed `getLocales()`'s raw `string[]` straight through, + * while `GetLocalesResponseSchema` declares `{ code, label, isDefault }[]` and + * service-i18n — the OTHER provider of this identical route — already emitted + * descriptors. One endpoint, two shapes, decided by which plugin mounted it. + * The dispatcher's copy of the field-labels derivation went wrong the same way + * in #3833, one route over, which is why both mappings are now shared. + * + * The route ledgers cannot catch this. They audit which routes exist and + * whether the SDK can address them, not what comes back. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { BaseResponseSchema } from '@objectstack/spec/api'; +import { + GetLocalesResponseSchema, + GetTranslationsResponseSchema, + GetFieldLabelsResponseSchema, +} from '@objectstack/spec/api'; + +/** The nested shape every producer writes (#3778). */ +const BUNDLE = { + objects: { + contact: { + label: '联系人', + fields: { + first_name: { label: '名' }, + email: { label: '邮箱', help: '主邮箱地址' }, + status: { label: '状态', options: { open: '进行中', closed: '已关闭' } }, + phone: { help: '优先手机' }, + }, + }, + }, + messages: { save: '保存' }, +}; + +describe('/i18n success-envelope conformance (dispatcher domain)', () => { + let dispatcher: HttpDispatcher; + let i18nService: Record; + + beforeEach(() => { + i18nService = { + getLocales: vi.fn().mockReturnValue(['en', 'zh-CN']), + getDefaultLocale: vi.fn().mockReturnValue('en'), + getTranslations: vi.fn().mockReturnValue(BUNDLE), + }; + const kernel = { + getService: vi.fn(async (name: string) => (name === 'i18n' ? i18nService : null)), + services: new Map(), + context: { getService: (name: string) => (name === 'i18n' ? i18nService : null) }, + }; + dispatcher = new HttpDispatcher(kernel as never); + }); + + /** Every success body must satisfy the shared envelope. */ + function expectEnvelope(body: unknown) { + const parsed = BaseResponseSchema.safeParse(body); + expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true); + expect((body as { success?: boolean }).success).toBe(true); + } + + it('GET /locales — body satisfies GetLocalesResponseSchema', async () => { + const result = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} } as never); + expect(result.response?.status).toBe(200); + expectEnvelope(result.response?.body); + + const parsed = GetLocalesResponseSchema.safeParse(result.response?.body?.data); + expect( + parsed.success, + `locales body does not match its declared schema: ${JSON.stringify(parsed.error?.issues)}`, + ).toBe(true); + expect(parsed.data?.locales).toEqual([ + { code: 'en', label: 'en', isDefault: true }, + { code: 'zh-CN', label: 'zh-CN', isDefault: false }, + ]); + }); + + it('GET /locales — a bare string[] is DEAD, so a revert cannot pass quietly', async () => { + const result = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} } as never); + const locales = result.response?.body?.data?.locales as unknown[]; + expect(locales.every((l) => typeof l === 'object' && l !== null)).toBe(true); + expect(locales).not.toContain('en'); + }); + + it('GET /translations/:locale — body satisfies GetTranslationsResponseSchema', async () => { + const result = await dispatcher.handleI18n('/translations/zh-CN', 'GET', {}, { request: {} } as never); + expect(result.response?.status).toBe(200); + expectEnvelope(result.response?.body); + + const parsed = GetTranslationsResponseSchema.safeParse(result.response?.body?.data); + expect( + parsed.success, + `translations body does not match its declared schema: ${JSON.stringify(parsed.error?.issues)}`, + ).toBe(true); + }); + + it('GET /labels/:object/:locale — body satisfies GetFieldLabelsResponseSchema', async () => { + const result = await dispatcher.handleI18n('/labels/contact/zh-CN', 'GET', {}, { request: {} } as never); + expect(result.response?.status).toBe(200); + expectEnvelope(result.response?.body); + + const parsed = GetFieldLabelsResponseSchema.safeParse(result.response?.body?.data); + expect( + parsed.success, + `labels body does not match its declared schema: ${JSON.stringify(parsed.error?.issues)}`, + ).toBe(true); + // The entries are objects carrying what the bundle holds — the gap #3847 + // closed, pinned here against the contract rather than a literal. + expect(parsed.data?.labels.email?.help).toBe('主邮箱地址'); + expect(parsed.data?.labels.status?.options?.open).toBe('进行中'); + }); + + it('an empty label map is still a conforming body', async () => { + const result = await dispatcher.handleI18n('/labels/unknown_object/zh-CN', 'GET', {}, { request: {} } as never); + expect(result.response?.status).toBe(200); + expect(GetFieldLabelsResponseSchema.safeParse(result.response?.body?.data).success).toBe(true); + }); + + /** + * A provider may omit the optional `getDefaultLocale`. The descriptor + * mapping must still produce a conforming body — `isDefault` is required by + * the schema, so an undefined default must yield `false`, not `undefined`. + */ + it('GET /locales conforms even when the provider omits getDefaultLocale', async () => { + delete i18nService.getDefaultLocale; + const result = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} } as never); + const parsed = GetLocalesResponseSchema.safeParse(result.response?.body?.data); + expect(parsed.success).toBe(true); + expect(parsed.data?.locales.every((l) => l.isDefault === false)).toBe(true); + }); +}); diff --git a/packages/services/service-i18n/src/i18n-service-plugin.ts b/packages/services/service-i18n/src/i18n-service-plugin.ts index 5f05da44a4..2d077b15b9 100644 --- a/packages/services/service-i18n/src/i18n-service-plugin.ts +++ b/packages/services/service-i18n/src/i18n-service-plugin.ts @@ -5,7 +5,7 @@ import { wireAuthoredTranslationSync } from '@objectstack/core'; import type { IHttpServer, IHttpRequest, IHttpResponse } from '@objectstack/spec/contracts'; import type { II18nService } from '@objectstack/spec/contracts'; import type { TranslationData } from '@objectstack/spec/system'; -import { resolveObjectFieldLabels } from '@objectstack/spec/system'; +import { resolveObjectFieldLabels, toLocaleDescriptors } from '@objectstack/spec/system'; import { FileI18nAdapter } from './file-i18n-adapter.js'; import type { FileI18nAdapterOptions } from './file-i18n-adapter.js'; @@ -182,18 +182,12 @@ export class I18nServicePlugin implements Plugin { // GET /i18n/locales httpServer.get(`${basePath}/locales`, async (_req: IHttpRequest, res: IHttpResponse) => { try { - const locales = i18n.getLocales(); - const defaultLocale = i18n.getDefaultLocale?.() ?? 'en'; - res.json({ - success: true, - data: { - locales: locales.map((code) => ({ - code, - label: code, - isDefault: code === defaultLocale, - })), - }, - }); + // Same shared mapping the dispatcher's `/i18n` domain uses — the two + // surfaces serve this route interchangeably, and each holding its own + // copy is what let them answer in different shapes (#3833's lesson, + // one route over). + const locales = toLocaleDescriptors(i18n.getLocales(), i18n.getDefaultLocale?.() ?? 'en'); + res.json({ success: true, data: { locales } }); } catch (error: any) { sendError(res, 500, 'INTERNAL', error?.message ?? 'Internal error'); } diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index cd5928a971..faa324aeb4 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -894,6 +894,7 @@ "LifecyclePolicyConfigSchema (const)", "LifecyclePolicyRule (type)", "LifecyclePolicyRuleSchema (const)", + "LocaleDescriptor (interface)", "LocaleSchema (const)", "LogDestination (type)", "LogDestinationSchema (const)", @@ -1323,6 +1324,7 @@ "resolveViewDescription (function)", "resolveViewLabel (function)", "s3StorageExample (const)", + "toLocaleDescriptors (function)", "translateAction (function)", "translateApp (function)", "translateDashboard (function)", diff --git a/packages/spec/src/system/i18n-resolver.test.ts b/packages/spec/src/system/i18n-resolver.test.ts index ac02889393..03441f63d4 100644 --- a/packages/spec/src/system/i18n-resolver.test.ts +++ b/packages/spec/src/system/i18n-resolver.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest'; import { ObjectTranslationDataSchema, TranslationDataSchema, type TranslationBundle } from './translation.zod'; -import { GetFieldLabelsResponseSchema } from '../api/protocol.zod'; +import { GetFieldLabelsResponseSchema, GetLocalesResponseSchema } from '../api/protocol.zod'; import { resolveViewLabel, resolveViewDescription, @@ -13,6 +13,7 @@ import { translateAction, translateMetadataDocument, resolveObjectFieldLabels, + toLocaleDescriptors, } from './i18n-resolver'; describe('ObjectTranslationDataSchema (_views/_actions extensions)', () => { @@ -1143,3 +1144,42 @@ describe('resolveObjectFieldLabels (objectstack#3833, #3847)', () => { expect(parsed.success).toBe(true); }); }); + +// ========================================== +// toLocaleDescriptors — the `/i18n/locales` body +// ========================================== + +describe('toLocaleDescriptors', () => { + it('maps codes to the descriptors GetLocalesResponseSchema declares', () => { + expect(toLocaleDescriptors(['en', 'zh-CN'], 'en')).toEqual([ + { code: 'en', label: 'en', isDefault: true }, + { code: 'zh-CN', label: 'zh-CN', isDefault: false }, + ]); + }); + + it('produces a body that satisfies GetLocalesResponseSchema', () => { + // Same guard as the field-labels one above: emitted value vs DECLARED + // contract, not vs a hand-written literal. `getLocales()` hands back a + // bare `string[]`, and passing it through unmapped is what made one + // endpoint answer in two shapes depending on the provider. + const parsed = GetLocalesResponseSchema.safeParse({ + locales: toLocaleDescriptors(['en', 'ja-JP'], 'ja-JP'), + }); + expect(parsed.success).toBe(true); + expect(parsed.data?.locales.find((l) => l.code === 'ja-JP')?.isDefault).toBe(true); + }); + + it('marks nothing default when the provider has no default locale', () => { + // `getDefaultLocale` is optional on `II18nService`. `isDefault` is + // REQUIRED by the schema, so an absent default must yield `false`, never + // `undefined` — which would fail the parse. + const out = toLocaleDescriptors(['en', 'fr'], undefined); + expect(out.every((l) => l.isDefault === false)).toBe(true); + expect(GetLocalesResponseSchema.safeParse({ locales: out }).success).toBe(true); + }); + + it('returns [] for an absent or empty locale list', () => { + expect(toLocaleDescriptors(undefined, 'en')).toEqual([]); + expect(toLocaleDescriptors([], 'en')).toEqual([]); + }); +}); diff --git a/packages/spec/src/system/i18n-resolver.ts b/packages/spec/src/system/i18n-resolver.ts index cfbd9c6c1a..985601e81f 100644 --- a/packages/spec/src/system/i18n-resolver.ts +++ b/packages/spec/src/system/i18n-resolver.ts @@ -724,6 +724,40 @@ function lookupObjectFieldAttr( return undefined; } +export interface LocaleDescriptor { + /** BCP-47 locale code. */ + code: string; + /** Display name. Falls back to the code — nothing in the tree carries one. */ + label: string; + /** Whether this is the stack's default locale. */ + isDefault: boolean; +} + +/** + * Turn `II18nService.getLocales()` — a bare `string[]` — into the descriptor + * list `GetLocalesResponseSchema` declares for `GET /i18n/locales`. + * + * Shared for the same reason `resolveObjectFieldLabels` is: both the + * dispatcher domain and service-i18n serve this route, and when each kept its + * own copy they diverged. service-i18n mapped to descriptors; the dispatcher + * passed the raw `string[]` straight through, so the SAME endpoint returned + * `['en','zh-CN']` or `[{code,label,isDefault}]` depending on which provider + * happened to mount it — and the dispatcher's form contradicted both the + * declared schema and the SDK's `GetLocalesResponse` type. Exactly the split + * #3833 found in the field-labels derivation, one route over. + * + * `label` is the code: no locale display-name source exists in the tree, and + * the schema requires the field. Inventing one here (an ICU display-name + * table) would be a product decision, not an implementation detail. + */ +export function toLocaleDescriptors( + codes: readonly string[] | undefined, + defaultLocale?: string, +): LocaleDescriptor[] { + if (!codes) return []; + return codes.map((code) => ({ code, label: code, isDefault: code === defaultLocale })); +} + /** * Enumerate an object's translated field labels out of ONE locale's * `TranslationData` — the `GET /i18n/labels/:object/:locale` body.