Skip to content

Commit 720c5ad

Browse files
authored
fix(runtime,i18n): field labels read the bundle shape producers write — one shared derivation (#3833) (#3846)
`GET /i18n/labels/:object/:locale` served through the dispatcher returned `{ labels: {} }` for every provider. Its derivation scanned for flat `o.<object>.fields.<field>` keys — the dialect #3778 retired. No producer has ever written it, and a real bundle's top-level keys are the `TranslationData` groups (`objects`, `apps`, `messages`, ...), so the prefix matched nothing. 4cca74c fixed the identical derivation in service-i18n and did not reach the dispatcher's copy. Not a rare fallback: `getFieldLabels` is optional on `II18nService` and nothing implements it — not `memory-i18n`, not `file-i18n-adapter` — so the dedicated- method branch both surfaces check first is dead in production and this derivation is the only path there is. Any stack served by the dispatcher got an empty map, indistinguishable from "this object has no translated labels". Nothing errored, nothing warned. Worse than the class it was found beside: #3676 ignored a declared filter and returned the full bundle, a correct superset. This returned nothing and said it was fine. The derivation now lives once, as `resolveObjectFieldLabels` in `packages/spec/src/system/i18n-resolver.ts`, beside the other resolvers that read `TranslationData`. Both surfaces call it — keeping a copy each is precisely how one got fixed and the other did not. Fields with no non-empty `label` stay omitted rather than emitted blank: partial translation is the normal state, and callers merge this map over their source labels, where a '' would erase them. The tests were fiction on both sides. The dispatcher's fallback test fed flat `o.contact.fields.first_name` keys and asserted labels came back, so it passed on data that cannot occur while production returned `{}` — the same failure mode as the client test retired in #3676. It now feeds the nested shape, and was confirmed to FAIL against the pre-fix code rather than merely passing after it. The same suite's mock declared a `getFieldLabels` no shipped provider has and returned flat-dialect data; both now reflect what a real provider does. The shared helper carries unit tests, including one pinning the retired flat dialect to `{}`. Filed separately, not addressed here: `GetFieldLabelsResponseSchema` declares `labels` as `Record<string, { label, help?, options? }>` while both surfaces emit `Record<string, string>` — a third declared != enforced gap on this endpoint (#3847), and a wire-shape change too breaking to fold into a correctness fix.
1 parent c757854 commit 720c5ad

7 files changed

Lines changed: 224 additions & 23 deletions

File tree

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/runtime": patch
4+
"@objectstack/service-i18n": patch
5+
---
6+
7+
fix(runtime,i18n): the dispatcher's field-labels route reads the bundle shape
8+
producers actually write — one shared derivation (#3833)
9+
10+
`GET /i18n/labels/:object/:locale` served through the dispatcher returned
11+
`{ labels: {} }` for every provider. Its derivation scanned for flat
12+
`o.<object>.fields.<field>` keys:
13+
14+
```ts
15+
const prefix = `o.${objectName}.fields.`;
16+
for (const [key, value] of Object.entries(translations)) { … }
17+
```
18+
19+
That dialect was retired by #3778 — no producer has ever written it, and a real
20+
bundle's top-level keys are the `TranslationData` groups (`objects`, `apps`,
21+
`messages`, …), so the prefix could not match anything. 4cca74c fixed the
22+
identical derivation in `service-i18n` and did not reach the dispatcher's copy.
23+
24+
This is not a rare fallback. `getFieldLabels` is optional on `II18nService` and
25+
**nothing implements it** — not `memory-i18n`, not `file-i18n-adapter` — so the
26+
dedicated-method branch both surfaces check first is dead in production and this
27+
derivation is the only path there is. Any stack served by the dispatcher (the
28+
AppPlugin in-memory provider auto-registered for stacks declaring translation
29+
bundles) got an empty map, indistinguishable from "this object has no translated
30+
labels": nothing errored, nothing warned.
31+
32+
Worse than the class it was found next to. #3676, which prompted the check,
33+
ignored a declared filter and returned the full bundle — a correct superset. This
34+
returned nothing and said it was fine.
35+
36+
The derivation now lives once, as `resolveObjectFieldLabels` in
37+
`packages/spec/src/system/i18n-resolver.ts`, alongside the other resolvers that
38+
read `TranslationData`. Both surfaces call it. Keeping a copy each is precisely
39+
how one got fixed and the other did not; the next bundle-shape change now has one
40+
place to land. Fields carrying no non-empty `label` stay omitted rather than
41+
emitted blank — partial translation is the normal state, and callers merge this
42+
map over their source labels, where a `''` would erase them.
43+
44+
### The tests were fiction on both sides
45+
46+
The dispatcher's fallback test fed flat `o.contact.fields.first_name` keys and
47+
asserted labels came back, so it passed on data that cannot occur while
48+
production returned `{}` — the same failure mode as the client test retired in
49+
#3676, which asserted a query string was built that no server read. It now feeds
50+
the nested shape, and was confirmed to fail against the pre-fix code (`expected
51+
{} to deeply equal { first_name: 'First Name', … }`) rather than merely passing
52+
after it. The shared helper carries its own unit tests, including one pinning
53+
that the retired flat dialect resolves to `{}`.
54+
55+
The same suite's mock also declared a `getFieldLabels` no shipped provider has,
56+
and returned flat-dialect data from `getTranslations`; both now reflect what a
57+
real provider does, with the divergence noted where it remains deliberate.
58+
59+
Not addressed here, filed separately: `GetFieldLabelsResponseSchema` declares
60+
`labels` as `Record<string, { label, help?, options? }>`, but both surfaces emit
61+
`Record<string, string>` — a third declared ≠ enforced gap in the same endpoint,
62+
and a wire-shape change too breaking to fold into a correctness fix.

packages/runtime/src/domains/i18n.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
*/
1818

1919
import { resolveLocale } from '@objectstack/core';
20-
import { CoreServiceName } from '@objectstack/spec/system';
20+
import { CoreServiceName, resolveObjectFieldLabels } from '@objectstack/spec/system';
21+
import type { TranslationData } from '@objectstack/spec/system';
2122
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
2223
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
2324

@@ -89,15 +90,16 @@ export async function handleI18nRequest(
8990
const labels = i18nService.getFieldLabels(objectName, locale);
9091
return { handled: true, response: deps.success({ object: objectName, locale, labels }) };
9192
}
92-
// Fallback: derive field labels from full translation bundle
93-
const translations = i18nService.getTranslations(locale);
94-
const prefix = `o.${objectName}.fields.`;
95-
const labels: Record<string, string> = {};
96-
for (const [key, value] of Object.entries(translations)) {
97-
if (key.startsWith(prefix)) {
98-
labels[key.substring(prefix.length)] = value as string;
99-
}
100-
}
93+
// Fallback: derive field labels from the locale's translation bundle.
94+
// This is not really a fallback — `getFieldLabels` is optional on
95+
// `II18nService` and nothing implements it, so this is the path every
96+
// provider takes. Shared with service-i18n's identical derivation so
97+
// the next bundle-shape change cannot fix one copy and miss the other,
98+
// which is how this one went on scanning the retired flat
99+
// `o.<object>.fields.<field>` dialect after #3778 and returned `{}`
100+
// for every provider (#3833).
101+
const translations = i18nService.getTranslations(locale) as TranslationData | undefined;
102+
const labels = resolveObjectFieldLabels(translations, objectName);
101103
return { handled: true, response: deps.success({ object: objectName, locale, labels }) };
102104
}
103105

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,7 +1801,16 @@ describe('HttpDispatcher', () => {
18011801
beforeEach(() => {
18021802
mockI18nService = {
18031803
getLocales: vi.fn().mockReturnValue(['en', 'zh-CN', 'ja']),
1804-
getTranslations: vi.fn().mockReturnValue({ 'o.account.label': '客户', 'o.account.fields.name': '名称' }),
1804+
// The nested shape every producer writes and #3778 converged
1805+
// on. This used to be flat `o.account.label` keys — a dialect
1806+
// no bundle has ever carried.
1807+
getTranslations: vi.fn().mockReturnValue({
1808+
objects: { account: { label: '客户', fields: { name: { label: '名称' } } } },
1809+
}),
1810+
// Declared optional on `II18nService` and implemented by NO
1811+
// shipped provider — the tests below that assert it is called
1812+
// cover the dispatcher's handling of a provider that supplies
1813+
// it, not a path any current stack takes. See #3833.
18051814
getFieldLabels: vi.fn().mockReturnValue({ name: '名称', industry: '行业' }),
18061815
};
18071816

@@ -1824,7 +1833,9 @@ describe('HttpDispatcher', () => {
18241833
expect(result.handled).toBe(true);
18251834
expect(result.response?.status).toBe(200);
18261835
expect(result.response?.body?.data?.locale).toBe('zh-CN');
1827-
expect(result.response?.body?.data?.translations).toEqual({ 'o.account.label': '客户', 'o.account.fields.name': '名称' });
1836+
expect(result.response?.body?.data?.translations).toEqual({
1837+
objects: { account: { label: '客户', fields: { name: { label: '名称' } } } },
1838+
});
18281839
expect(mockI18nService.getTranslations).toHaveBeenCalledWith('zh-CN');
18291840
});
18301841

@@ -1897,12 +1908,38 @@ describe('HttpDispatcher', () => {
18971908
expect((body.error as { code: unknown }).code).toBe(400);
18981909
});
18991910

1900-
it('should fallback to deriving labels from translations when getFieldLabels is missing', async () => {
1911+
/**
1912+
* This is the path EVERY provider takes, not an edge case:
1913+
* `getFieldLabels` is optional on `II18nService` and nothing implements
1914+
* it — not `memory-i18n`, not `file-i18n-adapter` — so the dedicated-
1915+
* method branch above is dead in production and this derivation always
1916+
* runs.
1917+
*
1918+
* Its predecessor fed flat `o.contact.fields.first_name` keys and
1919+
* asserted labels came back. That dialect was retired by #3778 (no
1920+
* producer ever wrote it), so the test passed on data that cannot
1921+
* occur while the real path — scanning for an `o.` prefix in a bundle
1922+
* whose top-level keys are `objects`/`apps`/`messages` — returned `{}`
1923+
* for every caller. Feeding the shape real bundles actually have is
1924+
* the whole point of the test (#3833).
1925+
*/
1926+
it('derives labels from the NESTED bundle shape every producer writes (#3833)', async () => {
19011927
delete mockI18nService.getFieldLabels;
19021928
mockI18nService.getTranslations.mockReturnValue({
1903-
'o.contact.fields.first_name': 'First Name',
1904-
'o.contact.fields.email': 'Email',
1905-
'o.contact.label': 'Contact',
1929+
objects: {
1930+
contact: {
1931+
label: 'Contact',
1932+
fields: {
1933+
first_name: { label: 'First Name' },
1934+
email: { label: 'Email', help: 'Primary address' },
1935+
// No label — partial translation is the normal
1936+
// state, and a blank entry would overwrite the
1937+
// caller's source label with an empty string.
1938+
phone: { help: 'Mobile preferred' },
1939+
},
1940+
},
1941+
},
1942+
messages: { save: 'Save' },
19061943
});
19071944

19081945
const result = await dispatcher.handleI18n('/labels/contact/en', 'GET', {}, { request: {} });
@@ -1914,6 +1951,18 @@ describe('HttpDispatcher', () => {
19141951
});
19151952
});
19161953

1954+
it('returns {} for an object the locale does not translate, without throwing', async () => {
1955+
delete mockI18nService.getFieldLabels;
1956+
mockI18nService.getTranslations.mockReturnValue({
1957+
objects: { contact: { fields: { email: { label: 'Email' } } } },
1958+
});
1959+
1960+
const result = await dispatcher.handleI18n('/labels/account/en', 'GET', {}, { request: {} });
1961+
expect(result.handled).toBe(true);
1962+
expect(result.response?.status).toBe(200);
1963+
expect(result.response?.body?.data?.labels).toEqual({});
1964+
});
1965+
19171966
it('should return 501 when i18n service is not available', async () => {
19181967
(kernel as any).getService = vi.fn().mockResolvedValue(null);
19191968
(kernel as any).services = new Map();

packages/services/service-i18n/src/i18n-service-plugin.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { wireAuthoredTranslationSync } from '@objectstack/core';
55
import type { IHttpServer, IHttpRequest, IHttpResponse } from '@objectstack/spec/contracts';
66
import type { II18nService } from '@objectstack/spec/contracts';
77
import type { TranslationData } from '@objectstack/spec/system';
8+
import { resolveObjectFieldLabels } from '@objectstack/spec/system';
89
import { FileI18nAdapter } from './file-i18n-adapter.js';
910
import type { FileI18nAdapterOptions } from './file-i18n-adapter.js';
1011

@@ -234,14 +235,11 @@ export class I18nServicePlugin implements Plugin {
234235
// That data is NESTED (`objects.<obj>.fields.<field>.label`) — the
235236
// flat dotted `o.<obj>.fields.<field>` keys this used to scan were a
236237
// third translation dialect that no producer ever wrote, so the
237-
// fallback always returned `{}` (#3778).
238+
// fallback always returned `{}` (#3778). The derivation now lives in
239+
// `resolveObjectFieldLabels` so the dispatcher's copy of it cannot
240+
// drift out of shape again the way it did after that fix (#3833).
238241
const data = i18n.getTranslations(locale) as TranslationData | undefined;
239-
const fields = data?.objects?.[objectName]?.fields ?? {};
240-
const labels: Record<string, string> = {};
241-
for (const [fieldName, field] of Object.entries(fields)) {
242-
const label = field?.label;
243-
if (typeof label === 'string' && label.length > 0) labels[fieldName] = label;
244-
}
242+
const labels = resolveObjectFieldLabels(data, objectName);
245243
res.json({ success: true, data: { object: objectName, locale, labels } });
246244
}
247245
} catch (error: any) {

packages/spec/api-surface.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,7 @@
13041304
"resolveMetadataFormLabels (function)",
13051305
"resolveMetadataTypeDescription (function)",
13061306
"resolveMetadataTypeLabel (function)",
1307+
"resolveObjectFieldLabels (function)",
13071308
"resolveSettingsActionConfirm (function)",
13081309
"resolveSettingsActionLabel (function)",
13091310
"resolveSettingsActionSuccess (function)",

packages/spec/src/system/i18n-resolver.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
resolveActionResultDialog,
1212
translateAction,
1313
translateMetadataDocument,
14+
resolveObjectFieldLabels,
1415
} from './i18n-resolver';
1516

1617
describe('ObjectTranslationDataSchema (_views/_actions extensions)', () => {
@@ -1038,3 +1039,53 @@ describe('translateObject inline actions (objectstack#3370)', () => {
10381039
expect(Object.hasOwn(out, 'actions')).toBe(false);
10391040
});
10401041
});
1042+
1043+
// ==========================================
1044+
// resolveObjectFieldLabels — the `/i18n/labels/:object/:locale` body
1045+
// ==========================================
1046+
1047+
describe('resolveObjectFieldLabels (objectstack#3833)', () => {
1048+
const data = TranslationDataSchema.parse({
1049+
objects: {
1050+
contact: {
1051+
label: 'Contact',
1052+
fields: {
1053+
first_name: { label: 'First Name' },
1054+
email: { label: 'Email', help: 'Primary address' },
1055+
phone: { help: 'Mobile preferred' },
1056+
},
1057+
},
1058+
},
1059+
messages: { save: 'Save' },
1060+
});
1061+
1062+
it('enumerates the labels a locale actually translates', () => {
1063+
expect(resolveObjectFieldLabels(data, 'contact')).toEqual({
1064+
first_name: 'First Name',
1065+
email: 'Email',
1066+
});
1067+
});
1068+
1069+
it('omits fields carrying no label rather than emitting a blank one', () => {
1070+
// Partial translation is the normal state (see ObjectTranslationDataSchema),
1071+
// and callers merge this over their source labels — a '' would erase them.
1072+
expect(resolveObjectFieldLabels(data, 'contact')).not.toHaveProperty('phone');
1073+
});
1074+
1075+
it('returns {} for an untranslated object, a bundle with no objects, and no bundle', () => {
1076+
expect(resolveObjectFieldLabels(data, 'account')).toEqual({});
1077+
expect(resolveObjectFieldLabels({ messages: { save: 'Save' } }, 'contact')).toEqual({});
1078+
expect(resolveObjectFieldLabels(undefined, 'contact')).toEqual({});
1079+
});
1080+
1081+
it('never matches the retired flat `o.<object>.fields.<field>` dialect', () => {
1082+
// The shape the dispatcher scanned for until #3833. It is not a bundle
1083+
// any producer writes, and reading it as one is what returned {} in
1084+
// production while a test built on the same fiction stayed green.
1085+
const flat = {
1086+
'o.contact.fields.first_name': 'First Name',
1087+
'o.contact.label': 'Contact',
1088+
} as unknown as Parameters<typeof resolveObjectFieldLabels>[0];
1089+
expect(resolveObjectFieldLabels(flat, 'contact')).toEqual({});
1090+
});
1091+
});

packages/spec/src/system/i18n-resolver.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,44 @@ function lookupObjectFieldAttr(
724724
return undefined;
725725
}
726726

727+
/**
728+
* Enumerate an object's translated field labels out of ONE locale's
729+
* `TranslationData` — the `GET /i18n/labels/:object/:locale` body.
730+
*
731+
* Distinct from `lookupObjectFieldAttr` above, which answers "what is this
732+
* one field called" against a whole bundle and a locale chain. This answers
733+
* "which fields does this locale translate at all", which is what the
734+
* endpoint returns and what no per-field lookup can produce.
735+
*
736+
* Shared deliberately. Both serving surfaces derive this map — the dispatcher
737+
* domain body and service-i18n's autonomous route — and they are the only
738+
* path there is: `getFieldLabels` is optional on `II18nService` and NOTHING
739+
* implements it (neither `memory-i18n` nor `file-i18n-adapter`), so the
740+
* "dedicated method" branch both surfaces check first is dead and this
741+
* derivation always runs. Keeping one copy each is how the dispatcher was
742+
* left scanning the retired flat `o.<object>.fields.<field>` dialect after
743+
* #3778 converged the tree on nested `objects.<object>.fields.<field>.label`
744+
* and fixed only service-i18n's copy — a scan that cannot match a real bundle,
745+
* so that route returned `{}` for every provider (#3833).
746+
*
747+
* Fields with no non-empty `label` are omitted rather than emitted blank:
748+
* partial translation is the normal state (see `ObjectTranslationDataSchema`),
749+
* and a caller merges what comes back over its source labels.
750+
*/
751+
export function resolveObjectFieldLabels(
752+
data: TranslationData | undefined,
753+
objectName: string,
754+
): Record<string, string> {
755+
const fields = data?.objects?.[objectName]?.fields;
756+
const labels: Record<string, string> = {};
757+
if (!fields) return labels;
758+
for (const [fieldName, field] of Object.entries(fields)) {
759+
const label = field?.label;
760+
if (typeof label === 'string' && label.length > 0) labels[fieldName] = label;
761+
}
762+
return labels;
763+
}
764+
727765
function lookupObjectFieldOption(
728766
bundle: TranslationBundle | undefined,
729767
objectName: string,

0 commit comments

Comments
 (0)