Skip to content

Commit 41d8082

Browse files
grolmusgrolmus
authored
feat(core): Auto-initialise customFields for entities that support them (#4965)
1 parent d537d58 commit 41d8082

3 files changed

Lines changed: 270 additions & 12 deletions

File tree

‎packages/core/src/bootstrap.spec.ts‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
import { getMetadataArgsStorage } from 'typeorm';
2+
import { afterEach, describe, expect, it } from 'vitest';
3+
4+
import { runPluginConfigurations } from './bootstrap';
5+
import { CustomFieldConfig } from './config/custom-field/custom-field-types';
6+
import { RuntimeVendureConfig } from './config/vendure-config';
7+
// Importing the core entities registers their `customFields` embedded columns in the
8+
// TypeORM metadata, which is how getEntityNamesWithCustomFields() detects the entities
9+
// that support custom fields. Imported for its side effect only.
10+
import './entity/entities';
11+
import { registerCustomEntityFields } from './entity/register-custom-entity-fields';
12+
import { VendurePlugin } from './plugin/vendure-plugin';
13+
14+
/**
15+
* Registers a `translations` relation (and a matching `customFields` embedded on the
16+
* translation target) directly in the TypeORM metadata, so we can exercise the different
17+
* shapes TypeORM allows for a relation target — a constructor closure, a bare string name,
18+
* or a closure returning a string — without declaring throwaway `@Entity` classes that would
19+
* pollute the global metadata for every other test in the process. Returns a cleanup fn.
20+
*/
21+
function registerTranslationRelation(baseName: string, type: unknown): () => void {
22+
const storage = getMetadataArgsStorage();
23+
const base = { name: baseName };
24+
const translationTarget = { name: `${baseName}Translation` };
25+
storage.relations.push({
26+
target: base,
27+
propertyName: 'translations',
28+
relationType: 'one-to-many',
29+
type,
30+
isLazy: false,
31+
options: {},
32+
} as any);
33+
storage.embeddeds.push({
34+
target: translationTarget,
35+
propertyName: 'customFields',
36+
prefix: undefined,
37+
type: () => Object,
38+
} as any);
39+
return () => {
40+
storage.relations.pop();
41+
storage.embeddeds.pop();
42+
};
43+
}
44+
45+
function makeConfig(partial: {
46+
plugins?: RuntimeVendureConfig['plugins'];
47+
customFields?: Record<string, CustomFieldConfig[]>;
48+
}): RuntimeVendureConfig {
49+
return { plugins: [], customFields: {}, ...partial } as unknown as RuntimeVendureConfig;
50+
}
51+
52+
describe('runPluginConfigurations()', () => {
53+
// OSS-408: entities that support custom fields get an empty array pre-initialised so a
54+
// plugin's `configuration` callback can extend them without a defensive guard.
55+
it('auto-initialises customFields for entities that support them', async () => {
56+
const config = makeConfig({});
57+
await runPluginConfigurations(config);
58+
expect(config.customFields.Product).toEqual([]);
59+
expect(config.customFields.Customer).toEqual([]);
60+
});
61+
62+
// OSS-408: translation entities also declare a `customFields` embedded (for localized
63+
// values), but must NOT be auto-initialised — a `config.customFields.<Entity>Translation`
64+
// entry makes the GraphQL schema builder emit a duplicate `customFields` field on the
65+
// `*TranslationInput` types ("... can only be defined once").
66+
it('does not auto-initialise translation entities', async () => {
67+
const config = makeConfig({});
68+
await runPluginConfigurations(config);
69+
expect(config.customFields.ProductTranslation).toBeUndefined();
70+
expect(config.customFields.CollectionTranslation).toBeUndefined();
71+
});
72+
73+
it('does not overwrite an existing customFields entry', async () => {
74+
const existing: CustomFieldConfig[] = [{ name: 'foo', type: 'string' }];
75+
const config = makeConfig({ customFields: { Product: existing } });
76+
await runPluginConfigurations(config);
77+
expect(config.customFields.Product).toBe(existing);
78+
});
79+
80+
// OSS-408: a `translations` relation target need not be a constructor closure. TypeORM also
81+
// accepts a bare string name and a closure returning one (both used to break circular imports).
82+
// Building the exclusion set must handle all three, or one such relation anywhere in the
83+
// process throws `relation.type is not a function` and kills every bootstrap (Michael's review).
84+
describe('translation relation target shapes', () => {
85+
let cleanup: (() => void) | undefined;
86+
afterEach(() => {
87+
cleanup?.();
88+
cleanup = undefined;
89+
});
90+
91+
it('does not throw on a string relation target', async () => {
92+
cleanup = registerTranslationRelation('Oss408StringTarget', 'Oss408StringTargetTranslation');
93+
const config = makeConfig({});
94+
await expect(runPluginConfigurations(config)).resolves.toBeDefined();
95+
// and the translation entity is still excluded from auto-init
96+
expect(config.customFields.Oss408StringTargetTranslation).toBeUndefined();
97+
});
98+
99+
it('does not throw on a closure returning a string target, and still excludes it', async () => {
100+
cleanup = registerTranslationRelation(
101+
'Oss408ClosureString',
102+
() => 'Oss408ClosureStringTranslation',
103+
);
104+
const config = makeConfig({});
105+
await expect(runPluginConfigurations(config)).resolves.toBeDefined();
106+
expect(config.customFields.Oss408ClosureStringTranslation).toBeUndefined();
107+
});
108+
109+
it('still excludes a constructor-closure translation target', async () => {
110+
cleanup = registerTranslationRelation('Oss408Closure', () => ({
111+
name: 'Oss408ClosureTranslation',
112+
}));
113+
const config = makeConfig({});
114+
await runPluginConfigurations(config);
115+
expect(config.customFields.Oss408ClosureTranslation).toBeUndefined();
116+
});
117+
});
118+
119+
it('lets a plugin extend a supported entity without a guard', async () => {
120+
@VendurePlugin({
121+
configuration: cfg => {
122+
// No `if (!cfg.customFields.Product) cfg.customFields.Product = []` guard needed.
123+
cfg.customFields.Product.push({ name: 'fromPlugin', type: 'string' });
124+
return cfg;
125+
},
126+
})
127+
class TestPlugin {}
128+
129+
const config = makeConfig({ plugins: [TestPlugin] });
130+
await runPluginConfigurations(config);
131+
expect(config.customFields.Product).toContainEqual({ name: 'fromPlugin', type: 'string' });
132+
});
133+
});
134+
135+
describe('registerCustomEntityFields()', () => {
136+
// OSS-408 / Michael's review: the translatable branch resolved the translation entity via
137+
// `(translationsMetadata.type as Function)()`, which threw `type is not a function` for a
138+
// bare-string relation target — the same crash class fixed in getEntityNamesWithCustomFields().
139+
// It now reuses getRelationTargetName(), so a translatable entity with a string translations
140+
// target and real custom fields registers without aborting bootstrap.
141+
it('does not throw when a translatable entity has a bare-string translations relation target', () => {
142+
const storage = getMetadataArgsStorage();
143+
class Oss408RegBase {}
144+
class Oss408RegBaseTranslation {}
145+
// Base entity declares a customFields embedded…
146+
storage.embeddeds.push({
147+
target: Oss408RegBase,
148+
propertyName: 'customFields',
149+
prefix: undefined,
150+
type: () => Oss408RegBase,
151+
} as any);
152+
// …a `translations` relation whose target is a BARE STRING (the crash case)…
153+
storage.relations.push({
154+
target: Oss408RegBase,
155+
propertyName: 'translations',
156+
relationType: 'one-to-many',
157+
type: 'Oss408RegBaseTranslation',
158+
isLazy: false,
159+
options: {},
160+
} as any);
161+
// …and the translation entity also declares a customFields embedded.
162+
storage.embeddeds.push({
163+
target: Oss408RegBaseTranslation,
164+
propertyName: 'customFields',
165+
prefix: undefined,
166+
type: () => Oss408RegBaseTranslation,
167+
} as any);
168+
169+
const config = {
170+
customFields: { Oss408RegBase: [{ name: 'foo', type: 'string' }] },
171+
dbConnectionOptions: { type: 'sqljs' },
172+
} as unknown as RuntimeVendureConfig;
173+
174+
try {
175+
expect(() => registerCustomEntityFields(config)).not.toThrow();
176+
} finally {
177+
storage.embeddeds.pop();
178+
storage.relations.pop();
179+
storage.embeddeds.pop();
180+
}
181+
});
182+
});

‎packages/core/src/bootstrap.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import { Logger } from './config/logger/vendure-logger';
1717
import { RuntimeVendureConfig, VendureConfig } from './config/vendure-config';
1818
import { Administrator } from './entity/administrator/administrator.entity';
1919
import { coreEntitiesMap } from './entity/entities';
20-
import { registerCustomEntityFields } from './entity/register-custom-entity-fields';
20+
import {
21+
getEntityNamesWithCustomFields,
22+
registerCustomEntityFields,
23+
} from './entity/register-custom-entity-fields';
2124
import { runEntityMetadataModifiers } from './entity/run-entity-metadata-modifiers';
2225
import { setEntityIdStrategy } from './entity/set-entity-id-strategy';
2326
import { setMoneyStrategy } from './entity/set-money-strategy';
@@ -375,6 +378,17 @@ function checkPluginCompatibility(
375378
* Run the configuration functions of all plugins and return the final config object.
376379
*/
377380
export async function runPluginConfigurations(config: RuntimeVendureConfig): Promise<RuntimeVendureConfig> {
381+
// Auto-initialise an empty custom-field array for every entity that supports custom
382+
// fields (core or plugin-defined), so a plugin's `configuration` callback can do
383+
// `config.customFields.SomeEntity.push(...)` without the defensive
384+
// `if (!config.customFields.SomeEntity) config.customFields.SomeEntity = []` guard.
385+
// Empty arrays are ignored by `registerCustomEntityFields`, so this is inert for
386+
// entities nobody extends. See OSS-408.
387+
for (const entityName of getEntityNamesWithCustomFields()) {
388+
if (!Object.prototype.hasOwnProperty.call(config.customFields, entityName)) {
389+
config.customFields[entityName] = [];
390+
}
391+
}
378392
for (const plugin of config.plugins) {
379393
const configFn = getConfigurationFunction(plugin);
380394
if (typeof configFn === 'function') {

‎packages/core/src/entity/register-custom-entity-fields.ts‎

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ManyToOne,
1515
} from 'typeorm';
1616
import { EmbeddedMetadataArgs } from 'typeorm/metadata-args/EmbeddedMetadataArgs';
17+
import { RelationMetadataArgs } from 'typeorm/metadata-args/RelationMetadataArgs';
1718
import { DateUtils } from 'typeorm/util/DateUtils';
1819

1920
import { CustomFieldConfig, CustomFields } from '../config/custom-field/custom-field-types';
@@ -27,6 +28,61 @@ import { EntityId } from './entity-id.decorator';
2728
*/
2829
const MAX_STRING_LENGTH = 65535;
2930

31+
/**
32+
* @description
33+
* Returns the names of all registered entities that support custom fields (i.e.
34+
* implement `HasCustomFields`). An entity supports custom fields when it declares
35+
* a `customFields` embedded property, so we detect them from the TypeORM metadata
36+
* rather than a runtime-unavailable `implements` check. Used to auto-initialise
37+
* `config.customFields[EntityName]` so plugins can extend any such entity without
38+
* a defensive guard (OSS-408).
39+
*
40+
* Translation entities are excluded: they carry their own `customFields` embedded
41+
* (to hold localized field values) but are never valid `config.customFields` keys —
42+
* localized custom fields are declared on the *base* entity. Auto-initialising an
43+
* entry for a translation entity would make the GraphQL schema builder emit a
44+
* duplicate `customFields` field on the `*TranslationInput` types (colliding with
45+
* the one derived from the base entity's localized fields — "Field
46+
* `CreateXTranslationInput.customFields` can only be defined once"). We detect
47+
* translation entities as the target of a `translations` relation, the same signal
48+
* `registerCustomEntityFields` uses to locate the translation type.
49+
*/
50+
export function getEntityNamesWithCustomFields(): string[] {
51+
const metadataArgsStorage = getMetadataArgsStorage();
52+
const translationEntityNames = new Set(
53+
metadataArgsStorage.relations
54+
.filter(relation => relation.propertyName === 'translations')
55+
.map(relation => getRelationTargetName(relation.type))
56+
.filter((name): name is string => name != null),
57+
);
58+
const names = metadataArgsStorage.embeddeds
59+
.filter(embedded => embedded.propertyName === 'customFields')
60+
.map(embedded => (typeof embedded.target === 'string' ? embedded.target : embedded.target.name))
61+
.filter(name => !translationEntityNames.has(name));
62+
return Array.from(new Set(names));
63+
}
64+
65+
/**
66+
* Resolves a TypeORM relation target to the target entity's name. The target may be a
67+
* constructor closure (`() => ProductTranslation`, the usual form), a bare string name, or a
68+
* closure returning a string — the latter two are both legal and commonly used to break
69+
* circular imports (`@OneToMany('ArticleTranslation', ...)`). Calling a non-function, as the
70+
* previous code did unconditionally, threw `relation.type is not a function` and aborted
71+
* bootstrap; a closure returning a string yielded `undefined` and silently failed to exclude
72+
* the translation entity.
73+
*/
74+
function getRelationTargetName(type: RelationMetadataArgs['type']): string | undefined {
75+
const resolved: unknown = typeof type === 'function' ? (type as () => unknown)() : type;
76+
if (typeof resolved === 'string') {
77+
return resolved;
78+
}
79+
if (typeof resolved === 'function') {
80+
return resolved.name;
81+
}
82+
// Anything else that carries a `name` (e.g. an EntitySchema-like object).
83+
return (resolved as { name?: string } | undefined)?.name;
84+
}
85+
3086
/**
3187
* Dynamically add columns to the custom field entity based on the CustomFields config.
3288
*/
@@ -274,17 +330,23 @@ export function registerCustomEntityFields(config: VendureConfig) {
274330
if (translationsMetadata) {
275331
// This entity is translatable, which means that we should
276332
// also register any localized custom fields on the related
277-
// EntityTranslation entity.
278-
const translationType: Function = (translationsMetadata.type as Function)();
279-
const customFieldsTranslationsMetadata = getCustomFieldsMetadata(translationType);
280-
const customFieldsTranslationClass = customFieldsTranslationsMetadata.type();
281-
if (customFieldsTranslationClass && typeof customFieldsTranslationClass !== 'string') {
282-
registerCustomFieldsForEntity(
283-
config,
284-
entityName,
285-
customFieldsTranslationClass as any,
286-
true,
287-
);
333+
// EntityTranslation entity. Resolve the target via the shared
334+
// helper so a bare-string or closure-returning-string relation
335+
// target (both legal, used to break circular imports) does not
336+
// throw `type is not a function` here — the same crash fixed in
337+
// getEntityNamesWithCustomFields().
338+
const translationEntityName = getRelationTargetName(translationsMetadata.type);
339+
if (translationEntityName != null) {
340+
const customFieldsTranslationsMetadata = getCustomFieldsMetadata(translationEntityName);
341+
const customFieldsTranslationClass = customFieldsTranslationsMetadata.type();
342+
if (customFieldsTranslationClass && typeof customFieldsTranslationClass !== 'string') {
343+
registerCustomFieldsForEntity(
344+
config,
345+
entityName,
346+
customFieldsTranslationClass as any,
347+
true,
348+
);
349+
}
288350
}
289351
} else {
290352
assertLocaleFieldsNotSpecified(config, entityName);

0 commit comments

Comments
 (0)