Skip to content

Commit 72fe3a5

Browse files
committed
feat: validate that the @key field is always included in all modules
Restricting the key field would mean references sometimes do not work
1 parent 5e064e9 commit 72fe3a5

File tree

2 files changed

+77
-0
lines changed

2 files changed

+77
-0
lines changed

spec/schema/ast-validation-modules/modules-validator.spec.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,53 @@ describe('modules validator', () => {
303303
});
304304
});
305305

306+
describe('on key fields', () => {
307+
it('accepts @modules(includeAllFields: true)', () => {
308+
assertValidatorAcceptsAndDoesNotWarn(
309+
`
310+
type Foo @rootEntity @modules(in: ["module1", "module2"], includeAllFields: true) {
311+
foo: String @key
312+
}
313+
`,
314+
{ withModuleDefinitions: true },
315+
);
316+
});
317+
318+
it('accepts @modules(all: true)', () => {
319+
assertValidatorAcceptsAndDoesNotWarn(
320+
`
321+
type Foo @rootEntity @modules(in: ["module1", "module2"]) {
322+
foo: String @key @modules(all: true)
323+
}
324+
`,
325+
{ withModuleDefinitions: true },
326+
);
327+
});
328+
329+
it('accepts explicitly listing exactly the modules from the type', () => {
330+
assertValidatorAcceptsAndDoesNotWarn(
331+
`
332+
type Foo @rootEntity @modules(in: ["module1", "module2 & module3"]) {
333+
foo: String @key @modules(in: ["module1", "module2 & module3"])
334+
}
335+
`,
336+
{ withModuleDefinitions: true },
337+
);
338+
});
339+
340+
it('rejects if the key field is restricted more than the type', () => {
341+
assertValidatorRejects(
342+
`
343+
type Foo @rootEntity @modules(in: ["module1"]) {
344+
foo: String @key @modules(in: ["module2"])
345+
}
346+
`,
347+
'Key fields must always be included in all modules of their declaring type. Set @modules(all: true). (Type "Foo" is included in module "module1", but the key field is not.)',
348+
{ withModuleDefinitions: true },
349+
);
350+
});
351+
});
352+
306353
describe('expressions', () => {
307354
it('accepts a single module', () => {
308355
assertValidatorAcceptsAndDoesNotWarn(

src/model/implementation/field.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,7 @@ export class Field implements ModelComponent {
403403
this.validateFlexSearch(context);
404404
this.validateAccessField(context);
405405
this.validateModulesDirective(context);
406+
this.validateModulesOfKeyField(context);
406407
this.validateModuleConsistency(context);
407408
}
408409

@@ -2000,6 +2001,35 @@ export class Field implements ModelComponent {
20002001
this.moduleSpecification.validate(context);
20012002
}
20022003

2004+
/**
2005+
* Checks the combination of @key and modules
2006+
*/
2007+
private validateModulesOfKeyField(context: ValidationContext) {
2008+
if (!this.isKeyField || !this.moduleSpecification || this.moduleSpecification.all) {
2009+
return;
2010+
}
2011+
2012+
// we could just disallow specifying modules altogether, but that might be unexpected
2013+
// we make sure to suggest the sensible thing - setting all: true - in the message though.
2014+
const missingClauses =
2015+
this.declaringType.effectiveModuleSpecification.orCombinedClauses.filter(
2016+
(clause) =>
2017+
!this.effectiveModuleSpecification.includedIn(clause.andCombinedModules),
2018+
);
2019+
if (missingClauses.length) {
2020+
const desc = describeModuleSpecification(
2021+
new EffectiveModuleSpecification({ orCombinedClauses: missingClauses }),
2022+
{ preposition: 'in' },
2023+
);
2024+
context.addMessage(
2025+
ValidationMessage.error(
2026+
`Key fields must always be included in all modules of their declaring type. Set @modules(all: true). (Type "${this.declaringType.name}" is included ${desc}, but the key field is not.)`,
2027+
this.input.typeNameAST || this.astNode,
2028+
),
2029+
);
2030+
}
2031+
}
2032+
20032033
/**
20042034
* Checks that this field is only included in modules that also include all prerequisites (type, collect path...)
20052035
*/

0 commit comments

Comments
 (0)